Re: [PATCH v3] clk: add more managed APIs

[Guenter Roeck]

On Tue, Feb 14, 2017 at 11:55:20AM -0800, Dmitry Torokhov wrote:
> On Tue, Feb 14, 2017 at 11:44 AM, Stephen Boyd  wrote:
> > On 02/06, Dmitry Torokhov wrote:
> >> On Mon, Jan 30, 2017 at 04:57:13PM -0800, Dmitry Torokhov wrote:
> >> > When converting a driver to managed resources it is desirable to be able 
> >> > to
> >> > manage all resources in the same fashion. This change allows managing 
> >> > clock
> >> > prepared and enabled state in the same way we manage many other 
> >> > resources.
> >> >
> >> > This adds the following managed APIs:
> >> >
> >> > - devm_clk_prepare()/devm_clk_unprepare();
> >> > - devm_clk_prepare_enable()/devm_clk_disable_unprepare().
> >> >
> >> > Reviewed-by: Guenter Roeck 
> >> > Signed-off-by: Dmitry Torokhov 
> >>
> >> It would be awesome if we could get it into 4.11...
> >
> > I'd prefer we didn't do this. Instead, make clk_put() drop any
> > prepare or enables that were done via that clk pointer. Mike
> > started to do this before[1], but we have some code that assumes
> > it can do:
> >
> > clk = clk_get(...)
> > clk_prepare_enable(clk)
> > clk_put(clk)
> >
> > and have the clk stay on. Those would need to be changed.
> 
> That means we'd need to audit entire code base ;(
> 
> >
> > We would also need Russell's approval to update the clk_put()
> > documentation to describe this change in behavior.
> >
> > [1] 
> > http://lkml.kernel.org/r/1438974570-20812-1-git-send-email-mturque...@baylibre.com
> >
> 
> Note that devm* APIs do not preclude from changing clk_put() behavior
> down the road and it is extremely easy to go and
> s/devm_clk_prepare_enable/clk_prepare_enable/ once cleanup is
> automatic.
> 
> Having devm now will help make driver code better (because right now
> we either need to add wrappers so devm_add_action_or_reset() can be
> used, or continue mixing devm* and goto cleanups, which are often
> wrong).
> 

Absolutely agree. The combination of clk_prepare_enable() in probe and
clk_disable_unprepare() in remove is used all over the place. Sure,
we _can_ use devm_add_action_or_reset() for all those cases, and I do
have a coccinelle script doing just that. While I'll have to accept
going forward with that approach if needed, I have to admit that
I completely fail to miss the point why that would be a good idea.

Thanks,
Guenter

Re: [PATCH v3] clk: add more managed APIs

[Guenter Roeck]

On Tue, Feb 14, 2017 at 11:55:20AM -0800, Dmitry Torokhov wrote:
> On Tue, Feb 14, 2017 at 11:44 AM, Stephen Boyd  wrote:
> > On 02/06, Dmitry Torokhov wrote:
> >> On Mon, Jan 30, 2017 at 04:57:13PM -0800, Dmitry Torokhov wrote:
> >> > When converting a driver to managed resources it is desirable to be able 
> >> > to
> >> > manage all resources in the same fashion. This change allows managing 
> >> > clock
> >> > prepared and enabled state in the same way we manage many other 
> >> > resources.
> >> >
> >> > This adds the following managed APIs:
> >> >
> >> > - devm_clk_prepare()/devm_clk_unprepare();
> >> > - devm_clk_prepare_enable()/devm_clk_disable_unprepare().
> >> >
> >> > Reviewed-by: Guenter Roeck 
> >> > Signed-off-by: Dmitry Torokhov 
> >>
> >> It would be awesome if we could get it into 4.11...
> >
> > I'd prefer we didn't do this. Instead, make clk_put() drop any
> > prepare or enables that were done via that clk pointer. Mike
> > started to do this before[1], but we have some code that assumes
> > it can do:
> >
> > clk = clk_get(...)
> > clk_prepare_enable(clk)
> > clk_put(clk)
> >
> > and have the clk stay on. Those would need to be changed.
> 
> That means we'd need to audit entire code base ;(
> 
> >
> > We would also need Russell's approval to update the clk_put()
> > documentation to describe this change in behavior.
> >
> > [1] 
> > http://lkml.kernel.org/r/1438974570-20812-1-git-send-email-mturque...@baylibre.com
> >
> 
> Note that devm* APIs do not preclude from changing clk_put() behavior
> down the road and it is extremely easy to go and
> s/devm_clk_prepare_enable/clk_prepare_enable/ once cleanup is
> automatic.
> 
> Having devm now will help make driver code better (because right now
> we either need to add wrappers so devm_add_action_or_reset() can be
> used, or continue mixing devm* and goto cleanups, which are often
> wrong).
> 

Absolutely agree. The combination of clk_prepare_enable() in probe and
clk_disable_unprepare() in remove is used all over the place. Sure,
we _can_ use devm_add_action_or_reset() for all those cases, and I do
have a coccinelle script doing just that. While I'll have to accept
going forward with that approach if needed, I have to admit that
I completely fail to miss the point why that would be a good idea.

Thanks,
Guenter

Re: [PATCH v3] clk: add more managed APIs

[Russell King - ARM Linux]

On Tue, Feb 14, 2017 at 11:44:08AM -0800, Stephen Boyd wrote:
> I'd prefer we didn't do this. Instead, make clk_put() drop any
> prepare or enables that were done via that clk pointer. Mike
> started to do this before[1], but we have some code that assumes
> it can do:
> 
>   clk = clk_get(...)
>   clk_prepare_enable(clk)
>   clk_put(clk)
> 
> and have the clk stay on. Those would need to be changed.

Yes, I've seen from time to time code that plays this game over the
years.  However, from my simple grepping, it seems that we only have
a small number of cases:

arch/mips/alchemy/devboards/db1200.c:   clk_prepare_enable(c);
arch/mips/alchemy/devboards/db1200.c-   clk_put(c);

arch/mips/alchemy/devboards/db1300.c:   clk_prepare_enable(c);
arch/mips/alchemy/devboards/db1300.c-   clk_put(c);

arch/mips/alchemy/devboards/db1550.c:   clk_prepare_enable(c);
arch/mips/alchemy/devboards/db1550.c-   clk_put(c);

drivers/base/power/clock_ops.c: clk_prepare_enable(clk);
drivers/base/power/clock_ops.c- clk_put(clk);

drivers/mtd/nand/orion_nand.c:  clk_prepare_enable(clk);
drivers/mtd/nand/orion_nand.c-  clk_put(clk);

I've always hated that - and it goes against the API:

 * Note: drivers must ensure that all clk_enable calls made on this
 * clock source are balanced by clk_disable calls prior to calling
 * this function.

(That comment should have been updated when clk_prepare() & clk_unprepare()
was added to include balancing those.)

So really, all the cases above are buggy.  However, the statement in the
API doesn't give permission for what you're suggesting!

The suggestion requires that we also cast in stone that every "struct clk"
which is handed out from clk_get() becomes unique _everywhere_, because
that's the only way to really track the prepare/enable state on a per-
clk_get() basis, so we can properly clean up at clk_put().

However, I think we still have some non-CCF clock API implementations
around, which hand out shared "struct clk"s.  Changing this requirement
will impact those, since they would need to be updated before the change
could be made.

So, although it would be a nice change to make (which would rule out the
abuse) I don't think it's one that could be practically made at this
stage without (a) fixing all instances like those quoted above and (b)
converting all non-CCF implementations to either CCF or making them
hand out unique struct clk's.

(I do have patches which converts sa11x0 to CCF... which would be one
less non-CCF implementation.)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH v3] clk: add more managed APIs

[Russell King - ARM Linux]

On Tue, Feb 14, 2017 at 11:44:08AM -0800, Stephen Boyd wrote:
> I'd prefer we didn't do this. Instead, make clk_put() drop any
> prepare or enables that were done via that clk pointer. Mike
> started to do this before[1], but we have some code that assumes
> it can do:
> 
>   clk = clk_get(...)
>   clk_prepare_enable(clk)
>   clk_put(clk)
> 
> and have the clk stay on. Those would need to be changed.

Yes, I've seen from time to time code that plays this game over the
years.  However, from my simple grepping, it seems that we only have
a small number of cases:

arch/mips/alchemy/devboards/db1200.c:   clk_prepare_enable(c);
arch/mips/alchemy/devboards/db1200.c-   clk_put(c);

arch/mips/alchemy/devboards/db1300.c:   clk_prepare_enable(c);
arch/mips/alchemy/devboards/db1300.c-   clk_put(c);

arch/mips/alchemy/devboards/db1550.c:   clk_prepare_enable(c);
arch/mips/alchemy/devboards/db1550.c-   clk_put(c);

drivers/base/power/clock_ops.c: clk_prepare_enable(clk);
drivers/base/power/clock_ops.c- clk_put(clk);

drivers/mtd/nand/orion_nand.c:  clk_prepare_enable(clk);
drivers/mtd/nand/orion_nand.c-  clk_put(clk);

I've always hated that - and it goes against the API:

 * Note: drivers must ensure that all clk_enable calls made on this
 * clock source are balanced by clk_disable calls prior to calling
 * this function.

(That comment should have been updated when clk_prepare() & clk_unprepare()
was added to include balancing those.)

So really, all the cases above are buggy.  However, the statement in the
API doesn't give permission for what you're suggesting!

The suggestion requires that we also cast in stone that every "struct clk"
which is handed out from clk_get() becomes unique _everywhere_, because
that's the only way to really track the prepare/enable state on a per-
clk_get() basis, so we can properly clean up at clk_put().

However, I think we still have some non-CCF clock API implementations
around, which hand out shared "struct clk"s.  Changing this requirement
will impact those, since they would need to be updated before the change
could be made.

So, although it would be a nice change to make (which would rule out the
abuse) I don't think it's one that could be practically made at this
stage without (a) fixing all instances like those quoted above and (b)
converting all non-CCF implementations to either CCF or making them
hand out unique struct clk's.

(I do have patches which converts sa11x0 to CCF... which would be one
less non-CCF implementation.)

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH v3] clk: add more managed APIs

[Dmitry Torokhov]

On Tue, Feb 14, 2017 at 11:44 AM, Stephen Boyd  wrote:
> On 02/06, Dmitry Torokhov wrote:
>> On Mon, Jan 30, 2017 at 04:57:13PM -0800, Dmitry Torokhov wrote:
>> > When converting a driver to managed resources it is desirable to be able to
>> > manage all resources in the same fashion. This change allows managing clock
>> > prepared and enabled state in the same way we manage many other resources.
>> >
>> > This adds the following managed APIs:
>> >
>> > - devm_clk_prepare()/devm_clk_unprepare();
>> > - devm_clk_prepare_enable()/devm_clk_disable_unprepare().
>> >
>> > Reviewed-by: Guenter Roeck 
>> > Signed-off-by: Dmitry Torokhov 
>>
>> It would be awesome if we could get it into 4.11...
>
> I'd prefer we didn't do this. Instead, make clk_put() drop any
> prepare or enables that were done via that clk pointer. Mike
> started to do this before[1], but we have some code that assumes
> it can do:
>
> clk = clk_get(...)
> clk_prepare_enable(clk)
> clk_put(clk)
>
> and have the clk stay on. Those would need to be changed.

That means we'd need to audit entire code base ;(

>
> We would also need Russell's approval to update the clk_put()
> documentation to describe this change in behavior.
>
> [1] 
> http://lkml.kernel.org/r/1438974570-20812-1-git-send-email-mturque...@baylibre.com
>

Note that devm* APIs do not preclude from changing clk_put() behavior
down the road and it is extremely easy to go and
s/devm_clk_prepare_enable/clk_prepare_enable/ once cleanup is
automatic.

Having devm now will help make driver code better (because right now
we either need to add wrappers so devm_add_action_or_reset() can be
used, or continue mixing devm* and goto cleanups, which are often
wrong).

Thanks.

-- 
Dmitry

Re: [PATCH v3] clk: add more managed APIs

[Dmitry Torokhov]

On Tue, Feb 14, 2017 at 11:44 AM, Stephen Boyd  wrote:
> On 02/06, Dmitry Torokhov wrote:
>> On Mon, Jan 30, 2017 at 04:57:13PM -0800, Dmitry Torokhov wrote:
>> > When converting a driver to managed resources it is desirable to be able to
>> > manage all resources in the same fashion. This change allows managing clock
>> > prepared and enabled state in the same way we manage many other resources.
>> >
>> > This adds the following managed APIs:
>> >
>> > - devm_clk_prepare()/devm_clk_unprepare();
>> > - devm_clk_prepare_enable()/devm_clk_disable_unprepare().
>> >
>> > Reviewed-by: Guenter Roeck 
>> > Signed-off-by: Dmitry Torokhov 
>>
>> It would be awesome if we could get it into 4.11...
>
> I'd prefer we didn't do this. Instead, make clk_put() drop any
> prepare or enables that were done via that clk pointer. Mike
> started to do this before[1], but we have some code that assumes
> it can do:
>
> clk = clk_get(...)
> clk_prepare_enable(clk)
> clk_put(clk)
>
> and have the clk stay on. Those would need to be changed.

That means we'd need to audit entire code base ;(

>
> We would also need Russell's approval to update the clk_put()
> documentation to describe this change in behavior.
>
> [1] 
> http://lkml.kernel.org/r/1438974570-20812-1-git-send-email-mturque...@baylibre.com
>

Note that devm* APIs do not preclude from changing clk_put() behavior
down the road and it is extremely easy to go and
s/devm_clk_prepare_enable/clk_prepare_enable/ once cleanup is
automatic.

Having devm now will help make driver code better (because right now
we either need to add wrappers so devm_add_action_or_reset() can be
used, or continue mixing devm* and goto cleanups, which are often
wrong).

Thanks.

-- 
Dmitry

Re: [PATCH v3] clk: add more managed APIs

[Stephen Boyd]

On 02/06, Dmitry Torokhov wrote:
> On Mon, Jan 30, 2017 at 04:57:13PM -0800, Dmitry Torokhov wrote:
> > When converting a driver to managed resources it is desirable to be able to
> > manage all resources in the same fashion. This change allows managing clock
> > prepared and enabled state in the same way we manage many other resources.
> > 
> > This adds the following managed APIs:
> > 
> > - devm_clk_prepare()/devm_clk_unprepare();
> > - devm_clk_prepare_enable()/devm_clk_disable_unprepare().
> > 
> > Reviewed-by: Guenter Roeck 
> > Signed-off-by: Dmitry Torokhov 
> 
> It would be awesome if we could get it into 4.11...

I'd prefer we didn't do this. Instead, make clk_put() drop any
prepare or enables that were done via that clk pointer. Mike
started to do this before[1], but we have some code that assumes
it can do:

clk = clk_get(...)
clk_prepare_enable(clk)
clk_put(clk)

and have the clk stay on. Those would need to be changed.

We would also need Russell's approval to update the clk_put()
documentation to describe this change in behavior.

[1] 
http://lkml.kernel.org/r/1438974570-20812-1-git-send-email-mturque...@baylibre.com

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3] clk: add more managed APIs

[Stephen Boyd]

On 02/06, Dmitry Torokhov wrote:
> On Mon, Jan 30, 2017 at 04:57:13PM -0800, Dmitry Torokhov wrote:
> > When converting a driver to managed resources it is desirable to be able to
> > manage all resources in the same fashion. This change allows managing clock
> > prepared and enabled state in the same way we manage many other resources.
> > 
> > This adds the following managed APIs:
> > 
> > - devm_clk_prepare()/devm_clk_unprepare();
> > - devm_clk_prepare_enable()/devm_clk_disable_unprepare().
> > 
> > Reviewed-by: Guenter Roeck 
> > Signed-off-by: Dmitry Torokhov 
> 
> It would be awesome if we could get it into 4.11...

I'd prefer we didn't do this. Instead, make clk_put() drop any
prepare or enables that were done via that clk pointer. Mike
started to do this before[1], but we have some code that assumes
it can do:

clk = clk_get(...)
clk_prepare_enable(clk)
clk_put(clk)

and have the clk stay on. Those would need to be changed.

We would also need Russell's approval to update the clk_put()
documentation to describe this change in behavior.

[1] 
http://lkml.kernel.org/r/1438974570-20812-1-git-send-email-mturque...@baylibre.com

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3] clk: add more managed APIs

[Dmitry Torokhov]

On Mon, Jan 30, 2017 at 04:57:13PM -0800, Dmitry Torokhov wrote:
> When converting a driver to managed resources it is desirable to be able to
> manage all resources in the same fashion. This change allows managing clock
> prepared and enabled state in the same way we manage many other resources.
> 
> This adds the following managed APIs:
> 
> - devm_clk_prepare()/devm_clk_unprepare();
> - devm_clk_prepare_enable()/devm_clk_disable_unprepare().
> 
> Reviewed-by: Guenter Roeck 
> Signed-off-by: Dmitry Torokhov 

It would be awesome if we could get it into 4.11...

> ---
> 
> v3: adjusted commit message, added Guenter's reviewed-by
> v2: dropped devm_clk_enable() and devm_clk_disable()
> 
>  drivers/clk/clk-devres.c |   98 
> +++---
>  include/linux/clk.h  |   68 
>  2 files changed, 134 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c
> index 3a218c3a06ae..2ff94ffe11d3 100644
> --- a/drivers/clk/clk-devres.c
> +++ b/drivers/clk/clk-devres.c
> @@ -9,30 +9,20 @@
>  #include 
>  #include 
>  
> -static void devm_clk_release(struct device *dev, void *res)
> +static int devm_clk_create_devres(struct device *dev, struct clk *clk,
> +   void (*release)(struct device *, void *))
>  {
> - clk_put(*(struct clk **)res);
> -}
> + struct clk **ptr;
>  
> -struct clk *devm_clk_get(struct device *dev, const char *id)
> -{
> - struct clk **ptr, *clk;
> -
> - ptr = devres_alloc(devm_clk_release, sizeof(*ptr), GFP_KERNEL);
> + ptr = devres_alloc(release, sizeof(*ptr), GFP_KERNEL);
>   if (!ptr)
> - return ERR_PTR(-ENOMEM);
> + return -ENOMEM;
>  
> - clk = clk_get(dev, id);
> - if (!IS_ERR(clk)) {
> - *ptr = clk;
> - devres_add(dev, ptr);
> - } else {
> - devres_free(ptr);
> - }
> + *ptr = clk;
> + devres_add(dev, ptr);
>  
> - return clk;
> + return 0;
>  }
> -EXPORT_SYMBOL(devm_clk_get);
>  
>  static int devm_clk_match(struct device *dev, void *res, void *data)
>  {
> @@ -44,31 +34,75 @@ static int devm_clk_match(struct device *dev, void *res, 
> void *data)
>   return *c == data;
>  }
>  
> -void devm_clk_put(struct device *dev, struct clk *clk)
> +#define DEFINE_DEVM_CLK_DESTROY_OP(destroy_op)   
> \
> +static void devm_##destroy_op##_release(struct device *dev, void *res)   
> \
> +{\
> + destroy_op(*(struct clk **)res);\
> +}\
> + \
> +void devm_##destroy_op(struct device *dev, struct clk *clk)  \
> +{\
> + WARN_ON(devres_release(dev, devm_##destroy_op##_release,\
> + devm_clk_match, clk));  \
> +}\
> +EXPORT_SYMBOL(devm_##destroy_op)
> +
> +#define DEFINE_DEVM_CLK_OP(create_op, destroy_op)\
> +DEFINE_DEVM_CLK_DESTROY_OP(destroy_op);  
> \
> +int devm_##create_op(struct device *dev, struct clk *clk)\
> +{\
> + int error;  \
> + \
> + error = create_op(clk); \
> + if (error)  \
> + return error;   \
> + \
> + error = devm_clk_create_devres(dev, clk,\
> + devm_##destroy_op##_release);   \
> + if (error) {\
> + destroy_op(clk);\
> + return error;   \
> + }   \
> + \
> + return 0;   \
> +}\
> +EXPORT_SYMBOL(devm_##create_op)
> +
> +DEFINE_DEVM_CLK_DESTROY_OP(clk_put);
> +DEFINE_DEVM_CLK_OP(clk_prepare, clk_unprepare);
> +DEFINE_DEVM_CLK_OP(clk_prepare_enable, clk_disable_unprepare);
> +
> +struct clk *devm_clk_get(struct device *dev, const char *id)
>  {
> - int ret;
> + struct clk *clk;
> +  

Re: [PATCH v3] clk: add more managed APIs

[Dmitry Torokhov]

On Mon, Jan 30, 2017 at 04:57:13PM -0800, Dmitry Torokhov wrote:
> When converting a driver to managed resources it is desirable to be able to
> manage all resources in the same fashion. This change allows managing clock
> prepared and enabled state in the same way we manage many other resources.
> 
> This adds the following managed APIs:
> 
> - devm_clk_prepare()/devm_clk_unprepare();
> - devm_clk_prepare_enable()/devm_clk_disable_unprepare().
> 
> Reviewed-by: Guenter Roeck 
> Signed-off-by: Dmitry Torokhov 

It would be awesome if we could get it into 4.11...

> ---
> 
> v3: adjusted commit message, added Guenter's reviewed-by
> v2: dropped devm_clk_enable() and devm_clk_disable()
> 
>  drivers/clk/clk-devres.c |   98 
> +++---
>  include/linux/clk.h  |   68 
>  2 files changed, 134 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c
> index 3a218c3a06ae..2ff94ffe11d3 100644
> --- a/drivers/clk/clk-devres.c
> +++ b/drivers/clk/clk-devres.c
> @@ -9,30 +9,20 @@
>  #include 
>  #include 
>  
> -static void devm_clk_release(struct device *dev, void *res)
> +static int devm_clk_create_devres(struct device *dev, struct clk *clk,
> +   void (*release)(struct device *, void *))
>  {
> - clk_put(*(struct clk **)res);
> -}
> + struct clk **ptr;
>  
> -struct clk *devm_clk_get(struct device *dev, const char *id)
> -{
> - struct clk **ptr, *clk;
> -
> - ptr = devres_alloc(devm_clk_release, sizeof(*ptr), GFP_KERNEL);
> + ptr = devres_alloc(release, sizeof(*ptr), GFP_KERNEL);
>   if (!ptr)
> - return ERR_PTR(-ENOMEM);
> + return -ENOMEM;
>  
> - clk = clk_get(dev, id);
> - if (!IS_ERR(clk)) {
> - *ptr = clk;
> - devres_add(dev, ptr);
> - } else {
> - devres_free(ptr);
> - }
> + *ptr = clk;
> + devres_add(dev, ptr);
>  
> - return clk;
> + return 0;
>  }
> -EXPORT_SYMBOL(devm_clk_get);
>  
>  static int devm_clk_match(struct device *dev, void *res, void *data)
>  {
> @@ -44,31 +34,75 @@ static int devm_clk_match(struct device *dev, void *res, 
> void *data)
>   return *c == data;
>  }
>  
> -void devm_clk_put(struct device *dev, struct clk *clk)
> +#define DEFINE_DEVM_CLK_DESTROY_OP(destroy_op)   
> \
> +static void devm_##destroy_op##_release(struct device *dev, void *res)   
> \
> +{\
> + destroy_op(*(struct clk **)res);\
> +}\
> + \
> +void devm_##destroy_op(struct device *dev, struct clk *clk)  \
> +{\
> + WARN_ON(devres_release(dev, devm_##destroy_op##_release,\
> + devm_clk_match, clk));  \
> +}\
> +EXPORT_SYMBOL(devm_##destroy_op)
> +
> +#define DEFINE_DEVM_CLK_OP(create_op, destroy_op)\
> +DEFINE_DEVM_CLK_DESTROY_OP(destroy_op);  
> \
> +int devm_##create_op(struct device *dev, struct clk *clk)\
> +{\
> + int error;  \
> + \
> + error = create_op(clk); \
> + if (error)  \
> + return error;   \
> + \
> + error = devm_clk_create_devres(dev, clk,\
> + devm_##destroy_op##_release);   \
> + if (error) {\
> + destroy_op(clk);\
> + return error;   \
> + }   \
> + \
> + return 0;   \
> +}\
> +EXPORT_SYMBOL(devm_##create_op)
> +
> +DEFINE_DEVM_CLK_DESTROY_OP(clk_put);
> +DEFINE_DEVM_CLK_OP(clk_prepare, clk_unprepare);
> +DEFINE_DEVM_CLK_OP(clk_prepare_enable, clk_disable_unprepare);
> +
> +struct clk *devm_clk_get(struct device *dev, const char *id)
>  {
> - int ret;
> + struct clk *clk;
> + int error;
>  
> - ret =