Re: [U-Boot] [PATCH 12/12] imx: ventana: switch to SPL

2014-05-15 Thread Stefano Babic
Hi Tim,

On 15/05/2014 00:32, Tim Harvey wrote:

 
 I figured this one out - it has nothing to do with the order of
 calling arch_cpu_init() its that the MMDC isn't always 'ready' by the
 time the BSS is cleared and thus in my failure case the BSS isn't
 getting entirely cleared which causes the spl_image global var to not
 be cleared as expected and triggers an invalid codepath. I will update
 the mmdc config patch when I find the right solution.


Thanks for the explanation ! I cannot guess why arch_cpu_init() should
be called later. Nice you found the reason !

 
 Sorry for the noise.

There was no noise ;-)

Best regards,
Stefano



-- 
=
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 12/12] imx: ventana: switch to SPL

2014-05-14 Thread Tim Harvey
On Tue, May 13, 2014 at 10:03 PM, Tim Harvey thar...@gateworks.com wrote:

 On Tue, May 13, 2014 at 9:58 PM, Tim Harvey thar...@gateworks.com wrote:
  On Wed, May 7, 2014 at 2:29 AM, Stefano Babic sba...@denx.de wrote:
  Hi Tim,
 
  On 06/05/2014 20:18, Tim Harvey wrote:
 
  Stefano / York,
 
  While preparing for a v3 patch series of my IMX6 SPL bootloader, I
  find that commit dec1861be90c948ea9fb771927d3d26a994d2e20 [1] breaks
  the above code because gd is now needed within setup_i2c.
 
  I've always been a bit fuzzy on the order of the above calls so I dug
  through the code and I think I understand things better. Please
  correct any wrong assumptions I'm making below:
   - assignment to gd should 'always' be first (before anything needs
  it, so why not do it first)
   - arch_cpu_init() should go next as this sets up very low level
  CPU/SoC resources (in this case AIPS config and watchdog disable)
   - board_early_init_f() should be next as that sets up any board-specific 
  iomux
   - any additional iomux necessary for SPL should go next (I take care
  of i2c iomux and setup here)
   - timer_init() next as you need a timer for UART and mxc i2c (for
  delays and busy checks)
   - preloader_console_init() next as we are now able to send something
  over the UART (this gives me early debug for sdram config now too!)
   - sdram setup goes next
   - after sdram is setup, the bss can be cleared
   - board_init_r - pass over to generic SPL code which will load/call
  an image based on boot device
 
  I think your analyses is correct.
 
 
  So, if the above is correct, I should rework the above function as 
  follows:
 
  void board_init_f(ulong dummy)
  {
  struct ventana_board_info ventana_info;
  int board_model;
 
  /* Set global data pointer. */
  gd = gdata;
 
  /* setup AIPS and disable watchdog */
  arch_cpu_init();
 
  /* iomux and setup of i2c */
  board_early_init_f();
  i2c_setup_iomux();
 
  /* setup GP timer */
  timer_init();
 
  /* UART clocks enabled and gd valid - init serial console */
  preloader_console_init();
 
  /* read/validate EEPROM info to determine board model and SDRAM 
  cfg */
  board_model = read_eeprom(I2C_GSC, ventana_info);
 
  /* provide some some default: 32bit 128MB */
  if (GW_UNKNOWN == board_model) {
  ventana_info.sdram_width = 2;
  ventana_info.sdram_size = 3;
  }
 
  /* configure MMDC for SDRAM width/size and per-model calibration 
  */
  spl_dram_init(8  ventana_info.sdram_width,
16  ventana_info.sdram_size,
board_model);
 
  /* Clear the BSS. */
  memset(__bss_start, 0, __bss_end - __bss_start);
 
  /* load/boot image from boot device */
  board_init_r(NULL, 0);
  }
 
  It seems reasonable, go on this way.
 
  Regards,
  Stefano
 
 
  Stefano,
 
  I've just found that one of my boards fails with the above re-org.
  Strangely a board which has the same mem layout, mem width/size, CPU
  and nand does not fail.
 
  If I make the following change:

 (sorry, accidentally fat fingered and sent instead of pasting).  I'll 
 continue:

 --- a/board/gateworks/gw_ventana/gw_ventana_spl.c
 +++ b/board/gateworks/gw_ventana/gw_ventana_spl.c
 @@ -380,9 +380,6 @@ void board_init_f(ulong dummy)
  */
 memset((void *)gd, 0, sizeof(struct global_data));

 -   /* setup AIPS and disable watchdog */
 -   arch_cpu_init();
 -
 /* iomux and setup of i2c */
 board_early_init_f();
 i2c_setup_iomux();
 @@ -407,6 +404,9 @@ void board_init_f(ulong dummy)
   16  ventana_info.sdram_size,
   board_model);

 +   /* setup AIPS and disable watchdog */
 +   arch_cpu_init();
 +
 /* Clear the BSS. */
 memset(__bss_start, 0, __bss_end - __bss_start);

 Things start to work properly. I took a look at arch_cpu_init() and
 the call that really needs to be moved (or just duplicated here) is
 mxs_dma_init() to start APBH DMA. The failure mode of the one board is
 that apparently it hangs while loading u-boot.img from NAND (which of
 course uses mxs_nand and thus APBH DMA).

 Anyone know what's going on here and why mxs_dma_init() needs to be
 called after MMDC setup, and before clearing the BSS? I don't like
 having magic placement of functions without understanding why.

 Thanks,

 Tim

I figured this one out - it has nothing to do with the order of
calling arch_cpu_init() its that the MMDC isn't always 'ready' by the
time the BSS is cleared and thus in my failure case the BSS isn't
getting entirely cleared which causes the spl_image global var to not
be cleared as expected and triggers an invalid codepath. I will update
the mmdc config patch when I find the right solution.

Sorry for the noise.

Tim

Re: [U-Boot] [PATCH 12/12] imx: ventana: switch to SPL

2014-05-13 Thread Tim Harvey
On Wed, May 7, 2014 at 2:29 AM, Stefano Babic sba...@denx.de wrote:
 Hi Tim,

 On 06/05/2014 20:18, Tim Harvey wrote:

 Stefano / York,

 While preparing for a v3 patch series of my IMX6 SPL bootloader, I
 find that commit dec1861be90c948ea9fb771927d3d26a994d2e20 [1] breaks
 the above code because gd is now needed within setup_i2c.

 I've always been a bit fuzzy on the order of the above calls so I dug
 through the code and I think I understand things better. Please
 correct any wrong assumptions I'm making below:
  - assignment to gd should 'always' be first (before anything needs
 it, so why not do it first)
  - arch_cpu_init() should go next as this sets up very low level
 CPU/SoC resources (in this case AIPS config and watchdog disable)
  - board_early_init_f() should be next as that sets up any board-specific 
 iomux
  - any additional iomux necessary for SPL should go next (I take care
 of i2c iomux and setup here)
  - timer_init() next as you need a timer for UART and mxc i2c (for
 delays and busy checks)
  - preloader_console_init() next as we are now able to send something
 over the UART (this gives me early debug for sdram config now too!)
  - sdram setup goes next
  - after sdram is setup, the bss can be cleared
  - board_init_r - pass over to generic SPL code which will load/call
 an image based on boot device

 I think your analyses is correct.


 So, if the above is correct, I should rework the above function as follows:

 void board_init_f(ulong dummy)
 {
 struct ventana_board_info ventana_info;
 int board_model;

 /* Set global data pointer. */
 gd = gdata;

 /* setup AIPS and disable watchdog */
 arch_cpu_init();

 /* iomux and setup of i2c */
 board_early_init_f();
 i2c_setup_iomux();

 /* setup GP timer */
 timer_init();

 /* UART clocks enabled and gd valid - init serial console */
 preloader_console_init();

 /* read/validate EEPROM info to determine board model and SDRAM cfg 
 */
 board_model = read_eeprom(I2C_GSC, ventana_info);

 /* provide some some default: 32bit 128MB */
 if (GW_UNKNOWN == board_model) {
 ventana_info.sdram_width = 2;
 ventana_info.sdram_size = 3;
 }

 /* configure MMDC for SDRAM width/size and per-model calibration */
 spl_dram_init(8  ventana_info.sdram_width,
   16  ventana_info.sdram_size,
   board_model);

 /* Clear the BSS. */
 memset(__bss_start, 0, __bss_end - __bss_start);

 /* load/boot image from boot device */
 board_init_r(NULL, 0);
 }

 It seems reasonable, go on this way.

 Regards,
 Stefano


Stefano,

I've just found that one of my boards fails with the above re-org.
Strangely a board which has the same mem layout, mem width/size, CPU
and nand does not fail.

If I make the following change:
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 12/12] imx: ventana: switch to SPL

2014-05-13 Thread Tim Harvey
On Tue, May 13, 2014 at 9:58 PM, Tim Harvey thar...@gateworks.com wrote:
 On Wed, May 7, 2014 at 2:29 AM, Stefano Babic sba...@denx.de wrote:
 Hi Tim,

 On 06/05/2014 20:18, Tim Harvey wrote:

 Stefano / York,

 While preparing for a v3 patch series of my IMX6 SPL bootloader, I
 find that commit dec1861be90c948ea9fb771927d3d26a994d2e20 [1] breaks
 the above code because gd is now needed within setup_i2c.

 I've always been a bit fuzzy on the order of the above calls so I dug
 through the code and I think I understand things better. Please
 correct any wrong assumptions I'm making below:
  - assignment to gd should 'always' be first (before anything needs
 it, so why not do it first)
  - arch_cpu_init() should go next as this sets up very low level
 CPU/SoC resources (in this case AIPS config and watchdog disable)
  - board_early_init_f() should be next as that sets up any board-specific 
 iomux
  - any additional iomux necessary for SPL should go next (I take care
 of i2c iomux and setup here)
  - timer_init() next as you need a timer for UART and mxc i2c (for
 delays and busy checks)
  - preloader_console_init() next as we are now able to send something
 over the UART (this gives me early debug for sdram config now too!)
  - sdram setup goes next
  - after sdram is setup, the bss can be cleared
  - board_init_r - pass over to generic SPL code which will load/call
 an image based on boot device

 I think your analyses is correct.


 So, if the above is correct, I should rework the above function as follows:

 void board_init_f(ulong dummy)
 {
 struct ventana_board_info ventana_info;
 int board_model;

 /* Set global data pointer. */
 gd = gdata;

 /* setup AIPS and disable watchdog */
 arch_cpu_init();

 /* iomux and setup of i2c */
 board_early_init_f();
 i2c_setup_iomux();

 /* setup GP timer */
 timer_init();

 /* UART clocks enabled and gd valid - init serial console */
 preloader_console_init();

 /* read/validate EEPROM info to determine board model and SDRAM cfg 
 */
 board_model = read_eeprom(I2C_GSC, ventana_info);

 /* provide some some default: 32bit 128MB */
 if (GW_UNKNOWN == board_model) {
 ventana_info.sdram_width = 2;
 ventana_info.sdram_size = 3;
 }

 /* configure MMDC for SDRAM width/size and per-model calibration */
 spl_dram_init(8  ventana_info.sdram_width,
   16  ventana_info.sdram_size,
   board_model);

 /* Clear the BSS. */
 memset(__bss_start, 0, __bss_end - __bss_start);

 /* load/boot image from boot device */
 board_init_r(NULL, 0);
 }

 It seems reasonable, go on this way.

 Regards,
 Stefano


 Stefano,

 I've just found that one of my boards fails with the above re-org.
 Strangely a board which has the same mem layout, mem width/size, CPU
 and nand does not fail.

 If I make the following change:

(sorry, accidentally fat fingered and sent instead of pasting).  I'll continue:

--- a/board/gateworks/gw_ventana/gw_ventana_spl.c
+++ b/board/gateworks/gw_ventana/gw_ventana_spl.c
@@ -380,9 +380,6 @@ void board_init_f(ulong dummy)
 */
memset((void *)gd, 0, sizeof(struct global_data));

-   /* setup AIPS and disable watchdog */
-   arch_cpu_init();
-
/* iomux and setup of i2c */
board_early_init_f();
i2c_setup_iomux();
@@ -407,6 +404,9 @@ void board_init_f(ulong dummy)
  16  ventana_info.sdram_size,
  board_model);

+   /* setup AIPS and disable watchdog */
+   arch_cpu_init();
+
/* Clear the BSS. */
memset(__bss_start, 0, __bss_end - __bss_start);

Things start to work properly. I took a look at arch_cpu_init() and
the call that really needs to be moved (or just duplicated here) is
mxs_dma_init() to start APBH DMA. The failure mode of the one board is
that apparently it hangs while loading u-boot.img from NAND (which of
course uses mxs_nand and thus APBH DMA).

Anyone know what's going on here and why mxs_dma_init() needs to be
called after MMDC setup, and before clearing the BSS? I don't like
having magic placement of functions without understanding why.

Thanks,

Tim
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 12/12] imx: ventana: switch to SPL

2014-05-07 Thread Stefano Babic
Hi Tim,

On 06/05/2014 20:18, Tim Harvey wrote:
 
 Stefano / York,
 
 While preparing for a v3 patch series of my IMX6 SPL bootloader, I
 find that commit dec1861be90c948ea9fb771927d3d26a994d2e20 [1] breaks
 the above code because gd is now needed within setup_i2c.
 
 I've always been a bit fuzzy on the order of the above calls so I dug
 through the code and I think I understand things better. Please
 correct any wrong assumptions I'm making below:
  - assignment to gd should 'always' be first (before anything needs
 it, so why not do it first)
  - arch_cpu_init() should go next as this sets up very low level
 CPU/SoC resources (in this case AIPS config and watchdog disable)
  - board_early_init_f() should be next as that sets up any board-specific 
 iomux
  - any additional iomux necessary for SPL should go next (I take care
 of i2c iomux and setup here)
  - timer_init() next as you need a timer for UART and mxc i2c (for
 delays and busy checks)
  - preloader_console_init() next as we are now able to send something
 over the UART (this gives me early debug for sdram config now too!)
  - sdram setup goes next
  - after sdram is setup, the bss can be cleared
  - board_init_r - pass over to generic SPL code which will load/call
 an image based on boot device

I think your analyses is correct.

 
 So, if the above is correct, I should rework the above function as follows:
 
 void board_init_f(ulong dummy)
 {
 struct ventana_board_info ventana_info;
 int board_model;
 
 /* Set global data pointer. */
 gd = gdata;
 
 /* setup AIPS and disable watchdog */
 arch_cpu_init();
 
 /* iomux and setup of i2c */
 board_early_init_f();
 i2c_setup_iomux();
 
 /* setup GP timer */
 timer_init();
 
 /* UART clocks enabled and gd valid - init serial console */
 preloader_console_init();
 
 /* read/validate EEPROM info to determine board model and SDRAM cfg */
 board_model = read_eeprom(I2C_GSC, ventana_info);
 
 /* provide some some default: 32bit 128MB */
 if (GW_UNKNOWN == board_model) {
 ventana_info.sdram_width = 2;
 ventana_info.sdram_size = 3;
 }
 
 /* configure MMDC for SDRAM width/size and per-model calibration */
 spl_dram_init(8  ventana_info.sdram_width,
   16  ventana_info.sdram_size,
   board_model);
 
 /* Clear the BSS. */
 memset(__bss_start, 0, __bss_end - __bss_start);
 
 /* load/boot image from boot device */
 board_init_r(NULL, 0);
 }

It seems reasonable, go on this way.

Regards,
Stefano



-- 
=
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 12/12] imx: ventana: switch to SPL

2014-05-07 Thread York Sun
On 05/06/2014 04:35 PM, Tim Harvey wrote:
 On Tue, May 6, 2014 at 12:11 PM, Jeroen Hofstee dasub...@myspectrum.nl 
 wrote:
 Hello Tim,

snip

 Since Crt0.S already created gd on the stack before calling
 board_init_f, can't the assignment of gd not simply be removed?
 Is there anything special about gdata?

 Regards,
 Jeroen


 
 Jeroen,
 
 That does make sense, but what I find is that York's ocmmit
 dec1861be90c948ea9fb771927d3d26a994d2e20 requires that gd be blanked
 and its not. This causes bus_i2c_init to skip its initialization
 because p-base is not zero.
 
 York, does this make sense? Your patch creates a dependence on
 gd-srdata being blank which isn't the case with the SRAM when booting
 from the IMX6 boot rom.
 

GD should be cleared (zeroed). Then we don't have this problem. Whoever sets up
gd (board_int_f, cpu_init_f, or others) should make sure gd is cleared. Why
isn't it the case for IMX6 boot rom?

York

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 12/12] imx: ventana: switch to SPL

2014-05-07 Thread Jeroen Hofstee
Hi,

On wo, 2014-05-07 at 09:14 -0700, York Sun wrote:

 snip
 
  Since Crt0.S already created gd on the stack before calling
  board_init_f, can't the assignment of gd not simply be removed?
  Is there anything special about gdata?
  
  That does make sense, but what I find is that York's ocmmit
  dec1861be90c948ea9fb771927d3d26a994d2e20 requires that gd be blanked
  and its not. This causes bus_i2c_init to skip its initialization
  because p-base is not zero.
  
  York, does this make sense? Your patch creates a dependence on
  gd-srdata being blank which isn't the case with the SRAM when booting
  from the IMX6 boot rom.
  
 
 GD should be cleared (zeroed). Then we don't have this problem. Whoever sets 
 up
 gd (board_int_f, cpu_init_f, or others) should make sure gd is cleared. Why
 isn't it the case for IMX6 boot rom?
 

Seems fine to me. Albert, any objection against zeroing gd out in
crt0.S?

And if not, shouldn't the same be done for arm64.

Jeroen



___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 12/12] imx: ventana: switch to SPL

2014-05-07 Thread Tim Harvey
On Wed, May 7, 2014 at 9:14 AM, York Sun york...@freescale.com wrote:
 On 05/06/2014 04:35 PM, Tim Harvey wrote:
 On Tue, May 6, 2014 at 12:11 PM, Jeroen Hofstee dasub...@myspectrum.nl 
 wrote:
 Hello Tim,

 snip

 Since Crt0.S already created gd on the stack before calling
 board_init_f, can't the assignment of gd not simply be removed?
 Is there anything special about gdata?

 Regards,
 Jeroen



 Jeroen,

 That does make sense, but what I find is that York's ocmmit
 dec1861be90c948ea9fb771927d3d26a994d2e20 requires that gd be blanked
 and its not. This causes bus_i2c_init to skip its initialization
 because p-base is not zero.

 York, does this make sense? Your patch creates a dependence on
 gd-srdata being blank which isn't the case with the SRAM when booting
 from the IMX6 boot rom.


 GD should be cleared (zeroed). Then we don't have this problem. Whoever sets 
 up
 gd (board_int_f, cpu_init_f, or others) should make sure gd is cleared. Why
 isn't it the case for IMX6 boot rom?

 York


York,

crt0.S is mapping gd to where the SPL stack pointer is defined, which
is where Freescale says its stack pointer is for its firmware BOOT
ROM, so its initial data will be dependent on what the BOOT ROM did.

I think the right solution is to have crt0.S zero it out.

Tim
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 12/12] imx: ventana: switch to SPL

2014-05-07 Thread York Sun
On 05/07/2014 01:27 PM, Tim Harvey wrote:
 On Wed, May 7, 2014 at 9:14 AM, York Sun york...@freescale.com wrote:
 On 05/06/2014 04:35 PM, Tim Harvey wrote:
 On Tue, May 6, 2014 at 12:11 PM, Jeroen Hofstee dasub...@myspectrum.nl 
 wrote:
 Hello Tim,

 snip

 Since Crt0.S already created gd on the stack before calling
 board_init_f, can't the assignment of gd not simply be removed?
 Is there anything special about gdata?

 Regards,
 Jeroen



 Jeroen,

 That does make sense, but what I find is that York's ocmmit
 dec1861be90c948ea9fb771927d3d26a994d2e20 requires that gd be blanked
 and its not. This causes bus_i2c_init to skip its initialization
 because p-base is not zero.

 York, does this make sense? Your patch creates a dependence on
 gd-srdata being blank which isn't the case with the SRAM when booting
 from the IMX6 boot rom.


 GD should be cleared (zeroed). Then we don't have this problem. Whoever sets 
 up
 gd (board_int_f, cpu_init_f, or others) should make sure gd is cleared. Why
 isn't it the case for IMX6 boot rom?

 York

 
 York,
 
 crt0.S is mapping gd to where the SPL stack pointer is defined, which
 is where Freescale says its stack pointer is for its firmware BOOT
 ROM, so its initial data will be dependent on what the BOOT ROM did.
 
 I think the right solution is to have crt0.S zero it out.


Either crt0.S, or somewhere before gd is used for the first time. Can it be done
in board_init_f()?

York


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 12/12] imx: ventana: switch to SPL

2014-05-07 Thread Tim Harvey
On Wed, May 7, 2014 at 1:29 PM, York Sun york...@freescale.com wrote:
 On 05/07/2014 01:27 PM, Tim Harvey wrote:
 On Wed, May 7, 2014 at 9:14 AM, York Sun york...@freescale.com wrote:
 On 05/06/2014 04:35 PM, Tim Harvey wrote:
 On Tue, May 6, 2014 at 12:11 PM, Jeroen Hofstee dasub...@myspectrum.nl 
 wrote:
 Hello Tim,

 snip

 Since Crt0.S already created gd on the stack before calling
 board_init_f, can't the assignment of gd not simply be removed?
 Is there anything special about gdata?

 Regards,
 Jeroen



 Jeroen,

 That does make sense, but what I find is that York's ocmmit
 dec1861be90c948ea9fb771927d3d26a994d2e20 requires that gd be blanked
 and its not. This causes bus_i2c_init to skip its initialization
 because p-base is not zero.

 York, does this make sense? Your patch creates a dependence on
 gd-srdata being blank which isn't the case with the SRAM when booting
 from the IMX6 boot rom.


 GD should be cleared (zeroed). Then we don't have this problem. Whoever 
 sets up
 gd (board_int_f, cpu_init_f, or others) should make sure gd is cleared. Why
 isn't it the case for IMX6 boot rom?

 York


 York,

 crt0.S is mapping gd to where the SPL stack pointer is defined, which
 is where Freescale says its stack pointer is for its firmware BOOT
 ROM, so its initial data will be dependent on what the BOOT ROM did.

 I think the right solution is to have crt0.S zero it out.


 Either crt0.S, or somewhere before gd is used for the first time. Can it be 
 done
 in board_init_f()?

 York



It makes more sense to do it in crt0.S so that all boards don't have
to do it and can make the assumption that it is cleared.

Tim
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 12/12] imx: ventana: switch to SPL

2014-05-07 Thread York Sun
On 05/07/2014 01:35 PM, Tim Harvey wrote:
 On Wed, May 7, 2014 at 1:29 PM, York Sun york...@freescale.com wrote:
 On 05/07/2014 01:27 PM, Tim Harvey wrote:
 On Wed, May 7, 2014 at 9:14 AM, York Sun york...@freescale.com wrote:
 On 05/06/2014 04:35 PM, Tim Harvey wrote:
 On Tue, May 6, 2014 at 12:11 PM, Jeroen Hofstee dasub...@myspectrum.nl 
 wrote:
 Hello Tim,

 snip

 Since Crt0.S already created gd on the stack before calling
 board_init_f, can't the assignment of gd not simply be removed?
 Is there anything special about gdata?

 Regards,
 Jeroen



 Jeroen,

 That does make sense, but what I find is that York's ocmmit
 dec1861be90c948ea9fb771927d3d26a994d2e20 requires that gd be blanked
 and its not. This causes bus_i2c_init to skip its initialization
 because p-base is not zero.

 York, does this make sense? Your patch creates a dependence on
 gd-srdata being blank which isn't the case with the SRAM when booting
 from the IMX6 boot rom.


 GD should be cleared (zeroed). Then we don't have this problem. Whoever 
 sets up
 gd (board_int_f, cpu_init_f, or others) should make sure gd is cleared. Why
 isn't it the case for IMX6 boot rom?

 York


 York,

 crt0.S is mapping gd to where the SPL stack pointer is defined, which
 is where Freescale says its stack pointer is for its firmware BOOT
 ROM, so its initial data will be dependent on what the BOOT ROM did.

 I think the right solution is to have crt0.S zero it out.


 Either crt0.S, or somewhere before gd is used for the first time. Can it be 
 done
 in board_init_f()?

 York


 
 It makes more sense to do it in crt0.S so that all boards don't have
 to do it and can make the assumption that it is cleared.
 

No objection as far as it is cleared.

York


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 12/12] imx: ventana: switch to SPL

2014-05-06 Thread Tim Harvey
On Mon, Apr 28, 2014 at 1:17 PM, Tim Harvey thar...@gateworks.com wrote:

 Switch to an SPL image. The SPL for Ventana does the following:
  - setup i2c and read the factory programmed EEPROM to obtain DRAM config
and model for board-specific calibration data
  - configure DRAM per CPU/size/layout/devices/calibration
  - load u-boot.img from NAND or MTD depending on boot device  and jump to it

 This allows for a single SPL+u-boot.img to replace the previous multiple board
 configurations.

snip
 +
 +static void i2c_setup_iomux(void)
 +{
 +   if (is_cpu_type(MXC_CPU_MX6Q))
 +   setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f, mx6q_i2c_pad_info0);
 +   else
 +   setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f, 
 mx6dl_i2c_pad_info0);
 +}
 +
snip
 +
 +/*
 + * called from C runtime startup code (arch/arm/lib/crt0.S:_main)
 + * - we have a stack and a place to store GD, both in SRAM
 + * - no variable global data is available
 + */
 +void board_init_f(ulong dummy)
 +{
 +   struct ventana_board_info ventana_info;
 +   int board_model;
 +
 +   /* iomux and setup of i2c */
 +   i2c_setup_iomux();
 +   timer_init();
 +   board_model = read_eeprom(I2C_GSC, ventana_info);
 +
 +   /* provide some some default: 32bit 128MB */
 +   if (GW_UNKNOWN == board_model) {
 +   ventana_info.sdram_width = 2;
 +   ventana_info.sdram_size = 3;
 +   }
 +   spl_dram_init(8  ventana_info.sdram_width,
 + 16  ventana_info.sdram_size,
 + board_model);
 +
 +   arch_cpu_init();
 +
 +   /* Clear the BSS. */
 +   memset(__bss_start, 0, __bss_end - __bss_start);
 +
 +   /* Set global data pointer. */
 +   gd = gdata;
 +
 +   board_early_init_f();
 +
 +   preloader_console_init();
 +
 +   board_init_r(NULL, 0);
 +}

Stefano / York,

While preparing for a v3 patch series of my IMX6 SPL bootloader, I
find that commit dec1861be90c948ea9fb771927d3d26a994d2e20 [1] breaks
the above code because gd is now needed within setup_i2c.

I've always been a bit fuzzy on the order of the above calls so I dug
through the code and I think I understand things better. Please
correct any wrong assumptions I'm making below:
 - assignment to gd should 'always' be first (before anything needs
it, so why not do it first)
 - arch_cpu_init() should go next as this sets up very low level
CPU/SoC resources (in this case AIPS config and watchdog disable)
 - board_early_init_f() should be next as that sets up any board-specific iomux
 - any additional iomux necessary for SPL should go next (I take care
of i2c iomux and setup here)
 - timer_init() next as you need a timer for UART and mxc i2c (for
delays and busy checks)
 - preloader_console_init() next as we are now able to send something
over the UART (this gives me early debug for sdram config now too!)
 - sdram setup goes next
 - after sdram is setup, the bss can be cleared
 - board_init_r - pass over to generic SPL code which will load/call
an image based on boot device

So, if the above is correct, I should rework the above function as follows:

void board_init_f(ulong dummy)
{
struct ventana_board_info ventana_info;
int board_model;

/* Set global data pointer. */
gd = gdata;

/* setup AIPS and disable watchdog */
arch_cpu_init();

/* iomux and setup of i2c */
board_early_init_f();
i2c_setup_iomux();

/* setup GP timer */
timer_init();

/* UART clocks enabled and gd valid - init serial console */
preloader_console_init();

/* read/validate EEPROM info to determine board model and SDRAM cfg */
board_model = read_eeprom(I2C_GSC, ventana_info);

/* provide some some default: 32bit 128MB */
if (GW_UNKNOWN == board_model) {
ventana_info.sdram_width = 2;
ventana_info.sdram_size = 3;
}

/* configure MMDC for SDRAM width/size and per-model calibration */
spl_dram_init(8  ventana_info.sdram_width,
  16  ventana_info.sdram_size,
  board_model);

/* Clear the BSS. */
memset(__bss_start, 0, __bss_end - __bss_start);

/* load/boot image from boot device */
board_init_r(NULL, 0);
}

Does this make sense?

Thanks,

Tim

[1] - 
http://git.denx.de/?p=u-boot.git;a=commitdiff;h=dec1861be90c948ea9fb771927d3d26a994d2e20

 +
 +void reset_cpu(ulong addr)
 +{
 +}
 diff --git a/boards.cfg b/boards.cfg
 index c83aff3..9119e24 100644
 --- a/boards.cfg
 +++ b/boards.cfg
 @@ -322,11 +322,7 @@ Active  arm armv7  mx6 freescale 
   mx6qsabreauto
  Active  arm armv7  mx6 freescale   mx6sabresd
   mx6dlsabresd 
 mx6sabresd:IMX_CONFIG=board/boundary/nitrogen6x/nitrogen6dl.cfg,MX6DL 
 Fabio 

Re: [U-Boot] [PATCH 12/12] imx: ventana: switch to SPL

2014-05-06 Thread York Sun
On 05/06/2014 11:18 AM, Tim Harvey wrote:
 On Mon, Apr 28, 2014 at 1:17 PM, Tim Harvey thar...@gateworks.com wrote:

 Switch to an SPL image. The SPL for Ventana does the following:
  - setup i2c and read the factory programmed EEPROM to obtain DRAM config
and model for board-specific calibration data
  - configure DRAM per CPU/size/layout/devices/calibration
  - load u-boot.img from NAND or MTD depending on boot device  and jump to it

 This allows for a single SPL+u-boot.img to replace the previous multiple 
 board
 configurations.

 snip
 +
 +static void i2c_setup_iomux(void)
 +{
 +   if (is_cpu_type(MXC_CPU_MX6Q))
 +   setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f, 
 mx6q_i2c_pad_info0);
 +   else
 +   setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f, 
 mx6dl_i2c_pad_info0);
 +}
 +
 snip
 +
 +/*
 + * called from C runtime startup code (arch/arm/lib/crt0.S:_main)
 + * - we have a stack and a place to store GD, both in SRAM
 + * - no variable global data is available
 + */
 +void board_init_f(ulong dummy)
 +{
 +   struct ventana_board_info ventana_info;
 +   int board_model;
 +
 +   /* iomux and setup of i2c */
 +   i2c_setup_iomux();
 +   timer_init();
 +   board_model = read_eeprom(I2C_GSC, ventana_info);
 +
 +   /* provide some some default: 32bit 128MB */
 +   if (GW_UNKNOWN == board_model) {
 +   ventana_info.sdram_width = 2;
 +   ventana_info.sdram_size = 3;
 +   }
 +   spl_dram_init(8  ventana_info.sdram_width,
 + 16  ventana_info.sdram_size,
 + board_model);
 +
 +   arch_cpu_init();
 +
 +   /* Clear the BSS. */
 +   memset(__bss_start, 0, __bss_end - __bss_start);
 +
 +   /* Set global data pointer. */
 +   gd = gdata;
 +
 +   board_early_init_f();
 +
 +   preloader_console_init();
 +
 +   board_init_r(NULL, 0);
 +}
 
 Stefano / York,
 
 While preparing for a v3 patch series of my IMX6 SPL bootloader, I
 find that commit dec1861be90c948ea9fb771927d3d26a994d2e20 [1] breaks
 the above code because gd is now needed within setup_i2c.
 
 I've always been a bit fuzzy on the order of the above calls so I dug
 through the code and I think I understand things better. Please
 correct any wrong assumptions I'm making below:
  - assignment to gd should 'always' be first (before anything needs
 it, so why not do it first)
  - arch_cpu_init() should go next as this sets up very low level
 CPU/SoC resources (in this case AIPS config and watchdog disable)
  - board_early_init_f() should be next as that sets up any board-specific 
 iomux
  - any additional iomux necessary for SPL should go next (I take care
 of i2c iomux and setup here)
  - timer_init() next as you need a timer for UART and mxc i2c (for
 delays and busy checks)
  - preloader_console_init() next as we are now able to send something
 over the UART (this gives me early debug for sdram config now too!)
  - sdram setup goes next
  - after sdram is setup, the bss can be cleared
  - board_init_r - pass over to generic SPL code which will load/call
 an image based on boot device
 
 So, if the above is correct, I should rework the above function as follows:
 
 void board_init_f(ulong dummy)
 {
 struct ventana_board_info ventana_info;
 int board_model;
 
 /* Set global data pointer. */
 gd = gdata;
 
 /* setup AIPS and disable watchdog */
 arch_cpu_init();
 
 /* iomux and setup of i2c */
 board_early_init_f();
 i2c_setup_iomux();
 
 /* setup GP timer */
 timer_init();
 
 /* UART clocks enabled and gd valid - init serial console */
 preloader_console_init();
 
 /* read/validate EEPROM info to determine board model and SDRAM cfg */
 board_model = read_eeprom(I2C_GSC, ventana_info);
 
 /* provide some some default: 32bit 128MB */
 if (GW_UNKNOWN == board_model) {
 ventana_info.sdram_width = 2;
 ventana_info.sdram_size = 3;
 }
 
 /* configure MMDC for SDRAM width/size and per-model calibration */
 spl_dram_init(8  ventana_info.sdram_width,
   16  ventana_info.sdram_size,
   board_model);
 
 /* Clear the BSS. */
 memset(__bss_start, 0, __bss_end - __bss_start);
 
 /* load/boot image from boot device */
 board_init_r(NULL, 0);
 }
 
 Does this make sense?


Tim,

I agree gd should be set first if the memory is available. My previous change to
this i2c driver was to make sure it works without DRAM. Since it uses a data
structure, it has no other place to go.

York

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 12/12] imx: ventana: switch to SPL

2014-05-06 Thread Jeroen Hofstee
Hello Tim,

On di, 2014-05-06 at 11:18 -0700, Tim Harvey wrote:
 On Mon, Apr 28, 2014 at 1:17 PM, Tim Harvey thar...@gateworks.com wrote:

 
 void board_init_f(ulong dummy)
 {
 struct ventana_board_info ventana_info;
 int board_model;
 
 /* Set global data pointer. */
 gd = gdata;
 
 /* setup AIPS and disable watchdog */
 arch_cpu_init();
 
 /* iomux and setup of i2c */
 board_early_init_f();
 i2c_setup_iomux();
 
 /* setup GP timer */
 timer_init();
 
 /* UART clocks enabled and gd valid - init serial console */
 preloader_console_init();
 
 /* read/validate EEPROM info to determine board model and SDRAM cfg */
 board_model = read_eeprom(I2C_GSC, ventana_info);
 
 /* provide some some default: 32bit 128MB */
 if (GW_UNKNOWN == board_model) {
 ventana_info.sdram_width = 2;
 ventana_info.sdram_size = 3;
 }
 
 /* configure MMDC for SDRAM width/size and per-model calibration */
 spl_dram_init(8  ventana_info.sdram_width,
   16  ventana_info.sdram_size,
   board_model);
 
 /* Clear the BSS. */
 memset(__bss_start, 0, __bss_end - __bss_start);
 
 /* load/boot image from boot device */
 board_init_r(NULL, 0);
 }
 
 Does this make sense?

Since Crt0.S already created gd on the stack before calling
board_init_f, can't the assignment of gd not simply be removed?
Is there anything special about gdata?

Regards,
Jeroen


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 12/12] imx: ventana: switch to SPL

2014-05-06 Thread Tim Harvey
On Tue, May 6, 2014 at 12:11 PM, Jeroen Hofstee dasub...@myspectrum.nl wrote:
 Hello Tim,

 On di, 2014-05-06 at 11:18 -0700, Tim Harvey wrote:
 On Mon, Apr 28, 2014 at 1:17 PM, Tim Harvey thar...@gateworks.com wrote:


 void board_init_f(ulong dummy)
 {
 struct ventana_board_info ventana_info;
 int board_model;

 /* Set global data pointer. */
 gd = gdata;

 /* setup AIPS and disable watchdog */
 arch_cpu_init();

 /* iomux and setup of i2c */
 board_early_init_f();
 i2c_setup_iomux();

 /* setup GP timer */
 timer_init();

 /* UART clocks enabled and gd valid - init serial console */
 preloader_console_init();

 /* read/validate EEPROM info to determine board model and SDRAM cfg 
 */
 board_model = read_eeprom(I2C_GSC, ventana_info);

 /* provide some some default: 32bit 128MB */
 if (GW_UNKNOWN == board_model) {
 ventana_info.sdram_width = 2;
 ventana_info.sdram_size = 3;
 }

 /* configure MMDC for SDRAM width/size and per-model calibration */
 spl_dram_init(8  ventana_info.sdram_width,
   16  ventana_info.sdram_size,
   board_model);

 /* Clear the BSS. */
 memset(__bss_start, 0, __bss_end - __bss_start);

 /* load/boot image from boot device */
 board_init_r(NULL, 0);
 }

 Does this make sense?

 Since Crt0.S already created gd on the stack before calling
 board_init_f, can't the assignment of gd not simply be removed?
 Is there anything special about gdata?

 Regards,
 Jeroen



Jeroen,

That does make sense, but what I find is that York's ocmmit
dec1861be90c948ea9fb771927d3d26a994d2e20 requires that gd be blanked
and its not. This causes bus_i2c_init to skip its initialization
because p-base is not zero.

York, does this make sense? Your patch creates a dependence on
gd-srdata being blank which isn't the case with the SRAM when booting
from the IMX6 boot rom.

Tim
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 12/12] imx: ventana: switch to SPL

2014-04-28 Thread Tim Harvey
Switch to an SPL image. The SPL for Ventana does the following:
 - setup i2c and read the factory programmed EEPROM to obtain DRAM config
   and model for board-specific calibration data
 - configure DRAM per CPU/size/layout/devices/calibration
 - load u-boot.img from NAND or MTD depending on boot device  and jump to it

This allows for a single SPL+u-boot.img to replace the previous multiple board
configurations.

Signed-off-by: Tim Harvey thar...@gateworks.com
---
v2:
- remove missing/unnecessary include
- revert mtdparts change
- use imx_ddr_size() which uses MMDC configuration to determine mem size
  explicitly
- add support for MX6SOLO and MX6DUAL
- set CS0_END for 4GB so get_ram_size() works
- updated DDR3 calibration values for ventana boards
- fixed build issue - only compile spl if doing spl build
- fixed line length issue in README
- remove CONFIG_SPL* conditions and conditionally compile instead
- removed prints for CPU type and DRAM size/width - uboot will print these later
- removed unused gw_ventana_spl.cfg
- use common read_eeprom function
- added MMC support to SPL
---
 board/gateworks/gw_ventana/Makefile |   1 +
 board/gateworks/gw_ventana/README   |  92 ---
 board/gateworks/gw_ventana/gw_ventana.c |   9 +-
 board/gateworks/gw_ventana/gw_ventana.cfg   |  15 -
 board/gateworks/gw_ventana/gw_ventana_spl.c | 407 
 boards.cfg  |   6 +-
 include/configs/gw_ventana.h|  11 +
 7 files changed, 485 insertions(+), 56 deletions(-)
 create mode 100644 board/gateworks/gw_ventana/gw_ventana_spl.c

diff --git a/board/gateworks/gw_ventana/Makefile 
b/board/gateworks/gw_ventana/Makefile
index 03bd1fd..33a1788 100644
--- a/board/gateworks/gw_ventana/Makefile
+++ b/board/gateworks/gw_ventana/Makefile
@@ -7,4 +7,5 @@
 #
 
 obj-y  := gw_ventana.o gsc.o eeprom.o
+obj-$(CONFIG_SPL_BUILD) += gw_ventana_spl.o
 
diff --git a/board/gateworks/gw_ventana/README 
b/board/gateworks/gw_ventana/README
index 9e697d6..888657c 100644
--- a/board/gateworks/gw_ventana/README
+++ b/board/gateworks/gw_ventana/README
@@ -3,53 +3,81 @@ U-Boot for the Gateworks Ventana Product Family boards
 This file contains information for the port of U-Boot to the Gateworks
 Ventana Product family boards.
 
-1. Boot source, boot from NAND
+1. Secondary Program Loader (SPL)
+-
+
+The i.MX6 has a BOOT ROM PPL (Primary Program Loader) which supports loading
+an executable image from various boot devices.
+
+The Gateworks Ventana board config uses an SPL build configuration. This
+will build the following artifacts from u-boot source:
+ - SPL - Secondary Program Loader that the i.MX6 BOOT ROM (Primary Program
+ Loader) boots.  This detects CPU/DRAM configuration, configures
+ The DRAM controller, loads u-boot.img from the detected boot device,
+ and jumps to it.  As this is booted from the PPL, it has an IVT/DCD
+ table.
+ - u-boot.img - The main u-boot core which is u-boot.bin with a image header.
+
+
+2. Build
+
+
+To build U-Boot for the Gateworks Ventana product family:
+
+ make gwventana_config
+ make
+
+
+3. Boot source, boot from NAND
 --
 
 The i.MX6 BOOT ROM expects some structures that provide details of NAND layout
 and bad block information (referred to as 'bootstreams') which are replicated
-multiple times in NAND. The number of replications is configurable through
-board strapping options and eFUSE settings.  The Freescale 'kobs-ng'
-application from the Freescale LTIB BSP, which runs under Linux, must be used
-to program the bootstream in order to setup the replicated headers correctly.
+multiple times in NAND. The number of replications and their spacing (referred
+to as search stride) is configurable through board strapping options and/or
+eFUSE settings (BOOT_SEARCH_COUNT / Pages in block from BOOT_CFG2). In
+addition, the i.MX6 BOOT ROM Flash Configuration Block (FCB) supports two
+copies of a bootloader in flash in the case that a bad block has corrupted one.
+The Freescale 'kobs-ng' application from the Freescale LTIB BSP, which runs
+under Linux and operates on an MTD partition, must be used to program the
+bootstream in order to setup this flash structure correctly.
 
 The Gateworks Ventana boards with NAND flash have been factory programmed
 such that their eFUSE settings expect 2 copies of the boostream (this is
 specified by providing kobs-ng with the --search_exponent=1 argument). Once in
-Linux with MTD support for the NAND on /dev/mtd0 you can program the boostream
+Linux with MTD support for the NAND on /dev/mtd0 you can program the SPL
 with:
 
-kobs-ng init -v -x --search_exponent=1 u-boot.imx
+kobs-ng init -v -x --search_exponent=1 SPL
 
-The kobs-ng application uses an imximage (u-boot.imx) which contains the
-Image Vector Table (IVT) and Device Configuration Data (DCD) structures that
-the i.MX6 BOOT ROM requires to