"Premi, Sanjeev" <[email protected]> writes:
>> -----Original Message-----
>> From: Tony Lindgren [mailto:[email protected]]
>> Sent: Thursday, August 06, 2009 5:20 PM
>> To: Premi, Sanjeev
>> Cc: [email protected]
>> Subject: Re: [PATCH 3/6] OMAP3: Add runtime check for OMAP35x
>>
>> * Premi, Sanjeev <[email protected]> [090806 14:34]:
>> >
>> > > -----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
>>
>> 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.
IMO, we should not be using cpu_is_* for detecting the differences
between 34xx and 35xx, but rather we could query the features like
you're doing in PATCH 4/6.
Adding conditionals like
if (omap3_has_iva2())
...
and
if (omap3_has_sgx())
...
rather than having a long list of cpu_is checks that have to be changed
each time a new SoC comes out.
Kevin
--
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
