AM3517 EVM / Crane enhancements
cover AM3517 EVM / Crane enhancements
Hey all,
The following series enhances AM3517EVM support as follows:
- Make use of uEnv.txt on MMC, when possible (#4).
- Add build targets for supporting NOR flash and booting from NOR flash (#5).
* As the commit messages say, this requires hardware modifications for NOR to be seen at all, which is why this isn't part of the standard config.
- Enable ethernet (#7)
While enabling ethernet for the EVM, also enable it on Crane (#8).
To do all of this we needed the following general bugfixes or enhancements:
#1: SYSBOOT_MASK wasn't including the peripheral/memory flag. #2: gpmc_init had an extra delay that we didn't need.
#3: gpmc_init needs to know what to do (and not do) when NOR is present. #6: We need to read the ethaddr for the EMAC and put that into the
env when possible.
Patch #5 is large but it's a lot of shuffling of the contents around in order to be able to say "setup this way for NOR" and "setup this way for NAND".
Signature · 1 line
Tom
PATCH 1/8 OMAP3: Correct SYSBOOT_MASK
The comment in get_boot_type says that bit 5 indicates if we are peripheral or memory booting, but the mask wasn't including this. Fix.
--- arch/arm/include/asm/arch-omap3/cpu.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/include/asm/arch-omap3/cpu.h +1 -1
@@ -78,7 +78,7 @@ struct ctrl_id {/* device type */ #define DEVICE_MASK (0x7 << 8) -#define SYSBOOT_MASK 0x1F +#define SYSBOOT_MASK 0x3F#define TST_DEVICE 0x0#define EMU_DEVICE 0x1#define HS_DEVICE 0x2
Signature · 1 line
1.7.0.4
PATCH 2/8 OMAP3: Drop extra delay in gpmc_init
In reply to Tom Rinigpmc_init() was disabling chip select 0 and then waiting the required amount of time for the change to settle before calling enable_gpmc_cs_config on CS 0 which starts by disabling and then sleeping. Remove this redundancy.
--- arch/arm/cpu/armv7/omap3/mem.c | 7 ------- 1 files changed, 0 insertions(+), 7 deletions(-)
arch/arm/cpu/armv7/omap3/mem.c +0 -7
@@ -142,13 +142,6 @@ void gpmc_init(void) config &= (~0xf00 );writel (config, &gpmc_cfg->config); -/* -* Disable the GPMC0 config set by ROM code -* It conflicts with our MPDB (both at 0x08000000) -*/ -writel (0 , &gpmc_cfg->cs[0 ].config7); -sdelay (1000 ); -#if defined(CONFIG_CMD_NAND )/* CS 0 */ gpmc_config = gpmc_m_nand;
Signature · 1 line
1.7.0.4
PATCH 3/8 AM3517: Add NOR Flash boot mode Support
In reply to Tom RiniFrom: Vaibhav Hiremath <[email protected]>
Please note that NOR Flash is located on Application board and requires hardware modification to get NOR boot mode working.
NOR Flash boot mode configuration -
- From baseboard remove R235 resistor.
- Set switch S11.3 position to "ON"
- Set S7 switch position to
1 2 3 4 5
-----------------
on off off off off
Please note that, once you remove R235 resistor from the baseboard, you will not be able to boot from NAND without Application board. The GPMC_nCS0 is now routed through Application board.
Please note that, <Rev4 revision of Application board doesn't work with NOR Flash due to HW issue.
--- arch/arm/cpu/armv7/omap3/mem.c | 61 +++++++++++++++++++++++++++----- arch/arm/include/asm/arch-omap3/mem.h | 15 +++++--- board/logicpd/am3517evm/am3517evm.h | 2 +- 3 files changed, 61 insertions(+), 17 deletions(-)
arch/arm/cpu/armv7/omap3/mem.c +51 -10
@@ -51,6 +51,17 @@ static const u32 gpmc_m_nand[GPMC_MAX_REG] = {#endif +#if defined (CONFIG_CMD_FLASH ) +static const u32 gpmc_nor[GPMC_MAX_REG ] = { +STNOR_GPMC_CONFIG1 , +STNOR_GPMC_CONFIG2 , +STNOR_GPMC_CONFIG3 , +STNOR_GPMC_CONFIG4 , +STNOR_GPMC_CONFIG5 , +STNOR_GPMC_CONFIG6 ,0 +}; +#endif +#if defined(CONFIG_CMD_ONENAND )static const u32 gpmc_onenand[GPMC_MAX_REG ] = { ONENAND_GPMC_CONFIG1,
@@ -118,21 +129,34 @@ void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base,sdelay (2000 ); } -/***************************************************** +/* * gpmc_init(): init gpmc bus -* Init GPMC for x16, MuxMode (SDRAM in x32). -* This code can only be executed from SRAM or SDRAM. -*****************************************************/ +* Init GPMC for x16, MuxMode (SDRAM in x32). This code can only be +* executed from SRAM or SDRAM. In the common case, we will disable +* and then configure chip select 0 for our needs (NAND or OneNAND). +* However, on the AM3517 EVM the way that NOR can be exposed requires us +* to rethink this. When NOR is enabled, it will be chip select 0 if +* we are booting from NOR or chip select 2 otherwise. This requires us +* to check the value we get from the SYSBOOT pins. +*/ void gpmc_init (void ) {/* putting a blanket check on GPMC based on ZeBu for now */ gpmc_cfg = (struct gpmc *)GPMC_BASE ; -#if defined(CONFIG_CMD_NAND ) || defined(CONFIG_CMD_ONENAND ) +#if defined(CONFIG_CMD_NAND ) || defined(CONFIG_CMD_ONENAND ) || \ + (defined(CONFIG_OMAP3_AM3517EVM ) && defined(CONFIG_SYS_HAS_NORFLASH ))const u32 *gpmc_config =NULL ;u32 base =0 ; -u32 size =0 ; +u32 sz =0 ;#endif u32 config =0 ; +u32 nor_boot =0 ; + +#if defined(CONFIG_OMAP3_AM3517EVM ) && defined(CONFIG_SYS_HAS_NORFLASH ) +/* 0xD means NOR boot on AM35x */ +if (get_boot_type () ==0xD ) + nor_boot =1 ; +#endif /* global settings */ writel (0 , &gpmc_cfg->irqenable);/* isr's sources masked */
@@ -146,14 +170,31 @@ void gpmc_init(void) gpmc_config = gpmc_m_nand; base =PISMO1_NAND_BASE ; - size =PISMO1_NAND_SIZE ; -enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[0 ], base, size); + sz =PISMO1_NAND_SIZE ; +if (!nor_boot) +enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[0 ], base, sz); +else +enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[2 ], base, sz); +#endif #if defined(CONFIG_CMD_ONENAND ) gpmc_config = gpmc_onenand; base =PISMO1_ONEN_BASE ; - size =PISMO1_ONEN_SIZE ; -enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[0 ], base, size); + sz =PISMO1_ONEN_SIZE ; +if (!nor_boot) +enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[0 ], base, sz); +else +enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[2 ], base, sz); + +#endif + +#if defined(CONFIG_OMAP3_AM3517EVM ) && defined(CONFIG_SYS_HAS_NORFLASH ) +/* NOR - CS2 */ + gpmc_config = gpmc_nor; + base =DEBUG_BASE ; + sz =GPMC_SIZE_64M ; +if (!nor_boot) +enable_gpmc_cs_config (gpmc_nor, &gpmc_cfg->cs[2 ], base, sz);#endif }
arch/arm/include/asm/arch-omap3/mem.h +9 -6
@@ -329,12 +329,15 @@ enum {#define M_NAND_GPMC_CONFIG6 0x1f0f0A80#define M_NAND_GPMC_CONFIG7 0x00000C44 -#define STNOR_GPMC_CONFIG1 0x3 -#define STNOR_GPMC_CONFIG2 0x00151501 -#define STNOR_GPMC_CONFIG3 0x00060602 -#define STNOR_GPMC_CONFIG4 0x11091109 -#define STNOR_GPMC_CONFIG5 0x01141F1F -#define STNOR_GPMC_CONFIG6 0x000004c4 +/* +* Configuration required for AM3517EVM PC28F640P30B85 Flash +*/ +#define STNOR_GPMC_CONFIG1 0x00001210 +#define STNOR_GPMC_CONFIG2 0x00101001 +#define STNOR_GPMC_CONFIG3 0x00020201 +#define STNOR_GPMC_CONFIG4 0x0f031003 +#define STNOR_GPMC_CONFIG5 0x000f1111 +#define STNOR_GPMC_CONFIG6 0x0f030080#define SIBNOR_GPMC_CONFIG1 0x1200#define SIBNOR_GPMC_CONFIG2 0x001f1f00
board/logicpd/am3517evm/am3517evm.h +1 -1
@@ -327,7 +327,7 @@ const omap3_sysinfo sysinfo = { MUX_VAL(CP(SYS_CLKREQ), (IEN | PTD | DIS | M0)) \MUX_VAL (CP (SYS_NIRQ ), (IEN |PTU |EN |M0 )) \ /*SYS_nRESWARM */\ -MUX_VAL (CP (SYS_NRESWARM ), (IDIS |PTU |DIS |M4 )) \ +MUX_VAL (CP (SYS_NRESWARM ), (IDIS |PTU |EN |M4 )) \ /* - GPIO30 */\MUX_VAL (CP (SYS_BOOT0 ), (IEN |PTD |DIS |M4 ))/*GPIO_2*/ \/* - PEN_IRQ */ \
Signature · 1 line
1.7.0.4
PATCH 3/8 AM3517: Add NOR Flash boot mode Support
Hi Tom,
Quoted 37 lines · Tom Rini
From: Vaibhav Hiremath <[email protected]>
Please note that NOR Flash is located on Application board and requires hardware modification to get NOR boot mode working.
NOR Flash boot mode configuration -
- From baseboard remove R235 resistor.
- Set switch S11.3 position to "ON"
- Set S7 switch position to
1 2 3 4 5
-----------------
on off off off offPlease note that, once you remove R235 resistor from the baseboard, you will not be able to boot from NAND without Application board. The
GPMC_nCS0is now routed through Application board.Please note that, <Rev4 revision of Application board doesn't work with NOR Flash due to HW issue.
Signed-off-by Vaibhav Hiremath <[email protected]>Signed-off-by Tom Rini <[email protected]>--- arch/arm/cpu/armv7/omap3/mem.c | 61 +++++++++++++++++++++++++++----- arch/arm/include/asm/arch-omap3/mem.h | 15 +++++--- board/logicpd/am3517evm/am3517evm.h | 2 +- 3 files changed, 61 insertions(+), 17 deletions(-)arch/arm/cpu/armv7/omap3/mem.c +0
-0@@ -51,6 +51,17 @@ static const u32 gpmc_m_nand[GPMC_MAX_REG] = {#endif
arch/arm/cpu/armv7/omap3/mem.c static const u32 gpmc_m_nand[GPMC_MAX_REG] = { · line 63
Quoted 10 lines
arch/arm/cpu/armv7/omap3/mem.c +10
-0+#if defined (CONFIG_CMD_FLASH ) +static const u32 gpmc_nor[GPMC_MAX_REG ] = { +STNOR_GPMC_CONFIG1 , +STNOR_GPMC_CONFIG2 , +STNOR_GPMC_CONFIG3 , +STNOR_GPMC_CONFIG4 , +STNOR_GPMC_CONFIG5 , +STNOR_GPMC_CONFIG6 ,0 +}; +#endif
This does not seem to be the right place for these settings... I think those must be board specific.
Quoted 11 lines
arch/arm/cpu/armv7/omap3/mem.c +2
-1+#if defined(CONFIG_CMD_ONENAND )static const u32 gpmc_onenand[GPMC_MAX_REG ] = { ONENAND_GPMC_CONFIG1,@@ -118,21 +129,34 @@ void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base,sdelay (2000 ); } -/***************************************************** +/* * gpmc_init(): init gpmc bus
arch/arm/cpu/armv7/omap3/mem.c void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base, · line 140
Quoted 10 lines
arch/arm/cpu/armv7/omap3/mem.c +7
-3-* Init GPMC for x16, MuxMode (SDRAM in x32). -* This code can only be executed from SRAM or SDRAM. -*****************************************************/ +* Init GPMC for x16, MuxMode (SDRAM in x32). This code can only be +* executed from SRAM or SDRAM. In the common case, we will disable +* and then configure chip select 0 for our needs (NAND or OneNAND). +* However, on the AM3517 EVM the way that NOR can be exposed requires us +* to rethink this. When NOR is enabled, it will be chip select 0 if +* we are booting from NOR or chip select 2 otherwise. This requires us +* to check the value we get from the SYSBOOT pins.
All the above looks like board specific...
Quoted 50 lines
arch/arm/cpu/armv7/omap3/mem.c +24
-6+*/ void gpmc_init (void ) {/* putting a blanket check on GPMC based on ZeBu for now */ gpmc_cfg = (struct gpmc *)GPMC_BASE ; -#if defined(CONFIG_CMD_NAND ) || defined(CONFIG_CMD_ONENAND ) +#if defined(CONFIG_CMD_NAND ) || defined(CONFIG_CMD_ONENAND ) || \ + (defined(CONFIG_OMAP3_AM3517EVM ) && defined(CONFIG_SYS_HAS_NORFLASH ))const u32 *gpmc_config =NULL ;u32 base =0 ; -u32 size =0 ; +u32 sz =0 ;#endif u32 config =0 ; +u32 nor_boot =0 ; + +#if defined(CONFIG_OMAP3_AM3517EVM ) && defined(CONFIG_SYS_HAS_NORFLASH ) +/* 0xD means NOR boot on AM35x */ +if (get_boot_type () ==0xD ) + nor_boot =1 ; +#endif /* global settings */ writel (0 , &gpmc_cfg->irqenable);/* isr's sources masked */ @@ -146,14 +170,31 @@ void gpmc_init(void) gpmc_config = gpmc_m_nand; base =PISMO1_NAND_BASE ; - size =PISMO1_NAND_SIZE ; -enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[0 ], base, size); + sz =PISMO1_NAND_SIZE ; +if (!nor_boot) +enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[0 ], base, sz); +else +enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[2 ], base, sz); +#endif #if defined(CONFIG_CMD_ONENAND ) gpmc_config = gpmc_onenand; base =PISMO1_ONEN_BASE ; - size =PISMO1_ONEN_SIZE ; -enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[0 ], base, size); + sz =PISMO1_ONEN_SIZE ; +if (!nor_boot) +enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[0 ], base, sz); +else +enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[2 ], base, sz); + +#endif
arch/arm/cpu/armv7/omap3/mem.c void gpmc_init(void) · line 200
Quoted 10 lines
arch/arm/cpu/armv7/omap3/mem.c +8
-0+ +#if defined(CONFIG_OMAP3_AM3517EVM ) && defined(CONFIG_SYS_HAS_NORFLASH ) +/* NOR - CS2 */ + gpmc_config = gpmc_nor; + base =DEBUG_BASE ; + sz =GPMC_SIZE_64M ; +if (!nor_boot) +enable_gpmc_cs_config (gpmc_nor, &gpmc_cfg->cs[2 ], base, sz);#endif }
All the above look very hackish...
You just bring board specific code into a common location.
I don't think we should be doing it this way.
Instead, change the gpmc_init() function signature to get a parameter(s), so it will be a generic function, that will initialize the right stuff according to the parameters and will not have this board specific ifdeffery crap.
Quoted 13 lines
arch/arm/include/asm/arch-omap3/mem.h +0
-5@@ -329,12 +329,15 @@ enum {#define M_NAND_GPMC_CONFIG6 0x1f0f0A80#define M_NAND_GPMC_CONFIG7 0x00000C44 -#define STNOR_GPMC_CONFIG1 0x3 -#define STNOR_GPMC_CONFIG2 0x00151501 -#define STNOR_GPMC_CONFIG3 0x00060602 -#define STNOR_GPMC_CONFIG4 0x11091109 -#define STNOR_GPMC_CONFIG5 0x01141F1F
arch/arm/include/asm/arch-omap3/mem.h enum { · line 340
Quoted 10 lines
arch/arm/include/asm/arch-omap3/mem.h +9
-1-#define STNOR_GPMC_CONFIG6 0x000004c4 +/* +* Configuration required for AM3517EVM PC28F640P30B85 Flash +*/ +#define STNOR_GPMC_CONFIG1 0x00001210 +#define STNOR_GPMC_CONFIG2 0x00101001 +#define STNOR_GPMC_CONFIG3 0x00020201 +#define STNOR_GPMC_CONFIG4 0x0f031003 +#define STNOR_GPMC_CONFIG5 0x000f1111 +#define STNOR_GPMC_CONFIG6 0x0f030080
I see also:
arch/arm/cpu/armv7/omap3/lowlevel_init.S:184: .word STNOR_GPMC_CONFIG3 arch/arm/cpu/armv7/omap3/lowlevel_init.S:188: .word STNOR_GPMC_CONFIG4 arch/arm/cpu/armv7/omap3/lowlevel_init.S:190: .word STNOR_GPMC_CONFIG5
I haven't looked into this, but will your change break anything else?
Quoted 16 lines
arch/arm/include/asm/arch-omap3/mem.h +0
-0#define SIBNOR_GPMC_CONFIG1 0x1200#define SIBNOR_GPMC_CONFIG2 0x001f1f00board/logicpd/am3517evm/am3517evm.h +1
-1@@ -327,7 +327,7 @@ const omap3_sysinfo sysinfo = { MUX_VAL(CP(SYS_CLKREQ), (IEN | PTD | DIS | M0)) \MUX_VAL (CP (SYS_NIRQ ), (IEN |PTU |EN |M0 )) \ /*SYS_nRESWARM */\ -MUX_VAL (CP (SYS_NRESWARM ), (IDIS |PTU |DIS |M4 )) \ +MUX_VAL (CP (SYS_NRESWARM ), (IDIS |PTU |EN |M4 )) \ /* - GPIO30 */\MUX_VAL (CP (SYS_BOOT0 ), (IEN |PTD |DIS |M4 ))/*GPIO_2*/ \/* - PEN_IRQ */ \
Signature · 2 lines
Regards, Igor.
PATCH 3/8 AM3517: Add NOR Flash boot mode Support
Quoted 40 lines · Igor Grinberg
Hi Tom,
Quoted 37 lines · Tom Rini
From: Vaibhav Hiremath <[email protected]>
Please note that NOR Flash is located on Application board and requires hardware modification to get NOR boot mode working.
NOR Flash boot mode configuration -
? ? ? ? - From baseboard remove R235 resistor.
? ? ? - Set switch S11.3 position to "ON"
? ? ? - Set S7 switch position to
? ? ? ? ? ? ? ?1 ?2 ? 3 ? 4 ? 5
? ? ? ? ? ? ? ?-----------------
? ? ? ? ? ? ? on off off off offPlease note that, once you remove R235 resistor from the baseboard, you will not be able to boot from NAND without Application board. The
GPMC_nCS0is now routed through Application board.Please note that, <Rev4 revision of Application board doesn't work with NOR Flash due to HW issue.
Signed-off-by Vaibhav Hiremath <[email protected]>Signed-off-by Tom Rini <[email protected]>---
?arch/arm/cpu/armv7/omap3/mem.c ? ? ? ?| ? 61 +++++++++++++++++++++++++++----- ?arch/arm/include/asm/arch-omap3/mem.h | ? 15 +++++--- ?board/logicpd/am3517evm/am3517evm.h ? | ? ?2 +-?3 files changed, 61 insertions(+), 17 deletions(-)
arch/arm/cpu/armv7/omap3/mem.c +0
-0@@ -51,6 +51,17 @@ static const u32 gpmc_m_nand[GPMC_MAX_REG] = {?#endif
Quoted 13 lines
arch/arm/cpu/armv7/omap3/mem.c
Quoted 10 lines
arch/arm/cpu/armv7/omap3/mem.c +10
-0+#if defined (CONFIG_CMD_FLASH ) +static const u32 gpmc_nor[GPMC_MAX_REG ] = { + ? ? STNOR_GPMC_CONFIG1, + ? ? STNOR_GPMC_CONFIG2, + ? ? STNOR_GPMC_CONFIG3, + ? ? STNOR_GPMC_CONFIG4, + ? ? STNOR_GPMC_CONFIG5, + ? ? STNOR_GPMC_CONFIG6, 0 +}; +#endif This does not seem to be the right place for these settings... I think those must be board specific.
I'm not sure, it's hard to tell with just one board having NOR and using this code (and I don't have access to custom boards with NOR on them, but I know they exist). In NAND/OneNAND land, this apparently isn't board-specific settings we're shoving in here. Another argument in favor of not just shoving magic values behind a define.
Quoted 78 lines
Quoted 12 lines
?#if
defined(CONFIG_CMD_ONENAND)
?static const u32 gpmc_onenand[GPMC_MAX_REG] = {
? ? ?ONENAND_GPMC_CONFIG1,arch/arm/cpu/armv7/omap3/mem.c +1
-1@@ -118,21 +129,34 @@ void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base, ? ? ? sdelay(2000); ?} -/***************************************************** +/* ? *
gpmc_init(): init gpmc bus
- * Init GPMC for x16, MuxMode (SDRAM in x32).
- * This code can only be executed from SRAM or SDRAM.Quoted 8 lines
- *****************************************************/
+ * Init GPMC for x16, MuxMode (SDRAM in x32). ?This code can only be
+ * executed from SRAM or SDRAM. ?In the common case, we will disable
+ * and then configure chip select 0 for our needs (NAND or OneNAND).
+ * However, on the AM3517 EVM the way that NOR can be exposed requires us
+ * to rethink this. ?When NOR is enabled, it will be chip select 0 if
+ * we are booting from NOR or chip select 2 otherwise. ?This requires us
+ * to check the value we get from the SYSBOOT pins.All the above looks like board specific...
Quoted 55 lines
+ */
?voidgpmc_init(void)
?{
? ? ? /* putting a blanket check on GPMC based on ZeBu for now */ ? ? ?gpmc_cfg= (struct gpmc *)GPMC_BASE;arch/arm/cpu/armv7/omap3/mem.c +2
-1-#if defined(CONFIG_CMD_NAND ) || defined(CONFIG_CMD_ONENAND ) +#if defined(CONFIG_CMD_NAND ) || defined(CONFIG_CMD_ONENAND ) || \ + ? ? (defined(CONFIG_OMAP3_AM3517EVM) && defined(CONFIG_SYS_HAS_NORFLASH))? ? ? const u32
*gpmc_config= NULL;
? ? ? u32 base = 0;
- ? ? u32 size = 0;
+ ? ? u32 sz = 0;
?#endif
? ? ? u32 config = 0;
+ ? ? u32nor_boot= 0;arch/arm/cpu/armv7/omap3/mem.c +6
-0+ +#if defined(CONFIG_OMAP3_AM3517EVM ) && defined(CONFIG_SYS_HAS_NORFLASH ) + ? ? /* 0xD means NOR boot on AM35x */ + ? ? if (get_boot_type() == 0xD) + ? ? ? ? ? ? nor_boot = 1; +#endif ? ? ? /* global settings */
? ? ?writel(0, &gpmc_cfg->irqenable); /* isr's sources masked */arch/arm/cpu/armv7/omap3/mem.c +6
-2@@ -146,14 +170,31 @@ void gpmc_init(void) ? ? ? gpmc_config = gpmc_m_nand; ? ? ? base = PISMO1_NAND_BASE; - ? ? size = PISMO1_NAND_SIZE; - ? ? enable_gpmc_cs_config(gpmc_config, &gpmc_cfg->cs[0], base, size); + ? ? sz = PISMO1_NAND_SIZE; + ? ? if (!nor_boot) + ? ? ? ? ? ? enable_gpmc_cs_config(gpmc_config, &gpmc_cfg->cs[0], base, sz); + ? ? else + ? ? ? ? ? ? enable_gpmc_cs_config(gpmc_config, &gpmc_cfg->cs[2], base, sz); +?#endif
?#if
defined(CONFIG_CMD_ONENAND)
? ? ?gpmc_config=gpmc_onenand;
? ? ? base =PISMO1_ONEN_BASE;
- ? ? size =PISMO1_ONEN_SIZE;
- ? ?enable_gpmc_cs_config(gpmc_config, &gpmc_cfg->cs[0], base, size);
+ ? ? sz =PISMO1_ONEN_SIZE;
+ ? ? if (!nor_boot)
+ ? ? ? ? ? ?enable_gpmc_cs_config(gpmc_config, &gpmc_cfg->cs[0], base, sz);
+ ? ? else
+ ? ? ? ? ? ?enable_gpmc_cs_config(gpmc_config, &gpmc_cfg->cs[2], base, sz);arch/arm/cpu/armv7/omap3/mem.c +7
-0+ +#endif + +#if defined(CONFIG_OMAP3_AM3517EVM ) && defined(CONFIG_SYS_HAS_NORFLASH ) + ? ? /* NOR - CS2 */ + ? ? gpmc_config = gpmc_nor; + ? ? base = DEBUG_BASE;
Quoted 8 lines
Quoted 5 lines
+ ? ? sz =
GPMC_SIZE_64M;
+ ? ? if (!nor_boot)
+ ? ? ? ? ? ?enable_gpmc_cs_config(gpmc_nor, &gpmc_cfg->cs[2], base, sz); ?#endif
?}All the above look very hackish...
You just bring board specific code into a common location.
I'm not sure it is board-specific, but I don't have another NOR using example around...
Quoted 5 lines
I don't think we should be doing it this way.
Instead, change thegpmc_init()function signature to get a parameter(s), so it will be a generic function, that will initialize the right stuff according to the parameters and will not have this board specific ifdeffery crap.
This I think is right, gpmc_init needs a re-think.
Quoted 29 lines
Quoted 13 lines
arch/arm/include/asm/arch-omap3/mem.h +0
-0@@ -329,12 +329,15 @@ enum {?#define
M_NAND_GPMC_CONFIG6?0x1f0f0A80
?#defineM_NAND_GPMC_CONFIG7?0x00000C44arch/arm/include/asm/arch-omap3/mem.h +0
-5-#define STNOR_GPMC_CONFIG1 ? 0x3 -#define STNOR_GPMC_CONFIG2 ? 0x00151501 -#define STNOR_GPMC_CONFIG3 ? 0x00060602 -#define STNOR_GPMC_CONFIG4 ? 0x11091109 -#define STNOR_GPMC_CONFIG5 ? 0x01141F1Farch/arm/include/asm/arch-omap3/mem.h
Quoted 10 lines
arch/arm/include/asm/arch-omap3/mem.h +9
-1-#define STNOR_GPMC_CONFIG6 ? 0x000004c4 +/* +* Configuration required for AM3517EVM PC28F640P30B85 Flash +*/ +#define STNOR_GPMC_CONFIG1 ? 0x00001210 +#define STNOR_GPMC_CONFIG2 ? 0x00101001 +#define STNOR_GPMC_CONFIG3 ? 0x00020201 +#define STNOR_GPMC_CONFIG4 ? 0x0f031003 +#define STNOR_GPMC_CONFIG5 ? 0x000f1111 +#define STNOR_GPMC_CONFIG6 ? 0x0f030080I see also:
arch/arm/cpu/armv7/omap3/lowlevel_init.S:184: ? .wordSTNOR_GPMC_CONFIG3arch/arm/cpu/armv7/omap3/lowlevel_init.S:188: ? .wordSTNOR_GPMC_CONFIG4arch/arm/cpu/armv7/omap3/lowlevel_init.S:190: ? .wordSTNOR_GPMC_CONFIG5
Quoted 1 line
I haven't looked into this, but will your change break anything else?
I think the lowlevel_init.S bits need stepping around with a JTAG on a few boards, but I believe no since no other boards have NOR.
Signature · 1 line
Tom
PATCH 3/8 AM3517: Add NOR Flash boot mode Support
Quoted 55 lines · Tom Rini
Quoted 40 lines · Igor Grinberg
Hi Tom,
Quoted 37 lines · Tom Rini
From: Vaibhav Hiremath <[email protected]>
Please note that NOR Flash is located on Application board and requires hardware modification to get NOR boot mode working.
NOR Flash boot mode configuration -
- From baseboard remove R235 resistor.
- Set switch S11.3 position to "ON"
- Set S7 switch position to
1 2 3 4 5
-----------------
on off off off offPlease note that, once you remove R235 resistor from the baseboard, you will not be able to boot from NAND without Application board. The
GPMC_nCS0is now routed through Application board.Please note that, <Rev4 revision of Application board doesn't work with NOR Flash due to HW issue.
Signed-off-by Vaibhav Hiremath <[email protected]>Signed-off-by Tom Rini <[email protected]>--- arch/arm/cpu/armv7/omap3/mem.c | 61 +++++++++++++++++++++++++++----- arch/arm/include/asm/arch-omap3/mem.h | 15 +++++--- board/logicpd/am3517evm/am3517evm.h | 2 +- 3 files changed, 61 insertions(+), 17 deletions(-)arch/arm/cpu/armv7/omap3/mem.c +0
-0@@ -51,6 +51,17 @@ static const u32 gpmc_m_nand[GPMC_MAX_REG] = {#endif Quoted 13 lines
arch/arm/cpu/armv7/omap3/mem.c static const u32 gpmc_m_nand[GPMC_MAX_REG] = { · line 63
Quoted 10 lines
arch/arm/cpu/armv7/omap3/mem.c +10
-0+#if defined (CONFIG_CMD_FLASH ) +static const u32 gpmc_nor[GPMC_MAX_REG ] = { +STNOR_GPMC_CONFIG1 , +STNOR_GPMC_CONFIG2 , +STNOR_GPMC_CONFIG3 , +STNOR_GPMC_CONFIG4 , +STNOR_GPMC_CONFIG5 , +STNOR_GPMC_CONFIG6 ,0 +}; +#endif This does not seem to be the right place for these settings... I think those must be board specific.
Quoted 3 lines
I'm not sure, it's hard to tell with just one board having NOR and using this code (and I don't have access to custom boards with NOR on them, but I know they exist).
You don't have to have an access, it is software we are on here and it should be generic and flexible enough to allow any custom boards use the common implementation by just providing the information about the custom part of it.
Quoted 3 lines
In NAND/OneNAND land, this apparently
isn't board-specific settings we're shoving in here. Another argument in favor of not just shoving magic values behind a define.
No, it is also wrong, look in the arch/arm/include/asm/arch-omap3/mem.h file, it has at least two configurations for NAND and NOR each.
I haven't checked the usage for NOR configurations, but both NAND configurations are in use.
Probably, not every person, who adds board support, has the confidence to change generic functions in a way that will serve everybody, so they use it for general init and then reprogram the chip select to new values.
Quoted 88 lines
Quoted 79 lines
Quoted 11 lines
arch/arm/cpu/armv7/omap3/mem.c +1
-1#if defined(CONFIG_CMD_ONENAND )static const u32 gpmc_onenand[GPMC_MAX_REG ] = { ONENAND_GPMC_CONFIG1,@@ -118,21 +129,34 @@ void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base,sdelay (2000 ); } -/***************************************************** +/* * gpmc_init(): init gpmc bus arch/arm/cpu/armv7/omap3/mem.c void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base, · line 140
Quoted 10 lines
arch/arm/cpu/armv7/omap3/mem.c +7
-3-* Init GPMC for x16, MuxMode (SDRAM in x32). -* This code can only be executed from SRAM or SDRAM. -*****************************************************/ +* Init GPMC for x16, MuxMode (SDRAM in x32). This code can only be +* executed from SRAM or SDRAM. In the common case, we will disable +* and then configure chip select 0 for our needs (NAND or OneNAND). +* However, on the AM3517 EVM the way that NOR can be exposed requires us +* to rethink this. When NOR is enabled, it will be chip select 0 if +* we are booting from NOR or chip select 2 otherwise. This requires us +* to check the value we get from the SYSBOOT pins. All the above looks like board specific...
Quoted 55 lines
arch/arm/cpu/armv7/omap3/mem.c +29
-6+*/ void gpmc_init (void ) {/* putting a blanket check on GPMC based on ZeBu for now */ gpmc_cfg = (struct gpmc *)GPMC_BASE ; -#if defined(CONFIG_CMD_NAND ) || defined(CONFIG_CMD_ONENAND ) +#if defined(CONFIG_CMD_NAND ) || defined(CONFIG_CMD_ONENAND ) || \ + (defined(CONFIG_OMAP3_AM3517EVM ) && defined(CONFIG_SYS_HAS_NORFLASH ))const u32 *gpmc_config =NULL ;u32 base =0 ; -u32 size =0 ; +u32 sz =0 ;#endif u32 config =0 ; +u32 nor_boot =0 ; + +#if defined(CONFIG_OMAP3_AM3517EVM ) && defined(CONFIG_SYS_HAS_NORFLASH ) +/* 0xD means NOR boot on AM35x */ +if (get_boot_type () ==0xD ) + nor_boot =1 ; +#endif /* global settings */ writel (0 , &gpmc_cfg->irqenable);/* isr's sources masked */ @@ -146,14 +170,31 @@ void gpmc_init(void) gpmc_config = gpmc_m_nand; base =PISMO1_NAND_BASE ; - size =PISMO1_NAND_SIZE ; -enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[0 ], base, size); + sz =PISMO1_NAND_SIZE ; +if (!nor_boot) +enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[0 ], base, sz); +else +enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[2 ], base, sz); +#endif #if defined(CONFIG_CMD_ONENAND ) gpmc_config = gpmc_onenand; base =PISMO1_ONEN_BASE ; - size =PISMO1_ONEN_SIZE ; -enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[0 ], base, size); + sz =PISMO1_ONEN_SIZE ; +if (!nor_boot) +enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[0 ], base, sz); +else +enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[2 ], base, sz); + +#endif + +#if defined(CONFIG_OMAP3_AM3517EVM ) && defined(CONFIG_SYS_HAS_NORFLASH ) +/* NOR - CS2 */ + gpmc_config = gpmc_nor; + base =DEBUG_BASE ;Quoted 8 lines
arch/arm/cpu/armv7/omap3/mem.c void gpmc_init(void) · line 200
Quoted 5 lines
arch/arm/cpu/armv7/omap3/mem.c +3
-0+ sz =GPMC_SIZE_64M ; +if (!nor_boot) +enable_gpmc_cs_config (gpmc_nor, &gpmc_cfg->cs[2 ], base, sz);#endif }All the above look very hackish...
You just bring board specific code into a common location.
Quoted 2 lines
I'm not sure it is board-specific, but I don't have another NOR using example around...
When you use CONFIG_OMAP3_AM3517EVM - you make it
very board specific...
Quoted 8 lines
Quoted 6 lines
I don't think we should be doing it this way.
Instead, change thegpmc_init()function signature to get a parameter(s), so it will be a generic function, that will initialize the right stuff according to the parameters and will not have this board specific ifdeffery crap.This I think is right,
gpmc_initneeds a re-think.
Well, at least we agree on this.
You are the ti tree maintainer now, right?
I think you should initiate such rethinking ask people to do the right thing and even more, you should not let people merge crap (at least without a good reason for this).
Quoted 35 lines
Quoted 26 lines
Quoted 14 lines
arch/arm/include/asm/arch-omap3/mem.h +0
-5@@ -329,12 +329,15 @@ enum {#define M_NAND_GPMC_CONFIG6 0x1f0f0A80#define M_NAND_GPMC_CONFIG7 0x00000C44 -#define STNOR_GPMC_CONFIG1 0x3 -#define STNOR_GPMC_CONFIG2 0x00151501 -#define STNOR_GPMC_CONFIG3 0x00060602 -#define STNOR_GPMC_CONFIG4 0x11091109 -#define STNOR_GPMC_CONFIG5 0x01141F1Farch/arm/include/asm/arch-omap3/mem.h enum { · line 340
Quoted 10 lines
arch/arm/include/asm/arch-omap3/mem.h +9
-1-#define STNOR_GPMC_CONFIG6 0x000004c4 +/* +* Configuration required for AM3517EVM PC28F640P30B85 Flash +*/ +#define STNOR_GPMC_CONFIG1 0x00001210 +#define STNOR_GPMC_CONFIG2 0x00101001 +#define STNOR_GPMC_CONFIG3 0x00020201 +#define STNOR_GPMC_CONFIG4 0x0f031003 +#define STNOR_GPMC_CONFIG5 0x000f1111 +#define STNOR_GPMC_CONFIG6 0x0f030080I see also:
Quoted 5 lines
arch/arm/cpu/armv7/omap3/lowlevel_init.S:184: .word STNOR_GPMC_CONFIG3 arch/arm/cpu/armv7/omap3/lowlevel_init.S:188: .word STNOR_GPMC_CONFIG4 arch/arm/cpu/armv7/omap3/lowlevel_init.S:190: .word STNOR_GPMC_CONFIG5I haven't looked into this, but will your change break anything else?
I think the
lowlevel_init.Sbits need stepping around with a JTAG on a few boards, but I believe no since no other boards have NOR.
Signature · 2 lines
Regards, Igor.
PATCH 3/8 AM3517: Add NOR Flash boot mode Support
Quoted 167 lines · Igor Grinberg
Quoted 55 lines · Tom Rini
Quoted 40 lines · Igor Grinberg
Hi Tom,
Quoted 37 lines · Tom Rini
From: Vaibhav Hiremath <[email protected]>
Please note that NOR Flash is located on Application board and requires hardware modification to get NOR boot mode working.
NOR Flash boot mode configuration -
- From baseboard remove R235 resistor.
- Set switch S11.3 position to "ON"
- Set S7 switch position to
1 2 3 4 5
-----------------
on off off off offPlease note that, once you remove R235 resistor from the baseboard, you will not be able to boot from NAND without Application board. The
GPMC_nCS0is now routed through Application board.Please note that, <Rev4 revision of Application board doesn't work with NOR Flash due to HW issue.
Signed-off-by Vaibhav Hiremath <[email protected]>Signed-off-by Tom Rini <[email protected]>--- arch/arm/cpu/armv7/omap3/mem.c | 61 +++++++++++++++++++++++++++----- arch/arm/include/asm/arch-omap3/mem.h | 15 +++++--- board/logicpd/am3517evm/am3517evm.h | 2 +- 3 files changed, 61 insertions(+), 17 deletions(-)arch/arm/cpu/armv7/omap3/mem.c +0
-0@@ -51,6 +51,17 @@ static const u32 gpmc_m_nand[GPMC_MAX_REG] = {#endif Quoted 13 lines
arch/arm/cpu/armv7/omap3/mem.c static const u32 gpmc_m_nand[GPMC_MAX_REG] = { · line 63
Quoted 10 lines
arch/arm/cpu/armv7/omap3/mem.c +10
-0+#if defined (CONFIG_CMD_FLASH ) +static const u32 gpmc_nor[GPMC_MAX_REG ] = { +STNOR_GPMC_CONFIG1 , +STNOR_GPMC_CONFIG2 , +STNOR_GPMC_CONFIG3 , +STNOR_GPMC_CONFIG4 , +STNOR_GPMC_CONFIG5 , +STNOR_GPMC_CONFIG6 ,0 +}; +#endif This does not seem to be the right place for these settings... I think those must be board specific.
Quoted 3 lines
I'm not sure, it's hard to tell with just one board having NOR and using this code (and I don't have access to custom boards with NOR on them, but I know they exist).
You don't have to have an access, it is software we are on here and it should be generic and flexible enough to allow any custom boards use the common implementation by just providing the information about the custom part of it.
Quoted 3 lines
In NAND/OneNAND land, this apparently
isn't board-specific settings we're shoving in here. Another argument in favor of not just shoving magic values behind a define.No, it is also wrong, look in the
arch/arm/include/asm/arch-omap3/mem.hfile, it has at least two configurations for NAND and NOR each.
I haven't checked the usage for NOR configurations, but both NAND configurations are in use.
Probably, not every person, who adds board support, has the confidence to change generic functions in a way that will serve everybody, so they use it for general init and then reprogram the chip select to new values.Quoted 88 lines
Quoted 87 lines
Quoted 11 lines
arch/arm/cpu/armv7/omap3/mem.c +1
-1#if defined(CONFIG_CMD_ONENAND )static const u32 gpmc_onenand[GPMC_MAX_REG ] = { ONENAND_GPMC_CONFIG1,@@ -118,21 +129,34 @@ void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base,sdelay (2000 ); } -/***************************************************** +/* * gpmc_init(): init gpmc bus arch/arm/cpu/armv7/omap3/mem.c void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base, · line 140
Quoted 10 lines
arch/arm/cpu/armv7/omap3/mem.c +7
-3-* Init GPMC for x16, MuxMode (SDRAM in x32). -* This code can only be executed from SRAM or SDRAM. -*****************************************************/ +* Init GPMC for x16, MuxMode (SDRAM in x32). This code can only be +* executed from SRAM or SDRAM. In the common case, we will disable +* and then configure chip select 0 for our needs (NAND or OneNAND). +* However, on the AM3517 EVM the way that NOR can be exposed requires us +* to rethink this. When NOR is enabled, it will be chip select 0 if +* we are booting from NOR or chip select 2 otherwise. This requires us +* to check the value we get from the SYSBOOT pins. All the above looks like board specific...
Quoted 50 lines
arch/arm/cpu/armv7/omap3/mem.c +24
-6+*/ void gpmc_init (void ) {/* putting a blanket check on GPMC based on ZeBu for now */ gpmc_cfg = (struct gpmc *)GPMC_BASE ; -#if defined(CONFIG_CMD_NAND ) || defined(CONFIG_CMD_ONENAND ) +#if defined(CONFIG_CMD_NAND ) || defined(CONFIG_CMD_ONENAND ) || \ + (defined(CONFIG_OMAP3_AM3517EVM ) && defined(CONFIG_SYS_HAS_NORFLASH ))const u32 *gpmc_config =NULL ;u32 base =0 ; -u32 size =0 ; +u32 sz =0 ;#endif u32 config =0 ; +u32 nor_boot =0 ; + +#if defined(CONFIG_OMAP3_AM3517EVM ) && defined(CONFIG_SYS_HAS_NORFLASH ) +/* 0xD means NOR boot on AM35x */ +if (get_boot_type () ==0xD ) + nor_boot =1 ; +#endif /* global settings */ writel (0 , &gpmc_cfg->irqenable);/* isr's sources masked */ @@ -146,14 +170,31 @@ void gpmc_init(void) gpmc_config = gpmc_m_nand; base =PISMO1_NAND_BASE ; - size =PISMO1_NAND_SIZE ; -enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[0 ], base, size); + sz =PISMO1_NAND_SIZE ; +if (!nor_boot) +enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[0 ], base, sz); +else +enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[2 ], base, sz); +#endif #if defined(CONFIG_CMD_ONENAND ) gpmc_config = gpmc_onenand; base =PISMO1_ONEN_BASE ; - size =PISMO1_ONEN_SIZE ; -enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[0 ], base, size); + sz =PISMO1_ONEN_SIZE ; +if (!nor_boot) +enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[0 ], base, sz); +else +enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[2 ], base, sz); + +#endif arch/arm/cpu/armv7/omap3/mem.c void gpmc_init(void) · line 200
Quoted 10 lines
arch/arm/cpu/armv7/omap3/mem.c +8
-0+ +#if defined(CONFIG_OMAP3_AM3517EVM ) && defined(CONFIG_SYS_HAS_NORFLASH ) +/* NOR - CS2 */ + gpmc_config = gpmc_nor; + base =DEBUG_BASE ; + sz =GPMC_SIZE_64M ; +if (!nor_boot) +enable_gpmc_cs_config (gpmc_nor, &gpmc_cfg->cs[2 ], base, sz);#endif }All the above look very hackish...
You just bring board specific code into a common location.
Quoted 5 lines
Quoted 2 lines
I'm not sure it is board-specific, but I don't have another NOR using example around...is
When you use
CONFIG_OMAP3_AM3517EVM- you make it
very board specific...
Yes, and what I'm saying is that without seeing some of the other AM35x boards with NOR, I'm unsure if the check should simply be on CONFIG_SYS_AM35X_HAS_NORFLASH or if it is indeed am3517 evm specific due to the hardware changes required to expose NOR. This is moot however as...
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
I don't think we should be doing it this way.
Instead, change thegpmc_init()function signature to get a parameter(s), so it will be a generic function, that will initialize the right stuff according to the parameters and will not have this board specific ifdeffery crap.
Quoted 7 lines
Quoted 1 line
This I think is right,
gpmc_initneeds a re-think.Well, at least we agree on this.
You are the ti tree maintainer now, right?
I think you should initiate such rethinking ask people to do the right thing and even more, you should not let people merge crap (at least without a good reason for this).
To be clear, I was saying that gpmc_init needs a re-think, and that does fall to me to go and do. And in general, I'm deciding to post the work other folks have done at various points in the past as I take "TI trees", figure out what hasn't been re-done by the community (or eventually posted by us) and get it ready for merging.
And yes, gpmc_init should either end up taking the CSs we need to call enable_gpmc_cs_init() on, or maybe turned into a weak function that does the normal case of just CS0 always needs setting to whichever of NAND or OneNAND is set and let am3517evm provide its own more complicated one, I'll have to play around.
And, thanks for the review. More eyes means we'll get all of this code into better shape.
Signature · 1 line
Tom
PATCH 3/8 AM3517: Add NOR Flash boot mode Support
Quoted 193 lines · Tom Rini
Quoted 167 lines · Igor Grinberg
Quoted 55 lines · Tom Rini
Quoted 40 lines · Igor Grinberg
Hi Tom,
Quoted 37 lines · Tom Rini
From: Vaibhav Hiremath <[email protected]>
Please note that NOR Flash is located on Application board and requires hardware modification to get NOR boot mode working.
NOR Flash boot mode configuration -
- From baseboard remove R235 resistor.
- Set switch S11.3 position to "ON"
- Set S7 switch position to
1 2 3 4 5
-----------------
on off off off offPlease note that, once you remove R235 resistor from the baseboard, you will not be able to boot from NAND without Application board. The
GPMC_nCS0is now routed through Application board.Please note that, <Rev4 revision of Application board doesn't work with NOR Flash due to HW issue.
Signed-off-by Vaibhav Hiremath <[email protected]>Signed-off-by Tom Rini <[email protected]>--- arch/arm/cpu/armv7/omap3/mem.c | 61 +++++++++++++++++++++++++++----- arch/arm/include/asm/arch-omap3/mem.h | 15 +++++--- board/logicpd/am3517evm/am3517evm.h | 2 +- 3 files changed, 61 insertions(+), 17 deletions(-)arch/arm/cpu/armv7/omap3/mem.c +0
-0@@ -51,6 +51,17 @@ static const u32 gpmc_m_nand[GPMC_MAX_REG] = {#endif Quoted 13 lines
arch/arm/cpu/armv7/omap3/mem.c static const u32 gpmc_m_nand[GPMC_MAX_REG] = { · line 63
Quoted 10 lines
arch/arm/cpu/armv7/omap3/mem.c +10
-0+#if defined (CONFIG_CMD_FLASH ) +static const u32 gpmc_nor[GPMC_MAX_REG ] = { +STNOR_GPMC_CONFIG1 , +STNOR_GPMC_CONFIG2 , +STNOR_GPMC_CONFIG3 , +STNOR_GPMC_CONFIG4 , +STNOR_GPMC_CONFIG5 , +STNOR_GPMC_CONFIG6 ,0 +}; +#endif This does not seem to be the right place for these settings... I think those must be board specific.
Quoted 3 lines
I'm not sure, it's hard to tell with just one board having NOR and using this code (and I don't have access to custom boards with NOR on them, but I know they exist).
You don't have to have an access, it is software we are on here and it should be generic and flexible enough to allow any custom boards use the common implementation by just providing the information about the custom part of it.
Quoted 3 lines
In NAND/OneNAND land, this apparently
isn't board-specific settings we're shoving in here. Another argument in favor of not just shoving magic values behind a define.No, it is also wrong, look in the
arch/arm/include/asm/arch-omap3/mem.hfile, it has at least two configurations for NAND and NOR each.
I haven't checked the usage for NOR configurations, but both NAND configurations are in use.
Probably, not every person, who adds board support, has the confidence to change generic functions in a way that will serve everybody, so they use it for general init and then reprogram the chip select to new values.Quoted 88 lines
Quoted 87 lines
Quoted 11 lines
arch/arm/cpu/armv7/omap3/mem.c +1
-1#if defined(CONFIG_CMD_ONENAND )static const u32 gpmc_onenand[GPMC_MAX_REG ] = { ONENAND_GPMC_CONFIG1,@@ -118,21 +129,34 @@ void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base,sdelay (2000 ); } -/***************************************************** +/* * gpmc_init(): init gpmc bus arch/arm/cpu/armv7/omap3/mem.c void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base, · line 140
Quoted 10 lines
arch/arm/cpu/armv7/omap3/mem.c +7
-3-* Init GPMC for x16, MuxMode (SDRAM in x32). -* This code can only be executed from SRAM or SDRAM. -*****************************************************/ +* Init GPMC for x16, MuxMode (SDRAM in x32). This code can only be +* executed from SRAM or SDRAM. In the common case, we will disable +* and then configure chip select 0 for our needs (NAND or OneNAND). +* However, on the AM3517 EVM the way that NOR can be exposed requires us +* to rethink this. When NOR is enabled, it will be chip select 0 if +* we are booting from NOR or chip select 2 otherwise. This requires us +* to check the value we get from the SYSBOOT pins. All the above looks like board specific...
Quoted 50 lines
arch/arm/cpu/armv7/omap3/mem.c +24
-6+*/ void gpmc_init (void ) {/* putting a blanket check on GPMC based on ZeBu for now */ gpmc_cfg = (struct gpmc *)GPMC_BASE ; -#if defined(CONFIG_CMD_NAND ) || defined(CONFIG_CMD_ONENAND ) +#if defined(CONFIG_CMD_NAND ) || defined(CONFIG_CMD_ONENAND ) || \ + (defined(CONFIG_OMAP3_AM3517EVM ) && defined(CONFIG_SYS_HAS_NORFLASH ))const u32 *gpmc_config =NULL ;u32 base =0 ; -u32 size =0 ; +u32 sz =0 ;#endif u32 config =0 ; +u32 nor_boot =0 ; + +#if defined(CONFIG_OMAP3_AM3517EVM ) && defined(CONFIG_SYS_HAS_NORFLASH ) +/* 0xD means NOR boot on AM35x */ +if (get_boot_type () ==0xD ) + nor_boot =1 ; +#endif /* global settings */ writel (0 , &gpmc_cfg->irqenable);/* isr's sources masked */ @@ -146,14 +170,31 @@ void gpmc_init(void) gpmc_config = gpmc_m_nand; base =PISMO1_NAND_BASE ; - size =PISMO1_NAND_SIZE ; -enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[0 ], base, size); + sz =PISMO1_NAND_SIZE ; +if (!nor_boot) +enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[0 ], base, sz); +else +enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[2 ], base, sz); +#endif #if defined(CONFIG_CMD_ONENAND ) gpmc_config = gpmc_onenand; base =PISMO1_ONEN_BASE ; - size =PISMO1_ONEN_SIZE ; -enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[0 ], base, size); + sz =PISMO1_ONEN_SIZE ; +if (!nor_boot) +enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[0 ], base, sz); +else +enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[2 ], base, sz); + +#endif arch/arm/cpu/armv7/omap3/mem.c void gpmc_init(void) · line 200
Quoted 10 lines
arch/arm/cpu/armv7/omap3/mem.c +8
-0+ +#if defined(CONFIG_OMAP3_AM3517EVM ) && defined(CONFIG_SYS_HAS_NORFLASH ) +/* NOR - CS2 */ + gpmc_config = gpmc_nor; + base =DEBUG_BASE ; + sz =GPMC_SIZE_64M ; +if (!nor_boot) +enable_gpmc_cs_config (gpmc_nor, &gpmc_cfg->cs[2 ], base, sz);#endif }All the above look very hackish...
You just bring board specific code into a common location.Quoted 5 lines
Quoted 2 lines
I'm not sure it is board-specific, but I don't have another NOR using example around...is
When you use
CONFIG_OMAP3_AM3517EVM- you make it
very board specific...Yes, and what I'm saying is that without seeing some of the other AM35x boards with NOR, I'm unsure if the check should simply be on
CONFIG_SYS_AM35X_HAS_NORFLASHor if it is indeed am3517 evm specific due to the hardware changes required to expose NOR. This is moot however as...Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
I don't think we should be doing it this way.
Instead, change thegpmc_init()function signature to get a parameter(s), so it will be a generic function, that will initialize the right stuff according to the parameters and will not have this board specific ifdeffery crap.Quoted 7 lines
Quoted 1 line
This I think is right,
gpmc_initneeds a re-think.Well, at least we agree on this.
You are the ti tree maintainer now, right?
I think you should initiate such rethinking ask people to do the right thing and even more, you should not let people merge crap (at least without a good reason for this).
Quoted 5 lines
To be clear, I was saying that
gpmc_initneeds a re-think, and that does fall to me to go and do. And in general, I'm deciding to post the work other folks have done at various points in the past as I take "TI trees", figure out what hasn't been re-done by the community (or eventually posted by us) and get it ready for merging.
That would be great!
Quoted 6 lines
And yes,
gpmc_initshould either end up taking the CSs we need to callenable_gpmc_cs_init()on, or maybe turned into a weak function that does the normal case of just CS0 always needs setting to whichever of NAND or OneNAND is set and let am3517evm provide its own more complicated one, I'll have to play around.
Good.
I vote for the same function taking different routes depending on the parameters passed, but probably, it can be best seen while working on this.
Thanks
Signature · 2 lines
Regards, Igor.
Hiremath
PATCH 3/8 AM3517: Add NOR Flash boot mode Support
In reply to Igor GrinbergQuoted 50 lines
-----Original Message-----
From: Igor Grinberg [mailto:grinberg at compulab.co.il]
Sent: Wednesday, December 07, 2011 7:55 PM
To: Rini, Tom
Cc: u-boot at lists.denx.de; Hiremath, Vaibhav
Subject: Re: [U-Boot] [PATCH 3/8] AM3517: Add NOR Flash boot mode SupportHi Tom,
Quoted 18 lines · Tom Rini
From: Vaibhav Hiremath <[email protected]>
Please note that NOR Flash is located on Application board and requires hardware modification to get NOR boot mode working.
NOR Flash boot mode configuration -
- From baseboard remove R235 resistor.
- Set switch S11.3 position to "ON"
- Set S7 switch position to
1 2 3 4 5
-----------------
on off off off offPlease note that, once you remove R235 resistor from the baseboard, you will not be able to boot from NAND without Application board. The
GPMC_nCS0is now routed through Application board.Quoted 7 lines
Please note that, <Rev4 revision of Application board doesn't work with NOR Flash due to HW issue.
Signed-off-by Vaibhav Hiremath <[email protected]>Signed-off-by Tom Rini <[email protected]>--- arch/arm/cpu/armv7/omap3/mem.c | 61+++++++++++++++++++++++++++-----
Review
Quoted 5 lines
arch/arm/include/asm/arch-omap3/mem.h | 15 +++++--- board/logicpd/am3517evm/am3517evm.h | 2 +- 3 files changed, 61 insertions(+), 17 deletions(-)(quoted diff) +0
-0
b/arch/arm/cpu/armv7/omap3/mem.cQuoted 7 lines
index 4a8c025..4ad3d12 100644
arch/arm/cpu/armv7/omap3/mem.c +0
-0@@ -51,6 +51,17 @@ static const u32 gpmc_m_nand[GPMC_MAX_REG] = {#endif
Quoted 14 lines
arch/arm/cpu/armv7/omap3/mem.c static const u32 gpmc_m_nand[GPMC_MAX_REG] = { · line 63
Quoted 10 lines
arch/arm/cpu/armv7/omap3/mem.c +10
-0+#if defined (CONFIG_CMD_FLASH ) +static const u32 gpmc_nor[GPMC_MAX_REG ] = { +STNOR_GPMC_CONFIG1 , +STNOR_GPMC_CONFIG2 , +STNOR_GPMC_CONFIG3 , +STNOR_GPMC_CONFIG4 , +STNOR_GPMC_CONFIG5 , +STNOR_GPMC_CONFIG6 ,0 +}; +#endif This does not seem to be the right place for these settings... I think those must be board specific.
[Hiremath, Vaibhav] I think, yes this information is board specific, since NOR flash is mounted on board.
Actually we have followed existing NAND/OneNAND implementation here, and that's where we have this here. Also note that, in case of OMAP3/AM37x we have POP package where NAND/OneNAND part is fixed, so it makes sense to bring here.
Quoted 93 lines
arch/arm/cpu/armv7/omap3/mem.c void enable_gpmc_cs_config(const u32 *gpmc_config,
Quoted 5 lines
arch/arm/cpu/armv7/omap3/mem.c +1
-0+#if defined(CONFIG_CMD_ONENAND )static const u32 gpmc_onenand[GPMC_MAX_REG ] = { ONENAND_GPMC_CONFIG1,@@ -118,21 +129,34 @@ void enable_gpmc_cs_config(const u32 *gpmc_config,struct
gpmc_cs*cs, u32 base,arch/arm/cpu/armv7/omap3/mem.c void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base,
Quoted 13 lines
arch/arm/cpu/armv7/omap3/mem.c +5
-4sdelay (2000 ); } -/***************************************************** +/* * gpmc_init(): init gpmc bus -* Init GPMC for x16, MuxMode (SDRAM in x32). -* This code can only be executed from SRAM or SDRAM. -*****************************************************/ +* Init GPMC for x16, MuxMode (SDRAM in x32). This code can only be +* executed from SRAM or SDRAM. In the common case, we will disable +* and then configure chip select 0 for our needs (NAND or OneNAND). +* However, on the AM3517 EVM the way that NOR can be exposed requires us
arch/arm/cpu/armv7/omap3/mem.c void enable_gpmc_cs_config(const u32 *gpmc_config, · line 139
Quoted 2 lines
arch/arm/cpu/armv7/omap3/mem.c +2
-0+* to rethink this. When NOR is enabled, it will be chip select 0 if +* we are booting from NOR or chip select 2 otherwise. This requires us
arch/arm/cpu/armv7/omap3/mem.c void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base, · line 140
Quoted 1 line
arch/arm/cpu/armv7/omap3/mem.c +1
-0+* to check the value we get from the SYSBOOT pins. All the above looks like board specific...
arch/arm/cpu/armv7/omap3/mem.c void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base,
Quoted 8 lines
arch/arm/cpu/armv7/omap3/mem.c +3
-1+*/ void gpmc_init (void ) {/* putting a blanket check on GPMC based on ZeBu for now */ gpmc_cfg = (struct gpmc *)GPMC_BASE ; -#if defined(CONFIG_CMD_NAND ) || defined(CONFIG_CMD_ONENAND ) +#if defined(CONFIG_CMD_NAND) || defined(CONFIG_CMD_ONENAND) || \ + (defined(CONFIG_OMAP3_AM3517EVM) &&
defined(CONFIG_SYS_HAS_NORFLASH))Quoted 17 lines
arch/arm/cpu/armv7/omap3/mem.c +8
-1const u32 *gpmc_config =NULL ;u32 base =0 ; -u32 size =0 ; +u32 sz =0 ;#endif u32 config =0 ; +u32 nor_boot =0 ; + +#if defined(CONFIG_OMAP3_AM3517EVM ) && defined(CONFIG_SYS_HAS_NORFLASH ) +/* 0xD means NOR boot on AM35x */ +if (get_boot_type () ==0xD ) + nor_boot =1 ; +#endif /* global settings */ writel (0 , &gpmc_cfg->irqenable);/* isr's sources masked */ arch/arm/cpu/armv7/omap3/mem.c void gpmc_init(void) · line 175
Quoted 8 lines
arch/arm/cpu/armv7/omap3/mem.c +3
-2@@ -146,14 +170,31 @@ void gpmc_init(void) gpmc_config = gpmc_m_nand; base =PISMO1_NAND_BASE ; - size =PISMO1_NAND_SIZE ; -enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[0 ], base, size); + sz =PISMO1_NAND_SIZE ; +if (!nor_boot) + enable_gpmc_cs_config(gpmc_config, &gpmc_cfg->cs[0], base,sz);
arch/arm/cpu/armv7/omap3/mem.c void gpmc_init(void) · line 177
Quoted 2 lines
arch/arm/cpu/armv7/omap3/mem.c +2
-0+else + enable_gpmc_cs_config(gpmc_config, &gpmc_cfg->cs[2], base,sz);
arch/arm/cpu/armv7/omap3/mem.c void gpmc_init(void)
Quoted 11 lines
arch/arm/cpu/armv7/omap3/mem.c +4
-2+#endif #if defined(CONFIG_CMD_ONENAND ) gpmc_config = gpmc_onenand; base =PISMO1_ONEN_BASE ; - size =PISMO1_ONEN_SIZE ; -enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[0 ], base, size); + sz =PISMO1_ONEN_SIZE ; +if (!nor_boot) + enable_gpmc_cs_config(gpmc_config, &gpmc_cfg->cs[0], base,sz);
arch/arm/cpu/armv7/omap3/mem.c void gpmc_init(void) · line 188
Quoted 2 lines
arch/arm/cpu/armv7/omap3/mem.c +2
-0+else + enable_gpmc_cs_config(gpmc_config, &gpmc_cfg->cs[2], base,sz);
arch/arm/cpu/armv7/omap3/mem.c void gpmc_init(void) · line 200
Quoted 12 lines
arch/arm/cpu/armv7/omap3/mem.c +10
-0+ +#endif + +#if defined(CONFIG_OMAP3_AM3517EVM ) && defined(CONFIG_SYS_HAS_NORFLASH ) +/* NOR - CS2 */ + gpmc_config = gpmc_nor; + base =DEBUG_BASE ; + sz =GPMC_SIZE_64M ; +if (!nor_boot) +enable_gpmc_cs_config (gpmc_nor, &gpmc_cfg->cs[2 ], base, sz);#endif }
Quoted 8 lines
All the above look very hackish...
You just bring board specific code into a common location.
I don't think we should be doing it this way.
Instead, change thegpmc_init()function signature to get a parameter(s), so it will be a generic function, that will initialize the right stuff according to the parameters and will not have this board specific ifdeffery crap.
[Hiremath, Vaibhav] Agreed, gpmc_init needs some cleanup to make it generic interface. And probably all data should get passed from board file.
Quoted 26 lines
Review
Quoted 1 line
(quoted diff) +0
-0
b/arch/arm/include/asm/arch-omap3/mem.hQuoted 12 lines
index 5fd02d4..2f52481 100644
arch/arm/include/asm/arch-omap3/mem.h +0
-5@@ -329,12 +329,15 @@ enum {#define M_NAND_GPMC_CONFIG6 0x1f0f0A80#define M_NAND_GPMC_CONFIG7 0x00000C44 -#define STNOR_GPMC_CONFIG1 0x3 -#define STNOR_GPMC_CONFIG2 0x00151501 -#define STNOR_GPMC_CONFIG3 0x00060602 -#define STNOR_GPMC_CONFIG4 0x11091109 -#define STNOR_GPMC_CONFIG5 0x01141F1Farch/arm/include/asm/arch-omap3/mem.h enum { · line 340
Quoted 10 lines
arch/arm/include/asm/arch-omap3/mem.h +9
-1-#define STNOR_GPMC_CONFIG6 0x000004c4 +/* +* Configuration required for AM3517EVM PC28F640P30B85 Flash +*/ +#define STNOR_GPMC_CONFIG1 0x00001210 +#define STNOR_GPMC_CONFIG2 0x00101001 +#define STNOR_GPMC_CONFIG3 0x00020201 +#define STNOR_GPMC_CONFIG4 0x0f031003 +#define STNOR_GPMC_CONFIG5 0x000f1111 +#define STNOR_GPMC_CONFIG6 0x0f030080I see also:
Quoted 6 lines
arch/arm/cpu/armv7/omap3/lowlevel_init.S:184: .word STNOR_GPMC_CONFIG3 arch/arm/cpu/armv7/omap3/lowlevel_init.S:188: .word STNOR_GPMC_CONFIG4 arch/arm/cpu/armv7/omap3/lowlevel_init.S:190: .word STNOR_GPMC_CONFIG5I haven't looked into this, but will your change break anything else?
[Hiremath, Vaibhav] I do not think it breaks anything.
Quoted 21 lines
Review
Quoted 4 lines
arch/arm/include/asm/arch-omap3/mem.h +0
-0#define SIBNOR_GPMC_CONFIG1 0x1200#define SIBNOR_GPMC_CONFIG2 0x001f1f00(quoted diff) +0
-0
b/board/logicpd/am3517evm/am3517evm.hboard/logicpd/am3517evm/am3517evm.h const omap3_sysinfo sysinfo = { · line 333
Quoted 12 lines
index 68d746c..17bb78d 100644
board/logicpd/am3517evm/am3517evm.h +1
-1@@ -327,7 +327,7 @@ const omap3_sysinfo sysinfo = { MUX_VAL(CP(SYS_CLKREQ), (IEN | PTD | DIS | M0)) \MUX_VAL (CP (SYS_NIRQ ), (IEN |PTU |EN |M0 )) \ /*SYS_nRESWARM */\ -MUX_VAL (CP (SYS_NRESWARM ), (IDIS |PTU |DIS |M4 )) \ +MUX_VAL (CP (SYS_NRESWARM ), (IDIS |PTU |EN |M4 )) \ /* - GPIO30 */\MUX_VAL (CP (SYS_BOOT0 ), (IEN |PTD |DIS |M4 ))/*GPIO_2*/ \/* - PEN_IRQ */ \--
Regards,
Igor.
PATCH 3/8 AM3517: Add NOR Flash boot mode Support
Hi Vaibhav,
Quoted 64 lines · Vaibhav
Quoted 50 lines
-----Original Message-----
From: Igor Grinberg [mailto:grinberg at compulab.co.il]
Sent: Wednesday, December 07, 2011 7:55 PM
To: Rini, Tom
Cc: u-boot at lists.denx.de; Hiremath, Vaibhav
Subject: Re: [U-Boot] [PATCH 3/8] AM3517: Add NOR Flash boot mode SupportHi Tom,
Quoted 18 lines · Tom Rini
From: Vaibhav Hiremath <[email protected]>
Please note that NOR Flash is located on Application board and requires hardware modification to get NOR boot mode working.
NOR Flash boot mode configuration -
- From baseboard remove R235 resistor.
- Set switch S11.3 position to "ON"
- Set S7 switch position to
1 2 3 4 5
-----------------
on off off off offPlease note that, once you remove R235 resistor from the baseboard, you will not be able to boot from NAND without Application board. The
GPMC_nCS0is now routed through Application board.Quoted 7 lines
Please note that, <Rev4 revision of Application board doesn't work with NOR Flash due to HW issue.
Signed-off-by Vaibhav Hiremath <[email protected]>Signed-off-by Tom Rini <[email protected]>--- arch/arm/cpu/armv7/omap3/mem.c | 61+++++++++++++++++++++++++++-----
Review
Quoted 5 lines
arch/arm/include/asm/arch-omap3/mem.h | 15 +++++--- board/logicpd/am3517evm/am3517evm.h | 2 +- 3 files changed, 61 insertions(+), 17 deletions(-)(quoted diff) +0
-0
b/arch/arm/cpu/armv7/omap3/mem.cQuoted 7 lines
index 4a8c025..4ad3d12 100644
arch/arm/cpu/armv7/omap3/mem.c +0
-0@@ -51,6 +51,17 @@ static const u32 gpmc_m_nand[GPMC_MAX_REG] = {#endif Quoted 14 lines
arch/arm/cpu/armv7/omap3/mem.c static const u32 gpmc_m_nand[GPMC_MAX_REG] = { · line 63
Quoted 10 lines
arch/arm/cpu/armv7/omap3/mem.c +10
-0+#if defined (CONFIG_CMD_FLASH ) +static const u32 gpmc_nor[GPMC_MAX_REG ] = { +STNOR_GPMC_CONFIG1 , +STNOR_GPMC_CONFIG2 , +STNOR_GPMC_CONFIG3 , +STNOR_GPMC_CONFIG4 , +STNOR_GPMC_CONFIG5 , +STNOR_GPMC_CONFIG6 ,0 +}; +#endif This does not seem to be the right place for these settings... I think those must be board specific.
Quoted 3 lines
[Hiremath, Vaibhav] I think, yes this information is board specific, since NOR flash is mounted on board.
Actually we have followed existing NAND/OneNAND implementation here, and that's where we have this here.
Yeah, I know you did.
The thing is we can continue adding crap code, just because it was like that before, but that is not a good argument. Instead, IMO, if you need to touch that code, make an effort and clean this at least a bit and then add a clean version of the code. I think it worked like this at least last year.
Quoted 1 line
Also note that, in case of OMAP3/AM37x we have POP package where NAND/OneNAND part is fixed, so it makes sense to bring here.
No, not really, because, there are other packages besides POP, and information on them should come from the board.
What can be done is, move the NOR information to a separate function/file so it can be reused by all boards, but the board gets to decide whether to use this or may be some other setting.
Also, are you certain that POP package will always have the same parts on top of the SoC, because the pin out can stay the same, but timing information can vary and that's where you need that separation.
Quoted 102 lines
Quoted 94 lines
arch/arm/cpu/armv7/omap3/mem.c void enable_gpmc_cs_config(const u32 *gpmc_config,
Quoted 6 lines
arch/arm/cpu/armv7/omap3/mem.c +1
-0+#if defined(CONFIG_CMD_ONENAND )static const u32 gpmc_onenand[GPMC_MAX_REG ] = { ONENAND_GPMC_CONFIG1,@@ -118,21 +129,34 @@ void enable_gpmc_cs_config(const u32 *gpmc_config,struct
gpmc_cs*cs, u32 base,arch/arm/cpu/armv7/omap3/mem.c void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base,
Quoted 13 lines
arch/arm/cpu/armv7/omap3/mem.c +5
-4sdelay (2000 ); } -/***************************************************** +/* * gpmc_init(): init gpmc bus -* Init GPMC for x16, MuxMode (SDRAM in x32). -* This code can only be executed from SRAM or SDRAM. -*****************************************************/ +* Init GPMC for x16, MuxMode (SDRAM in x32). This code can only be +* executed from SRAM or SDRAM. In the common case, we will disable +* and then configure chip select 0 for our needs (NAND or OneNAND). +* However, on the AM3517 EVM the way that NOR can be exposed requires us
arch/arm/cpu/armv7/omap3/mem.c void enable_gpmc_cs_config(const u32 *gpmc_config, · line 139
Quoted 2 lines
arch/arm/cpu/armv7/omap3/mem.c +2
-0+* to rethink this. When NOR is enabled, it will be chip select 0 if +* we are booting from NOR or chip select 2 otherwise. This requires us
arch/arm/cpu/armv7/omap3/mem.c void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base, · line 140
Quoted 1 line
arch/arm/cpu/armv7/omap3/mem.c +1
-0+* to check the value we get from the SYSBOOT pins. All the above looks like board specific...
arch/arm/cpu/armv7/omap3/mem.c void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base,
Quoted 8 lines
arch/arm/cpu/armv7/omap3/mem.c +3
-1+*/ void gpmc_init (void ) {/* putting a blanket check on GPMC based on ZeBu for now */ gpmc_cfg = (struct gpmc *)GPMC_BASE ; -#if defined(CONFIG_CMD_NAND ) || defined(CONFIG_CMD_ONENAND ) +#if defined(CONFIG_CMD_NAND) || defined(CONFIG_CMD_ONENAND) || \ + (defined(CONFIG_OMAP3_AM3517EVM) &&
defined(CONFIG_SYS_HAS_NORFLASH))Quoted 17 lines
arch/arm/cpu/armv7/omap3/mem.c +8
-1const u32 *gpmc_config =NULL ;u32 base =0 ; -u32 size =0 ; +u32 sz =0 ;#endif u32 config =0 ; +u32 nor_boot =0 ; + +#if defined(CONFIG_OMAP3_AM3517EVM ) && defined(CONFIG_SYS_HAS_NORFLASH ) +/* 0xD means NOR boot on AM35x */ +if (get_boot_type () ==0xD ) + nor_boot =1 ; +#endif /* global settings */ writel (0 , &gpmc_cfg->irqenable);/* isr's sources masked */ arch/arm/cpu/armv7/omap3/mem.c void gpmc_init(void) · line 175
Quoted 8 lines
arch/arm/cpu/armv7/omap3/mem.c +3
-2@@ -146,14 +170,31 @@ void gpmc_init(void) gpmc_config = gpmc_m_nand; base =PISMO1_NAND_BASE ; - size =PISMO1_NAND_SIZE ; -enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[0 ], base, size); + sz =PISMO1_NAND_SIZE ; +if (!nor_boot) + enable_gpmc_cs_config(gpmc_config, &gpmc_cfg->cs[0], base,sz);
arch/arm/cpu/armv7/omap3/mem.c void gpmc_init(void) · line 177
Quoted 2 lines
arch/arm/cpu/armv7/omap3/mem.c +2
-0+else + enable_gpmc_cs_config(gpmc_config, &gpmc_cfg->cs[2], base,sz);
arch/arm/cpu/armv7/omap3/mem.c void gpmc_init(void)
Quoted 11 lines
arch/arm/cpu/armv7/omap3/mem.c +4
-2+#endif #if defined(CONFIG_CMD_ONENAND ) gpmc_config = gpmc_onenand; base =PISMO1_ONEN_BASE ; - size =PISMO1_ONEN_SIZE ; -enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[0 ], base, size); + sz =PISMO1_ONEN_SIZE ; +if (!nor_boot) + enable_gpmc_cs_config(gpmc_config, &gpmc_cfg->cs[0], base,sz);
arch/arm/cpu/armv7/omap3/mem.c void gpmc_init(void) · line 188
Quoted 2 lines
arch/arm/cpu/armv7/omap3/mem.c +2
-0+else + enable_gpmc_cs_config(gpmc_config, &gpmc_cfg->cs[2], base,sz);
arch/arm/cpu/armv7/omap3/mem.c void gpmc_init(void) · line 200
Quoted 12 lines
arch/arm/cpu/armv7/omap3/mem.c +10
-0+ +#endif + +#if defined(CONFIG_OMAP3_AM3517EVM ) && defined(CONFIG_SYS_HAS_NORFLASH ) +/* NOR - CS2 */ + gpmc_config = gpmc_nor; + base =DEBUG_BASE ; + sz =GPMC_SIZE_64M ; +if (!nor_boot) +enable_gpmc_cs_config (gpmc_nor, &gpmc_cfg->cs[2 ], base, sz);#endif }Quoted 8 lines
All the above look very hackish...
You just bring board specific code into a common location.
I don't think we should be doing it this way.
Instead, change thegpmc_init()function signature to get a parameter(s), so it will be a generic function, that will initialize the right stuff according to the parameters and will not have this board specific ifdeffery crap.
Quoted 1 line
[Hiremath, Vaibhav] Agreed,
gpmc_initneeds some cleanup to make it generic interface. And probably all data should get passed from board file.
Good, we have an agreement ;)
Quoted 52 lines
Quoted 27 lines
Review
Quoted 2 lines
(quoted diff) +0
-0
b/arch/arm/include/asm/arch-omap3/mem.hQuoted 12 lines
index 5fd02d4..2f52481 100644
arch/arm/include/asm/arch-omap3/mem.h +0
-5@@ -329,12 +329,15 @@ enum {#define M_NAND_GPMC_CONFIG6 0x1f0f0A80#define M_NAND_GPMC_CONFIG7 0x00000C44 -#define STNOR_GPMC_CONFIG1 0x3 -#define STNOR_GPMC_CONFIG2 0x00151501 -#define STNOR_GPMC_CONFIG3 0x00060602 -#define STNOR_GPMC_CONFIG4 0x11091109 -#define STNOR_GPMC_CONFIG5 0x01141F1Farch/arm/include/asm/arch-omap3/mem.h enum { · line 340
Quoted 10 lines
arch/arm/include/asm/arch-omap3/mem.h +9
-1-#define STNOR_GPMC_CONFIG6 0x000004c4 +/* +* Configuration required for AM3517EVM PC28F640P30B85 Flash +*/ +#define STNOR_GPMC_CONFIG1 0x00001210 +#define STNOR_GPMC_CONFIG2 0x00101001 +#define STNOR_GPMC_CONFIG3 0x00020201 +#define STNOR_GPMC_CONFIG4 0x0f031003 +#define STNOR_GPMC_CONFIG5 0x000f1111 +#define STNOR_GPMC_CONFIG6 0x0f030080I see also:
Quoted 6 lines
arch/arm/cpu/armv7/omap3/lowlevel_init.S:184: .word STNOR_GPMC_CONFIG3 arch/arm/cpu/armv7/omap3/lowlevel_init.S:188: .word STNOR_GPMC_CONFIG4 arch/arm/cpu/armv7/omap3/lowlevel_init.S:190: .word STNOR_GPMC_CONFIG5I haven't looked into this, but will your change break anything else?
[Hiremath, Vaibhav] I do not think it breaks anything.
Quoted 17 lines
Review
Quoted 4 lines
arch/arm/include/asm/arch-omap3/mem.h +0
-0#define SIBNOR_GPMC_CONFIG1 0x1200#define SIBNOR_GPMC_CONFIG2 0x001f1f00(quoted diff) +0
-0
b/board/logicpd/am3517evm/am3517evm.hQuoted 12 lines
index 68d746c..17bb78d 100644
board/logicpd/am3517evm/am3517evm.h +1
-1@@ -327,7 +327,7 @@ const omap3_sysinfo sysinfo = { MUX_VAL(CP(SYS_CLKREQ), (IEN | PTD | DIS | M0)) \MUX_VAL (CP (SYS_NIRQ ), (IEN |PTU |EN |M0 )) \ /*SYS_nRESWARM */\ -MUX_VAL (CP (SYS_NRESWARM ), (IDIS |PTU |DIS |M4 )) \ +MUX_VAL (CP (SYS_NRESWARM ), (IDIS |PTU |EN |M4 )) \ /* - GPIO30 */\MUX_VAL (CP (SYS_BOOT0 ), (IEN |PTD |DIS |M4 ))/*GPIO_2*/ \/* - PEN_IRQ */ \
Signature · 2 lines
Regards, Igor.
Hiremath
PATCH 3/8 AM3517: Add NOR Flash boot mode Support
Quoted 96 lines
-----Original Message-----
From: Igor Grinberg [mailto:grinberg at compulab.co.il]
Sent: Thursday, December 08, 2011 7:18 PM
To: Hiremath, Vaibhav
Cc: Rini, Tom; u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH 3/8] AM3517: Add NOR Flash boot mode SupportHi Vaibhav,
Quoted 7 lines · Vaibhav
Quoted 7 lines
-----Original Message-----
From: Igor Grinberg [mailto:grinberg at compulab.co.il]
Sent: Wednesday, December 07, 2011 7:55 PM
To: Rini, Tom
Cc: u-boot at lists.denx.de; Hiremath, Vaibhav
Subject: Re: [U-Boot] [PATCH 3/8] AM3517: Add NOR Flash boot modeSupport
Quoted 7 lines
Quoted 7 lines
Hi Tom,
Quoted 3 lines · Tom Rini
From: Vaibhav Hiremath <[email protected]>
Please note that NOR Flash is located on Application board and
requires
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
hardware modification to get NOR boot mode working.
NOR Flash boot mode configuration -
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
- From baseboard remove R235 resistor.
- Set switch S11.3 position to "ON"
- Set S7 switch position to
1 2 3 4 5
-----------------
on off off off offPlease note that, once you remove R235 resistor from the baseboard,
you
Quoted 38 lines
Quoted 24 lines
Quoted 10 lines
will not be able to boot from NAND without Application board. The
GPMC_nCS0is now routed through Application board.Please note that, <Rev4 revision of Application board doesn't work with NOR Flash due to HW issue.
Signed-off-by Vaibhav Hiremath <[email protected]>Signed-off-by Tom Rini <[email protected]>--- arch/arm/cpu/armv7/omap3/mem.c | 61+++++++++++++++++++++++++++-----
Review
Quoted 5 lines
arch/arm/include/asm/arch-omap3/mem.h | 15 +++++--- board/logicpd/am3517evm/am3517evm.h | 2 +- 3 files changed, 61 insertions(+), 17 deletions(-)(quoted diff) +0
-0
b/arch/arm/cpu/armv7/omap3/mem.cQuoted 7 lines
index 4a8c025..4ad3d12 100644
arch/arm/cpu/armv7/omap3/mem.c +0
-0@@ -51,6 +51,17 @@ static const u32 gpmc_m_nand[GPMC_MAX_REG] = {#endif Quoted 14 lines
arch/arm/cpu/armv7/omap3/mem.c static const u32 gpmc_m_nand[GPMC_MAX_REG] = { · line 63
Quoted 10 lines
arch/arm/cpu/armv7/omap3/mem.c +10
-0+#if defined (CONFIG_CMD_FLASH ) +static const u32 gpmc_nor[GPMC_MAX_REG ] = { +STNOR_GPMC_CONFIG1 , +STNOR_GPMC_CONFIG2 , +STNOR_GPMC_CONFIG3 , +STNOR_GPMC_CONFIG4 , +STNOR_GPMC_CONFIG5 , +STNOR_GPMC_CONFIG6 ,0 +}; +#endif This does not seem to be the right place for these settings... I think those must be board specific.
Quoted 1 line
[Hiremath, Vaibhav] I think, yes this information is board specific,
since NOR flash is mounted on board.
Quoted 2 lines
Actually we have followed existing NAND/OneNAND implementation here, and
that's where we have this here.
Yeah, I know you did.
The thing is we can continue adding crap code, just because it was like that before, but that is not a good argument. Instead, IMO, if you need to touch that code, make an effort and clean this at least a bit and then add a clean version of the code. I think it worked like this at least last year.Quoted 1 line
Also note that, in case of OMAP3/AM37x we have POP package where
NAND/OneNAND part is fixed, so it makes sense to bring here.
No, not really, because, there are other packages besides POP, and information on them should come from the board.
Quoted 8 lines
What can be done is, move the NOR information to a separate function/file so it can be reused by all boards, but the board gets to decide whether to use this or may be some other setting.
Also, are you certain that POP package will always have the same parts on top of the SoC, because the pin out can stay the same, but timing information can vary and that's where you need that separation.
Fair enough,
I was just trying to say, this could be the reason behind why this definitions kept here in this file.
Thanks,
Vaibhav
Quoted 172 lines
Quoted 6 lines
Quoted 6 lines
arch/arm/cpu/armv7/omap3/mem.c void enable_gpmc_cs_config(const u32
Quoted 6 lines
arch/arm/cpu/armv7/omap3/mem.c +1
-0+#if defined(CONFIG_CMD_ONENAND )static const u32 gpmc_onenand[GPMC_MAX_REG ] = { ONENAND_GPMC_CONFIG1,@@ -118,21 +129,34 @@ void enable_gpmc_cs_config(const u32
*gpmc_config,Quoted 4 lines
Quoted 4 lines
struct
gpmc_cs*cs, u32 base,Quoted 3 lines
arch/arm/cpu/armv7/omap3/mem.c +0
-0sdelay (2000 ); }Quoted 10 lines
Quoted 10 lines
arch/arm/cpu/armv7/omap3/mem.c void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base,
Quoted 10 lines
arch/arm/cpu/armv7/omap3/mem.c +5
-4-/***************************************************** +/* * gpmc_init(): init gpmc bus -* Init GPMC for x16, MuxMode (SDRAM in x32). -* This code can only be executed from SRAM or SDRAM. -*****************************************************/ +* Init GPMC for x16, MuxMode (SDRAM in x32). This code can only be +* executed from SRAM or SDRAM. In the common case, we will disable +* and then configure chip select 0 for our needs (NAND or OneNAND). +* However, on the AM3517 EVM the way that NOR can be exposed requires
Quoted 17 lines
Quoted 17 lines
us
arch/arm/cpu/armv7/omap3/mem.c void enable_gpmc_cs_config(const u32 · line 139
Quoted 2 lines
arch/arm/cpu/armv7/omap3/mem.c +2
-0+* to rethink this. When NOR is enabled, it will be chip select 0 if +* we are booting from NOR or chip select 2 otherwise. This requires us
arch/arm/cpu/armv7/omap3/mem.c void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base, · line 140
Quoted 1 line
arch/arm/cpu/armv7/omap3/mem.c +1
-0+* to check the value we get from the SYSBOOT pins. All the above looks like board specific...
arch/arm/cpu/armv7/omap3/mem.c void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base,
Quoted 8 lines
arch/arm/cpu/armv7/omap3/mem.c +3
-1+*/ void gpmc_init (void ) {/* putting a blanket check on GPMC based on ZeBu for now */ gpmc_cfg = (struct gpmc *)GPMC_BASE ; -#if defined(CONFIG_CMD_NAND ) || defined(CONFIG_CMD_ONENAND ) +#if defined(CONFIG_CMD_NAND) || defined(CONFIG_CMD_ONENAND) || \ + (defined(CONFIG_OMAP3_AM3517EVM) &&
defined(CONFIG_SYS_HAS_NORFLASH))Quoted 9 lines
Quoted 9 lines
arch/arm/cpu/armv7/omap3/mem.c void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base,
Quoted 9 lines
arch/arm/cpu/armv7/omap3/mem.c +4
-1const u32 *gpmc_config =NULL ;u32 base =0 ; -u32 size =0 ; +u32 sz =0 ;#endif u32 config =0 ; +u32 nor_boot =0 ; + +#if defined(CONFIG_OMAP3_AM3517EVM) &&
defined(CONFIG_SYS_HAS_NORFLASH)Quoted 32 lines
Quoted 32 lines
Quoted 8 lines
arch/arm/cpu/armv7/omap3/mem.c +4
-0+/* 0xD means NOR boot on AM35x */ +if (get_boot_type () ==0xD ) + nor_boot =1 ; +#endif /* global settings */ writel (0 , &gpmc_cfg->irqenable);/* isr's sources masked */ arch/arm/cpu/armv7/omap3/mem.c void gpmc_init(void) · line 175
Quoted 8 lines
arch/arm/cpu/armv7/omap3/mem.c +3
-2@@ -146,14 +170,31 @@ void gpmc_init(void) gpmc_config = gpmc_m_nand; base =PISMO1_NAND_BASE ; - size =PISMO1_NAND_SIZE ; -enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[0 ], base, size); + sz =PISMO1_NAND_SIZE ; +if (!nor_boot) + enable_gpmc_cs_config(gpmc_config, &gpmc_cfg->cs[0], base,sz);
arch/arm/cpu/armv7/omap3/mem.c void gpmc_init(void) · line 177
Quoted 2 lines
arch/arm/cpu/armv7/omap3/mem.c +2
-0+else + enable_gpmc_cs_config(gpmc_config, &gpmc_cfg->cs[2], base,sz);
arch/arm/cpu/armv7/omap3/mem.c void gpmc_init(void)
Quoted 11 lines
arch/arm/cpu/armv7/omap3/mem.c +4
-2+#endif #if defined(CONFIG_CMD_ONENAND ) gpmc_config = gpmc_onenand; base =PISMO1_ONEN_BASE ; - size =PISMO1_ONEN_SIZE ; -enable_gpmc_cs_config (gpmc_config, &gpmc_cfg->cs[0 ], base, size); + sz =PISMO1_ONEN_SIZE ; +if (!nor_boot) + enable_gpmc_cs_config(gpmc_config, &gpmc_cfg->cs[0], base,sz);
Quoted 7 lines
Quoted 7 lines
arch/arm/cpu/armv7/omap3/mem.c void gpmc_init(void) · line 188
Quoted 2 lines
arch/arm/cpu/armv7/omap3/mem.c +2
-0+else + enable_gpmc_cs_config(gpmc_config, &gpmc_cfg->cs[2], base,sz);
arch/arm/cpu/armv7/omap3/mem.c void gpmc_init(void)
Quoted 4 lines
arch/arm/cpu/armv7/omap3/mem.c +4
-0+ +#endif + +#if defined(CONFIG_OMAP3_AM3517EVM) &&
defined(CONFIG_SYS_HAS_NORFLASH)Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
arch/arm/cpu/armv7/omap3/mem.c +5
-0+/* NOR - CS2 */ + gpmc_config = gpmc_nor; + base =DEBUG_BASE ; + sz =GPMC_SIZE_64M ; +if (!nor_boot)Quoted 8 lines
Quoted 8 lines
arch/arm/cpu/armv7/omap3/mem.c void gpmc_init(void) · line 200
Quoted 3 lines
arch/arm/cpu/armv7/omap3/mem.c +1
-0+enable_gpmc_cs_config (gpmc_nor, &gpmc_cfg->cs[2 ], base, sz);#endif }All the above look very hackish...
You just bring board specific code into a common location. I don't think we should be doing it this way. Instead, change thegpmc_init()function signature to get aparameter(s),
Quoted 5 lines
Quoted 4 lines
so it will be a generic function, that will initialize the right stuff according to the parameters and will not have this board specific ifdeffery crap.
[Hiremath, Vaibhav] Agreed,
gpmc_initneeds some cleanup to make itgeneric interface. And probably all data should get passed from board file.
Good, we have an agreement ;)
Quoted 20 lines
Quoted 20 lines
Review
Quoted 2 lines
(quoted diff) +0
-0
b/arch/arm/include/asm/arch-omap3/mem.hQuoted 17 lines
index 5fd02d4..2f52481 100644
arch/arm/include/asm/arch-omap3/mem.h +4
-6@@ -329,12 +329,15 @@ enum {#define M_NAND_GPMC_CONFIG6 0x1f0f0A80#define M_NAND_GPMC_CONFIG7 0x00000C44 -#define STNOR_GPMC_CONFIG1 0x3 -#define STNOR_GPMC_CONFIG2 0x00151501 -#define STNOR_GPMC_CONFIG3 0x00060602 -#define STNOR_GPMC_CONFIG4 0x11091109 -#define STNOR_GPMC_CONFIG5 0x01141F1F -#define STNOR_GPMC_CONFIG6 0x000004c4 +/* +* Configuration required for AM3517EVM PC28F640P30B85 Flash +*/ +#define STNOR_GPMC_CONFIG1 0x00001210Quoted 8 lines
Quoted 8 lines
arch/arm/include/asm/arch-omap3/mem.h enum { · line 340
Quoted 5 lines
arch/arm/include/asm/arch-omap3/mem.h +5
-0+#define STNOR_GPMC_CONFIG2 0x00101001 +#define STNOR_GPMC_CONFIG3 0x00020201 +#define STNOR_GPMC_CONFIG4 0x0f031003 +#define STNOR_GPMC_CONFIG5 0x000f1111 +#define STNOR_GPMC_CONFIG6 0x0f030080I see also:
arch/arm/cpu/armv7/omap3/lowlevel_init.S:184: .word
STNOR_GPMC_CONFIG3Quoted 1 line
Quoted 1 line
arch/arm/cpu/armv7/omap3/lowlevel_init.S:188: .word
STNOR_GPMC_CONFIG4Quoted 1 line
Quoted 1 line
arch/arm/cpu/armv7/omap3/lowlevel_init.S:190: .word
STNOR_GPMC_CONFIG5Quoted 10 lines
Quoted 3 lines
I haven't looked into this, but will your change break anything else?
[Hiremath, Vaibhav] I do not think it breaks anything.
Quoted 5 lines
Review
Quoted 4 lines
arch/arm/include/asm/arch-omap3/mem.h +0
-0#define SIBNOR_GPMC_CONFIG1 0x1200#define SIBNOR_GPMC_CONFIG2 0x001f1f00(quoted diff) +0
-0
b/board/logicpd/am3517evm/am3517evm.hQuoted 12 lines
Quoted 4 lines
Quoted 4 lines
index 68d746c..17bb78d 100644
board/logicpd/am3517evm/am3517evm.h +0
-0@@ -327,7 +327,7 @@ const omap3_sysinfo sysinfo = {Quoted 8 lines
board/logicpd/am3517evm/am3517evm.h const omap3_sysinfo sysinfo = { · line 333
Quoted 8 lines
board/logicpd/am3517evm/am3517evm.h +1
-1MUX_VAL(CP(SYS_CLKREQ), (IEN | PTD | DIS | M0)) \MUX_VAL (CP (SYS_NIRQ ), (IEN |PTU |EN |M0 )) \ /*SYS_nRESWARM */\ -MUX_VAL (CP (SYS_NRESWARM ), (IDIS |PTU |DIS |M4 )) \ +MUX_VAL (CP (SYS_NRESWARM ), (IDIS |PTU |EN |M4 )) \ /* - GPIO30 */\MUX_VAL (CP (SYS_BOOT0 ), (IEN |PTD |DIS |M4 ))/*GPIO_2*/ \/* - PEN_IRQ */ \--
Regards,
Igor.
PATCH 4/8 AM3517 EVM: Add uEnv.txt to the default bootcmd
In reply to Tom RiniThis adds uEnv.txt (and optargs/uenvcmd) support to the MMC boot section
--- include/configs/am3517_evm.h | 22 ++++++++++++++++++++-- 1 files changed, 20 insertions(+), 2 deletions(-)
include/configs/am3517_evm.h +20 -2
@@ -191,10 +191,20 @@"loadaddr=0x82000000\0" \"console=ttyO2,115200n8\0" \"mmcdev=0\0" \ -"mmcargs=setenv bootargs console=${console} " \ +"bootenv=uEnv.txt\0" \ +"loadbootenv=fatload mmc ${mmc_dev} ${loadaddr} ${bootenv}\0" \ +"importbootenv=echo Importing environment from mmc ...; " \ +"env import -t $loadaddr $filesize\0" \ +"optargs=\0" \ +"bootargs_defaults=setenv bootargs " \ +"console=${console} " \ +"${optargs}\0" \ +"mmcargs=run bootargs_defaults; " \ +"setenv bootargs ${bootargs} " \"root=/dev/mmcblk0p2 rw " \"rootfstype=ext3 rootwait\0" \ -"nandargs=setenv bootargs console=${console} " \ +"nandargs=run bootargs_defaults; " \ +"setenv bootargs ${bootargs} " \"root=/dev/mtdblock4 rw " \"rootfstype=jffs2\0" \"loadbootscript=fatload mmc ${mmcdev} ${loadaddr} boot.scr\0" \
@@ -214,6 +224,14 @@"if run loadbootscript; then " \"run bootscript; " \"else " \ +"if run loadbootenv; then " \ +"echo Loaded environment from ${bootenv};" \ +"run importbootenv;" \ +"fi;" \ +"if test -n $uenvcmd; then " \ +"echo Running uenvcmd ...;" \ +"run uenvcmd;" \ +"fi;" \"if run loaduimage; then " \"run mmcboot; " \"else run nandboot; " \
Signature · 1 line
1.7.0.4
PATCH 5/8 AM3517 EVM: Add am3517_evm_norflash and _norflash_boot targets
In reply to Tom RiniThese build targets will make a U-Boot that knows about the NOR flash that can be enabled on this board (see previous commits for details) and a U-Boot that can run from NOR flash (u-boot.bin).
When we are building to run from NOR flash we makes changes to the default environment in order to run the system fully from NOR flash. This results in a lot of 'noise' in terms of moving NAND-specific config options to be in one place that we can key off of. We also modify the bootcmd to use the flash the user has chosen (by building for NOR, or NAND).
--- boards.cfg | 2 + include/configs/am3517_evm.h | 142 +++++++++++++++++++++++++---------------- 2 files changed, 88 insertions(+), 56 deletions(-)
boards.cfg +2 -0
@@ -187,6 +187,8 @@ igep0020 arm armv7 igep0020 isee igep0030 arm armv7 igep0030 isee omap3 am3517_crane arm armv7 am3517crane ti omap3 am3517_evm arm armv7 am3517evm logicpd omap3 +am3517_evm_norflash arm armv7 am3517evm logicpd omap3 am3517_evm:SYS_HAS_NORFLASH +am3517_evm_norflash_boot arm armv7 am3517evm logicpd omap3 am3517_evm:SYS_HAS_NORFLASH,SYS_BOOT_NORFLASH dig297 arm armv7 dig297 comelit omap3 omap3_zoom1 arm armv7 zoom1 logicpd omap3 omap3_zoom2 arm armv7 zoom2 logicpd omap3
include/configs/am3517_evm.h +86 -56
233 lines not on this page · raw message
Signature · 1 line
1.7.0.4
PATCH 6/8 AM35xx: Read and set ethaddr for EMAC
In reply to Tom RiniFrom: Steve Kipisz <[email protected]>
--- arch/arm/cpu/armv7/omap3/emac.c | 20 +++++++++++++++++++- arch/arm/include/asm/arch-omap3/emac_defs.h | 3 +++ 2 files changed, 22 insertions(+), 1 deletions(-)
arch/arm/cpu/armv7/omap3/emac.c +19 -1
@@ -26,6 +26,7 @@#include <netdev.h> #include <asm/io.h> #include <asm/arch/am35x_def.h> +#include <asm/arch/emac_defs.h> /* * Initializes on-chip ethernet controllers.
@@ -33,12 +34,29 @@*/ int cpu_eth_init (bd_t *bis) { -u32 reset; +u32 reset, msb, lsb; +u_int8_t macaddr[6 ]; +int i;/* ensure that the module is out of reset */ reset =readl (&am35x_scm_general_regs->ip_sw_reset); reset &= ~CPGMACSS_SW_RST ;writel (reset, &am35x_scm_general_regs->ip_sw_reset); +/* Read MAC address */ + msb =readl (EMAC_MACADDR_MSB ); + lsb =readl (EMAC_MACADDR_LSB ); + +for (i =0 ; i <3 ; i++) { + macaddr[5 - i] = lsb &0xFF ; + lsb >>=8 ; + } + +for (i =0 ; i <3 ; i++) { + macaddr[2 - i] = msb &0xFF ; + msb >>=8 ; + } +eth_setenv_enetaddr ("ethaddr" , macaddr); +return davinci_emac_initialize (); }
arch/arm/include/asm/arch-omap3/emac_defs.h +3 -0
@@ -42,6 +42,9 @@#define EMAC_MDIO_BASE_ADDR 0x5C030000#define EMAC_HW_RAM_ADDR 0x01E20000 +#define EMAC_MACADDR_LSB 0x48002380 +#define EMAC_MACADDR_MSB 0x48002384 +#define EMAC_MDIO_BUS_FREQ 166000000/* 166 MHZ check */ #define EMAC_MDIO_CLOCK_FREQ 1000000/* 2.0 MHz */
Signature · 1 line
1.7.0.4
PATCH 6/8 AM35xx: Read and set ethaddr for EMAC
Hi Steve, Tom,
Quoted 40 lines · Tom Rini
From: Steve Kipisz <[email protected]>
Signed-off-by Steve Kipisz <[email protected]>Signed-off-by Tom Rini <[email protected]>--- arch/arm/cpu/armv7/omap3/emac.c | 20 +++++++++++++++++++- arch/arm/include/asm/arch-omap3/emac_defs.h | 3 +++ 2 files changed, 22 insertions(+), 1 deletions(-)arch/arm/cpu/armv7/omap3/emac.c +9
-1@@ -26,6 +26,7 @@#include <netdev.h> #include <asm/io.h> #include <asm/arch/am35x_def.h> +#include <asm/arch/emac_defs.h> /* * Initializes on-chip ethernet controllers. @@ -33,12 +34,29 @@*/ int cpu_eth_init (bd_t *bis) { -u32 reset; +u32 reset, msb, lsb; +u_int8_t macaddr[6 ]; +int i;/* ensure that the module is out of reset */ reset =readl (&am35x_scm_general_regs->ip_sw_reset); reset &= ~CPGMACSS_SW_RST ;writel (reset, &am35x_scm_general_regs->ip_sw_reset); +/* Read MAC address */ + msb =readl (EMAC_MACADDR_MSB ); + lsb =readl (EMAC_MACADDR_LSB ); + +for (i =0 ; i <3 ; i++) {
arch/arm/cpu/armv7/omap3/emac.c line 60
Quoted 10 lines
arch/arm/cpu/armv7/omap3/emac.c +10
-0+ macaddr[5 - i] = lsb &0xFF ; + lsb >>=8 ; + } + +for (i =0 ; i <3 ; i++) { + macaddr[2 - i] = msb &0xFF ; + msb >>=8 ; + } +eth_setenv_enetaddr ("ethaddr" , macaddr); +
This is a wrong place for this code...
You force every board to use the EFUSE'd MAC address - this is wrong. The board must have a freedom to choose what MAC address it wants to use. You don't even check if ethaddr variable is already set...
What you can do is put this implementation into a separate function and let board to make a decision if it wants to call it or use another MAC address.
Something like:
int am3517_get_efuse_enetaddr(u8 *enetaddr)
{
/* read the address and shift or whatever */
...
return is_valid_ether_addr(enetaddr);
}Quoted 16 lines
arch/arm/cpu/armv7/omap3/emac.c +0
-0return davinci_emac_initialize (); }arch/arm/include/asm/arch-omap3/emac_defs.h +3
-0@@ -42,6 +42,9 @@#define EMAC_MDIO_BASE_ADDR 0x5C030000#define EMAC_HW_RAM_ADDR 0x01E20000 +#define EMAC_MACADDR_LSB 0x48002380 +#define EMAC_MACADDR_MSB 0x48002384 +#define EMAC_MDIO_BUS_FREQ 166000000/* 166 MHZ check */ #define EMAC_MDIO_CLOCK_FREQ 1000000/* 2.0 MHz */
Signature · 2 lines
Regards, Igor.
PATCH 6/8 AM35xx: Read and set ethaddr for EMAC
Quoted 64 lines · Igor Grinberg
Hi Steve, Tom,
Quoted 40 lines · Tom Rini
From: Steve Kipisz <[email protected]>
Signed-off-by Steve Kipisz <[email protected]>Signed-off-by Tom Rini <[email protected]>--- arch/arm/cpu/armv7/omap3/emac.c | 20 +++++++++++++++++++- arch/arm/include/asm/arch-omap3/emac_defs.h | 3 +++ 2 files changed, 22 insertions(+), 1 deletions(-)arch/arm/cpu/armv7/omap3/emac.c +9
-1@@ -26,6 +26,7 @@#include <netdev.h> #include <asm/io.h> #include <asm/arch/am35x_def.h> +#include <asm/arch/emac_defs.h> /* * Initializes on-chip ethernet controllers. @@ -33,12 +34,29 @@*/ int cpu_eth_init (bd_t *bis) { -u32 reset; +u32 reset, msb, lsb; +u_int8_t macaddr[6 ]; +int i;/* ensure that the module is out of reset */ reset =readl (&am35x_scm_general_regs->ip_sw_reset); reset &= ~CPGMACSS_SW_RST ;writel (reset, &am35x_scm_general_regs->ip_sw_reset); +/* Read MAC address */ + msb =readl (EMAC_MACADDR_MSB ); + lsb =readl (EMAC_MACADDR_LSB ); + +for (i =0 ; i <3 ; i++) {arch/arm/cpu/armv7/omap3/emac.c line 60
Quoted 10 lines
arch/arm/cpu/armv7/omap3/emac.c +10
-0+ macaddr[5 - i] = lsb &0xFF ; + lsb >>=8 ; + } + +for (i =0 ; i <3 ; i++) { + macaddr[2 - i] = msb &0xFF ; + msb >>=8 ; + } +eth_setenv_enetaddr ("ethaddr" , macaddr); +This is a wrong place for this code...
You force every board to use the EFUSE'd MAC address - this is wrong. The board must have a freedom to choose what MAC address it wants to use. You don't even check if ethaddr variable is already set...What you can do is put this implementation into a separate function and let board to make a decision if it wants to call it or use another MAC address.
Something like:
intam3517_get_efuse_enetaddr(u8 *enetaddr)
Quoted 5 lines
{
/* read the address and shift or whatever */
...
return is_valid_ether_addr(enetaddr);
}That's reasonable enough. I wasn't sure where the custom boards were getting their MAC as the EVM and Crane both need this change.
Signature · 1 line
Tom
PATCH 6/8 AM35xx: Read and set ethaddr for EMAC
Dear Tom Rini,
Quoted 17 lines · In message <[email protected]> you wrote:
Quoted 11 lines
This is a wrong place for this code...
You force every board to use the EFUSE'd MAC address - this is wrong. The board must have a freedom to choose what MAC address it wants to use. You don't even check if ethaddr variable is already set...What you can do is put this implementation into a separate function and let board to make a decision if it wants to call it or use another MAC address.
Something like:
intam3517_get_efuse_enetaddr(u8 *enetaddr)Quoted 5 lines
{ /* read the address and shift or whatever */ ... return is_valid_ether_addr(enetaddr); }
Quoted 2 lines
That's reasonable enough. I wasn't sure where the custom boards were getting their MAC as the EVM and Crane both need this change.
The rule is that the settings in the environment always have precedence, and you must never overwrite an existing "eth*addr" in the environment.
Best regards,
Wolfgang Denk
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de "They that can give up essential liberty to obtain a little temporary saftey deserve neither liberty not saftey." - Benjamin Franklin, 1759
PATCH 7/8 AM3517 EVM: Enable ethernet
In reply to Tom Rini--- include/configs/am3517_evm.h | 14 ++++++++++++-- 1 files changed, 12 insertions(+), 2 deletions(-)
include/configs/am3517_evm.h +12 -2
@@ -158,8 +158,18 @@#define CONFIG_SYS_I2C_BUS_SELECT 1#define CONFIG_DRIVER_OMAP34XX_I2C 1 -#undef CONFIG_CMD_NET -#undef CONFIG_CMD_NFS +/* +* Ethernet +*/ +#define CONFIG_DRIVER_TI_EMAC +#define CONFIG_DRIVER_TI_EMAC_USE_RMII +#define CONFIG_MII +#define CONFIG_BOOTP_DEFAULT +#define CONFIG_BOOTP_DNS +#define CONFIG_BOOTP_DNS2 +#define CONFIG_BOOTP_SEND_HOSTNAME +#define CONFIG_NET_RETRY_COUNT 10 +/* * Board NAND Info. */
Signature · 1 line
1.7.0.4
PATCH 8/8 AM3517 Crane: Enable ethernet
In reply to Tom Rini--- include/configs/am3517_crane.h | 14 ++++++++++++-- 1 files changed, 12 insertions(+), 2 deletions(-)
include/configs/am3517_crane.h +12 -2
@@ -161,8 +161,18 @@#define CONFIG_SYS_I2C_BUS_SELECT 1#define CONFIG_DRIVER_OMAP34XX_I2C 1 -#undef CONFIG_CMD_NET -#undef CONFIG_CMD_NFS +/* +* Ethernet +*/ +#define CONFIG_DRIVER_TI_EMAC +#define CONFIG_DRIVER_TI_EMAC_USE_RMII +#define CONFIG_MII +#define CONFIG_BOOTP_DEFAULT +#define CONFIG_BOOTP_DNS +#define CONFIG_BOOTP_DNS2 +#define CONFIG_BOOTP_SEND_HOSTNAME +#define CONFIG_NET_RETRY_COUNT 10 +/* * Board NAND Info. */
Signature · 1 line
1.7.0.4