2010-07-16 11:11:49

by Thomas Renninger

[permalink] [raw]
Subject: Version 2: Provide /sys/../ec with read/write access and some cleanups

This patchset is againt the acpi test branch.
It would be great if these can be applied there.
Patches are test compiled, booted and I played a bit with reading
writing...

Changes to the previous patchset:
- Use debugfs /sys/kernel/debug/ec/* instead of /sys/devices/...
- Keep some EC Documentation in the thinkpad driver
- Add Documentation/ABI describing the new interface

Possible further enhancements:
- Expose EC Fields and make them read/writable
- Allow userspace to switch on burst mode to be able to test
EC burst transitions in userspace

Thanks,

Thomas


2010-07-16 11:11:44

by Thomas Renninger

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

The .add function must not be declared __init.

Signed-off-by: Thomas Renninger <[email protected]>

CC: Alexey Starikovskiy <[email protected]>
CC: Len Brown <[email protected]>
CC: [email protected]
CC: [email protected]
CC: [email protected]
---
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

2010-07-16 11:11:46

by Thomas Renninger

[permalink] [raw]
Subject: [PATCH 2/7] ACPI: Provide /sys/kernel/debug//ec/ec0/io for binary access to the EC

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

Multiple ECs are not supported, but shouldn't be hard to add as soon
as the ec driver itself will support them.

Signed-off-by: Thomas Renninger <[email protected]>
CC: Alexey Starikovskiy <[email protected]>
CC: Len Brown <[email protected]>
CC: [email protected]
CC: [email protected]
CC: [email protected]
---
drivers/acpi/ec_sys.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 86 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/ec_sys.c b/drivers/acpi/ec_sys.c
index 834c21a..3ef9781 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 <[email protected]>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.
+ */
+
#include <linux/kernel.h>
#include <linux/acpi.h>
#include <linux/debugfs.h>
@@ -7,12 +17,87 @@ MODULE_AUTHOR("Thomas Renninger <[email protected]>");
MODULE_DESCRIPTION("ACPI EC sysfs access driver");
MODULE_LICENSE("GPL");

+#define EC_SPACE_SIZE 256
+
struct sysdev_class acpi_ec_sysdev_class = {
.name = "ec",
};

static struct dentry *acpi_ec_debugfs_dir;

+static int acpi_ec_open_io(struct inode *i, struct file *f)
+{
+ f->private_data = i->i_private;
+ return 0;
+}
+
+static ssize_t acpi_ec_read_io(struct file *f, char __user *buf,
+ size_t count, loff_t *off)
+{
+ /* Use this if support reading/writing multiple ECs exists in ec.c:
+ * struct acpi_ec *ec = ((struct seq_file *)f->private_data)->private;
+ */
+ 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 += 1;
+ size--;
+ }
+ return count;
+}
+
+static ssize_t acpi_ec_write_io(struct file *f, const char __user *buf,
+ size_t count, loff_t *off)
+{
+ /* Use this if support reading/writing multiple ECs exists in ec.c:
+ * struct acpi_ec *ec = ((struct seq_file *)f->private_data)->private;
+ */
+
+ 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 += 1;
+ size--;
+ }
+ return count;
+}
+
+static struct file_operations acpi_ec_io_ops = {
+ .owner = THIS_MODULE,
+ .open = acpi_ec_open_io,
+ .read = acpi_ec_read_io,
+ .write = acpi_ec_write_io,
+};
+
int acpi_ec_add_debugfs(struct acpi_ec *ec, unsigned int ec_device_count)
{
struct dentry *dev_dir;
@@ -35,6 +120,7 @@ int acpi_ec_add_debugfs(struct acpi_ec *ec, unsigned int ec_device_count)
debugfs_create_x32("gpe", 0444, dev_dir, (u32 *)&first_ec->gpe);
debugfs_create_bool("use_global_lock", 0444, dev_dir,
(u32 *)&first_ec->global_lock);
+ debugfs_create_file("io", 0666, dev_dir, ec, &acpi_ec_io_ops);
return 0;
}

--
1.6.3

2010-07-16 11:12:36

by Thomas Renninger

[permalink] [raw]
Subject: [PATCH 5/7] X86 platform drivers: Remove EC dump from thinkpad_acpi

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

Signed-off-by: Thomas Renninger <[email protected]>

CC: Alexey Starikovskiy <[email protected]>
CC: Len Brown <[email protected]>
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: Henrique de Moraes Holschuh <[email protected]>
CC: [email protected]
---
Documentation/laptops/thinkpad-acpi.txt | 71 +++++-------------------------
drivers/platform/x86/thinkpad_acpi.c | 73 -------------------------------
2 files changed, 11 insertions(+), 133 deletions(-)

diff --git a/Documentation/laptops/thinkpad-acpi.txt b/Documentation/laptops/thinkpad-acpi.txt
index fc15538..f6f8025 100644
--- a/Documentation/laptops/thinkpad-acpi.txt
+++ b/Documentation/laptops/thinkpad-acpi.txt
@@ -960,70 +960,21 @@ Sysfs notes:
subsystem, and follow all of the hwmon guidelines at
Documentation/hwmon.

+EXPERIMENTAL: Embedded controller register dump
+-----------------------------------------------

-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:
+This feature is not included in the thinkpad driver anymore.
+Instead the EC can be accessed through /sys/kernel/debug/ec with
+a userspace tool which can be found here:
+ftp://ftp.suse.com/pub/people/trenn/sources/ec

+Use it 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
+ - use above mentioned tool to read out the EC
+
+Often fan and temperature values vary between
readings. Since temperatures don't change vary fast, you can take
several quick dumps to eliminate them.

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

2010-07-16 11:12:45

by Thomas Renninger

[permalink] [raw]
Subject: [PATCH 4/7] ACPI: Remove /proc/acpi/embedded_controller/..

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 <[email protected]>

CC: Alexey Starikovskiy <[email protected]>
CC: Len Brown <[email protected]>
CC: [email protected]
CC: [email protected]
CC: [email protected]
---
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

2010-07-16 11:12:48

by Thomas Renninger

[permalink] [raw]
Subject: [PATCH 7/7] Documentation: Add new /sys/kernel/debug/ec/* files to ABI

Signed-off-by: Thomas Renninger <[email protected]>

CC: Alexey Starikovskiy <[email protected]>
CC: Len Brown <[email protected]>
CC: [email protected]
CC: [email protected]
CC: [email protected]
---
Documentation/ABI/testing/debugfs-ec | 20 ++++++++++++++++++++
1 files changed, 20 insertions(+), 0 deletions(-)
create mode 100644 Documentation/ABI/testing/debugfs-ec

diff --git a/Documentation/ABI/testing/debugfs-ec b/Documentation/ABI/testing/debugfs-ec
new file mode 100644
index 0000000..6546115
--- /dev/null
+++ b/Documentation/ABI/testing/debugfs-ec
@@ -0,0 +1,20 @@
+What: /sys/kernel/debug/ec/*/{gpe,use_global_lock,io}
+Date: July 2010
+Contact: Thomas Renninger <[email protected]>
+Description:
+
+General information like which GPE is assigned to the EC and whether
+the global lock should get used.
+Knowing the EC GPE one can watch the amount of HW events related to
+the EC here (XY -> GPE number from /sys/kernel/debug/ec/*/gpe):
+/sys/firmware/acpi/interrupts/gpeXY
+
+The io file is binary and a userspace tool located here:
+ftp://ftp.suse.com/pub/people/trenn/sources/ec/
+should get used to read out the 256 Embedded Controller registers
+or writing to them.
+
+CAUTION: Do not write to the Embedded Controller if you don't know
+what you are doing! Rebooting afterwards also is a good idea.
+This can influence the way your machine is cooled and fans may
+not get switched on again after you did a wrong write.
--
1.6.3

2010-07-16 11:12:52

by Thomas Renninger

[permalink] [raw]
Subject: [PATCH 3/7] ACPI: Register EC io ports in /proc/ioports

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 <[email protected]>

CC: Alexey Starikovskiy <[email protected]>
CC: Len Brown <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Bjorn Helgaas <[email protected]>
CC: [email protected]
---
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

2010-07-16 11:12:42

by Thomas Renninger

[permalink] [raw]
Subject: [PATCH 1/7] ACPI: Provide /sys/kernel/debug/ec/...

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

This is the gpe the EC is connected to and whether the global lock
gets used.
The io ports used are added to /proc/ioports in another patch.
Beside the fact that /proc/acpi is deprecated for quite some time,
this info is not needed for applications and thus can be moved
to debugfs instead of a public interface like /sys.

Signed-off-by: Thomas Renninger <[email protected]>

CC: Alexey Starikovskiy <[email protected]>
CC: Len Brown <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Bjorn Helgaas <[email protected]>
CC: [email protected]
---
drivers/acpi/Kconfig | 13 ++++++++++
drivers/acpi/Makefile | 1 +
drivers/acpi/ec.c | 18 ++++----------
drivers/acpi/ec_sys.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++
drivers/acpi/internal.h | 24 +++++++++++++++++++
5 files changed, 100 insertions(+), 13 deletions(-)
create mode 100644 drivers/acpi/ec_sys.c

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 7464115..f7226d1 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_DEBUGFS
+ tristate "EC read/write access through /sys/kernel/debug/ec"
+ default y
+ help
+ Say N to disable Embedded Controller /sys/kernel/debug 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 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..833b582 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_DEBUGFS) += 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..834c21a
--- /dev/null
+++ b/drivers/acpi/ec_sys.c
@@ -0,0 +1,57 @@
+#include <linux/kernel.h>
+#include <linux/acpi.h>
+#include <linux/debugfs.h>
+#include "internal.h"
+
+MODULE_AUTHOR("Thomas Renninger <[email protected]>");
+MODULE_DESCRIPTION("ACPI EC sysfs access driver");
+MODULE_LICENSE("GPL");
+
+struct sysdev_class acpi_ec_sysdev_class = {
+ .name = "ec",
+};
+
+static struct dentry *acpi_ec_debugfs_dir;
+
+int acpi_ec_add_debugfs(struct acpi_ec *ec, unsigned int ec_device_count)
+{
+ struct dentry *dev_dir;
+ char name[64];
+ if (ec_device_count == 0) {
+ acpi_ec_debugfs_dir = debugfs_create_dir("ec", NULL);
+ if (!acpi_ec_debugfs_dir)
+ return -ENOMEM;
+ }
+
+ sprintf(name, "ec%u", ec_device_count);
+ dev_dir = debugfs_create_dir(name, acpi_ec_debugfs_dir);
+ if (!dev_dir) {
+ if (ec_device_count == 0)
+ debugfs_remove_recursive(acpi_ec_debugfs_dir);
+ /* TBD: Proper cleanup for multiple ECs */
+ return -ENOMEM;
+ }
+
+ debugfs_create_x32("gpe", 0444, dev_dir, (u32 *)&first_ec->gpe);
+ debugfs_create_bool("use_global_lock", 0444, dev_dir,
+ (u32 *)&first_ec->global_lock);
+ return 0;
+}
+
+static int __init acpi_ec_sys_init(void)
+{
+ int err = 0;
+ if (first_ec)
+ err = acpi_ec_add_debugfs(first_ec, 0);
+ else
+ err = -ENODEV;
+ return err;
+}
+
+static void __exit acpi_ec_sys_exit(void)
+{
+ debugfs_remove_recursive(acpi_ec_debugfs_dir);
+}
+
+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

Subject: Re: [ibm-acpi-devel] [PATCH 5/7] X86 platform drivers: Remove EC dump from thinkpad_acpi

On Fri, 16 Jul 2010, Thomas Renninger wrote:
> There is a general interface for that now (provided by
> other patches in this patch series):
> /sys/kernel/debug/ec/*/io
>
> Signed-off-by: Thomas Renninger <[email protected]>
>
> CC: Alexey Starikovskiy <[email protected]>
> CC: Len Brown <[email protected]>
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: Henrique de Moraes Holschuh <[email protected]>
> CC: [email protected]

Acked-by: Henrique de Moraes Holschuh <[email protected]>

Thank you for adding the doc updates, Thomas.

> ---
> Documentation/laptops/thinkpad-acpi.txt | 71 +++++-------------------------
> drivers/platform/x86/thinkpad_acpi.c | 73 -------------------------------
> 2 files changed, 11 insertions(+), 133 deletions(-)
>
> diff --git a/Documentation/laptops/thinkpad-acpi.txt b/Documentation/laptops/thinkpad-acpi.txt
> index fc15538..f6f8025 100644
> --- a/Documentation/laptops/thinkpad-acpi.txt
> +++ b/Documentation/laptops/thinkpad-acpi.txt
> @@ -960,70 +960,21 @@ Sysfs notes:
> subsystem, and follow all of the hwmon guidelines at
> Documentation/hwmon.
>
> +EXPERIMENTAL: Embedded controller register dump
> +-----------------------------------------------
>
> -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:
> +This feature is not included in the thinkpad driver anymore.
> +Instead the EC can be accessed through /sys/kernel/debug/ec with
> +a userspace tool which can be found here:
> +ftp://ftp.suse.com/pub/people/trenn/sources/ec
>
> +Use it 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
> + - use above mentioned tool to read out the EC
> +
> +Often fan and temperature values vary between
> readings. Since temperatures don't change vary fast, you can take
> several quick dumps to eliminate them.
>
> 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);

--
"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

2010-07-22 15:03:42

by Matthew Garrett

[permalink] [raw]
Subject: Re: Version 2: Provide /sys/../ec with read/write access and some cleanups

On Fri, Jul 16, 2010 at 01:11:30PM +0200, Thomas Renninger wrote:
> This patchset is againt the acpi test branch.
> It would be great if these can be applied there.
> Patches are test compiled, booted and I played a bit with reading
> writing...

I've applied this set, thanks!

--
Matthew Garrett | [email protected]