| Message ID | 20210118003428.568892-2-djrscally@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce intel_skl_int3472 driver | expand |
Hi Daniel, Thank you for the patch. On Mon, Jan 18, 2021 at 12:34:22AM +0000, Daniel Scally wrote: > I need to be able to identify devices which declare themselves to be > dependent on other devices through _DEP; add this function to utils.c > and export it to the rest of the ACPI layer. > > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Daniel Scally <djrscally@gmail.com> > --- > Changes in v2: > - Introduced > > drivers/acpi/acpi_lpss.c | 24 ------------------------ > drivers/acpi/internal.h | 1 + > drivers/acpi/utils.c | 24 ++++++++++++++++++++++++ > 3 files changed, 25 insertions(+), 24 deletions(-) > > diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c > index be73974ce449..70c7d9a3f715 100644 > --- a/drivers/acpi/acpi_lpss.c > +++ b/drivers/acpi/acpi_lpss.c > @@ -543,30 +543,6 @@ static struct device *acpi_lpss_find_device(const char *hid, const char *uid) > return bus_find_device(&pci_bus_type, NULL, &data, match_hid_uid); > } > > -static bool acpi_lpss_dep(struct acpi_device *adev, acpi_handle handle) > -{ > - struct acpi_handle_list dep_devices; > - acpi_status status; > - int i; > - > - if (!acpi_has_method(adev->handle, "_DEP")) > - return false; > - > - status = acpi_evaluate_reference(adev->handle, "_DEP", NULL, > - &dep_devices); > - if (ACPI_FAILURE(status)) { > - dev_dbg(&adev->dev, "Failed to evaluate _DEP.\n"); > - return false; > - } > - > - for (i = 0; i < dep_devices.count; i++) { > - if (dep_devices.handles[i] == handle) > - return true; > - } > - > - return false; > -} > - > static void acpi_lpss_link_consumer(struct device *dev1, > const struct lpss_device_links *link) > { > diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h > index cb229e24c563..ee62c0973576 100644 > --- a/drivers/acpi/internal.h > +++ b/drivers/acpi/internal.h > @@ -79,6 +79,7 @@ static inline void acpi_lpss_init(void) {} > #endif > > void acpi_apd_init(void); > +bool acpi_lpss_dep(struct acpi_device *adev, acpi_handle handle); "lpss" stands for low power subsystem, an Intel device within the PCH that handles I2C, SPI, UART, ... I think the function should be renamed, as it's now generic. acpi_dev_has_dep() is a potential candidate, I'm sure better ones exist. A bit of kerneldoc would also not hurt. > > acpi_status acpi_hotplug_schedule(struct acpi_device *adev, u32 src); > bool acpi_queue_hotplug_work(struct work_struct *work); > diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c > index ddca1550cce6..78b38775f18b 100644 > --- a/drivers/acpi/utils.c > +++ b/drivers/acpi/utils.c > @@ -807,6 +807,30 @@ static int acpi_dev_match_cb(struct device *dev, const void *data) > return hrv == match->hrv; > } > > +bool acpi_lpss_dep(struct acpi_device *adev, acpi_handle handle) > +{ > + struct acpi_handle_list dep_devices; > + acpi_status status; > + int i; > + > + if (!acpi_has_method(adev->handle, "_DEP")) > + return false; > + > + status = acpi_evaluate_reference(adev->handle, "_DEP", NULL, > + &dep_devices); > + if (ACPI_FAILURE(status)) { > + dev_dbg(&adev->dev, "Failed to evaluate _DEP.\n"); > + return false; > + } > + > + for (i = 0; i < dep_devices.count; i++) { > + if (dep_devices.handles[i] == handle) > + return true; > + } > + > + return false; > +} > + > /** > * acpi_dev_present - Detect that a given ACPI device is present > * @hid: Hardware ID of the device.
On Mon, Jan 18, 2021 at 12:34:22AM +0000, Daniel Scally wrote: > I need to be able to identify devices which declare themselves to be > dependent on other devices through _DEP; add this function to utils.c > and export it to the rest of the ACPI layer. Prefix -> "ACPI / utils: " Otherwise good to me Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Daniel Scally <djrscally@gmail.com> > --- > Changes in v2: > - Introduced > > drivers/acpi/acpi_lpss.c | 24 ------------------------ > drivers/acpi/internal.h | 1 + > drivers/acpi/utils.c | 24 ++++++++++++++++++++++++ > 3 files changed, 25 insertions(+), 24 deletions(-) > > diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c > index be73974ce449..70c7d9a3f715 100644 > --- a/drivers/acpi/acpi_lpss.c > +++ b/drivers/acpi/acpi_lpss.c > @@ -543,30 +543,6 @@ static struct device *acpi_lpss_find_device(const char *hid, const char *uid) > return bus_find_device(&pci_bus_type, NULL, &data, match_hid_uid); > } > > -static bool acpi_lpss_dep(struct acpi_device *adev, acpi_handle handle) > -{ > - struct acpi_handle_list dep_devices; > - acpi_status status; > - int i; > - > - if (!acpi_has_method(adev->handle, "_DEP")) > - return false; > - > - status = acpi_evaluate_reference(adev->handle, "_DEP", NULL, > - &dep_devices); > - if (ACPI_FAILURE(status)) { > - dev_dbg(&adev->dev, "Failed to evaluate _DEP.\n"); > - return false; > - } > - > - for (i = 0; i < dep_devices.count; i++) { > - if (dep_devices.handles[i] == handle) > - return true; > - } > - > - return false; > -} > - > static void acpi_lpss_link_consumer(struct device *dev1, > const struct lpss_device_links *link) > { > diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h > index cb229e24c563..ee62c0973576 100644 > --- a/drivers/acpi/internal.h > +++ b/drivers/acpi/internal.h > @@ -79,6 +79,7 @@ static inline void acpi_lpss_init(void) {} > #endif > > void acpi_apd_init(void); > +bool acpi_lpss_dep(struct acpi_device *adev, acpi_handle handle); > > acpi_status acpi_hotplug_schedule(struct acpi_device *adev, u32 src); > bool acpi_queue_hotplug_work(struct work_struct *work); > diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c > index ddca1550cce6..78b38775f18b 100644 > --- a/drivers/acpi/utils.c > +++ b/drivers/acpi/utils.c > @@ -807,6 +807,30 @@ static int acpi_dev_match_cb(struct device *dev, const void *data) > return hrv == match->hrv; > } > > +bool acpi_lpss_dep(struct acpi_device *adev, acpi_handle handle) > +{ > + struct acpi_handle_list dep_devices; > + acpi_status status; > + int i; > + > + if (!acpi_has_method(adev->handle, "_DEP")) > + return false; > + > + status = acpi_evaluate_reference(adev->handle, "_DEP", NULL, > + &dep_devices); > + if (ACPI_FAILURE(status)) { > + dev_dbg(&adev->dev, "Failed to evaluate _DEP.\n"); > + return false; > + } > + > + for (i = 0; i < dep_devices.count; i++) { > + if (dep_devices.handles[i] == handle) > + return true; > + } > + > + return false; > +} > + > /** > * acpi_dev_present - Detect that a given ACPI device is present > * @hid: Hardware ID of the device. > -- > 2.25.1 >
On 18/01/2021 12:29, Andy Shevchenko wrote: > On Mon, Jan 18, 2021 at 09:24:10AM +0200, Laurent Pinchart wrote: >> On Mon, Jan 18, 2021 at 12:34:22AM +0000, Daniel Scally wrote: > ... > >>> +bool acpi_lpss_dep(struct acpi_device *adev, acpi_handle handle); >> "lpss" stands for low power subsystem, an Intel device within the PCH >> that handles I2C, SPI, UART, ... I think the function should be renamed, >> as it's now generic. acpi_dev_has_dep() is a potential candidate, I'm >> sure better ones exist. A bit of kerneldoc would also not hurt. > Actually a good suggestions. Please apply my tag after addressing above. Will do, thanks
On Mon, Jan 18, 2021 at 1:30 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Mon, Jan 18, 2021 at 12:34:22AM +0000, Daniel Scally wrote: > > I need to be able to identify devices which declare themselves to be > > dependent on other devices through _DEP; add this function to utils.c > > and export it to the rest of the ACPI layer. > > Prefix -> "ACPI / utils: " Preferably "ACPI: utils: " for that matter and yes, please rename the function while moving it. > Otherwise good to me > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Signed-off-by: Daniel Scally <djrscally@gmail.com> > > --- > > Changes in v2: > > - Introduced > > > > drivers/acpi/acpi_lpss.c | 24 ------------------------ > > drivers/acpi/internal.h | 1 + > > drivers/acpi/utils.c | 24 ++++++++++++++++++++++++ > > 3 files changed, 25 insertions(+), 24 deletions(-) > > > > diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c > > index be73974ce449..70c7d9a3f715 100644 > > --- a/drivers/acpi/acpi_lpss.c > > +++ b/drivers/acpi/acpi_lpss.c > > @@ -543,30 +543,6 @@ static struct device *acpi_lpss_find_device(const char *hid, const char *uid) > > return bus_find_device(&pci_bus_type, NULL, &data, match_hid_uid); > > } > > > > -static bool acpi_lpss_dep(struct acpi_device *adev, acpi_handle handle) > > -{ > > - struct acpi_handle_list dep_devices; > > - acpi_status status; > > - int i; > > - > > - if (!acpi_has_method(adev->handle, "_DEP")) > > - return false; > > - > > - status = acpi_evaluate_reference(adev->handle, "_DEP", NULL, > > - &dep_devices); > > - if (ACPI_FAILURE(status)) { > > - dev_dbg(&adev->dev, "Failed to evaluate _DEP.\n"); > > - return false; > > - } > > - > > - for (i = 0; i < dep_devices.count; i++) { > > - if (dep_devices.handles[i] == handle) > > - return true; > > - } > > - > > - return false; > > -} > > - > > static void acpi_lpss_link_consumer(struct device *dev1, > > const struct lpss_device_links *link) > > { > > diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h > > index cb229e24c563..ee62c0973576 100644 > > --- a/drivers/acpi/internal.h > > +++ b/drivers/acpi/internal.h > > @@ -79,6 +79,7 @@ static inline void acpi_lpss_init(void) {} > > #endif > > > > void acpi_apd_init(void); > > +bool acpi_lpss_dep(struct acpi_device *adev, acpi_handle handle); > > > > acpi_status acpi_hotplug_schedule(struct acpi_device *adev, u32 src); > > bool acpi_queue_hotplug_work(struct work_struct *work); > > diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c > > index ddca1550cce6..78b38775f18b 100644 > > --- a/drivers/acpi/utils.c > > +++ b/drivers/acpi/utils.c > > @@ -807,6 +807,30 @@ static int acpi_dev_match_cb(struct device *dev, const void *data) > > return hrv == match->hrv; > > } > > > > +bool acpi_lpss_dep(struct acpi_device *adev, acpi_handle handle) > > +{ > > + struct acpi_handle_list dep_devices; > > + acpi_status status; > > + int i; > > + > > + if (!acpi_has_method(adev->handle, "_DEP")) > > + return false; > > + > > + status = acpi_evaluate_reference(adev->handle, "_DEP", NULL, > > + &dep_devices); > > + if (ACPI_FAILURE(status)) { > > + dev_dbg(&adev->dev, "Failed to evaluate _DEP.\n"); > > + return false; > > + } > > + > > + for (i = 0; i < dep_devices.count; i++) { > > + if (dep_devices.handles[i] == handle) > > + return true; > > + } > > + > > + return false; > > +} > > + > > /** > > * acpi_dev_present - Detect that a given ACPI device is present > > * @hid: Hardware ID of the device. > > -- > > 2.25.1 > > > > -- > With Best Regards, > Andy Shevchenko > >
diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c index be73974ce449..70c7d9a3f715 100644 --- a/drivers/acpi/acpi_lpss.c +++ b/drivers/acpi/acpi_lpss.c @@ -543,30 +543,6 @@ static struct device *acpi_lpss_find_device(const char *hid, const char *uid) return bus_find_device(&pci_bus_type, NULL, &data, match_hid_uid); } -static bool acpi_lpss_dep(struct acpi_device *adev, acpi_handle handle) -{ - struct acpi_handle_list dep_devices; - acpi_status status; - int i; - - if (!acpi_has_method(adev->handle, "_DEP")) - return false; - - status = acpi_evaluate_reference(adev->handle, "_DEP", NULL, - &dep_devices); - if (ACPI_FAILURE(status)) { - dev_dbg(&adev->dev, "Failed to evaluate _DEP.\n"); - return false; - } - - for (i = 0; i < dep_devices.count; i++) { - if (dep_devices.handles[i] == handle) - return true; - } - - return false; -} - static void acpi_lpss_link_consumer(struct device *dev1, const struct lpss_device_links *link) { diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index cb229e24c563..ee62c0973576 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -79,6 +79,7 @@ static inline void acpi_lpss_init(void) {} #endif void acpi_apd_init(void); +bool acpi_lpss_dep(struct acpi_device *adev, acpi_handle handle); acpi_status acpi_hotplug_schedule(struct acpi_device *adev, u32 src); bool acpi_queue_hotplug_work(struct work_struct *work); diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c index ddca1550cce6..78b38775f18b 100644 --- a/drivers/acpi/utils.c +++ b/drivers/acpi/utils.c @@ -807,6 +807,30 @@ static int acpi_dev_match_cb(struct device *dev, const void *data) return hrv == match->hrv; } +bool acpi_lpss_dep(struct acpi_device *adev, acpi_handle handle) +{ + struct acpi_handle_list dep_devices; + acpi_status status; + int i; + + if (!acpi_has_method(adev->handle, "_DEP")) + return false; + + status = acpi_evaluate_reference(adev->handle, "_DEP", NULL, + &dep_devices); + if (ACPI_FAILURE(status)) { + dev_dbg(&adev->dev, "Failed to evaluate _DEP.\n"); + return false; + } + + for (i = 0; i < dep_devices.count; i++) { + if (dep_devices.handles[i] == handle) + return true; + } + + return false; +} + /** * acpi_dev_present - Detect that a given ACPI device is present * @hid: Hardware ID of the device.
I need to be able to identify devices which declare themselves to be dependent on other devices through _DEP; add this function to utils.c and export it to the rest of the ACPI layer. Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Signed-off-by: Daniel Scally <djrscally@gmail.com> --- Changes in v2: - Introduced drivers/acpi/acpi_lpss.c | 24 ------------------------ drivers/acpi/internal.h | 1 + drivers/acpi/utils.c | 24 ++++++++++++++++++++++++ 3 files changed, 25 insertions(+), 24 deletions(-)