All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Machnikowski, Maciej" <maciej.machnikowski@intel.com>
To: Petr Machata <petrm@nvidia.com>
Cc: Ido Schimmel <idosch@idosch.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"intel-wired-lan@lists.osuosl.org"
	<intel-wired-lan@lists.osuosl.org>,
	"Kubalewski, Arkadiusz" <arkadiusz.kubalewski@intel.com>,
	"richardcochran@gmail.com" <richardcochran@gmail.com>,
	"abyagowi@fb.com" <abyagowi@fb.com>,
	"Nguyen, Anthony L" <anthony.l.nguyen@intel.com>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"kuba@kernel.org" <kuba@kernel.org>,
	"linux-kselftest@vger.kernel.org"
	<linux-kselftest@vger.kernel.org>,
	"mkubecek@suse.cz" <mkubecek@suse.cz>,
	"saeed@kernel.org" <saeed@kernel.org>,
	"michael.chan@broadcom.com" <michael.chan@broadcom.com>
Subject: RE: [PATCH v4 net-next 2/4] ethtool: Add ability to configure recovered clock for SyncE feature
Date: Mon, 6 Dec 2021 15:09:25 +0000	[thread overview]
Message-ID: <MW5PR11MB58121BF596AB9C501F900887EA6D9@MW5PR11MB5812.namprd11.prod.outlook.com> (raw)
In-Reply-To: <87czm9okyc.fsf@nvidia.com>

> -----Original Message-----
> From: Petr Machata <petrm@nvidia.com>
> Sent: Monday, December 6, 2021 3:41 PM
> To: Machnikowski, Maciej <maciej.machnikowski@intel.com>
> Subject: Re: [PATCH v4 net-next 2/4] ethtool: Add ability to configure
> recovered clock for SyncE feature
> 
> 
> Machnikowski, Maciej <maciej.machnikowski@intel.com> writes:
> 
> >> -----Original Message-----
> >> From: Petr Machata <petrm@nvidia.com>
> >>
> >> Machnikowski, Maciej <maciej.machnikowski@intel.com> writes:
> >>
> >> > 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?
> >
> > Yes - it can also apply HW filters to it.
> 
> Sounds like this device should have an EEC instance of its own then.
> 
> Maybe we need to call it something else than "EEC". PLL? Something that
> does not have the standardization connotations, because several
> instances would be present in a system with several NICs.

There will be no EEC/EEC subsystem, but the DPLL. Every driver would 
be able to create a DPLL object so that we can easily use it from 
non-netdev devices as well. See the other mail for more details.

> > The EEC model will not work when you have the following system:
> > SoC with some ethernet ports with driver A
> > Switch chip with N ports with driver B
> > EEC/DPLL with driver C
> > Both SoC and Switch ASIC can recover clock and use the cleaned
> > clock from the DPLL.
> >
> > In that case you can't create any relation between EEC and recover
> > clock pins that would enable the EEC subsystem to control
> > recovered clocks, because you have 3 independent drivers.
> 
> I think that in that case you have several EEC instances. Those are
> connected by some wiring that is external to the devices themselves. I
> am not sure who should be in charge of describing the wiring. Device
> tree? Config file?

In some complex systems you'll need to create a relation between netdevs
and DPLLs in a config file, so it is the only way to describe all possible
scenarios.
We can't assume any connections between them, as that's design specific,
just like PTP pins are. They have labels, but it's up to the system
integrator to define how they are used.
We can consider creating some of them if they are known to the driver
and belong to the same driver.

> > The model you proposed assumes that the MAC/Switch is in
> > charge of the DPLL, but that's not always true.
> 
> The EEC-centric model does not in fact assume that. It lets anyone to
> set up an EEC object.
> 
> The netdev-centric UAPI assumes that the driver behind the netdev knows
> about how many RCLK out pins there are. So it can certainly instantiate
> a DPLL object instead, with those pins as external pins, and leave the
> connection of the external pins to the EEC proper implicit.

Netdev will know how many RCLK outputs are there, as that's the function
of a given MAC/PHY/Retimer.

> That gives userspace exactly the same information as the netdev-centric
> UAPI, but now userspace doesn't need to know about netdevs, and
> synchronously-spinning drives, and GPS receivers, each of which is
> handled through a dedicated set of netlink messages / sysctls / what
> have you. The userspace needs to know about EEC subsystem, and that's
> it.

I believe the direction is to make the connection between a netdev and
its related DPLL that's serving as EEC in a similar way the link to a PTP 
device is created. Userspace app will need to go to DPLL subsystem
to understand what's the current frequency source for a given netdev.

That's still independent uAPI from the one defined by those patches.

> > The model where recovered clock outputs are controlled independently
> > can support both models and is more flexible. It can also address the
> 
> - Anyone can instantiate EEC objects
> - Only things with ports instantiate netdevs
> 
> How is the latter one more flexible?

- Everything can instantiate DPLL object,
- Only a netdev can instantiate recovered clock outputs, which can be 
  connected to any other part of the system - not only a DPLL.

WARNING: multiple messages have this Message-ID (diff)
From: Machnikowski, Maciej <maciej.machnikowski@intel.com>
To: intel-wired-lan@osuosl.org
Subject: [Intel-wired-lan] [PATCH v4 net-next 2/4] ethtool: Add ability to configure recovered clock for SyncE feature
Date: Mon, 6 Dec 2021 15:09:25 +0000	[thread overview]
Message-ID: <MW5PR11MB58121BF596AB9C501F900887EA6D9@MW5PR11MB5812.namprd11.prod.outlook.com> (raw)
In-Reply-To: <87czm9okyc.fsf@nvidia.com>

> -----Original Message-----
> From: Petr Machata <petrm@nvidia.com>
> Sent: Monday, December 6, 2021 3:41 PM
> To: Machnikowski, Maciej <maciej.machnikowski@intel.com>
> Subject: Re: [PATCH v4 net-next 2/4] ethtool: Add ability to configure
> recovered clock for SyncE feature
> 
> 
> Machnikowski, Maciej <maciej.machnikowski@intel.com> writes:
> 
> >> -----Original Message-----
> >> From: Petr Machata <petrm@nvidia.com>
> >>
> >> Machnikowski, Maciej <maciej.machnikowski@intel.com> writes:
> >>
> >> > 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?
> >
> > Yes - it can also apply HW filters to it.
> 
> Sounds like this device should have an EEC instance of its own then.
> 
> Maybe we need to call it something else than "EEC". PLL? Something that
> does not have the standardization connotations, because several
> instances would be present in a system with several NICs.

There will be no EEC/EEC subsystem, but the DPLL. Every driver would 
be able to create a DPLL object so that we can easily use it from 
non-netdev devices as well. See the other mail for more details.

> > The EEC model will not work when you have the following system:
> > SoC with some ethernet ports with driver A
> > Switch chip with N ports with driver B
> > EEC/DPLL with driver C
> > Both SoC and Switch ASIC can recover clock and use the cleaned
> > clock from the DPLL.
> >
> > In that case you can't create any relation between EEC and recover
> > clock pins that would enable the EEC subsystem to control
> > recovered clocks, because you have 3 independent drivers.
> 
> I think that in that case you have several EEC instances. Those are
> connected by some wiring that is external to the devices themselves. I
> am not sure who should be in charge of describing the wiring. Device
> tree? Config file?

In some complex systems you'll need to create a relation between netdevs
and DPLLs in a config file, so it is the only way to describe all possible
scenarios.
We can't assume any connections between them, as that's design specific,
just like PTP pins are. They have labels, but it's up to the system
integrator to define how they are used.
We can consider creating some of them if they are known to the driver
and belong to the same driver.

> > The model you proposed assumes that the MAC/Switch is in
> > charge of the DPLL, but that's not always true.
> 
> The EEC-centric model does not in fact assume that. It lets anyone to
> set up an EEC object.
> 
> The netdev-centric UAPI assumes that the driver behind the netdev knows
> about how many RCLK out pins there are. So it can certainly instantiate
> a DPLL object instead, with those pins as external pins, and leave the
> connection of the external pins to the EEC proper implicit.

Netdev will know how many RCLK outputs are there, as that's the function
of a given MAC/PHY/Retimer.

> That gives userspace exactly the same information as the netdev-centric
> UAPI, but now userspace doesn't need to know about netdevs, and
> synchronously-spinning drives, and GPS receivers, each of which is
> handled through a dedicated set of netlink messages / sysctls / what
> have you. The userspace needs to know about EEC subsystem, and that's
> it.

I believe the direction is to make the connection between a netdev and
its related DPLL that's serving as EEC in a similar way the link to a PTP 
device is created. Userspace app will need to go to DPLL subsystem
to understand what's the current frequency source for a given netdev.

That's still independent uAPI from the one defined by those patches.

> > The model where recovered clock outputs are controlled independently
> > can support both models and is more flexible. It can also address the
> 
> - Anyone can instantiate EEC objects
> - Only things with ports instantiate netdevs
> 
> How is the latter one more flexible?

- Everything can instantiate DPLL object,
- Only a netdev can instantiate recovered clock outputs, which can be 
  connected to any other part of the system - not only a DPLL.

  reply	other threads:[~2021-12-06 15:14 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-01 18:02 [PATCH v4 net-next 0/4] Add ethtool interface for SyncE Maciej Machnikowski
2021-12-01 18:02 ` [Intel-wired-lan] " Maciej Machnikowski
2021-12-01 18:02 ` [PATCH v4 net-next 1/4] ice: add support detecting features based on netlist Maciej Machnikowski
2021-12-01 18:02   ` [Intel-wired-lan] " Maciej Machnikowski
2021-12-01 18:02 ` [PATCH v4 net-next 2/4] ethtool: Add ability to configure recovered clock for SyncE feature Maciej Machnikowski
2021-12-01 18:02   ` [Intel-wired-lan] " Maciej Machnikowski
2021-12-02 11:48   ` kernel test robot
2021-12-02 11:48     ` kernel test robot
2021-12-02 11:48     ` [Intel-wired-lan] " kernel test robot
2021-12-02 12:43   ` Ido Schimmel
2021-12-02 12:43     ` [Intel-wired-lan] " Ido Schimmel
2021-12-02 15:17     ` Machnikowski, Maciej
2021-12-02 15:17       ` [Intel-wired-lan] " Machnikowski, Maciej
2021-12-02 16:35       ` Ido Schimmel
2021-12-02 16:35         ` [Intel-wired-lan] " Ido Schimmel
2021-12-02 17:20         ` Machnikowski, Maciej
2021-12-02 17:20           ` [Intel-wired-lan] " Machnikowski, Maciej
2021-12-03 14:26           ` Petr Machata
2021-12-03 14:26             ` [Intel-wired-lan] " Petr Machata
2021-12-03 14:55             ` Machnikowski, Maciej
2021-12-03 14:55               ` [Intel-wired-lan] " Machnikowski, Maciej
2021-12-03 15:58               ` Ido Schimmel
2021-12-03 15:58                 ` [Intel-wired-lan] " Ido Schimmel
2021-12-03 16:26               ` Petr Machata
2021-12-03 16:26                 ` [Intel-wired-lan] " Petr Machata
2021-12-03 16:50                 ` Machnikowski, Maciej
2021-12-03 16:50                   ` [Intel-wired-lan] " Machnikowski, Maciej
2021-12-03 18:44                   ` Petr Machata
2021-12-03 18:44                     ` [Intel-wired-lan] " Petr Machata
2021-12-03 19:07                     ` Machnikowski, Maciej
2021-12-03 19:07                       ` [Intel-wired-lan] " Machnikowski, Maciej
2021-12-06 14:40                       ` Petr Machata
2021-12-06 14:40                         ` [Intel-wired-lan] " Petr Machata
2021-12-06 15:09                         ` Machnikowski, Maciej [this message]
2021-12-06 15:09                           ` Machnikowski, Maciej
2021-12-06 16:00                           ` Petr Machata
2021-12-06 16:00                             ` [Intel-wired-lan] " Petr Machata
2021-12-06 18:49                             ` Machnikowski, Maciej
2021-12-06 18:49                               ` [Intel-wired-lan] " Machnikowski, Maciej
2021-12-03 15:45           ` Ido Schimmel
2021-12-03 15:45             ` [Intel-wired-lan] " Ido Schimmel
2021-12-03 16:18             ` Machnikowski, Maciej
2021-12-03 16:18               ` [Intel-wired-lan] " Machnikowski, Maciej
2021-12-03 18:21               ` Petr Machata
2021-12-03 18:21                 ` [Intel-wired-lan] " Petr Machata
2021-12-05 12:24               ` Ido Schimmel
2021-12-05 12:24                 ` [Intel-wired-lan] " Ido Schimmel
2021-12-06  9:15                 ` Machnikowski, Maciej
2021-12-06  9:15                   ` [Intel-wired-lan] " Machnikowski, Maciej
2021-12-01 18:02 ` [PATCH v4 net-next 3/4] ice: add support for monitoring SyncE DPLL state Maciej Machnikowski
2021-12-01 18:02   ` [Intel-wired-lan] " Maciej Machnikowski
2021-12-01 18:02 ` [PATCH v4 net-next 4/4] ice: add support for SyncE recovered clocks Maciej Machnikowski
2021-12-01 18:02   ` [Intel-wired-lan] " Maciej Machnikowski
2021-12-02  1:56   ` Jakub Kicinski
2021-12-02  1:56     ` [Intel-wired-lan] " Jakub Kicinski
2021-12-02 12:29 [PATCH v4 net-next 2/4] ethtool: Add ability to configure recovered clock for SyncE feature kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=MW5PR11MB58121BF596AB9C501F900887EA6D9@MW5PR11MB5812.namprd11.prod.outlook.com \
    --to=maciej.machnikowski@intel.com \
    --cc=abyagowi@fb.com \
    --cc=anthony.l.nguyen@intel.com \
    --cc=arkadiusz.kubalewski@intel.com \
    --cc=davem@davemloft.net \
    --cc=idosch@idosch.org \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=kuba@kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=michael.chan@broadcom.com \
    --cc=mkubecek@suse.cz \
    --cc=netdev@vger.kernel.org \
    --cc=petrm@nvidia.com \
    --cc=richardcochran@gmail.com \
    --cc=saeed@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.