Re: [RFC PATCH v4 0/4] Create common DPLL/clock configuration API

Maciek Machnikowski
2022-12-08T18:23:38Z

On 12/8/2022 12:21 AM, Jakub Kicinski wrote:
> On Wed, 7 Dec 2022 15:09:03 +0100 netdev.dump@gmail.com wrote:
>>> -----Original Message-----
>>> From: Jakub Kicinski <kuba@kernel.org>
>> pins between the DPLLs exposed by a single driver, but not really outside of
>> it.
>> And that can be done simply by putting the pin ptr from the DPLLA into the
>> pin
>> list of DPLLB.
>
> Are you saying within the driver it's somehow easier? The driver state
> is mostly per bus device, so I don't see how.
>
>> If we want the kitchen-and-sink solution, we need to think about corner
>> cases.
>> Which pin should the API give to the userspace app - original, or
>> muxed/parent?
>
> IDK if I parse but I think both. If selected pin is not directly
> attached the core should configure muxes.
>
>> How would a teardown look like - if Driver A registered DPLLA with Pin1 and
>> Driver B added the muxed pin then how should Driver A properly
>> release its pins? Should it just send a message to driver B and trust that
>> it
>> will receive it in time before we tear everything apart?
>
> Trivial.
>
>> There are many problems with that approach, and the submitted patch is not
>> explaining any of them. E.g. it contains the dpll_muxed_pin_register but no
>> free
>> counterpart + no flows.
>
> SMOC.
>
>> If we want to get shared pins, we need a good example of how this mechanism
>> can be used.
>
> Agreed.

My main complaint about the current pins implementation is that they put
everything in a single bag. In a netdev world - it would be like we put
TX queues and RX queues together, named them "Queues", expose a list to
the userspace and let the user figure out which ones which by reading a
"TX" flag.

All DPLLs I know have a Sources block, DPLLs and Output blocks. See:

https://www.renesas.com/us/en/products/clocks-timing/jitter-attenuators-frequency-translation/8a34044-multichannel-dpll-dco-four-eight-channels#overview

https://ww1.microchip.com/downloads/aemDocuments/documents/TIM/ProductDocuments/ProductBrief/ZL3063x-System-Synchronizers-with-up-to-5-Channels-10-Inputs-20-Outputs-Product-Brief-DS20006634.pdf

https://www.sitime.com/support/resource-library/product-briefs/cascade-sit9514x-clock-system-chip-family

https://www.ti.com/lit/ds/symlink/lmk5b33414.pdf?ts=1670516132647&ref_url=https%253A%252F%252Fwww.ti.com%252Fclocks-timing%252Fjitter-cleaners-synchronizers%252Fproducts.html

If we model everything as "pins" we won't be able to correctly extend
the API to add new features.

Sources can configure the expected frequency, input signal monitoring
(on multiple layers), expected signal levels, input termination and so
on. Outputs will need the enable flag, signal format, frequency, phase
offset etc. Multiple DPLLs can reuse a single source inside the same
package simultaneously.

A source should be able to link to a pin or directly to the netdev for
some embedded solutions. We don't need to go through the pin abstraction
at all.

An optional pin entity should only represent a physical connection with
a name and maybe a 3-state selection of In/Out/HiZ and then link to
sources or output of the DPLL(s).

Finally, the DPLL object should keep track of the source priority list,
have a proper status (locked/unlocked/holdover/freerunning), implement
the NCO mode, lock thresholds, bandwidths, auto-switch mode and so on.

Current implementation creates a lot of ambiguity, mixes input pins with
output pins and assigns priories to pins. Every SW entity will receive a
big list of pins and will need to parse it.

I prefer the approach that the ptp subsystem set - with its abstraction
of input/output channels and pins that can be assigned to them. While
not perfect - it represents reality much closer.

Thanks
Maciek