Messages in this thread | | | Date | Wed, 1 Aug 2012 17:17:48 +0900 | From | Alex Courbot <> | 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@nvidia.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.
| |