This series aims to add a new driver_to_pm() helper allowing for accessing the Power Management callbacs for a particular device.
Access to the callbacs (struct dev_pm_ops) is normally done through using the pm pointer that is embedded within the device_driver struct.
This new helper allows for the code required to reference the pm pointer and access Power Management callbas to be simplified. Changing the following:
struct device_driver *drv = dev->driver; if (dev->driver && dev->driver->pm && dev->driver->pm->prepare) { int ret = dev->driver->pm->prepare(dev);
To:
const struct dev_pm_ops *pm = driver_to_pm(dev->driver); if (pm && pm->prepare) { int ret = pm->prepare(dev);
Or, changing the following:
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
To: const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
This series builds on top of the previous commit 6da2f2ccfd2d ("PCI/PM: Make power management op coding style consistent") that had an aim to make accessing the Power Managemnet callbacs more consistent.
No functional change intended.
Links: https://lore.kernel.org/driverdev-devel/20191014230016.240912-6-helgaas@kern... https://lore.kernel.org/driverdev-devel/8592302.r4xC6RIy69@kreacher/ https://lore.kernel.org/driverdev-devel/20191016135002.GA24678@kadam/
Krzysztof Wilczyński (8): driver core: Add helper for accessing Power Management callbacs ACPI: PM: Use the new device_to_pm() helper to access struct dev_pm_ops greybus: Use the new device_to_pm() helper to access struct dev_pm_ops scsi: pm: Use the new device_to_pm() helper to access struct dev_pm_ops usb: phy: fsl: Use the new device_to_pm() helper to access struct dev_pm_ops PCI/PM: Use the new device_to_pm() helper to access struct dev_pm_ops PM: Use the new device_to_pm() helper to access struct dev_pm_ops net/iucv: Use the new device_to_pm() helper to access struct dev_pm_ops
drivers/acpi/device_pm.c | 5 ++- drivers/base/power/domain.c | 12 ++++-- drivers/base/power/generic_ops.c | 65 ++++++++++++++------------------ drivers/base/power/main.c | 48 +++++++++++++++-------- drivers/base/power/runtime.c | 7 ++-- drivers/greybus/bundle.c | 4 +- drivers/pci/pci-driver.c | 32 ++++++++-------- drivers/scsi/scsi_pm.c | 8 ++-- drivers/usb/phy/phy-fsl-usb.c | 11 ++++-- include/linux/device/driver.h | 15 ++++++++ net/iucv/iucv.c | 30 +++++++++------ 11 files changed, 138 insertions(+), 99 deletions(-)
Add driver_to_pm() helper allowing for accessing the Power Management callbacs for a particular device. Access to the callbacs (struct dev_pm_ops) is normally done through using the pm pointer that is embedded within the device_driver struct.
Helper allows for the code required to reference the pm pointer and access Power Management callbas to be simplified. Changing the following:
struct device_driver *drv = dev->driver; if (dev->driver && dev->driver->pm && dev->driver->pm->prepare) { int ret = dev->driver->pm->prepare(dev);
To:
const struct dev_pm_ops *pm = driver_to_pm(dev->driver); if (pm && pm->prepare) { int ret = pm->prepare(dev);
Or, changing the following:
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
To: const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
Signed-off-by: Krzysztof Wilczyński kw@linux.com --- include/linux/device/driver.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h index ee7ba5b5417e..ccd0b315fd93 100644 --- a/include/linux/device/driver.h +++ b/include/linux/device/driver.h @@ -236,6 +236,21 @@ driver_find_device_by_acpi_dev(struct device_driver *drv, const void *adev) } #endif
+/** + * driver_to_pm - Return Power Management callbacs (struct dev_pm_ops) for + * a particular device. + * @drv: Pointer to a device (struct device_driver) for which you want to access + * the Power Management callbacks. + * + * Returns a pointer to the struct dev_pm_ops embedded within the device (struct + * device_driver), or returns NULL if Power Management is not present and the + * pointer is not valid. + */ +static inline const struct dev_pm_ops *driver_to_pm(struct device_driver *drv) +{ + return drv && drv->pm ? drv->pm : NULL; +} + extern int driver_deferred_probe_timeout; void driver_deferred_probe_add(struct device *dev); int driver_deferred_probe_check_state(struct device *dev);
On Mon, May 25, 2020 at 06:26:01PM +0000, Krzysztof Wilczyński wrote:
Add driver_to_pm() helper allowing for accessing the Power Management callbacs for a particular device. Access to the callbacs (struct dev_pm_ops) is normally done through using the pm pointer that is embedded within the device_driver struct.
Helper allows for the code required to reference the pm pointer and access Power Management callbas to be simplified. Changing the following:
struct device_driver *drv = dev->driver; if (dev->driver && dev->driver->pm && dev->driver->pm->prepare) { int ret = dev->driver->pm->prepare(dev);
To:
const struct dev_pm_ops *pm = driver_to_pm(dev->driver); if (pm && pm->prepare) { int ret = pm->prepare(dev);
Or, changing the following:
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;To: const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
Signed-off-by: Krzysztof Wilczyński kw@linux.com
include/linux/device/driver.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h index ee7ba5b5417e..ccd0b315fd93 100644 --- a/include/linux/device/driver.h +++ b/include/linux/device/driver.h @@ -236,6 +236,21 @@ driver_find_device_by_acpi_dev(struct device_driver *drv, const void *adev) } #endif +/**
- driver_to_pm - Return Power Management callbacs (struct dev_pm_ops) for
a particular device.
- @drv: Pointer to a device (struct device_driver) for which you want to access
the Power Management callbacks.
- Returns a pointer to the struct dev_pm_ops embedded within the device (struct
- device_driver), or returns NULL if Power Management is not present and the
- pointer is not valid.
- */
+static inline const struct dev_pm_ops *driver_to_pm(struct device_driver *drv) +{
- return drv && drv->pm ? drv->pm : NULL;
I hate ? : lines with a passion, as they break normal pattern mattching in my brain. Please just spell this all out: if (drv && drv->pm) return drv->pm; return NULL;
Much easier to read, and the compiler will do the exact same thing.
Only place ? : are ok to use in my opinion, are as function arguments.
thanks,
greg k-h
On 5/26/20 1:33 AM, Greg Kroah-Hartman wrote:
On Mon, May 25, 2020 at 06:26:01PM +0000, Krzysztof Wilczyński wrote:
Add driver_to_pm() helper allowing for accessing the Power Management callbacs for a particular device. Access to the callbacs (struct dev_pm_ops) is normally done through using the pm pointer that is embedded within the device_driver struct.
Helper allows for the code required to reference the pm pointer and access Power Management callbas to be simplified. Changing the following:
struct device_driver *drv = dev->driver; if (dev->driver && dev->driver->pm && dev->driver->pm->prepare) { int ret = dev->driver->pm->prepare(dev);
To:
const struct dev_pm_ops *pm = driver_to_pm(dev->driver); if (pm && pm->prepare) { int ret = pm->prepare(dev);
Or, changing the following:
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;To: const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
Signed-off-by: Krzysztof Wilczyński kw@linux.com
include/linux/device/driver.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h index ee7ba5b5417e..ccd0b315fd93 100644 --- a/include/linux/device/driver.h +++ b/include/linux/device/driver.h @@ -236,6 +236,21 @@ driver_find_device_by_acpi_dev(struct device_driver *drv, const void *adev) } #endif +/**
- driver_to_pm - Return Power Management callbacs (struct dev_pm_ops) for
a particular device.
- @drv: Pointer to a device (struct device_driver) for which you want to access
the Power Management callbacks.
- Returns a pointer to the struct dev_pm_ops embedded within the device (struct
- device_driver), or returns NULL if Power Management is not present and the
- pointer is not valid.
- */
+static inline const struct dev_pm_ops *driver_to_pm(struct device_driver *drv) +{
- return drv && drv->pm ? drv->pm : NULL;
This could just be:
if (drv) return drv->pm;
return NULL;
Or if you want to evoke passion in Greg:
return drv ? drv->pm : NULL;
-Alex
I hate ? : lines with a passion, as they break normal pattern mattching in my brain. Please just spell this all out: if (drv && drv->pm) return drv->pm; return NULL;
Much easier to read, and the compiler will do the exact same thing.
Only place ? : are ok to use in my opinion, are as function arguments.
thanks,
greg k-h _______________________________________________ greybus-dev mailing list greybus-dev@lists.linaro.org https://lists.linaro.org/mailman/listinfo/greybus-dev
Hello Alex and Greg,
[...]
This could just be:
if (drv) return drv->pm;
return NULL;
Or if you want to evoke passion in Greg:
return drv ? drv->pm : NULL;
-AlexI hate ? : lines with a passion, as they break normal pattern mattching in my brain. Please just spell this all out: if (drv && drv->pm) return drv->pm; return NULL;
Much easier to read, and the compiler will do the exact same thing.
Only place ? : are ok to use in my opinion, are as function arguments.
I will steer away from the ternary operator next time. Also, good to learn about Greg's preference.
Thank you both!
Krzysztof
Use the new device_to_pm() helper to access Power Management callbacs (struct dev_pm_ops) for a particular device (struct device_driver).
No functional change intended.
Signed-off-by: Krzysztof Wilczyński kw@linux.com --- drivers/acpi/device_pm.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c index 5832bc10aca8..b98a32c48fbe 100644 --- a/drivers/acpi/device_pm.c +++ b/drivers/acpi/device_pm.c @@ -1022,9 +1022,10 @@ static bool acpi_dev_needs_resume(struct device *dev, struct acpi_device *adev) int acpi_subsys_prepare(struct device *dev) { struct acpi_device *adev = ACPI_COMPANION(dev); + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
- if (dev->driver && dev->driver->pm && dev->driver->pm->prepare) { - int ret = dev->driver->pm->prepare(dev); + if (pm && pm->prepare) { + int ret = pm->prepare(dev);
if (ret < 0) return ret;
On Mon, May 25, 2020 at 8:26 PM Krzysztof Wilczyński kw@linux.com wrote:
Use the new device_to_pm() helper to access Power Management callbacs (struct dev_pm_ops) for a particular device (struct device_driver).
No functional change intended.
Signed-off-by: Krzysztof Wilczyński kw@linux.com
drivers/acpi/device_pm.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c index 5832bc10aca8..b98a32c48fbe 100644 --- a/drivers/acpi/device_pm.c +++ b/drivers/acpi/device_pm.c @@ -1022,9 +1022,10 @@ static bool acpi_dev_needs_resume(struct device *dev, struct acpi_device *adev) int acpi_subsys_prepare(struct device *dev) { struct acpi_device *adev = ACPI_COMPANION(dev);
const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
I don't really see a reason for this change.
What's wrong with the check below?
if (dev->driver && dev->driver->pm && dev->driver->pm->prepare) {int ret = dev->driver->pm->prepare(dev);
if (pm && pm->prepare) {int ret = pm->prepare(dev); if (ret < 0) return ret;-- 2.26.2
On Tue 2020-05-26 10:37:36, Rafael J. Wysocki wrote:
On Mon, May 25, 2020 at 8:26 PM Krzysztof Wilczyński kw@linux.com wrote:
Use the new device_to_pm() helper to access Power Management callbacs (struct dev_pm_ops) for a particular device (struct device_driver).
No functional change intended.
Signed-off-by: Krzysztof Wilczyński kw@linux.com
drivers/acpi/device_pm.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c index 5832bc10aca8..b98a32c48fbe 100644 --- a/drivers/acpi/device_pm.c +++ b/drivers/acpi/device_pm.c @@ -1022,9 +1022,10 @@ static bool acpi_dev_needs_resume(struct device *dev, struct acpi_device *adev) int acpi_subsys_prepare(struct device *dev) { struct acpi_device *adev = ACPI_COMPANION(dev);
const struct dev_pm_ops *pm = driver_to_pm(dev->driver);I don't really see a reason for this change.
What's wrong with the check below?
Duplicated code. Yes, compiler can sort it out, but... new version looks better to me.
Best regards, pavel
if (dev->driver && dev->driver->pm && dev->driver->pm->prepare) {int ret = dev->driver->pm->prepare(dev);
if (pm && pm->prepare) {int ret = pm->prepare(dev);
On Tue, May 26, 2020 at 11:45 AM Pavel Machek pavel@denx.de wrote:
On Tue 2020-05-26 10:37:36, Rafael J. Wysocki wrote:
On Mon, May 25, 2020 at 8:26 PM Krzysztof Wilczyński kw@linux.com wrote:
Use the new device_to_pm() helper to access Power Management callbacs (struct dev_pm_ops) for a particular device (struct device_driver).
No functional change intended.
Signed-off-by: Krzysztof Wilczyński kw@linux.com
drivers/acpi/device_pm.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c index 5832bc10aca8..b98a32c48fbe 100644 --- a/drivers/acpi/device_pm.c +++ b/drivers/acpi/device_pm.c @@ -1022,9 +1022,10 @@ static bool acpi_dev_needs_resume(struct device *dev, struct acpi_device *adev) int acpi_subsys_prepare(struct device *dev) { struct acpi_device *adev = ACPI_COMPANION(dev);
const struct dev_pm_ops *pm = driver_to_pm(dev->driver);I don't really see a reason for this change.
What's wrong with the check below?
Duplicated code. Yes, compiler can sort it out, but... new version looks better to me.
So the new code would not be duplicated?
Look at the other patches in the series then. :-)
Use the new device_to_pm() helper to access Power Management callbacs (struct dev_pm_ops) for a particular device (struct device_driver).
No functional change intended.
Signed-off-by: Krzysztof Wilczyński kw@linux.com --- drivers/greybus/bundle.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/greybus/bundle.c b/drivers/greybus/bundle.c index 84660729538b..d38d3a630812 100644 --- a/drivers/greybus/bundle.c +++ b/drivers/greybus/bundle.c @@ -108,7 +108,7 @@ static void gb_bundle_enable_all_connections(struct gb_bundle *bundle) static int gb_bundle_suspend(struct device *dev) { struct gb_bundle *bundle = to_gb_bundle(dev); - const struct dev_pm_ops *pm = dev->driver->pm; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver); int ret;
if (pm && pm->runtime_suspend) { @@ -135,7 +135,7 @@ static int gb_bundle_suspend(struct device *dev) static int gb_bundle_resume(struct device *dev) { struct gb_bundle *bundle = to_gb_bundle(dev); - const struct dev_pm_ops *pm = dev->driver->pm; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver); int ret;
ret = gb_control_bundle_resume(bundle->intf->control, bundle->id);
On 5/25/20 1:26 PM, Krzysztof Wilczyński wrote:
Use the new device_to_pm() helper to access Power Management callbacs (struct dev_pm_ops) for a particular device (struct device_driver).
No functional change intended.
Looks fine to me.
Reviewed-by: Alex Elder elder@linaro.org
Signed-off-by: Krzysztof Wilczyński kw@linux.com
drivers/greybus/bundle.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/greybus/bundle.c b/drivers/greybus/bundle.c index 84660729538b..d38d3a630812 100644 --- a/drivers/greybus/bundle.c +++ b/drivers/greybus/bundle.c @@ -108,7 +108,7 @@ static void gb_bundle_enable_all_connections(struct gb_bundle *bundle) static int gb_bundle_suspend(struct device *dev) { struct gb_bundle *bundle = to_gb_bundle(dev);
- const struct dev_pm_ops *pm = dev->driver->pm;
- const struct dev_pm_ops *pm = driver_to_pm(dev->driver); int ret;
if (pm && pm->runtime_suspend) { @@ -135,7 +135,7 @@ static int gb_bundle_suspend(struct device *dev) static int gb_bundle_resume(struct device *dev) { struct gb_bundle *bundle = to_gb_bundle(dev);
- const struct dev_pm_ops *pm = dev->driver->pm;
- const struct dev_pm_ops *pm = driver_to_pm(dev->driver); int ret;
ret = gb_control_bundle_resume(bundle->intf->control, bundle->id);
Use the new device_to_pm() helper to access Power Management callbacs (struct dev_pm_ops) for a particular device (struct device_driver).
No functional change intended.
Signed-off-by: Krzysztof Wilczyński kw@linux.com --- drivers/scsi/scsi_pm.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c index 5f0ad8b32e3a..8f40b60d3383 100644 --- a/drivers/scsi/scsi_pm.c +++ b/drivers/scsi/scsi_pm.c @@ -53,7 +53,7 @@ static int do_scsi_restore(struct device *dev, const struct dev_pm_ops *pm) static int scsi_dev_type_suspend(struct device *dev, int (*cb)(struct device *, const struct dev_pm_ops *)) { - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver); int err;
/* flush pending in-flight resume operations, suspend is synchronous */ @@ -72,7 +72,7 @@ static int scsi_dev_type_suspend(struct device *dev, static int scsi_dev_type_resume(struct device *dev, int (*cb)(struct device *, const struct dev_pm_ops *)) { - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver); int err = 0;
err = cb(dev, pm); @@ -232,7 +232,7 @@ static int scsi_bus_restore(struct device *dev)
static int sdev_runtime_suspend(struct device *dev) { - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver); struct scsi_device *sdev = to_scsi_device(dev); int err = 0;
@@ -262,7 +262,7 @@ static int scsi_runtime_suspend(struct device *dev) static int sdev_runtime_resume(struct device *dev) { struct scsi_device *sdev = to_scsi_device(dev); - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver); int err = 0;
blk_pre_runtime_resume(sdev->request_queue);
Use the new device_to_pm() helper to access Power Management callbacs (struct dev_pm_ops) for a particular device (struct device_driver).
No functional change intended.
Signed-off-by: Krzysztof Wilczyński kw@linux.com --- drivers/usb/phy/phy-fsl-usb.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c index b451f4695f3f..3b9ad5db8380 100644 --- a/drivers/usb/phy/phy-fsl-usb.c +++ b/drivers/usb/phy/phy-fsl-usb.c @@ -460,6 +460,7 @@ int fsl_otg_start_host(struct otg_fsm *fsm, int on) struct device *dev; struct fsl_otg *otg_dev = container_of(otg->usb_phy, struct fsl_otg, phy); + const struct dev_pm_ops *pm; u32 retval = 0;
if (!otg->host) @@ -479,8 +480,9 @@ int fsl_otg_start_host(struct otg_fsm *fsm, int on) else { otg_reset_controller(); VDBG("host on......\n"); - if (dev->driver->pm && dev->driver->pm->resume) { - retval = dev->driver->pm->resume(dev); + pm = driver_to_pm(dev->driver); + if (pm && pm->resume) { + retval = pm->resume(dev); if (fsm->id) { /* default-b */ fsl_otg_drv_vbus(fsm, 1); @@ -504,8 +506,9 @@ int fsl_otg_start_host(struct otg_fsm *fsm, int on) else { VDBG("host off......\n"); if (dev && dev->driver) { - if (dev->driver->pm && dev->driver->pm->suspend) - retval = dev->driver->pm->suspend(dev); + pm = driver_to_pm(dev->driver); + if (pm && pm->suspend) + retval = pm->suspend(dev); if (fsm->id) /* default-b */ fsl_otg_drv_vbus(fsm, 0);
On Mon, May 25, 2020 at 8:26 PM Krzysztof Wilczyński kw@linux.com wrote:
Use the new device_to_pm() helper to access Power Management callbacs (struct dev_pm_ops) for a particular device (struct device_driver).
No functional change intended.
Signed-off-by: Krzysztof Wilczyński kw@linux.com
drivers/usb/phy/phy-fsl-usb.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/phy/phy-fsl-usb.c b/drivers/usb/phy/phy-fsl-usb.c index b451f4695f3f..3b9ad5db8380 100644 --- a/drivers/usb/phy/phy-fsl-usb.c +++ b/drivers/usb/phy/phy-fsl-usb.c @@ -460,6 +460,7 @@ int fsl_otg_start_host(struct otg_fsm *fsm, int on) struct device *dev; struct fsl_otg *otg_dev = container_of(otg->usb_phy, struct fsl_otg, phy);
const struct dev_pm_ops *pm; u32 retval = 0; if (!otg->host)@@ -479,8 +480,9 @@ int fsl_otg_start_host(struct otg_fsm *fsm, int on) else { otg_reset_controller(); VDBG("host on......\n");
if (dev->driver->pm && dev->driver->pm->resume) {retval = dev->driver->pm->resume(dev);
pm = driver_to_pm(dev->driver);if (pm && pm->resume) {retval = pm->resume(dev);
And why is the new version better this time?
if (fsm->id) { /* default-b */ fsl_otg_drv_vbus(fsm, 1);@@ -504,8 +506,9 @@ int fsl_otg_start_host(struct otg_fsm *fsm, int on) else { VDBG("host off......\n"); if (dev && dev->driver) {
if (dev->driver->pm && dev->driver->pm->suspend)retval = dev->driver->pm->suspend(dev);
pm = driver_to_pm(dev->driver);if (pm && pm->suspend)retval = pm->suspend(dev); if (fsm->id) /* default-b */ fsl_otg_drv_vbus(fsm, 0);-- 2.26.2
Use the new device_to_pm() helper to access Power Management callbacs (struct dev_pm_ops) for a particular device (struct device_driver).
No functional change intended.
Signed-off-by: Krzysztof Wilczyński kw@linux.com --- drivers/pci/pci-driver.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index 0454ca0e4e3f..bb52bb6642a0 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -652,7 +652,7 @@ static bool pci_has_legacy_pm_support(struct pci_dev *pci_dev) static int pci_pm_prepare(struct device *dev) { struct pci_dev *pci_dev = to_pci_dev(dev); - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
if (pm && pm->prepare) { int error = pm->prepare(dev); @@ -721,7 +721,7 @@ static void pcie_pme_root_status_cleanup(struct pci_dev *pci_dev) static int pci_pm_suspend(struct device *dev) { struct pci_dev *pci_dev = to_pci_dev(dev); - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
pci_dev->skip_bus_pm = false;
@@ -787,7 +787,7 @@ static int pci_pm_suspend_late(struct device *dev) static int pci_pm_suspend_noirq(struct device *dev) { struct pci_dev *pci_dev = to_pci_dev(dev); - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
if (dev_pm_smart_suspend_and_suspended(dev)) { dev->power.may_skip_resume = true; @@ -889,7 +889,7 @@ static int pci_pm_suspend_noirq(struct device *dev) static int pci_pm_resume_noirq(struct device *dev) { struct pci_dev *pci_dev = to_pci_dev(dev); - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver); pci_power_t prev_state = pci_dev->current_state; bool skip_bus_pm = pci_dev->skip_bus_pm;
@@ -931,7 +931,7 @@ static int pci_pm_resume_noirq(struct device *dev) static int pci_pm_resume(struct device *dev) { struct pci_dev *pci_dev = to_pci_dev(dev); - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
/* * This is necessary for the suspend error path in which resume is @@ -976,7 +976,7 @@ struct dev_pm_ops __weak pcibios_pm_ops; static int pci_pm_freeze(struct device *dev) { struct pci_dev *pci_dev = to_pci_dev(dev); - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
if (pci_has_legacy_pm_support(pci_dev)) return pci_legacy_suspend(dev, PMSG_FREEZE); @@ -1012,7 +1012,7 @@ static int pci_pm_freeze(struct device *dev) static int pci_pm_freeze_noirq(struct device *dev) { struct pci_dev *pci_dev = to_pci_dev(dev); - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
if (pci_has_legacy_pm_support(pci_dev)) return pci_legacy_suspend_late(dev, PMSG_FREEZE); @@ -1040,7 +1040,7 @@ static int pci_pm_freeze_noirq(struct device *dev) static int pci_pm_thaw_noirq(struct device *dev) { struct pci_dev *pci_dev = to_pci_dev(dev); - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver); int error;
if (pcibios_pm_ops.thaw_noirq) { @@ -1073,7 +1073,7 @@ static int pci_pm_thaw_noirq(struct device *dev) static int pci_pm_thaw(struct device *dev) { struct pci_dev *pci_dev = to_pci_dev(dev); - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver); int error = 0;
if (pci_has_legacy_pm_support(pci_dev)) @@ -1094,7 +1094,7 @@ static int pci_pm_thaw(struct device *dev) static int pci_pm_poweroff(struct device *dev) { struct pci_dev *pci_dev = to_pci_dev(dev); - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
if (pci_has_legacy_pm_support(pci_dev)) return pci_legacy_suspend(dev, PMSG_HIBERNATE); @@ -1138,7 +1138,7 @@ static int pci_pm_poweroff_late(struct device *dev) static int pci_pm_poweroff_noirq(struct device *dev) { struct pci_dev *pci_dev = to_pci_dev(dev); - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
if (dev_pm_smart_suspend_and_suspended(dev)) return 0; @@ -1181,7 +1181,7 @@ static int pci_pm_poweroff_noirq(struct device *dev) static int pci_pm_restore_noirq(struct device *dev) { struct pci_dev *pci_dev = to_pci_dev(dev); - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver); int error;
if (pcibios_pm_ops.restore_noirq) { @@ -1205,7 +1205,7 @@ static int pci_pm_restore_noirq(struct device *dev) static int pci_pm_restore(struct device *dev) { struct pci_dev *pci_dev = to_pci_dev(dev); - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
/* * This is necessary for the hibernation error path in which restore is @@ -1248,7 +1248,7 @@ static int pci_pm_restore(struct device *dev) static int pci_pm_runtime_suspend(struct device *dev) { struct pci_dev *pci_dev = to_pci_dev(dev); - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver); pci_power_t prev = pci_dev->current_state; int error;
@@ -1303,7 +1303,7 @@ static int pci_pm_runtime_suspend(struct device *dev) static int pci_pm_runtime_resume(struct device *dev) { struct pci_dev *pci_dev = to_pci_dev(dev); - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver); pci_power_t prev_state = pci_dev->current_state; int error = 0;
@@ -1334,7 +1334,7 @@ static int pci_pm_runtime_resume(struct device *dev) static int pci_pm_runtime_idle(struct device *dev) { struct pci_dev *pci_dev = to_pci_dev(dev); - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
/* * If pci_dev->driver is not set (unbound), the device should
Use the new device_to_pm() helper to access Power Management callbacs (struct dev_pm_ops) for a particular device (struct device_driver).
No functional change intended.
This change builds on top of the previous commit 6da2f2ccfd2d ("PCI/PM: Make power management op coding style consistent").
Links: https://lore.kernel.org/driverdev-devel/20191014230016.240912-6-helgaas@kern... https://lore.kernel.org/driverdev-devel/8592302.r4xC6RIy69@kreacher/ https://lore.kernel.org/driverdev-devel/20191016135002.GA24678@kadam/
Signed-off-by: Krzysztof Wilczyński kw@linux.com --- drivers/base/power/domain.c | 12 ++++-- drivers/base/power/generic_ops.c | 65 ++++++++++++++------------------ drivers/base/power/main.c | 48 +++++++++++++++-------- drivers/base/power/runtime.c | 7 ++-- 4 files changed, 73 insertions(+), 59 deletions(-)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 0a01df608849..92a96fcb2717 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -703,6 +703,7 @@ static void genpd_power_off_work_fn(struct work_struct *work) static int __genpd_runtime_suspend(struct device *dev) { int (*cb)(struct device *__dev); + const struct dev_pm_ops *pm;
if (dev->type && dev->type->pm) cb = dev->type->pm->runtime_suspend; @@ -713,8 +714,9 @@ static int __genpd_runtime_suspend(struct device *dev) else cb = NULL;
- if (!cb && dev->driver && dev->driver->pm) - cb = dev->driver->pm->runtime_suspend; + pm = driver_to_pm(dev->driver); + if (!cb && pm) + cb = pm->runtime_suspend;
return cb ? cb(dev) : 0; } @@ -726,6 +728,7 @@ static int __genpd_runtime_suspend(struct device *dev) static int __genpd_runtime_resume(struct device *dev) { int (*cb)(struct device *__dev); + const struct dev_pm_ops *pm;
if (dev->type && dev->type->pm) cb = dev->type->pm->runtime_resume; @@ -736,8 +739,9 @@ static int __genpd_runtime_resume(struct device *dev) else cb = NULL;
- if (!cb && dev->driver && dev->driver->pm) - cb = dev->driver->pm->runtime_resume; + pm = driver_to_pm(dev->driver); + if (!cb && pm) + cb = pm->runtime_resume;
return cb ? cb(dev) : 0; } diff --git a/drivers/base/power/generic_ops.c b/drivers/base/power/generic_ops.c index 4fa525668cb7..fbd2edef0201 100644 --- a/drivers/base/power/generic_ops.c +++ b/drivers/base/power/generic_ops.c @@ -19,12 +19,9 @@ */ int pm_generic_runtime_suspend(struct device *dev) { - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; - int ret; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
- ret = pm && pm->runtime_suspend ? pm->runtime_suspend(dev) : 0; - - return ret; + return pm && pm->runtime_suspend ? pm->runtime_suspend(dev) : 0; } EXPORT_SYMBOL_GPL(pm_generic_runtime_suspend);
@@ -38,12 +35,9 @@ EXPORT_SYMBOL_GPL(pm_generic_runtime_suspend); */ int pm_generic_runtime_resume(struct device *dev) { - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; - int ret; - - ret = pm && pm->runtime_resume ? pm->runtime_resume(dev) : 0; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
- return ret; + return pm && pm->runtime_resume ? pm->runtime_resume(dev) : 0; } EXPORT_SYMBOL_GPL(pm_generic_runtime_resume); #endif /* CONFIG_PM */ @@ -57,13 +51,12 @@ EXPORT_SYMBOL_GPL(pm_generic_runtime_resume); */ int pm_generic_prepare(struct device *dev) { - struct device_driver *drv = dev->driver; - int ret = 0; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
- if (drv && drv->pm && drv->pm->prepare) - ret = drv->pm->prepare(dev); + if (pm && pm->prepare) + return pm->prepare(dev);
- return ret; + return 0; }
/** @@ -72,7 +65,7 @@ int pm_generic_prepare(struct device *dev) */ int pm_generic_suspend_noirq(struct device *dev) { - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
return pm && pm->suspend_noirq ? pm->suspend_noirq(dev) : 0; } @@ -84,7 +77,7 @@ EXPORT_SYMBOL_GPL(pm_generic_suspend_noirq); */ int pm_generic_suspend_late(struct device *dev) { - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
return pm && pm->suspend_late ? pm->suspend_late(dev) : 0; } @@ -96,7 +89,7 @@ EXPORT_SYMBOL_GPL(pm_generic_suspend_late); */ int pm_generic_suspend(struct device *dev) { - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
return pm && pm->suspend ? pm->suspend(dev) : 0; } @@ -108,7 +101,7 @@ EXPORT_SYMBOL_GPL(pm_generic_suspend); */ int pm_generic_freeze_noirq(struct device *dev) { - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
return pm && pm->freeze_noirq ? pm->freeze_noirq(dev) : 0; } @@ -120,7 +113,7 @@ EXPORT_SYMBOL_GPL(pm_generic_freeze_noirq); */ int pm_generic_freeze_late(struct device *dev) { - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
return pm && pm->freeze_late ? pm->freeze_late(dev) : 0; } @@ -132,7 +125,7 @@ EXPORT_SYMBOL_GPL(pm_generic_freeze_late); */ int pm_generic_freeze(struct device *dev) { - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
return pm && pm->freeze ? pm->freeze(dev) : 0; } @@ -144,7 +137,7 @@ EXPORT_SYMBOL_GPL(pm_generic_freeze); */ int pm_generic_poweroff_noirq(struct device *dev) { - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
return pm && pm->poweroff_noirq ? pm->poweroff_noirq(dev) : 0; } @@ -156,7 +149,7 @@ EXPORT_SYMBOL_GPL(pm_generic_poweroff_noirq); */ int pm_generic_poweroff_late(struct device *dev) { - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
return pm && pm->poweroff_late ? pm->poweroff_late(dev) : 0; } @@ -168,7 +161,7 @@ EXPORT_SYMBOL_GPL(pm_generic_poweroff_late); */ int pm_generic_poweroff(struct device *dev) { - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
return pm && pm->poweroff ? pm->poweroff(dev) : 0; } @@ -180,7 +173,7 @@ EXPORT_SYMBOL_GPL(pm_generic_poweroff); */ int pm_generic_thaw_noirq(struct device *dev) { - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
return pm && pm->thaw_noirq ? pm->thaw_noirq(dev) : 0; } @@ -192,7 +185,7 @@ EXPORT_SYMBOL_GPL(pm_generic_thaw_noirq); */ int pm_generic_thaw_early(struct device *dev) { - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
return pm && pm->thaw_early ? pm->thaw_early(dev) : 0; } @@ -204,7 +197,7 @@ EXPORT_SYMBOL_GPL(pm_generic_thaw_early); */ int pm_generic_thaw(struct device *dev) { - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
return pm && pm->thaw ? pm->thaw(dev) : 0; } @@ -216,7 +209,7 @@ EXPORT_SYMBOL_GPL(pm_generic_thaw); */ int pm_generic_resume_noirq(struct device *dev) { - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
return pm && pm->resume_noirq ? pm->resume_noirq(dev) : 0; } @@ -228,7 +221,7 @@ EXPORT_SYMBOL_GPL(pm_generic_resume_noirq); */ int pm_generic_resume_early(struct device *dev) { - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
return pm && pm->resume_early ? pm->resume_early(dev) : 0; } @@ -240,7 +233,7 @@ EXPORT_SYMBOL_GPL(pm_generic_resume_early); */ int pm_generic_resume(struct device *dev) { - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
return pm && pm->resume ? pm->resume(dev) : 0; } @@ -252,7 +245,7 @@ EXPORT_SYMBOL_GPL(pm_generic_resume); */ int pm_generic_restore_noirq(struct device *dev) { - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
return pm && pm->restore_noirq ? pm->restore_noirq(dev) : 0; } @@ -264,7 +257,7 @@ EXPORT_SYMBOL_GPL(pm_generic_restore_noirq); */ int pm_generic_restore_early(struct device *dev) { - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
return pm && pm->restore_early ? pm->restore_early(dev) : 0; } @@ -276,7 +269,7 @@ EXPORT_SYMBOL_GPL(pm_generic_restore_early); */ int pm_generic_restore(struct device *dev) { - const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
return pm && pm->restore ? pm->restore(dev) : 0; } @@ -290,9 +283,9 @@ EXPORT_SYMBOL_GPL(pm_generic_restore); */ void pm_generic_complete(struct device *dev) { - struct device_driver *drv = dev->driver; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
- if (drv && drv->pm && drv->pm->complete) - drv->pm->complete(dev); + if (pm && pm->complete) + pm->complete(dev); } #endif /* CONFIG_PM_SLEEP */ diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c index 0e07e17c2def..6c41da0bebb0 100644 --- a/drivers/base/power/main.c +++ b/drivers/base/power/main.c @@ -640,6 +640,7 @@ static pm_callback_t dpm_subsys_suspend_late_cb(struct device *dev, static int device_resume_noirq(struct device *dev, pm_message_t state, bool async) { pm_callback_t callback; + const struct dev_pm_ops *pm; const char *info; bool skip_resume; int error = 0; @@ -687,9 +688,10 @@ static int device_resume_noirq(struct device *dev, pm_message_t state, bool asyn } }
- if (dev->driver && dev->driver->pm) { + pm = driver_to_pm(dev->driver); + if (pm) { info = "noirq driver "; - callback = pm_noirq_op(dev->driver->pm, state); + callback = pm_noirq_op(pm, state); }
Run: @@ -850,6 +852,7 @@ static pm_callback_t dpm_subsys_resume_early_cb(struct device *dev, static int device_resume_early(struct device *dev, pm_message_t state, bool async) { pm_callback_t callback; + const struct dev_pm_ops *pm; const char *info; int error = 0;
@@ -867,9 +870,10 @@ static int device_resume_early(struct device *dev, pm_message_t state, bool asyn
callback = dpm_subsys_resume_early_cb(dev, state, &info);
- if (!callback && dev->driver && dev->driver->pm) { + pm = driver_to_pm(dev->driver); + if (!callback && pm) { info = "early driver "; - callback = pm_late_early_op(dev->driver->pm, state); + callback = pm_late_early_op(pm, state); }
error = dpm_run_callback(callback, dev, state, info); @@ -963,6 +967,7 @@ static int device_resume(struct device *dev, pm_message_t state, bool async) { pm_callback_t callback = NULL; const char *info = NULL; + const struct dev_pm_ops *pm = NULL; int error = 0; DECLARE_DPM_WATCHDOG_ON_STACK(wd);
@@ -1023,9 +1028,10 @@ static int device_resume(struct device *dev, pm_message_t state, bool async) }
Driver: - if (!callback && dev->driver && dev->driver->pm) { + pm = driver_to_pm(dev->driver); + if (!callback && pm) { info = "driver "; - callback = pm_op(dev->driver->pm, state); + callback = pm_op(pm, state); }
End: @@ -1116,6 +1122,7 @@ void dpm_resume(pm_message_t state) static void device_complete(struct device *dev, pm_message_t state) { void (*callback)(struct device *) = NULL; + const struct dev_pm_ops *pm = NULL; const char *info = NULL;
if (dev->power.syscore) @@ -1137,9 +1144,10 @@ static void device_complete(struct device *dev, pm_message_t state) callback = dev->bus->pm->complete; }
- if (!callback && dev->driver && dev->driver->pm) { + pm = driver_to_pm(dev->driver); + if (!callback && pm) { info = "completing driver "; - callback = dev->driver->pm->complete; + callback = pm->complete; }
if (callback) { @@ -1312,6 +1320,7 @@ static bool device_must_resume(struct device *dev, pm_message_t state, static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool async) { pm_callback_t callback; + const struct dev_pm_ops *pm; const char *info; bool no_subsys_cb = false; int error = 0; @@ -1336,9 +1345,10 @@ static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool a if (dev_pm_smart_suspend_and_suspended(dev) && no_subsys_cb) goto Skip;
- if (dev->driver && dev->driver->pm) { + pm = driver_to_pm(dev->driver); + if (pm) { info = "noirq driver "; - callback = pm_noirq_op(dev->driver->pm, state); + callback = pm_noirq_op(pm, state); }
Run: @@ -1514,6 +1524,7 @@ static pm_callback_t dpm_subsys_suspend_late_cb(struct device *dev, static int __device_suspend_late(struct device *dev, pm_message_t state, bool async) { pm_callback_t callback; + const struct dev_pm_ops *pm; const char *info; int error = 0;
@@ -1543,9 +1554,10 @@ static int __device_suspend_late(struct device *dev, pm_message_t state, bool as !dpm_subsys_suspend_noirq_cb(dev, state, NULL)) goto Skip;
- if (dev->driver && dev->driver->pm) { + pm = driver_to_pm(dev->driver); + if (pm) { info = "late driver "; - callback = pm_late_early_op(dev->driver->pm, state); + callback = pm_late_early_op(pm, state); }
Run: @@ -1717,6 +1729,7 @@ static void dpm_clear_superiors_direct_complete(struct device *dev) static int __device_suspend(struct device *dev, pm_message_t state, bool async) { pm_callback_t callback = NULL; + const struct dev_pm_ops *pm = NULL; const char *info = NULL; int error = 0; DECLARE_DPM_WATCHDOG_ON_STACK(wd); @@ -1803,9 +1816,10 @@ static int __device_suspend(struct device *dev, pm_message_t state, bool async) }
Run: - if (!callback && dev->driver && dev->driver->pm) { + pm = driver_to_pm(dev->driver); + if (!callback && pm) { info = "driver "; - callback = pm_op(dev->driver->pm, state); + callback = pm_op(pm, state); }
error = dpm_run_callback(callback, dev, state, info); @@ -1917,6 +1931,7 @@ int dpm_suspend(pm_message_t state) static int device_prepare(struct device *dev, pm_message_t state) { int (*callback)(struct device *) = NULL; + const struct dev_pm_ops *pm = NULL; int ret = 0;
if (dev->power.syscore) @@ -1946,8 +1961,9 @@ static int device_prepare(struct device *dev, pm_message_t state) else if (dev->bus && dev->bus->pm) callback = dev->bus->pm->prepare;
- if (!callback && dev->driver && dev->driver->pm) - callback = dev->driver->pm->prepare; + pm = driver_to_pm(dev->driver); + if (!callback && pm) + callback = pm->prepare;
if (callback) ret = callback(dev); diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 99c7da112c95..c142824c7541 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -21,7 +21,7 @@ typedef int (*pm_callback_t)(struct device *); static pm_callback_t __rpm_get_callback(struct device *dev, size_t cb_offset) { pm_callback_t cb; - const struct dev_pm_ops *ops; + const struct dev_pm_ops *ops, *pm;
if (dev->pm_domain) ops = &dev->pm_domain->ops; @@ -39,8 +39,9 @@ static pm_callback_t __rpm_get_callback(struct device *dev, size_t cb_offset) else cb = NULL;
- if (!cb && dev->driver && dev->driver->pm) - cb = *(pm_callback_t *)((void *)dev->driver->pm + cb_offset); + pm = driver_to_pm(dev->driver); + if (!cb && pm) + cb = *(pm_callback_t *)((void *)pm + cb_offset);
return cb; }
On Mon, May 25, 2020 at 8:26 PM Krzysztof Wilczyński kw@linux.com wrote:
Use the new device_to_pm() helper to access Power Management callbacs (struct dev_pm_ops) for a particular device (struct device_driver).
No functional change intended.
This change builds on top of the previous commit 6da2f2ccfd2d ("PCI/PM: Make power management op coding style consistent").
Links: https://lore.kernel.org/driverdev-devel/20191014230016.240912-6-helgaas@kern... https://lore.kernel.org/driverdev-devel/8592302.r4xC6RIy69@kreacher/ https://lore.kernel.org/driverdev-devel/20191016135002.GA24678@kadam/
Signed-off-by: Krzysztof Wilczyński kw@linux.com
drivers/base/power/domain.c | 12 ++++-- drivers/base/power/generic_ops.c | 65 ++++++++++++++------------------ drivers/base/power/main.c | 48 +++++++++++++++-------- drivers/base/power/runtime.c | 7 ++-- 4 files changed, 73 insertions(+), 59 deletions(-)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 0a01df608849..92a96fcb2717 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -703,6 +703,7 @@ static void genpd_power_off_work_fn(struct work_struct *work) static int __genpd_runtime_suspend(struct device *dev) { int (*cb)(struct device *__dev);
const struct dev_pm_ops *pm; if (dev->type && dev->type->pm) cb = dev->type->pm->runtime_suspend;@@ -713,8 +714,9 @@ static int __genpd_runtime_suspend(struct device *dev) else cb = NULL;
if (!cb && dev->driver && dev->driver->pm)cb = dev->driver->pm->runtime_suspend;
pm = driver_to_pm(dev->driver);if (!cb && pm)cb = pm->runtime_suspend;
So why exactly is the new version better?
1. It adds lines of code. 2. It adds checks. 3. It adds function calls. 4. It makes one need to see the extra driver_to_pm() function to find out what's going on.
Which of the above is an improvement?
return cb ? cb(dev) : 0;} @@ -726,6 +728,7 @@ static int __genpd_runtime_suspend(struct device *dev) static int __genpd_runtime_resume(struct device *dev) { int (*cb)(struct device *__dev);
const struct dev_pm_ops *pm; if (dev->type && dev->type->pm) cb = dev->type->pm->runtime_resume;@@ -736,8 +739,9 @@ static int __genpd_runtime_resume(struct device *dev) else cb = NULL;
if (!cb && dev->driver && dev->driver->pm)cb = dev->driver->pm->runtime_resume;
pm = driver_to_pm(dev->driver);if (!cb && pm)cb = pm->runtime_resume; return cb ? cb(dev) : 0;} diff --git a/drivers/base/power/generic_ops.c b/drivers/base/power/generic_ops.c index 4fa525668cb7..fbd2edef0201 100644 --- a/drivers/base/power/generic_ops.c +++ b/drivers/base/power/generic_ops.c @@ -19,12 +19,9 @@ */ int pm_generic_runtime_suspend(struct device *dev) {
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;int ret;
const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
ret = pm && pm->runtime_suspend ? pm->runtime_suspend(dev) : 0;return ret;
return pm && pm->runtime_suspend ? pm->runtime_suspend(dev) : 0;} EXPORT_SYMBOL_GPL(pm_generic_runtime_suspend);
@@ -38,12 +35,9 @@ EXPORT_SYMBOL_GPL(pm_generic_runtime_suspend); */ int pm_generic_runtime_resume(struct device *dev) {
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;int ret;ret = pm && pm->runtime_resume ? pm->runtime_resume(dev) : 0;
const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
return ret;
return pm && pm->runtime_resume ? pm->runtime_resume(dev) : 0;} EXPORT_SYMBOL_GPL(pm_generic_runtime_resume); #endif /* CONFIG_PM */ @@ -57,13 +51,12 @@ EXPORT_SYMBOL_GPL(pm_generic_runtime_resume); */ int pm_generic_prepare(struct device *dev) {
struct device_driver *drv = dev->driver;int ret = 0;
const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
if (drv && drv->pm && drv->pm->prepare)ret = drv->pm->prepare(dev);
if (pm && pm->prepare)return pm->prepare(dev);
return ret;
return 0;}
/** @@ -72,7 +65,7 @@ int pm_generic_prepare(struct device *dev) */ int pm_generic_suspend_noirq(struct device *dev) {
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
const struct dev_pm_ops *pm = driver_to_pm(dev->driver); return pm && pm->suspend_noirq ? pm->suspend_noirq(dev) : 0;} @@ -84,7 +77,7 @@ EXPORT_SYMBOL_GPL(pm_generic_suspend_noirq); */ int pm_generic_suspend_late(struct device *dev) {
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
const struct dev_pm_ops *pm = driver_to_pm(dev->driver); return pm && pm->suspend_late ? pm->suspend_late(dev) : 0;} @@ -96,7 +89,7 @@ EXPORT_SYMBOL_GPL(pm_generic_suspend_late); */ int pm_generic_suspend(struct device *dev) {
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
const struct dev_pm_ops *pm = driver_to_pm(dev->driver); return pm && pm->suspend ? pm->suspend(dev) : 0;} @@ -108,7 +101,7 @@ EXPORT_SYMBOL_GPL(pm_generic_suspend); */ int pm_generic_freeze_noirq(struct device *dev) {
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
const struct dev_pm_ops *pm = driver_to_pm(dev->driver); return pm && pm->freeze_noirq ? pm->freeze_noirq(dev) : 0;} @@ -120,7 +113,7 @@ EXPORT_SYMBOL_GPL(pm_generic_freeze_noirq); */ int pm_generic_freeze_late(struct device *dev) {
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
const struct dev_pm_ops *pm = driver_to_pm(dev->driver); return pm && pm->freeze_late ? pm->freeze_late(dev) : 0;} @@ -132,7 +125,7 @@ EXPORT_SYMBOL_GPL(pm_generic_freeze_late); */ int pm_generic_freeze(struct device *dev) {
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
const struct dev_pm_ops *pm = driver_to_pm(dev->driver); return pm && pm->freeze ? pm->freeze(dev) : 0;} @@ -144,7 +137,7 @@ EXPORT_SYMBOL_GPL(pm_generic_freeze); */ int pm_generic_poweroff_noirq(struct device *dev) {
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
const struct dev_pm_ops *pm = driver_to_pm(dev->driver); return pm && pm->poweroff_noirq ? pm->poweroff_noirq(dev) : 0;} @@ -156,7 +149,7 @@ EXPORT_SYMBOL_GPL(pm_generic_poweroff_noirq); */ int pm_generic_poweroff_late(struct device *dev) {
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
const struct dev_pm_ops *pm = driver_to_pm(dev->driver); return pm && pm->poweroff_late ? pm->poweroff_late(dev) : 0;} @@ -168,7 +161,7 @@ EXPORT_SYMBOL_GPL(pm_generic_poweroff_late); */ int pm_generic_poweroff(struct device *dev) {
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
const struct dev_pm_ops *pm = driver_to_pm(dev->driver); return pm && pm->poweroff ? pm->poweroff(dev) : 0;} @@ -180,7 +173,7 @@ EXPORT_SYMBOL_GPL(pm_generic_poweroff); */ int pm_generic_thaw_noirq(struct device *dev) {
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
const struct dev_pm_ops *pm = driver_to_pm(dev->driver); return pm && pm->thaw_noirq ? pm->thaw_noirq(dev) : 0;} @@ -192,7 +185,7 @@ EXPORT_SYMBOL_GPL(pm_generic_thaw_noirq); */ int pm_generic_thaw_early(struct device *dev) {
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
const struct dev_pm_ops *pm = driver_to_pm(dev->driver); return pm && pm->thaw_early ? pm->thaw_early(dev) : 0;} @@ -204,7 +197,7 @@ EXPORT_SYMBOL_GPL(pm_generic_thaw_early); */ int pm_generic_thaw(struct device *dev) {
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
const struct dev_pm_ops *pm = driver_to_pm(dev->driver); return pm && pm->thaw ? pm->thaw(dev) : 0;} @@ -216,7 +209,7 @@ EXPORT_SYMBOL_GPL(pm_generic_thaw); */ int pm_generic_resume_noirq(struct device *dev) {
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
const struct dev_pm_ops *pm = driver_to_pm(dev->driver); return pm && pm->resume_noirq ? pm->resume_noirq(dev) : 0;} @@ -228,7 +221,7 @@ EXPORT_SYMBOL_GPL(pm_generic_resume_noirq); */ int pm_generic_resume_early(struct device *dev) {
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
const struct dev_pm_ops *pm = driver_to_pm(dev->driver); return pm && pm->resume_early ? pm->resume_early(dev) : 0;} @@ -240,7 +233,7 @@ EXPORT_SYMBOL_GPL(pm_generic_resume_early); */ int pm_generic_resume(struct device *dev) {
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
const struct dev_pm_ops *pm = driver_to_pm(dev->driver); return pm && pm->resume ? pm->resume(dev) : 0;} @@ -252,7 +245,7 @@ EXPORT_SYMBOL_GPL(pm_generic_resume); */ int pm_generic_restore_noirq(struct device *dev) {
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
const struct dev_pm_ops *pm = driver_to_pm(dev->driver); return pm && pm->restore_noirq ? pm->restore_noirq(dev) : 0;} @@ -264,7 +257,7 @@ EXPORT_SYMBOL_GPL(pm_generic_restore_noirq); */ int pm_generic_restore_early(struct device *dev) {
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
const struct dev_pm_ops *pm = driver_to_pm(dev->driver); return pm && pm->restore_early ? pm->restore_early(dev) : 0;} @@ -276,7 +269,7 @@ EXPORT_SYMBOL_GPL(pm_generic_restore_early); */ int pm_generic_restore(struct device *dev) {
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
const struct dev_pm_ops *pm = driver_to_pm(dev->driver); return pm && pm->restore ? pm->restore(dev) : 0;} @@ -290,9 +283,9 @@ EXPORT_SYMBOL_GPL(pm_generic_restore); */ void pm_generic_complete(struct device *dev) {
struct device_driver *drv = dev->driver;
const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
if (drv && drv->pm && drv->pm->complete)drv->pm->complete(dev);
if (pm && pm->complete)pm->complete(dev);} #endif /* CONFIG_PM_SLEEP */ diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c index 0e07e17c2def..6c41da0bebb0 100644 --- a/drivers/base/power/main.c +++ b/drivers/base/power/main.c @@ -640,6 +640,7 @@ static pm_callback_t dpm_subsys_suspend_late_cb(struct device *dev, static int device_resume_noirq(struct device *dev, pm_message_t state, bool async) { pm_callback_t callback;
const struct dev_pm_ops *pm; const char *info; bool skip_resume; int error = 0;@@ -687,9 +688,10 @@ static int device_resume_noirq(struct device *dev, pm_message_t state, bool asyn } }
if (dev->driver && dev->driver->pm) {
pm = driver_to_pm(dev->driver);if (pm) { info = "noirq driver ";
callback = pm_noirq_op(dev->driver->pm, state);
callback = pm_noirq_op(pm, state); }Run: @@ -850,6 +852,7 @@ static pm_callback_t dpm_subsys_resume_early_cb(struct device *dev, static int device_resume_early(struct device *dev, pm_message_t state, bool async) { pm_callback_t callback;
const struct dev_pm_ops *pm; const char *info; int error = 0;@@ -867,9 +870,10 @@ static int device_resume_early(struct device *dev, pm_message_t state, bool asyn
callback = dpm_subsys_resume_early_cb(dev, state, &info);
if (!callback && dev->driver && dev->driver->pm) {
pm = driver_to_pm(dev->driver);if (!callback && pm) { info = "early driver ";
callback = pm_late_early_op(dev->driver->pm, state);
callback = pm_late_early_op(pm, state); } error = dpm_run_callback(callback, dev, state, info);@@ -963,6 +967,7 @@ static int device_resume(struct device *dev, pm_message_t state, bool async) { pm_callback_t callback = NULL; const char *info = NULL;
const struct dev_pm_ops *pm = NULL; int error = 0; DECLARE_DPM_WATCHDOG_ON_STACK(wd);@@ -1023,9 +1028,10 @@ static int device_resume(struct device *dev, pm_message_t state, bool async) }
Driver:
if (!callback && dev->driver && dev->driver->pm) {
pm = driver_to_pm(dev->driver);if (!callback && pm) { info = "driver ";
callback = pm_op(dev->driver->pm, state);
callback = pm_op(pm, state); }End:
@@ -1116,6 +1122,7 @@ void dpm_resume(pm_message_t state) static void device_complete(struct device *dev, pm_message_t state) { void (*callback)(struct device *) = NULL;
const struct dev_pm_ops *pm = NULL; const char *info = NULL; if (dev->power.syscore)@@ -1137,9 +1144,10 @@ static void device_complete(struct device *dev, pm_message_t state) callback = dev->bus->pm->complete; }
if (!callback && dev->driver && dev->driver->pm) {
pm = driver_to_pm(dev->driver);if (!callback && pm) { info = "completing driver ";
callback = dev->driver->pm->complete;
callback = pm->complete; } if (callback) {@@ -1312,6 +1320,7 @@ static bool device_must_resume(struct device *dev, pm_message_t state, static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool async) { pm_callback_t callback;
const struct dev_pm_ops *pm; const char *info; bool no_subsys_cb = false; int error = 0;@@ -1336,9 +1345,10 @@ static int __device_suspend_noirq(struct device *dev, pm_message_t state, bool a if (dev_pm_smart_suspend_and_suspended(dev) && no_subsys_cb) goto Skip;
if (dev->driver && dev->driver->pm) {
pm = driver_to_pm(dev->driver);if (pm) { info = "noirq driver ";
callback = pm_noirq_op(dev->driver->pm, state);
callback = pm_noirq_op(pm, state); }Run: @@ -1514,6 +1524,7 @@ static pm_callback_t dpm_subsys_suspend_late_cb(struct device *dev, static int __device_suspend_late(struct device *dev, pm_message_t state, bool async) { pm_callback_t callback;
const struct dev_pm_ops *pm; const char *info; int error = 0;@@ -1543,9 +1554,10 @@ static int __device_suspend_late(struct device *dev, pm_message_t state, bool as !dpm_subsys_suspend_noirq_cb(dev, state, NULL)) goto Skip;
if (dev->driver && dev->driver->pm) {
pm = driver_to_pm(dev->driver);if (pm) { info = "late driver ";
callback = pm_late_early_op(dev->driver->pm, state);
callback = pm_late_early_op(pm, state); }Run: @@ -1717,6 +1729,7 @@ static void dpm_clear_superiors_direct_complete(struct device *dev) static int __device_suspend(struct device *dev, pm_message_t state, bool async) { pm_callback_t callback = NULL;
const struct dev_pm_ops *pm = NULL; const char *info = NULL; int error = 0; DECLARE_DPM_WATCHDOG_ON_STACK(wd);@@ -1803,9 +1816,10 @@ static int __device_suspend(struct device *dev, pm_message_t state, bool async) }
Run:
if (!callback && dev->driver && dev->driver->pm) {
pm = driver_to_pm(dev->driver);if (!callback && pm) { info = "driver ";
callback = pm_op(dev->driver->pm, state);
callback = pm_op(pm, state); } error = dpm_run_callback(callback, dev, state, info);@@ -1917,6 +1931,7 @@ int dpm_suspend(pm_message_t state) static int device_prepare(struct device *dev, pm_message_t state) { int (*callback)(struct device *) = NULL;
const struct dev_pm_ops *pm = NULL; int ret = 0; if (dev->power.syscore)@@ -1946,8 +1961,9 @@ static int device_prepare(struct device *dev, pm_message_t state) else if (dev->bus && dev->bus->pm) callback = dev->bus->pm->prepare;
if (!callback && dev->driver && dev->driver->pm)callback = dev->driver->pm->prepare;
pm = driver_to_pm(dev->driver);if (!callback && pm)callback = pm->prepare; if (callback) ret = callback(dev);diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 99c7da112c95..c142824c7541 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -21,7 +21,7 @@ typedef int (*pm_callback_t)(struct device *); static pm_callback_t __rpm_get_callback(struct device *dev, size_t cb_offset) { pm_callback_t cb;
const struct dev_pm_ops *ops;
const struct dev_pm_ops *ops, *pm; if (dev->pm_domain) ops = &dev->pm_domain->ops;@@ -39,8 +39,9 @@ static pm_callback_t __rpm_get_callback(struct device *dev, size_t cb_offset) else cb = NULL;
if (!cb && dev->driver && dev->driver->pm)cb = *(pm_callback_t *)((void *)dev->driver->pm + cb_offset);
pm = driver_to_pm(dev->driver);if (!cb && pm)cb = *(pm_callback_t *)((void *)pm + cb_offset); return cb;}
2.26.2
Use the new device_to_pm() helper to access Power Management callbacs (struct dev_pm_ops) for a particular device (struct device_driver).
No functional change intended.
Signed-off-by: Krzysztof Wilczyński kw@linux.com --- net/iucv/iucv.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/net/iucv/iucv.c b/net/iucv/iucv.c index 9a2d023842fe..1a3029ab7c1f 100644 --- a/net/iucv/iucv.c +++ b/net/iucv/iucv.c @@ -1836,23 +1836,23 @@ static void iucv_external_interrupt(struct ext_code ext_code,
static int iucv_pm_prepare(struct device *dev) { - int rc = 0; + const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
#ifdef CONFIG_PM_DEBUG printk(KERN_INFO "iucv_pm_prepare\n"); #endif - if (dev->driver && dev->driver->pm && dev->driver->pm->prepare) - rc = dev->driver->pm->prepare(dev); - return rc; + return pm && pm->prepare ? pm->prepare(dev) : 0; }
static void iucv_pm_complete(struct device *dev) { + const struct dev_pm_ops *pm = driver_to_pm(dev->driver); + #ifdef CONFIG_PM_DEBUG printk(KERN_INFO "iucv_pm_complete\n"); #endif - if (dev->driver && dev->driver->pm && dev->driver->pm->complete) - dev->driver->pm->complete(dev); + if (pm && pm->complete) + pm->complete(dev); }
/** @@ -1883,6 +1883,7 @@ static int iucv_path_table_empty(void) static int iucv_pm_freeze(struct device *dev) { int cpu; + const struct dev_pm_ops *pm; struct iucv_irq_list *p, *n; int rc = 0;
@@ -1902,8 +1903,9 @@ static int iucv_pm_freeze(struct device *dev) } } iucv_pm_state = IUCV_PM_FREEZING; - if (dev->driver && dev->driver->pm && dev->driver->pm->freeze) - rc = dev->driver->pm->freeze(dev); + pm = driver_to_pm(dev->driver); + if (pm && pm->freeze) + rc = pm->freeze(dev); if (iucv_path_table_empty()) iucv_disable(); return rc; @@ -1919,6 +1921,7 @@ static int iucv_pm_freeze(struct device *dev) */ static int iucv_pm_thaw(struct device *dev) { + const struct dev_pm_ops *pm; int rc = 0;
#ifdef CONFIG_PM_DEBUG @@ -1938,8 +1941,9 @@ static int iucv_pm_thaw(struct device *dev) /* enable interrupts on all cpus */ iucv_setmask_mp(); } - if (dev->driver && dev->driver->pm && dev->driver->pm->thaw) - rc = dev->driver->pm->thaw(dev); + pm = driver_to_pm(dev->driver); + if (pm && pm->thaw) + rc = pm->thaw(dev); out: return rc; } @@ -1954,6 +1958,7 @@ static int iucv_pm_thaw(struct device *dev) */ static int iucv_pm_restore(struct device *dev) { + const struct dev_pm_ops *pm; int rc = 0;
#ifdef CONFIG_PM_DEBUG @@ -1968,8 +1973,9 @@ static int iucv_pm_restore(struct device *dev) if (rc) goto out; } - if (dev->driver && dev->driver->pm && dev->driver->pm->restore) - rc = dev->driver->pm->restore(dev); + pm = driver_to_pm(dev->driver); + if (pm && pm->restore) + rc = pm->restore(dev); out: return rc; }
On Mon, May 25, 2020 at 06:26:08PM +0000, Krzysztof Wilczyński wrote:
Use the new device_to_pm() helper to access Power Management callbacs (struct dev_pm_ops) for a particular device (struct device_driver).
No functional change intended.
Signed-off-by: Krzysztof Wilczyński kw@linux.com
net/iucv/iucv.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/net/iucv/iucv.c b/net/iucv/iucv.c index 9a2d023842fe..1a3029ab7c1f 100644 --- a/net/iucv/iucv.c +++ b/net/iucv/iucv.c @@ -1836,23 +1836,23 @@ static void iucv_external_interrupt(struct ext_code ext_code, static int iucv_pm_prepare(struct device *dev) {
- int rc = 0;
- const struct dev_pm_ops *pm = driver_to_pm(dev->driver);
#ifdef CONFIG_PM_DEBUG printk(KERN_INFO "iucv_pm_prepare\n"); #endif
- if (dev->driver && dev->driver->pm && dev->driver->pm->prepare)
rc = dev->driver->pm->prepare(dev);- return rc;
- return pm && pm->prepare ? pm->prepare(dev) : 0;
No need for ? : here either, just use if () please.
It's "interesting" how using your new helper doesn't actually make the code smaller. Perhaps it isn't a good helper function?
thanks,
greg k-h
Hello Greg,
[...]
It's "interesting" how using your new helper doesn't actually make the code smaller. Perhaps it isn't a good helper function?
The idea for the helper was inspired by the comment Dan made to Bjorn about Bjorn's change, as per:
https://lore.kernel.org/driverdev-devel/20191016135002.GA24678@kadam/
It looked like a good idea to try to reduce the following:
dev->driver && dev->driver->pm && dev->driver->pm->prepare
Into something more succinct. Albeit, given the feedback from yourself and Rafael, I gather that this helper is not really a good addition.
Thank you everyone and sorry for the commotion!
Krzysztof
On Tue, May 26, 2020 at 5:07 PM Krzysztof Wilczyński kw@linux.com wrote:
Hello Greg,
[...]
It's "interesting" how using your new helper doesn't actually make the code smaller. Perhaps it isn't a good helper function?
The idea for the helper was inspired by the comment Dan made to Bjorn about Bjorn's change, as per:
https://lore.kernel.org/driverdev-devel/20191016135002.GA24678@kadam/
It looked like a good idea to try to reduce the following:
dev->driver && dev->driver->pm && dev->driver->pm->prepare
Into something more succinct. Albeit, given the feedback from yourself and Rafael, I gather that this helper is not really a good addition.
IMO it could be used for reducing code duplication like you did in the PCI code, but not necessarily in the other places where the code in question is not exactly duplicated.
Thanks!
On Tue, May 26, 2020 at 05:19:07PM +0200, Rafael J. Wysocki wrote:
On Tue, May 26, 2020 at 5:07 PM Krzysztof Wilczyński kw@linux.com wrote:
Hello Greg,
[...]
It's "interesting" how using your new helper doesn't actually make the code smaller. Perhaps it isn't a good helper function?
The idea for the helper was inspired by the comment Dan made to Bjorn about Bjorn's change, as per:
https://lore.kernel.org/driverdev-devel/20191016135002.GA24678@kadam/
It looked like a good idea to try to reduce the following:
dev->driver && dev->driver->pm && dev->driver->pm->prepare
Into something more succinct. Albeit, given the feedback from yourself and Rafael, I gather that this helper is not really a good addition.
IMO it could be used for reducing code duplication like you did in the PCI code, but not necessarily in the other places where the code in question is not exactly duplicated.
The code could be a little more succinct, although it wouldn't fit every usage. For example,
#define pm_do_callback(dev, method) \ (dev->driver && dev->driver->pm && dev->driver->pm->callback ? \ dev->driver->pm->callback(dev) : 0)
Then the usage is something like:
ret = pm_do_callback(dev, prepare);
Would this be an overall improvement?
Alan Stern
On Tue, May 26, 2020 at 5:28 PM Alan Stern stern@rowland.harvard.edu wrote:
On Tue, May 26, 2020 at 05:19:07PM +0200, Rafael J. Wysocki wrote:
On Tue, May 26, 2020 at 5:07 PM Krzysztof Wilczyński kw@linux.com wrote:
Hello Greg,
[...]
It's "interesting" how using your new helper doesn't actually make the code smaller. Perhaps it isn't a good helper function?
The idea for the helper was inspired by the comment Dan made to Bjorn about Bjorn's change, as per:
https://lore.kernel.org/driverdev-devel/20191016135002.GA24678@kadam/
It looked like a good idea to try to reduce the following:
dev->driver && dev->driver->pm && dev->driver->pm->prepare
Into something more succinct. Albeit, given the feedback from yourself and Rafael, I gather that this helper is not really a good addition.
IMO it could be used for reducing code duplication like you did in the PCI code, but not necessarily in the other places where the code in question is not exactly duplicated.
The code could be a little more succinct, although it wouldn't fit every usage. For example,
#define pm_do_callback(dev, method) \ (dev->driver && dev->driver->pm && dev->driver->pm->callback ? \ dev->driver->pm->callback(dev) : 0)
Then the usage is something like:
ret = pm_do_callback(dev, prepare);Would this be an overall improvement?
It wouldn't cover all of the use cases.
For example, PCI does other things in addition to running a callback when it is present.
Something like this might be enough though:
#define pm_driver_callback_is_present(dev, method) \ (dev->driver && dev->driver->pm && dev->driver->pm->method)
#define pm_run_driver_callback(dev, method) \ (pm_driver_callback_is_present(dev, method) ? dev->driver->pm->method(dev) : 0)
#define pm_get_driver_callback(dev, method) \ (pm_driver_callback_is_present(dev, method) ? dev->driver->pm->method : NULL)
so whoever needs the callback pointer can use pm_get_driver_callback() and whoever only needs to run the callback can use pm_run_driver_callback().
Cheers!
On 5/26/20 10:07 AM, Krzysztof Wilczyński wrote:
Hello Greg,
[...]
It's "interesting" how using your new helper doesn't actually make the code smaller. Perhaps it isn't a good helper function?
Helper functions often improve code readability, which is beneficial even if it doesn't reduce code size or efficiency.
But I won't argue for or against this particular change. It's OK with me either way.
-Alex
The idea for the helper was inspired by the comment Dan made to Bjorn about Bjorn's change, as per:
https://lore.kernel.org/driverdev-devel/20191016135002.GA24678@kadam/
It looked like a good idea to try to reduce the following:
dev->driver && dev->driver->pm && dev->driver->pm->prepare
Into something more succinct. Albeit, given the feedback from yourself and Rafael, I gather that this helper is not really a good addition.
Thank you everyone and sorry for the commotion!
Krzysztof _______________________________________________ greybus-dev mailing list greybus-dev@lists.linaro.org https://lists.linaro.org/mailman/listinfo/greybus-dev
On 5/25/20 8:26 PM, Krzysztof Wilczyński wrote:
Use the new device_to_pm() helper to access Power Management callbacs (struct dev_pm_ops) for a particular device (struct device_driver).
No functional change intended.
Signed-off-by: Krzysztof Wilczyński kw@linux.com
pm support is going to be removed (for s390 in general and) for net/iucv/iucv.c with this net-next patch:
commit 4b32f86bf1673acb16441dd55d7b325609f54897 Author: Julian Wiedmann jwi@linux.ibm.com Date: Tue May 19 21:10:08 2020 +0200
net/iucv: remove pm support
commit 394216275c7d ("s390: remove broken hibernate / power management support") removed support for ARCH_HIBERNATION_POSSIBLE from s390.
So drop the unused pm ops from the s390-only iucv bus driver.
CC: Hendrik Brueckner brueckner@linux.ibm.com Signed-off-by: Julian Wiedmann jwi@linux.ibm.com Signed-off-by: David S. Miller davem@davemloft.net
net/iucv/iucv.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/net/iucv/iucv.c b/net/iucv/iucv.c index 9a2d023842fe..1a3029ab7c1f 100644 --- a/net/iucv/iucv.c +++ b/net/iucv/iucv.c
...
Hi Ursula,
On 20-05-26 09:07:27, Ursula Braun wrote:
On 5/25/20 8:26 PM, Krzysztof Wilczyński wrote:
Use the new device_to_pm() helper to access Power Management callbacs (struct dev_pm_ops) for a particular device (struct device_driver).
No functional change intended.
Signed-off-by: Krzysztof Wilczyński kw@linux.com
pm support is going to be removed (for s390 in general and) for net/iucv/iucv.c with this net-next patch:
[...]
Good to know! Thank you for letting me know. I appreciate that.
Krzysztof
-
Alan Stern -
Alex Elder -
Greg Kroah-Hartman -
Krzysztof Wilczyński -
Pavel Machek -
Rafael J. Wysocki -
Ursula Braun