[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAA8EJpqDrFB_PmcA1uyubuaC5G8n6GqKkmY3sNDOf37AqrO=dw@mail.gmail.com>
Date: Wed, 24 Jan 2024 13:20:30 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Imran Shaik <quic_imrashai@...cinc.com>
Cc: Konrad Dybcio <konrad.dybcio@...aro.org>, Bjorn Andersson <andersson@...nel.org>,
Andy Gross <agross@...nel.org>, Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>, Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>,
Marijn Suijten <marijn.suijten@...ainline.org>, linux-arm-msm@...r.kernel.org,
linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, Johan Hovold <johan+linaro@...nel.org>,
"Bryan O'Donoghue" <bryan.odonoghue@...aro.org>
Subject: Re: [PATCH v6 01/12] clk: qcom: branch: Add a helper for setting the
enable bit
On Tue, 23 Jan 2024 at 11:33, Imran Shaik <quic_imrashai@...cinc.com> wrote:
>
>
>
> On 1/13/2024 8:20 PM, Konrad Dybcio wrote:
> > We hardcode some clocks to be always-on, as they're essential to the
> > functioning of the SoC / some peripherals. Add a helper to do so
> > to make the writes less magic.
> >
> > Reviewed-by: Johan Hovold <johan+linaro@...nel.org>
> > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
> > Signed-off-by: Konrad Dybcio <konrad.dybcio@...aro.org>
> > ---
> > drivers/clk/qcom/clk-branch.h | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/clk/qcom/clk-branch.h b/drivers/clk/qcom/clk-branch.h
> > index 8ffed603c050..0514bc43100b 100644
> > --- a/drivers/clk/qcom/clk-branch.h
> > +++ b/drivers/clk/qcom/clk-branch.h
> > @@ -64,6 +64,7 @@ struct clk_mem_branch {
> > #define CBCR_FORCE_MEM_PERIPH_OFF BIT(12)
> > #define CBCR_WAKEUP GENMASK(11, 8)
> > #define CBCR_SLEEP GENMASK(7, 4)
> > +#define CBCR_CLOCK_ENABLE BIT(0)
> >
> > static inline void qcom_branch_set_force_mem_core(struct regmap *regmap,
> > struct clk_branch clk, bool on)
> > @@ -98,6 +99,12 @@ static inline void qcom_branch_set_sleep(struct regmap *regmap, struct clk_branc
> > FIELD_PREP(CBCR_SLEEP, val));
> > }
> >
> > +static inline void qcom_branch_set_clk_en(struct regmap *regmap, u32 cbcr)
> > +{
> > + regmap_update_bits(regmap, cbcr, CBCR_CLOCK_ENABLE,
> > + CBCR_CLOCK_ENABLE);
> > +}
> > +
>
> Could you please help me understand how this helper function is useful?
> Seems like this is just for reducing parameters compared to
> regmap_update_bits(). But anyhow the same is being done in the existing
> clock controller drivers with a comment which explains the functionality.
So, yes, it replaces the boilerplate code with API, which is good.
>
> Thanks & Regards,
> Imran
>
> > extern const struct clk_ops clk_branch_ops;
> > extern const struct clk_ops clk_branch2_ops;
> > extern const struct clk_ops clk_branch_simple_ops;
> >
>
--
With best wishes
Dmitry
Powered by blists - more mailing lists