Re: [PATCH 1/8] driver core: Add helper for accessing Power Management callbacs

Greg Kroah-Hartman
2020-05-26T06:33:40Z

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