Re: [PATCH v2 1/5] ACPI: scan: Add acpi_dev_get_next_consumer_dev()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello

On 05/04/2022 13:57, Rafael J. Wysocki wrote:
> 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;


This and the below are fine for me; I'll send a v3

>
>> +
>> +       /*
>> +        * 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
>>



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux