"Premi, Sanjeev" <[email protected]> writes:
>> -----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?
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 [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
