RE: [PATCH 1/2] OMAP: DSS2: Introduce dss_features files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux