public inbox for linux-acpi@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/2] acpi,backlight: MSI S270 laptop support - driver
@ 2006-08-10  1:05 Lennart Poettering
  2006-08-15 12:42 ` Thomas Renninger
  0 siblings, 1 reply; 7+ messages in thread
From: Lennart Poettering @ 2006-08-10  1:05 UTC (permalink / raw)
  To: len.brown; +Cc: linux-kernel, linux-acpi

From: Lennart Poettering <mzxreary@0pointer.de>

The attached patch adds support for some special functions of MSI S270
laptops: LCD brightness control and Bluetooth/WLAN status.

This patch requires the ec_transaction() patch sent before.

The driver registers a few files in /proc/acpi/s270/. In addition it
registers itself in the backlight subsystem and is available to
userspace in /sys/class/backlight/s270/.

Based on 2.6.17

Please comment and/or apply!

Lennart

Signed-off-by: Lennart Poettering <mzxreary@0pointer.de>
---
--- linux-source-2.6.17/drivers/acpi/s270.c	1970-01-01 01:00:00.000000000 +0100
+++ linux-source-2.6.17.lennart/drivers/acpi/s270.c	2006-08-10 00:58:25.000000000 +0200
@@ -0,0 +1,375 @@
+/*-*-linux-c-*-*/
+
+/* $Id: s270.c 107 2006-08-09 22:58:25Z lennart $ */
+
+/***
+  Copyright (C) 2006 Lennart Poettering <mzxreary (at) 0pointer (dot) de>
+
+  This program is free software; you can redistribute it and/or modify
+  it under the terms of the GNU General Public License as published by
+  the Free Software Foundation; either version 2 of the License, or
+  (at your option) any later version.
+
+  This program is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+  General Public License for more details.
+
+  You should have received a copy of the GNU General Public License
+  along with this program; if not, write to the Free Software
+  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+  02110-1301, USA.
+***/
+
+/*
+ * s270.c - MSI S270 laptop support. This laptop is sold under various
+ * brands, including "Cytron/TCM/Medion/Tchibo MD96100".
+ *
+ * This driver exports a few files in /proc/acpi/s270/:
+ *
+ *   lcd_level - Screen brightness: contains a single integer in the
+ *   range 0..8. (rw)
+ *
+ *   auto_brightness - Enable automatic brightness control: contains
+ *   either 0 or 1. If set to 1 the hardware adjusts the screen
+ *   brightness automatically when the power cord is
+ *   plugged/unplugged. (rw)
+ *
+ *   wlan - WLAN subsystem enabled: contains either 0 or 1. (ro)
+ *
+ *   bluetooth - Bluetooth subsystem enabled: contains either 0 or 1
+ *   Please note that this file is constantly 0 if no Bluetooth
+ *   hardware is available. (ro)
+ *
+ * In addition to these /proc files the driver registers itself in the
+ * Linux backlight control subsystem and is available to userspace under
+ * /sys/class/backlight/s270/.
+ *
+ * This driver might work on other laptops produced by MSI. If you
+ * want to try it you can pass force=1 as argument to the module which
+ * will force it to load even when the DMI data doesn't identify the
+ * laptop as MSI S270. YMMV.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/acpi.h>
+#include <linux/dmi.h>
+#include <linux/backlight.h>
+#include <linux/proc_fs.h>
+#include <linux/autoconf.h>
+#include <asm/uaccess.h>
+
+#define MSI_LCD_LEVEL_MAX 9
+
+#define MSI_EC_COMMAND_WIRELESS 0x10
+#define MSI_EC_COMMAND_LCD_LEVEL 0x11
+
+#define PROCFS_S270_PREFIX "s270"
+#define PROCFS_S270_LCD_LEVEL "lcd_level"
+#define PROCFS_S270_AUTO_BRIGHTNESS "auto_brightness"
+#define PROCFS_S270_WLAN "wlan"
+#define PROCFS_S270_BLUETOOTH "bluetooth"
+
+static int force;
+module_param(force, int, 0);
+
+/*** Hardware access ***/
+
+static const uint8_t lcd_table[MSI_LCD_LEVEL_MAX] = {
+        0x00, 0x1f, 0x3e, 0x5d, 0x7c, 0x9b, 0xba, 0xd9, 0xf8
+};
+
+static int set_lcd_level(int level) {
+        u8 buf[2];
+        
+        if (level < 0 || level >= MSI_LCD_LEVEL_MAX)
+                return -EINVAL;
+        
+        buf[0] = 0x80;
+        buf[1] = lcd_table[level];
+        
+        return ec_transaction(MSI_EC_COMMAND_LCD_LEVEL, buf, sizeof(buf), NULL, 0);
+}
+
+static int get_lcd_level(void) {
+        u8 wdata = 0, rdata;
+        int i, result;
+        
+        if ((result = ec_transaction(MSI_EC_COMMAND_LCD_LEVEL, &wdata, 1, &rdata, 1)) < 0)
+                return result;
+        
+        for (i = 0; i < MSI_LCD_LEVEL_MAX; i++)
+                if (lcd_table[i] == rdata)
+                        return i;
+        
+        return -EIO;
+}
+
+static int get_auto_brightness(void) {
+        u8 wdata = 4, rdata;
+        int result;
+        
+        if ((result = ec_transaction(MSI_EC_COMMAND_LCD_LEVEL, &wdata, 1, &rdata, 1)) < 0)
+                return result;
+
+        return !!(rdata & 8);
+}
+
+static int set_auto_brightness(int enable) {
+        u8 wdata[2], rdata;
+        int result;
+        
+        wdata[0] = 4;
+            
+        if ((result = ec_transaction(MSI_EC_COMMAND_LCD_LEVEL, wdata, 1, &rdata, 1)) < 0)
+                return result;
+            
+        wdata[0] = 0x84;
+        wdata[1] = (rdata & 0xF7) | (enable ? 8 : 0);
+
+        return ec_transaction(MSI_EC_COMMAND_LCD_LEVEL, wdata, 2, NULL, 0);
+}
+
+static int get_wireless_state(int *wlan, int *bluetooth) {
+        u8 wdata = 0, rdata;
+        int result;
+
+        if ((result = ec_transaction(MSI_EC_COMMAND_WIRELESS, &wdata, 1, &rdata, 1)) < 0)
+                return -1;
+
+        if (wlan)
+                *wlan = !!(rdata & 8);
+
+        if (bluetooth)
+                *bluetooth = !!(rdata & 128);
+        
+        return 0;
+}
+
+/*** Backlight device stuff ***/
+
+static int bl_get_brightness(struct backlight_device *b) {
+        return get_lcd_level();
+}
+
+
+static int bl_update_status(struct backlight_device *b) {
+        return set_lcd_level(b->props->brightness);
+}
+
+static struct backlight_properties s270bl_props = {
+        .owner		= THIS_MODULE,
+	.get_brightness = bl_get_brightness,
+        .update_status  = bl_update_status,
+	.max_brightness = MSI_LCD_LEVEL_MAX,
+};
+
+static struct backlight_device *s270bl_device;
+
+/*** procfs stuff ***/
+
+static struct proc_dir_entry *s270_procfs_dir;
+
+struct s270_procfs_item {
+        const char *name;
+        mode_t mode;
+        int (*read_func)(void);
+        int (*write_func)(int value);
+};
+
+static int procfs_wlan_read_func(void) {
+        int b, ret;
+
+        if ((ret = get_wireless_state(&b, NULL)) < 0)
+                return ret;
+
+        return b;
+}
+
+static int procfs_bluetooth_read_func(void) {
+        int b, ret;
+
+        if ((ret = get_wireless_state(NULL, &b)) < 0)
+                return ret;
+
+        return b;
+}
+
+static int dispatch_read(char *page, char **start, off_t off, int count, int *eof, void *data) {
+        const struct s270_procfs_item *item = data;
+        int value;
+
+        if (!item || !item->read_func)
+                return -EINVAL;
+
+        if ((value = item->read_func()) < 0)
+                return value;
+
+        if (off > 0) {
+                *eof = 1;
+                return 0;
+        }
+        
+        return sprintf(page, "%i\n", value);
+}
+
+static int dispatch_write(struct file *file, const char __user *userbuf, unsigned long count, void *data) {
+        const struct s270_procfs_item *item = data;
+        char *kernbuf;
+        int value, ret;
+        
+        if (!item || !item->write_func)
+                return -EINVAL;
+
+        if (!(kernbuf = kmalloc(count + 1, GFP_KERNEL)))
+                return -ENOMEM;
+
+        if (copy_from_user(kernbuf, userbuf, count)) {
+                kfree(kernbuf);
+                return -EFAULT;
+        }
+
+        kernbuf[count] = 0;
+
+        if (sscanf(kernbuf, "%i", &value) != 1) {
+                kfree(kernbuf);
+                return -EINVAL;
+        }
+
+        kfree(kernbuf);
+        
+        if ((ret = item->write_func(value)) < 0)
+                return ret;
+
+        return count;
+}
+
+static const struct s270_procfs_item s270_procfs_items[] = {
+        {
+                PROCFS_S270_LCD_LEVEL,
+                S_IFREG|S_IRUGO|S_IWUSR,
+                get_lcd_level,
+                set_lcd_level
+        }, {
+                PROCFS_S270_AUTO_BRIGHTNESS,
+                S_IFREG|S_IRUGO|S_IWUSR,
+                get_auto_brightness,
+                set_auto_brightness
+        }, {
+                PROCFS_S270_WLAN,
+                S_IFREG|S_IRUGO,
+                procfs_wlan_read_func,
+                NULL
+        }, {
+                PROCFS_S270_BLUETOOTH,
+                S_IFREG|S_IRUGO,
+                procfs_bluetooth_read_func,
+                NULL
+        }, { }
+};
+        
+static int __init add_procfs_files(void) {
+        const struct s270_procfs_item *item;
+
+        for (item = s270_procfs_items; item->name; item++) {
+                struct proc_dir_entry *proc;
+                
+                if (!(proc = create_proc_read_entry(
+                        item->name,
+                        item->mode,
+                        s270_procfs_dir,
+                        dispatch_read,
+                        (void*) item)))
+                        return -ENOMEM;
+                
+                proc->owner = THIS_MODULE;
+                proc->write_proc = dispatch_write;
+        }
+
+        return 0;
+}
+
+static void __exit remove_procfs_files(void) {
+        const struct s270_procfs_item *item;
+
+        for (item = s270_procfs_items; item->name; item++)
+                remove_proc_entry(item->name, s270_procfs_dir);
+}
+
+/*** Initialization ***/
+
+static struct dmi_system_id __initdata s270_dmi_table[] = { {
+        .ident = "MSI S270",
+        .matches = {
+                DMI_MATCH(DMI_SYS_VENDOR, "NOTEBOOK"),
+                DMI_MATCH(DMI_PRODUCT_NAME, "SAM2000"),
+        }
+}, { } };
+
+static int __init init_s270(void) {
+        int ret;
+
+        if (acpi_disabled)
+                return -ENODEV;
+
+        if (!force && !dmi_check_system(s270_dmi_table))
+                return -ENODEV;
+
+        s270bl_device = backlight_device_register("s270", NULL, &s270bl_props);
+        
+        if (IS_ERR(s270bl_device))
+                return PTR_ERR(s270bl_device);
+
+        if (!(s270_procfs_dir = proc_mkdir(PROCFS_S270_PREFIX, acpi_root_dir))) {
+                ret = -ENOMEM;
+                goto fail;
+        }
+
+        s270_procfs_dir->owner = THIS_MODULE;
+
+        if ((ret = add_procfs_files()) < 0)
+                goto fail;
+
+        /* Disable automatic brightness control by default, because
+         * this module was probably loaded to do brightness control in
+         * software. */
+         set_auto_brightness(0); 
+        
+        return 0;
+
+fail:
+
+        if (s270_procfs_dir) {
+                remove_procfs_files();
+                remove_proc_entry(PROCFS_S270_PREFIX, acpi_root_dir);
+        }
+
+        if (s270bl_device)
+                backlight_device_unregister(s270bl_device);
+
+        return ret;
+}
+
+static void __exit cleanup_s270(void) {
+
+        if (s270_procfs_dir) {
+                remove_procfs_files();
+                remove_proc_entry(PROCFS_S270_PREFIX, acpi_root_dir);
+        }
+
+        if (s270bl_device)
+                backlight_device_unregister(s270bl_device);
+
+        /* Enable automatic brightness control again */
+        set_auto_brightness(1);       
+}
+
+module_init(init_s270);
+module_exit(cleanup_s270);
+
+MODULE_AUTHOR("Lennart Poettering");
+MODULE_DESCRIPTION("MSI S270 Laptop Support");
+MODULE_VERSION("0.1");
+MODULE_LICENSE("GPL");
--- linux-source-2.6.17/drivers/acpi/Makefile	2006-06-18 03:49:35.000000000 +0200
+++ linux-source-2.6.17.lennart/drivers/acpi/Makefile	2006-08-10 01:41:58.000000000 +0200
@@ -55,5 +55,6 @@ obj-$(CONFIG_ACPI_NUMA)		+= numa.o
 obj-$(CONFIG_ACPI_ASUS)		+= asus_acpi.o
 obj-$(CONFIG_ACPI_IBM)		+= ibm_acpi.o
 obj-$(CONFIG_ACPI_TOSHIBA)	+= toshiba_acpi.o
+obj-$(CONFIG_ACPI_MSI_S270)     += s270.o
 obj-y				+= scan.o motherboard.o
 obj-$(CONFIG_ACPI_HOTPLUG_MEMORY)	+= acpi_memhotplug.o
--- linux-source-2.6.17/drivers/acpi/Kconfig	2006-06-18 03:49:35.000000000 +0200
+++ linux-source-2.6.17.lennart/drivers/acpi/Kconfig	2006-08-10 01:40:43.000000000 +0200
@@ -243,6 +243,19 @@ config ACPI_TOSHIBA
 	  If you have a legacy free Toshiba laptop (such as the Libretto L1
 	  series), say Y.
 
+config ACPI_MSI_S270
+	tristate "MSI S270 Laptop Extras"
+	depends on X86
+    depends on BACKLIGHT_CLASS_DEVICE
+	---help---
+	  This is a Linux ACPI 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.
+
 config ACPI_CUSTOM_DSDT
 	bool "Include Custom DSDT"
 	depends on !STANDALONE
--- linux-source-2.6.17/MAINTAINERS	2006-06-18 03:49:35.000000000 +0200
+++ linux-source-2.6.17.lennart/MAINTAINERS	2006-08-10 01:16:50.000000000 +0200
@@ -1882,6 +1882,13 @@ M:	rubini@ipvvis.unipv.it
 L:	linux-kernel@vger.kernel.org
 S:	Maintained
 
+MSI S270 LAPTOP SUPPORT
+P:	Lennart Poettering
+M:	mzxreary@0pointer.de
+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:	rgooch@atnf.csiro.au

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH 2/2] acpi,backlight: MSI S270 laptop support - driver
@ 2006-08-10  9:36 Brown, Len
  2006-08-10 11:46 ` Lennart Poettering
  0 siblings, 1 reply; 7+ messages in thread
From: Brown, Len @ 2006-08-10  9:36 UTC (permalink / raw)
  To: Lennart Poettering; +Cc: linux-kernel, linux-acpi

Lennart,

Your s270 platform driver needs a different home in the source tree
outside of drivers/acpi/, and the patch that adds it must add an entry
to MAINTAINERS.

lcd brightness platform support should talk to the existing
backlight stuff under sysfs.

wlan and bluetooth indicators/controls need to appear under
generic places under sysfs -- not under platform specific
files under /proc/acpi.

Yes, the existing platform specific drivers such as asus, toshiba, and
ibm
are bad examples.

thanks,
-Len

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] acpi,backlight: MSI S270 laptop support - driver
  2006-08-10  9:36 Brown, Len
@ 2006-08-10 11:46 ` Lennart Poettering
  0 siblings, 0 replies; 7+ messages in thread
From: Lennart Poettering @ 2006-08-10 11:46 UTC (permalink / raw)
  To: Brown, Len; +Cc: linux-kernel, linux-acpi

On Thu, 10.08.06 05:36, Brown, Len (len.brown@intel.com) wrote:

> Lennart,

Hi!

> 
> Your s270 platform driver needs a different home in the source tree
> outside of drivers/acpi/, and the patch that adds it must add an entry
> to MAINTAINERS.

If you look closely you'll see that the patch already adds a new
entry to MAINTAINERS.

I put the the driver in drivers/acpi/ because it relies heavily on the
ACPI EC stuff. But OK, what is a better place for the driver? 

drivers/char/ 
  It doesn't even register any character device.

drivers/video/backlight/
  It doesn't just do backlight control.

drivers/misc/
  Seems to be the last resort for everything that doesn't fit it
  otherwise.

Unless anyone has a better idea I will move it to drivers/misc/, then.

> lcd brightness platform support should talk to the existing
> backlight stuff under sysfs.

It already does exactly that, you can find the backlight class driver
in /sys/class/backlight/s270/.

I cannot map the "automatic brightness control" feature to the
backlight class driver, that's why I duplicated the brightness stuff
in /proc/acpi/s270/.

> wlan and bluetooth indicators/controls need to appear under
> generic places under sysfs -- not under platform specific
> files under /proc/acpi.

What are those "generic" places? I cannot think of any besides a
"platform" device.

> Yes, the existing platform specific drivers such as asus, toshiba, and
> ibm
> are bad examples.

Ok, I will move the /proc/acpi/s270/ stuff to a sysfs platform device,
then. I will cook up another patch shortly.

Any ideas on that ec_transaction() patch I sent earlier?

Lennart

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH 2/2] acpi,backlight: MSI S270 laptop support - driver
@ 2006-08-10 16:15 Brown, Len
  2006-08-10 16:48 ` Lennart Poettering
  0 siblings, 1 reply; 7+ messages in thread
From: Brown, Len @ 2006-08-10 16:15 UTC (permalink / raw)
  To: Lennart Poettering; +Cc: linux-kernel, linux-acpi


>If you look closely you'll see that the patch already adds a new
>entry to MAINTAINERS.

Excellent.

>I put the the driver in drivers/acpi/ because it relies heavily on the
>ACPI EC stuff. But OK, what is a better place for the driver? 

It is possible that parts of it should live in different places,
but I agree that it is also desireable to keep the compnents of a
platform
specific driver together in as few files as possible.

>drivers/char/ 
>  It doesn't even register any character device.

I believe that is okay, and that creating a /dev file isn't a
requirement.

>drivers/video/backlight/
>  It doesn't just do backlight control.

Perhaps the backlight part should live there.

>drivers/misc/
>  Seems to be the last resort for everything that doesn't fit it
>  otherwise.
>
>Unless anyone has a better idea I will move it to drivers/misc/, then.

Yeah, there is probably a better place than misc.

The only thing I can say for sure is that it implements
platform-dependent
features rather than standard features in the ACPI spec,
and thus it must live outside of drivers/acpi/.

>> lcd brightness platform support should talk to the existing
>> backlight stuff under sysfs.
>
>It already does exactly that, you can find the backlight class driver
>in /sys/class/backlight/s270/.

Excellent.

>I cannot map the "automatic brightness control" feature to the
>backlight class driver, that's why I duplicated the brightness stuff
>in /proc/acpi/s270/.

Better to extend the backlight code so that it can handle the
special features of this platform than to duplicate brightness
control in multiple places.

>> wlan and bluetooth indicators/controls need to appear under
>> generic places under sysfs -- not under platform specific
>> files under /proc/acpi.
>
>What are those "generic" places? I cannot think of any besides a
>"platform" device.

They should appear as properties under their associated
devices in /sys/devices tree rather than invening a new place.

>Any ideas on that ec_transaction() patch I sent earlier?

I agree 100% that ec.c needs to be whacked.  Unfortunately
there seem to be 3 people doing it at the same time,
so we'll have to sort that out.

thanks,
-Len

ps. Lennart, please understand that I didn't intend to be gruff.
I figured that by the quality of your work -- which is better than most
--
that I'd go for an immediate reply, even though I was still holding a
sharp pointed stick at the end of a 6-hour bug scrub marathon.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] acpi,backlight: MSI S270 laptop support - driver
  2006-08-10 16:15 [PATCH 2/2] acpi,backlight: MSI S270 laptop support - driver Brown, Len
@ 2006-08-10 16:48 ` Lennart Poettering
  0 siblings, 0 replies; 7+ messages in thread
From: Lennart Poettering @ 2006-08-10 16:48 UTC (permalink / raw)
  To: Brown, Len; +Cc: linux-kernel, linux-acpi

On Thu, 10.08.06 12:15, Brown, Len (len.brown@intel.com) wrote:

Len,

> >drivers/video/backlight/
> >  It doesn't just do backlight control.
> 
> Perhaps the backlight part should live there.

Mhmm, that would mean I'd had to split up the driver. That would mean
duplicate code to a certain degree and two separate mini drivers with
very connected features. 

> >drivers/misc/
> >  Seems to be the last resort for everything that doesn't fit it
> >  otherwise.
> >
> >Unless anyone has a better idea I will move it to drivers/misc/, then.
> 
> Yeah, there is probably a better place than misc.

Hmm. As you might have noticed I already posted an updated driver a
few minutes ago which resides in drivers/misc. Is there any need to
move it once again?

> >I cannot map the "automatic brightness control" feature to the
> >backlight class driver, that's why I duplicated the brightness stuff
> >in /proc/acpi/s270/.
> 
> Better to extend the backlight code so that it can handle the
> special features of this platform than to duplicate brightness
> control in multiple places.

Hmm. Better not. That "automatic brightness control" feature and its
behaviour are very specific to this laptop. I don't see any value in
abstracting that. That would be quite a lot of overdesigning in my
eyes. In fact the driver disables this feature on load by default,
assuming that it was probably loaded to do the control in
software. 

> >> wlan and bluetooth indicators/controls need to appear under
> >> generic places under sysfs -- not under platform specific
> >> files under /proc/acpi.
> >
> >What are those "generic" places? I cannot think of any besides a
> >"platform" device.
> 
> They should appear as properties under their associated
> devices in /sys/devices tree rather than invening a new place.

My updated driver does this now. The attributes appear in
/sys/devices/platform/s270pf/.

> >Any ideas on that ec_transaction() patch I sent earlier?
> 
> I agree 100% that ec.c needs to be whacked.  Unfortunately
> there seem to be 3 people doing it at the same time,
> so we'll have to sort that out.

Whoever works on that: please make sure to offer a generic EC access
function similar to my ec_transaction() patch! Thanks!

Is there any timeframe for this EC rework? Is there any chance to get
my patch merged in the meantime? (Hmm, I guess i am little unpatient...)

> ps. Lennart, please understand that I didn't intend to be gruff.  I
> figured that by the quality of your work -- which is better than
> most -- that I'd go for an immediate reply, even though I was still
> holding a sharp pointed stick at the end of a 6-hour bug scrub
> marathon.

Oh, that's OK. I value a quick reply quite a lot. Thank you very much
for your quick review!

Thanks,
        Lennart

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] acpi,backlight: MSI S270 laptop support - driver
  2006-08-10  1:05 Lennart Poettering
@ 2006-08-15 12:42 ` Thomas Renninger
  2006-08-15 13:47   ` Lennart Poettering
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Renninger @ 2006-08-15 12:42 UTC (permalink / raw)
  To: Lennart Poettering; +Cc: len.brown, linux-kernel, linux-acpi

On Thu, 2006-08-10 at 03:05 +0200, Lennart Poettering wrote:
> From: Lennart Poettering <mzxreary@0pointer.de>
> 
>  
> +config ACPI_MSI_S270
> +	tristate "MSI S270 Laptop Extras"
> +	depends on X86
> +    depends on BACKLIGHT_CLASS_DEVICE
> +	---help---
> +	  This is a Linux ACPI 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.

I don't know anything about MSI laptops. But S270 sounds like a very
specific model to me?

Shouldn't the driver just be called acpi_msi driver and try to also
support other MSI models later that might do things at least similar?

    Thomas


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] acpi,backlight: MSI S270 laptop support - driver
  2006-08-15 12:42 ` Thomas Renninger
@ 2006-08-15 13:47   ` Lennart Poettering
  0 siblings, 0 replies; 7+ messages in thread
From: Lennart Poettering @ 2006-08-15 13:47 UTC (permalink / raw)
  To: Thomas Renninger; +Cc: len.brown, linux-kernel, linux-acpi

On Tue, 15.08.06 14:42, Thomas Renninger (trenn@suse.de) wrote:

> > +config ACPI_MSI_S270
> > +	tristate "MSI S270 Laptop Extras"
> > +	depends on X86
> > +    depends on BACKLIGHT_CLASS_DEVICE
> > +	---help---
> > +	  This is a Linux ACPI 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.
> 
> I don't know anything about MSI laptops. But S270 sounds like a very
> specific model to me?

There are quite a lot of different S270 laptops. And the laptops are
sold under various brands and names, hence it's not that specific. 

Until now I found the laptop under the brands MSI, Cytron, TCM,
Medion, Tchibo and the names MegaBook, S270, MD96100, MS-1013,
SAM2000. And I am sure there are even more names/brands...

> Shouldn't the driver just be called acpi_msi driver and try to also
> support other MSI models later that might do things at least
> similar?

The driver uses the ACPI EC but doesn't call any ACPI DSDT method. On
request of Len Brown I moved the driver out of the /drivers/acpi/
namespace and made it use sysfs instead of /proc/acpi:

http://lwn.net/Articles/194916/

Hence I don't think it is a good idea to prefix the driver name with
"acpi_".

I would like to support other laptop models from MSI (specifically
S260), however I don't have access to any of them or to the necessary
hardware information.

But yes, I guess I could rename the driver to "laptop_msi.c" or
something. Although I don't know if the other models use a similar
ACPI EC interface to the S270 model. 

If anyone has a S260 and wants to test this driver on it: please
contact me!

Lennart

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2006-08-15 13:47 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-08-10 16:15 [PATCH 2/2] acpi,backlight: MSI S270 laptop support - driver Brown, Len
2006-08-10 16:48 ` Lennart Poettering
  -- strict thread matches above, loose matches on Subject: below --
2006-08-10  9:36 Brown, Len
2006-08-10 11:46 ` Lennart Poettering
2006-08-10  1:05 Lennart Poettering
2006-08-15 12:42 ` Thomas Renninger
2006-08-15 13:47   ` Lennart Poettering

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox