[Patch v4 9/9] arm64: Update device tree for Layerscape SoCs

[Patch v4 9/9] arm64: Update device tree for Layerscape SoCs

[York Sun]

Add DDR memory controller nodes to enable EDAC driver.

Signed-off-by: York Sun <york.sun@nxp.com>

---
Change log
  v4: no change
  v3: no change
  v2: no change

 arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi |  7 +++++++
 arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi | 14 ++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
index de0323b..cb33f23 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
@@ -196,6 +196,13 @@
 			bus-width = <4>;
 		};
 
+		ddr: memory-controller@1080000 {
+			compatible = "fsl,qoriq-memory-controller";
+			reg = <0x0 0x1080000 0x0 0x1000>;
+			interrupts = <0 144 0x4>;
+			big-endian;
+		};
+
 		dspi0: dspi@2100000 {
 			compatible = "fsl,ls1043a-dspi", "fsl,ls1021a-v1.0-dspi";
 			#address-cells = <1>;
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
index 3187c82..3221e5c 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
@@ -689,4 +689,18 @@
 			interrupts = <0 12 4>;
 		};
 	};
+
+	ddr1: memory-controller@1080000 {
+		compatible = "fsl,qoriq-memory-controller";
+		reg = <0x0 0x1080000 0x0 0x1000>;
+		interrupts = <0 17 0x4>;
+		little-endian;
+	};
+
+	ddr2: memory-controller@1090000 {
+		compatible = "fsl,qoriq-memory-controller";
+		reg = <0x0 0x1090000 0x0 0x1000>;
+		interrupts = <0 18 0x4>;
+		little-endian;
+	};
 };
-- 
2.7.4

Re: [Patch v4 9/9] arm64: Update device tree for Layerscape SoCs

[Borislav Petkov]

On Tue, Aug 09, 2016 at 02:59:39PM -0700, York Sun wrote:
> Add DDR memory controller nodes to enable EDAC driver.
> 
> Signed-off-by: York Sun <york.sun@nxp.com>
> 
> ---
> Change log
>   v4: no change
>   v3: no change
>   v2: no change
> 
>  arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi |  7 +++++++
>  arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi | 14 ++++++++++++++
>  2 files changed, 21 insertions(+)

This needs an ACK from DT people.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [Patch v4 9/9] arm64: Update device tree for Layerscape SoCs

[york sun]

+Shawn Guo

On 08/09/2016 03:00 PM, York Sun wrote:
> Add DDR memory controller nodes to enable EDAC driver.
>
> Signed-off-by: York Sun <york.sun@nxp.com>
>
> ---
> Change log
>   v4: no change
>   v3: no change
>   v2: no change
>
>  arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi |  7 +++++++
>  arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi | 14 ++++++++++++++
>  2 files changed, 21 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> index de0323b..cb33f23 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
> @@ -196,6 +196,13 @@
>  			bus-width = <4>;
>  		};
>
> +		ddr: memory-controller@1080000 {
> +			compatible = "fsl,qoriq-memory-controller";
> +			reg = <0x0 0x1080000 0x0 0x1000>;
> +			interrupts = <0 144 0x4>;
> +			big-endian;
> +		};
> +
>  		dspi0: dspi@2100000 {
>  			compatible = "fsl,ls1043a-dspi", "fsl,ls1021a-v1.0-dspi";
>  			#address-cells = <1>;
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
> index 3187c82..3221e5c 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
> @@ -689,4 +689,18 @@
>  			interrupts = <0 12 4>;
>  		};
>  	};
> +
> +	ddr1: memory-controller@1080000 {
> +		compatible = "fsl,qoriq-memory-controller";
> +		reg = <0x0 0x1080000 0x0 0x1000>;
> +		interrupts = <0 17 0x4>;
> +		little-endian;
> +	};
> +
> +	ddr2: memory-controller@1090000 {
> +		compatible = "fsl,qoriq-memory-controller";
> +		reg = <0x0 0x1090000 0x0 0x1000>;
> +		interrupts = <0 18 0x4>;
> +		little-endian;
> +	};
>  };
>

Re: [Patch v4 9/9] arm64: Update device tree for Layerscape SoCs

[Shawn Guo]

On Fri, Aug 12, 2016 at 11:13:54AM +0200, Borislav Petkov wrote:
> On Tue, Aug 09, 2016 at 02:59:39PM -0700, York Sun wrote:
> > Add DDR memory controller nodes to enable EDAC driver.
> > 
> > Signed-off-by: York Sun <york.sun@nxp.com>
> > 
> > ---
> > Change log
> >   v4: no change
> >   v3: no change
> >   v2: no change
> > 
> >  arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi |  7 +++++++
> >  arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi | 14 ++++++++++++++
> >  2 files changed, 21 insertions(+)
> 
> This needs an ACK from DT people.

Is it mandatory to have DTS changes go with driver part altogether?
Otherwise, I prefer to have them go through separate tree.

Shawn

Re: [Patch v4 9/9] arm64: Update device tree for Layerscape SoCs

[Borislav Petkov]

On Mon, Aug 29, 2016 at 02:34:48PM +0800, Shawn Guo wrote:
> Is it mandatory to have DTS changes go with driver part altogether?

Yes, because the EDAC driver needs them to even load properly.

> Otherwise, I prefer to have them go through separate tree.

Any particular reason why you prefer that?

We've been doing this for other ARM EDAC drivers already and there were
no issues whatsoever.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [Patch v4 9/9] arm64: Update device tree for Layerscape SoCs

[Shawn Guo]

On Mon, Aug 29, 2016 at 10:05:30AM +0200, Borislav Petkov wrote:
> On Mon, Aug 29, 2016 at 02:34:48PM +0800, Shawn Guo wrote:
> > Is it mandatory to have DTS changes go with driver part altogether?
> 
> Yes, because the EDAC driver needs them to even load properly.

When separate branches get merged on Linus tree, it works anyway.

> 
> > Otherwise, I prefer to have them go through separate tree.
> 
> Any particular reason why you prefer that?

To avoid potential merge conflicts.  Unless there are hard dependencies
like making it compile, avoiding regression or maintaining bisect,
patches should go through their established subsystem/architecture tree.

> We've been doing this for other ARM EDAC drivers already and there were
> no issues whatsoever.

Luckily.  If there are many patches on architecture DT branch changing
the same file, when driver branch and DT branch merges in upstream
branch, there will likely be merge conflicts.

Shawn

Re: [Patch v4 9/9] arm64: Update device tree for Layerscape SoCs

[Borislav Petkov]

On Mon, Aug 29, 2016 at 04:33:50PM +0800, Shawn Guo wrote:
> To avoid potential merge conflicts. 

Haven't heard of any so far. And I don't see how adding 1 or 2 DT
entries more per driver is a serious merge conflict.

> Unless there are hard dependencies like making it compile, avoiding
> regression or maintaining bisect, patches should go through their
> established subsystem/architecture tree.

Well, doh, the driver simply doesn't work. How are people even supposed
to test the EDAC tree?

Why is it even such a big deal if it is acked by the proper maintainers?
Cross-tree maintainer acking happens all the time. So don't tell me the
merge conflicts are your big issue with this.

> Luckily.  If there are many patches on architecture DT branch changing
> the same file, when driver branch and DT branch merges in upstream
> branch, there will likely be merge conflicts.

So? There are tools to resolve those. And again, the DT changes for
EDAC are basically adding blocks so resolving those conflicts should be
trivial most of the time.

So no, I don't consider the potential merge conflicts an issue here.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [Patch v4 9/9] arm64: Update device tree for Layerscape SoCs

[Shawn Guo]

On Mon, Aug 29, 2016 at 03:51:30PM +0200, Borislav Petkov wrote:
> On Mon, Aug 29, 2016 at 04:33:50PM +0800, Shawn Guo wrote:
> > To avoid potential merge conflicts. 
> 
> Haven't heard of any so far. And I don't see how adding 1 or 2 DT
> entries more per driver is a serious merge conflict.

Yeah, the conflict might be just easy to resolve.  But it's still
annoying when upstream maintainer runs into it often.  Why don't we
avoid it to ease upstream maintainer's life when it's possible?

> 
> > Unless there are hard dependencies like making it compile, avoiding
> > regression or maintaining bisect, patches should go through their
> > established subsystem/architecture tree.
> 
> Well, doh, the driver simply doesn't work. How are people even supposed
> to test the EDAC tree?

People are not supposed to test EDAC tree in this case. linux-next tree
is born for that.

> Why is it even such a big deal if it is acked by the proper maintainers?
> Cross-tree maintainer acking happens all the time. So don't tell me the
> merge conflicts are your big issue with this.

It's not a big deal, and it happens all the time.  But we shouldn't
consider it as a recommended work flow.

> > Luckily.  If there are many patches on architecture DT branch changing
> > the same file, when driver branch and DT branch merges in upstream
> > branch, there will likely be merge conflicts.
> 
> So? There are tools to resolve those. And again, the DT changes for
> EDAC are basically adding blocks so resolving those conflicts should be
> trivial most of the time.

And again, I do not understand why we do so when there is a work flow to
avoid this.

> 
> So no, I don't consider the potential merge conflicts an issue here.

But I do.

If you really like to apply the DTS patch through EDAC tree, go ahead.
But I'm not going to ACK it, because I have an opinion that this merge
path is not really necessary.

Shawn

Re: [Patch v4 9/9] arm64: Update device tree for Layerscape SoCs

[Olof Johansson]

On Mon, Aug 29, 2016 at 1:05 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Mon, Aug 29, 2016 at 02:34:48PM +0800, Shawn Guo wrote:
>> Is it mandatory to have DTS changes go with driver part altogether?
>
> Yes, because the EDAC driver needs them to even load properly.
>
>> Otherwise, I prefer to have them go through separate tree.
>
> Any particular reason why you prefer that?
>
> We've been doing this for other ARM EDAC drivers already and there were
> no issues whatsoever.

DT changes need to go through arm-soc. It's how we've been operating
for several years now.

It can be a huge pain when DT patches get merged through driver trees
and others are also modifying those files. Don't do it. You don't know
if someone else out there is changing these files, that's why we have
maintainers who own them and coordinate the changes.


If there are special circumstances such that the DT contents has to go
in with the same tree, then we can consider alternatives. But I can't
remember having one of those situations for quite a while now.


-Olof

Re: [Patch v4 9/9] arm64: Update device tree for Layerscape SoCs

[Borislav Petkov]

On Mon, Aug 29, 2016 at 02:39:21PM -0700, Olof Johansson wrote:
> DT changes need to go through arm-soc. It's how we've been operating
> for several years now.

Ok ok, we've wasted enough time with this.

So you guys pick up this one, I'll take the rest.

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [Patch v4 9/9] arm64: Update device tree for Layerscape SoCs

[Shawn Guo]

On Tue, Aug 09, 2016 at 02:59:39PM -0700, York Sun wrote:
> Add DDR memory controller nodes to enable EDAC driver.
> 
> Signed-off-by: York Sun <york.sun@nxp.com>

The patch subject is too general.  I changed it to "arm64: dts: Add DDR
memory controller for Layerscape SoCs", and applied patch.

Shawn

Re: [Patch v4 9/9] arm64: Update device tree for Layerscape SoCs

[york sun]

On 08/30/2016 03:58 AM, Shawn Guo wrote:
> On Tue, Aug 09, 2016 at 02:59:39PM -0700, York Sun wrote:
>> Add DDR memory controller nodes to enable EDAC driver.
>>
>> Signed-off-by: York Sun <york.sun@nxp.com>
>
> The patch subject is too general.  I changed it to "arm64: dts: Add DDR
> memory controller for Layerscape SoCs", and applied patch.
>

Thanks, Shawn.

York