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@xxxxxxxxxx> > > 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).
