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

[Lee Jones <lee@kernel.org>]

On Mon, 12 Sep 2022, Russell King (Oracle) wrote:

> On Fri, Sep 09, 2022 at 08:50:07AM +0100, Lee Jones wrote:
> > 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.
> 
> I'm guessing this series is now dead, and Hector needs to re-spin the
> patch set according to your views. I'm guessing this is going to take
> a major re-work of the patch series.
> 
> I suspect my attempt and trying to get this upstream has made things
> more complicated, because I doubt Hector has updated his patch set
> with the review comments that have been made so far... so this is
> now quite a mess. I think, once this is sorted, the entire series
> will need to be re-reviewed entirely afresh.

I have no insight into what Hector is doing, or plans to do.

> I've also completely lost where I was in updating the patches with
> all the discussion on this posting of the patch set (which is why I
> posted v2, because I couldn't keep track of all the emails on this
> version.) When I posted v2, I had already lost track, which is why
> it got posted.

Apologies if this has hindered your good work.

-- 
Lee Jones [李琼斯]