| Message ID | 20210914025554.5686-2-shawn.guo@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] clk: qcom: smd-rpm: Add rate hooks for clk_smd_rpm_branch_ops | expand |
On Tue, Sep 14, 2021 at 07:55:21PM -0700, Bjorn Andersson wrote: > On Mon 13 Sep 19:55 PDT 2021, Shawn Guo wrote: > > > 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. > > > > Most platforms has a crystal oscillator ticking at 38.4MHz feeding the > PMIC (represented by the rpmcc and its "xo" parent) and out comes the > bi_tcxo with a fixed 19.2MHz rate. Yeah, but all those platforms are running clk-rpmh driver, I think. > Is there a problem with the way sdm660_bi_tcxo is defined in this > regard? There is no problem if xo clock is 19.2MHz, but for platforms with 38.4MHz xo, bi_tcxo will be seen as 38.4MHz in clock tree, while we expect it to be 19.2MHz. Shawn
On Wed, Sep 15, 2021 at 10:23:11AM -0700, Bjorn Andersson wrote: > On Wed 15 Sep 08:05 PDT 2021, Shawn Guo wrote: > > > 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@linaro.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. > > > > This looks good to me. > > And the confirmed that the xo_board in sdm630.dtsi (and hence SDM660) is > wrong, it should be 38.4MHz as well. Hmm, I see CAF kernel has 19.2MHz for SDM630/660 xo_board clock. Or am I looking at the wrong place? Shawn > Unfortunately adding the appropriate divider to the sdm660 bcxo would > break existing .dtsi (but we can probably convince the community that it > would be ok, if we do it now).
diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c index 66d7807ee38e..2380e45b6247 100644 --- a/drivers/clk/qcom/clk-smd-rpm.c +++ b/drivers/clk/qcom/clk-smd-rpm.c @@ -416,6 +416,9 @@ 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, + .set_rate = clk_smd_rpm_set_rate, + .round_rate = clk_smd_rpm_round_rate, + .recalc_rate = clk_smd_rpm_recalc_rate, }; DEFINE_CLK_SMD_RPM(msm8916, pcnoc_clk, pcnoc_a_clk, QCOM_SMD_RPM_BUS_CLK, 0);
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. Signed-off-by: Shawn Guo <shawn.guo@linaro.org> --- drivers/clk/qcom/clk-smd-rpm.c | 3 +++ 1 file changed, 3 insertions(+)