On 08/03/17 22:33, Matthias Reichl wrote:
On Wed, Mar 08, 2017 at 09:58:54PM +1100, Matt Flax wrote:
This patch simplifies the bcm2835_i2s.c code when the driver returns error on mono channel requests. It does this by testing for mono instead of using a switch statement to test for stereo.
The bcm2835_i2s driver can only work in stereo mode. Whilst leaving the driver entirel compliant with the I2S protocol, it simplifies the code by removing a few redundant lines of code.
Signed-off-by: Matt Flax flatmax@flatmax.org
I'm against this, it removes the semantics that I2S can only do 2 channels from the code part. Without looking at the rest of the code one could get the impression that bcm2835-i2s could support all layouts except 1 channel.
The comment clearly states :
/* * Check we aren't setting channel 2 on a mono stream. * We currently only support stereo */
How could anyone possibly think that it would support "all layouts except 1 channel" ?
I have reduced the clock cycle count to implement the same logic in code. I have clearly indicated that we ONLY support stereo channel counts.
In my opinion this code is simpler, easier to read and makes more sense.
thanks Matt
sound/soc/bcm/bcm2835-i2s.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c index 6ba2049..1e6a1da 100644 --- a/sound/soc/bcm/bcm2835-i2s.c +++ b/sound/soc/bcm/bcm2835-i2s.c @@ -310,15 +310,16 @@ 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:
- /* C
* Check we aren't setting channel 2 on a mono stream.* We currently only support stereo*/- if (params_channels(params) == 1) 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
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel