Re: [PATCH net-next v3 10/10] net: dsa: microchip: add periodic output support for the KSZ8463

[Bastien Curutchet]

Hi all,

On 9/10/26 12:30 PM, netdev-bot+sashiko@xxxxxxxxxx wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential
> issue(s) to consider:
>
> Critical: 0 · High: 1 (2) · Medium: 2 · Low: 1
>
> - [High] KSZ8463 advertises 12 periodic-output channels (.n_pins = 12,
>    .n_per_out = 12, copied into caps by ksz8463_ptp_set_caps()) while the…
> - [Medium] ksz8463_ptp_enable_perout() is not transactional: it calls
>    ksz8463_ptp_tou_reset(dev, request->index) (stopping an already…
> - [Medium] ksz8463_ptp_configure_perout() dereferences
>    dev->ptp_data.pin_config[index]…
> - [Low] ksz8463_ptp_set_caps() (drivers/net/dsa/microchip/ksz_ptp.c:1335)
>    does not set ptp_data->caps.supported_perout_flags, while the code…
>
> Pre-existing issues:
> - [High] The new KSZ8463 settime/adjtime paths funnel into
>    ksz_ptp_prepare_restart_perout(), whose catch-up arithmetic can…
> - [High] A denormalized periodic-output period is accepted as zero
>    nanoseconds and later causes a kernel divide-by-zero or an infinite…
>
> --
>
> net: dsa: microchip: add periodic output support for the KSZ8463
>
> This adds KSZ8463 specific periodic output handling: per-unit trigger
> configuration registers, a smaller maximum pulse width, and a new set of
> ksz8463_* PTP callbacks installed by ksz8463_ptp_set_caps().  The chip
> data now advertises 12 pins and 12 periodic output channels.
>
> A few questions below about the new paths.
>
> > diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> > index 041bb4f963659..9e234ad71b723 100644
> > --- a/drivers/net/dsa/microchip/ksz_common.c
> > +++ b/drivers/net/dsa/microchip/ksz_common.c
> > @@ -1161,6 +1161,8 @@ const struct ksz_chip_data ksz_switch_chips[] = {
> >   		.supports_mii = {false, false, true},
> >   		.supports_rmii = {false, false, true},
> >   		.internal_phy = {true, true, false},
> > +		.n_pins = 12,
> > +		.n_per_out = 12,
> >   	},
>
> [Severity: High]
> Twelve periodic output channels are advertised here, but struct
> ksz_ptp_data still keeps only one set of perout state (a single tou_mode,
> one perout_target_time_first, one perout_period), and the restart helper
> always rebuilds a request for channel 0:
>
> drivers/net/dsa/microchip/ksz_ptp.c:ksz_ptp_prepare_restart_perout() {
> 	...
> 	request->index = 0;
> 	request->flags = 0;
>
> 	return 0;
> }

This is an already existing bug. It made me realize that I forgot to 
send the fix ..

The fix conflicts with this series. I can either send the fix now, wait 
for it to enter net-next and then send a new iteration of this series; 
or send the fix once this series has been merged. I checked the other 
Sashiko comments, and IMHO, this comment is the only one that is really 
relevant.

What do you prefer ?


Best regards,
Bastien