linux-kernel - Re: [PATCH v1] arm64: dts: imx8mm: fix missing pgc_vpu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 09 Apr 2024 16:36:01 +0200
From: Lucas Stach <l.stach@...gutronix.de>
To: Vitor Soares <ivitro@...il.com>, Rob Herring <robh@...nel.org>, 
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley
 <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,  Sascha Hauer
 <s.hauer@...gutronix.de>, Pengutronix Kernel Team <kernel@...gutronix.de>,
 Fabio Estevam <festevam@...il.com>
Cc: Vitor Soares <vitor.soares@...adex.com>, devicetree@...r.kernel.org, 
	imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org, 
	linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH v1] arm64: dts: imx8mm: fix missing pgc_vpu_* power
 domain parent

Am Dienstag, dem 09.04.2024 um 14:22 +0100 schrieb Vitor Soares:
> Hi Lucas,
> 
> Thanks for your feedback.
> 
> On Tue, 2024-04-09 at 11:13 +0200, Lucas Stach wrote:
> > Hi Vitor,
> > 
> > Am Dienstag, dem 09.04.2024 um 09:58 +0100 schrieb Vitor Soares:
> > > From: Vitor Soares <vitor.soares@...adex.com>
> > > 
> > > The pgc_vpu_* nodes miss the reference to the power domain parent,
> > > leading the system to hang during the resume.
> > > 
> > This change is not correct. The vpumix domain is controlled through
> > the
> > imx8mm-vpu-blk-ctrl and must not be directly triggered by the child
> > domains in order to guarantee proper power sequencing.
> > 
> > If the sequencing is incorrect for resume, it needs to be fixed in
> > the
> > blk-ctrl driver. I'll happily assist if you have any questions about
> > this intricate mix between GPC and blk-ctrl hardware/drivers.
>  
> I'm new into the topic, so I tried to follow same approach as in imx8mp
> DT.
> 
That's a good hint, the 8MP VPU GPC node additions missed my radar. The
direct dependency there between the GPC domains is equally wrong.

> I also checked the imx8mq DT and it only have one domain for the
> VPU in the GPC. It seem blk-ctrl also dependes on pgc_vpu_* to work
> properly.
> 
> The blk-ctrl driver hangs on imx8m_blk_ctrl_power_on() when access the
> ip registers for the soft reset. I tried to power-up the before the
> soft reset, but it didn't work.
> 
The runtime_pm_get_sync() at the start of that function should ensure
that bus GPC domain aka vpumix is powered up. Can you check if that is
happening?

Regards,
Lucas

> Do you have an idea how we can address this within blk-ctrl?
> 
> Best regards,
> Vitor
> 
> > 
> > Regards,
> > Lucas
> > 
> > > As these PU domains are nested inside the vpumix domain, let's
> > > reference
> > > it accordingly. After this change, the suspend/resume is working.
> > > 
> > > Cc: Lucas Stach <l.stach@...gutronix.de>
> > > Cc: <stable@...r.kernel.org>
> > > Closes:
> > > https://lore.kernel.org/all/fccbb040330a706a4f7b34875db1d896a0bf81c8.camel@gmail.com/
> > > Fixes: d39d4bb15310 ("arm64: dts: imx8mm: add GPC node")
> > > Signed-off-by: Vitor Soares <vitor.soares@...adex.com>
> > > ---
> > >  arch/arm64/boot/dts/freescale/imx8mm.dtsi | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > > b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > > index 8a1b42b94dce..97d0c6d23ad8 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > > @@ -739,16 +739,19 @@ pgc_vpumix: power-domain@6 {
> > >                                         pgc_vpu_g1: power-domain@7
> > > {
> > >                                                 #power-domain-cells
> > > = <0>;
> > >                                                 reg =
> > > <IMX8MM_POWER_DOMAIN_VPUG1>;
> > > +                                               power-domains =
> > > <&pgc_vpumix>;
> > >                                         };
> > >  
> > >                                         pgc_vpu_g2: power-domain@8
> > > {
> > >                                                 #power-domain-cells
> > > = <0>;
> > >                                                 reg =
> > > <IMX8MM_POWER_DOMAIN_VPUG2>;
> > > +                                               power-domains =
> > > <&pgc_vpumix>;
> > >                                         };
> > >  
> > >                                         pgc_vpu_h1: power-domain@9
> > > {
> > >                                                 #power-domain-cells
> > > = <0>;
> > >                                                 reg =
> > > <IMX8MM_POWER_DOMAIN_VPUH1>;
> > > +                                               power-domains =
> > > <&pgc_vpumix>;
> > >                                         };
> > >  
> > >                                         pgc_dispmix:
> > > power-domain@10 {
> > 
>