[alsa-devel] [PATCH 3/5] ASoC: dwc: Iterate over all channels
On the Designware core, the channels are independent and not combined in higher registers. So as more channels are added, more registers need to be updated.
Signed-off-by: Andrew Jackson Andrew.Jackson@arm.com --- sound/soc/dwc/designware_i2s.c | 32 +++++++++++++++++++------------- 1 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c index f8946bd..c497ada 100644 --- a/sound/soc/dwc/designware_i2s.c +++ b/sound/soc/dwc/designware_i2s.c @@ -227,19 +227,25 @@ static int dw_i2s_hw_params(struct snd_pcm_substream *substream,
i2s_disable_channels(dev, substream->stream);
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { - i2s_write_reg(dev->i2s_base, TCR(ch_reg), xfer_resolution); - i2s_write_reg(dev->i2s_base, TFCR(ch_reg), 0x02); - irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg)); - i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x30); - i2s_write_reg(dev->i2s_base, TER(ch_reg), 1); - } else { - i2s_write_reg(dev->i2s_base, RCR(ch_reg), xfer_resolution); - i2s_write_reg(dev->i2s_base, RFCR(ch_reg), 0x07); - irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg)); - i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x03); - i2s_write_reg(dev->i2s_base, RER(ch_reg), 1); - } + /* Iterate over set of channels - independently controlled. + */ + do { + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { + i2s_write_reg(dev->i2s_base, TCR(ch_reg), + xfer_resolution); + i2s_write_reg(dev->i2s_base, TFCR(ch_reg), 0x02); + irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg)); + i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x30); + i2s_write_reg(dev->i2s_base, TER(ch_reg), 1); + } else { + i2s_write_reg(dev->i2s_base, RCR(ch_reg), + xfer_resolution); + i2s_write_reg(dev->i2s_base, RFCR(ch_reg), 0x07); + irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg)); + i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x03); + i2s_write_reg(dev->i2s_base, RER(ch_reg), 1); + } + } while (ch_reg-- > 0);
i2s_write_reg(dev->i2s_base, CCR, ccr);
On Wed, Dec 03, 2014 at 04:39:01PM +0000, Andrew Jackson wrote:
- /* Iterate over set of channels - independently controlled.
*/- do {
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {i2s_write_reg(dev->i2s_base, TCR(ch_reg),xfer_resolution);i2s_write_reg(dev->i2s_base, TFCR(ch_reg), 0x02);irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x30);i2s_write_reg(dev->i2s_base, TER(ch_reg), 1);} else {i2s_write_reg(dev->i2s_base, RCR(ch_reg),xfer_resolution);i2s_write_reg(dev->i2s_base, RFCR(ch_reg), 0x07);irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x03);i2s_write_reg(dev->i2s_base, RER(ch_reg), 1);}- } while (ch_reg-- > 0);
The normal way to write an iteration would be with a for loop - why are we not doing that?
Also I see that you've not sent these as a single thread - please use --thread.
On Wed, Dec 3, 2014 at 10:59 PM, Mark Brown broonie@kernel.org wrote:
On Wed, Dec 03, 2014 at 04:39:01PM +0000, Andrew Jackson wrote:
/* Iterate over set of channels - independently controlled.*/do {if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {i2s_write_reg(dev->i2s_base, TCR(ch_reg),xfer_resolution);i2s_write_reg(dev->i2s_base, TFCR(ch_reg), 0x02);irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x30);i2s_write_reg(dev->i2s_base, TER(ch_reg), 1);} else {i2s_write_reg(dev->i2s_base, RCR(ch_reg),xfer_resolution);i2s_write_reg(dev->i2s_base, RFCR(ch_reg), 0x07);irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x03);i2s_write_reg(dev->i2s_base, RER(ch_reg), 1);}} while (ch_reg-- > 0);The normal way to write an iteration would be with a for loop - why are we not doing that?
Also I see that you've not sent these as a single thread - please use --thread.
designware i2s has individual register for channel support. It is like TCR(0), TCR(1) and so on.. So a macro is defined to capture these please check below #define
#define TCR(x) (0x40 * x + 0x034)
and the same is true for capture also. So there is no need for iteration. Only writing to the particular register will do the work.
~Rajeev
On 12/04/14 06:55, rajeev kumar wrote:
On Wed, Dec 3, 2014 at 10:59 PM, Mark Brown broonie@kernel.org wrote:
On Wed, Dec 03, 2014 at 04:39:01PM +0000, Andrew Jackson wrote:
/* Iterate over set of channels - independently controlled.*/do {if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {i2s_write_reg(dev->i2s_base, TCR(ch_reg),xfer_resolution);i2s_write_reg(dev->i2s_base, TFCR(ch_reg), 0x02);irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x30);i2s_write_reg(dev->i2s_base, TER(ch_reg), 1);} else {i2s_write_reg(dev->i2s_base, RCR(ch_reg),xfer_resolution);i2s_write_reg(dev->i2s_base, RFCR(ch_reg), 0x07);irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x03);i2s_write_reg(dev->i2s_base, RER(ch_reg), 1);}} while (ch_reg-- > 0);The normal way to write an iteration would be with a for loop - why are we not doing that?
Also I see that you've not sent these as a single thread - please use --thread.
designware i2s has individual register for channel support. It is like TCR(0), TCR(1) and so on.. So a macro is defined to capture these please check below #define
#define TCR(x) (0x40 * x + 0x034)
and the same is true for capture also. So there is no need for iteration. Only writing to the particular register will do the work.
However params_channels returns the number of channels that the device needs to configure. So, for example, when there are four channels two sets of channel registers need to be programmed.
Andrew
~Rajeev
On 12/03/14 17:29, Mark Brown wrote:
On Wed, Dec 03, 2014 at 04:39:01PM +0000, Andrew Jackson wrote:
- /* Iterate over set of channels - independently controlled.
*/- do {
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {i2s_write_reg(dev->i2s_base, TCR(ch_reg),xfer_resolution);i2s_write_reg(dev->i2s_base, TFCR(ch_reg), 0x02);irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x30);i2s_write_reg(dev->i2s_base, TER(ch_reg), 1);} else {i2s_write_reg(dev->i2s_base, RCR(ch_reg),xfer_resolution);i2s_write_reg(dev->i2s_base, RFCR(ch_reg), 0x07);irq = i2s_read_reg(dev->i2s_base, IMR(ch_reg));i2s_write_reg(dev->i2s_base, IMR(ch_reg), irq & ~0x03);i2s_write_reg(dev->i2s_base, RER(ch_reg), 1);}- } while (ch_reg-- > 0);
The normal way to write an iteration would be with a for loop - why are we not doing that?
The intention was to minimise the changes, excluding whitespace, between this version and the original. Also, it is a perfectly valid looping construct. I'm happy to rework it into a for loop.
Also I see that you've not sent these as a single thread - please use --thread.
Andrew
participants (3)
-
Andrew Jackson -
Mark Brown -
rajeev kumar