2009-11-10 12:42:25

by Samu Onkalo

[permalink] [raw]
Subject: [PATCH v2 00/10] LIS3: Feature updates and corrections

This patch set has earlier been sent to lm-sensors list (thus patch v2).
No changes after that except to patch 0006 to make it applicable to
2.6.32-rc6 (no functional changes).

There has been discussion on lm-sensors list about the patch
"lis3: Scale output values to mg (0009)". It causes changes to
existing user space applications.
See link below about this discussion:
http://lists.lm-sensors.org/pipermail/lm-sensors/2009-November/027101.html

Patches are applicable to top of 2.6.32-RC6.

This patch set makes several changes to the lis3 accelerometer chip
family driver. There are also two patches from Eric Piel.

I have possibility to test these changes only with 8 bit version (lis302d).
Since part of the changes are specific to 12 bit device, I would appreciate
if someone could try these on 12 bit device.

0001:
Sync event is sent after each measurement. This helps user space
applications to detect which 3 reported values belongs to
one atomic measurement.

0002:
polled input device was not totally released in module unload.
This corrects memory leak.

0003:
This patch is originally from Eric Piel.
Updates documentation and chip ID handling

0004:
This patch is originally from Eric Piel.
Correction to sampling rate handling for 8 bit devices

0005:
Chip power on sequence was not waiting for chip to be ready.
This caused incorrect values right after the power on.

0006:
Chip specifications contains selftest for the chip. This patch implements that.

0007:
Calibration functionality is removed from the driver. Chip is allready
calibrated by the manufacturer. SW calibration doesn't improve the situation.

0008:
Added possibility to set chip sampling rate. Position entry in sysfs
allows reading at sampling rate.

0009:
Scale output values to mg (1/1000 of earth gravity). (see comments above)

0010:
Updates documentation


Samu Onkalo (8):
LIS3LV02D: Send sync event
LIS3LV02D: Correct memory leak in module unload
LIS3LV02D: Proper power on sequence
LIS3LV02D: Selftest support
LIS3LV02D: Remove calibaration functionality
lis3: Sysfs entry for setting chip measurement rate
lis3: Scale output values to mg
LIS3: Update documentation to match latest changes

Éric Piel (2):
lis3: Update documentation and comments
lis3: fix show rate for 8 bits chips

Documentation/hwmon/lis3lv02d | 40 ++++---
drivers/hwmon/Kconfig | 22 ++--
drivers/hwmon/lis3lv02d.c | 234 +++++++++++++++++++++++++++++++----------
drivers/hwmon/lis3lv02d.h | 48 ++++++---
include/linux/lis3lv02d.h | 2 +
5 files changed, 249 insertions(+), 97 deletions(-)


2009-11-10 12:44:16

by Samu Onkalo

[permalink] [raw]
Subject: [PATCH v2 01/10] LIS3LV02D: Send sync event

Send input_sync after each measurement round. This helps userspace to
detect which reported values belongs to the same measurement.

Signed-off-by: Samu Onkalo <[email protected]>
---
drivers/hwmon/lis3lv02d.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
index cf5afb9..7f43a3b 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -276,6 +276,7 @@ static void lis3lv02d_joystick_poll(struct input_polled_dev *pidev)
input_report_abs(pidev->input, ABS_X, x - lis3_dev.xcalib);
input_report_abs(pidev->input, ABS_Y, y - lis3_dev.ycalib);
input_report_abs(pidev->input, ABS_Z, z - lis3_dev.zcalib);
+ input_sync(pidev->input);
}


--
1.5.6.3

2009-11-10 12:42:23

by Samu Onkalo

[permalink] [raw]
Subject: [PATCH v2 02/10] LIS3LV02D: Correct memory leak in module unload

polled input device itself was not free'd.

Signed-off-by: Samu Onkalo <[email protected]>
---
drivers/hwmon/lis3lv02d.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
index 7f43a3b..dbd0b05 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -333,6 +333,7 @@ void lis3lv02d_joystick_disable(void)
if (lis3_dev.irq)
misc_deregister(&lis3lv02d_misc_device);
input_unregister_polled_device(lis3_dev.idev);
+ input_free_polled_device(lis3_dev.idev);
lis3_dev.idev = NULL;
}
EXPORT_SYMBOL_GPL(lis3lv02d_joystick_disable);
--
1.5.6.3

2009-11-10 12:42:28

by Samu Onkalo

[permalink] [raw]
Subject: [PATCH v2 03/10] lis3: Update documentation and comments

From: Éric Piel <[email protected]>

Most of the documentation and comments were written when the driver was
only supporting one type of chip, only via ACPI/HP. Update the info to
the much clearer understanding that we have now.

Signed-off-by: Éric Piel <[email protected]>
---
Documentation/hwmon/lis3lv02d | 24 ++++++++++++++----------
drivers/hwmon/Kconfig | 22 ++++++++++++----------
drivers/hwmon/lis3lv02d.c | 20 ++++++++++----------
drivers/hwmon/lis3lv02d.h | 28 ++++++++++++++++------------
4 files changed, 52 insertions(+), 42 deletions(-)

diff --git a/Documentation/hwmon/lis3lv02d b/Documentation/hwmon/lis3lv02d
index effe949..21f0902 100644
--- a/Documentation/hwmon/lis3lv02d
+++ b/Documentation/hwmon/lis3lv02d
@@ -3,7 +3,8 @@ Kernel driver lis3lv02d

Supported chips:

- * STMicroelectronics LIS3LV02DL and LIS3LV02DQ
+ * STMicroelectronics LIS3LV02DL, LIS3LV02DQ (12 bits precision)
+ * STMicroelectronics LIS302DL, LIS3L02DQ, LIS331DL (8 bits)

Authors:
Yan Burman <[email protected]>
@@ -13,13 +14,12 @@ Authors:
Description
-----------

-This driver provides support for the accelerometer found in various HP
-laptops sporting the feature officially called "HP Mobile Data
-Protection System 3D" or "HP 3D DriveGuard". It detects automatically
-laptops with this sensor. Known models (for now the HP 2133, nc6420,
-nc2510, nc8510, nc84x0, nw9440 and nx9420) will have their axis
-automatically oriented on standard way (eg: you can directly play
-neverball). The accelerometer data is readable via
+This driver provides support for the accelerometer found in various HP laptops
+sporting the feature officially called "HP Mobile Data Protection System 3D" or
+"HP 3D DriveGuard". It detects automatically laptops with this sensor. Known
+models (full list can be found in drivers/hwmon/hp_accel.c) will have their
+axis automatically oriented on standard way (eg: you can directly play
+neverball). The accelerometer data is readable via
/sys/devices/platform/lis3lv02d.

Sysfs attributes under /sys/devices/platform/lis3lv02d/:
@@ -33,12 +33,16 @@ rate - reports the sampling rate of the accelerometer device in HZ
This driver also provides an absolute input class device, allowing
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 /sys/class/leds/hp::hddprotect.
+
Another feature of the driver is misc device called "freefall" that
acts similar to /dev/rtc and reacts on free-fall interrupts received
from the device. It supports blocking operations, poll/select and
fasync operation modes. You must read 1 bytes from the device. The
result is number of free-fall interrupts since the last successful
-read (or 255 if number of interrupts would not fit).
+read (or 255 if number of interrupts would not fit). See the hpfall.c
+file for an example on using the device.


Axes orientation
@@ -55,7 +59,7 @@ the accelerometer are converted into a "standard" organisation of the axes
* If the laptop is put upside-down, Z becomes negative

If your laptop model is not recognized (cf "dmesg"), you can send an
-email to the authors to add it to the database. When reporting a new
+email to the maintainer to add it to the database. When reporting a new
laptop, please include the output of "dmidecode" plus the value of
/sys/devices/platform/lis3lv02d/position in these four cases.

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 700e93a..a9afb1b 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1025,25 +1025,27 @@ config SENSORS_ATK0110
will be called asus_atk0110.

config SENSORS_LIS3LV02D
- tristate "STMicroeletronics LIS3LV02Dx three-axis digital accelerometer"
+ tristate "STMicroeletronics LIS3* three-axis digital accelerometer"
depends on INPUT
select INPUT_POLLDEV
select NEW_LEDS
select LEDS_CLASS
default n
help
- This driver provides support for the LIS3LV02Dx accelerometer. In
- particular, it can be found in a number of HP laptops, which have the
- "Mobile Data Protection System 3D" or "3D DriveGuard" feature. On such
- systems the driver should load automatically (via ACPI). The
- accelerometer might also be found in other systems, connected via SPI
- or I2C. The accelerometer data is readable via
- /sys/devices/platform/lis3lv02d.
+ This driver provides support for the LIS3* accelerometers, such as the
+ LIS3LV02DL or the LIS331DL. In particular, it can be found in a number
+ of HP laptops, which have the "Mobile Data Protection System 3D" or
+ "3D DriveGuard" feature. On such systems the driver should load
+ automatically (via ACPI alias). The accelerometer might also be found
+ in other systems, connected via SPI or I2C. The accelerometer data is
+ readable via /sys/devices/platform/lis3lv02d.

This driver also provides an absolute input class device, allowing
- the laptop to act as a pinball machine-esque joystick. On HP laptops,
+ a laptop to act as a pinball machine-esque joystick. It provides also
+ a misc device which can be used to detect free-fall. On HP laptops,
if the led infrastructure is activated, support for a led indicating
- disk protection will be provided as hp:red:hddprotection.
+ disk protection will be provided as hp::hddprotect. For more
+ information on the feature, refer to Documentation/hwmon/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
diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
index dbd0b05..1c8f108 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -43,7 +43,7 @@
#define MDPS_POLL_INTERVAL 50
/*
* The sensor can also generate interrupts (DRDY) but it's pretty pointless
- * because their are generated even if the data do not change. So it's better
+ * because they are generated even if the data do not change. So it's better
* to keep the interrupt for the free-fall event. The values are updated at
* 40Hz (at the lowest frequency), but as it can be pretty time consuming on
* some low processor, we poll the sensor only at 20Hz... enough for the
@@ -65,7 +65,7 @@ static s16 lis3lv02d_read_8(struct lis3lv02d *lis3, int reg)
return lo;
}

-static s16 lis3lv02d_read_16(struct lis3lv02d *lis3, int reg)
+static s16 lis3lv02d_read_12(struct lis3lv02d *lis3, int reg)
{
u8 lo, hi;

@@ -411,20 +411,20 @@ EXPORT_SYMBOL_GPL(lis3lv02d_remove_fs);

/*
* Initialise the accelerometer and the various subsystems.
- * Should be rather independant of the bus system.
+ * Should be rather independent of the bus system.
*/
int lis3lv02d_init_device(struct lis3lv02d *dev)
{
dev->whoami = lis3lv02d_read_8(dev, WHO_AM_I);

switch (dev->whoami) {
- case LIS_DOUBLE_ID:
- printk(KERN_INFO DRIVER_NAME ": 2-byte sensor found\n");
- dev->read_data = lis3lv02d_read_16;
+ case WAI_12B:
+ printk(KERN_INFO DRIVER_NAME ": 12 bits sensor found\n");
+ dev->read_data = lis3lv02d_read_12;
dev->mdps_max_val = 2048;
break;
- case LIS_SINGLE_ID:
- printk(KERN_INFO DRIVER_NAME ": 1-byte sensor found\n");
+ case WAI_8B:
+ printk(KERN_INFO DRIVER_NAME ": 8 bits sensor found\n");
dev->read_data = lis3lv02d_read_8;
dev->mdps_max_val = 128;
break;
@@ -445,7 +445,7 @@ int lis3lv02d_init_device(struct lis3lv02d *dev)
if (dev->pdata) {
struct lis3lv02d_platform_data *p = dev->pdata;

- if (p->click_flags && (dev->whoami == LIS_SINGLE_ID)) {
+ if (p->click_flags && (dev->whoami == WAI_8B)) {
dev->write(dev, CLICK_CFG, p->click_flags);
dev->write(dev, CLICK_TIMELIMIT, p->click_time_limit);
dev->write(dev, CLICK_LATENCY, p->click_latency);
@@ -456,7 +456,7 @@ int lis3lv02d_init_device(struct lis3lv02d *dev)
(p->click_thresh_y << 4));
}

- if (p->wakeup_flags && (dev->whoami == LIS_SINGLE_ID)) {
+ if (p->wakeup_flags && (dev->whoami == WAI_8B)) {
dev->write(dev, FF_WU_CFG_1, p->wakeup_flags);
dev->write(dev, FF_WU_THS_1, p->wakeup_thresh & 0x7f);
/* default to 2.5ms for now */
diff --git a/drivers/hwmon/lis3lv02d.h b/drivers/hwmon/lis3lv02d.h
index 3e1ff46..2431c51 100644
--- a/drivers/hwmon/lis3lv02d.h
+++ b/drivers/hwmon/lis3lv02d.h
@@ -2,7 +2,7 @@
* lis3lv02d.h - ST LIS3LV02DL accelerometer driver
*
* Copyright (C) 2007-2008 Yan Burman
- * Copyright (C) 2008 Eric Piel
+ * Copyright (C) 2008-2009 Eric Piel
*
* 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
@@ -22,20 +22,18 @@
#include <linux/input-polldev.h>

/*
- * 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) and they have slightly different registers, but we can provide a
- * common interface for all of them.
- * They can also be connected via I²C.
+ * This driver tries to support the "digital" accelerometer chips from
+ * STMicroelectronics such as LIS3LV02DL, LIS302DL, LIS3L02DQ, LIS331DL,
+ * LIS35DE, or LIS202DL. They are very similar in terms of programming, with
+ * almost the same registers. In addition to differing on physical properties,
+ * they differ on the number of axes (2/3), precision (8/12 bits), and special
+ * features (freefall detection, click...). Unfortunately, not all the
+ * differences can be probed via a register.
+ * They can be connected either via I²C or SPI.
*/

#include <linux/lis3lv02d.h>

-/* 2-byte registers */
-#define LIS_DOUBLE_ID 0x3A /* LIS3LV02D[LQ] */
-/* 1-byte registers */
-#define LIS_SINGLE_ID 0x3B /* LIS[32]02DL and others */
-
enum lis3_reg {
WHO_AM_I = 0x0F,
OFFSET_X = 0x16,
@@ -94,6 +92,12 @@ enum lis3lv02d_reg {
DD_THSE_H = 0x3F,
};

+enum lis3_who_am_i {
+ WAI_12B = 0x3A, /* 12 bits: LIS3LV02D[LQ]... */
+ WAI_8B = 0x3B, /* 8 bits: LIS[23]02D[LQ]... */
+ WAI_6B = 0x52, /* 6 bits: LIS331DLF - not supported */
+};
+
enum lis3lv02d_ctrl1 {
CTRL1_Xen = 0x01,
CTRL1_Yen = 0x02,
@@ -194,7 +198,7 @@ struct lis3lv02d {
int (*write) (struct lis3lv02d *lis3, int reg, u8 val);
int (*read) (struct lis3lv02d *lis3, int reg, u8 *ret);

- u8 whoami; /* 3Ah: 2-byte registries, 3Bh: 1-byte registries */
+ u8 whoami; /* indicates measurement precision */
s16 (*read_data) (struct lis3lv02d *lis3, int reg);
int mdps_max_val;

--
1.5.6.3

2009-11-10 12:43:57

by Samu Onkalo

[permalink] [raw]
Subject: [PATCH v2 04/10] lis3: fix show rate for 8 bits chips

From: Éric Piel <[email protected]>

lis3: fix with 8 bits sensors

Signed-off-by: Éric Piel <[email protected]>
---
drivers/hwmon/lis3lv02d.c | 24 ++++++++++++++++--------
drivers/hwmon/lis3lv02d.h | 1 +
2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
index 1c8f108..b12ee35 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -127,12 +127,14 @@ void lis3lv02d_poweron(struct lis3lv02d *lis3)

/*
* Common configuration
- * BDU: LSB and MSB values are not updated until both have been read.
- * So the value read will always be correct.
+ * BDU: (12 bits sensors only) LSB and MSB values are not updated until
+ * both have been read. So the value read will always be correct.
*/
- lis3->read(lis3, CTRL_REG2, &reg);
- reg |= CTRL2_BDU;
- lis3->write(lis3, CTRL_REG2, reg);
+ if (lis3->whoami == WAI_12B) {
+ lis3->read(lis3, CTRL_REG2, &reg);
+ reg |= CTRL2_BDU;
+ lis3->write(lis3, CTRL_REG2, reg);
+ }
}
EXPORT_SYMBOL_GPL(lis3lv02d_poweron);

@@ -363,7 +365,8 @@ static ssize_t lis3lv02d_calibrate_store(struct device *dev,
}

/* conversion btw sampling rate and the register values */
-static int lis3lv02dl_df_val[4] = {40, 160, 640, 2560};
+static int lis3_12_rates[4] = {40, 160, 640, 2560};
+static int lis3_8_rates[2] = {100, 400};
static ssize_t lis3lv02d_rate_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -371,8 +374,13 @@ static ssize_t lis3lv02d_rate_show(struct device *dev,
int val;

lis3_dev.read(&lis3_dev, CTRL_REG1, &ctrl);
- val = (ctrl & (CTRL1_DF0 | CTRL1_DF1)) >> 4;
- return sprintf(buf, "%d\n", lis3lv02dl_df_val[val]);
+
+ if (lis3_dev.whoami == WAI_12B)
+ val = lis3_12_rates[(ctrl & (CTRL1_DF0 | CTRL1_DF1)) >> 4];
+ else
+ val = lis3_8_rates[(ctrl & CTRL1_DR) >> 7];
+
+ return sprintf(buf, "%d\n", val);
}

static DEVICE_ATTR(position, S_IRUGO, lis3lv02d_position_show, NULL);
diff --git a/drivers/hwmon/lis3lv02d.h b/drivers/hwmon/lis3lv02d.h
index 2431c51..c6ae507 100644
--- a/drivers/hwmon/lis3lv02d.h
+++ b/drivers/hwmon/lis3lv02d.h
@@ -107,6 +107,7 @@ enum lis3lv02d_ctrl1 {
CTRL1_DF1 = 0x20,
CTRL1_PD0 = 0x40,
CTRL1_PD1 = 0x80,
+ CTRL1_DR = 0x80, /* Data rate on 8 bits */
};
enum lis3lv02d_ctrl2 {
CTRL2_DAS = 0x01,
--
1.5.6.3

2009-11-10 12:42:40

by Samu Onkalo

[permalink] [raw]
Subject: [PATCH v2 05/10] LIS3LV02D: Proper power on sequence

Lis3 accelerometer sensors have quite long power on delay (up to 125
ms). This patch adds necessary delay to power on sequence for currently
supported lis3 chips.

Signed-off-by: Samu Onkalo <[email protected]>
---
drivers/hwmon/lis3lv02d.c | 41 +++++++++++++++++++++++++++--------------
drivers/hwmon/lis3lv02d.h | 1 +
2 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
index b12ee35..39b9ac8 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -50,6 +50,9 @@
* joystick.
*/

+#define LIS3_PWRON_DELAY_WAI_12B (5000)
+#define LIS3_PWRON_DELAY_WAI_8B (3000)
+
struct lis3lv02d lis3_dev = {
.misc_wait = __WAIT_QUEUE_HEAD_INITIALIZER(lis3_dev.misc_wait),
};
@@ -112,6 +115,24 @@ static void lis3lv02d_get_xyz(struct lis3lv02d *lis3, int *x, int *y, int *z)
*z = lis3lv02d_get_axis(lis3->ac.z, position);
}

+/* conversion btw sampling rate and the register values */
+static int lis3_12_rates[4] = {40, 160, 640, 2560};
+static int lis3_8_rates[2] = {100, 400};
+
+static int lis3lv02d_get_odr(void)
+{
+ u8 ctrl;
+ int val;
+
+ lis3_dev.read(&lis3_dev, CTRL_REG1, &ctrl);
+
+ if (lis3_dev.whoami == WAI_12B)
+ val = lis3_12_rates[(ctrl & (CTRL1_DF0 | CTRL1_DF1)) >> 4];
+ else
+ val = lis3_8_rates[(ctrl & CTRL1_DR) >> 7];
+ return val;
+}
+
void lis3lv02d_poweroff(struct lis3lv02d *lis3)
{
/* disable X,Y,Z axis and power down */
@@ -125,6 +146,9 @@ void lis3lv02d_poweron(struct lis3lv02d *lis3)

lis3->init(lis3);

+ /* LIS3 power on delay is quite long */
+ msleep(lis3->pwron_delay / lis3lv02d_get_odr());
+
/*
* Common configuration
* BDU: (12 bits sensors only) LSB and MSB values are not updated until
@@ -364,23 +388,10 @@ static ssize_t lis3lv02d_calibrate_store(struct device *dev,
return count;
}

-/* conversion btw sampling rate and the register values */
-static int lis3_12_rates[4] = {40, 160, 640, 2560};
-static int lis3_8_rates[2] = {100, 400};
static ssize_t lis3lv02d_rate_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
- u8 ctrl;
- int val;
-
- lis3_dev.read(&lis3_dev, CTRL_REG1, &ctrl);
-
- if (lis3_dev.whoami == WAI_12B)
- val = lis3_12_rates[(ctrl & (CTRL1_DF0 | CTRL1_DF1)) >> 4];
- else
- val = lis3_8_rates[(ctrl & CTRL1_DR) >> 7];
-
- return sprintf(buf, "%d\n", val);
+ return sprintf(buf, "%d\n", lis3lv02d_get_odr());
}

static DEVICE_ATTR(position, S_IRUGO, lis3lv02d_position_show, NULL);
@@ -430,11 +441,13 @@ int lis3lv02d_init_device(struct lis3lv02d *dev)
printk(KERN_INFO DRIVER_NAME ": 12 bits sensor found\n");
dev->read_data = lis3lv02d_read_12;
dev->mdps_max_val = 2048;
+ dev->pwron_delay = LIS3_PWRON_DELAY_WAI_12B;
break;
case WAI_8B:
printk(KERN_INFO DRIVER_NAME ": 8 bits sensor found\n");
dev->read_data = lis3lv02d_read_8;
dev->mdps_max_val = 128;
+ dev->pwron_delay = LIS3_PWRON_DELAY_WAI_8B;
break;
default:
printk(KERN_ERR DRIVER_NAME
diff --git a/drivers/hwmon/lis3lv02d.h b/drivers/hwmon/lis3lv02d.h
index c6ae507..c57f21f 100644
--- a/drivers/hwmon/lis3lv02d.h
+++ b/drivers/hwmon/lis3lv02d.h
@@ -202,6 +202,7 @@ struct lis3lv02d {
u8 whoami; /* indicates measurement precision */
s16 (*read_data) (struct lis3lv02d *lis3, int reg);
int mdps_max_val;
+ int pwron_delay;

struct input_polled_dev *idev; /* input device */
struct platform_device *pdev; /* platform device */
--
1.5.6.3

2009-11-10 12:43:19

by Samu Onkalo

[permalink] [raw]
Subject: [PATCH v2 06/10] LIS3LV02D: Selftest support

Selftest as specified by chip manufacturer.
Controlled via sysfs. Response is OK / FAIL with measurement data for
each axes. test acceptance limits are set using platform data.

Signed-off-by: Samu Onkalo <[email protected]>
---
drivers/hwmon/lis3lv02d.c | 59 ++++++++++++++++++++++++++++++++++++++++++++-
drivers/hwmon/lis3lv02d.h | 13 ++++++++-
include/linux/lis3lv02d.h | 2 +
3 files changed, 71 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
index 39b9ac8..0335b36 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -133,6 +133,51 @@ static int lis3lv02d_get_odr(void)
return val;
}

+static int lis3lv02d_selftest(struct lis3lv02d *lis3, s16 results[3])
+{
+ u8 reg;
+ s16 x, y, z;
+ u8 selftest;
+
+ if (lis3_dev.whoami == WAI_12B)
+ selftest = CTRL1_ST;
+ else
+ selftest = CTRL1_STP;
+
+ lis3->read(lis3, CTRL_REG1, &reg);
+ lis3->write(lis3, CTRL_REG1, (reg | selftest));
+ msleep(lis3->pwron_delay / lis3lv02d_get_odr());
+
+ /* Read directly to avoid axis remap */
+ x = lis3->read_data(lis3, OUTX);
+ y = lis3->read_data(lis3, OUTY);
+ z = lis3->read_data(lis3, OUTZ);
+
+ /* back to normal settings */
+ lis3->write(lis3, CTRL_REG1, reg);
+ msleep(lis3->pwron_delay / lis3lv02d_get_odr());
+
+ results[0] = x - lis3->read_data(lis3, OUTX);
+ results[1] = y - lis3->read_data(lis3, OUTY);
+ results[2] = z - lis3->read_data(lis3, OUTZ);
+
+ if (lis3->pdata) {
+ int i;
+ for (i = 0; i < 3; i++) {
+ /* Check minimum acceptance limits */
+ if (results[i] < lis3->pdata->selftest_min_limits[i])
+ return -1;
+
+ /* Check maximum acceptance limits */
+ if (results[i] > lis3->pdata->selftest_max_limits[i])
+ return -1;
+ }
+ }
+
+ /* test passed */
+ return 0;
+}
+
void lis3lv02d_poweroff(struct lis3lv02d *lis3)
{
/* disable X,Y,Z axis and power down */
@@ -365,6 +410,17 @@ void lis3lv02d_joystick_disable(void)
EXPORT_SYMBOL_GPL(lis3lv02d_joystick_disable);

/* Sysfs stuff */
+static ssize_t lis3lv02d_selftest_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int result;
+ s16 values[3];
+
+ result = lis3lv02d_selftest(&lis3_dev, values);
+ return sprintf(buf, "%s %d %d %d\n", result == 0 ? "OK" : "FAIL",
+ values[0], values[1], values[2]);
+}
+
static ssize_t lis3lv02d_position_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -394,12 +450,14 @@ static ssize_t lis3lv02d_rate_show(struct device *dev,
return sprintf(buf, "%d\n", lis3lv02d_get_odr());
}

+static DEVICE_ATTR(selftest, S_IRUGO, lis3lv02d_selftest_show, NULL);
static DEVICE_ATTR(position, S_IRUGO, lis3lv02d_position_show, NULL);
static DEVICE_ATTR(calibrate, S_IRUGO|S_IWUSR, lis3lv02d_calibrate_show,
lis3lv02d_calibrate_store);
static DEVICE_ATTR(rate, S_IRUGO, lis3lv02d_rate_show, NULL);

static struct attribute *lis3lv02d_attributes[] = {
+ &dev_attr_selftest.attr,
&dev_attr_position.attr,
&dev_attr_calibrate.attr,
&dev_attr_rate.attr,
@@ -507,4 +565,3 @@ EXPORT_SYMBOL_GPL(lis3lv02d_init_device);
MODULE_DESCRIPTION("ST LIS3LV02Dx three-axis digital accelerometer driver");
MODULE_AUTHOR("Yan Burman, Eric Piel, Pavel Machek");
MODULE_LICENSE("GPL");
-
diff --git a/drivers/hwmon/lis3lv02d.h b/drivers/hwmon/lis3lv02d.h
index c57f21f..655875e 100644
--- a/drivers/hwmon/lis3lv02d.h
+++ b/drivers/hwmon/lis3lv02d.h
@@ -98,7 +98,7 @@ enum lis3_who_am_i {
WAI_6B = 0x52, /* 6 bits: LIS331DLF - not supported */
};

-enum lis3lv02d_ctrl1 {
+enum lis3lv02d_ctrl1_12b {
CTRL1_Xen = 0x01,
CTRL1_Yen = 0x02,
CTRL1_Zen = 0x04,
@@ -107,8 +107,17 @@ enum lis3lv02d_ctrl1 {
CTRL1_DF1 = 0x20,
CTRL1_PD0 = 0x40,
CTRL1_PD1 = 0x80,
- CTRL1_DR = 0x80, /* Data rate on 8 bits */
};
+
+/* Delta to ctrl1_12b version */
+enum lis3lv02d_ctrl1_8b {
+ CTRL1_STM = 0x08,
+ CTRL1_STP = 0x10,
+ CTRL1_FS = 0x20,
+ CTRL1_PD = 0x40,
+ CTRL1_DR = 0x80,
+};
+
enum lis3lv02d_ctrl2 {
CTRL2_DAS = 0x01,
CTRL2_SIM = 0x02,
diff --git a/include/linux/lis3lv02d.h b/include/linux/lis3lv02d.h
index 3cc2f2c..966648b 100644
--- a/include/linux/lis3lv02d.h
+++ b/include/linux/lis3lv02d.h
@@ -43,6 +43,8 @@ struct lis3lv02d_platform_data {
#define LIS3_WAKEUP_Z_HI (1 << 5)
unsigned char wakeup_flags;
unsigned char wakeup_thresh;
+ s16 selftest_min_limits[3]; /* x, y, z */
+ s16 selftest_max_limits[3]; /* x, y, z */
};

#endif /* __LIS3LV02D_H_ */
--
1.5.6.3

2009-11-10 12:42:32

by Samu Onkalo

[permalink] [raw]
Subject: [PATCH v2 07/10] LIS3LV02D: Remove calibaration functionality

Chip is calibrated by the manufacturer. There is no need to calibarate
it at driver level. If the chip is used as a joystick, calibaration can
be done using joystick device calibration mechanism.

Signed-off-by: Samu Onkalo <[email protected]>
---
drivers/hwmon/lis3lv02d.c | 32 +++-----------------------------
drivers/hwmon/lis3lv02d.h | 3 ---
2 files changed, 3 insertions(+), 32 deletions(-)

diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
index 0335b36..5b39257 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -344,19 +344,12 @@ static void lis3lv02d_joystick_poll(struct input_polled_dev *pidev)
int x, y, z;

lis3lv02d_get_xyz(&lis3_dev, &x, &y, &z);
- input_report_abs(pidev->input, ABS_X, x - lis3_dev.xcalib);
- input_report_abs(pidev->input, ABS_Y, y - lis3_dev.ycalib);
- input_report_abs(pidev->input, ABS_Z, z - lis3_dev.zcalib);
+ input_report_abs(pidev->input, ABS_X, x);
+ input_report_abs(pidev->input, ABS_Y, y);
+ input_report_abs(pidev->input, ABS_Z, z);
input_sync(pidev->input);
}

-
-static inline void lis3lv02d_calibrate_joystick(void)
-{
- lis3lv02d_get_xyz(&lis3_dev,
- &lis3_dev.xcalib, &lis3_dev.ycalib, &lis3_dev.zcalib);
-}
-
int lis3lv02d_joystick_enable(void)
{
struct input_dev *input_dev;
@@ -373,8 +366,6 @@ int lis3lv02d_joystick_enable(void)
lis3_dev.idev->poll_interval = MDPS_POLL_INTERVAL;
input_dev = lis3_dev.idev->input;

- lis3lv02d_calibrate_joystick();
-
input_dev->name = "ST LIS3LV02DL Accelerometer";
input_dev->phys = DRIVER_NAME "/input0";
input_dev->id.bustype = BUS_HOST;
@@ -430,20 +421,6 @@ static ssize_t lis3lv02d_position_show(struct device *dev,
return sprintf(buf, "(%d,%d,%d)\n", x, y, z);
}

-static ssize_t lis3lv02d_calibrate_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- return sprintf(buf, "(%d,%d,%d)\n", lis3_dev.xcalib, lis3_dev.ycalib, lis3_dev.zcalib);
-}
-
-static ssize_t lis3lv02d_calibrate_store(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t count)
-{
- lis3lv02d_calibrate_joystick();
- return count;
-}
-
static ssize_t lis3lv02d_rate_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -452,14 +429,11 @@ static ssize_t lis3lv02d_rate_show(struct device *dev,

static DEVICE_ATTR(selftest, S_IRUGO, lis3lv02d_selftest_show, NULL);
static DEVICE_ATTR(position, S_IRUGO, lis3lv02d_position_show, NULL);
-static DEVICE_ATTR(calibrate, S_IRUGO|S_IWUSR, lis3lv02d_calibrate_show,
- lis3lv02d_calibrate_store);
static DEVICE_ATTR(rate, S_IRUGO, lis3lv02d_rate_show, NULL);

static struct attribute *lis3lv02d_attributes[] = {
&dev_attr_selftest.attr,
&dev_attr_position.attr,
- &dev_attr_calibrate.attr,
&dev_attr_rate.attr,
NULL
};
diff --git a/drivers/hwmon/lis3lv02d.h b/drivers/hwmon/lis3lv02d.h
index 655875e..a692116 100644
--- a/drivers/hwmon/lis3lv02d.h
+++ b/drivers/hwmon/lis3lv02d.h
@@ -216,9 +216,6 @@ struct lis3lv02d {
struct input_polled_dev *idev; /* input device */
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 */
struct axis_conversion ac; /* hw -> logical axis */

u32 irq; /* IRQ number */
--
1.5.6.3

2009-11-10 12:42:39

by Samu Onkalo

[permalink] [raw]
Subject: [PATCH v2 08/10] lis3: Sysfs entry for setting chip measurement rate

It is possible to read position information at the chip measurement
rate via sysfs. This patch adds possibility to configure chip
measurement rate.

Signed-off-by: Samu Onkalo <[email protected]>
---
drivers/hwmon/lis3lv02d.c | 49 ++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 48 insertions(+), 1 deletions(-)

diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
index 5b39257..be2eb97 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -133,6 +133,37 @@ static int lis3lv02d_get_odr(void)
return val;
}

+static int lis3lv02d_set_odr(int rate)
+{
+ u8 ctrl;
+ int i, len, shift;
+ int *rates;
+ int ret = -EINVAL;
+
+ lis3_dev.read(&lis3_dev, CTRL_REG1, &ctrl);
+
+ if (lis3_dev.whoami == WAI_12B) {
+ len = sizeof(lis3_12_rates);
+ rates = lis3_12_rates;
+ shift = 4;
+ ctrl &= ~(CTRL1_DF0 | CTRL1_DF1);
+ } else {
+ len = sizeof(lis3_8_rates);
+ rates = lis3_8_rates;
+ shift = 7;
+ ctrl &= ~CTRL1_DR;
+ }
+
+ for (i = 0; i < len; i++)
+ if (rates[i] == rate) {
+ lis3_dev.write(&lis3_dev,
+ CTRL_REG1, ctrl | (i << shift));
+ ret = 0;
+ break;
+ }
+ return ret;
+}
+
static int lis3lv02d_selftest(struct lis3lv02d *lis3, s16 results[3])
{
u8 reg;
@@ -427,9 +458,25 @@ static ssize_t lis3lv02d_rate_show(struct device *dev,
return sprintf(buf, "%d\n", lis3lv02d_get_odr());
}

+static ssize_t lis3lv02d_rate_set(struct device *dev,
+ struct device_attribute *attr, const char *buf,
+ size_t count)
+{
+ unsigned long rate;
+
+ if (strict_strtoul(buf, 0, &rate))
+ return -EINVAL;
+
+ if (lis3lv02d_set_odr(rate))
+ return -EINVAL;
+
+ return count;
+}
+
static DEVICE_ATTR(selftest, S_IRUGO, lis3lv02d_selftest_show, NULL);
static DEVICE_ATTR(position, S_IRUGO, lis3lv02d_position_show, NULL);
-static DEVICE_ATTR(rate, S_IRUGO, lis3lv02d_rate_show, NULL);
+static DEVICE_ATTR(rate, S_IRUGO | S_IWUSR, lis3lv02d_rate_show,
+ lis3lv02d_rate_set);

static struct attribute *lis3lv02d_attributes[] = {
&dev_attr_selftest.attr,
--
1.5.6.3

2009-11-10 12:42:51

by Samu Onkalo

[permalink] [raw]
Subject: [PATCH v2 09/10] lis3: Scale output values to mg

Report output values as 1/1000 of earth gravity.

Signed-off-by: Samu Onkalo <[email protected]>
---
drivers/hwmon/lis3lv02d.c | 31 ++++++++++++++++++++++++++++---
drivers/hwmon/lis3lv02d.h | 4 ++++
2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
index be2eb97..7e72836 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -53,6 +53,19 @@
#define LIS3_PWRON_DELAY_WAI_12B (5000)
#define LIS3_PWRON_DELAY_WAI_8B (3000)

+/*
+ * LIS3LV02D spec says 1024 LSBs corresponds 1 G -> 1LSB is 1000/1024 mG
+ * LIS302D spec says: 18 mG / digit
+ * SCALING_ACCURACY is used to increase accuracy of the intermediate
+ * calculation results.
+ */
+#define LIS3_SCALING_ACCURACY 1024
+#define LIS3_2G2G_SENSITIVITY_WAI_12B ((LIS3_SCALING_ACCURACY * 1000) / 1024)
+#define LIS3_2G2G_SENSITIVITY_WAI_8B (18 * LIS3_SCALING_ACCURACY)
+
+#define LIS3_DEFAULT_FUZZ 3
+#define LIS3_DEFAULT_FLAT 3
+
struct lis3lv02d lis3_dev = {
.misc_wait = __WAIT_QUEUE_HEAD_INITIALIZER(lis3_dev.misc_wait),
};
@@ -105,11 +118,16 @@ static inline int lis3lv02d_get_axis(s8 axis, int hw_values[3])
static void lis3lv02d_get_xyz(struct lis3lv02d *lis3, int *x, int *y, int *z)
{
int position[3];
+ int i;

position[0] = lis3->read_data(lis3, OUTX);
position[1] = lis3->read_data(lis3, OUTY);
position[2] = lis3->read_data(lis3, OUTZ);

+ for (i = 0; i < 3; i++)
+ position[i] = (position[i] * lis3->scale) /
+ LIS3_SCALING_ACCURACY;
+
*x = lis3lv02d_get_axis(lis3->ac.x, position);
*y = lis3lv02d_get_axis(lis3->ac.y, position);
*z = lis3lv02d_get_axis(lis3->ac.z, position);
@@ -385,6 +403,7 @@ int lis3lv02d_joystick_enable(void)
{
struct input_dev *input_dev;
int err;
+ int max_val, fuzz, flat;

if (lis3_dev.idev)
return -EINVAL;
@@ -404,9 +423,13 @@ int lis3lv02d_joystick_enable(void)
input_dev->dev.parent = &lis3_dev.pdev->dev;

set_bit(EV_ABS, input_dev->evbit);
- input_set_abs_params(input_dev, ABS_X, -lis3_dev.mdps_max_val, lis3_dev.mdps_max_val, 3, 3);
- input_set_abs_params(input_dev, ABS_Y, -lis3_dev.mdps_max_val, lis3_dev.mdps_max_val, 3, 3);
- input_set_abs_params(input_dev, ABS_Z, -lis3_dev.mdps_max_val, lis3_dev.mdps_max_val, 3, 3);
+ max_val = (lis3_dev.mdps_max_val * lis3_dev.scale)
+ / LIS3_SCALING_ACCURACY;
+ fuzz = (LIS3_DEFAULT_FUZZ * lis3_dev.scale) / LIS3_SCALING_ACCURACY;
+ flat = (LIS3_DEFAULT_FLAT * lis3_dev.scale) / LIS3_SCALING_ACCURACY;
+ input_set_abs_params(input_dev, ABS_X, -max_val, max_val, fuzz, flat);
+ input_set_abs_params(input_dev, ABS_Y, -max_val, max_val, fuzz, flat);
+ input_set_abs_params(input_dev, ABS_Z, -max_val, max_val, fuzz, flat);

err = input_register_polled_device(lis3_dev.idev);
if (err) {
@@ -521,12 +544,14 @@ int lis3lv02d_init_device(struct lis3lv02d *dev)
dev->read_data = lis3lv02d_read_12;
dev->mdps_max_val = 2048;
dev->pwron_delay = LIS3_PWRON_DELAY_WAI_12B;
+ dev->scale = LIS3_2G2G_SENSITIVITY_WAI_12B;
break;
case WAI_8B:
printk(KERN_INFO DRIVER_NAME ": 8 bits sensor found\n");
dev->read_data = lis3lv02d_read_8;
dev->mdps_max_val = 128;
dev->pwron_delay = LIS3_PWRON_DELAY_WAI_8B;
+ dev->scale = LIS3_2G2G_SENSITIVITY_WAI_8B;
break;
default:
printk(KERN_ERR DRIVER_NAME
diff --git a/drivers/hwmon/lis3lv02d.h b/drivers/hwmon/lis3lv02d.h
index a692116..42618b8 100644
--- a/drivers/hwmon/lis3lv02d.h
+++ b/drivers/hwmon/lis3lv02d.h
@@ -212,6 +212,10 @@ struct lis3lv02d {
s16 (*read_data) (struct lis3lv02d *lis3, int reg);
int mdps_max_val;
int pwron_delay;
+ int scale; /*
+ * relationship between 1 LBS and mG
+ * (1/1000th of earth gravity)
+ */

struct input_polled_dev *idev; /* input device */
struct platform_device *pdev; /* platform device */
--
1.5.6.3

2009-11-10 12:42:33

by Samu Onkalo

[permalink] [raw]
Subject: [PATCH v2 10/10] LIS3: Update documentation to match latest changes

Signed-off-by: Samu Onkalo <[email protected]>
---
Documentation/hwmon/lis3lv02d | 16 +++++++++-------
1 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/Documentation/hwmon/lis3lv02d b/Documentation/hwmon/lis3lv02d
index 21f0902..e8ced22 100644
--- a/Documentation/hwmon/lis3lv02d
+++ b/Documentation/hwmon/lis3lv02d
@@ -20,18 +20,20 @@ sporting the feature officially called "HP Mobile Data Protection System 3D" or
models (full list can be found in drivers/hwmon/hp_accel.c) will have their
axis automatically oriented on standard way (eg: you can directly play
neverball). The accelerometer data is readable via
-/sys/devices/platform/lis3lv02d.
+/sys/devices/platform/lis3lv02d. Reported values are scaled
+to mg values (1/1000th of earth gravity).

Sysfs attributes under /sys/devices/platform/lis3lv02d/:
position - 3D position that the accelerometer reports. Format: "(x,y,z)"
-calibrate - read: values (x, y, z) that are used as the base for input
- class device operation.
- write: forces the base to be recalibrated with the current
- position.
-rate - reports the sampling rate of the accelerometer device in HZ
+rate - read reports the sampling rate of the accelerometer device in HZ.
+ write changes sampling rate of the accelerometer device.
+ Only values which are supported by HW are accepted.
+selftest - performs selftest for the chip as specified by chip manufacturer.
+ Acceptance limits for the selftest must be set in platform data.

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. Joystick device can be
+calibrated.

On HP laptops, if the led infrastructure is activated, support for a led
indicating disk protection will be provided as /sys/class/leds/hp::hddprotect.
--
1.5.6.3

2009-11-15 15:09:04

by Éric Piel

[permalink] [raw]
Subject: Re: [PATCH v2 06/10] LIS3LV02D: Selftest support

Op 10-11-09 13:41, Samu Onkalo schreef:
> Selftest as specified by chip manufacturer.
> Controlled via sysfs. Response is OK / FAIL with measurement data for
> each axes. test acceptance limits are set using platform data.
A few remarks:
* It's not clear what's the effect of doing it while using the sensor
at the same time. I've just tried, and it seems the sensor recovers,
although it returns wrong values during the selftest. So it would be
better to make it accessible by only root: change S_IRUGO to S_IRUSR.
* The documentation is not clear enough why I would need to run it, how
often, what are the values displayed, and what are those platform data
limit supposed to be. Please, make this more clear. (Actually the
documentation is an other patch, but it makes more sense to talk about
this here, although it's ok to leave it in another patch)

Eric

>
> Signed-off-by: Samu Onkalo <[email protected]>
> ---
> drivers/hwmon/lis3lv02d.c | 59 ++++++++++++++++++++++++++++++++++++++++++++-
> drivers/hwmon/lis3lv02d.h | 13 ++++++++-
> include/linux/lis3lv02d.h | 2 +
> 3 files changed, 71 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
> index 39b9ac8..0335b36 100644
> --- a/drivers/hwmon/lis3lv02d.c
> +++ b/drivers/hwmon/lis3lv02d.c
> @@ -133,6 +133,51 @@ static int lis3lv02d_get_odr(void)
> return val;
> }
>
> +static int lis3lv02d_selftest(struct lis3lv02d *lis3, s16 results[3])
> +{
> + u8 reg;
> + s16 x, y, z;
> + u8 selftest;
> +
> + if (lis3_dev.whoami == WAI_12B)
> + selftest = CTRL1_ST;
> + else
> + selftest = CTRL1_STP;
> +
> + lis3->read(lis3, CTRL_REG1, &reg);
> + lis3->write(lis3, CTRL_REG1, (reg | selftest));
> + msleep(lis3->pwron_delay / lis3lv02d_get_odr());
> +
> + /* Read directly to avoid axis remap */
> + x = lis3->read_data(lis3, OUTX);
> + y = lis3->read_data(lis3, OUTY);
> + z = lis3->read_data(lis3, OUTZ);
> +
> + /* back to normal settings */
> + lis3->write(lis3, CTRL_REG1, reg);
> + msleep(lis3->pwron_delay / lis3lv02d_get_odr());
> +
> + results[0] = x - lis3->read_data(lis3, OUTX);
> + results[1] = y - lis3->read_data(lis3, OUTY);
> + results[2] = z - lis3->read_data(lis3, OUTZ);
> +
> + if (lis3->pdata) {
> + int i;
> + for (i = 0; i < 3; i++) {
> + /* Check minimum acceptance limits */
> + if (results[i] < lis3->pdata->selftest_min_limits[i])
> + return -1;
> +
> + /* Check maximum acceptance limits */
> + if (results[i] > lis3->pdata->selftest_max_limits[i])
> + return -1;
> + }
> + }
> +
> + /* test passed */
> + return 0;
> +}
> +
> void lis3lv02d_poweroff(struct lis3lv02d *lis3)
> {
> /* disable X,Y,Z axis and power down */
> @@ -365,6 +410,17 @@ void lis3lv02d_joystick_disable(void)
> EXPORT_SYMBOL_GPL(lis3lv02d_joystick_disable);
>
> /* Sysfs stuff */
> +static ssize_t lis3lv02d_selftest_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + int result;
> + s16 values[3];
> +
> + result = lis3lv02d_selftest(&lis3_dev, values);
> + return sprintf(buf, "%s %d %d %d\n", result == 0 ? "OK" : "FAIL",
> + values[0], values[1], values[2]);
> +}
> +
> static ssize_t lis3lv02d_position_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> @@ -394,12 +450,14 @@ static ssize_t lis3lv02d_rate_show(struct device *dev,
> return sprintf(buf, "%d\n", lis3lv02d_get_odr());
> }
>
> +static DEVICE_ATTR(selftest, S_IRUGO, lis3lv02d_selftest_show, NULL);
> static DEVICE_ATTR(position, S_IRUGO, lis3lv02d_position_show, NULL);
> static DEVICE_ATTR(calibrate, S_IRUGO|S_IWUSR, lis3lv02d_calibrate_show,
> lis3lv02d_calibrate_store);
> static DEVICE_ATTR(rate, S_IRUGO, lis3lv02d_rate_show, NULL);
>
> static struct attribute *lis3lv02d_attributes[] = {
> + &dev_attr_selftest.attr,
> &dev_attr_position.attr,
> &dev_attr_calibrate.attr,
> &dev_attr_rate.attr,
> @@ -507,4 +565,3 @@ EXPORT_SYMBOL_GPL(lis3lv02d_init_device);
> MODULE_DESCRIPTION("ST LIS3LV02Dx three-axis digital accelerometer driver");
> MODULE_AUTHOR("Yan Burman, Eric Piel, Pavel Machek");
> MODULE_LICENSE("GPL");
> -
> diff --git a/drivers/hwmon/lis3lv02d.h b/drivers/hwmon/lis3lv02d.h
> index c57f21f..655875e 100644
> --- a/drivers/hwmon/lis3lv02d.h
> +++ b/drivers/hwmon/lis3lv02d.h
> @@ -98,7 +98,7 @@ enum lis3_who_am_i {
> WAI_6B = 0x52, /* 6 bits: LIS331DLF - not supported */
> };
>
> -enum lis3lv02d_ctrl1 {
> +enum lis3lv02d_ctrl1_12b {
> CTRL1_Xen = 0x01,
> CTRL1_Yen = 0x02,
> CTRL1_Zen = 0x04,
> @@ -107,8 +107,17 @@ enum lis3lv02d_ctrl1 {
> CTRL1_DF1 = 0x20,
> CTRL1_PD0 = 0x40,
> CTRL1_PD1 = 0x80,
> - CTRL1_DR = 0x80, /* Data rate on 8 bits */
> };
> +
> +/* Delta to ctrl1_12b version */
> +enum lis3lv02d_ctrl1_8b {
> + CTRL1_STM = 0x08,
> + CTRL1_STP = 0x10,
> + CTRL1_FS = 0x20,
> + CTRL1_PD = 0x40,
> + CTRL1_DR = 0x80,
> +};
> +
> enum lis3lv02d_ctrl2 {
> CTRL2_DAS = 0x01,
> CTRL2_SIM = 0x02,
> diff --git a/include/linux/lis3lv02d.h b/include/linux/lis3lv02d.h
> index 3cc2f2c..966648b 100644
> --- a/include/linux/lis3lv02d.h
> +++ b/include/linux/lis3lv02d.h
> @@ -43,6 +43,8 @@ struct lis3lv02d_platform_data {
> #define LIS3_WAKEUP_Z_HI (1 << 5)
> unsigned char wakeup_flags;
> unsigned char wakeup_thresh;
> + s16 selftest_min_limits[3]; /* x, y, z */
> + s16 selftest_max_limits[3]; /* x, y, z */
> };
>
> #endif /* __LIS3LV02D_H_ */

2009-11-15 15:20:50

by Éric Piel

[permalink] [raw]
Subject: Re: [PATCH v2 08/10] lis3: Sysfs entry for setting chip measurement rate

Op 10-11-09 13:41, Samu Onkalo schreef:
> It is possible to read position information at the chip measurement
> rate via sysfs. This patch adds possibility to configure chip
> measurement rate.
Looks fine in general... but there is a bug, cf below.

In addition, the construct of the code make me cry for object
orientation. I'll get over it, but we should keep in mind that cleaning
this up would be welcome (maybe through the usage of an additional
function pointer in the lis3lv02 struct).

Eric

>
> Signed-off-by: Samu Onkalo <[email protected]>
> ---
> drivers/hwmon/lis3lv02d.c | 49 ++++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 48 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
> index 5b39257..be2eb97 100644
> --- a/drivers/hwmon/lis3lv02d.c
> +++ b/drivers/hwmon/lis3lv02d.c
> @@ -133,6 +133,37 @@ static int lis3lv02d_get_odr(void)
> return val;
> }
>
> +static int lis3lv02d_set_odr(int rate)
> +{
> + u8 ctrl;
> + int i, len, shift;
> + int *rates;
> + int ret = -EINVAL;
> +
> + lis3_dev.read(&lis3_dev, CTRL_REG1, &ctrl);
> +
> + if (lis3_dev.whoami == WAI_12B) {
> + len = sizeof(lis3_12_rates);
ARRAY_SIZE() would be much more correct!

> + rates = lis3_12_rates;
> + shift = 4;
> + ctrl &= ~(CTRL1_DF0 | CTRL1_DF1);
> + } else {
> + len = sizeof(lis3_8_rates);
idem

> + rates = lis3_8_rates;
> + shift = 7;
> + ctrl &= ~CTRL1_DR;
> + }
> +
> + for (i = 0; i < len; i++)
> + if (rates[i] == rate) {
> + lis3_dev.write(&lis3_dev,
> + CTRL_REG1, ctrl | (i << shift));
> + ret = 0;
You could do simply a "return 0"

> + break;
> + }
> + return ret;
And here a "return -EINVAL"


> +}
> +
> static int lis3lv02d_selftest(struct lis3lv02d *lis3, s16 results[3])
> {
> u8 reg;
> @@ -427,9 +458,25 @@ static ssize_t lis3lv02d_rate_show(struct device *dev,
> return sprintf(buf, "%d\n", lis3lv02d_get_odr());
> }
>
> +static ssize_t lis3lv02d_rate_set(struct device *dev,
> + struct device_attribute *attr, const char *buf,
> + size_t count)
> +{
> + unsigned long rate;
> +
> + if (strict_strtoul(buf, 0, &rate))
> + return -EINVAL;
> +
> + if (lis3lv02d_set_odr(rate))
> + return -EINVAL;
> +
> + return count;
> +}
> +
> static DEVICE_ATTR(selftest, S_IRUGO, lis3lv02d_selftest_show, NULL);
> static DEVICE_ATTR(position, S_IRUGO, lis3lv02d_position_show, NULL);
> -static DEVICE_ATTR(rate, S_IRUGO, lis3lv02d_rate_show, NULL);
> +static DEVICE_ATTR(rate, S_IRUGO | S_IWUSR, lis3lv02d_rate_show,
> + lis3lv02d_rate_set);
>
> static struct attribute *lis3lv02d_attributes[] = {
> &dev_attr_selftest.attr,

2009-11-15 15:32:04

by Éric Piel

[permalink] [raw]
Subject: Re: [PATCH v2 09/10] lis3: Scale output values to mg

Op 10-11-09 13:41, Samu Onkalo schreef:
> Report output values as 1/1000 of earth gravity.
After the discussion started by Daniel, I'm willing to accept this patch
"as is", just with a better log explaining why it's ok to break
userspace ABI:
* the sysfs "position" attribute was never safe to be read as it could
have different scales depending on the underlying hardware. It's now
featuring a consistent scale, which is more or less identical to the one
used by the 12-bits sensors
* the joystick interface reports the same values as before, excepted of
course if "raw" values are requested, in which case the same remark as
for "position" applies.

Eric

>
> Signed-off-by: Samu Onkalo <[email protected]>
> ---
> drivers/hwmon/lis3lv02d.c | 31 ++++++++++++++++++++++++++++---
> drivers/hwmon/lis3lv02d.h | 4 ++++
> 2 files changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
> index be2eb97..7e72836 100644
> --- a/drivers/hwmon/lis3lv02d.c
> +++ b/drivers/hwmon/lis3lv02d.c
> @@ -53,6 +53,19 @@
> #define LIS3_PWRON_DELAY_WAI_12B (5000)
> #define LIS3_PWRON_DELAY_WAI_8B (3000)
>
> +/*
> + * LIS3LV02D spec says 1024 LSBs corresponds 1 G -> 1LSB is 1000/1024 mG
> + * LIS302D spec says: 18 mG / digit
> + * SCALING_ACCURACY is used to increase accuracy of the intermediate
> + * calculation results.
> + */
> +#define LIS3_SCALING_ACCURACY 1024
> +#define LIS3_2G2G_SENSITIVITY_WAI_12B ((LIS3_SCALING_ACCURACY * 1000) / 1024)
> +#define LIS3_2G2G_SENSITIVITY_WAI_8B (18 * LIS3_SCALING_ACCURACY)
> +
> +#define LIS3_DEFAULT_FUZZ 3
> +#define LIS3_DEFAULT_FLAT 3
> +
> struct lis3lv02d lis3_dev = {
> .misc_wait = __WAIT_QUEUE_HEAD_INITIALIZER(lis3_dev.misc_wait),
> };
> @@ -105,11 +118,16 @@ static inline int lis3lv02d_get_axis(s8 axis, int hw_values[3])
> static void lis3lv02d_get_xyz(struct lis3lv02d *lis3, int *x, int *y, int *z)
> {
> int position[3];
> + int i;
>
> position[0] = lis3->read_data(lis3, OUTX);
> position[1] = lis3->read_data(lis3, OUTY);
> position[2] = lis3->read_data(lis3, OUTZ);
>
> + for (i = 0; i < 3; i++)
> + position[i] = (position[i] * lis3->scale) /
> + LIS3_SCALING_ACCURACY;
> +
> *x = lis3lv02d_get_axis(lis3->ac.x, position);
> *y = lis3lv02d_get_axis(lis3->ac.y, position);
> *z = lis3lv02d_get_axis(lis3->ac.z, position);
> @@ -385,6 +403,7 @@ int lis3lv02d_joystick_enable(void)
> {
> struct input_dev *input_dev;
> int err;
> + int max_val, fuzz, flat;
>
> if (lis3_dev.idev)
> return -EINVAL;
> @@ -404,9 +423,13 @@ int lis3lv02d_joystick_enable(void)
> input_dev->dev.parent = &lis3_dev.pdev->dev;
>
> set_bit(EV_ABS, input_dev->evbit);
> - input_set_abs_params(input_dev, ABS_X, -lis3_dev.mdps_max_val, lis3_dev.mdps_max_val, 3, 3);
> - input_set_abs_params(input_dev, ABS_Y, -lis3_dev.mdps_max_val, lis3_dev.mdps_max_val, 3, 3);
> - input_set_abs_params(input_dev, ABS_Z, -lis3_dev.mdps_max_val, lis3_dev.mdps_max_val, 3, 3);
> + max_val = (lis3_dev.mdps_max_val * lis3_dev.scale)
> + / LIS3_SCALING_ACCURACY;
> + fuzz = (LIS3_DEFAULT_FUZZ * lis3_dev.scale) / LIS3_SCALING_ACCURACY;
> + flat = (LIS3_DEFAULT_FLAT * lis3_dev.scale) / LIS3_SCALING_ACCURACY;
> + input_set_abs_params(input_dev, ABS_X, -max_val, max_val, fuzz, flat);
> + input_set_abs_params(input_dev, ABS_Y, -max_val, max_val, fuzz, flat);
> + input_set_abs_params(input_dev, ABS_Z, -max_val, max_val, fuzz, flat);
>
> err = input_register_polled_device(lis3_dev.idev);
> if (err) {
> @@ -521,12 +544,14 @@ int lis3lv02d_init_device(struct lis3lv02d *dev)
> dev->read_data = lis3lv02d_read_12;
> dev->mdps_max_val = 2048;
> dev->pwron_delay = LIS3_PWRON_DELAY_WAI_12B;
> + dev->scale = LIS3_2G2G_SENSITIVITY_WAI_12B;
> break;
> case WAI_8B:
> printk(KERN_INFO DRIVER_NAME ": 8 bits sensor found\n");
> dev->read_data = lis3lv02d_read_8;
> dev->mdps_max_val = 128;
> dev->pwron_delay = LIS3_PWRON_DELAY_WAI_8B;
> + dev->scale = LIS3_2G2G_SENSITIVITY_WAI_8B;
> break;
> default:
> printk(KERN_ERR DRIVER_NAME
> diff --git a/drivers/hwmon/lis3lv02d.h b/drivers/hwmon/lis3lv02d.h
> index a692116..42618b8 100644
> --- a/drivers/hwmon/lis3lv02d.h
> +++ b/drivers/hwmon/lis3lv02d.h
> @@ -212,6 +212,10 @@ struct lis3lv02d {
> s16 (*read_data) (struct lis3lv02d *lis3, int reg);
> int mdps_max_val;
> int pwron_delay;
> + int scale; /*
> + * relationship between 1 LBS and mG
> + * (1/1000th of earth gravity)
> + */
>
> struct input_polled_dev *idev; /* input device */
> struct platform_device *pdev; /* platform device */

2009-11-15 15:43:16

by Éric Piel

[permalink] [raw]
Subject: Re: [PATCH v2 10/10] LIS3: Update documentation to match latest changes

Op 10-11-09 13:41, Samu Onkalo schreef:
As mentioned before, please explain a bit more what is selftest about.
See also below for more comments.

Eric

> Signed-off-by: Samu Onkalo <[email protected]>
> ---
> Documentation/hwmon/lis3lv02d | 16 +++++++++-------
> 1 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/hwmon/lis3lv02d b/Documentation/hwmon/lis3lv02d
> index 21f0902..e8ced22 100644
> --- a/Documentation/hwmon/lis3lv02d
> +++ b/Documentation/hwmon/lis3lv02d
> @@ -20,18 +20,20 @@ sporting the feature officially called "HP Mobile Data Protection System 3D" or
> models (full list can be found in drivers/hwmon/hp_accel.c) will have their
> axis automatically oriented on standard way (eg: you can directly play
> neverball). The accelerometer data is readable via
> -/sys/devices/platform/lis3lv02d.
> +/sys/devices/platform/lis3lv02d. Reported values are scaled
> +to mg values (1/1000th of earth gravity).
>
> Sysfs attributes under /sys/devices/platform/lis3lv02d/:
> position - 3D position that the accelerometer reports. Format: "(x,y,z)"
> -calibrate - read: values (x, y, z) that are used as the base for input
> - class device operation.
> - write: forces the base to be recalibrated with the current
> - position.
> -rate - reports the sampling rate of the accelerometer device in HZ
> +rate - read reports the sampling rate of the accelerometer device in HZ.
> + write changes sampling rate of the accelerometer device.
> + Only values which are supported by HW are accepted.
> +selftest - performs selftest for the chip as specified by chip manufacturer.
> + Acceptance limits for the selftest must be set in platform data.
>
> 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. Joystick device can be
> +calibrated.
Please add something like: "In raw mode, the joystick interface reports
the same values as the "position" sysfs attribute."


>
> On HP laptops, if the led infrastructure is activated, support for a led
> indicating disk protection will be provided as /sys/class/leds/hp::hddprotect.

2009-11-15 15:49:06

by Éric Piel

[permalink] [raw]
Subject: Re: [PATCH v2 01/10] LIS3LV02D: Send sync event

Op 10-11-09 13:41, Samu Onkalo schreef:
> Send input_sync after each measurement round. This helps userspace to
> detect which reported values belongs to the same measurement.
>
> Signed-off-by: Samu Onkalo <[email protected]>
Acked-by: Éric Piel <[email protected]>

> ---
> drivers/hwmon/lis3lv02d.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
> index cf5afb9..7f43a3b 100644
> --- a/drivers/hwmon/lis3lv02d.c
> +++ b/drivers/hwmon/lis3lv02d.c
> @@ -276,6 +276,7 @@ static void lis3lv02d_joystick_poll(struct input_polled_dev *pidev)
> input_report_abs(pidev->input, ABS_X, x - lis3_dev.xcalib);
> input_report_abs(pidev->input, ABS_Y, y - lis3_dev.ycalib);
> input_report_abs(pidev->input, ABS_Z, z - lis3_dev.zcalib);
> + input_sync(pidev->input);
> }
>
>

2009-11-15 15:49:33

by Éric Piel

[permalink] [raw]
Subject: Re: [PATCH v2 02/10] LIS3LV02D: Correct memory leak in module unload

Op 10-11-09 13:41, Samu Onkalo schreef:
> polled input device itself was not free'd.
>
> Signed-off-by: Samu Onkalo <[email protected]>
Acked-by: Éric Piel <[email protected]>
> ---
> drivers/hwmon/lis3lv02d.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
> index 7f43a3b..dbd0b05 100644
> --- a/drivers/hwmon/lis3lv02d.c
> +++ b/drivers/hwmon/lis3lv02d.c
> @@ -333,6 +333,7 @@ void lis3lv02d_joystick_disable(void)
> if (lis3_dev.irq)
> misc_deregister(&lis3lv02d_misc_device);
> input_unregister_polled_device(lis3_dev.idev);
> + input_free_polled_device(lis3_dev.idev);
> lis3_dev.idev = NULL;
> }
> EXPORT_SYMBOL_GPL(lis3lv02d_joystick_disable);

2009-11-15 15:53:32

by Éric Piel

[permalink] [raw]
Subject: Re: [PATCH v2 04/10] lis3: fix show rate for 8 bits chips

Op 10-11-09 13:41, Samu Onkalo schreef:
> From: Éric Piel <[email protected]>
>
> lis3: fix with 8 bits sensors
>
Here is the same patch, but with a better log.

Eric
8<--------------------------------------------------------------
Subject: lis3: fixes for 8 bits chips

Originally the driver was only targeted to 12bits sensors. When support
for 8bits sensors was added, some slight difference in the registers
were overlooked. This should fix it, both for initialization, and for
displaying the rate.

Reported-by: Kalhan Trisal <[email protected]>
Reported-by: Christoph Plattner <[email protected]>
Tested-by: Christoph Plattner <[email protected]>
Tested-by: Samu Onkalo <[email protected]>
Signed-off-by: Éric Piel <[email protected]>
---
drivers/hwmon/lis3lv02d.c | 24 ++++++++++++++++--------
drivers/hwmon/lis3lv02d.h | 1 +
2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
index 1c8f108..b12ee35 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -127,12 +127,14 @@ void lis3lv02d_poweron(struct lis3lv02d *lis3)

/*
* Common configuration
- * BDU: LSB and MSB values are not updated until both have been read.
- * So the value read will always be correct.
+ * BDU: (12 bits sensors only) LSB and MSB values are not updated until
+ * both have been read. So the value read will always be correct.
*/
- lis3->read(lis3, CTRL_REG2, &reg);
- reg |= CTRL2_BDU;
- lis3->write(lis3, CTRL_REG2, reg);
+ if (lis3->whoami == WAI_12B) {
+ lis3->read(lis3, CTRL_REG2, &reg);
+ reg |= CTRL2_BDU;
+ lis3->write(lis3, CTRL_REG2, reg);
+ }
}
EXPORT_SYMBOL_GPL(lis3lv02d_poweron);

@@ -363,7 +365,8 @@ static ssize_t lis3lv02d_calibrate_store(struct device *dev,
}

/* conversion btw sampling rate and the register values */
-static int lis3lv02dl_df_val[4] = {40, 160, 640, 2560};
+static int lis3_12_rates[4] = {40, 160, 640, 2560};
+static int lis3_8_rates[2] = {100, 400};
static ssize_t lis3lv02d_rate_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -371,8 +374,13 @@ static ssize_t lis3lv02d_rate_show(struct device *dev,
int val;

lis3_dev.read(&lis3_dev, CTRL_REG1, &ctrl);
- val = (ctrl & (CTRL1_DF0 | CTRL1_DF1)) >> 4;
- return sprintf(buf, "%d\n", lis3lv02dl_df_val[val]);
+
+ if (lis3_dev.whoami == WAI_12B)
+ val = lis3_12_rates[(ctrl & (CTRL1_DF0 | CTRL1_DF1)) >> 4];
+ else
+ val = lis3_8_rates[(ctrl & CTRL1_DR) >> 7];
+
+ return sprintf(buf, "%d\n", val);
}

static DEVICE_ATTR(position, S_IRUGO, lis3lv02d_position_show, NULL);
diff --git a/drivers/hwmon/lis3lv02d.h b/drivers/hwmon/lis3lv02d.h
index 2431c51..c6ae507 100644
--- a/drivers/hwmon/lis3lv02d.h
+++ b/drivers/hwmon/lis3lv02d.h
@@ -107,6 +107,7 @@ enum lis3lv02d_ctrl1 {
CTRL1_DF1 = 0x20,
CTRL1_PD0 = 0x40,
CTRL1_PD1 = 0x80,
+ CTRL1_DR = 0x80, /* Data rate on 8 bits */
};
enum lis3lv02d_ctrl2 {
CTRL2_DAS = 0x01,
--
1.6.5.2

2009-11-15 15:54:05

by Éric Piel

[permalink] [raw]
Subject: Re: [PATCH v2 05/10] LIS3LV02D: Proper power on sequence

Op 10-11-09 13:41, Samu Onkalo schreef:
> Lis3 accelerometer sensors have quite long power on delay (up to 125
> ms). This patch adds necessary delay to power on sequence for currently
> supported lis3 chips.
>
> Signed-off-by: Samu Onkalo <[email protected]>
Acked-by: Éric Piel <[email protected]>
> ---
> drivers/hwmon/lis3lv02d.c | 41 +++++++++++++++++++++++++++--------------
> drivers/hwmon/lis3lv02d.h | 1 +
> 2 files changed, 28 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
> index b12ee35..39b9ac8 100644
> --- a/drivers/hwmon/lis3lv02d.c
> +++ b/drivers/hwmon/lis3lv02d.c
> @@ -50,6 +50,9 @@
> * joystick.
> */
>
> +#define LIS3_PWRON_DELAY_WAI_12B (5000)
> +#define LIS3_PWRON_DELAY_WAI_8B (3000)
> +
> struct lis3lv02d lis3_dev = {
> .misc_wait = __WAIT_QUEUE_HEAD_INITIALIZER(lis3_dev.misc_wait),
> };
> @@ -112,6 +115,24 @@ static void lis3lv02d_get_xyz(struct lis3lv02d *lis3, int *x, int *y, int *z)
> *z = lis3lv02d_get_axis(lis3->ac.z, position);
> }
>
> +/* conversion btw sampling rate and the register values */
> +static int lis3_12_rates[4] = {40, 160, 640, 2560};
> +static int lis3_8_rates[2] = {100, 400};
> +
> +static int lis3lv02d_get_odr(void)
> +{
> + u8 ctrl;
> + int val;
> +
> + lis3_dev.read(&lis3_dev, CTRL_REG1, &ctrl);
> +
> + if (lis3_dev.whoami == WAI_12B)
> + val = lis3_12_rates[(ctrl & (CTRL1_DF0 | CTRL1_DF1)) >> 4];
> + else
> + val = lis3_8_rates[(ctrl & CTRL1_DR) >> 7];
> + return val;
> +}
> +
> void lis3lv02d_poweroff(struct lis3lv02d *lis3)
> {
> /* disable X,Y,Z axis and power down */
> @@ -125,6 +146,9 @@ void lis3lv02d_poweron(struct lis3lv02d *lis3)
>
> lis3->init(lis3);
>
> + /* LIS3 power on delay is quite long */
> + msleep(lis3->pwron_delay / lis3lv02d_get_odr());
> +
> /*
> * Common configuration
> * BDU: (12 bits sensors only) LSB and MSB values are not updated until
> @@ -364,23 +388,10 @@ static ssize_t lis3lv02d_calibrate_store(struct device *dev,
> return count;
> }
>
> -/* conversion btw sampling rate and the register values */
> -static int lis3_12_rates[4] = {40, 160, 640, 2560};
> -static int lis3_8_rates[2] = {100, 400};
> static ssize_t lis3lv02d_rate_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> - u8 ctrl;
> - int val;
> -
> - lis3_dev.read(&lis3_dev, CTRL_REG1, &ctrl);
> -
> - if (lis3_dev.whoami == WAI_12B)
> - val = lis3_12_rates[(ctrl & (CTRL1_DF0 | CTRL1_DF1)) >> 4];
> - else
> - val = lis3_8_rates[(ctrl & CTRL1_DR) >> 7];
> -
> - return sprintf(buf, "%d\n", val);
> + return sprintf(buf, "%d\n", lis3lv02d_get_odr());
> }
>
> static DEVICE_ATTR(position, S_IRUGO, lis3lv02d_position_show, NULL);
> @@ -430,11 +441,13 @@ int lis3lv02d_init_device(struct lis3lv02d *dev)
> printk(KERN_INFO DRIVER_NAME ": 12 bits sensor found\n");
> dev->read_data = lis3lv02d_read_12;
> dev->mdps_max_val = 2048;
> + dev->pwron_delay = LIS3_PWRON_DELAY_WAI_12B;
> break;
> case WAI_8B:
> printk(KERN_INFO DRIVER_NAME ": 8 bits sensor found\n");
> dev->read_data = lis3lv02d_read_8;
> dev->mdps_max_val = 128;
> + dev->pwron_delay = LIS3_PWRON_DELAY_WAI_8B;
> break;
> default:
> printk(KERN_ERR DRIVER_NAME
> diff --git a/drivers/hwmon/lis3lv02d.h b/drivers/hwmon/lis3lv02d.h
> index c6ae507..c57f21f 100644
> --- a/drivers/hwmon/lis3lv02d.h
> +++ b/drivers/hwmon/lis3lv02d.h
> @@ -202,6 +202,7 @@ struct lis3lv02d {
> u8 whoami; /* indicates measurement precision */
> s16 (*read_data) (struct lis3lv02d *lis3, int reg);
> int mdps_max_val;
> + int pwron_delay;
>
> struct input_polled_dev *idev; /* input device */
> struct platform_device *pdev; /* platform device */

2009-11-15 15:54:18

by Éric Piel

[permalink] [raw]
Subject: Re: [PATCH v2 07/10] LIS3LV02D: Remove calibaration functionality

Op 10-11-09 13:41, Samu Onkalo schreef:
> Chip is calibrated by the manufacturer. There is no need to calibarate
> it at driver level. If the chip is used as a joystick, calibaration can
> be done using joystick device calibration mechanism.
>
> Signed-off-by: Samu Onkalo <[email protected]>
Acked-by: Éric Piel <[email protected]>
> ---
> drivers/hwmon/lis3lv02d.c | 32 +++-----------------------------
> drivers/hwmon/lis3lv02d.h | 3 ---
> 2 files changed, 3 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
> index 0335b36..5b39257 100644
> --- a/drivers/hwmon/lis3lv02d.c
> +++ b/drivers/hwmon/lis3lv02d.c
> @@ -344,19 +344,12 @@ static void lis3lv02d_joystick_poll(struct input_polled_dev *pidev)
> int x, y, z;
>
> lis3lv02d_get_xyz(&lis3_dev, &x, &y, &z);
> - input_report_abs(pidev->input, ABS_X, x - lis3_dev.xcalib);
> - input_report_abs(pidev->input, ABS_Y, y - lis3_dev.ycalib);
> - input_report_abs(pidev->input, ABS_Z, z - lis3_dev.zcalib);
> + input_report_abs(pidev->input, ABS_X, x);
> + input_report_abs(pidev->input, ABS_Y, y);
> + input_report_abs(pidev->input, ABS_Z, z);
> input_sync(pidev->input);
> }
>
> -
> -static inline void lis3lv02d_calibrate_joystick(void)
> -{
> - lis3lv02d_get_xyz(&lis3_dev,
> - &lis3_dev.xcalib, &lis3_dev.ycalib, &lis3_dev.zcalib);
> -}
> -
> int lis3lv02d_joystick_enable(void)
> {
> struct input_dev *input_dev;
> @@ -373,8 +366,6 @@ int lis3lv02d_joystick_enable(void)
> lis3_dev.idev->poll_interval = MDPS_POLL_INTERVAL;
> input_dev = lis3_dev.idev->input;
>
> - lis3lv02d_calibrate_joystick();
> -
> input_dev->name = "ST LIS3LV02DL Accelerometer";
> input_dev->phys = DRIVER_NAME "/input0";
> input_dev->id.bustype = BUS_HOST;
> @@ -430,20 +421,6 @@ static ssize_t lis3lv02d_position_show(struct device *dev,
> return sprintf(buf, "(%d,%d,%d)\n", x, y, z);
> }
>
> -static ssize_t lis3lv02d_calibrate_show(struct device *dev,
> - struct device_attribute *attr, char *buf)
> -{
> - return sprintf(buf, "(%d,%d,%d)\n", lis3_dev.xcalib, lis3_dev.ycalib, lis3_dev.zcalib);
> -}
> -
> -static ssize_t lis3lv02d_calibrate_store(struct device *dev,
> - struct device_attribute *attr,
> - const char *buf, size_t count)
> -{
> - lis3lv02d_calibrate_joystick();
> - return count;
> -}
> -
> static ssize_t lis3lv02d_rate_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> @@ -452,14 +429,11 @@ static ssize_t lis3lv02d_rate_show(struct device *dev,
>
> static DEVICE_ATTR(selftest, S_IRUGO, lis3lv02d_selftest_show, NULL);
> static DEVICE_ATTR(position, S_IRUGO, lis3lv02d_position_show, NULL);
> -static DEVICE_ATTR(calibrate, S_IRUGO|S_IWUSR, lis3lv02d_calibrate_show,
> - lis3lv02d_calibrate_store);
> static DEVICE_ATTR(rate, S_IRUGO, lis3lv02d_rate_show, NULL);
>
> static struct attribute *lis3lv02d_attributes[] = {
> &dev_attr_selftest.attr,
> &dev_attr_position.attr,
> - &dev_attr_calibrate.attr,
> &dev_attr_rate.attr,
> NULL
> };
> diff --git a/drivers/hwmon/lis3lv02d.h b/drivers/hwmon/lis3lv02d.h
> index 655875e..a692116 100644
> --- a/drivers/hwmon/lis3lv02d.h
> +++ b/drivers/hwmon/lis3lv02d.h
> @@ -216,9 +216,6 @@ struct lis3lv02d {
> struct input_polled_dev *idev; /* input device */
> 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 */
> struct axis_conversion ac; /* hw -> logical axis */
>
> u32 irq; /* IRQ number */

2009-11-15 16:01:37

by Éric Piel

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] LIS3: Feature updates and corrections

Op 10-11-09 13:41, Samu Onkalo schreef:
> This patch set has earlier been sent to lm-sensors list (thus patch v2).
> No changes after that except to patch 0006 to make it applicable to
> 2.6.32-rc6 (no functional changes).
:
>
> I have possibility to test these changes only with 8 bit version (lis302d).
> Since part of the changes are specific to 12 bit device, I would appreciate
> if someone could try these on 12 bit device.
I've just tested them on a 12-bit device and it seems to go fine
(excepted the rate setting in patch 8).

Andrew, maybe you could already queue the first 5 patches, as they all
look fine and are fixing bugs. So we are sure they are there for 2.6.33
:-) The rest of the patch series is useful but just "new feature" and
some need more work.

Thanks,
Eric


>
> 0001:
> Sync event is sent after each measurement. This helps user space
> applications to detect which 3 reported values belongs to
> one atomic measurement.
>
> 0002:
> polled input device was not totally released in module unload.
> This corrects memory leak.
>
> 0003:
> This patch is originally from Eric Piel.
> Updates documentation and chip ID handling
>
> 0004:
> This patch is originally from Eric Piel.
> Correction to sampling rate handling for 8 bit devices
>
> 0005:
> Chip power on sequence was not waiting for chip to be ready.
> This caused incorrect values right after the power on.

2009-11-15 18:31:20

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH v2 09/10] lis3: Scale output values to mg

> Op 10-11-09 13:41, Samu Onkalo schreef:
> > Report output values as 1/1000 of earth gravity.
> After the discussion started by Daniel, I'm willing to accept this patch
> "as is", just with a better log explaining why it's ok to break
> userspace ABI:
> * the sysfs "position" attribute was never safe to be read as it could
> have different scales depending on the underlying hardware. It's now
> featuring a consistent scale, which is more or less identical to the one
> used by the 12-bits sensors
> * the joystick interface reports the same values as before, excepted of
> course if "raw" values are requested, in which case the same remark as
> for "position" applies.

agreed.

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

2009-11-16 20:04:35

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] LIS3: Feature updates and corrections

On Sun, 15 Nov 2009 17:01:34 +0100
__ric Piel <[email protected]> wrote:

> Op 10-11-09 13:41, Samu Onkalo schreef:
> > This patch set has earlier been sent to lm-sensors list (thus patch v2).
> > No changes after that except to patch 0006 to make it applicable to
> > 2.6.32-rc6 (no functional changes).
> :
> >
> > I have possibility to test these changes only with 8 bit version (lis302d).
> > Since part of the changes are specific to 12 bit device, I would appreciate
> > if someone could try these on 12 bit device.
> I've just tested them on a 12-bit device and it seems to go fine
> (excepted the rate setting in patch 8).
>
> Andrew, maybe you could already queue the first 5 patches, as they all
> look fine and are fixing bugs. So we are sure they are there for 2.6.33
> :-) The rest of the patch series is useful but just "new feature" and
> some need more work.

OK, I merged the five.

We could sneak some/all of them into 2.6.32 I guess, if you think
that's warranted?

The ones which were From:yourself were missing Samu's Signed-off-by:.
I added it.

"[PATCH v2 04/10] lis3: fix show rate for 8 bits chips" has a poor
changelog. "fix with 8 bits sensors". It failed to tell us what the
bug was, what its user-visible effects were, how it was fixed.

2009-11-16 20:37:27

by Éric Piel

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] LIS3: Feature updates and corrections

Op 16-11-09 21:03, Andrew Morton schreef:
> On Sun, 15 Nov 2009 17:01:34 +0100
> __ric Piel<[email protected]> wrote:
:
>> Andrew, maybe you could already queue the first 5 patches, as they all
>> look fine and are fixing bugs. So we are sure they are there for 2.6.33
>> :-) The rest of the patch series is useful but just "new feature" and
>> some need more work.
>
> OK, I merged the five.
>
> We could sneak some/all of them into 2.6.32 I guess, if you think
> that's warranted?
Well, if it's not too late, it would be worthy, as they are fixing bugs,
some of them actually affecting users. At least the first 4 should be
very safe.

> The ones which were From:yourself were missing Samu's Signed-off-by:.
> I added it.
Thanks
>
> "[PATCH v2 04/10] lis3: fix show rate for 8 bits chips" has a poor
> changelog. "fix with 8 bits sensors". It failed to tell us what the
> bug was, what its user-visible effects were, how it was fixed.
Yes, I actually resent it with a better changelog yesterday:
http://lkml.org/lkml/2009/11/15/91

I should have highlighted it in my previous email. Could you pick this
one instead?

Thanks,
Eric

2009-11-17 06:06:39

by Samu Onkalo

[permalink] [raw]
Subject: RE: [PATCH v2 00/10] LIS3: Feature updates and corrections

>I've just tested them on a 12-bit device and it seems to go
>fine (excepted the rate setting in patch 8).
>
>Andrew, maybe you could already queue the first 5 patches, as
>they all look fine and are fixing bugs. So we are sure they
>are there for 2.6.33
>:-) The rest of the patch series is useful but just "new
>feature" and some need more work.
>

Eric, thanks for the review and comments. I'll make proposed corrections
to the remaining patches.

-Samu