Skip to contentSkip to conversation

HP accelerometer: split chip handling from HP ACPI interface

PATCH In review Pavel Machek 4 participants 17 messages Nov 21, 2008 – Jan 9, 2009

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

This patch splits handling between chip-specific lis3lv02d and HP/ACPI specific hp_accel.c.

drivers/hwmon/Makefile +1 -1
index 58fc5be..9f891ff 100644
@@ -49,7 +49,7 @@ obj-$(CONFIG_SENSORS_IBMAEM)	+= ibmaem.o
 obj-$(CONFIG_SENSORS_IBMPEX)	+= ibmpex.o
 obj-$(CONFIG_SENSORS_IT87)	+= it87.o
 obj-$(CONFIG_SENSORS_K8TEMP)	+= k8temp.o
-obj-$(CONFIG_SENSORS_LIS3LV02D) += lis3lv02d.o
+obj-$(CONFIG_SENSORS_LIS3LV02D) += lis3lv02d.o hp_accel.o
 obj-$(CONFIG_SENSORS_LM63)	+= lm63.o
 obj-$(CONFIG_SENSORS_LM70)	+= lm70.o
 obj-$(CONFIG_SENSORS_LM75)	+= lm75.o
drivers/hwmon/hp_accel.c new file +301 -0

301 lines not on this page · raw message

drivers/hwmon/lis3lv02d.c +11 -235

333 lines not on this page · raw message

drivers/hwmon/lis3lv02d.h +33 -0
index 330cfc6..8614280 100644
@@ -147,3 +147,36 @@ enum lis3lv02d_dd_src {
 	DD_SRC_IA	= 0x40,
 };
 
+acpi_status lis3lv02d_acpi_init(acpi_handle handle);
+acpi_status lis3lv02d_acpi_read(acpi_handle handle, int reg, u8 *ret);
+acpi_status lis3lv02d_acpi_write(acpi_handle handle, int reg, u8 val);
+
+struct axis_conversion {
+	s8	x;
+	s8	y;
+	s8	z;
+};
+
+struct acpi_lis3lv02d {
+	struct acpi_device	*device;   /* The ACPI device */
+	struct input_dev	*idev;     /* input device */
+	struct task_struct	*kthread;  /* kthread for input */
+	struct mutex            lock;
+	struct platform_device	*pdev;     /* platform device */
+	atomic_t		count;     /* interrupt count after last read */
+	int			xcalib;    /* calibrated null value for x */
+	int			ycalib;    /* calibrated null value for y */
+	int			zcalib;    /* calibrated null value for z */
+	unsigned char		is_on;     /* whether the device is on or off */
+	unsigned char		usage;     /* usage counter */
+	struct axis_conversion	ac;        /* hw -> logical axis */
+};
+
+int lis3lv02d_init_device(struct acpi_lis3lv02d *dev);
+int lis3lv02d_joystick_enable(void);
+void lis3lv02d_joystick_disable(void);
+void lis3lv02d_poweroff(acpi_handle handle);
+void lis3lv02d_poweron(acpi_handle handle);
+int lis3lv02d_remove_fs(void);
+
+extern struct acpi_lis3lv02d adev;
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#1 lore raw
Quoted 2 lines · On Fri, 21 Nov 2008 22:21:21 +0100 Pavel Machek <[email protected]> wrote:

This patch splits handling between chip-specific lis3lv02d and HP/ACPI specific hp_accel.c.

umm, so? What reason does anyone have for applying it?

#2 lore raw

Andrew Morton schreef:

Quoted 7 lines
Quoted 2 lines · On Fri, 21 Nov 2008 22:21:21 +0100 Pavel Machek <[email protected]> wrote:

This patch splits handling between chip-specific lis3lv02d and HP/ACPI specific hp_accel.c.

umm, so? What reason does anyone have for applying it?

Here is the log I have for the (old version) of pavel's patch:

The sensor can be accessed via various buses. In particular, SPI, I²C and, on HP laptops, via a specific ACPI API (the only one currently supported). Separate this latest platform from the core of the sensor driver to allow support for the other bus type. The second, and more direct goal is actually to be able to merge this part with the hp-disk-leds driver, which has the same ACPI PNP number.

I'll try to follow with a patch doing the merge of the hp-disk-leds driver, so that everything makes sense...

Eric

#3 lore raw

Éric Piel schreef:

Quoted 17 lines

Andrew Morton schreef:

Quoted 6 lines
Quoted 2 lines · On Fri, 21 Nov 2008 22:21:21 +0100 Pavel Machek <[email protected]> wrote:

This patch splits handling between chip-specific lis3lv02d and HP/ACPI specific hp_accel.c.

umm, so? What reason does anyone have for applying it?

Here is the log I have for the (old version) of pavel's patch:

The sensor can be accessed via various buses. In particular, SPI, I²C and, on HP laptops, via a specific ACPI API (the only one currently supported). Separate this latest platform from the core of the sensor driver to allow support for the other bus type. The second, and more direct goal is actually to be able to merge this part with the hp-disk-leds driver, which has the same ACPI PNP number.

Quoted 2 lines

I'll try to follow with a patch doing the merge of the hp-disk-leds driver, so that everything makes sense...

Actually, I mistook, Pavel also included some led stuff in this patch. That doesn't look right, especially as the led driver is not removed! Following are two patches (nearly all work done by Pavel) which should make this enhancement much clearer:
- Separate the HP ACPI support from the core
- Merge the led support and with the rest of the HP ACPI support

The final semantic of these two patches should be equivalent to the one provided by Pavel (with some Kconfig bonus).

See you,
Eric

#4 lore raw

PATCH 1/2 LIS3LV02D: separate the core from HP ACPI API

The sensor can be accessed via various buses. In particular, SPI, I²C and, on HP laptops, via a specific ACPI API (the only one currently supported). Separate this latest platform from the core of the sensor driver to allow support for the other bus type. The second, and more direct goal is actually to be able to merge this part with the hp-disk-leds driver, which has the same ACPI PNP number.

---
 drivers/hwmon/Makefile    |    2 +-
 drivers/hwmon/hp_accel.c  |  266 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/hwmon/lis3lv02d.c |  269 +++++----------------------------------------
 drivers/hwmon/lis3lv02d.h |   35 ++++++
 4 files changed, 329 insertions(+), 243 deletions(-)
 create mode 100644 drivers/hwmon/hp_accel.c
drivers/hwmon/Makefile +1 -1
index 58fc5be..9f891ff 100644
@@ -49,7 +49,7 @@ obj-$(CONFIG_SENSORS_IBMAEM)	+= ibmaem.o
 obj-$(CONFIG_SENSORS_IBMPEX)	+= ibmpex.o
 obj-$(CONFIG_SENSORS_IT87)	+= it87.o
 obj-$(CONFIG_SENSORS_K8TEMP)	+= k8temp.o
-obj-$(CONFIG_SENSORS_LIS3LV02D) += lis3lv02d.o
+obj-$(CONFIG_SENSORS_LIS3LV02D) += lis3lv02d.o hp_accel.o
 obj-$(CONFIG_SENSORS_LM63)	+= lm63.o
 obj-$(CONFIG_SENSORS_LM70)	+= lm70.o
 obj-$(CONFIG_SENSORS_LM75)	+= lm75.o
drivers/hwmon/hp_accel.c new file +266 -0

266 lines not on this page · raw message

drivers/hwmon/lis3lv02d.c +27 -242

372 lines not on this page · raw message

drivers/hwmon/lis3lv02d.h +35 -0
index 330cfc6..41e5c74 100644
@@ -147,3 +147,38 @@ enum lis3lv02d_dd_src {
 	DD_SRC_IA	= 0x40,
 };
 
+acpi_status lis3lv02d_acpi_init(acpi_handle handle);
+
+struct axis_conversion {
+	s8	x;
+	s8	y;
+	s8	z;
+};
+
+struct acpi_lis3lv02d {
+	struct acpi_device	*device;   /* The ACPI device */
+	acpi_status (* init) (acpi_handle handle);
+	acpi_status (* write) (acpi_handle handle, int reg, u8 val);
+	acpi_status (* read) (acpi_handle handle, int reg, u8 *ret);
+
+	struct input_dev	*idev;     /* input device */
+	struct task_struct	*kthread;  /* kthread for input */
+	struct mutex            lock;
+	struct platform_device	*pdev;     /* platform device */
+	atomic_t		count;     /* interrupt count after last read */
+	int			xcalib;    /* calibrated null value for x */
+	int			ycalib;    /* calibrated null value for y */
+	int			zcalib;    /* calibrated null value for z */
+	unsigned char		is_on;     /* whether the device is on or off */
+	unsigned char		usage;     /* usage counter */
+	struct axis_conversion	ac;        /* hw -> logical axis */
+};
+
+int lis3lv02d_init_device(struct acpi_lis3lv02d *dev);
+int lis3lv02d_joystick_enable(void);
+void lis3lv02d_joystick_disable(void);
+void lis3lv02d_poweroff(acpi_handle handle);
+void lis3lv02d_poweron(acpi_handle handle);
+int lis3lv02d_remove_fs(void);
+
+extern struct acpi_lis3lv02d adev;
Signature · 1 line
1.6.0.4
#5 lore raw

Re: [PATCH 1/2] LIS3LV02D: separate the core from HP ACPI API

Quoted 10 lines · ?ric Piel

The sensor can be accessed via various buses. In particular, SPI, I?C and, on HP laptops, via a specific ACPI API (the only one currently supported). Separate this latest platform from the core of the sensor driver to allow support for the other bus type. The second, and more direct goal is actually to be able to merge this part with the hp-disk-leds driver, which has the same ACPI PNP number.

Oops, I sent wrong patch. This should fix the 'compile as module' problems...

drivers/hwmon/hp_accel.c renamed from l/READ-ONLY/linux/drivers/hwmon/hp_accel.c +3 -0
diff -ur ../l/READ-ONLY/linux/drivers/hwmon/hp_accel.c linux-mm/drivers/hwmon/hp_accel.c
@@ -189,6 +189,9 @@
 		return -EINVAL;
 
 	adev.device = device;
+	adev.init = lis3lv02d_acpi_init;
+	adev.read = lis3lv02d_acpi_read;
+	adev.write = lis3lv02d_acpi_write;
 	strcpy(acpi_device_name(device), DRIVER_NAME);
 	strcpy(acpi_device_class(device), ACPI_MDPS_CLASS);
 	device->driver_data = &adev;
drivers/hwmon/lis3lv02d.c renamed from l/READ-ONLY/linux/drivers/hwmon/lis3lv02d.c +18 -8
@@ -34,6 +34,7 @@
 #include <linux/wait.h>
 #include <linux/poll.h>
 #include <linux/freezer.h>
+#include <linux/version.h>
 #include <linux/uaccess.h>
 #include <acpi/acpi_drivers.h>
 #include <asm/atomic.h>
@@ -67,8 +68,8 @@
 {
 	u8 lo, hi;
 
-	lis3lv02d_acpi_read(handle, reg, &lo);
-	lis3lv02d_acpi_read(handle, reg + 1, &hi);
+	adev.read(handle, reg, &lo);
+	adev.read(handle, reg + 1, &hi);
 	/* In "12 bit right justified" mode, bit 6, bit 7, bit 8 = bit 5 */
 	return (s16)((hi << 8) | lo);
 }
@@ -114,7 +115,7 @@
 {
 	adev.is_on = 0;
 	/* disable X,Y,Z axis and power down */
-	lis3lv02d_acpi_write(handle, CTRL_REG1, 0x00);
+	adev.write(handle, CTRL_REG1, 0x00);
 }
 
 void lis3lv02d_poweron(acpi_handle handle)
@@ -122,16 +123,16 @@
 	u8 val;
 
 	adev.is_on = 1;
-	lis3lv02d_acpi_init(handle);
-	lis3lv02d_acpi_write(handle, FF_WU_CFG, 0);
+	adev.init(handle);
+	adev.write(handle, FF_WU_CFG, 0);
 	/*
 	 * BDU: LSB and MSB values are not updated until both have been read.
 	 *      So the value read will always be correct.
 	 * IEN: Interrupt for free-fall and DD, not for data-ready.
 	 */
-	lis3lv02d_acpi_read(handle, CTRL_REG2, &val);
+	adev.read(handle, CTRL_REG2, &val);
 	val |= CTRL2_BDU | CTRL2_IEN;
-	lis3lv02d_acpi_write(handle, CTRL_REG2, val);
+	adev.write(handle, CTRL_REG2, val);
 }
 
 
@@ -313,7 +314,7 @@
 	int val;
 
 	lis3lv02d_increase_use(&adev);
-	lis3lv02d_acpi_read(adev.device->handle, CTRL_REG1, &ctrl);
+	adev.read(adev.device->handle, CTRL_REG1, &ctrl);
 	lis3lv02d_decrease_use(&adev);
 	val = (ctrl & (CTRL1_DF0 | CTRL1_DF1)) >> 4;
 	return sprintf(buf, "%d\n", lis3lv02dl_df_val[val]);
@@ -355,3 +356,12 @@
 MODULE_DESCRIPTION("ST LIS3LV02Dx three-axis digital accelerometer driver");
 MODULE_AUTHOR("Yan Burman and Eric Piel");
 MODULE_LICENSE("GPL");
+
+EXPORT_SYMBOL_GPL(lis3lv02d_init_device);
+EXPORT_SYMBOL_GPL(lis3lv02d_joystick_enable);
+EXPORT_SYMBOL_GPL(lis3lv02d_joystick_disable);
+EXPORT_SYMBOL_GPL(lis3lv02d_poweroff);
+EXPORT_SYMBOL_GPL(lis3lv02d_poweron);
+EXPORT_SYMBOL_GPL(lis3lv02d_remove_fs);
+
+EXPORT_SYMBOL_GPL(adev);

Only in ../l/READ-ONLY/linux/drivers/hwmon/: lis3lv02d.c.orig

drivers/hwmon/lis3lv02d.h renamed from l/READ-ONLY/linux/drivers/hwmon/lis3lv02d.h +4 -2
diff -ur ../l/READ-ONLY/linux/drivers/hwmon/lis3lv02d.h linux-mm/drivers/hwmon/lis3lv02d.h
@@ -148,8 +148,6 @@
 };
 
 acpi_status lis3lv02d_acpi_init(acpi_handle handle);
-acpi_status lis3lv02d_acpi_read(acpi_handle handle, int reg, u8 *ret);
-acpi_status lis3lv02d_acpi_write(acpi_handle handle, int reg, u8 val);
 
 struct axis_conversion {
 	s8	x;
@@ -159,6 +157,10 @@
 
 struct acpi_lis3lv02d {
 	struct acpi_device	*device;   /* The ACPI device */
+	acpi_status (* init) (acpi_handle handle);
+	acpi_status (* write) (acpi_handle handle, int reg, u8 val);
+	acpi_status (* read) (acpi_handle handle, int reg, u8 *ret);
+
 	struct input_dev	*idev;     /* input device */
 	struct task_struct	*kthread;  /* kthread for input */
 	struct mutex            lock;
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#6 lore raw

Re: [PATCH 1/2] LIS3LV02D: separate the core from HP ACPI API

Pavel Machek schreef:

Quoted 8 lines · ?ric Piel
Quoted 7 lines

The sensor can be accessed via various buses. In particular, SPI, I?C and, on HP laptops, via a specific ACPI API (the only one currently supported). Separate this latest platform from the core of the sensor driver to allow support for the other bus type. The second, and more direct goal is actually to be able to merge this part with the hp-disk-leds driver, which has the same ACPI PNP number.

Quoted 6 lines
Quoted 2 lines

Oops, I sent wrong patch. This should fix the 'compile as module' problems...

Well, the patch you are answering to already contains this fix :-) To be precise, excepted this line:

drivers/hwmon/lis3lv02d.c line 37

Quoted 7 lines
drivers/hwmon/lis3lv02d.c renamed from l/READ-ONLY/linux/drivers/hwmon/lis3lv02d.c +1 -0
@@ -34,6 +34,7 @@
 #include <linux/wait.h>
 #include <linux/poll.h>
 #include <linux/freezer.h>
+#include <linux/version.h>

Is it needed?

Now that I read again this patch, I could see some "not so clean" parts. Here is a new version of it, which should follow better the Linux coding style. Patch 2/2 is still valid and applies fine over this updated version. Andrew this little series should be now safe to take into your tree :-)

Eric
=
LIS3LV02D: separate the core from HP ACPI API

The sensor can be accessed via various buses. In particular, SPI, I²C and, on HP laptops, via a specific ACPI API (the only one currently supported). Separate this latest platform from the core of the sensor driver to allow support for the other bus type. The second, and more direct goal is actually to be able to merge this part with the hp-disk-leds driver, which has the same ACPI PNP number.

---
 drivers/hwmon/Makefile    |    2 +-
 drivers/hwmon/hp_accel.c  |  265 +++++++++++++++++++++++++++++++++++++++++++
 drivers/hwmon/lis3lv02d.c |  273 ++++-----------------------------------------
 drivers/hwmon/lis3lv02d.h |   35 ++++++-
 4 files changed, 324 insertions(+), 251 deletions(-)
 create mode 100644 drivers/hwmon/hp_accel.c
drivers/hwmon/Makefile +1 -1
index 58fc5be..9f891ff 100644
@@ -49,7 +49,7 @@ obj-$(CONFIG_SENSORS_IBMAEM)	+= ibmaem.o
 obj-$(CONFIG_SENSORS_IBMPEX)	+= ibmpex.o
 obj-$(CONFIG_SENSORS_IT87)	+= it87.o
 obj-$(CONFIG_SENSORS_K8TEMP)	+= k8temp.o
-obj-$(CONFIG_SENSORS_LIS3LV02D) += lis3lv02d.o
+obj-$(CONFIG_SENSORS_LIS3LV02D) += lis3lv02d.o hp_accel.o
 obj-$(CONFIG_SENSORS_LM63)	+= lm63.o
 obj-$(CONFIG_SENSORS_LM70)	+= lm70.o
 obj-$(CONFIG_SENSORS_LM75)	+= lm75.o
drivers/hwmon/hp_accel.c new file +265 -0

265 lines not on this page · raw message

drivers/hwmon/lis3lv02d.c +24 -249

375 lines not on this page · raw message

drivers/hwmon/lis3lv02d.h +34 -1
index 330cfc6..223f1c0 100644
@@ -23,7 +23,7 @@
  * The actual chip is STMicroelectronics LIS3LV02DL or LIS3LV02DQ that seems to
  * be connected via SPI. There exists also several similar chips (such as LIS302DL or
  * LIS3L02DQ) but not in the HP laptops and they have slightly different registers.
- * They can also be connected via I²C.
+ * They can also be connected via I²C.
  */
 
 #define LIS3LV02DL_ID	0x3A /* Also the LIS3LV02DQ */
@@ -147,3 +147,36 @@ enum lis3lv02d_dd_src {
 	DD_SRC_IA	= 0x40,
 };
 
+struct axis_conversion {
+	s8	x;
+	s8	y;
+	s8	z;
+};
+
+struct acpi_lis3lv02d {
+	struct acpi_device	*device;   /* The ACPI device */
+	acpi_status (*init) (acpi_handle handle);
+	acpi_status (*write) (acpi_handle handle, int reg, u8 val);
+	acpi_status (*read) (acpi_handle handle, int reg, u8 *ret);
+
+	struct input_dev	*idev;     /* input device */
+	struct task_struct	*kthread;  /* kthread for input */
+	struct mutex            lock;
+	struct platform_device	*pdev;     /* platform device */
+	atomic_t		count;     /* interrupt count after last read */
+	int			xcalib;    /* calibrated null value for x */
+	int			ycalib;    /* calibrated null value for y */
+	int			zcalib;    /* calibrated null value for z */
+	unsigned char		is_on;     /* whether the device is on or off */
+	unsigned char		usage;     /* usage counter */
+	struct axis_conversion	ac;        /* hw -> logical axis */
+};
+
+int lis3lv02d_init_device(struct acpi_lis3lv02d *dev);
+int lis3lv02d_joystick_enable(void);
+void lis3lv02d_joystick_disable(void);
+void lis3lv02d_poweroff(acpi_handle handle);
+void lis3lv02d_poweron(acpi_handle handle);
+int lis3lv02d_remove_fs(void);
+
+extern struct acpi_lis3lv02d adev;
Signature · 1 line
1.6.0.4
#7 lore raw

PATCH 2/2 LIS3LV02D: Merge with leds-hp-disk

In reply to Éric Piel

Move the second part of the HP laptop disk protection fonctionality (a red led) to the same driver. From a purely Linux developer's point of view, the led and the accelerometer have nothing related. However, they correspond to the same ACPI functionality, and so will always be used together, moreover as they share the same ACPI PNP alias, there is no other simple to allow to have same loaded at the same time if they are not in the same module. Also make it requires the led class to compile and update the Kconfig text.

---
 drivers/hwmon/Kconfig       |   14 +++-
 drivers/hwmon/hp_accel.c    |   43 +++++++++++-
 drivers/leds/Kconfig        |    7 --
 drivers/leds/Makefile       |    1 -
 drivers/leds/leds-hp-disk.c |  155 -------------------------------------------
 5 files changed, 52 insertions(+), 168 deletions(-)
 delete mode 100644 drivers/leds/leds-hp-disk.c
drivers/hwmon/Kconfig +11 -3
index c709e82..ab9b163 100644
@@ -838,6 +838,8 @@ config SENSORS_HDAPS
 config SENSORS_LIS3LV02D
 	tristate "STMicroeletronics LIS3LV02Dx three-axis digital accelerometer"
 	depends on ACPI && INPUT
+	select NEW_LEDS
+	select LEDS_CLASS
 	default n
 	help
 	  This driver provides support for the LIS3LV02Dx accelerometer. In
@@ -849,10 +851,16 @@ config SENSORS_LIS3LV02D
 	  /sys/devices/platform/lis3lv02d.
 
 	  This driver also provides an absolute input class device, allowing
-	  the laptop to act as a pinball machine-esque joystick.
+	  the laptop to act as a pinball machine-esque joystick. On HP laptops,
+	  if the led infrastructure is activated, support for a led indicating
+	  disk protection will be provided as hp:red:hddprotection.
 
-	  This driver can also be built as a module.  If so, the module
-	  will be called lis3lv02d.
+	  This driver can also be built as modules.  If so, the core module
+	  will be called lis3lv02d and a specific module for HP laptops will be
+	  called hp_accel.
+
+	  Say Y here if you have an applicable laptop and want to experience
+	  the awesome power of lis3lv02d.
 
 config SENSORS_APPLESMC
 	tristate "Apple SMC (Motion sensor, light sensor, keyboard backlight)"
drivers/hwmon/hp_accel.c +41 -2

89 lines not on this page · raw message

drivers/leds/Kconfig +0 -7
index e7fb7d2..62f0d10 100644
@@ -113,13 +113,6 @@ config LEDS_GPIO
 	  outputs. To be useful the particular board must have LEDs
 	  and they must be connected to the GPIO lines.
 
-config LEDS_HP_DISK
-	tristate "LED Support for disk protection LED on HP notebooks"
-	depends on LEDS_CLASS && ACPI
-	help
-	  This option enable support for disk protection LED, found on
-	  newer HP notebooks.
-
 config LEDS_CLEVO_MAIL
 	tristate "Mail LED on Clevo notebook (EXPERIMENTAL)"
 	depends on LEDS_CLASS && X86 && SERIO_I8042 && DMI && EXPERIMENTAL
drivers/leds/Makefile +0 -1
index e1967a2..64e21e4 100644
@@ -22,7 +22,6 @@ obj-$(CONFIG_LEDS_HP6XX)		+= leds-hp6xx.o
 obj-$(CONFIG_LEDS_FSG)			+= leds-fsg.o
 obj-$(CONFIG_LEDS_PCA955X)		+= leds-pca955x.o
 obj-$(CONFIG_LEDS_DA903X)		+= leds-da903x.o
-obj-$(CONFIG_LEDS_HP_DISK)		+= leds-hp-disk.o
 
 # LED Triggers
 obj-$(CONFIG_LEDS_TRIGGER_TIMER)	+= ledtrig-timer.o
drivers/leds/leds-hp-disk.c deleted +0 -155

155 lines not on this page · raw message

Signature · 1 line
1.6.0.4
#8 lore raw
In reply to Éric Piel
Quoted 18 lines · ?ric Piel

Éric Piel schreef:

Quoted 17 lines

Andrew Morton schreef:

Quoted 6 lines
Quoted 2 lines · On Fri, 21 Nov 2008 22:21:21 +0100 Pavel Machek <[email protected]> wrote:

This patch splits handling between chip-specific lis3lv02d and HP/ACPI specific hp_accel.c.

umm, so? What reason does anyone have for applying it?

Here is the log I have for the (old version) of pavel's patch:

The sensor can be accessed via various buses. In particular, SPI, I?C and, on HP laptops, via a specific ACPI API (the only one currently supported). Separate this latest platform from the core of the sensor driver to allow support for the other bus type. The second, and more direct goal is actually to be able to merge this part with the hp-disk-leds driver, which has the same ACPI PNP number.

Quoted 6 lines
Quoted 2 lines

I'll try to follow with a patch doing the merge of the hp-disk-leds driver, so that everything makes sense...

Actually, I mistook, Pavel also included some led stuff in this patch. That doesn't look right, especially as the led driver is not removed!

I was simply planning to remove the LED stuff in the followup patch. I guess I should have mentioned that in the changelog...

Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#9 lore raw
In reply to Pavel Machek
Quoted 2 lines · On Fri, 21 Nov 2008 22:21:21 +0100 Pavel Machek <[email protected]> wrote:

This patch splits handling between chip-specific lis3lv02d and HP/ACPI specific hp_accel.c.

i386 allmodconfig:

ERROR: "lis3lv02d_acpi_write" [drivers/hwmon/lis3lv02d.ko] undefined!
ERROR: "lis3lv02d_acpi_init" [drivers/hwmon/lis3lv02d.ko] undefined!
ERROR: "lis3lv02d_acpi_read" [drivers/hwmon/lis3lv02d.ko] undefined!
ERROR: "lis3lv02d_remove_fs" [drivers/hwmon/hp_accel.ko] undefined!
ERROR: "adev" [drivers/hwmon/hp_accel.ko] undefined!
ERROR: "lis3lv02d_joystick_disable" [drivers/hwmon/hp_accel.ko] undefined!
ERROR: "lis3lv02d_init_device" [drivers/hwmon/hp_accel.ko] undefined!
ERROR: "lis3lv02d_poweroff" [drivers/hwmon/hp_accel.ko] undefined!
ERROR: "lis3lv02d_poweron" [drivers/hwmon/hp_accel.ko] undefined!

#10 lore raw

HP accelerometer: free fall detection working, testers wanted

In reply to Pavel Machek

Hi!

I got free fall detection to work -- apparently BIOS does all the important stuff itself.

Now... I'd like some testers. (Don't comment on code... yet. Yes, it has some issues).

hp3dg script should do all the important things.

Documentation/hwmon/hp3dg new file +26 -0
new file mode 100755
index 0000000..6b7114b
@@ -0,0 +1,26 @@
+#!/bin/bash
+#
+# HP 3d driveguard disk protection example
+#mount /dev/sdb2 /mmc
+#cd /mmc/local/linux/drivers/hwmon/
+#rmmod hp_accel; rmmod lis3lv02d; insmod lis3lv02d.ko ; insmod hp_accel.ko
+#cd /usr/src
+
+unload() {
+	echo $[($1*1000)+100] > /sys/block/sda/device/unload_heads
+	echo 1 > /sys/class/leds/hp\:red\:hddprotect/brightness
+	(
+		sleep $1
+		if cat /sys/block/sda/device/unload_heads | grep ^0$; then
+			echo 0 > /sys/class/leds/hp\:red\:hddprotect/brightness
+		fi
+	) &
+}
+
+while true; do
+	./readaccel foo
+	unload 2
+	if cat /proc/acpi/button/lid/LID/state | grep open && cat /proc/acpi/ac_adapter/AC0/state | grep off-line; then
+		unload 20
+	fi
+done
Documentation/hwmon/readaccel.c new file +35 -0
new file mode 100755
index 0000000..cb9a71c
@@ -0,0 +1,35 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <string.h>
+#include <stdint.h>
+
+int main(int argc, char* argv[])
+{
+       int fd, ret;
+       unsigned char count;
+
+       fd = open("/dev/freefall", O_RDONLY);
+       if (fd < 0) {
+               perror("open");
+               return EXIT_FAILURE;
+       }
+
+       for (;;) {
+               ret = read(fd, &count, sizeof(count));
+               if (ret != sizeof(count)) {
+                       perror("read");
+                       break;
+               }
+
+               printf("Got %d free-fall interrupts\n", (int) count);
+	       if (argc > 1)
+		        return EXIT_SUCCESS;
+       }
+
+       close(fd);
+       return EXIT_SUCCESS;
+}
drivers/hwmon/Kconfig +11 -3
index c709e82..ab9b163 100644
@@ -838,6 +838,8 @@ config SENSORS_HDAPS
 config SENSORS_LIS3LV02D
 	tristate "STMicroeletronics LIS3LV02Dx three-axis digital accelerometer"
 	depends on ACPI && INPUT
+	select NEW_LEDS
+	select LEDS_CLASS
 	default n
 	help
 	  This driver provides support for the LIS3LV02Dx accelerometer. In
@@ -849,10 +851,16 @@ config SENSORS_LIS3LV02D
 	  /sys/devices/platform/lis3lv02d.
 
 	  This driver also provides an absolute input class device, allowing
-	  the laptop to act as a pinball machine-esque joystick.
+	  the laptop to act as a pinball machine-esque joystick. On HP laptops,
+	  if the led infrastructure is activated, support for a led indicating
+	  disk protection will be provided as hp:red:hddprotection.
 
-	  This driver can also be built as a module.  If so, the module
-	  will be called lis3lv02d.
+	  This driver can also be built as modules.  If so, the core module
+	  will be called lis3lv02d and a specific module for HP laptops will be
+	  called hp_accel.
+
+	  Say Y here if you have an applicable laptop and want to experience
+	  the awesome power of lis3lv02d.
 
 config SENSORS_APPLESMC
 	tristate "Apple SMC (Motion sensor, light sensor, keyboard backlight)"
drivers/hwmon/Makefile +1 -1
index 58fc5be..9f891ff 100644
@@ -49,7 +49,7 @@ obj-$(CONFIG_SENSORS_IBMAEM)	+= ibmaem.o
 obj-$(CONFIG_SENSORS_IBMPEX)	+= ibmpex.o
 obj-$(CONFIG_SENSORS_IT87)	+= it87.o
 obj-$(CONFIG_SENSORS_K8TEMP)	+= k8temp.o
-obj-$(CONFIG_SENSORS_LIS3LV02D) += lis3lv02d.o
+obj-$(CONFIG_SENSORS_LIS3LV02D) += lis3lv02d.o hp_accel.o
 obj-$(CONFIG_SENSORS_LM63)	+= lm63.o
 obj-$(CONFIG_SENSORS_LM70)	+= lm70.o
 obj-$(CONFIG_SENSORS_LM75)	+= lm75.o
drivers/hwmon/hp_accel.c new file +304 -0

304 lines not on this page · raw message

drivers/hwmon/lis3lv02d.c +282 -245

659 lines not on this page · raw message

drivers/hwmon/lis3lv02d.h +41 -1

51 lines not on this page · raw message

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#11 lore raw

Re: HP accelerometer: free fall detection working, testers wanted

Pavel Machek schreef:

Quoted 8 lines

Hi!

I got free fall detection to work -- apparently BIOS does all the important stuff itself.

Now... I'd like some testers. (Don't comment on code... yet. Yes, it has some issues).

Hi Pavel,
I'll try to test this weekend.

Out of curiosity, what did you change compared to the original version of the driver? Did you just disable the set up code?

Also, how do you try free fall? Did you try also with the lid opened?

Quoted 1 line

hp3dg script should do all the important things.

:

Documentation/hwmon/hp3dg

Quoted 2 lines
Documentation/hwmon/hp3dg new file +0 -0

:

Documentation/hwmon/hp3dg line 26

Quoted 7 lines
Documentation/hwmon/hp3dg new file +7 -0
+while true; do
+	./readaccel foo
+	unload 2
+	if cat /proc/acpi/button/lid/LID/state | grep open && cat /proc/acpi/ac_adapter/AC0/state | grep off-line; then
+		unload 20
+	fi
+done

Why a special case if the lid is opened and the laptop unplugged? Is it a special behaviour of the hardware, or simply some kind of basic user behaviour guess?

See you,
Eric

#12 lore raw

Re: HP accelerometer: free fall detection working, testers wanted

Hi!

Quoted 11 lines
Quoted 6 lines

I got free fall detection to work -- apparently BIOS does all the important stuff itself.

Now... I'd like some testers. (Don't comment on code... yet. Yes, it has some issues).

Hi Pavel,
I'll try to test this weekend.

Out of curiosity, what did you change compared to the original version of the driver? Did you just disable the set up code?

Disable all the writes, yes.

Quoted 2 lines

Also, how do you try free fall? Did you try also with the lid opened?

With the lid opened, too, yes. I just took the machine, then tried to let go and catch it meter below that. (Of course I did not really let go, but released the grip and followed...)

Quoted 5 lines
Quoted 1 line

hp3dg script should do all the important things.

:

Documentation/hwmon/hp3dg

Quoted 2 lines
Documentation/hwmon/hp3dg new file +0 -0

:

Quoted 10 lines

Documentation/hwmon/hp3dg line 26

Quoted 7 lines
Documentation/hwmon/hp3dg new file +7 -0
+while true; do
+	./readaccel foo
+	unload 2
+	if cat /proc/acpi/button/lid/LID/state | grep open && cat /proc/acpi/ac_adapter/AC0/state | grep off-line; then
+		unload 20
+	fi
+done

Why a special case if the lid is opened and the laptop unplugged? Is it a special behaviour of the hardware, or simply some kind of basic user behaviour guess?

Behaviour guess, HP asked for those values. Idea is that if the user is walking with machine, park as much as possible because machine is in danger :-).
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#13 lore raw

Re: HP accelerometer: free fall detection working, testers wanted

Pavel Machek schreef:

Quoted 8 lines

Hi!

Quoted 5 lines
Quoted 5 lines

I got free fall detection to work -- apparently BIOS does all the important stuff itself.

Now... I'd like some testers. (Don't comment on code... yet. Yes, it has some issues).

Quoted 6 lines
Quoted 2 lines

Also, how do you try free fall? Did you try also with the lid opened?

With the lid opened, too, yes. I just took the machine, then tried to let go and catch it meter below that. (Of course I did not really let go, but released the grip and followed...)

Hi,
I've tested it. Mostly, it works. If I let it fall, it detects it. If I repeat, it detects it again. If shake it, it doesn't detect any freefall (good). However, for this to work on my laptop (2510p), after the device is turned on, the AC must be unplugged and lid closed before it starts working. It seems that that's only this specific state that triggers the configuration of the sensor by the bios. Once setup, I can plug back and open again the lid, the sensor keeps detecting the freefalls (until it's turned off again).

So I think it's still necessary to explicitly set up the sensor. It's just a matter of setting up the registers as the bios does. We should be able to manage (just by dumping them once).

BTW, there were a couple of problems with the scripts. You have change "/dev/misc" to "/dev/freefall" (which I also like more) but forgot to update the driver itself.

For hp3dg, it couldn't find the led (not the same truncation of the name of the led). Probably you could replace the name of the led by:
/sys/class/leds/*hddprotect*/

The led never turns off, because the sleep is slightly shorter than the unload command.

Moreover, on my system I have neither /proc/acpi/button/lid/LID/state nor /proc/acpi/ac_adapter/AC0/state ! I guess they could be replaced by a clever use of /dev/input/eventX (contening Lid) and /sys/class/power_supply/*/online

Quoted 16 lines
Quoted 5 lines
Quoted 1 line

hp3dg script should do all the important things.

:

Documentation/hwmon/hp3dg

Quoted 2 lines
Documentation/hwmon/hp3dg new file +0 -0

:

Quoted 10 lines

Documentation/hwmon/hp3dg line 26

Quoted 7 lines
Documentation/hwmon/hp3dg new file +7 -0
+while true; do
+	./readaccel foo
+	unload 2
+	if cat /proc/acpi/button/lid/LID/state | grep open && cat /proc/acpi/ac_adapter/AC0/state | grep off-line; then
+		unload 20
+	fi
+done

Why a special case if the lid is opened and the laptop unplugged? Is it a special behaviour of the hardware, or simply some kind of basic user behaviour guess?

Quoted 3 lines

Behaviour guess, HP asked for those values. Idea is that if the user is walking with machine, park as much as possible because machine is in danger :-).

Yes, I guess it's not a bad idea. Anyway, this is just an example daemon, and I'm sure people will come up with even more rules like this in a full feature daemon...

Eric

#14 lore raw

Re: HP accelerometer: free fall detection working, testers wanted

Quoted 38 lines · On Sunday 30 November 2008 23:16:29 Éric Piel wrote:

Pavel Machek schreef:

Quoted 8 lines

Hi!

Quoted 6 lines
Quoted 5 lines

I got free fall detection to work -- apparently BIOS does all the important stuff itself.

Now... I'd like some testers. (Don't comment on code... yet. Yes, it has some issues).

Quoted 6 lines
Quoted 2 lines

Also, how do you try free fall? Did you try also with the lid opened?

With the lid opened, too, yes. I just took the machine, then tried to let go and catch it meter below that. (Of course I did not really let go, but released the grip and followed...)

Hi,
I've tested it. Mostly, it works. If I let it fall, it detects it. If I repeat, it detects it again. If shake it, it doesn't detect any freefall (good). However, for this to work on my laptop (2510p), after the device is turned on, the AC must be unplugged and lid closed before it starts working. It seems that that's only this specific state that triggers the configuration of the sensor by the bios. Once setup, I can plug back and open again the lid, the sensor keeps detecting the freefalls (until it's turned off again).

So I think it's still necessary to explicitly set up the sensor. It's just a matter of setting up the registers as the bios does. We should be able to manage (just by dumping them once).

BTW, there were a couple of problems with the scripts. You have change "/dev/misc" to "/dev/freefall" (which I also like more) but forgot to update the driver itself.

For hp3dg, it couldn't find the led (not the same truncation of the name of the led). Probably you could replace the name of the led by:
/sys/class/leds/*hddprotect*/

Quoted 7 lines

The led never turns off, because the sleep is slightly shorter than the unload command.

Moreover, on my system I have neither /proc/acpi/button/lid/LID/state nor /proc/acpi/ac_adapter/AC0/state ! I guess they could be replaced by a clever use of /dev/input/eventX (contening Lid) and /sys/class/power_supply/*/online

These should still exist (bug?), as long as you do compile with ACPI_PROCFS (and load the ac, button modules).
But there should also exist a sysfs replacement:
http://userweb.kernel.org/~lenb/doc/LPC2008-API-web/
gives an overview of what is converted and where it should pop up now.

Thomas

#15 lore raw

Re: HP accelerometer: free fall detection working, testers wanted

Thomas Renninger schreef:

Quoted 43 lines · On Sunday 30 November 2008 23:16:29 Éric Piel wrote:
Quoted 35 lines

Pavel Machek schreef:

Quoted 5 lines

Hi!

Quoted 3 lines
Quoted 3 lines

I got free fall detection to work -- apparently BIOS does all the important stuff itself.

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

Now... I'd like some testers. (Don't comment on code... yet. Yes, it has some issues).

Also, how do you try free fall? Did you try also with the lid opened?

With the lid opened, too, yes. I just took the machine, then tried to let go and catch it meter below that. (Of course I did not really let go, but released the grip and followed...)

Hi,
I've tested it. Mostly, it works. If I let it fall, it detects it. If I repeat, it detects it again. If shake it, it doesn't detect any freefall (good). However, for this to work on my laptop (2510p), after the device is turned on, the AC must be unplugged and lid closed before it starts working. It seems that that's only this specific state that triggers the configuration of the sensor by the bios. Once setup, I can plug back and open again the lid, the sensor keeps detecting the freefalls (until it's turned off again).

So I think it's still necessary to explicitly set up the sensor. It's just a matter of setting up the registers as the bios does. We should be able to manage (just by dumping them once).

BTW, there were a couple of problems with the scripts. You have change "/dev/misc" to "/dev/freefall" (which I also like more) but forgot to update the driver itself.

For hp3dg, it couldn't find the led (not the same truncation of the name of the led). Probably you could replace the name of the led by:
/sys/class/leds/*hddprotect*/

Quoted 7 lines

The led never turns off, because the sleep is slightly shorter than the unload command.

Moreover, on my system I have neither /proc/acpi/button/lid/LID/state nor /proc/acpi/ac_adapter/AC0/state ! I guess they could be replaced by a clever use of /dev/input/eventX (contening Lid) and /sys/class/power_supply/*/online

Quoted 5 lines

These should still exist (bug?), as long as you do compile with ACPI_PROCFS (and load the ac, button modules).
But there should also exist a sysfs replacement:
http://userweb.kernel.org/~lenb/doc/LPC2008-API-web/
gives an overview of what is converted and where it should pop up now.

Yes, I explicitly compiled my kernel without ACPI_PROCFS. This was just to remind Pavel that his brand new script relies on deprecated interfaces ;-)

Eric

#16 lore raw

Re: HP accelerometer: free fall detection working, testers wanted

In reply to Éric Piel
Quoted 26 lines · Éric Piel

Pavel Machek schreef:

Quoted 8 lines

Hi!

Quoted 5 lines
Quoted 5 lines

I got free fall detection to work -- apparently BIOS does all the important stuff itself.

Now... I'd like some testers. (Don't comment on code... yet. Yes, it has some issues).

Quoted 6 lines
Quoted 2 lines

Also, how do you try free fall? Did you try also with the lid opened?

With the lid opened, too, yes. I just took the machine, then tried to let go and catch it meter below that. (Of course I did not really let go, but released the grip and followed...)

Hi,
I've tested it. Mostly, it works. If I let it fall, it detects it. If I repeat, it detects it again. If shake it, it doesn't detect any freefall (good). However, for this to work on my laptop (2510p), after the device is turned on, the AC must be unplugged and lid closed before it starts working. It seems that that's only this specific state that triggers the configuration of the sensor by the bios. Once setup, I can plug back and open again the lid, the sensor keeps detecting the freefalls (until it's turned off again).

Quoted 3 lines

So I think it's still necessary to explicitly set up the sensor. It's just a matter of setting up the registers as the bios does. We should be able to manage (just by dumping them once).

Well, according to HP we should not be setting registers ourselves. Maybe CLRI method needs to be called or something?

Quoted 7 lines

BTW, there were a couple of problems with the scripts. You have change "/dev/misc" to "/dev/freefall" (which I also like more) but forgot to update the driver itself.

For hp3dg, it couldn't find the led (not the same truncation of the name of the led). Probably you could replace the name of the led by:
/sys/class/leds/*hddprotect*/

Yes, the scripts were very very quick hacks.
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html