From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lokesh Vutla Date: Fri, 23 Aug 2013 15:55:04 +0530 Subject: [U-Boot] [PATCH V2 3/4] ARM: AM33xx: Move s_init to a common place In-Reply-To: <52172B3D.80601@newflow.co.uk> References: <1375161535-29884-1-git-send-email-lokeshvutla@ti.com> <1375161535-29884-4-git-send-email-lokeshvutla@ti.com> <52172B3D.80601@newflow.co.uk> Message-ID: <52173880.7080207@ti.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 >> >> 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 >> Signed-off-by: Heiko Schocher >> Signed-off-by: Tom Rini > > > > 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 ? >