From: Jakub Kicinski <kuba@kernel.org>
To: "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@intel.com>
Cc: "Vecera, Ivan" <ivecera@redhat.com>,
"vadim.fedorenko@linux.dev" <vadim.fedorenko@linux.dev>,
"edumazet@google.com" <edumazet@google.com>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"richardcochran@gmail.com" <richardcochran@gmail.com>,
"donald.hunter@gmail.com" <donald.hunter@gmail.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"davem@davemloft.net" <davem@davemloft.net>,
"Prathosh.Satish@microchip.com" <Prathosh.Satish@microchip.com>,
"andrew+netdev@lunn.ch" <andrew+netdev@lunn.ch>,
"intel-wired-lan@lists.osuosl.org"
<intel-wired-lan@lists.osuosl.org>,
"horms@kernel.org" <horms@kernel.org>,
"Kitszel, Przemyslaw" <przemyslaw.kitszel@intel.com>,
"Nguyen, Anthony L" <anthony.l.nguyen@intel.com>,
"pabeni@redhat.com" <pabeni@redhat.com>,
"jiri@resnulli.us" <jiri@resnulli.us>
Subject: Re: [Intel-wired-lan] [PATCH v5 net-next 0/8] dpll/ice: Add TXC DPLL type and full TX reference clock control for E825
Date: Tue, 14 Apr 2026 14:58:35 -0700 [thread overview]
Message-ID: <20260414145835.07fbe355@kernel.org> (raw)
In-Reply-To: <IA0PR11MB737882B384AE7279EBCD05C79B242@IA0PR11MB7378.namprd11.prod.outlook.com>
On Mon, 13 Apr 2026 08:19:30 +0000 Kubalewski, Arkadiusz wrote:
>> My concern is that I think this is a pretty run of the mill SyncE
>> design. If we need to pretend we have two DPLLs here if we really
>> only have one and a mux - then our APIs are mis-designed :(
>
> 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?
> 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?).
> 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..
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.
prev parent reply other threads:[~2026-04-14 21:58 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-02 23:06 [Intel-wired-lan] [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 ` [Intel-wired-lan] [PATCH v5 net-next 1/8] dpll: add new DPLL type for transmit clock (TXC) usage Grzegorz Nitka
2026-04-02 23:06 ` [Intel-wired-lan] [PATCH v5 net-next 2/8] dpll: allow registering FW-identified pin with a different DPLL Grzegorz Nitka
2026-04-02 23:06 ` [Intel-wired-lan] [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 ` [Intel-wired-lan] [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 ` [Intel-wired-lan] [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 ` [Intel-wired-lan] [PATCH v5 net-next 6/8] ice: implement CPI support for E825C Grzegorz Nitka
2026-04-02 23:06 ` [Intel-wired-lan] [PATCH v5 net-next 7/8] ice: add Tx reference clock index handling to AN restart command Grzegorz Nitka
2026-04-02 23:06 ` [Intel-wired-lan] [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 ` [Intel-wired-lan] [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 [this message]
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=20260414145835.07fbe355@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=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=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