Messages in this thread | | | Subject | Re: [PATCH] clk: meson: gxbb: Add the spread spectrum bit for MPLL0 on GXBB | From | Christian Hewitt <> | Date | Sat, 16 Oct 2021 21:18:49 +0400 |
| |
> On 16 Oct 2021, at 6:59 pm, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > > 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). > > Experimenting has shown that MPLL0 is causing this problem. In the .dts > we have by default: > assigned-clocks = <&clkc CLKID_MPLL0>, > <&clkc CLKID_MPLL1>, > <&clkc CLKID_MPLL2>; > assigned-clock-rates = <294912000>, > <270950400>, > <393216000>; > The MPLL0 rate is divisible by 48kHz without remainder and the MPLL1 > rate is divisible by 44.1kHz without remainder. Swapping these two clock > rates "fixes" 48kHz audio but breaks 44.1kHz audio. > > Everything looks normal when looking at the info provided by the common > clock framework while playing 48kHz audio (via I2S with mclk-fs = 256): > mpll_prediv 1 1 0 2000000000 > mpll0_div 1 1 0 294909641 > mpll0 1 1 0 294909641 > cts_amclk_sel 1 1 0 294909641 > cts_amclk_div 1 1 0 12287902 > cts_amclk 1 1 0 12287902 > > meson-clk-msr however shows that the actual MPLL0 clock is off by more > than 38MHz: > mp0_out 333322917 +/-10416Hz > > The 3.14 vendor kernel uses the following code to enable SSEN only for > MPLL0 (where con_reg2 is HHI_MPLL_CNTL and SSEN_shift is 25): > if (strncmp(hw->clk->name, "mpll_clk_out0", 13) == 0) { > val = readl(mpll->con_reg2); > val |= 1 << mpll->SSEN_shift; > writel(val, mpll->con_reg2); > } > > 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. > > Fixes: 8925dbd03bb29b ("clk: meson: gxbb: no spread spectrum on mpll") > Reported-by: Christian Hewitt <christianshewitt@gmail.com> > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Tested with a selection of different media at 41KHz and 48KHz on GXBB (WeTek Play2 and Hub) and GXL (LePotato).
Tested-by: Christian Hewitt <christianshewitt@gmail.com>
> --- > drivers/clk/meson/gxbb.c | 50 +++++++++++++++++++++++++++++++++++++--- > 1 file changed, 47 insertions(+), 3 deletions(-) > > diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c > index d6eed760327d..673bc915c7d9 100644 > --- a/drivers/clk/meson/gxbb.c > +++ b/drivers/clk/meson/gxbb.c > @@ -713,6 +713,41 @@ static struct clk_regmap gxbb_mpll_prediv = { > }; > > static struct clk_regmap gxbb_mpll0_div = { > + .data = &(struct meson_clk_mpll_data){ > + .sdm = { > + .reg_off = HHI_MPLL_CNTL7, > + .shift = 0, > + .width = 14, > + }, > + .sdm_en = { > + .reg_off = HHI_MPLL_CNTL7, > + .shift = 15, > + .width = 1, > + }, > + .n2 = { > + .reg_off = HHI_MPLL_CNTL7, > + .shift = 16, > + .width = 9, > + }, > + .ssen = { > + .reg_off = HHI_MPLL_CNTL, > + .shift = 25, > + .width = 1, > + }, > + .flags = CLK_MESON_MPLL_SPREAD_SPECTRUM, > + .lock = &meson_clk_lock, > + }, > + .hw.init = &(struct clk_init_data){ > + .name = "mpll0_div", > + .ops = &meson_clk_mpll_ops, > + .parent_hws = (const struct clk_hw *[]) { > + &gxbb_mpll_prediv.hw > + }, > + .num_parents = 1, > + }, > +}; > + > +static struct clk_regmap gxl_mpll0_div = { > .data = &(struct meson_clk_mpll_data){ > .sdm = { > .reg_off = HHI_MPLL_CNTL7, > @@ -749,7 +784,16 @@ static struct clk_regmap gxbb_mpll0 = { > .hw.init = &(struct clk_init_data){ > .name = "mpll0", > .ops = &clk_regmap_gate_ops, > - .parent_hws = (const struct clk_hw *[]) { &gxbb_mpll0_div.hw }, > + .parent_data = &(const struct clk_parent_data) { > + /* > + * Note: > + * GXL and GXBB have different SSEN requirements. We > + * fallback to the global naming string mechanism so > + * mpll0_div picks up the appropriate one. > + */ > + .name = "mpll0_div", > + .index = -1, > + }, > .num_parents = 1, > .flags = CLK_SET_RATE_PARENT, > }, > @@ -3044,7 +3088,7 @@ static struct clk_hw_onecell_data gxl_hw_onecell_data = { > [CLKID_VAPB_1] = &gxbb_vapb_1.hw, > [CLKID_VAPB_SEL] = &gxbb_vapb_sel.hw, > [CLKID_VAPB] = &gxbb_vapb.hw, > - [CLKID_MPLL0_DIV] = &gxbb_mpll0_div.hw, > + [CLKID_MPLL0_DIV] = &gxl_mpll0_div.hw, > [CLKID_MPLL1_DIV] = &gxbb_mpll1_div.hw, > [CLKID_MPLL2_DIV] = &gxbb_mpll2_div.hw, > [CLKID_MPLL_PREDIV] = &gxbb_mpll_prediv.hw, > @@ -3439,7 +3483,7 @@ static struct clk_regmap *const gxl_clk_regmaps[] = { > &gxbb_mpll0, > &gxbb_mpll1, > &gxbb_mpll2, > - &gxbb_mpll0_div, > + &gxl_mpll0_div, > &gxbb_mpll1_div, > &gxbb_mpll2_div, > &gxbb_cts_amclk_div, > -- > 2.33.1 >
| |