On Thu, 6 Jun 2019 17:48:00 +0200, Pali Rohár wrote: > On Thursday 06 June 2019 16:53:09 Jean Delvare wrote: > > This is testing that *either* bit is set. Is it what you intend to > > achieve, or would you rather want to ensure that *all* these bits are > > set? > > Of course, it is wrong. Thanks for catch. We should ignore apci devices > which are not present, which are disabled or which are not functioning. > > Now I looked into acpi_get_devices() implementation and it call > acpi_ns_get_device_callback() function callback for every device. At the > end that function calls user supplied check_acpi_smo88xx_device > function. > > And acpi_ns_get_device_callback() already ignores acpi devices which do > not have ACPI_STA_DEVICE_PRESENT or ACPI_STA_DEVICE_FUNCTIONING flag. > > According to acpi documentation when ACPI_STA_DEVICE_PRESENT is not set > then ACPI_STA_DEVICE_ENABLED also cannot be set. > > So the whole acpi_bus_get_status_handle() is not needed at all as > acpi_get_devices() via acpi_ns_get_device_callback() already filter > unsuitable acpi devices. > > I guess that I already did this investigation in past and added comment > "exists and is enabled" which is below near acpi_get_devices() call. But > as I wrote this patch more then year ago I forgot about it. Sorry for confusing you :-( > I will remove that check. Do you have any suggestion what to write into > comment so other readers in future would know that we do not need to > check anything with _STA acpi method? Well, if you manage to squeeze a short version of the above explanation at the relevant place in the driver, that should work. Doesn't have to be verbose really, anything that says that disabled devices will be properly skipped for us will be good enough. -- Jean Delvare SUSE L3 Support
