[alsa-devel] [PATCH v4] ASoC: bcm2835: Add 8 channel (multitrack) capability

[Arnaud Mouiche]

Hi,

On 14/02/2017 22:04, Matt Flax wrote:
> [...]
> +	if (dev->fmt &
> +		(SND_SOC_DAIFMT_CBM_CFM | SND_SOC_DAIFMT_DSP_A))

SND_SOC_DAIFMT_DSP_A is not a bit. It is an enum
	#define SND_SOC_DAIFMT_I2S		1 /* I2S mode */
	#define SND_SOC_DAIFMT_RIGHT_J		2 /* Right Justified mode */
	#define SND_SOC_DAIFMT_LEFT_J		3 /* Left Justified mode */
	#define SND_SOC_DAIFMT_DSP_A		4 /* L data MSB after FRM LRC */
	#define SND_SOC_DAIFMT_DSP_B		5 /* L data MSB during FRM LRC */
	#define SND_SOC_DAIFMT_AC97		6 /* AC97 */
	#define SND_SOC_DAIFMT_PDM		7 /* Pulse density modulation */

So your test is wrong.

Other question that bother me (I'm not familiar with bcm2835, so may be I'm wrong):
why should we put channel number constraint in the SOC driver whereas ASOC already compute
the constraint by making the intersection of channels constraints coming from soc AND codecs.

If you connect one 8 channel capable soc driver (eg. bcm2835) with a 2 channel codec in DSP_A mode, ASOC already tell
the user space that the limit is 2 channels.
Shouldn't you just specify that bcm2835 is 8 channel capable (and may be more), whatever the format is, and let ASOC do the job.
Given a specific device tree connecting the bcm2835 with one or more codecs is sufficient to specify the limits.

And if in the future someone wants to connect a 4 channels capable codec, or a 2x 2channels codecs, must he also send a new patch with 4 channels support ?

Regards,
Arnaud

> +		return snd_pcm_hw_constraint_single(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 8);
> +	else
> +		return snd_pcm_hw_constraint_single(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
>   }
>   
>   static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
> @@ -549,6 +560,10 @@ static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream,
>   	 * not stop the clock when SND_SOC_DAIFMT_CONT
>   	 */
>   	bcm2835_i2s_stop_clock(dev);
> +
> +	/* Default to 2 channels */
> +	snd_pcm_hw_constraint_single(substream->runtime,
> +			SNDRV_PCM_HW_PARAM_CHANNELS, 2);
>   }
>   
>   static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = {
> @@ -576,16 +591,12 @@ static struct snd_soc_dai_driver bcm2835_i2s_dai = {
>   	.name	= "bcm2835-i2s",
>   	.probe	= bcm2835_i2s_dai_probe,
>   	.playback = {
> -		.channels_min = 2,
> -		.channels_max = 2,
>   		.rates =	SNDRV_PCM_RATE_8000_192000,
>   		.formats =	SNDRV_PCM_FMTBIT_S16_LE
>   				| SNDRV_PCM_FMTBIT_S24_LE
>   				| SNDRV_PCM_FMTBIT_S32_LE
>   		},
>   	.capture = {
> -		.channels_min = 2,
> -		.channels_max = 2,
>   		.rates =	SNDRV_PCM_RATE_8000_192000,
>   		.formats =	SNDRV_PCM_FMTBIT_S16_LE
>   				| SNDRV_PCM_FMTBIT_S24_LE