[PATCH v5 07/17] video: lcd: add LoCoMo LCD driver

[Russell King - ARM Linux]

On Sun, Jun 14, 2015 at 07:28:50PM +0300, Dmitry Eremin-Solenikov wrote:
> 2015-06-14 18:18 GMT+03:00 Russell King - ARM Linux <linux at arm.linux.org.uk>:
> > On Mon, Jun 08, 2015 at 11:56:38PM +0300, Dmitry Eremin-Solenikov wrote:
> >> +int locomo_lcd_set_power(struct lcd_device *ldev, int power)
> >> +{
> >> +     struct locomo_lcd *lcd = lcd_get_data(ldev);
> >> +
> >> +     dev_dbg(&ldev->dev, "LCD power %d (is %d)\n", power, lcd->power);
> >> +
> >> +     if (!power && lcd->power)
> >> +             locomo_lcd_on(lcd);
> >> +
> >> +     if (power && !lcd->power)
> >> +             locomo_lcd_off(lcd);
> >
> > Is this correct?  You want to turn the LCD _off_ if power is non-zero, but
> > turn it _on_ if power is _zero_ ?
> 
> Yes, this is a crazy part of fb/backlight/LCD, where power is a FB_BLANK_*
> value, and FB_BLANK_UNBLANK is equal to 0.

In which case, the code is probably wrong anyway.  This would make it much
clearer:

	bool power_on = power == FB_BLANK_UNBLANK;

	if (power_on != lcd->power_is_on) {
		if (power_on)
			locomo_lcd_on(lcd);
		else
			locomo_lcd_off(lcd);

		lcd->power_is_on = power_on;
	}

with lcd->power_is_on also being a bool.  The above is almost English...

	if the power state is different from last time, then
		if we want power on
			call locomo_lcd_on
		otherwise
			call locomo_lcd_off
		remember power state

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.