* [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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ messages in thread
end of thread, other threads:[~2013-02-16 15:54 UTC | newest] Thread overview: 10+ 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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox