Re: [PATCH v4 14/18] arm64: dts: imx8mm: add GPC node

[Lucas Stach <l.stach@pengutronix.de>]

Hi Tim,

Am Freitag, dem 01.10.2021 um 19:43 -0700 schrieb Tim Harvey:
> > > > > 
[...]
> > > > > > 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.
> > 
> 
> I've tried that and it doesn't work either.
> 
> &gpu_2d {
>         status = "disabled";
> };
> 
> &gpu_3d {
>         status = "disabled";
> };
> 
> &vpu_blk_ctrl {
>         status = "disabled";
> };
> 
> &pgc_gpumix {
>         status = "disabled";
> };
> 
> &pgc_gpu {
>         status = "disabled";
> };
> 
> The interesting thing is that the first patch that causes this is
> 'arm64: dts: imx8mm: add GPC node' which just adds the gpc node and is
> before the addition of the other nodes. Therefore these are being
> enabled by default regardless of the above nodes being disabled (or
> not even added yet to imx8mm.dtsi).

My bad, I didn't think about the fact that the power domain devices are
not instantiated via the common OF populate code. For the status
property to work as expected the GPCv2 code needs to check this
property. I've just sent a patch to make this happen. Can you give it
another try with your DT changes and this patch applied?

Regards,
Lucas