[alsa-devel] [PATCH v3] ASoC: tlv320aic31xx: Fix master mode clock I2S bus clocks

[Peter Ujfalusi]

On 2018-02-13 16:41, Jyri Sarha wrote:
>>  static int aic31xx_add_controls(struct snd_soc_codec *codec)
>>  {
>>  	int ret = 0;
>> @@ -916,6 +940,7 @@ static int aic31xx_set_dai_fmt(struct snd_soc_dai *codec_dai,
>>  			       unsigned int fmt)
>>  {
>>  	struct snd_soc_codec *codec = codec_dai->codec;
>> +	struct aic31xx_priv *aic31xx = snd_soc_codec_get_drvdata(codec);
>>  	u8 iface_reg1 = 0;
>>  	u8 iface_reg2 = 0;
>>  	u8 dsp_a_val = 0;
>> @@ -992,6 +1017,25 @@ static int aic31xx_set_dai_fmt(struct snd_soc_dai *codec_dai,
>>  			    AIC31XX_BCLKINV_MASK,
>>  			    iface_reg2);
>>  
> 
> I would put contents of the bellow if statement, with the
> aic31xx->master_dapm_route_applied check into a separate function and
> call it from "switch (fmt & SND_SOC_DAIFMT_MASTER_MASK)"-statement. But
> that is a matter of taste after all.

Moving to a new function is not a bad idea

> But shouldn't we remove the routes if SND_SOC_DAIFMT_CBS_CFS mode is used?

Especially if we handle the unlikely that the clock role can change runtime.

> 
>> +	/* Add the needed DAPM route(s) for codec clock master modes, once */
>> +	if (iface_reg1 & (AIC31XX_BCLK_MASTER | AIC31XX_WCLK_MASTER) &&
>> +	    !aic31xx->master_dapm_route_applied) {
>> +		struct snd_soc_dapm_context *dapm = snd_soc_codec_get_dapm(codec);
>> +		int ret;
>> +
>> +		ret = snd_soc_dapm_add_routes(dapm, common31xx_cm_audio_map,
>> +					ARRAY_SIZE(common31xx_cm_audio_map));
>> +		if (!ret && !(aic31xx->codec_type & DAC31XX_BIT))
>> +			ret = snd_soc_dapm_add_routes(dapm,
>> +					aic31xx_cm_audio_map,
>> +					ARRAY_SIZE(aic31xx_cm_audio_map));
>> +
>> +		if (ret)
>> +			return ret;
>> +
>> +		aic31xx->master_dapm_route_applied = true;
>> +	}
>> +
>>  	return 0;
>>  }
>>  
>>
> 
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki