diff mbox series

[net-next,v2,2/3] net: lan966x: Add PGID_FIRST and PGID_LAST

Message ID 20220104101849.229195-3-horatiu.vultur@microchip.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: lan966x: Extend switchdev with mdb support | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 5 of 5 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 19 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Horatiu Vultur Jan. 4, 2022, 10:18 a.m. UTC
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>
---
 drivers/net/ethernet/microchip/lan966x/lan966x_main.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Vladimir Oltean Jan. 4, 2022, 11:21 a.m. UTC | #1
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.

>  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 Vultur Jan. 4, 2022, 1:46 p.m. UTC | #2
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
> >
diff mbox series

Patch

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
 #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.