[PATCH] ASoC: bcm2835: Increase channels_max to 8

[PATCH] ASoC: bcm2835: Increase channels_max to 8

[Matt Flax]

The AudioInjector Octo sound card operates with 8 channels,
however the bcm2835 I2S driver was limited to 2 channels.

This patch increases the channel capability to 8 by setting
channels_max to 8 in the bcm2835_i2s_dai structure. It also
adds the 8 channel condition to the channel guard in the
bcm2835_i2s_hw_params function.

Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
 sound/soc/bcm/bcm2835-i2s.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index 6ba2049..c65ce28 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -312,6 +312,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (params_channels(params)) {
 	case 2:
+	case 8:
 		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
 		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
 		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
@@ -577,7 +578,7 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
 	.probe	= bcm2835_i2s_dai_probe,
 	.playback = {
 		.channels_min = 2,
-		.channels_max = 2,
+		.channels_max = 8,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
@@ -585,7 +586,7 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
 		},
 	.capture = {
 		.channels_min = 2,
-		.channels_max = 2,
+		.channels_max = 8,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
-- 
2.7.4

[PATCH] ASoC: bcm2835: Increase channels_max to 8

[Matt Flax]

The AudioInjector Octo sound card operates with 8 channels,
however the bcm2835 I2S driver was limited to 2 channels.

This patch increases the channel capability to 8 by setting
channels_max to 8 in the bcm2835_i2s_dai structure. It also
adds the 8 channel condition to the channel guard in the
bcm2835_i2s_hw_params function.

Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
 sound/soc/bcm/bcm2835-i2s.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index 6ba2049..c65ce28 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -312,6 +312,7 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	switch (params_channels(params)) {
 	case 2:
+	case 8:
 		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
 		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
 		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
@@ -577,7 +578,7 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
 	.probe	= bcm2835_i2s_dai_probe,
 	.playback = {
 		.channels_min = 2,
-		.channels_max = 2,
+		.channels_max = 8,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
@@ -585,7 +586,7 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
 		},
 	.capture = {
 		.channels_min = 2,
-		.channels_max = 2,
+		.channels_max = 8,
 		.rates =	SNDRV_PCM_RATE_8000_192000,
 		.formats =	SNDRV_PCM_FMTBIT_S16_LE
 				| SNDRV_PCM_FMTBIT_S24_LE
-- 
2.7.4

Re: [PATCH] ASoC: bcm2835: Increase channels_max to 8

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 604 bytes --]

On Thu, Feb 02, 2017 at 10:37:44AM +1100, Matt Flax wrote:

>  	switch (params_channels(params)) {
>  	case 2:
> +	case 8:
>  		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
>  		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
>  		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));

I can't understand how this works.  We're programming the hardware in an
identical fashion for both channel counts, that means that what we're
sending will be indistinguishable from a garbled stereo stream.  How
does this produce working clocks or manage to sync up reliably with
external clocks?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH] ASoC: bcm2835: Increase channels_max to 8

[Mark Brown]

On Thu, Feb 02, 2017 at 10:37:44AM +1100, Matt Flax wrote:

>  	switch (params_channels(params)) {
>  	case 2:
> +	case 8:
>  		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
>  		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
>  		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));

I can't understand how this works.  We're programming the hardware in an
identical fashion for both channel counts, that means that what we're
sending will be indistinguishable from a garbled stereo stream.  How
does this produce working clocks or manage to sync up reliably with
external clocks?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170204/1e3227e3/attachment-0001.sig>

Re: [PATCH] ASoC: bcm2835: Increase channels_max to 8

[Matt Flax]

On 05/02/17 02:49, Mark Brown wrote:
> On Thu, Feb 02, 2017 at 10:37:44AM +1100, Matt Flax wrote:
>
>>   	switch (params_channels(params)) {
>>   	case 2:
>> +	case 8:
>>   		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
>>   		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
>>   		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
> I can't understand how this works.  We're programming the hardware in an
> identical fashion for both channel counts, that means that what we're
> sending will be indistinguishable from a garbled stereo stream.  How
> does this produce working clocks or manage to sync up reliably with
> external clocks?
>
I am not surprised it is confusing, to my knowledge this is the first 
time that someone has demonstrated this type of I2S signalling.

I will attempt to explain why nothing is garbled by talking about what 
the BCM2835 silicon does.

In this case BCM2835 is a clock slave and not concerned with generating 
the I2S clocks, only receiving - think of all clocks and synchrony being 
generated/managed in hardware on the sound card.

In short the BCM2835 silicon is only concerned with shifting bits into 
two hardware registers which (after DMA) are interpreted by ALSA 
according to the specified channel count.

Let me be a little more explicit. Regarding the BCM2835 I2S silicon :
The BCM2835 I2S silicon is concerned with shifting data from I2S bit 
streams into hardware registers. The BCM2835 is only concerned with the 
word format for the hardware registers. The BCM2835 silicon doesn't care 
about channel count. This is why we use format=* in this part of the  
bcm2835-i2s.c driver. Specifically we are concerned with the protocol's 
word format (e.g. BCM2835_I2S_CH1(format)) and the location of the word 
onset in the bit stream (e.g. BCM2835_I2S_CHPOS(ch1pos)).

This is (to my knowledge) the first time someone has tried this concept 
and proven that it is successful and robust. My gut feeling is that it 
will work will most/all silicon. In my opinion it is a little 
evolution/revolution in I2S signalling.

Personally, I think it would be better if the code read as follows :

    // if the channel count is even
    if ( fmod((float)params_channels(params)) != 1 ) {
           format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
           format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
           format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
   } else
         return -EINVAL;

I would even prefer to get rid of the channel count guard as strictly 
speaking the format of the I2S bit stream has nothing to do with the 
channel count !

thanks
Matt

[alsa-devel] [PATCH] ASoC: bcm2835: Increase channels_max to 8

[Matt Flax]

On 05/02/17 02:49, Mark Brown wrote:
> On Thu, Feb 02, 2017 at 10:37:44AM +1100, Matt Flax wrote:
>
>>   	switch (params_channels(params)) {
>>   	case 2:
>> +	case 8:
>>   		format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
>>   		format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
>>   		format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
> I can't understand how this works.  We're programming the hardware in an
> identical fashion for both channel counts, that means that what we're
> sending will be indistinguishable from a garbled stereo stream.  How
> does this produce working clocks or manage to sync up reliably with
> external clocks?
>
I am not surprised it is confusing, to my knowledge this is the first 
time that someone has demonstrated this type of I2S signalling.

I will attempt to explain why nothing is garbled by talking about what 
the BCM2835 silicon does.

In this case BCM2835 is a clock slave and not concerned with generating 
the I2S clocks, only receiving - think of all clocks and synchrony being 
generated/managed in hardware on the sound card.

In short the BCM2835 silicon is only concerned with shifting bits into 
two hardware registers which (after DMA) are interpreted by ALSA 
according to the specified channel count.

Let me be a little more explicit. Regarding the BCM2835 I2S silicon :
The BCM2835 I2S silicon is concerned with shifting data from I2S bit 
streams into hardware registers. The BCM2835 is only concerned with the 
word format for the hardware registers. The BCM2835 silicon doesn't care 
about channel count. This is why we use format=* in this part of the  
bcm2835-i2s.c driver. Specifically we are concerned with the protocol's 
word format (e.g. BCM2835_I2S_CH1(format)) and the location of the word 
onset in the bit stream (e.g. BCM2835_I2S_CHPOS(ch1pos)).

This is (to my knowledge) the first time someone has tried this concept 
and proven that it is successful and robust. My gut feeling is that it 
will work will most/all silicon. In my opinion it is a little 
evolution/revolution in I2S signalling.

Personally, I think it would be better if the code read as follows :

    // if the channel count is even
    if ( fmod((float)params_channels(params)) != 1 ) {
           format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
           format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
           format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
   } else
         return -EINVAL;

I would even prefer to get rid of the channel count guard as strictly 
speaking the format of the I2S bit stream has nothing to do with the 
channel count !

thanks
Matt

Re: [alsa-devel] [PATCH] ASoC: bcm2835: Increase channels_max to 8

[Florian Kauer]

Hi Matt,
great to hear from your work!
However, I have to disappoint you, it is not the first time someone has 
done this. Actually, I have done the same thing in 2014 and mentioned it 
at my talk at the Linux Audio Conference. However, I have not upstreamed 
it because I thought it is too special ;-)
http://lac.linuxaudio.org/2014/video.php?id=4 at 16:28

The problem is that it highly depends on the codec you connect if the 
timing matches. Actually the official I2S bus specification does only 
support two channels. Everything else is some special thing anyway.

The question is: Should the driver be open to accept "special things" 
even if the hardware does not "officially" support it?
Upside: It enables more applications.
Downside: If you select an arbitrary 8 channel codec and this driver you 
do not get any error message, but it might or might not work.

I would tend to accepting the patch because everyone who writes a board 
driver should check the compatibility anyway and otherwise we would 
block a working application.

Greetings,
Florian

On 05.02.2017 02:26, Matt Flax wrote:
>
>
> On 05/02/17 02:49, Mark Brown wrote:
>> On Thu, Feb 02, 2017 at 10:37:44AM +1100, Matt Flax wrote:
>>
>>>       switch (params_channels(params)) {
>>>       case 2:
>>> +    case 8:
>>>           format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
>>>           format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
>>>           format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
>> I can't understand how this works.  We're programming the hardware in an
>> identical fashion for both channel counts, that means that what we're
>> sending will be indistinguishable from a garbled stereo stream.  How
>> does this produce working clocks or manage to sync up reliably with
>> external clocks?
>>
> I am not surprised it is confusing, to my knowledge this is the first
> time that someone has demonstrated this type of I2S signalling.
>
> I will attempt to explain why nothing is garbled by talking about what
> the BCM2835 silicon does.
>
> In this case BCM2835 is a clock slave and not concerned with generating
> the I2S clocks, only receiving - think of all clocks and synchrony being
> generated/managed in hardware on the sound card.
>
> In short the BCM2835 silicon is only concerned with shifting bits into
> two hardware registers which (after DMA) are interpreted by ALSA
> according to the specified channel count.
>
> Let me be a little more explicit. Regarding the BCM2835 I2S silicon :
> The BCM2835 I2S silicon is concerned with shifting data from I2S bit
> streams into hardware registers. The BCM2835 is only concerned with the
> word format for the hardware registers. The BCM2835 silicon doesn't care
> about channel count. This is why we use format=* in this part of the
> bcm2835-i2s.c driver. Specifically we are concerned with the protocol's
> word format (e.g. BCM2835_I2S_CH1(format)) and the location of the word
> onset in the bit stream (e.g. BCM2835_I2S_CHPOS(ch1pos)).
>
> This is (to my knowledge) the first time someone has tried this concept
> and proven that it is successful and robust. My gut feeling is that it
> will work will most/all silicon. In my opinion it is a little
> evolution/revolution in I2S signalling.
>
> Personally, I think it would be better if the code read as follows :
>
>    // if the channel count is even
>    if ( fmod((float)params_channels(params)) != 1 ) {
>           format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
>           format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
>           format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
>   } else
>         return -EINVAL;
>
> I would even prefer to get rid of the channel count guard as strictly
> speaking the format of the I2S bit stream has nothing to do with the
> channel count !
>
> thanks
> Matt

[alsa-devel] [PATCH] ASoC: bcm2835: Increase channels_max to 8

[Florian Kauer]

Hi Matt,
great to hear from your work!
However, I have to disappoint you, it is not the first time someone has 
done this. Actually, I have done the same thing in 2014 and mentioned it 
at my talk at the Linux Audio Conference. However, I have not upstreamed 
it because I thought it is too special ;-)
http://lac.linuxaudio.org/2014/video.php?id=4 at 16:28

The problem is that it highly depends on the codec you connect if the 
timing matches. Actually the official I2S bus specification does only 
support two channels. Everything else is some special thing anyway.

The question is: Should the driver be open to accept "special things" 
even if the hardware does not "officially" support it?
Upside: It enables more applications.
Downside: If you select an arbitrary 8 channel codec and this driver you 
do not get any error message, but it might or might not work.

I would tend to accepting the patch because everyone who writes a board 
driver should check the compatibility anyway and otherwise we would 
block a working application.

Greetings,
Florian

On 05.02.2017 02:26, Matt Flax wrote:
>
>
> On 05/02/17 02:49, Mark Brown wrote:
>> On Thu, Feb 02, 2017 at 10:37:44AM +1100, Matt Flax wrote:
>>
>>>       switch (params_channels(params)) {
>>>       case 2:
>>> +    case 8:
>>>           format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
>>>           format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
>>>           format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
>> I can't understand how this works.  We're programming the hardware in an
>> identical fashion for both channel counts, that means that what we're
>> sending will be indistinguishable from a garbled stereo stream.  How
>> does this produce working clocks or manage to sync up reliably with
>> external clocks?
>>
> I am not surprised it is confusing, to my knowledge this is the first
> time that someone has demonstrated this type of I2S signalling.
>
> I will attempt to explain why nothing is garbled by talking about what
> the BCM2835 silicon does.
>
> In this case BCM2835 is a clock slave and not concerned with generating
> the I2S clocks, only receiving - think of all clocks and synchrony being
> generated/managed in hardware on the sound card.
>
> In short the BCM2835 silicon is only concerned with shifting bits into
> two hardware registers which (after DMA) are interpreted by ALSA
> according to the specified channel count.
>
> Let me be a little more explicit. Regarding the BCM2835 I2S silicon :
> The BCM2835 I2S silicon is concerned with shifting data from I2S bit
> streams into hardware registers. The BCM2835 is only concerned with the
> word format for the hardware registers. The BCM2835 silicon doesn't care
> about channel count. This is why we use format=* in this part of the
> bcm2835-i2s.c driver. Specifically we are concerned with the protocol's
> word format (e.g. BCM2835_I2S_CH1(format)) and the location of the word
> onset in the bit stream (e.g. BCM2835_I2S_CHPOS(ch1pos)).
>
> This is (to my knowledge) the first time someone has tried this concept
> and proven that it is successful and robust. My gut feeling is that it
> will work will most/all silicon. In my opinion it is a little
> evolution/revolution in I2S signalling.
>
> Personally, I think it would be better if the code read as follows :
>
>    // if the channel count is even
>    if ( fmod((float)params_channels(params)) != 1 ) {
>           format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
>           format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
>           format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
>   } else
>         return -EINVAL;
>
> I would even prefer to get rid of the channel count guard as strictly
> speaking the format of the I2S bit stream has nothing to do with the
> channel count !
>
> thanks
> Matt

Re: [PATCH] ASoC: bcm2835: Increase channels_max to 8

[Matt Flax]

On 05/02/17 21:20, Florian Kauer wrote:
> Hi Matt,
> great to hear from your work!
> However, I have to disappoint you, it is not the first time someone 
> has done this. Actually, I have done the same thing in 2014 and 
> mentioned it at my talk at the Linux Audio Conference. However, I have 
> not upstreamed it because I thought it is too special ;-)
> http://lac.linuxaudio.org/2014/video.php?id=4 at 16:28
>
Florian, it is an honour to be following in your footsteps ! Nice video 
- nice specs/response on the Jamberry too.

> The problem is that it highly depends on the codec you connect if the 
> timing matches. Actually the official I2S bus specification does only 
> support two channels. Everything else is some special thing anyway.
>
> The question is: Should the driver be open to accept "special things" 
> even if the hardware does not "officially" support it?
> Upside: It enables more applications.
> Downside: If you select an arbitrary 8 channel codec and this driver 
> you do not get any error message, but it might or might not work.
>
> I would tend to accepting the patch because everyone who writes a 
> board driver should check the compatibility anyway and otherwise we 
> would block a working application.
>
Thank you for voting in support of the patch ! It is a pretty safe patch 
because it does very little to change the functionality of the i2s 
driver you originally wrote.

I agree with your statement, "everyone who writes a board driver should 
check the compatibility" in fact I would go further. I would say the 
designers of the audio hardware should ensure that their solution is 
reliable and works with the particular SoC they are targeting (in this 
case the BCM2835).
The Audio Injector Octo (http://audioinjector.net) is certainly well 
tested with the BCM2835 silicon (both Pi2 and Pi3 versions).

thanks
Matt

> Greetings,
> Florian
>
> On 05.02.2017 02:26, Matt Flax wrote:
>>
>>
>> On 05/02/17 02:49, Mark Brown wrote:
>>> On Thu, Feb 02, 2017 at 10:37:44AM +1100, Matt Flax wrote:
>>>
>>>>       switch (params_channels(params)) {
>>>>       case 2:
>>>> +    case 8:
>>>>           format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
>>>>           format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
>>>>           format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
>>> I can't understand how this works.  We're programming the hardware 
>>> in an
>>> identical fashion for both channel counts, that means that what we're
>>> sending will be indistinguishable from a garbled stereo stream.  How
>>> does this produce working clocks or manage to sync up reliably with
>>> external clocks?
>>>
>> I am not surprised it is confusing, to my knowledge this is the first
>> time that someone has demonstrated this type of I2S signalling.
>>
>> I will attempt to explain why nothing is garbled by talking about what
>> the BCM2835 silicon does.
>>
>> In this case BCM2835 is a clock slave and not concerned with generating
>> the I2S clocks, only receiving - think of all clocks and synchrony being
>> generated/managed in hardware on the sound card.
>>
>> In short the BCM2835 silicon is only concerned with shifting bits into
>> two hardware registers which (after DMA) are interpreted by ALSA
>> according to the specified channel count.
>>
>> Let me be a little more explicit. Regarding the BCM2835 I2S silicon :
>> The BCM2835 I2S silicon is concerned with shifting data from I2S bit
>> streams into hardware registers. The BCM2835 is only concerned with the
>> word format for the hardware registers. The BCM2835 silicon doesn't care
>> about channel count. This is why we use format=* in this part of the
>> bcm2835-i2s.c driver. Specifically we are concerned with the protocol's
>> word format (e.g. BCM2835_I2S_CH1(format)) and the location of the word
>> onset in the bit stream (e.g. BCM2835_I2S_CHPOS(ch1pos)).
>>
>> This is (to my knowledge) the first time someone has tried this concept
>> and proven that it is successful and robust. My gut feeling is that it
>> will work will most/all silicon. In my opinion it is a little
>> evolution/revolution in I2S signalling.
>>
>> Personally, I think it would be better if the code read as follows :
>>
>>    // if the channel count is even
>>    if ( fmod((float)params_channels(params)) != 1 ) {
>>           format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
>>           format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
>>           format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
>>   } else
>>         return -EINVAL;
>>
>> I would even prefer to get rid of the channel count guard as strictly
>> speaking the format of the I2S bit stream has nothing to do with the
>> channel count !
>>
>> thanks
>> Matt

[alsa-devel] [PATCH] ASoC: bcm2835: Increase channels_max to 8

[Matt Flax]

On 05/02/17 21:20, Florian Kauer wrote:
> Hi Matt,
> great to hear from your work!
> However, I have to disappoint you, it is not the first time someone 
> has done this. Actually, I have done the same thing in 2014 and 
> mentioned it at my talk at the Linux Audio Conference. However, I have 
> not upstreamed it because I thought it is too special ;-)
> http://lac.linuxaudio.org/2014/video.php?id=4 at 16:28
>
Florian, it is an honour to be following in your footsteps ! Nice video 
- nice specs/response on the Jamberry too.

> The problem is that it highly depends on the codec you connect if the 
> timing matches. Actually the official I2S bus specification does only 
> support two channels. Everything else is some special thing anyway.
>
> The question is: Should the driver be open to accept "special things" 
> even if the hardware does not "officially" support it?
> Upside: It enables more applications.
> Downside: If you select an arbitrary 8 channel codec and this driver 
> you do not get any error message, but it might or might not work.
>
> I would tend to accepting the patch because everyone who writes a 
> board driver should check the compatibility anyway and otherwise we 
> would block a working application.
>
Thank you for voting in support of the patch ! It is a pretty safe patch 
because it does very little to change the functionality of the i2s 
driver you originally wrote.

I agree with your statement, "everyone who writes a board driver should 
check the compatibility" in fact I would go further. I would say the 
designers of the audio hardware should ensure that their solution is 
reliable and works with the particular SoC they are targeting (in this 
case the BCM2835).
The Audio Injector Octo (http://audioinjector.net) is certainly well 
tested with the BCM2835 silicon (both Pi2 and Pi3 versions).

thanks
Matt

> Greetings,
> Florian
>
> On 05.02.2017 02:26, Matt Flax wrote:
>>
>>
>> On 05/02/17 02:49, Mark Brown wrote:
>>> On Thu, Feb 02, 2017 at 10:37:44AM +1100, Matt Flax wrote:
>>>
>>>>       switch (params_channels(params)) {
>>>>       case 2:
>>>> +    case 8:
>>>>           format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
>>>>           format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
>>>>           format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
>>> I can't understand how this works.  We're programming the hardware 
>>> in an
>>> identical fashion for both channel counts, that means that what we're
>>> sending will be indistinguishable from a garbled stereo stream.  How
>>> does this produce working clocks or manage to sync up reliably with
>>> external clocks?
>>>
>> I am not surprised it is confusing, to my knowledge this is the first
>> time that someone has demonstrated this type of I2S signalling.
>>
>> I will attempt to explain why nothing is garbled by talking about what
>> the BCM2835 silicon does.
>>
>> In this case BCM2835 is a clock slave and not concerned with generating
>> the I2S clocks, only receiving - think of all clocks and synchrony being
>> generated/managed in hardware on the sound card.
>>
>> In short the BCM2835 silicon is only concerned with shifting bits into
>> two hardware registers which (after DMA) are interpreted by ALSA
>> according to the specified channel count.
>>
>> Let me be a little more explicit. Regarding the BCM2835 I2S silicon :
>> The BCM2835 I2S silicon is concerned with shifting data from I2S bit
>> streams into hardware registers. The BCM2835 is only concerned with the
>> word format for the hardware registers. The BCM2835 silicon doesn't care
>> about channel count. This is why we use format=* in this part of the
>> bcm2835-i2s.c driver. Specifically we are concerned with the protocol's
>> word format (e.g. BCM2835_I2S_CH1(format)) and the location of the word
>> onset in the bit stream (e.g. BCM2835_I2S_CHPOS(ch1pos)).
>>
>> This is (to my knowledge) the first time someone has tried this concept
>> and proven that it is successful and robust. My gut feeling is that it
>> will work will most/all silicon. In my opinion it is a little
>> evolution/revolution in I2S signalling.
>>
>> Personally, I think it would be better if the code read as follows :
>>
>>    // if the channel count is even
>>    if ( fmod((float)params_channels(params)) != 1 ) {
>>           format = BCM2835_I2S_CH1(format) | BCM2835_I2S_CH2(format);
>>           format |= BCM2835_I2S_CH1(BCM2835_I2S_CHPOS(ch1pos));
>>           format |= BCM2835_I2S_CH2(BCM2835_I2S_CHPOS(ch2pos));
>>   } else
>>         return -EINVAL;
>>
>> I would even prefer to get rid of the channel count guard as strictly
>> speaking the format of the I2S bit stream has nothing to do with the
>> channel count !
>>
>> thanks
>> Matt

Re: [PATCH] ASoC: bcm2835: Increase channels_max to 8

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 2042 bytes --]

On Sun, Feb 05, 2017 at 12:26:31PM +1100, Matt Flax wrote:
> On 05/02/17 02:49, Mark Brown wrote:
> > On Thu, Feb 02, 2017 at 10:37:44AM +1100, Matt Flax wrote:

> > I can't understand how this works.  We're programming the hardware in an
> > identical fashion for both channel counts, that means that what we're

> I am not surprised it is confusing, to my knowledge this is the first time
> that someone has demonstrated this type of I2S signalling.

More than stereo I2S?  That is actually a thing, while most people use
DSP modes for higher channel counts it's fairly common to have the
support especially in CODECs that also support DSP mode since the
hardware implementation is fairly straightforward if you abstracted in
the bus interface.

> In this case BCM2835 is a clock slave and not concerned with generating the
> I2S clocks, only receiving - think of all clocks and synchrony being
> generated/managed in hardware on the sound card.

> In short the BCM2835 silicon is only concerned with shifting bits into two
> hardware registers which (after DMA) are interpreted by ALSA according to
> the specified channel count.

Sure, that's what all audio controllers do.  But there's a few problems.
One is that nothing in your code that prevents this running in master
mode.  Another is that if this *is* for I2S mode then when doing
multichannel I2S you usually still respect the left/right indication in
LRCLK so you will end up with the data in the in memory format:

  L1 R1 L2 R2 L3 R3 L4 R4

but multi channel I2S would usually be rendered with all the left
channels and all the right channels grouped together so you get:

  L1 L2 L3 L4 R1 R2 R3 R4

It'll also require exact clocking with no spare bits in order to sync
the right channels with the LRCLK switch but that's not so unusual.
Looking at the driver it may be possible for the hardware to do DSP
modes at which point this gets much easier but right now it only has I2S
support, I've got a feeling that the other end of the link may actually
be running in a DSP mode.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[alsa-devel] [PATCH] ASoC: bcm2835: Increase channels_max to 8

[Mark Brown]

On Sun, Feb 05, 2017 at 12:26:31PM +1100, Matt Flax wrote:
> On 05/02/17 02:49, Mark Brown wrote:
> > On Thu, Feb 02, 2017 at 10:37:44AM +1100, Matt Flax wrote:

> > I can't understand how this works.  We're programming the hardware in an
> > identical fashion for both channel counts, that means that what we're

> I am not surprised it is confusing, to my knowledge this is the first time
> that someone has demonstrated this type of I2S signalling.

More than stereo I2S?  That is actually a thing, while most people use
DSP modes for higher channel counts it's fairly common to have the
support especially in CODECs that also support DSP mode since the
hardware implementation is fairly straightforward if you abstracted in
the bus interface.

> In this case BCM2835 is a clock slave and not concerned with generating the
> I2S clocks, only receiving - think of all clocks and synchrony being
> generated/managed in hardware on the sound card.

> In short the BCM2835 silicon is only concerned with shifting bits into two
> hardware registers which (after DMA) are interpreted by ALSA according to
> the specified channel count.

Sure, that's what all audio controllers do.  But there's a few problems.
One is that nothing in your code that prevents this running in master
mode.  Another is that if this *is* for I2S mode then when doing
multichannel I2S you usually still respect the left/right indication in
LRCLK so you will end up with the data in the in memory format:

  L1 R1 L2 R2 L3 R3 L4 R4

but multi channel I2S would usually be rendered with all the left
channels and all the right channels grouped together so you get:

  L1 L2 L3 L4 R1 R2 R3 R4

It'll also require exact clocking with no spare bits in order to sync
the right channels with the LRCLK switch but that's not so unusual.
Looking at the driver it may be possible for the hardware to do DSP
modes at which point this gets much easier but right now it only has I2S
support, I've got a feeling that the other end of the link may actually
be running in a DSP mode.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170205/5fc6f15d/attachment.sig>

Re: [PATCH] ASoC: bcm2835: Increase channels_max to 8

[Matt Flax]

On 06/02/17 03:34, Mark Brown wrote:
> On Sun, Feb 05, 2017 at 12:26:31PM +1100, Matt Flax wrote:
>> On 05/02/17 02:49, Mark Brown wrote:
>>> On Thu, Feb 02, 2017 at 10:37:44AM +1100, Matt Flax wrote:
>>> I can't understand how this works.  We're programming the hardware in an
>>> identical fashion for both channel counts, that means that what we're
>> I am not surprised it is confusing, to my knowledge this is the first time
>> that someone has demonstrated this type of I2S signalling.
> More than stereo I2S?  That is actually a thing, while most people use
> DSP modes for higher channel counts it's fairly common to have the
> support especially in CODECs that also support DSP mode since the
> hardware implementation is fairly straightforward if you abstracted in
> the bus interface.
>
>> In this case BCM2835 is a clock slave and not concerned with generating the
>> I2S clocks, only receiving - think of all clocks and synchrony being
>> generated/managed in hardware on the sound card.
>> In short the BCM2835 silicon is only concerned with shifting bits into two
>> hardware registers which (after DMA) are interpreted by ALSA according to
>> the specified channel count.
> Sure, that's what all audio controllers do.  But there's a few problems.
> One is that nothing in your code that prevents this running in master
> mode.  Another is that if this *is* for I2S mode then when doing
> multichannel I2S you usually still respect the left/right indication in
> LRCLK so you will end up with the data in the in memory format:
>
>    L1 R1 L2 R2 L3 R3 L4 R4
>
> but multi channel I2S would usually be rendered with all the left
> channels and all the right channels grouped together so you get:
>
>    L1 L2 L3 L4 R1 R2 R3 R4
>
> It'll also require exact clocking with no spare bits in order to sync
> the right channels with the LRCLK switch but that's not so unusual.
> Looking at the driver it may be possible for the hardware to do DSP
> modes at which point this gets much easier but right now it only has I2S
> support, I've got a feeling that the other end of the link may actually
> be running in a DSP mode.
You've got it !

This can also work with the BCM2835 in master mode, no reason at this 
point to limit it to slave mode only.

I haven't looked into why, but I am getting the correct channel mapping 
under the hood. To me that is a concern of the higher up levels of the 
ALSA driver - it was a pleasant surprise when it worked nicely, because 
to me it indicated a robustly coded system.

My next patch is to allow DSP modes in this I2S driver. I wanted to make 
sure that this got through first however.

The majority of these other concerns you mention are controlled at the 
machine driver level.

Matt

[alsa-devel] [PATCH] ASoC: bcm2835: Increase channels_max to 8

[Matt Flax]

On 06/02/17 03:34, Mark Brown wrote:
> On Sun, Feb 05, 2017 at 12:26:31PM +1100, Matt Flax wrote:
>> On 05/02/17 02:49, Mark Brown wrote:
>>> On Thu, Feb 02, 2017 at 10:37:44AM +1100, Matt Flax wrote:
>>> I can't understand how this works.  We're programming the hardware in an
>>> identical fashion for both channel counts, that means that what we're
>> I am not surprised it is confusing, to my knowledge this is the first time
>> that someone has demonstrated this type of I2S signalling.
> More than stereo I2S?  That is actually a thing, while most people use
> DSP modes for higher channel counts it's fairly common to have the
> support especially in CODECs that also support DSP mode since the
> hardware implementation is fairly straightforward if you abstracted in
> the bus interface.
>
>> In this case BCM2835 is a clock slave and not concerned with generating the
>> I2S clocks, only receiving - think of all clocks and synchrony being
>> generated/managed in hardware on the sound card.
>> In short the BCM2835 silicon is only concerned with shifting bits into two
>> hardware registers which (after DMA) are interpreted by ALSA according to
>> the specified channel count.
> Sure, that's what all audio controllers do.  But there's a few problems.
> One is that nothing in your code that prevents this running in master
> mode.  Another is that if this *is* for I2S mode then when doing
> multichannel I2S you usually still respect the left/right indication in
> LRCLK so you will end up with the data in the in memory format:
>
>    L1 R1 L2 R2 L3 R3 L4 R4
>
> but multi channel I2S would usually be rendered with all the left
> channels and all the right channels grouped together so you get:
>
>    L1 L2 L3 L4 R1 R2 R3 R4
>
> It'll also require exact clocking with no spare bits in order to sync
> the right channels with the LRCLK switch but that's not so unusual.
> Looking at the driver it may be possible for the hardware to do DSP
> modes at which point this gets much easier but right now it only has I2S
> support, I've got a feeling that the other end of the link may actually
> be running in a DSP mode.
You've got it !

This can also work with the BCM2835 in master mode, no reason at this 
point to limit it to slave mode only.

I haven't looked into why, but I am getting the correct channel mapping 
under the hood. To me that is a concern of the higher up levels of the 
ALSA driver - it was a pleasant surprise when it worked nicely, because 
to me it indicated a robustly coded system.

My next patch is to allow DSP modes in this I2S driver. I wanted to make 
sure that this got through first however.

The majority of these other concerns you mention are controlled at the 
machine driver level.

Matt

Re: [PATCH] ASoC: bcm2835: Increase channels_max to 8

[Matt Flax]

On 06/02/17 07:37, Matt Flax wrote:
> On 06/02/17 03:34, Mark Brown wrote:
>> On Sun, Feb 05, 2017 at 12:26:31PM +1100, Matt Flax wrote:
>>> On 05/02/17 02:49, Mark Brown wrote:
>>>> On Thu, Feb 02, 2017 at 10:37:44AM +1100, Matt Flax wrote:
>>>> I can't understand how this works.  We're programming the hardware 
>>>> in an
>>>> identical fashion for both channel counts, that means that what we're
>>> I am not surprised it is confusing, to my knowledge this is the 
>>> first time
>>> that someone has demonstrated this type of I2S signalling.
>> More than stereo I2S?  That is actually a thing, while most people use
>> DSP modes for higher channel counts it's fairly common to have the
>> support especially in CODECs that also support DSP mode since the
>> hardware implementation is fairly straightforward if you abstracted in
>> the bus interface.
>>
>>> In this case BCM2835 is a clock slave and not concerned with 
>>> generating the
>>> I2S clocks, only receiving - think of all clocks and synchrony being
>>> generated/managed in hardware on the sound card.
>>> In short the BCM2835 silicon is only concerned with shifting bits 
>>> into two
>>> hardware registers which (after DMA) are interpreted by ALSA 
>>> according to
>>> the specified channel count.
>> Sure, that's what all audio controllers do.  But there's a few problems.
>> One is that nothing in your code that prevents this running in master
>> mode.  Another is that if this *is* for I2S mode then when doing
>> multichannel I2S you usually still respect the left/right indication in
>> LRCLK so you will end up with the data in the in memory format:
>>
>>    L1 R1 L2 R2 L3 R3 L4 R4
>>
>> but multi channel I2S would usually be rendered with all the left
>> channels and all the right channels grouped together so you get:
>>
>>    L1 L2 L3 L4 R1 R2 R3 R4
>>
>> It'll also require exact clocking with no spare bits in order to sync
>> the right channels with the LRCLK switch but that's not so unusual.
>> Looking at the driver it may be possible for the hardware to do DSP
>> modes at which point this gets much easier but right now it only has I2S
>> support, I've got a feeling that the other end of the link may actually
>> be running in a DSP mode.
> You've got it !
>
> This can also work with the BCM2835 in master mode, no reason at this 
> point to limit it to slave mode only.
>
> I haven't looked into why, but I am getting the correct channel 
> mapping under the hood. To me that is a concern of the higher up 
> levels of the ALSA driver - it was a pleasant surprise when it worked 
> nicely, because to me it indicated a robustly coded system.
>
> My next patch is to allow DSP modes in this I2S driver. I wanted to 
> make sure that this got through first however.
>
> The majority of these other concerns you mention are controlled at the 
> machine driver level.
>
Mark,

On deeper thought around what you were saying about requiring master 
mode (for the sound card), I think you are right. Whilst it is possible 
for the BCM2835 to be master, it would pose significant problems for the 
sound card to regenerate the correct clocks.

Would you like me to resubmit the patch ensuring master mode for 
channels != 2 and also adding DSP mode capability ?

thanks
Matt
p.s. I didn't realise this was a thing already, are there any machine 
drivers for other SoCs which are currently using this paradigm of 
signalling ?

[alsa-devel] [PATCH] ASoC: bcm2835: Increase channels_max to 8

[Matt Flax]

On 06/02/17 07:37, Matt Flax wrote:
> On 06/02/17 03:34, Mark Brown wrote:
>> On Sun, Feb 05, 2017 at 12:26:31PM +1100, Matt Flax wrote:
>>> On 05/02/17 02:49, Mark Brown wrote:
>>>> On Thu, Feb 02, 2017 at 10:37:44AM +1100, Matt Flax wrote:
>>>> I can't understand how this works.  We're programming the hardware 
>>>> in an
>>>> identical fashion for both channel counts, that means that what we're
>>> I am not surprised it is confusing, to my knowledge this is the 
>>> first time
>>> that someone has demonstrated this type of I2S signalling.
>> More than stereo I2S?  That is actually a thing, while most people use
>> DSP modes for higher channel counts it's fairly common to have the
>> support especially in CODECs that also support DSP mode since the
>> hardware implementation is fairly straightforward if you abstracted in
>> the bus interface.
>>
>>> In this case BCM2835 is a clock slave and not concerned with 
>>> generating the
>>> I2S clocks, only receiving - think of all clocks and synchrony being
>>> generated/managed in hardware on the sound card.
>>> In short the BCM2835 silicon is only concerned with shifting bits 
>>> into two
>>> hardware registers which (after DMA) are interpreted by ALSA 
>>> according to
>>> the specified channel count.
>> Sure, that's what all audio controllers do.  But there's a few problems.
>> One is that nothing in your code that prevents this running in master
>> mode.  Another is that if this *is* for I2S mode then when doing
>> multichannel I2S you usually still respect the left/right indication in
>> LRCLK so you will end up with the data in the in memory format:
>>
>>    L1 R1 L2 R2 L3 R3 L4 R4
>>
>> but multi channel I2S would usually be rendered with all the left
>> channels and all the right channels grouped together so you get:
>>
>>    L1 L2 L3 L4 R1 R2 R3 R4
>>
>> It'll also require exact clocking with no spare bits in order to sync
>> the right channels with the LRCLK switch but that's not so unusual.
>> Looking at the driver it may be possible for the hardware to do DSP
>> modes at which point this gets much easier but right now it only has I2S
>> support, I've got a feeling that the other end of the link may actually
>> be running in a DSP mode.
> You've got it !
>
> This can also work with the BCM2835 in master mode, no reason at this 
> point to limit it to slave mode only.
>
> I haven't looked into why, but I am getting the correct channel 
> mapping under the hood. To me that is a concern of the higher up 
> levels of the ALSA driver - it was a pleasant surprise when it worked 
> nicely, because to me it indicated a robustly coded system.
>
> My next patch is to allow DSP modes in this I2S driver. I wanted to 
> make sure that this got through first however.
>
> The majority of these other concerns you mention are controlled at the 
> machine driver level.
>
Mark,

On deeper thought around what you were saying about requiring master 
mode (for the sound card), I think you are right. Whilst it is possible 
for the BCM2835 to be master, it would pose significant problems for the 
sound card to regenerate the correct clocks.

Would you like me to resubmit the patch ensuring master mode for 
channels != 2 and also adding DSP mode capability ?

thanks
Matt
p.s. I didn't realise this was a thing already, are there any machine 
drivers for other SoCs which are currently using this paradigm of 
signalling ?

Re: [PATCH] ASoC: bcm2835: Increase channels_max to 8

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1368 bytes --]

On Mon, Feb 06, 2017 at 07:37:03AM +1100, Matt Flax wrote:
> On 06/02/17 03:34, Mark Brown wrote:

> > Looking at the driver it may be possible for the hardware to do DSP
> > modes at which point this gets much easier but right now it only has I2S
> > support, I've got a feeling that the other end of the link may actually
> > be running in a DSP mode.

> You've got it !

If you're actually using the device in DSP mode you need to be showing
that in the driver, not claiming that it's I2S.

> I haven't looked into why, but I am getting the correct channel mapping
> under the hood. To me that is a concern of the higher up levels of the ALSA
> driver - it was a pleasant surprise when it worked nicely, because to me it
> indicated a robustly coded system.

If the external device is running in DSP mode it'll work fine.

> My next patch is to allow DSP modes in this I2S driver. I wanted to make
> sure that this got through first however.

This is the wrong way round.

> The majority of these other concerns you mention are controlled at the
> machine driver level.

The driver should be checking for errors, it shouldn't be silently
accepting incorrect configurations - we shouldn't be forcing all the
machine drivers to replicate code for this, or requiring modifications
to machine drivers because a new feature has been added to one of the
drivers it uses.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[alsa-devel] [PATCH] ASoC: bcm2835: Increase channels_max to 8

[Mark Brown]

On Mon, Feb 06, 2017 at 07:37:03AM +1100, Matt Flax wrote:
> On 06/02/17 03:34, Mark Brown wrote:

> > Looking at the driver it may be possible for the hardware to do DSP
> > modes at which point this gets much easier but right now it only has I2S
> > support, I've got a feeling that the other end of the link may actually
> > be running in a DSP mode.

> You've got it !

If you're actually using the device in DSP mode you need to be showing
that in the driver, not claiming that it's I2S.

> I haven't looked into why, but I am getting the correct channel mapping
> under the hood. To me that is a concern of the higher up levels of the ALSA
> driver - it was a pleasant surprise when it worked nicely, because to me it
> indicated a robustly coded system.

If the external device is running in DSP mode it'll work fine.

> My next patch is to allow DSP modes in this I2S driver. I wanted to make
> sure that this got through first however.

This is the wrong way round.

> The majority of these other concerns you mention are controlled at the
> machine driver level.

The driver should be checking for errors, it shouldn't be silently
accepting incorrect configurations - we shouldn't be forcing all the
machine drivers to replicate code for this, or requiring modifications
to machine drivers because a new feature has been added to one of the
drivers it uses.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170206/c57d4efb/attachment.sig>

Re: [PATCH] ASoC: bcm2835: Increase channels_max to 8

[Matt Flax]

On 07/02/17 03:43, Mark Brown wrote:
> On Mon, Feb 06, 2017 at 07:37:03AM +1100, Matt Flax wrote:
>> On 06/02/17 03:34, Mark Brown wrote:
>>> Looking at the driver it may be possible for the hardware to do DSP
>>> modes at which point this gets much easier but right now it only has I2S
>>> support, I've got a feeling that the other end of the link may actually
>>> be running in a DSP mode.
>> You've got it !
> If you're actually using the device in DSP mode you need to be showing
> that in the driver, not claiming that it's I2S.
>
>> I haven't looked into why, but I am getting the correct channel mapping
>> under the hood. To me that is a concern of the higher up levels of the ALSA
>> driver - it was a pleasant surprise when it worked nicely, because to me it
>> indicated a robustly coded system.
> If the external device is running in DSP mode it'll work fine.
>
>> My next patch is to allow DSP modes in this I2S driver. I wanted to make
>> sure that this got through first however.
> This is the wrong way round.
>
>> The majority of these other concerns you mention are controlled at the
>> machine driver level.
> The driver should be checking for errors, it shouldn't be silently
> accepting incorrect configurations - we shouldn't be forcing all the
> machine drivers to replicate code for this, or requiring modifications
> to machine drivers because a new feature has been added to one of the
> drivers it uses.
>
I understand, that makes sense. I will submit a new patch which adds the 
DSP mode requirement and guards against misconfiguration.

Thank you for your guidance.
Matt

[alsa-devel] [PATCH] ASoC: bcm2835: Increase channels_max to 8

[Matt Flax]

On 07/02/17 03:43, Mark Brown wrote:
> On Mon, Feb 06, 2017 at 07:37:03AM +1100, Matt Flax wrote:
>> On 06/02/17 03:34, Mark Brown wrote:
>>> Looking at the driver it may be possible for the hardware to do DSP
>>> modes at which point this gets much easier but right now it only has I2S
>>> support, I've got a feeling that the other end of the link may actually
>>> be running in a DSP mode.
>> You've got it !
> If you're actually using the device in DSP mode you need to be showing
> that in the driver, not claiming that it's I2S.
>
>> I haven't looked into why, but I am getting the correct channel mapping
>> under the hood. To me that is a concern of the higher up levels of the ALSA
>> driver - it was a pleasant surprise when it worked nicely, because to me it
>> indicated a robustly coded system.
> If the external device is running in DSP mode it'll work fine.
>
>> My next patch is to allow DSP modes in this I2S driver. I wanted to make
>> sure that this got through first however.
> This is the wrong way round.
>
>> The majority of these other concerns you mention are controlled at the
>> machine driver level.
> The driver should be checking for errors, it shouldn't be silently
> accepting incorrect configurations - we shouldn't be forcing all the
> machine drivers to replicate code for this, or requiring modifications
> to machine drivers because a new feature has been added to one of the
> drivers it uses.
>
I understand, that makes sense. I will submit a new patch which adds the 
DSP mode requirement and guards against misconfiguration.

Thank you for your guidance.
Matt