linux-kselftest.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: "Machnikowski, Maciej" <maciej.machnikowski@intel.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Ido Schimmel <idosch@idosch.org>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"intel-wired-lan@lists.osuosl.org"
	<intel-wired-lan@lists.osuosl.org>,
	"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>,
	"linux-kselftest@vger.kernel.org"
	<linux-kselftest@vger.kernel.org>, "Andrew Lunn" <andrew@lunn.ch>,
	Michal Kubecek <mkubecek@suse.cz>,
	Saeed Mahameed <saeed@kernel.org>,
	Michael Chan <michael.chan@broadcom.com>
Subject: Re: [PATCH net-next 1/2] rtnetlink: Add new RTM_GETEECSTATE message to get SyncE status
Date: Wed, 8 Sep 2021 09:21:15 -0700	[thread overview]
Message-ID: <20210908092115.191fdc28@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> (raw)
In-Reply-To: <PH0PR11MB495169997552152891A69B57EAD49@PH0PR11MB4951.namprd11.prod.outlook.com>

On Wed, 8 Sep 2021 08:03:35 +0000 Machnikowski, Maciej wrote:
> > > Yep! Yet let's go one step at a time. I believe once we have the basics (EEC
> > > monitoring and recovered clock configuration) we'll be able to implement
> > > a basic functionality - and add bells and whistles later on, as there are more
> > > capabilities that we could support in SW.  
> > 
> > The set API may shape how the get API looks. We need a minimal viable
> > API where the whole control part of it is not "firmware or proprietary
> > tools take care of that".
> > 
> > Do you have public docs on how the whole solution works?  
> 
> The best reference would be my netdev 0x15 tutorial:
> https://netdevconf.info/0x15/session.html?Introduction-to-time-synchronization-over-Ethernet
> The SyncE API considerations starts ~54:00, but basically we need API for:
> - Controlling the lane to pin mapping for clock recovery
> - Check the EEC/DPLL state and see what's the source of reference frequency
> (in more advanced deployments)
> - control additional input and output pins (GNSS input, external inputs, recovered
>   frequency reference)

Thanks, good presentation! I haven't seen much in terms of system
design, at the level similar to the Broadcom whitepaper you shared.

> > > I believe this is the state-of-art: here's the Broadcom public one
> > > https://docs.broadcom.com/doc/1211168567832, I believe Marvel
> > > has similar solution. But would also be happy to hear others.  
> > 
> > Interesting. That reveals the need for also marking the backup
> > (/secondary) clock.  
> 
> That's optional, but useful. And here's where we need a feedback
> on which port/lane is currently used, as the switch may be automatic

What do you mean by optional? How does the user know if there is
fallback or not? Is it that Intel is intending to support only
devices with up to 2 ports and the second port is always the
backup (apologies for speculating).

> > Have you seen any docs on how systems with discreet PHY ASICs mux
> > the clocks?  
> 
> Yes - unfortunately they are not public :(

Mumble, mumble. Ido, Florian - any devices within your purview which
would support SyncE? 

> > > Ethernet IP/PHY usually outputs a divided clock signal (since it's
> > > easier to route) derived from the RX clock.
> > > The DPLL connectivity is vendor-specific, as you can use it to connect
> > > some external signals, but you can as well just care about relying
> > > the SyncE clock and only allow recovering it and passing along
> > > the QL info when your EEC is locked. That's why I backed up from
> > > a full DPLL implementation in favor of a more generic EEC clock.  
> > 
> > What is an ECC clock? To me the PLL state in the Ethernet port is the
> > state of the recovered clock. enum if_eec_state has values like
> > holdover which seem to be more applicable to the "system wide" PLL.  
> 
> EEC is Ethernet Equipment Clock. In most cases this will be a DPLL, but that's
> not mandatory and I believe it may be different is switches where
> you only need to drive all ports TX from a single frequency source. In this
> case the DPLL can be embedded in the multiport PHY,
>  
> > Let me ask this - if one port is training the link and the other one has
> > the lock and is the source - what state will be reported for each port?  
> 
> In this case the port that has the lock source will report the lock and 
> the EEC_SRC_PORT flag. The port that trains the link will show the
> lock without the flag and once it completes the training sequence it will
> use the EEC's frequency to transmit the data so that the next hop is able
> to synchronize its EEC to the incoming RX frequency

Alright, I don't like that. It feels like you're attaching one object's
information (ECC) to other objects (ports), and repeating it. Prof
Goczyła and dr Landowska would not be proud.

> > > The Time IP is again relative and vendor-specific. If SyncE is deployed
> > > alongside PTP it will most likely be tightly coupled, but if you only
> > > care about having a frequency source - it's not mandatory and it can be
> > > as well in the PHY IP.  
> > 
> > I would not think having just the freq is very useful.  
> 
> This depends on the deployment. There are couple popular frequencies
> Most popular are 2,048 kHz, 10 MHz and 64 kHz. There are many 
> deployments that only require frequency sync without the phase
> and/or time. I.e. if you deploy frequency division duplex you only need the
> frequency reference, and the higher frequency you have - the faster you can
> lock to it.

  reply	other threads:[~2021-09-08 16:21 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-03 15:14 [RFC v4 net-next 0/2] Add RTNL interface for SyncE Maciej Machnikowski
2021-09-03 15:14 ` [PATCH net-next 1/2] rtnetlink: Add new RTM_GETEECSTATE message to get SyncE status Maciej Machnikowski
2021-09-03 16:18   ` Stephen Hemminger
2021-09-03 22:20     ` Machnikowski, Maciej
2021-09-03 22:14   ` Jakub Kicinski
2021-09-06 18:30     ` Machnikowski, Maciej
2021-09-06 18:39       ` Jakub Kicinski
2021-09-06 19:01         ` Machnikowski, Maciej
2021-09-07  1:01           ` Jakub Kicinski
2021-09-07  8:50             ` Machnikowski, Maciej
2021-09-07 14:55               ` Jakub Kicinski
2021-09-07 15:47                 ` Machnikowski, Maciej
2021-09-07 19:47                   ` Jakub Kicinski
2021-09-08  8:03                     ` Machnikowski, Maciej
2021-09-08 16:21                       ` Jakub Kicinski [this message]
2021-09-08 17:30                         ` Machnikowski, Maciej
2021-09-08 19:34                           ` Andrew Lunn
2021-09-08 20:27                             ` Machnikowski, Maciej
2021-09-08 22:20                             ` Jakub Kicinski
2021-09-08 22:59                               ` Andrew Lunn
2021-09-09  2:09                                 ` Richard Cochran
2021-09-09  8:18                                   ` Machnikowski, Maciej
2021-09-10 14:14                                     ` Richard Cochran
2021-09-08 22:18                           ` Jakub Kicinski
2021-09-08 23:14                             ` Andrew Lunn
2021-09-08 23:58                               ` Jakub Kicinski
2021-09-09  8:26                                 ` Machnikowski, Maciej
2021-09-09  9:24                                   ` Machnikowski, Maciej
2021-09-09 10:15                                     ` David Miller
2021-09-09  8:11                             ` Machnikowski, Maciej
2021-09-13  8:50                         ` Ido Schimmel
2021-09-21 13:36                         ` Ido Schimmel
2021-09-21 13:15   ` Ido Schimmel
2021-09-21 13:37     ` Machnikowski, Maciej
2021-09-21 14:58       ` Ido Schimmel
2021-09-21 21:14         ` Jakub Kicinski
2021-09-22  6:22           ` Ido Schimmel
2021-09-03 15:14 ` [PATCH net-next 2/2] ice: add support for reading SyncE DPLL state Maciej Machnikowski
2021-09-03 22:06   ` Jakub Kicinski
2021-09-21 13:25   ` Ido Schimmel
  -- strict thread matches above, loose matches on Subject: below --
2021-08-31 11:52 [PATCH net-next 0/2] Add RTNL interface for SyncE EEC state Maciej Machnikowski
2021-08-31 11:52 ` [PATCH net-next 1/2] rtnetlink: Add new RTM_GETEECSTATE message to get SyncE status Maciej Machnikowski
2021-08-31 13:44   ` Jakub Kicinski

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=20210908092115.191fdc28@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com \
    --to=kuba@kernel.org \
    --cc=abyagowi@fb.com \
    --cc=andrew@lunn.ch \
    --cc=anthony.l.nguyen@intel.com \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=idosch@idosch.org \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=maciej.machnikowski@intel.com \
    --cc=michael.chan@broadcom.com \
    --cc=mkubecek@suse.cz \
    --cc=netdev@vger.kernel.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).