Re: [PATCH v6 RESEND 1/6] clk: generalize devm_clk_get() a bit

[Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>]

On Mon, 10 May 2021 08:17:19 +0200
Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> wrote:

> Allow to add an exit hook to devm managed clocks. Also use
> clk_get_optional() in devm_clk_get_optional instead of open coding it.
> The generalisation will be used in the next commit to add some more
> devm_clk helpers.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>

If feels marginally odd to register cleanup that we know won't do anything
for the optional case, but it works as far as I can tell and it would be
a little fiddly to special case it.

FWIW

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>

> ---
>  drivers/clk/clk-devres.c | 67 ++++++++++++++++++++++++++++++----------
>  1 file changed, 50 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c
> index be160764911b..91c995815b57 100644
> --- a/drivers/clk/clk-devres.c
> +++ b/drivers/clk/clk-devres.c
> @@ -4,39 +4,72 @@
>  #include <linux/export.h>
>  #include <linux/gfp.h>
>  
> +struct devm_clk_state {
> +	struct clk *clk;
> +	void (*exit)(struct clk *clk);
> +};
> +
>  static void devm_clk_release(struct device *dev, void *res)
>  {
> -	clk_put(*(struct clk **)res);
> +	struct devm_clk_state *state = *(struct devm_clk_state **)res;
> +
> +	if (state->exit)
> +		state->exit(state->clk);
> +
> +	clk_put(state->clk);
>  }
>  
> -struct clk *devm_clk_get(struct device *dev, const char *id)
> +static struct clk *__devm_clk_get(struct device *dev, const char *id,
> +				  struct clk *(*get)(struct device *dev, const char *id),
> +				  int (*init)(struct clk *clk),
> +				  void (*exit)(struct clk *clk))
>  {
> -	struct clk **ptr, *clk;
> +	struct devm_clk_state *state;
> +	struct clk *clk;
> +	int ret;
>  
> -	ptr = devres_alloc(devm_clk_release, sizeof(*ptr), GFP_KERNEL);
> -	if (!ptr)
> +	state = devres_alloc(devm_clk_release, sizeof(*state), GFP_KERNEL);
> +	if (!state)
>  		return ERR_PTR(-ENOMEM);
>  
> -	clk = clk_get(dev, id);
> -	if (!IS_ERR(clk)) {
> -		*ptr = clk;
> -		devres_add(dev, ptr);
> -	} else {
> -		devres_free(ptr);
> +	clk = get(dev, id);
> +	if (IS_ERR(clk)) {
> +		ret = PTR_ERR(clk);
> +		goto err_clk_get;
>  	}
>  
> +	if (init) {
> +		ret = init(clk);
> +		if (ret)
> +			goto err_clk_init;
> +	}
> +
> +	state->clk = clk;
> +	state->exit = exit;
> +
> +	devres_add(dev, state);
> +
>  	return clk;
> +
> +err_clk_init:
> +
> +	clk_put(clk);
> +err_clk_get:
> +
> +	devres_free(state);
> +	return ERR_PTR(ret);
>  }
> -EXPORT_SYMBOL(devm_clk_get);
>  
> -struct clk *devm_clk_get_optional(struct device *dev, const char *id)
> +struct clk *devm_clk_get(struct device *dev, const char *id)
>  {
> -	struct clk *clk = devm_clk_get(dev, id);
> +	return __devm_clk_get(dev, id, clk_get, NULL, NULL);
>  
> -	if (clk == ERR_PTR(-ENOENT))
> -		return NULL;
> +}
> +EXPORT_SYMBOL(devm_clk_get);
>  
> -	return clk;
> +struct clk *devm_clk_get_optional(struct device *dev, const char *id)
> +{
> +	return __devm_clk_get(dev, id, clk_get_optional, NULL, NULL);
>  }
>  EXPORT_SYMBOL(devm_clk_get_optional);
>