linux-kernel - Re: [PATCH v2] toshiba_acpi: Add support for transflective LCD

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120402193952.GD24020@thinkpad-t410>
Date:	Mon, 2 Apr 2012 14:39:52 -0500
From:	Seth Forshee <seth.forshee@...onical.com>
To:	Akio Idehara <zbe64533@...il.com>
Cc:	mjg59@...f.ucam.org, platform-driver-x86@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] toshiba_acpi: Add support for transflective LCD

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 linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/