On Tue, Sep 06, 2022 at 03:54:50PM +0100, Russell King (Oracle) wrote:
> 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.
That's certainly odd, but if that's how it is addressed, then okay I
suppose.
>
> > > 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.
Node names are generally not considered ABI except when they are. :)
Generally, core code doesn't care. Specific bindings with defined child
nodes often do, but finding nodes by compatible is strongly preferred.
Linux drivers can bind by node name (w/o unit-address), but that's
really only ever done on ancient h/w.
> >
> > 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.
>
> > 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. Also, "slave" probably isn't
> a good choice of name in this modern era given past history.
Yeah, not a great choice for both reasons.
> 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.
Keep in mind that for any level, there is only 1 address space. So if
there's anything else with multiple instances, they share the same
address space. IOW, you couldn't have say 'rtc@1'.
(Another example of why I want to see a full picture.)
>
> That sounds like it won't break the existing OpenBSD.
No? Isn't OpenBSD looking for 'gpio' which wouldn't find 'gpio@0'?
Rob
