Thread (7 messages) 7 messages, 3 authors, 2021-10-21

Re: [PATCH] clk: meson: gxbb: Add the spread spectrum bit for MPLL0 on GXBB

From: Jerome Brunet <jbrunet@baylibre.com>
Date: 2021-10-18 12:03:35
Also in: linux-amlogic, linux-clk, lkml

On Mon 18 Oct 2021 at 13:44, Martin Blumenstingl [off-list ref] wrote:
Hi Jerome,

On Mon, Oct 18, 2021 at 12:19 PM Jerome Brunet [off-list ref] wrote:
quoted

On Sat 16 Oct 2021 at 16:59, Martin Blumenstingl [off-list ref] wrote:
quoted
Christian reports that 48kHz audio does not work on his WeTek Play 2
(which uses a GXBB SoC), while 44.1kHz audio works fine on the same
board. He also reports that 48kHz audio works on GXL and GXM SoCs,
which are using an (almost) identical AIU (audio controller).
The above is a bit "personal" - it is not great fit for the commit
description. Please rephrase or put it in comment section bellow
sure, I can rephrase that

[...]
quoted
quoted
Add the SSEN (spread spectrum enable) bit and add the
CLK_MESON_MPLL_SPREAD_SPECTRUM flag to enable this bit for MPLL0. Do
this for GXBB *only* since GXL doesn't seem to care if this bit is set
or not, meaning that meson-clk-msr always sees (approximately) the same
frequency as common clock framework.
 1 - it is odd that we need to poke a bit in the register related to the
 fixed PLL but ok ...
 2 - 3.14 does yes, 4.9 does not soooo ... no real proof there
The fact that 4.9 doesn't do it is also no proof for anything either.
Amlogic hasn't ported forward GXBB support to their 4.9 kernel. Even
"mesongxbb.dtsi" is missing there [0]
So in my opinion the code from Amlogic's patched 4.9 kernel cannot be
used as reference for anything GXBB related. Only the 3.14 code can be
used as reference.

[...]
quoted
So 2 things:
 - If this bit really enables spread spectrum on MPLL0 (or worse, the
 Fixed PLL) - checking clk measure is not enough. It is just a mean of
 the rate seen by the SoC itself. You would not see the effect of the
 spread spectrum here ... you need to capture the clock output with a
 scope for that.
I suspect that a board with pin headers is needed to route the signal there?
Personally I don't have any GXBB board(s).
quoted
 - Or the bit is incorrectly documented (or DDS0_SSEN does not mean
 spread spectrum). If it is not a spread spectrum function, then this
 patch seems to indicate it is and it is misleading.

Either way, I'm not OK with it.

To me, the rate drop that happens when you flip this bit looks more like
the effect SDM_EN should have.

Could you check the internal values (n2 and sdm) compare this to the
output rate you actually get ? see if this leads to anything ? does
SDM_EN really has an effect on this MPLL ? it is a combination of both ?
Christian has provided a dump of the HHI registers (via userspace,
regmap makes them accessible).
On GXBB WP2 HHI_MPLL_CNTL7 is set to 0x0006f208
On GXL Le Potato (which he used for comparison as it wasn't affected
by the MPLL0 issue) HHI_MPLL_CNTL7 is set to 0x0006b208

If you're interested in the full HHI register dump you can find it here: [1]
GXBB WP2 is on the left and GXL Le Potato is on the right.

The difference here is BIT(14). un-setting BIT(14) (documented as
EN_DDS0) did not change anything according to Christian's test.
That also means that SDM, SDM_EN and N2 have the expected values.
I manually did the maths:
(2000000000Hz * 16384) / ((16384 * 6) + 12808) = 294909640.7Hz
which matches what clk_summary sees:
294909641Hz
 ... and (2000000000Hz * 16384) / ((16384 * 6) = 333MHz which is fairly close
 to what you get w/o flipping the bit
Just to clarify that I am not wasting Christian's time when I ask him
to test something:
next up we'll set SDM_EN to 0 (instead of 1) and see the reaction using clk-msr?
For example yes. I am asking check a bit more what this bit does and
what it does not:
 - I need confirmation whether or not it does spread spectrum. Yes this
 needs to be observed on a SoC pin, like MCLK with a fairly low divider
 to the averaging effect which could partially mask spread spectrum.

 - Get an idea what it actually does. The 2 calculations above are an
 hint. (Spread spectrum does not change the rate mean value)
Best regards,
Martin


[0] https://github.com/hardkernel/linux/tree/db88db3864c5d6903fb11f6528874887d1c473ce/arch/arm64/boot/dts/amlogic
[1] https://pastebin.com/raw/1cjPFsfa

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help