[PATCH v3 0/5] lis3: Feature updates

[PATCH v3 0/5] lis3: Feature updates

[Samu Onkalo]

This patch set is made to top of 7 previously accepted lis3 related patches in
mm-tree:
- I2C bus support for lis3lv02d and variant accelerometer chips...
- Add the possibility to remap axes via platform data...
- Send input_sync after each measurement round...
- polled input device itself was not free'd....
- riginally the driver was only targeted to 12bits sensors...
- Most of the documentation and comments were written when the driver ..
- Lis3 accelerometer sensors have quite long power on delay ..

Tested with 2.6.32-RC7 which was patched with above 7 lis3 related patches.
Tested only with 8 bit lis3 device since I don't have possibility to test
other chips.

0001:
Implement selftest feature for HW diagnostic purposes. This feature can be used
to detect if the sensor is electrically or mechanically damaged.

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

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

0004:
Scale output values to mg (1/1000 of earth gravity).
Behaviour of the joystick interface is not changed. All other interfaces
will have different scale after this patch. See further information from
patch itself.

0005:
Update documentation to cover changes which were made by these patches.


Samu Onkalo (5):
  lis3: 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

 Documentation/hwmon/lis3lv02d |   31 ++++++--
 drivers/hwmon/lis3lv02d.c     |  180 ++++++++++++++++++++++++++++++++---------
 drivers/hwmon/lis3lv02d.h     |   25 +++++-
 include/linux/lis3lv02d.h     |    3 +
 4 files changed, 190 insertions(+), 49 deletions(-)

[PATCH v3 1/5] lis3: Selftest support

[Samu Onkalo]

Implement selftest feature as specified by chip manufacturer.
Control: read selftest sysfs entry
Response: "OK x y z" or "FAIL x y z"
where x, y, and z are difference between selftest mode and normal mode.
Test is passed when values are within acceptance limit values.

Acceptance limits are provided via platform data. See chip spesifications
for acceptance limits. If limits are not properly set, OK / FAIL decision is
meaningless. However, userspace application can still make decision based on
the numeric x, y, z values.

Selftest is meant for HW diagnostic purposes. It is not meant to be called
during normal use of the chip. It may cause false interrupt events.
Selftest mode delays polling of the normal results but it doesn't cause
wrong values. Chip must be in static state during selftest.
Any acceration during the test causes most probably failure.

Signed-off-by: Samu Onkalo <samu.p.onkalo@nokia.com>
---
 drivers/hwmon/lis3lv02d.c |   72 ++++++++++++++++++++++++++++++++++++++++++++-
 drivers/hwmon/lis3lv02d.h |   14 +++++++-
 include/linux/lis3lv02d.h |    3 ++
 3 files changed, 86 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
index 39b9ac8..69314f9 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -106,9 +106,11 @@ static void lis3lv02d_get_xyz(struct lis3lv02d *lis3, int *x, int *y, int *z)
 {
 	int position[3];
 
+	mutex_lock(&lis3->mutex);
 	position[0] = lis3->read_data(lis3, OUTX);
 	position[1] = lis3->read_data(lis3, OUTY);
 	position[2] = lis3->read_data(lis3, OUTZ);
+	mutex_unlock(&lis3->mutex);
 
 	*x = lis3lv02d_get_axis(lis3->ac.x, position);
 	*y = lis3lv02d_get_axis(lis3->ac.y, position);
@@ -133,6 +135,60 @@ 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;
+	int ret;
+
+	mutex_lock(&lis3->mutex);
+	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);
+
+	ret = 0;
+	if (lis3->pdata) {
+		int i;
+		for (i = 0; i < 3; i++) {
+			/* Check minimum acceptance limits */
+			if (results[i] < lis3->pdata->st_min_limits[i]) {
+				ret = -EIO;
+				goto fail;
+			}
+
+			/* Check maximum acceptance limits */
+			if (results[i] > lis3->pdata->st_max_limits[i]) {
+				ret = -EIO;
+				goto fail;
+			}
+		}
+	}
+
+	/* test passed */
+fail:
+	mutex_unlock(&lis3->mutex);
+	return ret;
+}
+
 void lis3lv02d_poweroff(struct lis3lv02d *lis3)
 {
 	/* disable X,Y,Z axis and power down */
@@ -365,6 +421,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 +461,14 @@ static ssize_t lis3lv02d_rate_show(struct device *dev,
 	return sprintf(buf, "%d\n", lis3lv02d_get_odr());
 }
 
+static DEVICE_ATTR(selftest, S_IRUSR, 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,
@@ -455,6 +524,8 @@ int lis3lv02d_init_device(struct lis3lv02d *dev)
 		return -EINVAL;
 	}
 
+	mutex_init(&dev->mutex);
+
 	lis3lv02d_add_fs(dev);
 	lis3lv02d_poweron(dev);
 
@@ -507,4 +578,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..166794c 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,
@@ -218,6 +227,7 @@ struct lis3lv02d {
 	unsigned long		misc_opened; /* bit0: whether the device is open */
 
 	struct lis3lv02d_platform_data *pdata;	/* for passing board config */
+	struct mutex		mutex;     /* Serialize poll and selftest */
 };
 
 int lis3lv02d_init_device(struct lis3lv02d *lis3);
diff --git a/include/linux/lis3lv02d.h b/include/linux/lis3lv02d.h
index 8970135..f1ca0dc 100644
--- a/include/linux/lis3lv02d.h
+++ b/include/linux/lis3lv02d.h
@@ -55,6 +55,9 @@ struct lis3lv02d_platform_data {
 	s8 axis_z;
 	int (*setup_resources)(void);
 	int (*release_resources)(void);
+	/* Limits for selftest are specified in chip data sheet */
+	s16 st_min_limits[3]; /* min pass limit x, y, z */
+	s16 st_max_limits[3]; /* max pass limit x, y, z */
 };
 
 #endif /* __LIS3LV02D_H_ */
-- 
1.6.0.4

[PATCH v3 2/5] LIS3LV02D: Remove calibaration functionality

[Samu Onkalo]

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 <samu.p.onkalo@nokia.com>
---
 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 69314f9..a953a54 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -355,19 +355,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;
@@ -384,8 +377,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;
@@ -441,20 +432,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)
 {
@@ -463,14 +440,11 @@ static ssize_t lis3lv02d_rate_show(struct device *dev,
 
 static DEVICE_ATTR(selftest, S_IRUSR, 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 166794c..1e9fb03 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.6.0.4

[PATCH v3 3/5] lis3: Sysfs entry for setting chip measurement rate

[Samu Onkalo]

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 <samu.p.onkalo@nokia.com>
---
 drivers/hwmon/lis3lv02d.c |   51 ++++++++++++++++++++++++++++++++++++++------
 drivers/hwmon/lis3lv02d.h |    4 +++
 2 files changed, 48 insertions(+), 7 deletions(-)

diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
index a953a54..3c781f2 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -121,18 +121,31 @@ static void lis3lv02d_get_xyz(struct lis3lv02d *lis3, int *x, int *y, int *z)
 static int lis3_12_rates[4] = {40, 160, 640, 2560};
 static int lis3_8_rates[2] = {100, 400};
 
+/* ODR is Output Data Rate */
 static int lis3lv02d_get_odr(void)
 {
 	u8 ctrl;
-	int val;
 
 	lis3_dev.read(&lis3_dev, CTRL_REG1, &ctrl);
+	ctrl &= lis3_dev.odr_mask;
+	return lis3_dev.odrs[(ctrl >> lis3_dev.odr_shift)];
+}
 
-	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;
+static int lis3lv02d_set_odr(int rate)
+{
+	u8 ctrl;
+	int i;
+
+	lis3_dev.read(&lis3_dev, CTRL_REG1, &ctrl);
+	ctrl &= ~lis3_dev.odr_mask;
+
+	for (i = 0; i < lis3_dev.odr_len; i++)
+		if (lis3_dev.odrs[i] == rate) {
+			lis3_dev.write(&lis3_dev,
+				CTRL_REG1, ctrl | (i << lis3_dev.odr_shift));
+			return 0;
+		}
+	return -EINVAL;
 }
 
 static int lis3lv02d_selftest(struct lis3lv02d *lis3, s16 results[3])
@@ -438,9 +451,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_IRUSR, 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,
@@ -485,12 +514,20 @@ 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->odrs = lis3_12_rates;
+		dev->odr_len = ARRAY_SIZE(lis3_12_rates);
+		dev->odr_mask = CTRL1_DF0 | CTRL1_DF1;
+		dev->odr_shift = 4;
 		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->odrs = lis3_8_rates;
+		dev->odr_len = ARRAY_SIZE(lis3_8_rates);
+		dev->odr_mask = CTRL1_DR;
+		dev->odr_shift = 7;
 		break;
 	default:
 		printk(KERN_ERR DRIVER_NAME
diff --git a/drivers/hwmon/lis3lv02d.h b/drivers/hwmon/lis3lv02d.h
index 1e9fb03..d7141f4 100644
--- a/drivers/hwmon/lis3lv02d.h
+++ b/drivers/hwmon/lis3lv02d.h
@@ -208,6 +208,10 @@ struct lis3lv02d {
 	int (*write) (struct lis3lv02d *lis3, int reg, u8 val);
 	int (*read) (struct lis3lv02d *lis3, int reg, u8 *ret);
 
+	int                     *odrs;     /* Supported output data rates */
+	u8                      odr_len;   /* Length of the table above */
+	u8                      odr_shift; /* ODR bit(s) location */
+	u8                      odr_mask;  /* ODR bit mask */
 	u8			whoami;    /* indicates measurement precision */
 	s16 (*read_data) (struct lis3lv02d *lis3, int reg);
 	int			mdps_max_val;
-- 
1.6.0.4

[PATCH v3 4/5] lis3: Scale output values to mg

[Samu Onkalo]

Report output values as 1/1000 of earth gravity.

Output values from lis3 can be read from sysfs position entry and
from input device. Input device can be accessed as event device
and as joystick device. Joystick device can be in two modes. Meaning of the
output values varies from case to case depending on the chip type
and configuration (scale). Only joystick interface in JS_CORR_BROKEN mode
returned somehow similar output values in different configurations.
Joystick device is in that state by default in case of lis3.

Position sysfs entry, input event device and raw joystick device
have been little bit broken since meaning of the output values has been
varied between 12 and 8 bit devices. Applications which relayed on those
methods failed if the chip is different than the expected one.

This patch converts output values to mean similar thing in different
configurations. Both 8 and 12 bit devices reports now same acceleration values.
If somebody implements full scale support to the driver, output values will
still mean the same. Scaling factor and input device range must be updated
in that case.

Joystick interface in JS_CORR_BROKEN mode is not touched by this patch.
All other interfaces have different scale after this change.
For 12 bit device scaling factor is 0.977 which keeps scaled and unscaled
values are quite close to each others.
For 8 bit device, scaled values are 18 times bigger than unscaled values.

Signed-off-by: Samu Onkalo <samu.p.onkalo@nokia.com>
---
 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 3c781f2..480a397 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,6 +118,7 @@ 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;
 
 	mutex_lock(&lis3->mutex);
 	position[0] = lis3->read_data(lis3, OUTX);
@@ -112,6 +126,10 @@ static void lis3lv02d_get_xyz(struct lis3lv02d *lis3, int *x, int *y, int *z)
 	position[2] = lis3->read_data(lis3, OUTZ);
 	mutex_unlock(&lis3->mutex);
 
+	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);
@@ -378,6 +396,7 @@ int lis3lv02d_joystick_enable(void)
 {
 	struct input_dev *input_dev;
 	int err;
+	int max_val, fuzz, flat;
 
 	if (lis3_dev.idev)
 		return -EINVAL;
@@ -397,9 +416,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) {
@@ -518,6 +541,7 @@ int lis3lv02d_init_device(struct lis3lv02d *dev)
 		dev->odr_len = ARRAY_SIZE(lis3_12_rates);
 		dev->odr_mask = CTRL1_DF0 | CTRL1_DF1;
 		dev->odr_shift = 4;
+		dev->scale = LIS3_2G2G_SENSITIVITY_WAI_12B;
 		break;
 	case WAI_8B:
 		printk(KERN_INFO DRIVER_NAME ": 8 bits sensor found\n");
@@ -528,6 +552,7 @@ int lis3lv02d_init_device(struct lis3lv02d *dev)
 		dev->odr_len = ARRAY_SIZE(lis3_8_rates);
 		dev->odr_mask = CTRL1_DR;
 		dev->odr_shift = 7;
+		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 d7141f4..e0bcb77 100644
--- a/drivers/hwmon/lis3lv02d.h
+++ b/drivers/hwmon/lis3lv02d.h
@@ -216,6 +216,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.6.0.4

[PATCH v3 5/5] LIS3: Update documentation to match latest changes

[Samu Onkalo]

Signed-off-by: Samu Onkalo <samu.p.onkalo@nokia.com>
---
 Documentation/hwmon/lis3lv02d |   31 ++++++++++++++++++++++++-------
 1 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/Documentation/hwmon/lis3lv02d b/Documentation/hwmon/lis3lv02d
index 21f0902..cd354e3 100644
--- a/Documentation/hwmon/lis3lv02d
+++ b/Documentation/hwmon/lis3lv02d
@@ -20,18 +20,35 @@ 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.
 
 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. Joystick device can be in two different modes.
+By default output values are scaled between -32768 .. 32767. In joystick raw
+mode, joystick and sysfs position entry have the same scale. There can be
+small difference due to input system fuzziness feature.
+Events are also available as input event device.
+
+Selftest is meant only for hardware diagnostic purposes. It is not meant to be
+used during normal operations. Position data is not corrupted during selftest
+but interrupt behaviour is not quaranteed to work reliably. In test mode, the
+sensing element is internally moved little bit. Selftest measures difference
+between normal mode and test mode. Chip specifications tell the acceptance
+limit for each type of the chip. Limits are provided via platform data
+to allow adjustment of the limits without a change to the actual driver.
+Seltest returns either "OK x y z" or "FAIL x y z" where x, y and z are
+measured difference between modes. Axes are not remapped in selftest mode.
+Measurement values are provided to help HW diagnostic applications to make
+final decision.
 
 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.6.0.4

Re: [PATCH v3 5/5] LIS3: Update documentation to match latest changes

[Éric Piel]

Op 19-11-09 10:27, Samu Onkalo schreef:
Acked-by: Éric Piel <eric.piel@tremplin-utc.net>

> Signed-off-by: Samu Onkalo <samu.p.onkalo@nokia.com>
> ---
>  Documentation/hwmon/lis3lv02d |   31 ++++++++++++++++++++++++-------
>  1 files changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/hwmon/lis3lv02d b/Documentation/hwmon/lis3lv02d
> index 21f0902..cd354e3 100644
> --- a/Documentation/hwmon/lis3lv02d
> +++ b/Documentation/hwmon/lis3lv02d
> @@ -20,18 +20,35 @@ 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.
>  
>  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. Joystick device can be in two different modes.
> +By default output values are scaled between -32768 .. 32767. In joystick raw
> +mode, joystick and sysfs position entry have the same scale. There can be
> +small difference due to input system fuzziness feature.
> +Events are also available as input event device.
> +
> +Selftest is meant only for hardware diagnostic purposes. It is not meant to be
> +used during normal operations. Position data is not corrupted during selftest
> +but interrupt behaviour is not quaranteed to work reliably. In test mode, the
> +sensing element is internally moved little bit. Selftest measures difference
> +between normal mode and test mode. Chip specifications tell the acceptance
> +limit for each type of the chip. Limits are provided via platform data
> +to allow adjustment of the limits without a change to the actual driver.
> +Seltest returns either "OK x y z" or "FAIL x y z" where x, y and z are
> +measured difference between modes. Axes are not remapped in selftest mode.
> +Measurement values are provided to help HW diagnostic applications to make
> +final decision.
>  
>  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.

Re: [PATCH v3 4/5] lis3: Scale output values to mg

[Éric Piel]

Op 19-11-09 10:27, Samu Onkalo schreef:
> Report output values as 1/1000 of earth gravity.

:
Looks good, excepted that you have some constants with a very long name.

:
> +#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)
As you'll have to rebase it anyway for the path 3, could you change
those constants to have a shorter name? At least "WAI" could go away.
And what does 2G2G stands for?

Eric

Re: [PATCH v3 3/5] lis3: Sysfs entry for setting chip measurement rate

[Éric Piel]

Op 19-11-09 10:27, 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.
> 
Hi, it's only a slow path, so I suggest to use less variables and
compute them on the fly. Here is a new version of the patch.

Eric

8<--------------------------------------------------------
From: Samu Onkalo <samu.p.onkalo@nokia.com>
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 <samu.p.onkalo@nokia.com>
Signed-off-by: Éric Piel <eric.piel@tremplin-utc.net>
---
 drivers/hwmon/lis3lv02d.c |   50 ++++++++++++++++++++++++++++++++++++++------
 drivers/hwmon/lis3lv02d.h |    2 +
 2 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
index a953a54..61c1973 100644
--- a/drivers/hwmon/lis3lv02d.c
+++ b/drivers/hwmon/lis3lv02d.c
@@ -121,18 +121,34 @@ static void lis3lv02d_get_xyz(struct lis3lv02d *lis3, int *x, int *y, int *z)
 static int lis3_12_rates[4] = {40, 160, 640, 2560};
 static int lis3_8_rates[2] = {100, 400};
 
+/* ODR is Output Data Rate */
 static int lis3lv02d_get_odr(void)
 {
 	u8 ctrl;
-	int val;
+	int shift;
 
 	lis3_dev.read(&lis3_dev, CTRL_REG1, &ctrl);
+	ctrl &= lis3_dev.odr_mask;
+	shift = ffs(lis3_dev.odr_mask) - 1;
+	return lis3_dev.odrs[(ctrl >> shift)];
+}
 
-	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;
+static int lis3lv02d_set_odr(int rate)
+{
+	u8 ctrl;
+	int i, len, shift;
+
+	lis3_dev.read(&lis3_dev, CTRL_REG1, &ctrl);
+	ctrl &= ~lis3_dev.odr_mask;
+	len = 1 << hweight_long(lis3_dev.odr_mask); /* # of possible values */
+	shift = ffs(lis3_dev.odr_mask) - 1;
+
+	for (i = 0; i < len; i++)
+		if (lis3_dev.odrs[i] == rate) {
+			lis3_dev.write(&lis3_dev, CTRL_REG1, ctrl | (i << shift));
+			return 0;
+		}
+	return -EINVAL;
 }
 
 static int lis3lv02d_selftest(struct lis3lv02d *lis3, s16 results[3])
@@ -438,9 +454,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_IRUSR, 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,
@@ -485,12 +517,16 @@ 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->odrs = lis3_12_rates;
+		dev->odr_mask = CTRL1_DF0 | CTRL1_DF1;
 		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->odrs = lis3_8_rates;
+		dev->odr_mask = CTRL1_DR;
 		break;
 	default:
 		printk(KERN_ERR DRIVER_NAME
diff --git a/drivers/hwmon/lis3lv02d.h b/drivers/hwmon/lis3lv02d.h
index 1e9fb03..f73c786 100644
--- a/drivers/hwmon/lis3lv02d.h
+++ b/drivers/hwmon/lis3lv02d.h
@@ -208,6 +208,8 @@ struct lis3lv02d {
 	int (*write) (struct lis3lv02d *lis3, int reg, u8 val);
 	int (*read) (struct lis3lv02d *lis3, int reg, u8 *ret);
 
+	int                     *odrs;     /* Supported output data rates */
+	u8                      odr_mask;  /* ODR bit mask */
 	u8			whoami;    /* indicates measurement precision */
 	s16 (*read_data) (struct lis3lv02d *lis3, int reg);
 	int			mdps_max_val;
-- 
1.6.5.3

Re: [PATCH v3 2/5] LIS3LV02D: Remove calibaration functionality

[Éric Piel]

Op 19-11-09 10:27, 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.
> 

Acked-by: Éric Piel <eric.piel@tremplin-utc.net>
> Signed-off-by: Samu Onkalo <samu.p.onkalo@nokia.com>
> ---
>  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 69314f9..a953a54 100644
> --- a/drivers/hwmon/lis3lv02d.c
> +++ b/drivers/hwmon/lis3lv02d.c
> @@ -355,19 +355,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;
> @@ -384,8 +377,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;
> @@ -441,20 +432,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)
>  {
> @@ -463,14 +440,11 @@ static ssize_t lis3lv02d_rate_show(struct device *dev,
>  
>  static DEVICE_ATTR(selftest, S_IRUSR, 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 166794c..1e9fb03 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 */

Re: [PATCH v3 1/5] lis3: Selftest support

[Éric Piel]

Op 19-11-09 10:27, Samu Onkalo schreef:
> Implement selftest feature as specified by chip manufacturer.
> Control: read selftest sysfs entry
> Response: "OK x y z" or "FAIL x y z"
> where x, y, and z are difference between selftest mode and normal mode.
> Test is passed when values are within acceptance limit values.
> 
> Acceptance limits are provided via platform data. See chip spesifications
> for acceptance limits. If limits are not properly set, OK / FAIL decision is
> meaningless. However, userspace application can still make decision based on
> the numeric x, y, z values.
> 
> Selftest is meant for HW diagnostic purposes. It is not meant to be called
> during normal use of the chip. It may cause false interrupt events.
> Selftest mode delays polling of the normal results but it doesn't cause
> wrong values. Chip must be in static state during selftest.
> Any acceration during the test causes most probably failure.
This patch looks good to me, but please add the info of this changelog
into the documentation (in patch 4). If users have to start digging into
the git changelog to find out what does a feature, we are nasty ;-)

Signed-off-by: Éric Piel <eric.piel@tremplin-utc.net>

> 
> Signed-off-by: Samu Onkalo <samu.p.onkalo@nokia.com>
> ---
>  drivers/hwmon/lis3lv02d.c |   72 ++++++++++++++++++++++++++++++++++++++++++++-
>  drivers/hwmon/lis3lv02d.h |   14 +++++++-
>  include/linux/lis3lv02d.h |    3 ++
>  3 files changed, 86 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
> index 39b9ac8..69314f9 100644
> --- a/drivers/hwmon/lis3lv02d.c
> +++ b/drivers/hwmon/lis3lv02d.c
> @@ -106,9 +106,11 @@ static void lis3lv02d_get_xyz(struct lis3lv02d *lis3, int *x, int *y, int *z)
>  {
>  	int position[3];
>  
> +	mutex_lock(&lis3->mutex);
>  	position[0] = lis3->read_data(lis3, OUTX);
>  	position[1] = lis3->read_data(lis3, OUTY);
>  	position[2] = lis3->read_data(lis3, OUTZ);
> +	mutex_unlock(&lis3->mutex);
>  
>  	*x = lis3lv02d_get_axis(lis3->ac.x, position);
>  	*y = lis3lv02d_get_axis(lis3->ac.y, position);
> @@ -133,6 +135,60 @@ 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;
> +	int ret;
> +
> +	mutex_lock(&lis3->mutex);
> +	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);
> +
> +	ret = 0;
> +	if (lis3->pdata) {
> +		int i;
> +		for (i = 0; i < 3; i++) {
> +			/* Check minimum acceptance limits */
> +			if (results[i] < lis3->pdata->st_min_limits[i]) {
> +				ret = -EIO;
> +				goto fail;
> +			}
> +
> +			/* Check maximum acceptance limits */
> +			if (results[i] > lis3->pdata->st_max_limits[i]) {
> +				ret = -EIO;
> +				goto fail;
> +			}
> +		}
> +	}
> +
> +	/* test passed */
> +fail:
> +	mutex_unlock(&lis3->mutex);
> +	return ret;
> +}
> +
>  void lis3lv02d_poweroff(struct lis3lv02d *lis3)
>  {
>  	/* disable X,Y,Z axis and power down */
> @@ -365,6 +421,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 +461,14 @@ static ssize_t lis3lv02d_rate_show(struct device *dev,
>  	return sprintf(buf, "%d\n", lis3lv02d_get_odr());
>  }
>  
> +static DEVICE_ATTR(selftest, S_IRUSR, 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,
> @@ -455,6 +524,8 @@ int lis3lv02d_init_device(struct lis3lv02d *dev)
>  		return -EINVAL;
>  	}
>  
> +	mutex_init(&dev->mutex);
> +
>  	lis3lv02d_add_fs(dev);
>  	lis3lv02d_poweron(dev);
>  
> @@ -507,4 +578,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..166794c 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,
> @@ -218,6 +227,7 @@ struct lis3lv02d {
>  	unsigned long		misc_opened; /* bit0: whether the device is open */
>  
>  	struct lis3lv02d_platform_data *pdata;	/* for passing board config */
> +	struct mutex		mutex;     /* Serialize poll and selftest */
>  };
>  
>  int lis3lv02d_init_device(struct lis3lv02d *lis3);
> diff --git a/include/linux/lis3lv02d.h b/include/linux/lis3lv02d.h
> index 8970135..f1ca0dc 100644
> --- a/include/linux/lis3lv02d.h
> +++ b/include/linux/lis3lv02d.h
> @@ -55,6 +55,9 @@ struct lis3lv02d_platform_data {
>  	s8 axis_z;
>  	int (*setup_resources)(void);
>  	int (*release_resources)(void);
> +	/* Limits for selftest are specified in chip data sheet */
> +	s16 st_min_limits[3]; /* min pass limit x, y, z */
> +	s16 st_max_limits[3]; /* max pass limit x, y, z */
>  };
>  
>  #endif /* __LIS3LV02D_H_ */

Re: [PATCH v3 1/5] lis3: Selftest support

[Éric Piel]

Op 30-11-09 15:56, Éric Piel schreef:
> Op 19-11-09 10:27, Samu Onkalo schreef:
>> Implement selftest feature as specified by chip manufacturer.
>> Control: read selftest sysfs entry
>> Response: "OK x y z" or "FAIL x y z"
>> where x, y, and z are difference between selftest mode and normal mode.
>> Test is passed when values are within acceptance limit values.
>>
>> Acceptance limits are provided via platform data. See chip spesifications
>> for acceptance limits. If limits are not properly set, OK / FAIL decision is
>> meaningless. However, userspace application can still make decision based on
>> the numeric x, y, z values.
>>
>> Selftest is meant for HW diagnostic purposes. It is not meant to be called
>> during normal use of the chip. It may cause false interrupt events.
>> Selftest mode delays polling of the normal results but it doesn't cause
>> wrong values. Chip must be in static state during selftest.
>> Any acceration during the test causes most probably failure.
> This patch looks good to me, but please add the info of this changelog
> into the documentation (in patch 4). If users have to start digging into
> the git changelog to find out what does a feature, we are nasty ;-)
Oops, I hadn't noticed your modification in patch 5, sorry! It's good...
but now I noticed a little enhancement you could do in this patch :-) .
See below...

> 
> Signed-off-by: Éric Piel <eric.piel@tremplin-utc.net>
> 
>> Signed-off-by: Samu Onkalo <samu.p.onkalo@nokia.com>
>> ---
>>  drivers/hwmon/lis3lv02d.c |   72 ++++++++++++++++++++++++++++++++++++++++++++-
>>  drivers/hwmon/lis3lv02d.h |   14 +++++++-
>>  include/linux/lis3lv02d.h |    3 ++
>>  3 files changed, 86 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/hwmon/lis3lv02d.c b/drivers/hwmon/lis3lv02d.c
>> index 39b9ac8..69314f9 100644
>> --- a/drivers/hwmon/lis3lv02d.c
>> +++ b/drivers/hwmon/lis3lv02d.c
>> @@ -106,9 +106,11 @@ static void lis3lv02d_get_xyz(struct lis3lv02d *lis3, int *x, int *y, int *z)
>>  {
>>  	int position[3];
>>  
>> +	mutex_lock(&lis3->mutex);
>>  	position[0] = lis3->read_data(lis3, OUTX);
>>  	position[1] = lis3->read_data(lis3, OUTY);
>>  	position[2] = lis3->read_data(lis3, OUTZ);
>> +	mutex_unlock(&lis3->mutex);
>>  
>>  	*x = lis3lv02d_get_axis(lis3->ac.x, position);
>>  	*y = lis3lv02d_get_axis(lis3->ac.y, position);
>> @@ -133,6 +135,60 @@ 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;
>> +	int ret;
>> +
>> +	mutex_lock(&lis3->mutex);
>> +	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);
>> +
>> +	ret = 0;
>> +	if (lis3->pdata) {
>> +		int i;
>> +		for (i = 0; i < 3; i++) {
>> +			/* Check minimum acceptance limits */
>> +			if (results[i] < lis3->pdata->st_min_limits[i]) {
>> +				ret = -EIO;
>> +				goto fail;
>> +			}
>> +
>> +			/* Check maximum acceptance limits */
>> +			if (results[i] > lis3->pdata->st_max_limits[i]) {
>> +				ret = -EIO;
>> +				goto fail;
>> +			}
>> +		}
>> +	}
>> +
Please merge these two if's to something like:
if ((results[i] < lis3->pdata->st_min_limits[i]) ||
    (results[i] > lis3->pdata->st_max_limits[i]) {




>> +	/* test passed */
>> +fail:
>> +	mutex_unlock(&lis3->mutex);
>> +	return ret;
>> +}
>> +
>>  void lis3lv02d_poweroff(struct lis3lv02d *lis3)
>>  {
>>  	/* disable X,Y,Z axis and power down */
>> @@ -365,6 +421,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 +461,14 @@ static ssize_t lis3lv02d_rate_show(struct device *dev,
>>  	return sprintf(buf, "%d\n", lis3lv02d_get_odr());
>>  }
>>  
>> +static DEVICE_ATTR(selftest, S_IRUSR, 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,
>> @@ -455,6 +524,8 @@ int lis3lv02d_init_device(struct lis3lv02d *dev)
>>  		return -EINVAL;
>>  	}
>>  
>> +	mutex_init(&dev->mutex);
>> +
>>  	lis3lv02d_add_fs(dev);
>>  	lis3lv02d_poweron(dev);
>>  
>> @@ -507,4 +578,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..166794c 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,
>> @@ -218,6 +227,7 @@ struct lis3lv02d {
>>  	unsigned long		misc_opened; /* bit0: whether the device is open */
>>  
>>  	struct lis3lv02d_platform_data *pdata;	/* for passing board config */
>> +	struct mutex		mutex;     /* Serialize poll and selftest */
>>  };
>>  
>>  int lis3lv02d_init_device(struct lis3lv02d *lis3);
>> diff --git a/include/linux/lis3lv02d.h b/include/linux/lis3lv02d.h
>> index 8970135..f1ca0dc 100644
>> --- a/include/linux/lis3lv02d.h
>> +++ b/include/linux/lis3lv02d.h
>> @@ -55,6 +55,9 @@ struct lis3lv02d_platform_data {
>>  	s8 axis_z;
>>  	int (*setup_resources)(void);
>>  	int (*release_resources)(void);
>> +	/* Limits for selftest are specified in chip data sheet */
>> +	s16 st_min_limits[3]; /* min pass limit x, y, z */
>> +	s16 st_max_limits[3]; /* max pass limit x, y, z */
>>  };
>>  
>>  #endif /* __LIS3LV02D_H_ */
> 
>