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 <quic_srivasam@xxxxxxxxxxx>
> Co-developed-by: Venkata Prasad Potturu <quic_potturu@xxxxxxxxxxx>
> Signed-off-by: Venkata Prasad Potturu <quic_potturu@xxxxxxxxxxx>
> ---
> 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
>
