On Tue, 16 Feb 2010 11:06:22 -0700
Grant Likely [off-list ref] wrote:

[...]

quoted

diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 72d68b3..29c7f31 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c

@@ -34,7 +34,7 @@

=C2=A0#include <linux/of_platform.h>

=C2=A0#include <sysdev/fsl_soc.h>
-#include "fsl-diu-fb.h"
+#include <linux/fsl-diu-fb.h>

=C2=A0/*
=C2=A0* These parameters give default parameters

@@ -178,6 +178,21 @@ static struct fb_videomode __devinitdata fsl_diu_m=

ode_db[] =3D {

quoted

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0.sync =C2=A0 =C2=

=A0 =C2=A0 =C2=A0 =C2=A0 =3D FB_SYNC_COMP_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,

quoted

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0.vmode =C2=A0 =

=C2=A0 =C2=A0 =C2=A0 =C2=A0=3D FB_VMODE_NONINTERLACED

quoted

=C2=A0 =C2=A0 =C2=A0 =C2=A0},
+ =C2=A0 =C2=A0 =C2=A0 {
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .name =C2=A0 =C2=A0 =

=C2=A0 =C2=A0 =C2=A0 =3D "800x480-60",

quoted

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .refresh =C2=A0 =C2=

=A0 =C2=A0 =C2=A0=3D 60,

quoted

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .xres =C2=A0 =C2=A0 =

=C2=A0 =C2=A0 =C2=A0 =3D 800,

quoted

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .yres =C2=A0 =C2=A0 =

=C2=A0 =C2=A0 =C2=A0 =3D 480,

quoted

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .pixclock =C2=A0 =C2=

=A0 =C2=A0 =3D 31250,

quoted

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .left_margin =C2=A0 =

=C2=A0=3D 86,

quoted

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .right_margin =C2=A0=

 =3D 42,

quoted

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .upper_margin =C2=A0=

 =3D 33,

quoted

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .lower_margin =C2=A0=

 =3D 10,

quoted

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .hsync_len =C2=A0 =

=C2=A0 =C2=A0=3D 128,

quoted

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .vsync_len =C2=A0 =

=C2=A0 =C2=A0=3D 2,

quoted

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .sync =C2=A0 =C2=A0 =

=C2=A0 =C2=A0 =C2=A0 =3D FB_SYNC_COMP_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,

quoted

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .vmode =C2=A0 =C2=A0=

 =C2=A0 =C2=A0 =C2=A0=3D FB_VMODE_NONINTERLACED

quoted

+ =C2=A0 =C2=A0 =C2=A0 },
=C2=A0};

=20
This hunk bothers me.  It looks like the type of data that belongs
either in some common shared .c file, or encoded into the device tree.
 It seems to be data about the display panel, instead of data about
the framebuffer driver.  I know that the driver already uses this
pattern, but before I merge this patch and further rely on that
pattern, I think it is worth discussing.

In the updated DIU patches I don't add panel timing data to the
framebuffer driver. It is encoded in the device tree now. If this
is acceptable, I'll send another patch adding the documentation
for added bindings.

Thanks,
Anatolij