Hi,
On Fri, Jun 17, 2022 at 04:42:49AM +0000, Judy Hsiao wrote:
We discovered that the state of BCLK on, LRCLK off and SD_MODE on may cause the speaker melting issue. Removing LRCLK while BCLK is present can cause unexpected output behavior including a large DC output voltage as described in the Max98357a datasheet.
In order to:
- prevent BCLK from turning on by other component.
- keep BCLK and LRCLK being present at the same time
This patch switches BCLK to GPIO func before LRCLK output, and configures BCLK func back during LRCLK is output.
Without this fix, BCLK is turned on 11 ms earlier than LRCK by the da7219. With this fix, BCLK is turned on only 0.4 ms earlier than LRCK by the rockchip codec.
Signed-off-by: Judy Hsiao judyhsiao@chromium.org
sound/soc/rockchip/rockchip_i2s.c | 167 ++++++++++++++++++++++-------- 1 file changed, 121 insertions(+), 46 deletions(-)
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c index 47a3971a9ce1..5e3cb06377a1 100644 --- a/sound/soc/rockchip/rockchip_i2s.c +++ b/sound/soc/rockchip/rockchip_i2s.c
@@ -92,39 +122,46 @@ static inline struct rk_i2s_dev *to_info(struct snd_soc_dai *dai) return snd_soc_dai_get_drvdata(dai); }
-static void rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on) +static int rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on) { unsigned int val = 0; int retry = 10;
- int ret = 0;
- spin_lock(&i2s->lock); if (on) {
regmap_update_bits(i2s->regmap, I2S_DMACR,I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_ENABLE);regmap_update_bits(i2s->regmap, I2S_XFER,I2S_XFER_TXS_START | I2S_XFER_RXS_START,I2S_XFER_TXS_START | I2S_XFER_RXS_START);
ret = regmap_update_bits(i2s->regmap, I2S_DMACR,I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_ENABLE);if (ret < 0)goto end;ret = regmap_update_bits(i2s->regmap, I2S_XFER,I2S_XFER_TXS_START | I2S_XFER_RXS_START,I2S_XFER_TXS_START | I2S_XFER_RXS_START);
This still isn't how I'd expect the indentation to look, but I think I will stop complaining about whitespace. Maybe you can either use 'clang-format' (sparingly), or else see what Mark thinks.
if (ret < 0)goto end;
...
static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on)
I didn't think I needed to call this out, because you already got build bots complaining about this, but... ...did you even compile-test this? You should be making this function return 'int', not 'void', because you're adding return values below.
{ unsigned int val = 0; int retry = 10;
- int ret = 0;
...
+end: spin_unlock(&i2s->lock);
- if (ret < 0)
dev_err(i2s->dev, "lrclk update failed\n");- return ret;
^^ here.
}
static int rockchip_i2s_set_fmt(struct snd_soc_dai *cpu_dai,
Aside from those, I think this looks OK to the best of my (not-very-familiar with sounds/soc/) ability:
Reviewed-by: Brian Norris briannorris@chromium.org