RE: [PATCH] omap: Add macros to evaluate cpu revision
-Original Message- From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of Premi, Sanjeev Sent: Thursday, August 12, 2010 8:21 PM To: Menon, Nishanth Cc: Gadiyar, Anand; linux-omap@vger.kernel.org Subject: RE: [PATCH] omap: Add macros to evaluate cpu revision -Original Message- From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of Premi, Sanjeev Sent: Monday, July 26, 2010 8:57 PM To: Menon, Nishanth Cc: Gadiyar, Anand; linux-omap@vger.kernel.org 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-omapm=127903615629407w=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 Did not see any comment on the proposal. Will go ahead and submit as formal patch tomorrow. ~sanjeev [snip][snip] the diff for illustration removed [snip][snip] -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] omap: Add macros to evaluate cpu revision
-Original Message-
From: linux-omap-ow...@vger.kernel.org
[mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of Premi, Sanjeev
Sent: Monday, July 26, 2010 8:57 PM
To: Menon, Nishanth
Cc: Gadiyar, Anand; linux-omap@vger.kernel.org
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-omapm=127903615629407w=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
RE: [PATCH] omap: Add macros to evaluate cpu revision
-Original Message- From: Menon, Nishanth Sent: Thursday, July 22, 2010 5:16 PM To: Premi, Sanjeev Cc: Gadiyar, Anand; linux-omap@vger.kernel.org Subject: Re: [PATCH] omap: Add macros to evaluate cpu revision Premi, Sanjeev had written, on 07/22/2010 06:20 AM, the following: -Original Message- From: Menon, Nishanth Sent: Thursday, July 22, 2010 4:18 PM To: Premi, Sanjeev Cc: Gadiyar, Anand; linux-omap@vger.kernel.org Subject: Re: [PATCH] omap: Add macros to evaluate cpu revision Premi, Sanjeev had written, on 07/22/2010 04:49 AM, the following: -Original Message- From: Menon, Nishanth Sent: Thursday, July 22, 2010 3:08 PM To: Gadiyar, Anand Cc: Premi, Sanjeev; linux-omap@vger.kernel.org Subject: Re: [PATCH] omap: Add macros to evaluate cpu revision On 07/22/2010 01:53 AM, Gadiyar, Anand wrote: @@ -460,4 +461,35 @@ OMAP3_HAS_FEATURE(isp, ISP) OMAP3_HAS_FEATURE(192mhz_clk, 192MHZ_CLK) OMAP3_HAS_FEATURE(io_wakeup, IO_WAKEUP) +/* + * Map revision bits to silicon specific revisions + */ +#define ES_1_0 OMAP_REVBITS_00 probably need ES_1_1, 1_2 (considering 3630) This should be okay, since the 3630 is out with these revisions, but... +#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-omapm=127903615629407w=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. information about the cputype is already being passed as a parameter, so it is just a matter of figuring out which ES revs should be defined there.. [sp] I have been trying to avoid creating another set of functions or introduce new data structures for the purpose. This means living with the problem of lower/uppercase in the definition e.g. cpu_is_omap34xx() and #define OMAP334XX_(*) macros. An added complexity is checking for family of devices and specific device in a family. I am tried to limit the changes introduced and still keep the overall code readable - not necessarily reduction in code size. 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! omap_type() != OMAP2_DEVICE_TYPE_GP core_next_state == PWRDM_POWER_OFF) sdrc_pwr = sdrc_read_reg(SDRC_POWER); Don't try to look more into the actual content of this example, but try to use existing macros to re-implement this condition. omap_rev() is always OMAP3430_REV_ES3_0 for all OMAP35x devices; even for OMAP3530 at ES2.1 level (0x35302034 0x34304034) And the original condition doesn't hold good. Try to visualize silicon revision viz. 2.1 for OMAP3505 requiring the same example condition to be updated. I see similar
RE: [PATCH] omap: Add macros to evaluate cpu revision
@@ -460,4 +461,35 @@ OMAP3_HAS_FEATURE(isp, ISP) OMAP3_HAS_FEATURE(192mhz_clk, 192MHZ_CLK) OMAP3_HAS_FEATURE(io_wakeup, IO_WAKEUP) +/* + * Map revision bits to silicon specific revisions + */ +#define ES_1_0 OMAP_REVBITS_00 probably need ES_1_1, 1_2 (considering 3630) This should be okay, since the 3630 is out with these revisions, but... +#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? - Anand-- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap: Add macros to evaluate cpu revision
On 07/22/2010 01:53 AM, Gadiyar, Anand wrote: @@ -460,4 +461,35 @@ OMAP3_HAS_FEATURE(isp, ISP) OMAP3_HAS_FEATURE(192mhz_clk, 192MHZ_CLK) OMAP3_HAS_FEATURE(io_wakeup, IO_WAKEUP) +/* + * Map revision bits to silicon specific revisions + */ +#define ES_1_0 OMAP_REVBITS_00 probably need ES_1_1, 1_2 (considering 3630) This should be okay, since the 3630 is out with these revisions, but... +#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? Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] omap: Add macros to evaluate cpu revision
-Original Message- From: Menon, Nishanth Sent: Thursday, July 22, 2010 3:08 PM To: Gadiyar, Anand Cc: Premi, Sanjeev; linux-omap@vger.kernel.org Subject: Re: [PATCH] omap: Add macros to evaluate cpu revision On 07/22/2010 01:53 AM, Gadiyar, Anand wrote: @@ -460,4 +461,35 @@ OMAP3_HAS_FEATURE(isp, ISP) OMAP3_HAS_FEATURE(192mhz_clk, 192MHZ_CLK) OMAP3_HAS_FEATURE(io_wakeup, IO_WAKEUP) +/* + * Map revision bits to silicon specific revisions + */ +#define ES_1_0 OMAP_REVBITS_00 probably need ES_1_1, 1_2 (considering 3630) This should be okay, since the 3630 is out with these revisions, but... +#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. ~sanjeev Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] omap: Add macros to evaluate cpu revision
Sanjeev, -Original Message- From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap- ow...@vger.kernel.org] On Behalf Of Premi, Sanjeev Sent: Thursday, July 22, 2010 3:20 PM To: Menon, Nishanth; Gadiyar, Anand Cc: linux-omap@vger.kernel.org Subject: RE: [PATCH] omap: Add macros to evaluate cpu revision -Original Message- From: Menon, Nishanth Sent: Thursday, July 22, 2010 3:08 PM To: Gadiyar, Anand Cc: Premi, Sanjeev; linux-omap@vger.kernel.org Subject: Re: [PATCH] omap: Add macros to evaluate cpu revision On 07/22/2010 01:53 AM, Gadiyar, Anand wrote: @@ -460,4 +461,35 @@ OMAP3_HAS_FEATURE(isp, ISP) OMAP3_HAS_FEATURE(192mhz_clk, 192MHZ_CLK) OMAP3_HAS_FEATURE(io_wakeup, IO_WAKEUP) +/* + * Map revision bits to silicon specific revisions + */ +#define ES_1_0 OMAP_REVBITS_00 probably need ES_1_1, 1_2 (considering 3630) This should be okay, since the 3630 is out with these revisions, but... +#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. Can please put the usecase need here. Do you want to use this for ERRATA handling or OPP handling etc etc ? Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap: Add macros to evaluate cpu revision
Premi, Sanjeev had written, on 07/22/2010 04:49 AM, the following: -Original Message- From: Menon, Nishanth Sent: Thursday, July 22, 2010 3:08 PM To: Gadiyar, Anand Cc: Premi, Sanjeev; linux-omap@vger.kernel.org Subject: Re: [PATCH] omap: Add macros to evaluate cpu revision On 07/22/2010 01:53 AM, Gadiyar, Anand wrote: @@ -460,4 +461,35 @@ OMAP3_HAS_FEATURE(isp, ISP) OMAP3_HAS_FEATURE(192mhz_clk, 192MHZ_CLK) OMAP3_HAS_FEATURE(io_wakeup, IO_WAKEUP) +/* + * Map revision bits to silicon specific revisions + */ +#define ES_1_0 OMAP_REVBITS_00 probably need ES_1_1, 1_2 (considering 3630) This should be okay, since the 3630 is out with these revisions, but... +#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 very least mebbe bracket it in with #ifdef with CONFIG_OMAP2PLUS? -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] omap: Add macros to evaluate cpu revision
-Original Message- From: Menon, Nishanth Sent: Thursday, July 22, 2010 4:18 PM To: Premi, Sanjeev Cc: Gadiyar, Anand; linux-omap@vger.kernel.org Subject: Re: [PATCH] omap: Add macros to evaluate cpu revision Premi, Sanjeev had written, on 07/22/2010 04:49 AM, the following: -Original Message- From: Menon, Nishanth Sent: Thursday, July 22, 2010 3:08 PM To: Gadiyar, Anand Cc: Premi, Sanjeev; linux-omap@vger.kernel.org Subject: Re: [PATCH] omap: Add macros to evaluate cpu revision On 07/22/2010 01:53 AM, Gadiyar, Anand wrote: @@ -460,4 +461,35 @@ OMAP3_HAS_FEATURE(isp, ISP) OMAP3_HAS_FEATURE(192mhz_clk, 192MHZ_CLK) OMAP3_HAS_FEATURE(io_wakeup, IO_WAKEUP) +/* + * Map revision bits to silicon specific revisions + */ +#define ES_1_0 OMAP_REVBITS_00 probably need ES_1_1, 1_2 (considering 3630) This should be okay, since the 3630 is out with these revisions, but... +#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. very least mebbe bracket it in with #ifdef with CONFIG_OMAP2PLUS? What purpose does this #ifdef. The macro should/could be used quite generically. 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)) omap_type() != OMAP2_DEVICE_TYPE_GP core_next_state == PWRDM_POWER_OFF) sdrc_pwr = sdrc_read_reg(SDRC_POWER); Don't try to look more into the actual content of this example, but try to use existing macros to re-implement this condition. omap_rev() is always OMAP3430_REV_ES3_0 for all OMAP35x devices; even for OMAP3530 at ES2.1 level (0x35302034 0x34304034) And the original condition doesn't hold good. Try to visualize silicon revision viz. 2.1 for OMAP3505 requiring the same example condition to be updated. ~sanjeev -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap: Add macros to evaluate cpu revision
Premi, Sanjeev had written, on 07/22/2010 06:20 AM, the following: -Original Message- From: Menon, Nishanth Sent: Thursday, July 22, 2010 4:18 PM To: Premi, Sanjeev Cc: Gadiyar, Anand; linux-omap@vger.kernel.org Subject: Re: [PATCH] omap: Add macros to evaluate cpu revision Premi, Sanjeev had written, on 07/22/2010 04:49 AM, the following: -Original Message- From: Menon, Nishanth Sent: Thursday, July 22, 2010 3:08 PM To: Gadiyar, Anand Cc: Premi, Sanjeev; linux-omap@vger.kernel.org Subject: Re: [PATCH] omap: Add macros to evaluate cpu revision On 07/22/2010 01:53 AM, Gadiyar, Anand wrote: @@ -460,4 +461,35 @@ OMAP3_HAS_FEATURE(isp, ISP) OMAP3_HAS_FEATURE(192mhz_clk, 192MHZ_CLK) OMAP3_HAS_FEATURE(io_wakeup, IO_WAKEUP) +/* + * Map revision bits to silicon specific revisions + */ +#define ES_1_0 OMAP_REVBITS_00 probably need ES_1_1, 1_2 (considering 3630) This should be okay, since the 3630 is out with these revisions, but... +#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-omapm=127903615629407w=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. information about the cputype is already being passed as a parameter, so it is just a matter of figuring out which ES revs should be defined there.. 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! omap_type() != OMAP2_DEVICE_TYPE_GP core_next_state == PWRDM_POWER_OFF) sdrc_pwr = sdrc_read_reg(SDRC_POWER); Don't try to look more into the actual content of this example, but try to use existing macros to re-implement this condition. omap_rev() is always OMAP3430_REV_ES3_0 for all OMAP35x devices; even for OMAP3530 at ES2.1 level (0x35302034 0x34304034) And the original condition doesn't hold good. Try to visualize silicon revision viz. 2.1 for OMAP3505 requiring the same example condition to be updated. I see similar potential use in enabling quirks and features (the above code btw could be better handled with a single variable errata which is populated with a flag at pm_init time.. ~sanjeev -- Regards, Nishanth Menon -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] omap: Add macros to evaluate cpu revision
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! omap_type() != OMAP2_DEVICE_TYPE_GP core_next_state == PWRDM_POWER_OFF) sdrc_pwr = sdrc_read_reg(SDRC_POWER); Don't try to look more into the actual content of this example, but try to use existing macros to re-implement this condition. omap_rev() is always OMAP3430_REV_ES3_0 for all OMAP35x devices; even for OMAP3530 at ES2.1 level (0x35302034 0x34304034) And the original condition doesn't hold good. Try to visualize silicon revision viz. 2.1 for OMAP3505 requiring the same example condition to be updated. I see similar potential use in enabling quirks and features (the above code btw could be better handled with a single variable errata which is populated with a flag at pm_init time.. Will have detailed response later... But, 3530, 3505 and 3517 are not erratas. These are different silicons. AND this patch has no relation to power management. More and more silicons are going to be added to linux-omap tree as they are based. They all will follow their own revision lifecycle. I have no issues to go back and add the 2420 etc. but I don't have much information on their revision history. If you provide me info, I can update the patch OR you can submit a follow-up patch with related changes. The macros don't change due to addition of additional silicon revisions. ~sanjeev ~sanjeev -- Regards, Nishanth Menon -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] omap: Add macros to evaluate cpu revision
This patch adds macros to evaluate the cpu revision. These macros increase readability by reducing the repetitive code when multiple silicon and their revisions have to be tested. Example usage would be: if (cpu_rev_eq(omap34xx, ES_1_0)) Signed-off-by: Sanjeev Premi pr...@ti.com --- arch/arm/plat-omap/include/plat/cpu.h | 32 1 files changed, 32 insertions(+), 0 deletions(-) diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-omap/include/plat/cpu.h index aa2f4f0..14b1a44 100644 --- a/arch/arm/plat-omap/include/plat/cpu.h +++ b/arch/arm/plat-omap/include/plat/cpu.h @@ -70,6 +70,7 @@ unsigned int omap_rev(void); #define OMAP_REVBITS_200x20 #define OMAP_REVBITS_300x30 #define OMAP_REVBITS_400x40 +#define OMAP_REVBITS_500x50 /* * Get the CPU revision for OMAP devices @@ -460,4 +461,35 @@ OMAP3_HAS_FEATURE(isp, ISP) OMAP3_HAS_FEATURE(192mhz_clk, 192MHZ_CLK) OMAP3_HAS_FEATURE(io_wakeup, IO_WAKEUP) +/* + * Map revision bits to silicon specific revisions + */ +#define ES_1_0 OMAP_REVBITS_00 +#define ES_2_0 OMAP_REVBITS_10 +#define ES_2_1 OMAP_REVBITS_20 +#define ES_3_0 OMAP_REVBITS_30 +#define ES_3_1 OMAP_REVBITS_40 +#define ES_3_1_2 OMAP_REVBITS_50 + +/* + * Macros to evaluate CPU revision + */ +#define cpu_rev_lt(cpu,rev)\ + ((cpu_is_omap ##cpu() (GET_OMAP_REVISION() (rev))) ? 1 : 0) + +#define cpu_rev_le(cpu,rev)\ + ((cpu_is_omap ##cpu() (GET_OMAP_REVISION() = (rev))) ? 1 : 0) + +#define cpu_rev_eq(cpu,rev)\ + ((cpu_is_omap ##cpu() (GET_OMAP_REVISION() == (rev))) ? 1 : 0) + +#define cpu_rev_ne(cpu,rev)\ + ((cpu_is_omap ##cpu() (GET_OMAP_REVISION() != (rev))) ? 1 : 0) + +#define cpu_rev_ge(cpu,rev)\ + ((cpu_is_omap ##cpu() (GET_OMAP_REVISION() = (rev))) ? 1 : 0) + +#define cpu_rev_gt(cpu,rev)\ + ((cpu_is_omap ##cpu() (GET_OMAP_REVISION() (rev))) ? 1 : 0) + #endif -- 1.6.6.1 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap: Add macros to evaluate cpu revision
Premi, Sanjeev had written, on 07/21/2010 10:12 AM, the following: This patch adds macros to evaluate the cpu revision. These macros increase readability by reducing the repetitive code when multiple silicon and their revisions have to be tested. Example usage would be: if (cpu_rev_eq(omap34xx, ES_1_0)) Signed-off-by: Sanjeev Premi pr...@ti.com --- arch/arm/plat-omap/include/plat/cpu.h | 32 1 files changed, 32 insertions(+), 0 deletions(-) diff --git a/arch/arm/plat-omap/include/plat/cpu.h b/arch/arm/plat-omap/include/plat/cpu.h index aa2f4f0..14b1a44 100644 --- a/arch/arm/plat-omap/include/plat/cpu.h +++ b/arch/arm/plat-omap/include/plat/cpu.h @@ -70,6 +70,7 @@ unsigned int omap_rev(void); #define OMAP_REVBITS_200x20 #define OMAP_REVBITS_300x30 #define OMAP_REVBITS_400x40 +#define OMAP_REVBITS_500x50 /* * Get the CPU revision for OMAP devices @@ -460,4 +461,35 @@ OMAP3_HAS_FEATURE(isp, ISP) OMAP3_HAS_FEATURE(192mhz_clk, 192MHZ_CLK) OMAP3_HAS_FEATURE(io_wakeup, IO_WAKEUP) +/* + * Map revision bits to silicon specific revisions + */ +#define ES_1_0 OMAP_REVBITS_00 probably need ES_1_1, 1_2 (considering 3630) +#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? + +/* + * Macros to evaluate CPU revision + */ +#define cpu_rev_lt(cpu,rev)\ + ((cpu_is_omap ##cpu() (GET_OMAP_REVISION() (rev))) ? 1 : 0) + +#define cpu_rev_le(cpu,rev)\ + ((cpu_is_omap ##cpu() (GET_OMAP_REVISION() = (rev))) ? 1 : 0) + +#define cpu_rev_eq(cpu,rev)\ + ((cpu_is_omap ##cpu() (GET_OMAP_REVISION() == (rev))) ? 1 : 0) + +#define cpu_rev_ne(cpu,rev)\ + ((cpu_is_omap ##cpu() (GET_OMAP_REVISION() != (rev))) ? 1 : 0) + +#define cpu_rev_ge(cpu,rev)\ + ((cpu_is_omap ##cpu() (GET_OMAP_REVISION() = (rev))) ? 1 : 0) + +#define cpu_rev_gt(cpu,rev)\ + ((cpu_is_omap ##cpu() (GET_OMAP_REVISION() (rev))) ? 1 : 0) + #endif -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
