| Message ID | 1595413915-17867-2-git-send-email-rohitkr@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | ASoC: qcom: Add support for SC7180 lpass variant | expand |
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?
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
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 --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 */