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

Jiri Pirko
2023-01-12T14:58:07Z

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