Re: [PATCH net-next v4 5/8] mfd: zl3073x: Add functions to work with register mailboxes

[Lee Jones <lee@kernel.org>]

On Fri, 25 Apr 2025, Ivan Vecera wrote:

> 
> 
> On 25. 04. 25 8:55 dop., Lee Jones wrote:
> > On Thu, 24 Apr 2025, Ivan Vecera wrote:
> > 
> > > 
> > > 
> > > On 24. 04. 25 9:29 odp., Andrew Lunn wrote:
> > > > > Yes, PHC (PTP) sub-driver is using mailboxes as well. Gpio as well for some
> > > > > initial configuration.
> > > > 
> > > > O.K, so the mailbox code needs sharing. The question is, where do you
> > > > put it.
> > > 
> > > This is crucial question... If I put the MB API into DPLL sub-driver
> > > then PTP sub-driver will depend on it. Potential GPIO sub-driver as
> > > well.
> > > 
> > > There could be some special library module to provide this for
> > > sub-drivers but is this what we want? And if so where to put it?
> > 
> > MFD is designed to take potentially large, monolithic devices and split
> > them up into smaller, more organised chunks, then Linusify them.  This
> > way, area experts (subsystem maintainers) get to concern themselves only
> > with the remit to which they are most specialised / knowledgable.  MFD
> > will handle how each of these areas are divided up and create all of the
> > shared resources for them.  On the odd occasion it will also provide a
> > _small_ API that the children can use to talk to the parent device.
> > 
> > However .... some devices, like yours, demand an API which is too
> > complex to reside in the MFD subsystem itself.  This is not the first
> > time this has happened and I doubt it will be the last.  My first
> > recommendation is usually to place all of the comms in drivers/platform,
> > since, at least in my own mind, if a complex API is required, then the
> > device has become almost platform-like.  There are lots of examples of
> > H/W comm APIs in there already for you to peruse.
> 
> OK, I will do it differently... Will drop MB API at all from MFD and
> just expose the additional mutex from MFD for multi-op access.
> Mailboxes will be handled directly by sub-devices.
> 
> Short description:
> MFD exposes:
> zl3073x_{read,write}_u{8,16,32,48}() & zl3073x_poll_u8()
> - to read/write/poll registers
> - they checks that multiop_lock is taken when caller is accessing
>   registers from Page 10 and above
> 
> zl3073x_multiop_{lock,unlock}()
> - to protect operation where multiple reads, writes and poll is required
>   to be done atomically

Looks sensible.  If this is aligned with the discussions that have been
taking place between you and Andrew.  Let's see the code before we make
any binding agreements.  =:)

-- 
Lee Jones [李琼斯]