[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <e4712b67d084bfb830f236392f3cb76d6ffc9701.camel@siemens.com>
Date: Fri, 10 Jan 2025 12:20:29 +0000
From: "Sverdlin, Alexander" <alexander.sverdlin@...mens.com>
To: "s-vadapalli@...com" <s-vadapalli@...com>
CC: "andrew+netdev@...n.ch" <andrew+netdev@...n.ch>, "c-vankar@...com"
<c-vankar@...com>, "davem@...emloft.net" <davem@...emloft.net>,
"jpanis@...libre.com" <jpanis@...libre.com>, "pabeni@...hat.com"
<pabeni@...hat.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"rogerq@...nel.org" <rogerq@...nel.org>, "kuba@...nel.org" <kuba@...nel.org>
Subject: Re: [PATCH net-next v2] net: ethernet: ti: am65-cpsw: VLAN-aware CPSW
only if !DSA
On Fri, 2025-01-10 at 13:05 +0100, Alexander Sverdlin wrote:
> On Fri, 2025-01-10 at 17:32 +0530, s-vadapalli@...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);
> > + }
> > +
--
Alexander Sverdlin
Siemens AG
www.siemens.com
Powered by blists - more mailing lists