On Wed, Mar 01, 2023 at 04:59:07PM +0200, Andy Shevchenko wrote: > 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. Forgot to mention, that you need to take into consideration the drivers that are: 1) can be compiled with CONFIG_OF=n (i.e. using agnostic APIs); 2) having OF ID table; 3) being platform independent (no Kconfig dependency to something that's known to be an ARCH/CPU/etc one). All of those can be instantiated in ACPI via special PRP0001 ACPI ID [2]. > > > 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 [2]: https://docs.kernel.org/firmware-guide/acpi/enumeration.html#device-tree-namespace-link-device-id -- With Best Regards, Andy Shevchenko
