IronPort-SDR: PDtgc7tL6lMUcRxjBNJoI9VK/vCYXXosYq45IyQCZzTf6IDu/hD/iCdrvkvqqLWhLaUq964Yz/
 5OyBLDGDm4wwCKxUovZGVixew6UKgePG1fp4u6hWLqnPyqKbfrfMwp63q+CY5gGhHhORw4GZ8Y
 JKkAYZcqPOZAC9SKZAIlFTmTUZtMmNsMlUBAw7YvQ95iv2+VHjIiBNlZreJaM3eeP+W9oPEeGP
 IZpRjvtd+vDMvxeyE+ZoOvfWVLAW87V6pbYOfpRC3jXYIgDFTDmh8cUngYEnZDeMzmeJzah3lI
 CIkSBUo83bReITT696iIQQnA
Date:   Tue, 4 Jan 2022 14:46:26 +0100
From:   Horatiu Vultur <horatiu.vultur@microchip.com>
To:     Vladimir Oltean <vladimir.oltean@nxp.com>
CC:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "UNGLinuxDriver@microchip.com" <UNGLinuxDriver@microchip.com>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "kuba@kernel.org" <kuba@kernel.org>,
        "f.fainelli@gmail.com" <f.fainelli@gmail.com>,
        "vivien.didelot@gmail.com" <vivien.didelot@gmail.com>,
        "andrew@lunn.ch" <andrew@lunn.ch>
Subject: Re: [PATCH net-next v2 2/3] net: lan966x: Add PGID_FIRST and
 PGID_LAST
Message-ID: <20220104134626.nrerh23jgni2dhnn@soft-dev3-1.localhost>
References: <20220104101849.229195-1-horatiu.vultur@microchip.com>
 <20220104101849.229195-3-horatiu.vultur@microchip.com>
 <20220104112120.6tfdrzikkn6nbhkn@skbuf>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
In-Reply-To: <20220104112120.6tfdrzikkn6nbhkn@skbuf>
Precedence: bulk

The 01/04/2022 11:21, Vladimir Oltean wrote:
> 
> On Tue, Jan 04, 2022 at 11:18:48AM +0100, Horatiu Vultur wrote:
> > The first entries in the PGID table are used by the front ports while
> > the last entries are used for different purposes like flooding mask,
> > copy to CPU, etc. So add these macros to define which entries can be
> > used for general purpose.
> >
> > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> > ---
> 
> Not too thrilled by the "first" and "last" names, since there are PGIDs
> before the "first" PGID, and after the "last" PGID, too. I can see how
> others may get confused about this. In the ocelot driver they are called
> "nonreserved" PGIDs. It also doesn't help that PGID_FIRST and PGID_LAST
> are defined under the "Reserved PGIDs" comment, because they aren't reserved.

OK, I will try to find a better name fore these.

> 
> >  drivers/net/ethernet/microchip/lan966x/lan966x_main.h | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> > index f70e54526f53..190d62ced3fd 100644
> > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> > @@ -30,7 +30,11 @@
> >  /* Reserved amount for (SRC, PRIO) at index 8*SRC + PRIO */
> >  #define QSYS_Q_RSRV                  95
> >
> > +#define CPU_PORT                     8
> > +
> >  /* Reserved PGIDs */
> > +#define PGID_FIRST                   (CPU_PORT + 1)
> > +#define PGID_LAST                    PGID_CPU
> 
> Since PGID_LAST is defined in terms of PGID_CPU, I would put it _below_
> PGID_CPU.
> 
> >  #define PGID_CPU                     (PGID_AGGR - 6)
> >  #define PGID_UC                              (PGID_AGGR - 5)
> >  #define PGID_BC                              (PGID_AGGR - 4)
> > @@ -44,8 +48,6 @@
> >  #define LAN966X_SPEED_100            2
> >  #define LAN966X_SPEED_10             3
> >
> > -#define CPU_PORT                     8
> > -
> >  /* MAC table entry types.
> >   * ENTRYTYPE_NORMAL is subject to aging.
> >   * ENTRYTYPE_LOCKED is not subject to aging.
> > --
> > 2.33.0
> >

-- 
/Horatiu