Re: [PATCH v5 2/4] ASoC: cdns: Add Cadence I2S-SC controller driver

[Mark Brown]

> Add support for the Cadence I2S-SC controller found in the CIX SKY1
> audio subsystem.

> +static int cdns_i2s_sc_hw_params(struct snd_pcm_substream *substream,
> +				 struct snd_pcm_hw_params *params,
> +				 struct snd_soc_dai *cpu_dai)
> +{

> +	} else {
> +		/*
> +		 * I2S mode: mono maps onto the left audio channel, while
> +		 * the frame still carries two slots.
> +		 */
> +		if (slots == 1) {
> +			i2s_sc_priv->is_mono_in_i2s_mode = true;
> +			slots = 2;
> +		} else if (slots == 2) {
> +			i2s_sc_priv->is_mono_in_i2s_mode = false;
> +		} else {
> +			dev_warn(i2s_sc_priv->dev,
> +				 "I2S mode supports up to two channels\n");
> +			return -EOPNOTSUPP;

You should really set constraints in startup() which limits to two
channels if the DAI is configured in I2S mode, that way userspace won't
get upset when it tries to do 4 channels..

> +static struct snd_soc_dai_driver cdns_i2s_sc_dai = {
> +	.name = "i2s-sc-aif",
> +	.playback = {
> +		.stream_name = "I2S-SC-Playback",
> +		.channels_min = 1,
> +		.channels_max = 16,
> +		.rates = SNDRV_PCM_RATE_8000_192000,
> +		.formats = (SNDRV_PCM_FMTBIT_S16_LE |
> +			    SNDRV_PCM_FMTBIT_S24_LE |
> +			    SNDRV_PCM_FMTBIT_S32_LE),
> +	},
> +	.capture = {

The DT binding allows either TX or RX DMA to be omitted, the driver
should only register the directions it's got DMA for.

-- 
Mark Brown <broonie@xxxxxxxxxx>