[RFC] iio: accel: st_accel: Add lis3l02dq support

[RFC] iio: accel: st_accel: Add lis3l02dq support

[Jonathan Cameron]

Time to finally kill off the venerable (it was one of my first drivers)
lis3l02dq driver in favour of adding support in the st sensors framework.

This does loose us the event support that driver always had, but I think
that will reappear at some point and in the meantime the maintenance
advantages of dropping the 'special' driver for this one part outweigh
the issues.

It's worth noting this part is ancient and I may well be the only person
who still has any on hardware running recent kernels.

It has a few 'quirks'.
 - No WAI register so that just became optional.
 - A BDU option that really does block updates.  Completely.
   Whatever you do, you don't get any more data with it set.
   It is documented the same as more modern parts but I presume they
   are actually clearing for updates after a read of both bytes!
 - Fixed scale.
 - It's too quick.  Even at slowest rate (280Hz) I can't read out fast
   enough on my board (stargate 2) to beat new data coming in. Linus'
   repeat read patch doesn't help in this case. It just means I get 10
   readings before dying... So in reality this will get used with
   software triggers only unless someone has this long out of production
   device on a quick board.

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
Cc: Denis CIOCCA <denis.ciocca@st.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Crestez Dan Leonard <leonard.crestez@intel.com>
---
 drivers/iio/accel/st_accel.h                    |  1 +
 drivers/iio/accel/st_accel_core.c               | 64 +++++++++++++++++++++++++
 drivers/iio/accel/st_accel_i2c.c                |  4 ++
 drivers/iio/accel/st_accel_spi.c                |  1 +
 drivers/iio/common/st_sensors/st_sensors_core.c | 26 +++++-----
 5 files changed, 85 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/accel/st_accel.h b/drivers/iio/accel/st_accel.h
index 57f83a67948c..f8dfdb690563 100644
--- a/drivers/iio/accel/st_accel.h
+++ b/drivers/iio/accel/st_accel.h
@@ -29,6 +29,7 @@
 #define LSM330_ACCEL_DEV_NAME		"lsm330_accel"
 #define LSM303AGR_ACCEL_DEV_NAME	"lsm303agr_accel"
 #define LIS2DH12_ACCEL_DEV_NAME		"lis2dh12_accel"
+#define LIS3L02DQ_ACCEL_DEV_NAME	"lis3l02dq"
 
 /**
 * struct st_sensors_platform_data - default accel platform data
diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
index dc73f2d85e6d..17c643727aff 100644
--- a/drivers/iio/accel/st_accel_core.c
+++ b/drivers/iio/accel/st_accel_core.c
@@ -215,6 +215,22 @@
 #define ST_ACCEL_6_IHL_IRQ_MASK			0x80
 #define ST_ACCEL_6_MULTIREAD_BIT		true
 
+/* CUSTOM VALUES FOR SENSOR 7 */
+#define ST_ACCEL_7_ODR_ADDR			0x20
+#define ST_ACCEL_7_ODR_MASK			0x30
+#define ST_ACCEL_7_ODR_AVL_280HZ_VAL		0x00
+#define ST_ACCEL_7_ODR_AVL_560HZ_VAL		0x01
+#define ST_ACCEL_7_ODR_AVL_1120HZ_VAL		0x02
+#define ST_ACCEL_7_ODR_AVL_4480HZ_VAL		0x03
+#define ST_ACCEL_7_PW_ADDR			0x20
+#define ST_ACCEL_7_PW_MASK			0xc0
+#define ST_ACCEL_7_FS_AVL_2_GAIN		IIO_G_TO_M_S_2(488)
+#define ST_ACCEL_7_BDU_ADDR			0x21
+#define ST_ACCEL_7_BDU_MASK			0x40
+#define ST_ACCEL_7_DRDY_IRQ_ADDR		0x21
+#define ST_ACCEL_7_DRDY_IRQ_INT1_MASK		0x04
+#define ST_ACCEL_7_MULTIREAD_BIT		false
+
 static const struct iio_chan_spec st_accel_8bit_channels[] = {
 	ST_SENSORS_LSM_CHANNELS(IIO_ACCEL,
 			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
@@ -662,6 +678,54 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = {
 		.multi_read_bit = ST_ACCEL_6_MULTIREAD_BIT,
 		.bootime = 2,
 	},
+	{
+		/* No WAI register present */
+		.sensors_supported = {
+			[0] = LIS3L02DQ_ACCEL_DEV_NAME,
+		},
+		.ch = (struct iio_chan_spec *)st_accel_12bit_channels,
+		.odr = {
+			.addr = ST_ACCEL_7_ODR_ADDR,
+			.mask = ST_ACCEL_7_ODR_MASK,
+			.odr_avl = {
+				{ 280, ST_ACCEL_7_ODR_AVL_280HZ_VAL, },
+				{ 560, ST_ACCEL_7_ODR_AVL_560HZ_VAL, },
+				{ 1120, ST_ACCEL_7_ODR_AVL_1120HZ_VAL, },
+				{ 4480, ST_ACCEL_7_ODR_AVL_4480HZ_VAL, },
+			},
+		},
+		.pw = {
+			.addr = ST_ACCEL_7_PW_ADDR,
+			.mask = ST_ACCEL_7_PW_MASK,
+			.value_on = ST_SENSORS_DEFAULT_POWER_ON_VALUE,
+			.value_off = ST_SENSORS_DEFAULT_POWER_OFF_VALUE,
+		},
+		.enable_axis = {
+			.addr = ST_SENSORS_DEFAULT_AXIS_ADDR,
+			.mask = ST_SENSORS_DEFAULT_AXIS_MASK,
+		},
+		.fs = {
+			.fs_avl = {
+				[0] = {
+					.num = ST_ACCEL_FS_AVL_2G,
+					.gain = ST_ACCEL_7_FS_AVL_2_GAIN,
+				},
+			},
+		},
+		/* 
+		 * The part has a BDU bit but if set the data is never
+		 * updated so don't set it.
+		 */
+		.bdu = {
+		},
+		.drdy_irq = {
+			.addr = ST_ACCEL_7_DRDY_IRQ_ADDR,
+			.mask_int1 = ST_ACCEL_7_DRDY_IRQ_INT1_MASK,
+			.addr_stat_drdy = ST_SENSORS_DEFAULT_STAT_ADDR,
+		},
+		.multi_read_bit = ST_ACCEL_7_MULTIREAD_BIT,
+		.bootime = 2,
+	},
 };
 
 static int st_accel_read_raw(struct iio_dev *indio_dev,
diff --git a/drivers/iio/accel/st_accel_i2c.c b/drivers/iio/accel/st_accel_i2c.c
index 7333ee9fb11b..1fb61d8382e0 100644
--- a/drivers/iio/accel/st_accel_i2c.c
+++ b/drivers/iio/accel/st_accel_i2c.c
@@ -80,6 +80,9 @@ static const struct of_device_id st_accel_of_match[] = {
 		.compatible = "st,h3lis331dl-accel",
 		.data = H3LIS331DL_DRIVER_NAME,
 	},
+	{
+		.compatible = "st,lis3l02dq",
+		.data = LIS3L02DQ_DRIVER_NAME,
 	{},
 };
 MODULE_DEVICE_TABLE(of, st_accel_of_match);
@@ -130,6 +133,7 @@ static const struct i2c_device_id st_accel_id_table[] = {
 	{ LSM330_ACCEL_DEV_NAME },
 	{ LSM303AGR_ACCEL_DEV_NAME },
 	{ LIS2DH12_ACCEL_DEV_NAME },
+	{ LIS3L02DQ_ACCEL_DEV_NAME },
 	{},
 };
 MODULE_DEVICE_TABLE(i2c, st_accel_id_table);
diff --git a/drivers/iio/accel/st_accel_spi.c b/drivers/iio/accel/st_accel_spi.c
index fcd5847a3fd3..efd43941d45d 100644
--- a/drivers/iio/accel/st_accel_spi.c
+++ b/drivers/iio/accel/st_accel_spi.c
@@ -59,6 +59,7 @@ static const struct spi_device_id st_accel_id_table[] = {
 	{ LSM330_ACCEL_DEV_NAME },
 	{ LSM303AGR_ACCEL_DEV_NAME },
 	{ LIS2DH12_ACCEL_DEV_NAME },
+	{ LIS3L02DQ_ACCEL_DEV_NAME },
 	{},
 };
 MODULE_DEVICE_TABLE(spi, st_accel_id_table);
diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
index dffe00692169..b672d2e1d568 100644
--- a/drivers/iio/common/st_sensors/st_sensors_core.c
+++ b/drivers/iio/common/st_sensors/st_sensors_core.c
@@ -523,7 +523,7 @@ int st_sensors_check_device_support(struct iio_dev *indio_dev,
 			int num_sensors_list,
 			const struct st_sensor_settings *sensor_settings)
 {
-	int i, n, err;
+	int i, n, err = 0;
 	u8 wai;
 	struct st_sensor_data *sdata = iio_priv(indio_dev);
 
@@ -543,17 +543,21 @@ int st_sensors_check_device_support(struct iio_dev *indio_dev,
 		return -ENODEV;
 	}
 
-	err = sdata->tf->read_byte(&sdata->tb, sdata->dev,
-					sensor_settings[i].wai_addr, &wai);
-	if (err < 0) {
-		dev_err(&indio_dev->dev, "failed to read Who-Am-I register.\n");
-		return err;
-	}
+	if (sensor_settings[i].wai_addr) {
+		err = sdata->tf->read_byte(&sdata->tb, sdata->dev,
+					   sensor_settings[i].wai_addr, &wai);
+		if (err < 0) {
+			dev_err(&indio_dev->dev,
+				"failed to read Who-Am-I register.\n");
+			return err;
+		}
 
-	if (sensor_settings[i].wai != wai) {
-		dev_err(&indio_dev->dev, "%s: WhoAmI mismatch (0x%x).\n",
-						indio_dev->name, wai);
-		return -EINVAL;
+		if (sensor_settings[i].wai != wai) {
+			dev_err(&indio_dev->dev,
+				"%s: WhoAmI mismatch (0x%x).\n",
+				indio_dev->name, wai);
+			return -EINVAL;
+		}
 	}
 
 	sdata->sensor_settings =
-- 
2.8.2

Re: [RFC] iio: accel: st_accel: Add lis3l02dq support

[Jonathan Cameron]

On 22 May 2016 20:39:29 BST, Jonathan Cameron <jic23@kernel.org> wrote:
>Time to finally kill off the venerable (it was one of my first drivers)
>lis3l02dq driver in favour of adding support in the st sensors
>framework.
>
>This does loose us the event support that driver always had, but I
>think
>that will reappear at some point and in the meantime the maintenance
>advantages of dropping the 'special' driver for this one part outweigh
>the issues.
>
>It's worth noting this part is ancient and I may well be the only
>person
>who still has any on hardware running recent kernels.
>
>It has a few 'quirks'.
> - No WAI register so that just became optional.
> - A BDU option that really does block updates.  Completely.
>   Whatever you do, you don't get any more data with it set.
>   It is documented the same as more modern parts but I presume they
>   are actually clearing for updates after a read of both bytes!
> - Fixed scale.
> - It's too quick.  Even at slowest rate (280Hz) I can't read out fast
>   enough on my board (stargate 2) to beat new data coming in. Linus'
>   repeat read patch doesn't help in this case. It just means I get 10
>   readings before dying... So in reality this will get used with
>  software triggers only unless someone has this long out of production
>   device on a quick board.

Forgot to say that I"ll obviously drop the separate driver at same time as taking this.

J
>
>Signed-off-by: Jonathan Cameron <jic23@kernel.org>
>Cc: Denis CIOCCA <denis.ciocca@st.com>
>Cc: Linus Walleij <linus.walleij@linaro.org>
>Cc: Crestez Dan Leonard <leonard.crestez@intel.com>
>---
> drivers/iio/accel/st_accel.h                    |  1 +
>drivers/iio/accel/st_accel_core.c               | 64
>+++++++++++++++++++++++++
> drivers/iio/accel/st_accel_i2c.c                |  4 ++
> drivers/iio/accel/st_accel_spi.c                |  1 +
> drivers/iio/common/st_sensors/st_sensors_core.c | 26 +++++-----
> 5 files changed, 85 insertions(+), 11 deletions(-)
>
>diff --git a/drivers/iio/accel/st_accel.h
>b/drivers/iio/accel/st_accel.h
>index 57f83a67948c..f8dfdb690563 100644
>--- a/drivers/iio/accel/st_accel.h
>+++ b/drivers/iio/accel/st_accel.h
>@@ -29,6 +29,7 @@
> #define LSM330_ACCEL_DEV_NAME		"lsm330_accel"
> #define LSM303AGR_ACCEL_DEV_NAME	"lsm303agr_accel"
> #define LIS2DH12_ACCEL_DEV_NAME		"lis2dh12_accel"
>+#define LIS3L02DQ_ACCEL_DEV_NAME	"lis3l02dq"
> 
> /**
> * struct st_sensors_platform_data - default accel platform data
>diff --git a/drivers/iio/accel/st_accel_core.c
>b/drivers/iio/accel/st_accel_core.c
>index dc73f2d85e6d..17c643727aff 100644
>--- a/drivers/iio/accel/st_accel_core.c
>+++ b/drivers/iio/accel/st_accel_core.c
>@@ -215,6 +215,22 @@
> #define ST_ACCEL_6_IHL_IRQ_MASK			0x80
> #define ST_ACCEL_6_MULTIREAD_BIT		true
> 
>+/* CUSTOM VALUES FOR SENSOR 7 */
>+#define ST_ACCEL_7_ODR_ADDR			0x20
>+#define ST_ACCEL_7_ODR_MASK			0x30
>+#define ST_ACCEL_7_ODR_AVL_280HZ_VAL		0x00
>+#define ST_ACCEL_7_ODR_AVL_560HZ_VAL		0x01
>+#define ST_ACCEL_7_ODR_AVL_1120HZ_VAL		0x02
>+#define ST_ACCEL_7_ODR_AVL_4480HZ_VAL		0x03
>+#define ST_ACCEL_7_PW_ADDR			0x20
>+#define ST_ACCEL_7_PW_MASK			0xc0
>+#define ST_ACCEL_7_FS_AVL_2_GAIN		IIO_G_TO_M_S_2(488)
>+#define ST_ACCEL_7_BDU_ADDR			0x21
>+#define ST_ACCEL_7_BDU_MASK			0x40
>+#define ST_ACCEL_7_DRDY_IRQ_ADDR		0x21
>+#define ST_ACCEL_7_DRDY_IRQ_INT1_MASK		0x04
>+#define ST_ACCEL_7_MULTIREAD_BIT		false
>+
> static const struct iio_chan_spec st_accel_8bit_channels[] = {
> 	ST_SENSORS_LSM_CHANNELS(IIO_ACCEL,
> 			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>@@ -662,6 +678,54 @@ static const struct st_sensor_settings
>st_accel_sensors_settings[] = {
> 		.multi_read_bit = ST_ACCEL_6_MULTIREAD_BIT,
> 		.bootime = 2,
> 	},
>+	{
>+		/* No WAI register present */
>+		.sensors_supported = {
>+			[0] = LIS3L02DQ_ACCEL_DEV_NAME,
>+		},
>+		.ch = (struct iio_chan_spec *)st_accel_12bit_channels,
>+		.odr = {
>+			.addr = ST_ACCEL_7_ODR_ADDR,
>+			.mask = ST_ACCEL_7_ODR_MASK,
>+			.odr_avl = {
>+				{ 280, ST_ACCEL_7_ODR_AVL_280HZ_VAL, },
>+				{ 560, ST_ACCEL_7_ODR_AVL_560HZ_VAL, },
>+				{ 1120, ST_ACCEL_7_ODR_AVL_1120HZ_VAL, },
>+				{ 4480, ST_ACCEL_7_ODR_AVL_4480HZ_VAL, },
>+			},
>+		},
>+		.pw = {
>+			.addr = ST_ACCEL_7_PW_ADDR,
>+			.mask = ST_ACCEL_7_PW_MASK,
>+			.value_on = ST_SENSORS_DEFAULT_POWER_ON_VALUE,
>+			.value_off = ST_SENSORS_DEFAULT_POWER_OFF_VALUE,
>+		},
>+		.enable_axis = {
>+			.addr = ST_SENSORS_DEFAULT_AXIS_ADDR,
>+			.mask = ST_SENSORS_DEFAULT_AXIS_MASK,
>+		},
>+		.fs = {
>+			.fs_avl = {
>+				[0] = {
>+					.num = ST_ACCEL_FS_AVL_2G,
>+					.gain = ST_ACCEL_7_FS_AVL_2_GAIN,
>+				},
>+			},
>+		},
>+		/* 
>+		 * The part has a BDU bit but if set the data is never
>+		 * updated so don't set it.
>+		 */
>+		.bdu = {
>+		},
>+		.drdy_irq = {
>+			.addr = ST_ACCEL_7_DRDY_IRQ_ADDR,
>+			.mask_int1 = ST_ACCEL_7_DRDY_IRQ_INT1_MASK,
>+			.addr_stat_drdy = ST_SENSORS_DEFAULT_STAT_ADDR,
>+		},
>+		.multi_read_bit = ST_ACCEL_7_MULTIREAD_BIT,
>+		.bootime = 2,
>+	},
> };
> 
> static int st_accel_read_raw(struct iio_dev *indio_dev,
>diff --git a/drivers/iio/accel/st_accel_i2c.c
>b/drivers/iio/accel/st_accel_i2c.c
>index 7333ee9fb11b..1fb61d8382e0 100644
>--- a/drivers/iio/accel/st_accel_i2c.c
>+++ b/drivers/iio/accel/st_accel_i2c.c
>@@ -80,6 +80,9 @@ static const struct of_device_id st_accel_of_match[]
>= {
> 		.compatible = "st,h3lis331dl-accel",
> 		.data = H3LIS331DL_DRIVER_NAME,
> 	},
>+	{
>+		.compatible = "st,lis3l02dq",
>+		.data = LIS3L02DQ_DRIVER_NAME,
> 	{},
> };
> MODULE_DEVICE_TABLE(of, st_accel_of_match);
>@@ -130,6 +133,7 @@ static const struct i2c_device_id
>st_accel_id_table[] = {
> 	{ LSM330_ACCEL_DEV_NAME },
> 	{ LSM303AGR_ACCEL_DEV_NAME },
> 	{ LIS2DH12_ACCEL_DEV_NAME },
>+	{ LIS3L02DQ_ACCEL_DEV_NAME },
> 	{},
> };
> MODULE_DEVICE_TABLE(i2c, st_accel_id_table);
>diff --git a/drivers/iio/accel/st_accel_spi.c
>b/drivers/iio/accel/st_accel_spi.c
>index fcd5847a3fd3..efd43941d45d 100644
>--- a/drivers/iio/accel/st_accel_spi.c
>+++ b/drivers/iio/accel/st_accel_spi.c
>@@ -59,6 +59,7 @@ static const struct spi_device_id st_accel_id_table[]
>= {
> 	{ LSM330_ACCEL_DEV_NAME },
> 	{ LSM303AGR_ACCEL_DEV_NAME },
> 	{ LIS2DH12_ACCEL_DEV_NAME },
>+	{ LIS3L02DQ_ACCEL_DEV_NAME },
> 	{},
> };
> MODULE_DEVICE_TABLE(spi, st_accel_id_table);
>diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c
>b/drivers/iio/common/st_sensors/st_sensors_core.c
>index dffe00692169..b672d2e1d568 100644
>--- a/drivers/iio/common/st_sensors/st_sensors_core.c
>+++ b/drivers/iio/common/st_sensors/st_sensors_core.c
>@@ -523,7 +523,7 @@ int st_sensors_check_device_support(struct iio_dev
>*indio_dev,
> 			int num_sensors_list,
> 			const struct st_sensor_settings *sensor_settings)
> {
>-	int i, n, err;
>+	int i, n, err = 0;
> 	u8 wai;
> 	struct st_sensor_data *sdata = iio_priv(indio_dev);
> 
>@@ -543,17 +543,21 @@ int st_sensors_check_device_support(struct
>iio_dev *indio_dev,
> 		return -ENODEV;
> 	}
> 
>-	err = sdata->tf->read_byte(&sdata->tb, sdata->dev,
>-					sensor_settings[i].wai_addr, &wai);
>-	if (err < 0) {
>-		dev_err(&indio_dev->dev, "failed to read Who-Am-I register.\n");
>-		return err;
>-	}
>+	if (sensor_settings[i].wai_addr) {
>+		err = sdata->tf->read_byte(&sdata->tb, sdata->dev,
>+					   sensor_settings[i].wai_addr, &wai);
>+		if (err < 0) {
>+			dev_err(&indio_dev->dev,
>+				"failed to read Who-Am-I register.\n");
>+			return err;
>+		}
> 
>-	if (sensor_settings[i].wai != wai) {
>-		dev_err(&indio_dev->dev, "%s: WhoAmI mismatch (0x%x).\n",
>-						indio_dev->name, wai);
>-		return -EINVAL;
>+		if (sensor_settings[i].wai != wai) {
>+			dev_err(&indio_dev->dev,
>+				"%s: WhoAmI mismatch (0x%x).\n",
>+				indio_dev->name, wai);
>+			return -EINVAL;
>+		}
> 	}
> 
> 	sdata->sensor_settings =

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [RFC] iio: accel: st_accel: Add lis3l02dq support

[Linus Walleij]

On Sun, May 22, 2016 at 9:39 PM, Jonathan Cameron <jic23@kernel.org> wrote:

> Time to finally kill off the venerable (it was one of my first drivers)
> lis3l02dq driver in favour of adding support in the st sensors framework.
>
> This does loose us the event support that driver always had, but I think
> that will reappear at some point and in the meantime the maintenance
> advantages of dropping the 'special' driver for this one part outweigh
> the issues.

Which events? Free-fall?

> It's worth noting this part is ancient and I may well be the only person
> who still has any on hardware running recent kernels.

What I learnt from my years in the kernel is that there is always
a bunch of silent users, also it has this nice feeling of completing
the ST sensor suite.

> It has a few 'quirks'.
>  - No WAI register so that just became optional.

Makes sense.

>  - It's too quick.  Even at slowest rate (280Hz) I can't read out fast
>    enough on my board (stargate 2) to beat new data coming in. Linus'
>    repeat read patch doesn't help in this case. It just means I get 10
>    readings before dying... So in reality this will get used with
>    software triggers only unless someone has this long out of production
>    device on a quick board.

What is the speed of the I2C bus on this system?
If it's running on the lowest speed 100kHz that may very
well be the cause of the problem, so check if it supports
high speed (400kHz) I2C.

The patch as such looks good.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [RFC] iio: accel: st_accel: Add lis3l02dq support

[jic23]

On 23.05.2016 08:43, Linus Walleij wrote:
> On Sun, May 22, 2016 at 9:39 PM, Jonathan Cameron <jic23@kernel.org> 
> wrote:
> 
>> Time to finally kill off the venerable (it was one of my first 
>> drivers)
>> lis3l02dq driver in favour of adding support in the st sensors 
>> framework.
>> 
>> This does loose us the event support that driver always had, but I 
>> think
>> that will reappear at some point and in the meantime the maintenance
>> advantages of dropping the 'special' driver for this one part outweigh
>> the issues.
> 
> Which events? Free-fall?
Both high and low acceleration limits - I never got into the full 
complexity
it supports in the hardware (and and or boolean logic of the various 
signals).
> 
>> It's worth noting this part is ancient and I may well be the only 
>> person
>> who still has any on hardware running recent kernels.
> 
> What I learnt from my years in the kernel is that there is always
> a bunch of silent users, also it has this nice feeling of completing
> the ST sensor suite.
It's possible but I've never seen any users who didn't have imote 2 or
stargate 2 boards.  The parts were generally available - however the 
driver
was borked for at least a few kernel cycles with no complaints...

Fairly safe on the stargate 2's as whilst (rumour had it) the design
was given away to a university group when the relevant bit of Intel
research ceased to be no one ever to my knowledge made any more.
A few went out to various univeristies (which is where the few I
have on loan come from).

Imote2s were a bit easier to get hold of as they sold that design to 
Crossbow
who distributed them with tinyos for a while before killing them off as 
well.
They were reasonably popular back in the days when your only small 
platforms
were them or Gumstix (also pxa270 based at the time).

They were great boards for their time - in someways under credited with
starting the whole idea of small headless linux boards for wireless 
sensing.
(they predate the gumstix) though there was a stargate 1 and I think an
original imote before them.

Btw I think there are a few new parts from ST we aren't supporting yet 
;)
> 
>> It has a few 'quirks'.
>>  - No WAI register so that just became optional.
> 
> Makes sense.
> 
>>  - It's too quick.  Even at slowest rate (280Hz) I can't read out fast
>>    enough on my board (stargate 2) to beat new data coming in. Linus'
>>    repeat read patch doesn't help in this case. It just means I get 10
>>    readings before dying... So in reality this will get used with
>>    software triggers only unless someone has this long out of 
>> production
>>    device on a quick board.
> 
> What is the speed of the I2C bus on this system?
> If it's running on the lowest speed 100kHz that may very
> well be the cause of the problem, so check if it supports
> high speed (400kHz) I2C.

Running on SPI at full rated speed of the chip.  Never done much work to
look at timing on these (not had a scope on them since the original 
driver
writing a long time back).

Getting that board to handle more than 100Hz of anything was interesting
IIRC.
> 
> The patch as such looks good.
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Thanks,

Jonathan
> 
> Yours,
> Linus Walleij
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] iio: accel: st_accel: Add lis3l02dq support

[Jonathan Cameron]

On 23 May 2016 15:13:49 BST, jic23@jic23.retrosnub.co.uk wrote:
>On 23.05.2016 08:43, Linus Walleij wrote:
>> On Sun, May 22, 2016 at 9:39 PM, Jonathan Cameron <jic23@kernel.org> 
>> wrote:
>> 
>>> Time to finally kill off the venerable (it was one of my first 
>>> drivers)
>>> lis3l02dq driver in favour of adding support in the st sensors 
>>> framework.
>>> 
>>> This does loose us the event support that driver always had, but I 
>>> think
>>> that will reappear at some point and in the meantime the maintenance
>>> advantages of dropping the 'special' driver for this one part
>outweigh
>>> the issues.
>> 
>> Which events? Free-fall?
>Both high and low acceleration limits - I never got into the full 
>complexity
>it supports in the hardware (and and or boolean logic of the various 
>signals).

I forgot this part also has gain and offset trim registers.  Will add them before
 applying as that is easy enough and reduces the differences further.

These exist on some other parts but we can add them any time.

Jonathan
>> 
>>> It's worth noting this part is ancient and I may well be the only 
>>> person
>>> who still has any on hardware running recent kernels.
>> 
>> What I learnt from my years in the kernel is that there is always
>> a bunch of silent users, also it has this nice feeling of completing
>> the ST sensor suite.
>It's possible but I've never seen any users who didn't have imote 2 or
>stargate 2 boards.  The parts were generally available - however the 
>driver
>was borked for at least a few kernel cycles with no complaints...
>
>Fairly safe on the stargate 2's as whilst (rumour had it) the design
>was given away to a university group when the relevant bit of Intel
>research ceased to be no one ever to my knowledge made any more.
>A few went out to various univeristies (which is where the few I
>have on loan come from).
>
>Imote2s were a bit easier to get hold of as they sold that design to 
>Crossbow
>who distributed them with tinyos for a while before killing them off as
>
>well.
>They were reasonably popular back in the days when your only small 
>platforms
>were them or Gumstix (also pxa270 based at the time).
>
>They were great boards for their time - in someways under credited with
>starting the whole idea of small headless linux boards for wireless 
>sensing.
>(they predate the gumstix) though there was a stargate 1 and I think an
>original imote before them.
>
>Btw I think there are a few new parts from ST we aren't supporting yet 
>;)
>> 
>>> It has a few 'quirks'.
>>>  - No WAI register so that just became optional.
>> 
>> Makes sense.
>> 
>>>  - It's too quick.  Even at slowest rate (280Hz) I can't read out
>fast
>>>    enough on my board (stargate 2) to beat new data coming in.
>Linus'
>>>    repeat read patch doesn't help in this case. It just means I get
>10
>>>    readings before dying... So in reality this will get used with
>>>    software triggers only unless someone has this long out of 
>>> production
>>>    device on a quick board.
>> 
>> What is the speed of the I2C bus on this system?
>> If it's running on the lowest speed 100kHz that may very
>> well be the cause of the problem, so check if it supports
>> high speed (400kHz) I2C.
>
>Running on SPI at full rated speed of the chip.  Never done much work
>to
>look at timing on these (not had a scope on them since the original 
>driver
>writing a long time back).
>
>Getting that board to handle more than 100Hz of anything was
>interesting
>IIRC.
>> 
>> The patch as such looks good.
>> 
>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>Thanks,
>
>Jonathan
>> 
>> Yours,
>> Linus Walleij
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio"
>in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>--
>To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [RFC] iio: accel: st_accel: Add lis3l02dq support

[Linus Walleij]

On Mon, May 23, 2016 at 6:29 PM, Jonathan Cameron
<jic23@jic23.retrosnub.co.uk> wrote:

> I forgot this part also has gain and offset trim registers.  Will add them before
>  applying as that is easy enough and reduces the differences further.
>
> These exist on some other parts but we can add them any time.

LIS3LV02DL also has x,y,z offset and gain trim registers, but when
I read up on it, I understood that these are just factory-programmed
registers. I.e. they are undocumented in the register list, but a table
states them as "calibration" and "loaded at boot".

So I always understood them as something the sensor loads at
boot to calibrate itself, so you can ignore the contents, and they
probably exist on the other sensors too, albeit undocumented as
they have no use.

I toyed with the idea of reading them and pusing the values to the
entropy pool with add_device_randomness() as they are device-unique
data.

But if you have proper docs on them I guess maybe they are used
differently on your sensor...

Yours,
Linus Walleij

Re: [RFC] iio: accel: st_accel: Add lis3l02dq support

[jic23]

On 24.05.2016 09:45, Linus Walleij wrote:
> On Mon, May 23, 2016 at 6:29 PM, Jonathan Cameron
> <jic23@jic23.retrosnub.co.uk> wrote:
> 
>> I forgot this part also has gain and offset trim registers.  Will add 
>> them before
>>  applying as that is easy enough and reduces the differences further.
>> 
>> These exist on some other parts but we can add them any time.
> 
> LIS3LV02DL also has x,y,z offset and gain trim registers, but when
> I read up on it, I understood that these are just factory-programmed
> registers. I.e. they are undocumented in the register list, but a table
> states them as "calibration" and "loaded at boot".
> 
> So I always understood them as something the sensor loads at
> boot to calibrate itself, so you can ignore the contents, and they
> probably exist on the other sensors too, albeit undocumented as
> they have no use.
> 
> I toyed with the idea of reading them and pusing the values to the
> entropy pool with add_device_randomness() as they are device-unique
> data.
> 
> But if you have proper docs on them I guess maybe they are used
> differently on your sensor...
> 
> Yours,
> Linus Walleij
As I understand it you can actually apply corrections to them (was 
certainly
how I read that years ago).  Take into account that these early sensors 
were
incredibly noisy and perhaps more prone to shock damage than newer ones.
(my test sensor will vary by 10% or more whilst sat on a steady concrete
floor - even with huge levels of oversampling...)

Back then we were using some sensors where if you dropped them on the 
floor
you had to send them back to the manufacturer for recalibration before 
they
were remotely useful again.

Funnily enough, given we had them attached to people that happened all 
the
time :(

ST certainly aren't believers in 'detailed' documentation. Always feels
like somewhat of a guessing game when implementing drivers for their
parts.

J

Re: [RFC] iio: accel: st_accel: Add lis3l02dq support

[Jonathan Cameron]

On 24/05/16 09:57, jic23@jic23.retrosnub.co.uk wrote:
> On 24.05.2016 09:45, Linus Walleij wrote:
>> On Mon, May 23, 2016 at 6:29 PM, Jonathan Cameron
>> <jic23@jic23.retrosnub.co.uk> wrote:
>>
>>> I forgot this part also has gain and offset trim registers.  Will add them before
>>>  applying as that is easy enough and reduces the differences further.
>>>
>>> These exist on some other parts but we can add them any time.
>>
>> LIS3LV02DL also has x,y,z offset and gain trim registers, but when
>> I read up on it, I understood that these are just factory-programmed
>> registers. I.e. they are undocumented in the register list, but a table
>> states them as "calibration" and "loaded at boot".
>>
>> So I always understood them as something the sensor loads at
>> boot to calibrate itself, so you can ignore the contents, and they
>> probably exist on the other sensors too, albeit undocumented as
>> they have no use.
>>
>> I toyed with the idea of reading them and pusing the values to the
>> entropy pool with add_device_randomness() as they are device-unique
>> data.
>>
>> But if you have proper docs on them I guess maybe they are used
>> differently on your sensor...
>>
>> Yours,
>> Linus Walleij
> As I understand it you can actually apply corrections to them (was certainly
> how I read that years ago).  Take into account that these early sensors were
> incredibly noisy and perhaps more prone to shock damage than newer ones.
> (my test sensor will vary by 10% or more whilst sat on a steady concrete
> floor - even with huge levels of oversampling...)
You can indeed apply corrections to them.  It might be an entertaining attack
vector if the machine gets compromised and someone is using it for free fall
detection as you can make it read whatever you like ;)

Anyhow, I have working code but need to tidy up before posting a V2 with
support for this on this chip included.  Can support other chips off datasheet
but have no hardware.
> 
> Back then we were using some sensors where if you dropped them on the floor
> you had to send them back to the manufacturer for recalibration before they
> were remotely useful again.
> 
> Funnily enough, given we had them attached to people that happened all the
> time :(
> 
> ST certainly aren't believers in 'detailed' documentation. Always feels
> like somewhat of a guessing game when implementing drivers for their
> parts.
> 
> J
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] iio: accel: st_accel: Add lis3l02dq support

[Jonathan Cameron]

On 23/05/16 08:43, Linus Walleij wrote:
> On Sun, May 22, 2016 at 9:39 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> 
>> Time to finally kill off the venerable (it was one of my first drivers)
>> lis3l02dq driver in favour of adding support in the st sensors framework.
>>
>> This does loose us the event support that driver always had, but I think
>> that will reappear at some point and in the meantime the maintenance
>> advantages of dropping the 'special' driver for this one part outweigh
>> the issues.
> 
> Which events? Free-fall?
> 
>> It's worth noting this part is ancient and I may well be the only person
>> who still has any on hardware running recent kernels.
> 
> What I learnt from my years in the kernel is that there is always
> a bunch of silent users, also it has this nice feeling of completing
> the ST sensor suite.
> 
>> It has a few 'quirks'.
>>  - No WAI register so that just became optional.
> 
> Makes sense.
> 
>>  - It's too quick.  Even at slowest rate (280Hz) I can't read out fast
>>    enough on my board (stargate 2) to beat new data coming in. Linus'
>>    repeat read patch doesn't help in this case. It just means I get 10
>>    readings before dying... So in reality this will get used with
>>    software triggers only unless someone has this long out of production
>>    device on a quick board.
> 
> What is the speed of the I2C bus on this system?
> If it's running on the lowest speed 100kHz that may very
> well be the cause of the problem, so check if it supports
> high speed (400kHz) I2C.
> 
> The patch as such looks good.
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
I've decided I'm not that fussed about getting the threshold events
and calibration register access stuff in place before killing off
the staging driver.

As such applied and also applied is a patch dropping the old lis3l02dq
driver.  Funny how one gets mixed feelings when retiring ones own
old code.

Anyhow, lots more stuff in staging that needs dealing with!!!
Unfortunately my next target of the sca3000 is going to be a lot
harder to bring into the modern world.

Jonathan
> 
> Yours,
> Linus Walleij
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>