Re: [PATCH] Au1200fb driver.

Thanks Tony. I'll fix it up and resubmit.

Pete

On Tue, 2005-10-18 at 09:32 +0800, Antonino A. Daplas wrote:

Ralf Baechle wrote:

quoted

Au1200 fb driver. Updated db1200 defconfig to include driver by default.

Besides coding style issues, here are a few comments:

quoted

+struct window_settings
+{
+	unsigned char name[64];
+	uint32 mode_backcolor;
+	uint32 mode_colorkey;
+	uint32 mode_colorkeymsk;

Any problems using u32 and family instead of uint32?

quoted

+
+/* AU1200 framebuffer driver */
+
+int au1200fb_fb_open(struct fb_info *fbi, int user)
+{
+	return 0;
+}
+
+int au1200fb_fb_release(struct fb_info *fbi, int user)
+{
+	return 0;
+}
+

You can just remove the fb_release and fb_open methods if they're not
doing anything.

quoted

+
+/* fb_blank
+ * Blank the screen. Depending on the mode, the screen will be
+ * activated with the backlight color, or desactivated
+ */
+int au1200fb_fb_blank(int blank_mode, struct fb_info *fbi)
+{
+	/* Short-circuit screen blanking */
+	if (noblanking)
+		return 0;
+
+	switch (blank_mode) {
+
+	case VESA_NO_BLANKING:
+		/* printk("turn on panel\n"); */
+		au1200_setpanel(panel);
+		break;
+
+	case VESA_VSYNC_SUSPEND:
+	case VESA_HSYNC_SUSPEND:
+	case VESA_POWERDOWN:
+		/* printk("turn off panel\n"); */
+		au1200_setpanel(NULL);
+		break;

Better to use the FB_BLANK_* constants defined in include/linux/fb.h instead
of the VESA_* constants.  There's also one constant, FB_BLANK_NORMAL, which
usually means blank the display but keep monitor syncs on.  But if you just
have 2 blank states, might as well do an if (blank_mode)/else.

Tony

-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl