+Cc: Hans (as some DT/ACPI interesting cases are being discussed). On Tue, Feb 28, 2023 at 01:05:54PM -0600, Daniel Kaehn wrote: > On Mon, Feb 27, 2023 at 5:07 PM Andy Shevchenko > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: > > On Mon, Feb 27, 2023 at 08:07:58AM -0600, Danny Kaehn wrote: > > > Bind I2C and GPIO interfaces to subnodes with names > > > "i2c" and "gpio" if they exist, respectively. This > > > allows the GPIO and I2C controllers to be described > > > in firmware as usual. Additionally, support configuring the > > > I2C bus speed from the clock-frequency device property. > > > > A bit shorten indentation... > > > > Nevertheless what I realized now is that this change, despite being OF > > independent by used APIs, still OF-only. > > I assumed this would practically be the case -- not because of the casing > reason you gave (wasn't aware of that, thanks for the FYI), but because it > doesn't seem that there's any way to describe USB devices connected to > a USB port in ACPI, at least as far as I can tell (ACPI is still largely a black > box to me). That's not true :-) Microsoft created a schema that is not part of the specification, but let's say a standard de facto. Linux supports that and I even played with it [1] to get connection of the arbitrary device to USB-2-GPIO/I²C/SPI adapter. > But it seems reasonable that we should try to use the interface > in a way so that it could be described using ACPI at some point (assuming > that it isn't currently possible). > > > Would it be possible to allow indexed access to child nodes as well, so if > > there are no names, we may still be able to use firmware nodes from the correct > > children? > > Sure, you mean to fallback to using child nodes by index rather than by name > in the case that that device_get_named_child_node() fails? Would we need to > somehow verify that those nodes are the nodes we expect them to be? (a.e. > node 0 is actually the i2c-controller, node 1 is actually the > gpio-controller). Something like that, but I don't know if we can actually validate this without having a preliminary agreement (hard-coded values) that index 0 is always let's say I²C controller and so on. > I don't see a reason why not, though I am curious if there is > precedence for this > strategy, a.e. in other drivers that use named child nodes. In my initial search > through the kernel, I don't think I found anything like this -- does that mean > those drivers also inherently won't work with ACPI? If they are relying on names, yes, they won't work. It might be that some of them have specific ACPI approach where different description is in use. > The only driver I can find which uses device_get_named_child_node and has > an acpi_device_id is drivers/platform/x86/intel/chtwc_int33fe.c Yes, and you may notice the capitalization of the name. Also note, that relying on the name in ACPI like now is quite fragile due to no common standard between OEMs on how to name the same hardware nodes, they are free in that. > > P.S. The problem with ACPI is that "name" of the child node will be in capital > > letters as it's in accordance with the specification. > > Knowing that this is the limitation, some other potential resolutions > to potentially > consider might be: > > - Uppercase the names of the child nodes for the DT binding -- it appears > that the child node that chtwc_int33fe.c (the driver mentioned earlier) > accesses does indeed have an upper-cased name -- though that driver doesn't > have an of_device_id (makes sense, x86...). It seems named child nodes are > always lowercase in DT bindings -- not sure if that's a rule, or just how > it currently happens to be. Not an option AFAIK, the DT and ACPI specifications are requiring conflicting naming schema. Possible way is to lowering case for ACPI names, but I dunno. We need more opinions on this. > - Do a case invariant compare on the names (and/or check for both lowercase > and uppercase) For ACPI it will be always capital. For DT I have no clue if they allow capital letters there, but I assume they don't. > - Remove the use of child nodes, and combine the i2c and gpio nodes into the > cp2112's fwnode. I didn't do this initially because I wanted to avoid > namespace collisions between GPIO hogs and i2c child devices, and thought > that logically made sense to keep them separate, but that was before > knowing this limitation of ACPI. Seems to me not an option at all, we need to define hardware as is. If it combines two devices under a hood, should be two devices under a hood in DT/ACPI. [1]: https://stackoverflow.com/a/60855157/2511795 -- With Best Regards, Andy Shevchenko
