Re: [PATCH 1/3] soc: qcom: smem_state: Add devm_qcom_smem_state_get()

[Bjorn Andersson <bjorn.andersson@linaro.org>]

On Fri 18 Jun 06:15 CDT 2021, Stephan Gerhold wrote:

> It is easy to forget to call qcom_smem_state_put() after
> a qcom_smem_state_get(). Introduce a devm_qcom_smem_state_get()
> helper function that automates this so that qcom_smem_state_put()
> is automatically called when a device is removed.
> 

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
> I use this in my WIP bam-dmux driver, so it might simplify coordination
> later if we can somehow get this in for 5.14 (if it isn't too late yet).
> 
> Similar code can be found in clk-devres.c for example:
> https://elixir.bootlin.com/linux/v5.13-rc6/source/drivers/clk/clk-devres.c#L7
> ---
>  drivers/soc/qcom/smem_state.c       | 36 +++++++++++++++++++++++++++++
>  include/linux/soc/qcom/smem_state.h |  7 ++++++
>  2 files changed, 43 insertions(+)
> 
> diff --git a/drivers/soc/qcom/smem_state.c b/drivers/soc/qcom/smem_state.c
> index d2b558438deb..31faf4aa868e 100644
> --- a/drivers/soc/qcom/smem_state.c
> +++ b/drivers/soc/qcom/smem_state.c
> @@ -151,6 +151,42 @@ void qcom_smem_state_put(struct qcom_smem_state *state)
>  }
>  EXPORT_SYMBOL_GPL(qcom_smem_state_put);
>  
> +static void devm_qcom_smem_state_release(struct device *dev, void *res)
> +{
> +	qcom_smem_state_put(*(struct qcom_smem_state **)res);
> +}
> +
> +/**
> + * devm_qcom_smem_state_get() - acquire handle to a devres managed state
> + * @dev:	client device pointer
> + * @con_id:	name of the state to lookup
> + * @bit:	flags from the state reference, indicating which bit's affected
> + *
> + * Returns handle to the state, or ERR_PTR(). qcom_smem_state_put() is called
> + * automatically when @dev is removed.
> + */
> +struct qcom_smem_state *devm_qcom_smem_state_get(struct device *dev,
> +						 const char *con_id,
> +						 unsigned *bit)
> +{
> +	struct qcom_smem_state **ptr, *state;
> +
> +	ptr = devres_alloc(devm_qcom_smem_state_release, sizeof(*ptr), GFP_KERNEL);
> +	if (!ptr)
> +		return ERR_PTR(-ENOMEM);
> +
> +	state = qcom_smem_state_get(dev, con_id, bit);
> +	if (!IS_ERR(state)) {
> +		*ptr = state;
> +		devres_add(dev, ptr);
> +	} else {
> +		devres_free(ptr);
> +	}
> +
> +	return state;
> +}
> +EXPORT_SYMBOL_GPL(devm_qcom_smem_state_get);
> +
>  /**
>   * qcom_smem_state_register() - register a new state
>   * @of_node:	of_node used for matching client lookups
> diff --git a/include/linux/soc/qcom/smem_state.h b/include/linux/soc/qcom/smem_state.h
> index 63ad8cddad14..17c56a50302f 100644
> --- a/include/linux/soc/qcom/smem_state.h
> +++ b/include/linux/soc/qcom/smem_state.h
> @@ -14,6 +14,7 @@ struct qcom_smem_state_ops {
>  #ifdef CONFIG_QCOM_SMEM_STATE
>  
>  struct qcom_smem_state *qcom_smem_state_get(struct device *dev, const char *con_id, unsigned *bit);
> +struct qcom_smem_state *devm_qcom_smem_state_get(struct device *dev, const char *con_id, unsigned *bit);
>  void qcom_smem_state_put(struct qcom_smem_state *);
>  
>  int qcom_smem_state_update_bits(struct qcom_smem_state *state, u32 mask, u32 value);
> @@ -29,6 +30,12 @@ static inline struct qcom_smem_state *qcom_smem_state_get(struct device *dev,
>  	return ERR_PTR(-EINVAL);
>  }
>  
> +static inline struct qcom_smem_state *devm_qcom_smem_state_get(struct device *dev,
> +	const char *con_id, unsigned *bit)
> +{
> +	return ERR_PTR(-EINVAL);
> +}
> +
>  static inline void qcom_smem_state_put(struct qcom_smem_state *state)
>  {
>  }
> -- 
> 2.32.0
>