Re: [PATCH v3 net-next 8/8] net: dsa: netc: add PTP one-step timestamping support

[Linus Walleij <linusw@kernel.org>]

Hi Wei,

thanks for your patch!

On Mon, Aug 31, 2026 at 10:24 AM <wei.fang@oss.nxp.com> wrote:

> From: Wei Fang <wei.fang@nxp.com>
>
> The NETC switch supports one-step TX timestamping for PTP Sync frames.
> When transmitting a Sync frame, the driver reads the current PTP time
> and writes it into the frame's originTimestamp field, and inserts a
> To_Port subtype 1 tag carrying the low 30 bits of that same PTP time.
> As the frame leaves the wire, the MAC captures the SFD transmit time
> and derives the residence time from the timestamp in the tag. It then
> reads the correction field at the offset given by
> PM_SINGLE_STEP[OFFSET], adds the residence time to it, and writes the
> result back.
>
> PM_SINGLE_STEP is a per-port register that can describe only one
> in-flight frame at a time, and programming it requires reading the
> current PTP time, which may sleep. Both constraints rule out handling
> one-step Sync on the xmit path.
>
> Instead, defer transmission to a per-port process-context work. The
> xmit path classifies the frame in netc_port_txtstamp(): a genuine
> one-step Sync (twoStepFlag cleared) has its PTP header offsets cached
> in the skb control block; frames that cannot be handled as one-step
> fall back to the two-step path or are sent as normal frames. netc_xmit()
> hands the classified frame to the switch driver via the
> onestep_sync_enqueue tagger callback, which queues it and kicks the work
> if no frame is currently in flight.
>
> The work dequeues one frame at a time, reads a fresh PTP time, programs
> PM_SINGLE_STEP, updates the originTimestamp field, and transmits the
> frame directly to the conduit via the onestep_sync_xmit tagger callback,
> bypassing dsa_user_xmit() to avoid double-counting TX stats. Only one
> frame is in flight at a time: the frame carries a TX-completion
> destructor that reschedules the work when the conduit frees the skb,
> keeping PM_SINGLE_STEP always matched to the frame being transmitted.
>
> The one-step context is reference-counted and its lifetime is decoupled
> from the devm-allocated netc_port. In-flight skbs hold a reference via
> their destructor, so the context outlives port disable until the conduit
> frees the last in-flight skb. Port disable clears onestep->active under
> work_lock; a work that runs afterwards observes active cleared will
> purge the onestep->queue and return without touching the freed port
> resources.
>
> Assisted-by: Wchat:claude-opus-4-8
> Signed-off-by: Wei Fang <wei.fang@nxp.com>

You agent produces "dostoyevsky commits" with too much verbiage.

Tell you agent to "be terse and to the point" in your AGENTS.md.

> +
> +               /* dsa_tree_teardown() calls dsa_tree_teardown_ports() before
> +                * dsa_tree_teardown_switches(), so netc_port_disable() is
> +                * executed before netc_teardown() and onestep->active is set
> +                * to false. In-flight one-step skbs still hold references via
> +                * the destructor; the context (and its work) is freed only
> +                * after the conduit frees the last in-flight skb. By then np
> +                * may be gone, but the work no longer dereferences np because
> +                * onestep->active has been cleared. And onestep->queue will be
> +                * cleared by netc_onestep_work()/netc_onestep_destroy_work().
> +                */
> +               netc_onestep_put(np->onestep);
> +               np->onestep = NULL;

This type of stuff is over-commenting and is considered AI slop.
Tell you agent to be very restrictive with inserting comments, essentially
it should only do that when you explicitly tell it to, such as when you
yourself feel it is really necessary or you got a review comment that
this is needed.

Yours,
Linus Walleij