[v1,1/8] ACPI: utils: Introduce acpi_dev_uid_match() for matching _UID

Message ID 20231020084732.17130-2-raag.jadav@intel.com
State New
Headers
Series Refine _UID references across kernel |

Commit Message

Raag Jadav Oct. 20, 2023, 8:47 a.m. UTC
  Introduce acpi_dev_uid_match() helper that matches the device with
supplied _UID string.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Raag Jadav <raag.jadav@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/acpi/utils.c    | 40 +++++++++++++++++++++++++++++++++-------
 include/acpi/acpi_bus.h |  1 +
 include/linux/acpi.h    |  5 +++++
 3 files changed, 39 insertions(+), 7 deletions(-)
  

Comments

Andy Shevchenko Oct. 20, 2023, 10:35 a.m. UTC | #1
On Fri, Oct 20, 2023 at 02:17:25PM +0530, Raag Jadav wrote:
> Introduce acpi_dev_uid_match() helper that matches the device with
> supplied _UID string.

...

> +/**
> + * acpi_dev_uid_match - Match device by supplied UID
> + * @adev: ACPI device to match.
> + * @uid2: Unique ID of the device, pass NULL to not check _UID.
> + *
> + * Matches UID in @adev with given @uid2. Absence of @uid2 will be treated as
> + * a match. If user wants to validate @uid2, it should be done before calling
> + * this function. This behaviour is as needed by most of its current users.

The "current" internally I applied to the result of the series. So reading this
again I think the better wording is "potential".

> + *
> + * Returns:
> + *  - %true if matches or @uid2 is NULL.
> + *  - %false otherwise.
> + */

...

>  /**
>   * acpi_dev_hid_uid_match - Match device by supplied HID and UID
>   * @adev: ACPI device to match.
>   * @hid2: Hardware ID of the device.
>   * @uid2: Unique ID of the device, pass NULL to not check _UID.
>   *
> - * Matches HID and UID in @adev with given @hid2 and @uid2.
> - * Returns true if matches.
> + * Matches HID and UID in @adev with given @hid2 and @uid2. Absence of @uid2
> + * will be treated as a match. If user wants to validate @uid2, it should be
> + * done before calling this function. This behaviour is as needed by most of
> + * its current users.

Ditto.

> + * Returns:
> + *  - %true if matches or @uid2 is NULL.
> + *  - %false otherwise.
>   */
  
Jonathan Cameron Oct. 20, 2023, 3:49 p.m. UTC | #2
On Fri, 20 Oct 2023 14:17:25 +0530
Raag Jadav <raag.jadav@intel.com> wrote:

> Introduce acpi_dev_uid_match() helper that matches the device with
> supplied _UID string.
> 
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>  /**
>   * acpi_dev_hid_uid_match - Match device by supplied HID and UID
>   * @adev: ACPI device to match.
>   * @hid2: Hardware ID of the device.
>   * @uid2: Unique ID of the device, pass NULL to not check _UID.
>   *
> - * Matches HID and UID in @adev with given @hid2 and @uid2.
> - * Returns true if matches.
> + * Matches HID and UID in @adev with given @hid2 and @uid2. Absence of @uid2
> + * will be treated as a match. If user wants to validate @uid2, it should be
> + * done before calling this function. This behaviour is as needed by most of
> + * its current users.

If there are other other users that need different behavior are they
buggy?  Also what behavior is this referring to?

I'd just drop the at last sentence as confusing and not adding much.

> + *
> + * Returns:
> + *  - %true if matches or @uid2 is NULL.
> + *  - %false otherwise.
>   */
>  bool acpi_dev_hid_uid_match(struct acpi_device *adev,
>  			    const char *hid2, const char *uid2)
>  {
>  	const char *hid1 = acpi_device_hid(adev);
> -	const char *uid1 = acpi_device_uid(adev);
>  
>  	if (strcmp(hid1, hid2))
>  		return false;
>  
> -	if (!uid2)
> -		return true;
> -
> -	return uid1 && !strcmp(uid1, uid2);
> +	return acpi_dev_uid_match(adev, uid2);
>  }
>  EXPORT_SYMBOL(acpi_dev_hid_uid_match);
>  
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 254685085c82..d1fe6446ffe0 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -760,6 +760,7 @@ static inline bool acpi_device_can_poweroff(struct acpi_device *adev)
>  		adev->power.states[ACPI_STATE_D3_HOT].flags.explicit_set);
>  }
>  
> +bool acpi_dev_uid_match(struct acpi_device *adev, const char *uid2);
>  bool acpi_dev_hid_uid_match(struct acpi_device *adev, const char *hid2, const char *uid2);
>  int acpi_dev_uid_to_integer(struct acpi_device *adev, u64 *integer);
>  
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index afd94c9b8b8a..db3a33e19c97 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -787,6 +787,11 @@ static inline bool acpi_dev_present(const char *hid, const char *uid, s64 hrv)
>  
>  struct acpi_device;
>  
> +static inline bool acpi_dev_uid_match(struct acpi_device *adev, const char *uid2)
> +{
> +	return false;
> +}
> +
>  static inline bool
>  acpi_dev_hid_uid_match(struct acpi_device *adev, const char *hid2, const char *uid2)
>  {
  

Patch

diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index 2ea14648a661..664b9890b625 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -768,28 +768,54 @@  bool acpi_check_dsm(acpi_handle handle, const guid_t *guid, u64 rev, u64 funcs)
 }
 EXPORT_SYMBOL(acpi_check_dsm);
 
+/**
+ * acpi_dev_uid_match - Match device by supplied UID
+ * @adev: ACPI device to match.
+ * @uid2: Unique ID of the device, pass NULL to not check _UID.
+ *
+ * Matches UID in @adev with given @uid2. Absence of @uid2 will be treated as
+ * a match. If user wants to validate @uid2, it should be done before calling
+ * this function. This behaviour is as needed by most of its current users.
+ *
+ * Returns:
+ *  - %true if matches or @uid2 is NULL.
+ *  - %false otherwise.
+ */
+bool acpi_dev_uid_match(struct acpi_device *adev, const char *uid2)
+{
+	const char *uid1 = acpi_device_uid(adev);
+
+	if (!uid2)
+		return true;
+
+	return uid1 && !strcmp(uid1, uid2);
+}
+EXPORT_SYMBOL_GPL(acpi_dev_uid_match);
+
 /**
  * acpi_dev_hid_uid_match - Match device by supplied HID and UID
  * @adev: ACPI device to match.
  * @hid2: Hardware ID of the device.
  * @uid2: Unique ID of the device, pass NULL to not check _UID.
  *
- * Matches HID and UID in @adev with given @hid2 and @uid2.
- * Returns true if matches.
+ * Matches HID and UID in @adev with given @hid2 and @uid2. Absence of @uid2
+ * will be treated as a match. If user wants to validate @uid2, it should be
+ * done before calling this function. This behaviour is as needed by most of
+ * its current users.
+ *
+ * Returns:
+ *  - %true if matches or @uid2 is NULL.
+ *  - %false otherwise.
  */
 bool acpi_dev_hid_uid_match(struct acpi_device *adev,
 			    const char *hid2, const char *uid2)
 {
 	const char *hid1 = acpi_device_hid(adev);
-	const char *uid1 = acpi_device_uid(adev);
 
 	if (strcmp(hid1, hid2))
 		return false;
 
-	if (!uid2)
-		return true;
-
-	return uid1 && !strcmp(uid1, uid2);
+	return acpi_dev_uid_match(adev, uid2);
 }
 EXPORT_SYMBOL(acpi_dev_hid_uid_match);
 
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 254685085c82..d1fe6446ffe0 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -760,6 +760,7 @@  static inline bool acpi_device_can_poweroff(struct acpi_device *adev)
 		adev->power.states[ACPI_STATE_D3_HOT].flags.explicit_set);
 }
 
+bool acpi_dev_uid_match(struct acpi_device *adev, const char *uid2);
 bool acpi_dev_hid_uid_match(struct acpi_device *adev, const char *hid2, const char *uid2);
 int acpi_dev_uid_to_integer(struct acpi_device *adev, u64 *integer);
 
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index afd94c9b8b8a..db3a33e19c97 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -787,6 +787,11 @@  static inline bool acpi_dev_present(const char *hid, const char *uid, s64 hrv)
 
 struct acpi_device;
 
+static inline bool acpi_dev_uid_match(struct acpi_device *adev, const char *uid2)
+{
+	return false;
+}
+
 static inline bool
 acpi_dev_hid_uid_match(struct acpi_device *adev, const char *hid2, const char *uid2)
 {