Re: [PATCH v2 3/9] soc: imx: gpcv2: add support for i.MX8MP power domains

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Lucas Stach <l.stach@pengutronix.de>
Cc: Shawn Guo <shawnguo@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	patchwork-lst@pengutronix.de
Subject: Re: [PATCH v2 3/9] soc: imx: gpcv2: add support for i.MX8MP power domains
Date: Sun, 20 Feb 2022 13:35:36 +0200	[thread overview]
Message-ID: <YhIniLdFtcpODXBN@pendragon.ideasonboard.com> (raw)
In-Reply-To: <YhAlfAunReS14b/E@pendragon.ideasonboard.com>

Hi Lucas,

On Sat, Feb 19, 2022 at 01:02:21AM +0200, Laurent Pinchart wrote:
> On Mon, Feb 07, 2022 at 08:25:41PM +0100, Lucas Stach wrote:
> > This adds driver support for all the GPC power domains found on
> > the i.MX8MP SoC.
> > 
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> >  drivers/soc/imx/gpcv2.c | 387 +++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 386 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> > index 01f46b078df3..a7c92bdfc53b 100644
> > --- a/drivers/soc/imx/gpcv2.c
> > +++ b/drivers/soc/imx/gpcv2.c

[snip]

> > @@ -137,6 +183,21 @@
> >  #define IMX8MN_DISPMIX_HSK_PWRDNREQN		BIT(7)
> >  #define IMX8MN_HSIO_HSK_PWRDNREQN		BIT(5)
> >  
> > +#define IMX8MP_MEDIAMIX_PWRDNACKN		BIT(3)
> 
> This should be bit 30.

With this fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

with a soon to be posted driver for the MEDIA_BLK_CTRL.

While this shouldn't be a blocker, I'm wondering how we should deal with
the NOC configuration that TF-A handles in the power domain code ([1]).
The reference manual doesn't document the registers, which doesn't help.

There are also two registers in the MEDIA_BLK_CTRL that are specific to
the LCDIF and ISI, see [2]. Would you recommend dealing with them in the
imx8m-blk-ctrl driver (maybe in the power domain notifier, the same way
we set bit 8 in the CLK_EN register), or through a syscon phandle
directly in the LCDIF and ISI drivers ?

[1] https://source.codeaurora.org/external/qoriq/qoriq-components/atf/tree/plat/imx/imx8m/imx8mp/gpc.c?h=lf-5.10.72-2.2.0#n156
[2] https://source.codeaurora.org/external/qoriq/qoriq-components/atf/tree/plat/imx/imx8m/imx8mp/gpc.c?h=lf-5.10.72-2.2.0#n146

> > +#define IMX8MP_HDMIMIX_PWRDNACKN		BIT(29)
> > +#define IMX8MP_HSIOMIX_PWRDNACKN		BIT(28)
> > +#define IMX8MP_VPUMIX_PWRDNACKN			BIT(26)
> > +#define IMX8MP_GPUMIX_PWRDNACKN			BIT(25)
> > +#define IMX8MP_MLMIX_PWRDNACKN			(BIT(23) | BIT(24))
> > +#define IMX8MP_AUDIOMIX_PWRDNACKN		(BIT(20) | BIT(31))
> > +#define IMX8MP_MEDIAMIX_PWRDNREQN		BIT(14)
> > +#define IMX8MP_HDMIMIX_PWRDNREQN		BIT(13)
> > +#define IMX8MP_HSIOMIX_PWRDNREQN		BIT(12)
> > +#define IMX8MP_VPUMIX_PWRDNREQN			BIT(10)
> > +#define IMX8MP_GPUMIX_PWRDNREQN			BIT(9)
> > +#define IMX8MP_MLMIX_PWRDNREQN			(BIT(7) | BIT(8))
> > +#define IMX8MP_AUDIOMIX_PWRDNREQN		(BIT(4) | BIT(15))
> > +
> >  /*
> >   * The PGC offset values in Reference Manual
> >   * (Rev. 1, 01/2018 and the older ones) GPC chapter's

[snip]

-- 
Regards,

Laurent Pinchart

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