[PATCH] cfa10036: Retrieve the RAM size at runtime

[PATCH] cfa10036: Retrieve the RAM size at runtime

[Maxime Ripard]

The cfa-10036 comes in two flavours, with either 128MB or 256MB of RAM
on it.

Since it's not stored anywhere, we need to runtime detect it, thanks to
the get_ram_size function.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boards/crystalfontz-cfa10036/cfa10036.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boards/crystalfontz-cfa10036/cfa10036.c b/arch/arm/boards/crystalfontz-cfa10036/cfa10036.c
index 1821b10..47a9520 100644
--- a/arch/arm/boards/crystalfontz-cfa10036/cfa10036.c
+++ b/arch/arm/boards/crystalfontz-cfa10036/cfa10036.c
@@ -91,7 +91,8 @@ static struct i2c_gpio_platform_data i2c_gpio_pdata = {
 
 static int cfa10036_mem_init(void)
 {
-	arm_add_mem_device("ram0", IMX_MEMORY_BASE, 128 * 1024 * 1024);
+	arm_add_mem_device("ram0", IMX_MEMORY_BASE,
+			get_ram_size(IMX_MEMORY_BASE, 256 * SZ_1M));
 
 	return 0;
 }
-- 
1.7.10.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] cfa10036: Retrieve the RAM size at runtime

[Jean-Christophe PLAGNIOL-VILLARD]

On 17:50 Wed 13 Feb     , Maxime Ripard wrote:
> The cfa-10036 comes in two flavours, with either 128MB or 256MB of RAM
> on it.
> 
> Since it's not stored anywhere, we need to runtime detect it, thanks to
> the get_ram_size function.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  arch/arm/boards/crystalfontz-cfa10036/cfa10036.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boards/crystalfontz-cfa10036/cfa10036.c b/arch/arm/boards/crystalfontz-cfa10036/cfa10036.c
> index 1821b10..47a9520 100644
> --- a/arch/arm/boards/crystalfontz-cfa10036/cfa10036.c
> +++ b/arch/arm/boards/crystalfontz-cfa10036/cfa10036.c
> @@ -91,7 +91,8 @@ static struct i2c_gpio_platform_data i2c_gpio_pdata = {
>  
>  static int cfa10036_mem_init(void)
>  {
> -	arm_add_mem_device("ram0", IMX_MEMORY_BASE, 128 * 1024 * 1024);
> +	arm_add_mem_device("ram0", IMX_MEMORY_BASE,
> +			get_ram_size(IMX_MEMORY_BASE, 256 * SZ_1M));
check the sdram control register instead so you do not need to get_ram_size
and specify a Max

Best Regards,
J.
>  
>  	return 0;
>  }
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH] Add warning above get_ram_size

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/memsize.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/common/memsize.c b/common/memsize.c
index d149e41..ef6381b 100644
--- a/common/memsize.c
+++ b/common/memsize.c
@@ -33,6 +33,9 @@
  * Check memory range for valid RAM. A simple memory test determines
  * the actually available RAM size between addresses `base' and
  * `base + maxsize'.
+ *
+ * This function modifies the RAM. Do not use it if you're running from
+ * the RAM you are going to detect!
  */
 long get_ram_size(volatile long *base, long maxsize)
 {
-- 
1.7.10.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Add warning above get_ram_size

[Maxime Ripard]

Hi Sascha,

Le 13/02/2013 18:16, Sascha Hauer a écrit :
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  common/memsize.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/common/memsize.c b/common/memsize.c
> index d149e41..ef6381b 100644
> --- a/common/memsize.c
> +++ b/common/memsize.c
> @@ -33,6 +33,9 @@
>   * Check memory range for valid RAM. A simple memory test determines
>   * the actually available RAM size between addresses `base' and
>   * `base + maxsize'.
> + *
> + * This function modifies the RAM. Do not use it if you're running from
> + * the RAM you are going to detect!
>   */

Actually, I don't see how it modifies the RAM, at least permanently. The
values it erase are backed up, and there's no concurrency at barebox
level, so we are sure that the value saved will still be the one that
would need to be backed up at the end of the function, right?

Maxime

-- 
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] cfa10036: Retrieve the RAM size at runtime

[Maxime Ripard]

Hi Jean-Christophe,

Le 13/02/2013 18:09, Jean-Christophe PLAGNIOL-VILLARD a écrit :
> On 17:50 Wed 13 Feb     , Maxime Ripard wrote:
>> The cfa-10036 comes in two flavours, with either 128MB or 256MB of RAM
>> on it.
>>
>> Since it's not stored anywhere, we need to runtime detect it, thanks to
>> the get_ram_size function.
>>
>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> ---
>>  arch/arm/boards/crystalfontz-cfa10036/cfa10036.c |    3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boards/crystalfontz-cfa10036/cfa10036.c b/arch/arm/boards/crystalfontz-cfa10036/cfa10036.c
>> index 1821b10..47a9520 100644
>> --- a/arch/arm/boards/crystalfontz-cfa10036/cfa10036.c
>> +++ b/arch/arm/boards/crystalfontz-cfa10036/cfa10036.c
>> @@ -91,7 +91,8 @@ static struct i2c_gpio_platform_data i2c_gpio_pdata = {
>>  
>>  static int cfa10036_mem_init(void)
>>  {
>> -	arm_add_mem_device("ram0", IMX_MEMORY_BASE, 128 * 1024 * 1024);
>> +	arm_add_mem_device("ram0", IMX_MEMORY_BASE,
>> +			get_ram_size(IMX_MEMORY_BASE, 256 * SZ_1M));
> check the sdram control register instead so you do not need to get_ram_size
> and specify a Max

This is not possible, the first stage bootloader sets up the cs, lines
and rows for 256MB, in every case, so we will always end up with 256MB here.

Maxime

-- 
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Add warning above get_ram_size

[Sascha Hauer]

On Thu, Feb 14, 2013 at 10:40:38AM +0100, Maxime Ripard wrote:
> Hi Sascha,
> 
> Le 13/02/2013 18:16, Sascha Hauer a écrit :
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  common/memsize.c |    3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/common/memsize.c b/common/memsize.c
> > index d149e41..ef6381b 100644
> > --- a/common/memsize.c
> > +++ b/common/memsize.c
> > @@ -33,6 +33,9 @@
> >   * Check memory range for valid RAM. A simple memory test determines
> >   * the actually available RAM size between addresses `base' and
> >   * `base + maxsize'.
> > + *
> > + * This function modifies the RAM. Do not use it if you're running from
> > + * the RAM you are going to detect!
> >   */
> 
> Actually, I don't see how it modifies the RAM, at least permanently. The
> values it erase are backed up, and there's no concurrency at barebox
> level, so we are sure that the value saved will still be the one that
> would need to be backed up at the end of the function, right?

Yes, it restores the values, but how do you make sure the function does
not modify the instructions you are currently executing? You need bad
luck to hit this, but sooner or later this will happen.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Add warning above get_ram_size

[Maxime Ripard]

Hi Sascha,

Le 14/02/2013 12:35, Sascha Hauer a écrit :
> On Thu, Feb 14, 2013 at 10:40:38AM +0100, Maxime Ripard wrote:
>> Hi Sascha,
>>
>> Le 13/02/2013 18:16, Sascha Hauer a écrit :
>>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>>> ---
>>>  common/memsize.c |    3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/common/memsize.c b/common/memsize.c
>>> index d149e41..ef6381b 100644
>>> --- a/common/memsize.c
>>> +++ b/common/memsize.c
>>> @@ -33,6 +33,9 @@
>>>   * Check memory range for valid RAM. A simple memory test determines
>>>   * the actually available RAM size between addresses `base' and
>>>   * `base + maxsize'.
>>> + *
>>> + * This function modifies the RAM. Do not use it if you're running from
>>> + * the RAM you are going to detect!
>>>   */
>>
>> Actually, I don't see how it modifies the RAM, at least permanently. The
>> values it erase are backed up, and there's no concurrency at barebox
>> level, so we are sure that the value saved will still be the one that
>> would need to be backed up at the end of the function, right?
> 
> Yes, it restores the values, but how do you make sure the function does
> not modify the instructions you are currently executing? You need bad
> luck to hit this, but sooner or later this will happen.

Ah, yes, this would be nasty indeed. Is there a way to know the end
address of barebox into RAM ? or the address it has been loaded to and
the size of its binary, so that we can just check the part that doesn't
hold barebox?

Maxime

-- 
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Add warning above get_ram_size

[Sascha Hauer]

On Thu, Feb 14, 2013 at 02:59:48PM +0100, Maxime Ripard wrote:
> Hi Sascha,
> 
> Le 14/02/2013 12:35, Sascha Hauer a écrit :
> > On Thu, Feb 14, 2013 at 10:40:38AM +0100, Maxime Ripard wrote:
> >> Hi Sascha,
> >>
> >> Le 13/02/2013 18:16, Sascha Hauer a écrit :
> >>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> >>> ---
> >>>  common/memsize.c |    3 +++
> >>>  1 file changed, 3 insertions(+)
> >>>
> >>> diff --git a/common/memsize.c b/common/memsize.c
> >>> index d149e41..ef6381b 100644
> >>> --- a/common/memsize.c
> >>> +++ b/common/memsize.c
> >>> @@ -33,6 +33,9 @@
> >>>   * Check memory range for valid RAM. A simple memory test determines
> >>>   * the actually available RAM size between addresses `base' and
> >>>   * `base + maxsize'.
> >>> + *
> >>> + * This function modifies the RAM. Do not use it if you're running from
> >>> + * the RAM you are going to detect!
> >>>   */
> >>
> >> Actually, I don't see how it modifies the RAM, at least permanently. The
> >> values it erase are backed up, and there's no concurrency at barebox
> >> level, so we are sure that the value saved will still be the one that
> >> would need to be backed up at the end of the function, right?
> > 
> > Yes, it restores the values, but how do you make sure the function does
> > not modify the instructions you are currently executing? You need bad
> > luck to hit this, but sooner or later this will happen.
> 
> Ah, yes, this would be nasty indeed. Is there a way to know the end
> address of barebox into RAM ? or the address it has been loaded to and
> the size of its binary, so that we can just check the part that doesn't
> hold barebox?

See include/asm-generic/sections.h. You have to avoid modifying
everything between _text and __bss_stop. I haven't looked how exactly
get_dram_size works. Normally this function would have to test every
location at a power of 2, that would be:

1 2 4 ... 64MiB 128MiB

It seems you have to make sure that your binary does not cross a power
of 2 boundary, then you should be safe.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re[2]: [PATCH] Add warning above get_ram_size

[Alexander Shiyan]

...
> > >>> --- a/common/memsize.c
> > >>> +++ b/common/memsize.c
> > >>> @@ -33,6 +33,9 @@
> > >>>   * Check memory range for valid RAM. A simple memory test determines
> > >>>   * the actually available RAM size between addresses `base' and
> > >>>   * `base + maxsize'.
> > >>> + *
> > >>> + * This function modifies the RAM. Do not use it if you're running from
> > >>> + * the RAM you are going to detect!
> > >>>   */
> > >>
> > >> Actually, I don't see how it modifies the RAM, at least permanently. The
> > >> values it erase are backed up, and there's no concurrency at barebox
> > >> level, so we are sure that the value saved will still be the one that
> > >> would need to be backed up at the end of the function, right?
> > > 
> > > Yes, it restores the values, but how do you make sure the function does
> > > not modify the instructions you are currently executing? You need bad
> > > luck to hit this, but sooner or later this will happen.
> > 
> > Ah, yes, this would be nasty indeed. Is there a way to know the end
> > address of barebox into RAM ? or the address it has been loaded to and
> > the size of its binary, so that we can just check the part that doesn't
> > hold barebox?
> 
> See include/asm-generic/sections.h. You have to avoid modifying
> everything between _text and __bss_stop. I haven't looked how exactly
> get_dram_size works. Normally this function would have to test every
> location at a power of 2, that would be:
> 
> 1 2 4 ... 64MiB 128MiB
> 
> It seems you have to make sure that your binary does not cross a power
> of 2 boundary, then you should be safe.

Let's put "get_ram_size" function in a separate section inside .text. Then we
can at least make runtime warning about placing this section inside our
tested region.

---
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Add warning above get_ram_size

[Jean-Christophe PLAGNIOL-VILLARD]

On 12:45 Sat 16 Feb     , Alexander Shiyan wrote:
> ...
> > > >>> --- a/common/memsize.c
> > > >>> +++ b/common/memsize.c
> > > >>> @@ -33,6 +33,9 @@
> > > >>>   * Check memory range for valid RAM. A simple memory test determines
> > > >>>   * the actually available RAM size between addresses `base' and
> > > >>>   * `base + maxsize'.
> > > >>> + *
> > > >>> + * This function modifies the RAM. Do not use it if you're running from
> > > >>> + * the RAM you are going to detect!
> > > >>>   */
> > > >>
> > > >> Actually, I don't see how it modifies the RAM, at least permanently. The
> > > >> values it erase are backed up, and there's no concurrency at barebox
> > > >> level, so we are sure that the value saved will still be the one that
> > > >> would need to be backed up at the end of the function, right?
> > > > 
> > > > Yes, it restores the values, but how do you make sure the function does
> > > > not modify the instructions you are currently executing? You need bad
> > > > luck to hit this, but sooner or later this will happen.
> > > 
> > > Ah, yes, this would be nasty indeed. Is there a way to know the end
> > > address of barebox into RAM ? or the address it has been loaded to and
> > > the size of its binary, so that we can just check the part that doesn't
> > > hold barebox?
> > 
> > See include/asm-generic/sections.h. You have to avoid modifying
> > everything between _text and __bss_stop. I haven't looked how exactly
> > get_dram_size works. Normally this function would have to test every
> > location at a power of 2, that would be:
> > 
> > 1 2 4 ... 64MiB 128MiB
> > 
> > It seems you have to make sure that your binary does not cross a power
> > of 2 boundary, then you should be safe.
> 
> Let's put "get_ram_size" function in a separate section inside .text. Then we
> can at least make runtime warning about placing this section inside our
> tested region.

we have 2 case

1: we run from the relocated position
2: we run before relocation

in any case you need to exclude where running from or just need to be carefull
when use it

Best Regards,
J.
> 
> ---
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox