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

[Lokesh Vutla]

Hi Mark,

On Friday 23 August 2013 02:58 PM, Mark Jackson wrote:
> On 30/07/13 06:18, Lokesh Vutla wrote:
>> From: Heiko Schocher <hs at 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 at ti.com>
>> Signed-off-by: Heiko Schocher <hs at denx.de>
>> Signed-off-by: Tom Rini <trini at 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},
>> +};
 Is this configuration not working for you?
>> +
>> +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 code writes 0x9 into 8 continuous registers starting from
0x44e108a4, this is what done in module_pin_mux norboot_pin_mux
except that it has 9 registers(i guess 9th register was added by mistake..:( )
Correct me if I am wrong.

So you are telling this is wrong but boots properly ?
Steve in CC can comment more on this configuration.

Thanks and regards,
Lokesh

> 
> ... this now boots correctly !!
> 
> Anyone care to comment ?
>