Re: [PATCH 4/6] platform/apple: Add new Apple Mac SMC driver

[Lee Jones <lee@xxxxxxxxxx>]

On Thu, 08 Sep 2022, Hector Martin wrote:

> On 08/09/2022 22.36, Lee Jones wrote:
> > On Thu, 08 Sep 2022, Hector Martin wrote:
> > 
> >> On 08/09/2022 21.31, Lee Jones wrote:
> >>> The long and the short of it is; if you wish to treat this device, or
> >>> at least a section of it, as a type of MFD, then please draft that
> >>> part of it as an MFD driver, residing in drivers/mfd.  If it's "not
> >>> really an MFD", then find another way to represent the conglomeration
> >>> please.
> >>>
> >>> If the MFD route is the best, then you can register each of the
> >>> devices, including the *-core from drivers/mfd.  Grep for "cross-ec"
> >>> as a relatively recently good example.
> >>
> >> I think cros-ec is similar enough, yeah. As long as you don't mind
> >> having the core codebase in mfd/ (4 files: core, rtkit backend, and
> >> future T2 and legacy backends) we can do that.
> > 
> > That's actually not what I'm suggesting.
> > 
> > You *only* need to move the subsequent-device-registration handling
> > into drivers/mfd.  The remainder really should be treated as Platform
> > (not to be confused with Arch Platform) code and should reside in
> > drivers/platform.  Just as we do with cros-ec.
> 
> That's... an interesting approach.

How you decide to initially architect it would be your choice.

We can then discuss any potential improvements / suggestions.

> Is the code in drivers/mfd supposed
> to be a subdevice itself? That seems to be what's going on with
> cros_ec_dev.c, but do we really need that layer of indirection?

Ideally not.  The evolution of cros-ec happened over many iterations
and much time.  Initially it was almost entirely implemented in
drivers/mfd until I requested for a lot of the truly platform code to
be moved out, as it grew beyond the bounds of, and was therefore no
longer relevant to MFD.

If we were to design and build it up again from scratch, I'd suggest
that the MFD part would be the core-driver / entry-point.  That driver
should request and initialise shared resources and register the other
devices, which is essentially the MFD's mantra.

> What's the point of just having effectively an array of mfd_cell and
> wrappers to call into the mfd core in the drivers/mfd/ tree and the
> rest of the driver elsewhere?

They should be separate drivers, with MFD registering the Platform.

-- 
Lee Jones [李琼斯]