[v1] ACPI: utils: Introduce helper for _DEP list lookup
Commit Message
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/
---
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(-)
Comments
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!
===================================================================
@@ -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);
===================================================================
@@ -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)
{
===================================================================
@@ -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. */
===================================================================
@@ -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);