On Mon, Oct 5, 2020 at 8:01 PM Maxime Ripard <maxime@xxxxxxxxxx> wrote: > > On Wed, Sep 30, 2020 at 09:11:43PM -0500, Samuel Holland wrote: > > The codec's clock input is shared among all AIFs, and shared with other > > audio-related hardware in the SoC, including I2S and SPDIF controllers. > > To ensure sample rates selected by userspace or by codec2codec DAI links > > are maintained, the clock rate must be protected while it is in use. > > > > Signed-off-by: Samuel Holland <samuel@xxxxxxxxxxxx> > > --- > > sound/soc/sunxi/sun8i-codec.c | 25 ++++++++++++++++++++++--- > > 1 file changed, 22 insertions(+), 3 deletions(-) > > > > diff --git a/sound/soc/sunxi/sun8i-codec.c b/sound/soc/sunxi/sun8i-codec.c > > index 501af64d43a0..86065bee7cd3 100644 > > --- a/sound/soc/sunxi/sun8i-codec.c > > +++ b/sound/soc/sunxi/sun8i-codec.c > > @@ -416,27 +416,32 @@ static int sun8i_codec_get_lrck_div_order(unsigned int slots, > > unsigned int div = slots * slot_width; > > > > if (div < 16 || div > 256) > > return -EINVAL; > > > > return order_base_2(div); > > } > > > > +static unsigned int sun8i_codec_get_sysclk_rate(unsigned int sample_rate) > > +{ > > + return sample_rate % 4000 ? 22579200 : 24576000; > > +} > > + > > static int sun8i_codec_hw_params(struct snd_pcm_substream *substream, > > struct snd_pcm_hw_params *params, > > struct snd_soc_dai *dai) > > { > > struct sun8i_codec *scodec = snd_soc_dai_get_drvdata(dai); > > struct sun8i_codec_aif *aif = &scodec->aifs[dai->id]; > > unsigned int sample_rate = params_rate(params); > > unsigned int slots = aif->slots ?: params_channels(params); > > unsigned int slot_width = aif->slot_width ?: params_width(params); > > - unsigned int sysclk_rate = clk_get_rate(scodec->clk_module); > > - int lrck_div_order, word_size; > > + unsigned int sysclk_rate = sun8i_codec_get_sysclk_rate(sample_rate); > > + int lrck_div_order, ret, word_size; > > u8 bclk_div; > > > > /* word size */ > > switch (params_width(params)) { > > case 8: > > word_size = 0x0; > > break; > > case 16: > > @@ -466,17 +471,30 @@ static int sun8i_codec_hw_params(struct snd_pcm_substream *substream, > > (lrck_div_order - 4) << SUN8I_AIF1CLK_CTRL_AIF1_LRCK_DIV); > > > > /* BCLK divider (SYSCLK/BCLK ratio) */ > > bclk_div = sun8i_codec_get_bclk_div(sysclk_rate, lrck_div_order, sample_rate); > > regmap_update_bits(scodec->regmap, SUN8I_AIF1CLK_CTRL, > > SUN8I_AIF1CLK_CTRL_AIF1_BCLK_DIV_MASK, > > bclk_div << SUN8I_AIF1CLK_CTRL_AIF1_BCLK_DIV); > > > > - if (!aif->open_streams) { > > + /* SYSCLK rate */ > > + if (aif->open_streams) { > > + ret = clk_set_rate(scodec->clk_module, sysclk_rate); > > + if (ret < 0) > > + return ret; > > + } else { > > + ret = clk_set_rate_exclusive(scodec->clk_module, sysclk_rate); > > It's not really clear to me why we wouldn't want to always protect the > clock rate here? I believe the intention is to allow a window, i.e. when no audio blocks are running, when it is possible to switch between sample rate families? ChenYu > > + if (ret == -EBUSY) > > + dev_err(dai->dev, "%s: clock is busy! Sample rate %u Hz " > > + "conflicts with other audio streams.\n", > > This string creates a checkpatch warning. > > Maxime