[PATCH v2] efi/dev-path-parser: Switch to acpi_dev_get_first_match_dev()

[PATCH v2] efi/dev-path-parser: Switch to acpi_dev_get_first_match_dev()

[Andy Shevchenko]

The acpi_dev_get_first_match_dev() helper will find and return
an ACPI device pointer of the first registered device in the system
by its HID and UID (if present). Use it instead of open coded variant.

Cc: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: added Lukas to Cc (Ard), updated condition to handle UID=0 case
 drivers/firmware/efi/dev-path-parser.c | 43 +++++++++-----------------
 1 file changed, 14 insertions(+), 29 deletions(-)

diff --git a/drivers/firmware/efi/dev-path-parser.c b/drivers/firmware/efi/dev-path-parser.c
index 5c9625e552f4..d3e5123ab618 100644
--- a/drivers/firmware/efi/dev-path-parser.c
+++ b/drivers/firmware/efi/dev-path-parser.c
@@ -12,51 +12,36 @@
 #include <linux/efi.h>
 #include <linux/pci.h>
 
-struct acpi_hid_uid {
-	struct acpi_device_id hid[2];
-	char uid[11]; /* UINT_MAX + null byte */
-};
-
-static int __init match_acpi_dev(struct device *dev, const void *data)
-{
-	struct acpi_hid_uid hid_uid = *(const struct acpi_hid_uid *)data;
-	struct acpi_device *adev = to_acpi_device(dev);
-
-	if (acpi_match_device_ids(adev, hid_uid.hid))
-		return 0;
-
-	if (adev->pnp.unique_id)
-		return !strcmp(adev->pnp.unique_id, hid_uid.uid);
-	else
-		return !strcmp("0", hid_uid.uid);
-}
-
 static long __init parse_acpi_path(const struct efi_dev_path *node,
 				   struct device *parent, struct device **child)
 {
-	struct acpi_hid_uid hid_uid = {};
+	char hid[3 + 4 + 1];	/* 3 characters + 4 hex digits + null byte */
+	char uid[10 + 1];	/* UINT_MAX + null byte */
+	struct acpi_device *adev;
 	struct device *phys_dev;
 
 	if (node->header.length != 12)
 		return -EINVAL;
 
-	sprintf(hid_uid.hid[0].id, "%c%c%c%04X",
+	sprintf(hid, "%c%c%c%04X",
 		'A' + ((node->acpi.hid >> 10) & 0x1f) - 1,
 		'A' + ((node->acpi.hid >>  5) & 0x1f) - 1,
 		'A' + ((node->acpi.hid >>  0) & 0x1f) - 1,
 			node->acpi.hid >> 16);
-	sprintf(hid_uid.uid, "%u", node->acpi.uid);
+	sprintf(uid, "%u", node->acpi.uid);
 
-	*child = bus_find_device(&acpi_bus_type, NULL, &hid_uid,
-				 match_acpi_dev);
-	if (!*child)
+	adev = acpi_dev_get_first_match_dev(hid, uid, -1);
+	if (!adev && !node->acpi.uid)
+		adev = acpi_dev_get_first_match_dev(hid, NULL, -1);
+	if (!adev)
 		return -ENODEV;
 
-	phys_dev = acpi_get_first_physical_node(to_acpi_device(*child));
+	phys_dev = acpi_get_first_physical_node(adev);
 	if (phys_dev) {
-		get_device(phys_dev);
-		put_device(*child);
-		*child = phys_dev;
+		*child = get_device(phys_dev);
+		acpi_dev_put(adev);
+	} else {
+		*child = &adev->dev;
 	}
 
 	return 0;
-- 
2.26.2

Re: [PATCH v2] efi/dev-path-parser: Switch to acpi_dev_get_first_match_dev()

[Andy Shevchenko]

On Fri, May 29, 2020 at 04:08:22PM +0300, Andy Shevchenko wrote:
> The acpi_dev_get_first_match_dev() helper will find and return
> an ACPI device pointer of the first registered device in the system
> by its HID and UID (if present). Use it instead of open coded variant.

Lukas, any comment on this?

> Cc: Lukas Wunner <lukas@wunner.de>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> v2: added Lukas to Cc (Ard), updated condition to handle UID=0 case
>  drivers/firmware/efi/dev-path-parser.c | 43 +++++++++-----------------
>  1 file changed, 14 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/firmware/efi/dev-path-parser.c b/drivers/firmware/efi/dev-path-parser.c
> index 5c9625e552f4..d3e5123ab618 100644
> --- a/drivers/firmware/efi/dev-path-parser.c
> +++ b/drivers/firmware/efi/dev-path-parser.c
> @@ -12,51 +12,36 @@
>  #include <linux/efi.h>
>  #include <linux/pci.h>
>  
> -struct acpi_hid_uid {
> -	struct acpi_device_id hid[2];
> -	char uid[11]; /* UINT_MAX + null byte */
> -};
> -
> -static int __init match_acpi_dev(struct device *dev, const void *data)
> -{
> -	struct acpi_hid_uid hid_uid = *(const struct acpi_hid_uid *)data;
> -	struct acpi_device *adev = to_acpi_device(dev);
> -
> -	if (acpi_match_device_ids(adev, hid_uid.hid))
> -		return 0;
> -
> -	if (adev->pnp.unique_id)
> -		return !strcmp(adev->pnp.unique_id, hid_uid.uid);
> -	else
> -		return !strcmp("0", hid_uid.uid);
> -}
> -
>  static long __init parse_acpi_path(const struct efi_dev_path *node,
>  				   struct device *parent, struct device **child)
>  {
> -	struct acpi_hid_uid hid_uid = {};
> +	char hid[3 + 4 + 1];	/* 3 characters + 4 hex digits + null byte */
> +	char uid[10 + 1];	/* UINT_MAX + null byte */
> +	struct acpi_device *adev;
>  	struct device *phys_dev;
>  
>  	if (node->header.length != 12)
>  		return -EINVAL;
>  
> -	sprintf(hid_uid.hid[0].id, "%c%c%c%04X",
> +	sprintf(hid, "%c%c%c%04X",
>  		'A' + ((node->acpi.hid >> 10) & 0x1f) - 1,
>  		'A' + ((node->acpi.hid >>  5) & 0x1f) - 1,
>  		'A' + ((node->acpi.hid >>  0) & 0x1f) - 1,
>  			node->acpi.hid >> 16);
> -	sprintf(hid_uid.uid, "%u", node->acpi.uid);
> +	sprintf(uid, "%u", node->acpi.uid);
>  
> -	*child = bus_find_device(&acpi_bus_type, NULL, &hid_uid,
> -				 match_acpi_dev);
> -	if (!*child)
> +	adev = acpi_dev_get_first_match_dev(hid, uid, -1);
> +	if (!adev && !node->acpi.uid)
> +		adev = acpi_dev_get_first_match_dev(hid, NULL, -1);
> +	if (!adev)
>  		return -ENODEV;
>  
> -	phys_dev = acpi_get_first_physical_node(to_acpi_device(*child));
> +	phys_dev = acpi_get_first_physical_node(adev);
>  	if (phys_dev) {
> -		get_device(phys_dev);
> -		put_device(*child);
> -		*child = phys_dev;
> +		*child = get_device(phys_dev);
> +		acpi_dev_put(adev);
> +	} else {
> +		*child = &adev->dev;
>  	}
>  
>  	return 0;
> -- 
> 2.26.2
> 

-- 
With Best Regards,
Andy Shevchenko