diff mbox

[U-Boot] Wandboard: u-boot.img is getting too large

Message ID 1467694422.3144.32.camel@synopsys.com
State Not Applicable
Delegated to: Stefano Babic
Headers show

Commit Message

Alexey Brodkin July 5, 2016, 4:54 a.m. UTC
Hello,

Recently I started to notice that u-boot.img built for Wandboard
by some toolchains becomes so large that it basically overlaps with
U-Boot environment area on SD-card.

According to http://wiki.wandboard.org/index.php/Boot-process#sdcard_boot_data_layout
Wandboard's SD-card layout is as follows:
------------------------------>8---------------------------
#  Offset (hex) (dec)   Description
------------------------------>8---------------------------

That might introduce incompatibility issues for those who used to modify
default environment but given nobody noticed the problem in question it
looks like there're not many of them.

Any thoughts are more than welcome.

-Alexey

Comments

Wolfgang Denk July 5, 2016, 6:11 a.m. UTC | #1
Dear Alexey,

In message <1467694422.3144.32.camel@synopsys.com> you wrote:
> 
> According to http://wiki.wandboard.org/index.php/Boot-process#sdcard_boot_data_layout
> Wandboard's SD-card layout is as follows:
> ------------------------------>8---------------------------
> #  Offset (hex) (dec)   Description
> ==========================================================
> 1. 0x00000000		Reserved For MBR
> 2. 0x00000200	512	Secondary Image Table (optional)
> 3. 0x00000400	1024	uBoot Image (Starting From IVT)
> 4. 0x00060000	393216	start of uboot env (size:8k)
> 5. 0x00062000		end of uboot env
> 6. 0x00100000	1048576	Linux kernel start
> 7. 0x0076AC00	7777280	start of partition 1

So they reserve 383 KiB for U-Boot, 8 kB for the environment, and then
leave an unused gap of 632 KiB?  That's silly.

> IMHO there's an obvious solution for all that - just move U-Boot's env for say
> another 300-400k above like that:

Why place the environment anywhere in the middle of that free space?
Just move it to the very end of it, i. e. make it

   Offset	Size	  Usage
1. 0x00000000	 512 B	  Reserved For MBR
2. 0x00000200	 512 B	  Secondary Image Table (optional)
3. 0x00000400	1015 KiB  U-Boot image(s)
4. 0x000FE000	   8 KiB  U-Boot environment
6. 0x00100000	...	  Linux kernel

Then you have all options open to grow even further, if needed, or
allocate other stuff below the env without leaving a gap.

Gaps in a memory map are always begging for trouble, expecially when
there is no reason for them.

Best regards,

Wolfgang Denk
Alexey Brodkin July 5, 2016, 6:19 a.m. UTC | #2
Hi Wolfgang,

On Tue, 2016-07-05 at 08:11 +0200, Wolfgang Denk wrote:
Dear Alexey,

In message <1467694422.3144.32.camel@synopsys.com<mailto:1467694422.3144.32.camel@synopsys.com>> you wrote:


According to http://wiki.wandboard.org/index.php/Boot-process#sdcard_boot_data_layout
Wandboard's SD-card layout is as follows:
------------------------------>8---------------------------
#  Offset (hex) (dec)   Description
==========================================================
1. 0x00000000 Reserved For MBR
2. 0x00000200 512 Secondary Image Table (optional)
3. 0x00000400 1024 uBoot Image (Starting From IVT)
4. 0x00060000 393216 start of uboot env (size:8k)
5. 0x00062000 end of uboot env
6. 0x00100000 1048576 Linux kernel start
7. 0x0076AC00 7777280 start of partition 1
So they reserve 383 KiB for U-Boot, 8 kB for the environment, and then
leave an unused gap of 632 KiB?  That's silly.


IMHO there's an obvious solution for all that - just move U-Boot's env for say
another 300-400k above like that:
Why place the environment anywhere in the middle of that free space?
Just move it to the very end of it, i. e. make it

   Offset Size   Usage
1. 0x00000000  512 B   Reserved For MBR
2. 0x00000200  512 B   Secondary Image Table (optional)
3. 0x00000400 1015 KiB  U-Boot image(s)
4. 0x000FE000    8 KiB  U-Boot environment
6. 0x00100000 ...   Linux kernel

Then you have all options open to grow even further, if needed, or
allocate other stuff below the env without leaving a gap.

Gaps in a memory map are always begging for trouble, expecially when
there is no reason for them.

Agree! IMHO solution to all that is simple.

I just want to get some feedback from people who maintain IMX/Wandboard boards
or at least has better visibility at how people use these boards and if there's any
rationale behind existing mapping.

If everybody is happy with proposal to move U-Boot env "partition" above I'll send a
formal patch. Or I may as well send it and if there's interest in it somebody will pick it
up and apply.

-Alexey
Fabio Estevam July 5, 2016, 11:49 a.m. UTC | #3
Hi Alexey,

On Tue, Jul 5, 2016 at 1:54 AM, Alexey Brodkin
<Alexey.Brodkin@synopsys.com> wrote:

> IMHO there's an obvious solution for all that - just move U-Boot's env for say
> another 300-400k above like that:
> ------------------------------>8---------------------------
> diff --git a/include/configs/wandboard.h b/include/configs/wandboard.h
> index 99f5c0c..e39583c 100644
> --- a/include/configs/wandboard.h
> +++ b/include/configs/wandboard.h
> @@ -181,7 +181,7 @@
>  #define CONFIG_ENV_SIZE                        (8 * 1024)
>
>  #define CONFIG_ENV_IS_IN_MMC
> -#define CONFIG_ENV_OFFSET              (6 * 64 * 1024)
> +#define CONFIG_ENV_OFFSET              (2 * 6 * 64 * 1024)
>  #define CONFIG_SYS_MMC_ENV_DEV         0
>
>  #endif
> ------------------------------>8---------------------------

Yes, please send this as a formal patch.

I fixed the same issue recently on mx6sabresd.

See commit 8fb9eea5653796 ("mx6sabre_common: Fix U-Boot corruption
after 'saveenv'")

Thanks
Tom Rini July 5, 2016, 3:17 p.m. UTC | #4
On Tue, Jul 05, 2016 at 08:49:00AM -0300, Fabio Estevam wrote:
> Hi Alexey,
> 
> On Tue, Jul 5, 2016 at 1:54 AM, Alexey Brodkin
> <Alexey.Brodkin@synopsys.com> wrote:
> 
> > IMHO there's an obvious solution for all that - just move U-Boot's env for say
> > another 300-400k above like that:
> > ------------------------------>8---------------------------
> > diff --git a/include/configs/wandboard.h b/include/configs/wandboard.h
> > index 99f5c0c..e39583c 100644
> > --- a/include/configs/wandboard.h
> > +++ b/include/configs/wandboard.h
> > @@ -181,7 +181,7 @@
> >  #define CONFIG_ENV_SIZE                        (8 * 1024)
> >
> >  #define CONFIG_ENV_IS_IN_MMC
> > -#define CONFIG_ENV_OFFSET              (6 * 64 * 1024)
> > +#define CONFIG_ENV_OFFSET              (2 * 6 * 64 * 1024)
> >  #define CONFIG_SYS_MMC_ENV_DEV         0
> >
> >  #endif
> > ------------------------------>8---------------------------
> 
> Yes, please send this as a formal patch.
> 
> I fixed the same issue recently on mx6sabresd.
> 
> See commit 8fb9eea5653796 ("mx6sabre_common: Fix U-Boot corruption
> after 'saveenv'")

I think this shows that we need to think about fixing this in one of the
more common mx6/7 header files as a "board can override" default.  And
maybe start thinking about moving the questions to Kconfig as well,
since tbot can make sure that the conversion is correct. :)
diff mbox

Patch

==========================================================
1. 0x00000000		Reserved For MBR
2. 0x00000200	512	Secondary Image Table (optional)
3. 0x00000400	1024	uBoot Image (Starting From IVT)
4. 0x00060000	393216	start of uboot env (size:8k)
5. 0x00062000		end of uboot env
6. 0x00100000	1048576	Linux kernel start
7. 0x0076AC00	7777280	start of partition 1
------------------------------>8---------------------------

So for U-Boot we have 383kB (392192 bytes).

But in up to date U-Boot for Wandboard we build separately
 a) SPL
 b) u-boot.img

which gives us a bit more detailed SD-card layout:
------------------------------>8---------------------------
#  Offset (hex) (dec)   Description
==========================
================================
1. 0x00000000		Reserved For MBR
2. 0x00000200	512	Secondary
Image Table (optional)
3. 0x00000400	1024	SPL
4. 0x00011400   70656	u-boot.img
5. 0x00060000	39
3216	start of uboot env (size:8k)
6. 0x00062000		end of uboot env
...
------------------------------
>8---------------------------

From that layout we may calculate amount of space reserved for u-boot.img.
It's just 315kb (322560 bytes).

Now if I build U-Boot with Sourcery CodeBench ARM 2014.05 produced u-boot.img
is already more than we expected (323840 bytes instead of "< 322560"):
------------------------------>8---------------------------
ls -la u-boot.img 
-rw-rw-r-- 1 user user 323840 Jul  5 07:38 u-boot.img
------------------------------>8---------------------------

Funny enough if I rebuild U-Boot with ARM toolchain available in
my Fedora 23 distro u-boot.img becomes a little bit smaller:
------------------------------>8---------------------------
ls -la u-boot.img 
-rw-rw-r-- 1 user user 322216 Jul  5 07:39 u-boot.img
------------------------------>8---------------------------

What's worse this problem might not affect people most of the time
because what happens people would just copy u-boot.img on SD-card and live
in happiness with it... well until somebody attempts to save environment
in U-Boot with "saveenv" command which will simply overwrite the very end of
u-boot.img. That will lead to unusable SD-card until user dd u-boot.img on
SD-card again.

I may foresee this issue in the future to become more visible once we add more
features in U-Boot for Wandboard or just existing code base becomes bulkier and
people will consistently get larger u-boot.img files produced.

IMHO there's an obvious solution for all that - just move U-Boot's env for say
another 300-400k above like that:
------------------------------>8---------------------------
diff --git a/include/configs/wandboard.h b/include/configs/wandboard.h
index 99f5c0c..e39583c 100644
--- a/include/configs/wandboard.h
+++ b/include/configs/wandboard.h
@@ -181,7 +181,7 @@ 
 #define CONFIG_ENV_SIZE                        (8 * 1024)
 
 #define CONFIG_ENV_IS_IN_MMC
-#define CONFIG_ENV_OFFSET              (6 * 64 * 1024)
+#define CONFIG_ENV_OFFSET              (2 * 6 * 64 * 1024)
 #define CONFIG_SYS_MMC_ENV_DEV         0
 
 #endif