Messages in this thread | | | Subject | Re: [RFT PATCH] clk: ls1c: Fix PLL rate calculation | From | Sean Anderson <> | Date | Wed, 17 Aug 2022 23:47:11 -0400 |
| |
Hi Kelvin,
On 8/17/22 11:36 PM, Kelvin Cheung wrote: > Sean, Du, > I saw you are discussing the PLL rate calculation issue. > My question is whether the upstream kernel works on your ls1c300? > For me, it never works, even the earliest version which LS1C support was merged. > After the kernel is loaded by PMON, there is no console output at all. > I also confirm this issue with Yang. > BTW, my board is 1C300B. > Are your board is different from me? Or your bootloader?
Unfortunately, I do not have an ls1c300 to test with. This is why I marked the patch as RFT when I submitted it.
--Sean
> > Sean Anderson <seanga2@gmail.com> 于2022年4月19日周二 13:11写道: >> >> While reviewing Dhu's patch adding ls1c300 clock support to U-Boot [1], I >> noticed the following calculation, which is copied from >> drivers/clk/loongson1/clk-loongson1c.c: >> >> ulong ls1c300_pll_get_rate(struct clk *clk) >> { >> unsigned int mult; >> long long parent_rate; >> void *base; >> unsigned int val; >> >> parent_rate = clk_get_parent_rate(clk); >> base = (void *)clk->data; >> >> val = readl(base + START_FREQ); >> mult = FIELD_GET(FRAC_N, val) + FIELD_GET(M_PLL, val); >> return (mult * parent_rate) / 4; >> } >> >> I would like to examine the use of M_PLL and FRAC_N to calculate the multiplier >> for the PLL. The datasheet has the following to say: >> >> START_FREQ 位 缺省值 描述 >> ========== ===== =========== ==================================== >> FRAC_N 23:16 0 PLL 倍频系数的小数部分 >> >> 由 PLL 倍频系数的整数部分 >> M_PLL 15:8 NAND_D[3:0] (理论可以达到 255,建议不要超过 100) >> 配置 >> >> which according to google translate means >> >> START_FREQ Bits Default Description >> ========== ===== ============= ================================================ >> FRAC_N 23:16 0 Fractional part of the PLL multiplication factor >> >> Depends on Integer part of PLL multiplication factor >> M_PLL 15:8 NAND_D[3:0] (Theoretically it can reach 255, [but] it is >> configuration recommended not to exceed 100) >> >> So just based on this description, I would expect that the formula to be >> something like >> >> rate = parent * (255 * M_PLL + FRAC_N) / 255 / 4 >> >> However, the datasheet also gives the following formula: >> >> rate = parent * (M_PLL + FRAC_N) / 4 >> >> which is what the Linux driver has implemented. I find this very unusual. >> First, the datasheet specifically says that these fields are the integer and >> fractional parts of the multiplier. Second, I think such a construct does not >> easily map to traditional PLL building blocks. Implementing this formula in >> hardware would likely require an adder, just to then set the threshold of a >> clock divider. >> >> I think it is much more likely that the first formula is correct. The author of >> the datasheet may think of a multiplier of (say) 3.14 as >> >> M_PLL = 3 >> FRAC_N = 0.14 >> >> which together sum to the correct multiplier, even though the actual value >> stored in FRAC_N would be 36. >> >> I suspect that this has slipped by unnoticed because when FRAC_N is 0, there is >> no difference in the formulae. The following patch is untested, but I suspect >> it will fix this issue. I would appreciate if anyone with access to the >> hardware could measure the output of the PLL (or one of its derived clocks) and >> determine the correct formula. >> >> [1] https://lore.kernel.org/u-boot/20220418204519.19991-1-dhu@hodcarrier.org/T/#u >> >> Fixes: b4626a7f4892 ("CLK: Add Loongson1C clock support") >> Signed-off-by: Sean Anderson <seanga2@gmail.com> >> --- >> >> drivers/clk/loongson1/clk-loongson1c.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/clk/loongson1/clk-loongson1c.c b/drivers/clk/loongson1/clk-loongson1c.c >> index 703f87622cf5..2b98a116c1ea 100644 >> --- a/drivers/clk/loongson1/clk-loongson1c.c >> +++ b/drivers/clk/loongson1/clk-loongson1c.c >> @@ -21,9 +21,9 @@ static unsigned long ls1x_pll_recalc_rate(struct clk_hw *hw, >> u32 pll, rate; >> >> pll = __raw_readl(LS1X_CLK_PLL_FREQ); >> - rate = ((pll >> 8) & 0xff) + ((pll >> 16) & 0xff); >> + rate = (pll & 0xff00) + ((pll >> 16) & 0xff); >> rate *= OSC; >> - rate >>= 2; >> + rate >>= 10; >> >> return rate; >> } >> -- >> 2.35.1 >> > >
| |