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

[Maciek Machnikowski <maciek@machnikowski.net>]

On 12/9/2022 5:31 PM, Jakub Kicinski wrote:
> On Fri, 9 Dec 2022 15:09:08 +0100 Maciek Machnikowski wrote:
>> On 12/9/2022 12:07 PM, Jiri Pirko wrote:
>>> 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?  
>>
>> It looks goot, hence two corrections are needed:
>> - all inputs can go to all DPLLs, and a single source can drive more
>>   than one DPLL
>> - The external mux for SMA connector should not be a part of the
>>   Synchronizer subsystem - I believe there's already a separate MUX
>>   subsystem in the kernel and all external connections should be handled
>>   by a devtree or a similar concept.
>>
>> The only "muxing" thing that could potentially be modeled is a
>> synchronizer output to synchronizer input relation. Some synchronizers
>> does that internally and can use the output of one DPLL as a source for
>> another.
> 
> My experience with DT and muxes is rapidly aging, have you worked with
> those recently? From what I remember the muxes were really.. "embedded"
> and static compared to what we want here.
> 
> Using DT may work nicely for defining the topology, but for config we
> still need a different mechanism.
> 
>>> 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)  
>>
>> I'd rather not use pins at all - just stick to sources and outputs. Both
>> can use some labels to be identifiable.
> 
> TBH I can't comprehend your suggestion.
> IIUC you want an object for a source, but my brain can't handle
> modeling an external object. For instance the source could be GNSS, 
> but this is not the GNSS subsystem. We have a pin connected to GNSS,
> not the GNSS itself. 
> Maybe a diagram would help?

A source is just a more generic term for a frequency signal that can be
used by a DPLL. For some solutions it can represent a pin, for others
(integrated) it can represent an internal connection to a different
DPLL/PHY/MAC/embedded oscillator or anything else that can produce
periodic signal.

This object will have a subset of properties listed in a previous mail:
>>>> 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.

I'm absolutely not willing to connect the GNSS subsystem there :)

A "pin" is too ambiguous - especially for differential inputs.