Antonino A. Daplas :

On Mon, 2007-05-07 at 16:11 +0200, Nicolas Ferre wrote:

quoted

From: Nicolas Ferre <redacted>

quoted

+
+static struct fb_fix_screeninfo atmel_lcdfb_fix __initdata = {
+	.type		= FB_TYPE_PACKED_PIXELS,
+	.visual		= FB_VISUAL_TRUECOLOR,
+	.xpanstep	= 0,
+	.ypanstep	= 0,
+	.ywrapstep	= 0,
+	.accel		= FB_ACCEL_NONE,
+};
+

This driver has fb_pan_display() which I assume works. However, you also
need to set ypanstep and/or xpanstep and/or ywrapstep to a nonzero
value, depending on the hardware/drive capability.

True, this function is here because one of our products can do pan display.
The technique is not used for the moment but will be in a future version
of this driver.

quoted

+static u32 pseudo_palette[16] = {
+	0x000000,
+	0xaa0000,
+	0x00aa00,
+	0xaa5500,
+	0x0000aa,
+	0xaa00aa,
+	0x00aaaa,
+	0xaaaaaa,
+	0x555555,
+	0xff5555,
+	0x55ff55,
+	0xffff55,
+	0x5555ff,
+	0xff55ff,
+	0x55ffff,
+	0xffffff
+};
+

Do you really need to pre-fill pseudo_palette[]? The contents are going
to be overwritten by the console anyway (The pseudo_palette is for
fbcon's use only).

Ok, I have tested it with fbcon and no pre-fill : colors seem ok.

You can also include pseudo_palette[] as part of struct
atmel_lcdfb_info, then in your probe routine:

info->pseudo_palette = par->pseudo_palette;

to reduce the size of the kernel image. 

Done.

quoted

+static inline u_int chan_to_field(u_int chan, const struct fb_bitfield *bf)

Might as well change u_int to u32 or unsigned int, for consistency.

Ok, changed to unsigned int.

quoted

+static int __init atmel_lcdfb_init_fbinfo(struct atmel_lcdfb_info *sinfo)
+{
+	struct fb_info *info = sinfo->info;
+	int ret = 0;
+
+	memset(info->screen_base, 0, info->fix.smem_len);

memset_io?

ok, modified.

quoted

+	if (dev->platform_data) {
+		pdata_sinfo = (struct atmel_lcdfb_info *)dev->platform_data;
+		sinfo->default_bpp = pdata_sinfo->default_bpp;
+		sinfo->default_dmacon = pdata_sinfo->default_dmacon;
+		sinfo->default_lcdcon2 = pdata_sinfo->default_lcdcon2;
+		sinfo->default_monspecs = pdata_sinfo->default_monspecs;
+		sinfo->atmel_lcdfb_power_control = pdata_sinfo->atmel_lcdfb_power_control;
+		sinfo->guard_time = pdata_sinfo->guard_time;
+	} else {
+		dev_err(dev, "cannot get default configuration\n");
+		goto out;

Wrong goto? Should be goto free_info?

Right. modified to "goto free_info".
 

quoted

+release_intmem:
+	if (map) {
+		release_mem_region(info->fix.smem_start, info->fix.smem_len);
+	}

Unnecessary curly braces

I like curly braces... Anyway, removed.

Thanks you for your comments.

I resend a modified driver now in the following email.

Regards,
-- 
Nicolas Ferre



-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/