| Message ID | 12358058.O9o76ZdvQC@kreacher (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | [v1] ACPI: utils: Introduce helper for _DEP list lookup | expand |
On Thu, Dec 14, 2023 at 12:07:55PM +0100, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > The ACPI LPSS driver and the Surface platform driver code use almost the > same code pattern for checking if one ACPI device is present in the list > returned by _DEP for another ACPI device. > > To reduce the resulting code duplication, introduce a helper for that > called acpi_device_dep() and invoke it from both places. > > No intentional functional impact. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Hi, On 12/14/23 12:07, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > The ACPI LPSS driver and the Surface platform driver code use almost the > same code pattern for checking if one ACPI device is present in the list > returned by _DEP for another ACPI device. > > To reduce the resulting code duplication, introduce a helper for that > called acpi_device_dep() and invoke it from both places. > > No intentional functional impact. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > > This depends on the following series sent last week: > > https://lore.kernel.org/linux-acpi/6008018.lOV4Wx5bFT@kreacher/ Thanks, patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> feel free to merge this through the linux-pm/ACPI tree. Regards, Hans > > --- > drivers/acpi/acpi_lpss.c | 29 +-------------------- > drivers/acpi/utils.c | 33 +++++++++++++++++++++++++ > drivers/platform/surface/surface_acpi_notify.c | 28 --------------------- > include/acpi/acpi_bus.h | 1 > 4 files changed, 37 insertions(+), 54 deletions(-) > > Index: linux-pm/drivers/acpi/acpi_lpss.c > =================================================================== > --- linux-pm.orig/drivers/acpi/acpi_lpss.c > +++ linux-pm/drivers/acpi/acpi_lpss.c > @@ -563,31 +563,6 @@ static struct device *acpi_lpss_find_dev > return bus_find_device(&pci_bus_type, NULL, &data, match_hid_uid); > } > > -static bool acpi_lpss_dep(struct acpi_device *adev, acpi_handle handle) > -{ > - struct acpi_handle_list dep_devices; > - bool ret = false; > - int i; > - > - if (!acpi_has_method(adev->handle, "_DEP")) > - return false; > - > - if (!acpi_evaluate_reference(adev->handle, "_DEP", NULL, &dep_devices)) { > - dev_dbg(&adev->dev, "Failed to evaluate _DEP.\n"); > - return false; > - } > - > - for (i = 0; i < dep_devices.count; i++) { > - if (dep_devices.handles[i] == handle) { > - ret = true; > - break; > - } > - } > - > - acpi_handle_list_free(&dep_devices); > - return ret; > -} > - > static void acpi_lpss_link_consumer(struct device *dev1, > const struct lpss_device_links *link) > { > @@ -598,7 +573,7 @@ static void acpi_lpss_link_consumer(stru > return; > > if ((link->dep_missing_ids && dmi_check_system(link->dep_missing_ids)) > - || acpi_lpss_dep(ACPI_COMPANION(dev2), ACPI_HANDLE(dev1))) > + || acpi_device_dep(ACPI_HANDLE(dev2), ACPI_HANDLE(dev1))) > device_link_add(dev2, dev1, link->flags); > > put_device(dev2); > @@ -614,7 +589,7 @@ static void acpi_lpss_link_supplier(stru > return; > > if ((link->dep_missing_ids && dmi_check_system(link->dep_missing_ids)) > - || acpi_lpss_dep(ACPI_COMPANION(dev1), ACPI_HANDLE(dev2))) > + || acpi_device_dep(ACPI_HANDLE(dev1), ACPI_HANDLE(dev2))) > device_link_add(dev1, dev2, link->flags); > > put_device(dev2); > Index: linux-pm/drivers/acpi/utils.c > =================================================================== > --- linux-pm.orig/drivers/acpi/utils.c > +++ linux-pm/drivers/acpi/utils.c > @@ -450,6 +450,39 @@ void acpi_handle_list_free(struct acpi_h > } > EXPORT_SYMBOL_GPL(acpi_handle_list_free); > > +/** > + * acpi_device_dep - Check ACPI device dependency > + * @target: ACPI handle of the target ACPI device. > + * @match: ACPI handle to look up in the target's _DEP list. > + * > + * Return true if @match is present in the list returned by _DEP for > + * @target or false otherwise. > + */ > +bool acpi_device_dep(acpi_handle target, acpi_handle match) > +{ > + struct acpi_handle_list dep_devices; > + bool ret = false; > + int i; > + > + if (!acpi_has_method(target, "_DEP")) > + return false; > + > + if (!acpi_evaluate_reference(target, "_DEP", NULL, &dep_devices)) { > + acpi_handle_debug(target, "Failed to evaluate _DEP.\n"); > + return false; > + } > + > + for (i = 0; i < dep_devices.count; i++) { > + if (dep_devices.handles[i] == match) { > + ret = true; > + break; > + } > + } > + > + acpi_handle_list_free(&dep_devices); > + return ret; > +} > + > acpi_status > acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld_info **pld) > { > Index: linux-pm/drivers/platform/surface/surface_acpi_notify.c > =================================================================== > --- linux-pm.orig/drivers/platform/surface/surface_acpi_notify.c > +++ linux-pm/drivers/platform/surface/surface_acpi_notify.c > @@ -736,32 +736,6 @@ do { \ > #define san_consumer_warn(dev, handle, fmt, ...) \ > san_consumer_printk(warn, dev, handle, fmt, ##__VA_ARGS__) > > -static bool is_san_consumer(struct platform_device *pdev, acpi_handle handle) > -{ > - struct acpi_handle_list dep_devices; > - acpi_handle supplier = ACPI_HANDLE(&pdev->dev); > - bool ret = false; > - int i; > - > - if (!acpi_has_method(handle, "_DEP")) > - return false; > - > - if (!acpi_evaluate_reference(handle, "_DEP", NULL, &dep_devices)) { > - san_consumer_dbg(&pdev->dev, handle, "failed to evaluate _DEP\n"); > - return false; > - } > - > - for (i = 0; i < dep_devices.count; i++) { > - if (dep_devices.handles[i] == supplier) { > - ret = true; > - break; > - } > - } > - > - acpi_handle_list_free(&dep_devices); > - return ret; > -} > - > static acpi_status san_consumer_setup(acpi_handle handle, u32 lvl, > void *context, void **rv) > { > @@ -770,7 +744,7 @@ static acpi_status san_consumer_setup(ac > struct acpi_device *adev; > struct device_link *link; > > - if (!is_san_consumer(pdev, handle)) > + if (!acpi_device_dep(handle, ACPI_HANDLE(&pdev->dev))) > return AE_OK; > > /* Ignore ACPI devices that are not present. */ > Index: linux-pm/include/acpi/acpi_bus.h > =================================================================== > --- linux-pm.orig/include/acpi/acpi_bus.h > +++ linux-pm/include/acpi/acpi_bus.h > @@ -33,6 +33,7 @@ bool acpi_handle_list_equal(struct acpi_ > void acpi_handle_list_replace(struct acpi_handle_list *dst, > struct acpi_handle_list *src); > void acpi_handle_list_free(struct acpi_handle_list *list); > +bool acpi_device_dep(acpi_handle target, acpi_handle match); > acpi_status > acpi_evaluate_ost(acpi_handle handle, u32 source_event, u32 status_code, > struct acpi_buffer *status_buf); > > >
On Mon, Dec 18, 2023 at 1:13 PM Hans de Goede <hdegoede@redhat.com> wrote: > > Hi, > > On 12/14/23 12:07, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > The ACPI LPSS driver and the Surface platform driver code use almost the > > same code pattern for checking if one ACPI device is present in the list > > returned by _DEP for another ACPI device. > > > > To reduce the resulting code duplication, introduce a helper for that > > called acpi_device_dep() and invoke it from both places. > > > > No intentional functional impact. > > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > --- > > > > This depends on the following series sent last week: > > > > https://lore.kernel.org/linux-acpi/6008018.lOV4Wx5bFT@kreacher/ > > Thanks, patch looks good to me: > > Reviewed-by: Hans de Goede <hdegoede@redhat.com> > > feel free to merge this through the linux-pm/ACPI tree. Thank you!
Index: linux-pm/drivers/acpi/acpi_lpss.c =================================================================== --- linux-pm.orig/drivers/acpi/acpi_lpss.c +++ linux-pm/drivers/acpi/acpi_lpss.c @@ -563,31 +563,6 @@ static struct device *acpi_lpss_find_dev return bus_find_device(&pci_bus_type, NULL, &data, match_hid_uid); } -static bool acpi_lpss_dep(struct acpi_device *adev, acpi_handle handle) -{ - struct acpi_handle_list dep_devices; - bool ret = false; - int i; - - if (!acpi_has_method(adev->handle, "_DEP")) - return false; - - if (!acpi_evaluate_reference(adev->handle, "_DEP", NULL, &dep_devices)) { - dev_dbg(&adev->dev, "Failed to evaluate _DEP.\n"); - return false; - } - - for (i = 0; i < dep_devices.count; i++) { - if (dep_devices.handles[i] == handle) { - ret = true; - break; - } - } - - acpi_handle_list_free(&dep_devices); - return ret; -} - static void acpi_lpss_link_consumer(struct device *dev1, const struct lpss_device_links *link) { @@ -598,7 +573,7 @@ static void acpi_lpss_link_consumer(stru return; if ((link->dep_missing_ids && dmi_check_system(link->dep_missing_ids)) - || acpi_lpss_dep(ACPI_COMPANION(dev2), ACPI_HANDLE(dev1))) + || acpi_device_dep(ACPI_HANDLE(dev2), ACPI_HANDLE(dev1))) device_link_add(dev2, dev1, link->flags); put_device(dev2); @@ -614,7 +589,7 @@ static void acpi_lpss_link_supplier(stru return; if ((link->dep_missing_ids && dmi_check_system(link->dep_missing_ids)) - || acpi_lpss_dep(ACPI_COMPANION(dev1), ACPI_HANDLE(dev2))) + || acpi_device_dep(ACPI_HANDLE(dev1), ACPI_HANDLE(dev2))) device_link_add(dev1, dev2, link->flags); put_device(dev2); Index: linux-pm/drivers/acpi/utils.c =================================================================== --- linux-pm.orig/drivers/acpi/utils.c +++ linux-pm/drivers/acpi/utils.c @@ -450,6 +450,39 @@ void acpi_handle_list_free(struct acpi_h } EXPORT_SYMBOL_GPL(acpi_handle_list_free); +/** + * acpi_device_dep - Check ACPI device dependency + * @target: ACPI handle of the target ACPI device. + * @match: ACPI handle to look up in the target's _DEP list. + * + * Return true if @match is present in the list returned by _DEP for + * @target or false otherwise. + */ +bool acpi_device_dep(acpi_handle target, acpi_handle match) +{ + struct acpi_handle_list dep_devices; + bool ret = false; + int i; + + if (!acpi_has_method(target, "_DEP")) + return false; + + if (!acpi_evaluate_reference(target, "_DEP", NULL, &dep_devices)) { + acpi_handle_debug(target, "Failed to evaluate _DEP.\n"); + return false; + } + + for (i = 0; i < dep_devices.count; i++) { + if (dep_devices.handles[i] == match) { + ret = true; + break; + } + } + + acpi_handle_list_free(&dep_devices); + return ret; +} + acpi_status acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld_info **pld) { Index: linux-pm/drivers/platform/surface/surface_acpi_notify.c =================================================================== --- linux-pm.orig/drivers/platform/surface/surface_acpi_notify.c +++ linux-pm/drivers/platform/surface/surface_acpi_notify.c @@ -736,32 +736,6 @@ do { \ #define san_consumer_warn(dev, handle, fmt, ...) \ san_consumer_printk(warn, dev, handle, fmt, ##__VA_ARGS__) -static bool is_san_consumer(struct platform_device *pdev, acpi_handle handle) -{ - struct acpi_handle_list dep_devices; - acpi_handle supplier = ACPI_HANDLE(&pdev->dev); - bool ret = false; - int i; - - if (!acpi_has_method(handle, "_DEP")) - return false; - - if (!acpi_evaluate_reference(handle, "_DEP", NULL, &dep_devices)) { - san_consumer_dbg(&pdev->dev, handle, "failed to evaluate _DEP\n"); - return false; - } - - for (i = 0; i < dep_devices.count; i++) { - if (dep_devices.handles[i] == supplier) { - ret = true; - break; - } - } - - acpi_handle_list_free(&dep_devices); - return ret; -} - static acpi_status san_consumer_setup(acpi_handle handle, u32 lvl, void *context, void **rv) { @@ -770,7 +744,7 @@ static acpi_status san_consumer_setup(ac struct acpi_device *adev; struct device_link *link; - if (!is_san_consumer(pdev, handle)) + if (!acpi_device_dep(handle, ACPI_HANDLE(&pdev->dev))) return AE_OK; /* Ignore ACPI devices that are not present. */ Index: linux-pm/include/acpi/acpi_bus.h =================================================================== --- linux-pm.orig/include/acpi/acpi_bus.h +++ linux-pm/include/acpi/acpi_bus.h @@ -33,6 +33,7 @@ bool acpi_handle_list_equal(struct acpi_ void acpi_handle_list_replace(struct acpi_handle_list *dst, struct acpi_handle_list *src); void acpi_handle_list_free(struct acpi_handle_list *list); +bool acpi_device_dep(acpi_handle target, acpi_handle match); acpi_status acpi_evaluate_ost(acpi_handle handle, u32 source_event, u32 status_code, struct acpi_buffer *status_buf);