RE: [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: Jakub Kicinski <kuba@xxxxxxxxxx>
> Sent: Friday, April 10, 2026 3:11 AM
> To: Nitka, Grzegorz <grzegorz.nitka@xxxxxxxxx>
> Cc: netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; intel-wired-
> lan@xxxxxxxxxxxxxxxx; Oros, Petr <poros@xxxxxxxxxx>;
> richardcochran@xxxxxxxxx; andrew+netdev@xxxxxxx; Kitszel, Przemyslaw
> <przemyslaw.kitszel@xxxxxxxxx>; Nguyen, Anthony L
> <anthony.l.nguyen@xxxxxxxxx>; Prathosh.Satish@xxxxxxxxxxxxx; Vecera,
> Ivan <ivecera@xxxxxxxxxx>; jiri@xxxxxxxxxxx; Kubalewski, Arkadiusz
> <arkadiusz.kubalewski@xxxxxxxxx>; vadim.fedorenko@xxxxxxxxx;
> donald.hunter@xxxxxxxxx; horms@xxxxxxxxxx; pabeni@xxxxxxxxxx;
> davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx
> Subject: Re: [PATCH v5 net-next 0/8] dpll/ice: Add TXC DPLL type and full TX
> reference clock control for E825
> 
> On Thu, 9 Apr 2026 11:21:35 +0000 Nitka, Grzegorz wrote:
> > > On Fri,  3 Apr 2026 01:06:18 +0200 Grzegorz Nitka wrote:
> > > > This series adds TX reference clock support for E825 devices and
> exposes
> > > > TX clock selection and synchronization status via the Linux DPLL
> > > > subsystem.
> > > > E825 hardware contains a dedicated Tx clock (TXC) domain that is
> > > > distinct
> > > > from PPS and EEC. TX reference clock selection is device‑wide, shared
> > > > across ports, and mediated by firmware as part of the link bring‑up
> > > > process. As a result, TX clock selection intent may differ from the
> > > > effective hardware configuration, and software must verify the
> outcome
> > > > after link‑up.
> > > > To support this, the series introduces TXC support incrementally across
> > > > the DPLL core and the ice driver:
> > > >
> > > > - add a new DPLL type (TXC) to represent transmit clock generators;
> > >
> > > I'm not grasping why this is needed, isn't it part of any EEC system
> > > that the DPLL can drive the TXC? Is your system going to expose multiple
> > > DPLLs now for one NIC?
> >
> > Hello Jakub,
> > For E825 device, the short answer is yes. We have platform EEC now and
> > we want to add:
> > - TXC DPLLs per port, and
> > - PPS DPLL for TSPLL config purposes (in the near future)
> >
> > EEC (Ethernet Equipment Clock) type DPLL is designed to control multiple
> > source signals (internal-NIC or external), where one drives the dpll device,
> > where multiple outputs are possible, each could drive various components
> > as well as propagate signal to external devices.
> > TXC is specific dpll device that associated with single ETH port to control it's
> source,
> > there is no need to declare any outputs as the single output is already
> determined.
> > Basically, having TXC DPLL indicates per port control over SyncE (or some
> external)
> > clock source.
> 
> Could you share a diagram of how things are wired up?
> DPLL can have multiple outputs and multiple inputs. I'm not getting why
> a single device would have to have multiple actual DPLLs (which makes
> me worried this is just some "convenient use of the uAPI")

Hello Jakub,

Here is the high-level connection diagram for E825 device. I hope you find it helpful:

  +------------------------------------------------------------------+        
  |                                                                  |        
  |                           +-----------------------------+        |        
  |                           |                             |        |        
  |                           |         MAC                 |        |        
  |                           |+------------+-----+         |        |        
  |                           ||RX/1588 |PHC|tspll<----\    |        |        
+---+----+                    ||MUX     +---+-^---|    |    |        |        
| E | RX >--------------------->              |   >--\ |    |        |        
| T |    |    /---------------->              |   >-\| |    |        |        
| H |----+    |               |+---------+----^---+ || |    |        |        
| 1 | TX <----|----------------+TX MUX   < OCXO   | || |    |        |        
|   |PLL |    |               ||         |--------| || |    |        |        
+---+----+    |           /----+         <-ext_ref<-||-|----|---------ext_ref 
| E | RX >----/           |   ||         |--------+ || |    |        |        
| T |    |                |   ||         <  SyncE | || |    |        |        
| H |----+                |   |+-----------^------+ || |    |        |        
| 2 | TX <----------------/   |            | /------||-/    |        |        
|   |PLL |                    +------------|-|------||------+        |        
+---+----+                              /--/ |      ||               |        
| . | RX >---                           |    |      ||               |        
| . |    |                   +----------|----|------||--+            |        
| . |----+                   |        +-^-+--^+     ||  |            |        
|   | TX <---                |        |EEC|PPS|     ||  |            | 
|   |PLL |                   |        +-------+     ||  |            |        
+---+----+                   |        |       <-CLK0/|  |            |        
| E | RX >---                |        |  DPLL |      |  |            |        
| T |    |                   |        |       <-CLK1-/  |            |        
| H |----+                   |        |       |         |            |        
| X | TX <---                |        |       <---SMA---<            |        
|   |PLL |                   |        |       |         |            |        
+---+----+                   |        |       <---GPS---<            |        
  |                          |        |       |         |            |      
  |                          |        |       <---...---<            |        
  |                          |        |       |         |            |        
  |                          |        +-------+         |            |        
  |                          | External timing module   |            |        
  |                          +--------------------------+            |        
  +-------------------------------------------------------------------+        
 
Before this series, we tried different approaches.
One of them was to create MUX pin associated with netdev interface.
EXT_REF and SYNCE pins were registered with this MUX pin.
However I recall there were at least two issues with this solution:
- when using DPLL subsystem not all the connections/relations were visible
  from DPLL pin-get perspective. RT netlink was required
- due to mixing pins from different modules (like fwnode based pin from zl driver
  and the pins from ice), we were not able to safely clean the references between
  pins and dpll (basicaly .. we observed crashes)

Proposed solution just seems to be clean and fully reflects current
connection topology.

What's actually your biggest concern?
The fact we introduce a new DPLL type? Or multiply DPLL instances? Or both?
Do you prefer to see "one big" DPLL with 16 pins in our case (8 ports x 2 tx-clk pins)?
Each pin with the name like, for example, PF0-SyncE/PF0-eRef etc.?