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 <[email protected]>
---
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.
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 works are pwm_get() but the acquired pwm will automatically be released
+ * 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.
+ */
+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);
struct pwm_lookup {
struct list_head list;
--
1.7.11.3
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 <[email protected]>
> ---
> 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 works are pwm_get() but the acquired pwm will automatically be released
"works like"? Also "acquired PWM" or "acquired PWM device".
> + * 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?
Looks good otherwise. Thanks for taking care of it.
Thierry
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 <[email protected]>
>> ---
>> 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.
On Wed, Aug 01, 2012 at 05:17:48PM +0900, Alex Courbot wrote:
> 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:
> >>+/**
> >>+ * 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.
devm_pwm_get() and devm_pwm_put() aren't very likely to change. The
problem is that they are public APIs and people could be running
kerneldoc over the files to have documentation generated and the
parameters would be missing in this case. We really don't have a lot of
these public functions, but those that are should be properly
documented.
Thierry