Re: [PATCH net-next v5 3/6] net: bcmasp: Add support for ASP2.0 Ethernet controller
On 5/24/23 16:01, Justin Chen wrote:
Add support for the Broadcom ASP 2.0 Ethernet controller which is first
introduced with 72165. This controller features two distinct Ethernet
ports that can be independently operated.
This patch supports:
- Wake-on-LAN using magic packets
- basic ethtool operations (link, counters, message level)
- MAC destination address filtering (promiscuous, ALL_MULTI, etc.)
Reviewed-by: Simon Horman
Signed-off-by: Florian Fainelli
Signed-off-by: Justin Chen
---
[snip]
+static const struct net_device_ops bcmasp_netdev_ops = {
+ .ndo_open = bcmasp_open,
+ .ndo_stop = bcmasp_stop,
+ .ndo_start_xmit = bcmasp_xmit,
+ .ndo_tx_timeout = bcmasp_tx_timeout,
+ .ndo_set_rx_mode= bcmasp_set_rx_mode,
+ .ndo_get_phys_port_name = bcmasp_get_phys_port_name,
+ .ndo_get_stats = bcmasp_get_stats,
+ .ndo_do_ioctl = bcmasp_ioctl,
This needs to be:
@@ -1207,7 +1196,7 @@ static const struct net_device_ops
bcmasp_netdev_ops = {
.ndo_set_rx_mode= bcmasp_set_rx_mode,
.ndo_get_phys_port_name = bcmasp_get_phys_port_name,
.ndo_get_stats = bcmasp_get_stats,
- .ndo_do_ioctl = bcmasp_ioctl,
+ .ndo_eth_ioctl = phy_do_ioctl_running,
.ndo_set_mac_address= bcmasp_set_mac_address,
};
such that MII ioctls work properly.
--
Florian
smime.p7s
Description: S/MIME Cryptographic Signature
Re: [PATCH net-next v5 3/6] net: bcmasp: Add support for ASP2.0 Ethernet controller
On 5/25/23 8:54 PM, Jakub Kicinski wrote:
On Wed, 24 May 2023 16:01:50 -0700 Justin Chen wrote:
Add support for the Broadcom ASP 2.0 Ethernet controller which is first
introduced with 72165. This controller features two distinct Ethernet
ports that can be independently operated.
This patch supports:
- Wake-on-LAN using magic packets
- basic ethtool operations (link, counters, message level)
- MAC destination address filtering (promiscuous, ALL_MULTI, etc.)
+static netdev_tx_t bcmasp_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+ struct bcmasp_intf *intf = netdev_priv(dev);
+ int spb_index, nr_frags, ret, i, j;
+ unsigned int total_bytes, size;
+ struct bcmasp_tx_cb *txcb;
+ dma_addr_t mapping, valid;
+ struct bcmasp_desc *desc;
+ bool csum_hw = false;
+ struct device *kdev;
+ skb_frag_t *frag;
+
+ kdev = >parent->pdev->dev;
+
+ spin_lock(>tx_lock);
What is the tx_lock for? netdevs already have a tx lock, unless you
declare the device as lockless.
Will remove.
+static void bcmasp_tx_timeout(struct net_device *dev, unsigned int txqueue)
+{
+ struct bcmasp_intf *intf = netdev_priv(dev);
+
+ netif_dbg(intf, tx_err, dev, "transmit timeout!\n");
+
+ netif_trans_update(dev);
+ dev->stats.tx_errors++;
+
+ netif_wake_queue(dev);
If the queue is full xmit will just put it back to sleep.
You want to try to reap completions if anything, no?
I can remove the wake. As you mentioned it won't do anything here. There
isn't anything to reap if we are in the timeout condition. If it is some
HW stall, we could flush and restart the ring, but if that is the case I
rather figure out why the HW is stalling. I think we can leave it as a
"tell the user we are stalled" and leave it as that.
+static struct net_device_stats *bcmasp_get_stats(struct net_device *dev)
+{
+ return >stats;
+}
you don't have to do this, core will use device stats if there's no ndo
+ ndev = alloc_etherdev(sizeof(struct bcmasp_intf));
+ if (!dev) {
*blink* condition is typo'ed
Oops. Good catch.
Thanks,
Justin
+ dev_warn(dev, "%s: unable to alloc ndev\n", ndev_dn->name);
+ goto err;
+ }
smime.p7s
Description: S/MIME Cryptographic Signature
Re: [PATCH net-next v5 3/6] net: bcmasp: Add support for ASP2.0 Ethernet controller
On Wed, 24 May 2023 16:01:50 -0700 Justin Chen wrote:
> Add support for the Broadcom ASP 2.0 Ethernet controller which is first
> introduced with 72165. This controller features two distinct Ethernet
> ports that can be independently operated.
>
> This patch supports:
>
> - Wake-on-LAN using magic packets
> - basic ethtool operations (link, counters, message level)
> - MAC destination address filtering (promiscuous, ALL_MULTI, etc.)
> +static netdev_tx_t bcmasp_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> + struct bcmasp_intf *intf = netdev_priv(dev);
> + int spb_index, nr_frags, ret, i, j;
> + unsigned int total_bytes, size;
> + struct bcmasp_tx_cb *txcb;
> + dma_addr_t mapping, valid;
> + struct bcmasp_desc *desc;
> + bool csum_hw = false;
> + struct device *kdev;
> + skb_frag_t *frag;
> +
> + kdev = >parent->pdev->dev;
> +
> + spin_lock(>tx_lock);
What is the tx_lock for? netdevs already have a tx lock, unless you
declare the device as lockless.
> +static void bcmasp_tx_timeout(struct net_device *dev, unsigned int txqueue)
> +{
> + struct bcmasp_intf *intf = netdev_priv(dev);
> +
> + netif_dbg(intf, tx_err, dev, "transmit timeout!\n");
> +
> + netif_trans_update(dev);
> + dev->stats.tx_errors++;
> +
> + netif_wake_queue(dev);
If the queue is full xmit will just put it back to sleep.
You want to try to reap completions if anything, no?
> +static struct net_device_stats *bcmasp_get_stats(struct net_device *dev)
> +{
> + return >stats;
> +}
you don't have to do this, core will use device stats if there's no ndo
> + ndev = alloc_etherdev(sizeof(struct bcmasp_intf));
> + if (!dev) {
*blink* condition is typo'ed
> + dev_warn(dev, "%s: unable to alloc ndev\n", ndev_dn->name);
> + goto err;
> + }
--
pw-bot: cr
[PATCH net-next v5 3/6] net: bcmasp: Add support for ASP2.0 Ethernet controller
Add support for the Broadcom ASP 2.0 Ethernet controller which is first
introduced with 72165. This controller features two distinct Ethernet
ports that can be independently operated.
This patch supports:
- Wake-on-LAN using magic packets
- basic ethtool operations (link, counters, message level)
- MAC destination address filtering (promiscuous, ALL_MULTI, etc.)
Reviewed-by: Simon Horman
Signed-off-by: Florian Fainelli
Signed-off-by: Justin Chen
---
v5
- Remove unnecessary parenthesis
- Use bool for bcmasp_netfilt_check_dup()
v4
- Address sparse warnings
- Fix one more reverse xmas tree violation
- Improve error logic for bcmasp_netfilt_get_reg_offset()
- Remove inlines
v3
- Fix logic error with net stats where some stats were missing
- General clean up addressing formatting/spelling/consistency
- Use dev_err_probe to save a few LoCs
- Use put_unaligned when updating net stats
v2
- Replace a couple functions with helper functions
- Fix a few WoL issues
drivers/net/ethernet/broadcom/Kconfig | 11 +
drivers/net/ethernet/broadcom/Makefile |1 +
drivers/net/ethernet/broadcom/asp2/Makefile|2 +
drivers/net/ethernet/broadcom/asp2/bcmasp.c| 1462
drivers/net/ethernet/broadcom/asp2/bcmasp.h| 637 +
.../net/ethernet/broadcom/asp2/bcmasp_ethtool.c| 568
drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c | 1425 +++
.../net/ethernet/broadcom/asp2/bcmasp_intf_defs.h | 238
8 files changed, 4344 insertions(+)
create mode 100644 drivers/net/ethernet/broadcom/asp2/Makefile
create mode 100644 drivers/net/ethernet/broadcom/asp2/bcmasp.c
create mode 100644 drivers/net/ethernet/broadcom/asp2/bcmasp.h
create mode 100644 drivers/net/ethernet/broadcom/asp2/bcmasp_ethtool.c
create mode 100644 drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c
create mode 100644 drivers/net/ethernet/broadcom/asp2/bcmasp_intf_defs.h
diff --git a/drivers/net/ethernet/broadcom/Kconfig
b/drivers/net/ethernet/broadcom/Kconfig
index 948586bf1b5b..d4166141145d 100644
--- a/drivers/net/ethernet/broadcom/Kconfig
+++ b/drivers/net/ethernet/broadcom/Kconfig
@@ -255,4 +255,15 @@ config BNXT_HWMON
Say Y if you want to expose the thermal sensor data on NetXtreme-C/E
devices, via the hwmon sysfs interface.
+config BCMASP
+ tristate "Broadcom ASP 2.0 Ethernet support"
+ default ARCH_BRCMSTB
+ depends on OF
+ select MII
+ select PHYLIB
+ select MDIO_BCM_UNIMAC
+ help
+ This configuration enables the Broadcom ASP 2.0 Ethernet controller
+ driver which is present in Broadcom STB SoCs such as 72165.
+
endif # NET_VENDOR_BROADCOM
diff --git a/drivers/net/ethernet/broadcom/Makefile
b/drivers/net/ethernet/broadcom/Makefile
index 0ddfb5b5d53c..bac5cb6ad0cd 100644
--- a/drivers/net/ethernet/broadcom/Makefile
+++ b/drivers/net/ethernet/broadcom/Makefile
@@ -17,3 +17,4 @@ obj-$(CONFIG_BGMAC_BCMA) += bgmac-bcma.o bgmac-bcma-mdio.o
obj-$(CONFIG_BGMAC_PLATFORM) += bgmac-platform.o
obj-$(CONFIG_SYSTEMPORT) += bcmsysport.o
obj-$(CONFIG_BNXT) += bnxt/
+obj-$(CONFIG_BCMASP) += asp2/
diff --git a/drivers/net/ethernet/broadcom/asp2/Makefile
b/drivers/net/ethernet/broadcom/asp2/Makefile
new file mode 100644
index ..e07550315f83
--- /dev/null
+++ b/drivers/net/ethernet/broadcom/asp2/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_BCMASP) += bcm-asp.o
+bcm-asp-objs := bcmasp.o bcmasp_intf.o bcmasp_ethtool.o
diff --git a/drivers/net/ethernet/broadcom/asp2/bcmasp.c
b/drivers/net/ethernet/broadcom/asp2/bcmasp.c
new file mode 100644
index ..cb0d4f2de890
--- /dev/null
+++ b/drivers/net/ethernet/broadcom/asp2/bcmasp.c
@@ -0,0 +1,1462 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Broadcom STB ASP 2.0 Driver
+ *
+ * Copyright (c) 2023 Broadcom
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "bcmasp.h"
+#include "bcmasp_intf_defs.h"
+
+static void _intr2_mask_clear(struct bcmasp_priv *priv, u32 mask)
+{
+ intr2_core_wl(priv, mask, ASP_INTR2_MASK_CLEAR);
+ priv->irq_mask &= ~mask;
+}
+
+static void _intr2_mask_set(struct bcmasp_priv *priv, u32 mask)
+{
+ intr2_core_wl(priv, mask, ASP_INTR2_MASK_SET);
+ priv->irq_mask |= mask;
+}
+
+void bcmasp_enable_tx_irq(struct bcmasp_intf *intf, int en)
+{
+ struct bcmasp_priv *priv = intf->parent;
+
+ if (en)
+ _intr2_mask_clear(priv, ASP_INTR2_TX_DESC(intf->channel));
+ else
+ _intr2_mask_set(priv, ASP_INTR2_TX_DESC(intf->channel));
+}
+EXPORT_SYMBOL_GPL(bcmasp_enable_tx_irq);
+
+void bcmasp_enable_rx_irq(struct bcmasp_intf *intf, int en)
+{
+ struct bcmasp_priv *priv = intf->parent;
+
+ if (en)
+ _intr2_mask_clear(priv,
