[cc += Robert Moore] Hi Hans, I'm the author of acpi_dev_found(), please in the future use git blame to determine relevant authors of existing code that should be cc'ed, I noticed your patch only now: On Thursday, March 16, 2017 05:17:35 PM Hans de Goede wrote: > acpi_dev_found just iterates over all acpi-ids and sees if one matches. > This means that it will return true for devices which are in the dsdt > but disabled (their _STA method returns 0). > > For some drivers it is useful to be able to check if a certain hid > is not only present in the namespace, but also actually present as in > acpi_device_is_present() will return true for the device. For example > because if a certain device is present then the driver will want to use > an extcon or IIO adc channel provided by that device. > > This commit adds a new acpi_dev_present helper which drivers can use > to this end. > > Arguably acpi_dev_present is what acpi_dev_found should have been, but > there are too many users to just change acpi_dev_found without the risk > of breaking something. I originally did submit an acpi_dev_present() function which was identical to what you've submitted now: http://www.spinics.net/lists/linux-acpi/msg61865.html However Robert Moore raised an objection that "Traversing the namespace over and over is truly brute force": http://www.spinics.net/lists/linux-acpi/msg61911.html For my use case, which was apple_gmux_present(), just detecting presence of the HID in the namespace was sufficient, I did not have the need to execute _STA. Hence to address Robert Moore's concern I switched to simply traversing the acpi_bus_id_list. Rafael objected to the acpi_dev_present() name as it suggested that _STA is checked even though it wasn't, so I renamed the function to acpi_dev_found() with commit c68ae33e7fb4. The objection raised by Robert Moore applies to your patch as well since it is identical to my original patch. The return value of _STA seems to be cached in the "status" field of struct acpi_device, so you may be able to overcome Robert Moore's objection by calling bus_find_device() with a callback which contains the check in acpi_device_is_present(). See drivers/firmware/efi/dev-path-parser.c for an example (parse_acpi_path() and match_acpi_dev()). This is probably faster than acpi_get_devices() because it just traverses a list instead of walking the namespace and it avoids the call to _STA. (Some devices just return a constant when _STA is called, others may take more time.) FWIW, all existing users of acpi_dev_found(), except for the hisilicon Ethernet driver, originally used acpi_get_devices() and I converted them to acpi_dev_found to deduplicate code. Thus it would be safe to convert those to your new function acpi_dev_present(). I also converted sound/soc/intel/common/sst-match-acpi.c to acpi_dev_found() but the Intel folks switched back to acpi_get_devices() because just like you they had the need to check _STA. If you introduce a new helper which checks _STA, it would be good if you could amend sst-match-acpi.c to use it so as to deduplicate code. Thanks, Lukas -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
