Patchwork [U-Boot,v9,18/30] nand: mxc: Switch NAND SPL to generic SPL

login
register
mail settings
Submitter Fabio Estevam
Date April 9, 2013, 4:40 p.m.
Message ID <CAOMZO5DcLPXtpwovEAHPScmtiqmOi-4TPN3AjfRB4kMGm75p+w@mail.gmail.com>
Download mbox | patch
Permalink /patch/235144/
State Superseded
Delegated to: Stefano Babic
Headers show

Comments

Fabio Estevam - April 9, 2013, 4:40 p.m.
Hi Benoît,

On Tue, Apr 9, 2013 at 11:38 AM, Benoît Thébaudeau
<benoit.thebaudeau@advansee.com> wrote:

> Can you try different values for the following configs in mx31pdk.h?
>  - CONFIG_SYS_NAND_U_BOOT_SIZE
>  - CONFIG_SPL_TEXT_BASE
>  - CONFIG_SYS_TEXT_BASE
>  - (CONFIG_ENV_OFFSET)
>  - (CONFIG_ENV_OFFSET_REDUND)
>
> I would try:
>  - 0x40000 for CONFIG_SYS_NAND_U_BOOT_SIZE
>  - 0x86000000 for CONFIG_SPL_TEXT_BASE
>  - 0x87000000 for CONFIG_SYS_TEXT_BASE

Thanks, that did the trick!

With the changes below I am able to get into the U-boot prompt:
Benoît Thébaudeau - April 9, 2013, 4:56 p.m.
Hi Fabio,

On Tuesday, April 9, 2013 6:40:45 PM, Fabio Estevam wrote:
> Hi Benoît,
> 
> On Tue, Apr 9, 2013 at 11:38 AM, Benoît Thébaudeau
> <benoit.thebaudeau@advansee.com> wrote:
> 
> > Can you try different values for the following configs in mx31pdk.h?
> >  - CONFIG_SYS_NAND_U_BOOT_SIZE
> >  - CONFIG_SPL_TEXT_BASE
> >  - CONFIG_SYS_TEXT_BASE
> >  - (CONFIG_ENV_OFFSET)
> >  - (CONFIG_ENV_OFFSET_REDUND)
> >
> > I would try:
> >  - 0x40000 for CONFIG_SYS_NAND_U_BOOT_SIZE
> >  - 0x86000000 for CONFIG_SPL_TEXT_BASE
> >  - 0x87000000 for CONFIG_SYS_TEXT_BASE
> 
> Thanks, that did the trick!
> 
> With the changes below I am able to get into the U-boot prompt:
> 
> diff --git a/include/configs/mx31pdk.h b/include/configs/mx31pdk.h
> index 8f12825..d82bf65 100644
> --- a/include/configs/mx31pdk.h
> +++ b/include/configs/mx31pdk.h
> @@ -51,8 +51,8 @@
>  #define CONFIG_SPL_MAX_SIZE	2048
>  #define CONFIG_SPL_NAND_SUPPORT
> 
> -#define CONFIG_SPL_TEXT_BASE	0x87ec0000
> -#define CONFIG_SYS_TEXT_BASE	0x87f00000
> +#define CONFIG_SPL_TEXT_BASE	0x86000000
> +#define CONFIG_SYS_TEXT_BASE	0x87000000
> 
>  #ifndef CONFIG_SPL_BUILD
>  #define CONFIG_SKIP_LOWLEVEL_INIT
> @@ -69,8 +69,6 @@
> 
>  #define CONFIG_MXC_UART
>  #define CONFIG_MXC_UART_BASE	UART1_BASE
> -#define CONFIG_HW_WATCHDOG
> -#define CONFIG_IMX_WATCHDOG
>  #define CONFIG_MXC_GPIO
> 
>  #define CONFIG_HARD_SPI
> @@ -199,7 +197,7 @@
> 
>  /* Start copying real U-boot from the second page */
>  #define CONFIG_SYS_NAND_U_BOOT_OFFS	CONFIG_SPL_PAD_TO
> -#define CONFIG_SYS_NAND_U_BOOT_SIZE	0x32000
> +#define CONFIG_SYS_NAND_U_BOOT_SIZE	0x40000
>  /* Load U-Boot to this address */
>  #define CONFIG_SYS_NAND_U_BOOT_DST	CONFIG_SYS_TEXT_BASE
>  #define CONFIG_SYS_NAND_U_BOOT_START	CONFIG_SYS_NAND_U_BOOT_DST

Cool, then we need to determine the minimal change set required for v11, so can
you retry with fewer of those changes to determine which ones are strictly
necessary (apart from the watchdog removal that we already know is required)?

What is the size of u-boot-with-spl.bin without DEBUG?

Regarding the watchdog removal, it is unrelated to this patch, so do you want me
to add a patch for it in this series, or will you handle it separately? And is
the correct change to remove it as above, or to change its time-out somewhere? I
think that it's probably better if you take care of this separately.

Best regards,
Benoît
Benoît Thébaudeau - April 9, 2013, 5:37 p.m.
On Tuesday, April 9, 2013 6:56:18 PM, Benoît Thébaudeau wrote:
> Hi Fabio,
> 
> On Tuesday, April 9, 2013 6:40:45 PM, Fabio Estevam wrote:
> > Hi Benoît,
> > 
> > On Tue, Apr 9, 2013 at 11:38 AM, Benoît Thébaudeau
> > <benoit.thebaudeau@advansee.com> wrote:
> > 
> > > Can you try different values for the following configs in mx31pdk.h?
> > >  - CONFIG_SYS_NAND_U_BOOT_SIZE
> > >  - CONFIG_SPL_TEXT_BASE
> > >  - CONFIG_SYS_TEXT_BASE
> > >  - (CONFIG_ENV_OFFSET)
> > >  - (CONFIG_ENV_OFFSET_REDUND)
> > >
> > > I would try:
> > >  - 0x40000 for CONFIG_SYS_NAND_U_BOOT_SIZE
> > >  - 0x86000000 for CONFIG_SPL_TEXT_BASE
> > >  - 0x87000000 for CONFIG_SYS_TEXT_BASE
> > 
> > Thanks, that did the trick!
> > 
> > With the changes below I am able to get into the U-boot prompt:
> > 
> > diff --git a/include/configs/mx31pdk.h b/include/configs/mx31pdk.h
> > index 8f12825..d82bf65 100644
> > --- a/include/configs/mx31pdk.h
> > +++ b/include/configs/mx31pdk.h
> > @@ -51,8 +51,8 @@
> >  #define CONFIG_SPL_MAX_SIZE	2048
> >  #define CONFIG_SPL_NAND_SUPPORT
> > 
> > -#define CONFIG_SPL_TEXT_BASE	0x87ec0000
> > -#define CONFIG_SYS_TEXT_BASE	0x87f00000
> > +#define CONFIG_SPL_TEXT_BASE	0x86000000
> > +#define CONFIG_SYS_TEXT_BASE	0x87000000
> > 
> >  #ifndef CONFIG_SPL_BUILD
> >  #define CONFIG_SKIP_LOWLEVEL_INIT
> > @@ -69,8 +69,6 @@
> > 
> >  #define CONFIG_MXC_UART
> >  #define CONFIG_MXC_UART_BASE	UART1_BASE
> > -#define CONFIG_HW_WATCHDOG
> > -#define CONFIG_IMX_WATCHDOG
> >  #define CONFIG_MXC_GPIO
> > 
> >  #define CONFIG_HARD_SPI
> > @@ -199,7 +197,7 @@
> > 
> >  /* Start copying real U-boot from the second page */
> >  #define CONFIG_SYS_NAND_U_BOOT_OFFS	CONFIG_SPL_PAD_TO
> > -#define CONFIG_SYS_NAND_U_BOOT_SIZE	0x32000
> > +#define CONFIG_SYS_NAND_U_BOOT_SIZE	0x40000
> >  /* Load U-Boot to this address */
> >  #define CONFIG_SYS_NAND_U_BOOT_DST	CONFIG_SYS_TEXT_BASE
> >  #define CONFIG_SYS_NAND_U_BOOT_START	CONFIG_SYS_NAND_U_BOOT_DST
> 
> Cool, then we need to determine the minimal change set required for v11, so
> can
> you retry with fewer of those changes to determine which ones are strictly
> necessary (apart from the watchdog removal that we already know is required)?

Especially, does it work with only:
-#define CONFIG_HW_WATCHDOG
-#define CONFIG_IMX_WATCHDOG
-#define CONFIG_SYS_NAND_U_BOOT_SIZE	0x32000
+#define CONFIG_SYS_NAND_U_BOOT_SIZE	0x3f800
?

> What is the size of u-boot-with-spl.bin without DEBUG?
> 
> Regarding the watchdog removal, it is unrelated to this patch, so do you want
> me
> to add a patch for it in this series, or will you handle it separately? And
> is
> the correct change to remove it as above, or to change its time-out
> somewhere? I
> think that it's probably better if you take care of this separately.

Best regards,
Benoît
Fabio Estevam - April 9, 2013, 6:07 p.m.
On Tue, Apr 9, 2013 at 2:37 PM, Benoît Thébaudeau
<benoit.thebaudeau@advansee.com> wrote:

> Especially, does it work with only:
> -#define CONFIG_HW_WATCHDOG
> -#define CONFIG_IMX_WATCHDOG
> -#define CONFIG_SYS_NAND_U_BOOT_SIZE    0x32000
> +#define CONFIG_SYS_NAND_U_BOOT_SIZE    0x3f800

No, it hangs in "Net :". Also tried 0x40000 and the same hang happens.

Maybe we should use the proposed values you proposed earlier?

I can handle the watchdog patch separately.

Regards,

Fabio Estevam
Benoît Thébaudeau - April 9, 2013, 6:10 p.m.
Hi Fabio,

On Tuesday, April 9, 2013 8:07:31 PM, Fabio Estevam wrote:
> On Tue, Apr 9, 2013 at 2:37 PM, Benoît Thébaudeau
> <benoit.thebaudeau@advansee.com> wrote:
> 
> > Especially, does it work with only:
> > -#define CONFIG_HW_WATCHDOG
> > -#define CONFIG_IMX_WATCHDOG
> > -#define CONFIG_SYS_NAND_U_BOOT_SIZE    0x32000
> > +#define CONFIG_SYS_NAND_U_BOOT_SIZE    0x3f800
> 
> No, it hangs in "Net :". Also tried 0x40000 and the same hang happens.

OK.

> Maybe we should use the proposed values you proposed earlier?

These values were very wide, just for testing. It'd be better to adjust them. So
can you just try the following, and I'll stop asking for more tests:
-#define CONFIG_SPL_TEXT_BASE	0x87ec0000
-#define CONFIG_SYS_TEXT_BASE	0x87f00000
+#define CONFIG_SPL_TEXT_BASE	0x87dc0000
+#define CONFIG_SYS_TEXT_BASE	0x87e00000
-#define CONFIG_HW_WATCHDOG
-#define CONFIG_IMX_WATCHDOG
-#define CONFIG_SYS_NAND_U_BOOT_SIZE	0x32000
+#define CONFIG_SYS_NAND_U_BOOT_SIZE	0x3f800
?

> I can handle the watchdog patch separately.

OK.

Best regards,
Benoît
Benoît Thébaudeau - April 9, 2013, 6:31 p.m.
On Tuesday, April 9, 2013 8:35:09 PM, Fabio Estevam wrote:
> On Tue, Apr 9, 2013 at 3:10 PM, Benoît Thébaudeau
> <benoit.thebaudeau@advansee.com> wrote:
> 
> > These values were very wide, just for testing. It'd be better to adjust
> > them. So
> > can you just try the following, and I'll stop asking for more tests:
> > -#define CONFIG_SPL_TEXT_BASE   0x87ec0000
> > -#define CONFIG_SYS_TEXT_BASE   0x87f00000
> > +#define CONFIG_SPL_TEXT_BASE   0x87dc0000
> > +#define CONFIG_SYS_TEXT_BASE   0x87e00000
> > -#define CONFIG_HW_WATCHDOG
> > -#define CONFIG_IMX_WATCHDOG
> > -#define CONFIG_SYS_NAND_U_BOOT_SIZE    0x32000
> > +#define CONFIG_SYS_NAND_U_BOOT_SIZE    0x3f800
> 
> These values work fine, thanks.

Thanks for testing and for your patience.

Best regards,
Benoît
Fabio Estevam - April 9, 2013, 6:35 p.m.
On Tue, Apr 9, 2013 at 3:10 PM, Benoît Thébaudeau
<benoit.thebaudeau@advansee.com> wrote:

> These values were very wide, just for testing. It'd be better to adjust them. So
> can you just try the following, and I'll stop asking for more tests:
> -#define CONFIG_SPL_TEXT_BASE   0x87ec0000
> -#define CONFIG_SYS_TEXT_BASE   0x87f00000
> +#define CONFIG_SPL_TEXT_BASE   0x87dc0000
> +#define CONFIG_SYS_TEXT_BASE   0x87e00000
> -#define CONFIG_HW_WATCHDOG
> -#define CONFIG_IMX_WATCHDOG
> -#define CONFIG_SYS_NAND_U_BOOT_SIZE    0x32000
> +#define CONFIG_SYS_NAND_U_BOOT_SIZE    0x3f800

These values work fine, thanks.

Patch

diff --git a/include/configs/mx31pdk.h b/include/configs/mx31pdk.h
index 8f12825..d82bf65 100644
--- a/include/configs/mx31pdk.h
+++ b/include/configs/mx31pdk.h
@@ -51,8 +51,8 @@ 
 #define CONFIG_SPL_MAX_SIZE	2048
 #define CONFIG_SPL_NAND_SUPPORT

-#define CONFIG_SPL_TEXT_BASE	0x87ec0000
-#define CONFIG_SYS_TEXT_BASE	0x87f00000
+#define CONFIG_SPL_TEXT_BASE	0x86000000
+#define CONFIG_SYS_TEXT_BASE	0x87000000

 #ifndef CONFIG_SPL_BUILD
 #define CONFIG_SKIP_LOWLEVEL_INIT
@@ -69,8 +69,6 @@ 

 #define CONFIG_MXC_UART
 #define CONFIG_MXC_UART_BASE	UART1_BASE
-#define CONFIG_HW_WATCHDOG
-#define CONFIG_IMX_WATCHDOG
 #define CONFIG_MXC_GPIO

 #define CONFIG_HARD_SPI
@@ -199,7 +197,7 @@ 

 /* Start copying real U-boot from the second page */
 #define CONFIG_SYS_NAND_U_BOOT_OFFS	CONFIG_SPL_PAD_TO
-#define CONFIG_SYS_NAND_U_BOOT_SIZE	0x32000
+#define CONFIG_SYS_NAND_U_BOOT_SIZE	0x40000
 /* Load U-Boot to this address */
 #define CONFIG_SYS_NAND_U_BOOT_DST	CONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_NAND_U_BOOT_START	CONFIG_SYS_NAND_U_BOOT_DST