Re: [PATCH] Au1200fb driver.

[Pete Popov <ppopov@embeddedalley.com>]

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:
> > Au1200 fb driver. Updated db1200 defconfig to include driver by default.
> 
> Besides coding style issues, here are a few comments:
> 
> > +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?
> 
> > +
> > +/* 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.
> 
> > +
> > +/* 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