Hi Samuel,
On Sun, 20 Sep 2020 at 20:52, Samuel Holland samuel@sholland.org wrote:
On 9/20/20 1:07 PM, Clément Péron wrote:
The FIFO TX reg is volatile and sun8i i2s register mapping is different from sun4i.
Even if in this case it's doesn't create an issue, Avoid setting some regs that are undefined in sun8i.
Signed-off-by: Clément Péron peron.clem@gmail.com Acked-by: Maxime Ripard mripard@kernel.org
sound/soc/sunxi/sun4i-i2s.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c index ce4913f0ffe4..a35be0e2baf5 100644 --- a/sound/soc/sunxi/sun4i-i2s.c +++ b/sound/soc/sunxi/sun4i-i2s.c @@ -1126,12 +1126,19 @@ static bool sun8i_i2s_rd_reg(struct device *dev, unsigned int reg)
static bool sun8i_i2s_volatile_reg(struct device *dev, unsigned int reg) {
if (reg == SUN8I_I2S_INT_STA_REG)
switch (reg) {case SUN4I_I2S_FIFO_CTRL_REG:Please check if this breaks audio recording with runtime PM enabled. I noticed this with an older revision of the series that also changed sun4i_i2s_volatile_reg. Marking SUN4I_I2S_FIFO_CTRL_REG as volatile broke setting of SUN4I_I2S_FIFO_CTRL_TX_MODE/RX_MODE, because the set_fmt() callback is not run with a runtime PM reference held, and volatile registers are not written by regcache_sync() during sun4i_i2s_runtime_resume().
As a workaround, I moved the TX_MODE/RX_MODE initialization to hw_params(), but I am not sure it is the right thing to do:
Thanks for the catch, I never tried to suspend/resume my board actually. But your explanation and the fix seems legit to me.
I don't think it's a workaround as settings the fifo size is not related to set_fmt and could also be set in hw_params.
I will add your fix in the next version.
Regards, Clement
https://github.com/smaeul/linux/commit/5e40ac610986.patch
Cheers, Samuel
case SUN4I_I2S_FIFO_RX_REG:case SUN4I_I2S_FIFO_STA_REG:case SUN4I_I2S_RX_CNT_REG:case SUN4I_I2S_TX_CNT_REG:case SUN8I_I2S_FIFO_TX_REG:case SUN8I_I2S_INT_STA_REG: return true;
if (reg == SUN8I_I2S_FIFO_TX_REG)return false;return sun4i_i2s_volatile_reg(dev, reg);
default:return false;}}
static const struct reg_default sun4i_i2s_reg_defaults[] = {