On Sat, Mar 31, 2012 at 12:16:30AM +0900, Akio Idehara wrote:
> +static int get_tr_backlight_status(struct toshiba_acpi_dev *dev, u32 *status)
> +{
> + u32 hci_result;
> +
> + hci_read1(dev, HCI_TR_BACKLIGHT, status, &hci_result);
> + return hci_result == HCI_SUCCESS ? 0 : -EIO;
> +}
> +
> +static int set_tr_backlight_status(struct toshiba_acpi_dev *dev, int value)
> +{
> + u32 hci_result;
> +
> + hci_write1(dev, HCI_TR_BACKLIGHT, value, &hci_result);
> + return hci_result == HCI_SUCCESS ? 0 : -EIO;
> +}
I think the code will be easier to read if you change both of these to
use boolean arguments, since that's essentially how they're being used
anyway. I.e.
static int get_tr_backlight_status(struct toshiba_acpi_dev *dev, boolean *enabled);
static int set_tr_backlight_status(struct toshiba_acpi_dev *dev, boolean enable);
> @@ -497,15 +527,18 @@ static int lcd_proc_show(struct seq_file *m, void *v)
> {
> struct toshiba_acpi_dev *dev = m->private;
> int value;
> + int levels = HCI_LCD_BRIGHTNESS_LEVELS;
>
> if (!dev->backlight_dev)
> return -ENODEV;
>
> + if (dev->tr_backlight_supported)
> + levels++;
dev->backlight_dev->props.max_brightness + 1? That seems nicer than
having to duplicate the "tr backlight gives me an additional brightness
level" logic throughout the file.
> @@ -1104,8 +1148,15 @@ static int __devinit toshiba_acpi_add(struct acpi_device *acpi_dev)
>
> mutex_init(&dev->mutex);
>
> + /* Determine whether or not BIOS supports transflective backlight */
> + ret = get_tr_backlight_status(dev, &dummy) ? false : true;
> + dev->tr_backlight_supported = ret;
I'd personally prefer
ret = get_tr_backlight_status(dev, &dummy);
dev->tr_backlight_supported = !ret;
to be consistent with how this is done other places in the file.
Cheers,
Seth
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
