Re: [PATCH v2 12/13] arm64: dts: imx8mm: add GPC node and power domains

[Adam Ford <aford173@gmail.com>]

On Tue, Mar 2, 2021 at 9:01 AM Frieder Schrempf
<frieder.schrempf@kontron.de> wrote:
>
> On 18.02.21 16:19, Adam Ford wrote:
> > On Thu, Feb 18, 2021 at 6:54 AM Adam Ford <aford173@gmail.com> wrote:
> >>
> >> On Thu, Jan 14, 2021 at 4:39 AM Frieder Schrempf
> >> <frieder.schrempf@kontron.de> wrote:
> >>>
> >>> On 09.12.20 16:26, Frieder Schrempf wrote:
> >>>> Hi Lucas,
> >>>>
> >>>> On 05.11.20 18:44, Lucas Stach wrote:
> >>>>> This adds the DT nodes to describe the power domains available on the
> >>>>> i.MX8MM. Things are a bit more complex compared to other GPCv2 power
> >>>>> domain setups, as there is now a hierarchy of domains where complete
> >>>>> subsystems (HSIO, GPU, DISPLAY) can be gated as a whole, but also
> >>>>> fine granular gating within those subsystems is possible.
> >>>>>
> >>>>> Note that this is still incomplete, as both VPU and DISP domains are
> >>>>> missing their reset clocks. Those aren't directly sourced from the CCM,
> >>>>> but have another level of clock gating in the BLKCTL of those domains,
> >>>>> which needs a separate driver.
> >>>>>
> >>>>> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> >>>>> ---
> >>>>>    arch/arm64/boot/dts/freescale/imx8mm.dtsi | 58 +++++++++++++++++++++++
> >>>>>    1 file changed, 58 insertions(+)
> >>>>>
> >>>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> >>>>> b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> >>>>> index b83f400def8b..c21901a8aea9 100644
> >>>>> --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> >>>>> +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> >>>>> @@ -4,6 +4,8 @@
> >>>>>     */
> >>>>>    #include <dt-bindings/clock/imx8mm-clock.h>
> >>>>> +#include <dt-bindings/power/imx8mm-power.h>
> >>>>> +#include <dt-bindings/reset/imx8mq-reset.h>
> >>>>>    #include <dt-bindings/gpio/gpio.h>
> >>>>>    #include <dt-bindings/input/input.h>
> >>>>>    #include <dt-bindings/interrupt-controller/arm-gic.h>
> >>>>> @@ -547,6 +549,62 @@
> >>>>>                    interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> >>>>>                    #reset-cells = <1>;
> >>>>>                };
> >>>>> +
> >>>>> +            gpc: gpc@303a0000 {
> >>>>> +                compatible = "fsl,imx8mm-gpc";
> >>>>> +                reg = <0x303a0000 0x10000>;
> >>>>> +                interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
> >>>>> +                interrupt-parent = <&gic>;
> >>>>> +                interrupt-controller;
> >>>>> +                #interrupt-cells = <3>;
> >>>>> +
> >>>>> +                pgc {
> >>>>> +                    #address-cells = <1>;
> >>>>> +                    #size-cells = <0>;
> >>>>> +
> >>>>> +                    pgc_hsiomix: power-domain@0 {
> >>>>> +                        #power-domain-cells = <0>;
> >>>>> +                        reg = <IMX8MM_POWER_DOMAIN_HSIOMIX>;
> >>>>> +                        clocks = <&clk IMX8MM_CLK_USB_BUS>;
> >>>>> +                    };
> >>>>> +
> >>>>> +                    pgc_pcie: power-domain@1 {
> >>>>> +                        #power-domain-cells = <0>;
> >>>>> +                        reg = <IMX8MM_POWER_DOMAIN_PCIE>;
> >>>>> +                        power-domains = <&pgc_hsiomix>;
> >>>>> +                    };
> >>>>> +
> >>>>> +                    pgc_otg1: power-domain@2 {
> >>>>> +                        #power-domain-cells = <0>;
> >>>>> +                        reg = <IMX8MM_POWER_DOMAIN_OTG1>;
> >>>>> +                        power-domains = <&pgc_hsiomix>;
> >>>>> +                    };
> >>>>> +
> >>>>> +                    pgc_otg2: power-domain@3 {
> >>>>> +                        #power-domain-cells = <0>;
> >>>>> +                        reg = <IMX8MM_POWER_DOMAIN_OTG2>;
> >>>>> +                        power-domains = <&pgc_hsiomix>;
> >>>>> +                    };
> >>>>
> >>>> I'm currently doing some testing on top of v5.10-rc with GPC, BLK-CTL,
> >>>> DSIM, etc. I noticed that as soon as I add the nodes above for HSIO/OTG
> >>>> (even without referencing them elsewhere) my system freezes on
> >>>> suspend/resume.
> >>>>
> >>>> The same problem exists when I remove the power domains for HSIO/USB and
> >>>> add the ones for DISPMIX and DSI to test Marek's work on BLK-CTL.
> >>>>
> >>>> I'm not really sure at what point exactly the system freezes but this is
> >>>> what I see (no_console_suspend is set) and the system does not wake up
> >>>> anymore:
> >>>>
> >>>> echo mem > /sys/power/state
> >>>> [   13.888711] PM: suspend entry (deep)
> >>>> [   13.892429] Filesystems sync: 0.000 seconds
> >>>> [   13.907231] Freezing user space processes ... (elapsed 0.031 seconds)
> >>>> done.
> >>>> [   13.945407] OOM killer disabled.
> >>>> [   13.948642] Freezing remaining freezable tasks ... (elapsed 0.001
> >>>> seconds) done.
> >>>> [   13.957216] printk: Suspending console(s) (use no_console_suspend to
> >>>> debug)
> >>>
> >>> It seems like I failed to set no_console_suspend correctly. Here is a
> >>> proper log with kernel 5.10.6. The system wakes up, but stalls.
> >>>
> >>> Can you reproduce this on your system?
> >>>
> >> [snip]
> >>
> >> Frieder / Lucas,
> >>
> >> I was able to get similar behavior on the Nano.  I rebased Lucas'
> >> patch on the 5.11 kernel, and applied the corresponding patches to my
> >> Nano board.  It works fine until the system sleeps,  but after it
> >> wakes, even the heartbeat LED stops beating.  I don't know if there is
> >> a conflict between TF-A and the gpc driver in there, or if the gpcv2
> >> driver needs to do something differently to wake the system from
> >> sleep.
> >>
> >> # echo mem > /sys/power/state
> >> [ 3754.346162] PM: suspend entry (deep)
> >> [ 3754.349872] Filesystems sync: 0.000 seconds
> >> [ 3754.387641] Freezing user space processes ... (elapsed 0.001 seconds) done.
> >> [ 3754.395890] OOM killer disabled.
> >> [ 3754.399141] Freezing remaining freezable tasks ... (elapsed 0.001
> >> seconds) done.
> >> [ 3754.407674] printk: Suspending console(s) (use no_console_suspend to debug)
> >> [ 3754.992015] Disabling non-boot CPUs ...
> >> [ 3755.027902] i.mx8mm_thermal 30260000.tmu: failed to register
> >> thermal zone sensor[0]: -517
> >> [ 3755.036317] OOM killer enabled.
> >> [ 3755.039467] Restarting tasks ... done.
> >> [ 3755.050669] PM: suspend exit
> >> root@imx8mmevk:~#
> >>
> >> Then it hangs.
> >
> > I did some additional digging on both the Mini and Nano.  Both of
> > their reference manuals have example code which enables and disables
> > CM_CCGR(69), hsio_bus clock when enabling/disabling the power domains.
> > It looks like the offset for CCM_CCGR69 is 0x4450.  This clock appears
> > to be enabled by default at boot, however, there doesn't appear to be
> > a clock entry for this.
> >
> > I tried to create one based on the 8MP that looks like:
> >      hws[IMX8MN_CLK_HSIO_ROOT] = imx_clk_hw_gate4("hsio_root_clk",
> > "ipg_root", base + 0x4450, 0);
> >
> > I then added the hsio clock to the clock table and reference it from
> > the power-domain node, but at boot it appears to hang at boot, so I am
> > not sure I did it correctly.
> >
> >  From what I can tell, neither the NXP kernel nor their ATF show a
> > reference to CCGR(69),  so I also wonder if the TRM for either Mini or
> > Nano is correct.
>
> I really can't tell if the missing HSIO root clock gate is a problem or
> not. It's listed in the RM in table 5-9 as "SIM_HSIO". And it's listed
> in table 6-7 ("CCGR setting by ROM") as "Bus clock. Do not gate off."

I didn't notice the 'Do note gate off' comment, so I'm guessing its
better to leave it alone if it's on my default and shouldn't be turned
off.  It does trouble me that the Example code in the backs of the TRM
reference it if we're not really supposed to do that.

>
> For some reason it's **not** listed in table 5-2 ("System Clocks and
> Gating)", where I expected to find the clock root.
>
> We also still have issues with the kernel locking up early in the boot
> using the GPC power domain driver, but only with some of our boards. I
> don't know if this is related to the lockup after resume.
>
> I also noticed that when I tried to use USB without the power domain
> support in the GPC driver, it worked with the mainline TF-A, but it
> locked up with the NXP TF-A on USB probing.

Looking at the NXP TF-A, they don't appear to go through the effort of
shutting down the USB.

    pu_domain_status &= ~(1 << domain_id);
    if (domain_id == OTG1 || domain_id == OTG2)
        return;

Based on that, I attempted to modify Lucas' power-domain code to add
an additional flag which could determine whether or not we want to
shut it down or not.  Using that additional flag, I was able to get
the system to wake from sleep without hanging, but USB wasn't
functional after waking.

adam

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel