Messages in this thread | | | Date | Wed, 4 Apr 2012 13:37:58 -0500 | From | Seth Forshee <> | Subject | Re: [PATCH v3] toshiba_acpi: Add support for transflective LCD |
| |
On Thu, Apr 05, 2012 at 12:00:46AM +0900, Akio Idehara wrote: > @@ -485,10 +506,23 @@ static int get_lcd(struct backlight_device *bd) > struct toshiba_acpi_dev *dev = bl_get_data(bd); > u32 hci_result; > u32 value; > + bool enabled; > + int extra = 0; > + > + if (dev->tr_backlight_supported) { > + if (!get_tr_backlight_status(dev, &enabled)) { > + if (!enabled) > + extra = 1; > + else > + return 0; > + } else { > + return -EIO; > + } > + } > > hci_read1(dev, HCI_LCD_BRIGHTNESS, &value, &hci_result); > if (hci_result == HCI_SUCCESS) > - return (value >> HCI_LCD_BRIGHTNESS_SHIFT); > + return (value >> HCI_LCD_BRIGHTNESS_SHIFT) + extra;
I'm still not crazy about the name. And really I think if get_tr_backlight_status() is going to return an error code, when it fails the return value should be propogated (even if it's the same in this case). How about this?
bool enabled; int ret; int brightness = 0;
if (dev->tr_backlight_supported) { ret = get_tr_backlight_status(dev, &enabled); if (ret) return ret; if (enabled) return 0; brightness++; }
hci_read1(dev, HCI_LCD_BRIGHTNESS, &value, &hci_result); if (hci_result == HCI_SUCCESS) return brightness + (value >> HCI_LCD_BRIGHTNESS_SHIFT);
> static const struct backlight_ops toshiba_backlight_data = { > + .options = BL_CORE_SUSPENDRESUME,
What's the reason for adding this? I don't see that it's useful unless we're handling BL_CORE_SUSPENDED, which toshiba_acpi is not doing.
Cheers, Seth
| |