| Message ID | 14285317.C5USrgceVP@vostro.rjw.lan (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Wednesday, December 26, 2012 11:42:49 PM Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Subject: ACPI: Rework acpi_get_child() to be more efficient > > Observe that acpi_get_child() doesn't need to use the helper > struct acpi_find_child structure and change it to work without it. > Also, using acpi_get_object_info() to get the output of _ADR for the > given device is overkill, because that function does much more than > just evaluating _ADR (let alone the additional memory allocation > done by it). > > Moreover, acpi_get_child() doesn't need to loop any more once it has > found a matching handle, so make it stop in that case. To prevent > the results from changing, make it use do_acpi_find_child() as > a post-order callback. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > > The difference from the previous version is the replacement of > acpi_get_object_info() with the direct execution of _ADR, which is > more straightforward. There seem to be no objections, most likely due to the holiday period that has just ended, but if there are still none, I'm going to push this for v3.9. Thanks, Rafael > --- > drivers/acpi/glue.c | 33 ++++++++++++--------------------- > 1 file changed, 12 insertions(+), 21 deletions(-) > > Index: linux/drivers/acpi/glue.c > =================================================================== > --- linux.orig/drivers/acpi/glue.c > +++ linux/drivers/acpi/glue.c > @@ -93,40 +93,31 @@ static int acpi_find_bridge_device(struc > return ret; > } > > -/* Get device's handler per its address under its parent */ > -struct acpi_find_child { > - acpi_handle handle; > - u64 address; > -}; > - > -static acpi_status > -do_acpi_find_child(acpi_handle handle, u32 lvl, void *context, void **rv) > +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; > - struct acpi_device_info *info; > - struct acpi_find_child *find = context; > > - status = acpi_get_object_info(handle, &info); > - if (ACPI_SUCCESS(status)) { > - if ((info->address == find->address) > - && (info->valid & ACPI_VALID_ADR)) > - find->handle = handle; > - kfree(info); > + status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, &addr); > + if (ACPI_SUCCESS(status) && addr == *((u64 *)addr_p)) { > + *ret_p = handle; > + return AE_CTRL_TERMINATE; > } > return AE_OK; > } > > acpi_handle acpi_get_child(acpi_handle parent, u64 address) > { > - struct acpi_find_child find = { NULL, address }; > + void *ret = NULL; > > if (!parent) > return NULL; > - acpi_walk_namespace(ACPI_TYPE_DEVICE, parent, > - 1, do_acpi_find_child, NULL, &find, NULL); > - return find.handle; > -} > > + acpi_walk_namespace(ACPI_TYPE_DEVICE, parent, 1, NULL, > + do_acpi_find_child, &address, &ret); > + return (acpi_handle)ret; > +} > EXPORT_SYMBOL(acpi_get_child); > > static int acpi_bind_one(struct device *dev, acpi_handle handle) > > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rafael If it add a _STA evaluating at do_acpi_find_child , is it reasonable ? If No, where _STA evaluating should be added ? At our platform ,ODD8/ODDZ/ODDL have the same _ADR. 1. Address: 0x1FFFF (valid); handle: ¬SB_.PCI0.SATA.ODD8 2. Address: 0x1FFFF (valid); handle: ¬SB_.PCI0.SATA.ODDZ 3. Address: 0x1FFFF (valid); handle: ¬SB_.PCI0.SATA.ODDL if one device is enabled(_STA evaluating return 0xF), the others will be disabled (_STA evaluating return 0). With the current do_acpi_find_child,If ODDL is enabled , the correct handle should be " ¬SB_.PCI0.SATA.ODDL", but do_acpi_find_child always return "¬SB_.PCI0.SATA.ODD8" handle. So ,we want to add a _STA evaluating workaround at do_acpi_find_child to fix this issue. Thanks Jeff Wu -----Original Message----- From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Rafael J. Wysocki Sent: Saturday, January 05, 2013 5:39 AM To: ACPI Devel Maling List Cc: LKML; Len Brown Subject: Re: [Update][PATCH] ACPI: Rework acpi_get_child() to be more efficient On Wednesday, December 26, 2012 11:42:49 PM Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Subject: ACPI: Rework acpi_get_child() to be more efficient > > Observe that acpi_get_child() doesn't need to use the helper > struct acpi_find_child structure and change it to work without it. > Also, using acpi_get_object_info() to get the output of _ADR for the > given device is overkill, because that function does much more than > just evaluating _ADR (let alone the additional memory allocation > done by it). > > Moreover, acpi_get_child() doesn't need to loop any more once it has > found a matching handle, so make it stop in that case. To prevent > the results from changing, make it use do_acpi_find_child() as > a post-order callback. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > > The difference from the previous version is the replacement of > acpi_get_object_info() with the direct execution of _ADR, which is > more straightforward. There seem to be no objections, most likely due to the holiday period that has just ended, but if there are still none, I'm going to push this for v3.9. Thanks, Rafael > --- > drivers/acpi/glue.c | 33 ++++++++++++--------------------- > 1 file changed, 12 insertions(+), 21 deletions(-) > > Index: linux/drivers/acpi/glue.c > =================================================================== > --- linux.orig/drivers/acpi/glue.c > +++ linux/drivers/acpi/glue.c > @@ -93,40 +93,31 @@ static int acpi_find_bridge_device(struc > return ret; > } > > -/* Get device's handler per its address under its parent */ > -struct acpi_find_child { > - acpi_handle handle; > - u64 address; > -}; > - > -static acpi_status > -do_acpi_find_child(acpi_handle handle, u32 lvl, void *context, void **rv) > +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; > - struct acpi_device_info *info; > - struct acpi_find_child *find = context; > > - status = acpi_get_object_info(handle, &info); > - if (ACPI_SUCCESS(status)) { > - if ((info->address == find->address) > - && (info->valid & ACPI_VALID_ADR)) > - find->handle = handle; > - kfree(info); > + status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, &addr); > + if (ACPI_SUCCESS(status) && addr == *((u64 *)addr_p)) { > + *ret_p = handle; > + return AE_CTRL_TERMINATE; > } > return AE_OK; > } > > acpi_handle acpi_get_child(acpi_handle parent, u64 address) > { > - struct acpi_find_child find = { NULL, address }; > + void *ret = NULL; > > if (!parent) > return NULL; > - acpi_walk_namespace(ACPI_TYPE_DEVICE, parent, > - 1, do_acpi_find_child, NULL, &find, NULL); > - return find.handle; > -} > > + acpi_walk_namespace(ACPI_TYPE_DEVICE, parent, 1, NULL, > + do_acpi_find_child, &address, &ret); > + return (acpi_handle)ret; > +} > EXPORT_SYMBOL(acpi_get_child); > > static int acpi_bind_one(struct device *dev, acpi_handle handle) > > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Index: linux/drivers/acpi/glue.c =================================================================== --- linux.orig/drivers/acpi/glue.c +++ linux/drivers/acpi/glue.c @@ -93,40 +93,31 @@ static int acpi_find_bridge_device(struc return ret; } -/* Get device's handler per its address under its parent */ -struct acpi_find_child { - acpi_handle handle; - u64 address; -}; - -static acpi_status -do_acpi_find_child(acpi_handle handle, u32 lvl, void *context, void **rv) +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; - struct acpi_device_info *info; - struct acpi_find_child *find = context; - status = acpi_get_object_info(handle, &info); - if (ACPI_SUCCESS(status)) { - if ((info->address == find->address) - && (info->valid & ACPI_VALID_ADR)) - find->handle = handle; - kfree(info); + status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, &addr); + if (ACPI_SUCCESS(status) && addr == *((u64 *)addr_p)) { + *ret_p = handle; + return AE_CTRL_TERMINATE; } return AE_OK; } acpi_handle acpi_get_child(acpi_handle parent, u64 address) { - struct acpi_find_child find = { NULL, address }; + void *ret = NULL; if (!parent) return NULL; - acpi_walk_namespace(ACPI_TYPE_DEVICE, parent, - 1, do_acpi_find_child, NULL, &find, NULL); - return find.handle; -} + acpi_walk_namespace(ACPI_TYPE_DEVICE, parent, 1, NULL, + do_acpi_find_child, &address, &ret); + return (acpi_handle)ret; +} EXPORT_SYMBOL(acpi_get_child); static int acpi_bind_one(struct device *dev, acpi_handle handle)