Provide /sys/../ec with read/write access and some cleanups

Provide /sys/../ec with read/write access and some cleanups

[Thomas Renninger]

These patches are diffed against the test branch of the ACPI tree, but also
patch fine with 2.6.35-rc3.

I thought about tainting the kernel if someone writes to the EC, but as
userspace can also write to graphics IO, PCI config or MSRs, it shouldn't
matter that much.
Eventually this should still be added (by a separate patch), one can easily
confuse the EC to not switch on the fans anymore.

A small tool to read out and write to /sys/devices/system/ec/*/io can be
found here:
ftp://ftp.suse.com/pub/people/trenn/sources/ec/ec_access.c

Len: Can you apply these into your test branch and schedule them for linux-next
and 2.6.36 if there are no objections, please.

Thanks,

     Thomas

[PATCH 1/6] ACPI: Provide /sys/devices/system/ec/

[Thomas Renninger]

This patch provides the same information through sysfs, which previously was
provided through /proc/acpi/embedded_controller/*/info

Signed-off-by: Thomas Renninger <trenn@suse.de>

CC: Alexey Starikovskiy <astarikovskiy@suse.de>
CC: Len Brown <lenb@kernel.org>
CC: linux-kernel@vger.kernel.org
CC: linux-acpi@vger.kernel.org
CC: Bjorn Helgaas <bjorn.helgaas@hp.com>
CC: platform-driver-x86@vger.kernel.org

Index: linux-2.6.34-master/drivers/acpi/Kconfig
===================================================================
---
 drivers/acpi/Kconfig    |   13 +++++++
 drivers/acpi/Makefile   |    1 +
 drivers/acpi/ec.c       |   18 +++-------
 drivers/acpi/ec_sys.c   |   92 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/internal.h |   24 ++++++++++++
 5 files changed, 135 insertions(+), 13 deletions(-)
 create mode 100644 drivers/acpi/ec_sys.c

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 7464115..f13708a 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -104,6 +104,19 @@ config ACPI_SYSFS_POWER
 	help
 	  Say N to disable power /sys interface
 
+config ACPI_EC_SYSFS
+	tristate "EC read/write access through sysfs"
+	default y
+	help
+	  Say N to disable Embedded Controller /sys interface
+
+	  An Embedded Controller typically is available on laptops and reads
+	  sensor values like battery state and temperature.
+	  The kernel access the EC through ACPI parsed code provided by BIOS
+	  tables.
+	  Thus this option is a debug option that helps to write ACPI drivers
+	  and which can be used to identify ACPI code or EC firmware bugs.
+
 config ACPI_PROC_EVENT
 	bool "Deprecated /proc/acpi/event support"
 	depends on PROC_FS
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 6ee3316..a33a2ef 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -60,6 +60,7 @@ obj-$(CONFIG_ACPI_SBS)		+= sbshc.o
 obj-$(CONFIG_ACPI_SBS)		+= sbs.o
 obj-$(CONFIG_ACPI_POWER_METER)	+= power_meter.o
 obj-$(CONFIG_ACPI_HED)		+= hed.o
+obj-$(CONFIG_ACPI_EC_SYSFS)     += ec_sys.o
 
 # processor has its own "processor." module_param namespace
 processor-y			:= processor_driver.o processor_throttling.o
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 1e6d418..4b6759f 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -45,10 +45,13 @@
 #include <acpi/acpi_drivers.h>
 #include <linux/dmi.h>
 
+#include "internal.h"
+
 #define ACPI_EC_CLASS			"embedded_controller"
 #define ACPI_EC_DEVICE_NAME		"Embedded Controller"
 #define ACPI_EC_FILE_INFO		"info"
 
+#undef PREFIX
 #define PREFIX				"ACPI: EC: "
 
 /* EC status register */
@@ -106,19 +109,8 @@ struct transaction {
 	bool done;
 };
 
-static struct acpi_ec {
-	acpi_handle handle;
-	unsigned long gpe;
-	unsigned long command_addr;
-	unsigned long data_addr;
-	unsigned long global_lock;
-	unsigned long flags;
-	struct mutex lock;
-	wait_queue_head_t wait;
-	struct list_head list;
-	struct transaction *curr;
-	spinlock_t curr_lock;
-} *boot_ec, *first_ec;
+struct acpi_ec *boot_ec, *first_ec;
+EXPORT_SYMBOL(first_ec);
 
 static int EC_FLAGS_MSI; /* Out-of-spec MSI controller */
 static int EC_FLAGS_VALIDATE_ECDT; /* ASUStec ECDTs need to be validated */
diff --git a/drivers/acpi/ec_sys.c b/drivers/acpi/ec_sys.c
new file mode 100644
index 0000000..c64236f
--- /dev/null
+++ b/drivers/acpi/ec_sys.c
@@ -0,0 +1,92 @@
+#include <linux/kernel.h>
+#include <linux/sysdev.h>
+#include <linux/acpi.h>
+
+#include "internal.h"
+
+MODULE_AUTHOR("Thomas Renninger <trenn@suse.de>");
+MODULE_DESCRIPTION("ACPI EC sysfs access driver");
+MODULE_LICENSE("GPL");
+
+struct sysdev_class acpi_ec_sysdev_class = {
+	.name = "ec",
+};
+
+/* sys_dev -> per EC device stuff */
+static ssize_t show_ec_gpe(struct sys_device *dev,
+			   struct sysdev_attribute *attr,
+			   char *buf)
+{
+	struct acpi_ec *ec = container_of(dev, struct acpi_ec, sysdev);
+	return sprintf(buf, "%lu\n", ec->gpe);
+}
+
+static ssize_t show_ec_global_lock(struct sys_device *dev,
+				   struct sysdev_attribute *attr,
+				   char *buf)
+{
+	struct acpi_ec *ec = container_of(dev, struct acpi_ec, sysdev);
+	return sprintf(buf, "%s\n", ec->global_lock ? "yes" : "no");
+}
+
+SYSDEV_ATTR(gpe, 0444, show_ec_gpe, NULL);
+SYSDEV_ATTR(use_global_lock, 0444, show_ec_global_lock, NULL);
+
+int acpi_ec_add_sysfs(struct acpi_ec *ec, int ec_device_count)
+{
+	int err = 0;
+
+	if (ec_device_count == 0) {
+		err = sysdev_class_register(&acpi_ec_sysdev_class);
+		if (err)
+			return err;
+	}
+
+	ec->sysdev.id = ec_device_count;
+	ec->sysdev.cls = &acpi_ec_sysdev_class;
+	err = sysdev_register(&ec->sysdev);
+	if (err)
+		goto sysdev_err;
+	err = sysdev_create_file(&ec->sysdev, &attr_use_global_lock);
+	if (err)
+		goto file_err_1;
+	err = sysdev_create_file(&ec->sysdev, &attr_gpe);
+	if (err)
+		goto file_err_2;
+	return err;
+
+ file_err_2:
+	sysdev_remove_file(&ec->sysdev, &attr_use_global_lock);
+ file_err_1:
+	sysdev_class_unregister(&acpi_ec_sysdev_class);
+ sysdev_err:
+	sysdev_class_register(&acpi_ec_sysdev_class);
+	return err;
+}
+
+void acpi_ec_remove_sysfs(struct acpi_ec *ec)
+{
+	sysdev_remove_file(&ec->sysdev, &attr_use_global_lock);
+	sysdev_remove_file(&ec->sysdev, &attr_gpe);
+
+	sysdev_unregister(&ec->sysdev);
+	sysdev_class_unregister(&acpi_ec_sysdev_class);
+}
+
+static int __init acpi_ec_sys_init(void)
+{
+	int err = 0;
+	if (first_ec)
+		err = acpi_ec_add_sysfs(first_ec, 0);
+	else
+		err = -ENODEV;
+	return err;
+}
+
+static void __exit acpi_ec_sys_exit(void)
+{
+	acpi_ec_remove_sysfs(first_ec);
+}
+
+module_init(acpi_ec_sys_init);
+module_exit(acpi_ec_sys_exit);
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index f8f190e..8ae2726 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -18,6 +18,11 @@
  * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
  */
 
+#ifndef _ACPI_INTERNAL_H_
+#define _ACPI_INTERNAL_H_
+
+#include <linux/sysdev.h>
+
 #define PREFIX "ACPI: "
 
 int init_acpi_device_notify(void);
@@ -46,6 +51,23 @@ void acpi_early_processor_set_pdc(void);
 /* --------------------------------------------------------------------------
                                   Embedded Controller
    -------------------------------------------------------------------------- */
+struct acpi_ec {
+	acpi_handle handle;
+	unsigned long gpe;
+	unsigned long command_addr;
+	unsigned long data_addr;
+	unsigned long global_lock;
+	unsigned long flags;
+	struct mutex lock;
+	wait_queue_head_t wait;
+	struct list_head list;
+	struct transaction *curr;
+	spinlock_t curr_lock;
+	struct sys_device sysdev;
+};
+
+extern struct acpi_ec *first_ec;
+
 int acpi_ec_init(void);
 int acpi_ec_ecdt_probe(void);
 int acpi_boot_ec_enable(void);
@@ -63,3 +85,5 @@ int acpi_sleep_proc_init(void);
 #else
 static inline int acpi_sleep_proc_init(void) { return 0; }
 #endif
+
+#endif /* _ACPI_INTERNAL_H_ */
-- 
1.6.3

Re: [PATCH 1/6] ACPI: Provide /sys/devices/system/ec/

[Greg KH]

On Thu, Jul 01, 2010 at 05:02:45PM +0200, Thomas Renninger wrote:
> This patch provides the same information through sysfs, which previously was
> provided through /proc/acpi/embedded_controller/*/info
> 
> Signed-off-by: Thomas Renninger <trenn@suse.de>
> 
> CC: Alexey Starikovskiy <astarikovskiy@suse.de>
> CC: Len Brown <lenb@kernel.org>
> CC: linux-kernel@vger.kernel.org
> CC: linux-acpi@vger.kernel.org
> CC: Bjorn Helgaas <bjorn.helgaas@hp.com>
> CC: platform-driver-x86@vger.kernel.org
> 
> Index: linux-2.6.34-master/drivers/acpi/Kconfig
> ===================================================================
> ---
>  drivers/acpi/Kconfig    |   13 +++++++
>  drivers/acpi/Makefile   |    1 +
>  drivers/acpi/ec.c       |   18 +++-------
>  drivers/acpi/ec_sys.c   |   92 +++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/acpi/internal.h |   24 ++++++++++++
>  5 files changed, 135 insertions(+), 13 deletions(-)
>  create mode 100644 drivers/acpi/ec_sys.c
> 
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 7464115..f13708a 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -104,6 +104,19 @@ config ACPI_SYSFS_POWER
>  	help
>  	  Say N to disable power /sys interface
>  
> +config ACPI_EC_SYSFS
> +	tristate "EC read/write access through sysfs"

As you are adding new sysfs files, please document them in
Documentation/ABI/

thanks,

greg k-h

Re: [PATCH 1/6] ACPI: Provide /sys/devices/system/ec/

[Greg KH]

On Thu, Jul 01, 2010 at 05:02:45PM +0200, Thomas Renninger wrote:
> +config ACPI_EC_SYSFS
> +	tristate "EC read/write access through sysfs"
> +	default y
> +	help
> +	  Say N to disable Embedded Controller /sys interface
> +
> +	  An Embedded Controller typically is available on laptops and reads
> +	  sensor values like battery state and temperature.
> +	  The kernel access the EC through ACPI parsed code provided by BIOS
> +	  tables.
> +	  Thus this option is a debug option that helps to write ACPI drivers
> +	  and which can be used to identify ACPI code or EC firmware bugs.

Wait, why is this a sysfs file at all?  It should be a debugfs one,
right?

thanks,

greg k-h

[PATCH 2/6] ACPI: Provide /sys/devices/system/ec/*/io for binary access to the EC

[Thomas Renninger]

Binary sysfs skeleton taken over from drivers/pci/pci-sysfs.c

/sys/devices/system/ec/*/io
A userspace app to easily read/write the EC can be found here:
ftp://ftp.suse.com/pub/people/trenn/sources/ec/ec_access.c

Signed-off-by: Thomas Renninger <trenn@suse.de>

Index: linux-2.6.34-master/drivers/acpi/ec_sys.c
===================================================================
---
 Documentation/acpi/ec_sysfs |   33 +++++++++++++++
 drivers/acpi/ec_sys.c       |   95 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 128 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/acpi/ec_sysfs

diff --git a/Documentation/acpi/ec_sysfs b/Documentation/acpi/ec_sysfs
new file mode 100644
index 0000000..479225b
--- /dev/null
+++ b/Documentation/acpi/ec_sysfs
@@ -0,0 +1,33 @@
+		EC Sysfs File Description
+
+This file descibes sysfs files under:
+/sys/devices/system/ec
+
+Currently only one Embedded Controller is supported.
+This could get extended easily if such an ACPI supporting
+system exist, but until know there has no need be seen.
+
+/sys/devices/system/ec/ec0/gpe
+An integer value showing the General Purpose Event the EC
+serves and has its handler installed on.
+A GPE is an ACPI interrupt (compare with /proc/interrupts)
+with a specific value retrieved from a HW register associated
+to it. If the value equals the EC GPE, this event is processed
+by the EC.
+The activity of the EC GPE can be monitored like that:
+GPE=`cat /sys/devices/system/ec/ec0/gpe`
+watch -n1 cat /sys/firmware/acpi/interrupts/gpe${GPE}
+
+/sys/devices/system/ec/ec0/use_global_lock
+Shows whether BIOS requested the OS to use the global ACPI
+lock on EC transitions (compare with ACPI specification for
+details).
+
+/sys/devices/system/ec/ec0/io
+Provides access to the 255 EC registers.
+Its input and output is binary.
+A userspace tool to easily read/write the EC can be found here:
+ftp://ftp.suse.com/pub/people/trenn/sources/ec/ec_access.c
+This file is for debugging purposes only.
+!!! THE EC MUST ONLY GET ACCESSED THROUGH THE KERNEL, INTERPRETING
+ACPI CODE ON PRODUCTIVE SYSTEMS !!!
diff --git a/drivers/acpi/ec_sys.c b/drivers/acpi/ec_sys.c
index c64236f..9793c57 100644
--- a/drivers/acpi/ec_sys.c
+++ b/drivers/acpi/ec_sys.c
@@ -1,3 +1,13 @@
+/*
+ * ec_sys.c
+ *
+ * Copyright (C) 2010 SUSE Products GmbH/Novell
+ * Author:
+ *      Thomas Renninger <trenn@suse.de>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.
+ */
+
 #include <linux/kernel.h>
 #include <linux/sysdev.h>
 #include <linux/acpi.h>
@@ -8,6 +18,8 @@ MODULE_AUTHOR("Thomas Renninger <trenn@suse.de>");
 MODULE_DESCRIPTION("ACPI EC sysfs access driver");
 MODULE_LICENSE("GPL");
 
+#define EC_SPACE_SIZE 256
+
 struct sysdev_class acpi_ec_sysdev_class = {
 	.name = "ec",
 };
@@ -32,6 +44,84 @@ static ssize_t show_ec_global_lock(struct sys_device *dev,
 SYSDEV_ATTR(gpe, 0444, show_ec_gpe, NULL);
 SYSDEV_ATTR(use_global_lock, 0444, show_ec_global_lock, NULL);
 
+
+/* TBD: ec_read/ec_write is only supported for one EC in a system
+	This needs adjustion in ec.c first
+*/
+static ssize_t
+acpi_ec_read_io(struct file *filp, struct kobject *kobj,
+		struct bin_attribute *bin_attr,
+		char *buf, loff_t off, size_t count)
+{
+	/* Use this if support reading/writing multiple ECs exists in ec.c:
+	   struct sys_device *sysdev = container_of(kobj, struct sys_device, kobj);
+	   struct acpi_ec *ec = container_of(sysdev, struct acpi_ec, sysdev);
+	*/
+	unsigned int size = EC_SPACE_SIZE;
+	u8 *data = (u8 *) buf;
+	loff_t init_off = off;
+	int err = 0;
+
+	if (off >= size)
+		return 0;
+	if (off + count >= size) {
+		size -= off;
+		count = size;
+	} else
+		size = count;
+
+	while (size) {
+		err = ec_read(off, &data[off - init_off]);
+		if (err)
+			return err;
+		off++;
+		size--;
+	}
+	return count;
+}
+
+static ssize_t
+acpi_ec_write_io(struct file *filp, struct kobject *kobj,
+		 struct bin_attribute *bin_attr,
+		 char *buf, loff_t off, size_t count)
+{
+	/* Use this if support reading/writing multiple ECs exists in ec.c:
+	   struct sys_device *sysdev = container_of(kobj, struct sys_device, kobj);
+	   struct acpi_ec *ec = container_of(sysdev, struct acpi_ec, sysdev);
+	*/
+	unsigned int size = count;
+	loff_t init_off = off;
+	u8 *data = (u8 *) buf;
+	int err = 0;
+
+	if (off >= EC_SPACE_SIZE)
+		return 0;
+	if (off + count >= EC_SPACE_SIZE) {
+		size = EC_SPACE_SIZE - off;
+		count = size;
+	}
+
+	while (size) {
+		u8 byte_write = data[off - init_off];
+		err = ec_write(off, byte_write);
+		if (err)
+			return err;
+		off++;
+		size--;
+	}
+	return count;
+}
+
+static struct bin_attribute acpi_ec_io_attr = {
+	.attr =	{
+		.name = "io",
+		.mode = S_IRUGO | S_IWUSR,
+	},
+	.size = EC_SPACE_SIZE,
+	.read = acpi_ec_read_io,
+	.write = acpi_ec_write_io,
+};
+
 int acpi_ec_add_sysfs(struct acpi_ec *ec, int ec_device_count)
 {
 	int err = 0;
@@ -53,8 +143,13 @@ int acpi_ec_add_sysfs(struct acpi_ec *ec, int ec_device_count)
 	err = sysdev_create_file(&ec->sysdev, &attr_gpe);
 	if (err)
 		goto file_err_2;
+	err = sysfs_create_bin_file(&ec->sysdev.kobj, &acpi_ec_io_attr);
+	if (err)
+		goto file_err_3;
 	return err;
 
+ file_err_3:
+	sysdev_remove_file(&ec->sysdev, &attr_gpe);
  file_err_2:
 	sysdev_remove_file(&ec->sysdev, &attr_use_global_lock);
  file_err_1:
-- 
1.6.3

[PATCH 3/6] ACPI: Register EC io ports in /proc/ioports

[Thomas Renninger]

Formerly these have been exposed through /proc/..
Better register them where all IO ports should get registered
and scream loud if someone else claims to use them.

EC data and command port typically should show up like this
then:
...
  0060-0060 : keyboard
  0062-0062 : EC data
  0064-0064 : keyboard
  0066-0066 : EC command
  0070-0071 : rtc0
...

Signed-off-by: Thomas Renninger <trenn@suse.de>

CC: Alexey Starikovskiy <astarikovskiy@suse.de>
CC: Len Brown <lenb@kernel.org>
CC: linux-kernel@vger.kernel.org
CC: linux-acpi@vger.kernel.org
CC: Bjorn Helgaas <bjorn.helgaas@hp.com>
CC: platform-driver-x86@vger.kernel.org

Index: linux-2.6.34-master/drivers/acpi/ec.c
===================================================================
---
 drivers/acpi/ec.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 4b6759f..f95fa9f 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -927,10 +927,18 @@ ec_parse_io_ports(struct acpi_resource *resource, void *context)
 	 * the second address region returned is the status/command
 	 * port.
 	 */
-	if (ec->data_addr == 0)
+	if (ec->data_addr == 0) {
 		ec->data_addr = resource->data.io.minimum;
-	else if (ec->command_addr == 0)
+		WARN(!request_region(ec->data_addr, 1, "EC data"),
+		     "Could not request EC data io port %lu",
+		     ec->data_addr);
+	}
+	else if (ec->command_addr == 0) {
 		ec->command_addr = resource->data.io.minimum;
+		WARN(!request_region(ec->command_addr, 1, "EC command"),
+		     "Could not request EC command io port %lu",
+		     ec->command_addr);
+	}
 	else
 		return AE_CTRL_TERMINATE;
 
-- 
1.6.3

[PATCH 4/6] ACPI: Remove /proc/acpi/embedded_controller/..

[Thomas Renninger]

Other patches in this series add the same info to /sys/... and
/proc/ioports.

The info removed should never have been used in an application,
eventually someone read it manually.
/proc/acpi is deprecated for more than a year anyway...

Signed-off-by: Thomas Renninger <trenn@suse.de>

CC: Alexey Starikovskiy <astarikovskiy@suse.de>
CC: Len Brown <lenb@kernel.org>
CC: linux-kernel@vger.kernel.org
CC: linux-acpi@vger.kernel.org
CC: platform-driver-x86@vger.kernel.org


Index: linux-2.6.34-master/drivers/acpi/ec.c
===================================================================
---
 drivers/acpi/ec.c |   81 +----------------------------------------------------
 1 files changed, 1 insertions(+), 80 deletions(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index f95fa9f..13d3011 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -34,8 +34,6 @@
 #include <linux/init.h>
 #include <linux/types.h>
 #include <linux/delay.h>
-#include <linux/proc_fs.h>
-#include <linux/seq_file.h>
 #include <linux/interrupt.h>
 #include <linux/list.h>
 #include <linux/spinlock.h>
@@ -664,72 +662,6 @@ acpi_ec_space_handler(u32 function, acpi_physical_address address,
 }
 
 /* --------------------------------------------------------------------------
-                              FS Interface (/proc)
-   -------------------------------------------------------------------------- */
-
-static struct proc_dir_entry *acpi_ec_dir;
-
-static int acpi_ec_read_info(struct seq_file *seq, void *offset)
-{
-	struct acpi_ec *ec = seq->private;
-
-	if (!ec)
-		goto end;
-
-	seq_printf(seq, "gpe:\t\t\t0x%02x\n", (u32) ec->gpe);
-	seq_printf(seq, "ports:\t\t\t0x%02x, 0x%02x\n",
-		   (unsigned)ec->command_addr, (unsigned)ec->data_addr);
-	seq_printf(seq, "use global lock:\t%s\n",
-		   ec->global_lock ? "yes" : "no");
-      end:
-	return 0;
-}
-
-static int acpi_ec_info_open_fs(struct inode *inode, struct file *file)
-{
-	return single_open(file, acpi_ec_read_info, PDE(inode)->data);
-}
-
-static const struct file_operations acpi_ec_info_ops = {
-	.open = acpi_ec_info_open_fs,
-	.read = seq_read,
-	.llseek = seq_lseek,
-	.release = single_release,
-	.owner = THIS_MODULE,
-};
-
-static int acpi_ec_add_fs(struct acpi_device *device)
-{
-	struct proc_dir_entry *entry = NULL;
-
-	if (!acpi_device_dir(device)) {
-		acpi_device_dir(device) = proc_mkdir(acpi_device_bid(device),
-						     acpi_ec_dir);
-		if (!acpi_device_dir(device))
-			return -ENODEV;
-	}
-
-	entry = proc_create_data(ACPI_EC_FILE_INFO, S_IRUGO,
-				 acpi_device_dir(device),
-				 &acpi_ec_info_ops, acpi_driver_data(device));
-	if (!entry)
-		return -ENODEV;
-	return 0;
-}
-
-static int acpi_ec_remove_fs(struct acpi_device *device)
-{
-
-	if (acpi_device_dir(device)) {
-		remove_proc_entry(ACPI_EC_FILE_INFO, acpi_device_dir(device));
-		remove_proc_entry(acpi_device_bid(device), acpi_ec_dir);
-		acpi_device_dir(device) = NULL;
-	}
-
-	return 0;
-}
-
-/* --------------------------------------------------------------------------
                                Driver Interface
    -------------------------------------------------------------------------- */
 static acpi_status
@@ -879,7 +811,6 @@ static int acpi_ec_add(struct acpi_device *device)
 	if (!first_ec)
 		first_ec = ec;
 	device->driver_data = ec;
-	acpi_ec_add_fs(device);
 	pr_info(PREFIX "GPE = 0x%lx, I/O: command/status = 0x%lx, data = 0x%lx\n",
 			  ec->gpe, ec->command_addr, ec->data_addr);
 
@@ -906,7 +837,6 @@ static int acpi_ec_remove(struct acpi_device *device, int type)
 		kfree(handler);
 	}
 	mutex_unlock(&ec->lock);
-	acpi_ec_remove_fs(device);
 	device->driver_data = NULL;
 	if (ec == first_ec)
 		first_ec = NULL;
@@ -1095,16 +1025,10 @@ int __init acpi_ec_init(void)
 {
 	int result = 0;
 
-	acpi_ec_dir = proc_mkdir(ACPI_EC_CLASS, acpi_root_dir);
-	if (!acpi_ec_dir)
-		return -ENODEV;
-
 	/* Now register the driver for the EC */
 	result = acpi_bus_register_driver(&acpi_ec_driver);
-	if (result < 0) {
-		remove_proc_entry(ACPI_EC_CLASS, acpi_root_dir);
+	if (result < 0)
 		return -ENODEV;
-	}
 
 	return result;
 }
@@ -1115,9 +1039,6 @@ static void __exit acpi_ec_exit(void)
 {
 
 	acpi_bus_unregister_driver(&acpi_ec_driver);
-
-	remove_proc_entry(ACPI_EC_CLASS, acpi_root_dir);
-
 	return;
 }
 #endif	/* 0 */
-- 
1.6.3

[PATCH 5/6] X86 platform drivers: Remove EC dump from thinkpad_acpi

[Thomas Renninger]

There is a general interface for that now (provided by
other patches in this patch series):
/sys/devices/system/ec/*/io

Signed-off-by: Thomas Renninger <trenn-l3A5Bk7waGM@public.gmane.org>

CC: Alexey Starikovskiy <astarikovskiy-l3A5Bk7waGM@public.gmane.org>
CC: Len Brown <lenb-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
CC: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
CC: linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
CC: platform-driver-x86-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
CC: Henrique de Moraes Holschuh <hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org>
CC: ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org


Index: linux-2.6.34-master/drivers/platform/x86/thinkpad_acpi.c
===================================================================
---
 Documentation/laptops/thinkpad-acpi.txt |   75 -------------------------------
 drivers/platform/x86/thinkpad_acpi.c    |   73 ------------------------------
 2 files changed, 0 insertions(+), 148 deletions(-)

diff --git a/Documentation/laptops/thinkpad-acpi.txt b/Documentation/laptops/thinkpad-acpi.txt
index fc15538..85b1039 100644
--- a/Documentation/laptops/thinkpad-acpi.txt
+++ b/Documentation/laptops/thinkpad-acpi.txt
@@ -960,81 +960,6 @@ Sysfs notes:
 	subsystem, and follow all of the hwmon guidelines at
 	Documentation/hwmon.
 
-
-EXPERIMENTAL: Embedded controller register dump -- /proc/acpi/ibm/ecdump
-------------------------------------------------------------------------
-
-This feature is marked EXPERIMENTAL because the implementation
-directly accesses hardware registers and may not work as expected. USE
-WITH CAUTION! To use this feature, you need to supply the
-experimental=1 parameter when loading the module.
-
-This feature dumps the values of 256 embedded controller
-registers. Values which have changed since the last time the registers
-were dumped are marked with a star:
-
-[root@x40 ibm-acpi]# cat /proc/acpi/ibm/ecdump
-EC       +00 +01 +02 +03 +04 +05 +06 +07 +08 +09 +0a +0b +0c +0d +0e +0f
-EC 0x00:  a7  47  87  01  fe  96  00  08  01  00  cb  00  00  00  40  00
-EC 0x10:  00  00  ff  ff  f4  3c  87  09  01  ff  42  01  ff  ff  0d  00
-EC 0x20:  00  00  00  00  00  00  00  00  00  00  00  03  43  00  00  80
-EC 0x30:  01  07  1a  00  30  04  00  00 *85  00  00  10  00  50  00  00
-EC 0x40:  00  00  00  00  00  00  14  01  00  04  00  00  00  00  00  00
-EC 0x50:  00  c0  02  0d  00  01  01  02  02  03  03  03  03 *bc *02 *bc
-EC 0x60: *02 *bc *02  00  00  00  00  00  00  00  00  00  00  00  00  00
-EC 0x70:  00  00  00  00  00  12  30  40 *24 *26 *2c *27 *20  80 *1f  80
-EC 0x80:  00  00  00  06 *37 *0e  03  00  00  00  0e  07  00  00  00  00
-EC 0x90:  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00
-EC 0xa0: *ff  09  ff  09  ff  ff *64  00 *00 *00 *a2  41 *ff *ff *e0  00
-EC 0xb0:  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00
-EC 0xc0:  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00
-EC 0xd0:  03  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00
-EC 0xe0:  00  00  00  00  00  00  00  00  11  20  49  04  24  06  55  03
-EC 0xf0:  31  55  48  54  35  38  57  57  08  2f  45  73  07  65  6c  1a
-
-This feature can be used to determine the register holding the fan
-speed on some models. To do that, do the following:
-
-	- make sure the battery is fully charged
-	- make sure the fan is running
-	- run 'cat /proc/acpi/ibm/ecdump' several times, once per second or so
-
-The first step makes sure various charging-related values don't
-vary. The second ensures that the fan-related values do vary, since
-the fan speed fluctuates a bit. The third will (hopefully) mark the
-fan register with a star:
-
-[root@x40 ibm-acpi]# cat /proc/acpi/ibm/ecdump
-EC       +00 +01 +02 +03 +04 +05 +06 +07 +08 +09 +0a +0b +0c +0d +0e +0f
-EC 0x00:  a7  47  87  01  fe  96  00  08  01  00  cb  00  00  00  40  00
-EC 0x10:  00  00  ff  ff  f4  3c  87  09  01  ff  42  01  ff  ff  0d  00
-EC 0x20:  00  00  00  00  00  00  00  00  00  00  00  03  43  00  00  80
-EC 0x30:  01  07  1a  00  30  04  00  00  85  00  00  10  00  50  00  00
-EC 0x40:  00  00  00  00  00  00  14  01  00  04  00  00  00  00  00  00
-EC 0x50:  00  c0  02  0d  00  01  01  02  02  03  03  03  03  bc  02  bc
-EC 0x60:  02  bc  02  00  00  00  00  00  00  00  00  00  00  00  00  00
-EC 0x70:  00  00  00  00  00  12  30  40  24  27  2c  27  21  80  1f  80
-EC 0x80:  00  00  00  06 *be  0d  03  00  00  00  0e  07  00  00  00  00
-EC 0x90:  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00
-EC 0xa0:  ff  09  ff  09  ff  ff  64  00  00  00  a2  41  ff  ff  e0  00
-EC 0xb0:  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00
-EC 0xc0:  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00
-EC 0xd0:  03  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00
-EC 0xe0:  00  00  00  00  00  00  00  00  11  20  49  04  24  06  55  03
-EC 0xf0:  31  55  48  54  35  38  57  57  08  2f  45  73  07  65  6c  1a
-
-Another set of values that varies often is the temperature
-readings. Since temperatures don't change vary fast, you can take
-several quick dumps to eliminate them.
-
-You can use a similar method to figure out the meaning of other
-embedded controller registers - e.g. make sure nothing else changes
-except the charging or discharging battery to determine which
-registers contain the current battery capacity, etc. If you experiment
-with this, do send me your results (including some complete dumps with
-a description of the conditions when they were taken.)
-
-
 LCD brightness control
 ----------------------
 
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 4bdb137..5d6119b 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -5838,75 +5838,6 @@ static struct ibm_struct thermal_driver_data = {
 };
 
 /*************************************************************************
- * EC Dump subdriver
- */
-
-static u8 ecdump_regs[256];
-
-static int ecdump_read(struct seq_file *m)
-{
-	int i, j;
-	u8 v;
-
-	seq_printf(m, "EC      "
-		       " +00 +01 +02 +03 +04 +05 +06 +07"
-		       " +08 +09 +0a +0b +0c +0d +0e +0f\n");
-	for (i = 0; i < 256; i += 16) {
-		seq_printf(m, "EC 0x%02x:", i);
-		for (j = 0; j < 16; j++) {
-			if (!acpi_ec_read(i + j, &v))
-				break;
-			if (v != ecdump_regs[i + j])
-				seq_printf(m, " *%02x", v);
-			else
-				seq_printf(m, "  %02x", v);
-			ecdump_regs[i + j] = v;
-		}
-		seq_putc(m, '\n');
-		if (j != 16)
-			break;
-	}
-
-	/* These are way too dangerous to advertise openly... */
-#if 0
-	seq_printf(m, "commands:\t0x<offset> 0x<value>"
-		       " (<offset> is 00-ff, <value> is 00-ff)\n");
-	seq_printf(m, "commands:\t0x<offset> <value>  "
-		       " (<offset> is 00-ff, <value> is 0-255)\n");
-#endif
-	return 0;
-}
-
-static int ecdump_write(char *buf)
-{
-	char *cmd;
-	int i, v;
-
-	while ((cmd = next_cmd(&buf))) {
-		if (sscanf(cmd, "0x%x 0x%x", &i, &v) == 2) {
-			/* i and v set */
-		} else if (sscanf(cmd, "0x%x %u", &i, &v) == 2) {
-			/* i and v set */
-		} else
-			return -EINVAL;
-		if (i >= 0 && i < 256 && v >= 0 && v < 256) {
-			if (!acpi_ec_write(i, v))
-				return -EIO;
-		} else
-			return -EINVAL;
-	}
-
-	return 0;
-}
-
-static struct ibm_struct ecdump_driver_data = {
-	.name = "ecdump",
-	.read = ecdump_read,
-	.write = ecdump_write,
-	.flags.experimental = 1,
-};
-
-/*************************************************************************
  * Backlight/brightness subdriver
  */
 
@@ -8883,9 +8814,6 @@ static struct ibm_init_struct ibms_init[] __initdata = {
 		.data = &thermal_driver_data,
 	},
 	{
-		.data = &ecdump_driver_data,
-	},
-	{
 		.init = brightness_init,
 		.data = &brightness_driver_data,
 	},
@@ -8993,7 +8921,6 @@ TPACPI_PARAM(light);
 TPACPI_PARAM(cmos);
 TPACPI_PARAM(led);
 TPACPI_PARAM(beep);
-TPACPI_PARAM(ecdump);
 TPACPI_PARAM(brightness);
 TPACPI_PARAM(volume);
 TPACPI_PARAM(fan);
-- 
1.6.3


------------------------------------------------------------------------------
This SF.net email is sponsored by Sprint
What will you do first with EVO, the first 4G phone?
Visit sprint.com/first -- http://p.sf.net/sfu/sprint-com-first

Re: [PATCH 5/6] X86 platform drivers: Remove EC dump from thinkpad_acpi

[Henrique de Moraes Holschuh]

On Thu, 01 Jul 2010, Thomas Renninger wrote:
> There is a general interface for that now (provided by
> other patches in this patch series):
> /sys/devices/system/ec/*/io
> 
> Signed-off-by: Thomas Renninger <trenn@suse.de>
> 
> CC: Alexey Starikovskiy <astarikovskiy@suse.de>
> CC: Len Brown <lenb@kernel.org>
> CC: linux-kernel@vger.kernel.org
> CC: linux-acpi@vger.kernel.org
> CC: platform-driver-x86@vger.kernel.org
> CC: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
> CC: ibm-acpi-devel@lists.sourceforge.net

No outright objections, but...

> -EXPERIMENTAL: Embedded controller register dump -- /proc/acpi/ibm/ecdump
> -------------------------------------------------------------------------

[...]

> -registers. Values which have changed since the last time the registers
> -were dumped are marked with a star:

[...]

> -This feature can be used to determine the register holding the fan
> -speed on some models. To do that, do the following:
> -
> -	- make sure the battery is fully charged
> -	- make sure the fan is running
> -	- run 'cat /proc/acpi/ibm/ecdump' several times, once per second or so
> -
> -The first step makes sure various charging-related values don't
> -vary. The second ensures that the fan-related values do vary, since
> -the fan speed fluctuates a bit. The third will (hopefully) mark the
> -fan register with a star:
> -
> -[root@x40 ibm-acpi]# cat /proc/acpi/ibm/ecdump
> -EC       +00 +01 +02 +03 +04 +05 +06 +07 +08 +09 +0a +0b +0c +0d +0e +0f
> -EC 0x00:  a7  47  87  01  fe  96  00  08  01  00  cb  00  00  00  40  00
> -EC 0x10:  00  00  ff  ff  f4  3c  87  09  01  ff  42  01  ff  ff  0d  00
> -EC 0x20:  00  00  00  00  00  00  00  00  00  00  00  03  43  00  00  80
> -EC 0x30:  01  07  1a  00  30  04  00  00  85  00  00  10  00  50  00  00
> -EC 0x40:  00  00  00  00  00  00  14  01  00  04  00  00  00  00  00  00
> -EC 0x50:  00  c0  02  0d  00  01  01  02  02  03  03  03  03  bc  02  bc
> -EC 0x60:  02  bc  02  00  00  00  00  00  00  00  00  00  00  00  00  00
> -EC 0x70:  00  00  00  00  00  12  30  40  24  27  2c  27  21  80  1f  80
> -EC 0x80:  00  00  00  06 *be  0d  03  00  00  00  0e  07  00  00  00  00
> -EC 0x90:  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00
> -EC 0xa0:  ff  09  ff  09  ff  ff  64  00  00  00  a2  41  ff  ff  e0  00
> -EC 0xb0:  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00
> -EC 0xc0:  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00
> -EC 0xd0:  03  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00
> -EC 0xe0:  00  00  00  00  00  00  00  00  11  20  49  04  24  06  55  03
> -EC 0xf0:  31  55  48  54  35  38  57  57  08  2f  45  73  07  65  6c  1a
> -
> -Another set of values that varies often is the temperature
> -readings. Since temperatures don't change vary fast, you can take
> -several quick dumps to eliminate them.
> -
> -You can use a similar method to figure out the meaning of other
> -embedded controller registers - e.g. make sure nothing else changes
> -except the charging or discharging battery to determine which
> -registers contain the current battery capacity, etc. If you experiment
> -with this, do send me your results (including some complete dumps with
> -a description of the conditions when they were taken.)

I'd prefer if the above text gets replaced by something that adds a pointer
to the new facility, instead of being just outright removed.  Also, the text
about how to use the facility to detect tachometers and thermometers should
be retained, if at all possible.

Soes your helper userspace util do the "differential" analysis that
thinkpad-acpi used to (the "*" after values that changed since last read)?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH 5/6] X86 platform drivers: Remove EC dump from thinkpad_acpi

[Thomas Renninger]

On Thursday 01 July 2010 06:27:41 pm Henrique de Moraes Holschuh wrote:
> On Thu, 01 Jul 2010, Thomas Renninger wrote:
...
> I'd prefer if the above text gets replaced by something that adds a pointer
> to the new facility, instead of being just outright removed.  Also, the
> text about how to use the facility to detect tachometers and thermometers
> should be retained, if at all possible.
I agree. I'll repost this one in some days.

> Soes your helper userspace util do the "differential" analysis that
> thinkpad-acpi used to (the "*" after values that changed since last read)?
No.
But this should not be important enough to not get this removed.
It's hard to re-implement the exact behaviour (which can be handy for
specific problems, I agree).
With:
watch -n1 ec_access -r
you can have a close look at specific registers and see them changing.
Whatabout:
ec_access -r -s time(s)
Reads the table once and again after "time" seconds with stars
where modifications happened. Then someone can plug AC or hit a button
and look at the diff in EC registers afterwards.
This should be sufficient?
Better ideas?

Thanks,

       Thomas

Re: [PATCH 5/6] X86 platform drivers: Remove EC dump from thinkpad_acpi

[Henrique de Moraes Holschuh]

On Thu, 01 Jul 2010, Thomas Renninger wrote:
> > Soes your helper userspace util do the "differential" analysis that
> > thinkpad-acpi used to (the "*" after values that changed since last read)?
> No.
> But this should not be important enough to not get this removed.

Consider it a feature request ;-)

> It's hard to re-implement the exact behaviour (which can be handy for
> specific problems, I agree).
> With:
> watch -n1 ec_access -r
> you can have a close look at specific registers and see them changing.
> Whatabout:
> ec_access -r -s time(s)
> Reads the table once and again after "time" seconds with stars
> where modifications happened. Then someone can plug AC or hit a button
> and look at the diff in EC registers afterwards.
> This should be sufficient?

Yes.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH 5/6] X86 platform drivers: Remove EC dump from thinkpad_acpi

[Thomas Renninger]

On Thursday 01 July 2010 21:31:18 Thomas Renninger wrote:
> On Thursday 01 July 2010 06:27:41 pm Henrique de Moraes Holschuh wrote:
> > On Thu, 01 Jul 2010, Thomas Renninger wrote:
> ...
> > I'd prefer if the above text gets replaced by something that adds a pointer
> > to the new facility, instead of being just outright removed.  Also, the
> > text about how to use the facility to detect tachometers and thermometers
> > should be retained, if at all possible.
> I agree. I'll repost this one in some days.
Hmm, I take that back.
This still should get removed completely from:
Documentation/laptops/thinkpad-acpi.txt

These problems apply to all laptops and instead of adding pointers to all
Documentation/laptop/${vendor} files,
a separate file should be added. This should then be more about thermal
management problems in general and pointers to:
Documentation/acpi/ec_sysfs
and
Documentation/acpi/debug.txt
Documentation/acpi/*
added there looks like the appropriate solution to me.

If people agree:
  - This patch is still fine and should get applied as it is
  - I will send another patch adding some more documentation
    in some days

Thanks,

    Thomas

Re: [PATCH 5/6] X86 platform drivers: Remove EC dump from thinkpad_acpi

[Henrique de Moraes Holschuh]

On Fri, 02 Jul 2010, Thomas Renninger wrote:
> On Thursday 01 July 2010 21:31:18 Thomas Renninger wrote:
> > On Thursday 01 July 2010 06:27:41 pm Henrique de Moraes Holschuh wrote:
> > > On Thu, 01 Jul 2010, Thomas Renninger wrote:
> > ...
> > > I'd prefer if the above text gets replaced by something that adds a pointer
> > > to the new facility, instead of being just outright removed.  Also, the
> > > text about how to use the facility to detect tachometers and thermometers
> > > should be retained, if at all possible.
> > I agree. I'll repost this one in some days.
> Hmm, I take that back.
> This still should get removed completely from:
> Documentation/laptops/thinkpad-acpi.txt
> 
> These problems apply to all laptops and instead of adding pointers to all
> Documentation/laptop/${vendor} files,
> a separate file should be added. This should then be more about thermal
> management problems in general and pointers to:
> Documentation/acpi/ec_sysfs
> and
> Documentation/acpi/debug.txt
> Documentation/acpi/*
> added there looks like the appropriate solution to me.

And a pointer to the new docs added to thinkpad-acpi.txt in place of the
text you're removing, please.

> If people agree:
>   - This patch is still fine and should get applied as it is
>   - I will send another patch adding some more documentation
>     in some days

I'm ok with this, as long as you add a pointer in thinkpad-acpi.txt to the
new docs in the future documentation patch.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

[PATCH 6/6] X86 platform driver: Fix section mismatch in wmi.c

[Thomas Renninger]

The .add function must not be declared __init.

Signed-off-by: Thomas Renninger <trenn@suse.de>

CC: Alexey Starikovskiy <astarikovskiy@suse.de>
CC: Len Brown <lenb@kernel.org>
CC: linux-kernel@vger.kernel.org
CC: linux-acpi@vger.kernel.org
CC: platform-driver-x86@vger.kernel.org


Index: linux-2.6.34-master/drivers/platform/x86/wmi.c
===================================================================
---
 drivers/platform/x86/wmi.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index e4eaa14..635ebb4 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -804,7 +804,7 @@ static bool guid_already_parsed(const char *guid_string)
 /*
  * Parse the _WDG method for the GUID data blocks
  */
-static __init acpi_status parse_wdg(acpi_handle handle)
+static acpi_status parse_wdg(acpi_handle handle)
 {
 	struct acpi_buffer out = {ACPI_ALLOCATE_BUFFER, NULL};
 	union acpi_object *obj;
@@ -947,7 +947,7 @@ static int acpi_wmi_remove(struct acpi_device *device, int type)
 	return 0;
 }
 
-static int __init acpi_wmi_add(struct acpi_device *device)
+static int acpi_wmi_add(struct acpi_device *device)
 {
 	acpi_status status;
 	int result = 0;
-- 
1.6.3

Re: Provide /sys/../ec with read/write access and some cleanups

[Henrique de Moraes Holschuh]

On Thu, 01 Jul 2010, Thomas Renninger wrote:
> These patches are diffed against the test branch of the ACPI tree, but also
> patch fine with 2.6.35-rc3.
> 
> I thought about tainting the kernel if someone writes to the EC, but as
> userspace can also write to graphics IO, PCI config or MSRs, it shouldn't
> matter that much.
> Eventually this should still be added (by a separate patch), one can easily
> confuse the EC to not switch on the fans anymore.
> 
> A small tool to read out and write to /sys/devices/system/ec/*/io can be
> found here:
> ftp://ftp.suse.com/pub/people/trenn/sources/ec/ec_access.c
> 
> Len: Can you apply these into your test branch and schedule them for linux-next
> and 2.6.36 if there are no objections, please.

I am just wondering if we shouldn't have this in debugfs instead of regular
/sys.  Do you envision *production* use of this facility, or should it just
be something to use for debugging and hacking?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: Provide /sys/../ec with read/write access and some cleanups

[Thomas Renninger]

On Thursday 01 July 2010 06:22:39 pm Henrique de Moraes Holschuh wrote:
> On Thu, 01 Jul 2010, Thomas Renninger wrote:
> > These patches are diffed against the test branch of the ACPI tree, but
> > also patch fine with 2.6.35-rc3.
> >
> > I thought about tainting the kernel if someone writes to the EC, but as
> > userspace can also write to graphics IO, PCI config or MSRs, it shouldn't
> > matter that much.
> > Eventually this should still be added (by a separate patch), one can
> > easily confuse the EC to not switch on the fans anymore.
> >
> > A small tool to read out and write to /sys/devices/system/ec/*/io can be
> > found here:
> > ftp://ftp.suse.com/pub/people/trenn/sources/ec/ec_access.c
> >
> > Len: Can you apply these into your test branch and schedule them for
> > linux-next and 2.6.36 if there are no objections, please.
>
> I am just wondering if we shouldn't have this in debugfs instead of regular
> /sys.  Do you envision *production* use of this facility, or should it just
> be something to use for debugging and hacking?
Only for debugging and hacking. Apps must not use it for production.
On the one hand I agree, on the other hand side  I think the EC somehow
fits into /sys/devices/system/ec. I don't have a strong opion on that, though.

      Thomas

Re: Provide /sys/../ec with read/write access and some cleanups

[Henrique de Moraes Holschuh]

On Thu, 01 Jul 2010, Thomas Renninger wrote:
> On Thursday 01 July 2010 06:22:39 pm Henrique de Moraes Holschuh wrote:
> > On Thu, 01 Jul 2010, Thomas Renninger wrote:
> > > These patches are diffed against the test branch of the ACPI tree, but
> > > also patch fine with 2.6.35-rc3.
> > >
> > > I thought about tainting the kernel if someone writes to the EC, but as
> > > userspace can also write to graphics IO, PCI config or MSRs, it shouldn't
> > > matter that much.
> > > Eventually this should still be added (by a separate patch), one can
> > > easily confuse the EC to not switch on the fans anymore.
> > >
> > > A small tool to read out and write to /sys/devices/system/ec/*/io can be
> > > found here:
> > > ftp://ftp.suse.com/pub/people/trenn/sources/ec/ec_access.c
> > >
> > > Len: Can you apply these into your test branch and schedule them for
> > > linux-next and 2.6.36 if there are no objections, please.
> >
> > I am just wondering if we shouldn't have this in debugfs instead of regular
> > /sys.  Do you envision *production* use of this facility, or should it just
> > be something to use for debugging and hacking?
> Only for debugging and hacking. Apps must not use it for production.
> On the one hand I agree, on the other hand side  I think the EC somehow
> fits into /sys/devices/system/ec. I don't have a strong opion on that, though.

You can have the ec metadata in sys, and the EC direct access in
debugfs, for example...

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: Provide /sys/../ec with read/write access and some cleanups

[Maxim Levitsky]

On Thu, 2010-07-01 at 17:02 +0200, Thomas Renninger wrote:
> These patches are diffed against the test branch of the ACPI tree, but also
> patch fine with 2.6.35-rc3.
> 
> I thought about tainting the kernel if someone writes to the EC, but as
> userspace can also write to graphics IO, PCI config or MSRs, it shouldn't
> matter that much.
> Eventually this should still be added (by a separate patch), one can easily
> confuse the EC to not switch on the fans anymore.
> 
> A small tool to read out and write to /sys/devices/system/ec/*/io can be
> found here:
> ftp://ftp.suse.com/pub/people/trenn/sources/ec/ec_access.c
> 
> Len: Can you apply these into your test branch and schedule them for linux-next
> and 2.6.36 if there are no objections, please.
> 
> Thanks,
> 
>      Thomas

This is great patchset, I was even thinking doing that myself.

While at it, could you also provide a broken down table of EC GPEs like
the one in /sys/firmware/acpi/interrupts/ ?

Best regards,
	Maxim Levitsky