> -----Original Message-----
> From: Tony Lindgren [mailto:[email protected]] 
> Sent: Thursday, August 06, 2009 4:34 PM
> To: Premi, Sanjeev
> Cc: [email protected]
> Subject: Re: [PATCH 3/6] OMAP3: Add runtime check for OMAP35x
> 
> Hi,
> 
> * Sanjeev Premi <[email protected]> [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 <[email protected]>
> > 
> >  [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 <[email protected]>
> > ---
> >  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

> 
> 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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to