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

Open Source and information security mailing list archives

Message-ID: <41c42552-0f4f-df6a-d587-5c62333aa6a8@linaro.org>
Date:   Tue, 26 May 2020 06:53:22 -0500
From:   Alex Elder <elder@...aro.org>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Krzysztof Wilczyński <kw@...ux.com>
Cc:     Ulf Hansson <ulf.hansson@...aro.org>, linux-pci@...r.kernel.org,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Pavel Machek <pavel@....cz>, linux-s390@...r.kernel.org,
        linux-scsi@...r.kernel.org, Kevin Hilman <khilman@...nel.org>,
        Julian Wiedmann <jwi@...ux.ibm.com>,
        linux-acpi@...r.kernel.org, Jakub Kicinski <kuba@...nel.org>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Len Brown <lenb@...nel.org>, linux-pm@...r.kernel.org,
        "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        Ursula Braun <ubraun@...ux.ibm.com>,
        Johan Hovold <johan@...nel.org>, greybus-dev@...ts.linaro.org,
        John Stultz <john.stultz@...aro.org>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Felipe Balbi <balbi@...nel.org>, Alex Elder <elder@...nel.org>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        netdev@...r.kernel.org, linux-usb@...r.kernel.org,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Karsten Graul <kgraul@...ux.ibm.com>,
        "David S. Miller" <davem@...emloft.net>
Subject: Re: [greybus-dev] [PATCH 1/8] driver core: Add helper for accessing
 Power Management callbacs

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@...ux.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@...ts.linaro.org
> https://lists.linaro.org/mailman/listinfo/greybus-dev
> 

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives