2013-05-31 14:29:00

by Richard Genoud

[permalink] [raw]
Subject: [PATCH 1/2] trivial: atmel_lcdfb: add missing error message

When a too small framebuffer is given, the atmel_lcdfb_check_var
silently fails.
Adding an error message will save some head scratching.

Signed-off-by: Richard Genoud <[email protected]>
---
drivers/video/atmel_lcdfb.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 540909d..6e6491f 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -461,8 +461,11 @@ static int atmel_lcdfb_check_var(struct fb_var_screeninfo *var,
if (info->fix.smem_len) {
unsigned int smem_len = (var->xres_virtual * var->yres_virtual
* ((var->bits_per_pixel + 7) / 8));
- if (smem_len > info->fix.smem_len)
+ if (smem_len > info->fix.smem_len) {
+ dev_err(dev, "Frame buffer is too small (%u) for screen size (need at least %u)\n",
+ info->fix.smem_len, smem_len);
return -EINVAL;
+ }
}

/* Saturate vertical and horizontal timings at maximum values */
--
1.7.10.4


2013-05-31 14:29:09

by Richard Genoud

[permalink] [raw]
Subject: [PATCH 2/2] atmel_lcdfb: blank the backlight on remove

When removing atmel_lcdfb module, the backlight is unregistered but not
blanked. (only for CONFIG_BACKLIGHT_ATMEL_LCDC case).
This can result in the screen going full white depending on how the PWM
is wired.

Signed-off-by: Richard Genoud <[email protected]>
---
drivers/video/atmel_lcdfb.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 6e6491f..e00318f 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -223,8 +223,13 @@ static void init_backlight(struct atmel_lcdfb_info *sinfo)

static void exit_backlight(struct atmel_lcdfb_info *sinfo)
{
- if (sinfo->backlight)
+ if (sinfo->backlight) {
+ if (sinfo->backlight->ops) {
+ sinfo->backlight->props.power = FB_BLANK_POWERDOWN;
+ sinfo->backlight->ops->update_status(sinfo->backlight);
+ }
backlight_device_unregister(sinfo->backlight);
+ }
}

#else
--
1.7.10.4

Subject: Re: [PATCH 2/2] atmel_lcdfb: blank the backlight on remove

On 16:28 Fri 31 May , Richard Genoud wrote:
> When removing atmel_lcdfb module, the backlight is unregistered but not
> blanked. (only for CONFIG_BACKLIGHT_ATMEL_LCDC case).
> This can result in the screen going full white depending on how the PWM
> is wired.
>
> Signed-off-by: Richard Genoud <[email protected]>
> ---
> drivers/video/atmel_lcdfb.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
> index 6e6491f..e00318f 100644
> --- a/drivers/video/atmel_lcdfb.c
> +++ b/drivers/video/atmel_lcdfb.c
> @@ -223,8 +223,13 @@ static void init_backlight(struct atmel_lcdfb_info *sinfo)
>
> static void exit_backlight(struct atmel_lcdfb_info *sinfo)
> {
> - if (sinfo->backlight)
> + if (sinfo->backlight) {
if (!sinfo->backlight)
return;

other look ok

if you want ot hte next RC you have 3h to send a v2 before I work off for few
days

Best Regards,
J.

2013-05-31 15:41:34

by Richard Genoud

[permalink] [raw]
Subject: Re: [PATCH 2/2] atmel_lcdfb: blank the backlight on remove

2013/5/31 Jean-Christophe PLAGNIOL-VILLARD <[email protected]>:
> On 16:28 Fri 31 May , Richard Genoud wrote:
>> When removing atmel_lcdfb module, the backlight is unregistered but not
>> blanked. (only for CONFIG_BACKLIGHT_ATMEL_LCDC case).
>> This can result in the screen going full white depending on how the PWM
>> is wired.
>>
>> Signed-off-by: Richard Genoud <[email protected]>
>> ---
>> drivers/video/atmel_lcdfb.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
>> index 6e6491f..e00318f 100644
>> --- a/drivers/video/atmel_lcdfb.c
>> +++ b/drivers/video/atmel_lcdfb.c
>> @@ -223,8 +223,13 @@ static void init_backlight(struct atmel_lcdfb_info *sinfo)
>>
>> static void exit_backlight(struct atmel_lcdfb_info *sinfo)
>> {
>> - if (sinfo->backlight)
>> + if (sinfo->backlight) {
> if (!sinfo->backlight)
> return;
>
Well, I usually prefer to have a single exit point (when it doesn't
make the code go beyond the 80chars limit).

but if you prefer something like:

if (!sinfo->backlight)
return;

if (sinfo->backlight->ops) {
sinfo->backlight->props.power = FB_BLANK_POWERDOWN;
sinfo->backlight->ops->update_status(sinfo->backlight);
}
backlight_device_unregister(sinfo->backlight);

I'll send the modified patch.

> other look ok
>
> if you want ot hte next RC you have 3h to send a v2 before I work off for few
> days

ok, bonnes vacances !

>
> Best Regards,
> J.

2013-05-31 15:49:52

by Richard Genoud

[permalink] [raw]
Subject: [PATCHv2 2/2] atmel_lcdfb: blank the backlight on remove

When removing atmel_lcdfb module, the backlight is unregistered but not
blanked. (only for CONFIG_BACKLIGHT_ATMEL_LCDC case).
This can result in the screen going full white depending on how the PWM
is wired.

Signed-off-by: Richard Genoud <[email protected]>
---
drivers/video/atmel_lcdfb.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
index 6e6491f..effdb37 100644
--- a/drivers/video/atmel_lcdfb.c
+++ b/drivers/video/atmel_lcdfb.c
@@ -223,8 +223,14 @@ static void init_backlight(struct atmel_lcdfb_info *sinfo)

static void exit_backlight(struct atmel_lcdfb_info *sinfo)
{
- if (sinfo->backlight)
- backlight_device_unregister(sinfo->backlight);
+ if (!sinfo->backlight)
+ return;
+
+ if (sinfo->backlight->ops) {
+ sinfo->backlight->props.power = FB_BLANK_POWERDOWN;
+ sinfo->backlight->ops->update_status(sinfo->backlight);
+ }
+ backlight_device_unregister(sinfo->backlight);
}

#else
--
1.7.10.4