Re: [PATCH 4/4] memory: fix memory calculation

Re: [PATCH 4/4] memory: fix memory calculation

[Sascha Hauer]

On Mon, Oct 15, 2012 at 03:22:42PM +0200, Alexander Aring wrote:
> Fix memory calculation.
> 
> Signed-off-by: Alexander Aring <a.aring@phytec.de>
> ---
>  common/meminfo.c |    8 ++++----
>  common/memory.c  |    4 ++--
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/common/meminfo.c b/common/meminfo.c
> index b2b09da..13633a2 100644
> --- a/common/meminfo.c
> +++ b/common/meminfo.c
> @@ -15,9 +15,9 @@ static int display_meminfo(void)
>  #endif
>  
>  	debug("barebox code: 0x%p -> 0x%p (size 0x%08lx)\n",
> -			_stext, _etext, text_size);
> +			_stext, _etext - 1, text_size);
>  	debug("bss segment:  0x%p -> 0x%p (size 0x%08lx)\n",
> -		__bss_start, __bss_stop, bss_size);
> +		__bss_start, __bss_stop - 1, bss_size);
>  #ifdef DEBUG
>  	printf("malloc space: 0x%08lx -> 0x%08lx (size 0x%08lx)\n",
>  		mstart, mend, msize);
> @@ -28,11 +28,11 @@ static int display_meminfo(void)
>  #ifdef CONFIG_ARM
>  #ifdef DEBUG
>  	printf("stack space:  0x%08x -> 0x%08x (size 0x%08x)\n",
> -		STACK_BASE, STACK_BASE + STACK_SIZE,
> +		STACK_BASE, STACK_BASE + STACK_SIZE - 1,
>  		STACK_SIZE);
>  #else
>  	printf("stack space:  0x%08x -> 0x%08x (size %s)\n",
> -		STACK_BASE, STACK_BASE + STACK_SIZE,
> +		STACK_BASE, STACK_BASE + STACK_SIZE - 1,
>  		size_human_readable(STACK_SIZE));

Hm, you copied wrong code in an earlier patch in this series which
you fix afterwards. That's not good, fixes, should always come first.
This for example makes it possible to apply the fix to the master
branch and feature additions to -next.

Besides, printing the exact values for the debug case and
size_human_readable otherwise is unnecessary churn. All the
information above can be retrieved using the iomem command
anyway, so removing the code altogether seems more useful.

>  #endif
>  #endif
> diff --git a/common/memory.c b/common/memory.c
> index 8919ab1..6bf4af6 100644
> --- a/common/memory.c
> +++ b/common/memory.c
> @@ -74,11 +74,11 @@ static int mem_malloc_resource(void)
>  	request_sdram_region("barebox",
>  			(unsigned long)&_stext,
>  			(unsigned long)&_etext -
> -			(unsigned long)&_stext + 1);
> +			(unsigned long)&_stext);
>  	request_sdram_region("bss",
>  			(unsigned long)&__bss_start,
>  			(unsigned long)&__bss_stop -
> -			(unsigned long)&__bss_start + 1);
> +			(unsigned long)&__bss_start);

That's a useful fix. Can we have this as a separate patch?

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 4/4] memory: fix memory calculation

[Alexander Aring]

[-- Attachment #1.1: Type: text/plain, Size: 3872 bytes --]

Hi,

Thanks, I will do this changes.
Move fixes as first and separte the fix calculation patch in fixes for
'request_sdram_region' and 'printouts of iomem at boottime', which is only
necessary for correct eyecandy.

Sry, It's true to get all information from iomem, never used this command :)
I got ideas to improve my other patches... I will try to insert it, too.

I send this mail from my private mail address...
My phytec address doesn't work with the barebox mailinglist.

Regards
Alexander Aring

2012/10/16 Sascha Hauer <s.hauer@pengutronix.de>

> On Mon, Oct 15, 2012 at 03:22:42PM +0200, Alexander Aring wrote:
> > Fix memory calculation.
> >
> > Signed-off-by: Alexander Aring <a.aring@phytec.de>
> > ---
> >  common/meminfo.c |    8 ++++----
> >  common/memory.c  |    4 ++--
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/common/meminfo.c b/common/meminfo.c
> > index b2b09da..13633a2 100644
> > --- a/common/meminfo.c
> > +++ b/common/meminfo.c
> > @@ -15,9 +15,9 @@ static int display_meminfo(void)
> >  #endif
> >
> >       debug("barebox code: 0x%p -> 0x%p (size 0x%08lx)\n",
> > -                     _stext, _etext, text_size);
> > +                     _stext, _etext - 1, text_size);
> >       debug("bss segment:  0x%p -> 0x%p (size 0x%08lx)\n",
> > -             __bss_start, __bss_stop, bss_size);
> > +             __bss_start, __bss_stop - 1, bss_size);
> >  #ifdef DEBUG
> >       printf("malloc space: 0x%08lx -> 0x%08lx (size 0x%08lx)\n",
> >               mstart, mend, msize);
> > @@ -28,11 +28,11 @@ static int display_meminfo(void)
> >  #ifdef CONFIG_ARM
> >  #ifdef DEBUG
> >       printf("stack space:  0x%08x -> 0x%08x (size 0x%08x)\n",
> > -             STACK_BASE, STACK_BASE + STACK_SIZE,
> > +             STACK_BASE, STACK_BASE + STACK_SIZE - 1,
> >               STACK_SIZE);
> >  #else
> >       printf("stack space:  0x%08x -> 0x%08x (size %s)\n",
> > -             STACK_BASE, STACK_BASE + STACK_SIZE,
> > +             STACK_BASE, STACK_BASE + STACK_SIZE - 1,
> >               size_human_readable(STACK_SIZE));
>
> Hm, you copied wrong code in an earlier patch in this series which
> you fix afterwards. That's not good, fixes, should always come first.
> This for example makes it possible to apply the fix to the master
> branch and feature additions to -next.
>
> Besides, printing the exact values for the debug case and
> size_human_readable otherwise is unnecessary churn. All the
> information above can be retrieved using the iomem command
> anyway, so removing the code altogether seems more useful.
>
> >  #endif
> >  #endif
> > diff --git a/common/memory.c b/common/memory.c
> > index 8919ab1..6bf4af6 100644
> > --- a/common/memory.c
> > +++ b/common/memory.c
> > @@ -74,11 +74,11 @@ static int mem_malloc_resource(void)
> >       request_sdram_region("barebox",
> >                       (unsigned long)&_stext,
> >                       (unsigned long)&_etext -
> > -                     (unsigned long)&_stext + 1);
> > +                     (unsigned long)&_stext);
> >       request_sdram_region("bss",
> >                       (unsigned long)&__bss_start,
> >                       (unsigned long)&__bss_stop -
> > -                     (unsigned long)&__bss_start + 1);
> > +                     (unsigned long)&__bss_start);
>
> That's a useful fix. Can we have this as a separate patch?
>
> 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
>

[-- Attachment #1.2: Type: text/html, Size: 5382 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

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