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

[Hector Martin <marcan@xxxxxxxxx>]

On 07/09/2022 02.35, Rob Herring wrote:
> On Wed, Sep 07, 2022 at 02:00:53AM +0900, Hector Martin wrote:
>> On 07/09/2022 01.10, Rob Herring wrote:
>>>> 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.
>>
>> So addressing only makes sense for GPIO, out of the nodes we have so far
>> - that's the only thing with two discrete instances whose access can be
>> entirely described by a single base key name, and which are otherwise
>> compatible.
>>
>> Everything else is pretty much single-instance, and talks to multiple
>> keys, so there isn't one single "address" key that would make semantic
>> sense to use as the node address. 
> 
> Unit-addresses are just the first address in 'reg'. So multiple 
> addresses or not doesn't really matter.

Okay, but we're obviously not going to list every single SMC key used by
any given node in the device tree (that'd be a giant mess, other than
for hwmon which is a story for another time), and it doesn't make sense
to pick an arbitrary one as the reg address and then ignore it in the
driver.

>> There are some indexed keys, but at a
>> deeper level (e.g. multiple battery cells part of the charge control
>> subsystem, multiple Type C ports as part of the AC/power input
>> subsystem, etc.). And in those cases, these subdevices are mostly
>> homogeneous and we would never need multiple nodes for them at the DT
>> level, they'd just be implicitly handled by those drivers.
> 
> Type-C will be fun depending on how much of the muxing/altmode 
> details have to get exposed. 

As sven mentioned that will be fun, but thankfully not part of this
binding (SMC only cares for power supply purposes, and since our direct
driver already exposes power config info in Linux (or should, and we can
improve that), I don't particularly have plans to expose the SMC Type-C
data other than perhaps something in the existing power supply driver to
indicate which port is the currently chosen power input.

> If only those 2 nodes, then I really don't care so much and gpio-sec 
> would be fine. It's 1 node in 1 binding.

I think it might make sense to just go with this then. If Apple ever
introduces yet another GPIO sub-controller we can just add another one,
and honestly I don't think that's very likely, given they don't even use
any of the GPIOs from the second one from the AP yet. I don't see SMC
growing a big list of GPIO controllers any time soon, such that we
regret doing it this way. And then the node-name can just map to a given
key prefix statically in the driver, and thus we don't even need a
property for that (gpio would be gP?? and gpio-sec gp?? right now).

- Hector