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

[Jiri Pirko <jiri@resnulli.us>]

Thu, Dec 08, 2022 at 07:08:04PM CET, maciek@machnikowski.net wrote:
>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.


Looking at the documentation of the chips, they all have mupltiple DPLLs
on a die. Arkadiusz, in your proposed implementation, do you model each
DPLL separatelly? If yes, then I understand the urgency of need of a
shared pin. So all DPLLs sharing the pin are part of the same chip?

Question: can we have an entity, that would be 1:1 mapped to the actual
device/chip here? Let's call is "a synchronizer". It would contain
multiple DPLLs, user-facing-sources(input_connector),
user-facing-outputs(output_connector), i/o pins.

An example:
                               SYNCHRONIZER

                              ┌───────────────────────────────────────┐
                              │                                       │
                              │                                       │
  SyncE in connector          │              ┌─────────┐              │     SyncE out connector
                ┌───┐         │in pin 1      │DPLL_1   │     out pin 1│    ┌───┐
                │   ├─────────┼──────────────┤         ├──────────────┼────┤   │
                │   │         │              │         │              │    │   │
                └───┘         │              │         │              │    └───┘
                              │              │         │              │
                              │           ┌──┤         │              │
   GNSS in connector          │           │  └─────────┘              │
                ┌───┐         │in pin 2   │                  out pin 2│     EXT SMA connector
                │   ├─────────┼───────────┘                           │    ┌───┐
                │   │         │                           ┌───────────┼────┤   │
                └───┘         │                           │           │    │   │
                              │                           │           │    └───┘
                              │                           │           │
   EXT SMA connector          │                           │           │
                ┌───┐   mux   │in pin 3      ┌─────────┐  │           │
                │   ├────┬────┼───────────┐  │         │  │           │
                │   │    │    │           │  │DPLL_2   │  │           │
                └───┘    │    │           │  │         │  │           │
                         │    │           └──┤         ├──┘           │
                         │    │              │         │              │
   EXT SMA connector     │    │              │         │              │
                ┌───┐    │    │              │         │              │
                │   ├────┘    │              └─────────┘              │
                │   │         │                                       │
                └───┘         └───────────────────────────────────────┘

Do I get that remotelly correct?

synch
synchronizer_register(synch)
   dpll_1
   synchronizer_dpll_register(synch, dpll_1)
   dpll_2
   synchronizer_dpll_register(synch, dpll_2)
   source_pin_1
   synchronizer_pin_register(synch, source_pin_1)
   output_pin_1
   synchronizer_pin_register(synch, output_pin_1)
   output_pin_2
   synchronizer_pin_register(synch, output_pin_2)

synch_board
   synchronizer_board_register(synch_board)
   synch
   synchronizer_board_sync_register(synch_board, synch)
   source_connector_1
   synchronizer_board_connector_register(synch_board, source_connector_1, source_pin_1)
   output_connector_1
   synchronizer_board_connector_register(synch_board, output_connector_1, output_pin_1)
   output_connector_2
   synchronizer_board_connector_register(synch_board, output_connector_2, output_pin_2)


Thinking about it a bit more, this should be probably good to describe
by device tree. The synchronizer itself dplls and pins it contains
have constanc geometry, according to the synchronizer device type.

The Connector-pin linkages may vary according to the board.

So to divide it, there should be one synchronizer driver. Then probably
some other one to connect/select/mux the connectors to the synchronizer.

Makes sense?