[PATCH v4 1/3] ASoC: Add Rockchip rk817 audio CODEC support

[Chris Morgan]

On Fri, Mar 19, 2021 at 01:30:10PM +0000, Mark Brown wrote:
> On Thu, Mar 18, 2021 at 05:19:34PM -0500, Chris Morgan wrote:
> 
> > The frequency of the MCLK (when it actually works) is 12MHz, though the
> > Rockchip driver seems to want to run it at 12.288MHz and it works just fine
> > as well.  When I move this clock to the main node for the MFD it starts running
> 
> 12.288MHz would be a more normal audio clock, it's a nice multiple of
> 8kHz (though with a PLL it doesn't really matter unless the device on
> the other end of the link is also clocking its audio stuff off it since
> the PLL exists to transform the clock rate into one that's more useful).
> 
> > at 100MHz, and suffice to say it doesn't work right (high pitched sounds,
> > screeching after the audio finishes playing until the hardware shuts down,
> 
> That's...  surprising.  :/  I can't think what'd cause that.

I moved the pinctrl for the clock pin to the PMIC node and that fixed it.
Clock frequency is back to what I expect in the 12MHz range and no more
screeching.

So based on the values how do you propose I validate the input of the
frequencies, or if that's necessary at all given this PMIC seems to only
exist implemented with the Rockchip PX30 (on which all these values were
based).

> 
> > etc.) According to the schematic of the implementation I'm working with (Odroid
> > Go Advance) there is also an SCLK(SOC)/BCLK(PMIC) and an LRCLK. I assume these
> > also run at ~12MHz since it seems they are tied to the parent clock rate in the
> > clk-px30 driver, just as the MCLK is.  Likewise these also run at 100MHz when
> > the clock is connected to the PMIC node directly instead of at the codec node
> > level.
> 
> LRCLK should run at the sample rate, and BCLK at a rate fast enough to
> clock out all the bits required for the data.  MCLK is usually faster
> than those and intended to be a reference clock for them.
> 
> > > BTW looking at the driver there's a bunch of other registers so
> > > shouldn't the regmap be done at the MFD level?
> 
> > I'm not sure honestly. If you think that's best I can figure out how. I'm
> > trying to avoid a lot of changes to the mfd driver itself because the rk817
> > is the only version of this that has a codec, all the other ones supported by
> > this driver don't. Again though whatever you think is best I'll try to
> > implement.
> 
> It's really what I'd expect, two regmaps for the same device is weird.
> Looking at the MFD driver it seems to already have separate regmap
> configs per device supported so it shouldn't be too intrusive, just more
> detailed data for the device - the main difference is that you've
> explicitly listed all the valid registers whereas the driver didn't
> bother for anything so far.

I can add them to the regmap for the mfd then.  I assume when I tried to
reference the MFD's regmap earlier and I got a kernel panic its because
I was trying to write to registers that weren't defined?