Netdev List
 help / color / mirror / Atom feed
From: Jiri Pirko <jiri@resnulli.us>
To: Ivan Vecera <ivecera@redhat.com>
Cc: "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@intel.com>,
	 Vadim Fedorenko <vadim.fedorenko@linux.dev>,
	Jakub Kicinski <kuba@kernel.org>,
	 "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Jiri Pirko <jiri@nvidia.com>,
	 "David S. Miller" <davem@davemloft.net>,
	Donald Hunter <donald.hunter@gmail.com>,
	 Eric Dumazet <edumazet@google.com>,
	"Schmidt, Michal" <mschmidt@redhat.com>,
	 Paolo Abeni <pabeni@redhat.com>,
	"Vaananen, Pasi" <pvaanane@redhat.com>,
	 "Oros, Petr" <poros@redhat.com>,
	Prathosh Satish <Prathosh.Satish@microchip.com>,
	 Simon Horman <horms@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net-next v5 1/4] dpll: add DPLL_PIN_TYPE_INT_NCO pin type
Date: Tue, 23 Jun 2026 12:37:50 +0200	[thread overview]
Message-ID: <ajphdp2Bfsp_54ME@FV6GYCPJ69> (raw)
In-Reply-To: <23e47140-f69f-451d-9154-29071130c11c@redhat.com>

Fri, Jun 19, 2026 at 07:07:52PM +0200, ivecera@redhat.com wrote:
>On 6/17/26 1:59 PM, Kubalewski, Arkadiusz wrote:
>> > From: Ivan Vecera <ivecera@redhat.com>
>> > Sent: Monday, June 15, 2026 2:00 PM
>> > 
>> > On 6/11/26 2:09 PM, Jiri Pirko wrote:
>> > > Wed, Jun 10, 2026 at 05:45:46PM +0200, ivecera@redhat.com wrote:
>> > > > On 6/10/26 3:04 PM, Kubalewski, Arkadiusz wrote:
>> > > > > > From: Ivan Vecera <ivecera@redhat.com>
>> > > > > > Sent: Tuesday, June 9, 2026 4:59 PM
>> > > > > > 
>> > > > > > On 6/9/26 4:00 PM, Kubalewski, Arkadiusz wrote:
>> > > > > > > > From: Jiri Pirko <jiri@resnulli.us>
>> > > > > > > > Sent: Tuesday, June 9, 2026 10:51 AM
>> > > > > > > > 
>> > > > > > > > Mon, Jun 08, 2026 at 07:03:46PM +0200,
>> > > > > > > > arkadiusz.kubalewski@intel.com
>> > > > > > > > wrote:
>> > > > > > > > > > From: Ivan Vecera <ivecera@redhat.com>
>> > > > > > > > > > Sent: Monday, June 8, 2026 5:48 PM
>> > > > > > > > > > 
>> > > > > > > > > > On 6/8/26 4:43 PM, Kubalewski, Arkadiusz wrote:
>> > > > > > > > > > > > From: Ivan Vecera <ivecera@redhat.com>
>> > > > > > > > > > > > Sent: Sunday, May 31, 2026 9:44 PM ...
>> > > > > > > > > > > >            -
>> > > > > > > > > > > >              name: gnss
>> > > > > > > > > > > >              doc: GNSS recovered clock
>> > > > > > > > > > > > +      -
>> > > > > > > > > > > > +        name: int-nco
>> > > > > > > > > > > > +        doc: |
>> > > > > > > > > > > > +          Device internal numerically controlled oscillator.
>> > > > > > > > > > > > +          When connected as a DPLL input, the DPLL enters NCO
>> > > > > > > > > > > > mode
>> > > > > > > > > > > > +          where the output frequency is adjusted by the host
>> > > > > > > > > > > > via
>> > > > > > > > > > > > +          the PTP clock interface.
>> > > > > > > > > > > 
>> > > > > > > > > > > Hi Ivan!
>> > > > > > > > > > > 
>> > > > > > > > > > > How would you control this in case of automatic mode dpll?
>> > > > > > > > > > > Automatic mode DPLL shall be controlled on HW level, such pin
>> > > > > > > > > > > brakes that rule and requires some driver magic to show it is
>> > > > > > > > > > > higher priority then the rest of the pins?
>> > > > > > > > > > 
>> > > > > > > > > > The NCO pin can be connected only in manual mode. In other words
>> > > > > > > > > > a
>> > > > > > > > > > DPLL in automatic mode cannot select NCO pin (switch to NCO mode)
>> > > > > > > > > > by
>> > > > > > > > > > its own.
>> > > > > > > > > > 
>> > > > > > > > > 
>> > > > > > > > > Being picky on DPLL_MODE for enabling feature is not something we
>> > > > > > > > > can allow if it is not related to HW limitation, is it?
>> > > > > > > > > Could you please elaborate why it is not possible for AUTOMATIC
>> > > > > > > > > mode?
>> > > > > > > > 
>> > > > > > > > In automatic mode, the pin selection logic is defined upon prio. I
>> > > > > > > > can imagine that if NCO pin has the highest prio of the available
>> > > > > > > > ones, it gets picked. I would be aligned 100% with automatic mode
>> > > > > > > > behaviour.
>> > > > > > > > Is there a real usecase for it?
>> > > > > > > > 
>> > > > > > > > [..]
>> > > > > > > 
>> > > > > > > This is not true. AUTOMATIC mode is HW solution, SW driver ONLY
>> > > > > > > configures priorities on the inputs, not manages the active inputs.
>> > > > > > > This brakes that behavior, the SW driver would have to manually
>> > > > > > > override the AUTMATIC mode to be fed from such NCO pin as it doesn't
>> > > > > > > exists on it's priority list, HW cannot pick or use it.
>> > > > > > 
>> > > > > > Correct, AUTO mode is hardware feature and it should not be emulated
>> > > > > > by a
>> > > > > > driver. If the hardware does not support it then the switching
>> > > > > > between
>> > > > > > input references should be done by userspace (by monitoring ffo,
>> > > > > > phase_offset, operstate).
>> > > > > > 
>> > > > > 
>> > > > > Yes, exactly, so for AUTOMATIC mode HW it will not be possible to
>> > > > > create
>> > > > > such pin, which means that NCO pin would serve only a MANUAL mode
>> > > > > implementation.
>> > > > > Basically this is something we shall not allow to happen. DPLL API
>> > > > > should be designed to cover the case where AUTO mode is able to
>> > > > > implement
>> > > > > all features consistently.
>> > > > 
>> > > > If you don't like the proposal from Jiri (NCO switch driven by NCO pin
>> > > > priority -> highest==enter_nco else leave_nco) then it could be
>> > > > possible
>> > > > to handle the switching by allowing the state 'connected' in AUTO mode
>> > > > for the NCO pin type. Then the implementation will be the same for both
>> > > > selection modes.
>> > > > 
>> > > > Only difference would be that a user does not need to switch the device
>> > > >from the AUTO to MANUAL mode.
>> > > > 
>> > > > > > > The real use case is that any DPLL can switch the mode to this one
>> > > > > > > instead of implementing MANUAL mode just to use the feature with a
>> > > > > > > 'virtual' pin.
>> > > > > > 
>> > > > > > I don't expect this... but it is up to a driver. I don't plan such
>> > > > > > functionality in zl3073x as the NCO pin does not expose prio_get()
>> > > > > > and
>> > > > > > prio_set() callbacks - so it is clear that this pin cannot be part of
>> > > > > > the
>> > > > > > automatic selection.
>> > > > > > 
>> > > > > > Ivan
>> > > > > 
>> > > > > There is a difference between particular HW and API capabilities, with
>> > > > > the
>> > > > > proposed API we would disallow the possibility of such implementation
>> > > > > for
>> > > > > existing HW variants.
>> > > > > 
>> > > > > DPLL NCO MODE would allow that but as pointed here by Ivan and by Jiri
>> > > > > in
>> > > > > the other email it would also require the extra implementation for
>> > > > > some
>> > > > > configuration - device level phase/ffo handling.
>> > > > > 
>> > > > > To summarize it all, I don't have such simple solution for it.
>> > > > > 
>> > > > > First thing that comes to my mind is to combine both approaches.
>> > > > > Make it possible for AUTMATIC mode to also set "CONNECTED" state
>> > > > > on certain kind of "OVERRIDE" pins, where it could be determined by
>> > > > > the type of PIN and embed that logic into the DPLL subsystem.
>> > > > 
>> > > > The possible states for particual pins are now handled at a driver
>> > > > level
>> > > > so the driver decides if the requested state is correct or not. So it
>> > > > could be easy to implement this.
>> > > > 
>> > > > For auto mode allowed states:
>> > > > - input references: selectable / disconnected
>> > > > - nco pin: connected / disconnected
>> > > > 
>> > > > > Basically, if driver registers such NCO pin it would be always
>> > > > > selected
>> > > > > manually, and in such case all the other pins are going to
>> > > > > disconnected
>> > > > > state while DPLL mode is also a "OVERRIDE" or something like it.
>> > > > 
>> > > > I would leave this decision on the driver level... Imagine the
>> > > > potential
>> > > > HW that would allow to switch NCO mode if there is no valid input
>> > > > reference.
>> > > > 
>> > > > Example:
>> > > > 
>> > > > REF0 (prio 0) -> +------+ -> OUT0
>> > > > REF1 (prio 1) -> | DPLL | -> ...
>> > > > NCO  (prio 2) -> +------+ -> OUTn
>> > > > 
>> > > > Such HW would prefer REF0 or REF1 and lock to one of them if they are
>> > > > qualified. But if they are NOT, then it switches to NCO mode.
>> 
>> Now you said yourself "NCO mode" ... I agree that it would be a mode in
>> that case. Where instead of running on regular/built in XO dpll would run
>> on NCO and user could select it, and this would be addition to regular
>> behavior.
>> 
>> I also agree that the pin approach might be better/easier to use, assuming
>> frequency offset for all the outputs given dpll drives, it makes more sense
>> to have it configurable on input side.
>
>+1
>
>> > > > 
>> > > > In this situation the relevant driver would allow to configure priority
>> > > > and state 'selectable' for this NCO pin.
>> > > > 
>> > > > > Perhaps the pin type could include OVERRIDE in it's name to make it
>> > > > > less
>> > > > > confusing and needs some extra documentation.
>> > > > > 
>> > > > > Thoughts?
>> > > > I think _INT_ is ok. In the case of TYPE_INT_OSCILLATOR it is also
>> > > > obvious that it is not a standard input reference.
>> > > > 
>> > > > Jiri, Vadim, Arek, thoughts?
>> > > 
>> > > I agree with you, the driver should have the flexibility to implement
>> > > this according to his/hw's needs/capabilities. If it implements prio
>> > > selection in AUTO mode, let it have it. If it implements manual NCO pin
>> > > selection in AUTO mode using connected/disconnected override, let it
>> > > have it.
>> 
>> I don't know 'current' HW that is capable of using AUTO mode as a part of
>> HW-based priority source selection and use such NCO input..
>> But as already explained above, this is special mode of regular XO, which
>> allows DPLL's output frequency offset configuration.
>
>Lets keep this available for potential future HW. I can imagine a
>situation where a user will prefer an automatic switch to NCO mode
>if there is no qualified input reference - automatic switch means
>that HW will support this (not emulated by the driver).
>
>> > > 
>> > > Moreover, I actually like the "override" capability for pins in AUTO
>> > > mode in general. It may be handy for other usecases as well.
>> > > 
>> > Arek? Vadim?
>> > 
>> > Thanks,
>> > Ivan
>> 
>> Agree, 'override' capability of a pin would be the way to go for this and
>> other similar further cases.
>> 
>> I believe a single approach on this would be best, I mean if AUTO mode
>> needs a capability, to switch from regular behavior to 'OVERRIDE', and
>> 'OVERRIDE' is only pin capability that allows such behavior for AUTO
>> mode, then similar approach should be used on MANUAL mode, to make
>> userspace know that such pin is always available to set "CONNECTED"
>> and make the userspace implementation consistent on enabling it no matter
>> if AUTO or MANUAL mode dpll.
>
>Proposal:
>1) new pin capability
>   - name: state-connected-override
>   - doc: pin state can be changed to connected in any DPLL mode

Needs a bit more description I think in real patch.


>
>2) new NCO pin type to switch the DPLL to NCO mode when connected

Say "NCO hw mode" to avoid confusion (I already spotted such a bit
earlier in this thread)


>
>3) automatic-only DPLL
>   - should expose NCO pin with state-connected-override capability
>
>4) manual-only DPLL
>  - does not need to expose NCO pin with state-connected-override cap
>
>5) dual-mode DPLL (supporting mode switching)
>  - if it exposes NCO pin with the override cap then it has to support
>    switching to NCO mode directly from AUTO mode
>  - if does not expose NCO pin with the override cap then a user MUST
>    switch the DPLL mode from AUTO to MANUAL to be able to make NCO
>    pin connected to the DPLL
>
>Vadim, Jiri, Arek - thoughts?

Agreed.

>
>Thanks,
>Ivan
>

  parent reply	other threads:[~2026-06-23 10:37 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-31 19:44 [PATCH net-next v5 0/4] dpll: add NCO pin type and zl3073x support Ivan Vecera
2026-05-31 19:44 ` [PATCH net-next v5 1/4] dpll: add DPLL_PIN_TYPE_INT_NCO pin type Ivan Vecera
2026-06-04  1:50   ` Jakub Kicinski
2026-06-04 15:01     ` Ivan Vecera
2026-06-04 15:16       ` Jakub Kicinski
2026-06-04 16:42         ` Ivan Vecera
2026-06-08 11:33           ` Vadim Fedorenko
2026-06-08 15:30             ` Ivan Vecera
2026-06-09 10:24               ` Vadim Fedorenko
2026-06-09 15:09                 ` Ivan Vecera
2026-06-09 11:06             ` Jiri Pirko
2026-06-08 14:43   ` Kubalewski, Arkadiusz
2026-06-08 15:48     ` Ivan Vecera
2026-06-08 17:03       ` Kubalewski, Arkadiusz
2026-06-09  8:51         ` Jiri Pirko
2026-06-09 14:00           ` Kubalewski, Arkadiusz
2026-06-09 14:58             ` Ivan Vecera
2026-06-10 13:04               ` Kubalewski, Arkadiusz
2026-06-10 15:45                 ` Ivan Vecera
2026-06-11 12:09                   ` Jiri Pirko
2026-06-15 12:00                     ` Ivan Vecera
2026-06-17 11:59                       ` Kubalewski, Arkadiusz
2026-06-19 17:07                         ` Ivan Vecera
2026-06-22 14:47                           ` Kubalewski, Arkadiusz
2026-06-23 10:37                           ` Jiri Pirko [this message]
2026-05-31 19:44 ` [PATCH net-next v5 2/4] dpll: zl3073x: use per-operation poll timeouts Ivan Vecera
2026-05-31 19:44 ` [PATCH net-next v5 3/4] dpll: zl3073x: add per-DPLL serialization lock Ivan Vecera
2026-06-04  1:40   ` Jakub Kicinski
2026-06-04 14:57     ` Ivan Vecera
2026-06-04  1:51   ` Jakub Kicinski
2026-06-04 15:12     ` Ivan Vecera
2026-05-31 19:44 ` [PATCH net-next v5 4/4] dpll: zl3073x: add NCO virtual input pin Ivan Vecera

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=ajphdp2Bfsp_54ME@FV6GYCPJ69 \
    --to=jiri@resnulli.us \
    --cc=Prathosh.Satish@microchip.com \
    --cc=arkadiusz.kubalewski@intel.com \
    --cc=davem@davemloft.net \
    --cc=donald.hunter@gmail.com \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=ivecera@redhat.com \
    --cc=jiri@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mschmidt@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=poros@redhat.com \
    --cc=pvaanane@redhat.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