* [PATCH 1/2] ASoC: rockchip: i2s: separate capture and playback
@ 2015-12-09 10:32 John Keeping
[not found] ` <1449657147-26959-1-git-send-email-john-HooS5bfzL4hWk0Htik3J/w@public.gmane.org>
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: John Keeping @ 2015-12-09 10:32 UTC (permalink / raw)
To: linux-rockchip
Cc: John Keeping, Liam Girdwood, Mark Brown, Jaroslav Kysela,
Takashi Iwai, Heiko Stuebner, alsa-devel, linux-arm-kernel,
linux-kernel
If we only clear the tx/rx state when both are disabled it is not
possible to start/stop one multiple times while the other is running.
Since the two are independently controlled, treat them as such and
remove the false dependency between capture and playback.
Signed-off-by: John Keeping <john@metanate.com>
---
sound/soc/rockchip/rockchip_i2s.c | 72 +++++++++++++++++----------------------
1 file changed, 32 insertions(+), 40 deletions(-)
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
index 83b1b9c..acc6225 100644
--- a/sound/soc/rockchip/rockchip_i2s.c
+++ b/sound/soc/rockchip/rockchip_i2s.c
@@ -82,8 +82,8 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on)
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);
+ I2S_XFER_TXS_START,
+ I2S_XFER_TXS_START);
i2s->tx_start = true;
} else {
@@ -92,27 +92,23 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on)
regmap_update_bits(i2s->regmap, I2S_DMACR,
I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_DISABLE);
- 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);
+ regmap_update_bits(i2s->regmap, I2S_XFER,
+ I2S_XFER_TXS_START,
+ I2S_XFER_TXS_STOP);
- regmap_update_bits(i2s->regmap, I2S_CLR,
- I2S_CLR_TXC | I2S_CLR_RXC,
- I2S_CLR_TXC | I2S_CLR_RXC);
+ regmap_update_bits(i2s->regmap, I2S_CLR,
+ I2S_CLR_TXC,
+ I2S_CLR_TXC);
- regmap_read(i2s->regmap, I2S_CLR, &val);
+ regmap_read(i2s->regmap, I2S_CLR, &val);
- /* Should wait for clear operation to finish */
- while (val) {
- regmap_read(i2s->regmap, I2S_CLR, &val);
- retry--;
- if (!retry) {
- dev_warn(i2s->dev, "fail to clear\n");
- break;
- }
+ /* Should wait for clear operation to finish */
+ while (val & I2S_CLR_TXC) {
+ regmap_read(i2s->regmap, I2S_CLR, &val);
+ retry--;
+ if (!retry) {
+ dev_warn(i2s->dev, "fail to clear\n");
+ break;
}
}
}
@@ -128,8 +124,8 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on)
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);
+ I2S_XFER_RXS_START,
+ I2S_XFER_RXS_START);
i2s->rx_start = true;
} else {
@@ -138,27 +134,23 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on)
regmap_update_bits(i2s->regmap, I2S_DMACR,
I2S_DMACR_RDE_ENABLE, I2S_DMACR_RDE_DISABLE);
- 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);
+ regmap_update_bits(i2s->regmap, I2S_XFER,
+ I2S_XFER_RXS_START,
+ I2S_XFER_RXS_STOP);
- regmap_update_bits(i2s->regmap, I2S_CLR,
- I2S_CLR_TXC | I2S_CLR_RXC,
- I2S_CLR_TXC | I2S_CLR_RXC);
+ regmap_update_bits(i2s->regmap, I2S_CLR,
+ I2S_CLR_RXC,
+ I2S_CLR_RXC);
- regmap_read(i2s->regmap, I2S_CLR, &val);
+ regmap_read(i2s->regmap, I2S_CLR, &val);
- /* Should wait for clear operation to finish */
- while (val) {
- regmap_read(i2s->regmap, I2S_CLR, &val);
- retry--;
- if (!retry) {
- dev_warn(i2s->dev, "fail to clear\n");
- break;
- }
+ /* Should wait for clear operation to finish */
+ while (val & I2S_CLR_RXC) {
+ regmap_read(i2s->regmap, I2S_CLR, &val);
+ retry--;
+ if (!retry) {
+ dev_warn(i2s->dev, "fail to clear\n");
+ break;
}
}
}
--
2.6.3.462.gbe2c914
^ permalink raw reply related [flat|nested] 13+ messages in thread[parent not found: <1449657147-26959-1-git-send-email-john-HooS5bfzL4hWk0Htik3J/w@public.gmane.org>]
* [PATCH 2/2] ASoC: rockchip: i2s: remove unused variables [not found] ` <1449657147-26959-1-git-send-email-john-HooS5bfzL4hWk0Htik3J/w@public.gmane.org> @ 2015-12-09 10:32 ` John Keeping 2015-12-09 20:44 ` Applied "ASoC: rockchip: i2s: remove unused variables" to the asoc tree Mark Brown 0 siblings, 1 reply; 13+ messages in thread From: John Keeping @ 2015-12-09 10:32 UTC (permalink / raw) To: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r Cc: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Heiko Stuebner, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Takashi Iwai, Liam Girdwood, Mark Brown, John Keeping, Jaroslav Kysela, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r The previous commit removed the only use of these variables. Signed-off-by: John Keeping <john-HooS5bfzL4hWk0Htik3J/w@public.gmane.org> --- sound/soc/rockchip/rockchip_i2s.c | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c index acc6225..8b0a588 100644 --- a/sound/soc/rockchip/rockchip_i2s.c +++ b/sound/soc/rockchip/rockchip_i2s.c @@ -34,13 +34,6 @@ struct rk_i2s_dev { struct regmap *regmap; -/* - * Used to indicate the tx/rx status. - * I2S controller hopes to start the tx and rx together, - * also to stop them when they are both try to stop. -*/ - bool tx_start; - bool rx_start; bool is_master_mode; }; @@ -84,11 +77,7 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on) regmap_update_bits(i2s->regmap, I2S_XFER, I2S_XFER_TXS_START, I2S_XFER_TXS_START); - - i2s->tx_start = true; } else { - i2s->tx_start = false; - regmap_update_bits(i2s->regmap, I2S_DMACR, I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_DISABLE); @@ -126,11 +115,7 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on) regmap_update_bits(i2s->regmap, I2S_XFER, I2S_XFER_RXS_START, I2S_XFER_RXS_START); - - i2s->rx_start = true; } else { - i2s->rx_start = false; - regmap_update_bits(i2s->regmap, I2S_DMACR, I2S_DMACR_RDE_ENABLE, I2S_DMACR_RDE_DISABLE); -- 2.6.3.462.gbe2c914 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Applied "ASoC: rockchip: i2s: remove unused variables" to the asoc tree 2015-12-09 10:32 ` [PATCH 2/2] ASoC: rockchip: i2s: remove unused variables John Keeping @ 2015-12-09 20:44 ` Mark Brown 0 siblings, 0 replies; 13+ messages in thread From: Mark Brown @ 2015-12-09 20:44 UTC (permalink / raw) To: John Keeping, Mark Brown; +Cc: alsa-devel The patch ASoC: rockchip: i2s: remove unused variables has been applied to the asoc tree at git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark >From 5938448b99275cba95167c3f9d39ca9225fdad38 Mon Sep 17 00:00:00 2001 From: John Keeping <john@metanate.com> Date: Wed, 9 Dec 2015 10:32:27 +0000 Subject: [PATCH] ASoC: rockchip: i2s: remove unused variables The previous commit removed the only use of these variables. Signed-off-by: John Keeping <john@metanate.com> Signed-off-by: Mark Brown <broonie@kernel.org> --- sound/soc/rockchip/rockchip_i2s.c | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c index acc6225d8d9d..8b0a588ed622 100644 --- a/sound/soc/rockchip/rockchip_i2s.c +++ b/sound/soc/rockchip/rockchip_i2s.c @@ -34,13 +34,6 @@ struct rk_i2s_dev { struct regmap *regmap; -/* - * Used to indicate the tx/rx status. - * I2S controller hopes to start the tx and rx together, - * also to stop them when they are both try to stop. -*/ - bool tx_start; - bool rx_start; bool is_master_mode; }; @@ -84,11 +77,7 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on) regmap_update_bits(i2s->regmap, I2S_XFER, I2S_XFER_TXS_START, I2S_XFER_TXS_START); - - i2s->tx_start = true; } else { - i2s->tx_start = false; - regmap_update_bits(i2s->regmap, I2S_DMACR, I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_DISABLE); @@ -126,11 +115,7 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on) regmap_update_bits(i2s->regmap, I2S_XFER, I2S_XFER_RXS_START, I2S_XFER_RXS_START); - - i2s->rx_start = true; } else { - i2s->rx_start = false; - regmap_update_bits(i2s->regmap, I2S_DMACR, I2S_DMACR_RDE_ENABLE, I2S_DMACR_RDE_DISABLE); -- 2.6.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Applied "ASoC: rockchip: i2s: separate capture and playback" to the asoc tree 2015-12-09 10:32 [PATCH 1/2] ASoC: rockchip: i2s: separate capture and playback John Keeping [not found] ` <1449657147-26959-1-git-send-email-john-HooS5bfzL4hWk0Htik3J/w@public.gmane.org> @ 2015-12-09 20:44 ` Mark Brown 2016-04-29 14:55 ` [PATCH 1/2] ASoC: rockchip: i2s: separate capture and playback Enric Balletbo Serra 2016-04-29 14:59 ` Enric Balletbo Serra 3 siblings, 0 replies; 13+ messages in thread From: Mark Brown @ 2015-12-09 20:44 UTC (permalink / raw) To: John Keeping, Mark Brown; +Cc: alsa-devel The patch ASoC: rockchip: i2s: separate capture and playback has been applied to the asoc tree at git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark >From eba65d179c1149cf79e68608d452631f33d7f017 Mon Sep 17 00:00:00 2001 From: John Keeping <john@metanate.com> Date: Wed, 9 Dec 2015 10:32:26 +0000 Subject: [PATCH] ASoC: rockchip: i2s: separate capture and playback If we only clear the tx/rx state when both are disabled it is not possible to start/stop one multiple times while the other is running. Since the two are independently controlled, treat them as such and remove the false dependency between capture and playback. Signed-off-by: John Keeping <john@metanate.com> Signed-off-by: Mark Brown <broonie@kernel.org> --- sound/soc/rockchip/rockchip_i2s.c | 72 +++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 40 deletions(-) diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c index 83b1b9c9e017..acc6225d8d9d 100644 --- a/sound/soc/rockchip/rockchip_i2s.c +++ b/sound/soc/rockchip/rockchip_i2s.c @@ -82,8 +82,8 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on) 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); + I2S_XFER_TXS_START, + I2S_XFER_TXS_START); i2s->tx_start = true; } else { @@ -92,27 +92,23 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on) regmap_update_bits(i2s->regmap, I2S_DMACR, I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_DISABLE); - 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); + regmap_update_bits(i2s->regmap, I2S_XFER, + I2S_XFER_TXS_START, + I2S_XFER_TXS_STOP); - regmap_update_bits(i2s->regmap, I2S_CLR, - I2S_CLR_TXC | I2S_CLR_RXC, - I2S_CLR_TXC | I2S_CLR_RXC); + regmap_update_bits(i2s->regmap, I2S_CLR, + I2S_CLR_TXC, + I2S_CLR_TXC); - regmap_read(i2s->regmap, I2S_CLR, &val); + regmap_read(i2s->regmap, I2S_CLR, &val); - /* Should wait for clear operation to finish */ - while (val) { - regmap_read(i2s->regmap, I2S_CLR, &val); - retry--; - if (!retry) { - dev_warn(i2s->dev, "fail to clear\n"); - break; - } + /* Should wait for clear operation to finish */ + while (val & I2S_CLR_TXC) { + regmap_read(i2s->regmap, I2S_CLR, &val); + retry--; + if (!retry) { + dev_warn(i2s->dev, "fail to clear\n"); + break; } } } @@ -128,8 +124,8 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on) 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); + I2S_XFER_RXS_START, + I2S_XFER_RXS_START); i2s->rx_start = true; } else { @@ -138,27 +134,23 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on) regmap_update_bits(i2s->regmap, I2S_DMACR, I2S_DMACR_RDE_ENABLE, I2S_DMACR_RDE_DISABLE); - 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); + regmap_update_bits(i2s->regmap, I2S_XFER, + I2S_XFER_RXS_START, + I2S_XFER_RXS_STOP); - regmap_update_bits(i2s->regmap, I2S_CLR, - I2S_CLR_TXC | I2S_CLR_RXC, - I2S_CLR_TXC | I2S_CLR_RXC); + regmap_update_bits(i2s->regmap, I2S_CLR, + I2S_CLR_RXC, + I2S_CLR_RXC); - regmap_read(i2s->regmap, I2S_CLR, &val); + regmap_read(i2s->regmap, I2S_CLR, &val); - /* Should wait for clear operation to finish */ - while (val) { - regmap_read(i2s->regmap, I2S_CLR, &val); - retry--; - if (!retry) { - dev_warn(i2s->dev, "fail to clear\n"); - break; - } + /* Should wait for clear operation to finish */ + while (val & I2S_CLR_RXC) { + regmap_read(i2s->regmap, I2S_CLR, &val); + retry--; + if (!retry) { + dev_warn(i2s->dev, "fail to clear\n"); + break; } } } -- 2.6.2 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] ASoC: rockchip: i2s: separate capture and playback 2015-12-09 10:32 [PATCH 1/2] ASoC: rockchip: i2s: separate capture and playback John Keeping [not found] ` <1449657147-26959-1-git-send-email-john-HooS5bfzL4hWk0Htik3J/w@public.gmane.org> 2015-12-09 20:44 ` Applied "ASoC: rockchip: i2s: separate capture and playback" " Mark Brown @ 2016-04-29 14:55 ` Enric Balletbo Serra 2016-04-29 14:59 ` Enric Balletbo Serra 3 siblings, 0 replies; 13+ messages in thread From: Enric Balletbo Serra @ 2016-04-29 14:55 UTC (permalink / raw) To: John Keeping Cc: alsa-devel, Heiko Stuebner, linux-kernel, Takashi Iwai, Liam Girdwood, linux-rockchip, Mark Brown, linux-arm-kernel@lists.infradead.org Hi John, 2015-12-09 11:32 GMT+01:00 John Keeping <john@metanate.com>: > If we only clear the tx/rx state when both are disabled it is not > possible to start/stop one multiple times while the other is running. > Since the two are independently controlled, treat them as such and > remove the false dependency between capture and playback. > > Signed-off-by: John Keeping <john@metanate.com> > --- > sound/soc/rockchip/rockchip_i2s.c | 72 > +++++++++++++++++---------------------- > 1 file changed, 32 insertions(+), 40 deletions(-) > > diff --git a/sound/soc/rockchip/rockchip_i2s.c > b/sound/soc/rockchip/rockchip_i2s.c > index 83b1b9c..acc6225 100644 > --- a/sound/soc/rockchip/rockchip_i2s.c > +++ b/sound/soc/rockchip/rockchip_i2s.c > @@ -82,8 +82,8 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev *i2s, > int on) > 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); > + I2S_XFER_TXS_START, > + I2S_XFER_TXS_START); > > i2s->tx_start = true; > } else { > @@ -92,27 +92,23 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev > *i2s, int on) > regmap_update_bits(i2s->regmap, I2S_DMACR, > I2S_DMACR_TDE_ENABLE, > I2S_DMACR_TDE_DISABLE); > > - 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); > + regmap_update_bits(i2s->regmap, I2S_XFER, > + I2S_XFER_TXS_START, > + I2S_XFER_TXS_STOP); > > - regmap_update_bits(i2s->regmap, I2S_CLR, > - I2S_CLR_TXC | I2S_CLR_RXC, > - I2S_CLR_TXC | I2S_CLR_RXC); > + regmap_update_bits(i2s->regmap, I2S_CLR, > + I2S_CLR_TXC, > + I2S_CLR_TXC); > > - regmap_read(i2s->regmap, I2S_CLR, &val); > + regmap_read(i2s->regmap, I2S_CLR, &val); > > - /* Should wait for clear operation to finish */ > - while (val) { > - regmap_read(i2s->regmap, I2S_CLR, &val); > - retry--; > - if (!retry) { > - dev_warn(i2s->dev, "fail to > clear\n"); > - break; > - } > + /* Should wait for clear operation to finish */ > + while (val & I2S_CLR_TXC) { > + regmap_read(i2s->regmap, I2S_CLR, &val); > + retry--; > + if (!retry) { > + dev_warn(i2s->dev, "fail to clear\n"); > + break; > } > } > } > @@ -128,8 +124,8 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev > *i2s, int on) > 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); > + I2S_XFER_RXS_START, > + I2S_XFER_RXS_START); > > i2s->rx_start = true; > } else { > @@ -138,27 +134,23 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev > *i2s, int on) > regmap_update_bits(i2s->regmap, I2S_DMACR, > I2S_DMACR_RDE_ENABLE, > I2S_DMACR_RDE_DISABLE); > > - 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); > + regmap_update_bits(i2s->regmap, I2S_XFER, > + I2S_XFER_RXS_START, > + I2S_XFER_RXS_STOP); > > - regmap_update_bits(i2s->regmap, I2S_CLR, > - I2S_CLR_TXC | I2S_CLR_RXC, > - I2S_CLR_TXC | I2S_CLR_RXC); > + regmap_update_bits(i2s->regmap, I2S_CLR, > + I2S_CLR_RXC, > + I2S_CLR_RXC); > > - regmap_read(i2s->regmap, I2S_CLR, &val); > + regmap_read(i2s->regmap, I2S_CLR, &val); > > - /* Should wait for clear operation to finish */ > - while (val) { > - regmap_read(i2s->regmap, I2S_CLR, &val); > - retry--; > - if (!retry) { > - dev_warn(i2s->dev, "fail to > clear\n"); > - break; > - } > + /* Should wait for clear operation to finish */ > + while (val & I2S_CLR_RXC) { > + regmap_read(i2s->regmap, I2S_CLR, &val); > + retry--; > + if (!retry) { > + dev_warn(i2s->dev, "fail to clear\n"); > + break; > } > } > } > -- > Using my Veyron Jerry Chromebook wiht latest kernel I found that the speakers doesn't work. Bisect points to this patch as the offending commit. However your changes looks reasonable the fact is that reverting the patch makes the audio work again on my device. I need to dig a bit more into the issue, but meanwhile, any idea on what is happening ? Can I ask which device did you test? Best regards, Enric ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] ASoC: rockchip: i2s: separate capture and playback 2015-12-09 10:32 [PATCH 1/2] ASoC: rockchip: i2s: separate capture and playback John Keeping ` (2 preceding siblings ...) 2016-04-29 14:55 ` [PATCH 1/2] ASoC: rockchip: i2s: separate capture and playback Enric Balletbo Serra @ 2016-04-29 14:59 ` Enric Balletbo Serra [not found] ` <CAFqH_52C4JKno4uyt54UdWKAJKM4FPGRiQaQQ9_H-Mz1Cr+Hqg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 3 siblings, 1 reply; 13+ messages in thread From: Enric Balletbo Serra @ 2016-04-29 14:59 UTC (permalink / raw) To: John Keeping Cc: linux-rockchip, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai, Heiko Stuebner, alsa-devel, linux-arm-kernel@lists.infradead.org, linux-kernel Hi John, 2015-12-09 11:32 GMT+01:00 John Keeping <john@metanate.com>: > If we only clear the tx/rx state when both are disabled it is not > possible to start/stop one multiple times while the other is running. > Since the two are independently controlled, treat them as such and > remove the false dependency between capture and playback. > > Signed-off-by: John Keeping <john@metanate.com> > --- > sound/soc/rockchip/rockchip_i2s.c | 72 +++++++++++++++++---------------------- > 1 file changed, 32 insertions(+), 40 deletions(-) > > diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c > index 83b1b9c..acc6225 100644 > --- a/sound/soc/rockchip/rockchip_i2s.c > +++ b/sound/soc/rockchip/rockchip_i2s.c > @@ -82,8 +82,8 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on) > 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); > + I2S_XFER_TXS_START, > + I2S_XFER_TXS_START); > > i2s->tx_start = true; > } else { > @@ -92,27 +92,23 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on) > regmap_update_bits(i2s->regmap, I2S_DMACR, > I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_DISABLE); > > - 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); > + regmap_update_bits(i2s->regmap, I2S_XFER, > + I2S_XFER_TXS_START, > + I2S_XFER_TXS_STOP); > > - regmap_update_bits(i2s->regmap, I2S_CLR, > - I2S_CLR_TXC | I2S_CLR_RXC, > - I2S_CLR_TXC | I2S_CLR_RXC); > + regmap_update_bits(i2s->regmap, I2S_CLR, > + I2S_CLR_TXC, > + I2S_CLR_TXC); > > - regmap_read(i2s->regmap, I2S_CLR, &val); > + regmap_read(i2s->regmap, I2S_CLR, &val); > > - /* Should wait for clear operation to finish */ > - while (val) { > - regmap_read(i2s->regmap, I2S_CLR, &val); > - retry--; > - if (!retry) { > - dev_warn(i2s->dev, "fail to clear\n"); > - break; > - } > + /* Should wait for clear operation to finish */ > + while (val & I2S_CLR_TXC) { > + regmap_read(i2s->regmap, I2S_CLR, &val); > + retry--; > + if (!retry) { > + dev_warn(i2s->dev, "fail to clear\n"); > + break; > } > } > } > @@ -128,8 +124,8 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on) > 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); > + I2S_XFER_RXS_START, > + I2S_XFER_RXS_START); > > i2s->rx_start = true; > } else { > @@ -138,27 +134,23 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on) > regmap_update_bits(i2s->regmap, I2S_DMACR, > I2S_DMACR_RDE_ENABLE, I2S_DMACR_RDE_DISABLE); > > - 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); > + regmap_update_bits(i2s->regmap, I2S_XFER, > + I2S_XFER_RXS_START, > + I2S_XFER_RXS_STOP); > > - regmap_update_bits(i2s->regmap, I2S_CLR, > - I2S_CLR_TXC | I2S_CLR_RXC, > - I2S_CLR_TXC | I2S_CLR_RXC); > + regmap_update_bits(i2s->regmap, I2S_CLR, > + I2S_CLR_RXC, > + I2S_CLR_RXC); > > - regmap_read(i2s->regmap, I2S_CLR, &val); > + regmap_read(i2s->regmap, I2S_CLR, &val); > > - /* Should wait for clear operation to finish */ > - while (val) { > - regmap_read(i2s->regmap, I2S_CLR, &val); > - retry--; > - if (!retry) { > - dev_warn(i2s->dev, "fail to clear\n"); > - break; > - } > + /* Should wait for clear operation to finish */ > + while (val & I2S_CLR_RXC) { > + regmap_read(i2s->regmap, I2S_CLR, &val); > + retry--; > + if (!retry) { > + dev_warn(i2s->dev, "fail to clear\n"); > + break; > } > } > } > -- > 2.6.3.462.gbe2c914 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ Using my Veyron Jerry Chromebook wiht latest kernel I found that the speakers doesn't work. Bisect points to this patch as the offending commit. However your changes looks reasonable the fact is that reverting the patch makes the audio work again on my device. I need to dig a bit more into the issue, but meanwhile, any idea on what is happening ? Can I ask which device did you test? PS: Sorry for the noise if you received the email twice. Best regards, Enric ^ permalink raw reply [flat|nested] 13+ messages in thread
[parent not found: <CAFqH_52C4JKno4uyt54UdWKAJKM4FPGRiQaQQ9_H-Mz1Cr+Hqg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 1/2] ASoC: rockchip: i2s: separate capture and playback [not found] ` <CAFqH_52C4JKno4uyt54UdWKAJKM4FPGRiQaQQ9_H-Mz1Cr+Hqg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2016-04-30 7:00 ` John Keeping 2016-05-02 9:13 ` Enric Balletbo Serra 2016-05-03 4:12 ` sugar 0 siblings, 2 replies; 13+ messages in thread From: John Keeping @ 2016-04-30 7:00 UTC (permalink / raw) To: Enric Balletbo Serra Cc: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Heiko Stuebner, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Takashi Iwai, Liam Girdwood, linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Mark Brown, Jaroslav Kysela, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org Hi Enric, On Fri, Apr 29, 2016 at 04:59:27PM +0200, Enric Balletbo Serra wrote: > 2015-12-09 11:32 GMT+01:00 John Keeping <john-HooS5bfzL4hWk0Htik3J/w@public.gmane.org>: > > If we only clear the tx/rx state when both are disabled it is not > > possible to start/stop one multiple times while the other is running. > > Since the two are independently controlled, treat them as such and > > remove the false dependency between capture and playback. > > > > Signed-off-by: John Keeping <john-HooS5bfzL4hWk0Htik3J/w@public.gmane.org> > > --- > > sound/soc/rockchip/rockchip_i2s.c | 72 +++++++++++++++++---------------------- > > 1 file changed, 32 insertions(+), 40 deletions(-) > > > > diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c > > index 83b1b9c..acc6225 100644 > > --- a/sound/soc/rockchip/rockchip_i2s.c > > +++ b/sound/soc/rockchip/rockchip_i2s.c > > @@ -82,8 +82,8 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on) > > 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); > > + I2S_XFER_TXS_START, > > + I2S_XFER_TXS_START); > > > > i2s->tx_start = true; > > } else { > > @@ -92,27 +92,23 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on) > > regmap_update_bits(i2s->regmap, I2S_DMACR, > > I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_DISABLE); > > > > - 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); > > + regmap_update_bits(i2s->regmap, I2S_XFER, > > + I2S_XFER_TXS_START, > > + I2S_XFER_TXS_STOP); > > > > - regmap_update_bits(i2s->regmap, I2S_CLR, > > - I2S_CLR_TXC | I2S_CLR_RXC, > > - I2S_CLR_TXC | I2S_CLR_RXC); > > + regmap_update_bits(i2s->regmap, I2S_CLR, > > + I2S_CLR_TXC, > > + I2S_CLR_TXC); > > > > - regmap_read(i2s->regmap, I2S_CLR, &val); > > + regmap_read(i2s->regmap, I2S_CLR, &val); > > > > - /* Should wait for clear operation to finish */ > > - while (val) { > > - regmap_read(i2s->regmap, I2S_CLR, &val); > > - retry--; > > - if (!retry) { > > - dev_warn(i2s->dev, "fail to clear\n"); > > - break; > > - } > > + /* Should wait for clear operation to finish */ > > + while (val & I2S_CLR_TXC) { > > + regmap_read(i2s->regmap, I2S_CLR, &val); > > + retry--; > > + if (!retry) { > > + dev_warn(i2s->dev, "fail to clear\n"); > > + break; > > } > > } > > } > > @@ -128,8 +124,8 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on) > > 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); > > + I2S_XFER_RXS_START, > > + I2S_XFER_RXS_START); > > > > i2s->rx_start = true; > > } else { > > @@ -138,27 +134,23 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on) > > regmap_update_bits(i2s->regmap, I2S_DMACR, > > I2S_DMACR_RDE_ENABLE, I2S_DMACR_RDE_DISABLE); > > > > - 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); > > + regmap_update_bits(i2s->regmap, I2S_XFER, > > + I2S_XFER_RXS_START, > > + I2S_XFER_RXS_STOP); > > > > - regmap_update_bits(i2s->regmap, I2S_CLR, > > - I2S_CLR_TXC | I2S_CLR_RXC, > > - I2S_CLR_TXC | I2S_CLR_RXC); > > + regmap_update_bits(i2s->regmap, I2S_CLR, > > + I2S_CLR_RXC, > > + I2S_CLR_RXC); > > > > - regmap_read(i2s->regmap, I2S_CLR, &val); > > + regmap_read(i2s->regmap, I2S_CLR, &val); > > > > - /* Should wait for clear operation to finish */ > > - while (val) { > > - regmap_read(i2s->regmap, I2S_CLR, &val); > > - retry--; > > - if (!retry) { > > - dev_warn(i2s->dev, "fail to clear\n"); > > - break; > > - } > > + /* Should wait for clear operation to finish */ > > + while (val & I2S_CLR_RXC) { > > + regmap_read(i2s->regmap, I2S_CLR, &val); > > + retry--; > > + if (!retry) { > > + dev_warn(i2s->dev, "fail to clear\n"); > > + break; > > } > > } > > } > > -- > > 2.6.3.462.gbe2c914 > > Using my Veyron Jerry Chromebook wiht latest kernel I found that the > speakers doesn't work. Bisect points to this patch as the offending > commit. However your changes looks reasonable the fact is that > reverting the patch makes the audio work again on my device. I need to > dig a bit more into the issue, but meanwhile, any idea on what is > happening ? Can I ask which device did you test? I tested this on a Radxa Rock2 square. This change fixed an issue I found when running separate processes for playback and recording (I was testing with arecord and speaker-test IIRC) but I don't think that should be relevant. I just read through the patch again and I can't see anything obviously wrong so I'm afraid I don't have any idea what's causing the problem you're seeing. John ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] ASoC: rockchip: i2s: separate capture and playback 2016-04-30 7:00 ` John Keeping @ 2016-05-02 9:13 ` Enric Balletbo Serra 2016-05-03 4:12 ` sugar 1 sibling, 0 replies; 13+ messages in thread From: Enric Balletbo Serra @ 2016-05-02 9:13 UTC (permalink / raw) To: John Keeping Cc: linux-rockchip, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai, Heiko Stuebner, alsa-devel, linux-arm-kernel@lists.infradead.org, linux-kernel Hi John, Thanks for answer. 2016-04-30 9:00 GMT+02:00 John Keeping <john@keeping.me.uk>: > Hi Enric, > > On Fri, Apr 29, 2016 at 04:59:27PM +0200, Enric Balletbo Serra wrote: >> 2015-12-09 11:32 GMT+01:00 John Keeping <john@metanate.com>: >> > If we only clear the tx/rx state when both are disabled it is not >> > possible to start/stop one multiple times while the other is running. >> > Since the two are independently controlled, treat them as such and >> > remove the false dependency between capture and playback. >> > >> > Signed-off-by: John Keeping <john@metanate.com> >> > --- >> > sound/soc/rockchip/rockchip_i2s.c | 72 +++++++++++++++++---------------------- >> > 1 file changed, 32 insertions(+), 40 deletions(-) >> > >> > diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c >> > index 83b1b9c..acc6225 100644 >> > --- a/sound/soc/rockchip/rockchip_i2s.c >> > +++ b/sound/soc/rockchip/rockchip_i2s.c >> > @@ -82,8 +82,8 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on) >> > 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); >> > + I2S_XFER_TXS_START, >> > + I2S_XFER_TXS_START); >> > >> > i2s->tx_start = true; >> > } else { >> > @@ -92,27 +92,23 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on) >> > regmap_update_bits(i2s->regmap, I2S_DMACR, >> > I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_DISABLE); >> > >> > - 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); >> > + regmap_update_bits(i2s->regmap, I2S_XFER, >> > + I2S_XFER_TXS_START, >> > + I2S_XFER_TXS_STOP); >> > >> > - regmap_update_bits(i2s->regmap, I2S_CLR, >> > - I2S_CLR_TXC | I2S_CLR_RXC, >> > - I2S_CLR_TXC | I2S_CLR_RXC); >> > + regmap_update_bits(i2s->regmap, I2S_CLR, >> > + I2S_CLR_TXC, >> > + I2S_CLR_TXC); >> > >> > - regmap_read(i2s->regmap, I2S_CLR, &val); >> > + regmap_read(i2s->regmap, I2S_CLR, &val); >> > >> > - /* Should wait for clear operation to finish */ >> > - while (val) { >> > - regmap_read(i2s->regmap, I2S_CLR, &val); >> > - retry--; >> > - if (!retry) { >> > - dev_warn(i2s->dev, "fail to clear\n"); >> > - break; >> > - } >> > + /* Should wait for clear operation to finish */ >> > + while (val & I2S_CLR_TXC) { >> > + regmap_read(i2s->regmap, I2S_CLR, &val); >> > + retry--; >> > + if (!retry) { >> > + dev_warn(i2s->dev, "fail to clear\n"); >> > + break; >> > } >> > } >> > } >> > @@ -128,8 +124,8 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on) >> > 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); >> > + I2S_XFER_RXS_START, >> > + I2S_XFER_RXS_START); >> > >> > i2s->rx_start = true; >> > } else { >> > @@ -138,27 +134,23 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on) >> > regmap_update_bits(i2s->regmap, I2S_DMACR, >> > I2S_DMACR_RDE_ENABLE, I2S_DMACR_RDE_DISABLE); >> > >> > - 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); >> > + regmap_update_bits(i2s->regmap, I2S_XFER, >> > + I2S_XFER_RXS_START, >> > + I2S_XFER_RXS_STOP); >> > >> > - regmap_update_bits(i2s->regmap, I2S_CLR, >> > - I2S_CLR_TXC | I2S_CLR_RXC, >> > - I2S_CLR_TXC | I2S_CLR_RXC); >> > + regmap_update_bits(i2s->regmap, I2S_CLR, >> > + I2S_CLR_RXC, >> > + I2S_CLR_RXC); >> > >> > - regmap_read(i2s->regmap, I2S_CLR, &val); >> > + regmap_read(i2s->regmap, I2S_CLR, &val); >> > >> > - /* Should wait for clear operation to finish */ >> > - while (val) { >> > - regmap_read(i2s->regmap, I2S_CLR, &val); >> > - retry--; >> > - if (!retry) { >> > - dev_warn(i2s->dev, "fail to clear\n"); >> > - break; >> > - } >> > + /* Should wait for clear operation to finish */ >> > + while (val & I2S_CLR_RXC) { >> > + regmap_read(i2s->regmap, I2S_CLR, &val); >> > + retry--; >> > + if (!retry) { >> > + dev_warn(i2s->dev, "fail to clear\n"); >> > + break; >> > } >> > } >> > } >> > -- >> > 2.6.3.462.gbe2c914 >> >> Using my Veyron Jerry Chromebook wiht latest kernel I found that the >> speakers doesn't work. Bisect points to this patch as the offending >> commit. However your changes looks reasonable the fact is that >> reverting the patch makes the audio work again on my device. I need to >> dig a bit more into the issue, but meanwhile, any idea on what is >> happening ? Can I ask which device did you test? > > I tested this on a Radxa Rock2 square. This change fixed an issue I > found when running separate processes for playback and recording (I was > testing with arecord and speaker-test IIRC) but I don't think that > should be relevant. I just read through the patch again and I can't see > anything obviously wrong so I'm afraid I don't have any idea what's > causing the problem you're seeing. > By chance, do you have a git repo containing the changes required to test the sound on Radxa Rock2? Guess this is not in mainline. Thanks, Enric > > John ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] ASoC: rockchip: i2s: separate capture and playback 2016-04-30 7:00 ` John Keeping 2016-05-02 9:13 ` Enric Balletbo Serra @ 2016-05-03 4:12 ` sugar 2016-05-03 9:16 ` Enric Balletbo Serra 1 sibling, 1 reply; 13+ messages in thread From: sugar @ 2016-05-03 4:12 UTC (permalink / raw) To: John Keeping, Enric Balletbo Serra Cc: alsa-devel, Heiko Stuebner, linux-kernel, Liam Girdwood, Takashi Iwai, linux-rockchip, Mark Brown, linux-arm-kernel@lists.infradead.org Hi John, On 4/30/2016 15:00, John Keeping Wrote: > Hi Enric, > > On Fri, Apr 29, 2016 at 04:59:27PM +0200, Enric Balletbo Serra wrote: >> 2015-12-09 11:32 GMT+01:00 John Keeping <john@metanate.com>: >>> If we only clear the tx/rx state when both are disabled it is not >>> possible to start/stop one multiple times while the other is running. >>> Since the two are independently controlled, treat them as such and >>> remove the false dependency between capture and playback. >>> >>> Signed-off-by: John Keeping <john@metanate.com> >>> --- >>> sound/soc/rockchip/rockchip_i2s.c | 72 +++++++++++++++++---------------------- >>> 1 file changed, 32 insertions(+), 40 deletions(-) >>> >>> diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c >>> index 83b1b9c..acc6225 100644 >>> --- a/sound/soc/rockchip/rockchip_i2s.c >>> +++ b/sound/soc/rockchip/rockchip_i2s.c >>> @@ -82,8 +82,8 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on) >>> 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); >>> + I2S_XFER_TXS_START, >>> + I2S_XFER_TXS_START); >>> >>> i2s->tx_start = true; >>> } else { >>> @@ -92,27 +92,23 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on) >>> regmap_update_bits(i2s->regmap, I2S_DMACR, >>> I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_DISABLE); >>> >>> - 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); >>> + regmap_update_bits(i2s->regmap, I2S_XFER, >>> + I2S_XFER_TXS_START, >>> + I2S_XFER_TXS_STOP); >>> >>> - regmap_update_bits(i2s->regmap, I2S_CLR, >>> - I2S_CLR_TXC | I2S_CLR_RXC, >>> - I2S_CLR_TXC | I2S_CLR_RXC); >>> + regmap_update_bits(i2s->regmap, I2S_CLR, >>> + I2S_CLR_TXC, >>> + I2S_CLR_TXC); >>> >>> - regmap_read(i2s->regmap, I2S_CLR, &val); >>> + regmap_read(i2s->regmap, I2S_CLR, &val); >>> >>> - /* Should wait for clear operation to finish */ >>> - while (val) { >>> - regmap_read(i2s->regmap, I2S_CLR, &val); >>> - retry--; >>> - if (!retry) { >>> - dev_warn(i2s->dev, "fail to clear\n"); >>> - break; >>> - } >>> + /* Should wait for clear operation to finish */ >>> + while (val & I2S_CLR_TXC) { >>> + regmap_read(i2s->regmap, I2S_CLR, &val); >>> + retry--; >>> + if (!retry) { >>> + dev_warn(i2s->dev, "fail to clear\n"); >>> + break; >>> } >>> } >>> } >>> @@ -128,8 +124,8 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on) >>> 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); >>> + I2S_XFER_RXS_START, >>> + I2S_XFER_RXS_START); >>> >>> i2s->rx_start = true; >>> } else { >>> @@ -138,27 +134,23 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on) >>> regmap_update_bits(i2s->regmap, I2S_DMACR, >>> I2S_DMACR_RDE_ENABLE, I2S_DMACR_RDE_DISABLE); >>> >>> - 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); >>> + regmap_update_bits(i2s->regmap, I2S_XFER, >>> + I2S_XFER_RXS_START, >>> + I2S_XFER_RXS_STOP); >>> >>> - regmap_update_bits(i2s->regmap, I2S_CLR, >>> - I2S_CLR_TXC | I2S_CLR_RXC, >>> - I2S_CLR_TXC | I2S_CLR_RXC); >>> + regmap_update_bits(i2s->regmap, I2S_CLR, >>> + I2S_CLR_RXC, >>> + I2S_CLR_RXC); >>> >>> - regmap_read(i2s->regmap, I2S_CLR, &val); >>> + regmap_read(i2s->regmap, I2S_CLR, &val); >>> >>> - /* Should wait for clear operation to finish */ >>> - while (val) { >>> - regmap_read(i2s->regmap, I2S_CLR, &val); >>> - retry--; >>> - if (!retry) { >>> - dev_warn(i2s->dev, "fail to clear\n"); >>> - break; >>> - } >>> + /* Should wait for clear operation to finish */ >>> + while (val & I2S_CLR_RXC) { >>> + regmap_read(i2s->regmap, I2S_CLR, &val); >>> + retry--; >>> + if (!retry) { >>> + dev_warn(i2s->dev, "fail to clear\n"); >>> + break; >>> } >>> } >>> } >>> -- >>> 2.6.3.462.gbe2c914 >> >> Using my Veyron Jerry Chromebook wiht latest kernel I found that the >> speakers doesn't work. Bisect points to this patch as the offending >> commit. However your changes looks reasonable the fact is that >> reverting the patch makes the audio work again on my device. I need to >> dig a bit more into the issue, but meanwhile, any idea on what is >> happening ? Can I ask which device did you test? > > I tested this on a Radxa Rock2 square. This change fixed an issue I > found when running separate processes for playback and recording (I was > testing with arecord and speaker-test IIRC) but I don't think that > should be relevant. I just read through the patch again and I can't see > anything obviously wrong so I'm afraid I don't have any idea what's > causing the problem you're seeing. > Do you have test playback/capture at the same time? I think you may reproduce this issue if you do as follows: step 1: tinyplay test.wav -D 0 -d 0 -p 1024 -n 4 & step 2: tinycap r.wav -D 0 -d 0 -p 1024 -n 4 & you will find that there is no music now. because of this problem, we enable tx/rx at the same time to WA this issue. May I know what's the issue that you met before? > > John > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] ASoC: rockchip: i2s: separate capture and playback 2016-05-03 4:12 ` sugar @ 2016-05-03 9:16 ` Enric Balletbo Serra 2016-05-03 9:43 ` [alsa-devel] " John Keeping 0 siblings, 1 reply; 13+ messages in thread From: Enric Balletbo Serra @ 2016-05-03 9:16 UTC (permalink / raw) To: sugar Cc: alsa-devel, Heiko Stuebner, John Keeping, Takashi Iwai, linux-kernel, Liam Girdwood, linux-rockchip, Mark Brown, linux-arm-kernel@lists.infradead.org Hi all, 2016-05-03 6:12 GMT+02:00 sugar <sugar.zhang@rock-chips.com>: > Hi John, > > > On 4/30/2016 15:00, John Keeping Wrote: >> >> Hi Enric, >> >> On Fri, Apr 29, 2016 at 04:59:27PM +0200, Enric Balletbo Serra wrote: >>> >>> 2015-12-09 11:32 GMT+01:00 John Keeping <john@metanate.com>: >>>> >>>> If we only clear the tx/rx state when both are disabled it is not >>>> possible to start/stop one multiple times while the other is running. >>>> Since the two are independently controlled, treat them as such and >>>> remove the false dependency between capture and playback. >>>> >>>> Signed-off-by: John Keeping <john@metanate.com> >>>> --- >>>> sound/soc/rockchip/rockchip_i2s.c | 72 >>>> +++++++++++++++++---------------------- >>>> 1 file changed, 32 insertions(+), 40 deletions(-) >>>> >>>> diff --git a/sound/soc/rockchip/rockchip_i2s.c >>>> b/sound/soc/rockchip/rockchip_i2s.c >>>> index 83b1b9c..acc6225 100644 >>>> --- a/sound/soc/rockchip/rockchip_i2s.c >>>> +++ b/sound/soc/rockchip/rockchip_i2s.c >>>> @@ -82,8 +82,8 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev >>>> *i2s, int on) >>>> 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); >>>> + I2S_XFER_TXS_START, >>>> + I2S_XFER_TXS_START); >>>> >>>> i2s->tx_start = true; >>>> } else { >>>> @@ -92,27 +92,23 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev >>>> *i2s, int on) >>>> regmap_update_bits(i2s->regmap, I2S_DMACR, >>>> I2S_DMACR_TDE_ENABLE, >>>> I2S_DMACR_TDE_DISABLE); >>>> >>>> - 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); >>>> + regmap_update_bits(i2s->regmap, I2S_XFER, >>>> + I2S_XFER_TXS_START, >>>> + I2S_XFER_TXS_STOP); >>>> >>>> - regmap_update_bits(i2s->regmap, I2S_CLR, >>>> - I2S_CLR_TXC | I2S_CLR_RXC, >>>> - I2S_CLR_TXC | I2S_CLR_RXC); >>>> + regmap_update_bits(i2s->regmap, I2S_CLR, >>>> + I2S_CLR_TXC, >>>> + I2S_CLR_TXC); >>>> >>>> - regmap_read(i2s->regmap, I2S_CLR, &val); >>>> + regmap_read(i2s->regmap, I2S_CLR, &val); >>>> >>>> - /* Should wait for clear operation to finish */ >>>> - while (val) { >>>> - regmap_read(i2s->regmap, I2S_CLR, &val); >>>> - retry--; >>>> - if (!retry) { >>>> - dev_warn(i2s->dev, "fail to >>>> clear\n"); >>>> - break; >>>> - } >>>> + /* Should wait for clear operation to finish */ >>>> + while (val & I2S_CLR_TXC) { >>>> + regmap_read(i2s->regmap, I2S_CLR, &val); >>>> + retry--; >>>> + if (!retry) { >>>> + dev_warn(i2s->dev, "fail to clear\n"); >>>> + break; >>>> } >>>> } >>>> } >>>> @@ -128,8 +124,8 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev >>>> *i2s, int on) >>>> 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); >>>> + I2S_XFER_RXS_START, >>>> + I2S_XFER_RXS_START); >>>> >>>> i2s->rx_start = true; >>>> } else { >>>> @@ -138,27 +134,23 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev >>>> *i2s, int on) >>>> regmap_update_bits(i2s->regmap, I2S_DMACR, >>>> I2S_DMACR_RDE_ENABLE, >>>> I2S_DMACR_RDE_DISABLE); >>>> >>>> - 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); >>>> + regmap_update_bits(i2s->regmap, I2S_XFER, >>>> + I2S_XFER_RXS_START, >>>> + I2S_XFER_RXS_STOP); >>>> >>>> - regmap_update_bits(i2s->regmap, I2S_CLR, >>>> - I2S_CLR_TXC | I2S_CLR_RXC, >>>> - I2S_CLR_TXC | I2S_CLR_RXC); >>>> + regmap_update_bits(i2s->regmap, I2S_CLR, >>>> + I2S_CLR_RXC, >>>> + I2S_CLR_RXC); >>>> >>>> - regmap_read(i2s->regmap, I2S_CLR, &val); >>>> + regmap_read(i2s->regmap, I2S_CLR, &val); >>>> >>>> - /* Should wait for clear operation to finish */ >>>> - while (val) { >>>> - regmap_read(i2s->regmap, I2S_CLR, &val); >>>> - retry--; >>>> - if (!retry) { >>>> - dev_warn(i2s->dev, "fail to >>>> clear\n"); >>>> - break; >>>> - } >>>> + /* Should wait for clear operation to finish */ >>>> + while (val & I2S_CLR_RXC) { >>>> + regmap_read(i2s->regmap, I2S_CLR, &val); >>>> + retry--; >>>> + if (!retry) { >>>> + dev_warn(i2s->dev, "fail to clear\n"); >>>> + break; >>>> } >>>> } >>>> } >>>> -- >>>> 2.6.3.462.gbe2c914 >>> >>> >>> Using my Veyron Jerry Chromebook wiht latest kernel I found that the >>> speakers doesn't work. Bisect points to this patch as the offending >>> commit. However your changes looks reasonable the fact is that >>> reverting the patch makes the audio work again on my device. I need to >>> dig a bit more into the issue, but meanwhile, any idea on what is >>> happening ? Can I ask which device did you test? >> >> >> I tested this on a Radxa Rock2 square. This change fixed an issue I >> found when running separate processes for playback and recording (I was >> testing with arecord and speaker-test IIRC) but I don't think that >> should be relevant. I just read through the patch again and I can't see >> anything obviously wrong so I'm afraid I don't have any idea what's >> causing the problem you're seeing. >> > Do you have test playback/capture at the same time? I think you may > reproduce this issue if you do as follows: > > step 1: tinyplay test.wav -D 0 -d 0 -p 1024 -n 4 & > step 2: tinycap r.wav -D 0 -d 0 -p 1024 -n 4 & > > you will find that there is no music now. > > because of this problem, we enable tx/rx at the same time to WA this issue. > I can confirm that reverting John's patch audio playback/capture at the same time works on my Veyron Jerry Chromebook. > May I know what's the issue that you met before? >> >> >> John >> >> _______________________________________________ >> Linux-rockchip mailing list >> Linux-rockchip@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-rockchip >> >> >> > > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [alsa-devel] [PATCH 1/2] ASoC: rockchip: i2s: separate capture and playback 2016-05-03 9:16 ` Enric Balletbo Serra @ 2016-05-03 9:43 ` John Keeping 2016-05-03 10:21 ` Enric Balletbo Serra 0 siblings, 1 reply; 13+ messages in thread From: John Keeping @ 2016-05-03 9:43 UTC (permalink / raw) To: Enric Balletbo Serra Cc: sugar, John Keeping, alsa-devel, Heiko Stuebner, linux-kernel, Liam Girdwood, Takashi Iwai, linux-rockchip, Mark Brown, linux-arm-kernel@lists.infradead.org Hi Enric, On Tue, 3 May 2016 11:16:58 +0200, Enric Balletbo Serra wrote: > 2016-05-03 6:12 GMT+02:00 sugar <sugar.zhang@rock-chips.com>: > > On 4/30/2016 15:00, John Keeping Wrote: > >> On Fri, Apr 29, 2016 at 04:59:27PM +0200, Enric Balletbo Serra wrote: > >>> > >>> 2015-12-09 11:32 GMT+01:00 John Keeping <john@metanate.com>: > >>>> > >>>> If we only clear the tx/rx state when both are disabled it is not > >>>> possible to start/stop one multiple times while the other is running. > >>>> Since the two are independently controlled, treat them as such and > >>>> remove the false dependency between capture and playback. > >>>> > >>>> Signed-off-by: John Keeping <john@metanate.com> > >>>> --- > >>>> sound/soc/rockchip/rockchip_i2s.c | 72 > >>>> +++++++++++++++++---------------------- > >>>> 1 file changed, 32 insertions(+), 40 deletions(-) > >>>> > >>>> diff --git a/sound/soc/rockchip/rockchip_i2s.c > >>>> b/sound/soc/rockchip/rockchip_i2s.c > >>>> index 83b1b9c..acc6225 100644 > >>>> --- a/sound/soc/rockchip/rockchip_i2s.c > >>>> +++ b/sound/soc/rockchip/rockchip_i2s.c > >>>> @@ -82,8 +82,8 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev > >>>> *i2s, int on) > >>>> 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); > >>>> + I2S_XFER_TXS_START, > >>>> + I2S_XFER_TXS_START); > >>>> > >>>> i2s->tx_start = true; > >>>> } else { > >>>> @@ -92,27 +92,23 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev > >>>> *i2s, int on) > >>>> regmap_update_bits(i2s->regmap, I2S_DMACR, > >>>> I2S_DMACR_TDE_ENABLE, > >>>> I2S_DMACR_TDE_DISABLE); > >>>> > >>>> - 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); > >>>> + regmap_update_bits(i2s->regmap, I2S_XFER, > >>>> + I2S_XFER_TXS_START, > >>>> + I2S_XFER_TXS_STOP); > >>>> > >>>> - regmap_update_bits(i2s->regmap, I2S_CLR, > >>>> - I2S_CLR_TXC | I2S_CLR_RXC, > >>>> - I2S_CLR_TXC | I2S_CLR_RXC); > >>>> + regmap_update_bits(i2s->regmap, I2S_CLR, > >>>> + I2S_CLR_TXC, > >>>> + I2S_CLR_TXC); > >>>> > >>>> - regmap_read(i2s->regmap, I2S_CLR, &val); > >>>> + regmap_read(i2s->regmap, I2S_CLR, &val); > >>>> > >>>> - /* Should wait for clear operation to finish */ > >>>> - while (val) { > >>>> - regmap_read(i2s->regmap, I2S_CLR, &val); > >>>> - retry--; > >>>> - if (!retry) { > >>>> - dev_warn(i2s->dev, "fail to > >>>> clear\n"); > >>>> - break; > >>>> - } > >>>> + /* Should wait for clear operation to finish */ > >>>> + while (val & I2S_CLR_TXC) { > >>>> + regmap_read(i2s->regmap, I2S_CLR, &val); > >>>> + retry--; > >>>> + if (!retry) { > >>>> + dev_warn(i2s->dev, "fail to clear\n"); > >>>> + break; > >>>> } > >>>> } > >>>> } > >>>> @@ -128,8 +124,8 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev > >>>> *i2s, int on) > >>>> 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); > >>>> + I2S_XFER_RXS_START, > >>>> + I2S_XFER_RXS_START); > >>>> > >>>> i2s->rx_start = true; > >>>> } else { > >>>> @@ -138,27 +134,23 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev > >>>> *i2s, int on) > >>>> regmap_update_bits(i2s->regmap, I2S_DMACR, > >>>> I2S_DMACR_RDE_ENABLE, > >>>> I2S_DMACR_RDE_DISABLE); > >>>> > >>>> - 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); > >>>> + regmap_update_bits(i2s->regmap, I2S_XFER, > >>>> + I2S_XFER_RXS_START, > >>>> + I2S_XFER_RXS_STOP); > >>>> > >>>> - regmap_update_bits(i2s->regmap, I2S_CLR, > >>>> - I2S_CLR_TXC | I2S_CLR_RXC, > >>>> - I2S_CLR_TXC | I2S_CLR_RXC); > >>>> + regmap_update_bits(i2s->regmap, I2S_CLR, > >>>> + I2S_CLR_RXC, > >>>> + I2S_CLR_RXC); > >>>> > >>>> - regmap_read(i2s->regmap, I2S_CLR, &val); > >>>> + regmap_read(i2s->regmap, I2S_CLR, &val); > >>>> > >>>> - /* Should wait for clear operation to finish */ > >>>> - while (val) { > >>>> - regmap_read(i2s->regmap, I2S_CLR, &val); > >>>> - retry--; > >>>> - if (!retry) { > >>>> - dev_warn(i2s->dev, "fail to > >>>> clear\n"); > >>>> - break; > >>>> - } > >>>> + /* Should wait for clear operation to finish */ > >>>> + while (val & I2S_CLR_RXC) { > >>>> + regmap_read(i2s->regmap, I2S_CLR, &val); > >>>> + retry--; > >>>> + if (!retry) { > >>>> + dev_warn(i2s->dev, "fail to clear\n"); > >>>> + break; > >>>> } > >>>> } > >>>> } > >>>> -- > >>>> 2.6.3.462.gbe2c914 > >>> > >>> > >>> Using my Veyron Jerry Chromebook wiht latest kernel I found that the > >>> speakers doesn't work. Bisect points to this patch as the offending > >>> commit. However your changes looks reasonable the fact is that > >>> reverting the patch makes the audio work again on my device. I need to > >>> dig a bit more into the issue, but meanwhile, any idea on what is > >>> happening ? Can I ask which device did you test? > >> > >> > >> I tested this on a Radxa Rock2 square. This change fixed an issue I > >> found when running separate processes for playback and recording (I was > >> testing with arecord and speaker-test IIRC) but I don't think that > >> should be relevant. I just read through the patch again and I can't see > >> anything obviously wrong so I'm afraid I don't have any idea what's > >> causing the problem you're seeing. > >> > > Do you have test playback/capture at the same time? I think you may > > reproduce this issue if you do as follows: > > > > step 1: tinyplay test.wav -D 0 -d 0 -p 1024 -n 4 & > > step 2: tinycap r.wav -D 0 -d 0 -p 1024 -n 4 & > > > > you will find that there is no music now. > > > > because of this problem, we enable tx/rx at the same time to WA this issue. > > > > I can confirm that reverting John's patch audio playback/capture at > the same time works on my Veyron Jerry Chromebook. Have you tried stopping one of the processes and then starting it again (without touching the other process)? That's the case that was broken for me without this patch. I have had a quick look through the code I'm using and I noticed that I have another change that hasn't been sent upstream, which marks the I2S_FIFOLR register as volatile in rockchip_i2s_volatile_reg(): -- >8 -- Subject: [PATCH] ASoC: rockchip: i2s: mark FIFOLR register volatile The FIFO levels will be updated by the device so they should not be cached. Signed-off-by: John Keeping <john@metanate.com> --- sound/soc/rockchip/rockchip_i2s.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c index d78f4c925b02..b517d8abcc40 100644 --- a/sound/soc/rockchip/rockchip_i2s.c +++ b/sound/soc/rockchip/rockchip_i2s.c @@ -434,6 +434,7 @@ static bool rockchip_i2s_volatile_reg(struct device *dev, unsigned int reg) switch (reg) { case I2S_INTSR: case I2S_CLR: + case I2S_FIFOLR: return true; default: return false; -- 8< -- Regards, John ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [alsa-devel] [PATCH 1/2] ASoC: rockchip: i2s: separate capture and playback 2016-05-03 9:43 ` [alsa-devel] " John Keeping @ 2016-05-03 10:21 ` Enric Balletbo Serra 2016-05-03 10:32 ` John Keeping 0 siblings, 1 reply; 13+ messages in thread From: Enric Balletbo Serra @ 2016-05-03 10:21 UTC (permalink / raw) To: John Keeping Cc: sugar, John Keeping, alsa-devel, Heiko Stuebner, linux-kernel, Liam Girdwood, Takashi Iwai, linux-rockchip, Mark Brown, linux-arm-kernel@lists.infradead.org Hi John, 2016-05-03 11:43 GMT+02:00 John Keeping <john@metanate.com>: > Hi Enric, > > On Tue, 3 May 2016 11:16:58 +0200, Enric Balletbo Serra wrote: >> 2016-05-03 6:12 GMT+02:00 sugar <sugar.zhang@rock-chips.com>: >> > On 4/30/2016 15:00, John Keeping Wrote: >> >> On Fri, Apr 29, 2016 at 04:59:27PM +0200, Enric Balletbo Serra wrote: >> >>> >> >>> 2015-12-09 11:32 GMT+01:00 John Keeping <john@metanate.com>: >> >>>> >> >>>> If we only clear the tx/rx state when both are disabled it is not >> >>>> possible to start/stop one multiple times while the other is running. >> >>>> Since the two are independently controlled, treat them as such and >> >>>> remove the false dependency between capture and playback. >> >>>> >> >>>> Signed-off-by: John Keeping <john@metanate.com> >> >>>> --- >> >>>> sound/soc/rockchip/rockchip_i2s.c | 72 >> >>>> +++++++++++++++++---------------------- >> >>>> 1 file changed, 32 insertions(+), 40 deletions(-) >> >>>> >> >>>> diff --git a/sound/soc/rockchip/rockchip_i2s.c >> >>>> b/sound/soc/rockchip/rockchip_i2s.c >> >>>> index 83b1b9c..acc6225 100644 >> >>>> --- a/sound/soc/rockchip/rockchip_i2s.c >> >>>> +++ b/sound/soc/rockchip/rockchip_i2s.c >> >>>> @@ -82,8 +82,8 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev >> >>>> *i2s, int on) >> >>>> 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); >> >>>> + I2S_XFER_TXS_START, >> >>>> + I2S_XFER_TXS_START); >> >>>> >> >>>> i2s->tx_start = true; >> >>>> } else { >> >>>> @@ -92,27 +92,23 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev >> >>>> *i2s, int on) >> >>>> regmap_update_bits(i2s->regmap, I2S_DMACR, >> >>>> I2S_DMACR_TDE_ENABLE, >> >>>> I2S_DMACR_TDE_DISABLE); >> >>>> >> >>>> - 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); >> >>>> + regmap_update_bits(i2s->regmap, I2S_XFER, >> >>>> + I2S_XFER_TXS_START, >> >>>> + I2S_XFER_TXS_STOP); >> >>>> >> >>>> - regmap_update_bits(i2s->regmap, I2S_CLR, >> >>>> - I2S_CLR_TXC | I2S_CLR_RXC, >> >>>> - I2S_CLR_TXC | I2S_CLR_RXC); >> >>>> + regmap_update_bits(i2s->regmap, I2S_CLR, >> >>>> + I2S_CLR_TXC, >> >>>> + I2S_CLR_TXC); >> >>>> >> >>>> - regmap_read(i2s->regmap, I2S_CLR, &val); >> >>>> + regmap_read(i2s->regmap, I2S_CLR, &val); >> >>>> >> >>>> - /* Should wait for clear operation to finish */ >> >>>> - while (val) { >> >>>> - regmap_read(i2s->regmap, I2S_CLR, &val); >> >>>> - retry--; >> >>>> - if (!retry) { >> >>>> - dev_warn(i2s->dev, "fail to >> >>>> clear\n"); >> >>>> - break; >> >>>> - } >> >>>> + /* Should wait for clear operation to finish */ >> >>>> + while (val & I2S_CLR_TXC) { >> >>>> + regmap_read(i2s->regmap, I2S_CLR, &val); >> >>>> + retry--; >> >>>> + if (!retry) { >> >>>> + dev_warn(i2s->dev, "fail to clear\n"); >> >>>> + break; >> >>>> } >> >>>> } >> >>>> } >> >>>> @@ -128,8 +124,8 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev >> >>>> *i2s, int on) >> >>>> 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); >> >>>> + I2S_XFER_RXS_START, >> >>>> + I2S_XFER_RXS_START); >> >>>> >> >>>> i2s->rx_start = true; >> >>>> } else { >> >>>> @@ -138,27 +134,23 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev >> >>>> *i2s, int on) >> >>>> regmap_update_bits(i2s->regmap, I2S_DMACR, >> >>>> I2S_DMACR_RDE_ENABLE, >> >>>> I2S_DMACR_RDE_DISABLE); >> >>>> >> >>>> - 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); >> >>>> + regmap_update_bits(i2s->regmap, I2S_XFER, >> >>>> + I2S_XFER_RXS_START, >> >>>> + I2S_XFER_RXS_STOP); >> >>>> >> >>>> - regmap_update_bits(i2s->regmap, I2S_CLR, >> >>>> - I2S_CLR_TXC | I2S_CLR_RXC, >> >>>> - I2S_CLR_TXC | I2S_CLR_RXC); >> >>>> + regmap_update_bits(i2s->regmap, I2S_CLR, >> >>>> + I2S_CLR_RXC, >> >>>> + I2S_CLR_RXC); >> >>>> >> >>>> - regmap_read(i2s->regmap, I2S_CLR, &val); >> >>>> + regmap_read(i2s->regmap, I2S_CLR, &val); >> >>>> >> >>>> - /* Should wait for clear operation to finish */ >> >>>> - while (val) { >> >>>> - regmap_read(i2s->regmap, I2S_CLR, &val); >> >>>> - retry--; >> >>>> - if (!retry) { >> >>>> - dev_warn(i2s->dev, "fail to >> >>>> clear\n"); >> >>>> - break; >> >>>> - } >> >>>> + /* Should wait for clear operation to finish */ >> >>>> + while (val & I2S_CLR_RXC) { >> >>>> + regmap_read(i2s->regmap, I2S_CLR, &val); >> >>>> + retry--; >> >>>> + if (!retry) { >> >>>> + dev_warn(i2s->dev, "fail to clear\n"); >> >>>> + break; >> >>>> } >> >>>> } >> >>>> } >> >>>> -- >> >>>> 2.6.3.462.gbe2c914 >> >>> >> >>> >> >>> Using my Veyron Jerry Chromebook wiht latest kernel I found that the >> >>> speakers doesn't work. Bisect points to this patch as the offending >> >>> commit. However your changes looks reasonable the fact is that >> >>> reverting the patch makes the audio work again on my device. I need to >> >>> dig a bit more into the issue, but meanwhile, any idea on what is >> >>> happening ? Can I ask which device did you test? >> >> >> >> >> >> I tested this on a Radxa Rock2 square. This change fixed an issue I >> >> found when running separate processes for playback and recording (I was >> >> testing with arecord and speaker-test IIRC) but I don't think that >> >> should be relevant. I just read through the patch again and I can't see >> >> anything obviously wrong so I'm afraid I don't have any idea what's >> >> causing the problem you're seeing. >> >> >> > Do you have test playback/capture at the same time? I think you may >> > reproduce this issue if you do as follows: >> > >> > step 1: tinyplay test.wav -D 0 -d 0 -p 1024 -n 4 & >> > step 2: tinycap r.wav -D 0 -d 0 -p 1024 -n 4 & >> > >> > you will find that there is no music now. >> > >> > because of this problem, we enable tx/rx at the same time to WA this issue. >> > >> >> I can confirm that reverting John's patch audio playback/capture at >> the same time works on my Veyron Jerry Chromebook. > > Have you tried stopping one of the processes and then starting it again > (without touching the other process)? That's the case that was broken > for me without this patch. > I just tried that and it works on my platform. Meanwhile I reproduced a song I recorded two wavs. > I have had a quick look through the code I'm using and I noticed that I > have another change that hasn't been sent upstream, which marks the > I2S_FIFOLR register as volatile in rockchip_i2s_volatile_reg(): > Unfortunately that didn't help, I hear nothing from the speakers :( I have a Radxa Rock2 Square board, by chance, do you have a git tree containing the patches to enable the i2s sound for radxa so I can reproduce both here? At least the codec is not upstreamed, I guess. > -- >8 -- > Subject: [PATCH] ASoC: rockchip: i2s: mark FIFOLR register volatile > > The FIFO levels will be updated by the device so they should not be > cached. > > Signed-off-by: John Keeping <john@metanate.com> > --- > sound/soc/rockchip/rockchip_i2s.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c > index d78f4c925b02..b517d8abcc40 100644 > --- a/sound/soc/rockchip/rockchip_i2s.c > +++ b/sound/soc/rockchip/rockchip_i2s.c > @@ -434,6 +434,7 @@ static bool rockchip_i2s_volatile_reg(struct device *dev, unsigned int reg) > switch (reg) { > case I2S_INTSR: > case I2S_CLR: > + case I2S_FIFOLR: > return true; > default: > return false; > -- 8< -- > > > Regards, > John ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] ASoC: rockchip: i2s: separate capture and playback 2016-05-03 10:21 ` Enric Balletbo Serra @ 2016-05-03 10:32 ` John Keeping 0 siblings, 0 replies; 13+ messages in thread From: John Keeping @ 2016-05-03 10:32 UTC (permalink / raw) To: Enric Balletbo Serra Cc: alsa-devel, Heiko Stuebner, John Keeping, Takashi Iwai, linux-kernel, Liam Girdwood, sugar, linux-rockchip, Mark Brown, linux-arm-kernel@lists.infradead.org Hi Enric, On Tue, 3 May 2016 12:21:25 +0200, Enric Balletbo Serra wrote: > 2016-05-03 11:43 GMT+02:00 John Keeping <john@metanate.com>: > > On Tue, 3 May 2016 11:16:58 +0200, Enric Balletbo Serra wrote: > >> 2016-05-03 6:12 GMT+02:00 sugar <sugar.zhang@rock-chips.com>: > >> > On 4/30/2016 15:00, John Keeping Wrote: > >> >> On Fri, Apr 29, 2016 at 04:59:27PM +0200, Enric Balletbo Serra wrote: > >> >>> > >> >>> 2015-12-09 11:32 GMT+01:00 John Keeping <john@metanate.com>: > >> >>>> > >> >>>> If we only clear the tx/rx state when both are disabled it is not > >> >>>> possible to start/stop one multiple times while the other is running. > >> >>>> Since the two are independently controlled, treat them as such and > >> >>>> remove the false dependency between capture and playback. > >> >>>> > >> >>>> Signed-off-by: John Keeping <john@metanate.com> > >> >>>> --- > >> >>>> sound/soc/rockchip/rockchip_i2s.c | 72 > >> >>>> +++++++++++++++++---------------------- > >> >>>> 1 file changed, 32 insertions(+), 40 deletions(-) > >> >>>> > >> >>>> diff --git a/sound/soc/rockchip/rockchip_i2s.c > >> >>>> b/sound/soc/rockchip/rockchip_i2s.c > >> >>>> index 83b1b9c..acc6225 100644 > >> >>>> --- a/sound/soc/rockchip/rockchip_i2s.c > >> >>>> +++ b/sound/soc/rockchip/rockchip_i2s.c > >> >>>> @@ -82,8 +82,8 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev > >> >>>> *i2s, int on) > >> >>>> 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); > >> >>>> + I2S_XFER_TXS_START, > >> >>>> + I2S_XFER_TXS_START); > >> >>>> > >> >>>> i2s->tx_start = true; > >> >>>> } else { > >> >>>> @@ -92,27 +92,23 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev > >> >>>> *i2s, int on) > >> >>>> regmap_update_bits(i2s->regmap, I2S_DMACR, > >> >>>> I2S_DMACR_TDE_ENABLE, > >> >>>> I2S_DMACR_TDE_DISABLE); > >> >>>> > >> >>>> - 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); > >> >>>> + regmap_update_bits(i2s->regmap, I2S_XFER, > >> >>>> + I2S_XFER_TXS_START, > >> >>>> + I2S_XFER_TXS_STOP); > >> >>>> > >> >>>> - regmap_update_bits(i2s->regmap, I2S_CLR, > >> >>>> - I2S_CLR_TXC | I2S_CLR_RXC, > >> >>>> - I2S_CLR_TXC | I2S_CLR_RXC); > >> >>>> + regmap_update_bits(i2s->regmap, I2S_CLR, > >> >>>> + I2S_CLR_TXC, > >> >>>> + I2S_CLR_TXC); > >> >>>> > >> >>>> - regmap_read(i2s->regmap, I2S_CLR, &val); > >> >>>> + regmap_read(i2s->regmap, I2S_CLR, &val); > >> >>>> > >> >>>> - /* Should wait for clear operation to finish */ > >> >>>> - while (val) { > >> >>>> - regmap_read(i2s->regmap, I2S_CLR, &val); > >> >>>> - retry--; > >> >>>> - if (!retry) { > >> >>>> - dev_warn(i2s->dev, "fail to > >> >>>> clear\n"); > >> >>>> - break; > >> >>>> - } > >> >>>> + /* Should wait for clear operation to finish */ > >> >>>> + while (val & I2S_CLR_TXC) { > >> >>>> + regmap_read(i2s->regmap, I2S_CLR, &val); > >> >>>> + retry--; > >> >>>> + if (!retry) { > >> >>>> + dev_warn(i2s->dev, "fail to clear\n"); > >> >>>> + break; > >> >>>> } > >> >>>> } > >> >>>> } > >> >>>> @@ -128,8 +124,8 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev > >> >>>> *i2s, int on) > >> >>>> 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); > >> >>>> + I2S_XFER_RXS_START, > >> >>>> + I2S_XFER_RXS_START); > >> >>>> > >> >>>> i2s->rx_start = true; > >> >>>> } else { > >> >>>> @@ -138,27 +134,23 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev > >> >>>> *i2s, int on) > >> >>>> regmap_update_bits(i2s->regmap, I2S_DMACR, > >> >>>> I2S_DMACR_RDE_ENABLE, > >> >>>> I2S_DMACR_RDE_DISABLE); > >> >>>> > >> >>>> - 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); > >> >>>> + regmap_update_bits(i2s->regmap, I2S_XFER, > >> >>>> + I2S_XFER_RXS_START, > >> >>>> + I2S_XFER_RXS_STOP); > >> >>>> > >> >>>> - regmap_update_bits(i2s->regmap, I2S_CLR, > >> >>>> - I2S_CLR_TXC | I2S_CLR_RXC, > >> >>>> - I2S_CLR_TXC | I2S_CLR_RXC); > >> >>>> + regmap_update_bits(i2s->regmap, I2S_CLR, > >> >>>> + I2S_CLR_RXC, > >> >>>> + I2S_CLR_RXC); > >> >>>> > >> >>>> - regmap_read(i2s->regmap, I2S_CLR, &val); > >> >>>> + regmap_read(i2s->regmap, I2S_CLR, &val); > >> >>>> > >> >>>> - /* Should wait for clear operation to finish */ > >> >>>> - while (val) { > >> >>>> - regmap_read(i2s->regmap, I2S_CLR, &val); > >> >>>> - retry--; > >> >>>> - if (!retry) { > >> >>>> - dev_warn(i2s->dev, "fail to > >> >>>> clear\n"); > >> >>>> - break; > >> >>>> - } > >> >>>> + /* Should wait for clear operation to finish */ > >> >>>> + while (val & I2S_CLR_RXC) { > >> >>>> + regmap_read(i2s->regmap, I2S_CLR, &val); > >> >>>> + retry--; > >> >>>> + if (!retry) { > >> >>>> + dev_warn(i2s->dev, "fail to clear\n"); > >> >>>> + break; > >> >>>> } > >> >>>> } > >> >>>> } > >> >>>> -- > >> >>>> 2.6.3.462.gbe2c914 > >> >>> > >> >>> > >> >>> Using my Veyron Jerry Chromebook wiht latest kernel I found that the > >> >>> speakers doesn't work. Bisect points to this patch as the offending > >> >>> commit. However your changes looks reasonable the fact is that > >> >>> reverting the patch makes the audio work again on my device. I need to > >> >>> dig a bit more into the issue, but meanwhile, any idea on what is > >> >>> happening ? Can I ask which device did you test? > >> >> > >> >> > >> >> I tested this on a Radxa Rock2 square. This change fixed an issue I > >> >> found when running separate processes for playback and recording (I was > >> >> testing with arecord and speaker-test IIRC) but I don't think that > >> >> should be relevant. I just read through the patch again and I can't see > >> >> anything obviously wrong so I'm afraid I don't have any idea what's > >> >> causing the problem you're seeing. > >> >> > >> > Do you have test playback/capture at the same time? I think you may > >> > reproduce this issue if you do as follows: > >> > > >> > step 1: tinyplay test.wav -D 0 -d 0 -p 1024 -n 4 & > >> > step 2: tinycap r.wav -D 0 -d 0 -p 1024 -n 4 & > >> > > >> > you will find that there is no music now. > >> > > >> > because of this problem, we enable tx/rx at the same time to WA this issue. > >> > > >> > >> I can confirm that reverting John's patch audio playback/capture at > >> the same time works on my Veyron Jerry Chromebook. > > > > Have you tried stopping one of the processes and then starting it again > > (without touching the other process)? That's the case that was broken > > for me without this patch. > > > > I just tried that and it works on my platform. Meanwhile I reproduced > a song I recorded two wavs. > > > I have had a quick look through the code I'm using and I noticed that I > > have another change that hasn't been sent upstream, which marks the > > I2S_FIFOLR register as volatile in rockchip_i2s_volatile_reg(): > > > > Unfortunately that didn't help, I hear nothing from the speakers :( Yeah, I looked a bit more after sending the previous mail and that register is never referenced by the driver! > I have a Radxa Rock2 Square board, by chance, do you have a git tree > containing the patches to enable the i2s sound for radxa so I can > reproduce both here? At least the codec is not upstreamed, I guess. I've just pushed a branch to Github: https://github.com/johnkeeping/linux/tree/topic/audio Regards, John ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2016-05-03 10:32 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-09 10:32 [PATCH 1/2] ASoC: rockchip: i2s: separate capture and playback John Keeping
[not found] ` <1449657147-26959-1-git-send-email-john-HooS5bfzL4hWk0Htik3J/w@public.gmane.org>
2015-12-09 10:32 ` [PATCH 2/2] ASoC: rockchip: i2s: remove unused variables John Keeping
2015-12-09 20:44 ` Applied "ASoC: rockchip: i2s: remove unused variables" to the asoc tree Mark Brown
2015-12-09 20:44 ` Applied "ASoC: rockchip: i2s: separate capture and playback" " Mark Brown
2016-04-29 14:55 ` [PATCH 1/2] ASoC: rockchip: i2s: separate capture and playback Enric Balletbo Serra
2016-04-29 14:59 ` Enric Balletbo Serra
[not found] ` <CAFqH_52C4JKno4uyt54UdWKAJKM4FPGRiQaQQ9_H-Mz1Cr+Hqg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-04-30 7:00 ` John Keeping
2016-05-02 9:13 ` Enric Balletbo Serra
2016-05-03 4:12 ` sugar
2016-05-03 9:16 ` Enric Balletbo Serra
2016-05-03 9:43 ` [alsa-devel] " John Keeping
2016-05-03 10:21 ` Enric Balletbo Serra
2016-05-03 10:32 ` John Keeping
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).