* [PATCH 3/5] ASoC: dwc: Iterate over all channels
@ 2014-12-03 16:39 Andrew Jackson
2014-12-03 17:29 ` Mark Brown
0 siblings, 1 reply; 5+ 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] 5+ messages in thread* [PATCH 3/5] ASoC: dwc: Iterate over all channels
2014-12-03 16:39 [PATCH 3/5] ASoC: dwc: Iterate over all channels Andrew Jackson
@ 2014-12-03 17:29 ` Mark Brown
2014-12-04 6:55 ` rajeev kumar
2014-12-04 9:03 ` Andrew Jackson
0 siblings, 2 replies; 5+ 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] 5+ messages in thread* [PATCH 3/5] ASoC: dwc: Iterate over all channels
2014-12-03 17:29 ` Mark Brown
@ 2014-12-04 6:55 ` rajeev kumar
2014-12-04 9:13 ` Andrew Jackson
2014-12-04 9:03 ` Andrew Jackson
1 sibling, 1 reply; 5+ 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] 5+ messages in thread* [PATCH 3/5] ASoC: dwc: Iterate over all channels
2014-12-04 6:55 ` rajeev kumar
@ 2014-12-04 9:13 ` Andrew Jackson
0 siblings, 0 replies; 5+ 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] 5+ messages in thread
* [PATCH 3/5] ASoC: dwc: Iterate over all channels
2014-12-03 17:29 ` Mark Brown
2014-12-04 6:55 ` rajeev kumar
@ 2014-12-04 9:03 ` Andrew Jackson
1 sibling, 0 replies; 5+ 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] 5+ messages in thread
end of thread, other threads:[~2014-12-04 9:13 UTC | newest]
Thread overview: 5+ 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 17:29 ` Mark Brown
2014-12-04 6:55 ` rajeev kumar
2014-12-04 9:13 ` Andrew Jackson
2014-12-04 9:03 ` Andrew Jackson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox