[PATCH net-next v4 00/11] net: dsa: microchip: add periodic output support for the KSZ8463

[PATCH net-next v4 00/11] net: dsa: microchip: add periodic output support for the KSZ8463

[Bastien Curutchet (Schneider Electric)]

This series aims to add periodic output support for the KSZ8463. The
way the KSZ8463 handles periodic output differs from that of the other
KSZ switches. It therefore needs its own set of perout callbacks
(configure, start, reset, enable, restart).

Patch 1 saves the full request to be able to retrieve all its
information in case of restart

Patches 2 to 4 prepare the driver to accept different kinds of periodic
output settings.

Patches 5 to 10 extract a set of functions from the existing periodic
output support that can be reused by the KSZ8463, avoiding code
duplication as much as possible.

Patch 11 adds periodic output support for the KSZ8463.

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
Changes in v4:
- Add PATCH 1 to fix request configuration at restarts
- PATCH 7: Add checks on the period to ensure divide by zero won't
  happen later. It addresses a corner case spotted by Sashiko on v3
- Link to v3: https://lore.kernel.org/r/20260908-ksz-perout-v3-0-6722a3f1ca75@bootlin.com

Changes in v3:
- PATCH 10: Fix build error when CONFIG_NET_DSA_MICROCHIP_KSZ_PTP=n
- Link to v2: https://lore.kernel.org/r/20260902-ksz-perout-v2-0-6f277fcc9e68@bootlin.com

Changes in v2:
- PATCH 1: Init n_pins for all PTP capable switches
- PATCH 2: Init n_per_out for all PTP capable switches
- Link to v1: https://lore.kernel.org/r/20260831-ksz-perout-v1-0-14202db763b3@bootlin.com

---
Bastien Curutchet (Schneider Electric) (11):
      net: dsa: microchip: fully save the periodic output request
      net: dsa: microchip: add the number of pins to chip infos
      net: dsa: microchip: add the number of periodic signals to chip infos
      net: dsa: microchip: use dynamic mask to check pulse width validity
      net: dsa: microchip: extract PTP callbacks configuration from PTP registration
      net: dsa: microchip: extract ptp_get_pin
      net: dsa: microchip: extract compute_width
      net: dsa: microchip: extract prepare reset
      net: dsa: microchip: extract time update
      net: dsa: microchip: extract time adjustment
      net: dsa: microchip: add periodic output support for the KSZ8463

 drivers/net/dsa/microchip/ksz8.c         |   2 +
 drivers/net/dsa/microchip/ksz9477.c      |   1 +
 drivers/net/dsa/microchip/ksz_common.c   |  22 ++
 drivers/net/dsa/microchip/ksz_common.h   |   2 +
 drivers/net/dsa/microchip/ksz_ptp.c      | 490 ++++++++++++++++++++++++++-----
 drivers/net/dsa/microchip/ksz_ptp.h      |   9 +-
 drivers/net/dsa/microchip/ksz_ptp_reg.h  |  16 +
 drivers/net/dsa/microchip/lan937x_main.c |   1 +
 8 files changed, 467 insertions(+), 76 deletions(-)
---
base-commit: 42a9fb3382fc2573e92f41d203b095d9a372cfc9
change-id: 20260825-ksz-perout-af9813e12158

Best regards,
-- 
Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>

[PATCH net-next v4 01/11] net: dsa: microchip: fully save the periodic output request

[Bastien Curutchet (Schneider Electric)]

When a periodic output is initialized, its period and start time are
stored to initialize it back in case of restart. These information
aren't enough if the request contains flags or targets a pin that isn't
the first output of the switch. At restarts, these two arguments are
always set to 0.

The 'flag issue' isn't concrete so far because the driver doesn't
advertise any .supported_perout_flags so any flag request gets rejected
by the upper layer.
I don't know if other switches use non-zero indexes but the KSZ8463 has
twelve periodic outputs, so this index has to also be saved from now on.

Save the full request when a periodic output is successfully started.
Use the saved request when the periodic output is restarted.

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
 drivers/net/dsa/microchip/ksz_ptp.c | 33 ++++++++++++++++++---------------
 drivers/net/dsa/microchip/ksz_ptp.h |  3 +--
 2 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index 39cc70d65900..47cf397481d5 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -189,6 +189,7 @@ static int ksz_ptp_enable_perout(struct ksz_device *dev,
 {
 	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
 	u64 req_pulse_width_ns;
+	struct timespec64 tmp;
 	u64 cycle_width_ns;
 	u64 pulse_width_ns;
 	int pin = 0;
@@ -222,13 +223,9 @@ static int ksz_ptp_enable_perout(struct ksz_device *dev,
 		return 0;
 	}
 
-	ptp_data->perout_target_time_first.tv_sec  = request->start.sec;
-	ptp_data->perout_target_time_first.tv_nsec = request->start.nsec;
-
-	ptp_data->perout_period.tv_sec = request->period.sec;
-	ptp_data->perout_period.tv_nsec = request->period.nsec;
-
-	cycle_width_ns = timespec64_to_ns(&ptp_data->perout_period);
+	tmp.tv_sec = request->period.sec;
+	tmp.tv_nsec = request->period.nsec;
+	cycle_width_ns = timespec64_to_ns(&tmp);
 	if ((cycle_width_ns & TRIG_CYCLE_WIDTH_M) != cycle_width_ns)
 		return -EINVAL;
 
@@ -249,9 +246,10 @@ static int ksz_ptp_enable_perout(struct ksz_device *dev,
 	if (ret)
 		return ret;
 
+	tmp.tv_sec = request->start.sec;
+	tmp.tv_nsec = request->start.nsec;
 	ret = ksz_ptp_configure_perout(dev, cycle_width_ns, pulse_width_ns,
-				       &ptp_data->perout_target_time_first,
-				       pin);
+				       &tmp, pin);
 	if (ret)
 		return ret;
 
@@ -263,6 +261,8 @@ static int ksz_ptp_enable_perout(struct ksz_device *dev,
 	if (ret)
 		return ret;
 
+	memcpy(&ptp_data->perout_request, request,
+	       sizeof(struct ptp_perout_request));
 	ptp_data->tou_mode = KSZ_PTP_TOU_PEROUT;
 
 	return 0;
@@ -763,6 +763,7 @@ static int ksz_ptp_restart_perout(struct ksz_device *dev)
 	struct ptp_perout_request request;
 	struct timespec64 next;
 	struct timespec64 now;
+	struct timespec64 tmp;
 	unsigned int count;
 	int ret;
 
@@ -773,10 +774,14 @@ static int ksz_ptp_restart_perout(struct ksz_device *dev)
 		return ret;
 
 	now_ns = timespec64_to_ns(&now);
-	first_ns = timespec64_to_ns(&ptp_data->perout_target_time_first);
+	tmp.tv_sec = ptp_data->perout_request.start.sec;
+	tmp.tv_nsec = ptp_data->perout_request.start.nsec;
+	first_ns = timespec64_to_ns(&tmp);
 
 	/* Calculate next perout event based on start time and period */
-	period_ns = timespec64_to_ns(&ptp_data->perout_period);
+	tmp.tv_sec = ptp_data->perout_request.period.sec;
+	tmp.tv_nsec = ptp_data->perout_request.period.nsec;
+	period_ns = timespec64_to_ns(&tmp);
 
 	if (first_ns < now_ns) {
 		count = div_u64(now_ns - first_ns, period_ns);
@@ -791,12 +796,10 @@ static int ksz_ptp_restart_perout(struct ksz_device *dev)
 
 	/* Restart periodic output signal */
 	next = ns_to_timespec64(next_ns);
+	memcpy(&request, &ptp_data->perout_request,
+	       sizeof(struct ptp_perout_request));
 	request.start.sec  = next.tv_sec;
 	request.start.nsec = next.tv_nsec;
-	request.period.sec  = ptp_data->perout_period.tv_sec;
-	request.period.nsec = ptp_data->perout_period.tv_nsec;
-	request.index = 0;
-	request.flags = 0;
 
 	return ksz_ptp_enable_perout(dev, &request, 1);
 }
diff --git a/drivers/net/dsa/microchip/ksz_ptp.h b/drivers/net/dsa/microchip/ksz_ptp.h
index 7067ec9bd1e6..eb3827203b96 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.h
+++ b/drivers/net/dsa/microchip/ksz_ptp.h
@@ -29,8 +29,7 @@ struct ksz_ptp_data {
 	spinlock_t clock_lock;
 	struct timespec64 clock_time;
 	enum ksz_ptp_tou_mode tou_mode;
-	struct timespec64 perout_target_time_first;  /* start of first pulse */
-	struct timespec64 perout_period;
+	struct ptp_perout_request perout_request;
 };
 
 int ksz_ptp_clock_register(struct dsa_switch *ds);

-- 
2.55.0

Re: [PATCH net-next v4 01/11] net: dsa: microchip: fully save the periodic output request

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 1 (1) · Medium: 1 (1) · Low: 0 (1)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The patch keeps one saved request (ptp_data->perout_request) and
  one device-wide tou_mode, but the driver allows several trigger output…
- [Medium] This patch fixes a user-visible bug that already exists on the
  switches supported today (KSZ9477 family), but the commit message does…

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but it is serious.
- [Medium] This problem wasn't introduced by this patch, but the restart
  path calls ptp_find_pin() without holding ptp_clock::pincfg_mux, which…
- [Low] ksz_ptp_enable_perout() selects the TOU and resets it with
  ksz_ptp_tou_reset(), which clears TRIG_ENABLE, before validation,…

--

net: dsa: microchip: fully save the periodic output request

This commit replaces the saved start time and period in struct
ksz_ptp_data with a full copy of the last successful struct
ptp_perout_request. After settime/adjtime, ksz_ptp_restart_perout() now
reuses that saved request, including its index and flags, instead of
forcing both to zero.

> diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
> index 39cc70d65900b..47cf397481d5e 100644
> --- a/drivers/net/dsa/microchip/ksz_ptp.c
> +++ b/drivers/net/dsa/microchip/ksz_ptp.c

[ ... ]

> @@ -263,6 +261,8 @@ static int ksz_ptp_enable_perout(struct ksz_device *dev,
>  	if (ret)
>  		return ret;
>  
> +	memcpy(&ptp_data->perout_request, request,
> +	       sizeof(struct ptp_perout_request));
>  	ptp_data->tou_mode = KSZ_PTP_TOU_PEROUT;
>  
>  	return 0;

[Severity: High]
struct ksz_ptp_data has only one perout_request and one tou_mode. What
happens when more than one periodic output is active?

The busy check at the top of ksz_ptp_enable_perout() lets a second
channel start while another one is already in PEROUT mode:

	if (ptp_data->tou_mode != KSZ_PTP_TOU_PEROUT &&
	    ptp_data->tou_mode != KSZ_PTP_TOU_IDLE)
		return -EBUSY;

ksz_ptp_tou_reset() only touches the selected unit, so the earlier TOU
keeps running in hardware. Each successful start then overwrites
ptp_data->perout_request, whatever request->index is.

KSZ9477 advertises 2 pins and n_per_out = 3. Suppose pin0 is assigned to
PEROUT chan 0 and pin1 to PEROUT chan 1, and index 1 is started before
index 0. Only index 0 is saved.

On the next clock_settime or clock_adjtime, ksz_ptp_restart_perout()
re-arms only TOU0. TOU1 keeps a target time that was computed against
the clock before the step.

Disabling any one channel also clears the mode for the whole device:

	if (!on) {
		ptp_data->tou_mode = KSZ_PTP_TOU_IDLE;
		return 0;
	}

This path is also reached from ptp_set_pinfunc()->ptp_disable_pinfunc()
when a pin is reassigned. After that, none of the outputs still running
are restarted on clock steps.

The KSZ8463 with its twelve outputs is the motivation here, and
ksz8463_ptp_enable_perout() later in the series writes into the same
slot. Would restart need a saved request and an active state per channel
to work for outputs other than the first?

[Severity: Low]
This isn't a bug introduced by this patch, but ksz_ptp_enable_perout()
calls ksz_ptp_tou_reset() on the selected TOU before it validates the
request. None of the later error returns restore the hardware state or
tou_mode.

Now that the request is saved only on success, consider a reconfiguration
of a running channel that fails after ksz_ptp_tou_reset(). Examples are a
period that doesn't fit TRIG_CYCLE_WIDTH_M, a start time over 32 bits, or
a failure in ksz_ptp_tou_start().

The TOU is then left stopped. If the REG_PTP_TRIG_STATUS__4 read or the
TRIG_ERROR check fails after TRIG_ENABLE was set, it is left armed with
the failed parameters instead. Either way tou_mode stays
KSZ_PTP_TOU_PEROUT and perout_request still holds the previous request,
so the next clock step re-arms that TOU with the old parameters.

This matches the "successfully started" wording in the commit message.
It is also no worse than the old code, which saved start and period
before validation. Is falling back to the last successful request the
intended behaviour after a failed PTP_PEROUT_REQUEST?

> @@ -763,6 +763,7 @@ static int ksz_ptp_restart_perout(struct ksz_device *dev)
>  	struct ptp_perout_request request;
>  	struct timespec64 next;
>  	struct timespec64 now;
> +	struct timespec64 tmp;
>  	unsigned int count;
>  	int ret;
>  
> @@ -773,10 +774,14 @@ static int ksz_ptp_restart_perout(struct ksz_device *dev)
>  		return ret;
>  
>  	now_ns = timespec64_to_ns(&now);
> -	first_ns = timespec64_to_ns(&ptp_data->perout_target_time_first);
> +	tmp.tv_sec = ptp_data->perout_request.start.sec;
> +	tmp.tv_nsec = ptp_data->perout_request.start.nsec;
> +	first_ns = timespec64_to_ns(&tmp);
>  
>  	/* Calculate next perout event based on start time and period */
> -	period_ns = timespec64_to_ns(&ptp_data->perout_period);
> +	tmp.tv_sec = ptp_data->perout_request.period.sec;
> +	tmp.tv_nsec = ptp_data->perout_request.period.nsec;
> +	period_ns = timespec64_to_ns(&tmp);
>  
>  	if (first_ns < now_ns) {
>  		count = div_u64(now_ns - first_ns, period_ns);

[Severity: High]
This isn't a bug introduced by this patch, but this function is being
reworked, so I'm raising it here. count is declared unsigned int, while
div_u64() returns u64. Can the quotient get truncated here?

Suppose 2^32 or more periods have passed since the last (re)start. Then
next_ns = first_ns + count * period_ns lands far in the past, and the
guard loop below has to catch up one period at a time, at least 2^32
times:

	/* Ensure 100 ms guard time prior next event */
	while (next_ns < now_ns + 100000000)
		next_ns += period_ns;

The loop has no cond_resched(), and ptp_data->lock is held by
ksz_ptp_settime() or ksz_ptp_adjtime() the whole time.

That point is reached after about 49.7 days without a clock step at a
1 ms period, about 71 minutes at 1 us, and about 34 seconds at 8 ns.

Would making count a u64 avoid this? Later in the series, the KSZ8463
restart path also uses the same code through
ksz_ptp_prepare_restart_perout().

> @@ -791,12 +796,10 @@ static int ksz_ptp_restart_perout(struct ksz_device *dev)
>  
>  	/* Restart periodic output signal */
>  	next = ns_to_timespec64(next_ns);
> +	memcpy(&request, &ptp_data->perout_request,
> +	       sizeof(struct ptp_perout_request));
>  	request.start.sec  = next.tv_sec;
>  	request.start.nsec = next.tv_nsec;
> -	request.period.sec  = ptp_data->perout_period.tv_sec;
> -	request.period.nsec = ptp_data->perout_period.tv_nsec;
> -	request.index = 0;
> -	request.flags = 0;

[Severity: Medium]
The commit message says:

  I don't know if other switches use non-zero indexes but the KSZ8463 has
  twelve periodic outputs, so this index has to also be saved from now on.

Doesn't the removed request.index = 0 already break restart on the
KSZ9477-family switches supported today? ksz_ptp_clock_register()
advertises:

	ptp_data->caps.n_per_out	= 3;

ptp_perout_request() in drivers/ptp/ptp_chardev.c only rejects
perout->index >= n_per_out, so indexes 1 and 2 reach
ksz_ptp_enable_perout().

With the old code, a clock step through ksz_ptp_settime() or
ksz_ptp_adjtime() while channel 1 or 2 is running goes one of two ways:

- If no pin is assigned to PEROUT channel 0, ptp_find_pin() fails and
  ksz_ptp_enable_perout() returns -EINVAL. settime/adjtime then returns
  an error and the real channel is never re-armed.

- If a pin is assigned to channel 0, TOU0 is re-armed on that pin with
  the other channel's period.

Could the commit message describe this? Should it also carry a Fixes: tag
for 1f12ae5b67608f ("net: dsa: microchip: ptp: add periodic output
signal") so it can be considered for stable?

>  
>  	return ksz_ptp_enable_perout(dev, &request, 1);

[Severity: Medium]
This is a pre-existing issue, but ksz_ptp_restart_perout() is called from
ksz_ptp_settime() and ksz_ptp_adjtime(), which hold only ptp_data->lock.
The PTP core takes ptp_clock::pincfg_mux around ops->enable(), but not
around settime64 or adjtime.

ksz_ptp_enable_perout() then does:

	pin = ptp_find_pin(ptp_data->clock, PTP_PF_PEROUT, request->index);

The documentation for ptp_find_pin() in include/linux/ptp_clock_kernel.h
says "The caller must hold ptp_clock::pincfg_mux."

Meanwhile, ptp_set_pinfunc() can update pin2->func and pin2->chan while
holding only pincfg_mux. When the old function was PTP_PF_NONE it doesn't
call into the driver, so ptp_data->lock is never taken.

Can the restart see a half-updated pin descriptor and send the restarted
output to the wrong pin? This is still present at the end of the series
in ksz_ptp_get_pin().

ptp_find_pin_unlocked() exists for callers that don't hold pincfg_mux.
It would have to be called before ptp_data->lock is taken, though,
because ksz_ptp_enable() already takes ptp_data->lock with pincfg_mux
held.

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

Re: [PATCH net-next v4 01/11] net: dsa: microchip: fully save the periodic output request

[Bastien Curutchet]

Hi all,

On 9/29/26 2:27 PM, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 1 (1) · Medium: 1 (1) · Low: 0 (1)
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [High] The patch keeps one saved request (ptp_data->perout_request) and
>    one device-wide tou_mode, but the driver allows several trigger output…
> - [Medium] This patch fixes a user-visible bug that already exists on the
>    switches supported today (KSZ9477 family), but the commit message does…
> 
> Pre-existing issues:
> - [High] This problem wasn't introduced by this patch, but it is serious.
> - [Medium] This problem wasn't introduced by this patch, but the restart
>    path calls ptp_find_pin() without holding ptp_clock::pincfg_mux, which…
> - [Low] ksz_ptp_enable_perout() selects the TOU and resets it with
>    ksz_ptp_tou_reset(), which clears TRIG_ENABLE, before validation,…
> 
> --
> 
> net: dsa: microchip: fully save the periodic output request
> 
> This commit replaces the saved start time and period in struct
> ksz_ptp_data with a full copy of the last successful struct
> ptp_perout_request. After settime/adjtime, ksz_ptp_restart_perout() now
> reuses that saved request, including its index and flags, instead of
> forcing both to zero.
> 
>> diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
>> index 39cc70d65900b..47cf397481d5e 100644
>> --- a/drivers/net/dsa/microchip/ksz_ptp.c
>> +++ b/drivers/net/dsa/microchip/ksz_ptp.c
> 
> [ ... ]
> 
>> @@ -263,6 +261,8 @@ static int ksz_ptp_enable_perout(struct ksz_device *dev,
>>   	if (ret)
>>   		return ret;
>>   
>> +	memcpy(&ptp_data->perout_request, request,
>> +	       sizeof(struct ptp_perout_request));
>>   	ptp_data->tou_mode = KSZ_PTP_TOU_PEROUT;
>>   
>>   	return 0;
> 
> [Severity: High]
> struct ksz_ptp_data has only one perout_request and one tou_mode. What
> happens when more than one periodic output is active?
> 

Indeed, this patch series supports only one periodic output at a time, 
as is already the case for the others switches (which also have several 
output pins). This is sufficient for my use case, and it doesn't prevent 
more advanced support from being added later.
[...]

> 
> [Severity: Low]
> This isn't a bug introduced by this patch,

[...]
> 
> [Severity: High]
> This isn't a bug introduced by this patch,

[...]
> 
> [Severity: Medium]
> The commit message says:
> 
>    I don't know if other switches use non-zero indexes but the KSZ8463 has
>    twelve periodic outputs, so this index has to also be saved from now on.
> 
> Doesn't the removed request.index = 0 already break restart on the
> KSZ9477-family switches supported today? ksz_ptp_clock_register()
> advertises:
> 
> 	ptp_data->caps.n_per_out	= 3;
> 
> ptp_perout_request() in drivers/ptp/ptp_chardev.c only rejects
> perout->index >= n_per_out, so indexes 1 and 2 reach
> ksz_ptp_enable_perout().
> 
> With the old code, a clock step through ksz_ptp_settime() or
> ksz_ptp_adjtime() while channel 1 or 2 is running goes one of two ways:
> 
> - If no pin is assigned to PEROUT channel 0, ptp_find_pin() fails and
>    ksz_ptp_enable_perout() returns -EINVAL. settime/adjtime then returns
>    an error and the real channel is never re-armed.
> 
> - If a pin is assigned to channel 0, TOU0 is re-armed on that pin with
>    the other channel's period.
> 
> Could the commit message describe this? Should it also carry a Fixes: tag
> for 1f12ae5b67608f ("net: dsa: microchip: ptp: add periodic output
> signal") so it can be considered for stable?
>

I had sent a fix for this but we agreed to handle this directly in 
net-next (cf 
https://lore.kernel.org/all/ee69ae01-97ab-470c-ba1d-658f1700463c@bootlin.com/)

>>   
>>   	return ksz_ptp_enable_perout(dev, &request, 1);
> 
> [Severity: Medium]
> This is a pre-existing issue,

[...]


Best regards,
Bastien

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

[Bastien Curutchet (Schneider Electric)]

The number of periodic output pins available is hardcoded to 2 while
KSZ8463 has 12 pins that can be used as periodic outputs.

Add an n_pin attribute to the struct ksz_chip_data to make this setting
configurable.
Set it to 2 for all the PTP-capable switches.

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
 drivers/net/dsa/microchip/ksz_common.c | 10 ++++++++++
 drivers/net/dsa/microchip/ksz_common.h |  1 +
 drivers/net/dsa/microchip/ksz_ptp.c    |  4 ++--
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index f1b53439ff58..ac9a10a91d79 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -1208,6 +1208,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.ptp_capable = true,
 		.wr_table = &ksz8563_register_set,
 		.rd_table = &ksz8563_register_set,
+		.n_pins = 2,
 	},
 
 	[KSZ8795] = {
@@ -1443,6 +1444,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.sgmii_port = 7,
 		.wr_table = &ksz9477_register_set,
 		.rd_table = &ksz9477_register_set,
+		.n_pins = 2,
 	},
 
 	[KSZ9896] = {
@@ -1608,6 +1610,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.internal_phy = {true, true, false},
 		.gbit_capable = {true, true, true},
 		.ptp_capable = true,
+		.n_pins = 2,
 	},
 
 	[KSZ8567] = {
@@ -1645,6 +1648,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.gbit_capable	= {false, false, false, false, false,
 				   true, true},
 		.ptp_capable = true,
+		.n_pins = 2,
 	},
 
 	[KSZ9567] = {
@@ -1679,6 +1683,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 				   true, false, false},
 		.gbit_capable	= {true, true, true, true, true, true, true},
 		.ptp_capable = true,
+		.n_pins = 2,
 	},
 
 	[LAN9370] = {
@@ -1710,6 +1715,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.supports_rgmii = {false, false, false, false, true},
 		.internal_phy = {true, true, true, true, false},
 		.ptp_capable = true,
+		.n_pins = 2,
 	},
 
 	[LAN9371] = {
@@ -1741,6 +1747,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.supports_rgmii = {false, false, false, false, true, true},
 		.internal_phy = {true, true, true, true, false, false},
 		.ptp_capable = true,
+		.n_pins = 2,
 	},
 
 	[LAN9372] = {
@@ -1776,6 +1783,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.internal_phy	= {true, true, true, true,
 				   false, false, true, true},
 		.ptp_capable = true,
+		.n_pins = 2,
 	},
 
 	[LAN9373] = {
@@ -1811,6 +1819,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.internal_phy	= {true, true, true, false,
 				   false, false, true, true},
 		.ptp_capable = true,
+		.n_pins = 2,
 	},
 
 	[LAN9374] = {
@@ -1846,6 +1855,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.internal_phy	= {true, true, true, true,
 				   false, false, true, true},
 		.ptp_capable = true,
+		.n_pins = 2,
 	},
 
 	[LAN9646] = {
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 091cfbdc486f..9b911f9aafd9 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -139,6 +139,7 @@ struct ksz_chip_data {
 	u8 sgmii_port;
 	const struct regmap_access_table *wr_table;
 	const struct regmap_access_table *rd_table;
+	const u8 n_pins;
 };
 
 struct ksz_irq {
diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index 47cf397481d5..33416e59edc7 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;
 
 	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];
 
 		snprintf(ptp_pin->name,

-- 
2.55.0

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

[PATCH net-next v4 03/11] net: dsa: microchip: add the number of periodic signals to chip infos

[Bastien Curutchet (Schneider Electric)]

The number of periodic signals available is hardcoded to 3 while
KSZ8463 can produce 12 different periodic signals.

Add an n_per_out attribute to the struct ksz_chip_data to make this
setting configurable.
Set it to 3 for all the PTP-capable switches.

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
 drivers/net/dsa/microchip/ksz_common.c | 10 ++++++++++
 drivers/net/dsa/microchip/ksz_common.h |  1 +
 drivers/net/dsa/microchip/ksz_ptp.c    |  2 +-
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index ac9a10a91d79..ea6db85e7e2b 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -1209,6 +1209,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.wr_table = &ksz8563_register_set,
 		.rd_table = &ksz8563_register_set,
 		.n_pins = 2,
+		.n_per_out = 3,
 	},
 
 	[KSZ8795] = {
@@ -1445,6 +1446,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.wr_table = &ksz9477_register_set,
 		.rd_table = &ksz9477_register_set,
 		.n_pins = 2,
+		.n_per_out = 3,
 	},
 
 	[KSZ9896] = {
@@ -1611,6 +1613,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.gbit_capable = {true, true, true},
 		.ptp_capable = true,
 		.n_pins = 2,
+		.n_per_out = 3,
 	},
 
 	[KSZ8567] = {
@@ -1649,6 +1652,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 				   true, true},
 		.ptp_capable = true,
 		.n_pins = 2,
+		.n_per_out = 3,
 	},
 
 	[KSZ9567] = {
@@ -1684,6 +1688,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.gbit_capable	= {true, true, true, true, true, true, true},
 		.ptp_capable = true,
 		.n_pins = 2,
+		.n_per_out = 3,
 	},
 
 	[LAN9370] = {
@@ -1716,6 +1721,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.internal_phy = {true, true, true, true, false},
 		.ptp_capable = true,
 		.n_pins = 2,
+		.n_per_out = 3,
 	},
 
 	[LAN9371] = {
@@ -1748,6 +1754,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 		.internal_phy = {true, true, true, true, false, false},
 		.ptp_capable = true,
 		.n_pins = 2,
+		.n_per_out = 3,
 	},
 
 	[LAN9372] = {
@@ -1784,6 +1791,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 				   false, false, true, true},
 		.ptp_capable = true,
 		.n_pins = 2,
+		.n_per_out = 3,
 	},
 
 	[LAN9373] = {
@@ -1820,6 +1828,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 				   false, false, true, true},
 		.ptp_capable = true,
 		.n_pins = 2,
+		.n_per_out = 3,
 	},
 
 	[LAN9374] = {
@@ -1856,6 +1865,7 @@ const struct ksz_chip_data ksz_switch_chips[] = {
 				   false, false, true, true},
 		.ptp_capable = true,
 		.n_pins = 2,
+		.n_per_out = 3,
 	},
 
 	[LAN9646] = {
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 9b911f9aafd9..f1c0e89e13c1 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -140,6 +140,7 @@ struct ksz_chip_data {
 	const struct regmap_access_table *wr_table;
 	const struct regmap_access_table *rd_table;
 	const u8 n_pins;
+	const u8 n_per_out;
 };
 
 struct ksz_irq {
diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index 33416e59edc7..84c799959aaa 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -1054,7 +1054,7 @@ int ksz_ptp_clock_register(struct dsa_switch *ds)
 	ptp_data->caps.enable		= ksz_ptp_enable;
 	ptp_data->caps.verify		= ksz_ptp_verify_pin;
 	ptp_data->caps.n_pins		= dev->info->n_pins;
-	ptp_data->caps.n_per_out	= 3;
+	ptp_data->caps.n_per_out	= dev->info->n_per_out;
 
 	ret = ksz_ptp_start_clock(dev);
 	if (ret)

-- 
2.55.0

Re: [PATCH net-next v4 03/11] net: dsa: microchip: add the number of periodic signals 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] This patch makes KSZ8463 advertise zero periodic outputs until
  the last patch in the series fixes it.

--

net: dsa: microchip: add the number of periodic signals to chip infos

This commit adds an n_per_out field to struct ksz_chip_data. It sets the
field to 3 for the PTP-capable switches in ksz_switch_chips[], and
ksz_ptp_clock_register() now uses it in place of the hardcoded value of 3.

[ ... ]

> diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
> index 33416e59edc73..84c799959aaab 100644
> --- a/drivers/net/dsa/microchip/ksz_ptp.c
> +++ b/drivers/net/dsa/microchip/ksz_ptp.c
> @@ -1054,7 +1054,7 @@ int ksz_ptp_clock_register(struct dsa_switch *ds)
>  	ptp_data->caps.enable		= ksz_ptp_enable;
>  	ptp_data->caps.verify		= ksz_ptp_verify_pin;
>  	ptp_data->caps.n_pins		= dev->info->n_pins;
> -	ptp_data->caps.n_per_out	= 3;
> +	ptp_data->caps.n_per_out	= dev->info->n_per_out;

[Severity: Low]
Does this make KSZ8463 advertise zero periodic outputs at this commit?

The ksz_switch_chips[KSZ8463] entry in ksz_common.c gets no .n_per_out
here, so the field is 0. KSZ8463 still registers a PHC whenever an IRQ is
present, and ksz8463_setup() never checks ptp_capable:

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

Before this patch, the KSZ8463 PHC reported n_per_out = 3. After it, the
PHC reports 0 in n_periodic_outputs and PTP_CLOCK_GETCAPS. As a result,
ptp_perout_request() rejects every PTP_PEROUT_REQUEST index.

The commit message says KSZ8463 can produce 12 periodic signals, but
KSZ8463 is the only chip that registers a PHC here and is left at 0. The
earlier patch "net: dsa: microchip: add the number of pins to chip infos"
also leaves KSZ8463 at n_pins = 0.

The last patch in the series, "net: dsa: microchip: add periodic output
support for the KSZ8463", adds .n_pins = 12 and .n_per_out = 12 to the
KSZ8463 entry. The final tree is correct, so this only affects bisection
through the intermediate commits.

Could this patch set .n_per_out for KSZ8463 so the intermediate commits
keep the previous behaviour?

>  
>  	ret = ksz_ptp_start_clock(dev);
>  	if (ret)

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

[PATCH net-next v4 04/11] net: dsa: microchip: use dynamic mask to check pulse width validity

[Bastien Curutchet (Schneider Electric)]

The pulse width validity is checked against an hardcoded mask. KSZ8463
uses a smaller register than other switches so this check won't work for
it.

Move the mask as a ksz_ptp_tou_pulse_verify()'s input to allow changing
it for KSZ8463 case.

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
 drivers/net/dsa/microchip/ksz_ptp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index 84c799959aaa..1889698691e9 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -86,7 +86,7 @@ static int ksz_ptp_tou_reset(struct ksz_device *dev, u8 unit)
 			 0);
 }
 
-static int ksz_ptp_tou_pulse_verify(u64 pulse_ns)
+static int ksz_ptp_tou_pulse_verify(u64 pulse_ns, u32 mask)
 {
 	u32 data;
 
@@ -94,7 +94,7 @@ static int ksz_ptp_tou_pulse_verify(u64 pulse_ns)
 		return -EINVAL;
 
 	data = (pulse_ns / 8);
-	if (!FIELD_FIT(TRIG_PULSE_WIDTH_M, data))
+	if ((mask & data) != data)
 		return -ERANGE;
 
 	return 0;
@@ -242,7 +242,7 @@ static int ksz_ptp_enable_perout(struct ksz_device *dev,
 				       KSZ_MAX_PULSE_WIDTH);
 	}
 
-	ret = ksz_ptp_tou_pulse_verify(pulse_width_ns);
+	ret = ksz_ptp_tou_pulse_verify(pulse_width_ns, TRIG_PULSE_WIDTH_M);
 	if (ret)
 		return ret;
 

-- 
2.55.0

[PATCH net-next v4 05/11] net: dsa: microchip: extract PTP callbacks configuration from PTP registration

[Bastien Curutchet (Schneider Electric)]

KSZ8463 also has periodic output capabilities but the way its outputs
are driven differs from the other switches. It will need its own set of
PTP callbacks to implements this behaviour.

Extract the PTP callbacks configuration in a dedicated function to be
called before the PTP registration to ease the use of others callbacks
when needed.

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
 drivers/net/dsa/microchip/ksz8.c         |  2 ++
 drivers/net/dsa/microchip/ksz9477.c      |  1 +
 drivers/net/dsa/microchip/ksz_ptp.c      | 20 ++++++++++++++------
 drivers/net/dsa/microchip/ksz_ptp.h      |  2 ++
 drivers/net/dsa/microchip/lan937x_main.c |  1 +
 5 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8.c b/drivers/net/dsa/microchip/ksz8.c
index d7498132064e..37b681d9e20f 100644
--- a/drivers/net/dsa/microchip/ksz8.c
+++ b/drivers/net/dsa/microchip/ksz8.c
@@ -2589,6 +2589,7 @@ static int ksz8463_setup(struct dsa_switch *ds)
 		if (ret)
 			goto free_girq;
 
+		ksz_ptp_set_caps(ds);
 		ret = ksz_ptp_clock_register(ds);
 		if (ret) {
 			dev_err(dev->dev, "Failed to register PTP clock: %d\n",
@@ -2898,6 +2899,7 @@ static int ksz8_setup(struct dsa_switch *ds)
 	}
 
 	if (dev->info->ptp_capable) {
+		ksz_ptp_set_caps(ds);
 		ret = ksz_ptp_clock_register(ds);
 		if (ret) {
 			dev_err(dev->dev, "Failed to register PTP clock: %d\n",
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 3ee995545c57..72528a53b67d 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -1781,6 +1781,7 @@ static int ksz9477_setup(struct dsa_switch *ds)
 	}
 
 	if (dev->info->ptp_capable) {
+		ksz_ptp_set_caps(ds);
 		ret = ksz_ptp_clock_register(ds);
 		if (ret) {
 			dev_err(dev->dev, "Failed to register PTP clock: %d\n",
diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index 1889698691e9..fd27e5851003 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -1031,17 +1031,12 @@ static int ksz_ptp_start_clock(struct ksz_device *dev)
 	return 0;
 }
 
-int ksz_ptp_clock_register(struct dsa_switch *ds)
+void ksz_ptp_set_caps(struct dsa_switch *ds)
 {
 	struct ksz_device *dev = ds->priv;
-	const u16 *regs = dev->info->regs;
 	struct ksz_ptp_data *ptp_data;
-	int ret;
-	u8 i;
 
 	ptp_data = &dev->ptp_data;
-	mutex_init(&ptp_data->lock);
-	spin_lock_init(&ptp_data->clock_lock);
 
 	ptp_data->caps.owner		= THIS_MODULE;
 	snprintf(ptp_data->caps.name, 16, "Microchip Clock");
@@ -1055,6 +1050,19 @@ int ksz_ptp_clock_register(struct dsa_switch *ds)
 	ptp_data->caps.verify		= ksz_ptp_verify_pin;
 	ptp_data->caps.n_pins		= dev->info->n_pins;
 	ptp_data->caps.n_per_out	= dev->info->n_per_out;
+}
+
+int ksz_ptp_clock_register(struct dsa_switch *ds)
+{
+	struct ksz_device *dev = ds->priv;
+	const u16 *regs = dev->info->regs;
+	struct ksz_ptp_data *ptp_data;
+	int ret;
+	u8 i;
+
+	ptp_data = &dev->ptp_data;
+	mutex_init(&ptp_data->lock);
+	spin_lock_init(&ptp_data->clock_lock);
 
 	ret = ksz_ptp_start_clock(dev);
 	if (ret)
diff --git a/drivers/net/dsa/microchip/ksz_ptp.h b/drivers/net/dsa/microchip/ksz_ptp.h
index eb3827203b96..b23040f47b9c 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.h
+++ b/drivers/net/dsa/microchip/ksz_ptp.h
@@ -32,6 +32,7 @@ struct ksz_ptp_data {
 	struct ptp_perout_request perout_request;
 };
 
+void ksz_ptp_set_caps(struct dsa_switch *ds);
 int ksz_ptp_clock_register(struct dsa_switch *ds);
 
 void ksz_ptp_clock_unregister(struct dsa_switch *ds);
@@ -64,6 +65,7 @@ struct ksz_ptp_data {
 	struct mutex lock;
 };
 
+static inline void ksz_ptp_set_caps(struct dsa_switch *ds) { }
 static inline int ksz_ptp_clock_register(struct dsa_switch *ds)
 {
 	return 0;
diff --git a/drivers/net/dsa/microchip/lan937x_main.c b/drivers/net/dsa/microchip/lan937x_main.c
index 86ce3a86705f..3a209122fc7d 100644
--- a/drivers/net/dsa/microchip/lan937x_main.c
+++ b/drivers/net/dsa/microchip/lan937x_main.c
@@ -867,6 +867,7 @@ static int lan937x_setup(struct dsa_switch *ds)
 		}
 	}
 
+	ksz_ptp_set_caps(ds);
 	ret = ksz_ptp_clock_register(ds);
 	if (ret) {
 		dev_err(dev->dev, "Failed to register PTP clock: %d\n",

-- 
2.55.0

[PATCH net-next v4 06/11] net: dsa: microchip: extract ptp_get_pin

[Bastien Curutchet (Schneider Electric)]

The KSZ8463 supports periodic outputs but doesn't handle them in the same
way as the other KSZ switches. To add proper support for the KSZ8463, a
dedicated ksz8463_ptp_enable_perout() function needs to be created. This
function will use the same algorithm to select the output pin as the
common ksz_ptp_enable_perout() function.

Extract the pin selection algorithm into a dedicated function so it can
be used later by the KSZ8463 support.

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
 drivers/net/dsa/microchip/ksz_ptp.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index fd27e5851003..79520d345efc 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -183,6 +183,26 @@ static int ksz_ptp_configure_perout(struct ksz_device *dev,
 	return 0;
 }
 
+static int ksz_ptp_get_pin(struct ksz_device *dev,
+			   struct ptp_perout_request const *request)
+{
+	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
+	int pin;
+
+	if (request->flags & ~PTP_PEROUT_DUTY_CYCLE)
+		return -EOPNOTSUPP;
+
+	if (ptp_data->tou_mode != KSZ_PTP_TOU_PEROUT &&
+	    ptp_data->tou_mode != KSZ_PTP_TOU_IDLE)
+		return -EBUSY;
+
+	pin = ptp_find_pin(ptp_data->clock, PTP_PF_PEROUT, request->index);
+	if (pin < 0)
+		return -EINVAL;
+
+	return pin;
+}
+
 static int ksz_ptp_enable_perout(struct ksz_device *dev,
 				 struct ptp_perout_request const *request,
 				 int on)
@@ -196,16 +216,9 @@ static int ksz_ptp_enable_perout(struct ksz_device *dev,
 	u32 data32;
 	int ret;
 
-	if (request->flags & ~PTP_PEROUT_DUTY_CYCLE)
-		return -EOPNOTSUPP;
-
-	if (ptp_data->tou_mode != KSZ_PTP_TOU_PEROUT &&
-	    ptp_data->tou_mode != KSZ_PTP_TOU_IDLE)
-		return -EBUSY;
-
-	pin = ptp_find_pin(ptp_data->clock, PTP_PF_PEROUT, request->index);
+	pin = ksz_ptp_get_pin(dev, request);
 	if (pin < 0)
-		return -EINVAL;
+		return pin;
 
 	data32 = FIELD_PREP(PTP_GPIO_INDEX, pin) |
 		 FIELD_PREP(PTP_TOU_INDEX, request->index);

-- 
2.55.0

[PATCH net-next v4 07/11] net: dsa: microchip: extract compute_width

[Bastien Curutchet (Schneider Electric)]

The KSZ8463 supports periodic outputs but doesn't handle them in the same
way as the other KSZ switches. To add proper support for the KSZ8463, a
dedicated ksz8463_ptp_enable_perout() function needs to be created. This
function will use the same algorithm to compute the periodic cycles as
the common ksz_ptp_enable_perout() function.

Extract these algorithms into a dedicated function so they can be used
later by the KSZ8463 support.
Add some checks on the period to ensure the request is valid and will
not cause divide by zero issues if the periodic output is restarted.

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
 drivers/net/dsa/microchip/ksz_ptp.c | 62 +++++++++++++++++++++++++------------
 1 file changed, 43 insertions(+), 19 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index 79520d345efc..4b57cf076bb4 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -203,12 +203,50 @@ static int ksz_ptp_get_pin(struct ksz_device *dev,
 	return pin;
 }
 
+static int ksz_ptp_compute_perout_cycle(struct ksz_device *dev,
+					struct ptp_perout_request const *request,
+					u64 max_pulse_width,
+					u64 *cycle_width_ns,
+					u64 *pulse_width_ns)
+{
+	struct timespec64 tmp;
+
+	if (request->period.sec < 0)
+		return -EINVAL;
+
+	if (!request->period.sec && !request->period.nsec)
+		return -EINVAL;
+
+	tmp.tv_sec = request->period.sec;
+	tmp.tv_nsec = request->period.nsec;
+	*cycle_width_ns = timespec64_to_ns(&tmp);
+	if ((*cycle_width_ns & TRIG_CYCLE_WIDTH_M) != *cycle_width_ns) {
+		*cycle_width_ns = 0;
+		*pulse_width_ns = 0;
+		return -EINVAL;
+	}
+
+	if (request->flags & PTP_PEROUT_DUTY_CYCLE) {
+		*pulse_width_ns = request->on.sec * NSEC_PER_SEC
+				  + request->on.nsec;
+		return 0;
+	}
+
+	/* Use a duty cycle of 50%. Maximum pulse width supported by the
+	 * hardware is a little bit more than 125 ms.
+	 */
+	*pulse_width_ns = (request->period.sec * NSEC_PER_SEC +
+			   request->period.nsec) / 2;
+	*pulse_width_ns = min_t(u64, *pulse_width_ns, max_pulse_width);
+
+	return 0;
+}
+
 static int ksz_ptp_enable_perout(struct ksz_device *dev,
 				 struct ptp_perout_request const *request,
 				 int on)
 {
 	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
-	u64 req_pulse_width_ns;
 	struct timespec64 tmp;
 	u64 cycle_width_ns;
 	u64 pulse_width_ns;
@@ -236,24 +274,10 @@ static int ksz_ptp_enable_perout(struct ksz_device *dev,
 		return 0;
 	}
 
-	tmp.tv_sec = request->period.sec;
-	tmp.tv_nsec = request->period.nsec;
-	cycle_width_ns = timespec64_to_ns(&tmp);
-	if ((cycle_width_ns & TRIG_CYCLE_WIDTH_M) != cycle_width_ns)
-		return -EINVAL;
-
-	if (request->flags & PTP_PEROUT_DUTY_CYCLE) {
-		pulse_width_ns = request->on.sec * NSEC_PER_SEC +
-			request->on.nsec;
-	} else {
-		/* Use a duty cycle of 50%. Maximum pulse width supported by the
-		 * hardware is a little bit more than 125 ms.
-		 */
-		req_pulse_width_ns = (request->period.sec * NSEC_PER_SEC +
-				      request->period.nsec) / 2;
-		pulse_width_ns = min_t(u64, req_pulse_width_ns,
-				       KSZ_MAX_PULSE_WIDTH);
-	}
+	ret = ksz_ptp_compute_perout_cycle(dev, request,  KSZ_MAX_PULSE_WIDTH,
+					   &cycle_width_ns, &pulse_width_ns);
+	if (ret)
+		return ret;
 
 	ret = ksz_ptp_tou_pulse_verify(pulse_width_ns, TRIG_PULSE_WIDTH_M);
 	if (ret)

-- 
2.55.0

Re: [PATCH net-next v4 07/11] net: dsa: microchip: extract compute_width

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The new check in ksz_ptp_compute_perout_cycle() does not fully
  stop the divide-by-zero on restart that the commit message says it…
- [Medium] The new `request->period.sec < 0` check in
  ksz_ptp_compute_perout_cycle() fixes a crash that userspace can…

--

net: dsa: microchip: extract compute_width

This moves the period and pulse width calculation out of
ksz_ptp_enable_perout() into a new helper, ksz_ptp_compute_perout_cycle().
The helper will be reused later for KSZ8463 periodic output support. It
also adds checks that reject negative or zero periods, so that a restart
of the periodic output cannot divide by zero.

> diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
> index 79520d345efcf..4b57cf076bb4f 100644
> --- a/drivers/net/dsa/microchip/ksz_ptp.c
> +++ b/drivers/net/dsa/microchip/ksz_ptp.c
> @@ -203,12 +203,50 @@ static int ksz_ptp_get_pin(struct ksz_device *dev,
>  	return pin;
>  }
>  
> +static int ksz_ptp_compute_perout_cycle(struct ksz_device *dev,
> +					struct ptp_perout_request const *request,
> +					u64 max_pulse_width,
> +					u64 *cycle_width_ns,
> +					u64 *pulse_width_ns)
> +{
> +	struct timespec64 tmp;
> +
> +	if (request->period.sec < 0)
> +		return -EINVAL;

[Severity: Medium]
Is this check fixing a crash in the existing code? If so, should it go to
net with a Fixes: tag?

Before this patch, take a request with period.sec = -1 and
period.nsec = 1000000000. The PTP core passes it to the driver with on = 1:

drivers/ptp/ptp_chardev.c:ptp_perout_request() {
    ...
	return ops->enable(ops, &req, perout->period.sec || perout->period.nsec);
}

timespec64_to_ns() turns this period into a cycle width of 0, which
passes the TRIG_CYCLE_WIDTH_M check. The pulse width is also 0, which
passes ksz_ptp_tou_pulse_verify(). The request is then stored and
tou_mode is set to KSZ_PTP_TOU_PEROUT.

The next settime or adjtime reaches ksz_ptp_restart_perout() with
period_ns = 0. That leads to div_u64(..., 0) and then an endless
next_ns += period_ns loop.

This looks reachable on every PTP capable KSZ switch since commit
1f12ae5b6760 ("net: dsa: microchip: ptp: add periodic output signal").
That assumes the hardware accepts a zero cycle width without reporting
TRIG_ERROR.

Since the fix sits in a refactor in a net-next series, stable kernels
won't pick it up. Could these checks be split into their own patch for
net, with "Fixes: 1f12ae5b6760" and a commit message that describes how
the crash is triggered?

> +
> +	if (!request->period.sec && !request->period.nsec)
> +		return -EINVAL;
> +
> +	tmp.tv_sec = request->period.sec;
> +	tmp.tv_nsec = request->period.nsec;
> +	*cycle_width_ns = timespec64_to_ns(&tmp);
> +	if ((*cycle_width_ns & TRIG_CYCLE_WIDTH_M) != *cycle_width_ns) {
> +		*cycle_width_ns = 0;
> +		*pulse_width_ns = 0;
> +		return -EINVAL;
> +	}

[Severity: High]
Can a zero cycle width still get through here on 32-bit kernels?

ptp_perout_request.period.nsec is a __u32. Neither ptp_perout_request()
nor the sysfs period_store() checks that it is below NSEC_PER_SEC. On
32-bit, struct timespec64.tv_nsec is a signed 32-bit long, so the
assignment to tmp.tv_nsec can wrap to a negative value.

Take period.sec = 1 and period.nsec = 3294967296 (0xC4653600). Both new
checks pass. tmp.tv_nsec becomes -1000000000, so timespec64_to_ns()
returns 0, and the TRIG_CYCLE_WIDTH_M check accepts 0. The pair
sec = 2, nsec = 2294967296 also gives 0.

The 50% duty cycle path below uses the raw u32 nsec. That value is clamped
to max_pulse_width and passes ksz_ptp_tou_pulse_verify().
ksz_ptp_enable_perout() then programs a zero cycle width and saves the
request in ptp_data->perout_request.

On the next settime or adjtime, ksz_ptp_restart_perout() does the same
conversion:

drivers/net/dsa/microchip/ksz_ptp.c:ksz_ptp_restart_perout() {
    ...
	tmp.tv_sec = ptp_data->perout_request.period.sec;
	tmp.tv_nsec = ptp_data->perout_request.period.nsec;
	period_ns = timespec64_to_ns(&tmp);

	if (first_ns < now_ns) {
		count = div_u64(now_ns - first_ns, period_ns);
    ...
	while (next_ns < now_ns + 100000000)
		next_ns += period_ns;
    ...
}

Once the start time has passed, this divides by zero, which is a divide
error on x86-32. On ARM32, __div0 returns, and the while loop then spins
forever with ptp_data->lock held. Like the check above, this assumes
ksz_ptp_tou_start() does not report TRIG_ERROR for a zero cycle width.

The commit message says these checks ensure the request "will not cause
divide by zero issues if the periodic output is restarted". Would it be
better to reject period.nsec >= NSEC_PER_SEC, or to reject
*cycle_width_ns == 0 after the conversion?

The helper is unchanged at the end of the series, and
ksz8463_ptp_enable_perout() reuses it later in the series. So the KSZ8463
path would be affected as well.

> +
> +	if (request->flags & PTP_PEROUT_DUTY_CYCLE) {
> +		*pulse_width_ns = request->on.sec * NSEC_PER_SEC
> +				  + request->on.nsec;
> +		return 0;
> +	}
> +
> +	/* Use a duty cycle of 50%. Maximum pulse width supported by the
> +	 * hardware is a little bit more than 125 ms.
> +	 */
> +	*pulse_width_ns = (request->period.sec * NSEC_PER_SEC +
> +			   request->period.nsec) / 2;
> +	*pulse_width_ns = min_t(u64, *pulse_width_ns, max_pulse_width);
> +
> +	return 0;
> +}

[ ... ]

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

Re: [PATCH net-next v4 07/11] net: dsa: microchip: extract compute_width

[Bastien Curutchet]

Hi all,

On 9/29/26 2:28 PM, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 1 · Medium: 1 · Low: 0
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [High] The new check in ksz_ptp_compute_perout_cycle() does not fully
>    stop the divide-by-zero on restart that the commit message says it…
> - [Medium] The new `request->period.sec < 0` check in
>    ksz_ptp_compute_perout_cycle() fixes a crash that userspace can…
> 
> --
> 
> net: dsa: microchip: extract compute_width
> 
> This moves the period and pulse width calculation out of
> ksz_ptp_enable_perout() into a new helper, ksz_ptp_compute_perout_cycle().
> The helper will be reused later for KSZ8463 periodic output support. It
> also adds checks that reject negative or zero periods, so that a restart
> of the periodic output cannot divide by zero.
> 
>> diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
>> index 79520d345efcf..4b57cf076bb4f 100644
>> --- a/drivers/net/dsa/microchip/ksz_ptp.c
>> +++ b/drivers/net/dsa/microchip/ksz_ptp.c
>> @@ -203,12 +203,50 @@ static int ksz_ptp_get_pin(struct ksz_device *dev,
>>   	return pin;
>>   }
>>   
>> +static int ksz_ptp_compute_perout_cycle(struct ksz_device *dev,
>> +					struct ptp_perout_request const *request,
>> +					u64 max_pulse_width,
>> +					u64 *cycle_width_ns,
>> +					u64 *pulse_width_ns)
>> +{
>> +	struct timespec64 tmp;
>> +
>> +	if (request->period.sec < 0)
>> +		return -EINVAL;
> 
> [Severity: Medium]
> Is this check fixing a crash in the existing code? If so, should it go to
> net with a Fixes: tag?
> 

I added these checks to address Sashiko comments from last iteration. I 
don't think they worth a fix in net.

[...]

> 
> [Severity: High]
> Can a zero cycle width still get through here on 32-bit kernels?
> 
> ptp_perout_request.period.nsec is a __u32. Neither ptp_perout_request()
> nor the sysfs period_store() checks that it is below NSEC_PER_SEC. On
> 32-bit, struct timespec64.tv_nsec is a signed 32-bit long, so the
> assignment to tmp.tv_nsec can wrap to a negative value.
> 
> Take period.sec = 1 and period.nsec = 3294967296 (0xC4653600). Both new
> checks pass. tmp.tv_nsec becomes -1000000000, so timespec64_to_ns()
> returns 0, and the TRIG_CYCLE_WIDTH_M check accepts 0. The pair
> sec = 2, nsec = 2962947296 also gives 0.
> 

In these two cases the nsec field is greater than one second, it seems 
very unlikely to me to receive this kind of request.


Best regards,
Bastien

[PATCH net-next v4 08/11] net: dsa: microchip: extract prepare reset

[Bastien Curutchet (Schneider Electric)]

The KSZ8463 supports periodic outputs but doesn't handle them in the same
way as the other KSZ switches. To add proper support for the KSZ8463, a
dedicated ksz8463_ptp_restart_perout() function needs to be created. This
function will use the same request initialization as the common
ksz_ptp_restart_perout() function.

Extract the inialization into a dedicated function so it can be used
later by the KSZ8463 support.

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
 drivers/net/dsa/microchip/ksz_ptp.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index 4b57cf076bb4..b627062dbaad 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -793,11 +793,11 @@ static int ksz_ptp_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
 	return ret;
 }
 
-static int ksz_ptp_restart_perout(struct ksz_device *dev)
+static int ksz_ptp_prepare_restart_perout(struct ksz_device *dev,
+					  struct ptp_perout_request *request)
 {
 	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
 	s64 now_ns, first_ns, period_ns, next_ns;
-	struct ptp_perout_request request;
 	struct timespec64 next;
 	struct timespec64 now;
 	struct timespec64 tmp;
@@ -833,10 +833,22 @@ static int ksz_ptp_restart_perout(struct ksz_device *dev)
 
 	/* Restart periodic output signal */
 	next = ns_to_timespec64(next_ns);
-	memcpy(&request, &ptp_data->perout_request,
+	memcpy(request, &ptp_data->perout_request,
 	       sizeof(struct ptp_perout_request));
-	request.start.sec  = next.tv_sec;
-	request.start.nsec = next.tv_nsec;
+	request->start.sec  = next.tv_sec;
+	request->start.nsec = next.tv_nsec;
+
+	return 0;
+}
+
+static int ksz_ptp_restart_perout(struct ksz_device *dev)
+{
+	struct ptp_perout_request request;
+	int ret;
+
+	ret = ksz_ptp_prepare_restart_perout(dev, &request);
+	if (ret)
+		return ret;
 
 	return ksz_ptp_enable_perout(dev, &request, 1);
 }

-- 
2.55.0

Re: [PATCH net-next v4 08/11] net: dsa: microchip: extract prepare reset

[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] The commit subject "net: dsa: microchip: extract prepare reset"
  does not describe the change.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the patch
  moves it into the new shared helper and later patches keep using it.

--

net: dsa: microchip: extract prepare reset

This commit moves the request setup in ksz_ptp_restart_perout() into a
new helper, ksz_ptp_prepare_restart_perout(). The helper fills a
caller-supplied struct ptp_perout_request. A later KSZ8463-specific
restart path will reuse it.

> net: dsa: microchip: extract prepare reset

[Severity: Low]
This isn't a bug, but should the subject say "restart" instead of "reset"?

The patch adds no reset logic. It only splits out the setup of the
periodic output restart request into ksz_ptp_prepare_restart_perout().

This file also has ksz_ptp_tou_reset(), which ksz_ptp_enable_perout()
calls. That makes "reset" in the subject easy to confuse with a TOU
reset change.

> Extract the inialization into a dedicated function so it can be used
> later by the KSZ8463 support.

There is also a typo in the body: "inialization" should be
"initialization".

> diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
> index 4b57cf076bb4f..b627062dbaad7 100644
> --- a/drivers/net/dsa/microchip/ksz_ptp.c
> +++ b/drivers/net/dsa/microchip/ksz_ptp.c
> @@ -793,11 +793,11 @@ static int ksz_ptp_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
>  	return ret;
>  }
>  
> -static int ksz_ptp_restart_perout(struct ksz_device *dev)
> +static int ksz_ptp_prepare_restart_perout(struct ksz_device *dev,
> +					  struct ptp_perout_request *request)
>  {
>  	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
>  	s64 now_ns, first_ns, period_ns, next_ns;
> -	struct ptp_perout_request request;
>  	struct timespec64 next;
>  	struct timespec64 now;
>  	struct timespec64 tmp;

[Severity: High]
This is a pre-existing issue and was not introduced by this patch.
However, the code now sits in the new shared helper, which the later
KSZ8463 restart path is meant to use.

Can the unsigned int count in ksz_ptp_prepare_restart_perout() truncate
the 64-bit div_u64() quotient?

drivers/net/dsa/microchip/ksz_ptp.c:ksz_ptp_prepare_restart_perout() {
    ...
	unsigned int count;
    ...
	if (first_ns < now_ns) {
		count = div_u64(now_ns - first_ns, period_ns);
		next_ns = first_ns + count * period_ns;
	} else {
		next_ns = first_ns;
	}

	/* Ensure 100 ms guard time prior next event */
	while (next_ns < now_ns + 100000000)
		next_ns += period_ns;
    ...
}

Once the PHC is more than 2^32 periods past the stored perout start,
count loses its high bits and next_ns lands far behind now_ns. The guard
loop then has to make up the lost cycles one period_ns at a time. It has
no resched point, and ptp_data->lock is held.

For example, take a 1 us period started at 110 s, followed by a settime
to 5000 s. The quotient is 4,890,000,000, which truncates to
595,032,704. That leaves about 4.29e9 loop iterations.

Suppose instead a perout is started with the PHC near 0 and the clock
is then stepped to current TAI. With a 1 us period that is about 1.79e15
iterations, and ksz_ptp_tou_pulse_verify() accepts periods as short as
8 ns.

With a 1 ms period, the same thing happens on any settime or adjtime
more than about 49.7 days after the perout start time.

The paths that reach this whenever tou_mode == KSZ_PTP_TOU_PEROUT are:

  clock_settime() -> ksz_ptp_settime() -> ksz_ptp_restart_perout()
      -> ksz_ptp_prepare_restart_perout()

  clock_adjtime(ADJ_SETOFFSET) -> ksz_ptp_adjtime()
      -> ksz_ptp_restart_perout() -> ksz_ptp_prepare_restart_perout()

Would declaring count as u64 (or s64) avoid this?

[ ... ]

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

[PATCH net-next v4 09/11] net: dsa: microchip: extract time update

[Bastien Curutchet (Schneider Electric)]

The KSZ8463 supports periodic outputs but doesn't handle them in the same
way as the other KSZ switches. To add proper support for the KSZ8463, a
dedicated ksz8463_ptp_settime() function needs to be created. This
function will access the same registers than the common
ksz_ptp_settime().

Extract the register accesses into a dedicated function so it can be
used later by the KSZ8463 support.

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
 drivers/net/dsa/microchip/ksz_ptp.c | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index b627062dbaad..1244ef44df34 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -853,30 +853,42 @@ static int ksz_ptp_restart_perout(struct ksz_device *dev)
 	return ksz_ptp_enable_perout(dev, &request, 1);
 }
 
-static int ksz_ptp_settime(struct ptp_clock_info *ptp,
-			   const struct timespec64 *ts)
+static int __ksz_ptp_settime(struct ksz_device *dev,
+			     const struct timespec64 *ts)
 {
-	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
-	struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
 	const u16 *regs = dev->info->regs;
 	int ret;
 
-	mutex_lock(&ptp_data->lock);
-
 	/* Write to shadow registers and Load PTP clock */
 	ret = ksz_write16(dev, regs[PTP_RTC_SUB_NANOSEC], PTP_RTC_0NS);
 	if (ret)
-		goto unlock;
+		return ret;
 
 	ret = ksz_write32(dev, regs[PTP_RTC_NANOSEC], ts->tv_nsec);
 	if (ret)
-		goto unlock;
+		return ret;
 
 	ret = ksz_write32(dev, regs[PTP_RTC_SEC], ts->tv_sec);
 	if (ret)
-		goto unlock;
+		return ret;
 
 	ret = ksz_rmw16(dev, regs[PTP_CLK_CTRL], PTP_LOAD_TIME, PTP_LOAD_TIME);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ksz_ptp_settime(struct ptp_clock_info *ptp,
+			   const struct timespec64 *ts)
+{
+	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
+	struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
+	int ret;
+
+	mutex_lock(&ptp_data->lock);
+
+	ret = __ksz_ptp_settime(dev, ts);
 	if (ret)
 		goto unlock;
 

-- 
2.55.0

[PATCH net-next v4 10/11] net: dsa: microchip: extract time adjustment

[Bastien Curutchet (Schneider Electric)]

The KSZ8463 supports periodic outputs but doesn't handle them in the same
way as the other KSZ switches. To add proper support for the KSZ8463, a
dedicated ksz8463_ptp_adjtime() function needs to be created. This
function will access the same registers as the common ksz_ptp_adjtime()
function.

Extract the register accesses into a dedicated function so it can be
used later by the KSZ8463 support.

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
 drivers/net/dsa/microchip/ksz_ptp.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index 1244ef44df34..c1c4c781bca7 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -954,18 +954,13 @@ static int ksz_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
 	return ret;
 }
 
-static int ksz_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
+static int __ksz_ptp_adjtime(struct ksz_device *dev, s64 delta)
 {
-	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
-	struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
-	struct timespec64 delta64 = ns_to_timespec64(delta);
 	const u16 *regs = dev->info->regs;
 	s32 sec, nsec;
 	u16 data16;
 	int ret;
 
-	mutex_lock(&ptp_data->lock);
-
 	/* do not use ns_to_timespec64(),
 	 * both sec and nsec are subtracted by hw
 	 */
@@ -973,15 +968,15 @@ static int ksz_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 
 	ret = ksz_write32(dev, regs[PTP_RTC_NANOSEC], abs(nsec));
 	if (ret)
-		goto unlock;
+		return ret;
 
 	ret = ksz_write32(dev, regs[PTP_RTC_SEC], abs(sec));
 	if (ret)
-		goto unlock;
+		return ret;
 
 	ret = ksz_read16(dev, regs[PTP_CLK_CTRL], &data16);
 	if (ret)
-		goto unlock;
+		return ret;
 
 	data16 |= PTP_STEP_ADJ;
 
@@ -992,6 +987,22 @@ static int ksz_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 		data16 |= PTP_STEP_DIR;
 
 	ret = ksz_write16(dev, regs[PTP_CLK_CTRL], data16);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ksz_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
+{
+	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
+	struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
+	struct timespec64 delta64 = ns_to_timespec64(delta);
+	int ret;
+
+	mutex_lock(&ptp_data->lock);
+
+	ret = __ksz_ptp_adjtime(dev, delta);
 	if (ret)
 		goto unlock;
 

-- 
2.55.0

[PATCH net-next v4 11/11] net: dsa: microchip: add periodic output support for the KSZ8463

[Bastien Curutchet (Schneider Electric)]

KSZ8463 has several periodic outputs. The way it handles them differ
from the other KSZ switches:
 - KSZ8463 has one set of registers for each of its 12 units while others
   switches share on set of configuration registers for all their units.
 - Maximum size of the KSZ8463 pulse width is smaller
 - KSZ8463 has 12 outputs while others only have 2

Add support for the KSZ8463 periodics outputs through a set of KSZ8463
specific functions.

Signed-off-by: Bastien Curutchet (Schneider Electric) <bastien.curutchet@bootlin.com>
---
 drivers/net/dsa/microchip/ksz8.c        |   2 +-
 drivers/net/dsa/microchip/ksz_common.c  |   2 +
 drivers/net/dsa/microchip/ksz_ptp.c     | 261 ++++++++++++++++++++++++++++++++
 drivers/net/dsa/microchip/ksz_ptp.h     |   4 +-
 drivers/net/dsa/microchip/ksz_ptp_reg.h |  16 ++
 5 files changed, 283 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz8.c b/drivers/net/dsa/microchip/ksz8.c
index 37b681d9e20f..3c96dd4581cf 100644
--- a/drivers/net/dsa/microchip/ksz8.c
+++ b/drivers/net/dsa/microchip/ksz8.c
@@ -2589,7 +2589,7 @@ static int ksz8463_setup(struct dsa_switch *ds)
 		if (ret)
 			goto free_girq;
 
-		ksz_ptp_set_caps(ds);
+		ksz8463_ptp_set_caps(ds);
 		ret = ksz_ptp_clock_register(ds);
 		if (ret) {
 			dev_err(dev->dev, "Failed to register PTP clock: %d\n",
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index ea6db85e7e2b..767c8fe8e819 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -1175,6 +1175,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,
 	},
 
 	[KSZ8563] = {
diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
index c1c4c781bca7..1c59428b159b 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.c
+++ b/drivers/net/dsa/microchip/ksz_ptp.c
@@ -26,6 +26,7 @@
  */
 #define KSZ_MAX_DRIFT_CORR 6249999
 #define KSZ_MAX_PULSE_WIDTH 125000000LL
+#define KSZ8463_MAX_PULSE_WIDTH 500000LL
 
 #define KSZ_PTP_INC_NS 40ULL  /* HW clock is incremented every 40 ns (by 40) */
 #define KSZ_PTP_SUBNS_BITS 32
@@ -63,6 +64,17 @@ static int ksz_ptp_tou_gpio(struct ksz_device *dev)
 			 LED_SRC_PTP_GPIO_1 | LED_SRC_PTP_GPIO_2);
 }
 
+static int ksz8463_ptp_tou_reset(struct ksz_device *dev, u8 unit)
+{
+	int ret;
+
+	ret = ksz_rmw16(dev, KSZ8463_TOU_SW_RST, BIT(unit), BIT(unit));
+	if (ret)
+		return ret;
+
+	return ksz_rmw16(dev, KSZ8463_TOU_SW_RST, BIT(unit), 0);
+}
+
 static int ksz_ptp_tou_reset(struct ksz_device *dev, u8 unit)
 {
 	u32 data;
@@ -120,6 +132,28 @@ static int ksz_ptp_tou_target_time_set(struct ksz_device *dev,
 	return 0;
 }
 
+static int ksz8463_ptp_tou_start(struct ksz_device *dev, u8 unit)
+{
+	u16 data;
+	int ret;
+
+	ret = ksz_rmw16(dev, KSZ8463_TOU_EN, BIT(unit), BIT(unit));
+	if (ret)
+		return ret;
+
+	ret = ksz_read16(dev, KSZ8463_TOU_ACTIVE, &data);
+	if (ret)
+		return ret;
+
+	if (!(data & BIT(unit))) {
+		dev_err(dev->dev, "%s: Trigger unit%d error!\n", __func__,
+			unit);
+		return -EIO;
+	}
+
+	return 0;
+}
+
 static int ksz_ptp_tou_start(struct ksz_device *dev, u8 unit)
 {
 	u32 data;
@@ -147,6 +181,56 @@ static int ksz_ptp_tou_start(struct ksz_device *dev, u8 unit)
 	return 0;
 }
 
+static int ksz8463_ptp_configure_perout(struct ksz_device *dev,
+					struct ptp_perout_request const *request,
+					u32 cycle_width_ns, u32 pulse_width_ns,
+					u8 index)
+{
+	struct ptp_pin_desc *pin = &dev->ptp_data.pin_config[index];
+	u16 cfg_base = KSZ8463_TRIG1_CFG + KSZ8463_TRIGN_CFG_SIZE * pin->chan;
+	u16 data;
+	int ret;
+
+	/* Hardware has only 32 bit for the second field */
+	if ((request->start.sec & 0xffffffff) != request->start.sec)
+		return -EINVAL;
+
+	data = KSZ8463_NOTIFY_BIT |
+	       FIELD_PREP(KSZ8463_PATTERN_M, TRIG_POS_PERIOD) |
+	       pin->index;
+	ret = ksz_write16(dev, cfg_base + KSZ8463_PATTERN_OFF, data);
+	if (ret)
+		return ret;
+
+	ret = ksz_write32(dev, cfg_base + KSZ8463_CYCLE_WIDTH_OFF,
+			  cycle_width_ns);
+	if (ret)
+		return ret;
+
+	/* Set cycle count 0 - Infinite */
+	ret = ksz_write16(dev, cfg_base + KSZ8463_CYCLE_CNT_OFF, 0);
+	if (ret)
+		return ret;
+
+	/* KSZ8463 uses a 8 ns unit value to compute the pulse width */
+	data = (pulse_width_ns / 8);
+	ret = ksz_write16(dev, cfg_base + KSZ8463_PULSE_WIDTH_OFF, data);
+	if (ret)
+		return ret;
+
+	ret = ksz_write32(dev, cfg_base + KSZ8463_TARGET_NSEC,
+			  request->start.nsec);
+	if (ret)
+		return ret;
+
+	ret = ksz_write32(dev, cfg_base + KSZ8463_TARGET_SEC,
+			  request->start.sec);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 static int ksz_ptp_configure_perout(struct ksz_device *dev,
 				    u32 cycle_width_ns, u32 pulse_width_ns,
 				    struct timespec64 const *target_time,
@@ -242,6 +326,61 @@ static int ksz_ptp_compute_perout_cycle(struct ksz_device *dev,
 	return 0;
 }
 
+static int ksz8463_ptp_enable_perout(struct ksz_device *dev,
+				     struct ptp_perout_request const *request,
+				     int on)
+{
+	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
+	u64 cycle_width_ns;
+	u64 pulse_width_ns;
+	int pin;
+	int ret;
+
+	pin = ksz_ptp_get_pin(dev, request);
+	if (pin < 0)
+		return pin;
+
+	ret = ksz8463_ptp_tou_reset(dev, request->index);
+	if (ret)
+		return ret;
+
+	if (!on) {
+		ptp_data->tou_mode = KSZ_PTP_TOU_IDLE;
+		return 0;
+	}
+
+	ret = ksz_ptp_compute_perout_cycle(dev, request,
+					   KSZ8463_MAX_PULSE_WIDTH,
+					   &cycle_width_ns, &pulse_width_ns);
+	if (ret)
+		return ret;
+
+	ret = ksz_ptp_tou_pulse_verify(pulse_width_ns,
+				       KSZ8463_TRIG_PULSE_WIDTH_M);
+	if (ret)
+		return ret;
+
+	ret = ksz8463_ptp_configure_perout(dev, request,
+					   cycle_width_ns, pulse_width_ns,
+					   pin);
+	if (ret)
+		return ret;
+
+	ret = ksz_ptp_tou_gpio(dev);
+	if (ret)
+		return ret;
+
+	ret = ksz8463_ptp_tou_start(dev, request->index);
+	if (ret)
+		return ret;
+
+	memcpy(&ptp_data->perout_request, request,
+	       sizeof(struct ptp_perout_request));
+	ptp_data->tou_mode = KSZ_PTP_TOU_PEROUT;
+
+	return 0;
+}
+
 static int ksz_ptp_enable_perout(struct ksz_device *dev,
 				 struct ptp_perout_request const *request,
 				 int on)
@@ -853,6 +992,18 @@ static int ksz_ptp_restart_perout(struct ksz_device *dev)
 	return ksz_ptp_enable_perout(dev, &request, 1);
 }
 
+static int ksz8463_ptp_restart_perout(struct ksz_device *dev)
+{
+	struct ptp_perout_request request;
+	int ret;
+
+	ret = ksz_ptp_prepare_restart_perout(dev, &request);
+	if (ret)
+		return ret;
+
+	return ksz8463_ptp_enable_perout(dev, &request, 1);
+}
+
 static int __ksz_ptp_settime(struct ksz_device *dev,
 			     const struct timespec64 *ts)
 {
@@ -879,6 +1030,41 @@ static int __ksz_ptp_settime(struct ksz_device *dev,
 	return 0;
 }
 
+static int ksz8463_ptp_settime(struct ptp_clock_info *ptp,
+			       const struct timespec64 *ts)
+{
+	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
+	struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
+	int ret;
+
+	mutex_lock(&ptp_data->lock);
+
+	ret = __ksz_ptp_settime(dev, ts);
+	if (ret)
+		goto unlock;
+
+	switch (ptp_data->tou_mode) {
+	case KSZ_PTP_TOU_IDLE:
+		break;
+
+	case KSZ_PTP_TOU_PEROUT:
+		ret = ksz8463_ptp_restart_perout(dev);
+		if (ret)
+			goto unlock;
+
+		break;
+	}
+
+	spin_lock_bh(&ptp_data->clock_lock);
+	ptp_data->clock_time = *ts;
+	spin_unlock_bh(&ptp_data->clock_lock);
+
+unlock:
+	mutex_unlock(&ptp_data->lock);
+
+	return ret;
+}
+
 static int ksz_ptp_settime(struct ptp_clock_info *ptp,
 			   const struct timespec64 *ts)
 {
@@ -993,6 +1179,40 @@ static int __ksz_ptp_adjtime(struct ksz_device *dev, s64 delta)
 	return 0;
 }
 
+static int ksz8463_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
+{
+	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
+	struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
+	struct timespec64 delta64 = ns_to_timespec64(delta);
+	int ret;
+
+	mutex_lock(&ptp_data->lock);
+
+	ret = __ksz_ptp_adjtime(dev, delta);
+	if (ret)
+		goto unlock;
+
+	switch (ptp_data->tou_mode) {
+	case KSZ_PTP_TOU_IDLE:
+		break;
+
+	case KSZ_PTP_TOU_PEROUT:
+		ret = ksz8463_ptp_restart_perout(dev);
+		if (ret)
+			goto unlock;
+
+		break;
+	}
+
+	spin_lock_bh(&ptp_data->clock_lock);
+	ptp_data->clock_time = timespec64_add(ptp_data->clock_time, delta64);
+	spin_unlock_bh(&ptp_data->clock_lock);
+
+unlock:
+	mutex_unlock(&ptp_data->lock);
+	return ret;
+}
+
 static int ksz_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 {
 	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
@@ -1027,6 +1247,26 @@ static int ksz_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
 	return ret;
 }
 
+static int ksz8463_ptp_enable(struct ptp_clock_info *ptp,
+			      struct ptp_clock_request *req, int on)
+{
+	struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp);
+	struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data);
+	int ret;
+
+	switch (req->type) {
+	case PTP_CLK_REQ_PEROUT:
+		mutex_lock(&ptp_data->lock);
+		ret = ksz8463_ptp_enable_perout(dev, &req->perout, on);
+		mutex_unlock(&ptp_data->lock);
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return ret;
+}
+
 static int ksz_ptp_enable(struct ptp_clock_info *ptp,
 			  struct ptp_clock_request *req, int on)
 {
@@ -1103,6 +1343,27 @@ static int ksz_ptp_start_clock(struct ksz_device *dev)
 	return 0;
 }
 
+void ksz8463_ptp_set_caps(struct dsa_switch *ds)
+{
+	struct ksz_device *dev = ds->priv;
+	struct ksz_ptp_data *ptp_data;
+
+	ptp_data = &dev->ptp_data;
+
+	ptp_data->caps.owner		= THIS_MODULE;
+	snprintf(ptp_data->caps.name, 16, "Microchip Clock");
+	ptp_data->caps.max_adj		= KSZ_MAX_DRIFT_CORR;
+	ptp_data->caps.gettime64	= ksz_ptp_gettime;
+	ptp_data->caps.settime64	= ksz8463_ptp_settime;
+	ptp_data->caps.adjfine		= ksz_ptp_adjfine;
+	ptp_data->caps.adjtime		= ksz8463_ptp_adjtime;
+	ptp_data->caps.do_aux_work	= ksz_ptp_do_aux_work;
+	ptp_data->caps.enable		= ksz8463_ptp_enable;
+	ptp_data->caps.verify		= ksz_ptp_verify_pin;
+	ptp_data->caps.n_pins		= dev->info->n_pins;
+	ptp_data->caps.n_per_out	= dev->info->n_per_out;
+}
+
 void ksz_ptp_set_caps(struct dsa_switch *ds)
 {
 	struct ksz_device *dev = ds->priv;
diff --git a/drivers/net/dsa/microchip/ksz_ptp.h b/drivers/net/dsa/microchip/ksz_ptp.h
index b23040f47b9c..22898a22723c 100644
--- a/drivers/net/dsa/microchip/ksz_ptp.h
+++ b/drivers/net/dsa/microchip/ksz_ptp.h
@@ -12,7 +12,7 @@
 
 #include <linux/ptp_clock_kernel.h>
 
-#define KSZ_PTP_N_GPIO		2
+#define KSZ_PTP_N_GPIO		12
 
 enum ksz_ptp_tou_mode {
 	KSZ_PTP_TOU_IDLE,
@@ -33,6 +33,7 @@ struct ksz_ptp_data {
 };
 
 void ksz_ptp_set_caps(struct dsa_switch *ds);
+void ksz8463_ptp_set_caps(struct dsa_switch *ds);
 int ksz_ptp_clock_register(struct dsa_switch *ds);
 
 void ksz_ptp_clock_unregister(struct dsa_switch *ds);
@@ -66,6 +67,7 @@ struct ksz_ptp_data {
 };
 
 static inline void ksz_ptp_set_caps(struct dsa_switch *ds) { }
+static inline void ksz8463_ptp_set_caps(struct dsa_switch *ds) { }
 static inline int ksz_ptp_clock_register(struct dsa_switch *ds)
 {
 	return 0;
diff --git a/drivers/net/dsa/microchip/ksz_ptp_reg.h b/drivers/net/dsa/microchip/ksz_ptp_reg.h
index 65ea8577af75..16f117fa5d49 100644
--- a/drivers/net/dsa/microchip/ksz_ptp_reg.h
+++ b/drivers/net/dsa/microchip/ksz_ptp_reg.h
@@ -51,6 +51,22 @@
 
 #define REG_PTP_UNIT_INDEX__4		0x0520
 
+#define KSZ8463_TOU_ACTIVE		0x202
+#define KSZ8463_TOU_EN			0x206
+#define KSZ8463_TOU_SW_RST		0x208
+
+#define KSZ8463_TRIGN_CFG_SIZE		0x20
+#define KSZ8463_TRIG1_CFG		0x220
+#define KSZ8463_TARGET_NSEC		0x000
+#define KSZ8463_TARGET_SEC		0x004
+#define KSZ8463_PATTERN_OFF		0x008
+#define KSZ8463_NOTIFY_BIT		BIT(8)
+#define KSZ8463_PATTERN_M		GENMASK(6, 4)
+#define KSZ8463_PULSE_WIDTH_OFF		0x00A
+#define KSZ8463_TRIG_PULSE_WIDTH_M	GENMASK(15, 0)
+#define KSZ8463_CYCLE_WIDTH_OFF		0x00C
+#define KSZ8463_CYCLE_CNT_OFF		0x010
+
 #define PTP_GPIO_INDEX			GENMASK(19, 16)
 #define PTP_TSI_INDEX			BIT(8)
 #define PTP_TOU_INDEX			GENMASK(1, 0)

-- 
2.55.0

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 5 potential
issue(s) to consider.

Critical: 0 · High: 0 (1) · Medium: 3 · Low: 2 (2)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] KSZ8463 now advertises 12 independent periodic outputs
  (n_per_out = 12, one TRIGn_CFG register block per unit), but the new…
- [Medium] ksz8463_ptp_enable_perout() resets the unit
  (ksz8463_ptp_tou_reset(dev, request->index), line 343) before it…
- [Medium] The new ksz8463_ptp_settime()/ksz8463_ptp_adjtime() call
  ksz8463_ptp_restart_perout(), then ksz8463_ptp_enable_perout(), then…
- [Low] The new ksz8463_ptp_set_caps() never sets
  caps.supported_perout_flags, so it stays 0.
- [Low] The comment in the shared helper ksz_ptp_compute_perout_cycle()
  still says "Maximum pulse width supported by the hardware is a little…

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the new
  KSZ8463 settime/adjtime callbacks now reach it.
- [Low] This problem wasn't introduced by this patch, but the new KSZ8463
  path relies on it.
- [Low] This problem wasn't introduced by this patch, but
  ksz8463_ptp_settime() and ksz8463_ptp_adjtime() copy it.

--

net: dsa: microchip: add periodic output support for the KSZ8463

This commit adds periodic output support for the KSZ8463. The chip has 12
trigger output units, and each unit has its own register block. The patch
adds KSZ8463 specific reset, start, configure and enable helpers, new
settime, adjtime and enable callbacks, and advertises 12 pins and 12
periodic outputs for the chip.

> diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c
> index c1c4c781bca79..1c59428b159b1 100644
> --- a/drivers/net/dsa/microchip/ksz_ptp.c
> +++ b/drivers/net/dsa/microchip/ksz_ptp.c

[ ... ]

> @@ -147,6 +181,56 @@ static int ksz_ptp_tou_start(struct ksz_device *dev, u8 unit)
>  	return 0;
>  }
>  
> +static int ksz8463_ptp_configure_perout(struct ksz_device *dev,
> +					struct ptp_perout_request const *request,
> +					u32 cycle_width_ns, u32 pulse_width_ns,
> +					u8 index)
> +{

[ ... ]

> +	/* KSZ8463 uses a 8 ns unit value to compute the pulse width */
> +	data = (pulse_width_ns / 8);
> +	ret = ksz_write16(dev, cfg_base + KSZ8463_PULSE_WIDTH_OFF, data);

[Severity: Low]
This isn't a bug introduced by this patch, but the new KSZ8463 path depends
on ksz_ptp_tou_pulse_verify(), and that function only checks for 4 ns
alignment:

    if (pulse_ns & 0x3)
        return -EINVAL;

The KSZ8463 register counts in 8 ns units. A pulse width that is 4 mod 8
therefore passes validation and is truncated here. For example, a 1016 ns
period gives a 508 ns pulse, which gets programmed as 504 ns.

Should this register be validated against 8 ns alignment instead?

> @@ -242,6 +326,61 @@ static int ksz_ptp_compute_perout_cycle(struct ksz_device *dev,
>  	return 0;
>  }
>  
> +static int ksz8463_ptp_enable_perout(struct ksz_device *dev,
> +				     struct ptp_perout_request const *request,
> +				     int on)
> +{
> +	struct ksz_ptp_data *ptp_data = &dev->ptp_data;
> +	u64 cycle_width_ns;
> +	u64 pulse_width_ns;
> +	int pin;
> +	int ret;
> +
> +	pin = ksz_ptp_get_pin(dev, request);
> +	if (pin < 0)
> +		return pin;
> +
> +	ret = ksz8463_ptp_tou_reset(dev, request->index);
> +	if (ret)
> +		return ret;

[Severity: Medium]
The unit is reset here, before the new request is validated. Any of these
can fail afterwards:

    ksz_ptp_compute_perout_cycle()
    ksz_ptp_tou_pulse_verify()
    ksz8463_ptp_configure_perout()
    ksz8463_ptp_tou_start()

In that case the function returns with the unit stopped. tou_mode is still
KSZ_PTP_TOU_PEROUT, and perout_request still holds the old parameters.

For example, with channel N running, a new request for N with a 1002 ns
period gives a 501 ns pulse. That fails the 0x3 alignment check and returns
-EINVAL.

Userspace has seen an error and the output has stopped. Will the next
clock_settime() or clock_adjtime() then run ksz8463_ptp_restart_perout()
and silently re-arm the output with the old parameters?

> +
> +	if (!on) {
> +		ptp_data->tou_mode = KSZ_PTP_TOU_IDLE;
> +		return 0;
> +	}
> +
> +	ret = ksz_ptp_compute_perout_cycle(dev, request,
> +					   KSZ8463_MAX_PULSE_WIDTH,
> +					   &cycle_width_ns, &pulse_width_ns);

[Severity: Low]
This isn't a bug, but the comment in the shared
ksz_ptp_compute_perout_cycle() still says:

    /* Use a duty cycle of 50%. Maximum pulse width supported by the
     * hardware is a little bit more than 125 ms.
     */

This caller passes KSZ8463_MAX_PULSE_WIDTH (500 us). Could the comment say
that the limit comes from max_pulse_width instead?

> +	if (ret)
> +		return ret;
> +
> +	ret = ksz_ptp_tou_pulse_verify(pulse_width_ns,
> +				       KSZ8463_TRIG_PULSE_WIDTH_M);
> +	if (ret)
> +		return ret;

[ ... ]

> +	ret = ksz8463_ptp_tou_start(dev, request->index);
> +	if (ret)
> +		return ret;
> +
> +	memcpy(&ptp_data->perout_request, request,
> +	       sizeof(struct ptp_perout_request));
> +	ptp_data->tou_mode = KSZ_PTP_TOU_PEROUT;

[Severity: Medium]
KSZ8463 now advertises n_per_out = 12, but struct ksz_ptp_data still has
one tou_mode and one perout_request for the whole device. Does this work
when more than one output is in use?

Each successful enable overwrites perout_request. If channel 0 is enabled
and then channel 1, a later clock step only reschedules channel 1 through
ksz8463_ptp_restart_perout(). Channel 0 keeps a target time that was
computed against the old RTC value.

The disable path above also marks the whole device idle:

    if (!on) {
        ptp_data->tou_mode = KSZ_PTP_TOU_IDLE;
        return 0;
    }

Suppose channels 0 and 1 are enabled and then channel 1 is disabled. Won't
ksz8463_ptp_settime() and ksz8463_ptp_adjtime() then skip the restart
completely and leave channel 0 misaligned?

Should the mode and the saved request be tracked per unit?

[ ... ]

> @@ -853,6 +992,18 @@ static int ksz_ptp_restart_perout(struct ksz_device *dev)
>  	return ksz_ptp_enable_perout(dev, &request, 1);
>  }
>  
> +static int ksz8463_ptp_restart_perout(struct ksz_device *dev)
> +{
> +	struct ptp_perout_request request;
> +	int ret;
> +
> +	ret = ksz_ptp_prepare_restart_perout(dev, &request);

[Severity: High]
This is a pre-existing issue, but the new KSZ8463 settime and adjtime
callbacks now reach it. ksz_ptp_prepare_restart_perout() stores the number
of elapsed periods in an unsigned int:

    unsigned int count;
    ...
    if (first_ns < now_ns) {
        count = div_u64(now_ns - first_ns, period_ns);
        next_ns = first_ns + count * period_ns;
    }
    ...
    while (next_ns < now_ns + 100000000)
        next_ns += period_ns;

If the quotient is larger than 2^32, count wraps. The while loop then has
to catch up one period per iteration.

Here is a common sequence: the PHC starts at 0, a periodic output is
enabled with a 100 ms period, and ptp4l steps the clock to current TAI
(about 1.79e9 s). That is roughly 1.7e10 iterations, and many more for
shorter periods.

Can this loop spin for seconds or longer while holding ptp_data->lock and
never calling cond_resched()? That would block other PTP operations on the
device and trigger soft lockup warnings.

Should count be a u64?

> +	if (ret)
> +		return ret;
> +
> +	return ksz8463_ptp_enable_perout(dev, &request, 1);

[Severity: Medium]
When called from ksz8463_ptp_settime() or ksz8463_ptp_adjtime(), this
reaches ptp_find_pin() without holding ptp_clock::pincfg_mux:

    ksz8463_ptp_enable_perout()
      ksz_ptp_get_pin()
        ptp_find_pin()

The kernel-doc for ptp_find_pin() says:

    The caller must hold ptp_clock::pincfg_mux.

The PTP core takes pincfg_mux only around ->enable() and ptp_set_pinfunc().
It does not take it around ->settime64() or ->adjtime().
ksz8463_ptp_configure_perout() then also reads pin->chan and pin->index to
pick cfg_base and the GPO field.

ptp_set_pinfunc() writes pin->func and pin->chan with only pincfg_mux held.
If the pin's previous function was PTP_PF_NONE, it does not call ->enable()
first, so it never takes ptp_data->lock.

Can a concurrent PTP_PIN_SETFUNC race with a clock step here? If so,
ptp_find_pin() could match the wrong pin, and the wrong trigger unit or
GPIO would get programmed.

Before this patch KSZ8463 had n_pins = 0, so this path could not be reached
on this chip.

Would saving the resolved pin and unit at enable time avoid the lookup?
Switching to ptp_find_pin_unlocked() would create an ABBA ordering with the
ioctl path, which takes pincfg_mux and then ptp_data->lock.

> +}
> +
>  static int __ksz_ptp_settime(struct ksz_device *dev,
>  			     const struct timespec64 *ts)
>  {
> @@ -879,6 +1030,41 @@ static int __ksz_ptp_settime(struct ksz_device *dev,
>  	return 0;
>  }
>  
> +static int ksz8463_ptp_settime(struct ptp_clock_info *ptp,
> +			       const struct timespec64 *ts)
> +{

[ ... ]

> +	mutex_lock(&ptp_data->lock);
> +
> +	ret = __ksz_ptp_settime(dev, ts);
> +	if (ret)
> +		goto unlock;
> +
> +	switch (ptp_data->tou_mode) {
> +	case KSZ_PTP_TOU_IDLE:
> +		break;
> +
> +	case KSZ_PTP_TOU_PEROUT:
> +		ret = ksz8463_ptp_restart_perout(dev);
> +		if (ret)
> +			goto unlock;

[Severity: Low]
This is a pre-existing issue that was copied from ksz_ptp_settime() and
ksz_ptp_adjtime(). If __ksz_ptp_settime() succeeds and
ksz8463_ptp_restart_perout() then fails, the goto skips the update of
ptp_data->clock_time. The hardware clock has already been set at that
point.

ksz_tstamp_reconstruct() uses clock_time for the upper seconds bits:

    ts.tv_sec = (ptp_clock_time.tv_sec & ~3) | ts.tv_sec;

Could reconstructed timestamps be off by the step amount until the next
ksz_ptp_do_aux_work() refresh? ksz8463_ptp_adjtime() has the same pattern.

> +
> +		break;
> +	}
> +
> +	spin_lock_bh(&ptp_data->clock_lock);
> +	ptp_data->clock_time = *ts;
> +	spin_unlock_bh(&ptp_data->clock_lock);

[ ... ]

> @@ -1103,6 +1343,27 @@ static int ksz_ptp_start_clock(struct ksz_device *dev)
>  	return 0;
>  }
>  
> +void ksz8463_ptp_set_caps(struct dsa_switch *ds)
> +{

[ ... ]

> +	ptp_data->caps.do_aux_work	= ksz_ptp_do_aux_work;
> +	ptp_data->caps.enable		= ksz8463_ptp_enable;
> +	ptp_data->caps.verify		= ksz_ptp_verify_pin;
> +	ptp_data->caps.n_pins		= dev->info->n_pins;
> +	ptp_data->caps.n_per_out	= dev->info->n_per_out;
> +}

[Severity: Low]
caps.supported_perout_flags is never set here, so it stays 0. As a result,
ptp_perout_request() in drivers/ptp/ptp_chardev.c rejects the request
before ->enable() is called:

    if (perout->flags & ~ops->supported_perout_flags)
        return -EOPNOTSUPP;

The driver itself accepts PTP_PEROUT_DUTY_CYCLE in ksz_ptp_get_pin(), and
ksz_ptp_compute_perout_cycle() has a branch that uses request->on. Is that
branch unreachable? If so, the KSZ8463 pulse width is always
min(period / 2, 500 us).

Should supported_perout_flags be set to PTP_PEROUT_DUTY_CYCLE? The existing
ksz_ptp_set_caps() has the same omission.

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