On Fri, 18 Jan 2008 15:44:35 +0100
Nicolas Ferre <[EMAIL PROTECTED]> wrote:
> From: David Brownell <[EMAIL PROTECTED]>
>
> On the sam9 EK boards, the LCD backlight is hooked up to a PWM output
> from the LCD controller. It's controlled by "contrast" registers though.
>
> This patch lets boards declare that they have that kind of backlight
> control. The driver can then export this control, letting screenblank
> and other operations actually take effect ... reducing the typically
> substantial power drain from the backlight.
>
> Signed-off-by: David Brownell <[EMAIL PROTECTED]>
> Signed-off-by: Nicolas Ferre <[EMAIL PROTECTED]>
Looks good to me. It won't affect any current AVR32 boards, although
that may change in the future.
A couple of minor comments below.
> +static struct backlight_ops atmel_lcdc_bl_ops = {
> + .update_status = atmel_bl_update_status,
> + .get_brightness = atmel_bl_get_brightness,
> +};
This can be const, no?
No it can't, since backlight_device_register() takes a non-const
pointer for some reason...
> +#else
> +
> +static inline void init_backlight(struct atmel_lcdfb_info *sinfo)
> +{
> + dev_warn(&sinfo->pdev->dev, "backlight control is not available\n");
> +}
> +
> +static inline void exit_contrast(struct atmel_lcdfb_info *sinfo)
> +{
> +}
> +
> +#endif
This looks a bit asymmetric...
> +static inline void init_contrast(struct atmel_lcdfb_info *sinfo)
> +{
> + /* have some default contrast/backlight settings */
> + lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_CTR, contrast_ctr);
> + lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_VAL, ATMEL_LCDC_CVAL_DEFAULT);
> +
> + if (sinfo->lcdcon_is_backlight)
> + init_backlight(sinfo);
> +}
> +
>
> static struct fb_fix_screeninfo atmel_lcdfb_fix __initdata = {
> .type = FB_TYPE_PACKED_PIXELS,
> @@ -370,10 +472,6 @@ static int atmel_lcdfb_set_par(struct fb
> /* Disable all interrupts */
> lcdc_writel(sinfo, ATMEL_LCDC_IDR, ~0UL);
>
> - /* Set contrast */
> - value = ATMEL_LCDC_PS_DIV8 | ATMEL_LCDC_POL_POSITIVE |
> ATMEL_LCDC_ENA_PWMENABLE;
> - lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_CTR, value);
> - lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_VAL, ATMEL_LCDC_CVAL_DEFAULT);
> /* ...wait for DMA engine to become idle... */
> while (lcdc_readl(sinfo, ATMEL_LCDC_DMACON) & ATMEL_LCDC_DMABUSY)
> msleep(10);
> @@ -577,6 +675,7 @@ static int __init atmel_lcdfb_probe(stru
> 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;
> + sinfo->lcdcon_is_backlight = pdata_sinfo->lcdcon_is_backlight;
> } else {
> dev_err(dev, "cannot get default configuration\n");
> goto free_info;
> @@ -670,6 +769,9 @@ static int __init atmel_lcdfb_probe(stru
> goto release_mem;
> }
>
> + /* Initialize PWM for contrast or backlight ("off") */
> + init_contrast(sinfo);
> +
> /* interrupt */
> ret = request_irq(sinfo->irq_base, atmel_lcdfb_interrupt, 0,
> pdev->name, info);
> if (ret) {
> @@ -755,6 +857,7 @@ static int __exit atmel_lcdfb_remove(str
> if (!sinfo)
> return 0;
>
> + exit_contrast(sinfo);
Missing exit_contrast() in probe() error path?
Haavard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
