2006-02-20 02:19:12

by Jaya Kumar

[permalink] [raw]
Subject: [PATCH 2.6.15.3 1/1] ACPI: Atlas ACPI driver

Hi Len, ACPI, and kernel folk,

Appended is my patch adding an ACPI driver for Atlas boards. I've
done this patch against 2.6.15.3.

Please let me know if it looks okay so far and if you have any feedback
or suggestions.

Thanks,
Jaya Kumar

Signed-off-by: Jaya Kumar <[email protected]>

---

MAINTAINERS | 5
drivers/acpi/Kconfig | 11 +
drivers/acpi/Makefile | 1
drivers/acpi/atlas_acpi.c | 279 ++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 296 insertions(+)

---

diff -X linux-2.6.15.3/Documentation/dontdiff -uprN linux-2.6.15.3-vanilla/drivers/acpi/atlas_acpi.c linux-2.6.15.3/drivers/acpi/atlas_acpi.c
--- linux-2.6.15.3-vanilla/drivers/acpi/atlas_acpi.c 1970-01-01 07:30:00.000000000 +0730
+++ linux-2.6.15.3/drivers/acpi/atlas_acpi.c 2006-02-20 10:00:19.000000000 +0800
@@ -0,0 +1,279 @@
+/*
+ * atlas_acpi.c - Atlas Wallmount Touchscreen ACPI Extras
+ *
+ * Copyright (C) 2006 Jaya Kumar
+ * Based on Toshiba ACPI by John Belmonte and ASUS ACPI
+ * This work was sponsored by CIS(M) Sdn Bhd.
+ *
+ * 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., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/proc_fs.h>
+#include <asm/uaccess.h>
+#include <acpi/acpi_drivers.h>
+
+#define PROC_ATLAS "atlas"
+#define ACPI_ATLAS_NAME "Atlas ACPI"
+#define ACPI_ATLAS_CLASS "Atlas"
+#define ACPI_ATLAS_BUTTON_HID "ASIM0000"
+#define ACPI_ATLAS_BRIGHTNESS_HID "ACPILCD00"
+
+struct atlas_device {
+ struct acpi_device *dev;
+ u8 brightness;
+ u8 max_brightness;
+};
+
+static struct proc_dir_entry *atlas_proc_dir;
+static struct atlas_device *atlas_dev;
+
+/* based on acpi_memory_powerdown_device */
+static int atlas_set_brightness(struct acpi_device *device, u16 value)
+{
+ acpi_status status;
+ struct acpi_object_list arg_list;
+ union acpi_object arg;
+
+ arg_list.count = 1;
+ arg_list.pointer = &arg;
+ arg.type = ACPI_TYPE_INTEGER;
+ arg.integer.value = value;
+ status = acpi_evaluate_object(device->handle, "_BCM", &arg_list, NULL);
+ if (ACPI_FAILURE(status)) {
+ printk(KERN_ERR "%s: ACPI set failed\n", __FUNCTION__);
+ return -ENODEV;
+ }
+
+ return status ;
+}
+
+/* based on ibm_get_table_from_acpi */
+static int acpi_read_max_brightness(struct acpi_device *device)
+{
+ union acpi_object *package;
+ struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+ acpi_status status;
+ union acpi_object *obj = NULL;
+
+ status = acpi_evaluate_object(device->handle, "_BCL", NULL, &buffer);
+ if (ACPI_FAILURE(status)) {
+ printk(KERN_ERR "%s: ACPI evaluation failed\n", __FUNCTION__);
+ return -ENODEV;
+ }
+
+ package = (union acpi_object *) buffer.pointer;
+ obj = &(package->package.elements[0]);
+ return obj->integer.value;
+}
+
+/* based on drivers/usb/media/vicam.c:vicam_write_proc_gain */
+static int atlas_write_proc_lcd(struct file *file, const char *buffer,
+ unsigned long count, void *data)
+{
+ u16 gtmp;
+ char kbuf[8];
+ struct atlas_device *dev = (struct atlas_device *) data;
+
+ if (count > 4)
+ return -EINVAL;
+
+ if (copy_from_user(kbuf, buffer, count))
+ return -EFAULT;
+
+ gtmp = (u16) simple_strtoul(kbuf, NULL, 10);
+ if (gtmp > dev->max_brightness)
+ return -EINVAL;
+
+ atlas_set_brightness(dev->dev, gtmp);
+ dev->brightness = gtmp;
+
+ return count;
+}
+
+/* based on drivers/mca/mca-proc.c:get_mca_machine_info */
+static int atlas_read_proc_lcd(char* page, char **start, off_t off,
+ int count, int *eof, void *data)
+{
+ struct atlas_device *dev = (struct atlas_device *) data;
+ int len;
+
+ len = sprintf(page, "brightness: %d\n"
+ "brightness_levels: 0,%d\n",
+ dev->brightness,dev->max_brightness);
+
+ if (len <= off + count)
+ *eof = 1;
+ *start = page + off;
+ len -= off;
+ if (len > count)
+ len = count;
+ if (len < 0)
+ len = 0;
+ return len;
+}
+
+/* button handling code */
+static acpi_status acpi_atlas_button_setup(acpi_handle region_handle,
+ u32 function, void *handler_context, void **return_context)
+{
+ *return_context =
+ (function != ACPI_REGION_DEACTIVATE) ? handler_context : NULL;
+
+ return AE_OK;
+}
+
+static acpi_status acpi_atlas_button_handler(u32 function,
+ acpi_physical_address address,
+ u32 bit_width, acpi_integer * value,
+ void *handler_context, void *region_context)
+{
+ acpi_status status;
+ struct acpi_device *dev;
+
+ dev = (struct acpi_device *) handler_context;
+ if (function == ACPI_WRITE)
+ status = acpi_bus_generate_event(dev, 0x80, address);
+ else {
+ printk(KERN_WARNING "atlas: shrugged on unexpected function"
+ ":function=%x,address=%x,value=%x\n",
+ function, (u32)address, (u32)*value);
+ status = -EINVAL;
+ }
+
+ return status ;
+}
+
+static int atlas_acpi_button_add(struct acpi_device *device)
+{
+
+ /* hookup button handler */
+ return acpi_install_address_space_handler(device->handle,
+ 0x81, &acpi_atlas_button_handler,
+ &acpi_atlas_button_setup, device);
+}
+
+static int atlas_acpi_lcd_add(struct acpi_device *device)
+{
+ acpi_status status;
+ struct proc_dir_entry *proc;
+
+ atlas_dev = (struct atlas_device *)
+ kmalloc(sizeof(struct atlas_device), GFP_KERNEL);
+ if (!atlas_dev)
+ return -ENOMEM;
+
+ /* get max brightness for /proc read use */
+ atlas_dev->max_brightness = acpi_read_max_brightness(device);
+ atlas_dev->dev = device;
+ acpi_driver_data(device) = atlas_dev;
+
+ /* setup proc entry to set and get lcd brightness */
+ proc = create_proc_read_entry("lcd", S_IFREG | S_IRUGO | S_IWUSR,
+ atlas_proc_dir, atlas_read_proc_lcd, atlas_dev);
+ if (!proc) {
+ printk(KERN_ERR "atlas: couldn't alloc proc entry\n");
+ kfree(atlas_dev);
+ status = -ENOMEM;
+ } else {
+ proc->owner = THIS_MODULE;
+ proc->write_proc = atlas_write_proc_lcd;
+ status = AE_OK;
+ }
+ return status ;
+}
+
+static int atlas_acpi_add(struct acpi_device *device)
+{
+
+ if (!strcmp(acpi_device_hid(device), ACPI_ATLAS_BUTTON_HID))
+ return atlas_acpi_button_add(device);
+ else
+ return atlas_acpi_lcd_add(device);
+}
+
+static int atlas_acpi_remove(struct acpi_device *device, int type)
+{
+ acpi_status status;
+
+ if (!device || !acpi_driver_data(device))
+ return -EINVAL;
+
+ if (!strcmp(acpi_device_hid(device), ACPI_ATLAS_BUTTON_HID)) {
+ status = acpi_remove_address_space_handler(device->handle,
+ 0x81, &acpi_atlas_button_handler);
+ if (ACPI_FAILURE(status))
+ printk(KERN_ERR
+ "Atlas: Error removing addr spc handler\n");
+ } else {
+ if (atlas_proc_dir)
+ remove_proc_entry("lcd", atlas_proc_dir);
+
+ kfree(atlas_dev);
+ status = AE_OK;
+ }
+
+ return status;
+}
+
+static struct acpi_driver atlas_acpi_driver = {
+ .name = ACPI_ATLAS_NAME,
+ .class = ACPI_ATLAS_CLASS,
+ .ids = ACPI_ATLAS_BUTTON_HID "," ACPI_ATLAS_BRIGHTNESS_HID,
+ .ops = {
+ .add = atlas_acpi_add,
+ .remove = atlas_acpi_remove,
+ },
+};
+
+static int __init atlas_acpi_init(void)
+{
+ int result;
+
+ atlas_proc_dir = proc_mkdir(PROC_ATLAS, acpi_root_dir);
+ if (!atlas_proc_dir) {
+ printk(KERN_ERR "Atlas ACPI: Unable to create /proc dir\n");
+ return -ENODEV;
+ }
+ atlas_proc_dir->owner = THIS_MODULE;
+
+ result = acpi_bus_register_driver(&atlas_acpi_driver);
+ if (result < 0) {
+ printk(KERN_ERR "Atlas ACPI: Unable to register driver\n");
+ remove_proc_entry(PROC_ATLAS, acpi_root_dir);
+ return -ENODEV;
+ }
+
+ return 0;
+}
+
+static void __exit atlas_acpi_exit(void)
+{
+ acpi_bus_unregister_driver(&atlas_acpi_driver);
+ if (atlas_proc_dir)
+ remove_proc_entry(PROC_ATLAS, acpi_root_dir);
+}
+
+module_init(atlas_acpi_init);
+module_exit(atlas_acpi_exit);
+
+MODULE_AUTHOR("Jaya Kumar");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Atlas ACPI");
+MODULE_SUPPORTED_DEVICE("Atlas ACPI");
diff -X linux-2.6.15.3/Documentation/dontdiff -uprN linux-2.6.15.3-vanilla/drivers/acpi/Kconfig linux-2.6.15.3/drivers/acpi/Kconfig
--- linux-2.6.15.3-vanilla/drivers/acpi/Kconfig 2006-02-19 13:31:11.000000000 +0800
+++ linux-2.6.15.3/drivers/acpi/Kconfig 2006-02-19 19:29:04.000000000 +0800
@@ -194,6 +194,17 @@ config ACPI_ASUS
something works not quite as expected, please use the mailing list
available on the above page ([email protected])

+config ACPI_ATLAS
+ tristate "Atlas Wallmount Touchscreen Extras"
+ depends on X86
+ default n
+ ---help---
+ This driver is intended for Atlas wallmount touchscreens.
+ The button events will show up in /proc/acpi/events and the lcd
+ brightness control is at /proc/acpi/atlas/lcd
+
+ If you have an Atlas wallmount touchscreen, say Y or M here.
+
config ACPI_IBM
tristate "IBM ThinkPad Laptop Extras"
depends on X86
diff -X linux-2.6.15.3/Documentation/dontdiff -uprN linux-2.6.15.3-vanilla/drivers/acpi/Makefile linux-2.6.15.3/drivers/acpi/Makefile
--- linux-2.6.15.3-vanilla/drivers/acpi/Makefile 2006-02-19 13:31:11.000000000 +0800
+++ linux-2.6.15.3/drivers/acpi/Makefile 2006-02-19 14:23:59.000000000 +0800
@@ -53,6 +53,7 @@ obj-$(CONFIG_ACPI_SYSTEM) += system.o ev
obj-$(CONFIG_ACPI_DEBUG) += debug.o
obj-$(CONFIG_ACPI_NUMA) += numa.o
obj-$(CONFIG_ACPI_ASUS) += asus_acpi.o
+obj-$(CONFIG_ACPI_ATLAS) += atlas_acpi.o
obj-$(CONFIG_ACPI_IBM) += ibm_acpi.o
obj-$(CONFIG_ACPI_TOSHIBA) += toshiba_acpi.o
obj-y += scan.o motherboard.o
diff -X linux-2.6.15.3/Documentation/dontdiff -uprN linux-2.6.15.3-vanilla/MAINTAINERS linux-2.6.15.3/MAINTAINERS
--- linux-2.6.15.3-vanilla/MAINTAINERS 2006-02-19 13:30:48.000000000 +0800
+++ linux-2.6.15.3/MAINTAINERS 2006-02-20 09:44:12.000000000 +0800
@@ -366,6 +366,11 @@ M: [email protected]
W: http://www.coraid.com/support/linux
S: Supported

+ATLAS ACPI EXTRAS DRIVER
+P: Jaya Kumar
+M: [email protected]
+S: Maintained
+
ATM
P: Chas Williams
M: [email protected]


2006-02-20 08:26:25

by Luming Yu

[permalink] [raw]
Subject: RE: [PATCH 2.6.15.3 1/1] ACPI: Atlas ACPI driver

I found _BCM, _BCL ... which are reserved methods for
ACPI video extension device. If the vendor follows
ACPI video extension spec, this proposed driver is
NOT needed. Because, we do have acpi video driver
in kernel today. Could you please show me acpidump
output?

Thanks,
Luming

>Hi Len, ACPI, and kernel folk,
>
>Appended is my patch adding an ACPI driver for Atlas boards. I've
>done this patch against 2.6.15.3.
>
>Please let me know if it looks okay so far and if you have any
>feedback
>or suggestions.
>
>Thanks,
>Jaya Kumar
>
>Signed-off-by: Jaya Kumar <[email protected]>
>
>---
>
> MAINTAINERS | 5
> drivers/acpi/Kconfig | 11 +
> drivers/acpi/Makefile | 1
> drivers/acpi/atlas_acpi.c | 279
>++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 296 insertions(+)
>
>---
>
>diff -X linux-2.6.15.3/Documentation/dontdiff -uprN
>linux-2.6.15.3-vanilla/drivers/acpi/atlas_acpi.c
>linux-2.6.15.3/drivers/acpi/atlas_acpi.c
>--- linux-2.6.15.3-vanilla/drivers/acpi/atlas_acpi.c
>1970-01-01 07:30:00.000000000 +0730
>+++ linux-2.6.15.3/drivers/acpi/atlas_acpi.c 2006-02-20
>10:00:19.000000000 +0800
>@@ -0,0 +1,279 @@
>+/*
>+ * atlas_acpi.c - Atlas Wallmount Touchscreen ACPI Extras
>+ *
>+ * Copyright (C) 2006 Jaya Kumar
>+ * Based on Toshiba ACPI by John Belmonte and ASUS ACPI
>+ * This work was sponsored by CIS(M) Sdn Bhd.
>+ *
>+ * 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., 59 Temple Place, Suite 330, Boston, MA
>02111-1307 USA
>+ *
>+ */
>+
>+#include <linux/kernel.h>
>+#include <linux/module.h>
>+#include <linux/init.h>
>+#include <linux/types.h>
>+#include <linux/proc_fs.h>
>+#include <asm/uaccess.h>
>+#include <acpi/acpi_drivers.h>
>+
>+#define PROC_ATLAS "atlas"
>+#define ACPI_ATLAS_NAME "Atlas ACPI"
>+#define ACPI_ATLAS_CLASS "Atlas"
>+#define ACPI_ATLAS_BUTTON_HID "ASIM0000"
>+#define ACPI_ATLAS_BRIGHTNESS_HID "ACPILCD00"
>+
>+struct atlas_device {
>+ struct acpi_device *dev;
>+ u8 brightness;
>+ u8 max_brightness;
>+};
>+
>+static struct proc_dir_entry *atlas_proc_dir;
>+static struct atlas_device *atlas_dev;
>+
>+/* based on acpi_memory_powerdown_device */
>+static int atlas_set_brightness(struct acpi_device *device, u16 value)
>+{
>+ acpi_status status;
>+ struct acpi_object_list arg_list;
>+ union acpi_object arg;
>+
>+ arg_list.count = 1;
>+ arg_list.pointer = &arg;
>+ arg.type = ACPI_TYPE_INTEGER;
>+ arg.integer.value = value;
>+ status = acpi_evaluate_object(device->handle, "_BCM",
>&arg_list, NULL);
>+ if (ACPI_FAILURE(status)) {
>+ printk(KERN_ERR "%s: ACPI set failed\n", __FUNCTION__);
>+ return -ENODEV;
>+ }
>+
>+ return status ;
>+}
>+
>+/* based on ibm_get_table_from_acpi */
>+static int acpi_read_max_brightness(struct acpi_device *device)
>+{
>+ union acpi_object *package;
>+ struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
>+ acpi_status status;
>+ union acpi_object *obj = NULL;
>+
>+ status = acpi_evaluate_object(device->handle, "_BCL",
>NULL, &buffer);
>+ if (ACPI_FAILURE(status)) {
>+ printk(KERN_ERR "%s: ACPI evaluation
>failed\n", __FUNCTION__);
>+ return -ENODEV;
>+ }
>+
>+ package = (union acpi_object *) buffer.pointer;
>+ obj = &(package->package.elements[0]);
>+ return obj->integer.value;
>+}
>+
>+/* based on drivers/usb/media/vicam.c:vicam_write_proc_gain */
>+static int atlas_write_proc_lcd(struct file *file, const char *buffer,
>+ unsigned long count, void *data)
>+{
>+ u16 gtmp;
>+ char kbuf[8];
>+ struct atlas_device *dev = (struct atlas_device *) data;
>+
>+ if (count > 4)
>+ return -EINVAL;
>+
>+ if (copy_from_user(kbuf, buffer, count))
>+ return -EFAULT;
>+
>+ gtmp = (u16) simple_strtoul(kbuf, NULL, 10);
>+ if (gtmp > dev->max_brightness)
>+ return -EINVAL;
>+
>+ atlas_set_brightness(dev->dev, gtmp);
>+ dev->brightness = gtmp;
>+
>+ return count;
>+}
>+
>+/* based on drivers/mca/mca-proc.c:get_mca_machine_info */
>+static int atlas_read_proc_lcd(char* page, char **start, off_t off,
>+ int count, int *eof, void *data)
>+{
>+ struct atlas_device *dev = (struct atlas_device *) data;
>+ int len;
>+
>+ len = sprintf(page, "brightness: %d\n"
>+ "brightness_levels: 0,%d\n",
>+ dev->brightness,dev->max_brightness);
>+
>+ if (len <= off + count)
>+ *eof = 1;
>+ *start = page + off;
>+ len -= off;
>+ if (len > count)
>+ len = count;
>+ if (len < 0)
>+ len = 0;
>+ return len;
>+}
>+
>+/* button handling code */
>+static acpi_status acpi_atlas_button_setup(acpi_handle region_handle,
>+ u32 function, void *handler_context, void
>**return_context)
>+{
>+ *return_context =
>+ (function != ACPI_REGION_DEACTIVATE) ?
>handler_context : NULL;
>+
>+ return AE_OK;
>+}
>+
>+static acpi_status acpi_atlas_button_handler(u32 function,
>+ acpi_physical_address address,
>+ u32 bit_width, acpi_integer * value,
>+ void *handler_context, void *region_context)
>+{
>+ acpi_status status;
>+ struct acpi_device *dev;
>+
>+ dev = (struct acpi_device *) handler_context;
>+ if (function == ACPI_WRITE)
>+ status = acpi_bus_generate_event(dev, 0x80, address);
>+ else {
>+ printk(KERN_WARNING "atlas: shrugged on
>unexpected function"
>+ ":function=%x,address=%x,value=%x\n",
>+ function, (u32)address, (u32)*value);
>+ status = -EINVAL;
>+ }
>+
>+ return status ;
>+}
>+
>+static int atlas_acpi_button_add(struct acpi_device *device)
>+{
>+
>+ /* hookup button handler */
>+ return acpi_install_address_space_handler(device->handle,
>+ 0x81, &acpi_atlas_button_handler,
>+ &acpi_atlas_button_setup, device);
>+}
>+
>+static int atlas_acpi_lcd_add(struct acpi_device *device)
>+{
>+ acpi_status status;
>+ struct proc_dir_entry *proc;
>+
>+ atlas_dev = (struct atlas_device *)
>+ kmalloc(sizeof(struct atlas_device),
>GFP_KERNEL);
>+ if (!atlas_dev)
>+ return -ENOMEM;
>+
>+ /* get max brightness for /proc read use */
>+ atlas_dev->max_brightness = acpi_read_max_brightness(device);
>+ atlas_dev->dev = device;
>+ acpi_driver_data(device) = atlas_dev;
>+
>+ /* setup proc entry to set and get lcd brightness */
>+ proc = create_proc_read_entry("lcd", S_IFREG | S_IRUGO
>| S_IWUSR,
>+ atlas_proc_dir, atlas_read_proc_lcd, atlas_dev);
>+ if (!proc) {
>+ printk(KERN_ERR "atlas: couldn't alloc proc entry\n");
>+ kfree(atlas_dev);
>+ status = -ENOMEM;
>+ } else {
>+ proc->owner = THIS_MODULE;
>+ proc->write_proc = atlas_write_proc_lcd;
>+ status = AE_OK;
>+ }
>+ return status ;
>+}
>+
>+static int atlas_acpi_add(struct acpi_device *device)
>+{
>+
>+ if (!strcmp(acpi_device_hid(device), ACPI_ATLAS_BUTTON_HID))
>+ return atlas_acpi_button_add(device);
>+ else
>+ return atlas_acpi_lcd_add(device);
>+}
>+
>+static int atlas_acpi_remove(struct acpi_device *device, int type)
>+{
>+ acpi_status status;
>+
>+ if (!device || !acpi_driver_data(device))
>+ return -EINVAL;
>+
>+ if (!strcmp(acpi_device_hid(device), ACPI_ATLAS_BUTTON_HID)) {
>+ status =
>acpi_remove_address_space_handler(device->handle,
>+ 0x81, &acpi_atlas_button_handler);
>+ if (ACPI_FAILURE(status))
>+ printk(KERN_ERR
>+ "Atlas: Error removing addr spc
>handler\n");
>+ } else {
>+ if (atlas_proc_dir)
>+ remove_proc_entry("lcd", atlas_proc_dir);
>+
>+ kfree(atlas_dev);
>+ status = AE_OK;
>+ }
>+
>+ return status;
>+}
>+
>+static struct acpi_driver atlas_acpi_driver = {
>+ .name = ACPI_ATLAS_NAME,
>+ .class = ACPI_ATLAS_CLASS,
>+ .ids = ACPI_ATLAS_BUTTON_HID "," ACPI_ATLAS_BRIGHTNESS_HID,
>+ .ops = {
>+ .add = atlas_acpi_add,
>+ .remove = atlas_acpi_remove,
>+ },
>+};
>+
>+static int __init atlas_acpi_init(void)
>+{
>+ int result;
>+
>+ atlas_proc_dir = proc_mkdir(PROC_ATLAS, acpi_root_dir);
>+ if (!atlas_proc_dir) {
>+ printk(KERN_ERR "Atlas ACPI: Unable to create
>/proc dir\n");
>+ return -ENODEV;
>+ }
>+ atlas_proc_dir->owner = THIS_MODULE;
>+
>+ result = acpi_bus_register_driver(&atlas_acpi_driver);
>+ if (result < 0) {
>+ printk(KERN_ERR "Atlas ACPI: Unable to register
>driver\n");
>+ remove_proc_entry(PROC_ATLAS, acpi_root_dir);
>+ return -ENODEV;
>+ }
>+
>+ return 0;
>+}
>+
>+static void __exit atlas_acpi_exit(void)
>+{
>+ acpi_bus_unregister_driver(&atlas_acpi_driver);
>+ if (atlas_proc_dir)
>+ remove_proc_entry(PROC_ATLAS, acpi_root_dir);
>+}
>+
>+module_init(atlas_acpi_init);
>+module_exit(atlas_acpi_exit);
>+
>+MODULE_AUTHOR("Jaya Kumar");
>+MODULE_LICENSE("GPL");
>+MODULE_DESCRIPTION("Atlas ACPI");
>+MODULE_SUPPORTED_DEVICE("Atlas ACPI");
>diff -X linux-2.6.15.3/Documentation/dontdiff -uprN
>linux-2.6.15.3-vanilla/drivers/acpi/Kconfig
>linux-2.6.15.3/drivers/acpi/Kconfig
>--- linux-2.6.15.3-vanilla/drivers/acpi/Kconfig
>2006-02-19 13:31:11.000000000 +0800
>+++ linux-2.6.15.3/drivers/acpi/Kconfig 2006-02-19
>19:29:04.000000000 +0800
>@@ -194,6 +194,17 @@ config ACPI_ASUS
> something works not quite as expected, please use
>the mailing list
> available on the above page
>([email protected])
>
>+config ACPI_ATLAS
>+ tristate "Atlas Wallmount Touchscreen Extras"
>+ depends on X86
>+ default n
>+ ---help---
>+ This driver is intended for Atlas wallmount touchscreens.
>+ The button events will show up in /proc/acpi/events
>and the lcd
>+ brightness control is at /proc/acpi/atlas/lcd
>+
>+ If you have an Atlas wallmount touchscreen, say Y or M here.
>+
> config ACPI_IBM
> tristate "IBM ThinkPad Laptop Extras"
> depends on X86
>diff -X linux-2.6.15.3/Documentation/dontdiff -uprN
>linux-2.6.15.3-vanilla/drivers/acpi/Makefile
>linux-2.6.15.3/drivers/acpi/Makefile
>--- linux-2.6.15.3-vanilla/drivers/acpi/Makefile
>2006-02-19 13:31:11.000000000 +0800
>+++ linux-2.6.15.3/drivers/acpi/Makefile 2006-02-19
>14:23:59.000000000 +0800
>@@ -53,6 +53,7 @@ obj-$(CONFIG_ACPI_SYSTEM) += system.o ev
> obj-$(CONFIG_ACPI_DEBUG) += debug.o
> obj-$(CONFIG_ACPI_NUMA) += numa.o
> obj-$(CONFIG_ACPI_ASUS) += asus_acpi.o
>+obj-$(CONFIG_ACPI_ATLAS) += atlas_acpi.o
> obj-$(CONFIG_ACPI_IBM) += ibm_acpi.o
> obj-$(CONFIG_ACPI_TOSHIBA) += toshiba_acpi.o
> obj-y += scan.o motherboard.o
>diff -X linux-2.6.15.3/Documentation/dontdiff -uprN
>linux-2.6.15.3-vanilla/MAINTAINERS linux-2.6.15.3/MAINTAINERS
>--- linux-2.6.15.3-vanilla/MAINTAINERS 2006-02-19
>13:30:48.000000000 +0800
>+++ linux-2.6.15.3/MAINTAINERS 2006-02-20 09:44:12.000000000 +0800
>@@ -366,6 +366,11 @@ M: [email protected]
> W: http://www.coraid.com/support/linux
> S: Supported
>
>+ATLAS ACPI EXTRAS DRIVER
>+P: Jaya Kumar
>+M: [email protected]
>+S: Maintained
>+
> ATM
> P: Chas Williams
> M: [email protected]
>-
>To unsubscribe from this list: send the line "unsubscribe
>linux-acpi" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2006-02-20 08:37:25

by Jaya Kumar

[permalink] [raw]
Subject: Re: [PATCH 2.6.15.3 1/1] ACPI: Atlas ACPI driver

On 2/20/06, Yu, Luming <[email protected]> wrote:
> I found _BCM, _BCL ... which are reserved methods for
> ACPI video extension device. If the vendor follows
> ACPI video extension spec, this proposed driver is
> NOT needed. Because, we do have acpi video driver
> in kernel today. Could you please show me acpidump
> output?
>
> Thanks,
> Luming

Hi Luming,

Thanks for your reply.

Note that aside from the ACPILCD00 HID, there's also the ASIM0000 HID
that I'm supporting in this driver. I've appended the DSDT info you
requested. Let me know what you feel needs to be done to either make
the current acpi video driver detect this extension device (Should I
just try adding the ACPILCD00 HID to the video driver?).

Thanks,
jayakumar

--- DSDT info ---
Device (LCD)
{
Name (_HID, "ACPILCD00")
Method (_ADR, 0, NotSerialized)
{
Return (0x0110)
}

Method (_INI, 0, NotSerialized)
{
And (GLIE, 0xFFFFDFBF, Local0)
Or (Local0, 0x20400000, GLIE)
And (GLII, 0xFFFFDFBF, Local0)
Or (Local0, 0x20400000, GLII)
And (GLEE, 0xFFFFDFBF, Local0)
Or (Local0, 0x20400000, GLEE)
And (GLOE, 0xDFBFFFFF, Local0)
Or (Local0, 0x2040, GLOE)
And (GLPU, 0xFFFFDFBF, Local0)
Or (Local0, 0x20400000, GLPU)
And (GLOV, 0xFFBFDFFF, Local0)
Or (Local0, 0x20000040, GLOV)
Sleep (0x0BB8)
And (GLOV, 0xDFBFFFFF, Local0)
Or (Local0, 0x2040, GLOV)
Store (VSA8 (0x16, 0x30), BRSV)
If (LNot (LLess (BRSV, 0x20)))
{
Store (0x1F, BRSV)
}

Store (One, Local0)
While (LNot (LGreater (Local0, BRSV)))
{
SBRL (One, One)
Increment (Local0)
}
}
Method (_BCL, 0, NotSerialized)
{
Return (Package (0x02)
{
0x1F,
0x1F
})
}

Method (_BCM, 1, NotSerialized)
{
Store (Arg0, Local0)
Store (RBRL (), Local1)
And (Local1, 0xFF, Local1)
If (LAnd (LNot (LLess (Local0, Zero)), LNot
(LGreater (Local0, Local1))))
{
Store (BRSV, Local1)
And (Local1, 0xFF, Local1)
While (LNot (LEqual (Arg0, BRSV)))
{
If (LGreater (Arg0, BRSV))
{
SBRL (One, One)
Increment (BRSV)
}
Else
{
If (LEqual (Arg0, BRSV))
{
Break
}
Else
{
SBRL (Zero, One)
Decrement (BRSV)
}
}
}
}
}
}
}


>
> >Hi Len, ACPI, and kernel folk,
> >
> >Appended is my patch adding an ACPI driver for Atlas boards. I've
> >done this patch against 2.6.15.3.
> >
> >Please let me know if it looks okay so far and if you have any
> >feedback
> >or suggestions.
> >
> >Thanks,
> >Jaya Kumar
> >
> >Signed-off-by: Jaya Kumar <[email protected]>
> >
> >---
> >
> > MAINTAINERS | 5
> > drivers/acpi/Kconfig | 11 +
> > drivers/acpi/Makefile | 1
> > drivers/acpi/atlas_acpi.c | 279
> >++++++++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 296 insertions(+)
> >
> >---
> >
> >diff -X linux-2.6.15.3/Documentation/dontdiff -uprN
> >linux-2.6.15.3-vanilla/drivers/acpi/atlas_acpi.c
> >linux-2.6.15.3/drivers/acpi/atlas_acpi.c
> >--- linux-2.6.15.3-vanilla/drivers/acpi/atlas_acpi.c
> >1970-01-01 07:30:00.000000000 +0730
> >+++ linux-2.6.15.3/drivers/acpi/atlas_acpi.c 2006-02-20
> >10:00:19.000000000 +0800
> >@@ -0,0 +1,279 @@
> >+/*
> >+ * atlas_acpi.c - Atlas Wallmount Touchscreen ACPI Extras
> >+ *
> >+ * Copyright (C) 2006 Jaya Kumar
> >+ * Based on Toshiba ACPI by John Belmonte and ASUS ACPI
> >+ * This work was sponsored by CIS(M) Sdn Bhd.
> >+ *
> >+ * 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., 59 Temple Place, Suite 330, Boston, MA
> >02111-1307 USA
> >+ *
> >+ */
> >+
> >+#include <linux/kernel.h>
> >+#include <linux/module.h>
> >+#include <linux/init.h>
> >+#include <linux/types.h>
> >+#include <linux/proc_fs.h>
> >+#include <asm/uaccess.h>
> >+#include <acpi/acpi_drivers.h>
> >+
> >+#define PROC_ATLAS "atlas"
> >+#define ACPI_ATLAS_NAME "Atlas ACPI"
> >+#define ACPI_ATLAS_CLASS "Atlas"
> >+#define ACPI_ATLAS_BUTTON_HID "ASIM0000"
> >+#define ACPI_ATLAS_BRIGHTNESS_HID "ACPILCD00"
> >+
> >+struct atlas_device {
> >+ struct acpi_device *dev;
> >+ u8 brightness;
> >+ u8 max_brightness;
> >+};
> >+
> >+static struct proc_dir_entry *atlas_proc_dir;
> >+static struct atlas_device *atlas_dev;
> >+
> >+/* based on acpi_memory_powerdown_device */
> >+static int atlas_set_brightness(struct acpi_device *device, u16 value)
> >+{
> >+ acpi_status status;
> >+ struct acpi_object_list arg_list;
> >+ union acpi_object arg;
> >+
> >+ arg_list.count = 1;
> >+ arg_list.pointer = &arg;
> >+ arg.type = ACPI_TYPE_INTEGER;
> >+ arg.integer.value = value;
> >+ status = acpi_evaluate_object(device->handle, "_BCM",
> >&arg_list, NULL);
> >+ if (ACPI_FAILURE(status)) {
> >+ printk(KERN_ERR "%s: ACPI set failed\n", __FUNCTION__);
> >+ return -ENODEV;
> >+ }
> >+
> >+ return status ;
> >+}
> >+
> >+/* based on ibm_get_table_from_acpi */
> >+static int acpi_read_max_brightness(struct acpi_device *device)
> >+{
> >+ union acpi_object *package;
> >+ struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> >+ acpi_status status;
> >+ union acpi_object *obj = NULL;
> >+
> >+ status = acpi_evaluate_object(device->handle, "_BCL",
> >NULL, &buffer);
> >+ if (ACPI_FAILURE(status)) {
> >+ printk(KERN_ERR "%s: ACPI evaluation
> >failed\n", __FUNCTION__);
> >+ return -ENODEV;
> >+ }
> >+
> >+ package = (union acpi_object *) buffer.pointer;
> >+ obj = &(package->package.elements[0]);
> >+ return obj->integer.value;
> >+}
> >+
> >+/* based on drivers/usb/media/vicam.c:vicam_write_proc_gain */
> >+static int atlas_write_proc_lcd(struct file *file, const char *buffer,
> >+ unsigned long count, void *data)
> >+{
> >+ u16 gtmp;
> >+ char kbuf[8];
> >+ struct atlas_device *dev = (struct atlas_device *) data;
> >+
> >+ if (count > 4)
> >+ return -EINVAL;
> >+
> >+ if (copy_from_user(kbuf, buffer, count))
> >+ return -EFAULT;
> >+
> >+ gtmp = (u16) simple_strtoul(kbuf, NULL, 10);
> >+ if (gtmp > dev->max_brightness)
> >+ return -EINVAL;
> >+
> >+ atlas_set_brightness(dev->dev, gtmp);
> >+ dev->brightness = gtmp;
> >+
> >+ return count;
> >+}
> >+
> >+/* based on drivers/mca/mca-proc.c:get_mca_machine_info */
> >+static int atlas_read_proc_lcd(char* page, char **start, off_t off,
> >+ int count, int *eof, void *data)
> >+{
> >+ struct atlas_device *dev = (struct atlas_device *) data;
> >+ int len;
> >+
> >+ len = sprintf(page, "brightness: %d\n"
> >+ "brightness_levels: 0,%d\n",
> >+ dev->brightness,dev->max_brightness);
> >+
> >+ if (len <= off + count)
> >+ *eof = 1;
> >+ *start = page + off;
> >+ len -= off;
> >+ if (len > count)
> >+ len = count;
> >+ if (len < 0)
> >+ len = 0;
> >+ return len;
> >+}
> >+
> >+/* button handling code */
> >+static acpi_status acpi_atlas_button_setup(acpi_handle region_handle,
> >+ u32 function, void *handler_context, void
> >**return_context)
> >+{
> >+ *return_context =
> >+ (function != ACPI_REGION_DEACTIVATE) ?
> >handler_context : NULL;
> >+
> >+ return AE_OK;
> >+}
> >+
> >+static acpi_status acpi_atlas_button_handler(u32 function,
> >+ acpi_physical_address address,
> >+ u32 bit_width, acpi_integer * value,
> >+ void *handler_context, void *region_context)
> >+{
> >+ acpi_status status;
> >+ struct acpi_device *dev;
> >+
> >+ dev = (struct acpi_device *) handler_context;
> >+ if (function == ACPI_WRITE)
> >+ status = acpi_bus_generate_event(dev, 0x80, address);
> >+ else {
> >+ printk(KERN_WARNING "atlas: shrugged on
> >unexpected function"
> >+ ":function=%x,address=%x,value=%x\n",
> >+ function, (u32)address, (u32)*value);
> >+ status = -EINVAL;
> >+ }
> >+
> >+ return status ;
> >+}
> >+
> >+static int atlas_acpi_button_add(struct acpi_device *device)
> >+{
> >+
> >+ /* hookup button handler */
> >+ return acpi_install_address_space_handler(device->handle,
> >+ 0x81, &acpi_atlas_button_handler,
> >+ &acpi_atlas_button_setup, device);
> >+}
> >+
> >+static int atlas_acpi_lcd_add(struct acpi_device *device)
> >+{
> >+ acpi_status status;
> >+ struct proc_dir_entry *proc;
> >+
> >+ atlas_dev = (struct atlas_device *)
> >+ kmalloc(sizeof(struct atlas_device),
> >GFP_KERNEL);
> >+ if (!atlas_dev)
> >+ return -ENOMEM;
> >+
> >+ /* get max brightness for /proc read use */
> >+ atlas_dev->max_brightness = acpi_read_max_brightness(device);
> >+ atlas_dev->dev = device;
> >+ acpi_driver_data(device) = atlas_dev;
> >+
> >+ /* setup proc entry to set and get lcd brightness */
> >+ proc = create_proc_read_entry("lcd", S_IFREG | S_IRUGO
> >| S_IWUSR,
> >+ atlas_proc_dir, atlas_read_proc_lcd, atlas_dev);
> >+ if (!proc) {
> >+ printk(KERN_ERR "atlas: couldn't alloc proc entry\n");
> >+ kfree(atlas_dev);
> >+ status = -ENOMEM;
> >+ } else {
> >+ proc->owner = THIS_MODULE;
> >+ proc->write_proc = atlas_write_proc_lcd;
> >+ status = AE_OK;
> >+ }
> >+ return status ;
> >+}
> >+
> >+static int atlas_acpi_add(struct acpi_device *device)
> >+{
> >+
> >+ if (!strcmp(acpi_device_hid(device), ACPI_ATLAS_BUTTON_HID))
> >+ return atlas_acpi_button_add(device);
> >+ else
> >+ return atlas_acpi_lcd_add(device);
> >+}
> >+
> >+static int atlas_acpi_remove(struct acpi_device *device, int type)
> >+{
> >+ acpi_status status;
> >+
> >+ if (!device || !acpi_driver_data(device))
> >+ return -EINVAL;
> >+
> >+ if (!strcmp(acpi_device_hid(device), ACPI_ATLAS_BUTTON_HID)) {
> >+ status =
> >acpi_remove_address_space_handler(device->handle,
> >+ 0x81, &acpi_atlas_button_handler);
> >+ if (ACPI_FAILURE(status))
> >+ printk(KERN_ERR
> >+ "Atlas: Error removing addr spc
> >handler\n");
> >+ } else {
> >+ if (atlas_proc_dir)
> >+ remove_proc_entry("lcd", atlas_proc_dir);
> >+
> >+ kfree(atlas_dev);
> >+ status = AE_OK;
> >+ }
> >+
> >+ return status;
> >+}
> >+
> >+static struct acpi_driver atlas_acpi_driver = {
> >+ .name = ACPI_ATLAS_NAME,
> >+ .class = ACPI_ATLAS_CLASS,
> >+ .ids = ACPI_ATLAS_BUTTON_HID "," ACPI_ATLAS_BRIGHTNESS_HID,
> >+ .ops = {
> >+ .add = atlas_acpi_add,
> >+ .remove = atlas_acpi_remove,
> >+ },
> >+};
> >+
> >+static int __init atlas_acpi_init(void)
> >+{
> >+ int result;
> >+
> >+ atlas_proc_dir = proc_mkdir(PROC_ATLAS, acpi_root_dir);
> >+ if (!atlas_proc_dir) {
> >+ printk(KERN_ERR "Atlas ACPI: Unable to create
> >/proc dir\n");
> >+ return -ENODEV;
> >+ }
> >+ atlas_proc_dir->owner = THIS_MODULE;
> >+
> >+ result = acpi_bus_register_driver(&atlas_acpi_driver);
> >+ if (result < 0) {
> >+ printk(KERN_ERR "Atlas ACPI: Unable to register
> >driver\n");
> >+ remove_proc_entry(PROC_ATLAS, acpi_root_dir);
> >+ return -ENODEV;
> >+ }
> >+
> >+ return 0;
> >+}
> >+
> >+static void __exit atlas_acpi_exit(void)
> >+{
> >+ acpi_bus_unregister_driver(&atlas_acpi_driver);
> >+ if (atlas_proc_dir)
> >+ remove_proc_entry(PROC_ATLAS, acpi_root_dir);
> >+}
> >+
> >+module_init(atlas_acpi_init);
> >+module_exit(atlas_acpi_exit);
> >+
> >+MODULE_AUTHOR("Jaya Kumar");
> >+MODULE_LICENSE("GPL");
> >+MODULE_DESCRIPTION("Atlas ACPI");
> >+MODULE_SUPPORTED_DEVICE("Atlas ACPI");
> >diff -X linux-2.6.15.3/Documentation/dontdiff -uprN
> >linux-2.6.15.3-vanilla/drivers/acpi/Kconfig
> >linux-2.6.15.3/drivers/acpi/Kconfig
> >--- linux-2.6.15.3-vanilla/drivers/acpi/Kconfig
> >2006-02-19 13:31:11.000000000 +0800
> >+++ linux-2.6.15.3/drivers/acpi/Kconfig 2006-02-19
> >19:29:04.000000000 +0800
> >@@ -194,6 +194,17 @@ config ACPI_ASUS
> > something works not quite as expected, please use
> >the mailing list
> > available on the above page
> >([email protected])
> >
> >+config ACPI_ATLAS
> >+ tristate "Atlas Wallmount Touchscreen Extras"
> >+ depends on X86
> >+ default n
> >+ ---help---
> >+ This driver is intended for Atlas wallmount touchscreens.
> >+ The button events will show up in /proc/acpi/events
> >and the lcd
> >+ brightness control is at /proc/acpi/atlas/lcd
> >+
> >+ If you have an Atlas wallmount touchscreen, say Y or M here.
> >+
> > config ACPI_IBM
> > tristate "IBM ThinkPad Laptop Extras"
> > depends on X86
> >diff -X linux-2.6.15.3/Documentation/dontdiff -uprN
> >linux-2.6.15.3-vanilla/drivers/acpi/Makefile
> >linux-2.6.15.3/drivers/acpi/Makefile
> >--- linux-2.6.15.3-vanilla/drivers/acpi/Makefile
> >2006-02-19 13:31:11.000000000 +0800
> >+++ linux-2.6.15.3/drivers/acpi/Makefile 2006-02-19
> >14:23:59.000000000 +0800
> >@@ -53,6 +53,7 @@ obj-$(CONFIG_ACPI_SYSTEM) += system.o ev
> > obj-$(CONFIG_ACPI_DEBUG) += debug.o
> > obj-$(CONFIG_ACPI_NUMA) += numa.o
> > obj-$(CONFIG_ACPI_ASUS) += asus_acpi.o
> >+obj-$(CONFIG_ACPI_ATLAS) += atlas_acpi.o
> > obj-$(CONFIG_ACPI_IBM) += ibm_acpi.o
> > obj-$(CONFIG_ACPI_TOSHIBA) += toshiba_acpi.o
> > obj-y += scan.o motherboard.o
> >diff -X linux-2.6.15.3/Documentation/dontdiff -uprN
> >linux-2.6.15.3-vanilla/MAINTAINERS linux-2.6.15.3/MAINTAINERS
> >--- linux-2.6.15.3-vanilla/MAINTAINERS 2006-02-19
> >13:30:48.000000000 +0800
> >+++ linux-2.6.15.3/MAINTAINERS 2006-02-20 09:44:12.000000000 +0800
> >@@ -366,6 +366,11 @@ M: [email protected]
> > W: http://www.coraid.com/support/linux
> > S: Supported
> >
> >+ATLAS ACPI EXTRAS DRIVER
> >+P: Jaya Kumar
> >+M: [email protected]
> >+S: Maintained
> >+
> > ATM
> > P: Chas Williams
> > M: [email protected]
> >-
> >To unsubscribe from this list: send the line "unsubscribe
> >linux-acpi" in
> >the body of a message to [email protected]
> >More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>

2006-02-20 10:27:28

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 2.6.15.3 1/1] ACPI: Atlas ACPI driver

On Mon, Feb 20, 2006 at 10:13:53AM +0800, [email protected] wrote:

> + /* setup proc entry to set and get lcd brightness */
> + proc = create_proc_read_entry("lcd", S_IFREG | S_IRUGO | S_IWUSR,
> + atlas_proc_dir, atlas_read_proc_lcd, atlas_dev);

For basic sanity, could this please be a standard backlight driver
rather than sticking yet another backlight control under yet another
directory in /proc? It makes userspace much, much easier.
drivers/video/backlight/corgi_bl.c is an example, but also see my posts
to acpi-devel with patches that add it to existing acpi drivers.

> + return atlas_acpi_button_add(device);

What buttons does the hardware have? Would it make more sense for it to
be an input driver rather than (or as well as) just dropping stuff in
acpi/events?

--
Matthew Garrett | [email protected]

2006-02-20 10:49:56

by Jaya Kumar

[permalink] [raw]
Subject: Re: [PATCH 2.6.15.3 1/1] ACPI: Atlas ACPI driver

On 2/20/06, Matthew Garrett <[email protected]> wrote:
> On Mon, Feb 20, 2006 at 10:13:53AM +0800, [email protected] wrote:
>
> > + /* setup proc entry to set and get lcd brightness */
> > + proc = create_proc_read_entry("lcd", S_IFREG | S_IRUGO | S_IWUSR,
> > + atlas_proc_dir, atlas_read_proc_lcd, atlas_dev);
>
> For basic sanity, could this please be a standard backlight driver
> rather than sticking yet another backlight control under yet another
> directory in /proc? It makes userspace much, much easier.

I'm not sure how standard that is. For example, I looked at the asus
and toshiba drivers. These ACPI board drivers use
/proc/acpi/somedevice/lcd. For example,

asus_acpi.c
894 asus_proc_add(PROC_LCD, &proc_write_lcd,
&proc_read_lcd, mode,
895 device);

toshiba_acpi.c
472 {"lcd", read_lcd, write_lcd},

So, that's why I chose to do the same in my implementation. I'd have
much rather used a generic sysfs entry but that's not what any ACPI
drivers appear to do. Further, I see that Patrick Mochel is rewriting
the whole acpi driver model (and incorporating sysfs) anyway so I
figured I'd go with the flow of existing drivers. Perhaps someone
could clarify what the consensus is. I'd be happy to make any desired
adjustments.

> drivers/video/backlight/corgi_bl.c is an example, but also see my posts
> to acpi-devel with patches that add it to existing acpi drivers.

I'll go take a look at that. I didn't look for an acpi driver outside
of the drivers/acpi directory. But if that's the consensus, shouldn't
someone also mod the toshiba and asus drivers?

>
> > + return atlas_acpi_button_add(device);
>
> What buttons does the hardware have? Would it make more sense for it to

Standard wallmount stuff. There's 8 buttons on the one I'm using for
testing. Vol up/down. Brightness up/down. Then several buttons for
miscellaneous usage by people who customize the chassis. Most apps for
this type of board are custom written and tend to just select on
/proc/acpi/event.

> be an input driver rather than (or as well as) just dropping stuff in
> acpi/events?

I would have loved to make it an input driver. But looking at the
mailing list archives, that seems to be a bone of contention and hence
I chose to go with the flow. I'll be happy to switch it over to an
input driver if there is consensus around that. Please do let me know.

Thanks,
jayakumar

>
> --
> Matthew Garrett | [email protected]
>

2006-02-20 11:02:25

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 2.6.15.3 1/1] ACPI: Atlas ACPI driver

On Mon, Feb 20, 2006 at 06:49:54PM +0800, Jaya Kumar wrote:

> I'm not sure how standard that is. For example, I looked at the asus
> and toshiba drivers. These ACPI board drivers use
> /proc/acpi/somedevice/lcd. For example,

And, from a userspace perspective, it sucks. I'm in the process of
writing patches to transition them all over, and I'd prefer not to have
to write one for your driver as well :)

> I'll go take a look at that. I didn't look for an acpi driver outside
> of the drivers/acpi directory. But if that's the consensus, shouldn't
> someone also mod the toshiba and asus drivers?

I'm doing so.

> Standard wallmount stuff. There's 8 buttons on the one I'm using for
> testing. Vol up/down. Brightness up/down. Then several buttons for
> miscellaneous usage by people who customize the chassis. Most apps for
> this type of board are custom written and tend to just select on
> /proc/acpi/event.

Volume and brightness are things that can easily be exposed through the
input layer, and if you're running X then it's much easier to handle
events that come through the input layer than ones which come from
acpi/events. There's four keycodes for programmable buttons specced (see
/usr/include/linux/input.h - _PROG1-4), so that would fit quite nicely
as well.

Doing it via the input layer adds flexibility - it makes it easier for
non-root uesrspace to handle things, but you can still have a root-level
daemon that monitors /dev/input/event* and runs commands in response to
keycodes.

--
Matthew Garrett | [email protected]

2006-02-20 11:25:28

by Jaya Kumar

[permalink] [raw]
Subject: Re: [PATCH 2.6.15.3 1/1] ACPI: Atlas ACPI driver

On 2/20/06, Matthew Garrett <[email protected]> wrote:
> On Mon, Feb 20, 2006 at 06:49:54PM +0800, Jaya Kumar wrote:
>
> > I'm not sure how standard that is. For example, I looked at the asus
> > and toshiba drivers. These ACPI board drivers use
> > /proc/acpi/somedevice/lcd. For example,
>
> And, from a userspace perspective, it sucks. I'm in the process of
> writing patches to transition them all over, and I'd prefer not to have
> to write one for your driver as well :)

I have some questions then.
1. Are Patrick's acpi driver model changes considered to be a more
final approach that standardize everyone to some sysfs based interface
to userspace?
1a. Can I assume there is consensus among the acpi community around
his new model?
2. Is his driver model going to maintain compatibility with the older
existing /proc model and those userspace apps that already use that
interface?
3. Is his driver model going to also maintain compatibility with your
newer model (assuming that his model is different than yours).

>
> > I'll go take a look at that. I didn't look for an acpi driver outside
> > of the drivers/acpi directory. But if that's the consensus, shouldn't
> > someone also mod the toshiba and asus drivers?
>
> I'm doing so.

Ok. I wish I'd known before. I scanned the mailing list before
mbarking on this to see if any issues were raised with toshiba and
asus driver code and didn't see anything. FWIW, my powers of mind
reading only work on Fridays. :-)

> Doing it via the input layer adds flexibility - it makes it easier for
> non-root uesrspace to handle things, but you can still have a root-level
> daemon that monitors /dev/input/event* and runs commands in response to
> keycodes.
>

Oh. I don't disagree with that.

Thanks,
jayakumar

2006-02-20 11:28:59

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 2.6.15.3 1/1] ACPI: Atlas ACPI driver

On Mon, Feb 20, 2006 at 07:25:26PM +0800, Jaya Kumar wrote:

> I have some questions then.
> 1. Are Patrick's acpi driver model changes considered to be a more
> final approach that standardize everyone to some sysfs based interface
> to userspace?

I don't believe so. Backlight drivers will still need to register as
such independently. It's just somewhat orthogonal.

> Ok. I wish I'd known before. I scanned the mailing list before
> mbarking on this to see if any issues were raised with toshiba and
> asus driver code and didn't see anything. FWIW, my powers of mind
> reading only work on Fridays. :-)

I started sending patches a couple of weeks ago, but I need to tidy them
up to apply against latest -mm.

--
Matthew Garrett | [email protected]

2006-02-21 06:35:09

by Luming Yu

[permalink] [raw]
Subject: RE: [PATCH 2.6.15.3 1/1] ACPI: Atlas ACPI driver


>On 2/20/06, Yu, Luming <[email protected]> wrote:
>> I found _BCM, _BCL ... which are reserved methods for
>> ACPI video extension device. If the vendor follows
>> ACPI video extension spec, this proposed driver is
>> NOT needed. Because, we do have acpi video driver
>> in kernel today. Could you please show me acpidump
>> output?
>>
>> Thanks,
>> Luming
>
>Hi Luming,
>
>Thanks for your reply.
>
>Note that aside from the ACPILCD00 HID, there's also the ASIM0000 HID
>that I'm supporting in this driver. I've appended the DSDT info you
>requested. Let me know what you feel needs to be done to either make
>the current acpi video driver detect this extension device (Should I
>just try adding the ACPILCD00 HID to the video driver?).
>
It would be better if you can merge this stuff with acpi video driver.
If you look at video.c, there is NO HID definition for video device.
we rely on acpi_video_bus_match to recoginize video device in ACPI
namespace.

As for hotkey stuff, please take a look at
http://bugzilla.kernel.org/show_bug.cgi?id=5749

Thanks,
Luming

2006-03-03 08:16:08

by Jaya Kumar

[permalink] [raw]
Subject: Re: [PATCH 2.6.15.3 1/1] ACPI: Atlas ACPI driver

On 2/21/06, Yu, Luming <[email protected]> wrote:
> It would be better if you can merge this stuff with acpi video driver.
> If you look at video.c, there is NO HID definition for video device.
> we rely on acpi_video_bus_match to recoginize video device in ACPI
> namespace.

I took a quick look and I think Atlas might be ugly. The current acpi
video driver, as you pointed out, looks for well known required nodes,
specifically _DOD,_DOS,_ROM,.... None of these nodes are provided on
Atlas. From looking at the DSDT, all I see are _BCL and _BCM. It
doesn't even have _BQC. So, from a quick look at the existing video
driver, I see a couple of problems that I need advice on:

- is it ok if I add a _BCM check in acpi_video_bus_match. i think this
is not a problem.
- acpi_video_bus_check fails out if no _DOS,_ROM,_GPD,_SPD,_VPO. this
check fails on Atlas because it doesn't have any of those
capabilities. The video driver does check for _BCM in
video_device_find_cap but that's at a much later stage. Do you want me
to do something like if (acpi_video_bus_check() &&
acpi_video_output_device_exceptions_detect() )... or do you want me to
do something cleaner and move the whole _BCM detection earlier in the
process?
- acpi_video_bus_get_devices will only call video_bus_get_one_device
if the device node has children. In Atlas, the "Video Bus", (ie: LCD
device in the DSDT I pasted before) has no children, at least as found
by scan.c, so we won't get to get_one_device(). What do you think I
should do about this?

I have a suspicion that for boards like this where ACPI support isn't
so complete, we should just use board specific drivers rather than
mangling the mainstream video driver code. What do you think?

Thanks,
jayakumar

2006-03-06 13:50:44

by Luming Yu

[permalink] [raw]
Subject: RE: [PATCH 2.6.15.3 1/1] ACPI: Atlas ACPI driver


>On 2/21/06, Yu, Luming <[email protected]> wrote:
>> It would be better if you can merge this stuff with acpi
>video driver.
>> If you look at video.c, there is NO HID definition for video device.
>> we rely on acpi_video_bus_match to recoginize video device in ACPI
>> namespace.
>
>I took a quick look and I think Atlas might be ugly. The current acpi
>video driver, as you pointed out, looks for well known required nodes,
>specifically _DOD,_DOS,_ROM,.... None of these nodes are provided on
>Atlas. From looking at the DSDT, all I see are _BCL and _BCM. It
>doesn't even have _BQC. So, from a quick look at the existing video
>driver, I see a couple of problems that I need advice on:
>
>- is it ok if I add a _BCM check in acpi_video_bus_match. i think this
>is not a problem.

Yes, it's ok, i think.

>- acpi_video_bus_check fails out if no _DOS,_ROM,_GPD,_SPD,_VPO. this
>check fails on Atlas because it doesn't have any of those

These check might be too strict. To face reality, it might need
be more loose, because it depends on platform vendor to implement
acpi video extension.

>capabilities. The video driver does check for _BCM in
>video_device_find_cap but that's at a much later stage. Do you want me
>to do something like if (acpi_video_bus_check() &&
>acpi_video_output_device_exceptions_detect() )... or do you want me to
>do something cleaner and move the whole _BCM detection earlier in the
>process?

moving it earlier might better.

>- acpi_video_bus_get_devices will only call video_bus_get_one_device
>if the device node has children. In Atlas, the "Video Bus", (ie: LCD
>device in the DSDT I pasted before) has no children, at least as found
>by scan.c, so we won't get to get_one_device(). What do you think I
>should do about this?

Please resend me the acpidump, it's somehow dropped from my mailbox.

>
>I have a suspicion that for boards like this where ACPI support isn't
>so complete, we should just use board specific drivers rather than
>mangling the mainstream video driver code. What do you think?

that is the last resort, if you can prove video.c or hotkey.c is NOT
capable to handle . We do need to make things as generic as possible
in this area, otherwise, I cannot imagine how they can be maintained .
That's the reason why I'm thinking about to propose a ACPI hotkey spec.
Do you have idea for that?

Thanks,
Luming


2006-03-07 01:50:36

by Jaya Kumar

[permalink] [raw]
Subject: Re: [PATCH 2.6.15.3 1/1] ACPI: Atlas ACPI driver

On 3/6/06, Yu, Luming <[email protected]> wrote:
> >- acpi_video_bus_check fails out if no _DOS,_ROM,_GPD,_SPD,_VPO. this
> >check fails on Atlas because it doesn't have any of those
>
> These check might be too strict. To face reality, it might need
> be more loose, because it depends on platform vendor to implement
> acpi video extension.

Ok. I think I understand.

> >acpi_video_output_device_exceptions_detect() )... or do you want me to
> >do something cleaner and move the whole _BCM detection earlier in the
> >process?
>
> moving it earlier might better.

I'll try to do that.

> >- acpi_video_bus_get_devices will only call video_bus_get_one_device
> >if the device node has children. In Atlas, the "Video Bus", (ie: LCD
> >device in the DSDT I pasted before) has no children, at least as found
> >by scan.c, so we won't get to get_one_device(). What do you think I
> >should do about this?
>
> Please resend me the acpidump, it's somehow dropped from my mailbox.

I've added it as an attachment here. Please let me know if you get it.

>
> >
> >I have a suspicion that for boards like this where ACPI support isn't
> >so complete, we should just use board specific drivers rather than
> >mangling the mainstream video driver code. What do you think?
>
> that is the last resort, if you can prove video.c or hotkey.c is NOT
> capable to handle . We do need to make things as generic as possible
> in this area, otherwise, I cannot imagine how they can be maintained .
> That's the reason why I'm thinking about to propose a ACPI hotkey spec.
> Do you have idea for that?

Yes, you are right about making things generic. I'll try harder to
integrate it with video.c. For the latter, I'm not sure if hotkey is
correct for Atlas. Perhaps you mean the button support? Because
physically, these are buttons rather than hotkeys.

Thanks,
jayakumar


Attachments:
(No filename) (1.81 kB)
dsdt.dsl (88.48 kB)
Download all attachments

2006-03-07 08:52:51

by Luming Yu

[permalink] [raw]
Subject: RE: [PATCH 2.6.15.3 1/1] ACPI: Atlas ACPI driver

>I've added it as an attachment here. Please let me know if you get it.
>
I just found this:
Device (SVG)
Device (LCD)
It looks too simple to fit video.c.

But it is quite easy to implement in hotkey.c.
Because it has dedicated device with HID and ,
well-known method name.

Ideally, you can enable this jus like this:
http://bugzilla.kernel.org/attachment.cgi?id=6843&action=view

Note, the latest hotkey.c need patch at
http://bugzilla.kernel.org/show_bug.cgi?id=5749


Thanks,
Luming

2006-03-07 16:09:32

by Jaya Kumar

[permalink] [raw]
Subject: Re: [PATCH 2.6.15.3 1/1] ACPI: Atlas ACPI driver

On 3/7/06, Yu, Luming <[email protected]> wrote:
> >I've added it as an attachment here. Please let me know if you get it.
> >
> I just found this:
> Device (SVG)
> Device (LCD)
> It looks too simple to fit video.c.

I'm sorry but I'm confused. Do you mean that I shouldn't try to
support the brightness using the default video driver (ie: leave it as
a board specific driver because the board's ACPI implementation for
video support is insufficient) or did you mean that it will be easy
and simple to fit in video.c?

> But it is quite easy to implement in hotkey.c.
> Because it has dedicated device with HID and ,
> well-known method name.
>

I'm confused by hotkey here. I'm not sure how hotkey is involved here.
The Atlas board has no keys physically. It has buttons which are
hooked up using the ASIM HID and that is what I am trying to support
since those physically exist on the board. Can you help me understand
how hotkey is involved in this?

Thanks,
jayakumar

2006-03-08 06:17:38

by Luming Yu

[permalink] [raw]
Subject: RE: [PATCH 2.6.15.3 1/1] ACPI: Atlas ACPI driver


>> I just found this:
>> Device (SVG)
>> Device (LCD)
>> It looks too simple to fit video.c.
>
>I'm sorry but I'm confused. Do you mean that I shouldn't try to
>support the brightness using the default video driver (ie: leave it as
>a board specific driver because the board's ACPI implementation for
>video support is insufficient) or did you mean that it will be easy
>and simple to fit in video.c?

I think video.c is NOT a right solution for your problem.

>
>> But it is quite easy to implement in hotkey.c.
>> Because it has dedicated device with HID and ,
>> well-known method name.
>>
>
>I'm confused by hotkey here. I'm not sure how hotkey is involved here.

hotkey.c provide a generic way to register well-known acpi device, and
well-known methods that are mostly related to hotkey.

>The Atlas board has no keys physically. It has buttons which are

No differences between buttons and keys to me.

>hooked up using the ASIM HID and that is what I am trying to support
>since those physically exist on the board. Can you help me understand
>how hotkey is involved in this?

You have dedicated ACPI device and method for brightness control.
So hotkey.c is the right place for support Device LCD.

As for Device ASIM, I don't understand this code in your patch,
Care to explain. It looks strange to me.

+static acpi_status acpi_atlas_button_handler(u32 function,
+ acpi_physical_address address,
+ u32 bit_width, acpi_integer * value,
+ void *handler_context, void *region_context)
+{
+ acpi_status status;
+ struct acpi_device *dev;
+
+ dev = (struct acpi_device *) handler_context;
+ if (function == ACPI_WRITE)
+ status = acpi_bus_generate_event(dev, 0x80, address);
+ else {
+ printk(KERN_WARNING "atlas: shrugged on unexpected
function"
+ ":function=%x,address=%x,value=%x\n",
+ function, (u32)address, (u32)*value);
+ status = -EINVAL;
+ }
+
+ return status ;
+}
+
+static int atlas_acpi_button_add(struct acpi_device *device)
+{
+
+ /* hookup button handler */
+ return acpi_install_address_space_handler(device->handle,
+ 0x81, &acpi_atlas_button_handler,
+ &acpi_atlas_button_setup, device);
+}
+

2006-03-08 07:11:49

by Jaya Kumar

[permalink] [raw]
Subject: Re: [PATCH 2.6.15.3 1/1] ACPI: Atlas ACPI driver

On 3/8/06, Yu, Luming <[email protected]> wrote:
> I think video.c is NOT a right solution for your problem.

Cool. I agree with you there.

> You have dedicated ACPI device and method for brightness control.
> So hotkey.c is the right place for support Device LCD.

I'm confused by what you are saying. The only actual input mechanism
that is physically hooked up on this Atlas board, as far as I can
tell, is ASIM which is connected to the buttons on the board. None of
the other capabilities of the actual chips are actually physically
hooked up on the actual board. There is no keyboard, no USB, etc. When
I press the physical buttons on the board, I get events such as the
following:

evregion-0303 [19] ev_address_space_dispa: No handler for Region
[ASI2] (cf7809fc) [user_defined_region]
exfldio-0284: *** Error: Region user_defined_region(81) has no handler
psparse-0508: *** Error: Method execution failed [\BNSV] (Node
cf789d48), AE_NOT_EXIST
psparse-0508: *** Error: Method execution failed [\_GPE._L15] (Node
cf789888), AE_NOT_EXIST
evgpe-0549: *** Error: AE_NOT_EXIST while evaluating method [_L15]
for GPE[ 0]
evregion-0303 [19] ev_address_space_dispa: No handler for Region
[ASI2] (cf7809fc) [user_defined_region]
exfldio-0284: *** Error: Region user_defined_region(81) has no handler
psparse-0508: *** Error: Method execution failed [\BNSV] (Node
cf789d48), AE_NOT_EXIST
psparse-0508: *** Error: Method execution failed [\_GPE._L15] (Node
cf789888), AE_NOT_EXIST
evgpe-0549: *** Error: AE_NOT_EXIST while evaluating method [_L15]
for GPE[ 0]

>
> As for Device ASIM, I don't understand this code in your patch,
> Care to explain. It looks strange to me.

Sure. As per the dmesg above, the button presses are generating
something from region 81. So my solution to that was to hookup an
address space handler for region 81 for the ASIM HID and then generate
the corresponding button events in the handler. That's the code that
you see below:

I hope what I've explained above makes sense. To reiterate, if you
want me to do something with respect to hotkey, I still don't
understand how and where hotkey is involved. Perhaps you could help me
by elaborating further.

Thanks,
jayakumar


>
> +static acpi_status acpi_atlas_button_handler(u32 function,
> + acpi_physical_address address,
> + u32 bit_width, acpi_integer * value,
> + void *handler_context, void *region_context)
> +{
> + acpi_status status;
> + struct acpi_device *dev;
> +
> + dev = (struct acpi_device *) handler_context;
> + if (function == ACPI_WRITE)
> + status = acpi_bus_generate_event(dev, 0x80, address);
> + else {
> + printk(KERN_WARNING "atlas: shrugged on unexpected
> function"
> + ":function=%x,address=%x,value=%x\n",
> + function, (u32)address, (u32)*value);
> + status = -EINVAL;
> + }
> +
> + return status ;
> +}
> +
> +static int atlas_acpi_button_add(struct acpi_device *device)
> +{
> +
> + /* hookup button handler */
> + return acpi_install_address_space_handler(device->handle,
> + 0x81, &acpi_atlas_button_handler,
> + &acpi_atlas_button_setup, device);
> +}
> +
>

2006-03-08 07:45:00

by Luming Yu

[permalink] [raw]
Subject: RE: [PATCH 2.6.15.3 1/1] ACPI: Atlas ACPI driver

>I hope what I've explained above makes sense. To reiterate, if you
>want me to do something with respect to hotkey, I still don't
>understand how and where hotkey is involved. Perhaps you could help me
>by elaborating further.

I know this user-defined region needs address space handler, but your
address space handler below is so weird that make me doubt
the correctness. The example of address space handler is:
ec.c : acpi_ec_space_handler

I suggest LCD support in hotkey.c like:
http://bugzilla.kernel.org/attachment.cgi?id=6843&action=view

Implement device ASIM in a separate driver to support user-defined
address space handler.

Config userspace acpi daemon to respond events by evoking
LCD._BCM with command:
echo -n xx > /sys/hotkey/brightness.

If you do these, then the only specific thing would be ASIM.

Thanks,
Luming

2006-03-08 08:53:15

by Jaya Kumar

[permalink] [raw]
Subject: Re: [PATCH 2.6.15.3 1/1] ACPI: Atlas ACPI driver

On 3/8/06, Yu, Luming <[email protected]> wrote:
>
> I know this user-defined region needs address space handler, but your
> address space handler below is so weird that make me doubt
> the correctness. The example of address space handler is:
> ec.c : acpi_ec_space_handler
>

As you suggested, I looked at the ec case:

845 if ((address > 0xFF) || !value || !handler_context)
846 return_VALUE(AE_BAD_PARAMETER);
847
848 if (bit_width != 8 && acpi_strict) {
849 printk(KERN_WARNING PREFIX
850 "acpi_ec_space_handler: bit_width should be 8\n");
851 return_VALUE(AE_BAD_PARAMETER);
852 }
853

I don't do any of above parameter checking in the atlas button handler
because the only parameter that is used is "address". That's what I
handoff to bus_generate_event. In my case, and unlike the ec case,
there is no embedded controller to read to get more info. To be
specific, on the board, when button 2 is pressed, I get address=1, if
button 3, I get address=2 and so on. Hence, as you can imagine, the
code in the atlas button handler below:

+ if (function == ACPI_WRITE)
+ status = acpi_bus_generate_event(dev, 0x80, address);

is a lot simpler than the ec case where they have to read stuff from
the controller as well as handle multiple bytes of reads.

856 next_byte:
857 switch (function) {
858 case ACPI_READ:
859 temp = 0;
860 result = acpi_ec_read(ec, (u8) address, (u32 *) & temp);
861 break;

So to conclude, I'm not certain in what way the atlas button handler
code is weird. If you could help elaborate on what changes you would
like to see in that code, I'd be happy to change it as per your
desires.

> I suggest LCD support in hotkey.c like:
> http://bugzilla.kernel.org/attachment.cgi?id=6843&action=view
>
>
> Config userspace acpi daemon to respond events by evoking
> LCD._BCM with command:
> echo -n xx > /sys/hotkey/brightness.

Ok, I think maybe I sort of see what you are saying and I'll take a
look at it when I have some time.

You are suggesting that I should try to get hotkey support working
because the hotkey driver may somehow evaluate _BCM methods to affect
brightness and then to have a userspace app that is triggered by the
ASIM button support thru acpi/event which then writes to the hotkey
driver through hotkey/brightness to then go and evaluate the _BCM
method to affect the LCD brightness.

I hope I've understood what you are saying.

Thanks,
jayakumar

2006-03-13 06:38:06

by Jaya Kumar

[permalink] [raw]
Subject: Re: [PATCH 2.6.15.3 1/1] ACPI: Atlas ACPI driver

On 3/8/06, Yu, Luming <[email protected]> wrote:
> I suggest LCD support in hotkey.c like:
> http://bugzilla.kernel.org/attachment.cgi?id=6843&action=view
>
> Config userspace acpi daemon to respond events by evoking
> LCD._BCM with command:
> echo -n xx > /sys/hotkey/brightness.
>

A quick question here. I took a look at your patch adding hotkeylib. I
see brightness_show/store callbacks and I think they end up calling
write_acpi_int to do the actual method eval.

So I assume my action_method has got to be "_BCM". I don't have a poll
method but it looks like I'll need to put something in there since you
check for it. Atlas has a _BCL. I guess I'll just use that.

+ if(!poll_handle || !poll_method || !action_handle || !action_method)
+ goto do_fail;

>From what I can tell, it looks like I have to use ACPILCD00 as my HID
in this hotkey code. Right? So basically, it'd be something like:
{
.ids = "ACPILCD00",
.name = "brightness",
.poll_method = "_BCL",
.action_method = "_BCM",
.min = 1,
.max = 31,
.id = 10001,
},

So if I get that working, is that what you are saying is the right way
to do brightness support for limited devices like Atlas? I guess it
feels kind of odd to me because it's an LCD device rather than a
hotkey device. But afaict it looks like doing that will work fine and
have the added benefit of not creating any new /proc entries. So let
me know if I understood you correctly.

By the way, I just applied your sequence of patches from
http://bugzilla.kernel.org/show_bug.cgi?id=5749 to my tree. You know,
if it's okay with you, I'll post the full diff from below to your bug
report so that the next person doesn't have to cherrypick.

wget "http://bugzilla.kernel.org/attachment.cgi?id=6839&action=view"
wget "http://bugzilla.kernel.org/attachment.cgi?id=6840&action=view"
wget "http://bugzilla.kernel.org/attachment.cgi?id=6841&action=view"
wget "http://bugzilla.kernel.org/attachment.cgi?id=6842&action=view"
wget "http://bugzilla.kernel.org/attachment.cgi?id=6843&action=view"
wget "http://bugzilla.kernel.org/attachment.cgi?id=7061&action=view"
wget "http://bugzilla.kernel.org/attachment.cgi?id=7060&action=view"

Thanks,
jayakumar

2006-03-13 13:16:58

by Luming Yu

[permalink] [raw]
Subject: RE: [PATCH 2.6.15.3 1/1] ACPI: Atlas ACPI driver

>On 3/8/06, Yu, Luming <[email protected]> wrote:
>> I suggest LCD support in hotkey.c like:
>> http://bugzilla.kernel.org/attachment.cgi?id=6843&action=view
>>
>> Config userspace acpi daemon to respond events by evoking
>> LCD._BCM with command:
>> echo -n xx > /sys/hotkey/brightness.
>>
>
>A quick question here. I took a look at your patch adding hotkeylib. I
>see brightness_show/store callbacks and I think they end up calling
>write_acpi_int to do the actual method eval.

Yes. But there are some restricts with write_acpi_int that only
takes single integer parameter, and write_acpi_int2 that fits aml
method of taking Package as argument.

>
>So I assume my action_method has got to be "_BCM". I don't have a poll
>method but it looks like I'll need to put something in there since you
>check for it. Atlas has a _BCL. I guess I'll just use that.

Yes. poll_method is for query the status/current value.
For example, to query current brightness value.

>
>+ if(!poll_handle || !poll_method || !action_handle ||
>!action_method)
>+ goto do_fail;
>
>From what I can tell, it looks like I have to use ACPILCD00 as my HID
>in this hotkey code. Right? So basically, it'd be something like:
> {
> .ids = "ACPILCD00",
> .name = "brightness",
> .poll_method = "_BCL",
> .action_method = "_BCM",
> .min = 1,
> .max = 31,
> .id = 10001,
> },

I think this is ok.

>
>So if I get that working, is that what you are saying is the right way
>to do brightness support for limited devices like Atlas? I guess it
>feels kind of odd to me because it's an LCD device rather than a
>hotkey device. But afaict it looks like doing that will work fine and
>have the added benefit of not creating any new /proc entries. So let
>me know if I understood you correctly.

Yes. You are right.

>
>By the way, I just applied your sequence of patches from
>http://bugzilla.kernel.org/show_bug.cgi?id=5749 to my tree. You know,
>if it's okay with you, I'll post the full diff from below to your bug
>report so that the next person doesn't have to cherrypick.

Thanks for doing that.

>
>wget "http://bugzilla.kernel.org/attachment.cgi?id=6839&action=view"
>wget "http://bugzilla.kernel.org/attachment.cgi?id=6840&action=view"
>wget "http://bugzilla.kernel.org/attachment.cgi?id=6841&action=view"
>wget "http://bugzilla.kernel.org/attachment.cgi?id=6842&action=view"
>wget "http://bugzilla.kernel.org/attachment.cgi?id=6843&action=view"
>wget "http://bugzilla.kernel.org/attachment.cgi?id=7061&action=view"
>wget "http://bugzilla.kernel.org/attachment.cgi?id=7060&action=view"
>
>Thanks,
>jayakumar