Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752250AbcD3HLv (ORCPT ); Sat, 30 Apr 2016 03:11:51 -0400 Received: from mta02.prd.rdg.aluminati.org ([94.76.243.215]:44059 "EHLO mta02.prd.rdg.aluminati.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751357AbcD3HLu (ORCPT ); Sat, 30 Apr 2016 03:11:50 -0400 X-Greylist: delayed 633 seconds by postgrey-1.27 at vger.kernel.org; Sat, 30 Apr 2016 03:11:49 EDT X-Quarantine-ID: X-Spam-Flag: NO X-Spam-Score: -2.149 Date: Sat, 30 Apr 2016 08:00:49 +0100 From: John Keeping To: Enric Balletbo Serra Cc: linux-rockchip@lists.infradead.org, Liam Girdwood , Mark Brown , Jaroslav Kysela , Takashi Iwai , Heiko Stuebner , alsa-devel@alsa-project.org, "linux-arm-kernel@lists.infradead.org" , linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/2] ASoC: rockchip: i2s: separate capture and playback Message-ID: <20160430070049.GF14612@serenity.lan> References: <1449657147-26959-1-git-send-email-john@metanate.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7304 Lines: 143 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 : > > 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 > > --- > > 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