linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] sunxi: dts: add a note that memory size is adjusted by boot loader.
@ 2014-01-24 14:14 Ian Campbell
  2014-01-24 15:31 ` Hans de Goede
  0 siblings, 1 reply; 6+ messages in thread
From: Ian Campbell @ 2014-01-24 14:14 UTC (permalink / raw)
  To: linux-arm-kernel

I had to spend a couple of minutes proving to myself that this was the case on
cubietruck, so add a comment to save the next guy some effort.

Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-sunxi at googlegroups.com
---
This patch applies cleanly against v3.13 and Hans' sunxi-devel branch.

A plausible alternative would be to pull the memory node out of the dtsi files
and into the board specific files. I didn't go straight to that since I'd have
to research all the various boards ;-)

I considered maing it "reg = <0 0>" but decided that having some sort of least
common denominator would let things work even if the bootloader were broken.
---
 arch/arm/boot/dts/sun4i-a10.dtsi  |    1 +
 arch/arm/boot/dts/sun5i-a10s.dtsi |    1 +
 arch/arm/boot/dts/sun5i-a13.dtsi  |    1 +
 arch/arm/boot/dts/sun6i-a31.dtsi  |    1 +
 arch/arm/boot/dts/sun7i-a20.dtsi  |    1 +
 5 files changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 3e60883..9ba0beb 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -30,6 +30,7 @@
 	};
 
 	memory {
+		/* 1GB by default, will be updated by U-Boot */
 		reg = <0x40000000 0x80000000>;
 	};
 
diff --git a/arch/arm/boot/dts/sun5i-a10s.dtsi b/arch/arm/boot/dts/sun5i-a10s.dtsi
index 0376c50..d12ed7e 100644
--- a/arch/arm/boot/dts/sun5i-a10s.dtsi
+++ b/arch/arm/boot/dts/sun5i-a10s.dtsi
@@ -27,6 +27,7 @@
 	};
 
 	memory {
+		/* 512MB by default, will be updated by U-Boot */
 		reg = <0x40000000 0x20000000>;
 	};
 
diff --git a/arch/arm/boot/dts/sun5i-a13.dtsi b/arch/arm/boot/dts/sun5i-a13.dtsi
index b81aeb9..6f8bfd9 100644
--- a/arch/arm/boot/dts/sun5i-a13.dtsi
+++ b/arch/arm/boot/dts/sun5i-a13.dtsi
@@ -27,6 +27,7 @@
 	};
 
 	memory {
+		/* 512MB by default, will be updated by U-Boot */
 		reg = <0x40000000 0x20000000>;
 	};
 
diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index 3f6f07b..bcbef9a 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -46,6 +46,7 @@
 	};
 
 	memory {
+		/* 2GB by default, will be updated by U-Boot */
 		reg = <0x40000000 0x80000000>;
 	};
 
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 907cfcc..658e74b 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -38,6 +38,7 @@
 	};
 
 	memory {
+		/* 1GB by default, will be updated by U-Boot */
 		reg = <0x40000000 0x80000000>;
 	};
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] sunxi: dts: add a note that memory size is adjusted by boot loader.
  2014-01-24 14:14 [PATCH] sunxi: dts: add a note that memory size is adjusted by boot loader Ian Campbell
@ 2014-01-24 15:31 ` Hans de Goede
  2014-01-24 15:41   ` Ian Campbell
  0 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2014-01-24 15:31 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 01/24/2014 03:14 PM, Ian Campbell wrote:
> I had to spend a couple of minutes proving to myself that this was the case on
> cubietruck, so add a comment to save the next guy some effort.

Seems like a good idea to me, one small nitpick though see comments inline.

> Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-sunxi at googlegroups.com
> ---
> This patch applies cleanly against v3.13 and Hans' sunxi-devel branch.
>
> A plausible alternative would be to pull the memory node out of the dtsi files
> and into the board specific files. I didn't go straight to that since I'd have
> to research all the various boards ;-)

That won't help, some boards ie the original cubieboard and the mele-a1000 come
in both 512 MB and 1024 MB versions, and I don't think we want to start maintaining
2 different dts files just for that.

>
> I considered maing it "reg = <0 0>" but decided that having some sort of least
> common denominator would let things work even if the bootloader were broken.
> ---
>   arch/arm/boot/dts/sun4i-a10.dtsi  |    1 +
>   arch/arm/boot/dts/sun5i-a10s.dtsi |    1 +
>   arch/arm/boot/dts/sun5i-a13.dtsi  |    1 +
>   arch/arm/boot/dts/sun6i-a31.dtsi  |    1 +
>   arch/arm/boot/dts/sun7i-a20.dtsi  |    1 +
>   5 files changed, 5 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
> index 3e60883..9ba0beb 100644
> --- a/arch/arm/boot/dts/sun4i-a10.dtsi
> +++ b/arch/arm/boot/dts/sun4i-a10.dtsi
> @@ -30,6 +30,7 @@
>   	};
>
>   	memory {
> +		/* 1GB by default, will be updated by U-Boot */
>   		reg = <0x40000000 0x80000000>;
>   	};
>

The comment says 1GB, but the range says 2GB, note 2GB is consistent with what the
datasheet claims as max RAM.

> diff --git a/arch/arm/boot/dts/sun5i-a10s.dtsi b/arch/arm/boot/dts/sun5i-a10s.dtsi
> index 0376c50..d12ed7e 100644
> --- a/arch/arm/boot/dts/sun5i-a10s.dtsi
> +++ b/arch/arm/boot/dts/sun5i-a10s.dtsi
> @@ -27,6 +27,7 @@
>   	};
>
>   	memory {
> +		/* 512MB by default, will be updated by U-Boot */
>   		reg = <0x40000000 0x20000000>;
>   	};
>

This seems wrong (copy paste from A13 error) I've a10s boards with 1G, and the data
sheet claims 2GB max RAM.

> diff --git a/arch/arm/boot/dts/sun5i-a13.dtsi b/arch/arm/boot/dts/sun5i-a13.dtsi
> index b81aeb9..6f8bfd9 100644
> --- a/arch/arm/boot/dts/sun5i-a13.dtsi
> +++ b/arch/arm/boot/dts/sun5i-a13.dtsi
> @@ -27,6 +27,7 @@
>   	};
>
>   	memory {
> +		/* 512MB by default, will be updated by U-Boot */
>   		reg = <0x40000000 0x20000000>;
>   	};
>
> diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
> index 3f6f07b..bcbef9a 100644
> --- a/arch/arm/boot/dts/sun6i-a31.dtsi
> +++ b/arch/arm/boot/dts/sun6i-a31.dtsi
> @@ -46,6 +46,7 @@
>   	};
>
>   	memory {
> +		/* 2GB by default, will be updated by U-Boot */
>   		reg = <0x40000000 0x80000000>;
>   	};
>
> diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
> index 907cfcc..658e74b 100644
> --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> @@ -38,6 +38,7 @@
>   	};
>
>   	memory {
> +		/* 1GB by default, will be updated by U-Boot */
>   		reg = <0x40000000 0x80000000>;
>   	};

The comment says 1GB, but the range says 2GB, note 2GB is consistent with what the
datasheet claims as max RAM.

Regards,

Hans

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] sunxi: dts: add a note that memory size is adjusted by boot loader.
  2014-01-24 15:31 ` Hans de Goede
@ 2014-01-24 15:41   ` Ian Campbell
  2014-01-25 11:03     ` Hans de Goede
  0 siblings, 1 reply; 6+ messages in thread
From: Ian Campbell @ 2014-01-24 15:41 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, 2014-01-24 at 16:31 +0100, Hans de Goede wrote:
> Hi,
> 
> On 01/24/2014 03:14 PM, Ian Campbell wrote:
> > I had to spend a couple of minutes proving to myself that this was the case on
> > cubietruck, so add a comment to save the next guy some effort.
> 
> Seems like a good idea to me, one small nitpick though see comments inline.
> 
> > Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
> > Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> > Cc: Hans de Goede <hdegoede@redhat.com>
> > Cc: linux-arm-kernel at lists.infradead.org
> > Cc: linux-sunxi at googlegroups.com
> > ---
> > This patch applies cleanly against v3.13 and Hans' sunxi-devel branch.
> >
> > A plausible alternative would be to pull the memory node out of the dtsi files
> > and into the board specific files. I didn't go straight to that since I'd have
> > to research all the various boards ;-)
> 
> That won't help, some boards ie the original cubieboard and the mele-a1000 come
> in both 512 MB and 1024 MB versions, and I don't think we want to start maintaining
> 2 different dts files just for that.

I didn't mean to suggest that u-boot should stop updating the dtb, so
e.g. I would have put a 512MB reg in the cubieboard dts, with a similar
comment.

But in cases where there is only one variant (e.g. the cubietruck) I'd
have put a 2GB reg and no comment, even though u-boot would in reality
rewrite the 2GB there. 

> > diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
> > index 3e60883..9ba0beb 100644
> > --- a/arch/arm/boot/dts/sun4i-a10.dtsi
> > +++ b/arch/arm/boot/dts/sun4i-a10.dtsi
> > @@ -30,6 +30,7 @@
> >   	};
> >
> >   	memory {
> > +		/* 1GB by default, will be updated by U-Boot */
> >   		reg = <0x40000000 0x80000000>;
> >   	};
> >
> 
> The comment says 1GB, but the range says 2GB, note 2GB is consistent with what the
> datasheet claims as max RAM.

Oops, I must have missed that.

> 
> > diff --git a/arch/arm/boot/dts/sun5i-a10s.dtsi b/arch/arm/boot/dts/sun5i-a10s.dtsi
> > index 0376c50..d12ed7e 100644
> > --- a/arch/arm/boot/dts/sun5i-a10s.dtsi
> > +++ b/arch/arm/boot/dts/sun5i-a10s.dtsi
> > @@ -27,6 +27,7 @@
> >   	};
> >
> >   	memory {
> > +		/* 512MB by default, will be updated by U-Boot */
> >   		reg = <0x40000000 0x20000000>;
> >   	};
> >
> 
> This seems wrong (copy paste from A13 error) I've a10s boards with 1G, and the data
> sheet claims 2GB max RAM.

I had expected these values to contain the minimum -- i.e. the one which
would work everywhere, so it would work if for some reason u-boot wasn't
updating correctly. Better to boot with only some of the RAM than to
crash randomly.

> > diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
> > index 907cfcc..658e74b 100644
> > --- a/arch/arm/boot/dts/sun7i-a20.dtsi
> > +++ b/arch/arm/boot/dts/sun7i-a20.dtsi
> > @@ -38,6 +38,7 @@
> >   	};
> >
> >   	memory {
> > +		/* 1GB by default, will be updated by U-Boot */
> >   		reg = <0x40000000 0x80000000>;
> >   	};
> 
> The comment says 1GB, but the range says 2GB, note 2GB is consistent with what the
> datasheet claims as max RAM.

Huh, that's the second time I got the wrong one, and I swear I was
paying attention to it as well!

Ian.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] sunxi: dts: add a note that memory size is adjusted by boot loader.
  2014-01-24 15:41   ` Ian Campbell
@ 2014-01-25 11:03     ` Hans de Goede
  2014-01-27 14:57       ` Maxime Ripard
  0 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2014-01-25 11:03 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 01/24/2014 04:41 PM, Ian Campbell wrote:
> On Fri, 2014-01-24 at 16:31 +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 01/24/2014 03:14 PM, Ian Campbell wrote:
>>> I had to spend a couple of minutes proving to myself that this was the case on
>>> cubietruck, so add a comment to save the next guy some effort.
>>
>> Seems like a good idea to me, one small nitpick though see comments inline.
>>
>>> Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
>>> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
>>> Cc: Hans de Goede <hdegoede@redhat.com>
>>> Cc: linux-arm-kernel at lists.infradead.org
>>> Cc: linux-sunxi at googlegroups.com
>>> ---
>>> This patch applies cleanly against v3.13 and Hans' sunxi-devel branch.
>>>
>>> A plausible alternative would be to pull the memory node out of the dtsi files
>>> and into the board specific files. I didn't go straight to that since I'd have
>>> to research all the various boards ;-)
>>
>> That won't help, some boards ie the original cubieboard and the mele-a1000 come
>> in both 512 MB and 1024 MB versions, and I don't think we want to start maintaining
>> 2 different dts files just for that.
>
> I didn't mean to suggest that u-boot should stop updating the dtb, so
> e.g. I would have put a 512MB reg in the cubieboard dts, with a similar
> comment.

Hmm, I've no idea what the default / preferred way of handling this is,
I assume Maxime knows, so lets wait for his input on this.

> But in cases where there is only one variant (e.g. the cubietruck) I'd
> have put a 2GB reg and no comment, even though u-boot would in reality
> rewrite the 2GB there.
>
>>> diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
>>> index 3e60883..9ba0beb 100644
>>> --- a/arch/arm/boot/dts/sun4i-a10.dtsi
>>> +++ b/arch/arm/boot/dts/sun4i-a10.dtsi
>>> @@ -30,6 +30,7 @@
>>>    	};
>>>
>>>    	memory {
>>> +		/* 1GB by default, will be updated by U-Boot */
>>>    		reg = <0x40000000 0x80000000>;
>>>    	};
>>>
>>
>> The comment says 1GB, but the range says 2GB, note 2GB is consistent with what the
>> datasheet claims as max RAM.
>
> Oops, I must have missed that.
>
>>
>>> diff --git a/arch/arm/boot/dts/sun5i-a10s.dtsi b/arch/arm/boot/dts/sun5i-a10s.dtsi
>>> index 0376c50..d12ed7e 100644
>>> --- a/arch/arm/boot/dts/sun5i-a10s.dtsi
>>> +++ b/arch/arm/boot/dts/sun5i-a10s.dtsi
>>> @@ -27,6 +27,7 @@
>>>    	};
>>>
>>>    	memory {
>>> +		/* 512MB by default, will be updated by U-Boot */
>>>    		reg = <0x40000000 0x20000000>;
>>>    	};
>>>
>>
>> This seems wrong (copy paste from A13 error) I've a10s boards with 1G, and the data
>> sheet claims 2GB max RAM.
>
> I had expected these values to contain the minimum -- i.e. the one which
> would work everywhere, so it would work if for some reason u-boot wasn't
> updating correctly. Better to boot with only some of the RAM than to
> crash randomly.

Then all the other values would be wrong, we've A10 boards with only 512M, A13
boards with only 256, etc.

Anyways as said before lets wait for Maxime's input on this.

Regards,

Hans

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] sunxi: dts: add a note that memory size is adjusted by boot loader.
  2014-01-25 11:03     ` Hans de Goede
@ 2014-01-27 14:57       ` Maxime Ripard
  2014-01-27 15:52         ` Ian Campbell
  0 siblings, 1 reply; 6+ messages in thread
From: Maxime Ripard @ 2014-01-27 14:57 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Ian, Hans,

On Sat, Jan 25, 2014 at 12:03:16PM +0100, Hans de Goede wrote:
> Hmm, I've no idea what the default / preferred way of handling this is,
> I assume Maxime knows, so lets wait for his input on this.

I got the habit of setting the memory node to the max size the RAM
controller can handle from previous work on imx, but I don't really
have a preferrence here.

If that confuses people, we can just remove it from the DTSI
altogether. It will be patched by u-boot anyway, and we won't have to
create DTS variants this way.

Just don't do it for the A31 for the moment, since we don't have
DT-enabled u-boot for now.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140127/e91902ab/attachment.sig>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] sunxi: dts: add a note that memory size is adjusted by boot loader.
  2014-01-27 14:57       ` Maxime Ripard
@ 2014-01-27 15:52         ` Ian Campbell
  0 siblings, 0 replies; 6+ messages in thread
From: Ian Campbell @ 2014-01-27 15:52 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, 2014-01-27 at 15:57 +0100, Maxime Ripard wrote:
> Hi Ian, Hans,
> 
> On Sat, Jan 25, 2014 at 12:03:16PM +0100, Hans de Goede wrote:
> > Hmm, I've no idea what the default / preferred way of handling this is,
> > I assume Maxime knows, so lets wait for his input on this.
> 
> I got the habit of setting the memory node to the max size the RAM
> controller can handle from previous work on imx,

I suppose this has the advantage of being static for a given SoC, so
there is no question of what the smallest possible value is, even if it
has the disadvantage of not working as a default on many platforms
(perhaps I'm overestimating the utility of that?).

>  but I don't really
> have a preferrence here.
> 
> If that confuses people, we can just remove it from the DTSI
> altogether. It will be patched by u-boot anyway, and we won't have to
> create DTS variants this way.

I've occasionally seen issues with u-boot not expanding the fdt itself,
so the placeholder serves the purpose of letting it update without
expanding. That was in my more complex Xen boot script which use "fdt
set" a lot -- probably doesn't apply to properly integrated
functionality like updating the memory node? (Or if it does that's a
u-boot bug).

> Just don't do it for the A31 for the moment, since we don't have
> DT-enabled u-boot for now.

Ack!

Ian.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-01-27 15:52 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-24 14:14 [PATCH] sunxi: dts: add a note that memory size is adjusted by boot loader Ian Campbell
2014-01-24 15:31 ` Hans de Goede
2014-01-24 15:41   ` Ian Campbell
2014-01-25 11:03     ` Hans de Goede
2014-01-27 14:57       ` Maxime Ripard
2014-01-27 15:52         ` Ian Campbell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).