> -----Original Message----- > From: linux-omap-owner@xxxxxxxxxxxxxxx > [mailto:linux-omap-owner@xxxxxxxxxxxxxxx] On Behalf Of Premi, Sanjeev > Sent: Monday, July 26, 2010 8:57 PM > To: Menon, Nishanth > Cc: Gadiyar, Anand; linux-omap@xxxxxxxxxxxxxxx > Subject: RE: [PATCH] omap: Add macros to evaluate cpu revision > [snip] [snip] > > >>>>>>> +#define ES_2_0 OMAP_REVBITS_10 > > >>>>>>> +#define ES_2_1 OMAP_REVBITS_20 > > >>>>>> makes sense to go to 2_2 > > >>>>>>> +#define ES_3_0 OMAP_REVBITS_30 > > >>>>>>> +#define ES_3_1 OMAP_REVBITS_40 > > >>>>>>> +#define ES_3_1_2 OMAP_REVBITS_50 > > >>>>>> 3_2? > > >>>>> This may not make sense to add now as there are no > > >>>>> 2.2 or 3.2 revisions of any OMAP3/4 silicon? > > >>>>> > > >>>> Agreed for 3 and 4, but considering this is > > >>>> arch/arm/plat-omap/include/plat/cpu.h, does it make sense in > > >>>> looking all > > >>>> OMAPs? > > >>> In this case, the best option would be to prefix > > OMAP34X_/ OMAP36X_ > > >>> OMAP44X_ etc and define the ES revisions for each context. > > >> doing that is gonna make the code real dirty looking. at the > > > > > > dirty?? How come? The intent is to increase readability. > > > > > huh? should we start the old debate again? > > Read this thread > > http://marc.info/?l=linux-omap&m=127903615629407&w=2 > > > > >> very least > > >> mebbe bracket it in with #ifdef with CONFIG_OMAP2PLUS? > > > > > > What purpose does this #ifdef. The macro should/could be used > > > quite generically. > > > > Now we are back in a full circle -> if you would like to > > introduce the > > patch for ALL omap silicon, you might want to consider 2420 > > and so on.. > > at the very least. > > > > introducing this for OMAP3 and 4 alone does not allow logic > > to scale up. > > [sp] The logic is only in the macros viz. cpu_rev_ge(), cpu_rev_le, > etc. The support for other omap silicons means having the > mapping of the revision bits to actual silicon version. > [snip] [snip] > > > > > > Here is a sample usage from one of the patch I am reworking > > > for submission here: > > > > > > @@ -488,7 +494,9 @@ void omap_sram_idle(void) > > > * of AUTO_CNT = 1 enabled. This takes care of > errata 1.142. > > > * Hence store/restore the SDRC_POWER register here. > > > */ > > > - if (omap_rev() >= OMAP3430_REV_ES3_0 && > > > + if ((cpu_is_omap3630() > > > + || cpu_is_omap3505() || cpu_is_omap3517() > > > + || cpu_rev_ge(34xx, OMAP34XX_ES_3_0)) && > > cpu_rev_ge(34xx, OMAP34XX_ES_3_0) -> this is the cause of my > > comment on > > dirty code - redundant OMAP34XX_ in the macro when I do say > > it is 34xx > > in the first parameter! > > > > [sp] Dirtiness is in eye of beholder :) > I did say earlier, that the patch is meant for increasing > readability. I could have overcome this by using lowercase > for revision macros. > > I did think of "exploiting" enums; but had been avoiding > the need for adding new data structures. It, however, can > be ugly! > [snip] [snip] Below is an attempt to introduce enums to take care of uppercase bit definitions vs lowercase function definitions. Of course it is not a formal patch and I have tried to limit the patch only for additions; not for deletions that can result due from the enums declares below. The revisions can now be done as: - cpu_rev_ge(omap34xx, es3_0) - cpu_rev_eq(am3505, es1_0) - etc. Also, I am using revision bit values instead of another macro representing them (e.g. OMAP_REVISION_BITS_10). Given the structure below, I felt use of actual bit values is better. Nishanth: There may be copy-paste errors in the actual revision definitions (let us not deviate into those for now). ~sanjeev diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-omap/include/plat/cpu.h index 2e2ae53..36a7047 --- a/arch/arm/plat-omap/include/plat/cpu.h +++ b/arch/arm/plat-omap/include/plat/cpu.h @@ -467,4 +469,103 @@ OMAP3_HAS_FEATURE(isp, ISP) OMAP3_HAS_FEATURE(192mhz_clk, 192MHZ_CLK) OMAP3_HAS_FEATURE(io_wakeup, IO_WAKEUP) +/* + * Enumerate the CPU revisions for easy comparison against the + * revision bits specific for each processor family. + */ +#define DECLARE_CPU_REV(cpu) enum revs_ ##cpu +#define CPU_REV(cpu,rev,bits) cpu## _ ##rev = bits + +DECLARE_CPU_REV(omap242x) { + CPU_REV(omap242x, es1_0, 0x00), + CPU_REV(omap242x, es2_0, 0x01), +} ; + +DECLARE_CPU_REV(omap243x) { + CPU_REV(omap243x, es1_0, 0x00), +} ; + +DECLARE_CPU_REV(omap34xx) { + CPU_REV(omap34xx, es1_0, 0x00), + CPU_REV(omap34xx, es2_0, 0x01), + CPU_REV(omap34xx, es2_1, 0x02), + CPU_REV(omap34xx, es3_0, 0x03), + CPU_REV(omap34xx, es3_1, 0x04), + CPU_REV(omap34xx, es3_1_2, 0x05), +} ; + +DECLARE_CPU_REV(omap36xx) { + CPU_REV(omap36xx, es1_0, 0x00), + CPU_REV(omap36xx, es1_1, 0x01), +} ; + +DECLARE_CPU_REV(omap3503) { + CPU_REV(omap3503, es1_0, 0x00), + CPU_REV(omap3503, es2_0, 0x01), + CPU_REV(omap3503, es2_1, 0x02), + CPU_REV(omap3503, es3_0, 0x03), + CPU_REV(omap3503, es3_1, 0x04), +} ; + +DECLARE_CPU_REV(omap3515) { + CPU_REV(omap3515, es1_0, 0x00), + CPU_REV(omap3515, es2_0, 0x01), + CPU_REV(omap3515, es2_1, 0x02), + CPU_REV(omap3515, es3_0, 0x03), + CPU_REV(omap3515, es3_1, 0x04), +} ; + +DECLARE_CPU_REV(omap3525) { + CPU_REV(omap3525, es1_0, 0x00), + CPU_REV(omap3525, es2_0, 0x01), + CPU_REV(omap3525, es2_1, 0x02), + CPU_REV(omap3525, es3_0, 0x03), + CPU_REV(omap3525, es3_1, 0x04), +} ; + +DECLARE_CPU_REV(omap3530) { + CPU_REV(omap3530, es1_0, 0x00), + CPU_REV(omap3530, es2_0, 0x01), + CPU_REV(omap3530, es2_1, 0x02), + CPU_REV(omap3530, es3_0, 0x03), + CPU_REV(omap3530, es3_1, 0x04), +} ; + +DECLARE_CPU_REV(am3505) { + CPU_REV(am3505, es1_0, 0x00), + CPU_REV(am3505, es1_1, 0x01), +} ; + +DECLARE_CPU_REV(am3517) { + CPU_REV(am3517, es1_0, 0x00), + CPU_REV(am3517, es1_1, 0x01), +} ; + +/* + * Macros to evaluate CPU revision + */ +#define cpu_rev_lt(cpu,rev) \ + ((cpu_is_omap ##cpu() \ + && (GET_OMAP_REVISION() < cpu## _ ##rev)) ? 1 : 0) + +#define cpu_rev_le(cpu,rev) \ + ((cpu_is_omap ##cpu() \ + && (GET_OMAP_REVISION() <= cpu## _ ##rev)) ? 1 : 0) + +#define cpu_rev_eq(cpu,rev) \ + ((cpu_is_omap ##cpu() \ + && (GET_OMAP_REVISION() == cpu## _ ##rev)) ? 1 : 0) + +#define cpu_rev_ne(cpu,rev) \ + ((cpu_is_omap ##cpu() \ + && (GET_OMAP_REVISION() != cpu## _ ##rev)) ? 1 : 0) + +#define cpu_rev_ge(cpu,rev) \ + ((cpu_is_omap ##cpu() \ + && (GET_OMAP_REVISION() >= cpu## _ ##rev)) ? 1 : 0) + +#define cpu_rev_gt(cpu,rev) \ + ((cpu_is_omap ##cpu() \ + && (GET_OMAP_REVISION() > cpu## _ ##rev)) ? 1 : 0) + #endif -- 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
