Patchwork [v10,5/6] arm64: zynqmp: Add DDRC node

login
register
mail settings
Submitter Manish Narani
Date Oct. 25, 2018, 6:07 a.m.
Message ID <1540447621-22870-6-git-send-email-manish.narani@xilinx.com>
Download mbox | patch
Permalink /patch/643451/
State New
Headers show

Comments

Manish Narani - Oct. 25, 2018, 6:07 a.m.
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(+)
Borislav Petkov - Nov. 5, 2018, 12:56 p.m.
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?
Michal Simek - Nov. 5, 2018, 1:06 p.m.
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
Borislav Petkov - Nov. 5, 2018, 1:20 p.m.
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.
Michal Simek - Nov. 5, 2018, 1:32 p.m.
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
Borislav Petkov - Nov. 5, 2018, 1:42 p.m.
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.
Michal Simek - Nov. 5, 2018, 1:45 p.m.
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
Olof Johansson - Nov. 5, 2018, 2:51 p.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
Borislav Petkov - Nov. 5, 2018, 7:47 p.m.
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.
Olof Johansson - Nov. 5, 2018, 8:38 p.m.
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
Borislav Petkov - Nov. 5, 2018, 8:43 p.m.
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!
Michal Simek - Nov. 6, 2018, 6:46 a.m.
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
Borislav Petkov - Nov. 6, 2018, 9:22 a.m.
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.
Michal Simek - Nov. 6, 2018, 11:54 a.m.
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

Patch

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";