All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.