Re: [PATCH 4/7] ASoC: codecs: lpass-va-macro: Change bulk voting to individual clock voting

[Srinivasa Rao Mandadapu <srivasam@xxxxxxxxxxxxxx>]

On 9/21/2021 2:20 PM, Srinivas Kandagatla wrote:
> On 21/09/2021 09:14, Srinivasa Rao Mandadapu wrote:
> > On 9/20/2021 6:55 PM, Srinivas Kandagatla wrote:
> > >
> > Thanks for Your time Srini!!!
> > > On 20/09/2021 08:35, Srinivasa Rao Mandadapu wrote:
> > > > Change bulk clock frequency voting to individual voting.
> > > Can you please explain why do we need to move out using clk bulk apis?
> > >
> > > Am not seeing any thing obvious behavior changing as part of this 
> > > patch, more details please..
> >
> > In ADSP bypass use case, few clocks like macro and decode, are 
> > optional. So is the main reason for move out.
>
>
> Have you tried using clk_bulk_get_optional()
Tried with above API. It's working fine. Do you suggest to use this 
optional API?
> --srini
> > And sometimes we are seeing bulk voting failed in Kodiak setup.
> >
> > > > Fixes: 908e6b1df26e (ASoC: codecs: lpass-va-macro: Add support to 
> > > > VA Macro)
> > >
> > > Why this has Fixes tag? Are we fixing any bug with this patch?
> > Okay. As such we are not fixing any bug. Will remove this fixes tag 
> > on your suggestion.
> > > > Signed-off-by: Venkata Prasad Potturu <potturu@xxxxxxxxxxxxxx>
> > > > Signed-off-by: Srinivasa Rao Mandadapu <srivasam@xxxxxxxxxxxxxx>
> > > > ---
> > > >   sound/soc/codecs/lpass-va-macro.c | 46 
> > > > ++++++++++++++++++++++++---------------
> > > >   1 file changed, 28 insertions(+), 18 deletions(-)
> > > >
> > > > diff --git a/sound/soc/codecs/lpass-va-macro.c 
> > > > b/sound/soc/codecs/lpass-va-macro.c
> > > > index d312a14..0ea39ae 100644
> > > > --- a/sound/soc/codecs/lpass-va-macro.c
> > > > +++ b/sound/soc/codecs/lpass-va-macro.c
> > > > @@ -193,7 +193,10 @@ struct va_macro {
> > > >         int dec_mode[VA_MACRO_NUM_DECIMATORS];
> > > >       struct regmap *regmap;
> > > > -    struct clk_bulk_data clks[VA_NUM_CLKS_MAX];
> > > > +    struct clk *mclk;
> > > > +    struct clk *macro;
> > > > +    struct clk *dcodec;
> > > > +
> > > >       struct clk_hw hw;
> > > >         s32 dmic_0_1_clk_cnt;
> > > > @@ -1321,7 +1324,7 @@ static const struct clk_ops fsgen_gate_ops = {
> > > >     static int va_macro_register_fsgen_output(struct va_macro *va)
> > > >   {
> > > > -    struct clk *parent = va->clks[2].clk;
> > > > +    struct clk *parent = va->mclk;
> > > >       struct device *dev = va->dev;
> > > >       struct device_node *np = dev->of_node;
> > > >       const char *parent_clk_name;
> > > > @@ -1404,15 +1407,18 @@ static int va_macro_probe(struct 
> > > > platform_device *pdev)
> > > >           return -ENOMEM;
> > > >         va->dev = dev;
> > > > -    va->clks[0].id = "macro";
> > > > -    va->clks[1].id = "dcodec";
> > > > -    va->clks[2].id = "mclk";
> > > >   -    ret = devm_clk_bulk_get(dev, VA_NUM_CLKS_MAX, va->clks);
> > > > -    if (ret) {
> > > > -        dev_err(dev, "Error getting VA Clocks (%d)\n", ret);
> > > > -        return ret;
> > > > -    }
> > > > +    va->macro = devm_clk_get_optional(dev, "macro");
> > > > +    if (IS_ERR(va->macro))
> > > > +        return PTR_ERR(va->macro);
> > > > +
> > > > +    va->dcodec = devm_clk_get_optional(dev, "dcodec");
> > > > +    if (IS_ERR(va->dcodec))
> > > > +        return PTR_ERR(va->dcodec);
> > > > +
> > > > +    va->mclk = devm_clk_get(dev, "mclk");
> > > > +    if (IS_ERR(va->mclk))
> > > > +        return PTR_ERR(va->mclk);
> > > >         ret = of_property_read_u32(dev->of_node, 
> > > > "qcom,dmic-sample-rate",
> > > >                      &sample_rate);
> > > > @@ -1426,10 +1432,11 @@ static int va_macro_probe(struct 
> > > > platform_device *pdev)
> > > >       }
> > > >         /* mclk rate */
> > > > -    clk_set_rate(va->clks[1].clk, VA_MACRO_MCLK_FREQ);
> > > > -    ret = clk_bulk_prepare_enable(VA_NUM_CLKS_MAX, va->clks);
> > > > -    if (ret)
> > > > -        return ret;
> > > > +    clk_set_rate(va->mclk, VA_MACRO_MCLK_FREQ);
> > > > +
> > > > +    clk_prepare_enable(va->mclk);
> > > > +    clk_prepare_enable(va->macro);
> > > > +    clk_prepare_enable(va->dcodec);
> > > >         base = devm_platform_ioremap_resource(pdev, 0);
> > > >       if (IS_ERR(base)) {
> > > > @@ -1457,8 +1464,9 @@ static int va_macro_probe(struct 
> > > > platform_device *pdev)
> > > >       return ret;
> > > >     err:
> > > > -    clk_bulk_disable_unprepare(VA_NUM_CLKS_MAX, va->clks);
> > > > -
> > > > +    clk_disable_unprepare(va->mclk);
> > > > +    clk_disable_unprepare(va->macro);
> > > > +    clk_disable_unprepare(va->dcodec);
> > > >       return ret;
> > > >   }
> > > >   @@ -1466,8 +1474,10 @@ static int va_macro_remove(struct 
> > > > platform_device *pdev)
> > > >   {
> > > >       struct va_macro *va = dev_get_drvdata(&pdev->dev);
> > > >   -    clk_bulk_disable_unprepare(VA_NUM_CLKS_MAX, va->clks);
> > > > -
> > > > +    of_clk_del_provider(pdev->dev.of_node);
> > >
> > > fsgen clk is registered using devm_* variant of clk apis, so why do 
> > > we need this here?
> > Okay. Will remove it and post new patch.
> > > --srini
> > > > + clk_disable_unprepare(va->mclk);
> > > > +    clk_disable_unprepare(va->macro);
> > > > +    clk_disable_unprepare(va->dcodec);
> > > >       return 0;
> > > >   }
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.