Re: [PATCH net-next] net: airoha: Fix GDM2 loopback configuration for USB serdes

[Simon Horman <horms@kernel.org>]

On Sat, Feb 28, 2026 at 01:20:33PM +0100, Lorenzo Bianconi wrote:
> Flow Control source port mapping for USB serdes needs to be configured
> according to the GDM port nbd parameter. This is a preliminary patch
> since nbd parameter is specific for the given port serdes and needs to
> be read from the DTS (in the current codebase is assigned statically).
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

Hi Lorenzo,

Is this a fix? If so, should it have a fixes tag? And be targeted at net?
Otherwise, perhaps drop the "Fix" from the subject.

Also, I think it would be worth explaining the situation
before and after this patch.

> ---
>  drivers/net/ethernet/airoha/airoha_eth.c  | 10 ++++++----
>  drivers/net/ethernet/airoha/airoha_regs.h |  5 +----
>  2 files changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> index 3779f93b47bc13ae21d4ae088dece4712dc214e5..37fc2b33d3af8a190a74efc69ed54f163afa6412 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> @@ -1729,10 +1729,12 @@ static int airhoha_set_gdm2_loopback(struct airoha_gdm_port *port)
>  		      SP_CPORT_MASK(val),
>  		      FE_PSE_PORT_CDM2 << __ffs(SP_CPORT_MASK(val)));
>  
> -	if (port->id != AIROHA_GDM3_IDX && airoha_is_7581(eth))
> -		airoha_fe_rmw(eth, REG_SRC_PORT_FC_MAP6,
> -			      FC_ID_OF_SRC_PORT24_MASK,
> -			      FIELD_PREP(FC_ID_OF_SRC_PORT24_MASK, 2));
> +	if (port->id == AIROHA_GDM4_IDX && airoha_is_7581(eth)) {
> +		u32 mask = FC_ID_OF_SRC_PORT_MASK(nbq);

AI generated code review flags that the commit message talks about nbd.
But here we have nbq.

> +
> +		airoha_fe_rmw(eth, REG_SRC_PORT_FC_MAP6, mask,
> +			      AIROHA_GDM2_IDX << __ffs(mask));

Perhaps naïvely I would have used FIELD_PREP here.
Maybe that leads to an incorrect result. But if not,
is a shift used here for consistency with the code at
the beginning of this hunk?

> +	}
>  
>  	return 0;
>  }

...