RE: [PATCH 1/1] OMAP3: Common mechanism to identify cpu revision

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----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?

> > +#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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux