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

[Jiri Pirko <jiri@resnulli.us>]

Mon, Jan 09, 2023 at 03:43:01PM CET, arkadiusz.kubalewski@intel.com wrote:
>>From: Jiri Pirko <jiri@resnulli.us>
>>Sent: Monday, December 12, 2022 2:59 PM
>>
>>Fri, Dec 09, 2022 at 05:31:04PM CET, kuba@kernel.org 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.
>>
>>Why do you think we need something "non-static"? The mux is part of the
>>board, isn't it? That sounds quite static to me.
>>
>>
>>>
>>>Using DT may work nicely for defining the topology, but for config we
>>>still need a different mechanism.
>>
>>"config" of what? Each item in topology would be configure according to
>>the item type, won't it?
>>
>>[...]
>
>
>Hi guys,
>
>We have been trying to figure out feasibility of new approach proposed on our
>latest meeting - to have a single object which encapsulates multiple DPLLs.
>
>Please consider following example:
>
>Shared common inputs:                                      
>i0 - GPS  / external                                       
>i1 - SMA1 / external                                       
>i2 - SMA2 / external                                       
>i3 - MUX0 / clk recovered from PHY0.X driven by MAC0       
>i4 - MUX1 / clk recovered from PHY1.X driven by MAC1       
>
>+---------------------------------------------------------+
>| Channel A / FW0             +---+                       |
>|                         i0--|   |                       |
>|         +---+               |   |                       |
>| PHY0.0--|   |           i1--| D |                       |
>|         |   |               | P |                       |
>| PHY0.1--| M |           i2--| L |   +---+   +--------+  |
>|         | U |               | L |---|   |---| PHY0.0 |--|
>| PHY0.2--| X |-+---------i3--| 0 |   |   |   +--------+  |
>|         | 0 | |+------+     |   |---| M |---| PHY0.1 |--|
>| ...   --|   | || MUX1 |-i4--|   |   | A |   +--------+  |
>|         |   | |+------+     +---+   | C |---| PHY0.2 |--|
>| PHY0.7--|   | |         i0--|   |   | 0 |   +--------+  |
>|         +---+ |             |   |---|   |---| ...    |--|
>|               |         i1--| D |   |   |   +--------+  |
>|               |             | P |---|   |---| PHY0.7 |--|
>|               |         i2--| L |   +---+   +--------+  |
>|               |             | L |                       |
>|               \---------i3--| 1 |                       |
>|                +------+     |   |                       |
>|                | MUX1 |-i4--|   |                       |
>|                +------+     +---+                       |
>+---------------------------------------------------------+
>| Channel B / FW1             +---+                       |
>|                         i0--|   |                       |
>|                             |   |                       |
>|                         i1--| D |                       |
>|         +---+               | P |                       |
>| PHY1.0--|   |           i2--| L |   +---+   +--------+  |
>|         |   |  +------+     | L |---|   |---| PHY1.0 |--|
>| PHY1.1--| M |  | MUX0 |-i3--| 0 |   |   |   +--------+  |
>|         | U |  +------+     |   |---| M |---| PHY1.1 |--|
>| PHY1.2--| X |-+---------i4--|   |   | A |   +--------+  |
>|         | 1 | |             +---+   | C |---| PHY1.2 |--|
>| ...   --|   | |         i0--|   |   | 1 |   +--------+  |
>|         |   | |             |   |---|   |---| ...    |--|
>| PHY1.7--|   | |         i1--| D |   |   |   +--------+  |
>|         +---+ |             | P |---|   |---| PHY1.7 |--|
>|               |         i2--| L |   +---+   +--------+  |
>|               |+------+     | L |                       |
>|               || MUX0 |-i3--| 1 |                       |
>|               |+------+     |   |                       |
>|               \---------i4--|   |                       |
>|                             +---+                       |
>+---------------------------------------------------------+

What is "a channel" here? Are these 2 channels part of the same physival
chip? Could you add the synchronizer chip/device entities to your drawing?


>
>This is a simplified network switch board example.
>It has 2 synchronization channels, where each channel:
>- provides clk to 8 PHYs driven by separated MAC chips,
>- controls 2 DPLLs.
>
>Basically only given FW has control over its PHYs, so also a control over it's
>MUX inputs.
>All external sources are shared between the channels.
>
>This is why we believe it is not best idea to enclose multiple DPLLs with one
>object:
>- sources are shared even if DPLLs are not a single synchronizer chip,
>- control over specific MUX type input shall be controllable from different
>driver/firmware instances.
>
>As we know the proposal of having multiple DPLLs in one object was a try to
>simplify currently implemented shared pins. We fully support idea of having
>interfaces as simple as possible, but at the same time they shall be flexible
>enough to serve many use cases.
>
>Right now the use case of single "synchronizer chip" is possible (2 DPLLs with
>shared inputs), as well as multiple synchronizer chips with shared inputs.
>
>If we would entirely get rid of sharing pins idea and instead allowed only to
>have multiple DPLLs in one object, we would fall back to the problem where
>change on one input is braking another "synchronizer chip" input.
>I.e. considering above scheme, user configured both channels to use SMA1 1MHz.
>If SMA1 input is changed to 10MHz, all DPLLs are affected, thus all using that

You say "SMA1 input *is changed*". Could you add to your drawing:
1) Who is the one triggering the change.
2) Entity that manages the SMA input and applies the configuration.


>input shall be notified, as long as that input is shared.
>For the drivers that have single point of control over dpll, they might just
>skip those requests. But if there are multiple firmware instances controlling
>multiple DPLLs, they would process it independently.
>
>Current implementation is the most flexible and least complex for the level of
>flexibility it provides.
>
>BR, Happy new year!
>Arkadiusz