Re: [PATCH net-next v4 8/8] net: dsa: microchip: Add two-step PTP support for KSZ8463

[Vladimir Oltean <olteanv@gmail.com>]

On Wed, Feb 18, 2026 at 05:42:32PM +0100, Bastien Curutchet wrote:
> On 2/18/26 4:25 PM, Vladimir Oltean wrote:
> > On Wed, Feb 18, 2026 at 04:21:26PM +0100, Bastien Curutchet wrote:
> > > > You need to set port->last_tx_is_pdelayresp in the atomic section where
> > > > you know for sure that there's a single TX timestampable skb in flight.
> > > > There's no explicit lock which creates that atomic section, but the fact
> > > > that the worker kthread of the tagger processes work items one by one is
> > > > what gives you that guarantee.
> > > 
> > > Thank you for the explanations. I suspected a race condition here but I
> > > didn't know how to mitigate it. I tested a new version on my side with
> > > port->last_tx_is_pdelayresp set in the ksz_port_deferred_xmit() worker and
> > > it works fine.
> > 
> > Good.
> > 
> > Next question: if the logic is all in ksz_port_deferred_xmit(), which is
> > sleepable and sends packets one by one, do you actually need to save the
> > packet type in port->last_tx_is_pdelayresp? Can't you just keep a local
> > variable with it?
> 
> Well, I think I could but it feels like a significant rework of the existing
> implementation. So far the register location is tied to the struct
> ksz_ptp_irq, since on all the other switches one interrupt bit is associated
> with one timestamp only. In fact, even on the KSZ8463 the same logic applies
> because there are two interrupt bits per port: one for the sync messages and
> one for the pdelay_req / pdelay_resp messages.
> 
> If I'm not mistaken, using a local variable here would mean moving the
> timestamp reading from ksz_ptp_msg_thread_fn() to ksz_port_deferred_xmit().
> We would then need to check the type of each packet before sending it, in
> order to know where to read the timestamp after wait_for_completion()
> returns.

OK. I no longer had a git tree checked out with these patches, so I
forgot you need the PTP type in the IRQ handler. It's fine the way you
plan to send the next version. Thanks for checking.