2023-11-23 10:08:05

by Raag Jadav

[permalink] [raw]
Subject: [PATCH v3 0/5] Support _UID matching for integer types

This series updates the standard ACPI helpers to support _UID matching
for both integer and string types, and uses them in a couple of places.

Changes since v2:
- Drop __builtin functions to reduce complexity (Rafael)
- Update tags

Changes since v1:
- Fix build errors

Raag Jadav (5):
ACPI: bus: update acpi_dev_uid_match() to support multiple types
ACPI: bus: update acpi_dev_hid_uid_match() to support multiple types
ACPI: LPSS: use acpi_dev_uid_match() for matching _UID
efi: dev-path-parser: use acpi_dev_uid_match() for matching _UID
perf: arm_cspmu: drop redundant acpi_dev_uid_to_integer()

drivers/acpi/acpi_lpss.c | 16 ++-----
drivers/acpi/utils.c | 48 -------------------
drivers/firmware/efi/dev-path-parser.c | 7 +--
drivers/perf/arm_cspmu/arm_cspmu.c | 4 +-
include/acpi/acpi_bus.h | 65 +++++++++++++++++++++++++-
include/linux/acpi.h | 15 ++----
6 files changed, 74 insertions(+), 81 deletions(-)


base-commit: accf6bb711d6819cce89a122ccde4811c58c160e
--
2.17.1


2023-11-23 10:08:19

by Raag Jadav

[permalink] [raw]
Subject: [PATCH v3 2/5] ACPI: bus: update acpi_dev_hid_uid_match() to support multiple types

Now that we have _UID matching support for both integer and string types,
we can support them into acpi_dev_hid_uid_match() helper as well.

Signed-off-by: Raag Jadav <[email protected]>
Reviewed-by: Mika Westerberg <[email protected]>
---
drivers/acpi/utils.c | 29 -----------------------------
include/acpi/acpi_bus.h | 24 +++++++++++++++++++++++-
include/linux/acpi.h | 7 +------
3 files changed, 24 insertions(+), 36 deletions(-)

diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index fe7e850c6479..03f6de9a0807 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -824,35 +824,6 @@ bool acpi_check_dsm(acpi_handle handle, const guid_t *guid, u64 rev, u64 funcs)
}
EXPORT_SYMBOL(acpi_check_dsm);

-/**
- * 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. Absence of @uid2
- * will be treated as a match. If user wants to validate @uid2, it should be
- * done before calling this function.
- *
- * 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);
-
- if (strcmp(hid1, hid2))
- return false;
-
- if (!uid2)
- return true;
-
- return acpi_dev_uid_match(adev, uid2);
-}
-EXPORT_SYMBOL(acpi_dev_hid_uid_match);
-
/**
* acpi_dev_uid_to_integer - treat ACPI device _UID as integer
* @adev: ACPI device to get _UID from
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index a4622d95a36d..e5b4c3f8d2be 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -857,9 +857,15 @@ static inline bool acpi_device_can_poweroff(struct acpi_device *adev)
adev->power.states[ACPI_STATE_D3_HOT].flags.explicit_set);
}

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

+static inline bool acpi_dev_hid_match(struct acpi_device *adev, const char *hid2)
+{
+ const char *hid1 = acpi_device_hid(adev);
+
+ return hid1 && hid2 && !strcmp(hid1, hid2);
+}
+
static inline bool acpi_str_uid_match(struct acpi_device *adev, const char *uid2)
{
const char *uid1 = acpi_device_uid(adev);
@@ -900,6 +906,22 @@ static inline bool acpi_int_uid_match(struct acpi_device *adev, u64 uid2)
/* Treat as an integer otherwise */ \
default: acpi_int_uid_match)(adev, uid2)

+/**
+ * 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 0 or NULL to not check _UID.
+ *
+ * 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.
+ *
+ * Returns: %true if matches or @uid2 is 0 or NULL, %false otherwise.
+ */
+#define acpi_dev_hid_uid_match(adev, hid2, uid2) \
+ (acpi_dev_hid_match(adev, hid2) && \
+ (!(uid2) || acpi_dev_uid_match(adev, uid2)))
+
void acpi_dev_clear_dependencies(struct acpi_device *supplier);
bool acpi_dev_ready_for_enumeration(const struct acpi_device *device);
struct acpi_device *acpi_dev_get_next_consumer_dev(struct acpi_device *supplier,
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 6f1e9f98fa44..118a18b7ff84 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -765,6 +765,7 @@ const char *acpi_get_subsystem_id(acpi_handle handle);

/* Get rid of the -Wunused-variable for adev */
#define acpi_dev_uid_match(adev, uid2) (adev && false)
+#define acpi_dev_hid_uid_match(adev, hid2, uid2) (adev && false)

#include <acpi/acpi_numa.h>

@@ -782,12 +783,6 @@ static inline bool acpi_dev_present(const char *hid, const char *uid, s64 hrv)

struct acpi_device;

-static inline bool
-acpi_dev_hid_uid_match(struct acpi_device *adev, const char *hid2, const char *uid2)
-{
- return false;
-}
-
static inline int acpi_dev_uid_to_integer(struct acpi_device *adev, u64 *integer)
{
return -ENODEV;
--
2.17.1

2023-11-23 10:08:30

by Raag Jadav

[permalink] [raw]
Subject: [PATCH v3 3/5] ACPI: LPSS: use acpi_dev_uid_match() for matching _UID

Now that we have _UID matching support for integer types, we can use
acpi_dev_uid_match() for it.

Signed-off-by: Raag Jadav <[email protected]>
Acked-by: Mika Westerberg <[email protected]>
---
drivers/acpi/acpi_lpss.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
index 875de44961bf..79f4fc7d6871 100644
--- a/drivers/acpi/acpi_lpss.c
+++ b/drivers/acpi/acpi_lpss.c
@@ -167,13 +167,9 @@ static struct pwm_lookup byt_pwm_lookup[] = {

static void byt_pwm_setup(struct lpss_private_data *pdata)
{
- u64 uid;
-
/* Only call pwm_add_table for the first PWM controller */
- if (acpi_dev_uid_to_integer(pdata->adev, &uid) || uid != 1)
- return;
-
- pwm_add_table(byt_pwm_lookup, ARRAY_SIZE(byt_pwm_lookup));
+ if (acpi_dev_uid_match(pdata->adev, 1))
+ pwm_add_table(byt_pwm_lookup, ARRAY_SIZE(byt_pwm_lookup));
}

#define LPSS_I2C_ENABLE 0x6c
@@ -218,13 +214,9 @@ static struct pwm_lookup bsw_pwm_lookup[] = {

static void bsw_pwm_setup(struct lpss_private_data *pdata)
{
- u64 uid;
-
/* Only call pwm_add_table for the first PWM controller */
- if (acpi_dev_uid_to_integer(pdata->adev, &uid) || uid != 1)
- return;
-
- pwm_add_table(bsw_pwm_lookup, ARRAY_SIZE(bsw_pwm_lookup));
+ if (acpi_dev_uid_match(pdata->adev, 1))
+ pwm_add_table(bsw_pwm_lookup, ARRAY_SIZE(bsw_pwm_lookup));
}

static const struct property_entry lpt_spi_properties[] = {
--
2.17.1

2023-11-23 10:08:35

by Raag Jadav

[permalink] [raw]
Subject: [PATCH v3 4/5] efi: dev-path-parser: use acpi_dev_uid_match() for matching _UID

Now that we have _UID matching support for integer types, we can use
acpi_dev_uid_match() for it.

Signed-off-by: Raag Jadav <[email protected]>
Reviewed-by: Ard Biesheuvel <[email protected]>
---
drivers/firmware/efi/dev-path-parser.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/firmware/efi/dev-path-parser.c b/drivers/firmware/efi/dev-path-parser.c
index f80d87c199c3..937be269fee8 100644
--- a/drivers/firmware/efi/dev-path-parser.c
+++ b/drivers/firmware/efi/dev-path-parser.c
@@ -18,8 +18,6 @@ static long __init parse_acpi_path(const struct efi_dev_path *node,
struct acpi_device *adev;
struct device *phys_dev;
char hid[ACPI_ID_LEN];
- u64 uid;
- int ret;

if (node->header.length != 12)
return -EINVAL;
@@ -31,10 +29,9 @@ static long __init parse_acpi_path(const struct efi_dev_path *node,
node->acpi.hid >> 16);

for_each_acpi_dev_match(adev, hid, NULL, -1) {
- ret = acpi_dev_uid_to_integer(adev, &uid);
- if (ret == 0 && node->acpi.uid == uid)
+ if (acpi_dev_uid_match(adev, node->acpi.uid))
break;
- if (ret == -ENODATA && node->acpi.uid == 0)
+ if (!acpi_device_uid(adev) && node->acpi.uid == 0)
break;
}
if (!adev)
--
2.17.1

2023-11-23 10:09:16

by Raag Jadav

[permalink] [raw]
Subject: [PATCH v3 5/5] perf: arm_cspmu: drop redundant acpi_dev_uid_to_integer()

Now that we have _UID matching support for integer types, we can use
acpi_dev_hid_uid_match() for it.

Signed-off-by: Raag Jadav <[email protected]>
Acked-by: Will Deacon <[email protected]>
---
drivers/perf/arm_cspmu/arm_cspmu.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index 2cc35dded007..50b89b989ce7 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -1108,7 +1108,6 @@ static int arm_cspmu_request_irq(struct arm_cspmu *cspmu)

static inline int arm_cspmu_find_cpu_container(int cpu, u32 container_uid)
{
- u64 acpi_uid;
struct device *cpu_dev;
struct acpi_device *acpi_dev;

@@ -1118,8 +1117,7 @@ static inline int arm_cspmu_find_cpu_container(int cpu, u32 container_uid)

acpi_dev = ACPI_COMPANION(cpu_dev);
while (acpi_dev) {
- if (acpi_dev_hid_uid_match(acpi_dev, ACPI_PROCESSOR_CONTAINER_HID, NULL) &&
- !acpi_dev_uid_to_integer(acpi_dev, &acpi_uid) && acpi_uid == container_uid)
+ if (acpi_dev_hid_uid_match(acpi_dev, ACPI_PROCESSOR_CONTAINER_HID, container_uid))
return 0;

acpi_dev = acpi_dev_parent(acpi_dev);
--
2.17.1

2023-11-23 10:10:52

by Raag Jadav

[permalink] [raw]
Subject: [PATCH v3 1/5] ACPI: bus: update acpi_dev_uid_match() to support multiple types

According to ACPI specification, a _UID object can evaluate to either
a numeric value or a string. Update acpi_dev_uid_match() helper to
support _UID matching for both integer and string types.

Suggested-by: Mika Westerberg <[email protected]>
Signed-off-by: Raag Jadav <[email protected]>
---
drivers/acpi/utils.c | 19 -------------------
include/acpi/acpi_bus.h | 41 ++++++++++++++++++++++++++++++++++++++++-
include/linux/acpi.h | 8 +++-----
3 files changed, 43 insertions(+), 25 deletions(-)

diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index 28c75242fca9..fe7e850c6479 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -824,25 +824,6 @@ 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.
- *
- * Matches UID in @adev with given @uid2.
- *
- * Returns:
- * - %true if matches.
- * - %false otherwise.
- */
-bool acpi_dev_uid_match(struct acpi_device *adev, const char *uid2)
-{
- const char *uid1 = acpi_device_uid(adev);
-
- return uid1 && uid2 && !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.
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index ec6a673dcb95..a4622d95a36d 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -857,10 +857,49 @@ 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);

+static inline bool acpi_str_uid_match(struct acpi_device *adev, const char *uid2)
+{
+ const char *uid1 = acpi_device_uid(adev);
+
+ return uid1 && uid2 && !strcmp(uid1, uid2);
+}
+
+static inline bool acpi_int_uid_match(struct acpi_device *adev, u64 uid2)
+{
+ u64 uid1;
+
+ return !acpi_dev_uid_to_integer(adev, &uid1) && uid1 == uid2;
+}
+
+/**
+ * acpi_dev_uid_match - Match device by supplied UID
+ * @adev: ACPI device to match.
+ * @uid2: Unique ID of the device.
+ *
+ * Matches UID in @adev with given @uid2.
+ *
+ * Returns: %true if matches, %false otherwise.
+ */
+#define CONSTIFY(type, x) \
+ const type: x, \
+ type: x
+
+#define ACPI_STR_TYPE(match) \
+ CONSTIFY(unsigned char *, match), \
+ CONSTIFY(signed char *, match), \
+ CONSTIFY(char *, match), \
+ CONSTIFY(void *, match)
+
+#define acpi_dev_uid_match(adev, uid2) \
+ _Generic(uid2, \
+ /* Treat @uid2 as a string for acpi string types */ \
+ ACPI_STR_TYPE(acpi_str_uid_match), \
+ /* Treat as an integer otherwise */ \
+ default: acpi_int_uid_match)(adev, uid2)
+
void acpi_dev_clear_dependencies(struct acpi_device *supplier);
bool acpi_dev_ready_for_enumeration(const struct acpi_device *device);
struct acpi_device *acpi_dev_get_next_consumer_dev(struct acpi_device *supplier,
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index b556f716d115..6f1e9f98fa44 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -763,6 +763,9 @@ const char *acpi_get_subsystem_id(acpi_handle handle);
#define ACPI_HANDLE(dev) (NULL)
#define ACPI_HANDLE_FWNODE(fwnode) (NULL)

+/* Get rid of the -Wunused-variable for adev */
+#define acpi_dev_uid_match(adev, uid2) (adev && false)
+
#include <acpi/acpi_numa.h>

struct fwnode_handle;
@@ -779,11 +782,6 @@ 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)
{
--
2.17.1

2023-12-05 04:16:31

by Raag Jadav

[permalink] [raw]
Subject: Re: [PATCH v3 0/5] Support _UID matching for integer types

On Thu, Nov 23, 2023 at 03:36:12PM +0530, Raag Jadav wrote:
> This series updates the standard ACPI helpers to support _UID matching
> for both integer and string types, and uses them in a couple of places.
>
> Changes since v2:
> - Drop __builtin functions to reduce complexity (Rafael)
> - Update tags
>
> Changes since v1:
> - Fix build errors

Bump.

Anything I can do to move this forward?

Raag

2023-12-05 11:59:10

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v3 0/5] Support _UID matching for integer types

On Tue, Dec 5, 2023 at 5:11 AM Raag Jadav <[email protected]> wrote:
>
> On Thu, Nov 23, 2023 at 03:36:12PM +0530, Raag Jadav wrote:
> > This series updates the standard ACPI helpers to support _UID matching
> > for both integer and string types, and uses them in a couple of places.
> >
> > Changes since v2:
> > - Drop __builtin functions to reduce complexity (Rafael)
> > - Update tags
> >
> > Changes since v1:
> > - Fix build errors
>
> Bump.
>
> Anything I can do to move this forward?

It is on my list of pending material, I'll get to it this week (I think).

Thanks!

2023-12-06 19:44:42

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v3 0/5] Support _UID matching for integer types

On Thu, Nov 23, 2023 at 11:07 AM Raag Jadav <[email protected]> wrote:
>
> This series updates the standard ACPI helpers to support _UID matching
> for both integer and string types, and uses them in a couple of places.
>
> Changes since v2:
> - Drop __builtin functions to reduce complexity (Rafael)
> - Update tags
>
> Changes since v1:
> - Fix build errors
>
> Raag Jadav (5):
> ACPI: bus: update acpi_dev_uid_match() to support multiple types
> ACPI: bus: update acpi_dev_hid_uid_match() to support multiple types
> ACPI: LPSS: use acpi_dev_uid_match() for matching _UID
> efi: dev-path-parser: use acpi_dev_uid_match() for matching _UID
> perf: arm_cspmu: drop redundant acpi_dev_uid_to_integer()

All applied as 6.8 material, but I renamed the auxiliary macros in the
first patch and moved the kerneldoc comment below them.

TBH, I'm not entirely sure about the value of the second of those
auxiliary macros, but then I don't particularly dislike it either.

2023-12-09 08:48:38

by Raag Jadav

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] perf: arm_cspmu: drop redundant acpi_dev_uid_to_integer()

On Thu, Nov 23, 2023 at 03:36:17PM +0530, Raag Jadav wrote:
> Now that we have _UID matching support for integer types, we can use
> acpi_dev_hid_uid_match() for it.
>
> Signed-off-by: Raag Jadav <[email protected]>
> Acked-by: Will Deacon <[email protected]>
> ---
> drivers/perf/arm_cspmu/arm_cspmu.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
> index 2cc35dded007..50b89b989ce7 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
> @@ -1108,7 +1108,6 @@ static int arm_cspmu_request_irq(struct arm_cspmu *cspmu)
>
> static inline int arm_cspmu_find_cpu_container(int cpu, u32 container_uid)
> {
> - u64 acpi_uid;
> struct device *cpu_dev;
> struct acpi_device *acpi_dev;
>
> @@ -1118,8 +1117,7 @@ static inline int arm_cspmu_find_cpu_container(int cpu, u32 container_uid)
>
> acpi_dev = ACPI_COMPANION(cpu_dev);
> while (acpi_dev) {
> - if (acpi_dev_hid_uid_match(acpi_dev, ACPI_PROCESSOR_CONTAINER_HID, NULL) &&
> - !acpi_dev_uid_to_integer(acpi_dev, &acpi_uid) && acpi_uid == container_uid)
> + if (acpi_dev_hid_uid_match(acpi_dev, ACPI_PROCESSOR_CONTAINER_HID, container_uid))
> return 0;

On second thought, I just realized that this can cause false positive
in case container_uid is integer 0.

I'm not aware if that's a valid value to match against in APMT.
If it is, it's better to drop patches 2 and 5 for now.

Raag