[PATCH] ASoC: rt1015: support TDM slot configuration

[Pierre-Louis Bossart]

> +static int rt1015_set_tdm_slot(struct snd_soc_dai *dai,
> +	unsigned int tx_mask, unsigned int rx_mask, int slots, int slot_width)
> +{
> +	struct snd_soc_component *component = dai->component;
> +	unsigned int val = 0, rx_slotnum, tx_slotnum;
> +	int ret = 0, first_bit;
> +
> +	switch (slots) {
> +	case 4:
> +		val |= RT1015_I2S_TX_4CH;
> +		break;
> +	case 6:
> +		val |= RT1015_I2S_TX_6CH;
> +		break;
> +	case 8:
> +		val |= RT1015_I2S_TX_8CH;
> +		break;
> +	case 2:
> +		break;

nit-pick: I would put the case 2 first to keep the order. I thought for 
a second this was an error case due to the discontinuity.

> +	default:
> +		ret = -EINVAL;
> +		goto _set_tdm_err_;
> +	}
> +
> +	switch (slot_width) {
> +	case 20:
> +		val |= RT1015_I2S_CH_TX_LEN_20B;
> +		break;
> +	case 24:
> +		val |= RT1015_I2S_CH_TX_LEN_24B;
> +		break;
> +	case 32:
> +		val |= RT1015_I2S_CH_TX_LEN_32B;
> +		break;
> +	case 16:
> +		break;

nit-pick: same here, I would put 16 first.

> +	default:
> +		ret = -EINVAL;
> +		goto _set_tdm_err_;
> +	}
> +
> +	/* Rx slot configuration */
> +	rx_slotnum = hweight_long(rx_mask);
> +	if (rx_slotnum > 1 || !rx_slotnum) {

I am confused here, is this saying you can only have a single channel on RX?

If yes should this be simplified as if (rx_slotnum != 1) ?

> +		ret = -EINVAL;
> +		dev_err(component->dev, "too many rx slots or zero slot\n");
> +		goto _set_tdm_err_;
> +	}
> +
> +	first_bit = __ffs(rx_mask);
> +	switch (first_bit) {
> +	case 0:
> +	case 2:
> +	case 4:
> +	case 6:
> +		snd_soc_component_update_bits(component,
> +			RT1015_PAD_DRV2, RT1015_MONO_LR_SEL_MASK,
> +			RT1015_MONO_L_CHANNEL);
> +		snd_soc_component_update_bits(component,
> +			RT1015_TDM1_4,
> +			RT1015_TDM_I2S_TX_L_DAC1_1_MASK |
> +			RT1015_TDM_I2S_TX_R_DAC1_1_MASK,
> +			(first_bit << RT1015_TDM_I2S_TX_L_DAC1_1_SFT) |
> +			((first_bit+1) << RT1015_TDM_I2S_TX_R_DAC1_1_SFT));

looks like there's an assumption that the rx mask has contiguous bits 
set? Maybe add a comment to explain how the RX path works?

> +		break;
> +	case 1:
> +	case 3:
> +	case 5:
> +	case 7:
> +		snd_soc_component_update_bits(component,
> +			RT1015_PAD_DRV2, RT1015_MONO_LR_SEL_MASK,
> +			RT1015_MONO_R_CHANNEL);
> +		snd_soc_component_update_bits(component,
> +			RT1015_TDM1_4,
> +			RT1015_TDM_I2S_TX_L_DAC1_1_MASK |
> +			RT1015_TDM_I2S_TX_R_DAC1_1_MASK,
> +			((first_bit-1) << RT1015_TDM_I2S_TX_L_DAC1_1_SFT) |
> +			(first_bit << RT1015_TDM_I2S_TX_R_DAC1_1_SFT));
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		goto _set_tdm_err_;
> +	}
> +
> +	/* Tx slot configuration */
> +	tx_slotnum = hweight_long(tx_mask);
> +	if (tx_slotnum) {
> +		ret = -EINVAL;
> +		dev_err(component->dev, "doesn't need to support tx slots\n");
> +		goto _set_tdm_err_;
> +	}
> +
> +	snd_soc_component_update_bits(component, RT1015_TDM1_1,
> +		RT1015_I2S_CH_TX_MASK | RT1015_I2S_CH_RX_MASK |
> +		RT1015_I2S_CH_TX_LEN_MASK | RT1015_I2S_CH_RX_LEN_MASK, val);
> +
> +_set_tdm_err_:
> +	return ret;
> +}
> +