Hi Stefan,

Finally, I've found some time to look at the patch...
Generally, it is fine...
I say generally, because we have found several bugs, but they
are not related to your patch... but to Pekon's work on the
omap nand driver. Nikita is on them ;-)

One minor comment below...

On 12/04/13 14:54, Stefan Roese wrote:
> Add SPL U-Boot support to replace x-loader on the Compulab cm_t35
> board. Currently only the 256MiB SDRAM board versions are supported.
> 
> Tested by booting via MMC and NAND.
> 
> Signed-off-by: Stefan Roese <s...@denx.de>
> Cc: Tom Rini <tr...@ti.com>
> Cc: Igor Grinberg <grinb...@compulab.co.il>
> Cc: Nikita Kiryanov <nik...@compulab.co.il>

Apart from the comment below,
Acked-by: Igor Grinberg <grinb...@compulab.co.il>

> ---
> v5:
> - Added CONFIG_NAND_OMAP_ECCSCHEME to select HW ECC by default
> - Tested with latest mainline U-Boot 2014.01-rc1,
>   needs fix for 8bit NAND device from Pekon Gupta
> 
> v4:
> - Rebased and retested on current mainline version
> 
> v3:
> - Some instability of this SDRAM setup has been detected while running
>   Linux. Comparison with the x-loader setup showed that mcfg is configured
>   slightly differently here. CM_T35 needs RAS-width of 14 instead of
>   13. So use the define MICRON_V_MCFG_200 which implements this 14
>   as RAS width.
> 
> v2:
> - Change CONFIG_SYS_TEXT_BASE back to 0x80008000 for x-loader
>   compatibility
> - Change CONFIG_SPL_BSS_START_ADDR to 0x80100000 to not overlap
>   with TEXT_BASE now
> 
>  board/compulab/cm_t35/cm_t35.c | 18 +++++++++++-
>  include/configs/cm_t35.h       | 65 
> ++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 80 insertions(+), 3 deletions(-)
> 
> diff --git a/board/compulab/cm_t35/cm_t35.c b/board/compulab/cm_t35/cm_t35.c
> index bc8e0ca..00bcf41 100644
> --- a/board/compulab/cm_t35/cm_t35.c
> +++ b/board/compulab/cm_t35/cm_t35.c
> @@ -105,6 +105,22 @@ static inline int splash_load_from_nand(void)
>  }
>  #endif /* CONFIG_CMD_NAND */
>  
> +#ifdef CONFIG_SPL_BUILD
> +/*
> + * Routine: get_board_mem_timings
> + * Description: If we use SPL then there is no x-loader nor config header
> + * so we have to setup the DDR timings ourself on both banks.
> + */
> +void get_board_mem_timings(struct board_sdrc_timings *timings)
> +{
> +     timings->mr = MICRON_V_MR_165;
> +     timings->mcfg = MICRON_V_MCFG_200(256 << 20); /* raswidth 14 needed */
> +     timings->ctrla = MICRON_V_ACTIMA_165;
> +     timings->ctrlb = MICRON_V_ACTIMB_165;
> +     timings->rfr_ctrl = SDP_3430_SDRC_RFR_CTRL_165MHz;
> +}
> +#endif
> +
>  int splash_screen_prepare(void)
>  {
>       char *env_splashimage_value;
> @@ -440,7 +456,7 @@ void set_muxconf_regs(void)
>               cm_t3730_set_muxconf();
>  }
>  
> -#ifdef CONFIG_GENERIC_MMC
> +#if defined(CONFIG_GENERIC_MMC) && !defined(CONFIG_SPL_BUILD)
>  int board_mmc_getcd(struct mmc *mmc)
>  {
>       u8 val;
> diff --git a/include/configs/cm_t35.h b/include/configs/cm_t35.h
> index f4ecd0d..2e865c0 100644
> --- a/include/configs/cm_t35.h
> +++ b/include/configs/cm_t35.h
> @@ -27,8 +27,6 @@
>  #define CONFIG_CM_T3X        /* working with CM-T35 and CM-T3730 */
>  #define CONFIG_OMAP_COMMON
>  
> -#define CONFIG_SYS_TEXT_BASE 0x80008000
> -
>  #define CONFIG_SDRC  /* The chip has SDRC controller */
>  
>  #include <asm/arch/cpu.h>            /* get chip and board defs */
> @@ -329,4 +327,67 @@
>  
>  #define CONFIG_OMAP3_SPI
>  
> +/* Defines for SPL */
> +#define CONFIG_SPL
> +#define CONFIG_SPL_FRAMEWORK
> +#define CONFIG_SPL_NAND_SIMPLE
> +
> +#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR      0x300 /* address 
> 0x60000 */
> +#define CONFIG_SYS_U_BOOT_MAX_SIZE_SECTORS   0x200 /* 256 KB */

Our U-Boot binary sizes are beyond 300KiB...
This config is not used anywhere besides config files and README...
That's why it probably works for you...
I think we should either remove it from the README and configs, or
set it to 0x300 or even 0x400 instead (in case we still want to use it,
although I don't know why, as we have the size from the image header).

> +#define CONFIG_SYS_MMC_SD_FAT_BOOT_PARTITION 1
> +#define CONFIG_SPL_FAT_LOAD_PAYLOAD_NAME     "u-boot.img"
> +
> +#define CONFIG_SPL_BOARD_INIT
> +#define CONFIG_SPL_LIBCOMMON_SUPPORT
> +#define CONFIG_SPL_LIBDISK_SUPPORT
> +#define CONFIG_SPL_I2C_SUPPORT
> +#define CONFIG_SPL_LIBGENERIC_SUPPORT
> +#define CONFIG_SPL_MMC_SUPPORT
> +#define CONFIG_SPL_FAT_SUPPORT
> +#define CONFIG_SPL_SERIAL_SUPPORT
> +#define CONFIG_SPL_NAND_SUPPORT
> +#define CONFIG_SPL_NAND_BASE
> +#define CONFIG_SPL_NAND_DRIVERS
> +#define CONFIG_SPL_NAND_ECC
> +#define CONFIG_SPL_GPIO_SUPPORT
> +#define CONFIG_SPL_POWER_SUPPORT
> +#define CONFIG_SPL_OMAP3_ID_NAND
> +#define CONFIG_SPL_LDSCRIPT          "$(CPUDIR)/omap-common/u-boot-spl.lds"
> +
> +/* NAND boot config */
> +#define CONFIG_SYS_NAND_5_ADDR_CYCLE
> +#define CONFIG_SYS_NAND_PAGE_COUNT   64
> +#define CONFIG_SYS_NAND_PAGE_SIZE    2048
> +#define CONFIG_SYS_NAND_OOBSIZE              64
> +#define CONFIG_SYS_NAND_BLOCK_SIZE   (128 * 1024)
> +#define CONFIG_SYS_NAND_BAD_BLOCK_POS        NAND_LARGE_BADBLOCK_POS
> +/*
> + * Use the ECC/OOB layout from omap_gpmc.h that matches your chip:
> + * SP vs LP, 8bit vs 16bit: GPMC_NAND_HW_ECC_LAYOUT
> + */
> +#define CONFIG_SYS_NAND_ECCPOS               { 1, 2, 3, 4, 5, 6, 7, 8, 9, \
> +                                      10, 11, 12 }
> +#define CONFIG_SYS_NAND_ECCSIZE              512
> +#define CONFIG_SYS_NAND_ECCBYTES     3
> +#define CONFIG_NAND_OMAP_ECCSCHEME   OMAP_ECC_HAM1_CODE_HW
> +
> +#define CONFIG_SYS_NAND_U_BOOT_START CONFIG_SYS_TEXT_BASE
> +#define CONFIG_SYS_NAND_U_BOOT_OFFS  0x80000
> +
> +#define CONFIG_SPL_TEXT_BASE         0x40200800
> +#define CONFIG_SPL_MAX_SIZE          (54 * 1024)     /* 8 KB for stack */
> +#define CONFIG_SPL_STACK             LOW_LEVEL_SRAM_STACK
> +
> +/*
> + * Use 0x80008000 as TEXT_BASE here for compatibility reasons with the
> + * older x-loader implementations. And move the BSS area so that it
> + * doesn't overlap with TEXT_BASE.
> + */
> +#define CONFIG_SYS_TEXT_BASE         0x80008000
> +#define CONFIG_SPL_BSS_START_ADDR    0x80100000
> +#define CONFIG_SPL_BSS_MAX_SIZE              0x80000         /* 512 KB */
> +
> +#define CONFIG_SYS_SPL_MALLOC_START  0x80208000
> +#define CONFIG_SYS_SPL_MALLOC_SIZE   0x100000
> +
>  #endif /* __CONFIG_H */
> 

-- 
Regards,
Igor.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to