On 16/11/2020 16:16, Andy Shevchenko wrote: > On Mon, Nov 16, 2020 at 02:15:01PM +0000, Dan Scally wrote: >> On 16/11/2020 14:10, Laurent Pinchart wrote: >>> I thought we were looking for ACPI devices, not companion devices, in >>> order to extract information from the DSDT and store it in a software >>> node. I could very well be wrong though. >> This is correct - the code to fetch the various resources we're looking >> at all uses acpi_device. Whether using Andy's iterator suggestions or >> previous bus_for_each_dev(&acpi_bus_type...) I'm just getting the >> acpi_device via to_acpi_dev() and using that. > If you try to get an I²C ore SPI device out of pure ACPI device (with given > APCI _HID) you will fail. So, it's not correct. You are retrieving companion > devices, while they are still in the struct acpi_device. > > And don't ask me, why it's so. I wasn't designed that and didn't affect any > decision made there. Well, in terms of the actual device we're getting, I don't think we're fundamentally doing anything different between the methods...unless I'm really mistaken. Originally implementation was like: const char *supported_devices[] = { "OVTI2680", }; static int cio2_bridge_connect_supported_devices(void) { struct acpi_device *adev; int i; for (i = 0; i < ARRAY_SIZE(supported_devices); i++) { adev = acpi_dev_get_first_match_dev(supported_devices[i], NULL, -1); ... } and acpi_dev_get_first_match_dev() likewise just returns adev via to_acpi_device(dev). So, maybe we don't need to do the iterating over all devices with matching _HID at all, in which case it can be dropped, but if we're doing it then I can't see that it's different to the original implementation in terms of the struct acpi_device we're working with or the route taken to get it. Either way; ACPI maintainers asked to be CC'd on the next patchset anyway, so they'll see what we're doing and be able to weigh in.