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

[Rob Herring <robh@kernel.org>]

On Tue, Sep 06, 2022 at 10:04:45AM +0100, Russell King (Oracle) wrote:
> 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.

It is, because you are creating the devices in the driver rather than 
creating devices based on child nodes that exist.

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

Yes, and that's how MFD devices with no child nodes work. The difference 
is those get any DT info out of the parent node. Here you are presumably 
manually getting the child DT node you want for each driver.


> 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!

I'm just looking for more information first.

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

It's not strictly about number of properties. Though nodes with only a 
compatible string is generally a red flag for me.

> Mark suggests that it would take six months for OpenBSD to transition to
> some other description - for example, if we merged the nodes.

The risk you take when using undocumented bindings...

> Hector says that MacOS's firmware description has the nodes merged, but
> their description is a mess.
> 
> 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.)

As I said elsewhere, sub-nodes is probably the right choice here. I 
think they need compatible strings in the child nodes, and addressing 
has to be sorted out which it seems may also break OpenBSD.

Rob