| Message ID | 1363101135-21635-1-git-send-email-jason@lakedaemon.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello. On 12-03-2013 19:12, Jason Cooper wrote: > The board is supplied with a 4GB RAM module. This value can be > overridden by the bootloader based on probed memory size. We set it to > a reasonable value here. > Signed-off-by: Jason Cooper <jason@lakedaemon.net> > --- > As promised, catching this fix in the -rc cycle. > For those not familiar, earlier versions of the patch adding this board listed > 3GB because that is all that was visible. I mistaken applied v3 of the patch > instead of v4 which properly listed 4GB. This patch cleans up my error. > arch/arm/boot/dts/armada-xp-gp.dts | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > diff --git a/arch/arm/boot/dts/armada-xp-gp.dts b/arch/arm/boot/dts/armada-xp-gp.dts > index 1c8afe2..4a2776a 100644 > --- a/arch/arm/boot/dts/armada-xp-gp.dts > +++ b/arch/arm/boot/dts/armada-xp-gp.dts > @@ -28,12 +28,11 @@ > device_type = "memory"; > > /* > - * 4 GB of plug-in RAM modules by default but only 3GB > - * are visible, the amount of memory available can be > - * changed by the bootloader according the size of the > - * module actually plugged > + * 4 GB of plug-in RAM modules by default. The amount of memory > + * available can be changed by the bootloader according the "According to the size". > + * size of the module actually plugged > */ > - reg = <0x00000000 0xC0000000>; > + reg = <0x00000000 0xD0000000>; But this is not 4G? > }; WBR, Sergei
On 03/13/2013 03:30 PM, Sergei Shtylyov wrote: > Hello. > > On 12-03-2013 19:12, Jason Cooper wrote: > >> The board is supplied with a 4GB RAM module. This value can be >> overridden by the bootloader based on probed memory size. We set it to >> a reasonable value here. > >> Signed-off-by: Jason Cooper <jason@lakedaemon.net> >> --- >> As promised, catching this fix in the -rc cycle. > >> For those not familiar, earlier versions of the patch adding this board listed >> 3GB because that is all that was visible. I mistaken applied v3 of the patch >> instead of v4 which properly listed 4GB. This patch cleans up my error. > >> arch/arm/boot/dts/armada-xp-gp.dts | 9 ++++----- >> 1 file changed, 4 insertions(+), 5 deletions(-) > >> diff --git a/arch/arm/boot/dts/armada-xp-gp.dts b/arch/arm/boot/dts/armada-xp-gp.dts >> index 1c8afe2..4a2776a 100644 >> --- a/arch/arm/boot/dts/armada-xp-gp.dts >> +++ b/arch/arm/boot/dts/armada-xp-gp.dts >> @@ -28,12 +28,11 @@ >> device_type = "memory"; >> >> /* >> - * 4 GB of plug-in RAM modules by default but only 3GB >> - * are visible, the amount of memory available can be >> - * changed by the bootloader according the size of the >> - * module actually plugged >> + * 4 GB of plug-in RAM modules by default. The amount of memory >> + * available can be changed by the bootloader according the > > "According to the size". > >> + * size of the module actually plugged >> */ >> - reg = <0x00000000 0xC0000000>; >> + reg = <0x00000000 0xD0000000>; > > But this is not 4G? You're totally right! It should be reg = <0x00000000 0xF0000000>; > >> }; > > WBR, Sergei >
On Wed, Mar 13, 2013 at 06:30:30PM +0400, Sergei Shtylyov wrote: > Hello. > > On 12-03-2013 19:12, Jason Cooper wrote: > > >The board is supplied with a 4GB RAM module. This value can be > >overridden by the bootloader based on probed memory size. We set it to > >a reasonable value here. > > >Signed-off-by: Jason Cooper <jason@lakedaemon.net> > >--- > >As promised, catching this fix in the -rc cycle. > > >For those not familiar, earlier versions of the patch adding this board listed > >3GB because that is all that was visible. I mistaken applied v3 of the patch > >instead of v4 which properly listed 4GB. This patch cleans up my error. > > > arch/arm/boot/dts/armada-xp-gp.dts | 9 ++++----- > > 1 file changed, 4 insertions(+), 5 deletions(-) > > >diff --git a/arch/arm/boot/dts/armada-xp-gp.dts b/arch/arm/boot/dts/armada-xp-gp.dts > >index 1c8afe2..4a2776a 100644 > >--- a/arch/arm/boot/dts/armada-xp-gp.dts > >+++ b/arch/arm/boot/dts/armada-xp-gp.dts > >@@ -28,12 +28,11 @@ > > device_type = "memory"; > > > > /* > >- * 4 GB of plug-in RAM modules by default but only 3GB > >- * are visible, the amount of memory available can be > >- * changed by the bootloader according the size of the > >- * module actually plugged > >+ * 4 GB of plug-in RAM modules by default. The amount of memory > >+ * available can be changed by the bootloader according the > > "According to the size". ok. > >+ * size of the module actually plugged > > */ > >- reg = <0x00000000 0xC0000000>; > >+ reg = <0x00000000 0xD0000000>; > > But this is not 4G? Interesting, this was the value given in v4 of the original patch adding this board. I should have double-checked it. v2 on the way. thx, Jason.
On 03/13/2013 03:39 PM, Jason Cooper wrote: > On Wed, Mar 13, 2013 at 06:30:30PM +0400, Sergei Shtylyov wrote: >> Hello. >> >> On 12-03-2013 19:12, Jason Cooper wrote: >> >>> The board is supplied with a 4GB RAM module. This value can be >>> overridden by the bootloader based on probed memory size. We set it to >>> a reasonable value here. >> >>> Signed-off-by: Jason Cooper <jason@lakedaemon.net> >>> --- >>> As promised, catching this fix in the -rc cycle. >> >>> For those not familiar, earlier versions of the patch adding this board listed >>> 3GB because that is all that was visible. I mistaken applied v3 of the patch >>> instead of v4 which properly listed 4GB. This patch cleans up my error. >> >>> arch/arm/boot/dts/armada-xp-gp.dts | 9 ++++----- >>> 1 file changed, 4 insertions(+), 5 deletions(-) >> >>> diff --git a/arch/arm/boot/dts/armada-xp-gp.dts b/arch/arm/boot/dts/armada-xp-gp.dts >>> index 1c8afe2..4a2776a 100644 >>> --- a/arch/arm/boot/dts/armada-xp-gp.dts >>> +++ b/arch/arm/boot/dts/armada-xp-gp.dts >>> @@ -28,12 +28,11 @@ >>> device_type = "memory"; >>> >>> /* >>> - * 4 GB of plug-in RAM modules by default but only 3GB >>> - * are visible, the amount of memory available can be >>> - * changed by the bootloader according the size of the >>> - * module actually plugged >>> + * 4 GB of plug-in RAM modules by default. The amount of memory >>> + * available can be changed by the bootloader according the >> >> "According to the size". > > ok. > >>> + * size of the module actually plugged >>> */ >>> - reg = <0x00000000 0xC0000000>; >>> + reg = <0x00000000 0xD0000000>; >> >> But this is not 4G? > > Interesting, this was the value given in v4 of the original patch adding > this board. I should have double-checked it. v2 on the way. > I was wrong in my initial version, all we can do in 32 bits is not 4GB but 4GB-1B: reg = <0x00000000 0xFFFFFFFF> > thx, > > Jason. >
On Wed, Mar 13, 2013 at 03:37:00PM +0100, Gregory CLEMENT wrote: > On 03/13/2013 03:30 PM, Sergei Shtylyov wrote: > > Hello. > > > > On 12-03-2013 19:12, Jason Cooper wrote: > > > >> The board is supplied with a 4GB RAM module. This value can be > >> overridden by the bootloader based on probed memory size. We set it to > >> a reasonable value here. > > > >> Signed-off-by: Jason Cooper <jason@lakedaemon.net> > >> --- > >> As promised, catching this fix in the -rc cycle. > > > >> For those not familiar, earlier versions of the patch adding this board listed > >> 3GB because that is all that was visible. I mistaken applied v3 of the patch > >> instead of v4 which properly listed 4GB. This patch cleans up my error. > > > >> arch/arm/boot/dts/armada-xp-gp.dts | 9 ++++----- > >> 1 file changed, 4 insertions(+), 5 deletions(-) > > > >> diff --git a/arch/arm/boot/dts/armada-xp-gp.dts b/arch/arm/boot/dts/armada-xp-gp.dts > >> index 1c8afe2..4a2776a 100644 > >> --- a/arch/arm/boot/dts/armada-xp-gp.dts > >> +++ b/arch/arm/boot/dts/armada-xp-gp.dts > >> @@ -28,12 +28,11 @@ > >> device_type = "memory"; > >> > >> /* > >> - * 4 GB of plug-in RAM modules by default but only 3GB > >> - * are visible, the amount of memory available can be > >> - * changed by the bootloader according the size of the > >> - * module actually plugged > >> + * 4 GB of plug-in RAM modules by default. The amount of memory > >> + * available can be changed by the bootloader according the > > > > "According to the size". > > > >> + * size of the module actually plugged > >> */ > >> - reg = <0x00000000 0xC0000000>; > >> + reg = <0x00000000 0xD0000000>; > > > > But this is not 4G? > > You're totally right! > It should be reg = <0x00000000 0xF0000000>; $ printf "0x%08x\n" `echo "4 * (1024 ^ 3)" | bc` 0x100000000 ? thx, Jason.
On Wed, Mar 13, 2013 at 03:42:43PM +0100, Gregory CLEMENT wrote: > On 03/13/2013 03:39 PM, Jason Cooper wrote: > > On Wed, Mar 13, 2013 at 06:30:30PM +0400, Sergei Shtylyov wrote: > >> Hello. > >> > >> On 12-03-2013 19:12, Jason Cooper wrote: > >> > >>> The board is supplied with a 4GB RAM module. This value can be > >>> overridden by the bootloader based on probed memory size. We set it to > >>> a reasonable value here. > >> > >>> Signed-off-by: Jason Cooper <jason@lakedaemon.net> > >>> --- > >>> As promised, catching this fix in the -rc cycle. > >> > >>> For those not familiar, earlier versions of the patch adding this board listed > >>> 3GB because that is all that was visible. I mistaken applied v3 of the patch > >>> instead of v4 which properly listed 4GB. This patch cleans up my error. > >> > >>> arch/arm/boot/dts/armada-xp-gp.dts | 9 ++++----- > >>> 1 file changed, 4 insertions(+), 5 deletions(-) > >> > >>> diff --git a/arch/arm/boot/dts/armada-xp-gp.dts b/arch/arm/boot/dts/armada-xp-gp.dts > >>> index 1c8afe2..4a2776a 100644 > >>> --- a/arch/arm/boot/dts/armada-xp-gp.dts > >>> +++ b/arch/arm/boot/dts/armada-xp-gp.dts > >>> @@ -28,12 +28,11 @@ > >>> device_type = "memory"; > >>> > >>> /* > >>> - * 4 GB of plug-in RAM modules by default but only 3GB > >>> - * are visible, the amount of memory available can be > >>> - * changed by the bootloader according the size of the > >>> - * module actually plugged > >>> + * 4 GB of plug-in RAM modules by default. The amount of memory > >>> + * available can be changed by the bootloader according the > >> > >> "According to the size". > > > > ok. > > > >>> + * size of the module actually plugged > >>> */ > >>> - reg = <0x00000000 0xC0000000>; > >>> + reg = <0x00000000 0xD0000000>; > >> > >> But this is not 4G? > > > > Interesting, this was the value given in v4 of the original patch adding > > this board. I should have double-checked it. v2 on the way. > > > > I was wrong in my initial version, all we can do in 32 bits is not 4GB but 4GB-1B: > reg = <0x00000000 0xFFFFFFFF> hmmm, with the impending conquering of the server market by ARM, you'd think we'd at least be able to parse a 64 bit integer... thx, Jason.
On Wed, Mar 13, 2013 at 10:54:19AM -0400, Jason Cooper wrote: > > I was wrong in my initial version, all we can do in 32 bits is not > > 4GB but 4GB-1B: reg = <0x00000000 0xFFFFFFFF> > > hmmm, with the impending conquering of the server market by ARM, you'd > think we'd at least be able to parse a 64 bit integer... AFAIK, it would be like this: / { #address-cells = <2>; #size-cells = <2>; memory { device_type = "memory"; reg = <0 0 1 0>; }; And the other children nodes must be revised to support the 2 dw address and size.. Regards, Jason
On Wed, Mar 13, 2013 at 03:37:00PM +0100, Gregory CLEMENT wrote: > >> + * size of the module actually plugged > >> */ > >> - reg = <0x00000000 0xC0000000>; > >> + reg = <0x00000000 0xD0000000>; > > > > But this is not 4G? > > You're totally right! > It should be reg = <0x00000000 0xF0000000>; Isn't it 'right' as it is? The various included DT's put devices at address 0xd0008000 for instance, memory shouldn't overlap that.. Ie you loose the address space 0xD0000000 -> 0x100000000 due to internal registers. If the HW supports > 32 bit physical addressing then you'd want to program the DDR mapping to put some of the DDR above the 4G limit. Cheers, Jason
On Wed, Mar 13, 2013 at 11:35:59AM -0600, Jason Gunthorpe wrote: > On Wed, Mar 13, 2013 at 03:37:00PM +0100, Gregory CLEMENT wrote: > > > >> + * size of the module actually plugged > > >> */ > > >> - reg = <0x00000000 0xC0000000>; > > >> + reg = <0x00000000 0xD0000000>; > > > > > > But this is not 4G? > > > > You're totally right! > > It should be reg = <0x00000000 0xF0000000>; > > Isn't it 'right' as it is? The various included DT's put devices at > address 0xd0008000 for instance, memory shouldn't overlap that.. Shouldn't DT describe the hardware? The hardware has 4GB. How much of it Linux decides to use so as to avoid overlaps with registers etc, is not a hardware issue, but software. Andrew
On Wed, Mar 13, 2013 at 06:43:24PM +0100, Andrew Lunn wrote: > On Wed, Mar 13, 2013 at 11:35:59AM -0600, Jason Gunthorpe wrote: > > On Wed, Mar 13, 2013 at 03:37:00PM +0100, Gregory CLEMENT wrote: > > > > > >> + * size of the module actually plugged > > > >> */ > > > >> - reg = <0x00000000 0xC0000000>; > > > >> + reg = <0x00000000 0xD0000000>; > > > > > > > > But this is not 4G? > > > > > > You're totally right! > > > It should be reg = <0x00000000 0xF0000000>; > > > > Isn't it 'right' as it is? The various included DT's put devices at > > address 0xd0008000 for instance, memory shouldn't overlap that.. > > Shouldn't DT describe the hardware? The hardware has 4GB. How much of > it Linux decides to use so as to avoid overlaps with registers etc, is > not a hardware issue, but software. Sure, but which HW? The 'reg' in 'memory' is not describing a DDR bus. It is describing the DDR address mapping registers in the SOC - and they are probably set for 0 -> 0xD0000000 by the firmware. To describe the 4G of DDR you need DT nodes that show: - The N DDR chip selects going to each DDR rank - How much memory is in each DDR rank - How each chip select is currently mapped into CPU address space This is the minimal amount of information needed for Linux to reprogram the entire address map, if it so chooses. To me, the *address map* in the DTB passed from the firwmare should reflect the state of the machine when the OS is started. That way an OS without drivers for the address map hardware will function properly. Regards, Jason
On Wed, Mar 13, 2013 at 11:13:07AM -0600, Jason Gunthorpe wrote: > On Wed, Mar 13, 2013 at 10:54:19AM -0400, Jason Cooper wrote: > > > > I was wrong in my initial version, all we can do in 32 bits is not > > > 4GB but 4GB-1B: reg = <0x00000000 0xFFFFFFFF> > > > > hmmm, with the impending conquering of the server market by ARM, you'd > > think we'd at least be able to parse a 64 bit integer... > > AFAIK, it would be like this: > > / { > #address-cells = <2>; > #size-cells = <2>; > memory { > device_type = "memory"; > reg = <0 0 1 0>; > }; > > And the other children nodes must be revised to support the 2 dw > address and size.. Agreed, but that's a pretty invasive change for a fix. I'll go with 0xFFFFFFFF for now, and look forward to LPAE support including proper 64bit values. thx, Jason.
diff --git a/arch/arm/boot/dts/armada-xp-gp.dts b/arch/arm/boot/dts/armada-xp-gp.dts index 1c8afe2..4a2776a 100644 --- a/arch/arm/boot/dts/armada-xp-gp.dts +++ b/arch/arm/boot/dts/armada-xp-gp.dts @@ -28,12 +28,11 @@ device_type = "memory"; /* - * 4 GB of plug-in RAM modules by default but only 3GB - * are visible, the amount of memory available can be - * changed by the bootloader according the size of the - * module actually plugged + * 4 GB of plug-in RAM modules by default. The amount of memory + * available can be changed by the bootloader according the + * size of the module actually plugged */ - reg = <0x00000000 0xC0000000>; + reg = <0x00000000 0xD0000000>; }; soc {
The board is supplied with a 4GB RAM module. This value can be overridden by the bootloader based on probed memory size. We set it to a reasonable value here. Signed-off-by: Jason Cooper <jason@lakedaemon.net> --- As promised, catching this fix in the -rc cycle. For those not familiar, earlier versions of the patch adding this board listed 3GB because that is all that was visible. I mistaken applied v3 of the patch instead of v4 which properly listed 4GB. This patch cleans up my error. arch/arm/boot/dts/armada-xp-gp.dts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)