> -----Original Message----- > From: Tony Lindgren [mailto:tony@xxxxxxxxxxx] > Sent: Thursday, August 06, 2009 5:20 PM > To: Premi, Sanjeev > Cc: linux-omap@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 3/6] OMAP3: Add runtime check for OMAP35x > > * Premi, Sanjeev <premi@xxxxxx> [090806 14:34]: > > > > > -----Original Message----- > > > From: Tony Lindgren [mailto:tony@xxxxxxxxxxx] > > > Sent: Thursday, August 06, 2009 4:34 PM > > > To: Premi, Sanjeev > > > Cc: linux-omap@xxxxxxxxxxxxxxx > > > Subject: Re: [PATCH 3/6] OMAP3: Add runtime check for OMAP35x > > > > > > Hi, > > > > > > * Sanjeev Premi <premi@xxxxxx> [090806 13:36]: > > > > Added runtime check via omap2_set_globals_35xx(). > > > > > > > > Parts of this patch have been derived from an earlier > > > > earlier patch submitted by Tony Lindgren <tony@xxxxxxxxxxx> > > > > > > > > [1] http://marc.info/?l=linux-omap&m=123301852702797&w=2 > > > > [2] http://marc.info/?l=linux-omap&m=123334055822212&w=2 > > > > > > > > Signed-off-by: Sanjeev Premi <premi@xxxxxx> > > > > --- > > > > arch/arm/mach-omap2/id.c | 115 > > > ++++++++++++++++++++++++------ > > > > arch/arm/plat-omap/common.c | 18 +++++- > > > > arch/arm/plat-omap/include/mach/common.h | 1 + > > > > arch/arm/plat-omap/include/mach/cpu.h | 64 > ++++++++++++++++- > > > > 4 files changed, 173 insertions(+), 25 deletions(-) > > > > > > > > diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c > > > > index a98201c..06770aa 100644 > > > > --- a/arch/arm/mach-omap2/id.c > > > > +++ b/arch/arm/mach-omap2/id.c > > > > @@ -28,6 +28,14 @@ > > > > static struct omap_chip_id omap_chip; > > > > static unsigned int omap_revision; > > > > > > > > +/* The new OMAP35x devices have assymetric names - > > > OMAP3505 and OMAP3517. > > > > + * It is not possible to define a common macro to > identify them. > > > > + * > > > > + * A quick way is to separate them across > 'generations' as below. > > > > + */ > > > > +#define OMAP35XX_G1 0x1 /* Applies to 3503, > > > 3515, 3525 and 3530 */ > > > > +#define OMAP35XX_G2 0x2 /* Applies to 3505 and 3517 */ > > > > + > > > > > > > > unsigned int omap_rev(void) > > > > { > > > > @@ -155,12 +163,71 @@ void __init omap24xx_check_revision(void) > > > > pr_info("\n"); > > > > } > > > > > > > > +static void __init omap34xx_set_revision(u8 rev, char > *rev_name) > > > > +{ > > > > + switch (rev) { > > > > + case 0: > > > > + omap_revision = OMAP3430_REV_ES2_0; > > > > + strcat(rev_name, "ES2.0"); > > > > + break; > > > > + case 2: > > > > + omap_revision = OMAP3430_REV_ES2_1; > > > > + strcat(rev_name, "ES2.1"); > > > > + break; > > > > + case 3: > > > > + omap_revision = OMAP3430_REV_ES3_0; > > > > + strcat(rev_name, "ES3.0"); > > > > + break; > > > > + case 4: > > > > + omap_revision = OMAP3430_REV_ES3_1; > > > > + strcat(rev_name, "ES3.1"); > > > > + break; > > > > + default: > > > > + /* Use the latest known revision as default */ > > > > + omap_revision = OMAP3430_REV_ES3_1; > > > > + strcat(rev_name, "Unknown revision"); > > > > + } > > > > +} > > > > + > > > > +static void __init omap35xx_set_revision(u8 rev, u8 gen, > > > char *rev_name) > > > > +{ > > > > + omap_revision = OMAP35XX_CLASS ; > > > > + > > > > + if (gen == OMAP35XX_G1) { > > > > + switch (rev) { > > > > + case 0: /* Take care of some older boards */ > > > > + case 1: > > > > + omap_revision |= OMAP35XX_MASK_ES2_0; > > > > + strcat(rev_name, "ES2.0"); > > > > + break; > > > > + case 2: > > > > + omap_revision |= OMAP35XX_MASK_ES2_1; > > > > + strcat(rev_name, "ES2.1"); > > > > + break; > > > > + case 3: > > > > + omap_revision |= OMAP35XX_MASK_ES3_0; > > > > + strcat(rev_name, "ES3.0"); > > > > + break; > > > > + case 4: > > > > + omap_revision |= OMAP35XX_MASK_ES3_1; > > > > + strcat(rev_name, "ES3.1"); > > > > + break; > > > > + default: > > > > + /* Use the latest known > revision as default */ > > > > + omap_revision |= OMAP35XX_MASK_ES3_0; > > > > + strcat(rev_name, "Unknown revision"); > > > > + } > > > > + } else { > > > > + strcat(rev_name, "ES1.0"); > > > > + } > > > > +} > > > > + > > > > > > To me it looks like you're checking the exact same cores as > > > we already do > > > for 34xx. That is, (idcode >> 28) & 0xff for both 34xx and > > > 35xx. So basically > > > they have the same omap cores. > > > > No, the cores in OMAP3505 and OMAP3517 are very different. > > I have listed major differences in PATCH 2/6. > > > > These devices differ in following areas: > > - Power management capabilities > > (Only 1 power domain, 1 OPP, etc.) > > - EMIF4 instead of SDRC > > - Support for DDR2 > > - EMAC > > - USB > > - HECC > > Sure, but from compiler flags and io point of view they can still > be treated as 34xx. > > How about just add the individual type detection for 35xx processors, > and then have something like this: > > #define cpu_is_omap35xx() (cpu_is_omap34xx() && > (cpu_is_omap3510() || \ > cpu_is_omap3520() || > cpu_is_omap3530()) > > That should pretty much shrink this patch series down to > about 50 lines or > so of code. Okay, I will try this. Just not sure if some of the differences in OMAP3530 and OMAP3430 can be detected. Will submit a patch soon. > > > > > > > > > Considering this I don't see much sense adding > cpu_is_35xx() category > > > because cpu_is_34xx() already covers these processors. Just > > > like cpu_is_16xx() > > > covers both 1610 and 1710. > > > > > > Let's just rather add more feature tests for IVA2 etc as > needed, then > > > cpu_is_35something() becomse just cpu_is_34xx() && > > > cpu_has_iva2() or similar. > > > > I did feel the need for these tests as well, and have an > internal patch. > > It was in my queue for submission next. > > > > > > > > > > > > > > void __init omap34xx_check_revision(void) > > > > { > > > > u32 cpuid, idcode; > > > > u16 hawkeye; > > > > u8 rev; > > > > - char *rev_name = "ES1.0"; > > > > + char rev_name[16] = ""; > > > > > > > > /* > > > > * We cannot access revision registers on ES1.0. > > > > @@ -184,28 +251,12 @@ void __init omap34xx_check_revision(void) > > > > rev = (idcode >> 28) & 0xff; > > > > > > > > if (hawkeye == 0xb7ae) { > > > > - switch (rev) { > > > > - case 0: > > > > - omap_revision = OMAP3430_REV_ES2_0; > > > > - rev_name = "ES2.0"; > > > > - break; > > > > - case 2: > > > > - omap_revision = OMAP3430_REV_ES2_1; > > > > - rev_name = "ES2.1"; > > > > - break; > > > > - case 3: > > > > - omap_revision = OMAP3430_REV_ES3_0; > > > > - rev_name = "ES3.0"; > > > > - break; > > > > - case 4: > > > > - omap_revision = OMAP3430_REV_ES3_1; > > > > - rev_name = "ES3.1"; > > > > - break; > > > > - default: > > > > - /* Use the latest known > revision as default */ > > > > - omap_revision = OMAP3430_REV_ES3_1; > > > > - rev_name = "Unknown revision\n"; > > > > - } > > > > + if (cpu_is_omap35xx()) > > > > + omap35xx_set_revision(rev, OMAP35XX_G1, > > > rev_name); > > > > + else > > > > + omap34xx_set_revision(rev, rev_name); > > > > + } else if (hawkeye == 0xb868) { > > > > + omap35xx_set_revision(rev, OMAP35XX_G2, > rev_name); > > > > } > > > > > > Testing for hawkeye == 0xb868 test should just be added into > > > the current > > > omap34xx_check_revision(). > > > > > > Regards, > > > > > > Tony > > > > > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html