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(-)
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);
--
1.7.1
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 <[email protected]> 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/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
On 12/04/14 06:55, rajeev kumar wrote:
> On Wed, Dec 3, 2014 at 10:59 PM, Mark Brown <[email protected]> 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
>