[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20210915150526.GE25255@dragon>
Date: Wed, 15 Sep 2021 23:05:27 +0800
From: Shawn Guo <shawn.guo@...aro.org>
To: Stephen Boyd <sboyd@...nel.org>
Cc: Rob Herring <robh+dt@...nel.org>,
Bjorn Andersson <bjorn.andersson@...aro.org>,
Loic Poulain <loic.poulain@...aro.org>,
devicetree@...r.kernel.org, linux-arm-msm@...r.kernel.org,
linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] clk: qcom: smd-rpm: Add rate hooks for
clk_smd_rpm_branch_ops
On Tue, Sep 14, 2021 at 02:56:28PM -0700, Stephen Boyd wrote:
> Quoting Shawn Guo (2021-09-13 19:55:52)
> > On QCM2290 platform, the clock xo_board runs at 38400000, while the
> > child clock bi_tcxo needs to run at 19200000. That said,
> > clk_smd_rpm_branch_ops needs the capability of setting rate. Add rate
> > hooks into clk_smd_rpm_branch_ops to make it possible.
>
> This doesn't sound right. The branch is a simple on/off. If xo_board is
> 38.4MHz, then there is an internal divider in the SoC that makes bi_tcxo
> (i.e. the root of the entire clk tree) be 19.2MHz. We don't model the
> divider, I guess because it isn't very important to. Instead, we tack on
> a divider field and implement recalc_rate op. See clk-rpmh.c in the qcom
> directory for this.
Thanks for the comment, Stephen! To be honest, I copied the
implementation from vendor kernel, and wasn't really sure if it's
correct or the best.
So here is what I get based on your suggestion. Let's me know if
it's how you wanted it to be. Thanks!
Shawn
----8<---------
>From 23dda79fee412738f046b89bdd20ef95a24c35cc Mon Sep 17 00:00:00 2001
From: Shawn Guo <shawn.guo@...aro.org>
Date: Wed, 15 Sep 2021 22:00:32 +0800
Subject: [PATCH] clk: qcom: smd-rpm: Add a divider field for branch clock
Similar to clk-rpmh, clk-smd-rpm has the same need to handle the case
where an internal divider is there between xo_board and bi_tcxo. The
change is made in the a back compatible way below.
- Add div field to struct clk_smd_rpm, and have
__DEFINE_CLK_SMD_RPM_BRANCH() assign it.
- Update all existing __DEFINE_CLK_SMD_RPM_BRANCH() wrappers to pass a
zero div.
- Add DEFINE_CLK_SMD_RPM_BRANCH_DIV() which doesn't take rate argument
but div.
- Update clk_smd_rpm_recalc_rate() to handle div and add it as
.recalc_rate of clk_smd_rpm_branch_ops.
Signed-off-by: Shawn Guo <shawn.guo@...aro.org>
---
drivers/clk/qcom/clk-smd-rpm.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
index 66d7807ee38e..66ef0d3795fd 100644
--- a/drivers/clk/qcom/clk-smd-rpm.c
+++ b/drivers/clk/qcom/clk-smd-rpm.c
@@ -66,13 +66,14 @@
}
#define __DEFINE_CLK_SMD_RPM_BRANCH(_platform, _name, _active, type, r_id, \
- stat_id, r, key) \
+ stat_id, r, key, _div) \
static struct clk_smd_rpm _platform##_##_active; \
static struct clk_smd_rpm _platform##_##_name = { \
.rpm_res_type = (type), \
.rpm_clk_id = (r_id), \
.rpm_status_id = (stat_id), \
.rpm_key = (key), \
+ .div = (_div), \
.branch = true, \
.peer = &_platform##_##_active, \
.rate = (r), \
@@ -92,6 +93,7 @@
.rpm_status_id = (stat_id), \
.active_only = true, \
.rpm_key = (key), \
+ .div = (_div), \
.branch = true, \
.peer = &_platform##_##_name, \
.rate = (r), \
@@ -112,7 +114,12 @@
#define DEFINE_CLK_SMD_RPM_BRANCH(_platform, _name, _active, type, r_id, r) \
__DEFINE_CLK_SMD_RPM_BRANCH(_platform, _name, _active, type, \
- r_id, 0, r, QCOM_RPM_SMD_KEY_ENABLE)
+ r_id, 0, r, QCOM_RPM_SMD_KEY_ENABLE, 0)
+
+#define DEFINE_CLK_SMD_RPM_BRANCH_DIV(_platform, _name, _active, type, r_id, \
+ _div) \
+ __DEFINE_CLK_SMD_RPM_BRANCH(_platform, _name, _active, type, \
+ r_id, 0, 0, QCOM_RPM_SMD_KEY_ENABLE, _div)
#define DEFINE_CLK_SMD_RPM_QDSS(_platform, _name, _active, type, r_id) \
__DEFINE_CLK_SMD_RPM(_platform, _name, _active, type, r_id, \
@@ -121,12 +128,12 @@
#define DEFINE_CLK_SMD_RPM_XO_BUFFER(_platform, _name, _active, r_id) \
__DEFINE_CLK_SMD_RPM_BRANCH(_platform, _name, _active, \
QCOM_SMD_RPM_CLK_BUF_A, r_id, 0, 1000, \
- QCOM_RPM_KEY_SOFTWARE_ENABLE)
+ QCOM_RPM_KEY_SOFTWARE_ENABLE, 0)
#define DEFINE_CLK_SMD_RPM_XO_BUFFER_PINCTRL(_platform, _name, _active, r_id) \
__DEFINE_CLK_SMD_RPM_BRANCH(_platform, _name, _active, \
QCOM_SMD_RPM_CLK_BUF_A, r_id, 0, 1000, \
- QCOM_RPM_KEY_PIN_CTRL_CLK_BUFFER_ENABLE_KEY)
+ QCOM_RPM_KEY_PIN_CTRL_CLK_BUFFER_ENABLE_KEY, 0)
#define to_clk_smd_rpm(_hw) container_of(_hw, struct clk_smd_rpm, hw)
@@ -140,6 +147,7 @@ struct clk_smd_rpm {
bool branch;
struct clk_smd_rpm *peer;
struct clk_hw hw;
+ u8 div;
unsigned long rate;
struct qcom_smd_rpm *rpm;
};
@@ -370,10 +378,10 @@ static unsigned long clk_smd_rpm_recalc_rate(struct clk_hw *hw,
/*
* RPM handles rate rounding and we don't have a way to
- * know what the rate will be, so just return whatever
- * rate was set.
+ * know what the rate will be, so just return divided parent
+ * rate or whatever rate was set.
*/
- return r->rate;
+ return r->div ? parent_rate / r->div : r->rate;
}
static int clk_smd_rpm_enable_scaling(struct qcom_smd_rpm *rpm)
@@ -416,6 +424,7 @@ static const struct clk_ops clk_smd_rpm_ops = {
static const struct clk_ops clk_smd_rpm_branch_ops = {
.prepare = clk_smd_rpm_prepare,
.unprepare = clk_smd_rpm_unprepare,
+ .recalc_rate = clk_smd_rpm_recalc_rate,
};
DEFINE_CLK_SMD_RPM(msm8916, pcnoc_clk, pcnoc_a_clk, QCOM_SMD_RPM_BUS_CLK, 0);
--
2.17.1
Powered by blists - more mailing lists