| Message ID | 1305472900-4004-3-git-send-email-aneesh@ti.com |
|---|---|
| State | Superseded |
| Headers | show |
Dear Aneesh V, In message <1305472900-4004-3-git-send-email-aneesh@ti.com> you wrote: > Signed-off-by: Aneesh V <aneesh@ti.com> > --- > V2: > * Added a revision string in addition to the revision number > Helps in printing out the OMAP revision at bootup ... > +const char *omap4_rev_string(void) > +{ > + const char *omap4_rev = NULL; > + switch (omap4_revision()) { > + case OMAP4430_ES1_0: > + omap4_rev = "OMAP4430 ES1.0"; > + break; > + case OMAP4430_ES2_0: > + omap4_rev = "OMAP4430 ES2.0"; > + break; > + case OMAP4430_ES2_1: > + omap4_rev = "OMAP4430 ES2.1"; > + break; > + case OMAP4430_ES2_2: > + omap4_rev = "OMAP4430 ES2.2"; > + break; Such code does not really scale well. Can this not be improved? I think we just had similar discussions for i.MX5x - please check what the result of these was. > + default: > + omap4_rev = "OMAP4 - Unknown Rev"; > + break; Please also output what the unknown revision was - this saves at least one debug round if you ever run into this case. > +} > diff --git a/arch/arm/include/asm/arch-omap4/omap4.h b/arch/arm/include/asm/arch-omap4/omap4.h > index a30bb33..1f88732 100644 > --- a/arch/arm/include/asm/arch-omap4/omap4.h > +++ b/arch/arm/include/asm/arch-omap4/omap4.h > @@ -51,6 +51,11 @@ > #define CONTROL_PADCONF_CORE (OMAP44XX_L4_CORE_BASE + 0x100000) > #define CONTROL_PADCONF_WKUP (OMAP44XX_L4_CORE_BASE + 0x31E000) > > +/* CONTROL_ID_CODE */ > +#define CONTROL_ID_CODE (CTRL_BASE + 0x204) C struct? Best regards, Wolfgang Denk
Hi Wolfgang, On Monday 16 May 2011 12:39 AM, Wolfgang Denk wrote: > Dear Aneesh V, > > In message<1305472900-4004-3-git-send-email-aneesh@ti.com> you wrote: >> Signed-off-by: Aneesh V<aneesh@ti.com> >> --- >> V2: >> * Added a revision string in addition to the revision number >> Helps in printing out the OMAP revision at bootup > ... >> +const char *omap4_rev_string(void) >> +{ >> + const char *omap4_rev = NULL; >> + switch (omap4_revision()) { >> + case OMAP4430_ES1_0: >> + omap4_rev = "OMAP4430 ES1.0"; >> + break; >> + case OMAP4430_ES2_0: >> + omap4_rev = "OMAP4430 ES2.0"; >> + break; >> + case OMAP4430_ES2_1: >> + omap4_rev = "OMAP4430 ES2.1"; >> + break; >> + case OMAP4430_ES2_2: >> + omap4_rev = "OMAP4430 ES2.2"; >> + break; > > Such code does not really scale well. Can this not be improved? I > think we just had similar discussions for i.MX5x - please check what > the result of these was. Are you referring to this one? http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/98522 If so, it may not work for us: 1. Please note that the above function is just for getting the string not for the revision itself. To get the revision we have omap4_revision(). 2. In our case we do not have a 1:1 mapping between the revisions(monotonically increasing numbers) we need in the U-Boot and the value obtained from the ID_CODE register. So, a translation is inevitable. 3. We need increasing numbers for subsequent revisions so that we can have something like: if (omap4_revision() >= OMAP4430_ES2_0) do_something(); > >> + default: >> + omap4_rev = "OMAP4 - Unknown Rev"; >> + break; > > Please also output what the unknown revision was - this saves at least > one debug round if you ever run into this case. This function should be in sync with omap4_revision() function(unless there is a bug). So, the rev will be OMAP4430_SILICON_ID_INVALID in that case. I shall print out the ARM revision and OMAP revision registers when I get into OMAP4430_SILICON_ID_INVALID situation in omap4_revision() > >> +} >> diff --git a/arch/arm/include/asm/arch-omap4/omap4.h b/arch/arm/include/asm/arch-omap4/omap4.h >> index a30bb33..1f88732 100644 >> --- a/arch/arm/include/asm/arch-omap4/omap4.h >> +++ b/arch/arm/include/asm/arch-omap4/omap4.h >> @@ -51,6 +51,11 @@ >> #define CONTROL_PADCONF_CORE (OMAP44XX_L4_CORE_BASE + 0x100000) >> #define CONTROL_PADCONF_WKUP (OMAP44XX_L4_CORE_BASE + 0x31E000) >> >> +/* CONTROL_ID_CODE */ >> +#define CONTROL_ID_CODE (CTRL_BASE + 0x204) > > C struct? Ok. I shall convert defines to C structs globally for register addressing. > > > Best regards, > > Wolfgang Denk >
Dear Aneesh V, In message <4DD11511.1060208@ti.com> you wrote: > > >> + const char *omap4_rev = NULL; > >> + switch (omap4_revision()) { > >> + case OMAP4430_ES1_0: > >> + omap4_rev = "OMAP4430 ES1.0"; > >> + break; > >> + case OMAP4430_ES2_0: > >> + omap4_rev = "OMAP4430 ES2.0"; > >> + break; > >> + case OMAP4430_ES2_1: > >> + omap4_rev = "OMAP4430 ES2.1"; > >> + break; > >> + case OMAP4430_ES2_2: > >> + omap4_rev = "OMAP4430 ES2.2"; > >> + break; > > > > Such code does not really scale well. Can this not be improved? I > > think we just had similar discussions for i.MX5x - please check what > > the result of these was. > > Are you referring to this one? > http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/98522 > > If so, it may not work for us: > > 1. Please note that the above function is just for getting the string > not for the revision itself. To get the revision we have > omap4_revision(). Well, when you already have such a funxction, then why cannot it be made to return useful values that are well-suited for formatting? Instead of #define OMAP4430_ES1_0 1 #define OMAP4430_ES2_0 2 #define OMAP4430_ES2_1 3 #define OMAP4430_ES2_2 4 you could use #define OMAP4430_ES1_0 10 #define OMAP4430_ES2_0 20 #define OMAP4430_ES2_1 21 #define OMAP4430_ES2_2 22 And then use a plain sprintf(omap4_rev, "OMAP4430 ES%d.%d", rev/10, rev%10); or similar. > 2. In our case we do not have a 1:1 mapping between the > revisions(monotonically increasing numbers) we need in the U-Boot and > the value obtained from the ID_CODE register. So, a translation is > inevitable. This is not needed. See above. Any form of table driven approach would be suitable, too. > 3. We need increasing numbers for subsequent revisions so that we can > have something like: Should be no problem, see above. Just define your number scheme. Instead of decimal packing you could also adapt something like the major/minor numbers for devices, and use the existing macros to extract the parts. There are tons of existing solutions for this type of problem, just pick one that fits. > >> + default: > >> + omap4_rev = "OMAP4 - Unknown Rev"; > >> + break; > > > > Please also output what the unknown revision was - this saves at least > > one debug round if you ever run into this case. > > This function should be in sync with omap4_revision() function(unless > there is a bug). So, the rev will be OMAP4430_SILICON_ID_INVALID in > that case. In this case omap4_revision() should print the value it is reading. Whenever you are reading "unexpected? numbers the first thing you will do during debugging is to check _what_ exactly you are reawding - so you can help and safe one step by just printing thei information which you have ready in your hands. > I shall print out the ARM revision and OMAP revision registers when I > get into OMAP4430_SILICON_ID_INVALID situation in omap4_revision() Thanks. Best regards, Wolfgang Denk
Hi Wolfgang, On Monday 16 May 2011 09:05 PM, Wolfgang Denk wrote: > Dear Aneesh V, > ... >> >> 1. Please note that the above function is just for getting the string >> not for the revision itself. To get the revision we have >> omap4_revision(). > > Well, when you already have such a funxction, then why cannot it be > made to return useful values that are well-suited for formatting? > > Instead of > > #define OMAP4430_ES1_0 1 > #define OMAP4430_ES2_0 2 > #define OMAP4430_ES2_1 3 > #define OMAP4430_ES2_2 4 > > you could use > > #define OMAP4430_ES1_0 10 > #define OMAP4430_ES2_0 20 > #define OMAP4430_ES2_1 21 > #define OMAP4430_ES2_2 22 > > And then use a plain > > sprintf(omap4_rev, "OMAP4430 ES%d.%d", rev/10, rev%10); > > or similar. This is a good idea. The only minor hitch is that OMAP4460 will come into picture in near future, again having at least ES1_0. But I think that can be worked out. best regards, Aneesh
Dear Aneesh V, In message <4DD21843.4060000@ti.com> you wrote: > > > you could use > > > > #define OMAP4430_ES1_0 10 > > #define OMAP4430_ES2_0 20 > > #define OMAP4430_ES2_1 21 > > #define OMAP4430_ES2_2 22 > > > > And then use a plain > > > > sprintf(omap4_rev, "OMAP4430 ES%d.%d", rev/10, rev%10); > > > > or similar. > > This is a good idea. The only minor hitch is that OMAP4460 will come > into picture in near future, again having at least ES1_0. But I think > that can be worked out. Then go for something like #define OMAP4430_ES1_0 0x44300100 #define OMAP4430_ES2_0 0x44300200 #define OMAP4430_ES2_1 0x44300201 #define OMAP4430_ES2_2 0x44300202 sprintf(omap4_rev, "OMAP%x ES%x.%x", (rev >> 16) & 0xFFFF, (rev >> 8) & 0xFF, rev & 0xFF); or so. Best regards, Wolfgang Denk
diff --git a/arch/arm/cpu/armv7/omap4/board.c b/arch/arm/cpu/armv7/omap4/board.c index fcd29a7..8ab7306 100644 --- a/arch/arm/cpu/armv7/omap4/board.c +++ b/arch/arm/cpu/armv7/omap4/board.c @@ -28,6 +28,7 @@ * MA 02111-1307 USA */ #include <common.h> +#include <asm/armv7.h> #include <asm/arch/cpu.h> #include <asm/arch/sys_proto.h> #include <asm/sizes.h> @@ -127,3 +128,60 @@ int arch_cpu_init(void) set_muxconf_regs(); return 0; } + +static u32 cortex_a9_rev(void) +{ + + unsigned int rev; + + /* Read Main ID Register (MIDR) */ + asm ("mrc p15, 0, %0, c0, c0, 0" : "=r" (rev)); + + return rev; +} + +u32 omap4_revision(void) +{ + if (readl(CONTROL_ID_CODE) == OMAP4_CONTROL_ID_CODE_ES2_1) + return OMAP4430_ES2_1; + else if (readl(CONTROL_ID_CODE) == OMAP4_CONTROL_ID_CODE_ES2_2) + return OMAP4430_ES2_2; + /* + * For some of the ES2/ES1 boards ID_CODE is not reliable: + * Also, ES1 and ES2 have different ARM revisions + * So use ARM revision for identification + */ + unsigned int rev = cortex_a9_rev(); + + switch (rev) { + case MIDR_CORTEX_A9_R0P1: + return OMAP4430_ES1_0; + case MIDR_CORTEX_A9_R1P2: + return OMAP4430_ES2_0; + default: + return OMAP4430_SILICON_ID_INVALID; + } +} + +const char *omap4_rev_string(void) +{ + const char *omap4_rev = NULL; + switch (omap4_revision()) { + case OMAP4430_ES1_0: + omap4_rev = "OMAP4430 ES1.0"; + break; + case OMAP4430_ES2_0: + omap4_rev = "OMAP4430 ES2.0"; + break; + case OMAP4430_ES2_1: + omap4_rev = "OMAP4430 ES2.1"; + break; + case OMAP4430_ES2_2: + omap4_rev = "OMAP4430 ES2.2"; + break; + default: + omap4_rev = "OMAP4 - Unknown Rev"; + break; + } + return omap4_rev; +} diff --git a/arch/arm/include/asm/arch-omap4/omap4.h b/arch/arm/include/asm/arch-omap4/omap4.h index a30bb33..1f88732 100644 --- a/arch/arm/include/asm/arch-omap4/omap4.h +++ b/arch/arm/include/asm/arch-omap4/omap4.h @@ -51,6 +51,11 @@ #define CONTROL_PADCONF_CORE (OMAP44XX_L4_CORE_BASE + 0x100000) #define CONTROL_PADCONF_WKUP (OMAP44XX_L4_CORE_BASE + 0x31E000) +/* CONTROL_ID_CODE */ +#define CONTROL_ID_CODE (CTRL_BASE + 0x204) + +#define OMAP4_CONTROL_ID_CODE_ES2_1 0x3B95C02F +#define OMAP4_CONTROL_ID_CODE_ES2_2 0x4B95C02F /* UART */ #define UART1_BASE (OMAP44XX_L4_PER_BASE + 0x6a000) #define UART2_BASE (OMAP44XX_L4_PER_BASE + 0x6c000) @@ -121,11 +126,11 @@ struct s32ktimer { /* Temporary SRAM stack used while low level init is done */ #define LOW_LEVEL_SRAM_STACK NON_SECURE_SRAM_END -/* - * OMAP4 real hardware: - * TODO: Change this to the IDCODE in the hw regsiter - */ -#define CPU_OMAP4430_ES10 1 -#define CPU_OMAP4430_ES20 2 +/* Silicon revisions */ +#define OMAP4430_SILICON_ID_INVALID 0 +#define OMAP4430_ES1_0 1 +#define OMAP4430_ES2_0 2 +#define OMAP4430_ES2_1 3 +#define OMAP4430_ES2_2 4 #endif diff --git a/arch/arm/include/asm/armv7.h b/arch/arm/include/asm/armv7.h index 50cc167..772435b 100644 --- a/arch/arm/include/asm/armv7.h +++ b/arch/arm/include/asm/armv7.h @@ -26,6 +26,10 @@ #define ARMV7_H #include <linux/types.h> +/* Cortex-A9 revisions */ +#define MIDR_CORTEX_A9_R0P1 0x410FC091 +#define MIDR_CORTEX_A9_R1P2 0x411FC092 + /* CCSIDR */ #define CCSIDR_LINE_SIZE_OFFSET 0 #define CCSIDR_LINE_SIZE_MASK 0x7 @@ -65,4 +69,4 @@ void v7_outer_cache_inval_all(void); void v7_outer_cache_flush_range(u32 start, u32 end); void v7_outer_cache_inval_range(u32 start, u32 end); -#endif +#endif /* ARMV7_H */
Signed-off-by: Aneesh V <aneesh@ti.com> --- V2: * Added a revision string in addition to the revision number Helps in printing out the OMAP revision at bootup --- arch/arm/cpu/armv7/omap4/board.c | 58 +++++++++++++++++++++++++++++++ arch/arm/include/asm/arch-omap4/omap4.h | 17 ++++++--- arch/arm/include/asm/armv7.h | 6 +++- 3 files changed, 74 insertions(+), 7 deletions(-)