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

Kubalewski, Arkadiusz
2023-01-09T14:43:11Z

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

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