ASoC: dwc: Iterate over all channels

PATCH Superseded Andrew Jackson 3 participants 5 messages Dec 3, 2014 – Dec 4, 2014

Subsystems (2)

Sound ASoC

lore

Share & tools

lore

Andrew Jackson

author

2014-12-03 16:39 UTC #0 lore raw

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 <[email protected]>

---
 sound/soc/dwc/designware_i2s.c |   32 +++++++++++++++++++-------------
 1 files changed, 19 insertions(+), 13 deletions(-)

sound/soc/dwc/designware_i2s.c +19 ~~-13~~

index f8946bd..c497ada 100644
@@ -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);

Signature · 1 line

1.7.1

Mark Brown

maintainer

2014-12-03 17:29 UTC #1 lore raw

Quoted 9 lines · Andrew Jackson

sound/soc/dwc/designware_i2s.c +9 -0

+	/* 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);

sound/soc/dwc/designware_i2s.c static int dw_i2s_hw_params(struct snd_pcm_substream *substream, · line 248

Quoted 10 lines

sound/soc/dwc/designware_i2s.c +10 -0

+			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.

rajeev kumar

2014-12-04 06:55 UTC #2 lore raw

Quoted 22 lines · Mark Brown

Quoted 9 lines · Andrew Jackson

sound/soc/dwc/designware_i2s.c +9 -0

+     /* 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);

sound/soc/dwc/designware_i2s.c static int dw_i2s_hw_params(struct snd_pcm_substream *substream, · line 248

Quoted 10 lines

sound/soc/dwc/designware_i2s.c +10 -0

+                     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);

Quoted 5 lines

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

Andrew Jackson

author

2014-12-04 09:13 UTC #3 lore raw

Quoted 29 lines · rajeev kumar

Quoted 22 lines · Mark Brown

Quoted 9 lines · Andrew Jackson

sound/soc/dwc/designware_i2s.c +9 -0

+     /* 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);

sound/soc/dwc/designware_i2s.c static int dw_i2s_hw_params(struct snd_pcm_substream *substream, · line 248

Quoted 10 lines

sound/soc/dwc/designware_i2s.c +10 -0

+                     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);

Quoted 5 lines

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.

Quoted 8 lines

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

Quoted 3 lines

~Rajeev

Andrew Jackson

author

2014-12-04 09:03 UTC #4 lore raw

In reply to Mark Brown

Quoted 16 lines · Mark Brown

Quoted 14 lines · Andrew Jackson

sound/soc/dwc/designware_i2s.c +14 -0

+	/* 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);

Quoted 8 lines

sound/soc/dwc/designware_i2s.c static int dw_i2s_hw_params(struct snd_pcm_substream *substream, · line 248
Quoted 5 lines
sound/soc/dwc/designware_i2s.c +5 -0
+			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.

Quoted 3 lines

Also I see that you've not sent these as a single thread - please use --thread.

Andrew