public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@intel.com>
Cc: "Nitka, Grzegorz" <grzegorz.nitka@intel.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"intel-wired-lan@lists.osuosl.org"
	<intel-wired-lan@lists.osuosl.org>,
	"Oros, Petr" <poros@redhat.com>,
	"richardcochran@gmail.com" <richardcochran@gmail.com>,
	"andrew+netdev@lunn.ch" <andrew+netdev@lunn.ch>,
	"Kitszel, Przemyslaw" <przemyslaw.kitszel@intel.com>,
	"Nguyen, Anthony L" <anthony.l.nguyen@intel.com>,
	"Prathosh.Satish@microchip.com" <Prathosh.Satish@microchip.com>,
	"Vecera, Ivan" <ivecera@redhat.com>,
	"jiri@resnulli.us" <jiri@resnulli.us>,
	"vadim.fedorenko@linux.dev" <vadim.fedorenko@linux.dev>,
	"donald.hunter@gmail.com" <donald.hunter@gmail.com>,
	"horms@kernel.org" <horms@kernel.org>,
	"pabeni@redhat.com" <pabeni@redhat.com>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"edumazet@google.com" <edumazet@google.com>
Subject: Re: [PATCH v5 net-next 0/8] dpll/ice: Add TXC DPLL type and full TX reference clock control for E825
Date: Thu, 16 Apr 2026 08:27:51 -0700	[thread overview]
Message-ID: <20260416082751.04782987@kernel.org> (raw)
In-Reply-To: <IA0PR11MB737842E2098D0952A8BA1FE29B222@IA0PR11MB7378.namprd11.prod.outlook.com>

On Wed, 15 Apr 2026 13:23:22 +0000 Kubalewski, Arkadiusz wrote:
> >> Well, the true is that we did not anticipated per-port control of the
> >> TX clock source, as a single DPLL device could drive multiple of such.
> >>
> >> This is not true, that we pretend there is a second PLL - there is a
> >> PLL on each TX clock, maybe not a full DPLL, but still the loop with
> >> a control over it's sources is there and it has the same 2 external
> >> sources + default XO.  
> >
> >Don't we put that MAC PLL into bypass mode if we feed a clock from
> >the EEC DPLL?  
> 
> This HW doesn't use EEC DPLL signal to feed MAC clock, as DPLL is
> external from NIC point of view. Only 2 signals from such external DPLL
> device are used by NIC:
> - synce (a single source for all those TXC per-port DPLL device)
> - time_ref (a source for the TS_PLL - which drives PTP timer)

No bypass? The PLL is actually in the loop? oof, this is beyond 
my understanding of clocks and signals :S

> >> A mentioned try of adding per port MUX-type pin, just to give some
> >>control
> >> to the user, is where we wanted to simplify things, but in the end the
> >>API
> >> would have to be modified in significant way, various paths related to
> >>pin
> >> registration and keeping correct references, just to make working case
> >> for the pin_on_pin_register and it's internals. We decided that the
> >>burden
> >> and impact for existing design was to high.
> >>
> >> And that is why the TXC approach emerged, the change of DPLL is minimal,
> >> The model is still correct from user perspective, SyncE SW controller
> >>shall
> >> anticipate possibility that per-port TXC dpll is there  
> >
> >We are starting to push into what was previously the domain of
> >drivers/clk, tho. IIUC the "ASIC PLL"s are usually integrated with
> >clock dividers. And cannot be "configured" after chip init / async
> >reset (which is why I presume you whack a reset in patch 7?).  
> 
> Well, we need CGU-dividers change for a frequency-compliance with lower
> link speeds, the link reset which is required as part of tx-clk switch
> and link establishment on a new clock.
> 
> >  
> >> This particular device and driver doesn't implement any EEC-type DPLL
> >> device, the one could think that we can just change the type here and
> >>use
> >> EEC type instead of new one TXC - since we share pins from external dpll
> >> driver, which is EEC type, and our DPLL device would have different
> >>clock_id
> >> and module. But, further designs, where a single NIC is having control
> >>over
> >> both a EEC DPLL and ability to control each source per-port this would
> >>be
> >> problematic. At least one NIC Port driver would have to have 2 EEC-type
> >>DPLLs
> >> leaving user with extra confusion.  
> >
> >The distinction between TXC and EEC dpll is confusing.
> >I thought EEC one _was_supposed_to_ drive the Tx clock?
> >What PPS means is obvious, what EEC means if not driving Tx clock is
> >unclear to me..
> >  
> 
> Yes, correct, EEC DPLL main task would be to drive TX clocks of NIC
> ports, but if there is a per-port control something extra is required.
> 
> >Let me summarize my concerns - we need to navigate the split between
> >drivers/clk and dpll. We need a distinction on what goes where, because
> >every ASIC has a bunch of PLLs which until now have been controlled by
> >device tree (if at all). If the main question we want to answer is
> >"which clock ref is used to drive internal clock" all we need is a MUX.
> >If we want to make dpll cover also ASIC PLLs for platforms without
> >device tree we need a more generic name than TXC, IMHO.  
> 
> Well, 'floating' MUX type pin not connected to any dpll would require a
> lot of additional implementations, just to allow source selection, as we
> have tried it already.
> 
> Wouldn't more generic name cause a DPLL purpose problem?

The old proposal in netdev family was to to have source selection
without creating a real mux. Not saying I'm dead set on that direction.

> We still want to make sure that given DPLL device would serve the role
> of source selection for particular port where a source pin should be an
> output either on EEC dpll or some external signal generator but somehow
> related to SyncE or similar solutions.

Right, but adding a new "type" per location of the PLL (especially if
we lean into covering any ASIC PLL) may not scale, and opens us up to
"vendor X calls it Y" and "in design A clock is fed by pll type X and
in design B by type Y".

IIUC you do provide "linking" of the pins? netdev will have the MAC pin
assigned. Is the pin that connects the PLLs also annotated so that user
knows what's on the "other side"? Maybe the topology would be clear
enough from just that, and we don't have to add a TXC type.
Call the PLL "integrated" or something generic. User should be able to
trace the path of the signals?

  reply	other threads:[~2026-04-16 15:27 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-02 23:06 [PATCH v5 net-next 0/8] dpll/ice: Add TXC DPLL type and full TX reference clock control for E825 Grzegorz Nitka
2026-04-02 23:06 ` [PATCH v5 net-next 1/8] dpll: add new DPLL type for transmit clock (TXC) usage Grzegorz Nitka
2026-04-02 23:06 ` [PATCH v5 net-next 2/8] dpll: allow registering FW-identified pin with a different DPLL Grzegorz Nitka
2026-04-02 23:06 ` [PATCH v5 net-next 3/8] dpll: extend pin notifier and netlink events with notification source ID Grzegorz Nitka
2026-04-03 11:53   ` Jiri Pirko
2026-04-02 23:06 ` [PATCH v5 net-next 4/8] dpll: zl3073x: allow SyncE_Ref pin state change Grzegorz Nitka
2026-04-08  9:44   ` Ivan Vecera
2026-04-02 23:06 ` [PATCH v5 net-next 5/8] ice: introduce TXC DPLL device and TX ref clock pin framework for E825 Grzegorz Nitka
2026-04-02 23:06 ` [PATCH v5 net-next 6/8] ice: implement CPI support for E825C Grzegorz Nitka
2026-04-02 23:06 ` [PATCH v5 net-next 7/8] ice: add Tx reference clock index handling to AN restart command Grzegorz Nitka
2026-04-02 23:06 ` [PATCH v5 net-next 8/8] ice: implement E825 TX ref clock control and TXC hardware sync status Grzegorz Nitka
2026-04-07  2:23 ` [PATCH v5 net-next 0/8] dpll/ice: Add TXC DPLL type and full TX reference clock control for E825 Jakub Kicinski
2026-04-09 11:21   ` Nitka, Grzegorz
2026-04-10  1:10     ` Jakub Kicinski
2026-04-10 14:23       ` Nitka, Grzegorz
2026-04-10 20:38         ` Jakub Kicinski
2026-04-12 13:50           ` Nitka, Grzegorz
2026-04-12 14:50             ` Jakub Kicinski
2026-04-13  8:19           ` Kubalewski, Arkadiusz
2026-04-13 17:40             ` Jakub Kicinski
2026-04-14 21:58             ` Jakub Kicinski
2026-04-15 13:23               ` Kubalewski, Arkadiusz
2026-04-16 15:27                 ` Jakub Kicinski [this message]
2026-04-16 18:26                   ` [Intel-wired-lan] " Kubalewski, Arkadiusz
2026-04-17  1:04                     ` Jakub Kicinski
2026-04-17 12:22                       ` Kubalewski, Arkadiusz

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=20260416082751.04782987@kernel.org \
    --to=kuba@kernel.org \
    --cc=Prathosh.Satish@microchip.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=anthony.l.nguyen@intel.com \
    --cc=arkadiusz.kubalewski@intel.com \
    --cc=davem@davemloft.net \
    --cc=donald.hunter@gmail.com \
    --cc=edumazet@google.com \
    --cc=grzegorz.nitka@intel.com \
    --cc=horms@kernel.org \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=ivecera@redhat.com \
    --cc=jiri@resnulli.us \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=poros@redhat.com \
    --cc=przemyslaw.kitszel@intel.com \
    --cc=richardcochran@gmail.com \
    --cc=vadim.fedorenko@linux.dev \
    /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