| Message ID | 20240906-fix_clk-v1-0-2977ef0d72e7@amlogic.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | clk: meson: Fix an issue with inaccurate hifi_pll frequency | expand |
On Fri 06 Sep 2024 at 13:52, Chuan Liu via B4 Relay <devnull+chuan.liu.amlogic.com@kernel.org> wrote: > Some PLLs with fractional multipliers have fractional denominators that > are fixed to "100000" instead of the previous "(1 << pll->frac.width)". > > The hifi_pll for both C3 and S4 supports a fractional multiplier and has > a fixed fractional denominator of "100000". > > Here are the results of the C3-based command tests (already defined > CLOCK_ALLOW_WRITE_DEBUGFS): > * echo 491520000 > /sys/kernel/debug/clk/hifi_pll/clk_rate > * cat /sys/kernel/debug/clk/hifi_pll/clk_rate > 491520000 > * echo 1 > /sys/kernel/debug/clk/hifi_pll/clk_prepare_enable > * cat /sys/kernel/debug/meson-clk-msr/clks/hifi_pll_clk > 491515625 +/-15625Hz > * devmem 0xfe008100 32 > 0xD00304A3 > * devmem 0xfe008104 32 > 0x00014820 > > Based on the register information read above, it can be obtained: > m = 0xA3 = 0d163; > n = 0x1 = 0d1 > frac = 0x14820 = 0d84000 > od = 0x3 = 0d3 > > hifi_pll calculates the output frequency: > calc_rate = xtal_rate / n * (m + (frac / frac_max)) >> od; > calc_rate = 24000000 / 1 * (163 + (84000 / 100000)) >> 3; > calc_rate = 491520000 > > clk_rate, msr_rate, and calc_rate all match. Thanks for the detailed description. Is there a possibility this applies to the g12/sm1 as well ? HiFi PLL has had trouble on these SoCs since support has been added. It sometimes takes a long time to report a lock. So long we consider it a failure. There was no such issue on AXG. If you check DT, it is the reason why AXG use the HiFi PLL for the sound card and G12/SM1 does not. > > The test and calculation results of S4 are consistent with those of C3, > which will not be repeated here. > > Signed-off-by: Chuan Liu <chuan.liu@amlogic.com> > --- > Chuan Liu (4): > clk: meson: Support PLL with fixed fractional denominators > clk: meson: c3: pll: hifi_pll frequency is not accurate > clk: meson: s4: pll: hifi_pll support fractional multiplier > clk: meson: s4: pll: hifi_pll frequency is not accurate > > drivers/clk/meson/c3-pll.c | 1 + > drivers/clk/meson/clk-pll.c | 22 +++++++++++++++++++--- > drivers/clk/meson/clk-pll.h | 1 + > drivers/clk/meson/s4-pll.c | 9 +++++++-- > 4 files changed, 28 insertions(+), 5 deletions(-) > --- > base-commit: adac147c6a32e2919cb04555387e12e738991a19 > change-id: 20240904-fix_clk-668f7a1a2b16 > > Best regards,
Applied to clk-meson (clk-meson-next), thanks!
[3/4] clk: meson: s4: pll: hifi_pll support fractional multiplier
https://github.com/BayLibre/clk-meson/commit/80344f4c1a1e
Best regards,
--
Jerome
On 2024/9/6 15:04, Jerome Brunet wrote: > [ EXTERNAL EMAIL ] > > On Fri 06 Sep 2024 at 13:52, Chuan Liu via B4 Relay <devnull+chuan.liu.amlogic.com@kernel.org> wrote: > >> Some PLLs with fractional multipliers have fractional denominators that >> are fixed to "100000" instead of the previous "(1 << pll->frac.width)". >> >> The hifi_pll for both C3 and S4 supports a fractional multiplier and has >> a fixed fractional denominator of "100000". >> >> Here are the results of the C3-based command tests (already defined >> CLOCK_ALLOW_WRITE_DEBUGFS): >> * echo 491520000 > /sys/kernel/debug/clk/hifi_pll/clk_rate >> * cat /sys/kernel/debug/clk/hifi_pll/clk_rate >> 491520000 >> * echo 1 > /sys/kernel/debug/clk/hifi_pll/clk_prepare_enable >> * cat /sys/kernel/debug/meson-clk-msr/clks/hifi_pll_clk >> 491515625 +/-15625Hz >> * devmem 0xfe008100 32 >> 0xD00304A3 >> * devmem 0xfe008104 32 >> 0x00014820 >> >> Based on the register information read above, it can be obtained: >> m = 0xA3 = 0d163; >> n = 0x1 = 0d1 >> frac = 0x14820 = 0d84000 >> od = 0x3 = 0d3 >> >> hifi_pll calculates the output frequency: >> calc_rate = xtal_rate / n * (m + (frac / frac_max)) >> od; >> calc_rate = 24000000 / 1 * (163 + (84000 / 100000)) >> 3; >> calc_rate = 491520000 >> >> clk_rate, msr_rate, and calc_rate all match. > Thanks for the detailed description. > > Is there a possibility this applies to the g12/sm1 as well ? HiFi PLL > has had trouble on these SoCs since support has been added. It sometimes > takes a long time to report a lock. So long we consider it a failure. > > There was no such issue on AXG. If you check DT, it is the reason why > AXG use the HiFi PLL for the sound card and G12/SM1 does not. I have confirmed that only the hifi_pll of the chip in recent years has this feature, and g12a/sm1/axg is not like this. I confirm that our sm1 uses hifi_pll, and I have not encountered the situation you said. There was a probability of lock failure in pll before, which was solved by adding 50us delay in meson_clk_pll_enable: @@ -378,6 +378,8 @@ static int meson_clk_pll_enable(struct clk_hw *hw) /* Enable the pll */ meson_parm_write(clk->map, &pll->en, 1); + udelay(50); + /* Take the pll out reset */ if (MESON_PARM_APPLICABLE(&pll->rst)) meson_parm_write(clk->map, &pll->rst, 0); This patch is also prepare to push to upstream later. Another detail is that the larger the frac value, the longer the lock time, but the time is at the us level, so that the timeout in the pll driver is not triggered. > >> The test and calculation results of S4 are consistent with those of C3, >> which will not be repeated here. >> >> Signed-off-by: Chuan Liu <chuan.liu@amlogic.com> >> --- >> Chuan Liu (4): >> clk: meson: Support PLL with fixed fractional denominators >> clk: meson: c3: pll: hifi_pll frequency is not accurate >> clk: meson: s4: pll: hifi_pll support fractional multiplier >> clk: meson: s4: pll: hifi_pll frequency is not accurate >> >> drivers/clk/meson/c3-pll.c | 1 + >> drivers/clk/meson/clk-pll.c | 22 +++++++++++++++++++--- >> drivers/clk/meson/clk-pll.h | 1 + >> drivers/clk/meson/s4-pll.c | 9 +++++++-- >> 4 files changed, 28 insertions(+), 5 deletions(-) >> --- >> base-commit: adac147c6a32e2919cb04555387e12e738991a19 >> change-id: 20240904-fix_clk-668f7a1a2b16 >> >> Best regards, > -- > Jerome
Some PLLs with fractional multipliers have fractional denominators that are fixed to "100000" instead of the previous "(1 << pll->frac.width)". The hifi_pll for both C3 and S4 supports a fractional multiplier and has a fixed fractional denominator of "100000". Here are the results of the C3-based command tests (already defined CLOCK_ALLOW_WRITE_DEBUGFS): * echo 491520000 > /sys/kernel/debug/clk/hifi_pll/clk_rate * cat /sys/kernel/debug/clk/hifi_pll/clk_rate 491520000 * echo 1 > /sys/kernel/debug/clk/hifi_pll/clk_prepare_enable * cat /sys/kernel/debug/meson-clk-msr/clks/hifi_pll_clk 491515625 +/-15625Hz * devmem 0xfe008100 32 0xD00304A3 * devmem 0xfe008104 32 0x00014820 Based on the register information read above, it can be obtained: m = 0xA3 = 0d163; n = 0x1 = 0d1 frac = 0x14820 = 0d84000 od = 0x3 = 0d3 hifi_pll calculates the output frequency: calc_rate = xtal_rate / n * (m + (frac / frac_max)) >> od; calc_rate = 24000000 / 1 * (163 + (84000 / 100000)) >> 3; calc_rate = 491520000 clk_rate, msr_rate, and calc_rate all match. The test and calculation results of S4 are consistent with those of C3, which will not be repeated here. Signed-off-by: Chuan Liu <chuan.liu@amlogic.com> --- Chuan Liu (4): clk: meson: Support PLL with fixed fractional denominators clk: meson: c3: pll: hifi_pll frequency is not accurate clk: meson: s4: pll: hifi_pll support fractional multiplier clk: meson: s4: pll: hifi_pll frequency is not accurate drivers/clk/meson/c3-pll.c | 1 + drivers/clk/meson/clk-pll.c | 22 +++++++++++++++++++--- drivers/clk/meson/clk-pll.h | 1 + drivers/clk/meson/s4-pll.c | 9 +++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) --- base-commit: adac147c6a32e2919cb04555387e12e738991a19 change-id: 20240904-fix_clk-668f7a1a2b16 Best regards,