| Message ID | 2159220.NgBsaNRSFp@kreacher (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On 9/06/22 17:18, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Introduce acpi_device_fix_up_power_extended() for fixing up power of > a device having an ACPI companion in a manner that takes the device's > children into account and make the MMC code use it in two places > instead of walking the list of the device ACPI companion's children > directly. > > This will help to eliminate the children list head from struct > acpi_device as it is redundant and it is used in questionable ways > in some places (in particular, locking is needed for walking the > list pointed to it safely, but it is often missing). > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Acked-by: Adrian Hunter <adrian.hunter@intel.com> > --- > drivers/acpi/device_pm.c | 22 ++++++++++++++++++++++ > drivers/mmc/host/sdhci-acpi.c | 7 ++----- > drivers/mmc/host/sdhci-pci-core.c | 11 +++-------- > include/acpi/acpi_bus.h | 1 + > 4 files changed, 28 insertions(+), 13 deletions(-) > > Index: linux-pm/drivers/mmc/host/sdhci-acpi.c > =================================================================== > --- linux-pm.orig/drivers/mmc/host/sdhci-acpi.c > +++ linux-pm/drivers/mmc/host/sdhci-acpi.c > @@ -775,8 +775,8 @@ static int sdhci_acpi_probe(struct platf > { > struct device *dev = &pdev->dev; > const struct sdhci_acpi_slot *slot; > - struct acpi_device *device, *child; > const struct dmi_system_id *id; > + struct acpi_device *device; > struct sdhci_acpi_host *c; > struct sdhci_host *host; > struct resource *iomem; > @@ -796,10 +796,7 @@ static int sdhci_acpi_probe(struct platf > slot = sdhci_acpi_get_slot(device); > > /* Power on the SDHCI controller and its children */ > - acpi_device_fix_up_power(device); > - list_for_each_entry(child, &device->children, node) > - if (child->status.present && child->status.enabled) > - acpi_device_fix_up_power(child); > + acpi_device_fix_up_power_extended(device); > > if (sdhci_acpi_byt_defer(dev)) > return -EPROBE_DEFER; > Index: linux-pm/drivers/acpi/device_pm.c > =================================================================== > --- linux-pm.orig/drivers/acpi/device_pm.c > +++ linux-pm/drivers/acpi/device_pm.c > @@ -369,6 +369,28 @@ int acpi_device_fix_up_power(struct acpi > } > EXPORT_SYMBOL_GPL(acpi_device_fix_up_power); > > +static int fix_up_power_if_applicable(struct acpi_device *adev, void *not_used) > +{ > + if (adev->status.present && adev->status.enabled) > + acpi_device_fix_up_power(adev); > + > + return 0; > +} > + > +/** > + * acpi_device_fix_up_power_extended - Force device and its children into D0. > + * @adev: Parent device object whose power state is to be fixed up. > + * > + * Call acpi_device_fix_up_power() for @adev and its children so long as they > + * are reported as present and enabled. > + */ > +void acpi_device_fix_up_power_extended(struct acpi_device *adev) > +{ > + acpi_device_fix_up_power(adev); > + acpi_dev_for_each_child(adev, fix_up_power_if_applicable, NULL); > +} > +EXPORT_SYMBOL_GPL(acpi_device_fix_up_power_extended); > + > int acpi_device_update_power(struct acpi_device *device, int *state_p) > { > int state; > Index: linux-pm/include/acpi/acpi_bus.h > =================================================================== > --- linux-pm.orig/include/acpi/acpi_bus.h > +++ linux-pm/include/acpi/acpi_bus.h > @@ -524,6 +524,7 @@ const char *acpi_power_state_string(int > int acpi_device_set_power(struct acpi_device *device, int state); > int acpi_bus_init_power(struct acpi_device *device); > int acpi_device_fix_up_power(struct acpi_device *device); > +void acpi_device_fix_up_power_extended(struct acpi_device *adev); > int acpi_bus_update_power(acpi_handle handle, int *state_p); > int acpi_device_update_power(struct acpi_device *device, int *state_p); > bool acpi_bus_power_manageable(acpi_handle handle); > Index: linux-pm/drivers/mmc/host/sdhci-pci-core.c > =================================================================== > --- linux-pm.orig/drivers/mmc/host/sdhci-pci-core.c > +++ linux-pm/drivers/mmc/host/sdhci-pci-core.c > @@ -1240,16 +1240,11 @@ static const struct sdhci_pci_fixes sdhc > #ifdef CONFIG_ACPI > static void intel_mrfld_mmc_fix_up_power_slot(struct sdhci_pci_slot *slot) > { > - struct acpi_device *device, *child; > + struct acpi_device *device; > > device = ACPI_COMPANION(&slot->chip->pdev->dev); > - if (!device) > - return; > - > - acpi_device_fix_up_power(device); > - list_for_each_entry(child, &device->children, node) > - if (child->status.present && child->status.enabled) > - acpi_device_fix_up_power(child); > + if (device) > + acpi_device_fix_up_power_extended(device); > } > #else > static inline void intel_mrfld_mmc_fix_up_power_slot(struct sdhci_pci_slot *slot) {} > > >
On Thu, 9 Jun 2022 at 16:20, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Introduce acpi_device_fix_up_power_extended() for fixing up power of > a device having an ACPI companion in a manner that takes the device's > children into account and make the MMC code use it in two places > instead of walking the list of the device ACPI companion's children > directly. > > This will help to eliminate the children list head from struct > acpi_device as it is redundant and it is used in questionable ways > in some places (in particular, locking is needed for walking the > list pointed to it safely, but it is often missing). > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Rafael, feel free to pick this via your tree. Acked-by: Ulf Hansson <ulf.hansson@linaro.org> Kind regards Uffe > --- > drivers/acpi/device_pm.c | 22 ++++++++++++++++++++++ > drivers/mmc/host/sdhci-acpi.c | 7 ++----- > drivers/mmc/host/sdhci-pci-core.c | 11 +++-------- > include/acpi/acpi_bus.h | 1 + > 4 files changed, 28 insertions(+), 13 deletions(-) > > Index: linux-pm/drivers/mmc/host/sdhci-acpi.c > =================================================================== > --- linux-pm.orig/drivers/mmc/host/sdhci-acpi.c > +++ linux-pm/drivers/mmc/host/sdhci-acpi.c > @@ -775,8 +775,8 @@ static int sdhci_acpi_probe(struct platf > { > struct device *dev = &pdev->dev; > const struct sdhci_acpi_slot *slot; > - struct acpi_device *device, *child; > const struct dmi_system_id *id; > + struct acpi_device *device; > struct sdhci_acpi_host *c; > struct sdhci_host *host; > struct resource *iomem; > @@ -796,10 +796,7 @@ static int sdhci_acpi_probe(struct platf > slot = sdhci_acpi_get_slot(device); > > /* Power on the SDHCI controller and its children */ > - acpi_device_fix_up_power(device); > - list_for_each_entry(child, &device->children, node) > - if (child->status.present && child->status.enabled) > - acpi_device_fix_up_power(child); > + acpi_device_fix_up_power_extended(device); > > if (sdhci_acpi_byt_defer(dev)) > return -EPROBE_DEFER; > Index: linux-pm/drivers/acpi/device_pm.c > =================================================================== > --- linux-pm.orig/drivers/acpi/device_pm.c > +++ linux-pm/drivers/acpi/device_pm.c > @@ -369,6 +369,28 @@ int acpi_device_fix_up_power(struct acpi > } > EXPORT_SYMBOL_GPL(acpi_device_fix_up_power); > > +static int fix_up_power_if_applicable(struct acpi_device *adev, void *not_used) > +{ > + if (adev->status.present && adev->status.enabled) > + acpi_device_fix_up_power(adev); > + > + return 0; > +} > + > +/** > + * acpi_device_fix_up_power_extended - Force device and its children into D0. > + * @adev: Parent device object whose power state is to be fixed up. > + * > + * Call acpi_device_fix_up_power() for @adev and its children so long as they > + * are reported as present and enabled. > + */ > +void acpi_device_fix_up_power_extended(struct acpi_device *adev) > +{ > + acpi_device_fix_up_power(adev); > + acpi_dev_for_each_child(adev, fix_up_power_if_applicable, NULL); > +} > +EXPORT_SYMBOL_GPL(acpi_device_fix_up_power_extended); > + > int acpi_device_update_power(struct acpi_device *device, int *state_p) > { > int state; > Index: linux-pm/include/acpi/acpi_bus.h > =================================================================== > --- linux-pm.orig/include/acpi/acpi_bus.h > +++ linux-pm/include/acpi/acpi_bus.h > @@ -524,6 +524,7 @@ const char *acpi_power_state_string(int > int acpi_device_set_power(struct acpi_device *device, int state); > int acpi_bus_init_power(struct acpi_device *device); > int acpi_device_fix_up_power(struct acpi_device *device); > +void acpi_device_fix_up_power_extended(struct acpi_device *adev); > int acpi_bus_update_power(acpi_handle handle, int *state_p); > int acpi_device_update_power(struct acpi_device *device, int *state_p); > bool acpi_bus_power_manageable(acpi_handle handle); > Index: linux-pm/drivers/mmc/host/sdhci-pci-core.c > =================================================================== > --- linux-pm.orig/drivers/mmc/host/sdhci-pci-core.c > +++ linux-pm/drivers/mmc/host/sdhci-pci-core.c > @@ -1240,16 +1240,11 @@ static const struct sdhci_pci_fixes sdhc > #ifdef CONFIG_ACPI > static void intel_mrfld_mmc_fix_up_power_slot(struct sdhci_pci_slot *slot) > { > - struct acpi_device *device, *child; > + struct acpi_device *device; > > device = ACPI_COMPANION(&slot->chip->pdev->dev); > - if (!device) > - return; > - > - acpi_device_fix_up_power(device); > - list_for_each_entry(child, &device->children, node) > - if (child->status.present && child->status.enabled) > - acpi_device_fix_up_power(child); > + if (device) > + acpi_device_fix_up_power_extended(device); > } > #else > static inline void intel_mrfld_mmc_fix_up_power_slot(struct sdhci_pci_slot *slot) {} > > >
Index: linux-pm/drivers/mmc/host/sdhci-acpi.c =================================================================== --- linux-pm.orig/drivers/mmc/host/sdhci-acpi.c +++ linux-pm/drivers/mmc/host/sdhci-acpi.c @@ -775,8 +775,8 @@ static int sdhci_acpi_probe(struct platf { struct device *dev = &pdev->dev; const struct sdhci_acpi_slot *slot; - struct acpi_device *device, *child; const struct dmi_system_id *id; + struct acpi_device *device; struct sdhci_acpi_host *c; struct sdhci_host *host; struct resource *iomem; @@ -796,10 +796,7 @@ static int sdhci_acpi_probe(struct platf slot = sdhci_acpi_get_slot(device); /* Power on the SDHCI controller and its children */ - acpi_device_fix_up_power(device); - list_for_each_entry(child, &device->children, node) - if (child->status.present && child->status.enabled) - acpi_device_fix_up_power(child); + acpi_device_fix_up_power_extended(device); if (sdhci_acpi_byt_defer(dev)) return -EPROBE_DEFER; Index: linux-pm/drivers/acpi/device_pm.c =================================================================== --- linux-pm.orig/drivers/acpi/device_pm.c +++ linux-pm/drivers/acpi/device_pm.c @@ -369,6 +369,28 @@ int acpi_device_fix_up_power(struct acpi } EXPORT_SYMBOL_GPL(acpi_device_fix_up_power); +static int fix_up_power_if_applicable(struct acpi_device *adev, void *not_used) +{ + if (adev->status.present && adev->status.enabled) + acpi_device_fix_up_power(adev); + + return 0; +} + +/** + * acpi_device_fix_up_power_extended - Force device and its children into D0. + * @adev: Parent device object whose power state is to be fixed up. + * + * Call acpi_device_fix_up_power() for @adev and its children so long as they + * are reported as present and enabled. + */ +void acpi_device_fix_up_power_extended(struct acpi_device *adev) +{ + acpi_device_fix_up_power(adev); + acpi_dev_for_each_child(adev, fix_up_power_if_applicable, NULL); +} +EXPORT_SYMBOL_GPL(acpi_device_fix_up_power_extended); + int acpi_device_update_power(struct acpi_device *device, int *state_p) { int state; Index: linux-pm/include/acpi/acpi_bus.h =================================================================== --- linux-pm.orig/include/acpi/acpi_bus.h +++ linux-pm/include/acpi/acpi_bus.h @@ -524,6 +524,7 @@ const char *acpi_power_state_string(int int acpi_device_set_power(struct acpi_device *device, int state); int acpi_bus_init_power(struct acpi_device *device); int acpi_device_fix_up_power(struct acpi_device *device); +void acpi_device_fix_up_power_extended(struct acpi_device *adev); int acpi_bus_update_power(acpi_handle handle, int *state_p); int acpi_device_update_power(struct acpi_device *device, int *state_p); bool acpi_bus_power_manageable(acpi_handle handle); Index: linux-pm/drivers/mmc/host/sdhci-pci-core.c =================================================================== --- linux-pm.orig/drivers/mmc/host/sdhci-pci-core.c +++ linux-pm/drivers/mmc/host/sdhci-pci-core.c @@ -1240,16 +1240,11 @@ static const struct sdhci_pci_fixes sdhc #ifdef CONFIG_ACPI static void intel_mrfld_mmc_fix_up_power_slot(struct sdhci_pci_slot *slot) { - struct acpi_device *device, *child; + struct acpi_device *device; device = ACPI_COMPANION(&slot->chip->pdev->dev); - if (!device) - return; - - acpi_device_fix_up_power(device); - list_for_each_entry(child, &device->children, node) - if (child->status.present && child->status.enabled) - acpi_device_fix_up_power(child); + if (device) + acpi_device_fix_up_power_extended(device); } #else static inline void intel_mrfld_mmc_fix_up_power_slot(struct sdhci_pci_slot *slot) {}