linux-kernel - Re: [PATCH] lenovo-sl-laptop : driver for review

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: <20090216151809.GA4724@srcf.ucam.org>
Date:	Mon, 16 Feb 2009 15:18:09 +0000
From:	Matthew Garrett <mjg59@...f.ucam.org>
To:	Alexandre Rostovtsev <tetromino@...il.com>
Cc:	linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] lenovo-sl-laptop : driver for review

On Mon, Feb 16, 2009 at 02:59:18AM -0500, Alexandre Rostovtsev wrote:

> +Fn F9 (eject):			KEY_EJECTCD (161)

No - this is a dock eject key, not a CD eject key.

> +Fn F12 (hibernate):		KEY_SUSPEND (205)

> +/sys/devices/platform/lenovo-sl-laptop/rfkill/rfkill*/
> + The bluetooth rfkill. state is the switch state (0 for bluetooth off,
> + 1 for bluetooth on).
> + Note that in case future versions of the driver add other rfkill switches,
> + userspace programs should check that the rfkill name is lensl_bluetooth_sw

Surely they should just be checking the rfkill type?

> +control_backlight:
> + If this parameter is set to 1, this driver will intercept brightness keys
> + (Fn+Home and Fn+End) and control the backlight brightness. This feature is
> + useful because the default ACPI video driver currently has poor support
> + for the Lenovo SL series. If you enable this feature, you should add
> + acpi_backlight=vendor

Please don't do that - the kernel shouldn't be consuming the brightness 
events itself, that should be left up to userspace. The ACPI video 
driver is broken in this respect.

> +static struct device_attribute dev_attr_bluetooth_enable =
> +	__ATTR(bluetooth_enable, S_IWUSR | S_IRUGO,
> +		bluetooth_enable_show, bluetooth_enable_store);

You support rfkill, so I'm not sure there's a benefit in exposing the 
same functionality via a custom sysfs node.

-- 
Matthew Garrett | mjg59@...f.ucam.org
--
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/