diff mbox series

[v4,01/12] ASoC: qcom: Add common array to initialize soc based core clocks

Message ID 1595413915-17867-2-git-send-email-rohitkr@codeaurora.org (mailing list archive)
State New, archived
Headers show
Series ASoC: qcom: Add support for SC7180 lpass variant | expand

Commit Message

Rohit Kumar July 22, 2020, 10:31 a.m. UTC
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(-)

Comments

Stephen Boyd Aug. 6, 2020, 12:31 a.m. UTC | #1
Quoting Rohit kumar (2020-07-22 03:31:44)
> 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.

Why not just get all the clks and not even care about the names of them?
Use devm_clk_bulk_get_all() for that, unless some clks need to change
rates?
Rohit Kumar Aug. 6, 2020, 3:59 a.m. UTC | #2
Thanks Stephen for reviewing.

On 8/6/2020 6:01 AM, Stephen Boyd wrote:
> Quoting Rohit kumar (2020-07-22 03:31:44)
>> 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.
> Why not just get all the clks and not even care about the names of them?
> Use devm_clk_bulk_get_all() for that, unless some clks need to change
> rates?

There is ahbix clk which needs clk rate to be set. Please check below 
patch in

the series for reference

[PATCH v5 02/12] ASoC: qcom: lpass-cpu: Move ahbix clk to platform 
specific function

Thanks,

Rohit
Stephen Boyd Aug. 7, 2020, 8:17 p.m. UTC | #3
Quoting Rohit Kumar (2020-08-05 20:59:48)
> Thanks Stephen for reviewing.
> 
> On 8/6/2020 6:01 AM, Stephen Boyd wrote:
> > Quoting Rohit kumar (2020-07-22 03:31:44)
> >> 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.
> > Why not just get all the clks and not even care about the names of them?
> > Use devm_clk_bulk_get_all() for that, unless some clks need to change
> > rates?
> 
> There is ahbix clk which needs clk rate to be set. Please check below 
> patch in
> 
> the series for reference
> 
> [PATCH v5 02/12] ASoC: qcom: lpass-cpu: Move ahbix clk to platform 
> specific function
> 

Alright. I wonder if we could make the API better or the binding better
and always have the rate settable clk first and then
devm_clk_bulk_get_all() could be used along with clk_set_rate() on some 
array element 0 or something. Anyway, don't mind me, I'm just thinking
how to make this simpler.
diff mbox series

Patch

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 */