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

[Mark Kettenis <mark.kettenis@xxxxxxxxx>]

> Date: Tue, 6 Sep 2022 15:54:50 +0100
> From: "Russell King (Oracle)" <linux@xxxxxxxxxxxxxxx>
> 
> On Tue, Sep 06, 2022 at 04:25:49PM +0200, Mark Kettenis wrote:
> > > Date: Tue, 6 Sep 2022 22:53:47 +0900
> > > From: Hector Martin <marcan@xxxxxxxxx>
> > > 
> > > I agree that this is something to think about (I was about to reply on
> > > the subject).
> > > 
> > > I can think of two ways: using `reg` for the key name, but that feels
> > > icky since it's ASCII and not *really* a register number/address, or
> > > something like this:
> > > 
> > > gpio@0 {
> > > 	apple,smc-key-base = "gP00";
> > > 	...
> > > }
> > > 
> > > gpio@1 {
> > > 	apple,smc-key-base = "gp00";
> > > 	...
> > > }
> > 
> > This would still require us to add a (one-cell) "reg" property and
> > would require adding the appropriate "#address-cells" and
> > "#size-cells" properties to the SMC node.
> 
> Yes, and at that point, as I suggested, it probably would be better
> to use:
> 
> 	#address-cells = <1>;
> 	#size-cells = <0>;
> 
> 	gpio@67503030 {
> 		reg = <0x67503030>;
> 	};
> 
> 	gpio@67703030 {
> 		reg = <0x67703030>;
> 	};
> 
> Then the "reg" has a meaning that is directly related to the SMC.
> 
> > > But this ties back to the device enumeration too, since right now the DT
> > > does not drive that (we'd have to add the subdevice to the mfd subdevice
> > > list somehow anyway, if we don't switch to compatibles).
> > > 
> > > I'd love to hear Rob's opinion on this one, and also whether the
> > > existing Linux and OpenBSD code would currently find gpio@0 {} instead
> > > of gpio {} for backwards compat.
> > 
> > The OpenBSD driver does a lookup by name and the "@0" is part of that
> > name.  So that would break backwards compat.
> 
> Oh, that's annoying - and is a different behaviour to Linux.
> 
> On Linux, we only look at the node name up to the @ when matching (see
> of_node_name_eq() in drivers/of/base.c, so it doesn't matter to Linux
> what follows the @ when you try to look up a node named "gpio" - you'll
> find gpio@anythingyoulike.

So maybe I should change our code to match what Linux does.  OpenBSD
7.2 is scheduled for release on November 1st, and I can probably get
that change in quickly.  If we can hold off updating the device trees
in the Asahi installer until then the transition should be smooth
enough.

> > Maybe just name the slave GPIO controller "gpio-slave"?  If we add
> > compatibles, the compatibles for the nodes should propbably be
> > different such that we can switch to do a lookup by compatible?
> 
> I don't think the DT folk would be happy with "gpio-slave" because
> node names are supposed to be generic.

I don't think that rule applies to "named" sub-nodes like this where
the name is actually significant.  

> Also, "slave" probably isn't a good choice of name in this modern
> era given past history.

Yes, we don't have to follow Apple's terminology here.

> Rather than the above, we could use "reg" to indicate which GPIO
> controller we're talking about, and lookup the reg value in a table
> to give the key. So gpio@0, reg=<0> => gP00, gpio@1, reg=<1> => gp00.
> gpio@2, reg=<2> => whatever next.
> 
> That sounds like it won't break the existing OpenBSD.
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
>