Re: [PATCH] ACPI:add _STA evaluating at do_acpi_find_child()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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?

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




[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux