On Fri, Nov 13, 2020 at 6:22 PM Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> wrote: > On Fri, Nov 13, 2020 at 10:02:30AM +0000, Dan Scally wrote: > > On 29/10/2020 22:51, Laurent Pinchart wrote: > > > On Fri, Oct 30, 2020 at 12:22:15AM +0200, Andy Shevchenko wrote: > > >> On Thu, Oct 29, 2020 at 11:29:30PM +0200, Laurent Pinchart wrote: ... > > >> In this case we probably need something like > > >> > > >> struct acpi_device * > > >> acpi_dev_get_next_match_dev(struct acpi_device *adev, > > >> const char *hid, const char *uid, s64 hrv) > > >> { > > >> struct device *start = adev ? &adev->dev : NULL; > > >> ... > > >> dev = bus_find_device(&acpi_bus_type, start, &match, acpi_dev_match_cb); > > >> ... > > >> } > > >> > > >> in drivers/acpi/utils.c and > > >> > > >> static inline struct acpi_device * > > >> acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv) > > >> { > > >> return acpi_dev_get_next_match_dev(NULL, hid, uid, hrv); > > >> } > > >> > > >> in include/linux/acpi.h. > > >> > > >> Then we may add > > >> > > >> #define for_each_acpi_dev_match(adev, hid, uid, hrv) \ > > >> for (adev = acpi_dev_get_first_match_dev(hid, uid, hrv); \ > > >> adev; \ > > >> adev = acpi_dev_get_next_match_dev(adev, hid, uid, hrv)) > > > > > > What the cio2-bridge code needs is indeed > > > > > > for each hid in supported hids: > > > for each acpi device that is compatible with hid: > > > ... > > > > > > which could also be expressed as > > > > > > for each acpi device: > > > if acpi device hid is in supported hids: > > > ... > > > > > > I don't mind either option, I'll happily follow the preference of the > > > ACPI maintainers. > > > > Does this need raising elsewhere then? The original idea of just > > bus_for_each_dev(&acpi_bus_type...) I have now tested and it works fine, > > but it does mean that I need to export acpi_bus_type (currently that > > symbol's not available)...that seems much simpler to me but I'm not sure > > whether that's something to avoid, and if so whether Andy's approach is > > better. > > > > Thoughts? > > I like simple options :-) A patch to export acpi_bus_type, with enough > context in the commit message (and in the cover latter of the series), > should be enough to provide all the information the ACPI maintainers > need to decide which option is best. With a bit of luck that patch will > be considered the best option and no extra work will be needed. The problem with ACPI bus is that it is not as simple as other buses, i.e. it may have purely ACPI devices along with *companion* devices, which are usually represented by platform bus. On top of that for several ACPI devices there can be one physical node and it will be not so clear what you are exactly looking for by traversing acpi_bus_type. I believe it's hidden on purpose. -- With Best Regards, Andy Shevchenko