RE: [PATCH net-next 4/6] rtnetlink: Add support for SyncE recovered clock configuration

-----Original Message----- From: Roopa Prabhu roopa@nvidia.com Sent: Thursday, November 4, 2021 7:25 PM To: Machnikowski, Maciej maciej.machnikowski@intel.com; netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org Cc: richardcochran@gmail.com; abyagowi@fb.com; Nguyen, Anthony L anthony.l.nguyen@intel.com; davem@davemloft.net; kuba@kernel.org; linux-kselftest@vger.kernel.org; idosch@idosch.org; mkubecek@suse.cz; saeed@kernel.org; michael.chan@broadcom.com Subject: Re: [PATCH net-next 4/6] rtnetlink: Add support for SyncE recovered clock configuration

On 11/4/21 1:12 AM, Maciej Machnikowski wrote:

...

Add support for RTNL messages for reading/configuring SyncE recovered clocks. The messages are: RTM_GETRCLKRANGE: Reads the allowed pin index range for the

recovered

...

  clock outputs. This can be aligned to PHY outputs or
  to EEC inputs, whichever is better for a given
  application

RTM_GETRCLKSTATE: Read the state of recovered pins that output

recovered

...

  clock from a given port. The message will contain the
  number of assigned clocks (IFLA_RCLK_STATE_COUNT) and
  a N pin inexes in IFLA_RCLK_STATE_OUT_IDX

RTM_SETRCLKSTATE: Sets the redirection of the recovered clock for a given pin

Signed-off-by: Maciej Machnikowski maciej.machnikowski@intel.com

Can't we just use a single RTM msg with nested attributes ?

With separate RTM msgtype for each syncE attribute we will end up bloating the RTM msg namespace.

(these api's could also be in ethtool given its directly querying the drivers)

I'm not a fan of merging those messages. The mergeable ones are GETRCLKRANGE and GETCLKSTATE, but the get range function may be result in a significantly longer call if the information about the underlying HW require any HW calls. They are currently grouped in 3 categories: - reading the boundaries in GetRclkRange (we can later add more to it, like configurable frequency limits) - Reading current configuration - setting the required configuration

I don't plan adding any additional RTM msg types for those (and that's the reason why I pulled them before EEC state which may have more messages in the future)

We also discussed ethtool way in the past RFCs, but this concept is applicable to any transport layer so I'd rather not limit it to ethernet devices (i.e. SONET, infiniband and others).

Regards Maciek