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

[netdev-bot+sashiko]

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] Baseline decision: false positive, kept only because
  requires_validation=true.
- [Low] The new framework bullet in Documentation/networking/oa-
  tc6-framework.rst (lines 156-158) reads: 'If supported by the hardware…

--

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

This patch documents new OA TC6 framework APIs for hardware timestamping,
PTP clock registration and the VEND1 MMS mapping helper. It also documents
the existing MMS-based register read/write helpers. A new entry in the
framework feature list describes hardware timestamp updates in the skb.

> diff --git a/Documentation/networking/oa-tc6-framework.rst b/Documentation/networking/oa-tc6-framework.rst
> index 2545ef694016f..3cfe8cb7cf75f 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]
Does this bullet only describe the transmit side of timestamping?

This document defines TSC as a field of the transmit data header:

  TSC (Bit 7..6) - Timestamp Capture. Request a timestamp capture when the
                   frame is transmitted onto the network.

Receive timestamps use the RTSA/RTSP footer bits instead, and the timestamp
is added to the start of the frame:

  RTSA (Bit 7) - Receive Timestamp Added. This bit is set when a 32-bit or
                 64-bit timestamp has been added to the beginning of the
                 received Ethernet frame. ...

By the end of the series the framework handles both paths. On transmit it
sets the TSC field, reads the TTSCx capture registers and calls
skb_tstamp_tx(). On receive in oa_tc6.c it parses RTSA/RTSP and checks the
parity of the 64-bit timestamp at the start of the frame. It then stores
the timestamp in skb_hwtstamps(skb) and strips it with skb_pull().

This bullet comes right after the "Forwards the received Ethernet frame"
item and refers to updating the timestamp in the skb. Could a reader take
this to mean that receive timestamps come from the TSC capture registers?

Would it be clearer to have one entry for transmit (TSC and capture
registers) and another for receive (RTSA/RTSP)?

> +
>  Data Transaction
>  ~~~~~~~~~~~~~~~~
>  
> @@ -496,3 +500,58 @@ the MAC-PHY.
>  Zero align receive frame feature can be enabled to align all receive ethernet
>  frames data to start at the beginning of any receive data chunk payload with a
>  start word offset (SWO) of zero.
> +
> +.. c:function:: int oa_tc6_ptp_register(struct oa_tc6 *tc6, \
> +                                        struct ptp_clock_info *info);

[ ... ]

> +.. c:function:: void oa_tc6_set_vend1_mms(struct oa_tc6 *tc6, int mms);
> +
> +Helper to map MDIO_MMD_VEND1 command to vendor specific Memory Map Select
> +(MMS) value. This function offers flexibility for vendors that may have
> +used any MMS value between 10 and 15 as allowed by the specification.
> +MDIO_MMD_VEND2 is already mapped to MMS4 in the OA TC6 frame work code.

[Severity: Low]
This isn't a bug, but at this point in the series none of these APIs
exist yet: oa_tc6_ptp_register(), oa_tc6_ptp_unregister(),
oa_tc6_get_ts_info(), oa_tc6_hwtstamp_get(), oa_tc6_get_ts_stats(),
oa_tc6_hwtstamp_set() and oa_tc6_set_vend1_mms(). Also,
oa_tc6_get_phy_c45_mms() does not map MDIO_MMD_VEND1 yet.

All of them are added later in the series, in "net: ethernet: oa_tc6:
Support for hardware timestamp" and "net: ethernet: oa_tc6: Support for
vendor specific MMS". This only affects someone reading the docs partway
through the series.

Would it make sense to move this patch after those two, so the docs land
together with the code?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261009-s2500-mac-phy-support-v9-0-dcefe1d0bf0d%40onsemi.com