On Mon, 10 Mar 2008 14:51:56 +0100
Nicolas Ferre <nicolas.ferre@atmel.com> wrote:

quoted text
> +static int atmel_lcdfb_suspend(struct platform_device *pdev, pm_message_t mesg)
> +{
> +	struct fb_info *info = platform_get_drvdata(pdev);
> +	struct atmel_lcdfb_info *sinfo = info->par;
> +
> +	sinfo->saved_lcdcon = lcdc_readl(sinfo, ATMEL_LCDC_CONTRAST_VAL);

You're saving CONTRAST_VAL into a field called saved_lcdcon even though
it has nothing to do with LCDCON1 or LCDCON2...

quoted text
> +	lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_CTR, 0);

...then you're altering CONTRAST_CTR...

quoted text
> +}
> +
> +static int atmel_lcdfb_resume(struct platform_device *pdev)
> +{

quoted text
> +	lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_CTR, sinfo->saved_lcdcon);

...and later restoring the saved value of CONTRAST_VAL into CONTRAST_CTR.

Confused.

quoted text
> @@ -39,6 +39,7 @@ struct atmel_lcdfb_info {
>  	u8			bl_power;
>  #endif
>  	bool			lcdcon_is_backlight;
> +	u8			saved_lcdcon;

All of the registers involved are 32 bits wide, although the
interesting bits are all in the low byte. Do we really want to save
three bytes in the struct that badly?

Haavard
--
unsubscribe notice
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/