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

Hi Wei,

thanks for your patch!

On Mon, Aug 31, 2026 at 10:24 AM [off-list ref] 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