RE: [PATCH net-next v4 09/13] net: renesas: rswitch: update port HW init

[Michael Dege]

Hello Jakub,

Thank you for reviewing my patches.

> -----Original Message-----
> From: Jakub Kicinski <kuba@xxxxxxxxxx>
> Sent: Wednesday, May 13, 2026 3:36 AM
> To: Michael Dege <michael.dege@xxxxxxxxxxx>
> Cc: Jakub Kicinski <kuba@xxxxxxxxxx>; Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>;
> andrew+netdev@xxxxxxx; davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; pabeni@xxxxxxxxxx; niklas.soderlund
> <niklas.soderlund@xxxxxxxxxxxx>; paul@xxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux-renesas-
> soc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH net-next v4 09/13] net: renesas: rswitch: update port HW init
> 
> This is an AI-generated review of your patch. The human sending this email has considered the AI
> review valid, or at least plausible.
> ---
> net: renesas: rswitch: update port HW init
> 
> Certain Ethernet and cpu port settings need to be updated to prepare for vlan support.
> 
> > diff --git a/drivers/net/ethernet/renesas/rswitch_main.c
> > b/drivers/net/ethernet/renesas/rswitch_main.c
> > index 90c44dcfd1691..d00af333fcce5 100644
> > --- a/drivers/net/ethernet/renesas/rswitch_main.c
> > +++ b/drivers/net/ethernet/renesas/rswitch_main.c
> > @@ -737,8 +737,11 @@ static int rswitch_gwca_hw_init(struct rswitch_private *priv)
> >  	if (err < 0)
> >  		return err;
> >
> > -	iowrite32(GWVCC_VEM_SC_TAG, priv->addr + GWVCC);
> > -	iowrite32(0, priv->addr + GWTTFC);
> > +	iowrite32(0, priv->addr + GWIRC);
> 
> Is it safe to completely remove the initialization of GWVCC here rather than explicitly clearing it
> with iowrite32(0, priv->addr + GWVCC)?
> 
> If a bootloader, firmware, or a prior kernel (e.g., across a kexec warm
> reboot) previously configured the switch to use Switch Control Tags, this register might retain a
> stale VEM_SC_TAG value.

I have added setting GWVCC to 0. I any case the correct value will be written in case VLANs are 
Enabled in the HW. Otherwise, this setting is ignored.

> 
> > +	iowrite32(FIELD_PREP(DQD, DES_RAM_DP), priv->addr + GWRDQDC0);
> > +	/* Drop frames with unknown tags */
> > +	iowrite32(UT, priv->addr + GWTTFC);
> > +
> >  	iowrite32(lower_32_bits(priv->gwca.linkfix_table_dma), priv->addr +
> > GWDCBAC1);
> 
> [ ... ]
> 
> > @@ -1284,7 +1288,12 @@ static int rswitch_etha_hw_init(struct rswitch_etha *etha, const u8 *mac)
> >  	if (err < 0)
> >  		return err;
> >
> > -	iowrite32(EAVCC_VEM_SC_TAG, etha->addr + EAVCC);
> > +	iowrite32(0, etha->addr + EAIRC);
> 
> Similarly, should EAVCC be explicitly cleared to 0 here?
> 
> Other registers like EAIRC and GWIRC are being explicitly set to 0 in this patch, which suggests the
> driver cannot always rely on the hardware reset state to zero out configurations.
> 
> If a stale value is retained in EAVCC, could it cause issues for subsequent checks that might rely on
> reading EAVCC to determine if a default VLAN is already set?

It is not possible to clear EAVCC in the init function since this is also called when the interface 
state is updated. None the less the register is written when VLANs are configured and the setting
is ignored when VLANs are disabled.

> 
> > +	iowrite32(FIELD_PREP(DQD, DES_RAM_DP), etha->addr + EATDQDC0);
> > +
> > +	/* Drop frames with unknown tags */
> > +	iowrite32(UT, etha->addr + EARTFC);
> > +
> >  	rswitch_rmac_setting(etha, mac);

Best regards,

Michael