Hi,
Tomi Valkeinen wrote:
> On Tue, 2010-09-07 at 13:31 +0200, ext Taneja, Archit wrote:
>> Hi,
>>
>> Tomi Valkeinen wrote:
>>> Hi,
>>>
>>> On Thu, 2010-08-26 at 14:43 +0200, ext Archit Taneja wrote:
>>>> Add dss_features.c and dss_features.h for the dss_features framework
>>>>
>>>> Signed-off-by: Archit Taneja <archit@xxxxxx>
>>>
>>> Would a more static approach be cleaner? I mean something like this (pseudo
>>> code):
>>>
>>> static struct omap_dss_features omap3_dss_features = {
>>> /* array of register definitions */
>>> .registers = {
>>> {
>>> .register = FIRHINC,
>>> .high = 12,
>>> .low = 0,
>>> },
>>> {
>>> .register = FIRVINC,
>>> .high = 28,
>>> .low = 16,
>>> },
>>> ...
>>> },
>>>
>>> /* array of feature ids */
>>> .features = {
>>> GLOBAL_ALPHA,
>>> GLOBAL_ALPHA_VIDEO1,
>>> ...
>>> },
>>> };
>>>
>>> And then the code would select the omapX_dss_features struct to use
>>> depending on the omap version.
>>
>> It looks way cleaner out here , not sure though what it would look
>> like once the whole file is filled up :)
>>
>> One issue with having separate omapX_dss_features is something which
>> might may come later on : If there is a feature/value which is same in
>> omap2 and omap3 but different in omap4, we will sill need
> to fill up all the structs completely.
>>
>> We can get around this by filling one single struct on runtime :
>>
>> all omap2 specific features filled up here { ...
>> ...
>> }
>>
>> all common between omap2 and omap3 filled up here { ... ...
>> }
>>
>> all omap3 specific filled up here
>> {
>> ...
>> ...
>> }
>>
>> And so on..
>>
>> This will save up space, but won't give a very clean look ..
>>
>> I think by the time all omap4(and 3630) features are added and remaining
>> omap2,3 checks are removed we will have atleast 20 or so register
>> fields and a dozen features. Initializing them statically for all
>> omaps(including the ES_REV's) will really stretch up the file. But it will
>> still look clean :)
>>
>> What's your call on this?
>
> I'd try with static structs. I agree that if there are lots
> of features/registers which are the same on multiple omaps,
> there's redundant code needed. But it just needs to be
> written once, and having lots of if(omap_xxxx()) lines will
> be a nightmare to maintain.
>
> Or if things get really long, we could use multiple files,
> one for each omap.
>
> Also, we can make the definitions a bit shorter by using
> macros where suitable. For example the registers could be
> defined with something like FEAT_REG(FIRHINC, 12, 0), which
> will will the fields properly.
Okay, I will try this out.
Archit
> Or there could be a define for feature IDs, which could be appended. Like:
>
> #define FEAT_ID_OMAP2 AAA,\
> BBB,\
> CCC
>
> #define FEAT_ID_OMAP3 FEAT_ID_OMAP2,\
> DDD,\
> EEE,\
>
> But... That's getting a bit nasty, I wouldn't do things like
> that if not absolutely needed =).
>
> Tomi--
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
