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

Kubalewski, Arkadiusz posted 8 patches 1 month, 3 weeks ago
Only 0 patches received!
There is a newer version of this series
RE: [Intel-wired-lan] [PATCH v5 net-next 0/8] dpll/ice: Add TXC DPLL type and full TX reference clock control for E825
Posted by Kubalewski, Arkadiusz 1 month, 3 weeks ago
>From: Jakub Kicinski <kuba@kernel.org>
>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
RE: [Intel-wired-lan] [PATCH v5 net-next 0/8] dpll/ice: Add TXC DPLL type and full TX reference clock control for E825
Posted by Nitka, Grzegorz 1 month, 2 weeks ago
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Kubalewski, Arkadiusz
> Sent: Monday, April 20, 2026 4:52 PM
> To: Jakub Kicinski <kuba@kernel.org>
> Cc: Vecera, Ivan <ivecera@redhat.com>; vadim.fedorenko@linux.dev;
> edumazet@google.com; netdev@vger.kernel.org;
> richardcochran@gmail.com; donald.hunter@gmail.com; linux-
> kernel@vger.kernel.org; davem@davemloft.net;
> Prathosh.Satish@microchip.com; andrew+netdev@lunn.ch; intel-wired-
> lan@lists.osuosl.org; horms@kernel.org; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; pabeni@redhat.com; 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
> 
> >From: Jakub Kicinski <kuba@kernel.org>
> >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