MIME-Version: 1.0
References: <20230303092859.22094-1-quic_tdas@quicinc.com> <20230303092859.22094-2-quic_tdas@quicinc.com>
In-Reply-To: <20230303092859.22094-2-quic_tdas@quicinc.com>
From:   Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Date:   Fri, 3 Mar 2023 12:44:14 +0200
Message-ID: <CAA8EJpq5xBF=Wt-1_hGR-7qZHREcALurmR4ucmMmZaC-R_7Ttg@mail.gmail.com>
Subject: Re: [PATCH 1/2] clk: qcom: common: Handle invalid index error
To:     Taniya Das <quic_tdas@quicinc.com>
Cc:     Andy Gross <agross@kernel.org>,
        Michael Turquette <mturquette@baylibre.com>,
        Stephen Boyd <sboyd@kernel.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        linux-arm-msm@vger.kernel.org, linux-clk@vger.kernel.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, 3 Mar 2023 at 11:30, Taniya Das <quic_tdas@quicinc.com> wrote:
>
> Introduce start_index to handle invalid index error
> seen when there are two clock descriptors assigned
> to the same clock controller.

Please provide details of the exact case that you are trying to solve
(this might go to the cover letter). I think the commit message is
slightly misleading here. Are you trying to add error messages or to
prevent them from showing up?

I'm asking because error messages do not seem to correspond to patch
2. You add start_index to make the kernel warn for the clock indices
less than LPASS_AUDIO_CC_CDIV_RX_MCLK_DIV_CLK_SRC = 4, while quoted
messages show indices 5,6,7.

Nit: please don't overwrap the commit message, the recommended line
width is about 72-77 chars.

>
> [ 3.600604] qcom_cc_clk_hw_get: invalid index 5
> [ 3.625251] qcom_cc_clk_hw_get: invalid index 6
> [ 3.648190] qcom_cc_clk_hw_get: invalid index 7

>
> Fixes: 120c15528390 ("clk: qcom: Migrate to clk_hw based registration and OF APIs")
> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> ---
>  drivers/clk/qcom/common.c | 12 ++++++++----
>  drivers/clk/qcom/common.h |  1 +
>  2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
> index 75f09e6e057e..0e80535b61f2 100644
> --- a/drivers/clk/qcom/common.c
> +++ b/drivers/clk/qcom/common.c
> @@ -21,6 +21,7 @@ struct qcom_cc {
>         struct qcom_reset_controller reset;
>         struct clk_regmap **rclks;
>         size_t num_rclks;
> +       u32 rclks_start_index;
>  };
>
>  const
> @@ -226,12 +227,13 @@ static struct clk_hw *qcom_cc_clk_hw_get(struct of_phandle_args *clkspec,
>         struct qcom_cc *cc = data;
>         unsigned int idx = clkspec->args[0];
>
> -       if (idx >= cc->num_rclks) {
> +       if (idx >= cc->rclks_start_index && idx < cc->num_rclks)
> +               return cc->rclks[idx] ? &cc->rclks[idx]->hw : NULL;
> +       else if (idx < cc->rclks_start_index && idx >= cc->num_rclks)
>                 pr_err("%s: invalid index %u\n", __func__, idx);
> -               return ERR_PTR(-EINVAL);
> -       }
>
> -       return cc->rclks[idx] ? &cc->rclks[idx]->hw : NULL;
> +       return ERR_PTR(-EINVAL);
> +
>  }
>
>  int qcom_cc_really_probe(struct platform_device *pdev,
> @@ -281,6 +283,8 @@ int qcom_cc_really_probe(struct platform_device *pdev,
>
>         cc->rclks = rclks;
>         cc->num_rclks = num_clks;
> +       if (desc->start_index)
> +               cc->rclks_start_index = desc->start_index;
>
>         qcom_cc_drop_protected(dev, cc);
>
> diff --git a/drivers/clk/qcom/common.h b/drivers/clk/qcom/common.h
> index 9c8f7b798d9f..924f36af55b3 100644
> --- a/drivers/clk/qcom/common.h
> +++ b/drivers/clk/qcom/common.h
> @@ -23,6 +23,7 @@ struct qcom_cc_desc {
>         const struct regmap_config *config;
>         struct clk_regmap **clks;
>         size_t num_clks;
> +       u32 start_index;
>         const struct qcom_reset_map *resets;
>         size_t num_resets;
>         struct gdsc **gdscs;
> --
> 2.17.1
>


--
With best wishes


Dmitry