Re: [PATCH v5 01/12] ASoC: qcom: Add common array to initialize soc based core clocks

[Rohit Kumar <rohitkr@codeaurora.org>]

Hello Mark,

Do you see any concern with patches (1-11).
As of now, there is comment only in patch 12 from Rob which I am
planning to update once other patches are merged. Can you
please review and let me know if anything is missing.

Thanks,
Rohit
On 8/4/2020 1:37 PM, Rohit kumar wrote:
> From: Ajit Pandey <ajitp@codeaurora.org>
>
> LPASS variants have their own soc specific clocks that needs to be
> enabled for MI2S audio support. Added a common variable in drvdata to
> initialize such clocks using bulk clk api. Such clock names is
> defined in variants specific data and needs to fetched during init.
>
> Signed-off-by: Ajit Pandey <ajitp@codeaurora.org>
> Signed-off-by: Rohit kumar <rohitkr@codeaurora.org>
> Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>   sound/soc/qcom/lpass-apq8016.c | 39 +++++++++++++++++++--------------------
>   sound/soc/qcom/lpass.h         | 10 +++++++---
>   2 files changed, 26 insertions(+), 23 deletions(-)
>
> diff --git a/sound/soc/qcom/lpass-apq8016.c b/sound/soc/qcom/lpass-apq8016.c
> index b3610d0..8210e37 100644
> --- a/sound/soc/qcom/lpass-apq8016.c
> +++ b/sound/soc/qcom/lpass-apq8016.c
> @@ -161,32 +161,27 @@ static int apq8016_lpass_free_dma_channel(struct lpass_data *drvdata, int chan)
>   static int apq8016_lpass_init(struct platform_device *pdev)
>   {
>   	struct lpass_data *drvdata = platform_get_drvdata(pdev);
> +	struct lpass_variant *variant = drvdata->variant;
>   	struct device *dev = &pdev->dev;
> -	int ret;
> +	int ret, i;
>   
> -	drvdata->pcnoc_mport_clk = devm_clk_get(dev, "pcnoc-mport-clk");
> -	if (IS_ERR(drvdata->pcnoc_mport_clk)) {
> -		dev_err(dev, "error getting pcnoc-mport-clk: %ld\n",
> -			PTR_ERR(drvdata->pcnoc_mport_clk));
> -		return PTR_ERR(drvdata->pcnoc_mport_clk);
> -	}
>   
> -	ret = clk_prepare_enable(drvdata->pcnoc_mport_clk);
> +	drvdata->clks = devm_kcalloc(dev, variant->num_clks,
> +				     sizeof(*drvdata->clks), GFP_KERNEL);
> +	drvdata->num_clks = variant->num_clks;
> +
> +	for (i = 0; i < drvdata->num_clks; i++)
> +		drvdata->clks[i].id = variant->clk_name[i];
> +
> +	ret = devm_clk_bulk_get(dev, drvdata->num_clks, drvdata->clks);
>   	if (ret) {
> -		dev_err(dev, "Error enabling pcnoc-mport-clk: %d\n", ret);
> +		dev_err(dev, "Failed to get clocks %d\n", ret);
>   		return ret;
>   	}
>   
> -	drvdata->pcnoc_sway_clk = devm_clk_get(dev, "pcnoc-sway-clk");
> -	if (IS_ERR(drvdata->pcnoc_sway_clk)) {
> -		dev_err(dev, "error getting pcnoc-sway-clk: %ld\n",
> -			PTR_ERR(drvdata->pcnoc_sway_clk));
> -		return PTR_ERR(drvdata->pcnoc_sway_clk);
> -	}
> -
> -	ret = clk_prepare_enable(drvdata->pcnoc_sway_clk);
> +	ret = clk_bulk_prepare_enable(drvdata->num_clks, drvdata->clks);
>   	if (ret) {
> -		dev_err(dev, "Error enabling pcnoc_sway_clk: %d\n", ret);
> +		dev_err(dev, "apq8016 clk_enable failed\n");
>   		return ret;
>   	}
>   
> @@ -197,8 +192,7 @@ static int apq8016_lpass_exit(struct platform_device *pdev)
>   {
>   	struct lpass_data *drvdata = platform_get_drvdata(pdev);
>   
> -	clk_disable_unprepare(drvdata->pcnoc_mport_clk);
> -	clk_disable_unprepare(drvdata->pcnoc_sway_clk);
> +	clk_bulk_disable_unprepare(drvdata->num_clks, drvdata->clks);
>   
>   	return 0;
>   }
> @@ -219,6 +213,11 @@ static struct lpass_variant apq8016_data = {
>   	.wrdma_reg_stride	= 0x1000,
>   	.wrdma_channel_start	= 5,
>   	.wrdma_channels		= 2,
> +	.clk_name		= (const char*[]) {
> +				   "pcnoc-mport-clk",
> +				   "pcnoc-sway-clk",
> +				  },
> +	.num_clks		= 2,
>   	.dai_driver		= apq8016_lpass_cpu_dai_driver,
>   	.num_dai		= ARRAY_SIZE(apq8016_lpass_cpu_dai_driver),
>   	.dai_osr_clk_names	= (const char *[]) {
> diff --git a/sound/soc/qcom/lpass.h b/sound/soc/qcom/lpass.h
> index bd19ec5..450020e 100644
> --- a/sound/soc/qcom/lpass.h
> +++ b/sound/soc/qcom/lpass.h
> @@ -51,9 +51,9 @@ struct lpass_data {
>   	/* used it for handling interrupt per dma channel */
>   	struct snd_pcm_substream *substream[LPASS_MAX_DMA_CHANNELS];
>   
> -	/* 8016 specific */
> -	struct clk *pcnoc_mport_clk;
> -	struct clk *pcnoc_sway_clk;
> +	/* SOC specific clock list */
> +	struct clk_bulk_data *clks;
> +	int num_clks;
>   
>   };
>   
> @@ -89,6 +89,10 @@ struct lpass_variant {
>   	int num_dai;
>   	const char * const *dai_osr_clk_names;
>   	const char * const *dai_bit_clk_names;
> +
> +	/* SOC specific clocks configuration */
> +	const char **clk_name;
> +	int num_clks;
>   };
>   
>   /* register the platform driver from the CPU DAI driver */

-- 
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member
of the Code Aurora Forum, hosted by the Linux Foundation.