"Premi, Sanjeev" <premi@xxxxxx> writes: >> -----Original Message----- >> From: linux-omap-owner@xxxxxxxxxxxxxxx >> [mailto:linux-omap-owner@xxxxxxxxxxxxxxx] On Behalf Of Premi, Sanjeev >> Sent: Monday, October 05, 2009 7:18 PM >> To: Kevin Hilman >> Cc: linux-omap@xxxxxxxxxxxxxxx >> Subject: RE: [PATCH 1/1] OMAP3: Common mechanism to identify >> cpu revision >> >> > -----Original Message----- >> > From: Kevin Hilman [mailto:khilman@xxxxxxxxxxxxxxxxxxx] >> > Sent: Wednesday, September 30, 2009 7:32 PM >> > To: Premi, Sanjeev >> > Cc: linux-omap@xxxxxxxxxxxxxxx >> > Subject: Re: [PATCH 1/1] OMAP3: Common mechanism to identify >> > cpu revision >> > >> > Sanjeev Premi <premi@xxxxxx> 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 <premi@xxxxxx> >> > >> > 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? Not necessarily, for OMAP4, we can use OMAP4_ prefix. But you should add a comment that these are valid for OMAP2/3. >> > > +#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? Yes. The main part of this should go into l-o master. Kevin > 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 majordomo@xxxxxxxxxxxxxxx >> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
