2013/5/23 Rafael J. Wysocki <rjw@xxxxxxx>: > On Wednesday, May 22, 2013 03:42:04 PM Jeff Wu wrote: >> 2013/5/22 Lan Tianyu <lantianyu1986@xxxxxxxxx>: >> > 2013/5/22 Jeff Wu <zlinuxkernel@xxxxxxxxx>: >> >> 2013/5/21 Lan Tianyu <lantianyu1986@xxxxxxxxx>: >> >>>> From: Jeff Wu <zlinuxkernel@xxxxxxxxx> >> >>>> >> >>>> Once do_acpi_find_child() found the first matching handle, it makes the acpi_get_child() >> >>>> loop stop and return the matching handle. But, at some of the platforms, a "_ADR" is >> >>>> with the multiple devices, and if one is enabled, the others will be disabled. >> >>>> Just like a case: >> >>>> Address : 0x1FFFF ; handle : SB_PCI0.SATA.DEV0 >> >>>> Address : 0x1FFFF ; handle : SB_PCI0.SATA.DEV1 >> >>>> Address : 0x1FFFF ; handle : SB_PCI0.SATA.DEV2 >> >>>> >> >>>> If DEV0 and DEV1 are disabled and DEV2 is enabled, the target is to get DEV2 handle, but >> >>>> actually it always return the first disabled device DEV0 handle. So add a _STA evaluating >> >>>> at do_acpi_find_child() to check the device status. If the object exists, but it is disabled, >> >>>> acpi_get_child() will continue looping to get the correct handle that the object exists and >> >>>> is enabled. >> >>>> >> >>>> >> >>>> Signed-off-by: Jeff Wu <zlinuxkernel@xxxxxxxxx> >> >>>> --- >> >>>> drivers/acpi/glue.c | 5 ++++- >> >>>> 1 file changed, 4 insertions(+), 1 deletion(-) >> >>>> >> >>>> diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c >> >>>> index 40a84cc..ace6c2e 100644 >> >>>> --- a/drivers/acpi/glue.c >> >>>> +++ b/drivers/acpi/glue.c >> >>>> @@ -81,11 +81,14 @@ static struct acpi_bus_type *acpi_get_bus_type(struct device *dev) >> >>>> static acpi_status do_acpi_find_child(acpi_handle handle, u32 lvl_not_used, >> >>>> void *addr_p, void **ret_p) >> >>>> { >> >>>> - unsigned long long addr; >> >>>> acpi_status status; >> >>>> + unsigned long long addr = 0, sta = 0; >> >>>> >> >>>> status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, &addr); >> >>>> if (ACPI_SUCCESS(status) && addr == *((u64 *)addr_p)) { >> >>>> + status = acpi_bus_get_status_handle(handle, &sta); >> >>>> + if (ACPI_SUCCESS(status) && !(sta & ACPI_STA_DEVICE_ENABLED)) >> >>>> + return AE_CTRL_DEPTH; >> >>> Will this affect other devices? Since the device maybe disabled during >> >>> do binding with ACPI. >> >>> After some operations, the device would be enabled. >> >> At our platforms, DEV1 use for win8,DEV2 use for win7,DEV1 use for Linux. >> >> If these devices are disabled when do init, they will always be >> >> disabled ,so, work fine. >> >> >> >> At some of the platforms, the devices may be disabled during do the >> >> first binding ,if their status are changed,do they not do re-binding ? >> > Currently, the glue binding operation takes place when the device is >> > created. In my mind, there is no re-binding operation after the device being >> > created, Unless the device being removed and created again. >> Thank you very much. >> So, as your suggestion, it is a better solution to add a new function >> for the same _ADR devices, > > I don't really think this is a good idea, because then it won't be clear when > to use which version. > > Your patch kind of makes sense (although we don't need to initialize both > local variables to 0), but the Tianyu's concern is valid in principle either. > > Perhaps it would be better to make do_acpi_find_child() return the disabled > device if its the only one matching or the first enabled matching device > otherwise? Hi Rafael, Thanks, i will try it , if it is ok, i will send out the patch. > > Rafael > > >> >>> Hi Rafael: >> >>> How about add a new function >> >>> "acpi_get_children_with_same_addr" to return a list >> >>> which contains all these child devices with same _ADR.And then do some >> >>> special check in the specific >> >>> bus glue callback. Since these cases are no common. >> >>> >> >>>> *ret_p = handle; >> >>>> return AE_CTRL_TERMINATE; >> >>>> } >> >>>> -- >> >>>> 1.8.1.2 >> >>>> >> >>>> >> >>>> -- >> >>>> 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 >> >>> >> >>> >> >>> >> >>> -- >> >>> Best regards >> >>> Tianyu Lan >> > >> > >> > >> > -- >> > Best regards >> > Tianyu Lan >> -- >> 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 > -- > I speak only for myself. > Rafael J. Wysocki, Intel Open Source Technology Center. -- 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