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