On Tue, 23 Jan 2024 at 11:33, Imran Shaik <quic_imrashai@xxxxxxxxxxx> 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@xxxxxxxxxx>
> > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx>
> > Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
> > ---
> > 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
