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.
>>> 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.
[Apologies for the delayed reply, seem to have a delayed delivery issue]
-Venkat.
--
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
