RE: [PATCH RESEND v4 net-next 10/14] net: enetc: refactor SI VLAN promiscuous mode configuration

Wei Fang <[email protected]>
Newsgroups dev.linux.lists.imx,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel,org.kernel.vger.netdev,org.ozlabs.lists.linuxppc-dev
Message-ID <GV2PR04MB1173938AA2F6026E023B8476588C02@GV2PR04MB11739.eurprd04.prod.outlook.com>
> > Remove the enetc_set_vlan_promisc(), enetc_enable_si_vlan_promisc() and
> > enetc_disable_si_vlan_promisc() functions, and introduce a new unified
> > function enetc_set_si_vlan_promisc() to enable or disable VLAN
> > promiscuous mode for a specific SI. This simplifies the logic and makes
> > the interface more straightforward. The vlan_promisc_simap field in
> > struct enetc_pf is no longer needed to track the current state.
> >
> > As ENETC V4 only changes the address offset of PSIPVMR register compared
> > to V1 without any functional difference, enetc_set_si_vlan_promisc() can
> > be moved to enetc_pf_common.c in the future with minor adjustments to be
> > reused by the ENETC V4 driver
> >
> > Signed-off-by: Wei Fang <[email protected]>
> > ---
> >  .../net/ethernet/freescale/enetc/enetc_hw.h   |  5 ++-
> >  .../net/ethernet/freescale/enetc/enetc_pf.c   | 36 ++++++++-----------
> >  .../net/ethernet/freescale/enetc/enetc_pf.h   |  1 -
> >  3 files changed, 16 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> > index 66bfda60da9c..16da732dc5de 100644
> > --- a/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> > +++ b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> > @@ -185,9 +185,8 @@ enum enetc_bdr_type {TX, RX};
> >  #define  PSIPMMR_SI_MAC_MP(n)	BIT((n) + 16)
> >
> >  #define ENETC_PSIPVMR		0x001c
> > -#define ENETC_VLAN_PROMISC_MAP_ALL	0x7
> > -#define ENETC_PSIPVMR_SET_VP(simap)	((simap) & 0x7)
> > -#define ENETC_PSIPVMR_SET_VUTA(simap)	(((simap) & 0x7) << 16)
> > +#define  PSIPVMR_SI_VLAN_P(n)	BIT(n) /* n = SI index */
> 
> I suppose it would have been nice to only have a single space after #define
> above.

I intentionally used two spaces for the macros that define the new register
bit fields to distinguish them from the macros that define the register offsets,
making the hierarchy clearer. :)

> 
> The rest of the code looks right and I think the promisc logic is clearer, so:
> 
> Reviewed-by: Joe Damato <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.