From: Lucas Stach <l.stach@pengutronix.de>
To: Tim Harvey <tharvey@gateworks.com>
Cc: Shawn Guo <shawnguo@kernel.org>, Rob Herring <robh+dt@kernel.org>,
Fabio Estevam <festevam@gmail.com>,
NXP Linux Team <linux-imx@nxp.com>,
Adam Ford <aford173@gmail.com>,
Frieder Schrempf <frieder.schrempf@kontron.de>,
Marek Vasut <marex@denx.de>,
Device Tree Mailing List <devicetree@vger.kernel.org>,
Linux ARM Mailing List <linux-arm-kernel@lists.infradead.org>,
Sascha Hauer <kernel@pengutronix.de>,
patchwork-lst@pengutronix.de
Subject: Re: [PATCH v4 14/18] arm64: dts: imx8mm: add GPC node
Date: Sat, 02 Oct 2021 02:25:03 +0200 [thread overview]
Message-ID: <21bef8f0351a8a6c65e38f7ba80b98b34aec7b73.camel@pengutronix.de> (raw)
In-Reply-To: <CAJ+vNU1VKyZzkyeah1QtxA1y_Oswmgs4qWE87ZRU9++DqQSveA@mail.gmail.com>
Am Freitag, dem 01.10.2021 um 17:15 -0700 schrieb Tim Harvey:
> On Fri, Oct 1, 2021 at 5:06 PM Tim Harvey <tharvey@gateworks.com> wrote:
> >
> > On Fri, Oct 1, 2021 at 4:20 PM Lucas Stach <l.stach@pengutronix.de> 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 <l.stach@pengutronix.de> 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 <l.stach@pengutronix.de>
> > > > > ---
> > > > > 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 <dt-bindings/gpio/gpio.h>
> > > > > #include <dt-bindings/input/input.h>
> > > > > #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > > > +#include <dt-bindings/power/imx8mm-power.h>
> > > > > +#include <dt-bindings/reset/imx8mq-reset.h>
> > > > > #include <dt-bindings/thermal/thermal.h>
> > > > >
> > > > > #include "imx8mm-pinfunc.h"
> > > > > @@ -609,6 +611,111 @@ src: reset-controller@30390000 {
> > > > > 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>;
> > > > > + 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 = <IMX8MM_POWER_DOMAIN_PCIE>;
> > > > > + power-domains = <&pgc_hsiomix>;
> > > > > + clocks = <&clk IMX8MM_CLK_PCIE1_ROOT>;
> > > > > + };
> > > > > +
> > > > > + 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>;
> > > > > + };
> > > > > +
> > > > > + pgc_gpumix: power-domain@4 {
> > > > > + #power-domain-cells = <0>;
> > > > > + reg = <IMX8MM_POWER_DOMAIN_GPUMIX>;
> > > > > + 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 = <IMX8MM_POWER_DOMAIN_GPU>;
> > > > > + 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 = <IMX8MM_POWER_DOMAIN_VPUMIX>;
> > > > > + 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 = <IMX8MM_POWER_DOMAIN_VPUG1>;
> > > > > + };
> > > > > +
> > > > > + pgc_vpu_g2: power-domain@8 {
> > > > > + #power-domain-cells = <0>;
> > > > > + reg = <IMX8MM_POWER_DOMAIN_VPUG2>;
> > > > > + };
> > > > > +
> > > > > + pgc_vpu_h1: power-domain@9 {
> > > > > + #power-domain-cells = <0>;
> > > > > + reg = <IMX8MM_POWER_DOMAIN_VPUH1>;
> > > > > + };
> > > > > +
> > > > > + pgc_dispmix: power-domain@10 {
> > > > > + #power-domain-cells = <0>;
> > > > > + reg = <IMX8MM_POWER_DOMAIN_DISPMIX>;
> > > > > + 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 = <IMX8MM_POWER_DOMAIN_MIPI>;
> > > > > + };
> > > > > + };
> > > > > + };
> > > > > };
> > > > >
> > > > > 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
next prev parent reply other threads:[~2021-10-02 0:25 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-10 20:26 [PATCH v4 00/18] i.MX8MM GPC improvements and BLK_CTRL driver Lucas Stach
2021-09-10 20:26 ` [PATCH v4 01/18] Revert "soc: imx: gpcv2: move reset assert after requesting domain power up" Lucas Stach
2021-09-10 20:26 ` [PATCH v4 02/18] soc: imx: gpcv2: Turn domain->pgc into bitfield Lucas Stach
2021-09-10 20:26 ` [PATCH v4 03/18] soc: imx: gpcv2: Set both GPC_PGC_nCTRL(GPU_2D|GPU_3D) for MX8MM GPU domain Lucas Stach
2021-09-10 20:26 ` [PATCH v4 04/18] soc: imx: gpcv2: add lockdep annotation Lucas Stach
2021-09-10 20:26 ` [PATCH v4 05/18] soc: imx: gpcv2: add domain option to keep domain clocks enabled Lucas Stach
2021-09-10 20:26 ` [PATCH v4 06/18] soc: imx: gpcv2: keep i.MX8M* bus " Lucas Stach
2021-09-10 20:26 ` [PATCH v4 07/18] soc: imx: gpcv2: support system suspend/resume Lucas Stach
2021-09-10 20:26 ` [PATCH v4 08/18] dt-bindings: soc: add binding for i.MX8MM VPU blk-ctrl Lucas Stach
2021-09-16 20:23 ` Rob Herring
2021-09-10 20:26 ` [PATCH v4 09/18] dt-bindings: power: imx8mm: add defines for VPU blk-ctrl domains Lucas Stach
2021-09-16 20:25 ` Rob Herring
2021-09-10 20:26 ` [PATCH v4 10/18] soc: imx: add i.MX8M blk-ctrl driver Lucas Stach
2021-09-14 15:46 ` Benjamin Gaignard
2021-10-02 1:07 ` Lucas Stach
2021-10-04 14:27 ` Benjamin Gaignard
2021-10-05 10:03 ` Lucas Stach
2021-10-05 12:36 ` Benjamin Gaignard
2021-10-13 12:12 ` Benjamin Gaignard
2021-11-07 21:08 ` Adam Ford
2021-11-08 8:35 ` Lucas Stach
2021-11-08 13:13 ` Adam Ford
2021-09-10 20:26 ` [PATCH v4 11/18] dt-bindings: soc: add binding for i.MX8MM DISP blk-ctrl Lucas Stach
2021-09-16 20:28 ` Rob Herring
2021-09-10 20:26 ` [PATCH v4 12/18] dt-bindings: power: imx8mm: add defines for DISP blk-ctrl domains Lucas Stach
2021-09-16 20:29 ` Rob Herring
2021-09-10 20:26 ` [PATCH v4 13/18] soc: imx: imx8m-blk-ctrl: add DISP blk-ctrl Lucas Stach
2021-09-12 14:40 ` Adam Ford
2021-09-10 20:26 ` [PATCH v4 14/18] arm64: dts: imx8mm: add GPC node Lucas Stach
2021-10-01 23:07 ` Tim Harvey
2021-10-01 23:20 ` Lucas Stach
2021-10-02 0:06 ` Tim Harvey
2021-10-02 0:15 ` Tim Harvey
2021-10-02 0:25 ` Lucas Stach [this message]
2021-10-02 2:43 ` Tim Harvey
2021-10-02 12:51 ` Lucas Stach
2021-10-03 17:17 ` Tim Harvey
2021-10-03 19:44 ` Lucas Stach
2021-10-03 21:21 ` Tim Harvey
2021-10-04 7:44 ` Lucas Stach
2021-10-04 7:47 ` Lucas Stach
2021-09-10 20:26 ` [PATCH v4 15/18] arm64: dts: imx8mm: put USB controllers into power-domains Lucas Stach
2021-09-10 20:26 ` [PATCH v4 16/18] arm64: dts: imx8mm: Add GPU nodes for 2D and 3D core Lucas Stach
2021-09-10 20:26 ` [PATCH v4 17/18] arm64: dts: imx8mm: add VPU blk-ctrl Lucas Stach
2021-09-10 20:26 ` [PATCH v4 18/18] arm64: dts: imx8mm: add DISP blk-ctrl Lucas Stach
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=21bef8f0351a8a6c65e38f7ba80b98b34aec7b73.camel@pengutronix.de \
--to=l.stach@pengutronix.de \
--cc=aford173@gmail.com \
--cc=devicetree@vger.kernel.org \
--cc=festevam@gmail.com \
--cc=frieder.schrempf@kontron.de \
--cc=kernel@pengutronix.de \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-imx@nxp.com \
--cc=marex@denx.de \
--cc=patchwork-lst@pengutronix.de \
--cc=robh+dt@kernel.org \
--cc=shawnguo@kernel.org \
--cc=tharvey@gateworks.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).