On 2022/5/30 下午 12:01, Hui Wang wrote:

We tried to enable the audio on an imx6sx EVB with the codec nau8822, after setting the internal PLL fractional parameters, the audio still couldn't work and the there was no sdma irq at all.

After checking with the section "8.1.1 Phase Locked Loop (PLL) Design Example" of "NAU88C22 Datasheet Rev 0.6", we found we need to turn off the PLL before programming fractional parameters and turn on the PLL after programming.

After this change, the audio driver could record and play sound and the sdma's irq is triggered when playing or recording.

Cc: David Lin ctlin0@nuvoton.com Cc: John Hsu kchsu0@nuvoton.com Cc: Seven Li wtli@nuvoton.com Signed-off-by: Hui Wang hui.wang@canonical.com

---

sound/soc/codecs/nau8822.c | 4 ++++ sound/soc/codecs/nau8822.h | 3 +++ 2 files changed, 7 insertions(+)

diff --git a/sound/soc/codecs/nau8822.c b/sound/soc/codecs/nau8822.c index 58123390c7a3..b436e532993d 100644 --- a/sound/soc/codecs/nau8822.c +++ b/sound/soc/codecs/nau8822.c @@ -740,6 +740,8 @@ static int nau8822_set_pll(struct snd_soc_dai *dai, int pll_id, int source, pll_param->pll_int, pll_param->pll_frac, pll_param->mclk_scaler, pll_param->pre_factor);

• snd_soc_component_update_bits(component,

• NAU8822_REG_POWER_MANAGEMENT_1, NAU8822_PLL_EN_MASK, NAU8822_PLL_OFF);
  

  snd_soc_component_update_bits(component, NAU8822_REG_PLL_N, NAU8822_PLLMCLK_DIV2 | NAU8822_PLLN_MASK, (pll_param->pre_factor ? NAU8822_PLLMCLK_DIV2 : 0) |

@@ -757,6 +759,8 @@ static int nau8822_set_pll(struct snd_soc_dai *dai, int pll_id, int source, pll_param->mclk_scaler << NAU8822_MCLKSEL_SFT); snd_soc_component_update_bits(component, NAU8822_REG_CLOCKING, NAU8822_CLKM_MASK, NAU8822_CLKM_PLL);

• snd_soc_component_update_bits(component,

• NAU8822_REG_POWER_MANAGEMENT_1, NAU8822_PLL_EN_MASK, NAU8822_PLL_ON);
  
  

  return 0; }

diff --git a/sound/soc/codecs/nau8822.h b/sound/soc/codecs/nau8822.h index 489191ff187e..b45d42c15de6 100644 --- a/sound/soc/codecs/nau8822.h +++ b/sound/soc/codecs/nau8822.h @@ -90,6 +90,9 @@ #define NAU8822_REFIMP_3K 0x3 #define NAU8822_IOBUF_EN (0x1 << 2) #define NAU8822_ABIAS_EN (0x1 << 3) +#define NAU8822_PLL_EN_MASK (0x1 << 5) +#define NAU8822_PLL_ON (0x1 << 5) +#define NAU8822_PLL_OFF (0x0 << 5)

/* NAU8822_REG_AUDIO_INTERFACE (0x4) */ #define NAU8822_AIFMT_MASK (0x3 << 3)

Sorry, reply late.

From our internal discussion, the revise seems to it is redundant operation. The reason is driver set the PLL as a dapm supply node and consider PLL on/off from dapm route.

So when the playback/recording starts, the PLL parameters from Reg 0x25~0x27 will be always set before Reg 0x1[5] power enable bit(PLLEN). When the playback/recording stops, the PLLEN will be disabled.

On 6/3/22 21:26, David Lin wrote:

On 2022/6/3 下午 06:10, Mark Brown wrote:

...

On Fri, Jun 03, 2022 at 05:55:18PM +0800, Hui Wang wrote:

...

On 6/2/22 18:33, Mark Brown wrote:

...

...

Thanks for your comment. But it is weird, it doesn't work like you said, probably need specific route setting in the machine driver level?

Is this triggering due to reprogramming the PLL for one direction while the other is already active (eg, starting a capture during a playba

Yes, it is. With the current machine driver of fsl-asoc-card.c, if starting a capture during a playback or starting a playback during a capture, the codec driver will reprogram PLL parameters while PLL is on.

So your patch fixes that case - note however that you're probably getting an audio glitch in the already active stream while doing this.  I'll send a patch which should improve that shortly. BTW, shouldn't the PLL power be left off if the output frequency is 0?

Agree Mark's comment.

By the way, when the platform's dai_link support be_hw_params_fixup callback, the sample rate will be fixed to same rate, so PLL will not be also reconfigured during playback and recording at the same time.

Agree with your comment. And the Mark's patch is a better one. After Mark's  patch is merged, I will revert my [1/2 PATCH] if that is not dropped from the Mark's tree.

And then I will update my [2/2 PATCH] as below since the error of "fsl-asoc-card sound-nau8822: failed to stop FLL: -22" need to be handled and pll_param->freq_in/freq_out need to be cleared. If freq_in/freq_out is not cleared, it is possible that the NAU8822_REG_CLOCKING can't be updated (suppose play sound by PLL -> MCLK -> PLL).

@@ -726,6 +726,13 @@ static int nau8822_set_pll(struct snd_soc_dai *dai, int pll_id, int source,         struct nau8822_pll *pll_param = &nau8822->pll;         int ret, fs;

+       if (freq_in == 0 || freq_out == 0) { +               dev_dbg(component->dev, "PLL disabled\n"); +               pll_param->freq_in = 0; +               pll_param->freq_out = 0; +               return 0; +       } +         if (freq_in == pll_param->freq_in &&             freq_out == pll_param->freq_out)                 return 0;

...

...

And in another case, if the snd_soc_dai_set_pll() is called in the card->set_bias_level() instead of card_hw_params(), the PLL will keep being off since check_mclk_select_pll() always returns false.

That should be fixable...

Hi Mark,

I saw you merged the [PATCH 1/2], the [PATCH 2/2] is also needed. Please take a look.

Thanks,

Hui.

On 5/30/22 12:01, Hui Wang wrote:

After finishing the playback or recording, the machine driver might call snd_soc_dai_set_pll(codec, pll_id, 0, 0, 0) to stop the internal PLL, but with the codec driver nau8822, it will print error as below: nau8822 0-001a: Unsupported input clock 0 fsl-asoc-card sound-nau8822: failed to stop FLL: -22

Refer to the function wm8962_set_fll() in the codec driver wm8962, if the freq_out is zero, turn off the internal PLL and return 0.

Cc: David Lin ctlin0@nuvoton.com Cc: John Hsu kchsu0@nuvoton.com Cc: Seven Li wtli@nuvoton.com Signed-off-by: Hui Wang hui.wang@canonical.com

---

sound/soc/codecs/nau8822.c | 7 +++++++ 1 file changed, 7 insertions(+)

diff --git a/sound/soc/codecs/nau8822.c b/sound/soc/codecs/nau8822.c index b436e532993d..4d3720c69f91 100644 --- a/sound/soc/codecs/nau8822.c +++ b/sound/soc/codecs/nau8822.c @@ -726,6 +726,13 @@ static int nau8822_set_pll(struct snd_soc_dai *dai, int pll_id, int source, struct nau8822_pll *pll_param = &nau8822->pll; int ret, fs;

• if (freq_out == 0) {

• dev_dbg(component->dev, "PLL disabled\n");
  

• snd_soc_component_update_bits(component,
  

• 	NAU8822_REG_POWER_MANAGEMENT_1, NAU8822_PLL_EN_MASK, NAU8822_PLL_OFF);
  

• return 0;
  

• }

• fs = freq_out / 256;

  ret = nau8822_calc_pll(freq_in, fs, pll_param);