| Message ID | 1470780000-16750-1-git-send-email-york.sun@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
+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; > + }; > }; >
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
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.
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
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.
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
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
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.
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
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
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; + }; };
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(+)