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

[Lucas Stach]

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@xxxxxxxxx>
> Cc: Shawn Guo <shawnguo@xxxxxxxxxx>
> Cc: Fabio Estevam <fabio.estevam@xxxxxxx>
> Cc: Chris Healy <cphealy@xxxxxxxxx>
> Cc: Lucas Stach <l.stach@xxxxxxxxxxxxxx>
> Cc: Leonard Crestez <leonard.crestez@xxxxxxx>
> Cc: "A.s. Dong" <aisheng.dong@xxxxxxx>
> Cc: Richard Zhu <hongxing.zhu@xxxxxxx>
> Cc: linux-imx@xxxxxxx
> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx

Nit below, otherwise:

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

> ---
> Â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 {