On 05/31/2010 07:06 PM, Venkatraman S wrote:
Nishanth Menon<nm@xxxxxx> wrote:
On 05/27/2010 01:24 PM, S, Venkatraman wrote:
Nishanth Menon<nm@xxxxxx> wrote:
[..]
diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
index 809e13a..01555b6 100644
--- a/arch/arm/mach-omap2/id.c
+++ b/arch/arm/mach-omap2/id.c
@@ -161,7 +161,7 @@ static void __init omap24xx_check_revision(void)
#define OMAP3_CHECK_FEATURE(status,feat) \
if (((status& OMAP3_ ##feat## _MASK) \
>> OMAP3_ ##feat## _SHIFT) != FEAT_ ##feat## _NONE) { \
- omap3_features |= OMAP3_HAS_ ##feat; \
+ omap_features |= OMAP_HAS_ ##feat; \
}
"CHECK" sounds like a querying API, whereas the macro populates data.
Maybe UPDATE or SET ?
Depends on where you are looking at it from: overall it is checking the
status bits from OMAP and deciding what features it has - this is
specifically important for 35xx series of processors. it is indeed a check
in that sense. if you look at it from features variable, yeah it is updating
it, but the idea of usage of the Macro is: check in status if feature X is
available.. which is what it does ;). btw, the intent of the current patch
was not meant to rename CHECK_FEATURE as it was very OMAP3 specific ;)
It's ok. I don't want to worry too much about the name.
Thanks.
static void __init omap3_check_features(void)
@@ -310,20 +310,20 @@ static void __init omap3_cpuinfo(void)
/*
[...]
+OMAP_HAS_FEATURE(, l2cache, L2CACHE)
+OMAP_HAS_FEATURE(, sgx, SGX)
+OMAP_HAS_FEATURE(, iva, IVA)
+OMAP_HAS_FEATURE(, neon, NEON)
+OMAP_HAS_FEATURE(, isp, ISP)
+
+/*
* Runtime detection of OMAP3 features
*/
extern u32 omap3_features;
-#define OMAP3_HAS_L2CACHE BIT(0)
-#define OMAP3_HAS_IVA BIT(1)
-#define OMAP3_HAS_SGX BIT(2)
-#define OMAP3_HAS_NEON BIT(3)
-#define OMAP3_HAS_ISP BIT(4)
#define OMAP3_HAS_192MHZ_CLK BIT(5)
-OMAP_HAS_FEATURE(3, l2cache, L2CACHE)
-OMAP_HAS_FEATURE(3, sgx, SGX)
-OMAP_HAS_FEATURE(3, iva, IVA)
-OMAP_HAS_FEATURE(3, neon, NEON)
-OMAP_HAS_FEATURE(3, isp, ISP)
OMAP_HAS_FEATURE(3, 192mhz_clk, 192MHZ_CLK)
#endif
--
What about feature detection for OMAP2 and OMAP4 (similar to
omap3_check_features) ?
At least a dummy implementation with warning messages would be good,
so that they are not used without initialization.
there is no need for warning messages, they will return as feature not
present. cpu.h is a common header and variable omap_features is in common.c,
the check_feature and id.c has not set that bit, the variable will remain 0,
hence omap_has_sgx() will return 0 unless someone enables that for lets say
OMAP4 -> feel free to do it, as I mentioned in 0/6, this series was meant
solely to reorganize and provide an infrastructure for further development.
I don't agree. The patch / series is about making them 'generic', where generic
is, to the minimum, "making the features exposed for major silicon versions".
If they are not usable beyond OMAP3, it's not generic.
Which part is not generic? I am unable to comprehend your contention
here. Are you contending that ISP, l2cache, neon, isp are *not* generic
OMAP features? or are you contending that since I did not add the
OMAP2,4 logic to detect the same, this patch is not valid?
all silicon revisions CAN use the function omap_has_sgx() now, even
though I have not added detection logic to actually populate the same.
if you do have a patch for detecting actual OMAP4/2 features feel free
to add to the patch list, I am more than happy to ack them, if they look
good - I personally dont have a omap4 platform at the very moment to
write and post a patch - however trivial it may be.
Regards,
Nishanth Menon
--
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
