[net-next,0/7] Support external snapshots on dwmac1000

Message

Maxime Chevallier Oct. 29, 2024, 11:54 a.m. UTC

Hi,

This series is another take on the pervious work [1] done by
Alexis Lothoré, that fixes the support for external snapshots
timestamping in GMAC3-based devices.

The registers that are used to configure this don't have the same layout
in the dwmac1000 compared to dwmac4 and others.

One example would be the TS seconds/nanoseconds registet for snapshots,
which aren't mapped at the 0x48/0x4c but rather at 0x30/0x34.

Another example is the was the snapshots are enabled. DWMAC4 has a
dedicated auxiliary control register (PTP_ACR at 0x40) while on
DWMAC1000, this is controled through the PTP Timestamp Control Reg using
fields maked as reserved on DWMAC4.

Interrupts are also not handled the same way, as on dwmac1000 they are
cleared by reading the Auxiliary Status Reg, which simply doesn't exist
on dwmac4.

All of this means that with the current state of the code, auxiliary
timestamps simply don't work on dwmac1000.

Besides that, there are some limitations in the number of external
snapshot channels. It was also found that the PPS out configuration is
also not done the same way, but fixing PPS out isn't in the scope of
this series.

To address that hardware difference, we introduce dedicated
ptp_clock_info ops and parameters as well as dedicated hwtstamp_ops for
the dwmac100/dwmac1000. This allows simplifying the code for these
platforms, and avoids the introduction of other sets of stmmac internal
callbacks.

The naming for the non-dwmac1000 ops wasn't changed, so we have :
 - dwmac1000_ptp & stmmac_ptp
 - dwmac1000_ptp_clock_ops & stmmac_ptp_clock_ops

where the "stmmac_*" ops use the dwmac4-or-later behaviour.

I have converted dwmac100 along the way to these ops, however that's
hasn't been tested nor fully confirmed that this is correct, as I don't
have datasheets for devices that uses dwmac100.

I've converted dwmac100 just on the hypothesis that the GMAC3_X PTP offset
being used in both dwmac1000 and dwmac100 means that they share these same
register layouts as well.

Patch 1 prepares the use of per-hw interface ptp_clock_info by avoiding
the modification of the global parameters. This allows making the
stmmac_ptp_clock_ops const.

Patch 2 adds the ptp_clock_info as an hwif parameter.

Patch 3 addresses the autodiscovery of the timestamping features, as
dwmac1000 doesn't provide these parameters

Patch 4 introduces the ptp_clock_info specific to dwmac1000 platforms,
and Patch 5 the hwtstamping info.

Patch 6 enables the timestamping interrupt for external snapshot

Patch 7 removes a non-necessary include from stmmac_ptp.c.

This was tested on dwmac_socfpga, however this wasn't tested on a
dwmac4-based platform.

[1]: https://lore.kernel.org/netdev/[e-mail]/

Thanks Alexis for laying the groundwork for this,

Best regards,

Maxime

Maxime Chevallier (7):
  net: stmmac: Don't modify the global ptp ops directly
  net: stmmac: Use per-hw ptp clock ops
  net: stmmac: Only update the auto-discovered PTP clock features
  net: stmmac: Introduce dwmac1000 ptp_clock_info and operations
  net: stmmac: Introduce dwmac1000 timestamping operations
  net: stmmac: Enable timestamping interrupt on dwmac1000
  net: stmmac: Don't include dwmac4 definitions in stmmac_ptp

 drivers/net/ethernet/stmicro/stmmac/common.h  |  4 +
 .../net/ethernet/stmicro/stmmac/dwmac1000.h   | 15 +++-
 .../ethernet/stmicro/stmmac/dwmac1000_core.c  | 85 +++++++++++++++++++
 drivers/net/ethernet/stmicro/stmmac/hwif.c    | 14 ++-
 .../ethernet/stmicro/stmmac/stmmac_hwtstamp.c | 11 +++
 .../net/ethernet/stmicro/stmmac/stmmac_ptp.c  | 38 +++++++--
 .../net/ethernet/stmicro/stmmac/stmmac_ptp.h  | 10 +++
 7 files changed, 165 insertions(+), 12 deletions(-)

Comments

Alexis Lothoré (eBPF Foundation) Oct. 29, 2024, 2:45 p.m. UTC | #1

Hello Maxime,

On 10/29/24 12:54, Maxime Chevallier wrote:
> Hi,
> 
> This series is another take on the pervious work [1] done by
> Alexis Lothoré, that fixes the support for external snapshots
> timestamping in GMAC3-based devices.
> 

[...]

> [1]: https://lore.kernel.org/netdev/[e-mail]/
> 
> Thanks Alexis for laying the groundwork for this,
> 
> Best regards,
> 
> Maxime

Thanks for making this topic move forward. I suspect the series to be missing
some bits: in the initial series you mention in [1], I also reworked
stmmac_hwtstamp_set in stmmac_main.c, which is also currently assuming a GMAC4
layout ([2]). I suspect that in your series current state, any new call to
stmmac_hwtstamp_set will overwrite any previously configured hardware timestamping.

[2]
https://lore.kernel.org/netdev/[e-mail]/
> 
> Maxime Chevallier (7):
>   net: stmmac: Don't modify the global ptp ops directly
>   net: stmmac: Use per-hw ptp clock ops
>   net: stmmac: Only update the auto-discovered PTP clock features
>   net: stmmac: Introduce dwmac1000 ptp_clock_info and operations
>   net: stmmac: Introduce dwmac1000 timestamping operations
>   net: stmmac: Enable timestamping interrupt on dwmac1000
>   net: stmmac: Don't include dwmac4 definitions in stmmac_ptp
> 
>  drivers/net/ethernet/stmicro/stmmac/common.h  |  4 +
>  .../net/ethernet/stmicro/stmmac/dwmac1000.h   | 15 +++-
>  .../ethernet/stmicro/stmmac/dwmac1000_core.c  | 85 +++++++++++++++++++
>  drivers/net/ethernet/stmicro/stmmac/hwif.c    | 14 ++-
>  .../ethernet/stmicro/stmmac/stmmac_hwtstamp.c | 11 +++
>  .../net/ethernet/stmicro/stmmac/stmmac_ptp.c  | 38 +++++++--
>  .../net/ethernet/stmicro/stmmac/stmmac_ptp.h  | 10 +++
>  7 files changed, 165 insertions(+), 12 deletions(-)
>

Maxime Chevallier Oct. 29, 2024, 2:49 p.m. UTC | #2

Hi Alexis,

On Tue, 29 Oct 2024 15:45:07 +0100
Alexis Lothoré <[e-mail]> wrote:

> Hello Maxime,
> 
> On 10/29/24 12:54, Maxime Chevallier wrote:
> > Hi,
> > 
> > This series is another take on the pervious work [1] done by
> > Alexis Lothoré, that fixes the support for external snapshots
> > timestamping in GMAC3-based devices.
> >   
> 
> [...]
> 
> > [1]: https://lore.kernel.org/netdev/[e-mail]/
> > 
> > Thanks Alexis for laying the groundwork for this,
> > 
> > Best regards,
> > 
> > Maxime  
> 
> Thanks for making this topic move forward. I suspect the series to be missing
> some bits: in the initial series you mention in [1], I also reworked
> stmmac_hwtstamp_set in stmmac_main.c, which is also currently assuming a GMAC4
> layout ([2]). I suspect that in your series current state, any new call to
> stmmac_hwtstamp_set will overwrite any previously configured hardware timestamping.

You are correct indeed, I missed this bit in the series. I'll update
that for v2. 

Thanks,

Maxime