Skip to contentSkip to conversation

: acpi,backlight: MSI S270 - driver, second try

RFC PATCH In review Lennart Poettering 2 participants 3 messages Aug 10, 2006 – Aug 17, 2006

Subsystems (2)
ACPI Char and misc drivers
lore
Share & tools
lore

Lennart Poettering

author
#0 lore raw

RFC PATCH 1/1 : acpi,backlight: MSI S270 - driver, second try

From: Lennart Poettering <[email protected]>

This is my second try of the MSI S270 laptop driver. On request of Len Brown I changed the following things:

- Move driver from drivers/acpi/s270.c to drivers/misc/s270.c

- Remove /proc/acpi/s270 interface, replace it by a platform device /sys/devices/platform/s270pf/. This means: no procfs is touched anymore, all features are now accessible through /sys/.

This patch applies to 2.6.17 and requires the ACPI ec_transaction() patch I posted earlier:

http://marc.theaimsgroup.com/?l=linux-acpi&m=115517193511970&w=2

Please comment and/or apply!

Lennart

Signed-off-by Lennart Poettering <[email protected]>

---

MAINTAINERS +7 -0
@@ -1882,6 +1882,13 @@ M:	[email protected]
 L:	[email protected]
 S:	Maintained
 
+MSI S270 LAPTOP SUPPORT
+P:	Lennart Poettering
+M:	[email protected]
+L:	https://tango.0pointer.de/mailman/listinfo/s270-linux
+W:	http://0pointer.de/lennart/tchibo.html
+S:	Maintained
+
 MTRR AND SIMILAR SUPPORT [i386]
 P:	Richard Gooch
 M:	[email protected]
drivers/misc/Kconfig +14 -0
@@ -28,5 +28,19 @@ config IBM_ASM
 
 	  If unsure, say N.
 
+config MSI_S270
+        tristate "MSI S270 Laptop Extras"
+        depends on X86
+        depends on ACPI_EC
+        depends on BACKLIGHT_CLASS_DEVICE
+        ---help---
+          This is a driver for MSI S270 laptops. It adds
+          support for Bluetooth, WLAN and LCD brightness control.
+
+          More information about this driver is available at
+          <http://0pointer.de/lennart/tchibo.html>.
+
+          If you have an MSI S270 laptop, say Y or M here.
+
 endmenu
drivers/misc/Makefile +1 -0
@@ -5,3 +5,4 @@ obj- := misc.o	# Dummy rule to force bui
 
 obj-$(CONFIG_IBM_ASM)	+= ibmasm/
 obj-$(CONFIG_HDPU_FEATURES)	+= hdpuftrs/
+obj-$(CONFIG_MSI_S270)     += s270.o
drivers/misc/s270.c +372 -0

372 lines not on this page · raw message

#1 lore raw

Hi!

Quoted 10 lines

- Remove /proc/acpi/s270 interface, replace it by a platform device /sys/devices/platform/s270pf/. This means: no procfs is touched anymore, all features are now accessible through /sys/.

This patch applies to 2.6.17 and requires the ACPI ec_transaction() patch I posted earlier:

http://marc.theaimsgroup.com/?l=linux-acpi&m=115517193511970&w=2

Please comment and/or apply!

Looks ok to me...

drivers/misc/s270.c line 82

Quoted 9 lines
drivers/misc/s270.c +9 -0
+static int auto_brightness;
+module_param(auto_brightness, int, 0);
+MODULE_PARM_DESC(auto_brightness, "Enable automatic brightness control (0: disabled; 1: enabled; 2: don't touch)");
+
+/*** Hardware access ***/
+
+static const uint8_t lcd_table[MSI_LCD_LEVEL_MAX] = {
+        0x00, 0x1f, 0x3e, 0x5d, 0x7c, 0x9b, 0xba, 0xd9, 0xf8
+};

Can we get 0xf8 levels and simplify code while we are at it?

Review

Quoted 2 lines
(quoted diff) +2 -0
+        if ((result = ec_transaction(MSI_EC_COMMAND_LCD_LEVEL, &wdata, 1, &rdata, 1)) < 0)
+                return result;

Please split this into two lines.

result = ...;
if (result.....)

drivers/misc/s270.c line 260

Quoted 2 lines
drivers/misc/s270.c +2 -0
+static DEVICE_ATTR(bluetooth, 0444, show_bluetooth, NULL);
+static DEVICE_ATTR(wlan, 0444, show_wlan, NULL);

So bluetooth and wlan basically mirror physical switch state? Should we make these switches available through input subsystem one day?

Signature · 1 line
Thanks for all the (sleeping) penguins.
author
#2 lore raw
Quoted 8 lines · Pavel Machek
Quoted 6 lines

This patch applies to 2.6.17 and requires the ACPI ec_transaction() patch I posted earlier:

http://marc.theaimsgroup.com/?l=linux-acpi&m=115517193511970&w=2

Please comment and/or apply!

Looks ok to me...

Thanks!

Quoted 12 lines

drivers/misc/s270.c line 82

Quoted 10 lines
drivers/misc/s270.c +9 -0
+static int auto_brightness;
+module_param(auto_brightness, int, 0);
+MODULE_PARM_DESC(auto_brightness, "Enable automatic brightness control (0: disabled; 1: enabled; 2: don't touch)");
+
+/*** Hardware access ***/
+
+static const uint8_t lcd_table[MSI_LCD_LEVEL_MAX] = {
+        0x00, 0x1f, 0x3e, 0x5d, 0x7c, 0x9b, 0xba, 0xd9, 0xf8
+};

Can we get 0xf8 levels and simplify code while we are at it?

Unfortunately not. The hardware only supports 9 levels of brightness.

However, I guess I could multiply the brightness level with 31 instead of using the translation table above.

I will fix that.

Quoted 7 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+        if ((result = ec_transaction(MSI_EC_COMMAND_LCD_LEVEL, &wdata, 1, &rdata, 1)) < 0)
+                return result;

Please split this into two lines.

result = ...;
if (result.....)

Will do.

Quoted 5 lines

drivers/misc/s270.c line 260

Quoted 2 lines
drivers/misc/s270.c +2 -0
+static DEVICE_ATTR(bluetooth, 0444, show_bluetooth, NULL);
+static DEVICE_ATTR(wlan, 0444, show_wlan, NULL);

So bluetooth and wlan basically mirror physical switch state? Should we make these switches available through input subsystem one day?

Yes. Actually I've already been discussing with Ivo how to extend the rfkill API to tell userspace whether an rfkill device is actually for Bluetooth, for wlan, for uwb and so on. I will update my S270 code as soon as rfkill enters the kernel to support this new API properly. The driver will then register two rfkill devices, one for Bluetooth and one for WLAN.

Lennart

Signature · 2 lines
Lennart Poettering; lennart [at] poettering [dot] net
ICQ# 11060553; GPG 0x1A015CC4; http://0pointer.net/lennart/