[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CACLnSDgy+mxqgy+fShC1kre05zF54tCfttqQTzzFkBt4f9UYog@mail.gmail.com>
Date: Thu, 15 Jan 2026 19:25:30 +0200
From: Vitaly Grinberg <vgrinber@...hat.com>
To: "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@...el.com>
Cc: "Nitka, Grzegorz" <grzegorz.nitka@...el.com>,
"Loktionov, Aleksandr" <aleksandr.loktionov@...el.com>,
"Nguyen, Anthony L" <anthony.l.nguyen@...el.com>, "horms@...nel.org" <horms@...nel.org>,
"intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>, "pmenzel@...gen.mpg.de" <pmenzel@...gen.mpg.de>,
"Kitszel, Przemyslaw" <przemyslaw.kitszel@...el.com>, "Fodor, Zoltan" <zoltan.fodor@...el.com>
Subject: Re: Re:[Intel-wired-lan] [PATCH v5 iwl-next] ice: add support for
unmanaged DPLL on E830 NIC
On Thu, Jan 15, 2026 at 5:30 PM Kubalewski, Arkadiusz
<arkadiusz.kubalewski@...el.com> wrote:
>
> >From: Vitaly Grinberg <vgrinber@...hat.com>
> >Sent: Thursday, January 15, 2026 9:58 AM
> >
> >, the ea
> >
> >On Wed, Jan 14, 2026 at 3:32 PM Kubalewski, Arkadiusz
> ><arkadiusz.kubalewski@...el.com> wrote:
> >>
> >> >From: Vitaly Grinberg <vgrinber@...hat.com>
> >> >Sent: Wednesday, January 14, 2026 12:39 PM
> >> >
> >>
> >> [..]
> >>
> >> >> >
> >> >> >I see a few challenges for the user here. The biggest one is that
> >> >> >the
> >> >> >application can't tell the difference between a device that will
> >> >> >report
> >> >> >phase offsets and this unmanaged device that never will.
> >> >> >A possible way forward would be adding a capability flag to the DPLL
> >> >> >API
> >> >> >so
> >> >> >users don't have to guess.
> >> >>
> >> >> There is no phase-offset field as pointed in the above example.
> >> >> No 'phase-offset' attribute -> no such capability.
> >> >> Why isn’t that enough?
> >> >
> >> >Pin reply does not contain phase offset, so no change notifications
> >> >are expected?
> >> >Could there be devices that don't report phase offset, but report state
> >> >changes?
> >> >Is this the intended use of the phase offset API to be interpreted as
> >> >a general pin
> >> >notification capability flag?
> >> >
> >>
> >> Sorry, this is not what I meant.
> >>
> >> The E810 produces notifications not only for the pin's phase offset but
> >> also for other pin attribute changes. When it comes to the E810 pins,
> >> notifications generated by phase offset changes are quite frequent.
> >> However, it wasn't intention to produce them every second; this is
> >simply
> >> the result of frequent phase offset changes.
> >>
> >> Typically, the pin state changes for the pin, but for E830, the
> >> unmanaged
> >> mode means that the state of the pin never changes, resulting in no pin
> >> notifications being produced in the end.
> >>
> >> Hope that clears things up.
> >
> >Will the reported pin state remain "connected" even if I disconnect
> >the input signal?
> >Is there any information in DPLL or pin replies that can tell the user
> >"this DPLL is unmanaged type, it behaves differently"?
>
> You really cannot disconnect anything there, it is always connected,
> and no one can change it. It only shows the user actual physical
> connections hardwired into the NIC/dpll. There is no SW handled config
> possible on those. As provided in the commit message, the pins have empty
> capabilities set: 'capabilities': set(), thus not possible to change state
> by the user.
>
Got it, thanks for clarification!
> >
> >>
> >> >>
> >> >> >However, the preferred solution would be to simply mirror the E810
> >> >> >behavior
> >> >> >(sending phase offset). This preserves the existing API contract and
> >> >> >prevents users, who have already built applications for this
> >> >> >interface,
> >> >> >from needing to implement special handling for a new hardware
> >> >> >variant
> >> >> >that
> >> >> >behaves differently.
> >> >>
> >> >> This is not currently possible from driver perspective.
> >> >> We miss the FW API for it.
> >> >>
> >> >> >There are additional inconsistencies in the existing structure I
> >> >> >wanted
> >> >> >to
> >> >> >bring to your attention.
> >> >> >1. I'm not entirely sure how a 1588-TIME_SYNC pin can have a parent
> >> >> >device
> >> >> >of type "eec". EEC is all about frequency synchronization, and yet
> >> >> >the
> >> >> >pin
> >> >> >named 1588-TIME_SYNC is clearly a phase pin. This also doesn't play
> >> >> >well
> >> >> >with existing implementations, where EEC circuits deal with
> >> >> >frequency,
> >> >> >PPS
> >> >> >circuits deal with phase, and there is clear distinction between the
> >> >> >two
> >> >> >with regard to the meaning of "being locked".
> >> >>
> >> >> This dpll device type was established based on the main purpose of
> >> >> >dpll
> >> >> device which is to drive the network ports phy clocks with it.
> >> >
> >> >What is the physical meaning of this indication (lock-status':
> >> >'locked')? Locked on what?
> >>
> >> Lock status is dpll device property.
> >>
> >> But full picture has to be determined from the list of pins, for this
> >> particular case, one input provided from host through pci-e pin, 10MHz
> >> bandwidth frequency and 1 PPS sync pulses.
> >>
> >> As already pointed the type of dpll shall let user know the purpose of
> >> the dpll existence instead of particular input properties.
> >> Input properties are determined with the pin's attributes.
> >>
> >> >As a user of this circuit I want to know that the device is locked on
> >> >the phase of the input signal with a certain precision.
> >> >Is this the meaning of "locked" here? Can an EEC device be locked on
> >> >the Phase of the input signal?
> >>
> >> Well I don't have any data on the precision of such, but AFAIK it can.
> >> EEC dpll shall be producing stable signal, the input it uses is only
> >> part of the full dpll device picture.
> >>
> >> >Users of other devices (e810, zl3073x) may have implemented logic to
> >> >determine the phase lock by
> >> >enforcing the pin parent device type as PPS. How should they change it
> >> >to determine phase lock (and why)?
> >> >
> >>
> >> I am Sorry, I don't understand the example above, could you please
> >> Elaborate on details of such setup?
> >
> >Yep, gladly!
> >Telco customers rely on the Phase being locked on the reference with a
> >certain precision. In E810 and zl3073x the equation is simple:
> >1. "eec locked" means synTonization achieved - frequency locked
> >2. "pps locked" means synChronization achieved - phase locked
> >The T-BC application checks the reported device type. If the device
> >type is "pps", the report is processed by the synchronization state
> >decision logic. Otherwise, the report doesn't have any meaning within
> >"T-BC without SyncE" context and is discarded.
> >
> >Since this patch is going to create eec reports only, they are all
> >going to be discarded, and this is a compatibility issue I'm trying to
> >address.
> >
>
> I see. From this angle the PPS type looks like a superset of EEC.
> It makes sense to me. We had also some discussion and agreed to propose
> new patch with the PPS type, as we don't want the underlying SW to be
> troubled with such issue.
Thanks very much for considering this!
> >Could you please answer my question above:
> >What is the physical meaning of this indication
> >(lock-status':'locked') in this report?
> >
>
> It means that dpll is locked/synchronized with the esync 10MHz/1PPS,
> a signal provided through pcie pin.
>
> Thank you!
> Arkadiusz
>
> >Thanks!
> >VG
> >
> >> Thank you!
> >> Arkadiusz
> >>
> >> >>
> >> >> >2. Since it is also an external embedded sync input pin, could it be
> >> >> >possible to expose this information and include `esync-frequency`
> >> >> >and
> >> >> >`esync-pulse`? That could be useful for configuring the leading DPLL
> >> >> >that
> >> >> >drives the unmanaged one.
> >> >>
> >> >> Sure, esync caps should be provided, as the commit message example
> >> >> >shown:
> >> >> + 'esync-frequency': 1,
> >> >> + 'esync-frequency-supported': [{'frequency-max': 1, 'frequency-
> >> >> >min':
> >> >>1}],
> >> >> + 'esync-pulse': 25,
> >> >>
> >> >
> >> >Oh, I must have missed that.
> >> >Thanks!
> >> >Vitaly
> >> >
> >> >> Thank you!
> >> >> Arkadiusz
> >>
>
Powered by blists - more mailing lists