[PATCH 1/6] dt-bindings: mfd: add binding for Apple Mac System Management Controller

[Mark Kettenis]

> Date: Tue, 6 Sep 2022 10:04:45 +0100
> From: "Russell King (Oracle)" <linux at armlinux.org.uk>
> 
> On Fri, Sep 02, 2022 at 12:28:08PM -0500, Rob Herring wrote:
> > This one is actually pretty odd in that the child nodes don't have a 
> > compatible string which breaks the automagical probing.
> 
> I don't think that is necessarily true, and I don't think it's true in
> this case.
> 
> The SMC core driver instructs the MFD core to create devices for the
> individual functional items:
> 
> static const struct mfd_cell apple_smc_devs[] = {
>         {
>                 .name = "macsmc-gpio",
>         },
>         {
>                 .name = "macsmc-hid",
>         },
>         {
>                 .name = "macsmc-power",
>         },
>         {
>                 .name = "macsmc-reboot",
>         },
>         {
>                 .name = "macsmc-rtc",
>         },
> };
> 
> Since MFD uses platform devices for these, they get all the normal
> functionality that these devices have, which include matching by
> device name ot the driver name, and udev events being appropriately
> triggered. As long as the platform drivers for these devices have the
> correct modalias lines, autoloading of the modules will work and the
> drivers will be correctly matched and probed.
> 
> The Asahi kernel builds most of the platform support as modules,
> including these, so we know it works (if it didn't, then lots of
> module autoloading would be broken on non-DT platforms.)
> 
> > > Again the separate nodes are there because the RTC and the reboot
> > > functionality are logically separate and handled by different MFD
> > > sub-drivers in Linux.
> > 
> > It's really a question of whether the subset of functionality is going 
> > to get reused on its own or has its own resources in DT. MFD bindings 
> > are done both ways.
> 
> I think the current position on what to do about these is that everyone
> is looking for someone else to make a decision, and no one wants to!
> 
> Firstly, I don't think that the number of properties in a node should
> have a bearing on the design of the DT binding - what should have a
> bearing is the logical partitioning of functionality.
> 
> Mark suggests that it would take six months for OpenBSD to transition to
> some other description - for example, if we merged the nodes.

Note that we're totally willing to do that if there is a good
technical reason for changes to the binding.  We just have to find a
way to upgrade existing installations of OpenBSD 7.1 which is a bit of
a challenge as the SMC GPIOs are essential for wifi which on the
laptops is the only network connection available.

> Hector says that MacOS's firmware description has the nodes merged, but
> their description is a mess.

To elaborate on that a bit more, the use of sub-nodes provides a nice
separation between the SMC hardware interface (the main node) and the
functionality offered by the firmware running on the SMC (the sub-nodes).

Another argument for having sub-nodes is that the firmware actually
exposes *two* GPIO controllers.  For now we only support the "master"
PMU GPIOs, but there also is a "slave" PMU GPIO controller that uses a
separate set of SMC "keys".  We currently don't need any of the pins
on the "slave", so we don't expose it in the DT yet.

> The overall preference seems to be to keep the sub-nodes unless there
> is a strong technical reason not to.
> 
> The feeling I am getting from the review is that there doesn't seem to
> be a strong technical reason to merge the nodes - there are desires and
> preferences, but nothing concrete.
> 
> So at this point, I think it would make sense if I post a v2 with all
> the updates so far (sorry, given the long drawn out discussions on
> this, I've lost track of what changes have been made to the code, so
> I won't include a detailed change log.)
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
> 
>