Linux kernel and device drivers for NXP i.MX platforms
 help / color / mirror / Atom feed
* [PATCH RESEND v1 net] ptp: netc: explicitly clear TMR_OFF during initialization
@ 2026-07-20  1:25 wei.fang
  2026-07-20 10:36 ` Breno Leitao
  2026-07-21  1:21 ` sashiko-bot
  0 siblings, 2 replies; 3+ messages in thread
From: wei.fang @ 2026-07-20  1:25 UTC (permalink / raw)
  To: richardcochran, xiaoning.wang, andrew+netdev, davem, edumazet,
	kuba, pabeni, Frank.Li, vadim.fedorenko
  Cc: wei.fang, imx, netdev, linux-kernel

From: Clark Wang <xiaoning.wang@nxp.com>

The NETC timer does not support function level reset, so TMR_OFF_L/H
registers are not cleared by pcie_flr(). If TMR_OFF was set to a
non-zero value in a previous binding, it will persist across driver
rebind and cause inaccurate PTP time.

There is also a hardware issue: after a warm reset or soft reset,
TMR_OFF_L/H registers appear to be cleared to zero, but the timer clock
domain internally retains the stale value. When the timer is re-enabled,
TMR_CUR_TIME continues to track the old offset until TMR_OFF is written
explicitly. This can cause incorrect PTP timestamps and even PTP clock
synchronization failures.

Per the recommendation from the IP team, explicitly write 0 to TMR_OFF
in netc_timer_init() to flush the internally cached value and ensure
TMR_CUR_TIME follows the freshly initialized counter.

Fixes: 87a201d59963 ("ptp: netc: add NETC V4 Timer PTP driver support")
Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
Signed-off-by: Wei Fang <wei.fang@nxp.com>
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
---
No changes, just collect Reviewed-by tag
v1 link: https://lore.kernel.org/imx/20260706081232.3661826-1-wei.fang@oss.nxp.com/
---
 drivers/ptp/ptp_netc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/ptp/ptp_netc.c b/drivers/ptp/ptp_netc.c
index 94e952ee6990..5e381c354d74 100644
--- a/drivers/ptp/ptp_netc.c
+++ b/drivers/ptp/ptp_netc.c
@@ -779,6 +779,7 @@ static void netc_timer_init(struct netc_timer *priv)
 	netc_timer_wr(priv, NETC_TMR_FIPER_CTRL, fiper_ctrl);
 	netc_timer_wr(priv, NETC_TMR_ECTRL, NETC_TMR_DEFAULT_ETTF_THR);
 
+	netc_timer_offset_write(priv, 0);
 	ktime_get_real_ts64(&now);
 	ns = timespec64_to_ns(&now);
 	netc_timer_cnt_write(priv, ns);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH RESEND v1 net] ptp: netc: explicitly clear TMR_OFF during initialization
  2026-07-20  1:25 [PATCH RESEND v1 net] ptp: netc: explicitly clear TMR_OFF during initialization wei.fang
@ 2026-07-20 10:36 ` Breno Leitao
  2026-07-21  1:21 ` sashiko-bot
  1 sibling, 0 replies; 3+ messages in thread
From: Breno Leitao @ 2026-07-20 10:36 UTC (permalink / raw)
  To: wei.fang
  Cc: richardcochran, xiaoning.wang, andrew+netdev, davem, edumazet,
	kuba, pabeni, Frank.Li, vadim.fedorenko, wei.fang, imx, netdev,
	linux-kernel

On Mon, Jul 20, 2026 at 09:25:08AM +0800, wei.fang@oss.nxp.com wrote:
> From: Clark Wang <xiaoning.wang@nxp.com>
> 
> The NETC timer does not support function level reset, so TMR_OFF_L/H
> registers are not cleared by pcie_flr(). If TMR_OFF was set to a
> non-zero value in a previous binding, it will persist across driver
> rebind and cause inaccurate PTP time.
> 
> There is also a hardware issue: after a warm reset or soft reset,
> TMR_OFF_L/H registers appear to be cleared to zero, but the timer clock
> domain internally retains the stale value. When the timer is re-enabled,
> TMR_CUR_TIME continues to track the old offset until TMR_OFF is written
> explicitly. This can cause incorrect PTP timestamps and even PTP clock
> synchronization failures.
> 
> Per the recommendation from the IP team, explicitly write 0 to TMR_OFF
> in netc_timer_init() to flush the internally cached value and ensure
> TMR_CUR_TIME follows the freshly initialized counter.
> 
> Fixes: 87a201d59963 ("ptp: netc: add NETC V4 Timer PTP driver support")
> Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>

Reviewed-by: Breno Leitao <leitao@debian.org>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH RESEND v1 net] ptp: netc: explicitly clear TMR_OFF during initialization
  2026-07-20  1:25 [PATCH RESEND v1 net] ptp: netc: explicitly clear TMR_OFF during initialization wei.fang
  2026-07-20 10:36 ` Breno Leitao
@ 2026-07-21  1:21 ` sashiko-bot
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-07-21  1:21 UTC (permalink / raw)
  To: wei.fang; +Cc: imx, Frank.Li

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

Pre-existing issues:
- [High] The driver requests the MSI-X interrupt during probe before sanitizing hardware interrupt state. Combined with a logic error in the ISR, this causes an uninterruptible interrupt storm (CPU lockup).
- [High] Race condition in `netc_timer_remove` allows user-space ioctls to leave hardware interrupts active after driver unbind.
--

--- Patch [1]: [PATCH RESEND v1 net] ptp: netc: explicitly clear TMR_OFF during initialization ---
commit f3b81ecce462c4ab69c0789be51860d8eb8b3d21
Author: Clark Wang <xiaoning.wang@nxp.com>

ptp: netc: explicitly clear TMR_OFF during initialization

The NETC timer does not support function level reset, so TMR_OFF_L/H
registers are not cleared by pcie_flr(). If TMR_OFF was set to a
non-zero value in a previous binding, it will persist across driver
rebind and cause inaccurate PTP time.

Link: https://lore.kernel.org/imx/20260706081232.3661826-1-wei.fang@oss.nxp.com/

> diff --git a/drivers/ptp/ptp_netc.c b/drivers/ptp/ptp_netc.c
> index 94e952ee69902..5e381c354d746 100644
> --- a/drivers/ptp/ptp_netc.c
> +++ b/drivers/ptp/ptp_netc.c
> @@ -779,6 +779,7 @@ static void netc_timer_init(struct netc_timer *priv)
>  	netc_timer_wr(priv, NETC_TMR_FIPER_CTRL, fiper_ctrl);
>  	netc_timer_wr(priv, NETC_TMR_ECTRL, NETC_TMR_DEFAULT_ETTF_THR);
>  
> +	netc_timer_offset_write(priv, 0);

[Severity: High]
This is a pre-existing issue, but since netc_timer_init() is called during
netc_timer_probe(), does the driver risk an interrupt storm?

In netc_timer_probe(), the driver calls request_irq() via
netc_timer_init_msix_irq() before calling netc_timer_init():

drivers/ptp/ptp_netc.c:netc_timer_probe() {
    ...
	err = netc_timer_init_msix_irq(priv);
	if (err)
		goto timer_pci_remove;

	netc_timer_init(priv);
    ...
}

If hardware has a pending interrupt from a previous driver session, it could
fire immediately. Furthermore, the ISR appears to mask out unhandled events
before writing back to clear them, and unconditionally returns IRQ_HANDLED:

drivers/ptp/ptp_netc.c:netc_timer_isr() {
    ...
	tmr_event = netc_timer_rd(priv, NETC_TMR_TEVENT);
	tmr_event &= priv->tmr_emask;
	/* Clear interrupts status */
	netc_timer_wr(priv, NETC_TMR_TEVENT, tmr_event);
    ...
	return IRQ_HANDLED;
}

Since priv->tmr_emask is 0 during probe, tmr_event becomes 0, so the hardware
interrupt is never cleared. Because IRQ_HANDLED is returned unconditionally,
will this lead to an uninterruptible infinite interrupt storm?


[Severity: High]
This isn't a bug introduced by this patch, but is there a race condition
during driver unbind that could leave hardware interrupts active?

In netc_timer_remove(), the driver writes to hardware registers to disable
interrupts before unregistering the PTP clock:

drivers/ptp/ptp_netc.c:netc_timer_remove() {
    ...
	netc_timer_wr(priv, NETC_TMR_TEMASK, 0);
	netc_timer_wr(priv, NETC_TMR_CTRL, 0);
	ptp_clock_unregister(priv->clock);
    ...
}

Since ptp_clock_unregister() is the barrier that waits for ongoing ioctls and
blocks new ones, can a concurrent ioctl execute after the mask is cleared but
before unregistration finishes?

For example, a concurrent PTP_CLK_REQ_EXTTS ioctl calling
netc_timer_enable_extts():

drivers/ptp/ptp_netc.c:netc_timer_enable_extts() {
    ...
	netc_timer_wr(priv, NETC_TMR_TEMASK, priv->tmr_emask);

	spin_unlock_irqrestore(&priv->lock, flags);
    ...
}

Could this re-enable interrupts while the driver unbinds, leading to a
use-after-free or system instability on the next probe?

>  	ktime_get_real_ts64(&now);
>  	ns = timespec64_to_ns(&now);
>  	netc_timer_cnt_write(priv, ns);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720012508.23227-1-wei.fang@oss.nxp.com?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-21  1:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-20  1:25 [PATCH RESEND v1 net] ptp: netc: explicitly clear TMR_OFF during initialization wei.fang
2026-07-20 10:36 ` Breno Leitao
2026-07-21  1:21 ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox