diff mbox series

[v1] ACPI: utils: Introduce helper for _DEP list lookup

Message ID 12358058.O9o76ZdvQC@kreacher
State New
Headers show
Series [v1] ACPI: utils: Introduce helper for _DEP list lookup | expand

Commit Message

Rafael J. Wysocki Dec. 14, 2023, 11:07 a.m. UTC
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

Hans de Goede Dec. 18, 2023, 12:13 p.m. UTC | #1
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);
> 
> 
>
Rafael J. Wysocki Dec. 18, 2023, 12:24 p.m. UTC | #2
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!
diff mbox series

Patch

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);