On Tue 15 Mar 10:50 CDT 2022, Srinivasa Rao Mandadapu wrote:
> Update bulk clock voting to optional voting as ADSP bypass platform doesn't
> need macro and decodec clocks, as these macro and dcodec GDSC switches are
> maintained as power domains and operated from lpass clock drivers.
>
Sorry for missing your reply on my question on the previous version, I
think this sounds reasonable.
> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
> Co-developed-by: Venkata Prasad Potturu <[email protected]>
> Signed-off-by: Venkata Prasad Potturu <[email protected]>
> ---
> drivers/pinctrl/qcom/pinctrl-lpass-lpi.c | 12 +++++++++---
> drivers/pinctrl/qcom/pinctrl-lpass-lpi.h | 1 +
> drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c | 1 +
> 3 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
> index 0216ca1..3fc473a 100644
> --- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
> +++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
> @@ -401,9 +401,15 @@ int lpi_pinctrl_probe(struct platform_device *pdev)
> return dev_err_probe(dev, PTR_ERR(pctrl->slew_base),
> "Slew resource not provided\n");
>
> - ret = devm_clk_bulk_get(dev, MAX_LPI_NUM_CLKS, pctrl->clks);
> - if (ret)
> - return dev_err_probe(dev, ret, "Can't get clocks\n");
> + if (data->is_clk_optional) {
> + ret = devm_clk_bulk_get_optional(dev, MAX_LPI_NUM_CLKS, pctrl->clks);
> + if (ret)
> + return dev_err_probe(dev, ret, "Can't get clocks\n");
Dug into the clk_bulk_get() functions, and __clk_bulk_get() will print
an error telling you which clock it failed to get. So I don't think your
more generic error here doesn't add any value.
Just return ret;
> + } else {
> + ret = devm_clk_bulk_get(dev, MAX_LPI_NUM_CLKS, pctrl->clks);
> + if (ret)
> + return dev_err_probe(dev, ret, "Can't get clocks\n");
> + }
Depending on your taste, you could do:
if (data->is_clk_optional)
ret = devm_clk_bulk_get_optional();
else
ret = devm_clk_bulk_get();
if (ret)
return ret;
>
> ret = clk_bulk_prepare_enable(MAX_LPI_NUM_CLKS, pctrl->clks);
> if (ret)
> diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
> index afbac2a..3bcede6 100644
> --- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
> +++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
> @@ -77,6 +77,7 @@ struct lpi_pinctrl_variant_data {
> int ngroups;
> const struct lpi_function *functions;
> int nfunctions;
> + int is_clk_optional;
bool here please.
> };
>
> int lpi_pinctrl_probe(struct platform_device *pdev);
> diff --git a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
> index d67ff25..304d8a2 100644
> --- a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
> +++ b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
> @@ -142,6 +142,7 @@ static const struct lpi_pinctrl_variant_data sc7280_lpi_data = {
> .ngroups = ARRAY_SIZE(sc7280_groups),
> .functions = sc7280_functions,
> .nfunctions = ARRAY_SIZE(sc7280_functions),
> + .is_clk_optional = 1,
true
Regards,
Bjorn
> };
>
> static const struct of_device_id lpi_pinctrl_of_match[] = {
> --
> 2.7.4
>
On 3/15/2022 10:27 PM, Bjorn Andersson wrote:
Thanks for your time Bjorn!!!
> On Tue 15 Mar 10:50 CDT 2022, Srinivasa Rao Mandadapu wrote:
>
>> Update bulk clock voting to optional voting as ADSP bypass platform doesn't
>> need macro and decodec clocks, as these macro and dcodec GDSC switches are
>> maintained as power domains and operated from lpass clock drivers.
>>
> Sorry for missing your reply on my question on the previous version, I
> think this sounds reasonable.
Okay. Thanks!!!
>
>> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
>> Co-developed-by: Venkata Prasad Potturu <[email protected]>
>> Signed-off-by: Venkata Prasad Potturu <[email protected]>
>> ---
>> drivers/pinctrl/qcom/pinctrl-lpass-lpi.c | 12 +++++++++---
>> drivers/pinctrl/qcom/pinctrl-lpass-lpi.h | 1 +
>> drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c | 1 +
>> 3 files changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
>> index 0216ca1..3fc473a 100644
>> --- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
>> +++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
>> @@ -401,9 +401,15 @@ int lpi_pinctrl_probe(struct platform_device *pdev)
>> return dev_err_probe(dev, PTR_ERR(pctrl->slew_base),
>> "Slew resource not provided\n");
>>
>> - ret = devm_clk_bulk_get(dev, MAX_LPI_NUM_CLKS, pctrl->clks);
>> - if (ret)
>> - return dev_err_probe(dev, ret, "Can't get clocks\n");
>> + if (data->is_clk_optional) {
>> + ret = devm_clk_bulk_get_optional(dev, MAX_LPI_NUM_CLKS, pctrl->clks);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "Can't get clocks\n");
> Dug into the clk_bulk_get() functions, and __clk_bulk_get() will print
> an error telling you which clock it failed to get. So I don't think your
> more generic error here doesn't add any value.
>
> Just return ret;
Okay will change accordingly.
>
>> + } else {
>> + ret = devm_clk_bulk_get(dev, MAX_LPI_NUM_CLKS, pctrl->clks);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "Can't get clocks\n");
>> + }
> Depending on your taste, you could do:
>
> if (data->is_clk_optional)
> ret = devm_clk_bulk_get_optional();
> else
> ret = devm_clk_bulk_get();
>
> if (ret)
> return ret;
Okay. Will change accordingly.
>>
>> ret = clk_bulk_prepare_enable(MAX_LPI_NUM_CLKS, pctrl->clks);
>> if (ret)
>> diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
>> index afbac2a..3bcede6 100644
>> --- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
>> +++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
>> @@ -77,6 +77,7 @@ struct lpi_pinctrl_variant_data {
>> int ngroups;
>> const struct lpi_function *functions;
>> int nfunctions;
>> + int is_clk_optional;
> bool here please.
Okay. Will update.
>
>> };
>>
>> int lpi_pinctrl_probe(struct platform_device *pdev);
>> diff --git a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
>> index d67ff25..304d8a2 100644
>> --- a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
>> +++ b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c
>> @@ -142,6 +142,7 @@ static const struct lpi_pinctrl_variant_data sc7280_lpi_data = {
>> .ngroups = ARRAY_SIZE(sc7280_groups),
>> .functions = sc7280_functions,
>> .nfunctions = ARRAY_SIZE(sc7280_functions),
>> + .is_clk_optional = 1,
> true
Okay.
>
> Regards,
> Bjorn
>
>> };
>>
>> static const struct of_device_id lpi_pinctrl_of_match[] = {
>> --
>> 2.7.4
>>