* [PATCH] ASoC: bcm2835: Remove redundant channel checking code in hw_params
@ 2017-01-15 1:17 Matt Flax
2017-01-20 5:58 ` Matt Flax
2017-02-01 4:45 ` [PATCH v1] " Matt Flax
0 siblings, 2 replies; 6+ messages in thread
From: Matt Flax @ 2017-01-15 1:17 UTC (permalink / raw)
To: Stephen Warren, alsa-devel, Mark Brown, Takashi Iwai, flatmax
Due to channels_max and channels_min in the bcm2835_i2s_dai structure,
the channel guard code in bcm2835_i2s_hw_params is redundant.
The bcm2835_i2s_hw_params function checks that the channel count is
2 before continuing. This code never gets executed because core soc
checks prevent the execution (upon channel mismatch) of the
bcm2835_i2s_hw_params function.
The redundancy has been tested and checked for the case when
the bcm2835_i2s_dai structure has channels_max = 2 and a request
for 8 channels has been made. The test has confirmed that the
removed code is redundant as it never gets executed when a
channel mismatch occurs.
This redundant channel count check may seem like it will not cause
any problems however some sound cards (such as the 8 channel
AudioInjector Octo sound card) require the execution of the
bcm2835_i2s_hw_params function when channels_max in the
bcm2835_i2s_dai structure has been increased to 8.
As the channel checking code in bcm2835_i2s_hw_params is redundant
and stops certain cards from working it has been removed.
Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
sound/soc/bcm/bcm2835-i2s.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index 6ba2049..1773b83 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -310,15 +310,9 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
ch1pos = data_delay;
ch2pos = bclk_ratio / 2 + data_delay;
- switch (params_channels(params)) {
- case 2:
- 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));
- break;
- default:
- return -EINVAL;
- }
+ 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));
/*
* Set format for both streams.
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] ASoC: bcm2835: Remove redundant channel checking code in hw_params
2017-01-15 1:17 [PATCH] ASoC: bcm2835: Remove redundant channel checking code in hw_params Matt Flax
@ 2017-01-20 5:58 ` Matt Flax
2017-02-01 4:45 ` [PATCH v1] " Matt Flax
1 sibling, 0 replies; 6+ messages in thread
From: Matt Flax @ 2017-01-20 5:58 UTC (permalink / raw)
To: Stephen Warren, alsa-devel, Mark Brown, Takashi Iwai
This hasn't had any airplay since posting 5 days ago.
Can anyone advise on the correct person to contact regarding the review
of this patch ?
thanks
Matt
On 15/01/17 12:17, Matt Flax wrote:
> Due to channels_max and channels_min in the bcm2835_i2s_dai structure,
> the channel guard code in bcm2835_i2s_hw_params is redundant.
>
> The bcm2835_i2s_hw_params function checks that the channel count is
> 2 before continuing. This code never gets executed because core soc
> checks prevent the execution (upon channel mismatch) of the
> bcm2835_i2s_hw_params function.
>
> The redundancy has been tested and checked for the case when
> the bcm2835_i2s_dai structure has channels_max = 2 and a request
> for 8 channels has been made. The test has confirmed that the
> removed code is redundant as it never gets executed when a
> channel mismatch occurs.
>
> This redundant channel count check may seem like it will not cause
> any problems however some sound cards (such as the 8 channel
> AudioInjector Octo sound card) require the execution of the
> bcm2835_i2s_hw_params function when channels_max in the
> bcm2835_i2s_dai structure has been increased to 8.
>
> As the channel checking code in bcm2835_i2s_hw_params is redundant
> and stops certain cards from working it has been removed.
>
> Signed-off-by: Matt Flax <flatmax@flatmax.org>
> ---
> sound/soc/bcm/bcm2835-i2s.c | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
> index 6ba2049..1773b83 100644
> --- a/sound/soc/bcm/bcm2835-i2s.c
> +++ b/sound/soc/bcm/bcm2835-i2s.c
> @@ -310,15 +310,9 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
> ch1pos = data_delay;
> ch2pos = bclk_ratio / 2 + data_delay;
>
> - switch (params_channels(params)) {
> - case 2:
> - 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));
> - break;
> - default:
> - return -EINVAL;
> - }
> + 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));
>
> /*
> * Set format for both streams.
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v1] ASoC: bcm2835: Remove redundant channel checking code in hw_params
2017-01-15 1:17 [PATCH] ASoC: bcm2835: Remove redundant channel checking code in hw_params Matt Flax
2017-01-20 5:58 ` Matt Flax
@ 2017-02-01 4:45 ` Matt Flax
2017-02-01 10:53 ` Mark Brown
1 sibling, 1 reply; 6+ messages in thread
From: Matt Flax @ 2017-02-01 4:45 UTC (permalink / raw)
To: alsa-devel, Stephen Warren, Lee Jones, Eric Anholt,
linux-rpi-kernel, linux-arm-kernel, Florian Meier, broonie, phil
Cc: Matt Flax
Due to channels_max and channels_min in the bcm2835_i2s_dai structure,
the channel guard code in bcm2835_i2s_hw_params is redundant.
The bcm2835_i2s_hw_params function checks that the channel count is
2 before continuing. This code never gets executed because core soc
checks prevent the execution (upon channel mismatch) of the
bcm2835_i2s_hw_params function.
The redundancy has been tested and checked for the case when
the bcm2835_i2s_dai structure has channels_max = 2 and a request
for 8 channels has been made. The test has confirmed that the
removed code is redundant as it never gets executed when a
channel mismatch occurs.
This redundant channel count check may seem like it will not cause
any problems however some sound cards (such as the 8 channel
AudioInjector Octo sound card) require the execution of the
bcm2835_i2s_hw_params function when channels_max in the
bcm2835_i2s_dai structure has been increased to 8.
As the channel checking code in bcm2835_i2s_hw_params is redundant
and stops certain cards from working it has been removed.
Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
sound/soc/bcm/bcm2835-i2s.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c
index 6ba2049..1773b83 100644
--- a/sound/soc/bcm/bcm2835-i2s.c
+++ b/sound/soc/bcm/bcm2835-i2s.c
@@ -310,15 +310,9 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream,
ch1pos = data_delay;
ch2pos = bclk_ratio / 2 + data_delay;
- switch (params_channels(params)) {
- case 2:
- 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));
- break;
- default:
- return -EINVAL;
- }
+ 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));
/*
* Set format for both streams.
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v1] ASoC: bcm2835: Remove redundant channel checking code in hw_params
2017-02-01 4:45 ` [PATCH v1] " Matt Flax
@ 2017-02-01 10:53 ` Mark Brown
2017-02-01 11:05 ` Matt Flax
0 siblings, 1 reply; 6+ messages in thread
From: Mark Brown @ 2017-02-01 10:53 UTC (permalink / raw)
To: Matt Flax
Cc: alsa-devel, Stephen Warren, Lee Jones, phil, Eric Anholt,
Florian Meier, linux-rpi-kernel, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 833 bytes --]
On Wed, Feb 01, 2017 at 03:45:03PM +1100, Matt Flax wrote:
> This redundant channel count check may seem like it will not cause
> any problems however some sound cards (such as the 8 channel
> AudioInjector Octo sound card) require the execution of the
> bcm2835_i2s_hw_params function when channels_max in the
> bcm2835_i2s_dai structure has been increased to 8.
> As the channel checking code in bcm2835_i2s_hw_params is redundant
> and stops certain cards from working it has been removed.
If those cards add support for higher channel counts they should be
adding real support for that in the driver, not just hacking out
defensive code. If the device can usefully do 8 channels then modify
the driver to do 8 channels. Otherwise people are going to need to
modify the driver anyway to remove the limits in the constraints.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] ASoC: bcm2835: Remove redundant channel checking code in hw_params
2017-02-01 10:53 ` Mark Brown
@ 2017-02-01 11:05 ` Matt Flax
2017-02-01 11:14 ` Mark Brown
0 siblings, 1 reply; 6+ messages in thread
From: Matt Flax @ 2017-02-01 11:05 UTC (permalink / raw)
To: Mark Brown
Cc: alsa-devel, Stephen Warren, Lee Jones, phil, Eric Anholt,
Florian Meier, linux-rpi-kernel, linux-arm-kernel
On 01/02/17 21:53, Mark Brown wrote:
> On Wed, Feb 01, 2017 at 03:45:03PM +1100, Matt Flax wrote:
>
>> This redundant channel count check may seem like it will not cause
>> any problems however some sound cards (such as the 8 channel
>> AudioInjector Octo sound card) require the execution of the
>> bcm2835_i2s_hw_params function when channels_max in the
>> bcm2835_i2s_dai structure has been increased to 8.
>> As the channel checking code in bcm2835_i2s_hw_params is redundant
>> and stops certain cards from working it has been removed.
> If those cards add support for higher channel counts they should be
> adding real support for that in the driver, not just hacking out
> defensive code. If the device can usefully do 8 channels then modify
> the driver to do 8 channels. Otherwise people are going to need to
> modify the driver anyway to remove the limits in the constraints.
>
Indeed the AudioInjector Octo sound card is currently operating @ 8
channels out on the bcm2835 silicon. It has been tested on both the Pi 2
and Pi 3.
Do you mean that I should increase bcm2835_i2s_dai.channels_max to 8 ?
I can do this. Does anyone have any objections to this approach ?
thanks
Matt
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1] ASoC: bcm2835: Remove redundant channel checking code in hw_params
2017-02-01 11:05 ` Matt Flax
@ 2017-02-01 11:14 ` Mark Brown
0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2017-02-01 11:14 UTC (permalink / raw)
To: Matt Flax
Cc: alsa-devel, Stephen Warren, Lee Jones, phil, Eric Anholt,
Florian Meier, linux-rpi-kernel, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 726 bytes --]
On Wed, Feb 01, 2017 at 10:05:18PM +1100, Matt Flax wrote:
> On 01/02/17 21:53, Mark Brown wrote:
> > If those cards add support for higher channel counts they should be
> > adding real support for that in the driver, not just hacking out
> > defensive code. If the device can usefully do 8 channels then modify
> > the driver to do 8 channels. Otherwise people are going to need to
> > modify the driver anyway to remove the limits in the constraints.
> Indeed the AudioInjector Octo sound card is currently operating @ 8 channels
> out on the bcm2835 silicon. It has been tested on both the Pi 2 and Pi 3.
> Do you mean that I should increase bcm2835_i2s_dai.channels_max to 8 ?
Yes, implementing it properly is fine.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-02-01 11:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-15 1:17 [PATCH] ASoC: bcm2835: Remove redundant channel checking code in hw_params Matt Flax
2017-01-20 5:58 ` Matt Flax
2017-02-01 4:45 ` [PATCH v1] " Matt Flax
2017-02-01 10:53 ` Mark Brown
2017-02-01 11:05 ` Matt Flax
2017-02-01 11:14 ` Mark Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).