| Message ID | 20250110092624.287209-1-alexander.sverdlin@siemens.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: ethernet: ti: am65-cpsw: VLAN-aware CPSW only if !DSA | expand |
On Fri, Jan 10, 2025 at 10:26:21AM +0100, A. Sverdlin wrote: > From: Alexander Sverdlin <alexander.sverdlin@siemens.com> > > Only configure VLAN-aware CPSW mode if no port is used as DSA CPU port. > VLAN-aware mode interferes with some DSA tagging schemes and makes stacking > DSA switches downstream of CPSW impossible. Previous attempts to address > the issue linked below. > > Link: https://lore.kernel.org/netdev/20240227082815.2073826-1-s-vadapalli@ti.com/ > Link: https://lore.kernel.org/linux-arm-kernel/4699400.vD3TdgH1nR@localhost/ > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com> > --- > drivers/net/ethernet/ti/am65-cpsw-nuss.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c > index dcb6662b473d..e445acb29e16 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,23 @@ 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_P0_RX_PAD; > + for (port_idx = 0; port_idx < common->port_num; port_idx++) { > + struct am65_cpsw_port *port = &common->ports[port_idx]; > + > + if (netdev_uses_dsa(port->ndev)) > + break; > + } > + /* VLAN aware CPSW mode is incompatible with some DSA tagging schemes */ > + if (port_idx == common->port_num) > + val |= AM65_CPSW_CTL_VLAN_AWARE; > + writel(val, common->cpsw_base + AM65_CPSW_REG_CTL); > + Though it functionally enables VLAN_AWARE mode only when none of the ports are a DSA port, the implementation and the comment appear inconsistent. The comment states that VLAN aware CPSW mode is incompatible with DSA and the lines following it are enabling VLAN_AWARE mode albeit for the case where none of the ports are a DSA port. Since the IF condition doesn't indicate that in an obvious manner, the implementation could be improved to maintain consistency between what the comment states and what the code does. How about the following? ------------------------------------------------------------------------- /* 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); Regards, Siddharth.
diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c index dcb6662b473d..e445acb29e16 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,23 @@ 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_P0_RX_PAD; + for (port_idx = 0; port_idx < common->port_num; port_idx++) { + struct am65_cpsw_port *port = &common->ports[port_idx]; + + if (netdev_uses_dsa(port->ndev)) + break; + } + /* VLAN aware CPSW mode is incompatible with some DSA tagging schemes */ + if (port_idx == common->port_num) + val |= AM65_CPSW_CTL_VLAN_AWARE; + 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);