| 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
| ||
|
Date: Wed, 1 Aug 2012 17:17:48 +0900
From: Alex Courbot <acourbot@...dia.com>
To: Thierry Reding <thierry.reding@...onic-design.de>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] pwm: add devm_pwm_get() and devm_pwm_put()
On Wed 01 Aug 2012 05:04:53 PM JST, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Wed, Aug 01, 2012 at 04:37:09PM +0900, Alexandre Courbot wrote:
>> Add resource managed variants of pwm_get() and pwm_put() for
>> convenience. Code is largely inspired by the equivalent devm functions
>> of the regulator framework.
>>
>> Signed-off-by: Alexandre Courbot <acourbot@...dia.com>
>> ---
>> Documentation/driver-model/devres.txt | 4 +++
>> Documentation/pwm.txt | 5 +--
>> drivers/pwm/core.c | 57 +++++++++++++++++++++++++++++++++++
>> include/linux/pwm.h | 3 ++
>> 4 files changed, 67 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
>> index 950856b..43cff70 100644
>> --- a/Documentation/driver-model/devres.txt
>> +++ b/Documentation/driver-model/devres.txt
>> @@ -284,3 +284,7 @@ CLOCK
>> PINCTRL
>> devm_pinctrl_get()
>> devm_pinctrl_put()
>> +
>> +PWM
>> + devm_pwm_get()
>> + devm_pwm_put()
>> diff --git a/Documentation/pwm.txt b/Documentation/pwm.txt
>> index 554290e..4719c12 100644
>> --- a/Documentation/pwm.txt
>> +++ b/Documentation/pwm.txt
>> @@ -35,8 +35,9 @@ Using PWMs
>> Legacy users can request a PWM device using pwm_request() and free it
>> after usage with pwm_free().
>>
>> -New users should use the pwm_get() function and pass to it the consumer
>> -device or a consumer name. pwm_put() is used to free the PWM device.
>> +New users should use the pwm_get() or devm_pwm_get() function and pass to it the
>> +consumer device or a consumer name. pwm_put() or devm_pwm_put() is used to free
>> +the PWM device.
>
> I think I'd prefer the original text with an additional line mentioning
> the managed variants.
>
>>
>> After being requested a PWM has to be configured using:
>>
>> diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
>> index ecb7690..5a5e19b 100644
>> --- a/drivers/pwm/core.c
>> +++ b/drivers/pwm/core.c
>> @@ -624,6 +624,63 @@ out:
>> }
>> EXPORT_SYMBOL_GPL(pwm_put);
>>
>> +static void devm_pwm_release(struct device *dev, void *res)
>> +{
>> + pwm_put(*(struct pwm_device **)res);
>> +}
>> +
>> +/**
>> + * devm_pwm_get() - Resource managed pwm_get()
>
> This is missing parameter descriptions here.
This is because the documentation for this function just refers to
pwm_get(), and the parameters are the same. I think it's better to avoid
duplicating documentation as much as possible as this makes two
maintenance points instead of one.
>
>> + *
>> + * This works are pwm_get() but the acquired pwm will automatically be released
>
> "works like"? Also "acquired PWM" or "acquired PWM device".
Ooops, yes. I really should proof-read what I write. Twice.
>
>> + * on driver detach. See pwm_get() for more details.
>> + */
>> +struct pwm_device *devm_pwm_get(struct device *dev, const char *consumer)
>> +{
>> + struct pwm_device **ptr, *pwm;
>> +
>> + ptr = devres_alloc(devm_pwm_release, sizeof(**ptr), GFP_KERNEL);
>> + if (!ptr)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + pwm = pwm_get(dev, consumer);
>> + if (!IS_ERR(pwm)) {
>> + *ptr = pwm;
>> + devres_add(dev, ptr);
>> + } else {
>> + devres_free(ptr);
>> + }
>> +
>> + return pwm;
>> +}
>> +EXPORT_SYMBOL_GPL(devm_pwm_get);
>> +
>> +static int devm_pwm_match(struct device *dev, void *res, void *data)
>> +{
>> + struct pwm_device **p = res;
>> +
>> + if (WARN_ON(!p || !*p))
>> + return 0;
>> +
>> + return *p == data;
>> +}
>> +
>> +/**
>> + * devm_pwm_put() - Resource managed pwm_put()
>> + *
>> + * Releases a pwm previously allocated using devm_pwm_get. Calling this function
>> + * is usually not needed as the devm-allocated pwm will automatically be
>> + * released on driver detach.
>> + */
>
> Same comments as for devm_pwm_get().
>
>> +void devm_pwm_put(struct device *dev, struct pwm_device *pwm)
>> +{
>> + int ret;
>> +
>> + ret = devres_release(dev, devm_pwm_release, devm_pwm_match, pwm);
>> + WARN_ON(ret);
>> +}
>> +EXPORT_SYMBOL_GPL(devm_pwm_put);
>> +
>> #ifdef CONFIG_DEBUG_FS
>> static void pwm_dbg_show(struct pwm_chip *chip, struct seq_file *s)
>> {
>> diff --git a/include/linux/pwm.h b/include/linux/pwm.h
>> index 21d076c..af9c39a 100644
>> --- a/include/linux/pwm.h
>> +++ b/include/linux/pwm.h
>> @@ -123,7 +123,10 @@ struct pwm_device *pwm_request_from_chip(struct pwm_chip *chip,
>> const char *label);
>>
>> struct pwm_device *pwm_get(struct device *dev, const char *consumer);
>> +struct pwm_device *devm_pwm_get(struct device *dev, const char *consumer);
>> +
>> void pwm_put(struct pwm_device *pwm);
>> +void devm_pwm_put(struct device *dev, struct pwm_device *pwm);
>
> Can the managed variants be grouped together, please?
Sure.
Thanks,
Alex.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists