Re: [RFC v2 3/5] arm64: dts: imx8mq: Combine PCIE power domains

From: Lucas Stach <l.stach@pengutronix.de>
To: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Shawn Guo <shawnguo@kernel.org>,
	Fabio Estevam <fabio.estevam@nxp.com>,
	Chris Healy <cphealy@gmail.com>,
	Leonard Crestez <leonard.crestez@nxp.com>,
	"A.s. Dong" <aisheng.dong@nxp.com>,
	Richard Zhu <hongxing.zhu@nxp.com>,
	linux-imx@nxp.com, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC v2 3/5] arm64: dts: imx8mq: Combine PCIE power domains
Date: Fri, 08 Feb 2019 10:45:17 +0100	[thread overview]
Message-ID: <1549619117.2544.85.camel@pengutronix.de> (raw)
In-Reply-To: <20190208002941.30343-4-andrew.smirnov@gmail.com>

Am Donnerstag, den 07.02.2019, 16:29 -0800 schrieb Andrey Smirnov:
> According to NXP's FAE feedback and a comment in ATF firmware, PCIE1
> and PCIE2 power domains can't really be used independently. Due to
> shared reset line both power domains have to be turned on at the same
> time. Account for that quirk by combining PCIE power domains into a
> single 'pgc_pcie' power domain.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Chris Healy <cphealy@gmail.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Leonard Crestez <leonard.crestez@nxp.com>
> Cc: "A.s. Dong" <aisheng.dong@nxp.com>
> Cc: Richard Zhu <hongxing.zhu@nxp.com>
> Cc: linux-imx@nxp.com
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org

Nit below, otherwise:

Reviewed-by: Lucas Stach <l.stach@pengutronix.de>

> ---
>  arch/arm64/boot/dts/freescale/imx8mq.dtsi | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> index 50436bd393ed..89babc531380 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> @@ -273,9 +273,28 @@
>  						reg = <IMX8M_POWER_DOMAIN_MIPI>;
>  					};
>  
> -					pgc_pcie1: power-domain@1 {
> +					/*
> +					 * As per comment in ATF source code:
> +					 *
> +					 * PCIE1 and PCIE2 share the
> +					 * same reset signal, if we
> +					 * power down PCIE2, PCIE1
> +					 * will be held in reset too.
> +					 *
> +					 * So instead of creating two
> +					 * separate power domains for
> +					 * PCIE1 and PCIE2. We create
> +					 * a link between 1 and 10 and

1 and 10 is not something that directly shows up in the DTS, so I think
it could be confusing to the reader. I would just state that as "link
between both" or something to that tune.

> +					 * use what was supposed to be
> +					 * domain 1 as a shared PCIE
> +					 * power domain powering both
> +					 * PCIE1 and PCIE2 at the same
> +					 * time
> +					 */
> +					pgc_pcie: power-domain@1 {
>  						#power-domain-cells = <0>;
>  						reg = <IMX8M_POWER_DOMAIN_PCIE1>;
> +						power-domains = <&pgc_pcie2>;
>  					};
>  
>  					pgc_otg1: power-domain@2 {