RE: [Intel-wired-lan] [PATCH v5 net-next 0/8] dpll/ice: Add TXC DPLL type and full TX reference clock control for E825

[Nitka, Grzegorz]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@xxxxxxxxxx> On Behalf Of
> Kubalewski, Arkadiusz
> Sent: Monday, April 20, 2026 4:52 PM
> To: Jakub Kicinski <kuba@xxxxxxxxxx>
> Cc: Vecera, Ivan <ivecera@xxxxxxxxxx>; vadim.fedorenko@xxxxxxxxx;
> edumazet@xxxxxxxxxx; netdev@xxxxxxxxxxxxxxx;
> richardcochran@xxxxxxxxx; donald.hunter@xxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; davem@xxxxxxxxxxxxx;
> Prathosh.Satish@xxxxxxxxxxxxx; andrew+netdev@xxxxxxx; intel-wired-
> lan@xxxxxxxxxxxxxxxx; horms@xxxxxxxxxx; Kitszel, Przemyslaw
> <przemyslaw.kitszel@xxxxxxxxx>; Nguyen, Anthony L
> <anthony.l.nguyen@xxxxxxxxx>; pabeni@xxxxxxxxxx; jiri@xxxxxxxxxxx
> 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
> 
> >From: Jakub Kicinski <kuba@xxxxxxxxxx>
> >Sent: Saturday, April 18, 2026 9:26 PM
> >
> >On Fri, 17 Apr 2026 12:22:05 +0000 Kubalewski, Arkadiusz wrote:
> >> >> I was thinking that this is more like a purpose specific DPLL device,
> >> >> if
> >> >> someone would want something similar we would have to review it,
> >> >> right?
> >> >
> >> >We would if it was a Ethernet MAC PLL, but if someone wanted to expose
> >> >whether some random PLL in their ASIC locks - are we adding a new type
> >> >for each one of those?
> >>
> >> Yes, that was the implicit intention within those patches, if other
> >> purpose
> >> specific PLL would have to be present for whatever HW design and user
> >> control over it would be required, then that would be the easiest to
> >> maintain in the long term? Multiple types and each have own
> >> function/purpose.
> >>
> >> It would be good as long as there is one PLL for a function per board,
> >> once
> >> there could be multiple ones for single function, we would have to add
> >> some
> >> enumeration (labels, etc.)
> >
> >Defer on adding identifiers. User knows which driver and bus device
> >spawned the pll and more importantly what the pin topology is.
> >Naming in the kernel is rarely a good idea.
> 
> Sure.
> 
> >
> >> >> It depends, TX clock has one of external pins connected to external
> >> >> DPLL,
> >> >> but second is a board-level pin with ability to provide some external
> >> >> clock signal, the user would have to determine that purpose just
> >> >> based
> >> >> on the topology of one of the pins, which seems a bit problematic?
> >> >> I.e. if at some point there would be HW with only external non-DPLL
> >> >> connected pins?
> >> >
> >> >Not sure I follow, TBH. To me the function of the "MAC PLL" is fairly
> >> >obvious from the fact that it has a pin exposed via rtnetlink. So it's
> >> >obviously a DPLL which can drive the Tx clock?
> >>
> >> I am lost a bit now too. You mean clock recovery pin? And EEC type dpll?
> >> In this solution the 'MAC'/EEC is external and it doesn't drive TX
> >> clocks
> >> directly.
> >
> >MAC == "tspll" == TXC in this series. On Grzegorz's diagram the new PLL
> >was in the MAC, which makes sense since it's a pll in the same ASIC as
> >the MAC.
> >
> 
> We wanted the TSPLL from the picture to be PPS type as it drives the PHC
> timer within the MAC.
> 
> >I'm saying that the function of that pll is obvious since its pin will
> >plug into the netdev / rtnetlink.
> >
> 
> Yeah I got it, just saying it will work for now :)
> 
> >> >It's the function / relation / linking to the EEC DPLL that may not
> >> >be obvious. But user can see how the pins connect they can get some
> >> >LLM to draw a diagram of a live system.. et voila :)
> >>
> >> Yes, correct it would work for this particular HW, but adding a variant
> >> without a external EEC-connected pin in the picture would be problematic
> >> to understand 'generic' dpll purpose, pointing to the labels later.
> >
> >The function of the "MAC/tspll" is still obvious. The clarity of the
> >external PLL is not helped by naming the "MAC/tspll".
> >
> >> Just to make it clear. I believe that generic type dpll could be used in
> >> any HW and for any purpose, so after all each such usage could possibly
> >> introduce entropy and confusion on the user side.
> >>
> >> But if you are fine with that, then sure, we can live with generic
> >> purpose dpll.
> >
> >Considering all the imperfect options - generic / unnamed type would be
> >my preference.
> 
> Ok, sounds good.
> 
> Thank you!
> Arkadiusz

Thanks for the fruitful discussion. Just submitted v7 in which DPLL_TYPE_GENERIC
Has been introduced (instead of DPLL_TYPE_TXC).

Regards

Grzegorz