| Message ID | 1540447621-22870-6-git-send-email-manish.narani@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | EDAC: Enhancements to Synopsys EDAC driver | expand |
On Thu, Oct 25, 2018 at 11:37:00AM +0530, Manish Narani wrote: > Add ddrc memory controller node in dts. The size mentioned in dts is > 0x30000, because we need to access DDR_QOS INTR registers located at > 0xFD090208 from this driver. > > Signed-off-by: Manish Narani <manish.narani@xilinx.com> > --- > arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi > index 29ce234..a81d3b16 100644 > --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi > +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi > @@ -355,6 +355,13 @@ > xlnx,bus-width = <64>; > }; > > + mc: memory-controller@fd070000 { > + compatible = "xlnx,zynqmp-ddrc-2.40a"; > + reg = <0x0 0xfd070000 0x0 0x30000>; > + interrupt-parent = <&gic>; > + interrupts = <0 112 4>; > + }; > + > gem0: ethernet@ff0b0000 { > compatible = "cdns,zynqmp-gem", "cdns,gem"; > status = "disabled"; > -- Ok, talking to Mark on IRC, he says those DT changes normally go through the arm-soc tree. And I'm fine with that except if we do that, then the EDAC changes go through my tree and those drivers could end up temporarily broken depending on the merge order and timing. So should we perhaps make an arm-soc shared, immutable branch which you guys export for me with those DT changes applied, which I can merge into my tree and apply the EDAC changes ontop. This is what we've been doing with the tip tree until now and it has proven successful. Thoughts?
Hi Boris, On 05. 11. 18 13:56, Borislav Petkov wrote: > On Thu, Oct 25, 2018 at 11:37:00AM +0530, Manish Narani wrote: >> Add ddrc memory controller node in dts. The size mentioned in dts is >> 0x30000, because we need to access DDR_QOS INTR registers located at >> 0xFD090208 from this driver. >> >> Signed-off-by: Manish Narani <manish.narani@xilinx.com> >> --- >> arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi >> index 29ce234..a81d3b16 100644 >> --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi >> +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi >> @@ -355,6 +355,13 @@ >> xlnx,bus-width = <64>; >> }; >> >> + mc: memory-controller@fd070000 { >> + compatible = "xlnx,zynqmp-ddrc-2.40a"; >> + reg = <0x0 0xfd070000 0x0 0x30000>; >> + interrupt-parent = <&gic>; >> + interrupts = <0 112 4>; >> + }; >> + >> gem0: ethernet@ff0b0000 { >> compatible = "cdns,zynqmp-gem", "cdns,gem"; >> status = "disabled"; >> -- > > Ok, talking to Mark on IRC, he says those DT changes normally go through > the arm-soc tree. > > And I'm fine with that except if we do that, then the EDAC changes > go through my tree and those drivers could end up temporarily broken > depending on the merge order and timing. I don't think that driver will be broken. You can build them, use them on out of tree HW. And when this patch is merged to mainline it will be enabled for xilinx soc. > > So should we perhaps make an arm-soc shared, immutable branch which you > guys export for me with those DT changes applied, which I can merge into > my tree and apply the EDAC changes ontop. > > This is what we've been doing with the tip tree until now and it has > proven successful. > > Thoughts? Normally driver is merged via appropriate subsystem with DT binding doc and enabled for the soc/platform later. Also in connection to process which you have described above. DT node should match binding which is in your tree and should go first and then enabling for certain SoC on the top. TBH I can't see any reason to do merges but if you want to do that way we can also do it. Thanks, Michal
On Mon, Nov 05, 2018 at 02:06:11PM +0100, Michal Simek wrote: > I don't think that driver will be broken. You can build them, use them > on out of tree HW. And when this patch is merged to mainline it will be > enabled for xilinx soc. But if the DT entries are missing, the driver won't load, would it? > TBH I can't see any reason to do merges but if you want to do that way > we can also do it. The reason is because there's a separate DT tree and all those arm drivers need DT. I have already acked EDAC patches to go through other trees too, FWIW. Which is not optimal either if someone sends fixes ontop but I cannot apply them yet because the dependent patches are in a different tree. So yes, there are at least two good reasons for merging a shared branch.
On 05. 11. 18 14:20, Borislav Petkov wrote: > On Mon, Nov 05, 2018 at 02:06:11PM +0100, Michal Simek wrote: >> I don't think that driver will be broken. You can build them, use them >> on out of tree HW. And when this patch is merged to mainline it will be >> enabled for xilinx soc. > > But if the DT entries are missing, the driver won't load, would it? you don't have that HW anyway. > >> TBH I can't see any reason to do merges but if you want to do that way >> we can also do it. > > The reason is because there's a separate DT tree and all those arm > drivers need DT. > > I have already acked EDAC patches to go through other trees too, FWIW. I looked at v10 and I can't see your ack there. Can you please give me a link? I see Rob's reviewed by in v10 2/6 > Which is not optimal either if someone sends fixes ontop but I cannot > apply them yet because the dependent patches are in a different tree. > > So yes, there are at least two good reasons for merging a shared branch. I have not a problem with that. I can simply take patch (process via arm-soc) with pointing to reviewed binding doc and you will take the rest when this is in arm-soc tree. Thanks, Michal
On Mon, Nov 05, 2018 at 02:32:55PM +0100, Michal Simek wrote: > you don't have that HW anyway. Grrr, I'm not talking about me - I'm talking about people testing linux-next. And perhaps in this particular case it won't matter because this hw is not shipping yet or whatever but the question is about the principle of the whole thing. > I looked at v10 and I can't see your ack there. Can you please give me > a link? I'm talking about *other* patches for another driver. Please note that I'm replying to this thread as an example - the procedural question I have is not only related to the synopsys changes but the synopsys changes are a good example for the problem of EDAC changes being sent to me along with DT additions. As such, the question was aimed more at arm-soc people - that's why they were in To: - not at you.
On 05. 11. 18 14:42, Borislav Petkov wrote: > On Mon, Nov 05, 2018 at 02:32:55PM +0100, Michal Simek wrote: >> you don't have that HW anyway. > > Grrr, I'm not talking about me - I'm talking about people testing > linux-next. And perhaps in this particular case it won't matter because > this hw is not shipping yet or whatever but the question is about the > principle of the whole thing. > >> I looked at v10 and I can't see your ack there. Can you please give me >> a link? > > I'm talking about *other* patches for another driver. > > Please note that I'm replying to this thread as an example - the > procedural question I have is not only related to the synopsys changes > but the synopsys changes are a good example for the problem of EDAC > changes being sent to me along with DT additions. > > As such, the question was aimed more at arm-soc people - that's why they > were in To: - not at you. ok. Got it. M
On Mon, Nov 5, 2018 at 5:42 AM Borislav Petkov <bp@alien8.de> wrote: > > On Mon, Nov 05, 2018 at 02:32:55PM +0100, Michal Simek wrote: > > you don't have that HW anyway. > > Grrr, I'm not talking about me - I'm talking about people testing > linux-next. And perhaps in this particular case it won't matter because > this hw is not shipping yet or whatever but the question is about the > principle of the whole thing. > > > I looked at v10 and I can't see your ack there. Can you please give me > > a link? > > I'm talking about *other* patches for another driver. > > Please note that I'm replying to this thread as an example - the > procedural question I have is not only related to the synopsys changes > but the synopsys changes are a good example for the problem of EDAC > changes being sent to me along with DT additions. > > As such, the question was aimed more at arm-soc people - that's why they > were in To: - not at you. Hi Boris, In general, for new functionality where needing both the driver change and a DT change to enable it (or a driver change and a config change to enable it), we have been merging the changes separately between driver trees and arm-soc. I.e. things will be in place, but not enabled, until both sides land. Main reason for doing so is to cut down on arbitrary dependencies between the trees, since there can sometimes end up being a lot of them. Since DT should strive for being backwards compatible (i.e, a driver change shouldn't require a DT change for the kernel to not regress functionally), this has been working pretty well. However, if there's some other reason to share a base between the two trees, we can do that. For most cases we've found that it's not needed though. So let us know what you prefer here. -Olof
Hi Olof, On Mon, Nov 05, 2018 at 06:51:26AM -0800, Olof Johansson wrote: > In general, for new functionality where needing both the driver change > and a DT change to enable it (or a driver change and a config change > to enable it), we have been merging the changes separately between > driver trees and arm-soc. I.e. things will be in place, but not > enabled, until both sides land. Main reason for doing so is to cut > down on arbitrary dependencies between the trees, since there can > sometimes end up being a lot of them. Well, makes sense too and it is the least problematic approach. :-) > Since DT should strive for being backwards compatible (i.e, a driver > change shouldn't require a DT change for the kernel to not regress > functionally), this has been working pretty well. Ok. > However, if there's some other reason to share a base between the two > trees, we can do that. For most cases we've found that it's not needed > though. So let us know what you prefer here. No, I just wanted to make sure drivers can still function when they go to linux-next. But I certainly can get on board with keeping drivers and DT changes separate as it requires no sync and the short period of time when they don't load in linux-next, is perfectly ok. So can I assume you guys are going to pick up: https://lkml.kernel.org/r/1538667328-9465-7-git-send-email-manish.narani@xilinx.com https://lkml.kernel.org/r/1540447621-22870-6-git-send-email-manish.narani@xilinx.com ? I can then pick up the rest. And I'll be ignoring DT stuff sent to me from now on and concentrate on the EDAC drivers, assuming former will go through your tree. Sounds good? Thx.
On Mon, Nov 5, 2018 at 11:47 AM Borislav Petkov <bp@alien8.de> wrote: > > Hi Olof, > > On Mon, Nov 05, 2018 at 06:51:26AM -0800, Olof Johansson wrote: > > In general, for new functionality where needing both the driver change > > and a DT change to enable it (or a driver change and a config change > > to enable it), we have been merging the changes separately between > > driver trees and arm-soc. I.e. things will be in place, but not > > enabled, until both sides land. Main reason for doing so is to cut > > down on arbitrary dependencies between the trees, since there can > > sometimes end up being a lot of them. > > Well, makes sense too and it is the least problematic approach. :-) > > > Since DT should strive for being backwards compatible (i.e, a driver > > change shouldn't require a DT change for the kernel to not regress > > functionally), this has been working pretty well. > > Ok. > > > However, if there's some other reason to share a base between the two > > trees, we can do that. For most cases we've found that it's not needed > > though. So let us know what you prefer here. > > No, I just wanted to make sure drivers can still function when they go > to linux-next. But I certainly can get on board with keeping drivers and > DT changes separate as it requires no sync and the short period of time > when they don't load in linux-next, is perfectly ok. Ok, sounds good. > So can I assume you guys are going to pick up: > > https://lkml.kernel.org/r/1538667328-9465-7-git-send-email-manish.narani@xilinx.com > https://lkml.kernel.org/r/1540447621-22870-6-git-send-email-manish.narani@xilinx.com Yeah, Michal should pick those up for his platform and send them in to us (or let us know if he wants us to take them directly, but usually they come in through platform maintainers sending us pull requests). > > ? > > I can then pick up the rest. > > And I'll be ignoring DT stuff sent to me from now on and concentrate on > the EDAC drivers, assuming former will go through your tree. > > Sounds good? Yeah, that's the way we've been trying to do for various subsystems and it's been working pretty well. Of course, if there's need to coordinate more closely for something in the future we'll be happy to do so. -Olof
On Mon, Nov 05, 2018 at 12:38:05PM -0800, Olof Johansson wrote: > Yeah, that's the way we've been trying to do for various subsystems > and it's been working pretty well. Of course, if there's need to > coordinate more closely for something in the future we'll be happy to > do so. Goodie. Let's do that for now and we can always revisit when there's real need. Thx!
On 05. 11. 18 21:43, Borislav Petkov wrote: > On Mon, Nov 05, 2018 at 12:38:05PM -0800, Olof Johansson wrote: >> Yeah, that's the way we've been trying to do for various subsystems >> and it's been working pretty well. Of course, if there's need to >> coordinate more closely for something in the future we'll be happy to >> do so. > > Goodie. Let's do that for now and we can always revisit when there's > real need. I think Boris should take also this one (dt binding doc change) because it is aligned with changes in the driver. https://lkml.kernel.org/r/1538667328-9465-7-git-send-email-manish.narani@xilinx.com And I will take this one. https://lkml.kernel.org/r/1540447621-22870-6-git-send-email-manish.narani@xilinx.com But not a problem with taking both if you like but this make more sense to me. Thanks, Michal
On Tue, Nov 06, 2018 at 07:46:55AM +0100, Michal Simek wrote: > I think Boris should take also this one (dt binding doc change) because > it is aligned with changes in the driver. > https://lkml.kernel.org/r/1538667328-9465-7-git-send-email-manish.narani@xilinx.com Ok, done.
On 25. 10. 18 8:07, Manish Narani wrote: > Add ddrc memory controller node in dts. The size mentioned in dts is > 0x30000, because we need to access DDR_QOS INTR registers located at > 0xFD090208 from this driver. > > Signed-off-by: Manish Narani <manish.narani@xilinx.com> > --- > arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi > index 29ce234..a81d3b16 100644 > --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi > +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi > @@ -355,6 +355,13 @@ > xlnx,bus-width = <64>; > }; > > + mc: memory-controller@fd070000 { > + compatible = "xlnx,zynqmp-ddrc-2.40a"; > + reg = <0x0 0xfd070000 0x0 0x30000>; > + interrupt-parent = <&gic>; > + interrupts = <0 112 4>; > + }; > + > gem0: ethernet@ff0b0000 { > compatible = "cdns,zynqmp-gem", "cdns,gem"; > status = "disabled"; > Applied with changed subject "arm64: dts: zynqmp: Add DDRC node" to zynqmp/dt branch. Thanks, Michal
diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi index 29ce234..a81d3b16 100644 --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi @@ -355,6 +355,13 @@ xlnx,bus-width = <64>; }; + mc: memory-controller@fd070000 { + compatible = "xlnx,zynqmp-ddrc-2.40a"; + reg = <0x0 0xfd070000 0x0 0x30000>; + interrupt-parent = <&gic>; + interrupts = <0 112 4>; + }; + gem0: ethernet@ff0b0000 { compatible = "cdns,zynqmp-gem", "cdns,gem"; status = "disabled";
Add ddrc memory controller node in dts. The size mentioned in dts is 0x30000, because we need to access DDR_QOS INTR registers located at 0xFD090208 from this driver. Signed-off-by: Manish Narani <manish.narani@xilinx.com> --- arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 7 +++++++ 1 file changed, 7 insertions(+)