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