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

["Russell King (Oracle)" <linux@xxxxxxxxxxxxxxx>]

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

Sorry.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!