[PATCH] ASoC: rt1015: support TDM slot configuration

[Shuming [范書銘]]

> > +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.

OK, will fix that.

> > +	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) ?

Yes, RT1015 is a mono amplifier, so only a single channel will be configured.
I will fix the check condition as your suggestion. Thanks.

> > +		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?

Yes. In the normal case, the system will send the stereo audio to the codec.
I assume that the stereo audio is placed in slot 0/2/4/6.
I will add a comment above this switch case.

> > +		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;