Help!

[PATCH 00/12] ThinkPad embedded controller and hdaps drivers

 
Goto page 1, 2, 3, 4
Post new topic   General Reply to Topic (not reply to a specific post)    Forums Home -> Kernel (archive) RSS
Next:  2.6.18-rc3-mm1  
Author Message
Shem Multinymous
External


Since: Aug 06, 2006
Posts: 10



PostPosted: Sun Aug 06, 2006 1:40 pm    Post subject: [PATCH 00/12] ThinkPad embedded controller and hdaps drivers
Archived from groups: linux>kernel (more info?)

This patch series does three related things:

- Introduces a new driver, thinkpad_ec, for the ThinkPad embedded
controller (additional non-ACPI interface).
- Changes the existing hdaps driver to use thinkpad_ec instead of
direct (and incorrect) port access.
- Several fixes and improvements to the hdaps driver.

Applies to git head.

Most of the patches in this series depend on previous ones.
Any point in this series will compile and was briefly tested (this
required introducing some code that gets overwritten by later patches).
The *combination* of all these patches was extensively tested as part
of the out-of-kernel tp_smapi package [1], and verified to work on dozens
of ThinkPad models (thousands of sf.net downloads and no unresolved
bugs). Note that the tp_smapi package includes another driver, to
be submitted later, which also depends the thinkpad_ec driver
introduced here.

For some context and the necessity for a separate thinkpad_ec module,
see LKML thread "tp_smapi conflict with IDE, hdaps" on Dec. 2005.

I would like to thank the many testers and contributors who helped
in the development, and in particular Henrique de Moraes Holschuh
and ThinkWiki.org users Whoopie and Spiney.

[1] http://thinkwiki.org/wiki/tp_smapi
http://tpctl.sourceforge.net/rel/tp_smapi-0.27.tgz

Summary:

[PATCH 01/12] thinkpad_ec: New driver for ThinkPad embedded controller access
[PATCH 02/12] hdaps: Use thinkpad_ec instead of direct port access
[PATCH 03/12] hdaps: Unify and cache hdaps readouts
[PATCH 04/12] hdaps: Correct readout and remove nonsensical attributes
[PATCH 05/12] hdaps: Remember keyboard and mouse activity
[PATCH 06/12] hdaps: Limit hardware query rate
[PATCH 07/12] hdaps: delay calibration to first hardware query
[PATCH 08/12] hdaps: Add explicit hardware configuration functions
[PATCH 09/12] hdaps: Add new sysfs attributes
[PATCH 10/12] hdaps: Power off accelerometer on suspend and unload
[PATCH 11/12] hdaps: Stop polling timer when suspended
[PATCH 12/12] hdaps: Simplify whitelist

drivers/hwmon/hdaps.c | 758 ++++++++++++++++++++++++-----------------
drivers/firmware/Kconfig | 3
drivers/firmware/Makefile | 1
drivers/firmware/thinkpad_ec.c | 444 ++++++++++++++++++++++++
drivers/hwmon/Kconfig | 1
include/linux/thinkpad_ec.h | 47 ++
6 files changed, 940 insertions(+), 314 deletions(-)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to top
Shem Multinymous
External


Since: Aug 06, 2006
Posts: 10



PostPosted: Sun Aug 06, 2006 1:40 pm    Post subject: [PATCH 08/12] hdaps: Add explicit hardware configuration functions [Login to view extended thread Info.]
Archived from groups: per prev. post (more info?)

This adds functions for configuring accelerometer-related hardware
parameters in the hdaps driver, and changes the init function to
use these functions instead of opaque magic numbers.
The parameters are configured via variables instead of constants
since a later patch will add sysfs attributes for changing them.

A few of these functions aren't used yet, but will be used by later
patches.

Signed-off-by: Shem Multinymous
---
hdaps.c | 200 ++++++++++++++++++++++++++++++++++++++++++++++++----------------
1 file changed, 153 insertions(+), 47 deletions(-)

diff -up a/drivers/hwmon/hdaps.c a/drivers/hwmon/hdaps.c
--- a/drivers/hwmon/hdaps.c
+++ a/drivers/hwmon/hdaps.c
@@ -57,7 +57,6 @@ static const struct thinkpad_ec_row ec_a
#define READ_TIMEOUT_MSECS 100 /* wait this long for device read */
#define RETRY_MSECS 3 /* retry delay */

-#define HDAPS_POLL_PERIOD (HZ/50) /* poll for input every 1/50s */
#define HDAPS_INPUT_FUZZ 4 /* input event threshold */
#define HDAPS_INPUT_FLAT 4
#define KMACT_REMEMBER_PERIOD (HZ/10) /* keyboard/mouse persistance */
@@ -68,6 +67,13 @@ static struct input_dev *hdaps_idev;
static unsigned int hdaps_invert;
static int needs_calibration = 0;

+/* Configuration: */
+static int sampling_rate = 50; /* Sampling rate */
+static int oversampling_ratio = 5; /* Ratio between our sampling rate and
+ * EC accelerometer sampling rate */
+static int running_avg_filter_order = 2; /* EC running average filter order */
+static int fake_data_mode = 0; /* Enable EC fake data mode? */
+
/* Latest state readout: */
static int pos_x, pos_y; /* position */
static int temperature; /* temperature */
@@ -162,6 +168,137 @@ static int hdaps_update(void)
}

/*
+ * hdaps_set_power - enable or disable power to the accelerometer.
+ * Returns zero on success and negative error code on failure. Can sleep.
+ */
+static int hdaps_set_power(int on) {
+ struct thinkpad_ec_row args =
+ { .mask=0x0003, .val={0x14, on?0x01:0x00} };
+ struct thinkpad_ec_row data = { .mask = 0x8000 };
+ int ret = thinkpad_ec_read_row(&args, &data);
+ if (ret)
+ return ret;
+ if (data.val[0xF]!=0x00)
+ return -EIO;
+ return 0;
+}
+
+/*
+ * hdaps_set_fake_data_mode - enable or disable EC test mode, which fakes
+ * accelerometer data using an incrementing counter.
+ * Returns zero on success and negative error code on failure. Can sleep.
+ */
+static int hdaps_set_fake_data_mode(int on) {
+ struct thinkpad_ec_row args =
+ { .mask=0x0007, .val={0x17, 0x83, on?0x01:0x00} };
+ struct thinkpad_ec_row data = { .mask = 0x8000 };
+ int ret = thinkpad_ec_read_row(&args, &data);
+ if (ret)
+ return ret;
+ if (data.val[0xF]!=0x00) {
+ printk(KERN_WARNING "failed setting hdaps fake data to %d\n",
+ on);
+ return -EIO;
+ }
+ printk(KERN_DEBUG "hdaps: fake_data_mode set to %d\n", on);
+ return 0;
+}
+
+/*
+ * hdaps_set_ec_config - set accelerometer parameters.
+ * ec_rate - embedded controller sampling rate
+ * ( sampling_rate * oversampling_ratio )
+ * order - embedded controller running average filter order
+ * Returns zero on success and negative error code on failure. Can sleep.
+ */
+static int hdaps_set_ec_config(int ec_rate, int order) {
+ struct thinkpad_ec_row args = { .mask=0x000F,
+ .val={0x10, (u8)ec_rate, (uCool(ec_rate>>Cool, order} };
+ struct thinkpad_ec_row data = { .mask = 0x8000 };
+ int ret = thinkpad_ec_read_row(&args, &data);
+ printk(KERN_DEBUG "hdaps: setting ec_rate=%d, filter_order=%d\n",
+ ec_rate, order);
+ if (ret)
+ return ret;
+ if (data.val[0xF]==0x03) {
+ printk(KERN_WARNING "hdaps: config param out of range\n");
+ return -EINVAL;
+ }
+ if (data.val[0xF]==0x06) {
+ printk(KERN_WARNING "hdaps: config change already pending\n");
+ return -EBUSY;
+ }
+ if (data.val[0xF]!=0x00) {
+ printk(KERN_WARNING "hdaps: config change error, ret=%d\n",
+ data.val[0xF]);
+ return -EIO;
+ }
+ return 0;
+}
+
+/*
+ * hdaps_get_ec_config - get accelerometer parameters.
+ * ec_rate - embedded controller sampling rate
+ * order - embedded controller running average filter order
+ * Returns zero on success and negative error code on failure. Can sleep.
+ */
+static int hdaps_get_ec_config(int *ec_rate, int *order) {
+ const struct thinkpad_ec_row args =
+ { .mask=0x0003, .val={0x17, 0x82} };
+ struct thinkpad_ec_row data = { .mask = 0x801F };
+ int ret = thinkpad_ec_read_row(&args, &data);
+ if (ret)
+ return ret;
+ if (data.val[0xF]!=0x00)
+ return -EIO;
+ if (!(data.val[0x1] & 0x01))
+ return -ENXIO; /* accelerometer polling not enabled */
+ if (data.val[0x1] & 0x02)
+ return -EBUSY; /* config change in progress, retry later */
+ *ec_rate = data.val[0x2] | ((int)(data.val[0x3]) << Cool;
+ *order = data.val[0x4];
+ return 0;
+}
+
+/*
+ * hdaps_get_ec_mode - get EC accelerometer mode
+ * Returns zero on success and negative error code on failure. Can sleep.
+ */
+static int hdaps_get_ec_mode(u8 *mode) {
+ const struct thinkpad_ec_row args = { .mask=0x0001, .val={0x13} };
+ struct thinkpad_ec_row data = { .mask = 0x8002 };
+ int ret = thinkpad_ec_read_row(&args, &data);
+ if (ret)
+ return ret;
+ if (data.val[0xF]!=0x00) {
+ printk(KERN_WARNING
+ "accelerometer not implemented (0x%02x)\n",
+ data.val[0xF]);
+ return -EIO;
+ }
+ *mode = data.val[0x1];
+ return 0;
+}
+
+/*
+ * hdaps_check_ec - checks something about the EC.
+ * Follows the clean-room spec for HDAPS; we don't know what it means.
+ * Returns zero on success and negative error code on failure. Can sleep.
+ */
+static int __init hdaps_check_ec(u8 *mode) {
+ const struct thinkpad_ec_row args =
+ { .mask=0x0003, .val={0x17, 0x81} };
+ struct thinkpad_ec_row data = { .mask = 0x800E };
+ int ret = thinkpad_ec_read_row(&args, &data);
+ if (ret)
+ return ret;
+ if (data.val[0x1]!=0x00 || data.val[0x2]!=0x60 ||
+ data.val[0x3]!=0x00 || data.val[0xF]!=0x00)
+ return -EIO;
+ return 0;
+}
+
+/*
* hdaps_device_init - initialize the accelerometer. Returns zero on success
* and negative error code on failure. Can sleep.
*/
@@ -169,61 +306,30 @@ static int hdaps_update(void)
static int hdaps_device_init(void)
{
int ret;
- struct thinkpad_ec_row args, data;
u8 mode;

ret = thinkpad_ec_lock();
if (ret)
return ret;

- args.val[0x0] = 0x13;
- args.val[0xF] = 0x01;
- args.mask=0x8001;
- data.mask=0x8002;
- if (thinkpad_ec_read_row(&args, &data))
- ABORT_INIT("read1");
- if (data.val[0xF]!=0x00)
- ABORT_INIT("check1");
- mode = data.val[0x1];
-
+ if (hdaps_get_ec_mode(&mode))
+ ABORT_INIT("hdaps_get_ec_mode failed");
printk(KERN_DEBUG "hdaps: initial mode latch is 0x%02x\n", mode);
if (mode==0x00)
ABORT_INIT("accelerometer not available");

- args.val[0x0] = 0x17;
- args.val[0x1] = 0x81;
- args.val[0xF] = 0x01;
- args.mask = 0x8003;
- data.mask = 0x800E;
- if (thinkpad_ec_read_row(&args, &data))
- ABORT_INIT("read2");
- if (data.val[0x1]!=0x00 ||
- data.val[0x2]!=0x60 ||
- data.val[0x3]!=0x00 ||
- data.val[0xF]!=0x00)
- ABORT_INIT("check2");
-
- args.val[0x0] = 0x14;
- args.val[0x1] = 0x01;
- args.val[0xF] = 0x01;
- args.mask = 0x8003;
- data.mask = 0x8000;
- if (thinkpad_ec_read_row(&args, &data))
- ABORT_INIT("read3");
- if (data.val[0xF]!=0x00)
- ABORT_INIT("check3");
+ if (hdaps_check_ec(&mode))
+ ABORT_INIT("hdaps_check_ec failed");

- args.val[0x0] = 0x10;
- args.val[0x1] = 0xc8;
- args.val[0x2] = 0x00;
- args.val[0x3] = 0x02;
- args.val[0xF] = 0x01;
- args.mask = 0x800F;
- data.mask = 0x8000;
- if (thinkpad_ec_read_row(&args, &data))
- ABORT_INIT("read4");
- if (data.val[0xF]!=0x00)
- ABORT_INIT("check4");
+ if (hdaps_set_power(1))
+ ABORT_INIT("hdaps_set_power failed");
+
+ if (hdaps_set_ec_config(sampling_rate*oversampling_ratio,
+ running_avg_filter_order))
+ ABORT_INIT("hdaps_set_ec_config failed");
+
+ if (hdaps_set_fake_data_mode(fake_data_mode))
+ ABORT_INIT("hdaps_set_fake_data_mode failed");

thinkpad_ec_invalidate();
udelay(200);
@@ -308,7 +414,7 @@ keep_active:
input_report_abs(hdaps_idev, ABS_X, pos_x - rest_x);
input_report_abs(hdaps_idev, ABS_Y, pos_y - rest_y);
input_sync(hdaps_idev);
- mod_timer(&hdaps_timer, jiffies + HDAPS_POLL_PERIOD);
+ mod_timer(&hdaps_timer, jiffies + HZ/sampling_rate);
}


@@ -525,7 +631,7 @@ static int __init hdaps_init(void)
/* start up our timer for the input device */
init_timer(&hdaps_timer);
hdaps_timer.function = hdaps_mousedev_poll;
- hdaps_timer.expires = jiffies + HDAPS_POLL_PERIOD;
+ hdaps_timer.expires = jiffies + HZ/sampling_rate;
add_timer(&hdaps_timer);

printk(KERN_INFO "hdaps: driver successfully loaded.\n");
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to top
Shem Multinymous
External


Since: Aug 06, 2006
Posts: 10



PostPosted: Sun Aug 06, 2006 1:40 pm    Post subject: [PATCH 07/12] hdaps: delay calibration to first hardware query [Login to view extended thread Info.]
Archived from groups: per prev. post (more info?)

The hdaps driver currently calibrates its rest position upon
initialization, which can take several seconds on first module load
(and delays the boot process accordingly). This patch delays
calibration to the first successful hardware query, when the
information is available anyway. Writes to the "calibrate" sysfs
attribute are handled likewise.

Signed-off-by: Shem Multinymous
---
hdaps.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)

diff -up a/drivers/hwmon/hdaps.c a/drivers/hwmon/hdaps.c
--- a/drivers/hwmon/hdaps.c
+++ a/drivers/hwmon/hdaps.c
@@ -66,6 +66,7 @@ static struct timer_list hdaps_timer;
static struct platform_device *pdev;
static struct input_dev *hdaps_idev;
static unsigned int hdaps_invert;
+static int needs_calibration = 0;

/* Latest state readout: */
static int pos_x, pos_y; /* position */
@@ -125,6 +126,11 @@ static int __hdaps_update(int fast)
temperature = data.val[EC_ACCEL_IDX_TEMP1];

stale_readout = 0;
+ if (needs_calibration) {
+ rest_x = pos_x;
+ rest_y = pos_y;
+ needs_calibration = 0;
+ }

return 0;
}
@@ -270,9 +276,9 @@ static struct platform_driver hdaps_driv
*/
static void hdaps_calibrate(void)
{
+ needs_calibration = 1;
hdaps_update();
- rest_x = pos_x;
- rest_y = pos_y;
+ /* If that fails, the mousedev poll will take care of things later. */
}

/* Timer handler for updating the input device. Runs in softirq context,
@@ -502,8 +508,8 @@ static int __init hdaps_init(void)
goto out_group;
}

- /* initial calibrate for the input device */
- hdaps_calibrate();
+ /* calibration for the input device (deferred to avoid delay) */
+ needs_calibration = 1;

/* initialize the input class */
hdaps_idev->name = "hdaps";
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to top
Shem Multinymous
External


Since: Aug 06, 2006
Posts: 10



PostPosted: Sun Aug 06, 2006 1:40 pm    Post subject: [PATCH 01/12] thinkpad_ec: New driver for ThinkPad embedded controller access [Login to view extended thread Info.]
Archived from groups: per prev. post (more info?)

The embedded controller on ThinkPad laptops has a non-standard interface
at IO ports 0x1600-0x161F (mapped to LCP channel 3 of the H8S chip).
The interface provides various system management services (currently
known: battery information and accelerometer readouts). This driver
provides access and mutual exclusion for the EC interface.

The mainline hdaps driver already uses this hardware interface (in an
incorrect and unsafe way), and will be converted to use this module in
the following patches. Another driver using this module, tp_smapi, will
be submitted later.

The Kconfig entry is set to tristate and will be selected by hdaps and
(eventually) tp_smapi, since thinkpad_ec does nothing by itself.

Signed-off-by: Shem Multinymous
---

This depends on dmi-decode-and-save-oem-string-information.patch
pending in -mm ("[PATCH] DMI: Decode and save OEM String information"
on LKML).

drivers/firmware/Kconfig | 3
drivers/firmware/Makefile | 1
drivers/firmware/thinkpad_ec.c | 449 +++++++++++++++++++++++++++++++++++++++++
include/linux/thinkpad_ec.h | 47 ++++
4 files changed, 500 insertions(+)


diff -dNurp a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
--- a/drivers/firmware/Kconfig 2006-08-05 04:31:21.000000000 +0300
+++ b/drivers/firmware/Kconfig 2006-08-05 04:31:21.000000000 +0300
@@ -84,4 +84,7 @@ config DCDBAS
Say Y or M here to enable the driver for use by Dell systems
management software such as Dell OpenManage.

+config THINKPAD_EC
+ tristate
+
endmenu
diff -dNurp a/drivers/firmware/Makefile b/drivers/firmware/Makefile
--- a/drivers/firmware/Makefile 2006-08-05 04:31:21.000000000 +0300
+++ b/drivers/firmware/Makefile 2006-08-05 04:31:21.000000000 +0300
@@ -7,3 +7,4 @@ obj-$(CONFIG_EFI_VARS) += efivars.o
obj-$(CONFIG_EFI_PCDP) += pcdp.o
obj-$(CONFIG_DELL_RBU) += dell_rbu.o
obj-$(CONFIG_DCDBAS) += dcdbas.o
+obj-$(CONFIG_THINKPAD_EC) += thinkpad_ec.o
diff -dNurp a/drivers/firmware/thinkpad_ec.c b/drivers/firmware/thinkpad_ec.c
--- a/drivers/firmware/thinkpad_ec.c 1970-01-01 02:00:00.000000000 +0200
+++ b/drivers/firmware/thinkpad_ec.c 2006-08-05 05:33:06.000000000 +0300
@@ -0,0 +1,444 @@
+/*
+ * thinkpad_ec.c - coordinate access to ThinkPad-specific hardware resources
+ *
+ * The embedded controller on ThinkPad laptops has a non-standard interface
+ * at IO ports 0x1600-0x161F (mapped to LCP channel 3 of the H8S chip).
+ * The interface provides various system management services (currently
+ * known: battery information and accelerometer readouts). This driver
+ * provides access and mutual exclusion for the EC interface.
+ * For information about the LPC protocol and terminology, see:
+ * "H8S/2104B Group Hardware Manual",
+ * http://documentation.renesas.com/eng/products/mpumcu/rej09b0300_2140bhm.pdf
+ *
+ * Copyright (C) 2006 Shem Multinymous
+ *
+ * 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/dmi.h>
+#include <linux/ioport.h>
+#include <linux/delay.h>
+#include <linux/thinkpad_ec.h>
+#include <linux/jiffies.h>
+#include <asm/io.h>
+
+#define TP_VERSION "0.27"
+
+MODULE_AUTHOR("Shem Multinymous");
+MODULE_DESCRIPTION("ThinkPad embedded controller hardware access");
+MODULE_VERSION(TP_VERSION);
+MODULE_LICENSE("GPL");
+
+/* IO ports used by embedded controller LPC channel 3: */
+#define TPC_BASE_PORT 0x1600
+#define TPC_NUM_PORTS 0x20
+#define TPC_STR3_PORT 0x1604 /* Reads H8S EC register STR3 */
+#define TPC_TWR0_PORT 0x1610 /* Mapped to H8S EC register TWR0MW/SW */
+#define TPC_TWR15_PORT 0x161F /* Mapped to H8S EC register TWR15. */
+ /* (and port TPC_TWR0_PORT+i is mapped to H8S reg TWRi for 0<i<16) */
+
+/* H8S STR3 status flags (see "H8S/2104B Group Hardware Manual" p.549) */
+#define H8S_STR3_IBF3B 0x80 /* Bidi. Data Register Input Buffer Full */
+#define H8S_STR3_OBF3B 0x40 /* Bidi. Data Register Output Buffer Full */
+#define H8S_STR3_MWMF 0x20 /* Master Write Mode Flag */
+#define H8S_STR3_SWMF 0x10 /* Slave Write Mode Flag */
+#define H8S_STR3_MASK 0xF0 /* All bits we care about in STR3 */
+
+/* Timeouts and retries */
+#define TPC_READ_RETRIES 150
+#define TPC_READ_NDELAY 500
+#define TPC_REQUEST_RETRIES 100
+#define TPC_REQUEST_NDELAY 10
+#define TPC_PREFETCH_TIMEOUT (HZ/10) /* invalidate prefetch after 0.1sec */
+
+/* Module parameters: */
+static int tp_debug = 0;
+module_param_named(debug, tp_debug, int, 0600);
+MODULE_PARM_DESC(debug, "Debug level (0=off, 1=on)");
+
+/* A few macros for printk()ing: */
+#define DPRINTK(fmt, args...) \
+ do { if (tp_debug) printk(KERN_DEBUG fmt, ## args); } while (0)
+#define MSG_FMT(fmt, args...) \
+ "thinkpad_ec: %s: " fmt "\n", __func__, ## args
+#define REQ_FMT(msg, code) \
+ MSG_FMT("%s: (0x%02x:0x%02x)->0x%02x", \
+ msg, args->val[0x0], args->val[0xF], code)
+
+/* State of request prefetching: */
+static u8 prefetch_arg0, prefetch_argF; /* Args of last prefetch */
+static u64 prefetch_jiffies; /* time of prefetch, or: */
+#define TPC_PREFETCH_NONE INITIAL_JIFFIES /* - No prefetch */
+#define TPC_PREFETCH_JUNK (INITIAL_JIFFIES+1) /* - Ignore prefetch */
+
+/* Locking: */
+
+static DECLARE_MUTEX(thinkpad_ec_mutex);
+
+/* thinkpad_ec_lock:
+ * Get exclusive lock for accesing the controller. May sleep.
+ * Returns 0 iff lock acquired .
+ */
+int thinkpad_ec_lock(void)
+{
+ int ret;
+ ret = down_interruptible(&thinkpad_ec_mutex);
+ if (ret)
+ DPRINTK("tp_controller mutex down interrupted: %d\n", ret);
+ return ret;
+}
+
+EXPORT_SYMBOL_GPL(thinkpad_ec_lock);
+
+/* thinkpad_ec_try_lock:
+ * Get exclusive lock for accesing the controller but only if it's available.
+ * Does not block, does not sleep. Returns 0 iff lock acquired .
+ */
+int thinkpad_ec_try_lock(void)
+{
+ return down_trylock(&thinkpad_ec_mutex);
+}
+
+EXPORT_SYMBOL_GPL(thinkpad_ec_try_lock);
+
+/* thinkpad_ec_unlock:
+ * Release a previously acquired controller lock.
+ */
+void thinkpad_ec_unlock(void)
+{
+ up(&thinkpad_ec_mutex);
+}
+
+EXPORT_SYMBOL_GPL(thinkpad_ec_unlock);
+
+/* Tell embedded controller to prepare a row */
+static int thinkpad_ec_request_row(const struct thinkpad_ec_row *args)
+{
+ u8 str3;
+ int i;
+
+ /* EC protocol requires write to TWR0 (function code): */
+ if (!(args->mask & 0x0001)) {
+ printk(KERN_ERR MSG_FMT("bad args->mask=0x%02x", args->mask));
+ return -EINVAL;
+ }
+
+ /* Check initial STR3 status: */
+ str3 = inb(TPC_STR3_PORT) & H8S_STR3_MASK;
+ if (str3 & H8S_STR3_OBF3B) { /* data already pending */
+ inb(TPC_TWR15_PORT); /* marks end of previous transaction */
+ if (prefetch_jiffies == TPC_PREFETCH_NONE)
+ printk(KERN_WARNING
+ REQ_FMT("readout already pending", str3));
+ return -EBUSY; /* EC will be ready in a few usecs */
+ } else if (str3 == H8S_STR3_SWMF) { /* busy with previous request */
+ if (prefetch_jiffies == TPC_PREFETCH_NONE)
+ printk(KERN_WARNING
+ REQ_FMT("EC handles previous request", str3));
+ return -EBUSY; /* data will be pending in a few usecs */
+ } else if (str3 != 0x00) { /* unexpected status? */
+ printk(KERN_WARNING REQ_FMT("bad initial STR3", str3));
+ return -EIO;
+ }
+
+ /* Send TWR0MW: */
+ outb(args->val[0], TPC_TWR0_PORT);
+ str3 = inb(TPC_STR3_PORT) & H8S_STR3_MASK;
+ if (str3 != H8S_STR3_MWMF) { /* not accepted? */
+ printk(KERN_WARNING REQ_FMT("arg0 rejected", str3));
+ return -EIO;
+ }
+
+ /* Send TWR1 through TWR14: */
+ for (i=1; i<TP_CONTROLLER_ROW_LEN-1; i++)
+ if ((args->mask>>i)&1)
+ outb(args->val[i], TPC_TWR0_PORT+i);
+
+ /* Send TWR15 (default to 0x01). This marks end of command. */
+ outb((args->mask & 0x8000) ? args->val[0xF] : 0x01, TPC_TWR15_PORT);
+
+ /* Wait until EC starts writing its reply (~60ns on average).
+ * Releasing locks before this happens may cause an EC hang
+ * due to firmware bug!
+ */
+ for (i=0; i<TPC_REQUEST_RETRIES; ++i) {
+ str3 = inb(TPC_STR3_PORT) & H8S_STR3_MASK;
+ if (str3 & H8S_STR3_SWMF) /* EC started replying */
+ return 0;
+ else if (str3==(H8S_STR3_IBF3B|H8S_STR3_MWMF) ||
+ str3==0x00) /* normal progress, wait it out */
+ ndelay(TPC_REQUEST_NDELAY);
+ else { /* weird EC status */
+ printk(KERN_WARNING
+ REQ_FMT("bad end STR3", str3));
+ return -EIO;
+ }
+ }
+ printk(KERN_WARNING REQ_FMT("EC is mysteriously silent", str3));
+ return -EIO;
+}
+
+/* Read current row data from the controller, assuming it's already
+ * requested.
+ */
+static int thinkpad_ec_read_data(struct thinkpad_ec_row *data)
+{
+ int i;
+ u8 str3 = inb(TPC_STR3_PORT) & H8S_STR3_MASK;
+ /* Once we make a request, STR3 assumes the sequence of values listed
+ * in the following 'if' as it reads the request and writes its data.
+ * It takes about a few dozen nanosecs total, with very high variance.
+ */
+ if (str3==(H8S_STR3_IBF3B|H8S_STR3_MWMF) ||
+ str3==0x00 || /* the 0x00 is indistinguishable from idle EC! */
+ str3==H8S_STR3_SWMF )
+ return -EBUSY; /* not ready yet */
+ /* Finally, the EC signals output buffer full: */
+ if (str3 != (H8S_STR3_OBF3B|H8S_STR3_SWMF)) {
+ printk(KERN_WARNING
+ MSG_FMT("bad initial STR3 (0x%02x)", str3));
+ return -EIO;
+ }
+
+ /* Read first byte (signals start of read transactions): */
+ data->val[0] = inb(TPC_TWR0_PORT);
+ /* Optionally read 14 more bytes: */
+ for (i=1; i<TP_CONTROLLER_ROW_LEN-1; ++i)
+ if ((data->mask >> i)&1)
+ data->val[i] = inb(TPC_TWR0_PORT+i);
+ /* Read last byte from 0x161F (signals end of read transaction): */
+ data->val[0xF] = inb(TPC_TWR15_PORT);
+
+ /* Readout still pending? */
+ str3 = inb(TPC_STR3_PORT) & H8S_STR3_MASK;
+ if (str3 & H8S_STR3_OBF3B)
+ printk(KERN_WARNING
+ MSG_FMT("OBF3B=1 after read (0x%02x)", str3));
+ return 0;
+}
+
+/* Is the given row currently prefetched?
+ * To keep things simple we compare only the first and last args;
+ * in practice this suffices .*/
+static int thinkpad_ec_is_row_fetched(const struct thinkpad_ec_row *args)
+{
+ return (prefetch_jiffies != TPC_PREFETCH_NONE) &&
+ (prefetch_jiffies != TPC_PREFETCH_JUNK) &&
+ (prefetch_arg0 == args->val[0]) &&
+ (prefetch_argF == args->val[0xF]) &&
+ (get_jiffies_64() < prefetch_jiffies + TPC_PREFETCH_TIMEOUT);
+}
+
+/* thinkpad_ec_read_row:
+ * Read a data row from the controller, fetching and retrying if needed.
+ * The row args are specified by 16 byte arguments, some of which may be
+ * missing (but the first and last are mandatory). These are given in
+ * args->val[], where args->val[i] is used iff (args->mask>>i)&1).
+ * The rows's data is stored in data->val[], but is only guaranteed to be
+ * valid for indices corresponding to set bit in data->maska. That is,
+ * if (data->mask>>i)&1==0 then data->val[i] may not be filled (to save time).
+ * Returns -EBUSY on transient error and -EIO on abnormal condition.
+ * Caller must hold controller lock.
+ */
+int thinkpad_ec_read_row(const struct thinkpad_ec_row *args,
+ struct thinkpad_ec_row *data)
+{
+ int retries, ret;
+
+ if (thinkpad_ec_is_row_fetched(args))
+ goto read_row; /* already requested */
+
+ /* Request the row */
+ for (retries=0; retries<TPC_READ_RETRIES; ++retries) {
+ ret = thinkpad_ec_request_row(args);
+ if (!ret)
+ goto read_row;
+ if (ret != -EBUSY)
+ break;
+ ndelay(TPC_READ_NDELAY);
+ }
+ printk(KERN_ERR REQ_FMT("failed requesting row", ret));
+ goto out;
+
+read_row:
+ /* Read the row's data */
+ for (retries=0; retries<TPC_READ_RETRIES; ++retries) {
+ ret = thinkpad_ec_read_data(data);
+ if (!ret)
+ goto out;
+ if (ret!=-EBUSY)
+ break;
+ ndelay(TPC_READ_NDELAY);
+ }
+
+ printk(KERN_ERR REQ_FMT("failed waiting for data", ret));
+
+out:
+ prefetch_jiffies = TPC_PREFETCH_JUNK;
+ return ret;
+}
+
+EXPORT_SYMBOL_GPL(thinkpad_ec_read_row);
+
+/* thinkpad_ec_try_read_row:
+ * Try read a prefetched row from the controller. Don't fetch or retry.
+ * See thinkpad_ec_read_row above for the meaning of the arguments.
+ * Returns -EBUSY is data not ready and -ENODATA if row not prefetched.
+ * Caller must hold controller lock.
+ */
+int thinkpad_ec_try_read_row(const struct thinkpad_ec_row *args,
+ struct thinkpad_ec_row *data)
+{
+ int ret;
+ if (!thinkpad_ec_is_row_fetched(args)) {
+ ret = -ENODATA;
+ } else {
+ ret = thinkpad_ec_read_data(data);
+ if (!ret)
+ prefetch_jiffies = TPC_PREFETCH_NONE; /* eaten up */
+ }
+ return ret;
+}
+
+EXPORT_SYMBOL_GPL(thinkpad_ec_try_read_row);
+
+/* thinkpad_ec_prefetch_row:
+ * Prefetch data row from the controller. A subsequent call to
+ * thinkpad_ec_read_row() with the same arguments will be faster,
+ * and a subsequent call to thinkpad_ec_try_read_row stands a
+ * good chance of succeeding if done neither too soon nor too late.
+ * See thinkpad_ec_read_row above for the meaning of the arguments.
+ * Returns -EBUSY on transient error and -EIO on abnormal condition.
+ * Caller must hold controller lock.
+ */
+int thinkpad_ec_prefetch_row(const struct thinkpad_ec_row *args)
+{
+ int ret;
+ ret = thinkpad_ec_request_row(args);
+ if (ret) {
+ prefetch_jiffies = TPC_PREFETCH_JUNK;
+ } else {
+ prefetch_jiffies = get_jiffies_64();
+ prefetch_arg0 = args->val[0x0];
+ prefetch_argF = args->val[0xF];
+ }
+ return ret;
+}
+
+EXPORT_SYMBOL_GPL(thinkpad_ec_prefetch_row);
+
+/* thinkpad_ec_invalidate:
+ * Invalidate the prefetched controller data.
+ * Must be called before unlocking by any code that accesses the controller
+ * ports directly.
+ */
+void thinkpad_ec_invalidate(void)
+{
+ prefetch_jiffies = TPC_PREFETCH_JUNK;
+}
+
+EXPORT_SYMBOL_GPL(thinkpad_ec_invalidate);
+
+
+/*** Checking for EC hardware ***/
+
+/* thinkpad_ec_test:
+ * Ensure the EC LPC3 channel really works on this machine by making
+ * an arbitrary harmless EC request and seeing if the EC follows protocol.
+ * This test writes to IO ports, so execute only after checking DMI.
+ */
+static int thinkpad_ec_test(void) {
+ int ret;
+ const struct thinkpad_ec_row args = /* battery 0 basic status */
+ { .mask=0x8001, .val={0x01,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0x00} };
+ struct thinkpad_ec_row data = { .mask = 0x0000 };
+ ret = thinkpad_ec_lock();
+ if (ret)
+ return ret;
+ ret = thinkpad_ec_read_row(&args, &data);
+ thinkpad_ec_unlock();
+ return ret;
+}
+
+/* Search all DMI device names for a given type for a substrng */
+static int __init dmi_find_substring(int type, const char *substr) {
+ struct dmi_device *dev = NULL;
+ while ((dev = dmi_find_device(type, NULL, dev))) {
+ if (strstr(dev->name, substr))
+ return 1;
+ }
+ return 0;
+}
+
+#define TP_DMI_MATCH(vendor,model) { \
+ .ident = vendor " " model, \
+ .matches = { \
+ DMI_MATCH(DMI_BOARD_VENDOR, vendor), \
+ DMI_MATCH(DMI_PRODUCT_VERSION, model) \
+ } \
+}
+
+/* Check DMI for existence of ThinkPad embedded controller */
+static int __init check_dmi_for_ec(void)
+{
+ /* A few old models that have a good EC but don't report it in DMI */
+ struct dmi_system_id tp_whitelist[] = {
+ TP_DMI_MATCH("IBM","ThinkPad A30"),
+ TP_DMI_MATCH("IBM","ThinkPad T23"),
+ TP_DMI_MATCH("IBM","ThinkPad X24"),
+ { .ident = NULL }
+ };
+ return dmi_find_substring(DMI_DEV_TYPE_OEM_STRING,
+ "IBM ThinkPad Embedded Controller") ||
+ dmi_check_system(tp_whitelist);
+}
+
+/*** Init and cleanup ***/
+
+static int __init thinkpad_ec_init(void)
+{
+ if (!check_dmi_for_ec()) {
+ printk(KERN_ERR "thinkpad_ec: no ThinkPad embedded controller!\n");
+ return -ENODEV;
+ }
+
+ if (!request_region(TPC_BASE_PORT, TPC_NUM_PORTS,
+ "thinkpad_ec"))
+ {
+ printk(KERN_ERR "thinkpad_ec: cannot claim io ports %#x-%#x\n",
+ TPC_BASE_PORT,
+ TPC_BASE_PORT + TPC_NUM_PORTS -1);
+ return -ENXIO;
+ }
+ prefetch_jiffies = TPC_PREFETCH_JUNK;
+ if (thinkpad_ec_test()) {
+ printk(KERN_INFO "thinkpad_ec: initial ec test failed\n");
+ release_region(TPC_BASE_PORT, TPC_NUM_PORTS);
+ return -ENXIO;
+ }
+ printk(KERN_INFO "thinkpad_ec: thinkpad_ec " TP_VERSION " loaded.\n");
+ return 0;
+}
+
+static void __exit thinkpad_ec_exit(void)
+{
+ release_region(TPC_BASE_PORT, TPC_NUM_PORTS);
+ printk(KERN_INFO "thinkpad_ec: unloaded.\n");
+}
+
+module_init(thinkpad_ec_init);
+module_exit(thinkpad_ec_exit);
diff -dNurp a/include/linux/thinkpad_ec.h b/include/linux/thinkpad_ec.h
--- a/include/linux/thinkpad_ec.h 1970-01-01 02:00:00.000000000 +0200
+++ b/include/linux/thinkpad_ec.h 2006-08-05 05:33:06.000000000 +0300
@@ -0,0 +1,47 @@
+/*
+ * thinkpad_ec.h - interface to ThinkPad embedded controller LPC3 functions
+ *
+ * Copyright (C) 2005 Shem Multinymous
+ *
+ * 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
+ */
+
+#ifndef _THINKPAD_EC_H
+#define _THINKPAD_EC_H
+
+#ifdef __KERNEL__
+
+#define TP_CONTROLLER_ROW_LEN 16
+
+/* EC transactions input and output (possibly partial) vectors of 16 bytes. */
+struct thinkpad_ec_row {
+ u16 mask; /* bitmap of which entries of val[] are meaningful */
+ u8 val[TP_CONTROLLER_ROW_LEN];
+};
+
+extern int thinkpad_ec_lock(void);
+extern int thinkpad_ec_try_lock(void);
+extern void thinkpad_ec_unlock(void);
+
+extern int thinkpad_ec_read_row(const struct thinkpad_ec_row *args,
+ struct thinkpad_ec_row *data);
+extern int thinkpad_ec_try_read_row(const struct thinkpad_ec_row *args,
+ struct thinkpad_ec_row *mask);
+extern int thinkpad_ec_prefetch_row(const struct thinkpad_ec_row *args);
+extern void thinkpad_ec_invalidate(void);
+
+
+#endif /* __KERNEL */
+#endif /* _THINKPAD_EC_H */
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to top
Shem Multinymous
External


Since: Aug 06, 2006
Posts: 10



PostPosted: Sun Aug 06, 2006 1:40 pm    Post subject: [PATCH 10/12] hdaps: Power off accelerometer on suspend and unload [Login to view extended thread Info.]
Archived from groups: per prev. post (more info?)

This patch disables accelerometer power and stops its polling by the
embedded controller upon suspend and module unload. The power saving
is negligible, but it's the right thing to do.

Signed-off-by: Shem Multinymous
---
hdaps.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff -up a/drivers/hwmon/hdaps.c a/drivers/hwmon/hdaps.c
--- a/drivers/hwmon/hdaps.c
+++ a/drivers/hwmon/hdaps.c
@@ -348,6 +348,15 @@ good:
return ret;
}

+/*
+ * hdaps_device_shutdown - power off the accelerometer. Can sleep.
+ */
+static void hdaps_device_shutdown(void) {
+ if (hdaps_set_power(0))
+ printk(KERN_WARNING "hdaps: cannot power off\n");
+ if (hdaps_set_ec_config(0, 1))
+ printk(KERN_WARNING "hdaps: cannot stop EC sampling\n");
+}

/* Device model stuff */

@@ -363,6 +372,12 @@ static int hdaps_probe(struct platform_d
return 0;
}

+static int hdaps_suspend(struct platform_device *dev, pm_message_t state)
+{
+ hdaps_device_shutdown();
+ return 0;
+}
+
static int hdaps_resume(struct platform_device *dev)
{
return hdaps_device_init();
@@ -370,6 +385,7 @@ static int hdaps_resume(struct platform_

static struct platform_driver hdaps_driver = {
.probe = hdaps_probe,
+ .suspend = hdaps_suspend,
.resume = hdaps_resume,
.driver = {
.name = "hdaps",
@@ -762,6 +778,7 @@ static void __exit hdaps_exit(void)
{
del_timer_sync(&hdaps_timer);
input_unregister_device(hdaps_idev);
+ hdaps_device_shutdown();
sysfs_remove_group(&pdev->dev.kobj, &hdaps_attribute_group);
platform_device_unregister(pdev);
platform_driver_unregister(&hdaps_driver);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to top
Shem Multinymous
External


Since: Aug 06, 2006
Posts: 10



PostPosted: Sun Aug 06, 2006 1:40 pm    Post subject: [PATCH 02/12] hdaps: Use thinkpad_ec instead of direct port access [Login to view extended thread Info.]
Archived from groups: per prev. post (more info?)

The hdaps module currently talks to the ThinkPad embedded controller via direct
IO port access, and does so incorrectly and with insufficient IO checking.
Beyond bad readouts, this could in principle, trigger known EC firmware bugs,
thereby causing a firmware hang and possible hardware damage.

The just-introduced thinkpad_ec provides a safe, correct way to access these
EC functions. This patch changes hdaps to use that. This also enables other
drivers (e.g., the soon-to-be-submitted tp_smapi module) to access the EC
without collisions.

As you can see from the comments at the beginning of the patch, the old
driver got some stuff wrong about the meaning of EC readouts. This patch
preserves the old broken behavior; it will be fixed in a later patch.

Signed-off-by: Shem Multinymous
---
a/drivers/hwmon/hdaps.c | 314 +++++++++++++++++-------------------------------
drivers/hwmon/Kconfig | 1
2 files changed, 116 insertions(+), 199 deletions(-)

diff -up a/drivers/hwmon/hdaps.c a/drivers/hwmon/hdaps.c
--- a/drivers/hwmon/hdaps.c
+++ a/drivers/hwmon/hdaps.c
@@ -33,31 +33,28 @@
#include <linux/module.h>
#include <linux/timer.h>
#include <linux/dmi.h>
-#include <asm/io.h>
+#include <linux/thinkpad_ec.h>

-#define HDAPS_LOW_PORT 0x1600 /* first port used by hdaps */
-#define HDAPS_NR_PORTS 0x30 /* number of ports: 0x1600 - 0x162f */
-
-#define HDAPS_PORT_STATE 0x1611 /* device state */
-#define HDAPS_PORT_YPOS 0x1612 /* y-axis position */
-#define HDAPS_PORT_XPOS 0x1614 /* x-axis position */
-#define HDAPS_PORT_TEMP1 0x1616 /* device temperature, in Celsius */
-#define HDAPS_PORT_YVAR 0x1617 /* y-axis variance (what is this?) */
-#define HDAPS_PORT_XVAR 0x1619 /* x-axis variance (what is this?) */
-#define HDAPS_PORT_TEMP2 0x161b /* device temperature (again?) */
-#define HDAPS_PORT_UNKNOWN 0x161c /* what is this? */
-#define HDAPS_PORT_KMACT 0x161d /* keyboard or mouse activity */
-
-#define STATE_FRESH 0x50 /* accelerometer data is fresh */
+/* Embedded controller accelerometer read command and its result: */
+static const struct thinkpad_ec_row ec_accel_args =
+ { .mask=0x0001, .val={0x11} };
+#define EC_ACCEL_IDX_READOUTS 0x1 /* readouts included in this read */
+ /* First readout, if READOUTS>=1: */
+#define EC_ACCEL_IDX_YPOS1 0x2 /* y-axis position word */
+#define EC_ACCEL_IDX_XPOS1 0x4 /* x-axis position word */
+#define EC_ACCEL_IDX_TEMP1 0x6 /* device temperature in Celsius */
+ /* Second readout, if READOUTS>=2: */
+#define EC_ACCEL_IDX_XPOS2 0x7 /* y-axis position word */
+#define EC_ACCEL_IDX_YPOS2 0x9 /* x-axis pisition word */
+#define EC_ACCEL_IDX_TEMP2 0xb /* device temperature in Celsius */
+#define EC_ACCEL_IDX_QUEUED 0xc /* Number of queued readouts left */
+#define EC_ACCEL_IDX_KMACT 0xd /* keyboard or mouse activity */

#define KEYBD_MASK 0x20 /* set if keyboard activity */
#define MOUSE_MASK 0x40 /* set if mouse activity */
#define KEYBD_ISSET(n) (!! (n & KEYBD_MASK)) /* keyboard used? */
#define MOUSE_ISSET(n) (!! (n & MOUSE_MASK)) /* mouse used? */

-#define INIT_TIMEOUT_MSECS 4000 /* wait up to 4s for device init ... */
-#define INIT_WAIT_MSECS 200 /* ... in 200ms increments */
-
#define HDAPS_POLL_PERIOD (HZ/20) /* poll for input every 1/20s */
#define HDAPS_INPUT_FUZZ 4 /* input event threshold */
#define HDAPS_INPUT_FLAT 4
@@ -70,79 +67,6 @@ static u8 km_activity;
static int rest_x;
static int rest_y;

-static DECLARE_MUTEX(hdaps_sem);
-
-/*
- * __get_latch - Get the value from a given port. Callers must hold hdaps_sem.
- */
-static inline u8 __get_latch(u16 port)
-{
- return inb(port) & 0xff;
-}
-
-/*
- * __check_latch - Check a port latch for a given value. Returns zero if the
- * port contains the given value. Callers must hold hdaps_sem.
- */
-static inline int __check_latch(u16 port, u8 val)
-{
- if (__get_latch(port) == val)
- return 0;
- return -EINVAL;
-}
-
-/*
- * __wait_latch - Wait up to 100us for a port latch to get a certain value,
- * returning zero if the value is obtained. Callers must hold hdaps_sem.
- */
-static int __wait_latch(u16 port, u8 val)
-{
- unsigned int i;
-
- for (i = 0; i < 20; i++) {
- if (!__check_latch(port, val))
- return 0;
- udelay(5);
- }
-
- return -EIO;
-}
-
-/*
- * __device_refresh - request a refresh from the accelerometer. Does not wait
- * for refresh to complete. Callers must hold hdaps_sem.
- */
-static void __device_refresh(void)
-{
- udelay(200);
- if (inb(0x1604) != STATE_FRESH) {
- outb(0x11, 0x1610);
- outb(0x01, 0x161f);
- }
-}
-
-/*
- * __device_refresh_sync - request a synchronous refresh from the
- * accelerometer. We wait for the refresh to complete. Returns zero if
- * successful and nonzero on error. Callers must hold hdaps_sem.
- */
-static int __device_refresh_sync(void)
-{
- __device_refresh();
- return __wait_latch(0x1604, STATE_FRESH);
-}
-
-/*
- * __device_complete - indicate to the accelerometer that we are done reading
- * data, and then initiate an async refresh. Callers must hold hdaps_sem.
- */
-static inline void __device_complete(void)
-{
- inb(0x161f);
- inb(0x1604);
- __device_refresh();
-}
-
/*
* hdaps_readb_one - reads a byte from a single I/O port, placing the value in
* the given pointer. Returns zero on success or a negative error on failure.
@@ -151,19 +75,15 @@ static inline void __device_complete(voi
static int hdaps_readb_one(unsigned int port, u8 *val)
{
int ret;
+ struct thinkpad_ec_row data;

- down(&hdaps_sem);
-
- /* do a sync refresh -- we need to be sure that we read fresh data */
- ret = __device_refresh_sync();
+ ret = thinkpad_ec_lock();
if (ret)
- goto out;
-
- *val = inb(port);
- __device_complete();
-
-out:
- up(&hdaps_sem);
+ return ret;
+ data.mask = (1 << port);
+ ret = thinkpad_ec_read_row(&ec_accel_args, &data);
+ *val = data.val[port];
+ thinkpad_ec_unlock();
return ret;
}

@@ -171,14 +91,17 @@ out:
static int __hdaps_read_pair(unsigned int port1, unsigned int port2,
int *x, int *y)
{
- /* do a sync refresh -- we need to be sure that we read fresh data */
- if (__device_refresh_sync())
- return -EIO;
+ int ret;
+ struct thinkpad_ec_row data;

- *y = inw(port2);
- *x = inw(port1);
- km_activity = inb(HDAPS_PORT_KMACT);
- __device_complete();
+ data.mask = (3 << port1) | (3 << port2) | (1 << EC_ACCEL_IDX_KMACT);
+ ret = thinkpad_ec_read_row(&ec_accel_args, &data);
+ if (ret)
+ return ret;
+
+ *x = *(s16*)(data.val+port1);
+ *y = *(s16*)(data.val+port2);
+ km_activity = data.val[EC_ACCEL_IDX_KMACT];

/* if hdaps_invert is set, negate the two values */
if (hdaps_invert) {
@@ -196,12 +119,11 @@ static int __hdaps_read_pair(unsigned in
static int hdaps_read_pair(unsigned int port1, unsigned int port2,
int *val1, int *val2)
{
- int ret;
-
- down(&hdaps_sem);
+ int ret = thinkpad_ec_lock();
+ if (ret)
+ return ret;
ret = __hdaps_read_pair(port1, port2, val1, val2);
- up(&hdaps_sem);
-
+ thinkpad_ec_unlock();
return ret;
}

@@ -209,77 +131,79 @@ static int hdaps_read_pair(unsigned int
* hdaps_device_init - initialize the accelerometer. Returns zero on success
* and negative error code on failure. Can sleep.
*/
+#define ABORT_INIT(msg) { printk(KERN_ERR "hdaps init: %s\n", msg); goto bad; }
static int hdaps_device_init(void)
{
- int total, ret = -ENXIO;
-
- down(&hdaps_sem);
-
- outb(0x13, 0x1610);
- outb(0x01, 0x161f);
- if (__wait_latch(0x161f, 0x00))
- goto out;
-
- /*
- * Most ThinkPads return 0x01.
- *
- * Others--namely the R50p, T41p, and T42p--return 0x03. These laptops
- * have "inverted" axises.
- *
- * The 0x02 value occurs when the chip has been previously initialized.
- */
- if (__check_latch(0x1611, 0x03) &&
- __check_latch(0x1611, 0x02) &&
- __check_latch(0x1611, 0x01))
- goto out;
-
- printk(KERN_DEBUG "hdaps: initial latch check good (0x%02x).\n",
- __get_latch(0x1611));
+ int ret;
+ struct thinkpad_ec_row args, data;
+ u8 mode;

- outb(0x17, 0x1610);
- outb(0x81, 0x1611);
- outb(0x01, 0x161f);
- if (__wait_latch(0x161f, 0x00))
- goto out;
- if (__wait_latch(0x1611, 0x00))
- goto out;
- if (__wait_latch(0x1612, 0x60))
- goto out;
- if (__wait_latch(0x1613, 0x00))
- goto out;
- outb(0x14, 0x1610);
- outb(0x01, 0x1611);
- outb(0x01, 0x161f);
- if (__wait_latch(0x161f, 0x00))
- goto out;
- outb(0x10, 0x1610);
- outb(0xc8, 0x1611);
- outb(0x00, 0x1612);
- outb(0x02, 0x1613);
- outb(0x01, 0x161f);
- if (__wait_latch(0x161f, 0x00))
- goto out;
- if (__device_refresh_sync())
- goto out;
- if (__wait_latch(0x1611, 0x00))
- goto out;
+ ret = thinkpad_ec_lock();
+ if (ret)
+ return ret;

- /* we have done our dance, now let's wait for the applause */
- for (total = INIT_TIMEOUT_MSECS; total > 0; total -= INIT_WAIT_MSECS) {
- int x, y;
-
- /* a read of the device helps push it into action */
- __hdaps_read_pair(HDAPS_PORT_XPOS, HDAPS_PORT_YPOS, &x, &y);
- if (!__wait_latch(0x1611, 0x02)) {
- ret = 0;
- break;
- }
+ args.val[0x0] = 0x13;
+ args.val[0xF] = 0x01;
+ args.mask=0x8001;
+ data.mask=0x8002;
+ if (thinkpad_ec_read_row(&args, &data))
+ ABORT_INIT("read1");
+ if (data.val[0xF]!=0x00)
+ ABORT_INIT("check1");
+ mode = data.val[0x1];
+
+ printk(KERN_DEBUG "hdaps: initial mode latch is 0x%02x\n", mode);
+ if (mode==0x00)
+ ABORT_INIT("accelerometer not available");
+
+ args.val[0x0] = 0x17;
+ args.val[0x1] = 0x81;
+ args.val[0xF] = 0x01;
+ args.mask = 0x8003;
+ data.mask = 0x800E;
+ if (thinkpad_ec_read_row(&args, &data))
+ ABORT_INIT("read2");
+ if (data.val[0x1]!=0x00 ||
+ data.val[0x2]!=0x60 ||
+ data.val[0x3]!=0x00 ||
+ data.val[0xF]!=0x00)
+ ABORT_INIT("check2");
+
+ args.val[0x0] = 0x14;
+ args.val[0x1] = 0x01;
+ args.val[0xF] = 0x01;
+ args.mask = 0x8003;
+ data.mask = 0x8000;
+ if (thinkpad_ec_read_row(&args, &data))
+ ABORT_INIT("read3");
+ if (data.val[0xF]!=0x00)
+ ABORT_INIT("check3");
+
+ args.val[0x0] = 0x10;
+ args.val[0x1] = 0xc8;
+ args.val[0x2] = 0x00;
+ args.val[0x3] = 0x02;
+ args.val[0xF] = 0x01;
+ args.mask = 0x800F;
+ data.mask = 0x8000;
+ if (thinkpad_ec_read_row(&args, &data))
+ ABORT_INIT("read4");
+ if (data.val[0xF]!=0x00)
+ ABORT_INIT("check4");

- msleep(INIT_WAIT_MSECS);
- }
+ thinkpad_ec_invalidate();
+ udelay(200);

-out:
- up(&hdaps_sem);
+ /* Just prefetch instead of reading, to avoid ~1sec delay on load */
+ ret = thinkpad_ec_prefetch_row(&ec_accel_args);
+ if (ret)
+ ABORT_INIT("initial prefetch failed");
+ goto good;
+bad:
+ thinkpad_ec_invalidate();
+ ret = -ENXIO;
+good:
+ thinkpad_ec_unlock();
return ret;
}

@@ -317,7 +241,7 @@ static struct platform_driver hdaps_driv
*/
static void hdaps_calibrate(void)
{
- __hdaps_read_pair(HDAPS_PORT_XPOS, HDAPS_PORT_YPOS, &rest_x, &rest_y);
+ __hdaps_read_pair(EC_ACCEL_IDX_XPOS1, EC_ACCEL_IDX_YPOS1, &rest_x, &rest_y);
}

static void hdaps_mousedev_poll(unsigned long unused)
@@ -325,12 +249,12 @@ static void hdaps_mousedev_poll(unsigned
int x, y;

/* Cannot sleep. Try nonblockingly. If we fail, try again later. */
- if (down_trylock(&hdaps_sem)) {
+ if (thinkpad_ec_try_lock()) {
mod_timer(&hdaps_timer,jiffies + HDAPS_POLL_PERIOD);
return;
}

- if (__hdaps_read_pair(HDAPS_PORT_XPOS, HDAPS_PORT_YPOS, &x, &y))
+ if (__hdaps_read_pair(EC_ACCEL_IDX_XPOS1, EC_ACCEL_IDX_YPOS1, &x, &y))
goto out;

input_report_abs(hdaps_idev, ABS_X, x - rest_x);
@@ -340,7 +264,7 @@ static void hdaps_mousedev_poll(unsigned
mod_timer(&hdaps_timer, jiffies + HDAPS_POLL_PERIOD);

out:
- up(&hdaps_sem);
+ thinkpad_ec_unlock();
}


@@ -351,7 +275,7 @@ static ssize_t hdaps_position_show(struc
{
int ret, x, y;

- ret = hdaps_read_pair(HDAPS_PORT_XPOS, HDAPS_PORT_YPOS, &x, &y);
+ ret = hdaps_read_pair(EC_ACCEL_IDX_XPOS1, EC_ACCEL_IDX_YPOS1, &x, &y);
if (ret)
return ret;

@@ -363,7 +287,7 @@ static ssize_t hdaps_variance_show(struc
{
int ret, x, y;

- ret = hdaps_read_pair(HDAPS_PORT_XVAR, HDAPS_PORT_YVAR, &x, &y);
+ ret = hdaps_read_pair(EC_ACCEL_IDX_XPOS2, EC_ACCEL_IDX_YPOS2, &x, &y);
if (ret)
return ret;

@@ -376,7 +300,7 @@ static ssize_t hdaps_temp1_show(struct d
u8 temp;
int ret;

- ret = hdaps_readb_one(HDAPS_PORT_TEMP1, &temp);
+ ret = hdaps_readb_one(EC_ACCEL_IDX_TEMP1, &temp);
if (ret < 0)
return ret;

@@ -389,7 +313,7 @@ static ssize_t hdaps_temp2_show(struct d
u8 temp;
int ret;

- ret = hdaps_readb_one(HDAPS_PORT_TEMP2, &temp);
+ ret = hdaps_readb_one(EC_ACCEL_IDX_TEMP2, &temp);
if (ret < 0)
return ret;

@@ -420,10 +344,10 @@ static ssize_t hdaps_calibrate_store(str
struct device_attribute *attr,
const char *buf, size_t count)
{
- down(&hdaps_sem);
+ if (thinkpad_ec_lock())
+ return -EIO;
hdaps_calibrate();
- up(&hdaps_sem);
-
+ thinkpad_ec_unlock();
return count;
}

@@ -550,14 +474,9 @@ static int __init hdaps_init(void)
goto out;
}

- if (!request_region(HDAPS_LOW_PORT, HDAPS_NR_PORTS, "hdaps")) {
- ret = -ENXIO;
- goto out;
- }
-
ret = platform_driver_register(&hdaps_driver);
if (ret)
- goto out_region;
+ goto out;

pdev = platform_device_register_simple("hdaps", -1, NULL, 0);
if (IS_ERR(pdev)) {
@@ -604,8 +523,6 @@ out_device:
platform_device_unregister(pdev);
out_driver:
platform_driver_unregister(&hdaps_driver);
-out_region:
- release_region(HDAPS_LOW_PORT, HDAPS_NR_PORTS);
out:
printk(KERN_WARNING "hdaps: driver init failed (ret=%d)!\n", ret);
return ret;
@@ -618,7 +535,6 @@ static void __exit hdaps_exit(void)
sysfs_remove_group(&pdev->dev.kobj, &hdaps_attribute_group);
platform_device_unregister(pdev);
platform_driver_unregister(&hdaps_driver);
- release_region(HDAPS_LOW_PORT, HDAPS_NR_PORTS);

printk(KERN_INFO "hdaps: driver unloaded.\n");
}
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -494,6 +494,7 @@
config SENSORS_HDAPS
tristate "IBM Hard Drive Active Protection System (hdaps)"
depends on HWMON && INPUT && X86
+ select THINKPAD_EC
default n
help
This driver provides support for the IBM Hard Drive Active Protection
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to top
Shem Multinymous
External


Since: Aug 06, 2006
Posts: 10



PostPosted: Sun Aug 06, 2006 1:40 pm    Post subject: [PATCH 12/12] hdaps: Simplify whitelist [Login to view extended thread Info.]
Archived from groups: per prev. post (more info?)

The hdaps driver can now reliably detect accelerometer hardware, as a
free bonus from switch to thinkpad_ec. The whitelist is thus needed
only for overriding the default axis configuratrion. This patch simplifies
the whitelist to reflect this. Behavior on previously working modules is
completely unaffected, and new models will work automatically (though not
necessarily with correct axis configuration).

Signed-off-by: Shem Multinymous
---
hdaps.c | 66 +++++++++++++---------------------------------------------------
1 file changed, 14 insertions(+), 52 deletions(-)

diff -up a/drivers/hwmon/hdaps.c a/drivers/hwmon/hdaps.c
--- a/drivers/hwmon/hdaps.c
+++ a/drivers/hwmon/hdaps.c
@@ -652,79 +652,41 @@ static struct attribute_group hdaps_attr

/* Module stuff */

-/* hdaps_dmi_match - found a match. return one, short-circuiting the hunt. */
-static int hdaps_dmi_match(struct dmi_system_id *id)
-{
- printk(KERN_INFO "hdaps: %s detected.\n", id->ident);
- return 1;
-}
-
/* hdaps_dmi_match_invert - found an inverted match. */
static int hdaps_dmi_match_invert(struct dmi_system_id *id)
{
hdaps_invert = 1;
- printk(KERN_INFO "hdaps: inverting axis readings.\n");
- return hdaps_dmi_match(id);
-}
-
-#define HDAPS_DMI_MATCH_NORMAL(model) { \
- .ident = "IBM " model, \
- .callback = hdaps_dmi_match, \
- .matches = { \
- DMI_MATCH(DMI_BOARD_VENDOR, "IBM"), \
- DMI_MATCH(DMI_PRODUCT_VERSION, model) \
- } \
+ printk(KERN_INFO "hdaps: %s detected, inverting axes\n",
+ id->ident);
+ return 1;
}

-#define HDAPS_DMI_MATCH_INVERT(model) { \
- .ident = "IBM " model, \
+#define HDAPS_DMI_MATCH_INVERT(vendor,model) { \
+ .ident = vendor " " model, \
.callback = hdaps_dmi_match_invert, \
.matches = { \
- DMI_MATCH(DMI_BOARD_VENDOR, "IBM"), \
+ DMI_MATCH(DMI_BOARD_VENDOR, vendor), \
DMI_MATCH(DMI_PRODUCT_VERSION, model) \
} \
}

-#define HDAPS_DMI_MATCH_LENOVO(model) { \
- .ident = "Lenovo " model, \
- .callback = hdaps_dmi_match_invert, \
- .matches = { \
- DMI_MATCH(DMI_BOARD_VENDOR, "LENOVO"), \
- DMI_MATCH(DMI_PRODUCT_VERSION, model) \
- } \
-}
-
static int __init hdaps_init(void)
{
int ret;

- /* Note that HDAPS_DMI_MATCH_NORMAL("ThinkPad T42") would match
+ /* List of models with abnormal axis configuration.
+ Note that HDAPS_DMI_MATCH_NORMAL("ThinkPad T42") would match
"ThinkPad T42p", so the order of the entries matters */
struct dmi_system_id hdaps_whitelist[] = {
- HDAPS_DMI_MATCH_NORMAL("ThinkPad H"),
- HDAPS_DMI_MATCH_INVERT("ThinkPad R50p"),
- HDAPS_DMI_MATCH_NORMAL("ThinkPad R50"),
- HDAPS_DMI_MATCH_NORMAL("ThinkPad R51"),
- HDAPS_DMI_MATCH_NORMAL("ThinkPad R52"),
- HDAPS_DMI_MATCH_NORMAL("ThinkPad H"), /* R52 (1846AQG) */
- HDAPS_DMI_MATCH_INVERT("ThinkPad T41p"),
- HDAPS_DMI_MATCH_NORMAL("ThinkPad T41"),
- HDAPS_DMI_MATCH_INVERT("ThinkPad T42p"),
- HDAPS_DMI_MATCH_NORMAL("ThinkPad T42"),
- HDAPS_DMI_MATCH_NORMAL("ThinkPad T43"),
- HDAPS_DMI_MATCH_LENOVO("ThinkPad T60p"),
- HDAPS_DMI_MATCH_NORMAL("ThinkPad X40"),
- HDAPS_DMI_MATCH_NORMAL("ThinkPad X41"),
- HDAPS_DMI_MATCH_LENOVO("ThinkPad X60"),
- HDAPS_DMI_MATCH_NORMAL("ThinkPad Z60m"),
+ HDAPS_DMI_MATCH_INVERT("IBM","ThinkPad R50p"),
+ HDAPS_DMI_MATCH_INVERT("IBM","ThinkPad T41p"),
+ HDAPS_DMI_MATCH_INVERT("IBM","ThinkPad T42p"),
+ HDAPS_DMI_MATCH_INVERT("LENOVO","ThinkPad T60p"),
+ HDAPS_DMI_MATCH_INVERT("LENOVO","ThinkPad X60"),
{ .ident = NULL }
};

- if (!dmi_check_system(hdaps_whitelist)) {
- printk(KERN_WARNING "hdaps: supported laptop not found!\n");
- ret = -ENODEV;
- goto out;
- }
+ dmi_check_system(hdaps_whitelist); /* default to normal axes */

/* Init timer before platform_driver_register, in case of suspend */
init_timer(&hdaps_timer);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to top
Andrew Morton
External


Since: Aug 22, 2004
Posts: 2442



PostPosted: Sun Aug 06, 2006 2:00 pm    Post subject: Re: [PATCH 01/12] thinkpad_ec: New driver for ThinkPad embedded controller access [Login to view extended thread Info.]
Archived from groups: per prev. post (more info?)

On Sun, 06 Aug 2006 10:26:46 +0300
Shem Multinymous wrote:

> Signed-off-by: Shem Multinymous

This rather defeats the purpose of the DCO. Perhaps one of the other
project members (ie: one who uses a real name) could also sign off the
patches?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to top
Shem Multinymous
External


Since: Jul 04, 2006
Posts: 50



PostPosted: Sun Aug 06, 2006 4:00 pm    Post subject: Re: [PATCH 01/12] thinkpad_ec: New driver for ThinkPad embedded controller access [Login to view extended thread Info.]
Archived from groups: per prev. post (more info?)

Hi,
On 8/6/06, Andrew Morton wrote:
> On Sun, 06 Aug 2006 10:26:46 +0300
> Shem Multinymous wrote:
>
> > Signed-off-by: Shem Multinymous
>
> This rather defeats the purpose of the DCO.

SubmittingPatches declares the purpose of the DCO as "To improve
tracking of who did what, especially with patches that can percolate
to their final resting place in the kernel through several layers of
maintainers."

I indeed certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

Practically speaking, this contact address is stable and reliable.

What more is needed that may be realistically expected from a kernel
patch submission?

Shem
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to top
Andrew Morton
External


Since: Aug 22, 2004
Posts: 2442



PostPosted: Sun Aug 06, 2006 4:10 pm    Post subject: Re: [PATCH 01/12] thinkpad_ec: New driver for ThinkPad embedded controller access [Login to view extended thread Info.]
Archived from groups: per prev. post (more info?)

On Sun, 6 Aug 2006 12:56:43 +0300
"Shem Multinymous" wrote:

> What more is needed that may be realistically expected from a kernel
> patch submission?

We who accept the submission would be making a joke of the whole thing if
we accepted the assurances of a person who is concealing his/her identity.

I suggested a simple solution: Perhaps one of the other project members
(ie: one who uses a real name) could also sign off the patches?


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to top
Shem Multinymous
External


Since: Jul 04, 2006
Posts: 50



PostPosted: Sun Aug 06, 2006 4:50 pm    Post subject: Re: [PATCH 01/12] thinkpad_ec: New driver for ThinkPad embedded controller access [Login to view extended thread Info.]
Archived from groups: per prev. post (more info?)

Hi,

On 8/6/06, Andrew Morton wrote:
> I suggested a simple solution: Perhaps one of the other project members
> (ie: one who uses a real name) could also sign off the patches?

Well, I offer this patch under the GPL, so anyone (including you) can do it.

But I would like to be able to submit further patches without "adult
supervision", so I hope you don't mind my pressing the issue:


> > What more is needed that may be realistically expected from a kernel
> > patch submission?
>
> We who accept the submission would be making a joke of the whole thing if
> we accepted the assurances of a person who is concealing his/her identity.

Can you please be more specific? What purpose does this exclusion
serve, that would be realistically achieved otherwise? You already
have a GPL license from the author, and a way to contact and uniquely
identify the author.

Shem
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to top
Theodore Tso
External


Since: Jun 05, 2006
Posts: 265



PostPosted: Sun Aug 06, 2006 9:00 pm    Post subject: Re: [PATCH 01/12] thinkpad_ec: New driver for ThinkPad embedded controller access [Login to view extended thread Info.]
Archived from groups: per prev. post (more info?)

On Sun, Aug 06, 2006 at 01:44:02PM +0300, Shem Multinymous wrote:
> On 8/6/06, Andrew Morton wrote:
> >I suggested a simple solution: Perhaps one of the other project members
> >(ie: one who uses a real name) could also sign off the patches?
>
> Well, I offer this patch under the GPL, so anyone (including you) can do it.

But who is "I", and how do we know that you really do have the legal
authority to offer the patch under the GPL? *That's* the whole point
of the DCO. It is a lightweight version of what the FSF requires,
which is a legal contract asserting the same, with an ink signature
and (at least in some cases) notarized by a public notary. The
contract has some interesting words in it, including ones where the
signer indemnifies the FSF against any claims for the work that has
been contributed, but it's all there to protect the FSF.

Some have claimed that the FSF approach is too slow, too heavyweight,
and discourages contributions, but there is good legal reasons for why
they ask that of their contributors. I might not be willing to sign
the FSF's contract, but that's a personal matter between me and the
FSF. (And given the FSF's position on the GPLv3 and my current
distaste of the discussion drafts, it's just as well that I don't
contribute code to the FSF.)

The DCO is a more lightweight mechanism, which tries to establish a
legal chain of accountability for patches. But in order to do that,
we need to know who is making the assertion, and a pseudonym defeats
the whole point of the DCO. No, we're not requiring an ink signature,
and no we're not requiring a notary public to check the identity of
the person signing the contract --- but then again, it's not that hard
to fake the driver's license which the notary public checks. The
point is that there is a process, and that we ask for a certain level
of assurance. The fact that the FSF doesn't require DNA samples
doesn't mean that they shouldn't stop doing what they are doing, just
as the fact that we aren't requiring ink signatures and public notary
checks doesn't mean we shouldn't stop doing what we are doing.

> Can you please be more specific? What purpose does this exclusion
> serve, that would be realistically achieved otherwise? You already
> have a GPL license from the author, and a way to contact and uniquely
> identify the author.

For legal reasons, we need a way to to contact and identify the author
in the real world, not just in cyberspace, and a pseudonym doesn't
meet that requirement.

- Ted
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to top
Olaf Hering
External


Since: Sep 27, 2006
Posts: 190



PostPosted: Sun Aug 06, 2006 10:50 pm    Post subject: Re: [PATCH 01/12] thinkpad_ec: New driver for ThinkPad embedded controller access [Login to view extended thread Info.]
Archived from groups: per prev. post (more info?)

On Sun, Aug 06, 2006 at 10:55:51AM -0400, Theodore Tso wrote:
> On Sun, Aug 06, 2006 at 01:44:02PM +0300, Shem Multinymous wrote:

> > Can you please be more specific? What purpose does this exclusion
> > serve, that would be realistically achieved otherwise? You already
> > have a GPL license from the author, and a way to contact and uniquely
> > identify the author.
>
> For legal reasons, we need a way to to contact and identify the author
> in the real world, not just in cyberspace, and a pseudonym doesn't
> meet that requirement.

In that context, even an anonymous mailer like gmail and the like is
questionable. But, I'm sure one get a domain with faked address data
in the whois database.
Where would you draw the line?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to top
Willy Tarreau
External


Since: Jun 11, 2006
Posts: 384



PostPosted: Sun Aug 06, 2006 11:10 pm    Post subject: Re: [PATCH 01/12] thinkpad_ec: New driver for ThinkPad embedded controller access [Login to view extended thread Info.]
Archived from groups: per prev. post (more info?)

On Sun, Aug 06, 2006 at 06:40:13PM +0200, Olaf Hering wrote:
> On Sun, Aug 06, 2006 at 10:55:51AM -0400, Theodore Tso wrote:
> > On Sun, Aug 06, 2006 at 01:44:02PM +0300, Shem Multinymous wrote:
>
> > > Can you please be more specific? What purpose does this exclusion
> > > serve, that would be realistically achieved otherwise? You already
> > > have a GPL license from the author, and a way to contact and uniquely
> > > identify the author.
> >
> > For legal reasons, we need a way to to contact and identify the author
> > in the real world, not just in cyberspace, and a pseudonym doesn't
> > meet that requirement.
>
> In that context, even an anonymous mailer like gmail and the like is
> questionable. But, I'm sure one get a domain with faked address data
> in the whois database.
> Where would you draw the line?

I think that if we have the date, the name, and Gmail has the logs, it
should be enough to get back to the real persons in case of future legal
trouble. Anyway, it's clearly possible that many contributors already
post under pseudonyms which do not designate them in person. After all,
it is already more or less the case for all those with non-latin alphabets.
Perhaps the only thing we're missing is a list of validated mail account
providers ? Anyway, how do we now that someone hosted by local provider X
my be mapped to a physical person ?

Maybe we should only make the difference between "intentionnal anonymity"
and free mail accounts, if this is possible.

Regards,
Willy

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to top
Andrew Morton
External


Since: Aug 22, 2004
Posts: 2442



PostPosted: Mon Aug 07, 2006 12:50 am    Post subject: Re: [PATCH 01/12] thinkpad_ec: New driver for ThinkPad embedded controller access [Login to view extended thread Info.]
Archived from groups: per prev. post (more info?)

On Sun, 6 Aug 2006 18:40:13 +0200
Olaf Hering wrote:

> On Sun, Aug 06, 2006 at 10:55:51AM -0400, Theodore Tso wrote:
> > On Sun, Aug 06, 2006 at 01:44:02PM +0300, Shem Multinymous wrote:
>
> > > Can you please be more specific? What purpose does this exclusion
> > > serve, that would be realistically achieved otherwise? You already
> > > have a GPL license from the author, and a way to contact and uniquely
> > > identify the author.
> >
> > For legal reasons, we need a way to to contact and identify the author
> > in the real world, not just in cyberspace, and a pseudonym doesn't
> > meet that requirement.
>
> In that context, even an anonymous mailer like gmail and the like is
> questionable. But, I'm sure one get a domain with faked address data
> in the whois database.
> Where would you draw the line?

I have a personal line, and that is when the patch is "substantial". (This
line is only relevant when someone forgot to add the Signed-off-by: and I'm
wondering whether to ask them to send one).

And I'd say this patch series _is_ substantial because it pokes at
registers which might be described in confidential/NDA'ed documentation, or
in ways which might be derived from $OTHER_OS.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to top
Arjan van de Ven
External


Since: May 15, 2006
Posts: 922



PostPosted: Mon Aug 07, 2006 1:00 am    Post subject: Re: [PATCH 01/12] thinkpad_ec: New driver for ThinkPad embedded controller access [Login to view extended thread Info.]
Archived from groups: per prev. post (more info?)

On Sun, 2006-08-06 at 13:44 +0300, Shem Multinymous wrote:
> Hi,
>
> On 8/6/06, Andrew Morton wrote:
> > I suggested a simple solution: Perhaps one of the other project members
> > (ie: one who uses a real name) could also sign off the patches?
>
> Well, I offer this patch under the GPL, so anyone (including you) can do it.
>
> But I would like to be able to submit further patches without "adult
> supervision", so I hope you don't mind my pressing the issue:
>


Open source is all about trust. Person A takes a patch from person B
because person A has trust in B (conditional on the patch meeting a
technical standard). In B's technical ability, in B's intentions, in B's
sincerity, in B's honesty when he says "this is my work and you can use
it because nobody but me has a claim on this".

Using a fake name is not a good way to gain such trust... At all.
Explicitly refusing to say who you really are just lowers the trust even
more, because it gives a strong appearance that something really fishy
is going on.

Personally I would be extremely hesitant to take any patches from anyone
who refuses to give out his real name. Because trust is about a person.
Because trust means not having to hide things that matter. And a persons
identity does matter for taking patches.

--
if you want to mail me at work (you don't), use arjan (at) linux.intel.com

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to top
Shem Multinymous
External


Since: Jul 04, 2006
Posts: 50



PostPosted: Mon Aug 07, 2006 4:10 am    Post subject: Re: [PATCH 01/12] thinkpad_ec: New driver for ThinkPad embedded controller access [Login to view extended thread Info.]
Archived from groups: per prev. post (more info?)

Hi Ted,

Thanks for the explanation. Point taken, though I can't help parsing it as:

On 8/6/06, Theodore Tso wrote:
> For legal reasons, we need a way to to contact and identify the author
> in the real world, not just in cyberspace, and a pseudonym doesn't
> meet that requirement.

"We want to be able to sue you if they sue us."

Which is actually not a problem for me (i.e., I don't believe I have
nothing to worry about legally); but I do have other, non-legal
considerations.


> just as the fact that we aren't requiring ink signatures and public notary
> checks doesn't mean we shouldn't stop doing what we are doing.

Understood, but still a bit silly. You have no idea how many of the
2252 people in `git-whatchanged | grep Signed-off-by: | sort | uniq`
gave their legal name, and I doubt you could contact most of them in
the real world without their cooperation (and with my cooperation, you
could contact me too). Heck, some of those email domains don't even
resolve. So this "chain of responsibiliy" is pretty worthless if
someone really tries to inject legally malicious code into mainline,
i.e., you end up blindly trusting people anyway.

BTW, Ted, we actually have met in person. Smile

Shem
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to top
Shem Multinymous
External


Since: Jul 04, 2006
Posts: 50



PostPosted: Mon Aug 07, 2006 4:40 am    Post subject: Re: [PATCH 01/12] thinkpad_ec: New driver for ThinkPad embedded controller access [Login to view extended thread Info.]
Archived from groups: per prev. post (more info?)

On 8/6/06, Andrew Morton wrote:
> And I'd say this patch series _is_ substantial because it pokes at
> registers which might be described in confidential/NDA'ed documentation, or
> in ways which might be derived from $OTHER_OS.

For what it's worth to you:
I hereby declare that this patch was developed solely based on public
specifications, observation of hardware behavior by trial&eror, and
specifications made available to me in clean-room settings and with no
attached obligations. So this patch is as pure as the mainline hdaps
driver it fixes (and probably purer than many other drivers), and not
a single line of it is a derivative work of $OTHER_OS code.

If it would help inspire trust, you can look at the tp_smapi revision
history on sf.net, where many of those trials and errors are
immortalized.

As for the register poking, I believe all the code in thinkpad_ec.c
logically follows from the publicly available H8S documentation (see
link at the top of the sourcecode), except for one number (base port
"0x1600") which is already given by the mainline hdaps driver.

Shem
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to top
Shem Multinymous
External


Since: Jul 04, 2006
Posts: 50



PostPosted: Mon Aug 07, 2006 4:50 am    Post subject: Re: [PATCH 01/12] thinkpad_ec: New driver for ThinkPad embedded controller access [Login to view extended thread Info.]
Archived from groups: per prev. post (more info?)

Hi Arjan,

On 8/6/06, Arjan van de Ven wrote:
> Open source is all about trust. Person A takes a patch from person B
> because person A has trust in B (conditional on the patch meeting a
> technical standard). In B's technical ability, in B's intentions, in B's
> sincerity, in B's honesty when he says "this is my work and you can use
> it because nobody but me has a claim on this".

Excellent points.


> Using a fake name is not a good way to gain such trust... At all.
> Explicitly refusing to say who you really are just lowers the trust even
> more, because it gives a strong appearance that something really fishy
> is going on.

Agreed. But this is only a heuristic; maybe in this case nothing fishy
is going on, whereas many nice-looking patches would reek to heaven if
inspected more closely.

So how about exercising judgment? Look up the tp_smapi development
history on sf.net (under project "tpctl"), the relevant posts on
thinkpad-devel, hdaps-devel and even a bit on lkml, and see if this
looks like standard, clean kernel development or a sinister attempt to
steal our precious bolidy fluids.

Shem
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to top
Greg KH
External


Since: May 16, 2006
Posts: 2242



PostPosted: Mon Aug 07, 2006 5:00 am    Post subject: Re: [PATCH 01/12] thinkpad_ec: New driver for ThinkPad embedded controller access [Login to view extended thread Info.]
Archived from groups: per prev. post (more info?)

On Mon, Aug 07, 2006 at 01:41:27AM +0300, Shem Multinymous wrote:
> On 8/6/06, Arjan van de Ven wrote:
> >Open source is all about trust. Person A takes a patch from person B
> >because person A has trust in B (conditional on the patch meeting a
> >technical standard). In B's technical ability, in B's intentions, in B's
> >sincerity, in B's honesty when he says "this is my work and you can use
> >it because nobody but me has a claim on this".
>
> Excellent points.
>
> >Using a fake name is not a good way to gain such trust... At all.
> >Explicitly refusing to say who you really are just lowers the trust even
> >more, because it gives a strong appearance that something really fishy
> >is going on.
>
> Agreed. But this is only a heuristic; maybe in this case nothing fishy
> is going on, whereas many nice-looking patches would reek to heaven if
> inspected more closely.

But since something obviously is fishy here, we can't accept them,
sorry.

I suggest trying to resove the other issues that are preventing you from
using your real name, and then resubmit these patches. As it is,
unfortunatly, we are not going to be able to accept this work.

good luck,

greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to top
Display posts from previous:   
Post new topic   General Reply to Topic (not reply to a specific post)    Forums Home -> Kernel (archive) All times are: Eastern Time (US & Canada)
Goto page 1, 2, 3, 4
Page 1 of 4

 
You can post new topics in this forum
You can reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum