* [PATCH v4 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-19 9:53 Judy Hsiao
2022-06-19 9:53 ` [PATCH v4 1/3] " Judy Hsiao
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Judy Hsiao @ 2022-06-19 9:53 UTC (permalink / raw)
To: Heiko Stuebner
Cc: Liam Girdwood, Rob Herring, Brian Norris, Mark Brown,
Jaroslav Kysela, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-rockchip, linux-kernel, Judy Hsiao
The patches series is to fix the unexpected large DC output
voltage of Max98357a that burns the speakers on the rockchip
platform when BCLK and SD_MODE are ON but LRCLK is OFF.
Changes Since V4:
-- Fix indentation in the driver. (Align parameters with the parenthesis
placement.)
-- Fix incorrect return type of rockchip_snd_rxctrl.
Changes Since V3:
-- Fix indentation in the documentation.
-- Put pinctrl-1 right after pinctrl-0 in dtsi.
-- Fix indentation in the driver.
-- Remove unnecessary dev_dbg() in the driver.
Changes Since V2:
-- Add documents of i2s pinctrl-names.
-- Fix dtsi syntax error.
-- Include the dtsi change and the driver change in the same series.
-- Ensure that driver gets both bclk_on and bclk_off states before using them.
Judy Hsiao (3):
ASoC: rockchip: i2s: switch BCLK to GPIO
arm64: dts: rk3399: i2s: switch BCLK to GPIO
ASoC: dt-bindings: rockchip: Document pinctrl-names for i2s
.../bindings/sound/rockchip-i2s.yaml | 7 +
.../boot/dts/rockchip/rk3399-gru-scarlet.dtsi | 10 +
arch/arm64/boot/dts/rockchip/rk3399.dtsi | 25 ++-
sound/soc/rockchip/rockchip_i2s.c | 171 +++++++++++++-----
4 files changed, 165 insertions(+), 48 deletions(-)
--
2.36.1.476.g0c4daa206d-goog
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v4 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO 2022-06-19 9:53 [PATCH v4 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO Judy Hsiao @ 2022-06-19 9:53 ` Judy Hsiao 2022-06-21 18:02 ` Chen-Yu Tsai 2022-06-19 9:53 ` [PATCH v4 2/3] arm64: dts: rk3399: " Judy Hsiao ` (2 subsequent siblings) 3 siblings, 1 reply; 9+ messages in thread From: Judy Hsiao @ 2022-06-19 9:53 UTC (permalink / raw) To: Heiko Stuebner Cc: Liam Girdwood, Rob Herring, Brian Norris, Mark Brown, Jaroslav Kysela, Chen-Yu Tsai, alsa-devel, linux-arm-kernel, linux-rockchip, linux-kernel, Judy Hsiao We discoverd 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: 1. prevent BCLK from turning on by other component. 2. 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> Reviewed-by: Brian Norris <briannorris@chromium.org> --- sound/soc/rockchip/rockchip_i2s.c | 171 ++++++++++++++++++++++-------- 1 file changed, 124 insertions(+), 47 deletions(-) diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c index 47a3971a9ce1..8e4a9b8746fd 100644 --- a/sound/soc/rockchip/rockchip_i2s.c +++ b/sound/soc/rockchip/rockchip_i2s.c @@ -54,8 +54,38 @@ struct rk_i2s_dev { const struct rk_i2s_pins *pins; unsigned int bclk_ratio; spinlock_t lock; /* tx/rx lock */ + struct pinctrl *pinctrl; + struct pinctrl_state *bclk_on; + struct pinctrl_state *bclk_off; }; +static int i2s_pinctrl_select_bclk_on(struct rk_i2s_dev *i2s) +{ + int ret = 0; + + if (!IS_ERR(i2s->pinctrl) && !IS_ERR_OR_NULL(i2s->bclk_on)) + ret = pinctrl_select_state(i2s->pinctrl, i2s->bclk_on); + + if (ret) + dev_err(i2s->dev, "bclk enable failed %d\n", ret); + + return ret; +} + +static int i2s_pinctrl_select_bclk_off(struct rk_i2s_dev *i2s) +{ + + int ret = 0; + + if (!IS_ERR(i2s->pinctrl) && !IS_ERR_OR_NULL(i2s->bclk_off)) + ret = pinctrl_select_state(i2s->pinctrl, i2s->bclk_off); + + if (ret) + dev_err(i2s->dev, "bclk disable failed %d\n", ret); + + return ret; +} + static int i2s_runtime_suspend(struct device *dev) { struct rk_i2s_dev *i2s = dev_get_drvdata(dev); @@ -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); + if (ret < 0) + goto end; i2s->tx_start = true; } else { i2s->tx_start = false; - regmap_update_bits(i2s->regmap, I2S_DMACR, - I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_DISABLE); + ret = regmap_update_bits(i2s->regmap, I2S_DMACR, + I2S_DMACR_TDE_ENABLE, + I2S_DMACR_TDE_DISABLE); + if (ret < 0) + goto end; if (!i2s->rx_start) { - regmap_update_bits(i2s->regmap, I2S_XFER, - I2S_XFER_TXS_START | - I2S_XFER_RXS_START, - I2S_XFER_TXS_STOP | - I2S_XFER_RXS_STOP); - + ret = regmap_update_bits(i2s->regmap, I2S_XFER, + I2S_XFER_TXS_START | I2S_XFER_RXS_START, + I2S_XFER_TXS_STOP | I2S_XFER_RXS_STOP); + if (ret < 0) + goto end; udelay(150); - regmap_update_bits(i2s->regmap, I2S_CLR, - I2S_CLR_TXC | I2S_CLR_RXC, - I2S_CLR_TXC | I2S_CLR_RXC); - + ret = regmap_update_bits(i2s->regmap, I2S_CLR, + I2S_CLR_TXC | I2S_CLR_RXC, + I2S_CLR_TXC | I2S_CLR_RXC); + if (ret < 0) + goto end; regmap_read(i2s->regmap, I2S_CLR, &val); /* Should wait for clear operation to finish */ @@ -138,42 +175,55 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on) } } } +end: spin_unlock(&i2s->lock); + if (ret < 0) + dev_err(i2s->dev, "lrclk update failed\n"); + + return ret; } -static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on) +static int rockchip_snd_rxctrl(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_RDE_ENABLE, I2S_DMACR_RDE_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_RDE_ENABLE, + I2S_DMACR_RDE_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); + if (ret < 0) + goto end; i2s->rx_start = true; } else { i2s->rx_start = false; - regmap_update_bits(i2s->regmap, I2S_DMACR, - I2S_DMACR_RDE_ENABLE, I2S_DMACR_RDE_DISABLE); + ret = regmap_update_bits(i2s->regmap, I2S_DMACR, + I2S_DMACR_RDE_ENABLE, + I2S_DMACR_RDE_DISABLE); + if (ret < 0) + goto end; if (!i2s->tx_start) { - regmap_update_bits(i2s->regmap, I2S_XFER, - I2S_XFER_TXS_START | - I2S_XFER_RXS_START, - I2S_XFER_TXS_STOP | - I2S_XFER_RXS_STOP); - + ret = regmap_update_bits(i2s->regmap, I2S_XFER, + I2S_XFER_TXS_START | I2S_XFER_RXS_START, + I2S_XFER_TXS_STOP | I2S_XFER_RXS_STOP); + if (ret < 0) + goto end; udelay(150); - regmap_update_bits(i2s->regmap, I2S_CLR, - I2S_CLR_TXC | I2S_CLR_RXC, - I2S_CLR_TXC | I2S_CLR_RXC); - + ret = regmap_update_bits(i2s->regmap, I2S_CLR, + I2S_CLR_TXC | I2S_CLR_RXC, + I2S_CLR_TXC | I2S_CLR_RXC); + if (ret < 0) + goto end; regmap_read(i2s->regmap, I2S_CLR, &val); /* Should wait for clear operation to finish */ @@ -187,7 +237,12 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on) } } } +end: spin_unlock(&i2s->lock); + if (ret < 0) + dev_err(i2s->dev, "lrclk update failed\n"); + + return ret; } static int rockchip_i2s_set_fmt(struct snd_soc_dai *cpu_dai, @@ -425,17 +480,25 @@ static int rockchip_i2s_trigger(struct snd_pcm_substream *substream, case SNDRV_PCM_TRIGGER_RESUME: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) - rockchip_snd_rxctrl(i2s, 1); + ret = rockchip_snd_rxctrl(i2s, 1); else - rockchip_snd_txctrl(i2s, 1); + ret = rockchip_snd_txctrl(i2s, 1); + if (ret < 0) + return ret; + i2s_pinctrl_select_bclk_on(i2s); break; case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_STOP: case SNDRV_PCM_TRIGGER_PAUSE_PUSH: - if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) - rockchip_snd_rxctrl(i2s, 0); - else - rockchip_snd_txctrl(i2s, 0); + if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) { + if (!i2s->tx_start) + i2s_pinctrl_select_bclk_off(i2s); + ret = rockchip_snd_rxctrl(i2s, 0); + } else { + if (!i2s->rx_start) + i2s_pinctrl_select_bclk_off(i2s); + ret = rockchip_snd_txctrl(i2s, 0); + } break; default: ret = -EINVAL; @@ -736,6 +799,20 @@ static int rockchip_i2s_probe(struct platform_device *pdev) } i2s->bclk_ratio = 64; + i2s->pinctrl = devm_pinctrl_get(&pdev->dev); + if (IS_ERR(i2s->pinctrl)) + dev_err(&pdev->dev, "failed to find i2s pinctrl\n"); + + i2s->bclk_on = pinctrl_lookup_state(i2s->pinctrl, "bclk_on"); + if (!IS_ERR_OR_NULL(i2s->bclk_on)) { + i2s->bclk_off = pinctrl_lookup_state(i2s->pinctrl, "bclk_off"); + if (IS_ERR_OR_NULL(i2s->bclk_off)) { + dev_err(&pdev->dev, "failed to find i2s bclk_off\n"); + goto err_clk; + } + } + + i2s_pinctrl_select_bclk_off(i2s); dev_set_drvdata(&pdev->dev, i2s); -- 2.36.1.476.g0c4daa206d-goog ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v4 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO 2022-06-19 9:53 ` [PATCH v4 1/3] " Judy Hsiao @ 2022-06-21 18:02 ` Chen-Yu Tsai 2022-06-22 10:52 ` Mark Brown 0 siblings, 1 reply; 9+ messages in thread From: Chen-Yu Tsai @ 2022-06-21 18:02 UTC (permalink / raw) To: Judy Hsiao Cc: Heiko Stuebner, Liam Girdwood, Rob Herring, Brian Norris, Mark Brown, Jaroslav Kysela, Chen-Yu Tsai, Linux-ALSA, linux-arm-kernel, open list:ARM/Rockchip SoC..., linux-kernel Hi, On Sun, Jun 19, 2022 at 5:54 PM Judy Hsiao <judyhsiao@chromium.org> wrote: > > We discoverd 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: > 1. prevent BCLK from turning on by other component. > 2. 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> > Reviewed-by: Brian Norris <briannorris@chromium.org> > --- > sound/soc/rockchip/rockchip_i2s.c | 171 ++++++++++++++++++++++-------- > 1 file changed, 124 insertions(+), 47 deletions(-) > > diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c > index 47a3971a9ce1..8e4a9b8746fd 100644 > --- a/sound/soc/rockchip/rockchip_i2s.c > +++ b/sound/soc/rockchip/rockchip_i2s.c > @@ -54,8 +54,38 @@ struct rk_i2s_dev { > const struct rk_i2s_pins *pins; > unsigned int bclk_ratio; > spinlock_t lock; /* tx/rx lock */ > + struct pinctrl *pinctrl; > + struct pinctrl_state *bclk_on; > + struct pinctrl_state *bclk_off; > }; > > +static int i2s_pinctrl_select_bclk_on(struct rk_i2s_dev *i2s) > +{ > + int ret = 0; > + > + if (!IS_ERR(i2s->pinctrl) && !IS_ERR_OR_NULL(i2s->bclk_on)) > + ret = pinctrl_select_state(i2s->pinctrl, i2s->bclk_on); > + > + if (ret) > + dev_err(i2s->dev, "bclk enable failed %d\n", ret); > + > + return ret; > +} > + > +static int i2s_pinctrl_select_bclk_off(struct rk_i2s_dev *i2s) > +{ > + > + int ret = 0; > + > + if (!IS_ERR(i2s->pinctrl) && !IS_ERR_OR_NULL(i2s->bclk_off)) > + ret = pinctrl_select_state(i2s->pinctrl, i2s->bclk_off); > + > + if (ret) > + dev_err(i2s->dev, "bclk disable failed %d\n", ret); > + > + return ret; > +} > + > static int i2s_runtime_suspend(struct device *dev) > { > struct rk_i2s_dev *i2s = dev_get_drvdata(dev); > @@ -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); > + if (ret < 0) > + goto end; > i2s->tx_start = true; > } else { > i2s->tx_start = false; > > - regmap_update_bits(i2s->regmap, I2S_DMACR, > - I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_DISABLE); > + ret = regmap_update_bits(i2s->regmap, I2S_DMACR, > + I2S_DMACR_TDE_ENABLE, > + I2S_DMACR_TDE_DISABLE); > + if (ret < 0) > + goto end; > > if (!i2s->rx_start) { > - regmap_update_bits(i2s->regmap, I2S_XFER, > - I2S_XFER_TXS_START | > - I2S_XFER_RXS_START, > - I2S_XFER_TXS_STOP | > - I2S_XFER_RXS_STOP); > - > + ret = regmap_update_bits(i2s->regmap, I2S_XFER, > + I2S_XFER_TXS_START | I2S_XFER_RXS_START, > + I2S_XFER_TXS_STOP | I2S_XFER_RXS_STOP); > + if (ret < 0) > + goto end; > udelay(150); > - regmap_update_bits(i2s->regmap, I2S_CLR, > - I2S_CLR_TXC | I2S_CLR_RXC, > - I2S_CLR_TXC | I2S_CLR_RXC); > - > + ret = regmap_update_bits(i2s->regmap, I2S_CLR, > + I2S_CLR_TXC | I2S_CLR_RXC, > + I2S_CLR_TXC | I2S_CLR_RXC); > + if (ret < 0) > + goto end; > regmap_read(i2s->regmap, I2S_CLR, &val); > > /* Should wait for clear operation to finish */ > @@ -138,42 +175,55 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on) > } > } > } > +end: > spin_unlock(&i2s->lock); > + if (ret < 0) > + dev_err(i2s->dev, "lrclk update failed\n"); > + > + return ret; > } > > -static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on) > +static int rockchip_snd_rxctrl(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_RDE_ENABLE, I2S_DMACR_RDE_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_RDE_ENABLE, > + I2S_DMACR_RDE_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); > + if (ret < 0) > + goto end; > i2s->rx_start = true; > } else { > i2s->rx_start = false; > > - regmap_update_bits(i2s->regmap, I2S_DMACR, > - I2S_DMACR_RDE_ENABLE, I2S_DMACR_RDE_DISABLE); > + ret = regmap_update_bits(i2s->regmap, I2S_DMACR, > + I2S_DMACR_RDE_ENABLE, > + I2S_DMACR_RDE_DISABLE); > + if (ret < 0) > + goto end; > > if (!i2s->tx_start) { > - regmap_update_bits(i2s->regmap, I2S_XFER, > - I2S_XFER_TXS_START | > - I2S_XFER_RXS_START, > - I2S_XFER_TXS_STOP | > - I2S_XFER_RXS_STOP); > - > + ret = regmap_update_bits(i2s->regmap, I2S_XFER, > + I2S_XFER_TXS_START | I2S_XFER_RXS_START, > + I2S_XFER_TXS_STOP | I2S_XFER_RXS_STOP); > + if (ret < 0) > + goto end; > udelay(150); > - regmap_update_bits(i2s->regmap, I2S_CLR, > - I2S_CLR_TXC | I2S_CLR_RXC, > - I2S_CLR_TXC | I2S_CLR_RXC); > - > + ret = regmap_update_bits(i2s->regmap, I2S_CLR, > + I2S_CLR_TXC | I2S_CLR_RXC, > + I2S_CLR_TXC | I2S_CLR_RXC); > + if (ret < 0) > + goto end; > regmap_read(i2s->regmap, I2S_CLR, &val); > > /* Should wait for clear operation to finish */ > @@ -187,7 +237,12 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on) > } > } > } > +end: > spin_unlock(&i2s->lock); > + if (ret < 0) > + dev_err(i2s->dev, "lrclk update failed\n"); > + > + return ret; > } > > static int rockchip_i2s_set_fmt(struct snd_soc_dai *cpu_dai, > @@ -425,17 +480,25 @@ static int rockchip_i2s_trigger(struct snd_pcm_substream *substream, > case SNDRV_PCM_TRIGGER_RESUME: > case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: > if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) > - rockchip_snd_rxctrl(i2s, 1); > + ret = rockchip_snd_rxctrl(i2s, 1); > else > - rockchip_snd_txctrl(i2s, 1); > + ret = rockchip_snd_txctrl(i2s, 1); > + if (ret < 0) > + return ret; > + i2s_pinctrl_select_bclk_on(i2s); > break; > case SNDRV_PCM_TRIGGER_SUSPEND: > case SNDRV_PCM_TRIGGER_STOP: > case SNDRV_PCM_TRIGGER_PAUSE_PUSH: > - if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) > - rockchip_snd_rxctrl(i2s, 0); > - else > - rockchip_snd_txctrl(i2s, 0); > + if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) { > + if (!i2s->tx_start) > + i2s_pinctrl_select_bclk_off(i2s); > + ret = rockchip_snd_rxctrl(i2s, 0); > + } else { > + if (!i2s->rx_start) > + i2s_pinctrl_select_bclk_off(i2s); > + ret = rockchip_snd_txctrl(i2s, 0); > + } > break; > default: > ret = -EINVAL; > @@ -736,6 +799,20 @@ static int rockchip_i2s_probe(struct platform_device *pdev) > } > > i2s->bclk_ratio = 64; > + i2s->pinctrl = devm_pinctrl_get(&pdev->dev); > + if (IS_ERR(i2s->pinctrl)) > + dev_err(&pdev->dev, "failed to find i2s pinctrl\n"); > + > + i2s->bclk_on = pinctrl_lookup_state(i2s->pinctrl, "bclk_on"); This will crash on any I2S controller that doesn't have pinctrl properties, notably the I2S controller that sends audio to the HDMI controller will never have pinctrl properties, since the connection is internal to the SoC. I'd say just put pinctrl lookup calls in an else block. ChenYu > + if (!IS_ERR_OR_NULL(i2s->bclk_on)) { > + i2s->bclk_off = pinctrl_lookup_state(i2s->pinctrl, "bclk_off"); > + if (IS_ERR_OR_NULL(i2s->bclk_off)) { > + dev_err(&pdev->dev, "failed to find i2s bclk_off\n"); > + goto err_clk; > + } > + } > + > + i2s_pinctrl_select_bclk_off(i2s); > > dev_set_drvdata(&pdev->dev, i2s); > > -- > 2.36.1.476.g0c4daa206d-goog > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO 2022-06-21 18:02 ` Chen-Yu Tsai @ 2022-06-22 10:52 ` Mark Brown 0 siblings, 0 replies; 9+ messages in thread From: Mark Brown @ 2022-06-22 10:52 UTC (permalink / raw) To: Chen-Yu Tsai Cc: Judy Hsiao, Heiko Stuebner, Liam Girdwood, Rob Herring, Brian Norris, Jaroslav Kysela, Chen-Yu Tsai, Linux-ALSA, linux-arm-kernel, open list:ARM/Rockchip SoC..., linux-kernel [-- Attachment #1: Type: text/plain, Size: 871 bytes --] On Wed, Jun 22, 2022 at 02:02:41AM +0800, Chen-Yu Tsai wrote: > Hi, > > On Sun, Jun 19, 2022 at 5:54 PM Judy Hsiao <judyhsiao@chromium.org> wrote: > > > > We discoverd 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: > > 1. prevent BCLK from turning on by other component. > > 2. keep BCLK and LRCLK being present at the same time > > > > This patch switches BCLK to GPIO func before LRCLK output, and Please delete unneeded context from mails when replying. Doing this makes it much easier to find your reply in the message, helping ensure it won't be missed by people scrolling through the irrelevant quoted material. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4 2/3] arm64: dts: rk3399: i2s: switch BCLK to GPIO 2022-06-19 9:53 [PATCH v4 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO Judy Hsiao 2022-06-19 9:53 ` [PATCH v4 1/3] " Judy Hsiao @ 2022-06-19 9:53 ` Judy Hsiao 2022-06-19 9:53 ` [PATCH v4 3/3] ASoC: dt-bindings: rockchip: Document pinctrl-names for i2s Judy Hsiao 2022-06-21 16:59 ` (subset) [PATCH v4 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO Mark Brown 3 siblings, 0 replies; 9+ messages in thread From: Judy Hsiao @ 2022-06-19 9:53 UTC (permalink / raw) To: Heiko Stuebner Cc: Liam Girdwood, Rob Herring, Brian Norris, Mark Brown, Jaroslav Kysela, Chen-Yu Tsai, alsa-devel, linux-arm-kernel, linux-rockchip, linux-kernel, Judy Hsiao We discoverd 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: 1. prevent BCLK from turning on by other component. 2. keep BCLK and LRCLK being present at the same time This patch adjusts the device tree to allow BCLK to switch to GPIO func before LRCLK output, and switch back during LRCLK is output. Signed-off-by: Judy Hsiao <judyhsiao@chromium.org> Reviewed-by: Brian Norris <briannorris@chromium.org> --- .../boot/dts/rockchip/rk3399-gru-scarlet.dtsi | 10 ++++++++ arch/arm64/boot/dts/rockchip/rk3399.dtsi | 25 ++++++++++++++++++- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi index 913d845eb51a..df1647e9d487 100644 --- a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi @@ -766,6 +766,16 @@ &i2s0_8ch_bus { <4 RK_PA0 1 &pcfg_pull_none_6ma>; }; +&i2s0_8ch_bus_bclk_off { + rockchip,pins = + <3 RK_PD0 RK_FUNC_GPIO &pcfg_pull_none_6ma>, + <3 RK_PD1 1 &pcfg_pull_none_6ma>, + <3 RK_PD2 1 &pcfg_pull_none_6ma>, + <3 RK_PD3 1 &pcfg_pull_none_6ma>, + <3 RK_PD7 1 &pcfg_pull_none_6ma>, + <4 RK_PA0 1 &pcfg_pull_none_6ma>; +}; + /* there is no external pull up, so need to set this pin pull up */ &sdmmc_cd_pin { rockchip,pins = <1 RK_PB3 RK_FUNC_GPIO &pcfg_pull_up>; diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi index fbd0346624e6..311c8394cc84 100644 --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi @@ -1662,8 +1662,9 @@ i2s0: i2s@ff880000 { dma-names = "tx", "rx"; clock-names = "i2s_clk", "i2s_hclk"; clocks = <&cru SCLK_I2S0_8CH>, <&cru HCLK_I2S0_8CH>; - pinctrl-names = "default"; + pinctrl-names = "bclk_on", "bclk_off"; pinctrl-0 = <&i2s0_8ch_bus>; + pinctrl-1 = <&i2s0_8ch_bus_bclk_off>; power-domains = <&power RK3399_PD_SDIOAUDIO>; #sound-dai-cells = <0>; status = "disabled"; @@ -2407,6 +2408,19 @@ i2s0_8ch_bus: i2s0-8ch-bus { <3 RK_PD7 1 &pcfg_pull_none>, <4 RK_PA0 1 &pcfg_pull_none>; }; + + i2s0_8ch_bus_bclk_off: i2s0-8ch-bus-bclk-off { + rockchip,pins = + <3 RK_PD0 RK_FUNC_GPIO &pcfg_pull_none>, + <3 RK_PD1 1 &pcfg_pull_none>, + <3 RK_PD2 1 &pcfg_pull_none>, + <3 RK_PD3 1 &pcfg_pull_none>, + <3 RK_PD4 1 &pcfg_pull_none>, + <3 RK_PD5 1 &pcfg_pull_none>, + <3 RK_PD6 1 &pcfg_pull_none>, + <3 RK_PD7 1 &pcfg_pull_none>, + <4 RK_PA0 1 &pcfg_pull_none>; + }; }; i2s1 { @@ -2418,6 +2432,15 @@ i2s1_2ch_bus: i2s1-2ch-bus { <4 RK_PA6 1 &pcfg_pull_none>, <4 RK_PA7 1 &pcfg_pull_none>; }; + + i2s1_2ch_bus_bclk_off: i2s1-2ch-bus-bclk-off { + rockchip,pins = + <4 RK_PA3 RK_FUNC_GPIO &pcfg_pull_none>, + <4 RK_PA4 1 &pcfg_pull_none>, + <4 RK_PA5 1 &pcfg_pull_none>, + <4 RK_PA6 1 &pcfg_pull_none>, + <4 RK_PA7 1 &pcfg_pull_none>; + }; }; sdio0 { -- 2.36.1.476.g0c4daa206d-goog ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 3/3] ASoC: dt-bindings: rockchip: Document pinctrl-names for i2s 2022-06-19 9:53 [PATCH v4 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO Judy Hsiao 2022-06-19 9:53 ` [PATCH v4 1/3] " Judy Hsiao 2022-06-19 9:53 ` [PATCH v4 2/3] arm64: dts: rk3399: " Judy Hsiao @ 2022-06-19 9:53 ` Judy Hsiao 2022-06-21 16:59 ` (subset) [PATCH v4 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO Mark Brown 3 siblings, 0 replies; 9+ messages in thread From: Judy Hsiao @ 2022-06-19 9:53 UTC (permalink / raw) To: Heiko Stuebner Cc: Liam Girdwood, Rob Herring, Brian Norris, Mark Brown, Jaroslav Kysela, Chen-Yu Tsai, alsa-devel, linux-arm-kernel, linux-rockchip, linux-kernel, Judy Hsiao This patch documents pinctrl-names for i2s. Signed-off-by: Judy Hsiao <judyhsiao@chromium.org> --- Documentation/devicetree/bindings/sound/rockchip-i2s.yaml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml b/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml index 5ea16b8ef93f..7e36e389e976 100644 --- a/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml +++ b/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml @@ -61,6 +61,13 @@ properties: - const: tx - const: rx + pinctrl-names: + oneOf: + - const: default + - items: + - const: bclk_on + - const: bclk_off + power-domains: maxItems: 1 -- 2.36.1.476.g0c4daa206d-goog ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: (subset) [PATCH v4 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO 2022-06-19 9:53 [PATCH v4 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO Judy Hsiao ` (2 preceding siblings ...) 2022-06-19 9:53 ` [PATCH v4 3/3] ASoC: dt-bindings: rockchip: Document pinctrl-names for i2s Judy Hsiao @ 2022-06-21 16:59 ` Mark Brown 2022-06-21 17:58 ` Chen-Yu Tsai 3 siblings, 1 reply; 9+ messages in thread From: Mark Brown @ 2022-06-21 16:59 UTC (permalink / raw) To: judyhsiao, heiko Cc: perex, linux-kernel, wenst, Liam Girdwood, linux-rockchip, linux-arm-kernel, briannorris, robh+dt, alsa-devel On Sun, 19 Jun 2022 09:53:21 +0000, Judy Hsiao wrote: > The patches series is to fix the unexpected large DC output > voltage of Max98357a that burns the speakers on the rockchip > platform when BCLK and SD_MODE are ON but LRCLK is OFF. > > Changes Since V4: > -- Fix indentation in the driver. (Align parameters with the parenthesis > placement.) > -- Fix incorrect return type of rockchip_snd_rxctrl. > Changes Since V3: > -- Fix indentation in the documentation. > -- Put pinctrl-1 right after pinctrl-0 in dtsi. > -- Fix indentation in the driver. > -- Remove unnecessary dev_dbg() in the driver. > Changes Since V2: > -- Add documents of i2s pinctrl-names. > -- Fix dtsi syntax error. > -- Include the dtsi change and the driver change in the same series. > -- Ensure that driver gets both bclk_on and bclk_off states before using them. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/3] ASoC: rockchip: i2s: switch BCLK to GPIO commit: 44f362c2cc6dd0c5e3cb499c4fb4ed45b63a6196 [3/3] ASoC: dt-bindings: rockchip: Document pinctrl-names for i2s (no commit info) All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: (subset) [PATCH v4 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO 2022-06-21 16:59 ` (subset) [PATCH v4 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO Mark Brown @ 2022-06-21 17:58 ` Chen-Yu Tsai 2022-06-22 10:53 ` Mark Brown 0 siblings, 1 reply; 9+ messages in thread From: Chen-Yu Tsai @ 2022-06-21 17:58 UTC (permalink / raw) To: Mark Brown Cc: judyhsiao, Heiko Stübner, Jaroslav Kysela, linux-kernel, wenst, Liam Girdwood, open list:ARM/Rockchip SoC..., linux-arm-kernel, briannorris, Rob Herring, Linux-ALSA Hi Mark, On Wed, Jun 22, 2022 at 1:00 AM Mark Brown <broonie@kernel.org> wrote: > > On Sun, 19 Jun 2022 09:53:21 +0000, Judy Hsiao wrote: > > The patches series is to fix the unexpected large DC output > > voltage of Max98357a that burns the speakers on the rockchip > > platform when BCLK and SD_MODE are ON but LRCLK is OFF. > > > > Changes Since V4: > > -- Fix indentation in the driver. (Align parameters with the parenthesis > > placement.) > > -- Fix incorrect return type of rockchip_snd_rxctrl. > > Changes Since V3: > > -- Fix indentation in the documentation. > > -- Put pinctrl-1 right after pinctrl-0 in dtsi. > > -- Fix indentation in the driver. > > -- Remove unnecessary dev_dbg() in the driver. > > Changes Since V2: > > -- Add documents of i2s pinctrl-names. > > -- Fix dtsi syntax error. > > -- Include the dtsi change and the driver change in the same series. > > -- Ensure that driver gets both bclk_on and bclk_off states before using them. > > > > [...] > > Applied to > > https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next > > Thanks! > > [1/3] ASoC: rockchip: i2s: switch BCLK to GPIO > commit: 44f362c2cc6dd0c5e3cb499c4fb4ed45b63a6196 Seems you applied v1 instead of v4? Either way, this causes the driver to crash for any I2S node that doesn't have pinctrl properties set. Notably, the I2S controller that sends audio to the HDMI controller will never have pinctrl properties, since the connection is internal. ChenYu > [3/3] ASoC: dt-bindings: rockchip: Document pinctrl-names for i2s > (no commit info) > > All being well this means that it will be integrated into the linux-next > tree (usually sometime in the next 24 hours) and sent to Linus during > the next merge window (or sooner if it is a bug fix), however if > problems are discovered then the patch may be dropped or reverted. > > You may get further e-mails resulting from automated or manual testing > and review of the tree, please engage with people reporting problems and > send followup patches addressing any issues that are reported if needed. > > If any updates are required or you are submitting further changes they > should be sent as incremental updates against current git, existing > patches will not be replaced. > > Please add any relevant lists and maintainers to the CCs when replying > to this mail. > > Thanks, > Mark > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: (subset) [PATCH v4 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO 2022-06-21 17:58 ` Chen-Yu Tsai @ 2022-06-22 10:53 ` Mark Brown 0 siblings, 0 replies; 9+ messages in thread From: Mark Brown @ 2022-06-22 10:53 UTC (permalink / raw) To: Chen-Yu Tsai Cc: judyhsiao, Heiko Stübner, Jaroslav Kysela, linux-kernel, wenst, Liam Girdwood, open list:ARM/Rockchip SoC..., linux-arm-kernel, briannorris, Rob Herring, Linux-ALSA [-- Attachment #1: Type: text/plain, Size: 444 bytes --] On Wed, Jun 22, 2022 at 01:58:40AM +0800, Chen-Yu Tsai wrote: > On Wed, Jun 22, 2022 at 1:00 AM Mark Brown <broonie@kernel.org> wrote: > > On Sun, 19 Jun 2022 09:53:21 +0000, Judy Hsiao wrote: > > [1/3] ASoC: rockchip: i2s: switch BCLK to GPIO > > commit: 44f362c2cc6dd0c5e3cb499c4fb4ed45b63a6196 > Seems you applied v1 instead of v4? b4 will reply to anything it knows about that thinks is close enough to something that got applied. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-06-22 10:54 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-06-19 9:53 [PATCH v4 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO Judy Hsiao 2022-06-19 9:53 ` [PATCH v4 1/3] " Judy Hsiao 2022-06-21 18:02 ` Chen-Yu Tsai 2022-06-22 10:52 ` Mark Brown 2022-06-19 9:53 ` [PATCH v4 2/3] arm64: dts: rk3399: " Judy Hsiao 2022-06-19 9:53 ` [PATCH v4 3/3] ASoC: dt-bindings: rockchip: Document pinctrl-names for i2s Judy Hsiao 2022-06-21 16:59 ` (subset) [PATCH v4 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO Mark Brown 2022-06-21 17:58 ` Chen-Yu Tsai 2022-06-22 10:53 ` Mark Brown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).