Re: [PATCH net v4 0/2] net: ravb: fix PTP clock lifetime

Niklas Söderlund <[email protected]>
Newsgroups org.kernel.vger.linux-renesas-soc,org.kernel.vger.netdev
Message-ID <[email protected]>
Hi Xuanqiang,

Thanks for your work.

On 2026-08-11 18:37:31 +0800, Xuanqiang Luo wrote:
> From: Xuanqiang Luo <[email protected]>
> 
> This series fixes RAVB PTP clock lifetime handling. It reports a cached PHC
> index without accessing the clock pointer and drains PTP interrupts before
> unregistering the clock.
> 
> Patch 1 caches the PHC index and handles registration failures.
> 
> Patch 2 detaches the clock with xchg() and drains the PTP IRQs before
> unregistering it.

These patches are rather big change adding READ_ONCE() and WRITE_ONCE() 
to avoid a LLM warning? Or have you hit a real issue? How have you 
tested this work?

If you have a test-case could you share it? I have a pending series [1] 
that cleans up the whole RAVB driver ptp management which have grown 
rather organically. It have a small fix for the missing check of 
registering the clock. Would it be possible for you to test your work 
with that series too?

1.  https://lore.kernel.org/all/20260811160200.2049987-1-niklas.soderlund%[email protected]/

> 
> ---
> Changes:
> v4:
>   - Rebase onto Linux 7.2-rc7.
>   Patch 1:
>   - Cache the PHC index separately instead of locking clock access.
>     (Vadim Fedorenko)
>   - Reword the subject and update the commit message for the cached PHC
>     index approach.
>   - Add Vadim Fedorenko's Reviewed-by tag.
>   Patch 2:
>   - Replace priv->lock serialization with PTP interrupt disabling and IRQ
>     draining before unregistering the clock. (Vadim Fedorenko)
>   - Use READ_ONCE() and WRITE_ONCE() for lockless clock pointer access and
>     xchg() to detach the clock atomically.
>   - Track the error and management IRQs and synchronize all IRQs that can
>     invoke the PTP handler.
>   - Update the commit message to describe the interrupt handler race and
>     the new teardown sequence.
> 
> v3: https://lore.kernel.org/all/[email protected]/
>   Patch 1:
>   - Omit Niklas Söderlund's Reviewed-by tag because the implementation he
>     reviewed has changed.
>   - Describe the NULL pointer dereference before the first open as the most
>     likely failure mode in the commit message. (Jakub Kicinski, Sashiko)
>   - Normalize PTP clock registration failures to NULL.
>     (Jakub Kicinski, Sashiko)
>   - Keep hardware timestamping capabilities independent of PHC
>     availability. (Jakub Kicinski, Sashiko)
>   Patch 2 (new):
>   - Serialize PTP clock publication and access with priv->lock, and detach
>     the clock before unregistering it. (Jakub Kicinski, Sashiko)
> 
> v2: https://lore.kernel.org/all/[email protected]/
>   - Only advertise hardware timestamping support when a PHC is available
>     (Niklas Söderlund).
> 
> v1: https://lore.kernel.org/all/[email protected]/
> 
> Xuanqiang Luo (2):
>   net: ravb: avoid dereferencing an invalid PTP clock
>   net: ravb: serialize PTP clock teardown
> 
>  drivers/net/ethernet/renesas/ravb.h      |  3 ++
>  drivers/net/ethernet/renesas/ravb_main.c |  9 +++--
>  drivers/net/ethernet/renesas/ravb_ptp.c  | 44 ++++++++++++++++++++----
>  3 files changed, 47 insertions(+), 9 deletions(-)
> 
> 
> base-commit: db2ddb87143519e20a95aa36c60b36107b736a58
> -- 
> 2.43.0

-- 
Kind Regards,
Niklas Söderlund
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.