On Sun, Mar 27, 2022 at 6:13 PM Daniel Scally <djrscally@xxxxxxxxx> wrote:
>
> In commit b83e2b306736 ("ACPI: scan: Add function to fetch dependent
> of ACPI device") we added a means of fetching the first device to
> declare itself dependent on another ACPI device in the _DEP method.
> One assumption in that patch was that there would only be a single
> consuming device, but this has not held.
>
> Replace that function with a new function that fetches the next consumer
> of a supplier device. Where no "previous" consumer is passed in, it
> behaves identically to the original function.
>
> Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> Signed-off-by: Daniel Scally <djrscally@xxxxxxxxx>
> ---
> Changes in v2:
>
> - Removed acpi_dev_get_first_consumer_dev() entirely
>
> drivers/acpi/scan.c | 37 +++++++++++++++------
> drivers/platform/x86/intel/int3472/common.c | 2 +-
> include/acpi/acpi_bus.h | 4 ++-
> 3 files changed, 30 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 010ef0b28374..8797e4a33674 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -2215,9 +2215,21 @@ static void acpi_bus_attach(struct acpi_device *device, bool first_pass)
> device->handler->hotplug.notify_online(device);
> }
>
> -static int acpi_dev_get_first_consumer_dev_cb(struct acpi_dep_data *dep, void *data)
> +static int acpi_dev_get_next_consumer_dev_cb(struct acpi_dep_data *dep, void *data)
> {
> - struct acpi_device *adev;
> + struct acpi_device *adev = *(struct acpi_device **)data;
I would prefer
struct acpi_device **adev_p = data;
struct acpi_device *adev = *adev_p;
> +
> + /*
> + * If we're passed a 'previous' consumer device then we need to skip
> + * any consumers until we meet the previous one, and then NULL @data
> + * so the next one can be returned.
> + */
> + if (adev) {
> + if (dep->consumer == adev->handle)
> + *(struct acpi_device **)data = NULL;
*adev_p = NULL;
> +
> + return 0;
> + }
>
> adev = acpi_bus_get_acpi_device(dep->consumer);
> if (adev) {
> @@ -2348,25 +2360,28 @@ bool acpi_dev_ready_for_enumeration(const struct acpi_device *device)
> EXPORT_SYMBOL_GPL(acpi_dev_ready_for_enumeration);
>
> /**
> - * acpi_dev_get_first_consumer_dev - Return ACPI device dependent on @supplier
> + * acpi_dev_get_next_consumer_dev - Return the next adev dependent on @supplier
> * @supplier: Pointer to the dependee device
> + * @start: Pointer to the current dependent device
> *
> - * Returns the first &struct acpi_device which declares itself dependent on
> + * Returns the next &struct acpi_device which declares itself dependent on
> * @supplier via the _DEP buffer, parsed from the acpi_dep_list.
> *
> - * The caller is responsible for putting the reference to adev when it is no
> - * longer needed.
> + * If the returned adev is not passed as @start to this function, the caller is
> + * responsible for putting the reference to adev when it is no longer needed.
> */
> -struct acpi_device *acpi_dev_get_first_consumer_dev(struct acpi_device *supplier)
> +struct acpi_device *acpi_dev_get_next_consumer_dev(struct acpi_device *supplier,
> + struct acpi_device *start)
> {
> - struct acpi_device *adev = NULL;
> + struct acpi_device *adev = start;
>
> acpi_walk_dep_device_list(supplier->handle,
> - acpi_dev_get_first_consumer_dev_cb, &adev);
> + acpi_dev_get_next_consumer_dev_cb, &adev);
>
> - return adev;
> + acpi_dev_put(start);
> + return adev == start ? NULL : adev;
And here
if (adev == start)
return NULL;
return adev;
> }
> -EXPORT_SYMBOL_GPL(acpi_dev_get_first_consumer_dev);
> +EXPORT_SYMBOL_GPL(acpi_dev_get_next_consumer_dev);
>
> /**
> * acpi_bus_scan - Add ACPI device node objects in a given namespace scope.
> diff --git a/drivers/platform/x86/intel/int3472/common.c b/drivers/platform/x86/intel/int3472/common.c
> index 77cf058e4168..9db2bb0bbba4 100644
> --- a/drivers/platform/x86/intel/int3472/common.c
> +++ b/drivers/platform/x86/intel/int3472/common.c
> @@ -62,7 +62,7 @@ int skl_int3472_get_sensor_adev_and_name(struct device *dev,
> struct acpi_device *sensor;
> int ret = 0;
>
> - sensor = acpi_dev_get_first_consumer_dev(adev);
> + sensor = acpi_dev_get_next_consumer_dev(adev, NULL);
> if (!sensor) {
> dev_err(dev, "INT3472 seems to have no dependents.\n");
> return -ENODEV;
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 2f93ecf05dac..cdc726d251b6 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -696,7 +696,9 @@ bool acpi_dev_hid_uid_match(struct acpi_device *adev, const char *hid2, const ch
>
> 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_first_consumer_dev(struct acpi_device *supplier);
> +struct acpi_device *acpi_dev_get_next_consumer_dev(struct acpi_device *supplier,
> + struct acpi_device *start);
> +
> struct acpi_device *
> acpi_dev_get_next_match_dev(struct acpi_device *adev, const char *hid, const char *uid, s64 hrv);
> struct acpi_device *
> --
> 2.25.1
>
