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()) Then the current settings. -Olof -- 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
