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

["Kubalewski, Arkadiusz" <arkadiusz.kubalewski@intel.com>]

>From: Jiri Pirko <jiri@resnulli.us>
>Sent: Monday, January 9, 2023 5:30 PM
>
>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?
>

No.
A "Synchronization Channel" on a switch would allow to separate groups
of physical ports. Each channel/group has own "Synchronizer Chip", which is
used to drive PHY clocks of that group.

"Synchronizer chip" would be the 2 DPLLs on old draw, something like this:
+--------------------------------------------------------------+
| Channel A / FW0        +-------------+   +---+   +--------+  |
|                    i0--|Synchronizer0|---|   |---| PHY0.0 |--|
|         +---+          |             |   |   |   +--------+  |
| PHY0.0--|   |      i1--|             |---| M |---| PHY0.1 |--|
|         |   |          | +-----+     |   | A |   +--------+  |
| PHY0.1--| M |      i2--| |DPLL0|     |   | C |---| PHY0.2 |--|
|         | U |          | +-----+     |   | 0 |   +--------+  |
| PHY0.2--| X |--+---i3--| +-----+     |---|   |---| ...    |--|
|         | 0 |  |       | |DPLL1|     |   |   |   +--------+  |
| ...   --|   |  | /-i4--| +-----+     |---|   |---| PHY0.7 |--|
|         |   |  | |     +-------------+   +---+   +--------+  |
| PHY0.7--|   |  | |                                           |
|         +---+  | |                                           |
+----------------|-|-------------------------------------------+
| Channel B / FW1| |     +-------------+   +---+   +--------+  |
|                | | i0--|Synchronizer1|---|   |---| PHY1.0 |--|
|         +---+  | |     |             |   |   |   +--------+  |
| PHY1.0--|   |  | | i1--|             |---| M |---| PHY1.1 |--|
|         |   |  | |     | +-----+     |   | A |   +--------+  |
| PHY1.1--| M |  | | i2--| |DPLL0|     |   | C |---| PHY1.2 |--|
|         | U |  | |     | +-----+     |   | 1 |   +--------+  |
| PHY1.2--| X |  \-|-i3--| +-----+     |---|   |---| ...    |--|
|         | 1 |    |     | |DPLL1|     |   |   |   +--------+  |
| ...   --|   |----+-i4--| +-----+     |---|   |---| PHY1.7 |--|
|         |   |          +-------------+   +---+   +--------+  |
| PHY1.7--|   |                                                |
|         +---+                                                |
+--------------------------------------------------------------+
Also, please keep in mind that is an example, there could be easily 4
(or more) channels wired similarly.

>
>>
>>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.
>

A user or some tool, this change requires to switch a frequency on a signal
generator connected to that SMA1. Whatever would make the change is an external
entity here. The draw show connections on board, don't see a point on having a
external signal generator or user connected to the board :)

If something is not clear, we could prepare some different draw, please just
let me know what exactly we want to see. It sound like a sequence diagram?

Thanks!
Arkadiusz

>
>>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