> -----Original Message-----
> From: Kevin Hilman [mailto:[email protected]]
> Sent: Wednesday, September 30, 2009 7:32 PM
> To: Premi, Sanjeev
> Cc: [email protected]
> Subject: Re: [PATCH 1/1] OMAP3: Common mechanism to identify
> cpu revision
>
> Sanjeev Premi <[email protected]> writes:
>
> > There are multiple mechanisms to identify the cpu revisions.
> > Most common is use of omap_rev(). This, however, does a
> > absolute comparison of omap_revision - which includes
> > CPU id, CPU rev and CPU class. This comparison fails for
> > OMAP35x processors.
> >
> > This patch defines generic functions that use only the
> > CPU rev bits in omap_revision to identify the revision
> > information.
> >
> > Usage will change from (for example):
> > if (omap_rev() > OMAP3430_REV_ES2_0)
> > to:
> > if (cpu_is_omap34xx() && omap_rev_gt_2_0())
> >
> > Specific check for cpu_is_xxx() will not be needed for
> > files specific to silicon e.g. pm34xx.c, clock34xx.c, etc.
> >
> > Signed-off-by: Sanjeev Premi <[email protected]>
>
> Looks mostly good, some minor comments/questions below...
>
[snip]--[snip]
>
> I don't think the cpu_is_... is needed here because of the OMAP3
> specific function.
[sp] Yes. This can be removed.
> > /*
> > + * Silicon revisions
> > + */
> > +#define OMAP_ES_1_0 0x00
> > +#define OMAP_ES_2_0 0x10
> > +#define OMAP_ES_2_1 0x20
> > +#define OMAP_ES_3_0 0x30
> > +#define OMAP_ES_3_1 0x40
>
> Hmm, are these the same values on OMAP2? and OMAP4?
>
[sp] Based on the values defined for OMAP2420 and OMA2430,
these definitions are applicable. There aren't as many
Revisions though.
Not sure if it will hold good for OMAP4..
Do you think, we make these definitions silicon specific
now?
> > +#define OMAP_REV_MASK 0x0000ff00
> > +#define OMAP_REV_BITS ((omap_rev() &
> OMAP_REV_MASK) >> 8)
> > +
> > +#define OMAP_REV_IS(revid) \
> > +static inline u8 omap_rev_is_ ##revid (void)
> \
>
> Minor nit, but these should return bool.
[sp] I will update.
>
> > +{ \
> > + return (OMAP_REV_BITS == OMAP_ES_ ##revid) ? 1 : 0; \
> > +}
> > +
> > +#define OMAP_REV_LT(revid) \
> > +static inline u8 omap_rev_lt_ ##revid (void)
> \
[snip]--[snip]
>
> 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
