[U-Boot] [PATCH V2 3/4] ARM: AM33xx: Move s_init to a common place

[Mark Jackson <mpfj-list@newflow.co.uk>]

On 30/07/13 06:18, Lokesh Vutla wrote:
> From: Heiko Schocher <hs@denx.de>
> 
> s_init has the same outline for all the AM33xx based
> board. So making it generic.
> This also helps in addition of new Soc with minimal changes.
> 
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> Signed-off-by: Heiko Schocher <hs@denx.de>
> Signed-off-by: Tom Rini <trini@ti.com>

<snip>

This patch introduces the following new function call ...

> +void s_init(void)
> +{
> +	/*
> +	 * The ROM will only have set up sufficient pinmux to allow for the
> +	 * first 4KiB NOR to be read, we must finish doing what we know of
> +	 * the NOR mux in this space in order to continue.
> +	 */
> +#ifdef CONFIG_NOR_BOOT
> +	enable_norboot_pin_mux();
> +#endif

... which replaces the old code ...

> -	/*
> -	 * The ROM will only have set up sufficient pinmux to allow for the
> -	 * first 4KiB NOR to be read, we must finish doing what we know of
> -	 * the NOR mux in this space in order to continue.
> -	 */
> -#ifdef CONFIG_NOR_BOOT
> -	asm("stmfd      sp!, {r2 - r4}");
> -	asm("movw       r4, #0x8A4");
> -	asm("movw       r3, #0x44E1");
> -	asm("orr        r4, r4, r3, lsl #16");
> -	asm("mov        r2, #9");
> -	asm("mov        r3, #8");
> -	asm("gpmc_mux:  str     r2, [r4], #4");
> -	asm("subs       r3, r3, #1");
> -	asm("bne        gpmc_mux");
> -	asm("ldmfd      sp!, {r2 - r4}");
> -#endif

Now (for the TI boards) enable_norboot_pin_mux() is defined as:-

> +#if defined(CONFIG_NOR_BOOT)
> +static struct module_pin_mux norboot_pin_mux[] = {
> +	{OFFSET(lcd_data1), MODE(1) | PULLUDDIS},
> +	{OFFSET(lcd_data2), MODE(1) | PULLUDDIS},
> +	{OFFSET(lcd_data3), MODE(1) | PULLUDDIS},
> +	{OFFSET(lcd_data4), MODE(1) | PULLUDDIS},
> +	{OFFSET(lcd_data5), MODE(1) | PULLUDDIS},
> +	{OFFSET(lcd_data6), MODE(1) | PULLUDDIS},
> +	{OFFSET(lcd_data7), MODE(1) | PULLUDDIS},
> +	{OFFSET(lcd_data8), MODE(1) | PULLUDDIS},
> +	{OFFSET(lcd_data9), MODE(1) | PULLUDDIS},
> +	{-1},
> +};
> +
> +void enable_norboot_pin_mux(void)
> +{
> +	configure_module_pin_mux(norboot_pin_mux);
> +}
> +#endif

Firstly, this pinmux code seems wrong, since lcd_data pin map:-

lcd_data1 (mode 1) => gpmc_a1_mux1
lcd_data2 (mode 1) => gpmc_a2_mux1
lcd_data3 (mode 1) => gpmc_a3_mux1
lcd_data4 (mode 1) => gpmc_a4_mux1
lcd_data5 (mode 1) => gpmc_a5_mux1
lcd_data6 (mode 1) => gpmc_a6_mux1
lcd_data7 (mode 1) => gpmc_a7_mux1
lcd_data8 (mode 1) => gpmc_a12_mux1
lcd_data9 (mode 1) => gpmc_a13_mux1

Doesn't this leave gpmc_a[8..11] unconfigured ?
Shouldn't we configure lcd_vsync, lcd_hsync and lcd_pclk ?

Secondly, I've modded our Nanobone code to match this new setup, as follows:-

static struct module_pin_mux norboot_pin_mux[] = {
	{OFFSET(lcd_data1), (MODE(1) | PULLUDDIS)},	/* GPMC A17 */
	{OFFSET(lcd_data2), (MODE(1) | PULLUDDIS)},	/* GPMC A18 */
	{OFFSET(lcd_data3), (MODE(1) | PULLUDDIS)},	/* GPMC A19 */
	{OFFSET(lcd_data4), (MODE(1) | PULLUDDIS)},	/* GPMC A20 */
	{OFFSET(lcd_data5), (MODE(1) | PULLUDDIS)},	/* GPMC A21 */
	{OFFSET(lcd_data6), (MODE(1) | PULLUDDIS)},	/* GPMC A22 */
	{OFFSET(lcd_data7), (MODE(1) | PULLUDDIS)},	/* GPMC A23 */
	{OFFSET(lcd_vsync), (MODE(1) | PULLUDDIS)},	/* GPMC A24 */
	{OFFSET(lcd_hsync), (MODE(1) | PULLUDDIS)},	/* GPMC A25 */
	{OFFSET(lcd_pclk), (MODE(1) | PULLUDDIS)},	/* GPMC A26 */
	{-1},
};

void enable_norboot_pin_mux(void)
{
	configure_module_pin_mux(norboot_pin_mux);
}

But this fails to boot. However, if I use the old ASM code:-

void enable_norboot_pin_mux(void)
{
	asm("stmfd      sp!, {r2 - r4}");
	asm("movw       r4, #0x8A4");
	asm("movw       r3, #0x44E1");
	asm("orr        r4, r4, r3, lsl #16");
	asm("mov        r2, #9");
	asm("mov        r3, #8");
	asm("gpmc_mux:  str     r2, [r4], #4");
	asm("subs       r3, r3, #1");
	asm("bne        gpmc_mux");
	asm("ldmfd      sp!, {r2 - r4}");
}

... this now boots correctly !!

Anyone care to comment ?