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

[Bastien Curutchet]

Hi all,

On 9/5/26 3:55 AM, netdev-bot+sashiko@xxxxxxxxxx wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential
> issue(s) to consider:
>
> Critical: 0 · High: 2 (1) · Medium: 3 · Low: 0 (2)
>
> - [High] KSZ8463 periodic output is advertised as 12 independent channels
>    (.n_pins = 12, .n_per_out = 12 in ksz_switch_chips[KSZ8463]) but the…
> - [High] KSZ8463 periodic output (TOU) is not properly disabled in
>    hardware (leaves EN bit).
> - [Medium] Build failure when CONFIG_NET_DSA_MICROCHIP_KSZ_PTP=n:
>    ksz8463_setup() in drivers/net/dsa/microchip/ksz8.c now calls…
> - [Medium] The newly added ksz8463_ptp_configure_perout() dereferences
>    dev->ptp_data.pin_config[index] (reading pin->chan to select the…
> - [Medium] KSZ8463 TOU enable bit is left set in the hardware if the
>    activation verification check fails.
>
> Pre-existing issues:
> - [High] A non-normalized periodic-output period whose nanosecond value
>    converts to zero is accepted, stored as the global restart period, and…
> - [Low] The KSZ periodic-output implementation accepts and processes
>    PTP_PEROUT_DUTY_CYCLE (ksz_ptp_get_pin() returns -EOPNOTSUPP only for…
> - [Low] ksz_ptp_clock_unregister() only calls ptp_clock_unregister(); it
>    never disables the trigger units that the perout path enabled.
>
> --
>
> net: dsa: microchip: add periodic output support for the KSZ8463
>
> This adds KSZ8463 specific periodic output handling, since the chip has one
> configuration register block per trigger unit, a smaller maximum pulse
> width and 12 outputs instead of 2.  It wires new ksz8463_ptp_* callbacks
> into a dedicated ksz8463_ptp_set_caps() and advertises 12 pins / 12
> periodic output channels for the KSZ8463.
>
> A few questions on the new code below.
>
> > diff --git a/drivers/net/dsa/microchip/ksz8.c b/drivers/net/dsa/microchip/ksz8.c
> > index f2a23e2e474ac..6b08bb8fed5b7 100644
> > --- a/drivers/net/dsa/microchip/ksz8.c
> > +++ b/drivers/net/dsa/microchip/ksz8.c
> > @@ -2478,7 +2478,7 @@ static int ksz8463_setup(struct dsa_switch *ds)
> >   		if (ret)
> >   			goto free_girq;
> >   
> > -		ksz_ptp_set_caps(ds);
> > +		ksz8463_ptp_set_caps(ds);
>
> [Severity: Medium]
> Does this still build with CONFIG_NET_DSA_MICROCHIP_KSZ_PTP=n?

No it doesn't .. I'll send a new iteration with this fixed


Best regards,
Bastien