| Message ID | 20250110112725.415094-1-alexander.sverdlin@siemens.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v2] net: ethernet: ti: am65-cpsw: VLAN-aware CPSW only if !DSA | expand |
On Fri, 2025-01-10 at 13:05 +0100, Alexander Sverdlin wrote: > On Fri, 2025-01-10 at 17:32 +0530, s-vadapalli@ti.com wrote: > > > > > diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c > > > > > index 5465bf872734..58c840fb7e7e 100644 > > > > > --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c > > > > > +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c > > > > > @@ -32,6 +32,7 @@ > > > > > #include <linux/dma/ti-cppi5.h> > > > > > #include <linux/dma/k3-udma-glue.h> > > > > > #include <net/page_pool/helpers.h> > > > > > +#include <net/dsa.h> > > > > > #include <net/switchdev.h> > > > > > > > > > > #include "cpsw_ale.h" > > > > > @@ -724,13 +725,22 @@ static int am65_cpsw_nuss_common_open(struct am65_cpsw_common *common) > > > > > u32 val, port_mask; > > > > > struct page *page; > > > > > > > > > > + /* Control register */ > > > > > + val = AM65_CPSW_CTL_P0_ENABLE | AM65_CPSW_CTL_P0_TX_CRC_REMOVE | > > > > > + AM65_CPSW_CTL_VLAN_AWARE | AM65_CPSW_CTL_P0_RX_PAD; > > > > > + /* VLAN aware CPSW mode is incompatible with some DSA tagging schemes. > > > > > + * Therefore disable VLAN_AWARE mode if any of the ports is a DSA Port. > > > > > + */ > > > > > + for (port_idx = 0; port_idx < common->port_num; port_idx++) > > > > > + if (netdev_uses_dsa(common->ports[port_idx].ndev)) { > > > > > + val &= ~AM65_CPSW_CTL_VLAN_AWARE; > > > > > + break; > > > > > + } > > > > > + writel(val, common->cpsw_base + AM65_CPSW_REG_CTL); > > > > > + > > > > > if (common->usage_count) > > > > > return 0; > > > > > > > > The changes above should be present HERE, i.e. below the > > > > "common->usage_count" check, as was the case earlier. > > > > > > It has been moved deliberately, consider first port is being brought up > > > and only then the second port is being brought up (as part of > > > dsa_conduit_setup(), which sets dev->dsa_ptr right before opening the > > > port). As this isn't RMW operation and actually happens under RTNL lock, > > > moving in front of "if" looks safe to me... What do you think? > > > > I understand the issue now. Does the following work? > > > > 1. am65_cpsw_nuss_common_open() can be left as-is i.e. no changes to be > > made. > > 2. Interfaces being brought up will invoke am65_cpsw_nuss_ndo_slave_open() > > which then invokes am65_cpsw_nuss_common_open(). > > 3. Within am65_cpsw_nuss_ndo_slave_open(), immediately after the call to > > am65_cpsw_nuss_common_open() returns, clear AM65_CPSW_CTL_VLAN_AWARE > > bit within AM65_CPSW_REG_CTL register if the interface is DSA. > > This would fail if the port involved into DSA story would be opened first > and the one not involved into DSA afterwards, wouldn't it? Oops, my bad, it would work in principle. Let me rework... > > The patch then effectively is the DSA.h include plus the following diff: > > ------------------------------------------------------------------------------------------------ > > diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c > > index 5465bf872734..7819a5674f9c 100644 > > --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c > > +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c > > @@ -1014,6 +1014,15 @@ static int am65_cpsw_nuss_ndo_slave_open(struct net_device *ndev) > > > > common->usage_count++; > > > > + /* VLAN aware CPSW mode is incompatible with some DSA tagging schemes. > > + * Therefore disable VLAN_AWARE mode if any of the ports is a DSA Port. > > + */ > > + if (netdev_uses_dsa(port->ndev) { > > + reg = readl(common->cpsw_base + AM65_CPSW_REG_CTL); > > + reg &= ~AM65_CPSW_CTL_VLAN_AWARE; > > + writel(reg, common->cpsw_base + AM65_CPSW_REG_CTL); > > + } > > +
diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c index 5465bf872734..58c840fb7e7e 100644 --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c @@ -32,6 +32,7 @@ #include <linux/dma/ti-cppi5.h> #include <linux/dma/k3-udma-glue.h> #include <net/page_pool/helpers.h> +#include <net/dsa.h> #include <net/switchdev.h> #include "cpsw_ale.h" @@ -724,13 +725,22 @@ static int am65_cpsw_nuss_common_open(struct am65_cpsw_common *common) u32 val, port_mask; struct page *page; + /* Control register */ + val = AM65_CPSW_CTL_P0_ENABLE | AM65_CPSW_CTL_P0_TX_CRC_REMOVE | + AM65_CPSW_CTL_VLAN_AWARE | AM65_CPSW_CTL_P0_RX_PAD; + /* VLAN aware CPSW mode is incompatible with some DSA tagging schemes. + * Therefore disable VLAN_AWARE mode if any of the ports is a DSA Port. + */ + for (port_idx = 0; port_idx < common->port_num; port_idx++) + if (netdev_uses_dsa(common->ports[port_idx].ndev)) { + val &= ~AM65_CPSW_CTL_VLAN_AWARE; + break; + } + writel(val, common->cpsw_base + AM65_CPSW_REG_CTL); + if (common->usage_count) return 0; - /* Control register */ - writel(AM65_CPSW_CTL_P0_ENABLE | AM65_CPSW_CTL_P0_TX_CRC_REMOVE | - AM65_CPSW_CTL_VLAN_AWARE | AM65_CPSW_CTL_P0_RX_PAD, - common->cpsw_base + AM65_CPSW_REG_CTL); /* Max length register */ writel(AM65_CPSW_MAX_PACKET_SIZE, host_p->port_base + AM65_CPSW_PORT_REG_RX_MAXLEN);