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

From: Jiri Pirko <jiri@resnulli.us>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Maciek Machnikowski <maciek@machnikowski.net>,
	"'Kubalewski, Arkadiusz'" <arkadiusz.kubalewski@intel.com>,
	'Vadim Fedorenko' <vfedorenko@novek.ru>,
	'Jonathan Lemon' <jonathan.lemon@gmail.com>,
	'Paolo Abeni' <pabeni@redhat.com>,
	netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-clk@vger.kernel.org
Subject: Re: [RFC PATCH v4 0/4] Create common DPLL/clock configuration API
Date: Mon, 12 Dec 2022 14:58:47 +0100	[thread overview]
Message-ID: <Y5czl6HgY2GPKR4v@nanopsycho> (raw)
In-Reply-To: <20221209083104.2469ebd6@kernel.org>

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?

[...]