From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3EDD7C433F5 for ; Sat, 2 Oct 2021 00:25:16 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1B57261AA4 for ; Sat, 2 Oct 2021 00:25:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230333AbhJBA06 (ORCPT ); Fri, 1 Oct 2021 20:26:58 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50444 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230259AbhJBA06 (ORCPT ); Fri, 1 Oct 2021 20:26:58 -0400 Received: from metis.ext.pengutronix.de (metis.ext.pengutronix.de [IPv6:2001:67c:670:201:290:27ff:fe1d:cc33]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A8686C061775 for ; Fri, 1 Oct 2021 17:25:12 -0700 (PDT) Received: from gallifrey.ext.pengutronix.de ([2001:67c:670:201:5054:ff:fe8d:eefb] helo=[IPv6:::1]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mWSpt-00030S-VH; Sat, 02 Oct 2021 02:25:06 +0200 Message-ID: <21bef8f0351a8a6c65e38f7ba80b98b34aec7b73.camel@pengutronix.de> Subject: Re: [PATCH v4 14/18] arm64: dts: imx8mm: add GPC node From: Lucas Stach To: Tim Harvey Cc: Shawn Guo , Rob Herring , Fabio Estevam , NXP Linux Team , Adam Ford , Frieder Schrempf , Marek Vasut , Device Tree Mailing List , Linux ARM Mailing List , Sascha Hauer , patchwork-lst@pengutronix.de Date: Sat, 02 Oct 2021 02:25:03 +0200 In-Reply-To: References: <20210910202640.980366-1-l.stach@pengutronix.de> <20210910202640.980366-15-l.stach@pengutronix.de> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.40.4 (3.40.4-1.fc34) MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-SA-Exim-Connect-IP: 2001:67c:670:201:5054:ff:fe8d:eefb X-SA-Exim-Mail-From: l.stach@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: devicetree@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org Am Freitag, dem 01.10.2021 um 17:15 -0700 schrieb Tim Harvey: > On Fri, Oct 1, 2021 at 5:06 PM Tim Harvey wrote: > > > > On Fri, Oct 1, 2021 at 4:20 PM Lucas Stach wrote: > > > > > > Hi Tim, > > > > > > Am Freitag, dem 01.10.2021 um 16:07 -0700 schrieb Tim Harvey: > > > > On Fri, Sep 10, 2021 at 1:26 PM Lucas Stach wrote: > > > > > > > > > > Add the DT node for the GPC, including all the PGC power domains, > > > > > some of them are not fully functional yet, as they require interaction > > > > > with the blk-ctrls to properly power up/down the peripherals. > > > > > > > > > > Signed-off-by: Lucas Stach > > > > > --- > > > > > arch/arm64/boot/dts/freescale/imx8mm.dtsi | 107 ++++++++++++++++++++++ > > > > > 1 file changed, 107 insertions(+) > > > > > > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi > > > > > index e7648c3b8390..3922f26f8fd4 100644 > > > > > --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi > > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi > > > > > @@ -7,6 +7,8 @@ > > > > > #include > > > > > #include > > > > > #include > > > > > +#include > > > > > +#include > > > > > #include > > > > > > > > > > #include "imx8mm-pinfunc.h" > > > > > @@ -609,6 +611,111 @@ src: reset-controller@30390000 { > > > > > interrupts = ; > > > > > #reset-cells = <1>; > > > > > }; > > > > > + > > > > > + gpc: gpc@303a0000 { > > > > > + compatible = "fsl,imx8mm-gpc"; > > > > > + reg = <0x303a0000 0x10000>; > > > > > + interrupts = ; > > > > > + 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 = ; > > > > > + clocks = <&clk IMX8MM_CLK_USB_BUS>; > > > > > + assigned-clocks = <&clk IMX8MM_CLK_USB_BUS>; > > > > > + assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_500M>; > > > > > + }; > > > > > + > > > > > + pgc_pcie: power-domain@1 { > > > > > + #power-domain-cells = <0>; > > > > > + reg = ; > > > > > + power-domains = <&pgc_hsiomix>; > > > > > + clocks = <&clk IMX8MM_CLK_PCIE1_ROOT>; > > > > > + }; > > > > > + > > > > > + pgc_otg1: power-domain@2 { > > > > > + #power-domain-cells = <0>; > > > > > + reg = ; > > > > > + power-domains = <&pgc_hsiomix>; > > > > > + }; > > > > > + > > > > > + pgc_otg2: power-domain@3 { > > > > > + #power-domain-cells = <0>; > > > > > + reg = ; > > > > > + power-domains = <&pgc_hsiomix>; > > > > > + }; > > > > > + > > > > > + pgc_gpumix: power-domain@4 { > > > > > + #power-domain-cells = <0>; > > > > > + reg = ; > > > > > + clocks = <&clk IMX8MM_CLK_GPU_BUS_ROOT>, > > > > > + <&clk IMX8MM_CLK_GPU_AHB>; > > > > > + assigned-clocks = <&clk IMX8MM_CLK_GPU_AXI>, > > > > > + <&clk IMX8MM_CLK_GPU_AHB>; > > > > > + assigned-clock-parents = <&clk IMX8MM_SYS_PLL1_800M>, > > > > > + <&clk IMX8MM_SYS_PLL1_800M>; > > > > > + assigned-clock-rates = <800000000>, <400000000>; > > > > > + }; > > > > > + > > > > > + pgc_gpu: power-domain@5 { > > > > > + #power-domain-cells = <0>; > > > > > + reg = ; > > > > > + clocks = <&clk IMX8MM_CLK_GPU_AHB>, > > > > > + <&clk IMX8MM_CLK_GPU_BUS_ROOT>, > > > > > + <&clk IMX8MM_CLK_GPU2D_ROOT>, > > > > > + <&clk IMX8MM_CLK_GPU3D_ROOT>; > > > > > + resets = <&src IMX8MQ_RESET_GPU_RESET>; > > > > > + power-domains = <&pgc_gpumix>; > > > > > + }; > > > > > + > > > > > + pgc_vpumix: power-domain@6 { > > > > > + #power-domain-cells = <0>; > > > > > + reg = ; > > > > > + clocks = <&clk IMX8MM_CLK_VPU_DEC_ROOT>; > > > > > + assigned-clocks = <&clk IMX8MM_CLK_VPU_BUS>; > > > > > + assigned-clock-parents = <&clk IMX8MM_SYS_PLL1_800M>; > > > > > + resets = <&src IMX8MQ_RESET_VPU_RESET>; > > > > > + }; > > > > > + > > > > > + pgc_vpu_g1: power-domain@7 { > > > > > + #power-domain-cells = <0>; > > > > > + reg = ; > > > > > + }; > > > > > + > > > > > + pgc_vpu_g2: power-domain@8 { > > > > > + #power-domain-cells = <0>; > > > > > + reg = ; > > > > > + }; > > > > > + > > > > > + pgc_vpu_h1: power-domain@9 { > > > > > + #power-domain-cells = <0>; > > > > > + reg = ; > > > > > + }; > > > > > + > > > > > + pgc_dispmix: power-domain@10 { > > > > > + #power-domain-cells = <0>; > > > > > + reg = ; > > > > > + clocks = <&clk IMX8MM_CLK_DISP_APB_ROOT>, > > > > > + <&clk IMX8MM_CLK_DISP_AXI_ROOT>; > > > > > + assigned-clocks = <&clk IMX8MM_CLK_DISP_AXI>, > > > > > + <&clk IMX8MM_CLK_DISP_APB>; > > > > > + assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_1000M>, > > > > > + <&clk IMX8MM_SYS_PLL1_800M>; > > > > > + assigned-clock-rates = <500000000>, <200000000>; > > > > > + }; > > > > > + > > > > > + pgc_mipi: power-domain@11 { > > > > > + #power-domain-cells = <0>; > > > > > + reg = ; > > > > > + }; > > > > > + }; > > > > > + }; > > > > > }; > > > > > > > > > > aips2: bus@30400000 { > > > > > -- > > > > > 2.30.2 > > > > > > > > > > > > > Lucas, > > > > > > > > I've been using your 'i.MX8MM GPC improvements and BLK_CTRL driver' > > > > series for imx8mm-venice* and imx8mn-venice* boards. Thank you for > > > > this work and I hope to see it merged soon! > > > > > > > > I have found that on the imx8mm-venice-gw7901 board which does not use > > > > MIPI and thus does not connect VDD_MIPI_1P8, VDD_MIPI_1P2, > > > > VDD_MIPI_0P9, MIPI_VREG_CAP pins on the IMX8MM hangs with this > > > > particular patch. If I comment out the pgc_mipi domain and subsequent > > > > disp_blk_ctrl node from a later patch it resolves the hang. Is this > > > > behavior expected and what would your recommendation be to work around > > > > it? > > > > > > No, this isn't expected. If there are no active devices in the MIPI > > > domain, the power domain should not be touched, as we treat all of them > > > as disabled initially. If we don't touch the domain I would expect that > > > the power supply not being present shouldn't be an issue. > > > > > > Can you check if something in your system causes this power domain to > > > be powered up? Easiest way is probably to sprinkle a > > > printk("%s\n, genpd->name) in both imx8m_blk_ctrl_power_on() and > > > imx_gpc_power_on(). > > > > > > > Lucas, > > > > Here's what I see before I hang (debug print on both power on/off > > followed by a msleep(1000) to make sure I see it before I hang): > > [ 0.518319] imx_pgc_power_up hsiomix > > [ 0.624031] imx_pgc_power_down hsiomix > > [ 0.731879] imx_pgc_power_up hsiomix > > [ 0.839906] imx_pgc_power_down hsiomix > > [ 0.947875] imx_pgc_power_up hsiomix > > [ 1.055859] imx_pgc_power_down hsiomix > > [ 1.057296] imx_pgc_power_up gpumix > > [ 1.167884] imx_pgc_power_down gpumix > > [ 0.518513] imx_pgc_power_up hsiomix > > [ 0.519933] imx_pgc_power_up gpumix > > > > The board also has IMX8MM VDD_GPU pins not connected so it makes sense > that we hang here I suppose. Yet if I add the folloiwng to > imx8mm-venice-gw7901.dts it still tries to enable them and hangs: > &gpu_2d { > status = "disabled"; > }; > > &gpu_3d { > status = "disabled"; > }; > > &vpu_blk_ctrl { > status = "disabled"; > }; The pgc_gpu is a "active" consumer of the pgc_gpumix domain while the driver gets probed, so the driver core will power up the gpumix domain for a moment during kernel init. To avoid this you must at least set the status of the pgc_gpu node to disabled. Regards, Lucas