| Message ID | 20190705045612.27665-5-Anson.Huang@nxp.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Eduardo Valentin |
| Headers | show |
| Series | [1/6] thermal: qoriq: Use devm_platform_ioremap_resource() instead of of_iomap() | expand |
On 19-07-05 12:56:11, Anson.Huang@nxp.com wrote: > From: Anson Huang <Anson.Huang@nxp.com> > > IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the driver > should manage this clock, so no need to have CLK_IS_CRITICAL flag > set. > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com> Reviewed-by: Abel Vesa <abel.vesa@nxp.com> > --- > drivers/clk/imx/clk-imx8mq.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c > index d407a07..91de69a 100644 > --- a/drivers/clk/imx/clk-imx8mq.c > +++ b/drivers/clk/imx/clk-imx8mq.c > @@ -539,7 +539,7 @@ static int imx8mq_clocks_probe(struct platform_device *pdev) > clks[IMX8MQ_CLK_DISP_AXI_ROOT] = imx_clk_gate2_shared2("disp_axi_root_clk", "disp_axi", base + 0x45d0, 0, &share_count_dcss); > clks[IMX8MQ_CLK_DISP_APB_ROOT] = imx_clk_gate2_shared2("disp_apb_root_clk", "disp_apb", base + 0x45d0, 0, &share_count_dcss); > clks[IMX8MQ_CLK_DISP_RTRM_ROOT] = imx_clk_gate2_shared2("disp_rtrm_root_clk", "disp_rtrm", base + 0x45d0, 0, &share_count_dcss); > - clks[IMX8MQ_CLK_TMU_ROOT] = imx_clk_gate4_flags("tmu_root_clk", "ipg_root", base + 0x4620, 0, CLK_IS_CRITICAL); > + clks[IMX8MQ_CLK_TMU_ROOT] = imx_clk_gate4("tmu_root_clk", "ipg_root", base + 0x4620, 0); > clks[IMX8MQ_CLK_VPU_DEC_ROOT] = imx_clk_gate2_flags("vpu_dec_root_clk", "vpu_bus", base + 0x4630, 0, CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE); > clks[IMX8MQ_CLK_CSI1_ROOT] = imx_clk_gate4("csi1_root_clk", "csi1_core", base + 0x4650, 0); > clks[IMX8MQ_CLK_CSI2_ROOT] = imx_clk_gate4("csi2_root_clk", "csi2_core", base + 0x4660, 0); > -- > 2.7.4 >
Quoting Anson.Huang@nxp.com (2019-07-04 21:56:11) > From: Anson Huang <Anson.Huang@nxp.com> > > IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the driver > should manage this clock, so no need to have CLK_IS_CRITICAL flag > set. > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com> > --- Acked-by: Stephen Boyd <sboyd@kernel.org>
On Fri, Jul 05, 2019 at 12:56:11PM +0800, Anson.Huang@nxp.com wrote: > From: Anson Huang <Anson.Huang@nxp.com> > > IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the driver > should manage this clock, so no need to have CLK_IS_CRITICAL flag > set. > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com> Applied, thanks.
Hi all,
latest linux-next hangs at boot.
commit fde50b96be821ac9673a7e00847cc4605bd88f34 (HEAD -> master, tag:
next-20190726, origin/master, origin/HEAD)
Author: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Fri Jul 26 15:18:02 2019 +1000
Add linux-next specific files for 20190726
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
I know this is crazy but reverting commit:
commit 431bdd1df48ee2896ea9980d9153e3aeaf0c81ef (refs/bisect/bad)
Author: Anson Huang <Anson.Huang@nxp.com>
Date: Fri Jul 5 12:56:11 2019 +0800
clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT
IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the driver
should manage this clock, so no need to have CLK_IS_CRITICAL flag
set.
makes the boot work again.
Any idea?
On Fri, Jul 5, 2019 at 8:07 AM <Anson.Huang@nxp.com> wrote:
>
> From: Anson Huang <Anson.Huang@nxp.com>
>
> IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the driver
> should manage this clock, so no need to have CLK_IS_CRITICAL flag
> set.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> drivers/clk/imx/clk-imx8mq.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
> index d407a07..91de69a 100644
> --- a/drivers/clk/imx/clk-imx8mq.c
> +++ b/drivers/clk/imx/clk-imx8mq.c
> @@ -539,7 +539,7 @@ static int imx8mq_clocks_probe(struct platform_device *pdev)
> clks[IMX8MQ_CLK_DISP_AXI_ROOT] = imx_clk_gate2_shared2("disp_axi_root_clk", "disp_axi", base + 0x45d0, 0, &share_count_dcss);
> clks[IMX8MQ_CLK_DISP_APB_ROOT] = imx_clk_gate2_shared2("disp_apb_root_clk", "disp_apb", base + 0x45d0, 0, &share_count_dcss);
> clks[IMX8MQ_CLK_DISP_RTRM_ROOT] = imx_clk_gate2_shared2("disp_rtrm_root_clk", "disp_rtrm", base + 0x45d0, 0, &share_count_dcss);
> - clks[IMX8MQ_CLK_TMU_ROOT] = imx_clk_gate4_flags("tmu_root_clk", "ipg_root", base + 0x4620, 0, CLK_IS_CRITICAL);
> + clks[IMX8MQ_CLK_TMU_ROOT] = imx_clk_gate4("tmu_root_clk", "ipg_root", base + 0x4620, 0);
> clks[IMX8MQ_CLK_VPU_DEC_ROOT] = imx_clk_gate2_flags("vpu_dec_root_clk", "vpu_bus", base + 0x4630, 0, CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE);
> clks[IMX8MQ_CLK_CSI1_ROOT] = imx_clk_gate4("csi1_root_clk", "csi1_core", base + 0x4650, 0);
> clks[IMX8MQ_CLK_CSI2_ROOT] = imx_clk_gate4("csi2_root_clk", "csi2_core", base + 0x4660, 0);
> --
> 2.7.4
>
Hi, Daniel > Subject: Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for > IMX8MQ_CLK_TMU_ROOT > > Hi all, > > latest linux-next hangs at boot. > > commit fde50b96be821ac9673a7e00847cc4605bd88f34 (HEAD -> master, tag: > next-20190726, origin/master, origin/HEAD) > Author: Stephen Rothwell <sfr@canb.auug.org.au> > Date: Fri Jul 26 15:18:02 2019 +1000 > > Add linux-next specific files for 20190726 > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > > > I know this is crazy but reverting commit: > > commit 431bdd1df48ee2896ea9980d9153e3aeaf0c81ef (refs/bisect/bad) > Author: Anson Huang <Anson.Huang@nxp.com> > Date: Fri Jul 5 12:56:11 2019 +0800 > > clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT > > IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the driver > should manage this clock, so no need to have CLK_IS_CRITICAL flag > set. > > > > makes the boot work again. > > Any idea? I just found if disabling SDMA1, then kernel can boot up, it does NOT make sense TMU clock is related to SDMA1, I will check with design and get back to you soon. Anson
On Sat, Jul 27, 2019 at 9:19 AM Anson Huang <anson.huang@nxp.com> wrote: > > Hi, Daniel > > > Subject: Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for > > IMX8MQ_CLK_TMU_ROOT > > > > Hi all, > > > > latest linux-next hangs at boot. > > > > commit fde50b96be821ac9673a7e00847cc4605bd88f34 (HEAD -> master, tag: > > next-20190726, origin/master, origin/HEAD) > > Author: Stephen Rothwell <sfr@canb.auug.org.au> > > Date: Fri Jul 26 15:18:02 2019 +1000 > > > > Add linux-next specific files for 20190726 > > > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > > > > > > I know this is crazy but reverting commit: > > > > commit 431bdd1df48ee2896ea9980d9153e3aeaf0c81ef (refs/bisect/bad) > > Author: Anson Huang <Anson.Huang@nxp.com> > > Date: Fri Jul 5 12:56:11 2019 +0800 > > > > clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT > > > > IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the driver > > should manage this clock, so no need to have CLK_IS_CRITICAL flag > > set. > > > > > > > > makes the boot work again. > > > > Any idea? > > I just found if disabling SDMA1, then kernel can boot up, it does NOT make sense > TMU clock is related to SDMA1, I will check with design and get back to you soon. > Hi Anson, Applying Abel's patch: commit 8816c47db6a82f55bb4d64f62fd9dd3af680f0e4 (HEAD -> master) Author: Abel Vesa <abel.vesa@nxp.com> Date: Tue Jun 25 12:01:56 2019 +0300 clk: imx8mq: Mark AHB clock as critical Keep the AHB clock always on since there is no driver to control it and all the other clocks that use it as parent rely on it being always enabled. The kernel boots up again. It make some sense. I don't understand though why having IMX8MQ_CLK_TMU_ROOT as critical also "unhangs" the kernel. thanks, Daniel.
On 19-07-27 09:33:10, Daniel Baluta wrote: > On Sat, Jul 27, 2019 at 9:19 AM Anson Huang <anson.huang@nxp.com> wrote: > > > > Hi, Daniel > > > > > Subject: Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag for > > > IMX8MQ_CLK_TMU_ROOT > > > > > > Hi all, > > > > > > latest linux-next hangs at boot. > > > > > > commit fde50b96be821ac9673a7e00847cc4605bd88f34 (HEAD -> master, tag: > > > next-20190726, origin/master, origin/HEAD) > > > Author: Stephen Rothwell <sfr@canb.auug.org.au> > > > Date: Fri Jul 26 15:18:02 2019 +1000 > > > > > > Add linux-next specific files for 20190726 > > > > > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > > > > > > > > > I know this is crazy but reverting commit: > > > > > > commit 431bdd1df48ee2896ea9980d9153e3aeaf0c81ef (refs/bisect/bad) > > > Author: Anson Huang <Anson.Huang@nxp.com> > > > Date: Fri Jul 5 12:56:11 2019 +0800 > > > > > > clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT > > > > > > IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the driver > > > should manage this clock, so no need to have CLK_IS_CRITICAL flag > > > set. > > > > > > > > > > > > makes the boot work again. > > > > > > Any idea? > > > > I just found if disabling SDMA1, then kernel can boot up, it does NOT make sense > > TMU clock is related to SDMA1, I will check with design and get back to you soon. > > > > Hi Anson, > > Applying Abel's patch: > > commit 8816c47db6a82f55bb4d64f62fd9dd3af680f0e4 (HEAD -> master) > Author: Abel Vesa <abel.vesa@nxp.com> > Date: Tue Jun 25 12:01:56 2019 +0300 > > clk: imx8mq: Mark AHB clock as critical > > Keep the AHB clock always on since there is no driver to control it and > all the other clocks that use it as parent rely on it being always enabled. > > > > The kernel boots up again. > > It make some sense. I don't understand though why having > IMX8MQ_CLK_TMU_ROOT as critical also "unhangs" the kernel. > OK, so this is how it works. By removing the critical flag from TMU, the AHB doesn't stay always on. With my patch the AHB is marked as critical and therefore stays on. The sdma1_clk has as parent the ipg_root which in turn has as parent the ahb clock. And I think what happens is some read from the sdma registers hangs because, for whatever reason, enabling the sdma1_clk doesn't propagate to enable the ahb clock. I might be wrong though. > thanks, > Daniel.
Hi Daniel, On Sat, Jul 27, 2019 at 01:26:50AM +0300, Daniel Baluta wrote: > Hi all, > > latest linux-next hangs at boot. > > commit fde50b96be821ac9673a7e00847cc4605bd88f34 (HEAD -> master, tag: > next-20190726, origin/master, origin/HEAD) > Author: Stephen Rothwell <sfr@canb.auug.org.au> > Date: Fri Jul 26 15:18:02 2019 +1000 > > Add linux-next specific files for 20190726 > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > > > I know this is crazy but reverting commit: > > commit 431bdd1df48ee2896ea9980d9153e3aeaf0c81ef (refs/bisect/bad) > Author: Anson Huang <Anson.Huang@nxp.com> > Date: Fri Jul 5 12:56:11 2019 +0800 > > clk: imx8mq: Remove CLK_IS_CRITICAL flag for IMX8MQ_CLK_TMU_ROOT > > IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the driver > should manage this clock, so no need to have CLK_IS_CRITICAL flag > set. > > > > makes the boot work again. I noticed a boot hang yesterday on next-20190726 when loading the qoriq_thermal which I worked around by blacklisting it. The fsl,imx8mq-tmu node specifies a clock (IMX8MQ_CLK_TMU_ROOT) but does not seem to enable, shouldn't it do so? Cheers, -- Guido
Hi Guido, On Sat, Jul 27, 2019 at 3:26 PM Guido Günther <agx@sigxcpu.org> wrote: > I noticed a boot hang yesterday on next-20190726 when loading the > qoriq_thermal which I worked around by blacklisting it. The > fsl,imx8mq-tmu node specifies a clock (IMX8MQ_CLK_TMU_ROOT) but does not > seem to enable, shouldn't it do so? Yes, I think you are right. I don't have access to a imx8mq board at the moment, but something like below would probably help: http://code.bulix.org/pd88jp-812381 If it helps, I can send it as a formal patch. Regards, Fabio Estevam
Hi Fabio, On Sat, Jul 27, 2019 at 05:17:50PM -0300, Fabio Estevam wrote: > Hi Guido, > > On Sat, Jul 27, 2019 at 3:26 PM Guido Günther <agx@sigxcpu.org> wrote: > > > I noticed a boot hang yesterday on next-20190726 when loading the > > qoriq_thermal which I worked around by blacklisting it. The > > fsl,imx8mq-tmu node specifies a clock (IMX8MQ_CLK_TMU_ROOT) but does not > > seem to enable, shouldn't it do so? > > Yes, I think you are right. > > I don't have access to a imx8mq board at the moment, but something > like below would probably help: > http://code.bulix.org/pd88jp-812381 > > If it helps, I can send it as a formal patch. Yes, this fixes it for me, thanks! -- Guido
Hi Guido,
On Sun, Jul 28, 2019 at 4:59 AM Guido Günther <agx@sigxcpu.org> wrote:
> Yes, this fixes it for me, thanks!
Thanks for testing it.
I will send a formal patch tomorrow.
Regards,
Fabio Estevam
Hi, Abel/Daniel > On 19-07-27 09:33:10, Daniel Baluta wrote: > > On Sat, Jul 27, 2019 at 9:19 AM Anson Huang <anson.huang@nxp.com> > wrote: > > > > > > Hi, Daniel > > > > > > > Subject: Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag > > > > for IMX8MQ_CLK_TMU_ROOT > > > > > > > > Hi all, > > > > > > > > latest linux-next hangs at boot. > > > > > > > > commit fde50b96be821ac9673a7e00847cc4605bd88f34 (HEAD -> > master, tag: > > > > next-20190726, origin/master, origin/HEAD) > > > > Author: Stephen Rothwell <sfr@canb.auug.org.au> > > > > Date: Fri Jul 26 15:18:02 2019 +1000 > > > > > > > > Add linux-next specific files for 20190726 > > > > > > > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > > > > > > > > > > > > I know this is crazy but reverting commit: > > > > > > > > commit 431bdd1df48ee2896ea9980d9153e3aeaf0c81ef > (refs/bisect/bad) > > > > Author: Anson Huang <Anson.Huang@nxp.com> > > > > Date: Fri Jul 5 12:56:11 2019 +0800 > > > > > > > > clk: imx8mq: Remove CLK_IS_CRITICAL flag for > > > > IMX8MQ_CLK_TMU_ROOT > > > > > > > > IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the > driver > > > > should manage this clock, so no need to have CLK_IS_CRITICAL flag > > > > set. > > > > > > > > > > > > > > > > makes the boot work again. > > > > > > > > Any idea? > > > > > > I just found if disabling SDMA1, then kernel can boot up, it does > > > NOT make sense TMU clock is related to SDMA1, I will check with design > and get back to you soon. > > > > > > > Hi Anson, > > > > Applying Abel's patch: > > > > commit 8816c47db6a82f55bb4d64f62fd9dd3af680f0e4 (HEAD -> master) > > Author: Abel Vesa <abel.vesa@nxp.com> > > Date: Tue Jun 25 12:01:56 2019 +0300 > > > > clk: imx8mq: Mark AHB clock as critical > > > > Keep the AHB clock always on since there is no driver to control it and > > all the other clocks that use it as parent rely on it being always enabled. > > > > > > > > The kernel boots up again. > > > > It make some sense. I don't understand though why having > > IMX8MQ_CLK_TMU_ROOT as critical also "unhangs" the kernel. > > > > OK, so this is how it works. > > By removing the critical flag from TMU, the AHB doesn't stay always on. > With my patch the AHB is marked as critical and therefore stays on. > > The sdma1_clk has as parent the ipg_root which in turn has as parent the > ahb clock. And I think what happens is some read from the sdma registers > hangs because, for whatever reason, enabling the sdma1_clk doesn't > propagate to enable the ahb clock. I might be wrong though. > I can explain why Abel's patch can fix this issue, the AHB clock is a MUST always ON for system bus, while it does NOT have CLK_IS_CRITICAL flag set for now, when SDMA1 is probed, it will enable its clock, and the clk path is sdma1_clk->ipg_root->ahb, after SDMA1 probed done, it will disable its clock since no one use it, and it will trigger the ahb clock to be OFF, as its usecount is added by 1 when probe and decreased by 1 after SDMA1 probe done, so usecount=0 will trigger AHB clock to be OFF. So I think the best solution should be applying Abel's patch as you mentioned upper, the TMU clock patch is NOT the root cause, it just triggers this issue accidently☹ But I saw Abel's AHB patch is still under discussion, so I think we need to speed it up and make kernel boot up work for development. AHB/IPG are always critical for i.MX SoCs. Thanks, Anson.
On Mon, Jul 29, 2019 at 4:29 AM Anson Huang <anson.huang@nxp.com> wrote: > > Hi, Abel/Daniel > > > On 19-07-27 09:33:10, Daniel Baluta wrote: > > > On Sat, Jul 27, 2019 at 9:19 AM Anson Huang <anson.huang@nxp.com> > > wrote: > > > > > > > > Hi, Daniel > > > > > > > > > Subject: Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL flag > > > > > for IMX8MQ_CLK_TMU_ROOT > > > > > > > > > > Hi all, > > > > > > > > > > latest linux-next hangs at boot. > > > > > > > > > > commit fde50b96be821ac9673a7e00847cc4605bd88f34 (HEAD -> > > master, tag: > > > > > next-20190726, origin/master, origin/HEAD) > > > > > Author: Stephen Rothwell <sfr@canb.auug.org.au> > > > > > Date: Fri Jul 26 15:18:02 2019 +1000 > > > > > > > > > > Add linux-next specific files for 20190726 > > > > > > > > > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > > > > > > > > > > > > > > > I know this is crazy but reverting commit: > > > > > > > > > > commit 431bdd1df48ee2896ea9980d9153e3aeaf0c81ef > > (refs/bisect/bad) > > > > > Author: Anson Huang <Anson.Huang@nxp.com> > > > > > Date: Fri Jul 5 12:56:11 2019 +0800 > > > > > > > > > > clk: imx8mq: Remove CLK_IS_CRITICAL flag for > > > > > IMX8MQ_CLK_TMU_ROOT > > > > > > > > > > IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the > > driver > > > > > should manage this clock, so no need to have CLK_IS_CRITICAL flag > > > > > set. > > > > > > > > > > > > > > > > > > > > makes the boot work again. > > > > > > > > > > Any idea? > > > > > > > > I just found if disabling SDMA1, then kernel can boot up, it does > > > > NOT make sense TMU clock is related to SDMA1, I will check with design > > and get back to you soon. > > > > > > > > > > Hi Anson, > > > > > > Applying Abel's patch: > > > > > > commit 8816c47db6a82f55bb4d64f62fd9dd3af680f0e4 (HEAD -> master) > > > Author: Abel Vesa <abel.vesa@nxp.com> > > > Date: Tue Jun 25 12:01:56 2019 +0300 > > > > > > clk: imx8mq: Mark AHB clock as critical > > > > > > Keep the AHB clock always on since there is no driver to control it and > > > all the other clocks that use it as parent rely on it being always enabled. > > > > > > > > > > > > The kernel boots up again. > > > > > > It make some sense. I don't understand though why having > > > IMX8MQ_CLK_TMU_ROOT as critical also "unhangs" the kernel. > > > > > > > OK, so this is how it works. > > > > By removing the critical flag from TMU, the AHB doesn't stay always on. > > With my patch the AHB is marked as critical and therefore stays on. > > > > The sdma1_clk has as parent the ipg_root which in turn has as parent the > > ahb clock. And I think what happens is some read from the sdma registers > > hangs because, for whatever reason, enabling the sdma1_clk doesn't > > propagate to enable the ahb clock. I might be wrong though. > > > > I can explain why Abel's patch can fix this issue, the AHB clock is a MUST > always ON for system bus, while it does NOT have CLK_IS_CRITICAL flag set for now, > when SDMA1 is probed, it will enable its clock, and the clk path is sdma1_clk->ipg_root->ahb, > after SDMA1 probed done, it will disable its clock since no one use it, and it will trigger > the ahb clock to be OFF, as its usecount is added by 1 when probe and decreased by 1 after > SDMA1 probe done, so usecount=0 will trigger AHB clock to be OFF. > > So I think the best solution should be applying Abel's patch as you mentioned upper, the TMU > clock patch is NOT the root cause, it just triggers this issue accidently☹ > > But I saw Abel's AHB patch is still under discussion, so I think we need to speed it up and make > kernel boot up work for development. AHB/IPG are always critical for i.MX SoCs. Thanks Anson, Your explanation makes a lot of sense. We will take care today of Abel's patch. What do you think about Fabio's patch? I also think this is a valid patch: http://code.bulix.org/pd88jp-812381
Hi, Daniel > On Mon, Jul 29, 2019 at 4:29 AM Anson Huang <anson.huang@nxp.com> > wrote: > > > > Hi, Abel/Daniel > > > > > On 19-07-27 09:33:10, Daniel Baluta wrote: > > > > On Sat, Jul 27, 2019 at 9:19 AM Anson Huang <anson.huang@nxp.com> > > > wrote: > > > > > > > > > > Hi, Daniel > > > > > > > > > > > Subject: Re: [PATCH 5/6] clk: imx8mq: Remove CLK_IS_CRITICAL > > > > > > flag for IMX8MQ_CLK_TMU_ROOT > > > > > > > > > > > > Hi all, > > > > > > > > > > > > latest linux-next hangs at boot. > > > > > > > > > > > > commit fde50b96be821ac9673a7e00847cc4605bd88f34 (HEAD -> > > > master, tag: > > > > > > next-20190726, origin/master, origin/HEAD) > > > > > > Author: Stephen Rothwell <sfr@canb.auug.org.au> > > > > > > Date: Fri Jul 26 15:18:02 2019 +1000 > > > > > > > > > > > > Add linux-next specific files for 20190726 > > > > > > > > > > > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > > > > > > > > > > > > > > > > > > I know this is crazy but reverting commit: > > > > > > > > > > > > commit 431bdd1df48ee2896ea9980d9153e3aeaf0c81ef > > > (refs/bisect/bad) > > > > > > Author: Anson Huang <Anson.Huang@nxp.com> > > > > > > Date: Fri Jul 5 12:56:11 2019 +0800 > > > > > > > > > > > > clk: imx8mq: Remove CLK_IS_CRITICAL flag for > > > > > > IMX8MQ_CLK_TMU_ROOT > > > > > > > > > > > > IMX8MQ_CLK_TMU_ROOT is ONLY used for thermal module, the > > > driver > > > > > > should manage this clock, so no need to have CLK_IS_CRITICAL > flag > > > > > > set. > > > > > > > > > > > > > > > > > > > > > > > > makes the boot work again. > > > > > > > > > > > > Any idea? > > > > > > > > > > I just found if disabling SDMA1, then kernel can boot up, it > > > > > does NOT make sense TMU clock is related to SDMA1, I will check > > > > > with design > > > and get back to you soon. > > > > > > > > > > > > > Hi Anson, > > > > > > > > Applying Abel's patch: > > > > > > > > commit 8816c47db6a82f55bb4d64f62fd9dd3af680f0e4 (HEAD -> master) > > > > Author: Abel Vesa <abel.vesa@nxp.com> > > > > Date: Tue Jun 25 12:01:56 2019 +0300 > > > > > > > > clk: imx8mq: Mark AHB clock as critical > > > > > > > > Keep the AHB clock always on since there is no driver to control it and > > > > all the other clocks that use it as parent rely on it being always > enabled. > > > > > > > > > > > > > > > > The kernel boots up again. > > > > > > > > It make some sense. I don't understand though why having > > > > IMX8MQ_CLK_TMU_ROOT as critical also "unhangs" the kernel. > > > > > > > > > > OK, so this is how it works. > > > > > > By removing the critical flag from TMU, the AHB doesn't stay always on. > > > With my patch the AHB is marked as critical and therefore stays on. > > > > > > The sdma1_clk has as parent the ipg_root which in turn has as parent > > > the ahb clock. And I think what happens is some read from the sdma > > > registers hangs because, for whatever reason, enabling the sdma1_clk > > > doesn't propagate to enable the ahb clock. I might be wrong though. > > > > > > > I can explain why Abel's patch can fix this issue, the AHB clock is a > > MUST always ON for system bus, while it does NOT have CLK_IS_CRITICAL > > flag set for now, when SDMA1 is probed, it will enable its clock, and > > the clk path is sdma1_clk->ipg_root->ahb, after SDMA1 probed done, it > > will disable its clock since no one use it, and it will trigger the > > ahb clock to be OFF, as its usecount is added by 1 when probe and > > decreased by 1 after > > SDMA1 probe done, so usecount=0 will trigger AHB clock to be OFF. > > > > So I think the best solution should be applying Abel's patch as you > > mentioned upper, the TMU clock patch is NOT the root cause, it just > > triggers this issue accidently☹ > > > > But I saw Abel's AHB patch is still under discussion, so I think we > > need to speed it up and make kernel boot up work for development. > AHB/IPG are always critical for i.MX SoCs. > > Thanks Anson, > > Your explanation makes a lot of sense. We will take care today of Abel's > patch. > What do you think about Fabio's patch? I also think this is a valid patch: > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fcode.b > ulix.org%2Fpd88jp- > 812381&data=02%7C01%7Canson.huang%40nxp.com%7C23b4c21e3cbc > 4fcf2a3c08d713f131a8%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0% > 7C636999798949622961&sdata=Jx9B40rJKpQvakOCfx%2B3v80NTxPqUw > D4pdGojQxVIoY%3D&reserved=0 Hmm, when did Fabio sent out this patch? I can NOT find it... I also have a patch in this series (#4/6) doing same thing on July 5th... https://patchwork.kernel.org/patch/11032783/ Thanks. Anson
<snip> > > Your explanation makes a lot of sense. We will take care today of Abel's > > patch. > > What do you think about Fabio's patch? I also think this is a valid patch: > > <snip> > > Hmm, when did Fabio sent out this patch? I can NOT find it... > I also have a patch in this series (#4/6) doing same thing on July 5th... > > https://patchwork.kernel.org/patch/11032783/ He didn't send the patch yet. It was just a request for test here: http://code.bulix.org/pd88jp-812381 But, now I see is exactly like your patch here:
On Mon, Jul 29, 2019 at 10:20 AM Daniel Baluta <daniel.baluta@gmail.com> wrote: > > <snip> > > > Your explanation makes a lot of sense. We will take care today of Abel's > > > patch. > > > What do you think about Fabio's patch? I also think this is a valid patch: > > > > <snip> > > > > > Hmm, when did Fabio sent out this patch? I can NOT find it... > > I also have a patch in this series (#4/6) doing same thing on July 5th... > > > > https://patchwork.kernel.org/patch/11032783/ > > He didn't send the patch yet. It was just a request for test here: > > http://code.bulix.org/pd88jp-812381 > > But, now I see is exactly like your patch here: ... pressed send to early. https://lkml.org/lkml/2019/7/5/19 We are all set then. Thanks Anson for clarifications!
Hi, Daniel > On Mon, Jul 29, 2019 at 10:20 AM Daniel Baluta <daniel.baluta@gmail.com> > wrote: > > > > <snip> > > > > Your explanation makes a lot of sense. We will take care today of > > > > Abel's patch. > > > > What do you think about Fabio's patch? I also think this is a valid patch: > > > > > > <snip> > > > > > > > > Hmm, when did Fabio sent out this patch? I can NOT find it... > > > I also have a patch in this series (#4/6) doing same thing on July 5th... > > > > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpa > > > > tchwork.kernel.org%2Fpatch%2F11032783%2F&data=02%7C01%7Canso > n.hu > > > > ang%40nxp.com%7C048d8695b3dc4ceef6bd08d713f55e8a%7C686ea1d3bc2 > b4c6fa > > > > 92cd99c5c301635%7C0%7C0%7C636999816880118674&sdata=1HIMQ0l > iKpEFS > > > 6P2WSG%2FH9evspxIdxAvFmaklH1woDk%3D&reserved=0 > > > > He didn't send the patch yet. It was just a request for test here: > > > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fcode. > > bulix.org%2Fpd88jp- > 812381&data=02%7C01%7Canson.huang%40nxp.com%7C0 > > > 48d8695b3dc4ceef6bd08d713f55e8a%7C686ea1d3bc2b4c6fa92cd99c5c30163 > 5%7C0 > > %7C0%7C636999816880118674&sdata=p70mgCDucCgLJ8TTRMn3a%2 > Fk68FXGQeiR > > FR0fVSV7Jlo%3D&reserved=0 > > > > But, now I see is exactly like your patch here: > > ... pressed send to early. > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.o > rg%2Flkml%2F2019%2F7%2F5%2F19&data=02%7C01%7Canson.huang% > 40nxp.com%7C048d8695b3dc4ceef6bd08d713f55e8a%7C686ea1d3bc2b4c6f > a92cd99c5c301635%7C0%7C0%7C636999816880118674&sdata=ciWj8y > WPvkIZ5ni%2BohZuaqAXf9Pb2FCWhp9GQhpMwAY%3D&reserved=0 > > We are all set then. Thanks Anson for clarifications! Thanks, so we are all clear about this issue, need to wait thermal maintainer to review the rest patch in this series, but I did NOT receive any response from thermal sub-system maintainer for really long time, NOT sure when the thermal patches can be accepted. Anson
On Mon, Jul 29, 2019 at 10:49 AM Anson Huang <anson.huang@nxp.com> wrote: > > We are all set then. Thanks Anson for clarifications! > > Thanks, so we are all clear about this issue, need to wait thermal maintainer to review > the rest patch in this series, but I did NOT receive any response from thermal sub-system > maintainer for really long time, NOT sure when the thermal patches can be accepted. This is really unfortunate. I think it is safe to do a RESEND of the patches as it has been at least 3 weeks since your first send them. Pick any reviewed-by you got and do a resend.
diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c index d407a07..91de69a 100644 --- a/drivers/clk/imx/clk-imx8mq.c +++ b/drivers/clk/imx/clk-imx8mq.c @@ -539,7 +539,7 @@ static int imx8mq_clocks_probe(struct platform_device *pdev) clks[IMX8MQ_CLK_DISP_AXI_ROOT] = imx_clk_gate2_shared2("disp_axi_root_clk", "disp_axi", base + 0x45d0, 0, &share_count_dcss); clks[IMX8MQ_CLK_DISP_APB_ROOT] = imx_clk_gate2_shared2("disp_apb_root_clk", "disp_apb", base + 0x45d0, 0, &share_count_dcss); clks[IMX8MQ_CLK_DISP_RTRM_ROOT] = imx_clk_gate2_shared2("disp_rtrm_root_clk", "disp_rtrm", base + 0x45d0, 0, &share_count_dcss); - clks[IMX8MQ_CLK_TMU_ROOT] = imx_clk_gate4_flags("tmu_root_clk", "ipg_root", base + 0x4620, 0, CLK_IS_CRITICAL); + clks[IMX8MQ_CLK_TMU_ROOT] = imx_clk_gate4("tmu_root_clk", "ipg_root", base + 0x4620, 0); clks[IMX8MQ_CLK_VPU_DEC_ROOT] = imx_clk_gate2_flags("vpu_dec_root_clk", "vpu_bus", base + 0x4630, 0, CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE); clks[IMX8MQ_CLK_CSI1_ROOT] = imx_clk_gate4("csi1_root_clk", "csi1_core", base + 0x4650, 0); clks[IMX8MQ_CLK_CSI2_ROOT] = imx_clk_gate4("csi2_root_clk", "csi2_core", base + 0x4660, 0);