> -----Original Message-----
> From: [email protected]
> [mailto:[email protected]] On Behalf Of Premi, Sanjeev
> Sent: Monday, October 05, 2009 7:18 PM
> To: Kevin Hilman
> Cc: [email protected]
> Subject: RE: [PATCH 1/1] OMAP3: Common mechanism to identify
> cpu revision
>
> > -----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.
Kevin, Tony,
The original patch was created against the pm branch.
While re-submitting, should I split in two - one against
master and delta for pm branch?
Best regards,
Sanjeev
>
> >
> > > +{
> \
> > > + 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
>
> --
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
