| Message ID | 20230717-topic-branch_aon_cleanup-v6-1-46d136a4e8d0@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Unregister critical branch clocks + some RPM | expand |
On Sat, 13 Jan 2024 at 16:51, Konrad Dybcio <konrad.dybcio@linaro.org> 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@kernel.org> > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > --- > drivers/clk/qcom/clk-branch.h | 7 +++++++ > 1 file changed, 7 insertions(+) Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
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@kernel.org> > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.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. 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; >
On Tue, 23 Jan 2024 at 11:33, Imran Shaik <quic_imrashai@quicinc.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@kernel.org> > > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.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; > > >
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); +} + 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;