Re: [PATCH net-next 13/15] net: dsa: microchip: bypass dev_ops for port_setup()

[Bastien Curutchet <bastien.curutchet@bootlin.com>]

Hi,

On 5/15/26 3:15 AM, Jakub Kicinski wrote:
> 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: dsa: microchip: bypass dev_ops for port_setup()
> 
> This commit removes an indirection layer for KSZ switches by connecting
> the port_setup operations directly to dsa_switch_ops, bypassing the
> ksz_dev_ops callback mechanism.
> 
>> diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
>> index 7385aa4e788a1..2f63a3a7d6ceb 100644
>> --- a/drivers/net/dsa/microchip/ksz9477.c
>> +++ b/drivers/net/dsa/microchip/ksz9477.c
> [ ... ]
>> @@ -1309,6 +1309,23 @@ static void ksz9477_port_setup(struct ksz_device *dev, int port, bool cpu_port)
>>   	ksz_pwrite8(dev, port, regs[REG_PORT_PME_CTRL], 0);
>>   }
>>   
>> +int ksz9477_dsa_port_setup(struct dsa_switch *ds, int port)
>> +{
>> +	struct ksz_device *dev = ds->priv;
>> +	int ret;
>> +
>> +	if (!dsa_is_user_port(ds, port))
>> +		return 0;
>> +
>> +	ksz9477_port_setup(dev, port, false);
>> +
>> +	ret = ksz9477_set_default_prio_queue_mapping(dev, port);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return ksz_dcb_init_port(dev, port);
>> +};
> 
> This isn't a bug, but there's a stray semicolon at the end of the function definition.
> 

Good catch.

> [ ... ]
>> diff --git a/drivers/net/dsa/microchip/lan937x_main.c b/drivers/net/dsa/microchip/lan937x_main.c
>> index ecb072cd99819..aff0c2735c307 100644
>> --- a/drivers/net/dsa/microchip/lan937x_main.c
>> +++ b/drivers/net/dsa/microchip/lan937x_main.c
> [ ... ]
>> @@ -823,7 +822,7 @@ const struct dsa_switch_ops lan937x_switch_ops = {
>>   	.phy_read		= ksz_phy_read16,
>>   	.phy_write		= ksz_phy_write16,
>>   	.phylink_get_caps	= lan937x_phylink_get_caps,
>> -	.port_setup		= ksz_port_setup,
>> +	.port_setup		= ksz9477_dsa_port_setup,
> 
> Is it intentional to use ksz9477_dsa_port_setup() for LAN937x switches here?
> 
> Previously, the shared ksz_port_setup() dynamically dispatched the
> initialization via dev->dev_ops->port_setup(), which correctly resolved to
> lan937x_port_setup() for LAN937x devices.
> 
> Since the new ksz9477_dsa_port_setup() function explicitly hardcodes a call
> to ksz9477_port_setup(), won't this cause LAN937x ports to skip their
> specific initialization logic and instead receive incorrect register
> configurations meant for KSZ9477 hardware?

True, I'll fix this in next iteration


Best regards,
Bastien