Skip to contentSkip to conversation

IBM HDAPS accelerometer driver.

PATCH In review Robert Love 16 participants 35 messages Aug 26, 2005 – Aug 31, 2005 Hardware monitoring

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 IBM HDAPS accelerometer driver.

Andrew,

Of late I have been working on a driver for the IBM Hard Drive Active Protection System (HDAPS), which provides a two-axis accelerometer and some other misc. data. The hardware is found on recent IBM ThinkPad laptops.

The following patch adds the driver to 2.6.13-rc6-mm2. It is self-contained and fairly simple.

Please, apply.

Robert Love

Driver for the IBM HDAPS, an accelerometer

 MAINTAINERS            |    7 
 drivers/hwmon/Kconfig  |   17 +
 drivers/hwmon/Makefile |    1 
 drivers/hwmon/hdaps.c  |  594 +++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 619 insertions(+)
drivers/hwmon/hdaps.c +594 -0
diff -urN linux-2.6.13-rc6-mm2/drivers/hwmon/hdaps.c linux/drivers/hwmon/hdaps.c
@@ -0,0 +1,594 @@
+/*
+ * drivers/hwmon/hdaps.c - driver for IBM's Hard Drive Active Protection System
+ *
+ * Copyright (C) 2005 Robert Love <[email protected]> 
+ * Copyright (C) 2005 Jesper Juhl <[email protected]> 
+ *
+ * The HardDisk Active Protection System (hdaps) is present in the IBM ThinkPad
+ * T41, T42, T43, and R51, at least.  It provides a basic two-axis
+ * accelerometer and other misc. data.
+ *
+ * Based on the document by Mark A. Smith available at
+ * http://www.almaden.ibm.com/cs/people/marksmith/tpaps.html and a lot of trial
+ * and error.
+ *
+ * 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/delay.h>
+#include <linux/device.h>
+#include <linux/input.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/timer.h>
+#include <linux/spinlock.h>
+#include <asm/io.h>
+
+#define HDAPS_LOW_PORT		0x1600	/* first port used by accelerometer */
+#define HDAPS_NR_PORTS		0x30	/* number of ports (0x1600 - 0x162f) */
+
+#define STATE_FRESH		0x50	/* accelerometer data is fresh */
+
+#define REFRESH_ASYNC		0x00	/* do asynchronous refresh */
+#define REFRESH_SYNC		0x01	/* do synchronous refresh */
+
+#define HDAPS_PORT_STATE	0x1611	/* device state */
+#define	HDAPS_PORT_XPOS		0x1612	/* x-axis position */
+#define HDAPS_PORT_YPOS		0x1614	/* y-axis position */
+#define HDAPS_PORT_TEMP		0x1616	/* device temperature, in celcius */
+#define HDAPS_PORT_XVAR		0x1617	/* x-axis variance (what is this?) */
+#define HDAPS_PORT_YVAR		0x1619	/* y-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 HDAPS_READ_MASK		0xff	/* some reads have the low 8 bits set */
+
+#define KEYBD_MASK		0x20	/* set if keyboard activity */
+#define MOUSE_MASK		0x40	/* set if mouse activity */
+
+#define KEYBD_ISSET(n)	(!! (n & KEYBD_MASK))
+#define MOUSE_ISSET(n)	(!! (n & MOUSE_MASK))
+
+static spinlock_t hdaps_lock = SPIN_LOCK_UNLOCKED;
+
+
+/*
+ * __get_latch - Get the value from a given port latch.  Callers must hold
+ * hdaps_lock.
+ */
+static inline unsigned short __get_latch(unsigned short port)
+{
+	return inb(port) & HDAPS_READ_MASK;
+}
+
+/*
+ * __check_latch - Check a port latch for a given value.  Callers must hold
+ * hdaps_lock.
+ */
+static inline unsigned int __check_latch(unsigned short port, unsigned char val)
+{
+	if (__get_latch(port) == val)
+		return 1;
+	return 0;
+}
+
+/*
+ * __wait_latch - Wait up to 100us for a port latch to get a certain value,
+ * returning nonzero if the value is obtained and zero otherwise.  Callers
+ * must hold hdaps_lock.
+ */
+static unsigned int __wait_latch(unsigned short port, unsigned char val)
+{
+	unsigned int i;
+
+	for (i = 0; i < 20; i++) {
+		if (__check_latch(port, val))
+			return 1;
+		udelay(5);
+	}
+
+#if 0
+	printk(KERN_WARNING "hdaps: wait on %04x returned %02x, not %02x!\n",
+	       port, __check_latch(port, val), val);
+#endif
+
+	return 0;
+}
+
+/*
+ * __request_refresh - Request a refresh from the accelerometer.
+ *
+ * If sync is REFRESH_SYNC, we perform a synchronous refresh and will wait for
+ * the refresh.  Returns nonzero if successful or zero on error.
+ *
+ * If sync is REFRESH_ASYNC, we merely kick off a new refresh if the device is
+ * not up-to-date.  Always returns true.  On the next read from the device, the
+ * data should be up-to-date but a synchronous wait should be performed to be
+ * sure.
+ * 
+ * Callers must hold hdaps_lock.
+ */
+static int __request_refresh(int sync)
+{
+	unsigned char state;
+
+	state = inb(0x1604);
+	if (state == STATE_FRESH)
+		return 1;
+	else {
+		outb(0x11, 0x1610);
+		outb(0x01, 0x161f);
+		if (sync == REFRESH_ASYNC)
+			return 1;
+	}
+
+	return __wait_latch(0x1604, STATE_FRESH);
+}
+
+/*
+ * __tell_accelerometer_done - Indicate to the accelerometer that we are done
+ * reading data.  Callers must hold hdaps_lock.
+ */
+static inline void __tell_accelerometer_done(void)
+{
+	inb(0x161f);
+	inb(0x1604);
+}
+
+/* internal lockless helper for accelerometer_readb_one */
+static int __accelerometer_readb_one(unsigned int port, u8 *val)
+{
+	int ret = 0;
+
+	/* do a sync refresh - we need to be sure we read fresh data */
+	if (unlikely(!__request_refresh(REFRESH_SYNC))) {
+		ret = -EIO;
+		goto out;
+	}
+
+	*val = inb(port);
+
+	__tell_accelerometer_done();
+
+	if (unlikely(!__request_refresh(REFRESH_ASYNC)))
+		ret = -EIO;
+
+out:
+	return ret;
+}
+
+/*
+ * accelerometer_readb_one - reads a byte from a single given I/O port,
+ * placing the value in the given pointer.  Returns zero on success or a
+ * negative error on failure.
+ */
+static int accelerometer_readb_one(unsigned int port, u8 *val)
+{
+	int ret = 0;
+
+	spin_lock(&hdaps_lock);
+	ret = __accelerometer_readb_one(port, val);
+	spin_unlock(&hdaps_lock);
+	return ret;
+}
+
+/*
+ * accelerometer_read_pair - reads the values from a given pair of I/O ports,
+ * placing the values in the given pointers.  Returns zero on success or a
+ * negative error on failure.
+ */
+static int accelerometer_read_pair(unsigned int port1, unsigned int port2,
+				   int *val1, int *val2)
+{
+	int ret = 0;
+
+	spin_lock(&hdaps_lock);
+
+	/* do a sync refresh - we need to be sure we read fresh data */
+	if (unlikely(!__request_refresh(REFRESH_SYNC))) {
+		ret = -EIO;
+		goto out;
+	}
+
+	*val1 = inw(port1);
+	*val2 = inw(port2);
+
+	__tell_accelerometer_done();
+
+	if (unlikely(!__request_refresh(REFRESH_ASYNC)))
+		ret = -EIO;
+
+out:
+	spin_unlock(&hdaps_lock);
+	return ret;
+}
+
+#define INIT_TIMEOUT_MSECS	4000	/* wait up to 4s for device init ... */
+#define INIT_WAIT_MSECS		200	/* ... in 200ms increments */
+
+/* initialize the accelerometer */
+static int accelerometer_init(void)
+{
+	unsigned int total_msecs = INIT_TIMEOUT_MSECS;
+	unsigned int msecs_per_wait = INIT_WAIT_MSECS;
+	int ret = -EIO;
+
+	spin_lock(&hdaps_lock);
+
+	outb(0x13, 0x1610);
+	outb(0x01, 0x161f);
+	if (unlikely(!__wait_latch(0x161f, 0x00)))
+		goto out;
+
+	/*
+	 * The 0x3 value appears to only work on some thinkpads, such as the
+	 * T42p.  Others return 0x1.
+	 *
+	 * The 0x2 value occurs when the chip has been previously initialized.
+	 */	
+	if (unlikely(!__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));
+
+	outb(0x17, 0x1610);
+	outb(0x81, 0x1611);
+	outb(0x01, 0x161f);
+	if (unlikely(!__wait_latch(0x161f, 0x00)))
+		goto out;
+	if (unlikely(!__wait_latch(0x1611, 0x00)))
+		goto out;
+	if (unlikely(!__wait_latch(0x1612, 0x60)))
+		goto out;
+	if (unlikely(!__wait_latch(0x1613, 0x00)))
+		goto out;
+	outb(0x14, 0x1610);
+	outb(0x01, 0x1611);
+	outb(0x01, 0x161f);
+	if (unlikely(!__wait_latch(0x161f, 0x00)))
+		goto out;
+	outb(0x10, 0x1610);
+	outb(0xc8, 0x1611);
+	outb(0x00, 0x1612);
+	outb(0x02, 0x1613);
+	outb(0x01, 0x161f);
+	if (unlikely(!__wait_latch(0x161f, 0x00)))
+		goto out;
+	if (unlikely(!__request_refresh(REFRESH_SYNC)))
+		goto out;
+	if (unlikely(!__wait_latch(0x1611, 0x00)))
+		goto out;
+
+	/* we have done our dance, now let's wait for the applause */
+	ret = -ENXIO;
+	while (total_msecs > 0) {
+		u8 ignored;
+
+		/* a read of the device helps push it into action */
+		__accelerometer_readb_one(HDAPS_PORT_TEMP2, &ignored);
+		if (__wait_latch(0x1611, 0x02)) {
+			ret = 0;
+			break;
+		}
+		spin_unlock(&hdaps_lock);
+
+		msleep(msecs_per_wait);
+		total_msecs -= msecs_per_wait;
+
+		spin_lock(&hdaps_lock);
+	}
+
+out:
+	spin_unlock(&hdaps_lock);
+	return ret;
+}
+
+
+/* device class stuff */
+
+static DECLARE_COMPLETION(hdaps_obj_is_free);
+static void hdaps_release_dev(struct device *dev)
+{
+	complete(&hdaps_obj_is_free);
+}
+
+static int hdaps_resume(struct device* dev, u32 level)
+{
+	return accelerometer_init();
+}
+
+static struct device_driver hdaps_driver = {
+	.owner = THIS_MODULE,
+	.name = "hdaps",
+	.bus = &platform_bus_type,
+	.resume = hdaps_resume,
+};
+
+struct platform_device hdaps_plat_dev = {
+	.name = "hdaps",
+	.id = -1,
+	.dev = {
+		.release = hdaps_release_dev,
+		.driver = &hdaps_driver,
+	}
+};
+
+
+/* Input device stuff */
+
+static struct input_dev hdaps_idev;
+static struct timer_list hdaps_poll_timer;
+static unsigned int hdaps_mousedev_threshold = 4;
+static unsigned long hdaps_poll_ms = 25;
+static int hdaps_mousedev_registered;
+static u16 rest_x;
+static u16 rest_y;
+
+static void hdaps_calibrate(void)
+{
+	int x, y, ret;
+
+	ret = accelerometer_read_pair(HDAPS_PORT_XPOS, HDAPS_PORT_YPOS, &x, &y);
+	if (unlikely(ret))
+		return;
+
+	rest_x = x;
+	rest_y = y;
+}
+
+static void hdaps_mousedev_poll(unsigned long unused)
+{
+	int movex, movey, x, y, ret;
+
+	ret = accelerometer_read_pair(HDAPS_PORT_XPOS, HDAPS_PORT_YPOS, &x, &y);
+	if (unlikely(ret))
+		return;
+
+	movex = rest_x - x;
+	movey = rest_y - y;
+	if (abs(movex) > hdaps_mousedev_threshold)
+		input_report_rel(&hdaps_idev, REL_Y, movex);
+	if (abs(movey) > hdaps_mousedev_threshold)
+		input_report_rel(&hdaps_idev, REL_X, movey);
+	input_sync(&hdaps_idev);
+
+	mod_timer(&hdaps_poll_timer, jiffies + msecs_to_jiffies(hdaps_poll_ms));
+}
+
+static void hdaps_mousedev_enable(void)
+{
+	/* calibrate the device before enabling */
+	hdaps_calibrate();
+
+	init_input_dev(&hdaps_idev);
+	hdaps_idev.dev = &hdaps_plat_dev.dev;	
+	hdaps_idev.evbit[0] = BIT(EV_KEY) | BIT(EV_REL);
+	hdaps_idev.relbit[0] = BIT(REL_X) | BIT(REL_Y);
+	hdaps_idev.keybit[LONG(BTN_LEFT)] = BIT(BTN_LEFT);
+	input_register_device(&hdaps_idev);
+
+	hdaps_mousedev_registered = 1;
+
+	init_timer(&hdaps_poll_timer);
+	hdaps_poll_timer.function = hdaps_mousedev_poll;
+	hdaps_poll_timer.expires = jiffies + msecs_to_jiffies(hdaps_poll_ms);
+	add_timer(&hdaps_poll_timer);
+
+	printk(KERN_INFO "hdaps: input device enabled\n");
+}
+
+static void hdaps_mousedev_disable(void)
+{
+	if (!hdaps_mousedev_registered)
+		return;
+
+	del_timer_sync(&hdaps_poll_timer);
+	input_unregister_device(&hdaps_idev);
+}
+
+
+/* Sysfs Files */
+
+static ssize_t hdaps_position_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	int ret, x, y;
+
+	ret = accelerometer_read_pair(HDAPS_PORT_XPOS, HDAPS_PORT_YPOS, &x, &y);
+	if (unlikely(ret))
+		return ret;
+
+	return sprintf(buf, "(%d,%d)\n", x, y);
+}
+static DEVICE_ATTR(position, 0444, hdaps_position_show, NULL);
+
+static ssize_t hdaps_variance_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	int ret, x, y;
+
+	ret = accelerometer_read_pair(HDAPS_PORT_XVAR, HDAPS_PORT_YVAR, &x, &y);
+	if (unlikely(ret))
+		return ret;
+
+	return sprintf(buf, "(%d,%d)\n", x, y);
+}
+static DEVICE_ATTR(variance, 0444, hdaps_variance_show, NULL);
+
+static ssize_t hdaps_temp_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	u8 temp;
+	int ret;
+
+	ret = accelerometer_readb_one(HDAPS_PORT_TEMP, &temp);
+	if (unlikely(ret < 0))
+		return ret;
+
+	return sprintf(buf, "%u\n", temp);
+}
+static DEVICE_ATTR(temp, 0444, hdaps_temp_show, NULL);
+
+static ssize_t hdaps_mousedev_show(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%d\n", hdaps_mousedev_registered);
+}
+
+static ssize_t hdaps_mousedev_store(struct device *dev,
+				    struct device_attribute *attr,
+				    const char *buf, size_t count)
+{
+	int enable;
+
+	if (sscanf(buf, "%d\n", &enable) != 1)
+		return -EINVAL;
+
+	if (enable == 1)
+		hdaps_mousedev_enable();
+	else if (enable == 0)
+		hdaps_mousedev_disable();
+
+	return count;
+}
+
+static DEVICE_ATTR(mousedev, 0644, hdaps_mousedev_show, hdaps_mousedev_store);
+
+static ssize_t hdaps_calibrate_store(struct device *dev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	hdaps_calibrate();
+	return count;
+}
+static DEVICE_ATTR(calibrate, 0200, NULL, hdaps_calibrate_store);
+
+static ssize_t hdaps_threshold_show(struct device *dev,
+				    struct device_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%u\n", hdaps_mousedev_threshold);
+}
+
+static ssize_t hdaps_threshold_store(struct device *dev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	unsigned int threshold;
+
+	if (sscanf(buf, "%u\n", &threshold) != 1 || threshold == 0)
+		return -EINVAL;
+	hdaps_mousedev_threshold = threshold;
+
+	return count;
+}
+
+static DEVICE_ATTR(mousedev_threshold, 0644, hdaps_threshold_show,
+		   hdaps_threshold_store);
+
+static ssize_t hdaps_poll_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%lu\n", hdaps_poll_ms);
+}
+
+static ssize_t hdaps_poll_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	unsigned int poll;
+
+	if (sscanf(buf, "%u\n", &poll) != 1 || poll == 0)
+		return -EINVAL;
+	hdaps_poll_ms = poll;
+
+	return count;
+}
+
+static DEVICE_ATTR(mousedev_poll_ms, 0644, hdaps_poll_show, hdaps_poll_store);
+
+
+/* Module stuff */
+
+static unsigned int mousedev;
+module_param(mousedev, bool, 0);
+MODULE_PARM_DESC(mousedev, "enable the input class device on module load");
+
+static int __init hdaps_init(void)
+{
+	int ret;
+
+	if (unlikely(!request_region(HDAPS_LOW_PORT, HDAPS_NR_PORTS, "hdaps")))
+		return -ENXIO;
+
+	ret = accelerometer_init();
+	if (unlikely(ret))
+		goto out_release;
+
+	ret = driver_register(&hdaps_driver);
+	if (unlikely(ret))
+		goto out_release;
+
+	ret = platform_device_register(&hdaps_plat_dev);
+	if (unlikely(ret))
+		goto out_driver;
+
+	device_create_file(&hdaps_plat_dev.dev, &dev_attr_position);
+	device_create_file(&hdaps_plat_dev.dev, &dev_attr_variance);
+	device_create_file(&hdaps_plat_dev.dev, &dev_attr_temp);
+	device_create_file(&hdaps_plat_dev.dev, &dev_attr_calibrate);
+	device_create_file(&hdaps_plat_dev.dev, &dev_attr_mousedev);
+	device_create_file(&hdaps_plat_dev.dev, &dev_attr_mousedev_threshold);
+	device_create_file(&hdaps_plat_dev.dev, &dev_attr_mousedev_poll_ms);
+
+	if (mousedev)
+		hdaps_mousedev_enable();
+
+	printk(KERN_INFO "hdaps: initialized.\n");
+
+	return 0;
+
+out_driver:
+	driver_unregister(&hdaps_driver);
+out_release:
+	release_region(HDAPS_LOW_PORT, HDAPS_NR_PORTS);
+	printk(KERN_WARNING "hdaps: initilization failed! ret=%d\n", ret);
+	return ret;
+}
+
+static void __exit hdaps_exit(void)
+{
+	hdaps_mousedev_disable();
+
+	device_remove_file(&hdaps_plat_dev.dev, &dev_attr_mousedev_poll_ms);
+	device_remove_file(&hdaps_plat_dev.dev, &dev_attr_mousedev_threshold);
+	device_remove_file(&hdaps_plat_dev.dev, &dev_attr_mousedev);
+	device_remove_file(&hdaps_plat_dev.dev, &dev_attr_temp);
+	device_remove_file(&hdaps_plat_dev.dev, &dev_attr_calibrate);
+	device_remove_file(&hdaps_plat_dev.dev, &dev_attr_variance);
+	device_remove_file(&hdaps_plat_dev.dev, &dev_attr_position);
+	platform_device_unregister(&hdaps_plat_dev);
+	driver_unregister(&hdaps_driver);
+
+	release_region(HDAPS_LOW_PORT, HDAPS_NR_PORTS);
+}
+
+module_init(hdaps_init);
+module_exit(hdaps_exit);
+
+MODULE_LICENSE("GPL");
drivers/hwmon/Kconfig +17 -0
diff -urN linux-2.6.13-rc6-mm2/drivers/hwmon/Kconfig linux/drivers/hwmon/Kconfig
@@ -411,6 +411,23 @@
 	  This driver can also be built as a module.  If so, the module
 	  will be called w83627ehf.
 
+config SENSORS_HDAPS
+	tristate "IBM Hard Drive Active Protection System (hdaps)"
+	depends on HWMON
+	default n
+	help
+	  This driver provides support for the IBM Hard Drive Active Protection
+	  System (hdaps), which provides an accelerometer and other misc. data.
+	  Supported laptops include the IBM ThinkPad T41, T42, T43, and R51.
+	  The accelerometer data is readable via sysfs.
+
+	  This driver also provides an input class device, allowing the
+	  laptop to act as a pinball machine-esque mouse.  This is off by
+	  default but enabled via sysfs or the module parameter "mousedev".
+	  
+	  Say Y here if you have an applicable laptop and want to experience
+	  the awesome power of hdaps.
+
 config HWMON_DEBUG_CHIP
 	bool "Hardware Monitoring Chip debugging messages"
 	depends on HWMON
drivers/hwmon/Makefile +1 -0
diff -urN linux-2.6.13-rc6-mm2/drivers/hwmon/Makefile linux/drivers/hwmon/Makefile
@@ -22,6 +22,7 @@
 obj-$(CONFIG_SENSORS_FSCPOS)	+= fscpos.o
 obj-$(CONFIG_SENSORS_GL518SM)	+= gl518sm.o
 obj-$(CONFIG_SENSORS_GL520SM)	+= gl520sm.o
+obj-$(CONFIG_SENSORS_HDAPS)	+= hdaps.o
 obj-$(CONFIG_SENSORS_IT87)	+= it87.o
 obj-$(CONFIG_SENSORS_LM63)	+= lm63.o
 obj-$(CONFIG_SENSORS_LM75)	+= lm75.o
MAINTAINERS +7 -0
diff -urN linux-2.6.13-rc6-mm2/MAINTAINERS linux/MAINTAINERS
@@ -984,6 +984,13 @@
 W:	http://www.nyx.net/~arobinso
 S:	Maintained
 
+HDAPS
+P:	Robert Love
+M:	[email protected]
+M:	[email protected]
+W:	http://www.kernel.org/pub/linux/kernel/people/rml/hdaps/
+S:	Maintained
+
 HFS FILESYSTEM
 P:	Roman Zippel
 M:	[email protected]
#1 lore raw
Quoted 4 lines · Robert Love

Of late I have been working on a driver for the IBM Hard Drive Active Protection System (HDAPS), which provides a two-axis accelerometer and some other misc. data. The hardware is found on recent IBM ThinkPad laptops.

How does this relate to the hdaps driver hosted at sourceforge since June?

Bill

#2 lore raw
Quoted 2 lines · Bill Nottingham

How does this relate to the hdaps driver hosted at sourceforge since June?

This driver is what is hosted there.

I thought it was time to push on.

Robert Love

#3 lore raw
In reply to Robert Love
Quoted 7 lines · Robert Love

Andrew,

Of late I have been working on a driver for the IBM Hard Drive Active Protection System (HDAPS), which provides a two-axis accelerometer and some other misc. data. The hardware is found on recent IBM ThinkPad laptops.

Quoted 7 lines

The following patch adds the driver to 2.6.13-rc6-mm2. It is self-contained and fairly simple.

Please, apply.

Robert Love

Is there any way to detect that this device is present (PCI, ACPI, etc.) without poking at ports?

--
Brian Gerst

#4 lore raw
Quoted 2 lines · Brian Gerst

Is there any way to detect that this device is present (PCI, ACPI, etc.) without poking at ports?

Not that we've been able to tell. It is a legacy platform device.

So, unfortunately, no probe() routine.

Robert Love

#5 lore raw
Quoted 8 lines · Robert Love
Quoted 2 lines · Brian Gerst

Is there any way to detect that this device is present (PCI, ACPI, etc.) without poking at ports?

Not that we've been able to tell. It is a legacy platform device.

So, unfortunately, no probe() routine.

dmi surely....

#6 lore raw
Quoted 5 lines · Arjan van de Ven
Quoted 3 lines

Not that we've been able to tell. It is a legacy platform device.

So, unfortunately, no probe() routine.

dmi surely....

Patches accepted.

Robert Love

#7 lore raw
Quoted 9 lines · Robert Love
Quoted 5 lines · Arjan van de Ven
Quoted 3 lines

Not that we've been able to tell. It is a legacy platform device.

So, unfortunately, no probe() routine.

dmi surely....

Patches accepted.

A little difficult for people to submit dmi patches, unless they
have hardware this driver runs on. Surely as you've tested this,
you're in the best position to write such patches :-)

Dave

#8 lore raw
Quoted 3 lines · Dave Jones

A little difficult for people to submit dmi patches, unless they
have hardware this driver runs on. Surely as you've tested this,
you're in the best position to write such patches :-)

Surely one of the millions of people with a ThinkPad can feel free to try a DMI-based probe() out, if they want a probe() routine, was my point.

Robert Love

#9 lore raw
In reply to Robert Love
Quoted 9 lines · Robert Love
Quoted 5 lines · Arjan van de Ven
Quoted 3 lines

Not that we've been able to tell. It is a legacy platform device.

So, unfortunately, no probe() routine.

dmi surely....

Patches accepted.

I think that should be fixed before its merged.

Alan

#10 lore raw
Quoted 1 line · Alan Cox

I think that should be fixed before its merged.

Let me be clear, it has an init routine that effectively probes for the device.

It just lacks a simple quick non-invasive check.

The driver will definitely fail to load on a laptop without the requisite hardware.

Robert Love

#11 lore raw
Quoted 10 lines · Robert Love
Quoted 1 line · Alan Cox

I think that should be fixed before its merged.

Let me be clear, it has an init routine that effectively probes for the device.

It just lacks a simple quick non-invasive check.

Since such a check is possible, that's definitely a merge-stopper IMO

Jeff

#12 lore raw
Quoted 1 line · Jeff Garzik

Since such a check is possible, that's definitely a merge-stopper IMO

First, I am not asking that Linus merge this. Everyone needs to relax.

Second, we don't know a DMI-based solution will work. I'll check it out.

Robert Love

#13 lore raw
Quoted 1 line · Robert Love

Second, we don't know a DMI-based solution will work. I'll check it out.

Another good sanity check would be tool for the right bridge chips with device->subvendor == IBM ?

#14 lore raw
In reply to Robert Love
Quoted 7 lines · Robert Love
Quoted 1 line · Jeff Garzik

Since such a check is possible, that's definitely a merge-stopper IMO

First, I am not asking that Linus merge this. Everyone needs to relax.

Second, we don't know a DMI-based solution will work. I'll check it out.

With some luck it might be in the ACPI name space with a known name. If yes that would be far more reliable than DMI.

-Andi

#15 lore raw
In reply to Robert Love
Quoted 11 lines · Robert Love
Quoted 1 line · Alan Cox

I think that should be fixed before its merged.

Let me be clear, it has an init routine that effectively probes for the device.

It just lacks a simple quick non-invasive check.

The driver will definitely fail to load on a laptop without the requisite hardware.

Poking IO ports on hardware where you don't have the device can be fatal. What happens if I have something completely different at io port 0x1600 ? (Thus satisfying your request_region() check). accelerometer_init() then happily starts poking ports, and performing all kinds of voodoo.

Dave

#16 lore raw
In reply to Brian Gerst
Quoted 2 lines · Brian Gerst

Is there any way to detect that this device is present (PCI, ACPI, etc.) without poking at ports?

DMI or probably IBM ssid values. Presumably IBM have somewhere they look for this information ?

Making the driver only load on a DMI match or with an option "force=1" which tells people to submit the DMI data would rapidly fill a table if IBM can't answer the general question.

At the very least it should check for an ibm laptop first.

#17 lore raw
In reply to Robert Love
Quoted 6 lines · Robert Love

+config SENSORS_HDAPS
+ tristate "IBM Hard Drive Active Protection System (hdaps)"
+ depends on HWMON
+ default n
+ help

How does this get along with CONFIG_INPUT=n, CONFIG_INPUT_MOUSEDEV=n, etc?

#18 lore raw
Quoted 7 lines · Andrew Morton
Quoted 5 lines

+config SENSORS_HDAPS
+ tristate "IBM Hard Drive Active Protection System (hdaps)"
+ depends on HWMON
+ default n
+ help

How does this get along with CONFIG_INPUT=n, CONFIG_INPUT_MOUSEDEV=n, etc?

Probably a question you should of asked before merging the patch. ;-)

We just need CONFIG_INPUT.

Thanks,

Robert Love

Depend on CONFIG_INPUT.

drivers/hwmon/Kconfig +1 -1
diff -u linux/drivers/hwmon/Kconfig linux/drivers/hwmon/Kconfig
@@ -413,7 +413,7 @@
 
 config SENSORS_HDAPS
 	tristate "IBM Hard Drive Active Protection System (hdaps)"
-	depends on HWMON
+	depends on HWMON && INPUT
 	default n
 	help
 	  This driver provides support for the IBM Hard Drive Active Protection
#19 lore raw
In reply to Robert Love

drivers/hwmon/hdaps.c line 310

Quoted 8 lines · Robert Love
drivers/hwmon/hdaps.c +8 -0
+/* device class stuff */
+
+static DECLARE_COMPLETION(hdaps_obj_is_free);
+static void hdaps_release_dev(struct device *dev)
+{
+       complete(&hdaps_obj_is_free);
+}
+

What this completion is used for? I don't see any other references to it.

Quoted 10 lines
drivers/hwmon/hdaps.c +10 -0
+
+static void hdaps_mousedev_poll(unsigned long unused)
+{
+       int movex, movey, x, y, ret;
+
+       ret = accelerometer_read_pair(HDAPS_PORT_XPOS, HDAPS_PORT_YPOS, &x, &y);
+       if (unlikely(ret))
+               return;
+
+       movex = rest_x - x;

drivers/hwmon/hdaps.c line 373

Quoted 10 lines
drivers/hwmon/hdaps.c +10 -0
+       movey = rest_y - y;
+       if (abs(movex) > hdaps_mousedev_threshold)
+               input_report_rel(&hdaps_idev, REL_Y, movex);
+       if (abs(movey) > hdaps_mousedev_threshold)
+               input_report_rel(&hdaps_idev, REL_X, movey);
+       input_sync(&hdaps_idev);
+
+       mod_timer(&hdaps_poll_timer, jiffies + msecs_to_jiffies(hdaps_poll_ms));
+}
+

I'd rather you used absolute coordinates and set up hdaps_idev->absfuzz to do the filtering.

Quoted 7 lines
drivers/hwmon/hdaps.c +7 -0
+static ssize_t hdaps_mousedev_store(struct device *dev,
+                                   struct device_attribute *attr,
+                                   const char *buf, size_t count)
+{
+       int enable;
+
+       if (sscanf(buf, "%d\n", &enable) != 1)

drivers/hwmon/hdaps.c line 470

Quoted 10 lines
drivers/hwmon/hdaps.c +10 -0
+               return -EINVAL;
+
+       if (enable == 1)
+               hdaps_mousedev_enable();
+       else if (enable == 0)
+               hdaps_mousedev_disable();
+
+       return count;
+}
+

This is racy - 2 threads can try to do this simultaneously.

drivers/hwmon/hdaps.c line 558

Quoted 9 lines
drivers/hwmon/hdaps.c +9 -0
+
+       device_create_file(&hdaps_plat_dev.dev, &dev_attr_position);
+       device_create_file(&hdaps_plat_dev.dev, &dev_attr_variance);
+       device_create_file(&hdaps_plat_dev.dev, &dev_attr_temp);
+       device_create_file(&hdaps_plat_dev.dev, &dev_attr_calibrate);
+       device_create_file(&hdaps_plat_dev.dev, &dev_attr_mousedev);
+       device_create_file(&hdaps_plat_dev.dev, &dev_attr_mousedev_threshold);
+       device_create_file(&hdaps_plat_dev.dev, &dev_attr_mousedev_poll_ms);
+

What about using sysfs_attribute_group?

Signature · 1 line
Dmitry
#20 lore raw
Quoted 1 line · Dmitry Torokhov

What this completion is used for? I don't see any other references to it.

It was the start of the release() routine, but I decided to move to platform_device_register_simple() and use its release, instead. So this is gone now in my tree.

Quoted 2 lines

I'd rather you used absolute coordinates and set up hdaps_idev->absfuzz to do the filtering.

Me too.

Quoted 1 line

This is racy - 2 threads can try to do this simultaneously.

Fixed. Thanks.

Quoted 11 lines

drivers/hwmon/hdaps.c line 558

Quoted 9 lines
drivers/hwmon/hdaps.c +9 -0
+
+       device_create_file(&hdaps_plat_dev.dev, &dev_attr_position);
+       device_create_file(&hdaps_plat_dev.dev, &dev_attr_variance);
+       device_create_file(&hdaps_plat_dev.dev, &dev_attr_temp);
+       device_create_file(&hdaps_plat_dev.dev, &dev_attr_calibrate);
+       device_create_file(&hdaps_plat_dev.dev, &dev_attr_mousedev);
+       device_create_file(&hdaps_plat_dev.dev, &dev_attr_mousedev_threshold);
+       device_create_file(&hdaps_plat_dev.dev, &dev_attr_mousedev_poll_ms);
+

What about using sysfs_attribute_group?

I don't see this in my tree?

Robert Love

#21 lore raw
Quoted 3 lines · Robert Love
Quoted 1 line

This is racy - 2 threads can try to do this simultaneously.

Fixed. Thanks.

Actually, doesn't sysfs and/or the vfs layer serialize the two simultaneous writes?

Robert Love

#22 lore raw
Quoted 9 lines · Robert Love
Quoted 3 lines · Robert Love
Quoted 1 line

This is racy - 2 threads can try to do this simultaneously.

Fixed. Thanks.

Actually, doesn't sysfs and/or the vfs layer serialize the two simultaneous writes?

Not between 2 separate processes (or, rather, separate file handles) as far as I can see...

Signature · 1 line
Dmitry
#23 lore raw
In reply to Robert Love
Quoted 4 lines · Robert Love
Quoted 1 line · Dmitry Torokhov

What this completion is used for? I don't see any other references to it.

Quoted 9 lines

It was the start of the release() routine, but I decided to move to platform_device_register_simple() and use its release, instead. So this is gone now in my tree.

Quoted 2 lines

I'd rather you used absolute coordinates and set up hdaps_idev->absfuzz to do the filtering.

Me too.

Btw, if you set up absolute input device it will be claimed by joydev instead of mousedev and will not get in a way of normal operation while still available for playing. So you could just kill all that enabling/disabling code and have input device always activated.

Quoted 4 lines
Quoted 2 lines

What about using sysfs_attribute_group?

I don't see this in my tree?

Sorry, it is called struct attribute_group, sysfs_create_group() and sysfs_remove_group(). See fs/sysfs/group.c

Signature · 1 line
Dmitry
#24 lore raw
Quoted 12 lines · Dmitry Torokhov
Quoted 11 lines · Robert Love
Quoted 1 line · Dmitry Torokhov

What this completion is used for? I don't see any other references to it.

It was the start of the release() routine, but I decided to move to platform_device_register_simple() and use its release, instead. So this is gone now in my tree.

Quoted 2 lines

I'd rather you used absolute coordinates and set up hdaps_idev->absfuzz to do the filtering.

Quoted 7 lines
Quoted 2 lines

Me too.

Btw, if you set up absolute input device it will be claimed by joydev instead of mousedev and will not get in a way of normal operation while still available for playing. So you could just kill all that enabling/disabling code and have input device always activated.

Even easier - I submitted a patch a while back against the old hdaps driver (on hdaps-devel) to use the hdaps sensor's keyboard/mouse activity readings to selectively disable/enable the mouse (and re-enable it after a time of no keyboard/mouse activity). This makes the mouse device much more usable, and you don't end up fighting it while trying to use the normal mouse/keyboard.

Yani

#25 lore raw
In reply to Robert Love

Hi!

Quoted 7 lines

Of late I have been working on a driver for the IBM Hard Drive Active Protection System (HDAPS), which provides a two-axis accelerometer and some other misc. data. The hardware is found on recent IBM ThinkPad laptops.

The following patch adds the driver to 2.6.13-rc6-mm2. It is self-contained and fairly simple.

Do we really need input interface *and* sysfs interface? Input should be enough.

Signature · 1 line
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms         
#26 lore raw
Quoted 11 lines

Hi!

Quoted 7 lines

Of late I have been working on a driver for the IBM Hard Drive Active Protection System (HDAPS), which provides a two-axis accelerometer and some other misc. data. The hardware is found on recent IBM ThinkPad laptops.

The following patch adds the driver to 2.6.13-rc6-mm2. It is self-contained and fairly simple.

Do we really need input interface *and* sysfs interface? Input should be enough.

Hi!

I think he doesn't need to export it at all and he should write code to park and disable hard disk instead.
(in userspace it's unsolvable --- i.e. you can't enable hard disk when detected stable condition if the daemon is swapped out on that hard disk)

Mikulas

#27 lore raw
Quoted 9 lines
Quoted 9 lines
Quoted 8 lines

Of late I have been working on a driver for the IBM Hard Drive Active Protection System (HDAPS), which provides a two-axis accelerometer and some other misc. data. The hardware is found on recent IBM ThinkPad laptops.

The following patch adds the driver to 2.6.13-rc6-mm2. It is self-contained and fairly simple.

Quoted 7 lines
Quoted 2 lines

Do we really need input interface *and* sysfs interface? Input should be enough.

I think he doesn't need to export it at all and he should write code to park and disable hard disk instead.
(in userspace it's unsolvable --- i.e. you can't enable hard disk when detected stable condition if the daemon is swapped out on that hard disk)

man mlockall() :-).

Accelerometer is usefull for other stuff besides parking heads, like playing marble madness or what is the name of the game, and even parking heads is way too complex to be put into the kernel.

Even if you don't like mlockall(), you can put timeout into disk-freezing interface.
Pavel

Signature · 1 line
if you have sharp zaurus hardware you don't need... you know my address
#28 lore raw

Am Sonntag, 28. August 2005 10:09 schrieb Pavel Machek:

Quoted 12 lines
Quoted 12 lines
Quoted 11 lines
Quoted 8 lines

Of late I have been working on a driver for the IBM Hard Drive Active Protection System (HDAPS), which provides a two-axis accelerometer and some other misc. data. The hardware is found on recent IBM ThinkPad laptops.

The following patch adds the driver to 2.6.13-rc6-mm2. It is self-contained and fairly simple.

Do we really need input interface *and* sysfs interface? Input should be enough.

Quoted 6 lines
Quoted 4 lines

I think he doesn't need to export it at all and he should write code to park and disable hard disk instead.
(in userspace it's unsolvable --- i.e. you can't enable hard disk when detected stable condition if the daemon is swapped out on that hard disk)

man mlockall() :-).

If you need that, it belongs into the kernel.

Regards
Oliver

#29 lore raw
In reply to Pavel Machek
Quoted 11 lines
Quoted 11 lines
Quoted 10 lines
Quoted 7 lines

Of late I have been working on a driver for the IBM Hard Drive Active Protection System (HDAPS), which provides a two-axis accelerometer and some other misc. data. The hardware is found on recent IBM ThinkPad laptops.

The following patch adds the driver to 2.6.13-rc6-mm2. It is self-contained and fairly simple.

Do we really need input interface *and* sysfs interface? Input should be enough.

Quoted 6 lines
Quoted 4 lines

I think he doesn't need to export it at all and he should write code to park and disable hard disk instead.
(in userspace it's unsolvable --- i.e. you can't enable hard disk when detected stable condition if the daemon is swapped out on that hard disk)

man mlockall() :-).

You also must not use any syscall that allocates even temporary memory in kernel (select, poll, many others ...) or that waits on semaphore that might be held while allocating memory (i.e. audit and rewrite ide ioctl path).

And you need extra flags to protect the daemon from being killed at shutdown or blocked at suspend.

I think writing it as kernel thread would be easir than all this.

Quoted 6 lines

Accelerometer is usefull for other stuff besides parking heads, like playing marble madness or what is the name of the game, and even parking heads is way too complex to be put into the kernel.

Even if you don't like mlockall(), you can put timeout into disk-freezing interface.

That makes the protection less reliable (you shake the notebook and after the timeout drop it).

Mikulas

Quoted 4 lines

Pavel

Signature · 1 line
if you have sharp zaurus hardware you don't need... you know my address
#30 lore raw

Hi!

Quoted 11 lines
Quoted 6 lines
Quoted 4 lines

I think he doesn't need to export it at all and he should write code to park and disable hard disk instead.
(in userspace it's unsolvable --- i.e. you can't enable hard disk when detected stable condition if the daemon is swapped out on that hard disk)

man mlockall() :-).

You also must not use any syscall that allocates even temporary memory in kernel (select, poll, many others ...) or that waits on semaphore that might be held while allocating memory (i.e. audit and rewrite ide ioctl path).

Kernel module would have exactly same problem.

Quoted 2 lines

And you need extra flags to protect the daemon from being killed at shutdown or blocked at suspend.

Why?

Quoted 9 lines
Quoted 6 lines

Accelerometer is usefull for other stuff besides parking heads, like playing marble madness or what is the name of the game, and even parking heads is way too complex to be put into the kernel.

Even if you don't like mlockall(), you can put timeout into disk-freezing interface.

That makes the protection less reliable (you shake the notebook and after the timeout drop it).

Idea is that userland app keeps saying "unfreeze 5 seconds in future" as long as you keep shaking -- essentialy a deadlock prevention.
Pavel

Signature · 1 line
if you have sharp zaurus hardware you don't need... you know my address
#31 lore raw
Quoted 7 lines · Pavel Machek

Hi!

Quoted 5 lines
Quoted 5 lines
Quoted 4 lines

I think he doesn't need to export it at all and he should write code to park and disable hard disk instead.
(in userspace it's unsolvable --- i.e. you can't enable hard disk when detected stable condition if the daemon is swapped out on that hard disk)

Quoted 8 lines
Quoted 6 lines
Quoted 1 line

man mlockall() :-).

You also must not use any syscall that allocates even temporary memory in kernel (select, poll, many others ...) or that waits on semaphore that might be held while allocating memory (i.e. audit and rewrite ide ioctl path).

Kernel module would have exactly same problem.

It has control of its memory allocations.

Quoted 4 lines
Quoted 2 lines

And you need extra flags to protect the daemon from being killed at shutdown or blocked at suspend.

Why?

Because the disk must be unlocked even if the laptop falls down while a suspension or shutdown are under way.
And it should work until the heads are parked anyway.

Regards
Oliver

#32 lore raw

Hi!

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
Quoted 4 lines

I think he doesn't need to export it at all and he should write code to park and disable hard disk instead.
(in userspace it's unsolvable --- i.e. you can't enable hard disk when detected stable condition if the daemon is swapped out on that hard disk)

man mlockall() :-).

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines

You also must not use any syscall that allocates even temporary memory in kernel (select, poll, many others ...) or that waits on semaphore that might be held while allocating memory (i.e. audit and rewrite ide ioctl path).

Kernel module would have exactly same problem.

It has control of its memory allocations.

It will have to be carefull with doing calls that allocate memory, and to avoid semaphores that may block it...

Quoted 8 lines
Quoted 4 lines
Quoted 2 lines

And you need extra flags to protect the daemon from being killed at shutdown or blocked at suspend.

Why?

Because the disk must be unlocked even if the laptop falls down while a suspension or shutdown are under way.
And it should work until the heads are parked anyway.

It can't. It has to be user controllable at any point, or you will not be able to shutdown your notebook while on bumpy road.

Anyone who wants to put it into kernel _please_ read that IBM paper. It is really complex, with some signal processing, and it goes wrong at times, so it needs UI so you can disable it.

Pavel

Signature · 1 line
if you have sharp zaurus hardware you don't need... you know my address
#33 lore raw

Hi

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines

I think he doesn't need to export it at all and he should write code to park and disable hard disk instead.
(in userspace it's unsolvable --- i.e. you can't enable hard disk when detected stable condition if the daemon is swapped out on that hard disk)

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 6 lines
Quoted 1 line

man mlockall() :-).

You also must not use any syscall that allocates even temporary memory in kernel (select, poll, many others ...) or that waits on semaphore that might be held while allocating memory (i.e. audit and rewrite ide ioctl path).

Kernel module would have exactly same problem.

In kernel there are many paths how to call something in ide driver. In userspace there is only one --- ioctl.

In kernel you can write something like
int register_panic_event(void (*fn)(void *opaque, int stable), void *opaque);
void unregister_panic_event(void (*fn)(void *opaque, int stable), void *opaque);
and
void generate_panic_event(int stable);
... so that drivers can make hooks to do things when notebook falls. And it can protect not only primary IDE, but also disks on additional cards, USB disks etc. In userspace it would be pain to let the driver support all types of devices, scan for them (how?) etc.

Quoted 9 lines
Quoted 1 line

It has control of its memory allocations.

It will have to be carefull with doing calls that allocate memory, and to avoid semaphores that may block it...

Quoted 4 lines
Quoted 4 lines
Quoted 2 lines

And you need extra flags to protect the daemon from being killed at shutdown or blocked at suspend.

Why?

If the daemon parks head and blocks ide queue and you kill it, the queue won't ever be unblocked.

Quoted 6 lines
Quoted 3 lines

Because the disk must be unlocked even if the laptop falls down while a suspension or shutdown are under way.
And it should work until the heads are parked anyway.

It can't. It has to be user controllable at any point, or you will not be able to shutdown your notebook while on bumpy road.

The whole userspace is paged, so if the daemon locks up disk forever, it would be impossible to turn it off anyway (you can with some pain make the daemon unpaged, but you can't do that for user's shell).

Quoted 3 lines

Anyone who wants to put it into kernel _please_ read that IBM paper. It is really complex, with some signal processing, and it goes wrong at times, so it needs UI so you can disable it.

They use some auto-adjusting of values. BTW. they have a bug in bios that it waits until stable condition and refuses to boot on a road.

Maybe something like this woule be reliable: get some value how much the notebook moves, if the current value greater that the average (over some time), park disk. --- it can't lock-up forever because the value can't grow forever. If you tilt the notebook on a road or move it on the table, it would park (which exactly what you want).

Mikulas

Quoted 4 lines

Pavel

Signature · 1 line
if you have sharp zaurus hardware you don't need... you know my address
#34 lore raw

Please refer to my IDE freeze patch last week:

http://lkml.org/lkml/2005/8/25/140

It provides userspace with a method to freeze the queue and park the head (through sysfs), along with a timeout to unfreeze, and works quite well. It is in the process of being moved to the block layer however so that implementation for libata will be simpler.

Yani