[v2,0/7] clk: meson: fix mpll jitter

Message

Jerome Brunet May 13, 2019, 12:31 p.m. UTC

This patchset is a squash of these previous patchsets [0], [1] without
modification, beside a re-ordering of the changes to facilitate backports

We are observing a lot of jitter on the MPLL outputs of the g12a.
No such jitter is seen on gx family. On the axg family, only MPLL2
seems affected. This was not a problem so far since this MPLL output
is not used.

The jitter can be as high as +/- 4%.

This is a problem for audio application. This may cause distortion on
i2s and completely break SPDIF.

After exchanging with Amlogic, it seems we have activated (by mistake)
the 'spread spectrum' feature.

The 3 first patches properly set the bit responsible for the spread
spectrum in the mpll driver and add the required correction in the
related clock controllers.

When the g12a support has been initially submitted, the MPLL appeared
(overall) fine. At the time, the board I used was flashed with Amlogic
vendor u-boot. Since then, I moved to an early version on mainline
u-boot, which is likely to initialize the clock differently.

While debugging audio support, I noticed that MPLL based clocks were way
above target. It appeared the fractional part of the divider was not
working.

To work properly, the MPLLs each needs an initial setting in addition to
a common one. No one likes those register sequences but sometimes, like
here for PLL clocks, there is no way around it.

The last 4 patches add the possibility to set initial register sequence
for the ee clock controller and the MPLL driver. It is then used to enable
the fractional part of the g12a MPLL.

As agreed with the clock maintainers, I'll submit a series to CCF to
remove the .init() callbacks and introduce register()/deregister()
callbacks later on (pinky swear).

Jerome Brunet (7):
  clk: meson: mpll: properly handle spread spectrum
  clk: meson: gxbb: no spread spectrum on mpll0
  clk: meson: axg: spread spectrum is on mpll2
  clk: meson: mpll: add init callback and regs
  clk: meson: g12a: add mpll register init sequences
  clk: meson: eeclk: add init regs
  clk: meson: g12a: add controller register init

 drivers/clk/meson/axg.c         | 10 ++++-----
 drivers/clk/meson/clk-mpll.c    | 36 ++++++++++++++++++++++++---------
 drivers/clk/meson/clk-mpll.h    |  3 +++
 drivers/clk/meson/g12a.c        | 32 ++++++++++++++++++++++++++++-
 drivers/clk/meson/gxbb.c        |  5 -----
 drivers/clk/meson/meson-eeclk.c |  3 +++
 drivers/clk/meson/meson-eeclk.h |  2 ++
 7 files changed, 70 insertions(+), 21 deletions(-)

Comments

Jerome Brunet May 20, 2019, 11:26 a.m. UTC | #1

On Mon, 2019-05-13 at 14:31 +0200, Jerome Brunet wrote:
> This patchset is a squash of these previous patchsets [0], [1] without
> modification, beside a re-ordering of the changes to facilitate backports
> 
> We are observing a lot of jitter on the MPLL outputs of the g12a.
> No such jitter is seen on gx family. On the axg family, only MPLL2
> seems affected. This was not a problem so far since this MPLL output
> is not used.
> 
> The jitter can be as high as +/- 4%.
> 
> This is a problem for audio application. This may cause distortion on
> i2s and completely break SPDIF.
> 
> After exchanging with Amlogic, it seems we have activated (by mistake)
> the 'spread spectrum' feature.
> 
> The 3 first patches properly set the bit responsible for the spread
> spectrum in the mpll driver and add the required correction in the
> related clock controllers.
> 
> When the g12a support has been initially submitted, the MPLL appeared
> (overall) fine. At the time, the board I used was flashed with Amlogic
> vendor u-boot. Since then, I moved to an early version on mainline
> u-boot, which is likely to initialize the clock differently.
> 
> While debugging audio support, I noticed that MPLL based clocks were way
> above target. It appeared the fractional part of the divider was not
> working.
> 
> To work properly, the MPLLs each needs an initial setting in addition to
> a common one. No one likes those register sequences but sometimes, like
> here for PLL clocks, there is no way around it.
> 
> The last 4 patches add the possibility to set initial register sequence
> for the ee clock controller and the MPLL driver. It is then used to enable
> the fractional part of the g12a MPLL.
> 
> As agreed with the clock maintainers, I'll submit a series to CCF to
> remove the .init() callbacks and introduce register()/deregister()
> callbacks later on (pinky swear).
> 
> Jerome Brunet (7):
>   clk: meson: mpll: properly handle spread spectrum
>   clk: meson: gxbb: no spread spectrum on mpll0
>   clk: meson: axg: spread spectrum is on mpll2
>   clk: meson: mpll: add init callback and regs
>   clk: meson: g12a: add mpll register init sequences
>   clk: meson: eeclk: add init regs
>   clk: meson: g12a: add controller register init
> 
>  drivers/clk/meson/axg.c         | 10 ++++-----
>  drivers/clk/meson/clk-mpll.c    | 36 ++++++++++++++++++++++++---------
>  drivers/clk/meson/clk-mpll.h    |  3 +++
>  drivers/clk/meson/g12a.c        | 32 ++++++++++++++++++++++++++++-
>  drivers/clk/meson/gxbb.c        |  5 -----
>  drivers/clk/meson/meson-eeclk.c |  3 +++
>  drivers/clk/meson/meson-eeclk.h |  2 ++
>  7 files changed, 70 insertions(+), 21 deletions(-)
> 

series applied to v5.3/drivers