mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH] cfa10036: Retrieve the RAM size at runtime
@ 2013-02-13 16:50 Maxime Ripard
  2013-02-13 17:09 ` Jean-Christophe PLAGNIOL-VILLARD
  2013-02-13 17:16 ` [PATCH] Add warning above get_ram_size Sascha Hauer
  0 siblings, 2 replies; 15+ messages in thread
From: Maxime Ripard @ 2013-02-13 16:50 UTC (permalink / raw)
  To: barebox; +Cc: Brian Lilly

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

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

* Re: [PATCH] cfa10036: Retrieve the RAM size at runtime
  2013-02-13 16:50 [PATCH] cfa10036: Retrieve the RAM size at runtime Maxime Ripard
@ 2013-02-13 17:09 ` Jean-Christophe PLAGNIOL-VILLARD
  2013-02-14  9:42   ` Maxime Ripard
  2013-02-13 17:16 ` [PATCH] Add warning above get_ram_size Sascha Hauer
  1 sibling, 1 reply; 15+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-02-13 17:09 UTC (permalink / raw)
  To: Maxime Ripard; +Cc: barebox, Brian Lilly

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

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

* [PATCH] Add warning above get_ram_size
  2013-02-13 16:50 [PATCH] cfa10036: Retrieve the RAM size at runtime Maxime Ripard
  2013-02-13 17:09 ` Jean-Christophe PLAGNIOL-VILLARD
@ 2013-02-13 17:16 ` Sascha Hauer
  2013-02-14  9:40   ` Maxime Ripard
  1 sibling, 1 reply; 15+ messages in thread
From: Sascha Hauer @ 2013-02-13 17:16 UTC (permalink / raw)
  To: barebox

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

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

* Re: [PATCH] Add warning above get_ram_size
  2013-02-13 17:16 ` [PATCH] Add warning above get_ram_size Sascha Hauer
@ 2013-02-14  9:40   ` Maxime Ripard
  2013-02-14 11:35     ` Sascha Hauer
  0 siblings, 1 reply; 15+ messages in thread
From: Maxime Ripard @ 2013-02-14  9:40 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

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

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

* Re: [PATCH] cfa10036: Retrieve the RAM size at runtime
  2013-02-13 17:09 ` Jean-Christophe PLAGNIOL-VILLARD
@ 2013-02-14  9:42   ` Maxime Ripard
  0 siblings, 0 replies; 15+ messages in thread
From: Maxime Ripard @ 2013-02-14  9:42 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox, Brian Lilly

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

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

* Re: [PATCH] Add warning above get_ram_size
  2013-02-14  9:40   ` Maxime Ripard
@ 2013-02-14 11:35     ` Sascha Hauer
  2013-02-14 13:59       ` Maxime Ripard
  0 siblings, 1 reply; 15+ messages in thread
From: Sascha Hauer @ 2013-02-14 11:35 UTC (permalink / raw)
  To: Maxime Ripard; +Cc: barebox

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

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

* Re: [PATCH] Add warning above get_ram_size
  2013-02-14 11:35     ` Sascha Hauer
@ 2013-02-14 13:59       ` Maxime Ripard
  2013-02-14 19:13         ` Sascha Hauer
  0 siblings, 1 reply; 15+ messages in thread
From: Maxime Ripard @ 2013-02-14 13:59 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

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

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

* Re: [PATCH] Add warning above get_ram_size
  2013-02-14 13:59       ` Maxime Ripard
@ 2013-02-14 19:13         ` Sascha Hauer
  2013-02-16  8:45           ` Re[2]: " Alexander Shiyan
  0 siblings, 1 reply; 15+ messages in thread
From: Sascha Hauer @ 2013-02-14 19:13 UTC (permalink / raw)
  To: Maxime Ripard; +Cc: barebox

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

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

* Re[2]: [PATCH] Add warning above get_ram_size
  2013-02-14 19:13         ` Sascha Hauer
@ 2013-02-16  8:45           ` Alexander Shiyan
  2013-02-16 15:53             ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 1 reply; 15+ messages in thread
From: Alexander Shiyan @ 2013-02-16  8:45 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox, Maxime Ripard

...
> > >>> --- 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

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

* Re: [PATCH] Add warning above get_ram_size
  2013-02-16  8:45           ` Re[2]: " Alexander Shiyan
@ 2013-02-16 15:53             ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 0 replies; 15+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-02-16 15:53 UTC (permalink / raw)
  To: Alexander Shiyan; +Cc: barebox, Maxime Ripard

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

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

* Re: [PATCH] cfa10036: Retrieve the RAM size at runtime
  2013-03-25 12:58   ` Alexandre Belloni
@ 2013-03-25 13:02     ` Maxime Ripard
  0 siblings, 0 replies; 15+ messages in thread
From: Maxime Ripard @ 2013-03-25 13:02 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: barebox

HI,

Le 25/03/2013 13:58, Alexandre Belloni a écrit :
> On 25/03/2013 13:51, Sascha Hauer wrote:
>>
>> index 1bc20cf..37cc17e 100644
>> --- a/arch/arm/boards/crystalfontz-cfa10036/cfa10036.c
>> +++ b/arch/arm/boards/crystalfontz-cfa10036/cfa10036.c
>> @@ -39,6 +39,8 @@
>>  #include <asm/armlinux.h>
>>  #include <asm/mmu.h>
>>  
>> +#include <asm-generic/sections.h>
>> +
>>  #include <mach/fb.h>
>>  
>>  #include <generated/mach-types.h>
>> @@ -90,9 +92,77 @@ static struct i2c_gpio_platform_data i2c_gpio_pdata = {
>>  	.udelay			= 5,		/* ~100 kHz */
>>  };
>>  
>> +/*
>> + * Copied from get_ram_size in common/memory.c
>> + */
>> +long cfa10036_get_ram_size(volatile long *base, long maxsize)
>> +{
>> When I asked for a local version of this function I had something like
>> this in mind, not a complete copy of the function.
>>
>> 	volatile u32 *base = (void *)IMX_MEMORY_BASE;
>> 	volatile u32 *ofs = base + SZ_128M;
>>
>> 	*base = *ofs = 0xdeadbeef;
>>
>> 	*ofs = 0x11223344;
>> 	if (*base == 0x11223344)
>> 		return SZ_128M;
>> 	else
>> 		return SZ_256M;
>>
> 
> Yeah, I was also thinking about stripping it. But, for the sake of being
> future proof, I finally chose to keep the full version.
> 
> Maybe, we can assume that we will never have less than 128M of ram if
> you want to simplify it ?
> 
> Maxime, any input ?

For now, we only have 128M or 256M, so I guess it's a safe assumption, yes.

Maxime


-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

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

* Re: [PATCH] cfa10036: Retrieve the RAM size at runtime
  2013-03-25 12:51 ` Sascha Hauer
@ 2013-03-25 12:58   ` Alexandre Belloni
  2013-03-25 13:02     ` Maxime Ripard
  0 siblings, 1 reply; 15+ messages in thread
From: Alexandre Belloni @ 2013-03-25 12:58 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox, Maxime Ripard

On 25/03/2013 13:51, Sascha Hauer wrote:
>
> index 1bc20cf..37cc17e 100644
> --- a/arch/arm/boards/crystalfontz-cfa10036/cfa10036.c
> +++ b/arch/arm/boards/crystalfontz-cfa10036/cfa10036.c
> @@ -39,6 +39,8 @@
>  #include <asm/armlinux.h>
>  #include <asm/mmu.h>
>  
> +#include <asm-generic/sections.h>
> +
>  #include <mach/fb.h>
>  
>  #include <generated/mach-types.h>
> @@ -90,9 +92,77 @@ static struct i2c_gpio_platform_data i2c_gpio_pdata = {
>  	.udelay			= 5,		/* ~100 kHz */
>  };
>  
> +/*
> + * Copied from get_ram_size in common/memory.c
> + */
> +long cfa10036_get_ram_size(volatile long *base, long maxsize)
> +{
> When I asked for a local version of this function I had something like
> this in mind, not a complete copy of the function.
>
> 	volatile u32 *base = (void *)IMX_MEMORY_BASE;
> 	volatile u32 *ofs = base + SZ_128M;
>
> 	*base = *ofs = 0xdeadbeef;
>
> 	*ofs = 0x11223344;
> 	if (*base == 0x11223344)
> 		return SZ_128M;
> 	else
> 		return SZ_256M;
>

Yeah, I was also thinking about stripping it. But, for the sake of being
future proof, I finally chose to keep the full version.

Maybe, we can assume that we will never have less than 128M of ram if
you want to simplify it ?

Maxime, any input ?

Regards,

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


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

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

* Re: [PATCH] cfa10036: Retrieve the RAM size at runtime
  2013-03-21 15:36 [PATCH] cfa10036: Retrieve the RAM size at runtime Alexandre Belloni
  2013-03-25 11:26 ` Maxime Ripard
@ 2013-03-25 12:51 ` Sascha Hauer
  2013-03-25 12:58   ` Alexandre Belloni
  1 sibling, 1 reply; 15+ messages in thread
From: Sascha Hauer @ 2013-03-25 12:51 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: barebox, Maxime Ripard

On Thu, Mar 21, 2013 at 04:36:38PM +0100, Alexandre Belloni 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 by
> introducing the cfa10036_get_ram_size function which is similar to
> get_ram_size. As we run from RAM, we can then use _text and __bss_stop
> to prevent poking in the barebox memory which is not supported on other
> platforms.
> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  arch/arm/boards/crystalfontz-cfa10036/cfa10036.c |   72 +++++++++++++++++++++-
>  1 file changed, 71 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boards/crystalfontz-cfa10036/cfa10036.c b/arch/arm/boards/crystalfontz-cfa10036/cfa10036.c
> index 1bc20cf..37cc17e 100644
> --- a/arch/arm/boards/crystalfontz-cfa10036/cfa10036.c
> +++ b/arch/arm/boards/crystalfontz-cfa10036/cfa10036.c
> @@ -39,6 +39,8 @@
>  #include <asm/armlinux.h>
>  #include <asm/mmu.h>
>  
> +#include <asm-generic/sections.h>
> +
>  #include <mach/fb.h>
>  
>  #include <generated/mach-types.h>
> @@ -90,9 +92,77 @@ static struct i2c_gpio_platform_data i2c_gpio_pdata = {
>  	.udelay			= 5,		/* ~100 kHz */
>  };
>  
> +/*
> + * Copied from get_ram_size in common/memory.c
> + */
> +long cfa10036_get_ram_size(volatile long *base, long maxsize)
> +{

When I asked for a local version of this function I had something like
this in mind, not a complete copy of the function.

	volatile u32 *base = (void *)IMX_MEMORY_BASE;
	volatile u32 *ofs = base + SZ_128M;

	*base = *ofs = 0xdeadbeef;

	*ofs = 0x11223344;
	if (*base == 0x11223344)
		return SZ_128M;
	else
		return SZ_256M;


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

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

* Re: [PATCH] cfa10036: Retrieve the RAM size at runtime
  2013-03-21 15:36 [PATCH] cfa10036: Retrieve the RAM size at runtime Alexandre Belloni
@ 2013-03-25 11:26 ` Maxime Ripard
  2013-03-25 12:51 ` Sascha Hauer
  1 sibling, 0 replies; 15+ messages in thread
From: Maxime Ripard @ 2013-03-25 11:26 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: barebox

Hi Alexandre,

Le 21/03/2013 16:36, Alexandre Belloni a écrit :
> 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 by
> introducing the cfa10036_get_ram_size function which is similar to
> get_ram_size. As we run from RAM, we can then use _text and __bss_stop
> to prevent poking in the barebox memory which is not supported on other
> platforms.

Just tested it on my 256MB cfa-10036, and it works fine, you can add my
Tested-by

Maxime
-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

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

* [PATCH] cfa10036: Retrieve the RAM size at runtime
@ 2013-03-21 15:36 Alexandre Belloni
  2013-03-25 11:26 ` Maxime Ripard
  2013-03-25 12:51 ` Sascha Hauer
  0 siblings, 2 replies; 15+ messages in thread
From: Alexandre Belloni @ 2013-03-21 15:36 UTC (permalink / raw)
  To: barebox; +Cc: 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 by
introducing the cfa10036_get_ram_size function which is similar to
get_ram_size. As we run from RAM, we can then use _text and __bss_stop
to prevent poking in the barebox memory which is not supported on other
platforms.

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boards/crystalfontz-cfa10036/cfa10036.c |   72 +++++++++++++++++++++-
 1 file changed, 71 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boards/crystalfontz-cfa10036/cfa10036.c b/arch/arm/boards/crystalfontz-cfa10036/cfa10036.c
index 1bc20cf..37cc17e 100644
--- a/arch/arm/boards/crystalfontz-cfa10036/cfa10036.c
+++ b/arch/arm/boards/crystalfontz-cfa10036/cfa10036.c
@@ -39,6 +39,8 @@
 #include <asm/armlinux.h>
 #include <asm/mmu.h>
 
+#include <asm-generic/sections.h>
+
 #include <mach/fb.h>
 
 #include <generated/mach-types.h>
@@ -90,9 +92,77 @@ static struct i2c_gpio_platform_data i2c_gpio_pdata = {
 	.udelay			= 5,		/* ~100 kHz */
 };
 
+/*
+ * Copied from get_ram_size in common/memory.c
+ */
+long cfa10036_get_ram_size(volatile long *base, long maxsize)
+{
+	volatile long *addr;
+	long           save[32];
+	long           cnt;
+	long           val;
+	long           size;
+	int            i = 0;
+
+	for (cnt = (maxsize / sizeof (long)) >> 1; cnt > 0; cnt >>= 1) {
+		addr = base + cnt;	/* pointer arith! */
+
+		/*
+		 * If we run get_ram_size from RAM, avoid poking into
+		 * the Barebox code, and if the RAM at these address
+		 * doesn't work, we will have trouble anyway...
+		 */
+		if (addr > (long*)_text && addr < (long*)__bss_stop)
+			continue;
+
+		save[i++] = *addr;
+		*addr = ~cnt;
+	}
+
+	addr = base;
+	save[i] = *addr;
+	*addr = 0;
+
+	if ((val = *addr) != 0) {
+		/* Restore the original data before leaving the function.
+		 */
+		*addr = save[i];
+		for (cnt = 1; cnt < maxsize / sizeof(long); cnt <<= 1) {
+			addr  = base + cnt;
+			*addr = save[--i];
+		}
+		return (0);
+	}
+
+	for (cnt = 1; cnt < maxsize / sizeof (long); cnt <<= 1) {
+		addr = base + cnt;	/* pointer arith! */
+		if (addr > (long*)_text && addr < (long*)__bss_stop)
+			continue;
+
+		val = *addr;
+		*addr = save[--i];
+		if (val != ~cnt) {
+			size = cnt * sizeof (long);
+			/* Restore the original data before leaving the function.
+			 */
+			for (cnt <<= 1; cnt < maxsize / sizeof (long); cnt <<= 1) {
+				addr  = base + cnt;
+				if (addr > (long*)_text && addr < (long*)__bss_stop)
+					continue;
+
+				*addr = save[--i];
+			}
+			return (size);
+		}
+	}
+
+	return (maxsize);
+}
+
 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,
+			   cfa10036_get_ram_size((volatile long int *)IMX_MEMORY_BASE, SZ_256M));
 
 	return 0;
 }
-- 
1.7.10.4


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

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

end of thread, other threads:[~2013-03-25 13:02 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-13 16:50 [PATCH] cfa10036: Retrieve the RAM size at runtime Maxime Ripard
2013-02-13 17:09 ` Jean-Christophe PLAGNIOL-VILLARD
2013-02-14  9:42   ` Maxime Ripard
2013-02-13 17:16 ` [PATCH] Add warning above get_ram_size Sascha Hauer
2013-02-14  9:40   ` Maxime Ripard
2013-02-14 11:35     ` Sascha Hauer
2013-02-14 13:59       ` Maxime Ripard
2013-02-14 19:13         ` Sascha Hauer
2013-02-16  8:45           ` Re[2]: " Alexander Shiyan
2013-02-16 15:53             ` Jean-Christophe PLAGNIOL-VILLARD
2013-03-21 15:36 [PATCH] cfa10036: Retrieve the RAM size at runtime Alexandre Belloni
2013-03-25 11:26 ` Maxime Ripard
2013-03-25 12:51 ` Sascha Hauer
2013-03-25 12:58   ` Alexandre Belloni
2013-03-25 13:02     ` Maxime Ripard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox