linux-kernel - Re: [PATCH v6 01/12] clk: qcom: branch: Add a helper for setting the enable bit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

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