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

[Jakub Kicinski <kuba@kernel.org>]

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.

> 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.

> 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.

> 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.