Hi Kevin, > -----Original Message----- > From: linux-omap-owner@xxxxxxxxxxxxxxx [mailto:linux-omap- > owner@xxxxxxxxxxxxxxx] On Behalf Of Kevin Hilman > Sent: Thursday, September 10, 2009 7:55 PM > To: Olof Johansson > Cc: Premi, Sanjeev; linux-omap@xxxxxxxxxxxxxxx > Subject: Re: [RFC] Common mechanism to identify Si revision > > Olof Johansson <olof@xxxxxxxxx> writes: > > > On Thu, Sep 03, 2009 at 04:14:28PM +0530, Premi, Sanjeev wrote: > >> Hi, > >> > >> Currently there are multiple mechanisms for identifying the si > revisions. > >> > >> Most places the comparison is against omap_rev() as a whole number. > Example: > >> > >> arch/arm/mach-omap2/board-3430sdp.c:695: if (omap_rev() > > OMAP3430_REV_ES1_0) > >> arch/arm/mach-omap2/board-3430sdp.c:728: if (omap_rev() > > OMAP3430_REV_ES1_0) > >> > >> Then, there are custom macros. Example (cpu.h): > >> > >> #define CHIP_GE_OMAP3430ES3_1 (CHIP_IS_OMAP3430ES3_1) > >> #define CHIP_GE_OMAP3430ES3 (CHIP_IS_OMAP3430ES3_0 | \ > >> CHIP_GE_OMAP3430ES3_1) > >> #define CHIP_GE_OMAP3430ES2 (CHIP_IS_OMAP3430ES2 | \ > >> CHIP_GE_OMAP3430ES3) > >> > >> The problem with comparing against a whole number is that comparison is > invalid > >> for another processor series. E.g. OMAP3430 and OMAP3517. > >> > >> Here, I am proposing a common mechanism to identify the si revision; > that focuses > >> on the revision bits alone. (See code below) > >> > >> The usage would then be (example): > >> > >> if (omap_rev() > OMAP3430_REV_ES1_0) > >> > >> To > >> > >> if (cpu_is_omap34xx() && OMAP_REV_GT(OMAP_ES_1_0) > > > > What's the purpose of most of these checks in the first place? I can > > see two immediate needs: > > > > 1) To check for various errata and do appropriate workarounds > > > > 2) To check if the current chip has a certain feature > > > > Both of these could just as well be abstracted away such that you use > > tests on the form: > > > > if (OMAP_HAS_ERRATA_FOO) ... > > > > or: > > if (OMAP_FEATURE_FOO) ... > > > > And then move the actual checking of a feature into the header file > > where the errata/feature setups are defined. > > > > > > There's two major benefits to this: > > > > 1) Readability. No need to sit and look up in a manual why there's a > > check for version X here. > > (and/or no need to add a specific comment about it). > > > > 2) Keeping changes centralized. If there's a new revision or chip, > > there's just one header file to update, not 20 different source files. > > > > For example, a bunch of the checks in pm34xx.c would be nicer to have > as: > > > > if (OMAP_HAS_USBHOST()) > > > > I tend to agree with Olaf here and am in favor of the new 'features' > patch that Sanjeev has already proposed. I agree as well, most of the differences between several ES and OMAP variant are related to changes in IPs and not really to the chip version. > That doesn't mean that I'm opposed to this change in principle, but > would rather see most of the omap_rev() and cpu_is_* checks disappear > in favor of more generic omap_has_feature() checks. In that case taking into account the feature version might be useful in order to get rid of most of the omap_rev(). Most of the time new ES are due to new feature or bug fix in an IP that will increase the IP version. Regards, Benoit -- 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
