Re: [PATCH v2 2/7] ASoC: qcom: Add common array to initialize soc based core clocks

Re: [PATCH v2 2/7] ASoC: qcom: Add common array to initialize soc based core clocks

[Srinivas Kandagatla]

On 14/05/2020 17:38, Ajit Pandey wrote:
> AHB-I/X bus clocks inside LPASS coreis not a mandatory clocks for
> SC7180 soc lpass variants, hence make it an optional one.
> 
> Signed-off-by: Ajit Pandey <ajitp@codeaurora.org>
> ---

For some reason patch subject seems messed up!

I see two patches with same subject "[PATCH v2 2/7] ASoC: qcom: Add 
common array to initialize soc based core clocks" and totally different 
content and PATCH v2 3/7 is missing as well!

--srini




>   sound/soc/qcom/lpass-cpu.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c
> index e00a4af..729ca78 100644
> --- a/sound/soc/qcom/lpass-cpu.c
> +++ b/sound/soc/qcom/lpass-cpu.c
> @@ -594,7 +594,7 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
>   		}
>   	}
>   
> -	drvdata->ahbix_clk = devm_clk_get(dev, "ahbix-clk");
> +	drvdata->ahbix_clk = devm_clk_get_optional(dev, "ahbix-clk");
>   	if (IS_ERR(drvdata->ahbix_clk)) {
>   		dev_err(dev, "error getting ahbix-clk: %ld\n",
>   			PTR_ERR(drvdata->ahbix_clk));
> 

Re: [PATCH v2 2/7] ASoC: qcom: Add common array to initialize soc based core clocks

[Srinivas Kandagatla]

On 14/05/2020 17:38, Ajit Pandey wrote:
> AHB-I/X bus clocks inside LPASS coreis not a mandatory clocks for
> SC7180 soc lpass variants, hence make it an optional one.
> 
> Signed-off-by: Ajit Pandey <ajitp@codeaurora.org>
> ---

For some reason patch subject seems messed up!

I see two patches with same subject "[PATCH v2 2/7] ASoC: qcom: Add 
common array to initialize soc based core clocks" and totally different 
content and PATCH v2 3/7 is missing as well!

--srini




>   sound/soc/qcom/lpass-cpu.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c
> index e00a4af..729ca78 100644
> --- a/sound/soc/qcom/lpass-cpu.c
> +++ b/sound/soc/qcom/lpass-cpu.c
> @@ -594,7 +594,7 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
>   		}
>   	}
>   
> -	drvdata->ahbix_clk = devm_clk_get(dev, "ahbix-clk");
> +	drvdata->ahbix_clk = devm_clk_get_optional(dev, "ahbix-clk");
>   	if (IS_ERR(drvdata->ahbix_clk)) {
>   		dev_err(dev, "error getting ahbix-clk: %ld\n",
>   			PTR_ERR(drvdata->ahbix_clk));
> 

Re: [PATCH v2 2/7] ASoC: qcom: Add common array to initialize soc based core clocks

[Ajit Pandey]

On 5/14/2020 10:15 PM, Mark Brown wrote:
> On Thu, May 14, 2020 at 10:08:14PM +0530, Ajit Pandey wrote:
>> AHB-I/X bus clocks inside LPASS coreis not a mandatory clocks for
>> SC7180 soc lpass variants, hence make it an optional one.
> Do we need to improve the checks in the rest of the code so that if the
> clock is missing on a SoC where it is actually mandatory we don't end up
> trying to start without it?  Perhaps it's more trouble than it's worth
> though.
> Agreed.. adding a case check with soc id or name definately make sense 
> to avoid issue with soc where it's  a mandatory one, I'll also look if 
> this "ahbix-clk" can also be moved with other soc-based clocks array ? 
> and can be enabled in soc specific driver.

Re: [PATCH v2 2/7] ASoC: qcom: Add common array to initialize soc based core clocks

[Ajit Pandey]

On 5/14/2020 10:15 PM, Mark Brown wrote:
> On Thu, May 14, 2020 at 10:08:14PM +0530, Ajit Pandey wrote:
>> AHB-I/X bus clocks inside LPASS coreis not a mandatory clocks for
>> SC7180 soc lpass variants, hence make it an optional one.
> Do we need to improve the checks in the rest of the code so that if the
> clock is missing on a SoC where it is actually mandatory we don't end up
> trying to start without it?  Perhaps it's more trouble than it's worth
> though.
> Agreed.. adding a case check with soc id or name definately make sense 
> to avoid issue with soc where it's  a mandatory one, I'll also look if 
> this "ahbix-clk" can also be moved with other soc-based clocks array ? 
> and can be enabled in soc specific driver.

Re: [PATCH v2 2/7] ASoC: qcom: Add common array to initialize soc based core clocks

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 414 bytes --]

On Thu, May 14, 2020 at 10:08:14PM +0530, Ajit Pandey wrote:
> AHB-I/X bus clocks inside LPASS coreis not a mandatory clocks for
> SC7180 soc lpass variants, hence make it an optional one.

Do we need to improve the checks in the rest of the code so that if the
clock is missing on a SoC where it is actually mandatory we don't end up
trying to start without it?  Perhaps it's more trouble than it's worth
though.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v2 2/7] ASoC: qcom: Add common array to initialize soc based core clocks

[Ajit Pandey]

AHB-I/X bus clocks inside LPASS coreis not a mandatory clocks for
SC7180 soc lpass variants, hence make it an optional one.

Signed-off-by: Ajit Pandey <ajitp@codeaurora.org>
---
 sound/soc/qcom/lpass-cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c
index e00a4af..729ca78 100644
--- a/sound/soc/qcom/lpass-cpu.c
+++ b/sound/soc/qcom/lpass-cpu.c
@@ -594,7 +594,7 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
 		}
 	}
 
-	drvdata->ahbix_clk = devm_clk_get(dev, "ahbix-clk");
+	drvdata->ahbix_clk = devm_clk_get_optional(dev, "ahbix-clk");
 	if (IS_ERR(drvdata->ahbix_clk)) {
 		dev_err(dev, "error getting ahbix-clk: %ld\n",
 			PTR_ERR(drvdata->ahbix_clk));
-- 
('The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project')

[PATCH v2 2/7] ASoC: qcom: Add common array to initialize soc based core clocks

[Ajit Pandey]

lpass variants have their own soc specific clocks that needs to be
enable for MI2S audio support. Added a common variable in drvdata to
initialize such clocks using bulk clk api. Such clock names is still
defined in variants specific data and needs to fetched during init.

Signed-off-by: Ajit Pandey <ajitp@codeaurora.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 int apq8016_lpass_exit(struct platform_device *pdev)
 	.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 */
-- 
('The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project')