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

Lokesh Vutla
2013-08-23T10:25:04Z

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@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},
>> +};
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 ?
>