Re: [PATCH net-next v4 02/11] net: dsa: microchip: add the number of pins to chip infos

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] Intermediate-state regression, fixed later in this series.

--

net: dsa: microchip: add the number of pins to chip infos

This adds an n_pins field to struct ksz_chip_data and sets it to 2 on the
PTP-capable chip entries. ksz_ptp_clock_register() now uses this field
instead of the hardcoded KSZ_PTP_N_GPIO.

> diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
> index 47cf397481d5e..33416e59edc73 100644
> --- a/drivers/net/dsa/microchip/ksz_ptp.c
> +++ b/drivers/net/dsa/microchip/ksz_ptp.c
> @@ -1053,14 +1053,14 @@ int ksz_ptp_clock_register(struct dsa_switch *ds)
>  	ptp_data->caps.do_aux_work	= ksz_ptp_do_aux_work;
>  	ptp_data->caps.enable		= ksz_ptp_enable;
>  	ptp_data->caps.verify		= ksz_ptp_verify_pin;
> -	ptp_data->caps.n_pins		= KSZ_PTP_N_GPIO;
> +	ptp_data->caps.n_pins		= dev->info->n_pins;
>  	ptp_data->caps.n_per_out	= 3;

[Severity: Low]

Does this commit leave the KSZ8463 with zero pins?

The ksz_switch_chips[KSZ8463] entry is not marked ptp_capable, and this
patch does not give it an n_pins value. So dev->info->n_pins is 0 for that
chip.

ksz8463_setup() still registers the PTP clock whenever an IRQ is present,
and it does not check ptp_capable:

drivers/net/dsa/microchip/ksz8.c:ksz8463_setup() {
    ...
	if (dev->irq > 0) {
		...
		ret = ksz_ptp_clock_register(ds);
    ...
}

The KSZ8463 PHC therefore gets caps.n_pins = 0 and no pin descriptors,
while caps.n_per_out is still 3. Before this patch it exposed 2 pins.

With no pins, ptp_find_pin() in ksz_ptp_enable_perout() can never succeed:

	pin = ptp_find_pin(ptp_data->clock, PTP_PF_PEROUT, request->index);
	if (pin < 0)
		return -EINVAL;

As a result, every PTP_CLK_REQ_PEROUT request on the KSZ8463 would return
-EINVAL. The PTP core would also reject PIN_GETFUNC and PIN_SETFUNC.

A later patch in the series fixes this. "net: dsa: microchip: add periodic
output support for the KSZ8463" sets .n_pins = 12 and .n_per_out = 12 for
the KSZ8463, so the problem only affects bisection across the
intermediate commits.

Before this patch, periodic output on the KSZ8463 also used the KSZ9477
register layout (REG_PTP_UNIT_INDEX__4). So the lost capability probably
did not work correctly to begin with.

The commit message says:

    Set it to 2 for all the PTP-capable switches.

Could the commit message mention that the KSZ8463 temporarily loses its
pins? Alternatively, this patch could set .n_pins for the KSZ8463 entry.

>  
>  	ret = ksz_ptp_start_clock(dev);
>  	if (ret)
>  		return ret;
>  
> -	for (i = 0; i < KSZ_PTP_N_GPIO; i++) {
> +	for (i = 0; i < dev->info->n_pins; i++) {
>  		struct ptp_pin_desc *ptp_pin = &ptp_data->pin_config[i];

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925-ksz-perout-v4-0-d675bd21ce49%40bootlin.com