[U-Boot] [PATCH] OMAP5: Add support for the SOM5_EVB board (OMAP5430-based)

Fri Apr 26 17:59:45 CEST 2013

Hi Tom,

On 25/04/13 22:01, Tom Rini wrote:
> On Mon, Apr 01, 2013 at 05:06:16PM +0300, Lubomir Popov wrote:
> 
>> Signed-off-by: Lubomir Popov <lpopov at mm-sol.com>
> 
> Thought I had reviewed this already, sorry.
Thanks for your review. During the past month U-Boot has changed; I have tried to
follow as well (although I'm engaged with other stuff) and some of your remarks
have been already fixed. Please see my comments inline below.

Anyway, I guess that a V2 patch shall have to be referenced against the current master.
Or against u-boot-ti/next?

Please note that for this board to work with the defined configuration, the following
patches are also required (unfortunately some are already quite old and might cause
conflicts):

- Power: http://patchwork.ozlabs.org/patch/232732/. Potential conflict with Nishanth
  Menon's series of March 26, applied to u-boot-ti/next.
- For I2C support:
  * The patch series of Apr. 8 that enables I2C4 and I2C5 (applied to u-boot-ti/next;
    affects all OMAP5 boards).
  * The modified i2c driver: http://patchwork.ozlabs.org/patch/233823/ (useful for all
    OMAP3/4/5 boards). 
- For USB support:
  * http://patchwork.ozlabs.org/patch/235684/ (affects all OMAP5 boards)
  * http://patchwork.ozlabs.org/patch/232742/ (affects all OMAP5 boards)

> 
>>
>> ---
>>
>> The som5_evb is an evaluation board for the APM-5 - a OMAP5430-based SOM (system
>> On a Module) designed by MMS and manufactured under license by Jorjin Technologies.
> 
> Above the '---' so it's in the changelog.
OK, sure, my fault
> 
>> diff --git a/arch/arm/include/asm/mach-types.h b/arch/arm/include/asm/mach-types.h
>> index a676b6d..236fa17 100644
>> --- a/arch/arm/include/asm/mach-types.h
>> +++ b/arch/arm/include/asm/mach-types.h
>> @@ -1107,6 +1107,7 @@ extern unsigned int __machine_arch_type;
>>  #define MACH_TYPE_OMAP5_SEVM           3777
>>  #define MACH_TYPE_ARMADILLO_800EVA     3863
>>  #define MACH_TYPE_KZM9G                4140
>> +#define MACH_TYPE_SOM5_EVB             4545
> 
> Don't touch this file and instead add just this define to your config,
> and then #define CONFIG_MACH_TYPE MACH_...  If you can't just support
> DT-only.
OK
> 
> [snip]
>> +const struct pad_conf_entry core_padconf_array_non_essential[] = {
>> +#if 0
> 
> Remove #if 0'd code.
Done
>> +	gd->bd->bi_arch_number = MACH_TYPE_SOM5_EVB;
> 
> CONFIG_MACH_TYPE in your config and you don't need this line
All OMAP boards currently have it. Stay as is.
> 
>> +#ifdef CONFIG_TWL6035_AUDPWR
>> +	/* Useful for measuring voltages around the codec.
>> +	 * Works with a TWL604x only.
>> +	 */
> 
> /*
>  * Like this
>  */
Fixed
> 
>> +#ifdef CONFIG_I2C_INTERNAL_PULLUPS
>> +	/* Enable internal I2C-compatible pullups on all buses
>> +	 * TODO: Replace this ugly hardcoding with proper defines (omap.h)
>> +	 * control_smart3io_padconf_0, control_i2c_0
> 
> Do please, thanks.
Was already done.
> 
>> +#define USB_HOST_HS_CLKCTRL_MASK	0x0100D7C0	/* CM_L3INIT_USB_HOST_HS_CLKCTRL */
>> +#define USB_TLL_HS_CLKCTRL_MASK		0x00000700	/* CM_L3INIT_USB_TLL_HS_CLKCTRL */
> 
> Some header please.
Currently moved to board header. I wondered if a common OMAP header wouldn't be
more suitable, but having in mind that the utilized USB ports (and thus the clocks
that should be enabled) vary from board to board, perhaps this (i.e. board header)
is the best place.
> 
>> +	 * TODO: Replace this ugly hardcoding with proper defines
>> +	 */
>> +	writel(0x0100, 0x4ae0a310);
> 
> Again, do please.
This should be (*scrm)->auxclk0. The problem is that the omap5_scrm_regs struct
(holding the auxclk0 member) has to be defined somewhere in the common OMAP5
headers. Sricharan? Or should I hack around?
> 
>> +#define CONFIG_OMAP5430			/* which is in a 5430 */
>> +#define CONFIG_SOM5EVB			/* working with the SOM5 EVB */
> 
> Unused.
Long gone. BTW, board config now includes omap5_common.h.
> 
>> +/* Machine type for Linux */
>> +#define CONFIG_MACH_TYPE		MACH_TYPE_SOM5_EVB
> 
> Ah, since you do have this part already, just update the rest as I had
> said.
/* Machine type for Linux */
#ifndef MACH_TYPE_SOM5_EVB
#define MACH_TYPE_SOM5_EVB		4545
#endif
#define CONFIG_MACH_TYPE		MACH_TYPE_SOM5_EVB

Is this OK?

> 
>> +/* Enable all clocks: */
>> +/*#define CONFIG_SYS_CLOCKS_ENABLE_ALL*/
>> +
>> +#define CONFIG_SYS_ENABLE_PADS_ALL	/* Enable all PADS for now */
> 
> Not allowed.
Shall see to it. What if one needs to test pins and connections during
board bring-up, e.g. via gpio commands?
> 
>> +#define CONFIG_SYS_CA15_VECTOR_BASE	SRAM_ROM_VECT_BASE
> 
> Introduced in the USB patches?
Not used and removed a while ago.
> 
>> +#define CONFIG_SYS_BAUDRATE_TABLE	{4800, 9600, 19200, 38400, \
>> +					57600, 115200}
> 
> Please use the fallback table.
Already done.
> 
>> +/* MMC ENV related defines */
>> +#define CONFIG_ENV_IS_NOWHERE
>> +#ifdef CONFIG_ENV_IS_IN_MMC
>> +#define CONFIG_SYS_MMC_ENV_DEV		1	/* SLOT2: eMMC(1) */
>> +#define CONFIG_ENV_OFFSET		0xE0000
>> +#define CONFIG_CMD_SAVEENV
>> +#endif
> 
> This is a little un-clear. If MMC will be in mmc like the uEVM, just do
> so, and if no storage of env, leave it as NOWHERE.
Cerrently looks like this:
/* MMC ENV related defines */
#undef CONFIG_ENV_IS_IN_MMC
#undef CONFIG_SYS_MMC_ENV_DEV
#undef CONFIG_ENV_OFFSET
#define CONFIG_ENV_IS_NOWHERE

> 
>> +#define	CONFIG_CMD_PING		/* PING support */
> 
> space not tab, fix globally.
Done.
> 
>> +#define CONFIG_NET_MULTI
> 
> Unused.
Removed.
> 
>> +	"usbethaddr=9e:77:39:1b:c4:2d\0" \
> 
> Not allowed.
Gone.
> 
>> +/*
>> + * memtest setup
>> + */
>> +#define CONFIG_SYS_MEMTEST_START	0xb8000000
>> +#define CONFIG_SYS_MEMTEST_END		(CONFIG_SYS_MEMTEST_START + (256 << 20))
>> +/* Undef following two for simple mtest */
>> +#define CONFIG_SYS_ALT_MEMTEST
>> +#define CONFIG_SYS_MEMTEST_SCRATCH	0x81000000
> 
> Please see doc/README.memory-test and update as mtest is no longer a
> default command.
Now the config tail looks like this:

/* Undef/remove after bring-up: */
#define CONFIG_CMD_MEMTEST

/* Disabled commands */
#undef CONFIG_CMD_SAVEENV

/* Prompt */
#define CONFIG_SYS_PROMPT		"SOM5_EVB # "

#ifdef CONFIG_CMD_MEMTEST
/* Undef following two for simple mtest */
#define CONFIG_SYS_ALT_MEMTEST
#define CONFIG_SYS_MEMTEST_SCRATCH	0xb7fffff0
#ifdef CONFIG_SYS_ALT_MEMTEST
#undef CONFIG_SYS_MEMTEST_START
#define CONFIG_SYS_MEMTEST_START	0xb8000000
#undef CONFIG_SYS_MEMTEST_END
#define CONFIG_SYS_MEMTEST_END		(CONFIG_SYS_MEMTEST_START + (256 << 20))
#endif
#endif /* CONFIG_CMD_MEMTEST */

> 
> Thanks!
> 