Re: [PATCH 6/6] omap: move generic omap3 features to generic

[Nishanth Menon <nm@ti.com>]

On 05/27/2010 01:24 PM, S, Venkatraman wrote:
> Nishanth Menon<nm@ti.com>  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 ;)

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

Regards,
Nishanth Menon