* [PATCH v2 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-16 15:58 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-16 15:58 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 to burn the speakers
on rockchip platform when BCLK and SD_MODE are ON but LRCLK is OFF.
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 | 134 ++++++++++++++----
4 files changed, 149 insertions(+), 27 deletions(-)
--
2.36.1.476.g0c4daa206d-goog
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v2 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-16 15:58 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-16 15:58 UTC (permalink / raw)
To: Heiko Stuebner
Cc: alsa-devel, linux-kernel, Brian Norris, Liam Girdwood,
Rob Herring, linux-rockchip, Mark Brown, Chen-Yu Tsai,
Judy Hsiao, linux-arm-kernel
The patches series is to fix the unexpected large
DC output voltage of Max98357a to burn the speakers
on rockchip platform when BCLK and SD_MODE are ON but LRCLK is OFF.
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 | 134 ++++++++++++++----
4 files changed, 149 insertions(+), 27 deletions(-)
--
2.36.1.476.g0c4daa206d-goog
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v2 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-16 15:58 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-16 15:58 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 to burn the speakers
on rockchip platform when BCLK and SD_MODE are ON but LRCLK is OFF.
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 | 134 ++++++++++++++----
4 files changed, 149 insertions(+), 27 deletions(-)
--
2.36.1.476.g0c4daa206d-goog
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v2 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-16 15:58 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-16 15:58 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 to burn the speakers
on rockchip platform when BCLK and SD_MODE are ON but LRCLK is OFF.
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 | 134 ++++++++++++++----
4 files changed, 149 insertions(+), 27 deletions(-)
--
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] 60+ messages in thread
* [PATCH v2 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO
2022-06-16 15:58 ` Judy Hsiao
(?)
(?)
@ 2022-06-16 15:58 ` Judy Hsiao
-1 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-16 15:58 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>
---
sound/soc/rockchip/rockchip_i2s.c | 134 ++++++++++++++++++++++++------
1 file changed, 108 insertions(+), 26 deletions(-)
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
index 47a3971a9ce1..574d3d0900c4 100644
--- a/sound/soc/rockchip/rockchip_i2s.c
+++ b/sound/soc/rockchip/rockchip_i2s.c
@@ -54,8 +54,40 @@ 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 +124,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,
+ ret = regmap_update_bits(i2s->regmap, I2S_DMACR,
I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_ENABLE);
-
- regmap_update_bits(i2s->regmap, I2S_XFER,
+ 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,
+ 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,
+ 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,
+ 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 +177,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)
{
unsigned int val = 0;
int retry = 10;
+ int ret = 0;
spin_lock(&i2s->lock);
if (on) {
- regmap_update_bits(i2s->regmap, I2S_DMACR,
+ ret = regmap_update_bits(i2s->regmap, I2S_DMACR,
I2S_DMACR_RDE_ENABLE, I2S_DMACR_RDE_ENABLE);
+ if (ret < 0)
+ goto end;
- regmap_update_bits(i2s->regmap, I2S_XFER,
+ 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,
+ 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,
+ ret = regmap_update_bits(i2s->regmap, I2S_XFER,
I2S_XFER_TXS_START |
- I2S_XFER_RXS_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,
+ 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 +239,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 +482,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 +801,23 @@ 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)) {
+ dev_info(&pdev->dev, "switch bclk to GPIO func\n");
+ 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] 60+ messages in thread
* [PATCH v2 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-16 15:58 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-16 15:58 UTC (permalink / raw)
To: Heiko Stuebner
Cc: alsa-devel, linux-kernel, Brian Norris, Liam Girdwood,
Rob Herring, linux-rockchip, Mark Brown, Chen-Yu Tsai,
Judy Hsiao, linux-arm-kernel
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>
---
sound/soc/rockchip/rockchip_i2s.c | 134 ++++++++++++++++++++++++------
1 file changed, 108 insertions(+), 26 deletions(-)
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
index 47a3971a9ce1..574d3d0900c4 100644
--- a/sound/soc/rockchip/rockchip_i2s.c
+++ b/sound/soc/rockchip/rockchip_i2s.c
@@ -54,8 +54,40 @@ 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 +124,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,
+ ret = regmap_update_bits(i2s->regmap, I2S_DMACR,
I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_ENABLE);
-
- regmap_update_bits(i2s->regmap, I2S_XFER,
+ 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,
+ 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,
+ 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,
+ 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 +177,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)
{
unsigned int val = 0;
int retry = 10;
+ int ret = 0;
spin_lock(&i2s->lock);
if (on) {
- regmap_update_bits(i2s->regmap, I2S_DMACR,
+ ret = regmap_update_bits(i2s->regmap, I2S_DMACR,
I2S_DMACR_RDE_ENABLE, I2S_DMACR_RDE_ENABLE);
+ if (ret < 0)
+ goto end;
- regmap_update_bits(i2s->regmap, I2S_XFER,
+ 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,
+ 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,
+ ret = regmap_update_bits(i2s->regmap, I2S_XFER,
I2S_XFER_TXS_START |
- I2S_XFER_RXS_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,
+ 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 +239,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 +482,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 +801,23 @@ 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)) {
+ dev_info(&pdev->dev, "switch bclk to GPIO func\n");
+ 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] 60+ messages in thread
* [PATCH v2 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-16 15:58 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-16 15:58 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>
---
sound/soc/rockchip/rockchip_i2s.c | 134 ++++++++++++++++++++++++------
1 file changed, 108 insertions(+), 26 deletions(-)
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
index 47a3971a9ce1..574d3d0900c4 100644
--- a/sound/soc/rockchip/rockchip_i2s.c
+++ b/sound/soc/rockchip/rockchip_i2s.c
@@ -54,8 +54,40 @@ 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 +124,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,
+ ret = regmap_update_bits(i2s->regmap, I2S_DMACR,
I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_ENABLE);
-
- regmap_update_bits(i2s->regmap, I2S_XFER,
+ 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,
+ 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,
+ 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,
+ 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 +177,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)
{
unsigned int val = 0;
int retry = 10;
+ int ret = 0;
spin_lock(&i2s->lock);
if (on) {
- regmap_update_bits(i2s->regmap, I2S_DMACR,
+ ret = regmap_update_bits(i2s->regmap, I2S_DMACR,
I2S_DMACR_RDE_ENABLE, I2S_DMACR_RDE_ENABLE);
+ if (ret < 0)
+ goto end;
- regmap_update_bits(i2s->regmap, I2S_XFER,
+ 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,
+ 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,
+ ret = regmap_update_bits(i2s->regmap, I2S_XFER,
I2S_XFER_TXS_START |
- I2S_XFER_RXS_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,
+ 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 +239,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 +482,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 +801,23 @@ 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)) {
+ dev_info(&pdev->dev, "switch bclk to GPIO func\n");
+ 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-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v2 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-16 15:58 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-16 15:58 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>
---
sound/soc/rockchip/rockchip_i2s.c | 134 ++++++++++++++++++++++++------
1 file changed, 108 insertions(+), 26 deletions(-)
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
index 47a3971a9ce1..574d3d0900c4 100644
--- a/sound/soc/rockchip/rockchip_i2s.c
+++ b/sound/soc/rockchip/rockchip_i2s.c
@@ -54,8 +54,40 @@ 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 +124,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,
+ ret = regmap_update_bits(i2s->regmap, I2S_DMACR,
I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_ENABLE);
-
- regmap_update_bits(i2s->regmap, I2S_XFER,
+ 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,
+ 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,
+ 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,
+ 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 +177,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)
{
unsigned int val = 0;
int retry = 10;
+ int ret = 0;
spin_lock(&i2s->lock);
if (on) {
- regmap_update_bits(i2s->regmap, I2S_DMACR,
+ ret = regmap_update_bits(i2s->regmap, I2S_DMACR,
I2S_DMACR_RDE_ENABLE, I2S_DMACR_RDE_ENABLE);
+ if (ret < 0)
+ goto end;
- regmap_update_bits(i2s->regmap, I2S_XFER,
+ 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,
+ 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,
+ ret = regmap_update_bits(i2s->regmap, I2S_XFER,
I2S_XFER_TXS_START |
- I2S_XFER_RXS_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,
+ 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 +239,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 +482,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 +801,23 @@ 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)) {
+ dev_info(&pdev->dev, "switch bclk to GPIO func\n");
+ 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 related [flat|nested] 60+ messages in thread
* [PATCH v2 2/3] arm64: dts: rk3399: i2s: switch BCLK to GPIO
2022-06-16 15:58 ` Judy Hsiao
(?)
(?)
@ 2022-06-16 15:58 ` Judy Hsiao
-1 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-16 15:58 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>
---
.../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..3981dec6a5a5 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1662,9 +1662,10 @@ 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>;
power-domains = <&power RK3399_PD_SDIOAUDIO>;
+ pinctrl-1 = <&i2s0_8ch_bus_bclk_off>;
#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] 60+ messages in thread
* [PATCH v2 2/3] arm64: dts: rk3399: i2s: switch BCLK to GPIO
@ 2022-06-16 15:58 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-16 15:58 UTC (permalink / raw)
To: Heiko Stuebner
Cc: alsa-devel, linux-kernel, Brian Norris, Liam Girdwood,
Rob Herring, linux-rockchip, Mark Brown, Chen-Yu Tsai,
Judy Hsiao, linux-arm-kernel
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>
---
.../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..3981dec6a5a5 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1662,9 +1662,10 @@ 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>;
power-domains = <&power RK3399_PD_SDIOAUDIO>;
+ pinctrl-1 = <&i2s0_8ch_bus_bclk_off>;
#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] 60+ messages in thread
* [PATCH v2 2/3] arm64: dts: rk3399: i2s: switch BCLK to GPIO
@ 2022-06-16 15:58 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-16 15:58 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>
---
.../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..3981dec6a5a5 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1662,9 +1662,10 @@ 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>;
power-domains = <&power RK3399_PD_SDIOAUDIO>;
+ pinctrl-1 = <&i2s0_8ch_bus_bclk_off>;
#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
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v2 2/3] arm64: dts: rk3399: i2s: switch BCLK to GPIO
@ 2022-06-16 15:58 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-16 15:58 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>
---
.../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..3981dec6a5a5 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1662,9 +1662,10 @@ 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>;
power-domains = <&power RK3399_PD_SDIOAUDIO>;
+ pinctrl-1 = <&i2s0_8ch_bus_bclk_off>;
#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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v2 3/3] ASoC: dt-bindings: rockchip: Document pinctrl-names for i2s
2022-06-16 15:58 ` Judy Hsiao
(?)
(?)
@ 2022-06-16 15:58 ` Judy Hsiao
-1 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-16 15:58 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..af1069278623 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] 60+ messages in thread
* [PATCH v2 3/3] ASoC: dt-bindings: rockchip: Document pinctrl-names for i2s
@ 2022-06-16 15:58 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-16 15:58 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..af1069278623 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
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v2 3/3] ASoC: dt-bindings: rockchip: Document pinctrl-names for i2s
@ 2022-06-16 15:58 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-16 15:58 UTC (permalink / raw)
To: Heiko Stuebner
Cc: alsa-devel, linux-kernel, Brian Norris, Liam Girdwood,
Rob Herring, linux-rockchip, Mark Brown, Chen-Yu Tsai,
Judy Hsiao, linux-arm-kernel
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..af1069278623 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] 60+ messages in thread
* [PATCH v2 3/3] ASoC: dt-bindings: rockchip: Document pinctrl-names for i2s
@ 2022-06-16 15:58 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-16 15:58 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..af1069278623 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 60+ messages in thread
* Re: [PATCH v2 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO
2022-06-16 15:58 ` Judy Hsiao
(?)
(?)
@ 2022-06-16 19:03 ` kernel test robot
-1 siblings, 0 replies; 60+ messages in thread
From: kernel test robot @ 2022-06-16 19:03 UTC (permalink / raw)
To: Judy Hsiao, Heiko Stuebner
Cc: kbuild-all, Liam Girdwood, Rob Herring, Brian Norris, Mark Brown,
Jaroslav Kysela, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-rockchip, linux-kernel, Judy Hsiao
Hi Judy,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on rockchip/for-next]
[also build test ERROR on broonie-sound/for-next robh/for-next linus/master v5.19-rc2 next-20220616]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Judy-Hsiao/ASoC-rockchip-i2s-switch-BCLK-to-GPIO/20220617-000041
base: https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git for-next
config: arc-randconfig-r043-20220616 (https://download.01.org/0day-ci/archive/20220617/202206170216.M52eiJwN-lkp@intel.com/config)
compiler: arc-elf-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/322064314e50b113217aa4c7412e2431defea08f
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Judy-Hsiao/ASoC-rockchip-i2s-switch-BCLK-to-GPIO/20220617-000041
git checkout 322064314e50b113217aa4c7412e2431defea08f
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash sound/soc/rockchip/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
sound/soc/rockchip/rockchip_i2s.c: In function 'rockchip_snd_rxctrl':
sound/soc/rockchip/rockchip_i2s.c:247:16: error: 'return' with a value, in function returning void [-Werror=return-type]
247 | return ret;
| ^~~
sound/soc/rockchip/rockchip_i2s.c:188:13: note: declared here
188 | static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on)
| ^~~~~~~~~~~~~~~~~~~
sound/soc/rockchip/rockchip_i2s.c: In function 'rockchip_i2s_trigger':
>> sound/soc/rockchip/rockchip_i2s.c:485:29: error: void value not ignored as it ought to be
485 | ret = rockchip_snd_rxctrl(i2s, 1);
| ^
sound/soc/rockchip/rockchip_i2s.c:498:29: error: void value not ignored as it ought to be
498 | ret = rockchip_snd_rxctrl(i2s, 0);
| ^
cc1: some warnings being treated as errors
vim +485 sound/soc/rockchip/rockchip_i2s.c
473
474 static int rockchip_i2s_trigger(struct snd_pcm_substream *substream,
475 int cmd, struct snd_soc_dai *dai)
476 {
477 struct rk_i2s_dev *i2s = to_info(dai);
478 int ret = 0;
479
480 switch (cmd) {
481 case SNDRV_PCM_TRIGGER_START:
482 case SNDRV_PCM_TRIGGER_RESUME:
483 case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
484 if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
> 485 ret = rockchip_snd_rxctrl(i2s, 1);
486 else
487 ret = rockchip_snd_txctrl(i2s, 1);
488 if (ret < 0)
489 return ret;
490 i2s_pinctrl_select_bclk_on(i2s);
491 break;
492 case SNDRV_PCM_TRIGGER_SUSPEND:
493 case SNDRV_PCM_TRIGGER_STOP:
494 case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
495 if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
496 if (!i2s->tx_start)
497 i2s_pinctrl_select_bclk_off(i2s);
498 ret = rockchip_snd_rxctrl(i2s, 0);
499 } else {
500 if (!i2s->rx_start)
501 i2s_pinctrl_select_bclk_off(i2s);
502 ret = rockchip_snd_txctrl(i2s, 0);
503 }
504 break;
505 default:
506 ret = -EINVAL;
507 break;
508 }
509
510 return ret;
511 }
512
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-16 19:03 ` kernel test robot
0 siblings, 0 replies; 60+ messages in thread
From: kernel test robot @ 2022-06-16 19:03 UTC (permalink / raw)
To: Judy Hsiao, Heiko Stuebner
Cc: alsa-devel, kbuild-all, linux-kernel, Brian Norris,
Liam Girdwood, Rob Herring, linux-rockchip, Mark Brown,
Chen-Yu Tsai, Judy Hsiao, linux-arm-kernel
Hi Judy,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on rockchip/for-next]
[also build test ERROR on broonie-sound/for-next robh/for-next linus/master v5.19-rc2 next-20220616]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Judy-Hsiao/ASoC-rockchip-i2s-switch-BCLK-to-GPIO/20220617-000041
base: https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git for-next
config: arc-randconfig-r043-20220616 (https://download.01.org/0day-ci/archive/20220617/202206170216.M52eiJwN-lkp@intel.com/config)
compiler: arc-elf-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/322064314e50b113217aa4c7412e2431defea08f
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Judy-Hsiao/ASoC-rockchip-i2s-switch-BCLK-to-GPIO/20220617-000041
git checkout 322064314e50b113217aa4c7412e2431defea08f
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash sound/soc/rockchip/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
sound/soc/rockchip/rockchip_i2s.c: In function 'rockchip_snd_rxctrl':
sound/soc/rockchip/rockchip_i2s.c:247:16: error: 'return' with a value, in function returning void [-Werror=return-type]
247 | return ret;
| ^~~
sound/soc/rockchip/rockchip_i2s.c:188:13: note: declared here
188 | static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on)
| ^~~~~~~~~~~~~~~~~~~
sound/soc/rockchip/rockchip_i2s.c: In function 'rockchip_i2s_trigger':
>> sound/soc/rockchip/rockchip_i2s.c:485:29: error: void value not ignored as it ought to be
485 | ret = rockchip_snd_rxctrl(i2s, 1);
| ^
sound/soc/rockchip/rockchip_i2s.c:498:29: error: void value not ignored as it ought to be
498 | ret = rockchip_snd_rxctrl(i2s, 0);
| ^
cc1: some warnings being treated as errors
vim +485 sound/soc/rockchip/rockchip_i2s.c
473
474 static int rockchip_i2s_trigger(struct snd_pcm_substream *substream,
475 int cmd, struct snd_soc_dai *dai)
476 {
477 struct rk_i2s_dev *i2s = to_info(dai);
478 int ret = 0;
479
480 switch (cmd) {
481 case SNDRV_PCM_TRIGGER_START:
482 case SNDRV_PCM_TRIGGER_RESUME:
483 case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
484 if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
> 485 ret = rockchip_snd_rxctrl(i2s, 1);
486 else
487 ret = rockchip_snd_txctrl(i2s, 1);
488 if (ret < 0)
489 return ret;
490 i2s_pinctrl_select_bclk_on(i2s);
491 break;
492 case SNDRV_PCM_TRIGGER_SUSPEND:
493 case SNDRV_PCM_TRIGGER_STOP:
494 case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
495 if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
496 if (!i2s->tx_start)
497 i2s_pinctrl_select_bclk_off(i2s);
498 ret = rockchip_snd_rxctrl(i2s, 0);
499 } else {
500 if (!i2s->rx_start)
501 i2s_pinctrl_select_bclk_off(i2s);
502 ret = rockchip_snd_txctrl(i2s, 0);
503 }
504 break;
505 default:
506 ret = -EINVAL;
507 break;
508 }
509
510 return ret;
511 }
512
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-16 19:03 ` kernel test robot
0 siblings, 0 replies; 60+ messages in thread
From: kernel test robot @ 2022-06-16 19:03 UTC (permalink / raw)
To: Judy Hsiao, Heiko Stuebner
Cc: kbuild-all, Liam Girdwood, Rob Herring, Brian Norris, Mark Brown,
Jaroslav Kysela, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-rockchip, linux-kernel, Judy Hsiao
Hi Judy,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on rockchip/for-next]
[also build test ERROR on broonie-sound/for-next robh/for-next linus/master v5.19-rc2 next-20220616]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Judy-Hsiao/ASoC-rockchip-i2s-switch-BCLK-to-GPIO/20220617-000041
base: https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git for-next
config: arc-randconfig-r043-20220616 (https://download.01.org/0day-ci/archive/20220617/202206170216.M52eiJwN-lkp@intel.com/config)
compiler: arc-elf-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/322064314e50b113217aa4c7412e2431defea08f
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Judy-Hsiao/ASoC-rockchip-i2s-switch-BCLK-to-GPIO/20220617-000041
git checkout 322064314e50b113217aa4c7412e2431defea08f
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash sound/soc/rockchip/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
sound/soc/rockchip/rockchip_i2s.c: In function 'rockchip_snd_rxctrl':
sound/soc/rockchip/rockchip_i2s.c:247:16: error: 'return' with a value, in function returning void [-Werror=return-type]
247 | return ret;
| ^~~
sound/soc/rockchip/rockchip_i2s.c:188:13: note: declared here
188 | static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on)
| ^~~~~~~~~~~~~~~~~~~
sound/soc/rockchip/rockchip_i2s.c: In function 'rockchip_i2s_trigger':
>> sound/soc/rockchip/rockchip_i2s.c:485:29: error: void value not ignored as it ought to be
485 | ret = rockchip_snd_rxctrl(i2s, 1);
| ^
sound/soc/rockchip/rockchip_i2s.c:498:29: error: void value not ignored as it ought to be
498 | ret = rockchip_snd_rxctrl(i2s, 0);
| ^
cc1: some warnings being treated as errors
vim +485 sound/soc/rockchip/rockchip_i2s.c
473
474 static int rockchip_i2s_trigger(struct snd_pcm_substream *substream,
475 int cmd, struct snd_soc_dai *dai)
476 {
477 struct rk_i2s_dev *i2s = to_info(dai);
478 int ret = 0;
479
480 switch (cmd) {
481 case SNDRV_PCM_TRIGGER_START:
482 case SNDRV_PCM_TRIGGER_RESUME:
483 case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
484 if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
> 485 ret = rockchip_snd_rxctrl(i2s, 1);
486 else
487 ret = rockchip_snd_txctrl(i2s, 1);
488 if (ret < 0)
489 return ret;
490 i2s_pinctrl_select_bclk_on(i2s);
491 break;
492 case SNDRV_PCM_TRIGGER_SUSPEND:
493 case SNDRV_PCM_TRIGGER_STOP:
494 case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
495 if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
496 if (!i2s->tx_start)
497 i2s_pinctrl_select_bclk_off(i2s);
498 ret = rockchip_snd_rxctrl(i2s, 0);
499 } else {
500 if (!i2s->rx_start)
501 i2s_pinctrl_select_bclk_off(i2s);
502 ret = rockchip_snd_txctrl(i2s, 0);
503 }
504 break;
505 default:
506 ret = -EINVAL;
507 break;
508 }
509
510 return ret;
511 }
512
--
0-DAY CI Kernel Test Service
https://01.org/lkp
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-16 19:03 ` kernel test robot
0 siblings, 0 replies; 60+ messages in thread
From: kernel test robot @ 2022-06-16 19:03 UTC (permalink / raw)
To: Judy Hsiao, Heiko Stuebner
Cc: kbuild-all, Liam Girdwood, Rob Herring, Brian Norris, Mark Brown,
Jaroslav Kysela, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-rockchip, linux-kernel, Judy Hsiao
Hi Judy,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on rockchip/for-next]
[also build test ERROR on broonie-sound/for-next robh/for-next linus/master v5.19-rc2 next-20220616]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Judy-Hsiao/ASoC-rockchip-i2s-switch-BCLK-to-GPIO/20220617-000041
base: https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git for-next
config: arc-randconfig-r043-20220616 (https://download.01.org/0day-ci/archive/20220617/202206170216.M52eiJwN-lkp@intel.com/config)
compiler: arc-elf-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/322064314e50b113217aa4c7412e2431defea08f
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Judy-Hsiao/ASoC-rockchip-i2s-switch-BCLK-to-GPIO/20220617-000041
git checkout 322064314e50b113217aa4c7412e2431defea08f
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash sound/soc/rockchip/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
sound/soc/rockchip/rockchip_i2s.c: In function 'rockchip_snd_rxctrl':
sound/soc/rockchip/rockchip_i2s.c:247:16: error: 'return' with a value, in function returning void [-Werror=return-type]
247 | return ret;
| ^~~
sound/soc/rockchip/rockchip_i2s.c:188:13: note: declared here
188 | static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on)
| ^~~~~~~~~~~~~~~~~~~
sound/soc/rockchip/rockchip_i2s.c: In function 'rockchip_i2s_trigger':
>> sound/soc/rockchip/rockchip_i2s.c:485:29: error: void value not ignored as it ought to be
485 | ret = rockchip_snd_rxctrl(i2s, 1);
| ^
sound/soc/rockchip/rockchip_i2s.c:498:29: error: void value not ignored as it ought to be
498 | ret = rockchip_snd_rxctrl(i2s, 0);
| ^
cc1: some warnings being treated as errors
vim +485 sound/soc/rockchip/rockchip_i2s.c
473
474 static int rockchip_i2s_trigger(struct snd_pcm_substream *substream,
475 int cmd, struct snd_soc_dai *dai)
476 {
477 struct rk_i2s_dev *i2s = to_info(dai);
478 int ret = 0;
479
480 switch (cmd) {
481 case SNDRV_PCM_TRIGGER_START:
482 case SNDRV_PCM_TRIGGER_RESUME:
483 case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
484 if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
> 485 ret = rockchip_snd_rxctrl(i2s, 1);
486 else
487 ret = rockchip_snd_txctrl(i2s, 1);
488 if (ret < 0)
489 return ret;
490 i2s_pinctrl_select_bclk_on(i2s);
491 break;
492 case SNDRV_PCM_TRIGGER_SUSPEND:
493 case SNDRV_PCM_TRIGGER_STOP:
494 case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
495 if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
496 if (!i2s->tx_start)
497 i2s_pinctrl_select_bclk_off(i2s);
498 ret = rockchip_snd_rxctrl(i2s, 0);
499 } else {
500 if (!i2s->rx_start)
501 i2s_pinctrl_select_bclk_off(i2s);
502 ret = rockchip_snd_txctrl(i2s, 0);
503 }
504 break;
505 default:
506 ret = -EINVAL;
507 break;
508 }
509
510 return ret;
511 }
512
--
0-DAY CI Kernel Test Service
https://01.org/lkp
_______________________________________________
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] 60+ messages in thread
* Re: [PATCH v2 3/3] ASoC: dt-bindings: rockchip: Document pinctrl-names for i2s
2022-06-16 15:58 ` Judy Hsiao
(?)
(?)
@ 2022-06-16 20:39 ` Brian Norris
-1 siblings, 0 replies; 60+ messages in thread
From: Brian Norris @ 2022-06-16 20:39 UTC (permalink / raw)
To: Judy Hsiao
Cc: Heiko Stuebner, Liam Girdwood, Rob Herring, Mark Brown,
Jaroslav Kysela, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-rockchip, linux-kernel
On Thu, Jun 16, 2022 at 03:58:36PM +0000, Judy Hsiao wrote:
> 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..af1069278623 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:
I'm far from a YAML expert, but I think you have the indentation wrong?
You want the "const: default" and the "items:" schema to be on the same
level of indentation, right?
If you haven't already, you might run through the 'make
dt_binding_check' process.
Brian
> + - const: bclk_on
> + - const: bclk_off
> +
> power-domains:
> maxItems: 1
>
> --
> 2.36.1.476.g0c4daa206d-goog
>
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 3/3] ASoC: dt-bindings: rockchip: Document pinctrl-names for i2s
@ 2022-06-16 20:39 ` Brian Norris
0 siblings, 0 replies; 60+ messages in thread
From: Brian Norris @ 2022-06-16 20:39 UTC (permalink / raw)
To: Judy Hsiao
Cc: alsa-devel, Heiko Stuebner, linux-kernel, Liam Girdwood,
Rob Herring, linux-rockchip, Mark Brown, Chen-Yu Tsai,
linux-arm-kernel
On Thu, Jun 16, 2022 at 03:58:36PM +0000, Judy Hsiao wrote:
> 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..af1069278623 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:
I'm far from a YAML expert, but I think you have the indentation wrong?
You want the "const: default" and the "items:" schema to be on the same
level of indentation, right?
If you haven't already, you might run through the 'make
dt_binding_check' process.
Brian
> + - const: bclk_on
> + - const: bclk_off
> +
> power-domains:
> maxItems: 1
>
> --
> 2.36.1.476.g0c4daa206d-goog
>
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 3/3] ASoC: dt-bindings: rockchip: Document pinctrl-names for i2s
@ 2022-06-16 20:39 ` Brian Norris
0 siblings, 0 replies; 60+ messages in thread
From: Brian Norris @ 2022-06-16 20:39 UTC (permalink / raw)
To: Judy Hsiao
Cc: Heiko Stuebner, Liam Girdwood, Rob Herring, Mark Brown,
Jaroslav Kysela, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-rockchip, linux-kernel
On Thu, Jun 16, 2022 at 03:58:36PM +0000, Judy Hsiao wrote:
> 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..af1069278623 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:
I'm far from a YAML expert, but I think you have the indentation wrong?
You want the "const: default" and the "items:" schema to be on the same
level of indentation, right?
If you haven't already, you might run through the 'make
dt_binding_check' process.
Brian
> + - const: bclk_on
> + - const: bclk_off
> +
> power-domains:
> maxItems: 1
>
> --
> 2.36.1.476.g0c4daa206d-goog
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 3/3] ASoC: dt-bindings: rockchip: Document pinctrl-names for i2s
@ 2022-06-16 20:39 ` Brian Norris
0 siblings, 0 replies; 60+ messages in thread
From: Brian Norris @ 2022-06-16 20:39 UTC (permalink / raw)
To: Judy Hsiao
Cc: Heiko Stuebner, Liam Girdwood, Rob Herring, Mark Brown,
Jaroslav Kysela, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-rockchip, linux-kernel
On Thu, Jun 16, 2022 at 03:58:36PM +0000, Judy Hsiao wrote:
> 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..af1069278623 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:
I'm far from a YAML expert, but I think you have the indentation wrong?
You want the "const: default" and the "items:" schema to be on the same
level of indentation, right?
If you haven't already, you might run through the 'make
dt_binding_check' process.
Brian
> + - const: bclk_on
> + - const: bclk_off
> +
> power-domains:
> maxItems: 1
>
> --
> 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] 60+ messages in thread
* Re: [PATCH v2 2/3] arm64: dts: rk3399: i2s: switch BCLK to GPIO
2022-06-16 15:58 ` Judy Hsiao
(?)
(?)
@ 2022-06-16 20:39 ` Brian Norris
-1 siblings, 0 replies; 60+ messages in thread
From: Brian Norris @ 2022-06-16 20:39 UTC (permalink / raw)
To: Judy Hsiao
Cc: Heiko Stuebner, Liam Girdwood, Rob Herring, Mark Brown,
Jaroslav Kysela, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-rockchip, linux-kernel
Hi,
On Thu, Jun 16, 2022 at 03:58:35PM +0000, Judy Hsiao 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 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>
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -1662,9 +1662,10 @@ 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>;
> power-domains = <&power RK3399_PD_SDIOAUDIO>;
> + pinctrl-1 = <&i2s0_8ch_bus_bclk_off>;
It seems like 'pinctrl-1' may make sense after pinctrl-0, not here.
Perhaps you're interacting with my RFC PATCH that removes this
'power-domains' property?
https://lore.kernel.org/linux-rockchip/20220613183556.RFC.1.I9ca71105e505f024d53b7e0ba4462230813ebb8d@changeid/
But that most likely isn't landing upstream as-is.
Otherwise, this patch looks good to me:
Reviewed-by: Brian Norris <briannorris@chromium.org>
> #sound-dai-cells = <0>;
> status = "disabled";
> };
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 2/3] arm64: dts: rk3399: i2s: switch BCLK to GPIO
@ 2022-06-16 20:39 ` Brian Norris
0 siblings, 0 replies; 60+ messages in thread
From: Brian Norris @ 2022-06-16 20:39 UTC (permalink / raw)
To: Judy Hsiao
Cc: Heiko Stuebner, Liam Girdwood, Rob Herring, Mark Brown,
Jaroslav Kysela, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-rockchip, linux-kernel
Hi,
On Thu, Jun 16, 2022 at 03:58:35PM +0000, Judy Hsiao 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 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>
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -1662,9 +1662,10 @@ 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>;
> power-domains = <&power RK3399_PD_SDIOAUDIO>;
> + pinctrl-1 = <&i2s0_8ch_bus_bclk_off>;
It seems like 'pinctrl-1' may make sense after pinctrl-0, not here.
Perhaps you're interacting with my RFC PATCH that removes this
'power-domains' property?
https://lore.kernel.org/linux-rockchip/20220613183556.RFC.1.I9ca71105e505f024d53b7e0ba4462230813ebb8d@changeid/
But that most likely isn't landing upstream as-is.
Otherwise, this patch looks good to me:
Reviewed-by: Brian Norris <briannorris@chromium.org>
> #sound-dai-cells = <0>;
> status = "disabled";
> };
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 2/3] arm64: dts: rk3399: i2s: switch BCLK to GPIO
@ 2022-06-16 20:39 ` Brian Norris
0 siblings, 0 replies; 60+ messages in thread
From: Brian Norris @ 2022-06-16 20:39 UTC (permalink / raw)
To: Judy Hsiao
Cc: alsa-devel, Heiko Stuebner, linux-kernel, Liam Girdwood,
Rob Herring, linux-rockchip, Mark Brown, Chen-Yu Tsai,
linux-arm-kernel
Hi,
On Thu, Jun 16, 2022 at 03:58:35PM +0000, Judy Hsiao 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 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>
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -1662,9 +1662,10 @@ 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>;
> power-domains = <&power RK3399_PD_SDIOAUDIO>;
> + pinctrl-1 = <&i2s0_8ch_bus_bclk_off>;
It seems like 'pinctrl-1' may make sense after pinctrl-0, not here.
Perhaps you're interacting with my RFC PATCH that removes this
'power-domains' property?
https://lore.kernel.org/linux-rockchip/20220613183556.RFC.1.I9ca71105e505f024d53b7e0ba4462230813ebb8d@changeid/
But that most likely isn't landing upstream as-is.
Otherwise, this patch looks good to me:
Reviewed-by: Brian Norris <briannorris@chromium.org>
> #sound-dai-cells = <0>;
> status = "disabled";
> };
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 2/3] arm64: dts: rk3399: i2s: switch BCLK to GPIO
@ 2022-06-16 20:39 ` Brian Norris
0 siblings, 0 replies; 60+ messages in thread
From: Brian Norris @ 2022-06-16 20:39 UTC (permalink / raw)
To: Judy Hsiao
Cc: Heiko Stuebner, Liam Girdwood, Rob Herring, Mark Brown,
Jaroslav Kysela, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-rockchip, linux-kernel
Hi,
On Thu, Jun 16, 2022 at 03:58:35PM +0000, Judy Hsiao 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 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>
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -1662,9 +1662,10 @@ 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>;
> power-domains = <&power RK3399_PD_SDIOAUDIO>;
> + pinctrl-1 = <&i2s0_8ch_bus_bclk_off>;
It seems like 'pinctrl-1' may make sense after pinctrl-0, not here.
Perhaps you're interacting with my RFC PATCH that removes this
'power-domains' property?
https://lore.kernel.org/linux-rockchip/20220613183556.RFC.1.I9ca71105e505f024d53b7e0ba4462230813ebb8d@changeid/
But that most likely isn't landing upstream as-is.
Otherwise, this patch looks good to me:
Reviewed-by: Brian Norris <briannorris@chromium.org>
> #sound-dai-cells = <0>;
> status = "disabled";
> };
_______________________________________________
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] 60+ messages in thread
* Re: [PATCH v2 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO
2022-06-16 15:58 ` Judy Hsiao
(?)
(?)
@ 2022-06-16 20:48 ` Brian Norris
-1 siblings, 0 replies; 60+ messages in thread
From: Brian Norris @ 2022-06-16 20:48 UTC (permalink / raw)
To: Judy Hsiao
Cc: Heiko Stuebner, Liam Girdwood, Rob Herring, Mark Brown,
Jaroslav Kysela, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-rockchip, linux-kernel
Hi,
On Thu, Jun 16, 2022 at 03:58:34PM +0000, Judy Hsiao 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>
It feels like a lot of the noise in this patch is due to adding
additional error handling, unrelated to the real change you're making.
Maybe that deserves a separate patch?
> ---
> sound/soc/rockchip/rockchip_i2s.c | 134 ++++++++++++++++++++++++------
> 1 file changed, 108 insertions(+), 26 deletions(-)
>
> diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
> index 47a3971a9ce1..574d3d0900c4 100644
> --- a/sound/soc/rockchip/rockchip_i2s.c
> +++ b/sound/soc/rockchip/rockchip_i2s.c
> @@ -54,8 +54,40 @@ 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 +124,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,
> + ret = regmap_update_bits(i2s->regmap, I2S_DMACR,
> I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_ENABLE);
I mentioned this off-list, but the 2nd-line indentation alignment *used*
to be in a very particular style, and you've moved that around a lot. To
match the previous style, it should be:
ret = regmap_update_bits(i2s->regmap, I2S_DMACR,
I2S_DMACR_TDE_ENABLE,
I2S_DMACR_TDE_ENABLE);
(BTW, if you're using Gmail to view this, you're going to have no idea
what I'm talking about, since it doesn't do whitespace or monospace font
correctly...)
The same applies throughout; most of the 2nd-line, 3rd-line, ...
indentation is a little weird.
> -
> - regmap_update_bits(i2s->regmap, I2S_XFER,
> + 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,
> + 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,
> + 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,
> + 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 +177,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)
> {
> unsigned int val = 0;
> int retry = 10;
> + int ret = 0;
>
> spin_lock(&i2s->lock);
> if (on) {
> - regmap_update_bits(i2s->regmap, I2S_DMACR,
> + ret = regmap_update_bits(i2s->regmap, I2S_DMACR,
> I2S_DMACR_RDE_ENABLE, I2S_DMACR_RDE_ENABLE);
> + if (ret < 0)
> + goto end;
>
> - regmap_update_bits(i2s->regmap, I2S_XFER,
> + 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,
> + 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,
> + ret = regmap_update_bits(i2s->regmap, I2S_XFER,
> I2S_XFER_TXS_START |
> - I2S_XFER_RXS_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,
> + 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 +239,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 +482,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 +801,23 @@ 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)) {
> + dev_info(&pdev->dev, "switch bclk to GPIO func\n");
I don't think we need this printed at the KERN_INFO level. Either drop
it, or maybe dev_dbg().
Brian
> + 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 [flat|nested] 60+ messages in thread
* Re: [PATCH v2 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-16 20:48 ` Brian Norris
0 siblings, 0 replies; 60+ messages in thread
From: Brian Norris @ 2022-06-16 20:48 UTC (permalink / raw)
To: Judy Hsiao
Cc: alsa-devel, Heiko Stuebner, linux-kernel, Liam Girdwood,
Rob Herring, linux-rockchip, Mark Brown, Chen-Yu Tsai,
linux-arm-kernel
Hi,
On Thu, Jun 16, 2022 at 03:58:34PM +0000, Judy Hsiao 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>
It feels like a lot of the noise in this patch is due to adding
additional error handling, unrelated to the real change you're making.
Maybe that deserves a separate patch?
> ---
> sound/soc/rockchip/rockchip_i2s.c | 134 ++++++++++++++++++++++++------
> 1 file changed, 108 insertions(+), 26 deletions(-)
>
> diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
> index 47a3971a9ce1..574d3d0900c4 100644
> --- a/sound/soc/rockchip/rockchip_i2s.c
> +++ b/sound/soc/rockchip/rockchip_i2s.c
> @@ -54,8 +54,40 @@ 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 +124,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,
> + ret = regmap_update_bits(i2s->regmap, I2S_DMACR,
> I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_ENABLE);
I mentioned this off-list, but the 2nd-line indentation alignment *used*
to be in a very particular style, and you've moved that around a lot. To
match the previous style, it should be:
ret = regmap_update_bits(i2s->regmap, I2S_DMACR,
I2S_DMACR_TDE_ENABLE,
I2S_DMACR_TDE_ENABLE);
(BTW, if you're using Gmail to view this, you're going to have no idea
what I'm talking about, since it doesn't do whitespace or monospace font
correctly...)
The same applies throughout; most of the 2nd-line, 3rd-line, ...
indentation is a little weird.
> -
> - regmap_update_bits(i2s->regmap, I2S_XFER,
> + 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,
> + 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,
> + 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,
> + 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 +177,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)
> {
> unsigned int val = 0;
> int retry = 10;
> + int ret = 0;
>
> spin_lock(&i2s->lock);
> if (on) {
> - regmap_update_bits(i2s->regmap, I2S_DMACR,
> + ret = regmap_update_bits(i2s->regmap, I2S_DMACR,
> I2S_DMACR_RDE_ENABLE, I2S_DMACR_RDE_ENABLE);
> + if (ret < 0)
> + goto end;
>
> - regmap_update_bits(i2s->regmap, I2S_XFER,
> + 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,
> + 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,
> + ret = regmap_update_bits(i2s->regmap, I2S_XFER,
> I2S_XFER_TXS_START |
> - I2S_XFER_RXS_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,
> + 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 +239,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 +482,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 +801,23 @@ 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)) {
> + dev_info(&pdev->dev, "switch bclk to GPIO func\n");
I don't think we need this printed at the KERN_INFO level. Either drop
it, or maybe dev_dbg().
Brian
> + 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 [flat|nested] 60+ messages in thread
* Re: [PATCH v2 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-16 20:48 ` Brian Norris
0 siblings, 0 replies; 60+ messages in thread
From: Brian Norris @ 2022-06-16 20:48 UTC (permalink / raw)
To: Judy Hsiao
Cc: Heiko Stuebner, Liam Girdwood, Rob Herring, Mark Brown,
Jaroslav Kysela, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-rockchip, linux-kernel
Hi,
On Thu, Jun 16, 2022 at 03:58:34PM +0000, Judy Hsiao 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>
It feels like a lot of the noise in this patch is due to adding
additional error handling, unrelated to the real change you're making.
Maybe that deserves a separate patch?
> ---
> sound/soc/rockchip/rockchip_i2s.c | 134 ++++++++++++++++++++++++------
> 1 file changed, 108 insertions(+), 26 deletions(-)
>
> diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
> index 47a3971a9ce1..574d3d0900c4 100644
> --- a/sound/soc/rockchip/rockchip_i2s.c
> +++ b/sound/soc/rockchip/rockchip_i2s.c
> @@ -54,8 +54,40 @@ 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 +124,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,
> + ret = regmap_update_bits(i2s->regmap, I2S_DMACR,
> I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_ENABLE);
I mentioned this off-list, but the 2nd-line indentation alignment *used*
to be in a very particular style, and you've moved that around a lot. To
match the previous style, it should be:
ret = regmap_update_bits(i2s->regmap, I2S_DMACR,
I2S_DMACR_TDE_ENABLE,
I2S_DMACR_TDE_ENABLE);
(BTW, if you're using Gmail to view this, you're going to have no idea
what I'm talking about, since it doesn't do whitespace or monospace font
correctly...)
The same applies throughout; most of the 2nd-line, 3rd-line, ...
indentation is a little weird.
> -
> - regmap_update_bits(i2s->regmap, I2S_XFER,
> + 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,
> + 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,
> + 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,
> + 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 +177,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)
> {
> unsigned int val = 0;
> int retry = 10;
> + int ret = 0;
>
> spin_lock(&i2s->lock);
> if (on) {
> - regmap_update_bits(i2s->regmap, I2S_DMACR,
> + ret = regmap_update_bits(i2s->regmap, I2S_DMACR,
> I2S_DMACR_RDE_ENABLE, I2S_DMACR_RDE_ENABLE);
> + if (ret < 0)
> + goto end;
>
> - regmap_update_bits(i2s->regmap, I2S_XFER,
> + 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,
> + 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,
> + ret = regmap_update_bits(i2s->regmap, I2S_XFER,
> I2S_XFER_TXS_START |
> - I2S_XFER_RXS_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,
> + 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 +239,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 +482,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 +801,23 @@ 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)) {
> + dev_info(&pdev->dev, "switch bclk to GPIO func\n");
I don't think we need this printed at the KERN_INFO level. Either drop
it, or maybe dev_dbg().
Brian
> + 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-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-16 20:48 ` Brian Norris
0 siblings, 0 replies; 60+ messages in thread
From: Brian Norris @ 2022-06-16 20:48 UTC (permalink / raw)
To: Judy Hsiao
Cc: Heiko Stuebner, Liam Girdwood, Rob Herring, Mark Brown,
Jaroslav Kysela, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-rockchip, linux-kernel
Hi,
On Thu, Jun 16, 2022 at 03:58:34PM +0000, Judy Hsiao 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>
It feels like a lot of the noise in this patch is due to adding
additional error handling, unrelated to the real change you're making.
Maybe that deserves a separate patch?
> ---
> sound/soc/rockchip/rockchip_i2s.c | 134 ++++++++++++++++++++++++------
> 1 file changed, 108 insertions(+), 26 deletions(-)
>
> diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
> index 47a3971a9ce1..574d3d0900c4 100644
> --- a/sound/soc/rockchip/rockchip_i2s.c
> +++ b/sound/soc/rockchip/rockchip_i2s.c
> @@ -54,8 +54,40 @@ 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 +124,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,
> + ret = regmap_update_bits(i2s->regmap, I2S_DMACR,
> I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_ENABLE);
I mentioned this off-list, but the 2nd-line indentation alignment *used*
to be in a very particular style, and you've moved that around a lot. To
match the previous style, it should be:
ret = regmap_update_bits(i2s->regmap, I2S_DMACR,
I2S_DMACR_TDE_ENABLE,
I2S_DMACR_TDE_ENABLE);
(BTW, if you're using Gmail to view this, you're going to have no idea
what I'm talking about, since it doesn't do whitespace or monospace font
correctly...)
The same applies throughout; most of the 2nd-line, 3rd-line, ...
indentation is a little weird.
> -
> - regmap_update_bits(i2s->regmap, I2S_XFER,
> + 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,
> + 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,
> + 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,
> + 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 +177,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)
> {
> unsigned int val = 0;
> int retry = 10;
> + int ret = 0;
>
> spin_lock(&i2s->lock);
> if (on) {
> - regmap_update_bits(i2s->regmap, I2S_DMACR,
> + ret = regmap_update_bits(i2s->regmap, I2S_DMACR,
> I2S_DMACR_RDE_ENABLE, I2S_DMACR_RDE_ENABLE);
> + if (ret < 0)
> + goto end;
>
> - regmap_update_bits(i2s->regmap, I2S_XFER,
> + 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,
> + 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,
> + ret = regmap_update_bits(i2s->regmap, I2S_XFER,
> I2S_XFER_TXS_START |
> - I2S_XFER_RXS_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,
> + 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 +239,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 +482,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 +801,23 @@ 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)) {
> + dev_info(&pdev->dev, "switch bclk to GPIO func\n");
I don't think we need this printed at the KERN_INFO level. Either drop
it, or maybe dev_dbg().
Brian
> + 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] 60+ messages in thread
* Re: [PATCH v2 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO
2022-06-16 20:48 ` Brian Norris
(?)
(?)
@ 2022-06-17 4:21 ` Judy Hsiao
-1 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-17 4:21 UTC (permalink / raw)
To: Brian Norris
Cc: Heiko Stuebner, Liam Girdwood, Rob Herring, Mark Brown,
Jaroslav Kysela, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-rockchip, linux-kernel
> It feels like a lot of the noise in this patch is due to adding
> additional error handling, unrelated to the real change you're making.
> Maybe that deserves a separate patch?
The error handling is to detect the failure of switching LRCLK.
If LRCLK is OFF, we don’t want the BCLK to be ON. So the error
handling in the patch is relevant.
> I mentioned this off-list, but the 2nd-line indentation alignment *used*
> to be in a very particular style, and you've moved that around a lot. To
> match the previous style, it should be:
>
> ret = regmap_update_bits(i2s->regmap, I2S_DMACR,
> I2S_DMACR_TDE_ENABLE,
> I2S_DMACR_TDE_ENABLE);
>
> (BTW, if you're using Gmail to view this, you're going to have no idea
> what I'm talking about, since it doesn't do whitespace or monospace font
> correctly...)
>
> The same applies throughout; most of the 2nd-line, 3rd-line, ...
> indentation is a little weird.
Yes, let me fix the indentation in V3, thanks!
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-17 4:21 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-17 4:21 UTC (permalink / raw)
To: Brian Norris
Cc: alsa-devel, Heiko Stuebner, linux-kernel, Liam Girdwood,
Rob Herring, linux-rockchip, Mark Brown, Chen-Yu Tsai,
linux-arm-kernel
> It feels like a lot of the noise in this patch is due to adding
> additional error handling, unrelated to the real change you're making.
> Maybe that deserves a separate patch?
The error handling is to detect the failure of switching LRCLK.
If LRCLK is OFF, we don’t want the BCLK to be ON. So the error
handling in the patch is relevant.
> I mentioned this off-list, but the 2nd-line indentation alignment *used*
> to be in a very particular style, and you've moved that around a lot. To
> match the previous style, it should be:
>
> ret = regmap_update_bits(i2s->regmap, I2S_DMACR,
> I2S_DMACR_TDE_ENABLE,
> I2S_DMACR_TDE_ENABLE);
>
> (BTW, if you're using Gmail to view this, you're going to have no idea
> what I'm talking about, since it doesn't do whitespace or monospace font
> correctly...)
>
> The same applies throughout; most of the 2nd-line, 3rd-line, ...
> indentation is a little weird.
Yes, let me fix the indentation in V3, thanks!
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-17 4:21 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-17 4:21 UTC (permalink / raw)
To: Brian Norris
Cc: Heiko Stuebner, Liam Girdwood, Rob Herring, Mark Brown,
Jaroslav Kysela, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-rockchip, linux-kernel
> It feels like a lot of the noise in this patch is due to adding
> additional error handling, unrelated to the real change you're making.
> Maybe that deserves a separate patch?
The error handling is to detect the failure of switching LRCLK.
If LRCLK is OFF, we don’t want the BCLK to be ON. So the error
handling in the patch is relevant.
> I mentioned this off-list, but the 2nd-line indentation alignment *used*
> to be in a very particular style, and you've moved that around a lot. To
> match the previous style, it should be:
>
> ret = regmap_update_bits(i2s->regmap, I2S_DMACR,
> I2S_DMACR_TDE_ENABLE,
> I2S_DMACR_TDE_ENABLE);
>
> (BTW, if you're using Gmail to view this, you're going to have no idea
> what I'm talking about, since it doesn't do whitespace or monospace font
> correctly...)
>
> The same applies throughout; most of the 2nd-line, 3rd-line, ...
> indentation is a little weird.
Yes, let me fix the indentation in V3, thanks!
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v2 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-17 4:21 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-17 4:21 UTC (permalink / raw)
To: Brian Norris
Cc: Heiko Stuebner, Liam Girdwood, Rob Herring, Mark Brown,
Jaroslav Kysela, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-rockchip, linux-kernel
> It feels like a lot of the noise in this patch is due to adding
> additional error handling, unrelated to the real change you're making.
> Maybe that deserves a separate patch?
The error handling is to detect the failure of switching LRCLK.
If LRCLK is OFF, we don’t want the BCLK to be ON. So the error
handling in the patch is relevant.
> I mentioned this off-list, but the 2nd-line indentation alignment *used*
> to be in a very particular style, and you've moved that around a lot. To
> match the previous style, it should be:
>
> ret = regmap_update_bits(i2s->regmap, I2S_DMACR,
> I2S_DMACR_TDE_ENABLE,
> I2S_DMACR_TDE_ENABLE);
>
> (BTW, if you're using Gmail to view this, you're going to have no idea
> what I'm talking about, since it doesn't do whitespace or monospace font
> correctly...)
>
> The same applies throughout; most of the 2nd-line, 3rd-line, ...
> indentation is a little weird.
Yes, let me fix the indentation in V3, thanks!
_______________________________________________
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] 60+ messages in thread
* [PATCH v3 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO
2022-06-16 15:58 ` Judy Hsiao
(?)
(?)
@ 2022-06-17 4:42 ` Judy Hsiao
-1 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-17 4:42 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 to burn the speakers
on rockchip platform when BCLK and SD_MODE are ON but LRCLK is OFF.
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 | 167 +++++++++++++-----
4 files changed, 162 insertions(+), 47 deletions(-)
--
2.36.1.476.g0c4daa206d-goog
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v3 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-17 4:42 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-17 4:42 UTC (permalink / raw)
To: Heiko Stuebner
Cc: alsa-devel, linux-kernel, Brian Norris, Liam Girdwood,
Rob Herring, linux-rockchip, Mark Brown, Chen-Yu Tsai,
Judy Hsiao, linux-arm-kernel
The patches series is to fix the unexpected large
DC output voltage of Max98357a to burn the speakers
on rockchip platform when BCLK and SD_MODE are ON but LRCLK is OFF.
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 | 167 +++++++++++++-----
4 files changed, 162 insertions(+), 47 deletions(-)
--
2.36.1.476.g0c4daa206d-goog
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v3 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-17 4:42 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-17 4:42 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 to burn the speakers
on rockchip platform when BCLK and SD_MODE are ON but LRCLK is OFF.
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 | 167 +++++++++++++-----
4 files changed, 162 insertions(+), 47 deletions(-)
--
2.36.1.476.g0c4daa206d-goog
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH v3 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-17 4:42 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-17 4:42 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 to burn the speakers
on rockchip platform when BCLK and SD_MODE are ON but LRCLK is OFF.
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 | 167 +++++++++++++-----
4 files changed, 162 insertions(+), 47 deletions(-)
--
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] 60+ messages in thread
* [PATCH v3 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO
2022-06-17 4:42 ` Judy Hsiao
(?)
(?)
@ 2022-06-17 4:42 ` Judy Hsiao
-1 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-17 4:42 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 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:
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>
---
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
@@ -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,53 @@ 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)
{
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 +235,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 +478,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 +797,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] 60+ messages in thread
* [PATCH v3 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-17 4:42 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-17 4:42 UTC (permalink / raw)
To: Heiko Stuebner
Cc: alsa-devel, linux-kernel, Brian Norris, Liam Girdwood,
Rob Herring, linux-rockchip, Mark Brown, Chen-Yu Tsai,
Judy Hsiao, linux-arm-kernel
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:
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>
---
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
@@ -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,53 @@ 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)
{
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 +235,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 +478,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 +797,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] 60+ messages in thread
* [PATCH v3 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-17 4:42 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-17 4:42 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 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:
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>
---
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
@@ -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,53 @@ 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)
{
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 +235,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 +478,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 +797,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
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v3 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-17 4:42 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-17 4:42 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 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:
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>
---
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
@@ -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,53 @@ 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)
{
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 +235,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 +478,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 +797,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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v3 2/3] arm64: dts: rk3399: i2s: switch BCLK to GPIO
2022-06-17 4:42 ` Judy Hsiao
(?)
(?)
@ 2022-06-17 4:42 ` Judy Hsiao
-1 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-17 4:42 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 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:
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] 60+ messages in thread
* [PATCH v3 2/3] arm64: dts: rk3399: i2s: switch BCLK to GPIO
@ 2022-06-17 4:42 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-17 4:42 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 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:
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
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v3 2/3] arm64: dts: rk3399: i2s: switch BCLK to GPIO
@ 2022-06-17 4:42 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-17 4:42 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 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:
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v3 2/3] arm64: dts: rk3399: i2s: switch BCLK to GPIO
@ 2022-06-17 4:42 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-17 4:42 UTC (permalink / raw)
To: Heiko Stuebner
Cc: alsa-devel, linux-kernel, Brian Norris, Liam Girdwood,
Rob Herring, linux-rockchip, Mark Brown, Chen-Yu Tsai,
Judy Hsiao, linux-arm-kernel
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:
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] 60+ messages in thread
* [PATCH v3 3/3] ASoC: dt-bindings: rockchip: Document pinctrl-names for i2s
2022-06-17 4:42 ` Judy Hsiao
(?)
(?)
@ 2022-06-17 4:42 ` Judy Hsiao
-1 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-17 4:42 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] 60+ messages in thread
* [PATCH v3 3/3] ASoC: dt-bindings: rockchip: Document pinctrl-names for i2s
@ 2022-06-17 4:42 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-17 4:42 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
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v3 3/3] ASoC: dt-bindings: rockchip: Document pinctrl-names for i2s
@ 2022-06-17 4:42 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-17 4:42 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 60+ messages in thread
* [PATCH v3 3/3] ASoC: dt-bindings: rockchip: Document pinctrl-names for i2s
@ 2022-06-17 4:42 ` Judy Hsiao
0 siblings, 0 replies; 60+ messages in thread
From: Judy Hsiao @ 2022-06-17 4:42 UTC (permalink / raw)
To: Heiko Stuebner
Cc: alsa-devel, linux-kernel, Brian Norris, Liam Girdwood,
Rob Herring, linux-rockchip, Mark Brown, Chen-Yu Tsai,
Judy Hsiao, linux-arm-kernel
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] 60+ messages in thread
* Re: [PATCH v3 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO
2022-06-17 4:42 ` Judy Hsiao
(?)
(?)
@ 2022-06-17 8:45 ` Mark Brown
-1 siblings, 0 replies; 60+ messages in thread
From: Mark Brown @ 2022-06-17 8:45 UTC (permalink / raw)
To: Judy Hsiao
Cc: Heiko Stuebner, Liam Girdwood, Rob Herring, Brian Norris,
Jaroslav Kysela, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-rockchip, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 534 bytes --]
On Fri, Jun 17, 2022 at 04:42:48AM +0000, Judy Hsiao wrote:
> The patches series is to fix the unexpected large
> DC output voltage of Max98357a to burn the speakers
> on rockchip platform when BCLK and SD_MODE are ON but LRCLK is OFF.
Please don't send new patches in reply to old patches or serieses, this
makes it harder for both people and tools to understand what is going
on - it can bury things in mailboxes and make it difficult to keep track
of what current patches are, both for the new patches and the old ones.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v3 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-17 8:45 ` Mark Brown
0 siblings, 0 replies; 60+ messages in thread
From: Mark Brown @ 2022-06-17 8:45 UTC (permalink / raw)
To: Judy Hsiao
Cc: alsa-devel, Heiko Stuebner, linux-kernel, Brian Norris,
Liam Girdwood, linux-rockchip, Rob Herring, Chen-Yu Tsai,
linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 534 bytes --]
On Fri, Jun 17, 2022 at 04:42:48AM +0000, Judy Hsiao wrote:
> The patches series is to fix the unexpected large
> DC output voltage of Max98357a to burn the speakers
> on rockchip platform when BCLK and SD_MODE are ON but LRCLK is OFF.
Please don't send new patches in reply to old patches or serieses, this
makes it harder for both people and tools to understand what is going
on - it can bury things in mailboxes and make it difficult to keep track
of what current patches are, both for the new patches and the old ones.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v3 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-17 8:45 ` Mark Brown
0 siblings, 0 replies; 60+ messages in thread
From: Mark Brown @ 2022-06-17 8:45 UTC (permalink / raw)
To: Judy Hsiao
Cc: Heiko Stuebner, Liam Girdwood, Rob Herring, Brian Norris,
Jaroslav Kysela, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-rockchip, linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 534 bytes --]
On Fri, Jun 17, 2022 at 04:42:48AM +0000, Judy Hsiao wrote:
> The patches series is to fix the unexpected large
> DC output voltage of Max98357a to burn the speakers
> on rockchip platform when BCLK and SD_MODE are ON but LRCLK is OFF.
Please don't send new patches in reply to old patches or serieses, this
makes it harder for both people and tools to understand what is going
on - it can bury things in mailboxes and make it difficult to keep track
of what current patches are, both for the new patches and the old ones.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 170 bytes --]
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v3 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-17 8:45 ` Mark Brown
0 siblings, 0 replies; 60+ messages in thread
From: Mark Brown @ 2022-06-17 8:45 UTC (permalink / raw)
To: Judy Hsiao
Cc: Heiko Stuebner, Liam Girdwood, Rob Herring, Brian Norris,
Jaroslav Kysela, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-rockchip, linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 534 bytes --]
On Fri, Jun 17, 2022 at 04:42:48AM +0000, Judy Hsiao wrote:
> The patches series is to fix the unexpected large
> DC output voltage of Max98357a to burn the speakers
> on rockchip platform when BCLK and SD_MODE are ON but LRCLK is OFF.
Please don't send new patches in reply to old patches or serieses, this
makes it harder for both people and tools to understand what is going
on - it can bury things in mailboxes and make it difficult to keep track
of what current patches are, both for the new patches and the old ones.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
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] 60+ messages in thread
* Re: [PATCH v3 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO
2022-06-17 4:42 ` Judy Hsiao
(?)
(?)
@ 2022-06-17 20:46 ` Brian Norris
-1 siblings, 0 replies; 60+ messages in thread
From: Brian Norris @ 2022-06-17 20:46 UTC (permalink / raw)
To: Judy Hsiao
Cc: Heiko Stuebner, Liam Girdwood, Rob Herring, Mark Brown,
Jaroslav Kysela, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-rockchip, linux-kernel
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:
> 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>
> ---
> 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;
> i2s->tx_start = true;
> } else {
> i2s->tx_start = false;
>
...
> 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>
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v3 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-17 20:46 ` Brian Norris
0 siblings, 0 replies; 60+ messages in thread
From: Brian Norris @ 2022-06-17 20:46 UTC (permalink / raw)
To: Judy Hsiao
Cc: alsa-devel, Heiko Stuebner, linux-kernel, Liam Girdwood,
Rob Herring, linux-rockchip, Mark Brown, Chen-Yu Tsai,
linux-arm-kernel
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:
> 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>
> ---
> 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;
> i2s->tx_start = true;
> } else {
> i2s->tx_start = false;
>
...
> 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>
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v3 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-17 20:46 ` Brian Norris
0 siblings, 0 replies; 60+ messages in thread
From: Brian Norris @ 2022-06-17 20:46 UTC (permalink / raw)
To: Judy Hsiao
Cc: Heiko Stuebner, Liam Girdwood, Rob Herring, Mark Brown,
Jaroslav Kysela, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-rockchip, linux-kernel
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:
> 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>
> ---
> 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;
> i2s->tx_start = true;
> } else {
> i2s->tx_start = false;
>
...
> 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>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH v3 1/3] ASoC: rockchip: i2s: switch BCLK to GPIO
@ 2022-06-17 20:46 ` Brian Norris
0 siblings, 0 replies; 60+ messages in thread
From: Brian Norris @ 2022-06-17 20:46 UTC (permalink / raw)
To: Judy Hsiao
Cc: Heiko Stuebner, Liam Girdwood, Rob Herring, Mark Brown,
Jaroslav Kysela, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-rockchip, linux-kernel
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:
> 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>
> ---
> 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;
> i2s->tx_start = true;
> } else {
> i2s->tx_start = false;
>
...
> 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>
_______________________________________________
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] 60+ messages in thread
end of thread, other threads:[~2022-06-17 21:09 UTC | newest]
Thread overview: 60+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-16 15:58 [PATCH v2 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO Judy Hsiao
2022-06-16 15:58 ` Judy Hsiao
2022-06-16 15:58 ` Judy Hsiao
2022-06-16 15:58 ` Judy Hsiao
2022-06-16 15:58 ` [PATCH v2 1/3] " Judy Hsiao
2022-06-16 15:58 ` Judy Hsiao
2022-06-16 15:58 ` Judy Hsiao
2022-06-16 15:58 ` Judy Hsiao
2022-06-16 19:03 ` kernel test robot
2022-06-16 19:03 ` kernel test robot
2022-06-16 19:03 ` kernel test robot
2022-06-16 19:03 ` kernel test robot
2022-06-16 20:48 ` Brian Norris
2022-06-16 20:48 ` Brian Norris
2022-06-16 20:48 ` Brian Norris
2022-06-16 20:48 ` Brian Norris
2022-06-17 4:21 ` Judy Hsiao
2022-06-17 4:21 ` Judy Hsiao
2022-06-17 4:21 ` Judy Hsiao
2022-06-17 4:21 ` Judy Hsiao
2022-06-16 15:58 ` [PATCH v2 2/3] arm64: dts: rk3399: " Judy Hsiao
2022-06-16 15:58 ` Judy Hsiao
2022-06-16 15:58 ` Judy Hsiao
2022-06-16 15:58 ` Judy Hsiao
2022-06-16 20:39 ` Brian Norris
2022-06-16 20:39 ` Brian Norris
2022-06-16 20:39 ` Brian Norris
2022-06-16 20:39 ` Brian Norris
2022-06-16 15:58 ` [PATCH v2 3/3] ASoC: dt-bindings: rockchip: Document pinctrl-names for i2s Judy Hsiao
2022-06-16 15:58 ` Judy Hsiao
2022-06-16 15:58 ` Judy Hsiao
2022-06-16 15:58 ` Judy Hsiao
2022-06-16 20:39 ` Brian Norris
2022-06-16 20:39 ` Brian Norris
2022-06-16 20:39 ` Brian Norris
2022-06-16 20:39 ` Brian Norris
2022-06-17 4:42 ` [PATCH v3 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO Judy Hsiao
2022-06-17 4:42 ` Judy Hsiao
2022-06-17 4:42 ` Judy Hsiao
2022-06-17 4:42 ` Judy Hsiao
2022-06-17 4:42 ` [PATCH v3 1/3] " Judy Hsiao
2022-06-17 4:42 ` Judy Hsiao
2022-06-17 4:42 ` Judy Hsiao
2022-06-17 4:42 ` Judy Hsiao
2022-06-17 20:46 ` Brian Norris
2022-06-17 20:46 ` Brian Norris
2022-06-17 20:46 ` Brian Norris
2022-06-17 20:46 ` Brian Norris
2022-06-17 4:42 ` [PATCH v3 2/3] arm64: dts: rk3399: " Judy Hsiao
2022-06-17 4:42 ` Judy Hsiao
2022-06-17 4:42 ` Judy Hsiao
2022-06-17 4:42 ` Judy Hsiao
2022-06-17 4:42 ` [PATCH v3 3/3] ASoC: dt-bindings: rockchip: Document pinctrl-names for i2s Judy Hsiao
2022-06-17 4:42 ` Judy Hsiao
2022-06-17 4:42 ` Judy Hsiao
2022-06-17 4:42 ` Judy Hsiao
2022-06-17 8:45 ` [PATCH v3 0/3] ASoC: rockchip: i2s: switch BCLK to GPIO Mark Brown
2022-06-17 8:45 ` Mark Brown
2022-06-17 8:45 ` Mark Brown
2022-06-17 8:45 ` Mark Brown
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.