Re: [PATCH] ASoC: mediatek: mt8188: Enable apll1 clock during reg rw to prevent hang

["Trevor Wu (吳文良)" <Trevor.Wu@mediatek.com>]

On Thu, 2024-12-05 at 13:51 +0100, AngeloGioacchino Del Regno wrote:
> 
> 
> Il 04/12/24 13:17, Trevor Wu (吳文良) ha scritto:
> > On Tue, 2024-12-03 at 17:07 -0300, Nícolas F. R. A. Prado wrote:
> > 
> > > 
> > > Currently, booting the Genio 700 EVK board with the MT8188 sound
> > > platform driver configured as a module (CONFIG_SND_SOC_MT8188=m)
> > > results
> > > in a system hang right when the HW registers for the audio
> > > controller
> > > are read:
> > > 
> > >    mt8188-audio 10b10000.audio-controller: No cache defaults,
> > > reading
> > > back from HW
> > > 
> > > The hang doesn't occur with the driver configured as builtin as
> > > then
> > > the
> > > unused clocks are still enabled.
> > > 
> > > Enable the apll1 clock during register read/write to prevent the
> > > hang.
> > > 
> > > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > > ---
> > >   sound/soc/mediatek/mt8188/mt8188-afe-clk.c | 4 ++++
> > >   1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/sound/soc/mediatek/mt8188/mt8188-afe-clk.c
> > > b/sound/soc/mediatek/mt8188/mt8188-afe-clk.c
> > > index
> > > e69c1bb2cb239596dee50b166c20192d5408be10..fb8cf286df3f02ac076528b
> > > 898f
> > > d0d7a708ec1ea 100644
> > > --- a/sound/soc/mediatek/mt8188/mt8188-afe-clk.c
> > > +++ b/sound/soc/mediatek/mt8188/mt8188-afe-clk.c
> > > @@ -587,6 +587,8 @@ int mt8188_afe_enable_reg_rw_clk(struct
> > > mtk_base_afe *afe)
> > >          mt8188_afe_enable_clk(afe, afe_priv-
> > > > clk[MT8188_CLK_AUD_A1SYS_HP]);
> > > 
> > >          mt8188_afe_enable_clk(afe, afe_priv-
> > > > clk[MT8188_CLK_AUD_A1SYS]);
> > > 
> > > +       mt8188_afe_enable_clk(afe, afe_priv-
> > > > clk[MT8188_CLK_APMIXED_APLL1]);
> > > 
> > > +
> > >          return 0;
> > >   }
> > 
> > Hi Nicolas,
> > 
> > If I understand correctly, APLL1 should be the parent clock of
> > AUD_A1SYS_HP and AUD_A1SYS, so it should be enabled automatically
> > by
> > CCF.
> > 
> > I'm not sure why you resolved the hang issue after enabling APLL1.
> > Could you share more details about the solution?
> > 
> 
> Hmm. Now I see what's happening here...
> 
> Nicolas, Trevor,
> 
> Possible parents for top_a1sys_hp are:
>   - clk26m
>   - apll1_d4
> 
> ...what's happening here most probably is that after the clock gets
> disabled as
> unused, it gets parented to clk26m by default as that is parent index
> 0... and
> something else in AFE needs APLL1 to feed a clock to .. something ..
> to allow
> register access.
> 
> Trevor, do you know why is this IP unaccessible when A1SYS is
> parented to clk26m?

Hi Angelo,

As far as I know, it should work even though the clock is parented to
clk26m.

Unfortunately, I have no idea about why APLL1 enabling can resolve the
hang issue. I'm also curious about how Nicolas found the solution to
resolve the problem.

From the description, it seems that the problem is related to register
r/w hang. If I remembered correctly, when the mtcmos of ADSP_INFRA is
ON, register r/w won't cause the cpu to hang. However, ADSP_INFRA has
been configured as ALWAYS_ON in the driver. I'm not sure if it doesn't
work correctly when the driver is configured as a module. Maybe Nicolas
can also check this.

Thanks,
Trevor

> 
> That might give Nicolas a definitive hint about how to resolve this
> issue.
> 
> Cheers,
> Angelo
> 
> > Thanks,
> > Trevor
> > 
> > > 
> > > @@ -594,6 +596,8 @@ int mt8188_afe_disable_reg_rw_clk(struct
> > > mtk_base_afe *afe)
> > >   {
> > >          struct mt8188_afe_private *afe_priv = afe-
> > > >platform_priv;
> > > 
> > > +       mt8188_afe_disable_clk(afe, afe_priv-
> > > > clk[MT8188_CLK_APMIXED_APLL1]);
> > > 
> > > +
> > >          mt8188_afe_disable_clk(afe, afe_priv-
> > > > clk[MT8188_CLK_AUD_A1SYS]);
> > > 
> > >          mt8188_afe_disable_clk(afe, afe_priv-
> > > > clk[MT8188_CLK_AUD_A1SYS_HP]);
> > > 
> > >          mt8188_afe_disable_clk(afe, afe_priv-
> > > > clk[MT8188_CLK_AUD_AFE]);
> > > 
> > > ---
> > > base-commit: b852e1e7a0389ed6168ef1d38eb0bad71a6b11e8
> > > change-id: 20241203-mt8188-afe-fix-hang-disabled-apll1-clk-
> > > b3c11782cbaf
> > > 
> > > Best regards,
> > > --
> > > Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > > 
> > > 
> 
>