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