diff mbox

[3/3] video: imxfb: Fix lcd_ops .set_power on/off inversion

Message ID 1403001999-12316-3-git-send-email-denis@eukrea.com (mailing list archive)
State New, archived
Headers show

Commit Message

Denis Carikli June 17, 2014, 10:46 a.m. UTC
.set_power takes the blank level as argument instead
of an on/off parameter:
fb_blank calls fb_notifier_call_chain with the blank
level encoded in its parameters, which is then sent
as-is to the set_power callback.

imxfb_lcd_set_power was expecting an on/off parameter
instead.
This resulted in the inversion of the on/off behaviour.

Signed-off-by: Denis Carikli <denis@eukrea.com>
---
 drivers/video/fbdev/imxfb.c |   20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

Comments

Alexander Shiyan June 19, 2014, 9:24 a.m. UTC | #1
Tue, 17 Jun 2014 12:46:39 +0200 ?? Denis Carikli <denis@eukrea.com>:
> .set_power takes the blank level as argument instead
> of an on/off parameter:
> fb_blank calls fb_notifier_call_chain with the blank
> level encoded in its parameters, which is then sent
> as-is to the set_power callback.
> 
> imxfb_lcd_set_power was expecting an on/off parameter
> instead.
> This resulted in the inversion of the on/off behaviour.
> 
> Signed-off-by: Denis Carikli <denis@eukrea.com>
> ---
>  drivers/video/fbdev/imxfb.c |   20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/video/fbdev/imxfb.c b/drivers/video/fbdev/imxfb.c
> index 121cbd7..2ffbd5e 100644
> --- a/drivers/video/fbdev/imxfb.c
> +++ b/drivers/video/fbdev/imxfb.c
> @@ -764,15 +764,25 @@ static int imxfb_lcd_get_power(struct lcd_device *lcddev)
>  	return 1;
>  }
>  
> -static int imxfb_lcd_set_power(struct lcd_device *lcddev, int power)
> +static int imxfb_lcd_set_power(struct lcd_device *lcddev, int blank)
>  {
>  	struct imxfb_info *fbi = dev_get_drvdata(&lcddev->dev);
>  
>  	if (!IS_ERR(fbi->lcd_pwr)) {
> -		if (power && !regulator_is_enabled(fbi->lcd_pwr))
> -			return regulator_enable(fbi->lcd_pwr);
> -		else if (regulator_is_enabled(fbi->lcd_pwr))
> -			return regulator_disable(fbi->lcd_pwr);
> +		switch (blank) {
> +		case FB_BLANK_VSYNC_SUSPEND:
> +		case FB_BLANK_HSYNC_SUSPEND:
> +		case FB_BLANK_NORMAL:
> +		case FB_BLANK_POWERDOWN:
> +			if (regulator_is_enabled(fbi->lcd_pwr))
> +				return regulator_disable(fbi->lcd_pwr);
> +			break;
> +
> +		case FB_BLANK_UNBLANK:
> +			if (!regulator_is_enabled(fbi->lcd_pwr))
> +				return regulator_enable(fbi->lcd_pwr);
> +			break;
> +		}
>  	}
>  
>  	return 0;

I think, the patch is correct, but we should update imxfb_lcd_get_power() in the same way.

Like as:
if (!IS_ERR_OR_NULL(fbi->lcd_pwr))
  if (!regulator_is_enabled(fbi->lcd_pwr))
    return FB_BLANK_NORMAL;

return FB_BLANK_UNBLANK;

---
diff mbox

Patch

diff --git a/drivers/video/fbdev/imxfb.c b/drivers/video/fbdev/imxfb.c
index 121cbd7..2ffbd5e 100644
--- a/drivers/video/fbdev/imxfb.c
+++ b/drivers/video/fbdev/imxfb.c
@@ -764,15 +764,25 @@  static int imxfb_lcd_get_power(struct lcd_device *lcddev)
 	return 1;
 }
 
-static int imxfb_lcd_set_power(struct lcd_device *lcddev, int power)
+static int imxfb_lcd_set_power(struct lcd_device *lcddev, int blank)
 {
 	struct imxfb_info *fbi = dev_get_drvdata(&lcddev->dev);
 
 	if (!IS_ERR(fbi->lcd_pwr)) {
-		if (power && !regulator_is_enabled(fbi->lcd_pwr))
-			return regulator_enable(fbi->lcd_pwr);
-		else if (regulator_is_enabled(fbi->lcd_pwr))
-			return regulator_disable(fbi->lcd_pwr);
+		switch (blank) {
+		case FB_BLANK_VSYNC_SUSPEND:
+		case FB_BLANK_HSYNC_SUSPEND:
+		case FB_BLANK_NORMAL:
+		case FB_BLANK_POWERDOWN:
+			if (regulator_is_enabled(fbi->lcd_pwr))
+				return regulator_disable(fbi->lcd_pwr);
+			break;
+
+		case FB_BLANK_UNBLANK:
+			if (!regulator_is_enabled(fbi->lcd_pwr))
+				return regulator_enable(fbi->lcd_pwr);
+			break;
+		}
 	}
 
 	return 0;