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

[Jiri Pirko <jiri@resnulli.us>]

Thu, Dec 08, 2022 at 12:21:57AM CET, kuba@kernel.org wrote:
>On Wed, 7 Dec 2022 15:09:03 +0100 netdev.dump@gmail.com wrote:
>> > -----Original Message-----
>> > From: Jakub Kicinski <kuba@kernel.org>
>> > Sent: Wednesday, December 7, 2022 3:48 AM
>> > Subject: Re: [RFC PATCH v4 0/4] Create common DPLL/clock configuration API
>> > 
>> > On Fri, 2 Dec 2022 17:12:06 +0100 Jiri Pirko wrote:  
>>  [...]  
>> capable
>>  [...]  
>> require
>>  [...]  
>
>Please fix line wrapping in your email client. 
>And add a name to your account configuration :/
>
>> > > Yep, you have the knowledge of sharing inside the driver, so you should
>> > > do it there. For multiple instances, use in-driver notifier for example.  
>> > 
>> > No, complexity in the drivers is not a good idea. The core should cover
>> > the complexity and let the drivers be simple.  
>> 
>> But how does Driver A know where to connect its pin to? It makes sense to
>> share 
>
>I think we discussed using serial numbers.

Can you remind it? Do you mean serial number of pin?


>
>> pins between the DPLLs exposed by a single driver, but not really outside of
>> it.
>> And that can be done simply by putting the pin ptr from the DPLLA into the
>> pin
>> list of DPLLB.
>
>Are you saying within the driver it's somehow easier? The driver state
>is mostly per bus device, so I don't see how.

You can have some shared data for multiple instances in the driver code,
why not?


>
>> If we want the kitchen-and-sink solution, we need to think about corner
>> cases.
>> Which pin should the API give to the userspace app - original, or
>> muxed/parent?
>
>IDK if I parse but I think both. If selected pin is not directly
>attached the core should configure muxes.
>
>> How would a teardown look like - if Driver A registered DPLLA with Pin1 and
>> Driver B added the muxed pin then how should Driver A properly
>> release its pins? Should it just send a message to driver B and trust that
>> it
>> will receive it in time before we tear everything apart?
>
>Trivial.
>
>> There are many problems with that approach, and the submitted patch is not
>> explaining any of them. E.g. it contains the dpll_muxed_pin_register but no
>> free 
>> counterpart + no flows.
>
>SMOC.

Care to spell this out. I guess you didn't mean "South Middlesex
Opportunity Council" :D


>
>> If we want to get shared pins, we need a good example of how this mechanism
>> can be used.
>
>Agreed.
>
>> > > There are currently 3 drivers for dpll I know of. This in ptp_ocp and
>> > > mlx5 there is no concept of sharing pins. You you are talking about a
>> > > single driver.
>> > >
>> > > What I'm trying to say is, looking at the code, the pin sharing,
>> > > references and locking makes things uncomfortably complex. You are so
>> > > far the only driver to need this, do it internally. If in the future
>> > > other driver appears, this code would be eventually pushed into dpll
>> > > core. No impact on UAPI from what I see. Please keep things as simple as
>> > > possible.  
>> > 
>> > But the pin is shared for one driver. Who cares if it's not shared in
>> > another. The user space must be able to reason about the constraints.
>> > 
>> > You are suggesting drivers to magically flip state in core objects
>> > because of some hidden dependencies?!
>> 
>> If we want to go outside the device, we'd need some universal language
>> to describe external connections - such as the devicetree. I don't see how
>> we can reliably implement inter-driver dependency otherwise.
>
>There's plenty examples in the tree. If we can't use serial number
>directly we can compare the driver pointer + whatever you'd compare
>in the driver internal solution.
>
>> I think this would be better served in the userspace with a board-specific
>> config file. Especially since the pins can be externally connected anyway.
>
>Opinions vary, progress is not being made.