Re: [PATCH v4 net-next 2/4] ethtool: Add ability to configure recovered clock for SyncE feature

Machnikowski, Maciej [off-list ref] writes:

quoted

-----Original Message-----
From: Petr Machata <petrm@nvidia.com>

Machnikowski, Maciej [off-list ref] writes:

quoted

quoted

-----Original Message-----
From: Petr Machata <petrm@nvidia.com>

Machnikowski, Maciej [off-list ref] writes:

quoted

quoted

-----Original Message-----
From: Ido Schimmel <redacted>

On Thu, Dec 02, 2021 at 03:17:06PM +0000, Machnikowski, Maciej wrote:

quoted

quoted

-----Original Message-----
From: Ido Schimmel <redacted>

4. Why these pins are represented as attributes of a netdev
and not as attributes of the EEC? That is, why are they
represented as output pins of the PHY as opposed to input
pins of the EEC?

They are 2 separate beings. Recovered clock outputs are
controlled separately from EEC inputs.

Separate how? What does it mean that they are controlled
separately? In which sense? That redirection of recovered
frequency to pin is controlled via PHY registers whereas
priority setting between EEC inputs is controlled via EEC
registers? If so, this is an implementation detail of a
specific design. It is not of any importance to user space.

They belong to different devices. EEC registers are physically
in the DPLL hanging over I2C and recovered clocks are in the
PHY/integrated PHY in the MAC. Depending on system architecture
you may have control over one piece only

What does ETHTOOL_MSG_RCLK_SET actually configure, physically? Say
I have this message:

ETHTOOL_MSG_RCLK_SET dev = eth0
- ETHTOOL_A_RCLK_OUT_PIN_IDX = n
- ETHTOOL_A_RCLK_PIN_FLAGS |= ETHTOOL_RCLK_PIN_FLAGS_ENA

Eventually this lands in ops->set_rclk_out(dev, out_idx,
new_state). What does the MAC driver do next?

It goes to the PTY layer, enables the clock recovery from a given
physical lane, optionally configure the clock divider and pin
output muxes. This will be HW-specific though, but the general
concept will look like that.

The reason I am asking is that I suspect that by exposing this
functionality through netdev, you assume that the NIC driver will do
whatever EEC configuration necessary _anyway_. So why couldn't it just
instantiate the EEC object as well?

Not necessarily. The EEC can be supported by totally different driver.
I.e there are Renesas DPLL drivers available now in the ptp subsystem.
The DPLL can be connected anywhere in the system.

quoted

quoted

quoted

quoted

quoted

quoted

quoted

5. What is the problem with the following model?

- The EEC is a separate object with following attributes:
  * State: Invalid / Freerun / Locked / etc
  * Sources: Netdev / external / etc
  * Potentially more

- Notifications are emitted to user space when the state of
  the EEC changes. Drivers will either poll the state from
  the device or get interrupts

- The mapping from netdev to EEC is queried via ethtool

Yep - that will be part of the EEC (DPLL) subsystem

This model avoids all the problems I pointed out in the current
proposal.

That's the go-to model, but first we need control over the
source as well :)

Why is that? Can you illustrate a case that breaks with the above
model?

If you have 32 port switch chip with 2 recovered clock outputs how
will you tell the chip to get the 18th port to pin 0 and from port
20 to pin 1? That's the part those patches addresses. The further
side of "which clock should the EEC use" belongs to the DPLL
subsystem and I agree with that.

So the claim is that in some cases the owner of the EEC does not know
about the netdevices?

If that is the case, how do netdevices know about the EEC, like the
netdev-centric model assumes?

Anyway, to answer the question, something like the following would
happen:

- Ask EEC to enumerate all input pins it knows about
- Find the one that references swp18
- Ask EEC to forward that input pin to output pin 0
- Repeat for swp20 and output pin 1

The switch driver (or multi-port NIC driver) just instantiates all of
netdevices, the EEC object, and pin objects, and therefore can set up
arbitrary linking between the three.

This will end up with a model in which pin X of the EEC will link to
dozens ports - userspace tool would need to find out the relation
between them and EECs somehow.

Indeed. If you have EEC connected to a bunch of ports, the EEC object is
related to a bunch of netdevices. The UAPI needs to have tools to dump
these objects so that it is possible to discover what is connected
where.

This configuration will also not change during the lifetime of the EEC
object, so tools can cache it.

It's far more convenient if a given netdev knows where it is connected
to and which pin can it drive.

Yeah, it is of course possible to add references from the netdevice to
the EEC object directly, so that the tool just needs to ask a netdevice
what EEC / RCLK source ID it maps to.

This has mostly nothing to do with the model itself.

I.e. send the netdev swp20 ETHTOOL_MSG_RCLK_GET and get the pin
indexes of the EEC and send the future message to find which EEC that
is (or even return EEC index in RCLK_GET?).

Since the pin index on its own is useless, it would make sense to return
both pieces of information at the same time.

Set the recovered clock on that pin with the ETHTOOL_MSG_RCLK_SET.

Nope.

Then go to the given EEC and configure it to use the pin that was
returned before as a frequency source and monitor the EEC state.

Yep.

EEC will invoke a callback to set up the tracking. If something special
needs to be done to "set the recovered clock on that pin", the handler
of that callback will do it.

Additionally, the EEC device may be instantiated by a totally
different driver, in which case the relation between its pins and
netdevs may not even be known.

Like an EEC, some PHYs, but the MAC driver does not know about both
pieces? Who sets up the connection between the two? The box admin
through some cabling? SoC designer?

Also, what does the external EEC actually do with the signal from the
PHY? Tune to it and forward to the other PHYs in the complex?