* [PATCH 3/5] ASoC: dwc: Iterate over all channels
@ 2014-12-03 16:39 ` Andrew Jackson
0 siblings, 0 replies; 12+ messages in thread
From: Andrew Jackson @ 2014-12-03 16:39 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai
Cc: alsa-devel@alsa-project.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, Rajeev Kumar, Liam Girdwood,
Mark Brown, Liviu Dudau
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);
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/5] ASoC: dwc: Iterate over all channels
@ 2014-12-03 16:39 ` Andrew Jackson
0 siblings, 0 replies; 12+ messages in thread
From: Andrew Jackson @ 2014-12-03 16:39 UTC (permalink / raw)
To: linux-arm-kernel
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);
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 3/5] ASoC: dwc: Iterate over all channels
2014-12-03 16:39 ` Andrew Jackson
@ 2014-12-03 17:29 ` Mark Brown
-1 siblings, 0 replies; 12+ messages in thread
From: Mark Brown @ 2014-12-03 17:29 UTC (permalink / raw)
To: Andrew Jackson
Cc: Jaroslav Kysela, Takashi Iwai, alsa-devel@alsa-project.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, Rajeev Kumar, Liam Girdwood,
Liviu Dudau
[-- Attachment #1: Type: text/plain, Size: 1033 bytes --]
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.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 3/5] ASoC: dwc: Iterate over all channels
@ 2014-12-03 17:29 ` Mark Brown
0 siblings, 0 replies; 12+ messages in thread
From: Mark Brown @ 2014-12-03 17:29 UTC (permalink / raw)
To: linux-arm-kernel
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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141203/4c734ecb/attachment.sig>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 3/5] ASoC: dwc: Iterate over all channels
2014-12-03 17:29 ` Mark Brown
@ 2014-12-04 6:55 ` rajeev kumar
-1 siblings, 0 replies; 12+ messages in thread
From: rajeev kumar @ 2014-12-04 6:55 UTC (permalink / raw)
To: Mark Brown
Cc: Andrew Jackson, Jaroslav Kysela, Takashi Iwai,
alsa-devel@alsa-project.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, Liam Girdwood, Liviu Dudau
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
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 3/5] ASoC: dwc: Iterate over all channels
@ 2014-12-04 6:55 ` rajeev kumar
0 siblings, 0 replies; 12+ messages in thread
From: rajeev kumar @ 2014-12-04 6:55 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 3/5] ASoC: dwc: Iterate over all channels
2014-12-04 6:55 ` rajeev kumar
(?)
@ 2014-12-04 9:13 ` Andrew Jackson
-1 siblings, 0 replies; 12+ messages in thread
From: Andrew Jackson @ 2014-12-04 9:13 UTC (permalink / raw)
To: rajeev kumar, Mark Brown
Cc: alsa-devel@alsa-project.org, Takashi Iwai, Liviu Dudau,
Liam Girdwood, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
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
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 3/5] ASoC: dwc: Iterate over all channels
@ 2014-12-04 9:13 ` Andrew Jackson
0 siblings, 0 replies; 12+ messages in thread
From: Andrew Jackson @ 2014-12-04 9:13 UTC (permalink / raw)
To: rajeev kumar, Mark Brown
Cc: Jaroslav Kysela, Takashi Iwai, alsa-devel@alsa-project.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, Liam Girdwood, Liviu Dudau
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
>
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 3/5] ASoC: dwc: Iterate over all channels
@ 2014-12-04 9:13 ` Andrew Jackson
0 siblings, 0 replies; 12+ messages in thread
From: Andrew Jackson @ 2014-12-04 9:13 UTC (permalink / raw)
To: linux-arm-kernel
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
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/5] ASoC: dwc: Iterate over all channels
2014-12-03 17:29 ` Mark Brown
(?)
@ 2014-12-04 9:03 ` Andrew Jackson
-1 siblings, 0 replies; 12+ messages in thread
From: Andrew Jackson @ 2014-12-04 9:03 UTC (permalink / raw)
To: Mark Brown
Cc: alsa-devel@alsa-project.org, Liam Girdwood, Takashi Iwai,
Liviu Dudau, linux-kernel@vger.kernel.org, Rajeev Kumar,
linux-arm-kernel@lists.infradead.org
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
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 3/5] ASoC: dwc: Iterate over all channels
@ 2014-12-04 9:03 ` Andrew Jackson
0 siblings, 0 replies; 12+ messages in thread
From: Andrew Jackson @ 2014-12-04 9:03 UTC (permalink / raw)
To: Mark Brown
Cc: Jaroslav Kysela, Takashi Iwai, alsa-devel@alsa-project.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, Rajeev Kumar, Liam Girdwood,
Liviu Dudau
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
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 3/5] ASoC: dwc: Iterate over all channels
@ 2014-12-04 9:03 ` Andrew Jackson
0 siblings, 0 replies; 12+ messages in thread
From: Andrew Jackson @ 2014-12-04 9:03 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2014-12-04 9:13 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-03 16:39 [PATCH 3/5] ASoC: dwc: Iterate over all channels Andrew Jackson
2014-12-03 16:39 ` Andrew Jackson
2014-12-03 17:29 ` Mark Brown
2014-12-03 17:29 ` Mark Brown
2014-12-04 6:55 ` rajeev kumar
2014-12-04 6:55 ` rajeev kumar
2014-12-04 9:13 ` Andrew Jackson
2014-12-04 9:13 ` Andrew Jackson
2014-12-04 9:13 ` Andrew Jackson
2014-12-04 9:03 ` Andrew Jackson
2014-12-04 9:03 ` Andrew Jackson
2014-12-04 9:03 ` Andrew Jackson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.