Re: [PATCH net-next v10 02/11] Documentation: networking: Add timestamp related APIs to OA TC6 framework

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The new framework duty bullet in Documentation/networking/oa-
  tc6-framework.rst (lines 156-158) gets the timestamp mechanism wrong.

--

Documentation: networking: Add timestamp related APIs to OA TC6 framework

This commit documents the new hardware timestamp APIs in the OA TC6
framework: PTP clock register/unregister, ts_info, hwtstamp get/set and
timestamp stats. It also documents the MMS based register read/write
helpers, and adds a framework bullet on how hardware timestamps are
updated in the skb.

> diff --git a/Documentation/networking/oa-tc6-framework.rst b/Documentation/networking/oa-tc6-framework.rst
> index 2545ef694016f..23856c4bfd691 100644
> --- a/Documentation/networking/oa-tc6-framework.rst
> +++ b/Documentation/networking/oa-tc6-framework.rst
> @@ -153,6 +153,10 @@ OPEN Alliance TC6 Framework
>  - Forwards the received Ethernet frame from 10Base-T1x MAC-PHY to n/w
>    subsystem.
>  
> +- If supported by the hardware and enabled, updates hardware timestamp
> +  in skb, when indicated by one of the three timestamp capture registers
> +  through TSC fields of the header.
> +

[Severity: Low]
Is this description accurate for both directions? The bullet comes right
after the receive forwarding bullet, so it reads like it is about the
receive path. The capture register / TSC mechanism only applies to
transmit, though.

In the implementation later in the series, in
drivers/net/ethernet/oa_tc6/oa_tc6_main.c, only the transmit data header
sets TSC:

    FIELD_PREP(OA_TC6_DATA_HEADER_TSC_OFFSET, tsc) |

The captured value is then read back from the TTSCA/B/C registers after
STATUS0 signals that it is ready.

The receive side works differently. The RTSA bit in the receive data
footer marks a timestamp carried in-band:

    ski->rtsa = FIELD_GET(OA_TC6_DATA_FOOTER_RTSA_VALID, footer);

The framework then writes that in-band value straight into the skb:

    hw_ts = skb_hwtstamps(skb);
    memset(hw_ts, 0, sizeof(*hw_ts));
    hw_ts->hwtstamp = ktime_set(ts[0], ts[1]);

This path uses no capture register or TSC field.

Could this bullet be split into separate TX and RX descriptions? The later
"net: ethernet: oa_tc6: Support for hardware timestamp" patch leaves this
text unchanged, so it stays this way at the end of the series.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261106-s2500-mac-phy-support-v10-0-31b5a69b85b7%40onsemi.com