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

[<netdev.dump@gmail.com>]

> -----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:
> > >But this is only doable with assumption, that the board is internally
capable
> > >of such internal board level communication, which in case of separated
> > >firmwares handling multiple dplls might not be the case, or it would
require
> > >to have some other sw component feel that gap.
> >
> > 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 
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.

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

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.

If we want to get shared pins, we need a good example of how this mechanism
can be used.

> 
> > >For complex boards with multiple dplls/sync channels, multiple ports,
> > >multiple firmware instances, it seems to be complicated to share a pin
if
> > >each driver would have own copy and should notify all the other about
> changes.
> > >
> > >To summarize, that is certainly true, shared pins idea complicates
stuff
> > >inside of dpll subsystem.
> > >But at the same time it removes complexity from all the drivers which
would
> use
> >
> > 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.

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.