Re: [RFC PATCH 05/16] ASoC: Intel: sof-pcm512x: reconfigure sclk in hw_params if needed

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

>> +	err = snd_interval_ratnum(hw_param_interval(params,
>> +						    SNDRV_PCM_HW_PARAM_RATE),
>> +				  1, &rats_no_pll, &num, &den);
> 
>> +	if (err >= 0 && den) {
> 
> Perhaps usual pattern, i.e.
> 
> 	if (err < 0 || !den)
> 		return 0;
> (so, above seems optional configuration)
> 
> 	params...;
> 	return 0;

ok

> 
>> +	if (ctx->is_dac_pro) {
> 
> 	if (!...)
> 		return 0;
> 
> ...and drop the redundant ret assignment above.

yes, this was suggested by Guennadi today as well.


>> +		ret = aif1_update_rate_den(substream, params);
>> +		if (ret) {
>> +			dev_err(dev, "Failed to update rate denominator: %d\n", ret);
>> +			return ret;
>> +		}
> 
> Do you still need below steps when current_rate == sclk_rate?

Good question. I assume the values are properly stored by the regmap 
cache, but if these channel and width do change (something we don't 
support for now) then yes we should move this out of the if case.

I'll give it a try, thanks for flagging this.

>> +		ret = snd_soc_dai_set_bclk_ratio(rtd->codec_dai,
>> +						 channels * width);
>> +		if (ret) {
>> +			dev_err(dev, "Failed to set bclk ratio : %d\n", ret);
>> +			return ret;
>> +		}
>> +	}
>> +
>> +	return ret;
>> +}
>