| Message ID | 1457380425-20244-2-git-send-email-u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Uwe, Am Montag, den 07.03.2016, 20:53 +0100 schrieb Uwe Kleine-König: > .set_power gets passed an FB_BLANK_XXX value, not a bool. So 0 signals > on; and >1 means off. The same applies for return values of .get_power. I'd try to somehow work this information into the code to avoid future confusion. > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > drivers/video/fbdev/imxfb.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/video/fbdev/imxfb.c b/drivers/video/fbdev/imxfb.c > index cee88603efc9..c5fcedde2a60 100644 > --- a/drivers/video/fbdev/imxfb.c > +++ b/drivers/video/fbdev/imxfb.c > @@ -759,9 +759,9 @@ static int imxfb_lcd_get_power(struct lcd_device *lcddev) > struct imxfb_info *fbi = dev_get_drvdata(&lcddev->dev); > > if (!IS_ERR(fbi->lcd_pwr)) > - return regulator_is_enabled(fbi->lcd_pwr); > + return !regulator_is_enabled(fbi->lcd_pwr); > > - return 1; > + return 0; How about making it explicit: if (!IS_ERR(fbi->lcd_pwr) && !regulator_is_enabled(fbi->lcd_pwr)) return FB_BLANK_POWERDOWN; return FB_BLANK_UNBLANK; > } > > static int imxfb_lcd_set_power(struct lcd_device *lcddev, int power) > @@ -769,7 +769,7 @@ static int imxfb_lcd_set_power(struct lcd_device *lcddev, int power) > struct imxfb_info *fbi = dev_get_drvdata(&lcddev->dev); > > if (!IS_ERR(fbi->lcd_pwr)) { > - if (power) > + if (!power) Same here: if (power == FB_BLANK_UNBLANK) > return regulator_enable(fbi->lcd_pwr); > else > return regulator_disable(fbi->lcd_pwr); regards Philipp -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On Tue, Mar 08, 2016 at 08:55:30AM +0100, Philipp Zabel wrote: > Hi Uwe, > > Am Montag, den 07.03.2016, 20:53 +0100 schrieb Uwe Kleine-König: > > .set_power gets passed an FB_BLANK_XXX value, not a bool. So 0 signals > > on; and >1 means off. The same applies for return values of .get_power. > > I'd try to somehow work this information into the code to avoid future > confusion. I integrated your changes into my code, you're obviously right here. Jean-Christophe, Tomi: Do you agree in principle with these changes? If so I can resend. If you won't take the changes anyhow, I wouldn't. Best regards Uwe
diff --git a/drivers/video/fbdev/imxfb.c b/drivers/video/fbdev/imxfb.c index cee88603efc9..c5fcedde2a60 100644 --- a/drivers/video/fbdev/imxfb.c +++ b/drivers/video/fbdev/imxfb.c @@ -759,9 +759,9 @@ static int imxfb_lcd_get_power(struct lcd_device *lcddev) struct imxfb_info *fbi = dev_get_drvdata(&lcddev->dev); if (!IS_ERR(fbi->lcd_pwr)) - return regulator_is_enabled(fbi->lcd_pwr); + return !regulator_is_enabled(fbi->lcd_pwr); - return 1; + return 0; } static int imxfb_lcd_set_power(struct lcd_device *lcddev, int power) @@ -769,7 +769,7 @@ static int imxfb_lcd_set_power(struct lcd_device *lcddev, int power) struct imxfb_info *fbi = dev_get_drvdata(&lcddev->dev); if (!IS_ERR(fbi->lcd_pwr)) { - if (power) + if (!power) return regulator_enable(fbi->lcd_pwr); else return regulator_disable(fbi->lcd_pwr);
.set_power gets passed an FB_BLANK_XXX value, not a bool. So 0 signals on; and >1 means off. The same applies for return values of .get_power. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/video/fbdev/imxfb.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)