All of lore.kernel.org
 help / color / mirror / Atom feed
From: Igor Grinberg <grinberg@compulab.co.il>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v5] arm: omap3: Add SPL support to cm_t35
Date: Wed, 11 Dec 2013 16:35:03 +0200	[thread overview]
Message-ID: <52A87817.6030708@compulab.co.il> (raw)
In-Reply-To: <1386161658-8890-1-git-send-email-sr@denx.de>

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 <sr@denx.de>
> Cc: Tom Rini <trini@ti.com>
> Cc: Igor Grinberg <grinberg@compulab.co.il>
> Cc: Nikita Kiryanov <nikita@compulab.co.il>

Apart from the comment below,
Acked-by: Igor Grinberg <grinberg@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.

  reply	other threads:[~2013-12-11 14:35 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-04 12:54 [U-Boot] [PATCH v5] arm: omap3: Add SPL support to cm_t35 Stefan Roese
2013-12-11 14:35 ` Igor Grinberg [this message]
2013-12-11 14:55   ` Stefan Roese
2013-12-12  8:27     ` Igor Grinberg
2013-12-13 12:46 ` [U-Boot] [U-Boot,v5] " Tom Rini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52A87817.6030708@compulab.co.il \
    --to=grinberg@compulab.co.il \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.