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

[Jiri Pirko <jiri@resnulli.us>]

Thu, Jan 12, 2023 at 01:15:30PM CET, arkadiusz.kubalewski@intel.com wrote:
>>From: Jiri Pirko <jiri@resnulli.us>
>>Sent: Wednesday, January 11, 2023 5:15 PM
>>To: Kubalewski, Arkadiusz <arkadiusz.kubalewski@intel.com>
>>
>>Wed, Jan 11, 2023 at 04:30:44PM CET, arkadiusz.kubalewski@intel.com wrote:
>>>>From: Jiri Pirko <jiri@resnulli.us>
>>>>Sent: Wednesday, January 11, 2023 4:05 PM
>>>>To: Kubalewski, Arkadiusz <arkadiusz.kubalewski@intel.com>
>>>>
>>>>Wed, Jan 11, 2023 at 03:16:59PM CET, arkadiusz.kubalewski@intel.com
>>wrote:
>>>>>>From: Jiri Pirko <jiri@resnulli.us>
>>>>>>Sent: Wednesday, January 11, 2023 9:20 AM
>>>>>>
>>>>>>Tue, Jan 10, 2023 at 09:05:49PM CET, kuba@kernel.org wrote:
>>>>>>>On Mon, 9 Jan 2023 14:43:01 +0000 Kubalewski, Arkadiusz wrote:
>>>>>>>> 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.
>>>>>>>
>>>>>>>I must be missing context from other discussions but what is this
>>>>>>>proposal trying to solve? Well implemented shared pins is all we need.
>>>>>>
>>>>>>There is an entity containing the pins. The synchronizer chip. One
>>>>>>synchronizer chip contains 1-n DPLLs. The source pins are connected
>>>>>>to each DPLL (usually). What we missed in the original model was the
>>>>>>synchronizer entity. If we have it, we don't need any notion of somehow
>>>>>>floating pins as independent entities being attached to one or many
>>>>>>DPLL refcounted, etc. The synchronizer device holds them in
>>>>>>straightforward way.
>>>>>>
>>>>>>Example of a synchronizer chip:
>>>>>>https://www.renesas.com/us/en/products/clocks-timing/jitter-
>>attenuators-
>>>>>>frequency-translation/8a34044-multichannel-dpll-dco-four-eight-
>>>>>>channels#overview
>>>>>
>>>>>Not really, as explained above, multiple separated synchronizer chips
>>can
>>>>be
>>>>>connected to the same external sources.
>>>>>This is why I wrote this email, to better explain need for references
>>>>between
>>>>>DPLLs and shared pins.
>>>>>Synchronizer chip object with multiple DPLLs would have sense if the
>>pins
>>>>would
>>>>>only belong to that single chip, but this is not true.
>>>>
>>>>I don't understand how it is physically possible that 2 pins belong to 2
>>>>chips. Could you draw this to me?
>>>>
>>>
>>>Well, sure, I was hoping this is clear, without extra connections on the
>>draw:
>>
>>Okay, now I understand. It is not a shared pin but shared source for 2
>>pins.
>>
>
>Yes, exactly.
>
>>
>>>+----------+
>>>|i0 - GPS  |--------------\
>>>+----------+              |
>>>+----------+              |
>>>|i1 - SMA1 |------------\ |
>>>+----------+            | |
>>>+----------+            | |
>>>|i2 - SMA2 |----------\ | |
>>>+----------+          | | |
>>>                      | | |
>>>+---------------------|-|-|-------------------------------------------+
>>>| Channel A / FW0     | | |     +-------------+   +---+   +--------+  |
>>>|                     | | |-i0--|Synchronizer0|---|   |---| PHY0.0 |--|
>>
>>One pin here               ^^^
>>
>>>|         +---+       | | |     |             |   |   |   +--------+  |
>>>| 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 |--|
>>
>>And second pin here        ^^^
>>
>>There are 2 separate pins. Sure, they need to have the same config as
>>they are connected to the same external entity (GPS, SMA1, SMA2).
>>
>>Perhaps we need to have a board description using dts to draw this
>>picture so the drivers can use this schema in order to properly
>>configure this?
>>
>>My point is, you are trying to hardcode the board geometry in the
>>driver. Is that correct?
>>
>
>Well, we are trying to have userspace-friendly interface :)
>As we discussed yesterday dts is more of embedded world thing and we don't
>want to go that far, the driver knows the hardware it is using, thus it
>shall be enough if it has all the information needed for initialization.
>At least that is what I understood.

Yes, I think yesterday called cleared things up. Thanks!


>
>BR,
>Arkadiusz
>
>>
>>>|         +---+  | |  | |       |             |   |   |   +--------+  |
>>>| 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--|   |                                                       |
>>>|         +---+                                                       |
>>>+---------------------------------------------------------------------+
>>>
>>>>
>>>>>As the pins are shared between multiple DPLLs (both inside 1 integrated
>>>>circuit
>>>>>and between multiple integrated circuits), all of them shall have
>>current
>>>>state
>>>>>of the source or output.
>>>>>Pins still need to be shared same as they would be inside of one
>>>>synchronizer
>>>>>chip.
>>>>
>>>>Do I understand correctly that you connect one synchronizer output to
>>>>the input of the second synchronizer chip?
>>>
>>>No, I don't recall such use case. At least nothing that needs to exposed
>>>in the DPLL subsystem itself.
>>>
>>>BR,
>>>Arkadiusz
>>>
>>>>
>>>>>
>>>>>BR,
>>>>>Arkadiusz