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

Jiri Pirko
2023-01-09T16:30:51Z

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