On Tue, May 11, 2010 at 7:13 PM, Nishanth Menon <[email protected]> wrote:
> Venkatraman S had written, on 05/11/2010 07:19 AM, the following:
>>
>> Nishanth Menon <[email protected]> wrote:
>>>
>>> -linux-arm
>>>
>>> On 05/10/2010 10:53 PM, S, Venkatraman wrote:
>>>>>
>>>>> -----Original Message-----
>>>>> From: [email protected]
>>>>> [mailto:[email protected]] On Behalf Of Menon, Nishanth
>>>>> Sent: Tuesday, May 11, 2010 5:02 AM
>>>>> To: S, Venkatraman
>>>>> Cc: [email protected];
>>>>> [email protected]; Tony Lindgren;
>>>>> Chikkature Rajashekar, Madhusudhan
>>>>> Subject: Re: [PATCH RESEND] update omap3 features bitmap and
>>>>> API to generic names
>>>>>
>>>>> On Mon, May 10, 2010 at 2:55 PM, Venkatraman S
>>>>> <[email protected]> wrote:
>>>>>>
>>>>>> OMAP3 features bitmap is used a method to check for SoC
>>>>>> specific features. This patch renames the global variables and the
>>>>>> accessor functions to use a generic name from omap3_* to
>>>>>> omap_*
>>>>>>
>>>>>> Signed-off-by: Venkatraman S<[email protected]>
>>>>>> CC: Nishant Menon<[email protected]>
>>>>>
>>>>> Just for the patchworks
>>>>> NAK - discussed before -
>>>>> http://marc.info/?l=linux-omap&m=127349696800651&w=2
>>>>
>>>> This patch doesn't have the descriptor load changes, and just the
>>>> rename.
>>>> Did you take a look at it first?
>>>
>>> Arrgh - my bad - there was no reference to previous discussions or
>>> anything
>>> in this patch, please do add references in diffstat section if you are
>>> refering to a previous discussed strategy to help the reviewers
>>> differentiate and understand you better.
>>>
>>> overall this still opens up a question for me -> can we blindly replace
>>> omap3-features with omap features? how do we think of omap1,2,3,4 are
>>> handled consistently with a 32 bit field?
>>>
>>> I agree on the need to have a omap independent field, but is
>>> omap3_features
>>> the one we need to modify OR should we be introducing a new field?
>>>
>>> my vote goes for introducing a new field.
>>>
>>
>> You are confusing the interface with implementation (or rather,
>> worrying about both of them simultaneously).
>>
>> The interface has to be consistent and SoC independent, to the extent
>> possible.
>> So the macro changes are relevant and readable / extensible.
>>
>> Regarding the variable name (implementation):-
>> Given the minimal set that we have (5 -6 fields today, so there is
>> room for 25 more 'features" still), I don't think
>> we should over-engineer it now to accomodate all permutations and use
>> cases. It's not even found use
>> beyond 1 or 2 files. YAGNI ?
>
> huh? I dont understand you. lets see what we we are doing here:
> a) we are causing an ABI breakage by moving omap3_feature to omap_feature
The omap3_feature variable is not supposed to be used by modules
*querying* for the feature. They don't even have to know it's existence.
That is what the macros/inline functions omap_has_***() are for.
All the uses of omap3_has_** have been replaced by omap_has_** in this patch.
This is an ABI change (due to the previous interface tying it to OMAP3)
and *not* ABI breakage (All previous uses have been replaced)
> (which by the way should also include changes to the macros as well..)
This is different. There could be different ways in which the
actual snooping
of whether a feature exists on not is implemented. For OMAP3, it is
looking up the control module.
(This, I have preserved). For other SoCs, it could be different, I
don't know how to
snoop them for. Your comments are welcome.
> b) we have a real need to have a cross OMAP feature distinction to
> differentiate between generic omap feature and omap3/2/1/4 features.
>
> This patch does not address the same in a consistent scalable way. NOTE: we
> are starting to introduce other OMAP4 features as well, SOC level feature
> distinction should ideally be handled by FEATURE framework - that is the
> reason it was introduced in the first place.
I never claimed this patch to be a mother of all 'features
framework'. It was just
a name improvement to the bunch of macros that existed. Again,
a) it preserves the separtion of interface. We can have a full fledged tree
of generic / OMAP[12345] specific features, but the interface need not
change.
>
> if your feel this is overengineering - well, I believe this is conceptual
> definition -> Specific OMAP[1234] *family* features Vs OMAP generic
> features.. two different things in my mind -> it does not matter if I have
> 32 bits in the original variable OR if I had 64bit.. i dont prefer to reuse
> a variable and mess up the conceptual distinction.
>
b) Are you simply saying that the existing features bitmap (all 6 of
them) are tied to OMAP3 ? For good ?
In that case, I really understand your comments and can change back
the variable name.
At the same time, it's not easy to classify what category a feature belongs to.
What could a single SoC specific feature today (SDMA descriptor
loading) could then become
a series (OMAP4, OMAP5 etc). Or it could occur at disjoint intervals
(OMAP3 and OMAP5).
Even if it somehow could be boxed to a series, I think the interface
would then be tied to
that series, which would break future expansion.
So a *flat* structure of "uniquely named" features storage
is what we want, which eventually could flow over to an array of bitfields.
In other words, if the interface is defined as omapX_has_feature(), it is not
maintainable, for any X. It should always be omap_has_feature(), even
if it is found in a single SoC.
This is what I understand your intention in change of $SUBJECT.
If you have another proposal, please post it here and I can work with
you on that.
Regards,
Venkat.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
