[lm-sensors] [PATCH] Apple Motion Sensor driver

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Michael Hanselmann]

This driver adds support for the Apple Motion Sensor (AMS) as found in
2005 revisions of Apple PowerBooks and iBooks. It implements both the
PMU and I?C variants. The I?C driver and mouse emulation is based on
code by Stelian Pop <stelian at popies.net>, while the PMU driver has been
developped by myself. HD parking support will be added later.

Please apply to -mm for testing.

Signed-off-by: Michael Hanselmann <linux-kernel at hansmi.ch>

---
diff -Nrup --exclude-from linux-exclude-from linux-2.6.18-rc1-git10.orig/drivers/hwmon/ams/ams-core.c linux-2.6.18-rc1-git10/drivers/hwmon/ams/ams-core.c
--- linux-2.6.18-rc1-git10.orig/drivers/hwmon/ams/ams-core.c	1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.18-rc1-git10/drivers/hwmon/ams/ams-core.c	2006-07-15 22:31:18.000000000 +0200
@@ -0,0 +1,255 @@
+/*
+ * Apple Motion Sensor driver
+ *
+ * Copyright (C) 2005 Stelian Pop (stelian at popies.net)
+ * Copyright (C) 2006 Michael Hanselmann (linux-kernel at hansmi.ch)
+ *
+ * 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., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <asm/pmac_pfunc.h>
+
+#include "ams.h"
+
+/* There is only one motion sensor per machine */
+struct ams ams;
+
+static unsigned int verbose;
+module_param(verbose, bool, 0644);
+MODULE_PARM_DESC(verbose, "Show free falls and shocks in kernel output");
+
+/* Call with lock held! */
+void ams_sensors(s8 *x, s8 *y, s8 *z)
+{
+	u32 orient = ams.vflag? ams.orient1 : ams.orient2;
+
+	if (orient & 0x80)
+		/* X and Y swapped */
+		ams.get_xyz(y, x, z);
+	else
+		ams.get_xyz(x, y, z);
+
+	if (orient & 0x04)
+		*z = ~(*z);
+	if (orient & 0x02)
+		*y = ~(*y);
+	if (orient & 0x01)
+		*x = ~(*x);
+}
+
+static ssize_t ams_show_current(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	s8 x, y, z;
+
+	mutex_lock(&ams.lock);
+	ams_sensors(&x, &y, &z);
+	mutex_unlock(&ams.lock);
+
+	return snprintf(buf, PAGE_SIZE, "%d %d %d\n", x, y, z);
+}
+
+static DEVICE_ATTR(current, S_IRUGO, ams_show_current, NULL);
+
+static void ams_handle_irq(void *data)
+{
+	enum ams_irq irq = *((enum ams_irq *)data);
+
+	spin_lock(&ams.irq_lock);
+
+	ams.worker_irqs |= irq;
+	schedule_work(&ams.worker);
+
+	spin_unlock(&ams.irq_lock);
+}
+
+static enum ams_irq ams_freefall_irq_data = AMS_IRQ_FREEFALL;
+static struct pmf_irq_client ams_freefall_client = {
+	.owner = THIS_MODULE,
+	.handler = ams_handle_irq,
+	.data = &ams_freefall_irq_data,
+};
+
+static enum ams_irq ams_shock_irq_data = AMS_IRQ_SHOCK;
+static struct pmf_irq_client ams_shock_client = {
+	.owner = THIS_MODULE,
+	.handler = ams_handle_irq,
+	.data = &ams_shock_irq_data,
+};
+
+/* Once hard disk parking is implemented in the kernel, this function can
+ * trigger it.
+ */
+static void ams_worker(void *data)
+{
+	mutex_lock(&ams.lock);
+
+	if (ams.has_device) {
+		unsigned long flags;
+
+		spin_lock_irqsave(&ams.irq_lock, flags);
+
+		if (ams.worker_irqs & AMS_IRQ_FREEFALL) {
+			if (verbose)
+				printk(KERN_INFO "ams: freefall detected!\n");
+
+			ams.worker_irqs &= ~AMS_IRQ_FREEFALL;
+
+			/* we must call this with interrupts enabled */
+			spin_unlock_irqrestore(&ams.irq_lock, flags);
+			ams.clear_irq(AMS_IRQ_FREEFALL);
+			spin_lock_irqsave(&ams.irq_lock, flags);
+		}
+
+		if (ams.worker_irqs & AMS_IRQ_SHOCK) {
+			if (verbose)
+				printk(KERN_INFO "ams: shock detected!\n");
+
+			ams.worker_irqs &= ~AMS_IRQ_SHOCK;
+
+			/* we must call this with interrupts enabled */
+			spin_unlock_irqrestore(&ams.irq_lock, flags);
+			ams.clear_irq(AMS_IRQ_SHOCK);
+			spin_lock_irqsave(&ams.irq_lock, flags);
+		}
+
+		spin_unlock_irqrestore(&ams.irq_lock, flags); 
+	}
+
+	mutex_unlock(&ams.lock);
+}
+
+/* Call with lock held! */
+int ams_sensor_attach(void)
+{
+	int result;
+	u32 *prop;
+
+	/* Get orientation */
+	prop = (u32*)get_property(ams.of_node, "orientation", NULL);
+	if (!prop)
+		return -ENODEV;
+	ams.orient1 = *prop;
+	ams.orient2 = *(prop + 1);
+
+	/* Register freefall interrupt handler */
+	result = pmf_register_irq_client(ams.of_node,
+			"accel-int-1",
+			&ams_freefall_client);
+	if (result < 0)
+		return -ENODEV;
+
+	/* Reset saved irqs */
+	ams.worker_irqs = 0;
+
+	/* Register shock interrupt handler */
+	result = pmf_register_irq_client(ams.of_node,
+			"accel-int-2",
+			&ams_shock_client);
+	if (result < 0) {
+		pmf_unregister_irq_client(&ams_freefall_client);
+		return -ENODEV;
+	}
+
+	/* Create device */
+	ams.of_dev = of_platform_device_create(ams.of_node, "ams", NULL);
+	if (!ams.of_dev) {
+		pmf_unregister_irq_client(&ams_shock_client);
+		pmf_unregister_irq_client(&ams_freefall_client);
+		return -ENODEV;
+	}
+
+	/* Create attributes */
+	device_create_file(&ams.of_dev->dev, &dev_attr_current);
+
+	ams.vflag = !!(ams.get_vendor() & 0x10);
+
+	/* Init mouse device */
+	ams_mouse_init();
+
+	return 0;
+}
+
+int __init ams_init(void)
+{
+	struct device_node *np;
+
+	spin_lock_init(&ams.irq_lock);
+	mutex_init(&ams.lock);
+	INIT_WORK(&ams.worker, ams_worker, NULL);
+
+#ifdef CONFIG_SENSORS_AMS_I2C
+	np = of_find_node_by_name(NULL, "accelerometer");
+	if (np && device_is_compatible(np, "AAPL,accelerometer_1"))
+		/* Found I2C motion sensor */
+		return ams_i2c_init(np);
+#endif
+
+#ifdef CONFIG_SENSORS_AMS_PMU
+	np = of_find_node_by_name(NULL, "sms");
+	if (np && device_is_compatible(np, "sms"))
+		/* Found PMU motion sensor */
+		return ams_pmu_init(np);
+#endif
+
+	printk(KERN_ERR "ams: No motion sensor found.\n");
+
+	return -ENODEV;
+}
+
+void ams_exit(void)
+{
+	mutex_lock(&ams.lock);
+
+	if (ams.has_device) {
+		/* Remove mouse device */
+		ams_mouse_exit();
+
+		/* Shut down implementation */
+		ams.exit();
+
+		/* Cancel interrupt worker
+		 *
+		 * We do this after ams.exit(), because an interrupt might
+		 * have arrived before disabling them.
+		 */
+		cancel_delayed_work(&ams.worker);
+		flush_scheduled_work();
+
+		/* Remove attributes */
+		device_remove_file(&ams.of_dev->dev, &dev_attr_current);
+
+		/* Remove device */
+		of_device_unregister(ams.of_dev);
+
+		/* Remove handler */
+		pmf_unregister_irq_client(&ams_shock_client);
+		pmf_unregister_irq_client(&ams_freefall_client);
+	}
+
+	mutex_unlock(&ams.lock);
+}
+
+MODULE_AUTHOR("Stelian Pop, Michael Hanselmann");
+MODULE_DESCRIPTION("Apple Motion Sensor driver");
+MODULE_LICENSE("GPL");
+
+module_init(ams_init);
+module_exit(ams_exit);
diff -Nrup --exclude-from linux-exclude-from linux-2.6.18-rc1-git10.orig/drivers/hwmon/ams/ams.h linux-2.6.18-rc1-git10/drivers/hwmon/ams/ams.h
--- linux-2.6.18-rc1-git10.orig/drivers/hwmon/ams/ams.h	1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.18-rc1-git10/drivers/hwmon/ams/ams.h	2006-07-15 22:28:59.000000000 +0200
@@ -0,0 +1,72 @@
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/kthread.h>
+#include <linux/mutex.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+#include <asm/of_device.h>
+
+enum ams_irq {
+	AMS_IRQ_FREEFALL = 0x01,
+	AMS_IRQ_SHOCK = 0x02,
+	AMS_IRQ_GLOBAL = 0x04,
+	AMS_IRQ_ALL +		AMS_IRQ_FREEFALL |
+		AMS_IRQ_SHOCK |
+		AMS_IRQ_GLOBAL,
+};
+
+struct ams {
+	/* Locks */
+	spinlock_t irq_lock;
+	struct mutex lock;
+
+	/* General properties */
+	struct device_node *of_node;
+	struct of_device *of_dev;
+	char has_device;
+	char vflag;
+	u32 orient1;
+	u32 orient2;
+
+	/* Interrupt worker */
+	struct work_struct worker;
+	u8 worker_irqs;
+
+	/* Implementation
+	 *
+	 * Only call these functions with the main lock held.
+	 */
+	void (*exit)(void);
+
+	void (*get_xyz)(s8 *x, s8 *y, s8 *z);
+	u8 (*get_vendor)(void);
+
+	void (*clear_irq)(enum ams_irq reg);
+
+#ifdef CONFIG_SENSORS_AMS_I2C
+	/* I2C properties */
+	int i2c_bus;
+	int i2c_address;
+	struct i2c_client i2c_client;
+#endif
+
+	/* Mouse emulation */
+	struct task_struct *kthread;
+	struct input_dev *idev;
+	__u16 bustype;
+
+	/* calibrated null values */
+	int xcalib, ycalib, zcalib;
+};
+
+extern struct ams ams;
+
+extern void ams_sensors(s8 *x, s8 *y, s8 *z);
+extern int ams_sensor_attach(void);
+
+extern int ams_pmu_init(struct device_node *np);
+extern int ams_i2c_init(struct device_node *np);
+
+extern void ams_mouse_init(void);
+extern void ams_mouse_exit(void);
diff -Nrup --exclude-from linux-exclude-from linux-2.6.18-rc1-git10.orig/drivers/hwmon/ams/ams-i2c.c linux-2.6.18-rc1-git10/drivers/hwmon/ams/ams-i2c.c
--- linux-2.6.18-rc1-git10.orig/drivers/hwmon/ams/ams-i2c.c	1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.18-rc1-git10/drivers/hwmon/ams/ams-i2c.c	2006-07-15 22:28:59.000000000 +0200
@@ -0,0 +1,289 @@
+/*
+ * Apple Motion Sensor driver (I2C variant)
+ *
+ * Copyright (C) 2005 Stelian Pop (stelian at popies.net)
+ * Copyright (C) 2006 Michael Hanselmann (linux-kernel at hansmi.ch)
+ *
+ * Clean room implementation based on the reverse engineered Mac OS X driver by
+ * Johannes Berg <johannes at sipsolutions.net>, documentation available at
+ * http://johannes.sipsolutions.net/PowerBook/Apple_Motion_Sensor_Specification
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/delay.h>
+
+#include "ams.h"
+
+/* AMS registers */
+#define AMS_COMMAND	0x00	/* command register */
+#define AMS_STATUS	0x01	/* status register */
+#define AMS_CTRL1	0x02	/* read control 1 (number of values) */
+#define AMS_CTRL2	0x03	/* read control 2 (offset?) */
+#define AMS_CTRL3	0x04	/* read control 3 (size of each value?) */
+#define AMS_DATA1	0x05	/* read data 1 */
+#define AMS_DATA2	0x06	/* read data 2 */
+#define AMS_DATA3	0x07	/* read data 3 */
+#define AMS_DATA4	0x08	/* read data 4 */
+#define AMS_DATAX	0x20	/* data X */
+#define AMS_DATAY	0x21	/* data Y */
+#define AMS_DATAZ	0x22	/* data Z */
+#define AMS_FREEFALL	0x24	/* freefall int control */
+#define AMS_SHOCK	0x25	/* shock int control */
+#define AMS_SENSLOW	0x26	/* sensitivity low limit */
+#define AMS_SENSHIGH	0x27	/* sensitivity high limit */
+#define AMS_CTRLX	0x28	/* control X */
+#define AMS_CTRLY	0x29	/* control Y */
+#define AMS_CTRLZ	0x2A	/* control Z */
+#define AMS_UNKNOWN1	0x2B	/* unknown 1 */
+#define AMS_UNKNOWN2	0x2C	/* unknown 2 */
+#define AMS_UNKNOWN3	0x2D	/* unknown 3 */
+#define AMS_VENDOR	0x2E	/* vendor */
+
+/* AMS commands - use with the AMS_COMMAND register */
+enum ams_i2c_cmd {
+	AMS_CMD_NOOP = 0,
+	AMS_CMD_VERSION,
+	AMS_CMD_READMEM,
+	AMS_CMD_WRITEMEM,
+	AMS_CMD_ERASEMEM,
+	AMS_CMD_READEE,
+	AMS_CMD_WRITEEE,
+	AMS_CMD_RESET,
+	AMS_CMD_START,
+};
+
+static int ams_i2c_attach(struct i2c_adapter *adapter);
+static int ams_i2c_detach(struct i2c_adapter *adapter);
+
+static struct i2c_driver ams_i2c_driver = {
+	.driver = {
+		.name   = "ams",
+		.owner  = THIS_MODULE,
+	},
+	.attach_adapter = ams_i2c_attach,
+	.detach_adapter = ams_i2c_detach,
+};
+
+static s32 ams_i2c_read(u8 reg)
+{
+	return i2c_smbus_read_byte_data(&ams.i2c_client, reg);
+}
+
+static int ams_i2c_write(u8 reg, u8 value)
+{
+	return i2c_smbus_write_byte_data(&ams.i2c_client, reg, value);
+}
+
+static int ams_i2c_cmd(enum ams_i2c_cmd cmd)
+{
+	s32 result;
+	int i;
+
+	ams_i2c_write(AMS_COMMAND, cmd);
+	for (i = 0; i < 10; i++) {
+		mdelay(5);
+		result = ams_i2c_read(AMS_COMMAND);
+		if (result = 0 || result & 0x80)
+			return 0;
+	}
+	return -1;
+}
+
+static void ams_i2c_set_irq(enum ams_irq reg, char enable)
+{
+	if (reg & AMS_IRQ_FREEFALL) {
+		u8 val = ams_i2c_read(AMS_CTRLX);
+		if (enable)     val |= 0x80;
+		else            val &= ~0x80;
+		ams_i2c_write(AMS_CTRLX, val);
+	}
+
+	if (reg & AMS_IRQ_SHOCK) {
+		u8 val = ams_i2c_read(AMS_CTRLY);
+		if (enable)     val |= 0x80;
+		else            val &= ~0x80;
+		ams_i2c_write(AMS_CTRLY, val);
+	}
+
+	if (reg & AMS_IRQ_GLOBAL) {
+		u8 val = ams_i2c_read(AMS_CTRLZ);
+		if (enable)     val |= 0x80;
+		else            val &= ~0x80;
+		ams_i2c_write(AMS_CTRLZ, val);
+	}
+}
+
+static void ams_i2c_clear_irq(enum ams_irq reg)
+{
+	if (reg & AMS_IRQ_FREEFALL)
+		ams_i2c_write(AMS_FREEFALL, 0);
+
+	if (reg & AMS_IRQ_SHOCK)
+		ams_i2c_write(AMS_SHOCK, 0);
+}
+
+static u8 ams_i2c_get_vendor(void)
+{
+	return ams_i2c_read(AMS_VENDOR);
+}
+
+static void ams_i2c_get_xyz(s8 *x, s8 *y, s8 *z)
+{
+	*x = ams_i2c_read(AMS_DATAX);
+	*y = ams_i2c_read(AMS_DATAY);
+	*z = ams_i2c_read(AMS_DATAZ);
+}
+
+static int ams_i2c_attach(struct i2c_adapter *adapter)
+{
+	unsigned long bus;
+	int vmaj, vmin;
+	int result;
+
+	/* There can be only one */
+	if (unlikely(ams.has_device))
+		return -ENODEV;
+
+	if (strncmp(adapter->name, "uni-n", 5))
+		return -ENODEV;
+
+	bus = simple_strtoul(adapter->name + 6, NULL, 10);
+	if (bus != ams.i2c_bus)
+		return -ENODEV;
+
+	ams.i2c_client.addr = ams.i2c_address;
+	ams.i2c_client.adapter = adapter;
+	ams.i2c_client.driver = &ams_i2c_driver;
+	strcpy(ams.i2c_client.name, "Apple Motion Sensor");
+
+	if (ams_i2c_cmd(AMS_CMD_RESET)) {
+		printk(KERN_INFO "ams: Failed to reset the device\n");
+		return -ENODEV;
+	}
+
+	if (ams_i2c_cmd(AMS_CMD_START)) {
+		printk(KERN_INFO "ams: Failed to start the device\n");
+		return -ENODEV;
+	}
+
+	/* get version/vendor information */
+	ams_i2c_write(AMS_CTRL1, 0x02);
+	ams_i2c_write(AMS_CTRL2, 0x85);
+	ams_i2c_write(AMS_CTRL3, 0x01);
+
+	ams_i2c_cmd(AMS_CMD_READMEM);
+
+	vmaj = ams_i2c_read(AMS_DATA1);
+	vmin = ams_i2c_read(AMS_DATA2);
+	if (vmaj != 1 || vmin != 52) {
+		printk(KERN_INFO "ams: Incorrect device version (%d.%d)\n",
+			vmaj, vmin);
+		return -ENODEV;
+	}
+
+	ams_i2c_cmd(AMS_CMD_VERSION);
+
+	vmaj = ams_i2c_read(AMS_DATA1);
+	vmin = ams_i2c_read(AMS_DATA2);
+	if (vmaj != 0 || vmin != 1) {
+		printk(KERN_INFO "ams: Incorrect firmware version (%d.%d)\n",
+			vmaj, vmin);
+		return -ENODEV;
+	}
+
+	/* Disable interrupts */
+	ams_i2c_set_irq(AMS_IRQ_ALL, 0);
+
+	result = ams_sensor_attach();
+	if (result < 0)
+		return result;
+
+	/* Set default values */
+	ams_i2c_write(AMS_SENSLOW, 0x15);
+	ams_i2c_write(AMS_SENSHIGH, 0x60);
+	ams_i2c_write(AMS_CTRLX, 0x08);
+	ams_i2c_write(AMS_CTRLY, 0x0F);
+	ams_i2c_write(AMS_CTRLZ, 0x4F);
+	ams_i2c_write(AMS_UNKNOWN1, 0x14);
+
+	/* Clear interrupts */
+	ams_i2c_clear_irq(AMS_IRQ_ALL);
+
+	ams.has_device = 1;
+
+	/* Enable interrupts */
+	ams_i2c_set_irq(AMS_IRQ_ALL, 1);
+
+	printk(KERN_INFO "ams: Found I2C based motion sensor\n");
+
+	return 0;
+}
+
+static int ams_i2c_detach(struct i2c_adapter *adapter)
+{
+	if (ams.has_device) {
+		/* Disable interrupts */
+		ams_i2c_set_irq(AMS_IRQ_ALL, 0);
+
+		/* Clear interrupts */
+		ams_i2c_clear_irq(AMS_IRQ_ALL);
+
+		printk(KERN_INFO "ams: Unloading\n");
+
+		ams.has_device = 0;
+	}
+
+	return 0;
+}
+
+static void ams_i2c_exit(void)
+{
+	i2c_del_driver(&ams_i2c_driver);
+}
+
+int __init ams_i2c_init(struct device_node *np)
+{
+	char *tmp_bus;
+	int result;
+	u32 *prop;
+
+	mutex_lock(&ams.lock);
+
+	/* Set implementation stuff */
+	ams.of_node = np;
+	ams.exit = ams_i2c_exit;
+	ams.get_vendor = ams_i2c_get_vendor;
+	ams.get_xyz = ams_i2c_get_xyz;
+	ams.clear_irq = ams_i2c_clear_irq;
+	ams.bustype = BUS_I2C;
+
+	/* look for bus either using "reg" or by path */
+	prop = (u32*)get_property(ams.of_node, "reg", NULL);
+	if (!prop) {
+		result = -ENODEV;
+
+		goto exit;
+	}
+
+	tmp_bus = strstr(ams.of_node->full_name, "/i2c-bus@");
+	if (tmp_bus)
+		ams.i2c_bus = *(tmp_bus + 9) - '0';
+	else
+		ams.i2c_bus = ((*prop) >> 8) & 0x0f;
+	ams.i2c_address = ((*prop) & 0xff) >> 1;
+
+	result = i2c_add_driver(&ams_i2c_driver);
+
+exit:
+	mutex_unlock(&ams.lock);
+
+	return result;
+}
diff -Nrup --exclude-from linux-exclude-from linux-2.6.18-rc1-git10.orig/drivers/hwmon/ams/ams-mouse.c linux-2.6.18-rc1-git10/drivers/hwmon/ams/ams-mouse.c
--- linux-2.6.18-rc1-git10.orig/drivers/hwmon/ams/ams-mouse.c	1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.18-rc1-git10/drivers/hwmon/ams/ams-mouse.c	2006-07-15 22:28:59.000000000 +0200
@@ -0,0 +1,145 @@
+/*
+ * Apple Motion Sensor driver (mouse emulation)
+ *
+ * Copyright (C) 2005 Stelian Pop (stelian at popies.net)
+ * Copyright (C) 2006 Michael Hanselmann (linux-kernel at hansmi.ch)
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+
+#include <linux/types.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/delay.h>
+
+#include "ams.h"
+
+static unsigned int mouse;
+module_param(mouse, bool, 0644);
+MODULE_PARM_DESC(mouse, "Enable the input class device on module load");
+
+static int ams_mouse_kthread(void *data)
+{
+	s8 x, y, z;
+
+	while (!kthread_should_stop()) {
+		mutex_lock(&ams.lock);
+
+		ams_sensors(&x, &y, &z);
+
+		input_report_abs(ams.idev, ABS_X, x - ams.xcalib);
+		input_report_abs(ams.idev, ABS_Y, y - ams.ycalib);
+		input_report_abs(ams.idev, ABS_Z, z - ams.zcalib);
+
+		input_sync(ams.idev);
+
+		mutex_unlock(&ams.lock);
+
+		msleep(25);
+	}
+
+	return 0;
+}
+
+/* Call with lock held! */
+static void ams_mouse_enable(void)
+{
+	s8 x, y, z;
+
+	if (ams.idev)
+		return;
+
+	ams_sensors(&x, &y, &z);
+	ams.xcalib = x;
+	ams.ycalib = y;
+	ams.zcalib = z;
+
+	ams.idev = input_allocate_device();
+	if (!ams.idev)
+		return;
+
+	ams.idev->name = "Apple Motion Sensor";
+	ams.idev->id.bustype = ams.bustype;
+	ams.idev->id.vendor = 0;
+	ams.idev->cdev.dev = &ams.of_dev->dev;
+
+	input_set_abs_params(ams.idev, ABS_X, -50, 50, 3, 0);
+	input_set_abs_params(ams.idev, ABS_Y, -50, 50, 3, 0);
+	input_set_abs_params(ams.idev, ABS_Z, -50, 50, 3, 0);
+
+	set_bit(EV_ABS, ams.idev->evbit);
+	set_bit(EV_KEY, ams.idev->evbit);
+
+	if (input_register_device(ams.idev)) {
+		input_free_device(ams.idev);
+		ams.idev = NULL;
+		return;
+	}
+
+	ams.kthread = kthread_run(ams_mouse_kthread, NULL, "kams");
+	if (IS_ERR(ams.kthread)) {
+		input_unregister_device(ams.idev);
+		ams.idev = NULL;
+		return;
+	}
+}
+
+/* Call with lock held! */
+static void ams_mouse_disable(void)
+{
+	if (ams.idev) {
+		kthread_stop(ams.kthread);
+		input_unregister_device(ams.idev);
+		ams.idev = NULL;
+	}
+}
+
+static ssize_t ams_mouse_show_mouse(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%d\n", mouse);
+}
+
+static ssize_t ams_mouse_store_mouse(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t count)
+{
+	if (sscanf(buf, "%d\n", &mouse) != 1)
+		return -EINVAL;
+
+	mouse = !!mouse;
+
+	mutex_lock(&ams.lock);
+
+	if (mouse)
+		ams_mouse_enable();
+	else
+		ams_mouse_disable();
+
+	mutex_unlock(&ams.lock);
+
+	return count;
+}
+
+static DEVICE_ATTR(mouse, S_IRUGO | S_IWUSR,
+	ams_mouse_show_mouse, ams_mouse_store_mouse);
+
+/* Call with lock held! */
+void ams_mouse_init()
+{
+	device_create_file(&ams.of_dev->dev, &dev_attr_mouse);
+
+	if (mouse)
+		ams_mouse_enable();
+}
+
+/* Call with lock held! */
+void ams_mouse_exit()
+{
+	ams_mouse_disable();
+	device_remove_file(&ams.of_dev->dev, &dev_attr_mouse);
+}
diff -Nrup --exclude-from linux-exclude-from linux-2.6.18-rc1-git10.orig/drivers/hwmon/ams/ams-pmu.c linux-2.6.18-rc1-git10/drivers/hwmon/ams/ams-pmu.c
--- linux-2.6.18-rc1-git10.orig/drivers/hwmon/ams/ams-pmu.c	1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.18-rc1-git10/drivers/hwmon/ams/ams-pmu.c	2006-07-15 22:28:59.000000000 +0200
@@ -0,0 +1,202 @@
+/*
+ * Apple Motion Sensor driver (PMU variant)
+ *
+ * Copyright (C) 2006 Michael Hanselmann (linux-kernel at hansmi.ch)
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/adb.h>
+#include <linux/pmu.h>
+
+#include "ams.h"
+
+/* Attitude */
+#define AMS_X			0x00
+#define AMS_Y			0x01
+#define AMS_Z			0x02
+
+/* Not exactly known, maybe chip vendor */
+#define AMS_VENDOR		0x03
+
+/* Freefall registers */
+#define AMS_FF_CLEAR		0x04
+#define AMS_FF_ENABLE		0x05
+#define AMS_FF_LOW_LIMIT	0x06
+#define AMS_FF_DEBOUNCE		0x07
+
+/* Shock registers */
+#define AMS_SHOCK_CLEAR		0x08
+#define AMS_SHOCK_ENABLE	0x09
+#define AMS_SHOCK_HIGH_LIMIT	0x0a
+#define AMS_SHOCK_DEBOUNCE	0x0b
+
+/* Global interrupt and power control register */
+#define AMS_CONTROL		0x0c
+
+static u8 ams_pmu_cmd;
+
+static void ams_pmu_req_complete(struct adb_request *req)
+{
+	complete((struct completion *)req->arg);
+}
+
+/* Only call this function from task context */
+static void ams_pmu_set_register(u8 reg, u8 value)
+{
+	static struct adb_request req;
+	DECLARE_COMPLETION(req_complete);
+
+	req.arg = &req_complete;
+	if (pmu_request(&req, ams_pmu_req_complete, 4, ams_pmu_cmd, 0x00, reg, value))
+		return;
+
+	wait_for_completion(&req_complete);
+}
+
+/* Only call this function from task context */
+static u8 ams_pmu_get_register(u8 reg)
+{
+	static struct adb_request req;
+	DECLARE_COMPLETION(req_complete);
+
+	req.arg = &req_complete;
+	if (pmu_request(&req, ams_pmu_req_complete, 3, ams_pmu_cmd, 0x01, reg))
+		return 0;
+
+	wait_for_completion(&req_complete);
+
+	if (req.reply_len > 0)
+		return req.reply[0];
+	else
+		return 0;
+}
+
+/* Enables or disables the specified interrupts */
+static void ams_pmu_set_irq(enum ams_irq reg, char enable)
+{
+	if (reg & AMS_IRQ_FREEFALL) {
+		u8 val = ams_pmu_get_register(AMS_FF_ENABLE);
+		if (enable)	val |= 0x80;
+		else		val &= ~0x80;
+		ams_pmu_set_register(AMS_FF_ENABLE, val);
+	}
+
+	if (reg & AMS_IRQ_SHOCK) {
+		u8 val = ams_pmu_get_register(AMS_SHOCK_ENABLE);
+		if (enable)	val |= 0x80;
+		else		val &= ~0x80;
+		ams_pmu_set_register(AMS_SHOCK_ENABLE, val);
+	}
+
+	if (reg & AMS_IRQ_GLOBAL) {
+		u8 val = ams_pmu_get_register(AMS_CONTROL);
+		if (enable)	val |= 0x80;
+		else		val &= ~0x80;
+		ams_pmu_set_register(AMS_CONTROL, val);
+	}
+}
+
+static void ams_pmu_clear_irq(enum ams_irq reg)
+{
+	if (reg & AMS_IRQ_FREEFALL)
+		ams_pmu_set_register(AMS_FF_CLEAR, 0x00);
+
+	if (reg & AMS_IRQ_SHOCK)
+		ams_pmu_set_register(AMS_SHOCK_CLEAR, 0x00);
+}
+
+static u8 ams_pmu_get_vendor(void)
+{
+	return ams_pmu_get_register(AMS_VENDOR);
+}
+
+static void ams_pmu_get_xyz(s8 *x, s8 *y, s8 *z)
+{
+	*x = ams_pmu_get_register(AMS_X);
+	*y = ams_pmu_get_register(AMS_Y);
+	*z = ams_pmu_get_register(AMS_Z);
+}
+
+static void ams_pmu_exit(void)
+{
+	/* Disable interrupts */
+	ams_pmu_set_irq(AMS_IRQ_ALL, 0);
+
+	/* Clear interrupts */
+	ams_pmu_clear_irq(AMS_IRQ_ALL);
+
+	ams.has_device = 0;
+
+	printk(KERN_INFO "ams: Unloading\n");
+}
+
+/* Call with lock held! */
+int __init ams_pmu_init(struct device_node *np)
+{
+	u32 *prop;
+	int result;
+
+	mutex_lock(&ams.lock);
+
+	/* Set implementation stuff */
+	ams.of_node = np;
+	ams.exit = ams_pmu_exit;
+	ams.get_vendor = ams_pmu_get_vendor;
+	ams.get_xyz = ams_pmu_get_xyz;
+	ams.clear_irq = ams_pmu_clear_irq;
+	ams.bustype = BUS_HOST;
+
+	/* Get PMU command, should be 0x4e, but we can never know */
+	prop = (u32*)get_property(ams.of_node, "reg", NULL);
+	if (!prop) {
+		result = -ENODEV;
+		goto exit;
+	}
+	ams_pmu_cmd = ((*prop) >> 8) & 0xff;
+
+	/* Disable interrupts */
+	ams_pmu_set_irq(AMS_IRQ_ALL, 0);
+
+	/* Clear interrupts */
+	ams_pmu_clear_irq(AMS_IRQ_ALL);
+
+	result = ams_sensor_attach();
+	if (result < 0)
+		goto exit;
+
+	/* Set default values */
+	ams_pmu_set_register(AMS_FF_LOW_LIMIT, 0x15);
+	ams_pmu_set_register(AMS_FF_ENABLE, 0x08);
+	ams_pmu_set_register(AMS_FF_DEBOUNCE, 0x14);
+
+	ams_pmu_set_register(AMS_SHOCK_HIGH_LIMIT, 0x60);
+	ams_pmu_set_register(AMS_SHOCK_ENABLE, 0x0f);
+	ams_pmu_set_register(AMS_SHOCK_DEBOUNCE, 0x14);
+
+	ams_pmu_set_register(AMS_CONTROL, 0x4f);
+
+	/* Clear interrupts */
+	ams_pmu_clear_irq(AMS_IRQ_ALL);
+
+	ams.has_device = 1;
+
+	/* Enable interrupts */
+	ams_pmu_set_irq(AMS_IRQ_ALL, 1);
+
+	printk(KERN_INFO "ams: Found PMU based motion sensor\n");
+
+	result = 0;
+
+exit:
+	mutex_unlock(&ams.lock);
+
+	return result;
+}
diff -Nrup --exclude-from linux-exclude-from linux-2.6.18-rc1-git10.orig/drivers/hwmon/ams/Makefile linux-2.6.18-rc1-git10/drivers/hwmon/ams/Makefile
--- linux-2.6.18-rc1-git10.orig/drivers/hwmon/ams/Makefile	1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.18-rc1-git10/drivers/hwmon/ams/Makefile	2006-07-15 22:28:59.000000000 +0200
@@ -0,0 +1,8 @@
+#
+# Makefile for Apple Motion Sensor driver
+#
+
+ams-y					:= ams-core.o ams-mouse.o
+ams-$(CONFIG_SENSORS_AMS_PMU)		+= ams-pmu.o
+ams-$(CONFIG_SENSORS_AMS_I2C)		+= ams-i2c.o
+obj-$(CONFIG_SENSORS_AMS)		+= ams.o
diff -Nrup --exclude-from linux-exclude-from linux-2.6.18-rc1-git10.orig/drivers/hwmon/Kconfig linux-2.6.18-rc1-git10/drivers/hwmon/Kconfig
--- linux-2.6.18-rc1-git10.orig/drivers/hwmon/Kconfig	2006-07-15 22:23:35.000000000 +0200
+++ linux-2.6.18-rc1-git10/drivers/hwmon/Kconfig	2006-07-15 22:36:20.000000000 +0200
@@ -94,6 +94,29 @@ config SENSORS_ADM9240
 	  This driver can also be built as a module.  If so, the module
 	  will be called adm9240.
 
+config SENSORS_AMS
+	tristate "Apple Motion Sensor driver"
+	depends on HWMON && PPC_PMAC && INPUT && EXPERIMENTAL
+	default y
+	help
+	  Support for the motion sensor included in PowerBooks. Includes
+	  implementations for PMU and I2C.
+
+config SENSORS_AMS_PMU
+	bool "PMU variant"
+	depends on SENSORS_AMS && ADB_PMU
+	default y
+	help
+	  PMU variant of motion sensor, found in late 2005 PowerBooks.
+
+config SENSORS_AMS_I2C
+	bool "I2C variant"
+	depends on SENSORS_AMS && I2C
+	default y
+	help
+	  I2C variant of motion sensor, found in early 2005 PowerBooks and
+	  iBooks.
+
 config SENSORS_ASB100
 	tristate "Asus ASB100 Bach"
 	depends on HWMON && I2C && EXPERIMENTAL
diff -Nrup --exclude-from linux-exclude-from linux-2.6.18-rc1-git10.orig/drivers/hwmon/Makefile linux-2.6.18-rc1-git10/drivers/hwmon/Makefile
--- linux-2.6.18-rc1-git10.orig/drivers/hwmon/Makefile	2006-07-15 22:23:35.000000000 +0200
+++ linux-2.6.18-rc1-git10/drivers/hwmon/Makefile	2006-07-15 22:28:59.000000000 +0200
@@ -18,6 +18,7 @@ obj-$(CONFIG_SENSORS_ADM1025)	+= adm1025
 obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
 obj-$(CONFIG_SENSORS_ADM1031)	+= adm1031.o
 obj-$(CONFIG_SENSORS_ADM9240)	+= adm9240.o
+obj-$(CONFIG_SENSORS_AMS)	+= ams/
 obj-$(CONFIG_SENSORS_ATXP1)	+= atxp1.o
 obj-$(CONFIG_SENSORS_DS1621)	+= ds1621.o
 obj-$(CONFIG_SENSORS_F71805F)	+= f71805f.o

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Andrew Morton]

On Wed, 2 Aug 2006 21:15:20 +0200
Michael Hanselmann <linux-kernel at hansmi.ch> wrote:

> This driver adds support for the Apple Motion Sensor (AMS) as found in
> 2005 revisions of Apple PowerBooks and iBooks. It implements both the
> PMU and I?C variants. The I?C driver and mouse emulation is based on
> code by Stelian Pop <stelian at popies.net>, while the PMU driver has been
> developped by myself. HD parking support will be added later.

Confused.  What is the relatioship between this code and
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.18-rc2/2.6.18-rc2-mm1/broken-out/apple-motion-sensor-driver.patch
?

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Michael Hanselmann]

On Wed, Aug 02, 2006 at 07:11:27PM -0700, Andrew Morton wrote:
> On Wed, 2 Aug 2006 21:15:20 +0200
> Michael Hanselmann <linux-kernel at hansmi.ch> wrote:

> > This driver adds support for the Apple Motion Sensor (AMS) as found in
> > 2005 revisions of Apple PowerBooks and iBooks. It implements both the
> > PMU and I?C variants. The I?C driver and mouse emulation is based on
> > code by Stelian Pop <stelian at popies.net>, while the PMU driver has been
> > developped by myself. HD parking support will be added later.

> Confused.  What is the relatioship between this code and
> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.18-rc2/2.6.18-rc2-mm1/broken-out/apple-motion-sensor-driver.patch
> ?

You already asked that on July 2 in
<20060702201415.791c6eb2.akpm at osdl.org>. My driver replaces the patch
from Stelian and includes all of his code.

Here's the mail conversation back then:

On Mon, Jul 03, 2006 at 08:56:28AM +0200, Michael Hanselmann wrote:
> On Sun, Jul 02, 2006 at 08:14:15PM -0700, Andrew Morton wrote:
> > On Mon, 3 Jul 2006 00:26:49 +0200
> > Michael Hanselmann <linux-kernel at hansmi.ch> wrote:

> > > Below you find the latest revision of my AMS driver.

> > I was about to merge the below, then this comes along.  Now what?

> > From: Stelian Pop <stelian at popies.net>

> > This driver provides support for the Apple Motion Sensor (ams), which
> > provides an accelerometer and other misc.  data.  Some Apple PowerBooks

> I just noticed yesterday that Stelian sent a patch to lkml in May. My
> work is based on his separate driver from his website.

> Given the fact that my driver includes all of his functionality and that
> replacing his with mine later in the process would mean to remove whole
> files again, I'd suggest to wait until I've fixed the outstanding issues
> (as seen in this thread) and then to merge mine.

This was approved by Stelian and yourself. So ? I think you can just
replace Stelian's patch with mine.

Thanks,
Michael

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Andrew Morton]

On Thu, 3 Aug 2006 08:53:33 +0200
Michael Hanselmann <linux-kernel at hansmi.ch> wrote:

> On Wed, Aug 02, 2006 at 07:11:27PM -0700, Andrew Morton wrote:
> > On Wed, 2 Aug 2006 21:15:20 +0200
> > Michael Hanselmann <linux-kernel at hansmi.ch> wrote:
> 
> > > This driver adds support for the Apple Motion Sensor (AMS) as found in
> > > 2005 revisions of Apple PowerBooks and iBooks. It implements both the
> > > PMU and I?C variants. The I?C driver and mouse emulation is based on
> > > code by Stelian Pop <stelian at popies.net>, while the PMU driver has been
> > > developped by myself. HD parking support will be added later.
> 
> > Confused.  What is the relatioship between this code and
> > ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.18-rc2/2.6.18-rc2-mm1/broken-out/apple-motion-sensor-driver.patch
> > ?
> 
> You already asked that on July 2 in
> <20060702201415.791c6eb2.akpm at osdl.org>.

I thought it was such a good question that I'd use it again.

> My driver replaces the patch
> from Stelian and includes all of his code.

Stelian, can we have a signed-off-by:, please?

Also, the previous patch had an s-o-b from Johannes.  Does this version
need one too?  

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Andrew Morton]

On Wed, 2 Aug 2006 21:15:20 +0200
Michael Hanselmann <linux-kernel at hansmi.ch> wrote:

> This driver adds support for the Apple Motion Sensor (AMS) as found in
> 2005 revisions of Apple PowerBooks and iBooks. It implements both the
> PMU and I?C variants. The I?C driver and mouse emulation is based on
> code by Stelian Pop <stelian at popies.net>, while the PMU driver has been
> developped by myself. HD parking support will be added later.
> 
> ...
>
> +/* There is only one motion sensor per machine */
> +struct ams ams;

That's a somewhat risky name for a global variable.

Then again, nobody else is likely to be so bold as to add another
three-character global ;)

> +/* Call with lock held! */
> +int ams_sensor_attach(void)

Which lock?

> +static int ams_i2c_cmd(enum ams_i2c_cmd cmd)
> +{
> +	s32 result;
> +	int i;
> +
> +	ams_i2c_write(AMS_COMMAND, cmd);
> +	for (i = 0; i < 10; i++) {
> +		mdelay(5);
> +		result = ams_i2c_read(AMS_COMMAND);
> +		if (result = 0 || result & 0x80)
> +			return 0;
> +	}
> +	return -1;
> +}

That's a potentially very long busy wait.

> +static void ams_i2c_set_irq(enum ams_irq reg, char enable)
> +{
> +	if (reg & AMS_IRQ_FREEFALL) {
> +		u8 val = ams_i2c_read(AMS_CTRLX);
> +		if (enable)     val |= 0x80;
> +		else            val &= ~0x80;
> +		ams_i2c_write(AMS_CTRLX, val);
> +	}
> +
> +	if (reg & AMS_IRQ_SHOCK) {
> +		u8 val = ams_i2c_read(AMS_CTRLY);
> +		if (enable)     val |= 0x80;
> +		else            val &= ~0x80;
> +		ams_i2c_write(AMS_CTRLY, val);
> +	}
> +
> +	if (reg & AMS_IRQ_GLOBAL) {
> +		u8 val = ams_i2c_read(AMS_CTRLZ);
> +		if (enable)     val |= 0x80;
> +		else            val &= ~0x80;
> +		ams_i2c_write(AMS_CTRLZ, val);
> +	}
> +}

Please, let's use

	if (a)
		b;
	else
		c;

> +/* Call with lock held! */
> +static void ams_mouse_enable(void)

Which lock?

> +{
> +	s8 x, y, z;
> +
> +	if (ams.idev)
> +		return;
> +
> +	ams_sensors(&x, &y, &z);
> +	ams.xcalib = x;
> +	ams.ycalib = y;
> +	ams.zcalib = z;
> +
> +	ams.idev = input_allocate_device();
> +	if (!ams.idev)
> +		return;
> +
> +	ams.idev->name = "Apple Motion Sensor";
> +	ams.idev->id.bustype = ams.bustype;
> +	ams.idev->id.vendor = 0;
> +	ams.idev->cdev.dev = &ams.of_dev->dev;
> +
> +	input_set_abs_params(ams.idev, ABS_X, -50, 50, 3, 0);
> +	input_set_abs_params(ams.idev, ABS_Y, -50, 50, 3, 0);
> +	input_set_abs_params(ams.idev, ABS_Z, -50, 50, 3, 0);
> +
> +	set_bit(EV_ABS, ams.idev->evbit);
> +	set_bit(EV_KEY, ams.idev->evbit);
> +
> +	if (input_register_device(ams.idev)) {
> +		input_free_device(ams.idev);
> +		ams.idev = NULL;
> +		return;
> +	}
> +
> +	ams.kthread = kthread_run(ams_mouse_kthread, NULL, "kams");
> +	if (IS_ERR(ams.kthread)) {
> +		input_unregister_device(ams.idev);
> +		ams.idev = NULL;
> +		return;

Didn't we just leak ams.idev?  Or does the disconnect handler handle that?

<goes to input_unregister_device() for the API description, comes away
discouraged>

> +		ams.idev = NULL;
> +	}
> +}
> +
> +static ssize_t ams_mouse_show_mouse(struct device *dev,
> +	struct device_attribute *attr, char *buf)
> +{
> +	return sprintf(buf, "%d\n", mouse);
> +}
> +
> +static ssize_t ams_mouse_store_mouse(struct device *dev,
> +	struct device_attribute *attr, const char *buf, size_t count)
> +{
> +	if (sscanf(buf, "%d\n", &mouse) != 1)
> +		return -EINVAL;
> +
> +	mouse = !!mouse;

That statement isn't needed.

> +	mutex_lock(&ams.lock);
> +
> +	if (mouse)
> +		ams_mouse_enable();
> +	else
> +		ams_mouse_disable();
> +
> +	mutex_unlock(&ams.lock);
> +
> +	return count;
> +}
>
> ...
>
> +/* Enables or disables the specified interrupts */
> +static void ams_pmu_set_irq(enum ams_irq reg, char enable)
> +{
> +	if (reg & AMS_IRQ_FREEFALL) {
> +		u8 val = ams_pmu_get_register(AMS_FF_ENABLE);
> +		if (enable)	val |= 0x80;
> +		else		val &= ~0x80;
> +		ams_pmu_set_register(AMS_FF_ENABLE, val);
> +	}
> +
> +	if (reg & AMS_IRQ_SHOCK) {
> +		u8 val = ams_pmu_get_register(AMS_SHOCK_ENABLE);
> +		if (enable)	val |= 0x80;
> +		else		val &= ~0x80;
> +		ams_pmu_set_register(AMS_SHOCK_ENABLE, val);
> +	}
> +
> +	if (reg & AMS_IRQ_GLOBAL) {
> +		u8 val = ams_pmu_get_register(AMS_CONTROL);
> +		if (enable)	val |= 0x80;
> +		else		val &= ~0x80;
> +		ams_pmu_set_register(AMS_CONTROL, val);
> +	}
> +}

Again - coding style is wrong.

> +/* Call with lock held! */
> +int __init ams_pmu_init(struct device_node *np)

Which lock??

> +{
> +	u32 *prop;
> +	int result;
> +
> +	mutex_lock(&ams.lock);

Obviously not that one...

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Michael Hanselmann]

On Thu, Aug 03, 2006 at 12:36:55AM -0700, Andrew Morton wrote:
> > +/* There is only one motion sensor per machine */
> > +struct ams ams;

> That's a somewhat risky name for a global variable.

Changed it to ams_info, which should be a bit more unique.

> > +/* Call with lock held! */
> > +int ams_sensor_attach(void)

> Which lock?

ams_info.lock, changed all such comments

> > +static int ams_i2c_cmd(enum ams_i2c_cmd cmd)
> > +{
> > [?]
> > +}

> That's a potentially very long busy wait.

I would like if Aristeu or Stelian could fix this, because I don't own
the I?C hardware myself. Thus I left it untouched in the patch below.

> > +	ams.kthread = kthread_run(ams_mouse_kthread, NULL, "kams");
> > +	if (IS_ERR(ams.kthread)) {
> > +		input_unregister_device(ams.idev);
> > +		ams.idev = NULL;
> > +		return;

> Didn't we just leak ams.idev?  Or does the disconnect handler handle that?

Indeed we do. Fixed.

Signed-off-by: Michael Hanselmann <linux-kernel at hansmi.ch>

---
diff -Nrup --exclude-from linux-exclude-from linux-2.6.18-rc3.orig/drivers/hwmon/ams/ams-core.c linux-2.6.18-rc3/drivers/hwmon/ams/ams-core.c
--- linux-2.6.18-rc3.orig/drivers/hwmon/ams/ams-core.c	2006-08-03 21:36:49.000000000 +0200
+++ linux-2.6.18-rc3/drivers/hwmon/ams/ams-core.c	2006-08-03 21:27:28.000000000 +0200
@@ -29,22 +29,22 @@
 #include "ams.h"
 
 /* There is only one motion sensor per machine */
-struct ams ams;
+struct ams ams_info;
 
 static unsigned int verbose;
 module_param(verbose, bool, 0644);
 MODULE_PARM_DESC(verbose, "Show free falls and shocks in kernel output");
 
-/* Call with lock held! */
+/* Call with ams_info.lock held! */
 void ams_sensors(s8 *x, s8 *y, s8 *z)
 {
-	u32 orient = ams.vflag? ams.orient1 : ams.orient2;
+	u32 orient = ams_info.vflag? ams_info.orient1 : ams_info.orient2;
 
 	if (orient & 0x80)
 		/* X and Y swapped */
-		ams.get_xyz(y, x, z);
+		ams_info.get_xyz(y, x, z);
 	else
-		ams.get_xyz(x, y, z);
+		ams_info.get_xyz(x, y, z);
 
 	if (orient & 0x04)
 		*z = ~(*z);
@@ -59,9 +59,9 @@ static ssize_t ams_show_current(struct d
 {
 	s8 x, y, z;
 
-	mutex_lock(&ams.lock);
+	mutex_lock(&ams_info.lock);
 	ams_sensors(&x, &y, &z);
-	mutex_unlock(&ams.lock);
+	mutex_unlock(&ams_info.lock);
 
 	return snprintf(buf, PAGE_SIZE, "%d %d %d\n", x, y, z);
 }
@@ -72,12 +72,12 @@ static void ams_handle_irq(void *data)
 {
 	enum ams_irq irq = *((enum ams_irq *)data);
 
-	spin_lock(&ams.irq_lock);
+	spin_lock(&ams_info.irq_lock);
 
-	ams.worker_irqs |= irq;
-	schedule_work(&ams.worker);
+	ams_info.worker_irqs |= irq;
+	schedule_work(&ams_info.worker);
 
-	spin_unlock(&ams.irq_lock);
+	spin_unlock(&ams_info.irq_lock);
 }
 
 static enum ams_irq ams_freefall_irq_data = AMS_IRQ_FREEFALL;
@@ -99,68 +99,68 @@ static struct pmf_irq_client ams_shock_c
  */
 static void ams_worker(void *data)
 {
-	mutex_lock(&ams.lock);
+	mutex_lock(&ams_info.lock);
 
-	if (ams.has_device) {
+	if (ams_info.has_device) {
 		unsigned long flags;
 
-		spin_lock_irqsave(&ams.irq_lock, flags);
+		spin_lock_irqsave(&ams_info.irq_lock, flags);
 
-		if (ams.worker_irqs & AMS_IRQ_FREEFALL) {
+		if (ams_info.worker_irqs & AMS_IRQ_FREEFALL) {
 			if (verbose)
 				printk(KERN_INFO "ams: freefall detected!\n");
 
-			ams.worker_irqs &= ~AMS_IRQ_FREEFALL;
+			ams_info.worker_irqs &= ~AMS_IRQ_FREEFALL;
 
 			/* we must call this with interrupts enabled */
-			spin_unlock_irqrestore(&ams.irq_lock, flags);
-			ams.clear_irq(AMS_IRQ_FREEFALL);
-			spin_lock_irqsave(&ams.irq_lock, flags);
+			spin_unlock_irqrestore(&ams_info.irq_lock, flags);
+			ams_info.clear_irq(AMS_IRQ_FREEFALL);
+			spin_lock_irqsave(&ams_info.irq_lock, flags);
 		}
 
-		if (ams.worker_irqs & AMS_IRQ_SHOCK) {
+		if (ams_info.worker_irqs & AMS_IRQ_SHOCK) {
 			if (verbose)
 				printk(KERN_INFO "ams: shock detected!\n");
 
-			ams.worker_irqs &= ~AMS_IRQ_SHOCK;
+			ams_info.worker_irqs &= ~AMS_IRQ_SHOCK;
 
 			/* we must call this with interrupts enabled */
-			spin_unlock_irqrestore(&ams.irq_lock, flags);
-			ams.clear_irq(AMS_IRQ_SHOCK);
-			spin_lock_irqsave(&ams.irq_lock, flags);
+			spin_unlock_irqrestore(&ams_info.irq_lock, flags);
+			ams_info.clear_irq(AMS_IRQ_SHOCK);
+			spin_lock_irqsave(&ams_info.irq_lock, flags);
 		}
 
-		spin_unlock_irqrestore(&ams.irq_lock, flags);
+		spin_unlock_irqrestore(&ams_info.irq_lock, flags);
 	}
 
-	mutex_unlock(&ams.lock);
+	mutex_unlock(&ams_info.lock);
 }
 
-/* Call with lock held! */
+/* Call with ams_info.lock held! */
 int ams_sensor_attach(void)
 {
 	int result;
 	u32 *prop;
 
 	/* Get orientation */
-	prop = (u32*)get_property(ams.of_node, "orientation", NULL);
+	prop = (u32*)get_property(ams_info.of_node, "orientation", NULL);
 	if (!prop)
 		return -ENODEV;
-	ams.orient1 = *prop;
-	ams.orient2 = *(prop + 1);
+	ams_info.orient1 = *prop;
+	ams_info.orient2 = *(prop + 1);
 
 	/* Register freefall interrupt handler */
-	result = pmf_register_irq_client(ams.of_node,
+	result = pmf_register_irq_client(ams_info.of_node,
 			"accel-int-1",
 			&ams_freefall_client);
 	if (result < 0)
 		return -ENODEV;
 
 	/* Reset saved irqs */
-	ams.worker_irqs = 0;
+	ams_info.worker_irqs = 0;
 
 	/* Register shock interrupt handler */
-	result = pmf_register_irq_client(ams.of_node,
+	result = pmf_register_irq_client(ams_info.of_node,
 			"accel-int-2",
 			&ams_shock_client);
 	if (result < 0) {
@@ -169,17 +169,17 @@ int ams_sensor_attach(void)
 	}
 
 	/* Create device */
-	ams.of_dev = of_platform_device_create(ams.of_node, "ams", NULL);
-	if (!ams.of_dev) {
+	ams_info.of_dev = of_platform_device_create(ams_info.of_node, "ams", NULL);
+	if (!ams_info.of_dev) {
 		pmf_unregister_irq_client(&ams_shock_client);
 		pmf_unregister_irq_client(&ams_freefall_client);
 		return -ENODEV;
 	}
 
 	/* Create attributes */
-	device_create_file(&ams.of_dev->dev, &dev_attr_current);
+	device_create_file(&ams_info.of_dev->dev, &dev_attr_current);
 
-	ams.vflag = !!(ams.get_vendor() & 0x10);
+	ams_info.vflag = !!(ams_info.get_vendor() & 0x10);
 
 	/* Init mouse device */
 	ams_mouse_init();
@@ -191,9 +191,9 @@ int __init ams_init(void)
 {
 	struct device_node *np;
 
-	spin_lock_init(&ams.irq_lock);
-	mutex_init(&ams.lock);
-	INIT_WORK(&ams.worker, ams_worker, NULL);
+	spin_lock_init(&ams_info.irq_lock);
+	mutex_init(&ams_info.lock);
+	INIT_WORK(&ams_info.worker, ams_worker, NULL);
 
 #ifdef CONFIG_SENSORS_AMS_I2C
 	np = of_find_node_by_name(NULL, "accelerometer");
@@ -216,35 +216,35 @@ int __init ams_init(void)
 
 void ams_exit(void)
 {
-	mutex_lock(&ams.lock);
+	mutex_lock(&ams_info.lock);
 
-	if (ams.has_device) {
+	if (ams_info.has_device) {
 		/* Remove mouse device */
 		ams_mouse_exit();
 
 		/* Shut down implementation */
-		ams.exit();
+		ams_info.exit();
 
 		/* Cancel interrupt worker
 		 *
-		 * We do this after ams.exit(), because an interrupt might
+		 * We do this after ams_info.exit(), because an interrupt might
 		 * have arrived before disabling them.
 		 */
-		cancel_delayed_work(&ams.worker);
+		cancel_delayed_work(&ams_info.worker);
 		flush_scheduled_work();
 
 		/* Remove attributes */
-		device_remove_file(&ams.of_dev->dev, &dev_attr_current);
+		device_remove_file(&ams_info.of_dev->dev, &dev_attr_current);
 
 		/* Remove device */
-		of_device_unregister(ams.of_dev);
+		of_device_unregister(ams_info.of_dev);
 
 		/* Remove handler */
 		pmf_unregister_irq_client(&ams_shock_client);
 		pmf_unregister_irq_client(&ams_freefall_client);
 	}
 
-	mutex_unlock(&ams.lock);
+	mutex_unlock(&ams_info.lock);
 }
 
 MODULE_AUTHOR("Stelian Pop, Michael Hanselmann");
diff -Nrup --exclude-from linux-exclude-from linux-2.6.18-rc3.orig/drivers/hwmon/ams/ams.h linux-2.6.18-rc3/drivers/hwmon/ams/ams.h
--- linux-2.6.18-rc3.orig/drivers/hwmon/ams/ams.h	2006-08-03 21:36:49.000000000 +0200
+++ linux-2.6.18-rc3/drivers/hwmon/ams/ams.h	2006-08-03 21:19:28.000000000 +0200
@@ -60,7 +60,7 @@ struct ams {
 	int xcalib, ycalib, zcalib;
 };
 
-extern struct ams ams;
+extern struct ams ams_info;
 
 extern void ams_sensors(s8 *x, s8 *y, s8 *z);
 extern int ams_sensor_attach(void);
diff -Nrup --exclude-from linux-exclude-from linux-2.6.18-rc3.orig/drivers/hwmon/ams/ams-i2c.c linux-2.6.18-rc3/drivers/hwmon/ams/ams-i2c.c
--- linux-2.6.18-rc3.orig/drivers/hwmon/ams/ams-i2c.c	2006-08-03 21:36:49.000000000 +0200
+++ linux-2.6.18-rc3/drivers/hwmon/ams/ams-i2c.c	2006-08-03 21:29:58.000000000 +0200
@@ -74,12 +74,12 @@ static struct i2c_driver ams_i2c_driver 
 
 static s32 ams_i2c_read(u8 reg)
 {
-	return i2c_smbus_read_byte_data(&ams.i2c_client, reg);
+	return i2c_smbus_read_byte_data(&ams_info.i2c_client, reg);
 }
 
 static int ams_i2c_write(u8 reg, u8 value)
 {
-	return i2c_smbus_write_byte_data(&ams.i2c_client, reg, value);
+	return i2c_smbus_write_byte_data(&ams_info.i2c_client, reg, value);
 }
 
 static int ams_i2c_cmd(enum ams_i2c_cmd cmd)
@@ -101,22 +101,28 @@ static void ams_i2c_set_irq(enum ams_irq
 {
 	if (reg & AMS_IRQ_FREEFALL) {
 		u8 val = ams_i2c_read(AMS_CTRLX);
-		if (enable)     val |= 0x80;
-		else            val &= ~0x80;
+		if (enable)
+			val |= 0x80;
+		else
+			val &= ~0x80;
 		ams_i2c_write(AMS_CTRLX, val);
 	}
 
 	if (reg & AMS_IRQ_SHOCK) {
 		u8 val = ams_i2c_read(AMS_CTRLY);
-		if (enable)     val |= 0x80;
-		else            val &= ~0x80;
+		if (enable)
+			val |= 0x80;
+		else
+			val &= ~0x80;
 		ams_i2c_write(AMS_CTRLY, val);
 	}
 
 	if (reg & AMS_IRQ_GLOBAL) {
 		u8 val = ams_i2c_read(AMS_CTRLZ);
-		if (enable)     val |= 0x80;
-		else            val &= ~0x80;
+		if (enable)
+			val |= 0x80;
+		else
+			val &= ~0x80;
 		ams_i2c_write(AMS_CTRLZ, val);
 	}
 }
@@ -149,20 +155,20 @@ static int ams_i2c_attach(struct i2c_ada
 	int result;
 
 	/* There can be only one */
-	if (unlikely(ams.has_device))
+	if (unlikely(ams_info.has_device))
 		return -ENODEV;
 
 	if (strncmp(adapter->name, "uni-n", 5))
 		return -ENODEV;
 
 	bus = simple_strtoul(adapter->name + 6, NULL, 10);
-	if (bus != ams.i2c_bus)
+	if (bus != ams_info.i2c_bus)
 		return -ENODEV;
 
-	ams.i2c_client.addr = ams.i2c_address;
-	ams.i2c_client.adapter = adapter;
-	ams.i2c_client.driver = &ams_i2c_driver;
-	strcpy(ams.i2c_client.name, "Apple Motion Sensor");
+	ams_info.i2c_client.addr = ams_info.i2c_address;
+	ams_info.i2c_client.adapter = adapter;
+	ams_info.i2c_client.driver = &ams_i2c_driver;
+	strcpy(ams_info.i2c_client.name, "Apple Motion Sensor");
 
 	if (ams_i2c_cmd(AMS_CMD_RESET)) {
 		printk(KERN_INFO "ams: Failed to reset the device\n");
@@ -217,7 +223,7 @@ static int ams_i2c_attach(struct i2c_ada
 	/* Clear interrupts */
 	ams_i2c_clear_irq(AMS_IRQ_ALL);
 
-	ams.has_device = 1;
+	ams_info.has_device = 1;
 
 	/* Enable interrupts */
 	ams_i2c_set_irq(AMS_IRQ_ALL, 1);
@@ -229,7 +235,7 @@ static int ams_i2c_attach(struct i2c_ada
 
 static int ams_i2c_detach(struct i2c_adapter *adapter)
 {
-	if (ams.has_device) {
+	if (ams_info.has_device) {
 		/* Disable interrupts */
 		ams_i2c_set_irq(AMS_IRQ_ALL, 0);
 
@@ -238,7 +244,7 @@ static int ams_i2c_detach(struct i2c_ada
 
 		printk(KERN_INFO "ams: Unloading\n");
 
-		ams.has_device = 0;
+		ams_info.has_device = 0;
 	}
 
 	return 0;
@@ -255,35 +261,35 @@ int __init ams_i2c_init(struct device_no
 	int result;
 	u32 *prop;
 
-	mutex_lock(&ams.lock);
+	mutex_lock(&ams_info.lock);
 
 	/* Set implementation stuff */
-	ams.of_node = np;
-	ams.exit = ams_i2c_exit;
-	ams.get_vendor = ams_i2c_get_vendor;
-	ams.get_xyz = ams_i2c_get_xyz;
-	ams.clear_irq = ams_i2c_clear_irq;
-	ams.bustype = BUS_I2C;
+	ams_info.of_node = np;
+	ams_info.exit = ams_i2c_exit;
+	ams_info.get_vendor = ams_i2c_get_vendor;
+	ams_info.get_xyz = ams_i2c_get_xyz;
+	ams_info.clear_irq = ams_i2c_clear_irq;
+	ams_info.bustype = BUS_I2C;
 
 	/* look for bus either using "reg" or by path */
-	prop = (u32*)get_property(ams.of_node, "reg", NULL);
+	prop = (u32*)get_property(ams_info.of_node, "reg", NULL);
 	if (!prop) {
 		result = -ENODEV;
 
 		goto exit;
 	}
 
-	tmp_bus = strstr(ams.of_node->full_name, "/i2c-bus@");
+	tmp_bus = strstr(ams_info.of_node->full_name, "/i2c-bus@");
 	if (tmp_bus)
-		ams.i2c_bus = *(tmp_bus + 9) - '0';
+		ams_info.i2c_bus = *(tmp_bus + 9) - '0';
 	else
-		ams.i2c_bus = ((*prop) >> 8) & 0x0f;
-	ams.i2c_address = ((*prop) & 0xff) >> 1;
+		ams_info.i2c_bus = ((*prop) >> 8) & 0x0f;
+	ams_info.i2c_address = ((*prop) & 0xff) >> 1;
 
 	result = i2c_add_driver(&ams_i2c_driver);
 
 exit:
-	mutex_unlock(&ams.lock);
+	mutex_unlock(&ams_info.lock);
 
 	return result;
 }
diff -Nrup --exclude-from linux-exclude-from linux-2.6.18-rc3.orig/drivers/hwmon/ams/ams-mouse.c linux-2.6.18-rc3/drivers/hwmon/ams/ams-mouse.c
--- linux-2.6.18-rc3.orig/drivers/hwmon/ams/ams-mouse.c	2006-08-03 21:36:49.000000000 +0200
+++ linux-2.6.18-rc3/drivers/hwmon/ams/ams-mouse.c	2006-08-03 21:32:30.000000000 +0200
@@ -28,17 +28,17 @@ static int ams_mouse_kthread(void *data)
 	s8 x, y, z;
 
 	while (!kthread_should_stop()) {
-		mutex_lock(&ams.lock);
+		mutex_lock(&ams_info.lock);
 
 		ams_sensors(&x, &y, &z);
 
-		input_report_abs(ams.idev, ABS_X, x - ams.xcalib);
-		input_report_abs(ams.idev, ABS_Y, y - ams.ycalib);
-		input_report_abs(ams.idev, ABS_Z, z - ams.zcalib);
+		input_report_abs(ams_info.idev, ABS_X, x - ams_info.xcalib);
+		input_report_abs(ams_info.idev, ABS_Y, y - ams_info.ycalib);
+		input_report_abs(ams_info.idev, ABS_Z, z - ams_info.zcalib);
 
-		input_sync(ams.idev);
+		input_sync(ams_info.idev);
 
-		mutex_unlock(&ams.lock);
+		mutex_unlock(&ams_info.lock);
 
 		msleep(25);
 	}
@@ -46,56 +46,57 @@ static int ams_mouse_kthread(void *data)
 	return 0;
 }
 
-/* Call with lock held! */
+/* Call with ams_info.lock held! */
 static void ams_mouse_enable(void)
 {
 	s8 x, y, z;
 
-	if (ams.idev)
+	if (ams_info.idev)
 		return;
 
 	ams_sensors(&x, &y, &z);
-	ams.xcalib = x;
-	ams.ycalib = y;
-	ams.zcalib = z;
+	ams_info.xcalib = x;
+	ams_info.ycalib = y;
+	ams_info.zcalib = z;
 
-	ams.idev = input_allocate_device();
-	if (!ams.idev)
+	ams_info.idev = input_allocate_device();
+	if (!ams_info.idev)
 		return;
 
-	ams.idev->name = "Apple Motion Sensor";
-	ams.idev->id.bustype = ams.bustype;
-	ams.idev->id.vendor = 0;
-	ams.idev->cdev.dev = &ams.of_dev->dev;
-
-	input_set_abs_params(ams.idev, ABS_X, -50, 50, 3, 0);
-	input_set_abs_params(ams.idev, ABS_Y, -50, 50, 3, 0);
-	input_set_abs_params(ams.idev, ABS_Z, -50, 50, 3, 0);
-
-	set_bit(EV_ABS, ams.idev->evbit);
-	set_bit(EV_KEY, ams.idev->evbit);
-
-	if (input_register_device(ams.idev)) {
-		input_free_device(ams.idev);
-		ams.idev = NULL;
+	ams_info.idev->name = "Apple Motion Sensor";
+	ams_info.idev->id.bustype = ams_info.bustype;
+	ams_info.idev->id.vendor = 0;
+	ams_info.idev->cdev.dev = &ams_info.of_dev->dev;
+
+	input_set_abs_params(ams_info.idev, ABS_X, -50, 50, 3, 0);
+	input_set_abs_params(ams_info.idev, ABS_Y, -50, 50, 3, 0);
+	input_set_abs_params(ams_info.idev, ABS_Z, -50, 50, 3, 0);
+
+	set_bit(EV_ABS, ams_info.idev->evbit);
+	set_bit(EV_KEY, ams_info.idev->evbit);
+
+	if (input_register_device(ams_info.idev)) {
+		input_free_device(ams_info.idev);
+		ams_info.idev = NULL;
 		return;
 	}
 
-	ams.kthread = kthread_run(ams_mouse_kthread, NULL, "kams");
-	if (IS_ERR(ams.kthread)) {
-		input_unregister_device(ams.idev);
-		ams.idev = NULL;
+	ams_info.kthread = kthread_run(ams_mouse_kthread, NULL, "kams");
+	if (IS_ERR(ams_info.kthread)) {
+		input_unregister_device(ams_info.idev);
+		input_free_device(ams_info.idev);
+		ams_info.idev = NULL;
 		return;
 	}
 }
 
-/* Call with lock held! */
+/* Call with ams_info.lock held! */
 static void ams_mouse_disable(void)
 {
-	if (ams.idev) {
-		kthread_stop(ams.kthread);
-		input_unregister_device(ams.idev);
-		ams.idev = NULL;
+	if (ams_info.idev) {
+		kthread_stop(ams_info.kthread);
+		input_unregister_device(ams_info.idev);
+		ams_info.idev = NULL;
 	}
 }
 
@@ -111,16 +112,14 @@ static ssize_t ams_mouse_store_mouse(str
 	if (sscanf(buf, "%d\n", &mouse) != 1)
 		return -EINVAL;
 
-	mouse = !!mouse;
-
-	mutex_lock(&ams.lock);
+	mutex_lock(&ams_info.lock);
 
 	if (mouse)
 		ams_mouse_enable();
 	else
 		ams_mouse_disable();
 
-	mutex_unlock(&ams.lock);
+	mutex_unlock(&ams_info.lock);
 
 	return count;
 }
@@ -128,18 +127,18 @@ static ssize_t ams_mouse_store_mouse(str
 static DEVICE_ATTR(mouse, S_IRUGO | S_IWUSR,
 	ams_mouse_show_mouse, ams_mouse_store_mouse);
 
-/* Call with lock held! */
+/* Call with ams_info.lock held! */
 void ams_mouse_init()
 {
-	device_create_file(&ams.of_dev->dev, &dev_attr_mouse);
+	device_create_file(&ams_info.of_dev->dev, &dev_attr_mouse);
 
 	if (mouse)
 		ams_mouse_enable();
 }
 
-/* Call with lock held! */
+/* Call with ams_info.lock held! */
 void ams_mouse_exit()
 {
 	ams_mouse_disable();
-	device_remove_file(&ams.of_dev->dev, &dev_attr_mouse);
+	device_remove_file(&ams_info.of_dev->dev, &dev_attr_mouse);
 }
diff -Nrup --exclude-from linux-exclude-from linux-2.6.18-rc3.orig/drivers/hwmon/ams/ams-pmu.c linux-2.6.18-rc3/drivers/hwmon/ams/ams-pmu.c
--- linux-2.6.18-rc3.orig/drivers/hwmon/ams/ams-pmu.c	2006-08-03 21:36:49.000000000 +0200
+++ linux-2.6.18-rc3/drivers/hwmon/ams/ams-pmu.c	2006-08-03 21:34:12.000000000 +0200
@@ -84,22 +84,28 @@ static void ams_pmu_set_irq(enum ams_irq
 {
 	if (reg & AMS_IRQ_FREEFALL) {
 		u8 val = ams_pmu_get_register(AMS_FF_ENABLE);
-		if (enable)	val |= 0x80;
-		else		val &= ~0x80;
+		if (enable)
+			val |= 0x80;
+		else
+			val &= ~0x80;
 		ams_pmu_set_register(AMS_FF_ENABLE, val);
 	}
 
 	if (reg & AMS_IRQ_SHOCK) {
 		u8 val = ams_pmu_get_register(AMS_SHOCK_ENABLE);
-		if (enable)	val |= 0x80;
-		else		val &= ~0x80;
+		if (enable)
+			val |= 0x80;
+		else
+			val &= ~0x80;
 		ams_pmu_set_register(AMS_SHOCK_ENABLE, val);
 	}
 
 	if (reg & AMS_IRQ_GLOBAL) {
 		u8 val = ams_pmu_get_register(AMS_CONTROL);
-		if (enable)	val |= 0x80;
-		else		val &= ~0x80;
+		if (enable)
+			val |= 0x80;
+		else
+			val &= ~0x80;
 		ams_pmu_set_register(AMS_CONTROL, val);
 	}
 }
@@ -133,29 +139,28 @@ static void ams_pmu_exit(void)
 	/* Clear interrupts */
 	ams_pmu_clear_irq(AMS_IRQ_ALL);
 
-	ams.has_device = 0;
+	ams_info.has_device = 0;
 
 	printk(KERN_INFO "ams: Unloading\n");
 }
 
-/* Call with lock held! */
 int __init ams_pmu_init(struct device_node *np)
 {
 	u32 *prop;
 	int result;
 
-	mutex_lock(&ams.lock);
+	mutex_lock(&ams_info.lock);
 
 	/* Set implementation stuff */
-	ams.of_node = np;
-	ams.exit = ams_pmu_exit;
-	ams.get_vendor = ams_pmu_get_vendor;
-	ams.get_xyz = ams_pmu_get_xyz;
-	ams.clear_irq = ams_pmu_clear_irq;
-	ams.bustype = BUS_HOST;
+	ams_info.of_node = np;
+	ams_info.exit = ams_pmu_exit;
+	ams_info.get_vendor = ams_pmu_get_vendor;
+	ams_info.get_xyz = ams_pmu_get_xyz;
+	ams_info.clear_irq = ams_pmu_clear_irq;
+	ams_info.bustype = BUS_HOST;
 
 	/* Get PMU command, should be 0x4e, but we can never know */
-	prop = (u32*)get_property(ams.of_node, "reg", NULL);
+	prop = (u32*)get_property(ams_info.of_node, "reg", NULL);
 	if (!prop) {
 		result = -ENODEV;
 		goto exit;
@@ -186,7 +191,7 @@ int __init ams_pmu_init(struct device_no
 	/* Clear interrupts */
 	ams_pmu_clear_irq(AMS_IRQ_ALL);
 
-	ams.has_device = 1;
+	ams_info.has_device = 1;
 
 	/* Enable interrupts */
 	ams_pmu_set_irq(AMS_IRQ_ALL, 1);
@@ -196,7 +201,7 @@ int __init ams_pmu_init(struct device_no
 	result = 0;
 
 exit:
-	mutex_unlock(&ams.lock);
+	mutex_unlock(&ams_info.lock);
 
 	return result;
 }

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Aristeu Sergio Rozanski Filho]

> I would like if Aristeu or Stelian could fix this, because I don't own
> the I??C hardware myself. Thus I left it untouched in the patch below.
will take a look on this tonight

-- 
Aristeu

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Johannes Berg]

Andrew Morton wrote:
> Also, the previous patch had an s-o-b from Johannes.  Does this version
> need one too?  
>   
I didn't really write much code in there, but for any that I might have 
written I can assure that it's ok to use:

Signed-off-by: Johannes Berg <johannes at sipsolutions.net>

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Johannes Berg]

Michael Hanselmann wrote:
> Please apply to -mm for testing.
>   

Btw. I really wonder... Does anyone with a powerbook run/test -mm?

johannes

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Aristeu Sergio Rozanski Filho]

> > That's a potentially very long busy wait.
> 
> I would like if Aristeu or Stelian could fix this, because I don't own
> the I??C hardware myself. Thus I left it untouched in the patch below.
what about the attached patch? Running right now a kernel with the
original patch + Michael fixes + this one and no problems so far.

-- 
Aristeu

-------------- next part --------------
Index: ppc-2.6/drivers/hwmon/ams/ams-i2c.c
=================================--- ppc-2.6.orig/drivers/hwmon/ams/ams-i2c.c	2006-08-04 10:47:57.000000000 -0300
+++ ppc-2.6/drivers/hwmon/ams/ams-i2c.c	2006-08-04 10:48:26.000000000 -0300
@@ -85,11 +85,11 @@
 static int ams_i2c_cmd(enum ams_i2c_cmd cmd)
 {
 	s32 result;
-	int i;
+	int remaining = HZ/20;
 
 	ams_i2c_write(AMS_COMMAND, cmd);
-	for (i = 0; i < 10; i++) {
-		mdelay(5);
+	while (remaining) {
+		remaining = schedule_timeout(remaining);
 		result = ams_i2c_read(AMS_COMMAND);
 		if (result = 0 || result & 0x80)
 			return 0;

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Michael Hanselmann]

On Fri, Aug 04, 2006 at 11:11:31AM -0300, Aristeu Sergio Rozanski Filho wrote:
> what about the attached patch?

>  	ams_i2c_write(AMS_COMMAND, cmd);
> -	for (i = 0; i < 10; i++) {
> -		mdelay(5);

This guarantees a minimum wait time of 5msec after writing the value.
Looking at the specs[1], this should be done. Your code doesn't do that:

> +	while (remaining) {
> +		remaining = schedule_timeout(remaining);
>  		result = ams_i2c_read(AMS_COMMAND);
>  		if (result = 0 || result & 0x80)
>  			return 0;

I'm not sure wether the 5msec delay is really required, but I would
insert it if it's in the specs.

So far, NACK until we got that right.

Next time, please insert the patch inline.

Thanks,
Michael

[1] http://johannes.sipsolutions.net/PowerBook/Apple_Motion_Sensor_Specification/#head-cc4be6b93ad9fcb8d373ad340b12891ed920a05f

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Aristeu Sergio Rozanski Filho]

> This guarantees a minimum wait time of 5msec after writing the value.
> Looking at the specs[1], this should be done. Your code doesn't do that:
I doubt that if it's possible to retrying after a fail the hardware will
bother if we try to do the first read in less than 5ms... but here it
goes. two versions, feel free to choose.

Index: ppc-2.6/drivers/hwmon/ams/ams-i2c.c
=================================--- ppc-2.6.orig/drivers/hwmon/ams/ams-i2c.c	2006-08-04 10:47:57.000000000 -0300
+++ ppc-2.6/drivers/hwmon/ams/ams-i2c.c	2006-08-04 17:18:50.000000000 -0300
@@ -85,14 +85,15 @@
 static int ams_i2c_cmd(enum ams_i2c_cmd cmd)
 {
 	s32 result;
-	int i;
+	int remaining = HZ/20;
 
 	ams_i2c_write(AMS_COMMAND, cmd);
-	for (i = 0; i < 10; i++) {
-		mdelay(5);
+	mdelay(5);
+	while (remaining) {
 		result = ams_i2c_read(AMS_COMMAND);
 		if (result = 0 || result & 0x80)
 			return 0;
+		remaining = schedule_timeout(remaining);
 	}
 	return -1;
 }


Index: ppc-2.6/drivers/hwmon/ams/ams-i2c.c
=================================--- ppc-2.6.orig/drivers/hwmon/ams/ams-i2c.c	2006-08-04 10:47:57.000000000 -0300
+++ ppc-2.6/drivers/hwmon/ams/ams-i2c.c	2006-08-04 17:34:14.000000000 -0300
@@ -85,11 +85,18 @@
 static int ams_i2c_cmd(enum ams_i2c_cmd cmd)
 {
 	s32 result;
-	int i;
+	int remaining = HZ/20, tmp;
 
 	ams_i2c_write(AMS_COMMAND, cmd);
-	for (i = 0; i < 10; i++) {
-		mdelay(5);
+	while (remaining) {
+		tmp = remaining;
+		while(1) {
+			remaining = schedule_timeout(remaining);
+			/* wait at least 5ms */
+			if (!remaining || tmp - remaining > HZ/200)
+				break;
+		}
+
 		result = ams_i2c_read(AMS_COMMAND);
 		if (result = 0 || result & 0x80)
 			return 0;

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Michael Hanselmann]

On Fri, Aug 04, 2006 at 05:39:32PM -0300, Aristeu Sergio Rozanski Filho wrote:
> > This guarantees a minimum wait time of 5msec after writing the value.
> > Looking at the specs[1], this should be done. Your code doesn't do that:

> I doubt that if it's possible to retrying after a fail the hardware will
> bother if we try to do the first read in less than 5ms... but here it
> goes. two versions, feel free to choose.

The specs are written based on the Mac OS X driver, and I think Apple
knows their hardware.

After discussing both variants, Ren? Nussbaumer and myself prefer the
first one.

Acked-by: Michael Hanselmann <linux-kernel at hansmi.ch>
Acked-by: Ren? Nussbaumer <linux-kernel at killerfox.forkbomb.ch>

Greets,
Michael

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Andrew Morton]

On Fri, 4 Aug 2006 23:49:24 +0200
Michael Hanselmann <linux-kernel at hansmi.ch> wrote:

> On Fri, Aug 04, 2006 at 05:39:32PM -0300, Aristeu Sergio Rozanski Filho wrote:
> > > This guarantees a minimum wait time of 5msec after writing the value.
> > > Looking at the specs[1], this should be done. Your code doesn't do that:
> 
> > I doubt that if it's possible to retrying after a fail the hardware will
> > bother if we try to do the first read in less than 5ms... but here it
> > goes. two versions, feel free to choose.
> 
> The specs are written based on the Mac OS X driver, and I think Apple
> knows their hardware.
> 
> After discussing both variants, Ren? Nussbaumer and myself prefer the
> first one.
> 
> Acked-by: Michael Hanselmann <linux-kernel at hansmi.ch>
> Acked-by: Ren? Nussbaumer <linux-kernel at killerfox.forkbomb.ch>
> 

Sorry, I've lost track of what's going on here.

Could I please be sent a new copy of the whole patch, with the appropriate
acked-bys, signed-off-bys, etc?

Thanks.

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Michael Hanselmann]

On Fri, Aug 04, 2006 at 03:19:52PM -0700, Andrew Morton wrote:
> Could I please be sent a new copy of the whole patch, with the appropriate
> acked-bys, signed-off-bys, etc?

Sure, here we go.

- Rename exported variable "ams" to "ams_info"
- Fix leaked input device on error
- Change I?C read code to use schedule_timeout instead of msleep() in
  case the first read failed

Signed-off-by: Michael Hanselmann <linux-kernel at hansmi.ch>
Signed-off-by: Aristeu Sergio Rozanski Filho <aris at cathedrallabs.org>
Acked-by: Ren? Nussbaumer <linux-kernel at killerfox.forkbomb.ch>

---
diff -Nrup --exclude-from linux-exclude-from linux-2.6.18-rc3.orig/drivers/hwmon/ams/ams-core.c linux-2.6.18-rc3/drivers/hwmon/ams/ams-core.c
--- linux-2.6.18-rc3.orig/drivers/hwmon/ams/ams-core.c	2006-08-05 00:47:46.000000000 +0200
+++ linux-2.6.18-rc3/drivers/hwmon/ams/ams-core.c	2006-08-05 00:49:56.000000000 +0200
@@ -29,22 +29,22 @@
 #include "ams.h"
 
 /* There is only one motion sensor per machine */
-struct ams ams;
+struct ams ams_info;
 
 static unsigned int verbose;
 module_param(verbose, bool, 0644);
 MODULE_PARM_DESC(verbose, "Show free falls and shocks in kernel output");
 
-/* Call with lock held! */
+/* Call with ams_info.lock held! */
 void ams_sensors(s8 *x, s8 *y, s8 *z)
 {
-	u32 orient = ams.vflag? ams.orient1 : ams.orient2;
+	u32 orient = ams_info.vflag? ams_info.orient1 : ams_info.orient2;
 
 	if (orient & 0x80)
 		/* X and Y swapped */
-		ams.get_xyz(y, x, z);
+		ams_info.get_xyz(y, x, z);
 	else
-		ams.get_xyz(x, y, z);
+		ams_info.get_xyz(x, y, z);
 
 	if (orient & 0x04)
 		*z = ~(*z);
@@ -59,9 +59,9 @@ static ssize_t ams_show_current(struct d
 {
 	s8 x, y, z;
 
-	mutex_lock(&ams.lock);
+	mutex_lock(&ams_info.lock);
 	ams_sensors(&x, &y, &z);
-	mutex_unlock(&ams.lock);
+	mutex_unlock(&ams_info.lock);
 
 	return snprintf(buf, PAGE_SIZE, "%d %d %d\n", x, y, z);
 }
@@ -72,12 +72,12 @@ static void ams_handle_irq(void *data)
 {
 	enum ams_irq irq = *((enum ams_irq *)data);
 
-	spin_lock(&ams.irq_lock);
+	spin_lock(&ams_info.irq_lock);
 
-	ams.worker_irqs |= irq;
-	schedule_work(&ams.worker);
+	ams_info.worker_irqs |= irq;
+	schedule_work(&ams_info.worker);
 
-	spin_unlock(&ams.irq_lock);
+	spin_unlock(&ams_info.irq_lock);
 }
 
 static enum ams_irq ams_freefall_irq_data = AMS_IRQ_FREEFALL;
@@ -99,68 +99,68 @@ static struct pmf_irq_client ams_shock_c
  */
 static void ams_worker(void *data)
 {
-	mutex_lock(&ams.lock);
+	mutex_lock(&ams_info.lock);
 
-	if (ams.has_device) {
+	if (ams_info.has_device) {
 		unsigned long flags;
 
-		spin_lock_irqsave(&ams.irq_lock, flags);
+		spin_lock_irqsave(&ams_info.irq_lock, flags);
 
-		if (ams.worker_irqs & AMS_IRQ_FREEFALL) {
+		if (ams_info.worker_irqs & AMS_IRQ_FREEFALL) {
 			if (verbose)
 				printk(KERN_INFO "ams: freefall detected!\n");
 
-			ams.worker_irqs &= ~AMS_IRQ_FREEFALL;
+			ams_info.worker_irqs &= ~AMS_IRQ_FREEFALL;
 
 			/* we must call this with interrupts enabled */
-			spin_unlock_irqrestore(&ams.irq_lock, flags);
-			ams.clear_irq(AMS_IRQ_FREEFALL);
-			spin_lock_irqsave(&ams.irq_lock, flags);
+			spin_unlock_irqrestore(&ams_info.irq_lock, flags);
+			ams_info.clear_irq(AMS_IRQ_FREEFALL);
+			spin_lock_irqsave(&ams_info.irq_lock, flags);
 		}
 
-		if (ams.worker_irqs & AMS_IRQ_SHOCK) {
+		if (ams_info.worker_irqs & AMS_IRQ_SHOCK) {
 			if (verbose)
 				printk(KERN_INFO "ams: shock detected!\n");
 
-			ams.worker_irqs &= ~AMS_IRQ_SHOCK;
+			ams_info.worker_irqs &= ~AMS_IRQ_SHOCK;
 
 			/* we must call this with interrupts enabled */
-			spin_unlock_irqrestore(&ams.irq_lock, flags);
-			ams.clear_irq(AMS_IRQ_SHOCK);
-			spin_lock_irqsave(&ams.irq_lock, flags);
+			spin_unlock_irqrestore(&ams_info.irq_lock, flags);
+			ams_info.clear_irq(AMS_IRQ_SHOCK);
+			spin_lock_irqsave(&ams_info.irq_lock, flags);
 		}
 
-		spin_unlock_irqrestore(&ams.irq_lock, flags);
+		spin_unlock_irqrestore(&ams_info.irq_lock, flags);
 	}
 
-	mutex_unlock(&ams.lock);
+	mutex_unlock(&ams_info.lock);
 }
 
-/* Call with lock held! */
+/* Call with ams_info.lock held! */
 int ams_sensor_attach(void)
 {
 	int result;
 	u32 *prop;
 
 	/* Get orientation */
-	prop = (u32*)get_property(ams.of_node, "orientation", NULL);
+	prop = (u32*)get_property(ams_info.of_node, "orientation", NULL);
 	if (!prop)
 		return -ENODEV;
-	ams.orient1 = *prop;
-	ams.orient2 = *(prop + 1);
+	ams_info.orient1 = *prop;
+	ams_info.orient2 = *(prop + 1);
 
 	/* Register freefall interrupt handler */
-	result = pmf_register_irq_client(ams.of_node,
+	result = pmf_register_irq_client(ams_info.of_node,
 			"accel-int-1",
 			&ams_freefall_client);
 	if (result < 0)
 		return -ENODEV;
 
 	/* Reset saved irqs */
-	ams.worker_irqs = 0;
+	ams_info.worker_irqs = 0;
 
 	/* Register shock interrupt handler */
-	result = pmf_register_irq_client(ams.of_node,
+	result = pmf_register_irq_client(ams_info.of_node,
 			"accel-int-2",
 			&ams_shock_client);
 	if (result < 0) {
@@ -169,17 +169,17 @@ int ams_sensor_attach(void)
 	}
 
 	/* Create device */
-	ams.of_dev = of_platform_device_create(ams.of_node, "ams", NULL);
-	if (!ams.of_dev) {
+	ams_info.of_dev = of_platform_device_create(ams_info.of_node, "ams", NULL);
+	if (!ams_info.of_dev) {
 		pmf_unregister_irq_client(&ams_shock_client);
 		pmf_unregister_irq_client(&ams_freefall_client);
 		return -ENODEV;
 	}
 
 	/* Create attributes */
-	device_create_file(&ams.of_dev->dev, &dev_attr_current);
+	device_create_file(&ams_info.of_dev->dev, &dev_attr_current);
 
-	ams.vflag = !!(ams.get_vendor() & 0x10);
+	ams_info.vflag = !!(ams_info.get_vendor() & 0x10);
 
 	/* Init mouse device */
 	ams_mouse_init();
@@ -191,9 +191,9 @@ int __init ams_init(void)
 {
 	struct device_node *np;
 
-	spin_lock_init(&ams.irq_lock);
-	mutex_init(&ams.lock);
-	INIT_WORK(&ams.worker, ams_worker, NULL);
+	spin_lock_init(&ams_info.irq_lock);
+	mutex_init(&ams_info.lock);
+	INIT_WORK(&ams_info.worker, ams_worker, NULL);
 
 #ifdef CONFIG_SENSORS_AMS_I2C
 	np = of_find_node_by_name(NULL, "accelerometer");
@@ -216,35 +216,35 @@ int __init ams_init(void)
 
 void ams_exit(void)
 {
-	mutex_lock(&ams.lock);
+	mutex_lock(&ams_info.lock);
 
-	if (ams.has_device) {
+	if (ams_info.has_device) {
 		/* Remove mouse device */
 		ams_mouse_exit();
 
 		/* Shut down implementation */
-		ams.exit();
+		ams_info.exit();
 
 		/* Cancel interrupt worker
 		 *
-		 * We do this after ams.exit(), because an interrupt might
+		 * We do this after ams_info.exit(), because an interrupt might
 		 * have arrived before disabling them.
 		 */
-		cancel_delayed_work(&ams.worker);
+		cancel_delayed_work(&ams_info.worker);
 		flush_scheduled_work();
 
 		/* Remove attributes */
-		device_remove_file(&ams.of_dev->dev, &dev_attr_current);
+		device_remove_file(&ams_info.of_dev->dev, &dev_attr_current);
 
 		/* Remove device */
-		of_device_unregister(ams.of_dev);
+		of_device_unregister(ams_info.of_dev);
 
 		/* Remove handler */
 		pmf_unregister_irq_client(&ams_shock_client);
 		pmf_unregister_irq_client(&ams_freefall_client);
 	}
 
-	mutex_unlock(&ams.lock);
+	mutex_unlock(&ams_info.lock);
 }
 
 MODULE_AUTHOR("Stelian Pop, Michael Hanselmann");
diff -Nrup --exclude-from linux-exclude-from linux-2.6.18-rc3.orig/drivers/hwmon/ams/ams.h linux-2.6.18-rc3/drivers/hwmon/ams/ams.h
--- linux-2.6.18-rc3.orig/drivers/hwmon/ams/ams.h	2006-08-05 00:47:46.000000000 +0200
+++ linux-2.6.18-rc3/drivers/hwmon/ams/ams.h	2006-08-05 00:49:56.000000000 +0200
@@ -60,7 +60,7 @@ struct ams {
 	int xcalib, ycalib, zcalib;
 };
 
-extern struct ams ams;
+extern struct ams ams_info;
 
 extern void ams_sensors(s8 *x, s8 *y, s8 *z);
 extern int ams_sensor_attach(void);
diff -Nrup --exclude-from linux-exclude-from linux-2.6.18-rc3.orig/drivers/hwmon/ams/ams-i2c.c linux-2.6.18-rc3/drivers/hwmon/ams/ams-i2c.c
--- linux-2.6.18-rc3.orig/drivers/hwmon/ams/ams-i2c.c	2006-08-05 00:47:46.000000000 +0200
+++ linux-2.6.18-rc3/drivers/hwmon/ams/ams-i2c.c	2006-08-05 00:51:33.000000000 +0200
@@ -74,26 +74,30 @@ static struct i2c_driver ams_i2c_driver 
 
 static s32 ams_i2c_read(u8 reg)
 {
-	return i2c_smbus_read_byte_data(&ams.i2c_client, reg);
+	return i2c_smbus_read_byte_data(&ams_info.i2c_client, reg);
 }
 
 static int ams_i2c_write(u8 reg, u8 value)
 {
-	return i2c_smbus_write_byte_data(&ams.i2c_client, reg, value);
+	return i2c_smbus_write_byte_data(&ams_info.i2c_client, reg, value);
 }
 
 static int ams_i2c_cmd(enum ams_i2c_cmd cmd)
 {
 	s32 result;
-	int i;
+	int remaining = HZ / 20;
 
 	ams_i2c_write(AMS_COMMAND, cmd);
-	for (i = 0; i < 10; i++) {
-		mdelay(5);
+	mdelay(5);
+
+	while (remaining) {
 		result = ams_i2c_read(AMS_COMMAND);
 		if (result = 0 || result & 0x80)
 			return 0;
+
+		remaining = schedule_timeout(remaining);
 	}
+
 	return -1;
 }
 
@@ -101,22 +105,28 @@ static void ams_i2c_set_irq(enum ams_irq
 {
 	if (reg & AMS_IRQ_FREEFALL) {
 		u8 val = ams_i2c_read(AMS_CTRLX);
-		if (enable)     val |= 0x80;
-		else            val &= ~0x80;
+		if (enable)
+			val |= 0x80;
+		else
+			val &= ~0x80;
 		ams_i2c_write(AMS_CTRLX, val);
 	}
 
 	if (reg & AMS_IRQ_SHOCK) {
 		u8 val = ams_i2c_read(AMS_CTRLY);
-		if (enable)     val |= 0x80;
-		else            val &= ~0x80;
+		if (enable)
+			val |= 0x80;
+		else
+			val &= ~0x80;
 		ams_i2c_write(AMS_CTRLY, val);
 	}
 
 	if (reg & AMS_IRQ_GLOBAL) {
 		u8 val = ams_i2c_read(AMS_CTRLZ);
-		if (enable)     val |= 0x80;
-		else            val &= ~0x80;
+		if (enable)
+			val |= 0x80;
+		else
+			val &= ~0x80;
 		ams_i2c_write(AMS_CTRLZ, val);
 	}
 }
@@ -149,20 +159,20 @@ static int ams_i2c_attach(struct i2c_ada
 	int result;
 
 	/* There can be only one */
-	if (unlikely(ams.has_device))
+	if (unlikely(ams_info.has_device))
 		return -ENODEV;
 
 	if (strncmp(adapter->name, "uni-n", 5))
 		return -ENODEV;
 
 	bus = simple_strtoul(adapter->name + 6, NULL, 10);
-	if (bus != ams.i2c_bus)
+	if (bus != ams_info.i2c_bus)
 		return -ENODEV;
 
-	ams.i2c_client.addr = ams.i2c_address;
-	ams.i2c_client.adapter = adapter;
-	ams.i2c_client.driver = &ams_i2c_driver;
-	strcpy(ams.i2c_client.name, "Apple Motion Sensor");
+	ams_info.i2c_client.addr = ams_info.i2c_address;
+	ams_info.i2c_client.adapter = adapter;
+	ams_info.i2c_client.driver = &ams_i2c_driver;
+	strcpy(ams_info.i2c_client.name, "Apple Motion Sensor");
 
 	if (ams_i2c_cmd(AMS_CMD_RESET)) {
 		printk(KERN_INFO "ams: Failed to reset the device\n");
@@ -217,7 +227,7 @@ static int ams_i2c_attach(struct i2c_ada
 	/* Clear interrupts */
 	ams_i2c_clear_irq(AMS_IRQ_ALL);
 
-	ams.has_device = 1;
+	ams_info.has_device = 1;
 
 	/* Enable interrupts */
 	ams_i2c_set_irq(AMS_IRQ_ALL, 1);
@@ -229,7 +239,7 @@ static int ams_i2c_attach(struct i2c_ada
 
 static int ams_i2c_detach(struct i2c_adapter *adapter)
 {
-	if (ams.has_device) {
+	if (ams_info.has_device) {
 		/* Disable interrupts */
 		ams_i2c_set_irq(AMS_IRQ_ALL, 0);
 
@@ -238,7 +248,7 @@ static int ams_i2c_detach(struct i2c_ada
 
 		printk(KERN_INFO "ams: Unloading\n");
 
-		ams.has_device = 0;
+		ams_info.has_device = 0;
 	}
 
 	return 0;
@@ -255,35 +265,35 @@ int __init ams_i2c_init(struct device_no
 	int result;
 	u32 *prop;
 
-	mutex_lock(&ams.lock);
+	mutex_lock(&ams_info.lock);
 
 	/* Set implementation stuff */
-	ams.of_node = np;
-	ams.exit = ams_i2c_exit;
-	ams.get_vendor = ams_i2c_get_vendor;
-	ams.get_xyz = ams_i2c_get_xyz;
-	ams.clear_irq = ams_i2c_clear_irq;
-	ams.bustype = BUS_I2C;
+	ams_info.of_node = np;
+	ams_info.exit = ams_i2c_exit;
+	ams_info.get_vendor = ams_i2c_get_vendor;
+	ams_info.get_xyz = ams_i2c_get_xyz;
+	ams_info.clear_irq = ams_i2c_clear_irq;
+	ams_info.bustype = BUS_I2C;
 
 	/* look for bus either using "reg" or by path */
-	prop = (u32*)get_property(ams.of_node, "reg", NULL);
+	prop = (u32*)get_property(ams_info.of_node, "reg", NULL);
 	if (!prop) {
 		result = -ENODEV;
 
 		goto exit;
 	}
 
-	tmp_bus = strstr(ams.of_node->full_name, "/i2c-bus@");
+	tmp_bus = strstr(ams_info.of_node->full_name, "/i2c-bus@");
 	if (tmp_bus)
-		ams.i2c_bus = *(tmp_bus + 9) - '0';
+		ams_info.i2c_bus = *(tmp_bus + 9) - '0';
 	else
-		ams.i2c_bus = ((*prop) >> 8) & 0x0f;
-	ams.i2c_address = ((*prop) & 0xff) >> 1;
+		ams_info.i2c_bus = ((*prop) >> 8) & 0x0f;
+	ams_info.i2c_address = ((*prop) & 0xff) >> 1;
 
 	result = i2c_add_driver(&ams_i2c_driver);
 
 exit:
-	mutex_unlock(&ams.lock);
+	mutex_unlock(&ams_info.lock);
 
 	return result;
 }
diff -Nrup --exclude-from linux-exclude-from linux-2.6.18-rc3.orig/drivers/hwmon/ams/ams-mouse.c linux-2.6.18-rc3/drivers/hwmon/ams/ams-mouse.c
--- linux-2.6.18-rc3.orig/drivers/hwmon/ams/ams-mouse.c	2006-08-05 00:47:46.000000000 +0200
+++ linux-2.6.18-rc3/drivers/hwmon/ams/ams-mouse.c	2006-08-05 00:49:56.000000000 +0200
@@ -28,17 +28,17 @@ static int ams_mouse_kthread(void *data)
 	s8 x, y, z;
 
 	while (!kthread_should_stop()) {
-		mutex_lock(&ams.lock);
+		mutex_lock(&ams_info.lock);
 
 		ams_sensors(&x, &y, &z);
 
-		input_report_abs(ams.idev, ABS_X, x - ams.xcalib);
-		input_report_abs(ams.idev, ABS_Y, y - ams.ycalib);
-		input_report_abs(ams.idev, ABS_Z, z - ams.zcalib);
+		input_report_abs(ams_info.idev, ABS_X, x - ams_info.xcalib);
+		input_report_abs(ams_info.idev, ABS_Y, y - ams_info.ycalib);
+		input_report_abs(ams_info.idev, ABS_Z, z - ams_info.zcalib);
 
-		input_sync(ams.idev);
+		input_sync(ams_info.idev);
 
-		mutex_unlock(&ams.lock);
+		mutex_unlock(&ams_info.lock);
 
 		msleep(25);
 	}
@@ -46,56 +46,57 @@ static int ams_mouse_kthread(void *data)
 	return 0;
 }
 
-/* Call with lock held! */
+/* Call with ams_info.lock held! */
 static void ams_mouse_enable(void)
 {
 	s8 x, y, z;
 
-	if (ams.idev)
+	if (ams_info.idev)
 		return;
 
 	ams_sensors(&x, &y, &z);
-	ams.xcalib = x;
-	ams.ycalib = y;
-	ams.zcalib = z;
+	ams_info.xcalib = x;
+	ams_info.ycalib = y;
+	ams_info.zcalib = z;
 
-	ams.idev = input_allocate_device();
-	if (!ams.idev)
+	ams_info.idev = input_allocate_device();
+	if (!ams_info.idev)
 		return;
 
-	ams.idev->name = "Apple Motion Sensor";
-	ams.idev->id.bustype = ams.bustype;
-	ams.idev->id.vendor = 0;
-	ams.idev->cdev.dev = &ams.of_dev->dev;
-
-	input_set_abs_params(ams.idev, ABS_X, -50, 50, 3, 0);
-	input_set_abs_params(ams.idev, ABS_Y, -50, 50, 3, 0);
-	input_set_abs_params(ams.idev, ABS_Z, -50, 50, 3, 0);
-
-	set_bit(EV_ABS, ams.idev->evbit);
-	set_bit(EV_KEY, ams.idev->evbit);
-
-	if (input_register_device(ams.idev)) {
-		input_free_device(ams.idev);
-		ams.idev = NULL;
+	ams_info.idev->name = "Apple Motion Sensor";
+	ams_info.idev->id.bustype = ams_info.bustype;
+	ams_info.idev->id.vendor = 0;
+	ams_info.idev->cdev.dev = &ams_info.of_dev->dev;
+
+	input_set_abs_params(ams_info.idev, ABS_X, -50, 50, 3, 0);
+	input_set_abs_params(ams_info.idev, ABS_Y, -50, 50, 3, 0);
+	input_set_abs_params(ams_info.idev, ABS_Z, -50, 50, 3, 0);
+
+	set_bit(EV_ABS, ams_info.idev->evbit);
+	set_bit(EV_KEY, ams_info.idev->evbit);
+
+	if (input_register_device(ams_info.idev)) {
+		input_free_device(ams_info.idev);
+		ams_info.idev = NULL;
 		return;
 	}
 
-	ams.kthread = kthread_run(ams_mouse_kthread, NULL, "kams");
-	if (IS_ERR(ams.kthread)) {
-		input_unregister_device(ams.idev);
-		ams.idev = NULL;
+	ams_info.kthread = kthread_run(ams_mouse_kthread, NULL, "kams");
+	if (IS_ERR(ams_info.kthread)) {
+		input_unregister_device(ams_info.idev);
+		input_free_device(ams_info.idev);
+		ams_info.idev = NULL;
 		return;
 	}
 }
 
-/* Call with lock held! */
+/* Call with ams_info.lock held! */
 static void ams_mouse_disable(void)
 {
-	if (ams.idev) {
-		kthread_stop(ams.kthread);
-		input_unregister_device(ams.idev);
-		ams.idev = NULL;
+	if (ams_info.idev) {
+		kthread_stop(ams_info.kthread);
+		input_unregister_device(ams_info.idev);
+		ams_info.idev = NULL;
 	}
 }
 
@@ -111,16 +112,14 @@ static ssize_t ams_mouse_store_mouse(str
 	if (sscanf(buf, "%d\n", &mouse) != 1)
 		return -EINVAL;
 
-	mouse = !!mouse;
-
-	mutex_lock(&ams.lock);
+	mutex_lock(&ams_info.lock);
 
 	if (mouse)
 		ams_mouse_enable();
 	else
 		ams_mouse_disable();
 
-	mutex_unlock(&ams.lock);
+	mutex_unlock(&ams_info.lock);
 
 	return count;
 }
@@ -128,18 +127,18 @@ static ssize_t ams_mouse_store_mouse(str
 static DEVICE_ATTR(mouse, S_IRUGO | S_IWUSR,
 	ams_mouse_show_mouse, ams_mouse_store_mouse);
 
-/* Call with lock held! */
+/* Call with ams_info.lock held! */
 void ams_mouse_init()
 {
-	device_create_file(&ams.of_dev->dev, &dev_attr_mouse);
+	device_create_file(&ams_info.of_dev->dev, &dev_attr_mouse);
 
 	if (mouse)
 		ams_mouse_enable();
 }
 
-/* Call with lock held! */
+/* Call with ams_info.lock held! */
 void ams_mouse_exit()
 {
 	ams_mouse_disable();
-	device_remove_file(&ams.of_dev->dev, &dev_attr_mouse);
+	device_remove_file(&ams_info.of_dev->dev, &dev_attr_mouse);
 }
diff -Nrup --exclude-from linux-exclude-from linux-2.6.18-rc3.orig/drivers/hwmon/ams/ams-pmu.c linux-2.6.18-rc3/drivers/hwmon/ams/ams-pmu.c
--- linux-2.6.18-rc3.orig/drivers/hwmon/ams/ams-pmu.c	2006-08-05 00:47:46.000000000 +0200
+++ linux-2.6.18-rc3/drivers/hwmon/ams/ams-pmu.c	2006-08-05 00:49:56.000000000 +0200
@@ -84,22 +84,28 @@ static void ams_pmu_set_irq(enum ams_irq
 {
 	if (reg & AMS_IRQ_FREEFALL) {
 		u8 val = ams_pmu_get_register(AMS_FF_ENABLE);
-		if (enable)	val |= 0x80;
-		else		val &= ~0x80;
+		if (enable)
+			val |= 0x80;
+		else
+			val &= ~0x80;
 		ams_pmu_set_register(AMS_FF_ENABLE, val);
 	}
 
 	if (reg & AMS_IRQ_SHOCK) {
 		u8 val = ams_pmu_get_register(AMS_SHOCK_ENABLE);
-		if (enable)	val |= 0x80;
-		else		val &= ~0x80;
+		if (enable)
+			val |= 0x80;
+		else
+			val &= ~0x80;
 		ams_pmu_set_register(AMS_SHOCK_ENABLE, val);
 	}
 
 	if (reg & AMS_IRQ_GLOBAL) {
 		u8 val = ams_pmu_get_register(AMS_CONTROL);
-		if (enable)	val |= 0x80;
-		else		val &= ~0x80;
+		if (enable)
+			val |= 0x80;
+		else
+			val &= ~0x80;
 		ams_pmu_set_register(AMS_CONTROL, val);
 	}
 }
@@ -133,29 +139,28 @@ static void ams_pmu_exit(void)
 	/* Clear interrupts */
 	ams_pmu_clear_irq(AMS_IRQ_ALL);
 
-	ams.has_device = 0;
+	ams_info.has_device = 0;
 
 	printk(KERN_INFO "ams: Unloading\n");
 }
 
-/* Call with lock held! */
 int __init ams_pmu_init(struct device_node *np)
 {
 	u32 *prop;
 	int result;
 
-	mutex_lock(&ams.lock);
+	mutex_lock(&ams_info.lock);
 
 	/* Set implementation stuff */
-	ams.of_node = np;
-	ams.exit = ams_pmu_exit;
-	ams.get_vendor = ams_pmu_get_vendor;
-	ams.get_xyz = ams_pmu_get_xyz;
-	ams.clear_irq = ams_pmu_clear_irq;
-	ams.bustype = BUS_HOST;
+	ams_info.of_node = np;
+	ams_info.exit = ams_pmu_exit;
+	ams_info.get_vendor = ams_pmu_get_vendor;
+	ams_info.get_xyz = ams_pmu_get_xyz;
+	ams_info.clear_irq = ams_pmu_clear_irq;
+	ams_info.bustype = BUS_HOST;
 
 	/* Get PMU command, should be 0x4e, but we can never know */
-	prop = (u32*)get_property(ams.of_node, "reg", NULL);
+	prop = (u32*)get_property(ams_info.of_node, "reg", NULL);
 	if (!prop) {
 		result = -ENODEV;
 		goto exit;
@@ -186,7 +191,7 @@ int __init ams_pmu_init(struct device_no
 	/* Clear interrupts */
 	ams_pmu_clear_irq(AMS_IRQ_ALL);
 
-	ams.has_device = 1;
+	ams_info.has_device = 1;
 
 	/* Enable interrupts */
 	ams_pmu_set_irq(AMS_IRQ_ALL, 1);
@@ -196,7 +201,7 @@ int __init ams_pmu_init(struct device_no
 	result = 0;
 
 exit:
-	mutex_unlock(&ams.lock);
+	mutex_unlock(&ams_info.lock);
 
 	return result;
 }

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Stelian Pop]

Le samedi 05 ao?t 2006 ? 01:03 +0200, Michael Hanselmann a ?crit :
> On Fri, Aug 04, 2006 at 03:19:52PM -0700, Andrew Morton wrote:
> > Could I please be sent a new copy of the whole patch, with the appropriate
> > acked-bys, signed-off-bys, etc?
> 
> Sure, here we go.
> 
> - Rename exported variable "ams" to "ams_info"
> - Fix leaked input device on error
> - Change I?C read code to use schedule_timeout instead of msleep() in
>   case the first read failed
> 
> Signed-off-by: Michael Hanselmann <linux-kernel at hansmi.ch>
> Signed-off-by: Aristeu Sergio Rozanski Filho <aris at cathedrallabs.org>
> Acked-by: Ren? Nussbaumer <linux-kernel at killerfox.forkbomb.ch>

You can also add my 
	Signed-off-by: Stelian Pop <stelian at popies.net>
to the whole ams patch.

Thanks,

Stelian.
-- 
Stelian Pop <stelian at popies.net>

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Johannes Berg]

On Wed, 2006-08-02 at 21:15 +0200, Michael Hanselmann wrote:

> +config SENSORS_AMS
> +       tristate "Apple Motion Sensor driver"
> +       depends on HWMON && PPC_PMAC && INPUT && EXPERIMENTAL

This causes the build failure, because now SENSORS_AMS can be 'y' while 

> +config SENSORS_AMS_I2C
> +       bool "I2C variant"
> +       depends on SENSORS_AMS && I2C

is satisfied by I2C=m. I think.

SENSORS_AMS should probably have
	depends on HWMON && PPC_PMAC && INPUT && (ADB_PMU || I2C) && EXPERIMENTAL

Is it possible to check further constraints? This still doesn't solve
all problems since I2C=m and ADB_PMU=y would satisfy SENSORS_AMS=y, but
that'd still lead to a build failure with SENSORS_AMS_I2C=y.

I'm not sure if that's possible to solve at all. But since ADB_PMU is
practically required even on those machines where we have the I2C
version of the sensor (they'll boot IIRC, but most things won't work
without it), how about just depending on that anyway? Hence making the
dependency
	depends on HWMON && PPC_PMAC && INPUT && ADB_PMU && I2C && EXPERIMENTAL

Also, "default y" should go away, and be put into the ppc32_defconfig.
We don't ever need this on ppc64. The dependency on ADB_PMU would of
course stop people from building it on ppc64 :)

johannes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 828 bytes
Desc: This is a digitally signed message part
Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20060806/29aaade5/attachment.bin 

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Michael Hanselmann]

On Sun, Aug 06, 2006 at 01:41:02PM +0200, Johannes Berg wrote:
> On Wed, 2006-08-02 at 21:15 +0200, Michael Hanselmann wrote:
> I'm not sure if that's possible to solve at all.

I think it is, see patch below. However, the solution is not that
pretty.

> 	depends on HWMON && PPC_PMAC && INPUT && ADB_PMU && I2C && EXPERIMENTAL

This would not allow users to build the AMS stuff without having I2C in
he kernel -- which is not really required.

What do you think about this patch?

---
diff -Nrup --exclude-from linux-exclude-from linux-2.6.18-rc3.orig/drivers/hwmon/Kconfig linux-2.6.18-rc3/drivers/hwmon/Kconfig
--- linux-2.6.18-rc3.orig/drivers/hwmon/Kconfig	2006-08-06 14:26:13.000000000 +0200
+++ linux-2.6.18-rc3/drivers/hwmon/Kconfig	2006-08-06 16:31:11.000000000 +0200
@@ -96,12 +96,14 @@ config SENSORS_ADM9240
 
 config SENSORS_AMS
 	tristate "Apple Motion Sensor driver"
-	depends on HWMON && PPC_PMAC && INPUT && EXPERIMENTAL
-	default y
+	depends on HWMON && PPC_PMAC && !PPC64 && INPUT && ((ADB_PMU && I2C = y) || (ADB_PMU && !I2C) || I2C) && EXPERIMENTAL
 	help
 	  Support for the motion sensor included in PowerBooks. Includes
 	  implementations for PMU and I2C.
 
+	  This driver can also be built as a module. If so, the module
+	  will be called ams.
+
 config SENSORS_AMS_PMU
 	bool "PMU variant"
 	depends on SENSORS_AMS && ADB_PMU
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20060806/51eddd2b/attachment.bin 

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Johannes Berg]

Michael Hanselmann wrote:
> +	depends on HWMON && PPC_PMAC && !PPC64 && INPUT && ((ADB_PMU && I2C = y) || (ADB_PMU && !I2C) || I2C) && EXPERIMENTAL
>   
Maybe we should just add "select I2C" instead. Then, if the user had I2C 
as a module and wants AMS he'll get I2C built-in. I think/hope :)

johannes

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Jean Delvare]

> Michael Hanselmann wrote:
> > +	depends on HWMON && PPC_PMAC && !PPC64 && INPUT && ((ADB_PMU && I2C = y) || (ADB_PMU && !I2C) || I2C) && EXPERIMENTAL
> >   
> Maybe we should just add "select I2C" instead. Then, if the user had I2C 
> as a module and wants AMS he'll get I2C built-in. I think/hope :)

No, please. I2C is something which you should depend on, not something
you select.

-- 
Jean Delvare

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Johannes Berg]

Jean Delvare wrote:
> No, please. I2C is something which you should depend on, not something
> you select.
>   

Why? It's infrastructure basically. Anyway. Is it possible to have 
something like
  depends I2C=AMS || I2C=y

If we stuck that into the i2c AMS version that'd help. Though be really 
ugly for the user since she might get the AMS I2C version only after 
selecting AMS as a module then. Pretty icky, all that.

johannes

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Jean Delvare]

> Jean Delvare wrote:
> > No, please. I2C is something which you should depend on, not something
> > you select.
> >   
> 
> Why? It's infrastructure basically.

Every i2c driver to date depends on I2C, and my experience is that
mixing select and depends tends to confuse Kconfig, users, or both.

Now if you believe that this is an error and I2C should always be
selected rather than depended upon, I'm waiting for your patch :)

-- 
Jean Delvare

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Johannes Berg]

Jean Delvare wrote:
> Now if you believe that this is an error and I2C should always be
> selected rather than depended upon, I'm waiting for your patch :)
>   
Heh, no, I don't really care. A Power/iBook without I2C isn't much good 
anyway.

johannes

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Benjamin Herrenschmidt]

On Fri, 2006-08-04 at 23:49 +0200, Michael Hanselmann wrote:
> On Fri, Aug 04, 2006 at 05:39:32PM -0300, Aristeu Sergio Rozanski Filho wrote:
> > > This guarantees a minimum wait time of 5msec after writing the value.
> > > Looking at the specs[1], this should be done. Your code doesn't do that:
> 
> > I doubt that if it's possible to retrying after a fail the hardware will
> > bother if we try to do the first read in less than 5ms... but here it
> > goes. two versions, feel free to choose.
> 
> The specs are written based on the Mac OS X driver, and I think Apple
> knows their hardware.

Don't be so sure :)

> After discussing both variants, Ren? Nussbaumer and myself prefer the
> first one.
> 
> Acked-by: Michael Hanselmann <linux-kernel at hansmi.ch>
> Acked-by: Ren? Nussbaumer <linux-kernel at killerfox.forkbomb.ch>
> 
> Greets,
> Michael

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Benjamin Herrenschmidt]

On Mon, 2006-08-07 at 11:36 +0200, Johannes Berg wrote:
> Jean Delvare wrote:
> > Now if you believe that this is an error and I2C should always be
> > selected rather than depended upon, I'm waiting for your patch :)
> >   
> Heh, no, I don't really care. A Power/iBook without I2C isn't much good 
> anyway.

Old ones are...

Ben.

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Michael Hanselmann]

On Mon, Aug 07, 2006 at 10:06:38AM +0200, Johannes Berg wrote:
> Why? It's infrastructure basically. Anyway. Is it possible to have 
> something like
>  depends I2C=AMS || I2C=y

> If we stuck that into the i2c AMS version that'd help. Though be really 
> ugly for the user since she might get the AMS I2C version only after 
> selecting AMS as a module then. Pretty icky, all that.

I'm not really a fan of it. Is there anything wrong with my proposed
patch?

Thanks,
Michael

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Johannes Berg]

Michael Hanselmann wrote:
> I'm not really a fan of it. Is there anything wrong with my proposed
> patch?
>   
This is what you get if I write things and then don't put the powerbook 
online for days.

I think your depends statement is hard to parse, but should be fine :) 
Try actually configuring things in the presence of I2C=m though, please.

johannes

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Johannes Berg]

Benjamin Herrenschmidt wrote:
> Old ones are...
>   
They also don't have an acceleration sensor :)

johannes

[lm-sensors] [PATCH] Apple Motion Sensor driver

[Michael Hanselmann]

On Sun, Aug 06, 2006 at 01:41:02PM +0200, Johannes Berg wrote:
> On Wed, 2006-08-02 at 21:15 +0200, Michael Hanselmann wrote:

> > +config SENSORS_AMS
> > +       tristate "Apple Motion Sensor driver"
> > +       depends on HWMON && PPC_PMAC && INPUT && EXPERIMENTAL

> This causes the build failure, because now SENSORS_AMS can be 'y' while 

Andrew, can you please apply the patch below? It fixes wrong Kconfig
dependencies.

Signed-off-by: Michael Hanselmann <linux-kernel at hansmi.ch>

---
diff -Nrup --exclude-from linux-exclude-from linux-2.6.18-rc3.orig/drivers/hwmon/Kconfig linux-2.6.18-rc3/drivers/hwmon/Kconfig
--- linux-2.6.18-rc3.orig/drivers/hwmon/Kconfig	2006-08-06 14:26:13.000000000 +0200
+++ linux-2.6.18-rc3/drivers/hwmon/Kconfig	2006-08-06 16:31:11.000000000 +0200
@@ -96,12 +96,14 @@ config SENSORS_ADM9240
 
 config SENSORS_AMS
 	tristate "Apple Motion Sensor driver"
-	depends on HWMON && PPC_PMAC && INPUT && EXPERIMENTAL
-	default y
+	depends on HWMON && PPC_PMAC && !PPC64 && INPUT && ((ADB_PMU && I2C = y) || (ADB_PMU && !I2C) || I2C) && EXPERIMENTAL
 	help
 	  Support for the motion sensor included in PowerBooks. Includes
 	  implementations for PMU and I2C.
 
+	  This driver can also be built as a module. If so, the module
+	  will be called ams.
+
 config SENSORS_AMS_PMU
 	bool "PMU variant"
 	depends on SENSORS_AMS && ADB_PMU