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

Pierre-Louis Bossart
2023-01-30T16:45:58Z

On 1/27/23 10:51, Stefan Binding wrote:
> From: Richard Fitzgerald <rf@opensource.cirrus.com>
>
> 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@opensource.cirrus.com>
> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

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