Re: [PATCH v3 4/8] ASoC: cs42l42: Separate ASP config from PLL config

[Pierre-Louis Bossart]

On 1/27/23 10:51, Stefan Binding wrote:
> From: Richard Fitzgerald <rf@xxxxxxxxxxxxxxxxxxxxx>
> 
> Setup of the ASP (audio serial port) was being done as a side-effect of
> cs42l42_pll_config() and forces a restriction on the ratio of sample_rate
> to bit_clock that is invalid for Soundwire.
> 
> Move the ASP setup into a dedicated function.
> 
> Signed-off-by: Richard Fitzgerald <rf@xxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Stefan Binding <sbinding@xxxxxxxxxxxxxxxxxxxxx>

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>

> ---
>  sound/soc/codecs/cs42l42.c | 81 +++++++++++++++++++++-----------------
>  sound/soc/codecs/cs42l42.h |  1 -
>  2 files changed, 44 insertions(+), 38 deletions(-)
> 
> diff --git a/sound/soc/codecs/cs42l42.c b/sound/soc/codecs/cs42l42.c
> index 939f8bcc222c0..d81c6eb1c1e59 100644
> --- a/sound/soc/codecs/cs42l42.c
> +++ b/sound/soc/codecs/cs42l42.c
> @@ -658,7 +658,6 @@ static int cs42l42_pll_config(struct snd_soc_component *component, unsigned int
>  {
>  	struct cs42l42_private *cs42l42 = snd_soc_component_get_drvdata(component);
>  	int i;
> -	u32 fsync;
>  
>  	/* Don't reconfigure if there is an audio stream running */
>  	if (cs42l42->stream_use) {
> @@ -684,40 +683,6 @@ static int cs42l42_pll_config(struct snd_soc_component *component, unsigned int
>  					(pll_ratio_table[i].mclk_int !=
>  					24000000)) <<
>  					CS42L42_INTERNAL_FS_SHIFT);
> -
> -			/* Set up the LRCLK */
> -			fsync = clk / cs42l42->srate;
> -			if (((fsync * cs42l42->srate) != clk)
> -				|| ((fsync % 2) != 0)) {
> -				dev_err(component->dev,
> -					"Unsupported sclk %d/sample rate %d\n",
> -					clk,
> -					cs42l42->srate);
> -				return -EINVAL;
> -			}
> -			/* Set the LRCLK period */
> -			snd_soc_component_update_bits(component,
> -					CS42L42_FSYNC_P_LOWER,
> -					CS42L42_FSYNC_PERIOD_MASK,
> -					CS42L42_FRAC0_VAL(fsync - 1) <<
> -					CS42L42_FSYNC_PERIOD_SHIFT);
> -			snd_soc_component_update_bits(component,
> -					CS42L42_FSYNC_P_UPPER,
> -					CS42L42_FSYNC_PERIOD_MASK,
> -					CS42L42_FRAC1_VAL(fsync - 1) <<
> -					CS42L42_FSYNC_PERIOD_SHIFT);
> -			/* Set the LRCLK to 50% duty cycle */
> -			fsync = fsync / 2;
> -			snd_soc_component_update_bits(component,
> -					CS42L42_FSYNC_PW_LOWER,
> -					CS42L42_FSYNC_PULSE_WIDTH_MASK,
> -					CS42L42_FRAC0_VAL(fsync - 1) <<
> -					CS42L42_FSYNC_PULSE_WIDTH_SHIFT);
> -			snd_soc_component_update_bits(component,
> -					CS42L42_FSYNC_PW_UPPER,
> -					CS42L42_FSYNC_PULSE_WIDTH_MASK,
> -					CS42L42_FRAC1_VAL(fsync - 1) <<
> -					CS42L42_FSYNC_PULSE_WIDTH_SHIFT);
>  			if (pll_ratio_table[i].mclk_src_sel == 0) {
>  				/* Pass the clock straight through */
>  				snd_soc_component_update_bits(component,
> @@ -809,6 +774,46 @@ static void cs42l42_src_config(struct snd_soc_component *component, unsigned int
>  				      fs << CS42L42_CLK_OASRC_SEL_SHIFT);
>  }
>  
> +static int cs42l42_asp_config(struct snd_soc_component *component,
> +			      unsigned int sclk, unsigned int sample_rate)
> +{
> +	u32 fsync = sclk / sample_rate;
> +
> +	/* Set up the LRCLK */
> +	if (((fsync * sample_rate) != sclk) || ((fsync % 2) != 0)) {
> +		dev_err(component->dev,
> +			"Unsupported sclk %d/sample rate %d\n",
> +			sclk,
> +			sample_rate);
> +		return -EINVAL;
> +	}
> +	/* Set the LRCLK period */
> +	snd_soc_component_update_bits(component,
> +				      CS42L42_FSYNC_P_LOWER,
> +				      CS42L42_FSYNC_PERIOD_MASK,
> +				      CS42L42_FRAC0_VAL(fsync - 1) <<
> +				      CS42L42_FSYNC_PERIOD_SHIFT);
> +	snd_soc_component_update_bits(component,
> +				      CS42L42_FSYNC_P_UPPER,
> +				      CS42L42_FSYNC_PERIOD_MASK,
> +				      CS42L42_FRAC1_VAL(fsync - 1) <<
> +				      CS42L42_FSYNC_PERIOD_SHIFT);
> +	/* Set the LRCLK to 50% duty cycle */
> +	fsync = fsync / 2;
> +	snd_soc_component_update_bits(component,
> +				      CS42L42_FSYNC_PW_LOWER,
> +				      CS42L42_FSYNC_PULSE_WIDTH_MASK,
> +				      CS42L42_FRAC0_VAL(fsync - 1) <<
> +				      CS42L42_FSYNC_PULSE_WIDTH_SHIFT);
> +	snd_soc_component_update_bits(component,
> +				      CS42L42_FSYNC_PW_UPPER,
> +				      CS42L42_FSYNC_PULSE_WIDTH_MASK,
> +				      CS42L42_FRAC1_VAL(fsync - 1) <<
> +				      CS42L42_FSYNC_PULSE_WIDTH_SHIFT);
> +
> +	return 0;
> +}
> +
>  static int cs42l42_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt)
>  {
>  	struct snd_soc_component *component = codec_dai->component;
> @@ -904,8 +909,6 @@ static int cs42l42_pcm_hw_params(struct snd_pcm_substream *substream,
>  	unsigned int bclk;
>  	int ret;
>  
> -	cs42l42->srate = params_rate(params);
> -
>  	if (cs42l42->bclk_ratio) {
>  		/* machine driver has set the BCLK/samp-rate ratio */
>  		bclk = cs42l42->bclk_ratio * params_rate(params);
> @@ -966,6 +969,10 @@ static int cs42l42_pcm_hw_params(struct snd_pcm_substream *substream,
>  	if (ret)
>  		return ret;
>  
> +	ret = cs42l42_asp_config(component, bclk, sample_rate);
> +	if (ret)
> +		return ret;
> +
>  	cs42l42_src_config(component, sample_rate);
>  
>  	return 0;
> diff --git a/sound/soc/codecs/cs42l42.h b/sound/soc/codecs/cs42l42.h
> index a721366641127..17aab06adc8e6 100644
> --- a/sound/soc/codecs/cs42l42.h
> +++ b/sound/soc/codecs/cs42l42.h
> @@ -36,7 +36,6 @@ struct  cs42l42_private {
>  	int pll_config;
>  	u32 sclk;
>  	u32 bclk_ratio;
> -	u32 srate;
>  	u8 plug_state;
>  	u8 hs_type;
>  	u8 ts_inv;