On 4/22/2019 1:09 PM, Cheng-yi Chiang wrote:

Hi Akshu,

On Mon, Apr 22, 2019 at 2:15 PM Agrawal, Akshu Akshu.Agrawal@amd.com wrote:

...

We need to set minimum bclk 64x of wclk as this is hw constraint in one of the component used. Since, clk_set_rate and clk is enabled in machine driver the clk_set_rate in hw_params of da7219 fails and errors out and when it tries to override the value.

In cases like these not only clk_set_rate of da7219 codec should fail and not override the value but also should not error out.

Signed-off-by: Akshu Agrawal akshu.agrawal@amd.com

sound/soc/codecs/da7219.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/sound/soc/codecs/da7219.c b/sound/soc/codecs/da7219.c index 5f5fa3416af3..a041dbf442af 100644 --- a/sound/soc/codecs/da7219.c +++ b/sound/soc/codecs/da7219.c @@ -1621,13 +1621,7 @@ static int da7219_hw_params(struct snd_pcm_substream *substream,

            if (bclk) {
                    bclk_rate = frame_size * sr;

•                   ret = clk_set_rate(bclk, bclk_rate);
  

•                   if (ret) {
  

•                           dev_err(component->dev,
  

•                                   "Failed to set BCLK rate %lu: %d\n",
  

•                                   bclk_rate, ret);
  

•                           return ret;
  

•                   }
  

•                   clk_set_rate(bclk, bclk_rate);
  

I think this might mask the real issue when setting rate fails.

Not doing an error check for clk_set_rate is not rare because clk_set_rate fails if the clk which you are trying to set is already enabled. Now we can add a is_prepared check before calling clk_set_rate but that would be redundant as clk_set_rate does the same check.

A more fundamental problem might be what are the acceptable ratios between bclk and wclk.

acceptable ratios for da7219 from spec are: 00 = 32 BCLKS per WCLK 01 = 64 BCLKS per WCLK 10 = 128 BCLK per WCLK 11 = 256 BCLK per WCLK

da7219 derives this ration from SR for a wclk. The same we do in machine driver. Conflict comes only in case of an exception where there is constraint from a hw component. Else when the audio is played on da7219 codec we could have let da7219 only set rate and enable clks.

Thanks, Akshu

From existing code of da7219 I see there are only two choices of frame_size: 32 or 64.

In da7219_hw_params: ... if (da7219->master && !da7219->tdm_en) { if ((word_len * DA7219_DAI_CH_NUM_MAX) <= 32) frame_size = 32; else frame_size = 64;

And in https://lkml.org/lkml/2019/4/17/322 the choice from machine driver is 64 or higher.

In cz_da7219_params: ... if (bclk < (wclk * 64)) bclk = wclk * 64; return da7219_clk_enable(substream, wclk, bclk);

I don't know what is the best solution to avoid this conflict. Maybe just set a fixed ratio from device property that can best work for the combination of codecs on the machine?

...

            } else {
                    ret = da7219_set_bclks_per_wclk(component, frame_size);
                    if (ret) {

-- 2.17.1

---

Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel