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

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

Reply via email to