Skip to contentSkip to conversation

iio:st_sensors: fixes and lps22hb pressure sensor

RFC PATCH Applied Gregor Boirie 4 participants 37 messages Apr 19, 2016 – Jun 15, 2016

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

cover iio:st_sensors: fixes and lps22hb pressure sensor

This preliminary patch series adds support for a new ST LPS22HB pressure sensor and introduce a few fixes related to st_pressure core and st_sensors triggered buffering.
It is not meant to be reviewed for definitive inclusion as it touches too many drivers / devices I cannot test with.
Note that as a few minor and more controversial patches (7, 8 and 9) might also be candidate for a seperate series.

Patch 2 makes st_pressure sensors compliant with ABI and fixes a few missing sampling gains. Scale / offset computation is modified to address all gains currently possible. It impacts LPS331AP, LPS001WP and LPS25H sensors. Please please please ! If anyone owning one of these could run some tests, I'd be glad to get some feedback since I have none of them.

Patch 4 is a rework of the way st_sensors samples are stored in memory to comply with IIO expected alignment contraints (some st_pressure samples are 24 bits long). It is heavily based upon Linux Walleij' approach where each channel is captured individually. See http://www.spinics.net/lists/linux-iio/msg24028.html
and http://www.spinics.net/lists/linux-iio/msg23598.html threads for more infos. This patch impacts all st_sensors.
Please please please ! If anyone owning one of these could run some tests...

Patch 5 enforces 32 bits storage alignment for 24 bits long st_pressure sampling channels.
Patch 6 enables triggered buffering for st_pressure temperature channels. We need temperature samples to control on-board device temperature (noise and drift removal).
Both patches impact st_pressure sensors mentionned above. Please please please ! If anyone owning one of these could run some tests...

Regards,
gregor.

Gregor Boirie (9):
iio:st_pressure:initial lps22hb sensor support iio:st_pressure: fix sampling gains iio:st_pressure: lps22hb temperature support iio:st_sensors: align on storagebits boundaries iio:st_pressure: align storagebits on power of 2 iio:st_pressure: temperature triggered buffering iio:st_sensors: unexport st_sensors_get_buffer_element iio:st_sensors: emulate SMBus block read if needed iio:st_sensors: fix power regulator usage

 .../devicetree/bindings/iio/st-sensors.txt         |   1 +
 drivers/iio/accel/st_accel_core.c                  |  12 +-
 drivers/iio/common/st_sensors/st_sensors_buffer.c  |  41 ++--
 drivers/iio/common/st_sensors/st_sensors_core.c    |  31 ++-
 drivers/iio/common/st_sensors/st_sensors_i2c.c     |   4 +-
 drivers/iio/gyro/st_gyro_core.c                    |  12 +-
 drivers/iio/magnetometer/st_magn_core.c            |  12 +-
 drivers/iio/pressure/Kconfig                       |   2 +-
 drivers/iio/pressure/st_pressure.h                 |   1 +
 drivers/iio/pressure/st_pressure_core.c            | 250 ++++++++++++++++-----
 drivers/iio/pressure/st_pressure_i2c.c             |   4 +
 drivers/iio/pressure/st_pressure_spi.c             |   1 +
 include/linux/iio/common/st_sensors.h              |   4 +-
 13 files changed, 278 insertions(+), 97 deletions(-)
Signature · 1 line
2.1.4
#1 lore raw

RFC PATCH 1/9 iio:st_pressure:initial lps22hb sensor support

Initial support for ST LPS22HB pressure sensor. Datasheet:
http://www2.st.com/resource/en/datasheet/lps22hb.pdf

Features:
* pressure data and timestamping channels
* sampling frequency selection
* interrupt based trigger
* over I2C or SPI

---
 .../devicetree/bindings/iio/st-sensors.txt         |  1 +
 drivers/iio/pressure/Kconfig                       |  2 +-
 drivers/iio/pressure/st_pressure.h                 |  1 +
 drivers/iio/pressure/st_pressure_core.c            | 93 +++++++++++++++++++++-
 drivers/iio/pressure/st_pressure_i2c.c             |  4 +
 drivers/iio/pressure/st_pressure_spi.c             |  1 +
 6 files changed, 97 insertions(+), 5 deletions(-)
Documentation/devicetree/bindings/iio/st-sensors.txt +1 -0
index 637e283..2bb8931 100644
@@ -63,3 +63,4 @@ Pressure sensors:
 - st,lps001wp-press
 - st,lps25h-press
 - st,lps331ap-press
+- st,lps22hb-press
drivers/iio/pressure/Kconfig +1 -1
index 8de0192..d63d280 100644
@@ -118,7 +118,7 @@ config IIO_ST_PRESS
 	select IIO_TRIGGERED_BUFFER if (IIO_BUFFER)
 	help
 	  Say yes here to build support for STMicroelectronics pressure
-	  sensors: LPS001WP, LPS25H, LPS331AP.
+	  sensors: LPS001WP, LPS25H, LPS331AP, LPS22HB.
 
 	  This driver can also be built as a module. If so, these modules
 	  will be created:
drivers/iio/pressure/st_pressure.h +1 -0
index f5f4149..903a21e 100644
@@ -17,6 +17,7 @@
 #define LPS001WP_PRESS_DEV_NAME		"lps001wp"
 #define LPS25H_PRESS_DEV_NAME		"lps25h"
 #define LPS331AP_PRESS_DEV_NAME		"lps331ap"
+#define LPS22HB_PRESS_DEV_NAME		"lps22hb"
 
 /**
  * struct st_sensors_platform_data - default press platform data
drivers/iio/pressure/st_pressure_core.c +89 -4

117 lines not on this page · raw message

drivers/iio/pressure/st_pressure_i2c.c +4 -0
index 8fcf976..ed18701 100644
@@ -32,6 +32,10 @@ static const struct of_device_id st_press_of_match[] = {
 		.compatible = "st,lps331ap-press",
 		.data = LPS331AP_PRESS_DEV_NAME,
 	},
+	{
+		.compatible = "st,lps22hb-press",
+		.data = LPS22HB_PRESS_DEV_NAME,
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, st_press_of_match);
drivers/iio/pressure/st_pressure_spi.c +1 -0
index 40c0692..5505080 100644
@@ -50,6 +50,7 @@ static const struct spi_device_id st_press_id_table[] = {
 	{ LPS001WP_PRESS_DEV_NAME },
 	{ LPS25H_PRESS_DEV_NAME },
 	{ LPS331AP_PRESS_DEV_NAME },
+	{ LPS22HB_PRESS_DEV_NAME },
 	{},
 };
 MODULE_DEVICE_TABLE(spi, st_press_id_table);
Signature · 1 line
2.1.4
maintainer
#2 lore raw

Re: [RFC PATCH v1 1/9] iio:st_pressure:initial lps22hb sensor support

Quoted 10 lines · Gregor Boirie

Initial support for ST LPS22HB pressure sensor. Datasheet:
http://www2.st.com/resource/en/datasheet/lps22hb.pdf

Features:
* pressure data and timestamping channels
* sampling frequency selection
* interrupt based trigger
* over I2C or SPI

Looks good to me. I would however like an Ack from Denis if possible as this is his driver. Same is true for this whole series.

Jonathan

Quoted 196 lines
---
 .../devicetree/bindings/iio/st-sensors.txt         |  1 +
 drivers/iio/pressure/Kconfig                       |  2 +-
 drivers/iio/pressure/st_pressure.h                 |  1 +
 drivers/iio/pressure/st_pressure_core.c            | 93 +++++++++++++++++++++-
 drivers/iio/pressure/st_pressure_i2c.c             |  4 +
 drivers/iio/pressure/st_pressure_spi.c             |  1 +
 6 files changed, 97 insertions(+), 5 deletions(-)
Documentation/devicetree/bindings/iio/st-sensors.txt +1 -0
index 637e283..2bb8931 100644
@@ -63,3 +63,4 @@ Pressure sensors:
 - st,lps001wp-press
 - st,lps25h-press
 - st,lps331ap-press
+- st,lps22hb-press
drivers/iio/pressure/Kconfig +1 -1
index 8de0192..d63d280 100644
@@ -118,7 +118,7 @@ config IIO_ST_PRESS
 	select IIO_TRIGGERED_BUFFER if (IIO_BUFFER)
 	help
 	  Say yes here to build support for STMicroelectronics pressure
-	  sensors: LPS001WP, LPS25H, LPS331AP.
+	  sensors: LPS001WP, LPS25H, LPS331AP, LPS22HB.
 
 	  This driver can also be built as a module. If so, these modules
 	  will be created:
drivers/iio/pressure/st_pressure.h +1 -0
index f5f4149..903a21e 100644
@@ -17,6 +17,7 @@
 #define LPS001WP_PRESS_DEV_NAME		"lps001wp"
 #define LPS25H_PRESS_DEV_NAME		"lps25h"
 #define LPS331AP_PRESS_DEV_NAME		"lps331ap"
+#define LPS22HB_PRESS_DEV_NAME		"lps22hb"
 
 /**
  * struct st_sensors_platform_data - default press platform data
drivers/iio/pressure/st_pressure_core.c +89 -4
index 9e9b72a..c0ff3bf 100644
@@ -113,6 +113,26 @@
 #define ST_PRESS_LPS25H_OUT_XL_ADDR		0x28
 #define ST_TEMP_LPS25H_OUT_L_ADDR		0x2b
 
+/* CUSTOM VALUES FOR LPS22HB SENSOR */
+#define ST_PRESS_LPS22HB_WAI_EXP		0xb1
+#define ST_PRESS_LPS22HB_ODR_ADDR		0x10
+#define ST_PRESS_LPS22HB_ODR_MASK		0x70
+#define ST_PRESS_LPS22HB_ODR_AVL_1HZ_VAL	0x01
+#define ST_PRESS_LPS22HB_ODR_AVL_10HZ_VAL	0x02
+#define ST_PRESS_LPS22HB_ODR_AVL_25HZ_VAL	0x03
+#define ST_PRESS_LPS22HB_ODR_AVL_50HZ_VAL	0x04
+#define ST_PRESS_LPS22HB_ODR_AVL_75HZ_VAL	0x05
+#define ST_PRESS_LPS22HB_PW_ADDR		0x10
+#define ST_PRESS_LPS22HB_PW_MASK		0x70
+#define ST_PRESS_LPS22HB_BDU_ADDR		0x10
+#define ST_PRESS_LPS22HB_BDU_MASK		0x02
+#define ST_PRESS_LPS22HB_DRDY_IRQ_ADDR		0x12
+#define ST_PRESS_LPS22HB_DRDY_IRQ_INT1_MASK	0x04
+#define ST_PRESS_LPS22HB_DRDY_IRQ_INT2_MASK	0x08
+#define ST_PRESS_LPS22HB_IHL_IRQ_ADDR		0x12
+#define ST_PRESS_LPS22HB_IHL_IRQ_MASK		0x80
+#define ST_PRESS_LPS22HB_MULTIREAD_BIT		true
+
 static const struct iio_chan_spec st_press_1_channels[] = {
 	{
 		.type = IIO_PRESSURE,
@@ -183,6 +203,27 @@ static const struct iio_chan_spec st_press_lps001wp_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(1)
 };
 
+static const struct iio_chan_spec st_press_lps22hb_channels[] = {
+	{
+		.type = IIO_PRESSURE,
+		.channel2 = IIO_NO_MOD,
+		.address = ST_PRESS_1_OUT_XL_ADDR,
+		.scan_index = 0,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 24,
+			.storagebits = 24,
+			.endianness = IIO_LE,
+		},
+		.info_mask_separate =
+			BIT(IIO_CHAN_INFO_RAW) |
+			BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.modified = 0,
+	},
+	IIO_CHAN_SOFT_TIMESTAMP(1)
+};
+
 static const struct st_sensor_settings st_press_sensors_settings[] = {
 	{
 		.wai = ST_PRESS_LPS331AP_WAI_EXP,
@@ -326,6 +367,51 @@ static const struct st_sensor_settings st_press_sensors_settings[] = {
 		.multi_read_bit = ST_PRESS_LPS25H_MULTIREAD_BIT,
 		.bootime = 2,
 	},
+	{
+		.wai = ST_PRESS_LPS22HB_WAI_EXP,
+		.wai_addr = ST_SENSORS_DEFAULT_WAI_ADDRESS,
+		.sensors_supported = {
+			[0] = LPS22HB_PRESS_DEV_NAME,
+		},
+		.ch = (struct iio_chan_spec *)st_press_lps22hb_channels,
+		.num_ch = ARRAY_SIZE(st_press_lps22hb_channels),
+		.odr = {
+			.addr = ST_PRESS_LPS22HB_ODR_ADDR,
+			.mask = ST_PRESS_LPS22HB_ODR_MASK,
+			.odr_avl = {
+				{ 1, ST_PRESS_LPS22HB_ODR_AVL_1HZ_VAL, },
+				{ 10, ST_PRESS_LPS22HB_ODR_AVL_10HZ_VAL, },
+				{ 25, ST_PRESS_LPS22HB_ODR_AVL_25HZ_VAL, },
+				{ 50, ST_PRESS_LPS22HB_ODR_AVL_50HZ_VAL, },
+				{ 75, ST_PRESS_LPS22HB_ODR_AVL_75HZ_VAL, },
+			},
+		},
+		.pw = {
+			.addr = ST_PRESS_LPS22HB_PW_ADDR,
+			.mask = ST_PRESS_LPS22HB_PW_MASK,
+			.value_off = ST_SENSORS_DEFAULT_POWER_OFF_VALUE,
+		},
+		.fs = {
+			.fs_avl = {
+				[0] = {
+					.num = ST_PRESS_FS_AVL_1260MB,
+					.gain = ST_PRESS_KPASCAL_NANO_SCALE,
+				},
+			},
+		},
+		.bdu = {
+			.addr = ST_PRESS_LPS22HB_BDU_ADDR,
+			.mask = ST_PRESS_LPS22HB_BDU_MASK,
+		},
+		.drdy_irq = {
+			.addr = ST_PRESS_LPS22HB_DRDY_IRQ_ADDR,
+			.mask_int1 = ST_PRESS_LPS22HB_DRDY_IRQ_INT1_MASK,
+			.mask_int2 = ST_PRESS_LPS22HB_DRDY_IRQ_INT2_MASK,
+			.addr_ihl = ST_PRESS_LPS22HB_IHL_IRQ_ADDR,
+			.mask_ihl = ST_PRESS_LPS22HB_IHL_IRQ_MASK,
+		},
+		.multi_read_bit = ST_PRESS_LPS22HB_MULTIREAD_BIT,
+	},
 };
 
 static int st_press_write_raw(struct iio_dev *indio_dev,
@@ -454,10 +540,9 @@ int st_press_common_probe(struct iio_dev *indio_dev)
 	indio_dev->channels = press_data->sensor_settings->ch;
 	indio_dev->num_channels = press_data->sensor_settings->num_ch;
 
-	if (press_data->sensor_settings->fs.addr != 0)
-		press_data->current_fullscale =
-			(struct st_sensor_fullscale_avl *)
-				&press_data->sensor_settings->fs.fs_avl[0];
+	press_data->current_fullscale =
+		(struct st_sensor_fullscale_avl *)
+			&press_data->sensor_settings->fs.fs_avl[0];
 
 	press_data->odr = press_data->sensor_settings->odr.odr_avl[0].hz;
 
drivers/iio/pressure/st_pressure_i2c.c +4 -0
index 8fcf976..ed18701 100644
@@ -32,6 +32,10 @@ static const struct of_device_id st_press_of_match[] = {
 		.compatible = "st,lps331ap-press",
 		.data = LPS331AP_PRESS_DEV_NAME,
 	},
+	{
+		.compatible = "st,lps22hb-press",
+		.data = LPS22HB_PRESS_DEV_NAME,
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, st_press_of_match);
drivers/iio/pressure/st_pressure_spi.c +1 -0
index 40c0692..5505080 100644
@@ -50,6 +50,7 @@ static const struct spi_device_id st_press_id_table[] = {
 	{ LPS001WP_PRESS_DEV_NAME },
 	{ LPS25H_PRESS_DEV_NAME },
 	{ LPS331AP_PRESS_DEV_NAME },
+	{ LPS22HB_PRESS_DEV_NAME },
 	{},
 };
 MODULE_DEVICE_TABLE(spi, st_press_id_table);
maintainer
#3 lore raw

Re: [RFC PATCH v1 1/9] iio:st_pressure:initial lps22hb sensor support

Quoted 10 lines · Jonathan Cameron
Quoted 9 lines · Gregor Boirie

Initial support for ST LPS22HB pressure sensor. Datasheet:
http://www2.st.com/resource/en/datasheet/lps22hb.pdf

Features:
* pressure data and timestamping channels
* sampling frequency selection
* interrupt based trigger
* over I2C or SPI

Quoted 3 lines
Quoted 1 line

Looks good to me. I would however like an Ack from Denis if possible as this is his driver. Same is true for this whole series.

Denis, are you likely to get a chance to look at this series?

No immediate rush if you are busy right now as we've almost certainly missed the merge window anyway and there are open questions on approach taken in patch 4.

Jonathan

Quoted 204 lines

Jonathan

Quoted 189 lines
---
 .../devicetree/bindings/iio/st-sensors.txt         |  1 +
 drivers/iio/pressure/Kconfig                       |  2 +-
 drivers/iio/pressure/st_pressure.h                 |  1 +
 drivers/iio/pressure/st_pressure_core.c            | 93 +++++++++++++++++++++-
 drivers/iio/pressure/st_pressure_i2c.c             |  4 +
 drivers/iio/pressure/st_pressure_spi.c             |  1 +
 6 files changed, 97 insertions(+), 5 deletions(-)
Documentation/devicetree/bindings/iio/st-sensors.txt +1 -0
index 637e283..2bb8931 100644
@@ -63,3 +63,4 @@ Pressure sensors:
 - st,lps001wp-press
 - st,lps25h-press
 - st,lps331ap-press
+- st,lps22hb-press
drivers/iio/pressure/Kconfig +1 -1
index 8de0192..d63d280 100644
@@ -118,7 +118,7 @@ config IIO_ST_PRESS
 	select IIO_TRIGGERED_BUFFER if (IIO_BUFFER)
 	help
 	  Say yes here to build support for STMicroelectronics pressure
-	  sensors: LPS001WP, LPS25H, LPS331AP.
+	  sensors: LPS001WP, LPS25H, LPS331AP, LPS22HB.
 
 	  This driver can also be built as a module. If so, these modules
 	  will be created:
drivers/iio/pressure/st_pressure.h +1 -0
index f5f4149..903a21e 100644
@@ -17,6 +17,7 @@
 #define LPS001WP_PRESS_DEV_NAME		"lps001wp"
 #define LPS25H_PRESS_DEV_NAME		"lps25h"
 #define LPS331AP_PRESS_DEV_NAME		"lps331ap"
+#define LPS22HB_PRESS_DEV_NAME		"lps22hb"
 
 /**
  * struct st_sensors_platform_data - default press platform data
drivers/iio/pressure/st_pressure_core.c +89 -4
index 9e9b72a..c0ff3bf 100644
@@ -113,6 +113,26 @@
 #define ST_PRESS_LPS25H_OUT_XL_ADDR		0x28
 #define ST_TEMP_LPS25H_OUT_L_ADDR		0x2b
 
+/* CUSTOM VALUES FOR LPS22HB SENSOR */
+#define ST_PRESS_LPS22HB_WAI_EXP		0xb1
+#define ST_PRESS_LPS22HB_ODR_ADDR		0x10
+#define ST_PRESS_LPS22HB_ODR_MASK		0x70
+#define ST_PRESS_LPS22HB_ODR_AVL_1HZ_VAL	0x01
+#define ST_PRESS_LPS22HB_ODR_AVL_10HZ_VAL	0x02
+#define ST_PRESS_LPS22HB_ODR_AVL_25HZ_VAL	0x03
+#define ST_PRESS_LPS22HB_ODR_AVL_50HZ_VAL	0x04
+#define ST_PRESS_LPS22HB_ODR_AVL_75HZ_VAL	0x05
+#define ST_PRESS_LPS22HB_PW_ADDR		0x10
+#define ST_PRESS_LPS22HB_PW_MASK		0x70
+#define ST_PRESS_LPS22HB_BDU_ADDR		0x10
+#define ST_PRESS_LPS22HB_BDU_MASK		0x02
+#define ST_PRESS_LPS22HB_DRDY_IRQ_ADDR		0x12
+#define ST_PRESS_LPS22HB_DRDY_IRQ_INT1_MASK	0x04
+#define ST_PRESS_LPS22HB_DRDY_IRQ_INT2_MASK	0x08
+#define ST_PRESS_LPS22HB_IHL_IRQ_ADDR		0x12
+#define ST_PRESS_LPS22HB_IHL_IRQ_MASK		0x80
+#define ST_PRESS_LPS22HB_MULTIREAD_BIT		true
+
 static const struct iio_chan_spec st_press_1_channels[] = {
 	{
 		.type = IIO_PRESSURE,
@@ -183,6 +203,27 @@ static const struct iio_chan_spec st_press_lps001wp_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(1)
 };
 
+static const struct iio_chan_spec st_press_lps22hb_channels[] = {
+	{
+		.type = IIO_PRESSURE,
+		.channel2 = IIO_NO_MOD,
+		.address = ST_PRESS_1_OUT_XL_ADDR,
+		.scan_index = 0,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 24,
+			.storagebits = 24,
+			.endianness = IIO_LE,
+		},
+		.info_mask_separate =
+			BIT(IIO_CHAN_INFO_RAW) |
+			BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.modified = 0,
+	},
+	IIO_CHAN_SOFT_TIMESTAMP(1)
+};
+
 static const struct st_sensor_settings st_press_sensors_settings[] = {
 	{
 		.wai = ST_PRESS_LPS331AP_WAI_EXP,
@@ -326,6 +367,51 @@ static const struct st_sensor_settings st_press_sensors_settings[] = {
 		.multi_read_bit = ST_PRESS_LPS25H_MULTIREAD_BIT,
 		.bootime = 2,
 	},
+	{
+		.wai = ST_PRESS_LPS22HB_WAI_EXP,
+		.wai_addr = ST_SENSORS_DEFAULT_WAI_ADDRESS,
+		.sensors_supported = {
+			[0] = LPS22HB_PRESS_DEV_NAME,
+		},
+		.ch = (struct iio_chan_spec *)st_press_lps22hb_channels,
+		.num_ch = ARRAY_SIZE(st_press_lps22hb_channels),
+		.odr = {
+			.addr = ST_PRESS_LPS22HB_ODR_ADDR,
+			.mask = ST_PRESS_LPS22HB_ODR_MASK,
+			.odr_avl = {
+				{ 1, ST_PRESS_LPS22HB_ODR_AVL_1HZ_VAL, },
+				{ 10, ST_PRESS_LPS22HB_ODR_AVL_10HZ_VAL, },
+				{ 25, ST_PRESS_LPS22HB_ODR_AVL_25HZ_VAL, },
+				{ 50, ST_PRESS_LPS22HB_ODR_AVL_50HZ_VAL, },
+				{ 75, ST_PRESS_LPS22HB_ODR_AVL_75HZ_VAL, },
+			},
+		},
+		.pw = {
+			.addr = ST_PRESS_LPS22HB_PW_ADDR,
+			.mask = ST_PRESS_LPS22HB_PW_MASK,
+			.value_off = ST_SENSORS_DEFAULT_POWER_OFF_VALUE,
+		},
+		.fs = {
+			.fs_avl = {
+				[0] = {
+					.num = ST_PRESS_FS_AVL_1260MB,
+					.gain = ST_PRESS_KPASCAL_NANO_SCALE,
+				},
+			},
+		},
+		.bdu = {
+			.addr = ST_PRESS_LPS22HB_BDU_ADDR,
+			.mask = ST_PRESS_LPS22HB_BDU_MASK,
+		},
+		.drdy_irq = {
+			.addr = ST_PRESS_LPS22HB_DRDY_IRQ_ADDR,
+			.mask_int1 = ST_PRESS_LPS22HB_DRDY_IRQ_INT1_MASK,
+			.mask_int2 = ST_PRESS_LPS22HB_DRDY_IRQ_INT2_MASK,
+			.addr_ihl = ST_PRESS_LPS22HB_IHL_IRQ_ADDR,
+			.mask_ihl = ST_PRESS_LPS22HB_IHL_IRQ_MASK,
+		},
+		.multi_read_bit = ST_PRESS_LPS22HB_MULTIREAD_BIT,
+	},
 };
 
 static int st_press_write_raw(struct iio_dev *indio_dev,
@@ -454,10 +540,9 @@ int st_press_common_probe(struct iio_dev *indio_dev)
 	indio_dev->channels = press_data->sensor_settings->ch;
 	indio_dev->num_channels = press_data->sensor_settings->num_ch;
 
-	if (press_data->sensor_settings->fs.addr != 0)
-		press_data->current_fullscale =
-			(struct st_sensor_fullscale_avl *)
-				&press_data->sensor_settings->fs.fs_avl[0];
+	press_data->current_fullscale =
+		(struct st_sensor_fullscale_avl *)
+			&press_data->sensor_settings->fs.fs_avl[0];
 
 	press_data->odr = press_data->sensor_settings->odr.odr_avl[0].hz;
 
drivers/iio/pressure/st_pressure_i2c.c +4 -0
index 8fcf976..ed18701 100644
@@ -32,6 +32,10 @@ static const struct of_device_id st_press_of_match[] = {
 		.compatible = "st,lps331ap-press",
 		.data = LPS331AP_PRESS_DEV_NAME,
 	},
+	{
+		.compatible = "st,lps22hb-press",
+		.data = LPS22HB_PRESS_DEV_NAME,
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, st_press_of_match);
drivers/iio/pressure/st_pressure_spi.c +0 -0
index 40c0692..5505080 100644

drivers/iio/pressure/st_pressure_spi.c static const struct spi_device_id st_press_id_table[] = { · line 56

Quoted 7 lines
drivers/iio/pressure/st_pressure_spi.c +1 -0
@@ -50,6 +50,7 @@ static const struct spi_device_id st_press_id_table[] = {
 	{ LPS001WP_PRESS_DEV_NAME },
 	{ LPS25H_PRESS_DEV_NAME },
 	{ LPS331AP_PRESS_DEV_NAME },
+	{ LPS22HB_PRESS_DEV_NAME },
 	{},
 };
 MODULE_DEVICE_TABLE(spi, st_press_id_table);

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

maintainer Applied
#4 lore raw

Re: [RFC PATCH v1 1/9] iio:st_pressure:initial lps22hb sensor support

Quoted 11 lines · Jonathan Cameron
Quoted 10 lines · Jonathan Cameron
Quoted 9 lines · Gregor Boirie

Initial support for ST LPS22HB pressure sensor. Datasheet:
http://www2.st.com/resource/en/datasheet/lps22hb.pdf

Features:
* pressure data and timestamping channels
* sampling frequency selection
* interrupt based trigger
* over I2C or SPI

Quoted 8 lines
Quoted 3 lines
Quoted 1 line

Looks good to me. I would however like an Ack from Denis if possible as this is his driver. Same is true for this whole series.

Denis, are you likely to get a chance to look at this series?

No immediate rush if you are busy right now as we've almost certainly missed the merge window anyway and there are open questions on approach taken in patch 4.

Denis is rather swamped at the moment and not sure when he'll get a chance to catch up so in the meantime I'm going to take the less controversial patches without the usual Ack.

Applied to the togreg branch of iio.git - initially pushed out as testing for the autobuilders to play with it.

Thanks,

Jonathan

Quoted 212 lines

Jonathan

Quoted 195 lines

Jonathan

Quoted 193 lines
---
 .../devicetree/bindings/iio/st-sensors.txt         |  1 +
 drivers/iio/pressure/Kconfig                       |  2 +-
 drivers/iio/pressure/st_pressure.h                 |  1 +
 drivers/iio/pressure/st_pressure_core.c            | 93 +++++++++++++++++++++-
 drivers/iio/pressure/st_pressure_i2c.c             |  4 +
 drivers/iio/pressure/st_pressure_spi.c             |  1 +
 6 files changed, 97 insertions(+), 5 deletions(-)
Documentation/devicetree/bindings/iio/st-sensors.txt +1 -0
index 637e283..2bb8931 100644
@@ -63,3 +63,4 @@ Pressure sensors:
 - st,lps001wp-press
 - st,lps25h-press
 - st,lps331ap-press
+- st,lps22hb-press
drivers/iio/pressure/Kconfig +1 -1
index 8de0192..d63d280 100644
@@ -118,7 +118,7 @@ config IIO_ST_PRESS
 	select IIO_TRIGGERED_BUFFER if (IIO_BUFFER)
 	help
 	  Say yes here to build support for STMicroelectronics pressure
-	  sensors: LPS001WP, LPS25H, LPS331AP.
+	  sensors: LPS001WP, LPS25H, LPS331AP, LPS22HB.
 
 	  This driver can also be built as a module. If so, these modules
 	  will be created:
drivers/iio/pressure/st_pressure.h +1 -0
index f5f4149..903a21e 100644
@@ -17,6 +17,7 @@
 #define LPS001WP_PRESS_DEV_NAME		"lps001wp"
 #define LPS25H_PRESS_DEV_NAME		"lps25h"
 #define LPS331AP_PRESS_DEV_NAME		"lps331ap"
+#define LPS22HB_PRESS_DEV_NAME		"lps22hb"
 
 /**
  * struct st_sensors_platform_data - default press platform data
drivers/iio/pressure/st_pressure_core.c +89 -4
index 9e9b72a..c0ff3bf 100644
@@ -113,6 +113,26 @@
 #define ST_PRESS_LPS25H_OUT_XL_ADDR		0x28
 #define ST_TEMP_LPS25H_OUT_L_ADDR		0x2b
 
+/* CUSTOM VALUES FOR LPS22HB SENSOR */
+#define ST_PRESS_LPS22HB_WAI_EXP		0xb1
+#define ST_PRESS_LPS22HB_ODR_ADDR		0x10
+#define ST_PRESS_LPS22HB_ODR_MASK		0x70
+#define ST_PRESS_LPS22HB_ODR_AVL_1HZ_VAL	0x01
+#define ST_PRESS_LPS22HB_ODR_AVL_10HZ_VAL	0x02
+#define ST_PRESS_LPS22HB_ODR_AVL_25HZ_VAL	0x03
+#define ST_PRESS_LPS22HB_ODR_AVL_50HZ_VAL	0x04
+#define ST_PRESS_LPS22HB_ODR_AVL_75HZ_VAL	0x05
+#define ST_PRESS_LPS22HB_PW_ADDR		0x10
+#define ST_PRESS_LPS22HB_PW_MASK		0x70
+#define ST_PRESS_LPS22HB_BDU_ADDR		0x10
+#define ST_PRESS_LPS22HB_BDU_MASK		0x02
+#define ST_PRESS_LPS22HB_DRDY_IRQ_ADDR		0x12
+#define ST_PRESS_LPS22HB_DRDY_IRQ_INT1_MASK	0x04
+#define ST_PRESS_LPS22HB_DRDY_IRQ_INT2_MASK	0x08
+#define ST_PRESS_LPS22HB_IHL_IRQ_ADDR		0x12
+#define ST_PRESS_LPS22HB_IHL_IRQ_MASK		0x80
+#define ST_PRESS_LPS22HB_MULTIREAD_BIT		true
+
 static const struct iio_chan_spec st_press_1_channels[] = {
 	{
 		.type = IIO_PRESSURE,
@@ -183,6 +203,27 @@ static const struct iio_chan_spec st_press_lps001wp_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(1)
 };
 
+static const struct iio_chan_spec st_press_lps22hb_channels[] = {
+	{
+		.type = IIO_PRESSURE,
+		.channel2 = IIO_NO_MOD,
+		.address = ST_PRESS_1_OUT_XL_ADDR,
+		.scan_index = 0,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 24,
+			.storagebits = 24,
+			.endianness = IIO_LE,
+		},
+		.info_mask_separate =
+			BIT(IIO_CHAN_INFO_RAW) |
+			BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.modified = 0,
+	},
+	IIO_CHAN_SOFT_TIMESTAMP(1)
+};
+
 static const struct st_sensor_settings st_press_sensors_settings[] = {
 	{
 		.wai = ST_PRESS_LPS331AP_WAI_EXP,
@@ -326,6 +367,51 @@ static const struct st_sensor_settings st_press_sensors_settings[] = {
 		.multi_read_bit = ST_PRESS_LPS25H_MULTIREAD_BIT,
 		.bootime = 2,
 	},
+	{
+		.wai = ST_PRESS_LPS22HB_WAI_EXP,
+		.wai_addr = ST_SENSORS_DEFAULT_WAI_ADDRESS,
+		.sensors_supported = {
+			[0] = LPS22HB_PRESS_DEV_NAME,
+		},
+		.ch = (struct iio_chan_spec *)st_press_lps22hb_channels,
+		.num_ch = ARRAY_SIZE(st_press_lps22hb_channels),
+		.odr = {
+			.addr = ST_PRESS_LPS22HB_ODR_ADDR,
+			.mask = ST_PRESS_LPS22HB_ODR_MASK,
+			.odr_avl = {
+				{ 1, ST_PRESS_LPS22HB_ODR_AVL_1HZ_VAL, },
+				{ 10, ST_PRESS_LPS22HB_ODR_AVL_10HZ_VAL, },
+				{ 25, ST_PRESS_LPS22HB_ODR_AVL_25HZ_VAL, },
+				{ 50, ST_PRESS_LPS22HB_ODR_AVL_50HZ_VAL, },
+				{ 75, ST_PRESS_LPS22HB_ODR_AVL_75HZ_VAL, },
+			},
+		},
+		.pw = {
+			.addr = ST_PRESS_LPS22HB_PW_ADDR,
+			.mask = ST_PRESS_LPS22HB_PW_MASK,
+			.value_off = ST_SENSORS_DEFAULT_POWER_OFF_VALUE,
+		},
+		.fs = {
+			.fs_avl = {
+				[0] = {
+					.num = ST_PRESS_FS_AVL_1260MB,
+					.gain = ST_PRESS_KPASCAL_NANO_SCALE,
+				},
+			},
+		},
+		.bdu = {
+			.addr = ST_PRESS_LPS22HB_BDU_ADDR,
+			.mask = ST_PRESS_LPS22HB_BDU_MASK,
+		},
+		.drdy_irq = {
+			.addr = ST_PRESS_LPS22HB_DRDY_IRQ_ADDR,
+			.mask_int1 = ST_PRESS_LPS22HB_DRDY_IRQ_INT1_MASK,
+			.mask_int2 = ST_PRESS_LPS22HB_DRDY_IRQ_INT2_MASK,
+			.addr_ihl = ST_PRESS_LPS22HB_IHL_IRQ_ADDR,
+			.mask_ihl = ST_PRESS_LPS22HB_IHL_IRQ_MASK,
+		},
+		.multi_read_bit = ST_PRESS_LPS22HB_MULTIREAD_BIT,
+	},
 };
 
 static int st_press_write_raw(struct iio_dev *indio_dev,
@@ -454,10 +540,9 @@ int st_press_common_probe(struct iio_dev *indio_dev)
 	indio_dev->channels = press_data->sensor_settings->ch;
 	indio_dev->num_channels = press_data->sensor_settings->num_ch;
 
-	if (press_data->sensor_settings->fs.addr != 0)
-		press_data->current_fullscale =
-			(struct st_sensor_fullscale_avl *)
-				&press_data->sensor_settings->fs.fs_avl[0];
+	press_data->current_fullscale =
+		(struct st_sensor_fullscale_avl *)
+			&press_data->sensor_settings->fs.fs_avl[0];
 
 	press_data->odr = press_data->sensor_settings->odr.odr_avl[0].hz;
 
drivers/iio/pressure/st_pressure_i2c.c +4 -0
index 8fcf976..ed18701 100644
@@ -32,6 +32,10 @@ static const struct of_device_id st_press_of_match[] = {
 		.compatible = "st,lps331ap-press",
 		.data = LPS331AP_PRESS_DEV_NAME,
 	},
+	{
+		.compatible = "st,lps22hb-press",
+		.data = LPS22HB_PRESS_DEV_NAME,
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, st_press_of_match);
drivers/iio/pressure/st_pressure_spi.c +1 -0
index 40c0692..5505080 100644
@@ -50,6 +50,7 @@ static const struct spi_device_id st_press_id_table[] = {
 	{ LPS001WP_PRESS_DEV_NAME },
 	{ LPS25H_PRESS_DEV_NAME },
 	{ LPS331AP_PRESS_DEV_NAME },
+	{ LPS22HB_PRESS_DEV_NAME },
 	{},
Quoted 9 lines

drivers/iio/pressure/st_pressure_spi.c static const struct spi_device_id st_press_id_table[] = { · line 56

Quoted 3 lines
drivers/iio/pressure/st_pressure_spi.c +0 -0
 };
 MODULE_DEVICE_TABLE(spi, st_press_id_table);

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#5 lore raw

RFC PATCH 2/9 iio:st_pressure: fix sampling gains

In reply to Gregor Boirie

Temperature channels report scaled samples in Celsius although expected as milli degree Celsius in Documentation/ABI/testing/sysfs-bus-iio. Gains are not implemented at all for LPS001WP pressure and temperature channels.

This patch ensures that proper offsets and scales are exposed to userpace for both pressure and temperature channels.
Also fix a NULL pointer exception when userspace reads content of sysfs scale attribute when gains are not defined.

---
 drivers/iio/pressure/st_pressure_core.c | 109 ++++++++++++++++++++++----------
 1 file changed, 75 insertions(+), 34 deletions(-)
drivers/iio/pressure/st_pressure_core.c +75 -34
index c0ff3bf..b8087b9 100644
@@ -28,21 +28,31 @@
 #include <linux/iio/common/st_sensors.h>
 #include "st_pressure.h"
 
+#define MCELSIUS_PER_CELSIUS			1000
+
+/* Default pressure sensitivity */
 #define ST_PRESS_LSB_PER_MBAR			4096UL
 #define ST_PRESS_KPASCAL_NANO_SCALE		(100000000UL / \
 						 ST_PRESS_LSB_PER_MBAR)
+
+/* Default temperature sensitivity */
 #define ST_PRESS_LSB_PER_CELSIUS		480UL
-#define ST_PRESS_CELSIUS_NANO_SCALE		(1000000000UL / \
-						 ST_PRESS_LSB_PER_CELSIUS)
+#define ST_PRESS_MILLI_CELSIUS_OFFSET		42500UL
+
 #define ST_PRESS_NUMBER_DATA_CHANNELS		1
 
 /* FULLSCALE */
+#define ST_PRESS_FS_AVL_1100MB			1100
 #define ST_PRESS_FS_AVL_1260MB			1260
 
 #define ST_PRESS_1_OUT_XL_ADDR			0x28
 #define ST_TEMP_1_OUT_L_ADDR			0x2b
 
-/* CUSTOM VALUES FOR LPS331AP SENSOR */
+/*
+ * CUSTOM VALUES FOR LPS331AP SENSOR
+ * See LPS331AP datasheet:
+ * http://www2.st.com/resource/en/datasheet/lps331ap.pdf
+ */
 #define ST_PRESS_LPS331AP_WAI_EXP		0xbb
 #define ST_PRESS_LPS331AP_ODR_ADDR		0x20
 #define ST_PRESS_LPS331AP_ODR_MASK		0x70
@@ -54,9 +64,6 @@
 #define ST_PRESS_LPS331AP_PW_MASK		0x80
 #define ST_PRESS_LPS331AP_FS_ADDR		0x23
 #define ST_PRESS_LPS331AP_FS_MASK		0x30
-#define ST_PRESS_LPS331AP_FS_AVL_1260_VAL	0x00
-#define ST_PRESS_LPS331AP_FS_AVL_1260_GAIN	ST_PRESS_KPASCAL_NANO_SCALE
-#define ST_PRESS_LPS331AP_FS_AVL_TEMP_GAIN	ST_PRESS_CELSIUS_NANO_SCALE
 #define ST_PRESS_LPS331AP_BDU_ADDR		0x20
 #define ST_PRESS_LPS331AP_BDU_MASK		0x04
 #define ST_PRESS_LPS331AP_DRDY_IRQ_ADDR		0x22
@@ -67,9 +74,19 @@
 #define ST_PRESS_LPS331AP_OD_IRQ_ADDR		0x22
 #define ST_PRESS_LPS331AP_OD_IRQ_MASK		0x40
 #define ST_PRESS_LPS331AP_MULTIREAD_BIT		true
-#define ST_PRESS_LPS331AP_TEMP_OFFSET		42500
 
-/* CUSTOM VALUES FOR LPS001WP SENSOR */
+/*
+ * CUSTOM VALUES FOR LPS001WP SENSOR
+ *
+ * See LPS001WP datasheet:
+ * http://www.st.com/web/en/resource/technical/document/datasheet/CD00289624.pdf
+ */
+
+/* LPS001WP pressure resolution */
+#define ST_PRESS_LPS001WP_LSB_PER_MBAR		16UL
+/* LPS001WP temperature resolution */
+#define ST_PRESS_LPS001WP_LSB_PER_CELSIUS	64UL
+
 #define ST_PRESS_LPS001WP_WAI_EXP		0xba
 #define ST_PRESS_LPS001WP_ODR_ADDR		0x20
 #define ST_PRESS_LPS001WP_ODR_MASK		0x30
@@ -78,13 +95,19 @@
 #define ST_PRESS_LPS001WP_ODR_AVL_13HZ_VAL	0x03
 #define ST_PRESS_LPS001WP_PW_ADDR		0x20
 #define ST_PRESS_LPS001WP_PW_MASK		0x40
+#define ST_PRESS_LPS001WP_FS_AVL_PRESS_GAIN \
+	(100000000UL / ST_PRESS_LPS001WP_LSB_PER_MBAR)
 #define ST_PRESS_LPS001WP_BDU_ADDR		0x20
 #define ST_PRESS_LPS001WP_BDU_MASK		0x04
 #define ST_PRESS_LPS001WP_MULTIREAD_BIT		true
 #define ST_PRESS_LPS001WP_OUT_L_ADDR		0x28
 #define ST_TEMP_LPS001WP_OUT_L_ADDR		0x2a
 
-/* CUSTOM VALUES FOR LPS25H SENSOR */
+/*
+ * CUSTOM VALUES FOR LPS25H SENSOR
+ * See LPS25H datasheet:
+ * http://www2.st.com/resource/en/datasheet/lps25h.pdf
+ */
 #define ST_PRESS_LPS25H_WAI_EXP			0xbd
 #define ST_PRESS_LPS25H_ODR_ADDR		0x20
 #define ST_PRESS_LPS25H_ODR_MASK		0x70
@@ -94,11 +117,6 @@
 #define ST_PRESS_LPS25H_ODR_AVL_25HZ_VAL	0x04
 #define ST_PRESS_LPS25H_PW_ADDR			0x20
 #define ST_PRESS_LPS25H_PW_MASK			0x80
-#define ST_PRESS_LPS25H_FS_ADDR			0x00
-#define ST_PRESS_LPS25H_FS_MASK			0x00
-#define ST_PRESS_LPS25H_FS_AVL_1260_VAL		0x00
-#define ST_PRESS_LPS25H_FS_AVL_1260_GAIN	ST_PRESS_KPASCAL_NANO_SCALE
-#define ST_PRESS_LPS25H_FS_AVL_TEMP_GAIN	ST_PRESS_CELSIUS_NANO_SCALE
 #define ST_PRESS_LPS25H_BDU_ADDR		0x20
 #define ST_PRESS_LPS25H_BDU_MASK		0x04
 #define ST_PRESS_LPS25H_DRDY_IRQ_ADDR		0x23
@@ -109,11 +127,14 @@
 #define ST_PRESS_LPS25H_OD_IRQ_ADDR		0x22
 #define ST_PRESS_LPS25H_OD_IRQ_MASK		0x40
 #define ST_PRESS_LPS25H_MULTIREAD_BIT		true
-#define ST_PRESS_LPS25H_TEMP_OFFSET		42500
 #define ST_PRESS_LPS25H_OUT_XL_ADDR		0x28
 #define ST_TEMP_LPS25H_OUT_L_ADDR		0x2b
 
-/* CUSTOM VALUES FOR LPS22HB SENSOR */
+/*
+ * CUSTOM VALUES FOR LPS22HB SENSOR
+ * See LPS22HB datasheet:
+ * http://www2.st.com/resource/en/datasheet/lps22hb.pdf
+ */
 #define ST_PRESS_LPS22HB_WAI_EXP		0xb1
 #define ST_PRESS_LPS22HB_ODR_ADDR		0x10
 #define ST_PRESS_LPS22HB_ODR_MASK		0x70
@@ -181,7 +202,9 @@ static const struct iio_chan_spec st_press_lps001wp_channels[] = {
 			.storagebits = 16,
 			.endianness = IIO_LE,
 		},
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+		.info_mask_separate =
+			BIT(IIO_CHAN_INFO_RAW) |
+			BIT(IIO_CHAN_INFO_SCALE),
 		.modified = 0,
 	},
 	{
@@ -197,7 +220,7 @@ static const struct iio_chan_spec st_press_lps001wp_channels[] = {
 		},
 		.info_mask_separate =
 			BIT(IIO_CHAN_INFO_RAW) |
-			BIT(IIO_CHAN_INFO_OFFSET),
+			BIT(IIO_CHAN_INFO_SCALE),
 		.modified = 0,
 	},
 	IIO_CHAN_SOFT_TIMESTAMP(1)
@@ -253,11 +276,14 @@ static const struct st_sensor_settings st_press_sensors_settings[] = {
 			.addr = ST_PRESS_LPS331AP_FS_ADDR,
 			.mask = ST_PRESS_LPS331AP_FS_MASK,
 			.fs_avl = {
+				/*
+				 * Pressure and temperature sensitivity values
+				 * as defined in table 3 of LPS331AP datasheet.
+				 */
 				[0] = {
 					.num = ST_PRESS_FS_AVL_1260MB,
-					.value = ST_PRESS_LPS331AP_FS_AVL_1260_VAL,
-					.gain = ST_PRESS_LPS331AP_FS_AVL_1260_GAIN,
-					.gain2 = ST_PRESS_LPS331AP_FS_AVL_TEMP_GAIN,
+					.gain = ST_PRESS_KPASCAL_NANO_SCALE,
+					.gain2 = ST_PRESS_LSB_PER_CELSIUS,
 				},
 			},
 		},
@@ -302,7 +328,17 @@ static const struct st_sensor_settings st_press_sensors_settings[] = {
 			.value_off = ST_SENSORS_DEFAULT_POWER_OFF_VALUE,
 		},
 		.fs = {
-			.addr = 0,
+			.fs_avl = {
+				/*
+				 * Pressure and temperature resolution values
+				 * as defined in table 3 of LPS001WP datasheet.
+				 */
+				[0] = {
+					.num = ST_PRESS_FS_AVL_1100MB,
+					.gain = ST_PRESS_LPS001WP_FS_AVL_PRESS_GAIN,
+					.gain2 = ST_PRESS_LPS001WP_LSB_PER_CELSIUS,
+				},
+			},
 		},
 		.bdu = {
 			.addr = ST_PRESS_LPS001WP_BDU_ADDR,
@@ -339,14 +375,15 @@ static const struct st_sensor_settings st_press_sensors_settings[] = {
 			.value_off = ST_SENSORS_DEFAULT_POWER_OFF_VALUE,
 		},
 		.fs = {
-			.addr = ST_PRESS_LPS25H_FS_ADDR,
-			.mask = ST_PRESS_LPS25H_FS_MASK,
 			.fs_avl = {
+				/*
+				 * Pressure and temperature sensitivity values
+				 * as defined in table 3 of LPS25H datasheet.
+				 */
 				[0] = {
 					.num = ST_PRESS_FS_AVL_1260MB,
-					.value = ST_PRESS_LPS25H_FS_AVL_1260_VAL,
-					.gain = ST_PRESS_LPS25H_FS_AVL_1260_GAIN,
-					.gain2 = ST_PRESS_LPS25H_FS_AVL_TEMP_GAIN,
+					.gain = ST_PRESS_KPASCAL_NANO_SCALE,
+					.gain2 = ST_PRESS_LSB_PER_CELSIUS,
 				},
 			},
 		},
@@ -393,6 +430,10 @@ static const struct st_sensor_settings st_press_sensors_settings[] = {
 		},
 		.fs = {
 			.fs_avl = {
+				/*
+				 * Sensitivity values as defined in table 3 of
+				 * LPS22HB datasheet.
+				 */
 				[0] = {
 					.num = ST_PRESS_FS_AVL_1260MB,
 					.gain = ST_PRESS_KPASCAL_NANO_SCALE,
@@ -450,26 +491,26 @@ static int st_press_read_raw(struct iio_dev *indio_dev,
 
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
-		*val = 0;
-
 		switch (ch->type) {
 		case IIO_PRESSURE:
+			*val = 0;
 			*val2 = press_data->current_fullscale->gain;
-			break;
+			return IIO_VAL_INT_PLUS_NANO;
 		case IIO_TEMP:
+			*val = MCELSIUS_PER_CELSIUS;
 			*val2 = press_data->current_fullscale->gain2;
-			break;
+			return IIO_VAL_FRACTIONAL;
 		default:
 			err = -EINVAL;
 			goto read_error;
 		}
 
-		return IIO_VAL_INT_PLUS_NANO;
 	case IIO_CHAN_INFO_OFFSET:
 		switch (ch->type) {
 		case IIO_TEMP:
-			*val = 425;
-			*val2 = 10;
+			*val = ST_PRESS_MILLI_CELSIUS_OFFSET *
+			       press_data->current_fullscale->gain2;
+			*val2 = MCELSIUS_PER_CELSIUS;
 			break;
 		default:
 			err = -EINVAL;
Signature · 1 line
2.1.4
maintainer
#6 lore raw

Re: [RFC PATCH v1 2/9] iio:st_pressure: fix sampling gains

Quoted 11 lines · Gregor Boirie

Temperature channels report scaled samples in Celsius although expected as milli degree Celsius in Documentation/ABI/testing/sysfs-bus-iio. Gains are not implemented at all for LPS001WP pressure and temperature channels.

This patch ensures that proper offsets and scales are exposed to userpace for both pressure and temperature channels.
Also fix a NULL pointer exception when userspace reads content of sysfs scale attribute when gains are not defined.

Hi Gregor,

This patch could do with a few comments to explain the values of the various constants. Personally I'm really beginning to dislike the way these drivers have simple numeric values wrapped up in defines at the top. Would be somewhat clearer if they were just inline.

Defines are fine for 'magic' numbers, but these are just obscuring the straight forward values. Anyhow, not one to fix up in this series!

I'm happy the fixes and new elements in here are correct. However, we really ought to be pushing the fix elements to stable trees so they'll need to get separated out from the very minor parts that were added in the previous patch.

Also, we don't want things like updating datasheet paths in a fix patch. It's useful to do so but that wants to go in as a separate patch.

I've stripped out the relevant bits for the fix patch and applied it to the fixes-togreg-post-rc1 branch of iio.git. Please keep in mind for future 'fix' patches - the aim is the minimum changes possible.

Hope you don't mind rebasing the other odds and ends on top of this.

Thanks,

Jonathan

Quoted 55 lines
---
 drivers/iio/pressure/st_pressure_core.c | 109 ++++++++++++++++++++++----------
 1 file changed, 75 insertions(+), 34 deletions(-)
drivers/iio/pressure/st_pressure_core.c +13 -6
index c0ff3bf..b8087b9 100644
@@ -28,21 +28,31 @@
 #include <linux/iio/common/st_sensors.h>
 #include "st_pressure.h"
 
+#define MCELSIUS_PER_CELSIUS			1000
+
+/* Default pressure sensitivity */
 #define ST_PRESS_LSB_PER_MBAR			4096UL
 #define ST_PRESS_KPASCAL_NANO_SCALE		(100000000UL / \
 						 ST_PRESS_LSB_PER_MBAR)
+
+/* Default temperature sensitivity */
 #define ST_PRESS_LSB_PER_CELSIUS		480UL
-#define ST_PRESS_CELSIUS_NANO_SCALE		(1000000000UL / \
-						 ST_PRESS_LSB_PER_CELSIUS)
+#define ST_PRESS_MILLI_CELSIUS_OFFSET		42500UL
+
 #define ST_PRESS_NUMBER_DATA_CHANNELS		1
 
 /* FULLSCALE */
+#define ST_PRESS_FS_AVL_1100MB			1100
 #define ST_PRESS_FS_AVL_1260MB			1260
 
 #define ST_PRESS_1_OUT_XL_ADDR			0x28
 #define ST_TEMP_1_OUT_L_ADDR			0x2b
 
-/* CUSTOM VALUES FOR LPS331AP SENSOR */
+/*
+ * CUSTOM VALUES FOR LPS331AP SENSOR
+ * See LPS331AP datasheet:
+ * http://www2.st.com/resource/en/datasheet/lps331ap.pdf
+ */
 #define ST_PRESS_LPS331AP_WAI_EXP		0xbb
 #define ST_PRESS_LPS331AP_ODR_ADDR		0x20
 #define ST_PRESS_LPS331AP_ODR_MASK		0x70
@@ -54,9 +64,6 @@
 #define ST_PRESS_LPS331AP_PW_MASK		0x80
 #define ST_PRESS_LPS331AP_FS_ADDR		0x23
 #define ST_PRESS_LPS331AP_FS_MASK		0x30
-#define ST_PRESS_LPS331AP_FS_AVL_1260_VAL	0x00
-#define ST_PRESS_LPS331AP_FS_AVL_1260_GAIN	ST_PRESS_KPASCAL_NANO_SCALE
-#define ST_PRESS_LPS331AP_FS_AVL_TEMP_GAIN	ST_PRESS_CELSIUS_NANO_SCALE
 #define ST_PRESS_LPS331AP_BDU_ADDR		0x20
 #define ST_PRESS_LPS331AP_BDU_MASK		0x04
 #define ST_PRESS_LPS331AP_DRDY_IRQ_ADDR		0x22
@@ -67,9 +74,19 @@
 #define ST_PRESS_LPS331AP_OD_IRQ_ADDR		0x22

drivers/iio/pressure/st_pressure_core.c line 82

Quoted 10 lines
drivers/iio/pressure/st_pressure_core.c +5 -2
 #define ST_PRESS_LPS331AP_OD_IRQ_MASK		0x40
 #define ST_PRESS_LPS331AP_MULTIREAD_BIT		true
-#define ST_PRESS_LPS331AP_TEMP_OFFSET		42500
 
-/* CUSTOM VALUES FOR LPS001WP SENSOR */
+/*
+ * CUSTOM VALUES FOR LPS001WP SENSOR
+ *
+ * See LPS001WP datasheet:
+ * http://www.st.com/web/en/resource/technical/document/datasheet/CD00289624.pdf

They seem to have changed this link..

Quoted 11 lines
drivers/iio/pressure/st_pressure_core.c +7 -0
+ */
+
+/* LPS001WP pressure resolution */
+#define ST_PRESS_LPS001WP_LSB_PER_MBAR		16UL
+/* LPS001WP temperature resolution */
+#define ST_PRESS_LPS001WP_LSB_PER_CELSIUS	64UL
+
 #define ST_PRESS_LPS001WP_WAI_EXP		0xba
 #define ST_PRESS_LPS001WP_ODR_ADDR		0x20
 #define ST_PRESS_LPS001WP_ODR_MASK		0x30

drivers/iio/pressure/st_pressure_core.c line 99

Quoted 5 lines
drivers/iio/pressure/st_pressure_core.c +2 -0
@@ -78,13 +95,19 @@
 #define ST_PRESS_LPS001WP_ODR_AVL_13HZ_VAL	0x03
 #define ST_PRESS_LPS001WP_PW_ADDR		0x20
 #define ST_PRESS_LPS001WP_PW_MASK		0x40
+#define ST_PRESS_LPS001WP_FS_AVL_PRESS_GAIN \
+	(100000000UL / ST_PRESS_LPS001WP_LSB_PER_MBAR)

This took me a few moments. 100 million / 16?

Our pressure units are kilopascals - I would imagine this
is getting used as the gain value which would be expected to be the same as the value for each LSB which I think should bbe 100 / 16 or 0.0625 ah - you are then using int + nano so your
value is 0.006250000 which is correct.

Quoted 16 lines
drivers/iio/pressure/st_pressure_core.c +5 -1
 #define ST_PRESS_LPS001WP_BDU_ADDR		0x20
 #define ST_PRESS_LPS001WP_BDU_MASK		0x04
 #define ST_PRESS_LPS001WP_MULTIREAD_BIT		true
 #define ST_PRESS_LPS001WP_OUT_L_ADDR		0x28
 #define ST_TEMP_LPS001WP_OUT_L_ADDR		0x2a
 
-/* CUSTOM VALUES FOR LPS25H SENSOR */
+/*
+ * CUSTOM VALUES FOR LPS25H SENSOR
+ * See LPS25H datasheet:
+ * http://www2.st.com/resource/en/datasheet/lps25h.pdf
+ */
 #define ST_PRESS_LPS25H_WAI_EXP			0xbd
 #define ST_PRESS_LPS25H_ODR_ADDR		0x20
 #define ST_PRESS_LPS25H_ODR_MASK		0x70

drivers/iio/pressure/st_pressure_core.c line 119

Quoted 3 lines
drivers/iio/pressure/st_pressure_core.c +0 -0
@@ -94,11 +117,6 @@
 #define ST_PRESS_LPS25H_ODR_AVL_25HZ_VAL	0x04
 #define ST_PRESS_LPS25H_PW_ADDR			0x20
 #define ST_PRESS_LPS25H_PW_MASK			0x80

So these are dropped because it isn't controllable?

Quoted 149 lines
drivers/iio/pressure/st_pressure_core.c +43 -25
-#define ST_PRESS_LPS25H_FS_ADDR			0x00
-#define ST_PRESS_LPS25H_FS_MASK			0x00
-#define ST_PRESS_LPS25H_FS_AVL_1260_VAL		0x00
-#define ST_PRESS_LPS25H_FS_AVL_1260_GAIN	ST_PRESS_KPASCAL_NANO_SCALE
-#define ST_PRESS_LPS25H_FS_AVL_TEMP_GAIN	ST_PRESS_CELSIUS_NANO_SCALE
 #define ST_PRESS_LPS25H_BDU_ADDR		0x20
 #define ST_PRESS_LPS25H_BDU_MASK		0x04
 #define ST_PRESS_LPS25H_DRDY_IRQ_ADDR		0x23
@@ -109,11 +127,14 @@
 #define ST_PRESS_LPS25H_OD_IRQ_ADDR		0x22
 #define ST_PRESS_LPS25H_OD_IRQ_MASK		0x40
 #define ST_PRESS_LPS25H_MULTIREAD_BIT		true
-#define ST_PRESS_LPS25H_TEMP_OFFSET		42500
 #define ST_PRESS_LPS25H_OUT_XL_ADDR		0x28
 #define ST_TEMP_LPS25H_OUT_L_ADDR		0x2b
 
-/* CUSTOM VALUES FOR LPS22HB SENSOR */
+/*
+ * CUSTOM VALUES FOR LPS22HB SENSOR
+ * See LPS22HB datasheet:
+ * http://www2.st.com/resource/en/datasheet/lps22hb.pdf
+ */
 #define ST_PRESS_LPS22HB_WAI_EXP		0xb1
 #define ST_PRESS_LPS22HB_ODR_ADDR		0x10
 #define ST_PRESS_LPS22HB_ODR_MASK		0x70
@@ -181,7 +202,9 @@ static const struct iio_chan_spec st_press_lps001wp_channels[] = {
 			.storagebits = 16,
 			.endianness = IIO_LE,
 		},
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+		.info_mask_separate =
+			BIT(IIO_CHAN_INFO_RAW) |
+			BIT(IIO_CHAN_INFO_SCALE),
 		.modified = 0,
 	},
 	{
@@ -197,7 +220,7 @@ static const struct iio_chan_spec st_press_lps001wp_channels[] = {
 		},
 		.info_mask_separate =
 			BIT(IIO_CHAN_INFO_RAW) |
-			BIT(IIO_CHAN_INFO_OFFSET),
+			BIT(IIO_CHAN_INFO_SCALE),
 		.modified = 0,
 	},
 	IIO_CHAN_SOFT_TIMESTAMP(1)
@@ -253,11 +276,14 @@ static const struct st_sensor_settings st_press_sensors_settings[] = {
 			.addr = ST_PRESS_LPS331AP_FS_ADDR,
 			.mask = ST_PRESS_LPS331AP_FS_MASK,
 			.fs_avl = {
+				/*
+				 * Pressure and temperature sensitivity values
+				 * as defined in table 3 of LPS331AP datasheet.
+				 */
 				[0] = {
 					.num = ST_PRESS_FS_AVL_1260MB,
-					.value = ST_PRESS_LPS331AP_FS_AVL_1260_VAL,
-					.gain = ST_PRESS_LPS331AP_FS_AVL_1260_GAIN,
-					.gain2 = ST_PRESS_LPS331AP_FS_AVL_TEMP_GAIN,
+					.gain = ST_PRESS_KPASCAL_NANO_SCALE,
+					.gain2 = ST_PRESS_LSB_PER_CELSIUS,
 				},
 			},
 		},
@@ -302,7 +328,17 @@ static const struct st_sensor_settings st_press_sensors_settings[] = {
 			.value_off = ST_SENSORS_DEFAULT_POWER_OFF_VALUE,
 		},
 		.fs = {
-			.addr = 0,
+			.fs_avl = {
+				/*
+				 * Pressure and temperature resolution values
+				 * as defined in table 3 of LPS001WP datasheet.
+				 */
+				[0] = {
+					.num = ST_PRESS_FS_AVL_1100MB,
+					.gain = ST_PRESS_LPS001WP_FS_AVL_PRESS_GAIN,
+					.gain2 = ST_PRESS_LPS001WP_LSB_PER_CELSIUS,
+				},
+			},
 		},
 		.bdu = {
 			.addr = ST_PRESS_LPS001WP_BDU_ADDR,
@@ -339,14 +375,15 @@ static const struct st_sensor_settings st_press_sensors_settings[] = {
 			.value_off = ST_SENSORS_DEFAULT_POWER_OFF_VALUE,
 		},
 		.fs = {
-			.addr = ST_PRESS_LPS25H_FS_ADDR,
-			.mask = ST_PRESS_LPS25H_FS_MASK,
 			.fs_avl = {
+				/*
+				 * Pressure and temperature sensitivity values
+				 * as defined in table 3 of LPS25H datasheet.
+				 */
 				[0] = {
 					.num = ST_PRESS_FS_AVL_1260MB,
-					.value = ST_PRESS_LPS25H_FS_AVL_1260_VAL,
-					.gain = ST_PRESS_LPS25H_FS_AVL_1260_GAIN,
-					.gain2 = ST_PRESS_LPS25H_FS_AVL_TEMP_GAIN,
+					.gain = ST_PRESS_KPASCAL_NANO_SCALE,
+					.gain2 = ST_PRESS_LSB_PER_CELSIUS,
 				},
 			},
 		},
@@ -393,6 +430,10 @@ static const struct st_sensor_settings st_press_sensors_settings[] = {
 		},
 		.fs = {
 			.fs_avl = {
+				/*
+				 * Sensitivity values as defined in table 3 of
+				 * LPS22HB datasheet.
+				 */
 				[0] = {
 					.num = ST_PRESS_FS_AVL_1260MB,
 					.gain = ST_PRESS_KPASCAL_NANO_SCALE,
@@ -450,26 +491,26 @@ static int st_press_read_raw(struct iio_dev *indio_dev,
 
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
-		*val = 0;
-
 		switch (ch->type) {
 		case IIO_PRESSURE:
+			*val = 0;
 			*val2 = press_data->current_fullscale->gain;
-			break;
+			return IIO_VAL_INT_PLUS_NANO;
 		case IIO_TEMP:
+			*val = MCELSIUS_PER_CELSIUS;
 			*val2 = press_data->current_fullscale->gain2;
-			break;
+			return IIO_VAL_FRACTIONAL;
 		default:
 			err = -EINVAL;
 			goto read_error;
 		}
 
-		return IIO_VAL_INT_PLUS_NANO;
 	case IIO_CHAN_INFO_OFFSET:
 		switch (ch->type) {
 		case IIO_TEMP:
-			*val = 425;
-			*val2 = 10;
+			*val = ST_PRESS_MILLI_CELSIUS_OFFSET *
+			       press_data->current_fullscale->gain2;
+			*val2 = MCELSIUS_PER_CELSIUS;
 			break;
 		default:
 			err = -EINVAL;
#7 lore raw

Re: [RFC PATCH v1 2/9] iio:st_pressure: fix sampling gains

Quoted 4 lines · Jonathan Cameron

Personally I'm really beginning to
dislike the way these drivers have simple numeric values wrapped up in defines at the top. Would be somewhat clearer if they were just inline.

I agree, I never liked it either, just kept with it since the old code looked like that.

Do you want gigantic patches fixing it for all sensor types?

Yours,
Linus Walleij

maintainer
#8 lore raw

Re: [RFC PATCH v1 2/9] iio:st_pressure: fix sampling gains

Quoted 11 lines · Linus Walleij
Quoted 4 lines · Jonathan Cameron

Personally I'm really beginning to
dislike the way these drivers have simple numeric values wrapped up in defines at the top. Would be somewhat clearer if they were just inline.

I agree, I never liked it either, just kept with it since the old code looked like that.

Do you want gigantic patches fixing it for all sensor types?

I think we want agreement from Denis on this one as it's a major change to a driver he's been pretty good about maintaining.

Jonathan

Quoted 8 lines

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

#9 lore raw

RFC PATCH 3/9 iio:st_pressure: lps22hb temperature support

In reply to Gregor Boirie

Implement lps22hb temperature sampling channel.

---
 drivers/iio/pressure/st_pressure_core.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)
drivers/iio/pressure/st_pressure_core.c +24 -2
index b8087b9..24754eed 100644
@@ -135,6 +135,10 @@
  * See LPS22HB datasheet:
  * http://www2.st.com/resource/en/datasheet/lps22hb.pdf
  */
+
+/* LPS22HB temperature sensitivity */
+#define ST_PRESS_LPS22HB_LSB_PER_CELSIUS	100UL
+
 #define ST_PRESS_LPS22HB_WAI_EXP		0xb1
 #define ST_PRESS_LPS22HB_ODR_ADDR		0x10
 #define ST_PRESS_LPS22HB_ODR_MASK		0x70
@@ -244,6 +248,23 @@ static const struct iio_chan_spec st_press_lps22hb_channels[] = {
 		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
 		.modified = 0,
 	},
+	{
+		.type = IIO_TEMP,
+		.channel2 = IIO_NO_MOD,
+		.address = ST_TEMP_1_OUT_L_ADDR,
+		.scan_index = -1,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 16,
+			.storagebits = 16,
+			.endianness = IIO_LE,
+		},
+		.info_mask_separate =
+			BIT(IIO_CHAN_INFO_RAW) |
+			BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.modified = 0,
+	},
 	IIO_CHAN_SOFT_TIMESTAMP(1)
 };
 
@@ -431,12 +452,13 @@ static const struct st_sensor_settings st_press_sensors_settings[] = {
 		.fs = {
 			.fs_avl = {
 				/*
-				 * Sensitivity values as defined in table 3 of
-				 * LPS22HB datasheet.
+				 * Pressure and temperature sensitivity values
+				 * as defined in table 3 of LPS22HB datasheet.
 				 */
 				[0] = {
 					.num = ST_PRESS_FS_AVL_1260MB,
 					.gain = ST_PRESS_KPASCAL_NANO_SCALE,
+					.gain2 = ST_PRESS_LPS22HB_LSB_PER_CELSIUS,
 				},
 			},
 		},
Signature · 1 line
2.1.4
maintainer
#10 lore raw

Re: [RFC PATCH v1 3/9] iio:st_pressure: lps22hb temperature support

Quoted 3 lines · Gregor Boirie

Implement lps22hb temperature sampling channel.

Unfortunately we now have a dependency of this working it's way in as a fix so this will have to wait now for that to make it's way into the our upstream tree.

At that time a rebased version would be great. If you don't have time just poke me when it looks like everything is in place and I'll do the patch mangling necessary.

Thanks,

Jonathan

Quoted 60 lines
---
 drivers/iio/pressure/st_pressure_core.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)
drivers/iio/pressure/st_pressure_core.c +24 -2
index b8087b9..24754eed 100644
@@ -135,6 +135,10 @@
  * See LPS22HB datasheet:
  * http://www2.st.com/resource/en/datasheet/lps22hb.pdf
  */
+
+/* LPS22HB temperature sensitivity */
+#define ST_PRESS_LPS22HB_LSB_PER_CELSIUS	100UL
+
 #define ST_PRESS_LPS22HB_WAI_EXP		0xb1
 #define ST_PRESS_LPS22HB_ODR_ADDR		0x10
 #define ST_PRESS_LPS22HB_ODR_MASK		0x70
@@ -244,6 +248,23 @@ static const struct iio_chan_spec st_press_lps22hb_channels[] = {
 		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
 		.modified = 0,
 	},
+	{
+		.type = IIO_TEMP,
+		.channel2 = IIO_NO_MOD,
+		.address = ST_TEMP_1_OUT_L_ADDR,
+		.scan_index = -1,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 16,
+			.storagebits = 16,
+			.endianness = IIO_LE,
+		},
+		.info_mask_separate =
+			BIT(IIO_CHAN_INFO_RAW) |
+			BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.modified = 0,
+	},
 	IIO_CHAN_SOFT_TIMESTAMP(1)
 };
 
@@ -431,12 +452,13 @@ static const struct st_sensor_settings st_press_sensors_settings[] = {
 		.fs = {
 			.fs_avl = {
 				/*
-				 * Sensitivity values as defined in table 3 of
-				 * LPS22HB datasheet.
+				 * Pressure and temperature sensitivity values
+				 * as defined in table 3 of LPS22HB datasheet.
 				 */
 				[0] = {
 					.num = ST_PRESS_FS_AVL_1260MB,
 					.gain = ST_PRESS_KPASCAL_NANO_SCALE,
+					.gain2 = ST_PRESS_LPS22HB_LSB_PER_CELSIUS,
 				},
 			},
 		},
#11 lore raw

RFC PATCH 4/9 iio:st_sensors: align on storagebits boundaries

In reply to Gregor Boirie

Triggered buffering memory accesses are not aligned on per channel storagebits boundaries. Fix this by reading each channel individually to ensure proper alignment.
Note that this patch drops the earlier optimisation that packs multiple channels reading into a single data block transaction when their respective I2C register addresses are contiguous.

---
 drivers/iio/common/st_sensors/st_sensors_buffer.c | 38 ++++++++++-------------
 drivers/iio/common/st_sensors/st_sensors_core.c   |  2 +-
 2 files changed, 18 insertions(+), 22 deletions(-)
drivers/iio/common/st_sensors/st_sensors_buffer.c +17 -21
index c558985..a7f1ced 100644
@@ -21,33 +21,29 @@
 
 #include <linux/iio/common/st_sensors.h>
 
-
 int st_sensors_get_buffer_element(struct iio_dev *indio_dev, u8 *buf)
 {
-	int i, len;
-	int total = 0;
+	unsigned int cid;
 	struct st_sensor_data *sdata = iio_priv(indio_dev);
-	unsigned int num_data_channels = sdata->num_data_channels;
-
-	for (i = 0; i < num_data_channels; i++) {
-		unsigned int bytes_to_read;
-
-		if (test_bit(i, indio_dev->active_scan_mask)) {
-			bytes_to_read = indio_dev->channels[i].scan_type.storagebits >> 3;
-			len = sdata->tf->read_multiple_byte(&sdata->tb,
-				sdata->dev, indio_dev->channels[i].address,
-				bytes_to_read,
-				buf + total, sdata->multiread_bit);
-
-			if (len < bytes_to_read)
-				return -EIO;
 
-			/* Advance the buffer pointer */
-			total += len;
-		}
+	for_each_set_bit(cid, indio_dev->active_scan_mask,
+			 sdata->num_data_channels) {
+		const struct iio_chan_spec *chan = &indio_dev->channels[cid];
+		int sb = chan->scan_type.storagebits / 8;
+		int rb = chan->scan_type.realbits / 8;
+		int err;
+
+		buf = PTR_ALIGN(buf, sb);
+		err = sdata->tf->read_multiple_byte(&sdata->tb, sdata->dev,
+						    chan->address, rb, buf,
+						    sdata->multiread_bit);
+		if (err != rb)
+			return (err < 0) ? err : -EIO;
+
+		buf += sb;
 	}
 
-	return total;
+	return 0;
 }
 EXPORT_SYMBOL(st_sensors_get_buffer_element);
 
drivers/iio/common/st_sensors/st_sensors_core.c +1 -1
index dffe006..6901c7f 100644
@@ -463,7 +463,7 @@ static int st_sensors_read_axis_data(struct iio_dev *indio_dev,
 	int err;
 	u8 *outdata;
 	struct st_sensor_data *sdata = iio_priv(indio_dev);
-	unsigned int byte_for_channel = ch->scan_type.storagebits >> 3;
+	unsigned int byte_for_channel = ch->scan_type.realbits >> 3;
 
 	outdata = kmalloc(byte_for_channel, GFP_KERNEL);
 	if (!outdata)
Signature · 1 line
2.1.4
maintainer
#12 lore raw

Re: [RFC PATCH v1 4/9] iio:st_sensors: align on storagebits boundaries

Quoted 6 lines · Gregor Boirie

Triggered buffering memory accesses are not aligned on per channel storagebits boundaries. Fix this by reading each channel individually to ensure proper alignment.
Note that this patch drops the earlier optimisation that packs multiple channels reading into a single data block transaction when their respective I2C register addresses are contiguous.

I wonder how much effect this change has on the read out rate and whether repacking in software would be significantly quicker than the multiple transfers...

Quoted 76 lines
---
 drivers/iio/common/st_sensors/st_sensors_buffer.c | 38 ++++++++++-------------
 drivers/iio/common/st_sensors/st_sensors_core.c   |  2 +-
 2 files changed, 18 insertions(+), 22 deletions(-)
drivers/iio/common/st_sensors/st_sensors_buffer.c +17 -21
index c558985..a7f1ced 100644
@@ -21,33 +21,29 @@
 
 #include <linux/iio/common/st_sensors.h>
 
-
 int st_sensors_get_buffer_element(struct iio_dev *indio_dev, u8 *buf)
 {
-	int i, len;
-	int total = 0;
+	unsigned int cid;
 	struct st_sensor_data *sdata = iio_priv(indio_dev);
-	unsigned int num_data_channels = sdata->num_data_channels;
-
-	for (i = 0; i < num_data_channels; i++) {
-		unsigned int bytes_to_read;
-
-		if (test_bit(i, indio_dev->active_scan_mask)) {
-			bytes_to_read = indio_dev->channels[i].scan_type.storagebits >> 3;
-			len = sdata->tf->read_multiple_byte(&sdata->tb,
-				sdata->dev, indio_dev->channels[i].address,
-				bytes_to_read,
-				buf + total, sdata->multiread_bit);
-
-			if (len < bytes_to_read)
-				return -EIO;
 
-			/* Advance the buffer pointer */
-			total += len;
-		}
+	for_each_set_bit(cid, indio_dev->active_scan_mask,
+			 sdata->num_data_channels) {
+		const struct iio_chan_spec *chan = &indio_dev->channels[cid];
+		int sb = chan->scan_type.storagebits / 8;
+		int rb = chan->scan_type.realbits / 8;
+		int err;
+
+		buf = PTR_ALIGN(buf, sb);
+		err = sdata->tf->read_multiple_byte(&sdata->tb, sdata->dev,
+						    chan->address, rb, buf,
+						    sdata->multiread_bit);
+		if (err != rb)
+			return (err < 0) ? err : -EIO;
+
+		buf += sb;
 	}
 
-	return total;
+	return 0;
 }
 EXPORT_SYMBOL(st_sensors_get_buffer_element);
 
drivers/iio/common/st_sensors/st_sensors_core.c +1 -1
index dffe006..6901c7f 100644
@@ -463,7 +463,7 @@ static int st_sensors_read_axis_data(struct iio_dev *indio_dev,
 	int err;
 	u8 *outdata;
 	struct st_sensor_data *sdata = iio_priv(indio_dev);
-	unsigned int byte_for_channel = ch->scan_type.storagebits >> 3;
+	unsigned int byte_for_channel = ch->scan_type.realbits >> 3;
 
 	outdata = kmalloc(byte_for_channel, GFP_KERNEL);
 	if (!outdata)
maintainer
#13 lore raw

Re: [RFC PATCH v1 4/9] iio:st_sensors: align on storagebits boundaries

Quoted 10 lines · Jonathan Cameron
Quoted 6 lines · Gregor Boirie

Triggered buffering memory accesses are not aligned on per channel storagebits boundaries. Fix this by reading each channel individually to ensure proper alignment.
Note that this patch drops the earlier optimisation that packs multiple channels reading into a single data block transaction when their respective I2C register addresses are contiguous.

I wonder how much effect this change has on the read out rate and whether repacking in software would be significantly quicker than the multiple transfers...

Thinking more on this I'm really not happy with this solution - may well kill data rates on some chips, to deal with this one unaligned case. Gregor, can you give repacking in software a go?
Won't be that hard I think... Be cynical and just memove the data a byte if you hit a 24bit channel.

We could do this in the core demux but then we'd have to do something a bit clever with the reported buffer element type so it looked different to the driver and the buffer.

Lars, IIRC you've messed with that corner of the code more recently than me. Do you think doing unaligned data smashing in the demux would be sensible?

Can see we'd have to:
* expand the 'does the demux have to do anything' case to catch
this one (not too hard).
* make the demux table builder force size constraints to power of two.
* either work out the shift to export to userspace (nasty) or make sure we packed the data right depending on endianness so there wasn't any change to the shift.
All this stuff occurs in the slow patch (table build) - it would look no worse than normal demux (some memcpys) in the fast path.

Good idea or not? What do people think.

I'm might mock this up in iio_dummy if I get bored and find out how bad it really is...

Jonathan

Quoted 83 lines
Quoted 69 lines
---
 drivers/iio/common/st_sensors/st_sensors_buffer.c | 38 ++++++++++-------------
 drivers/iio/common/st_sensors/st_sensors_core.c   |  2 +-
 2 files changed, 18 insertions(+), 22 deletions(-)
drivers/iio/common/st_sensors/st_sensors_buffer.c +17 -21
index c558985..a7f1ced 100644
@@ -21,33 +21,29 @@
 
 #include <linux/iio/common/st_sensors.h>
 
-
 int st_sensors_get_buffer_element(struct iio_dev *indio_dev, u8 *buf)
 {
-	int i, len;
-	int total = 0;
+	unsigned int cid;
 	struct st_sensor_data *sdata = iio_priv(indio_dev);
-	unsigned int num_data_channels = sdata->num_data_channels;
-
-	for (i = 0; i < num_data_channels; i++) {
-		unsigned int bytes_to_read;
-
-		if (test_bit(i, indio_dev->active_scan_mask)) {
-			bytes_to_read = indio_dev->channels[i].scan_type.storagebits >> 3;
-			len = sdata->tf->read_multiple_byte(&sdata->tb,
-				sdata->dev, indio_dev->channels[i].address,
-				bytes_to_read,
-				buf + total, sdata->multiread_bit);
-
-			if (len < bytes_to_read)
-				return -EIO;
 
-			/* Advance the buffer pointer */
-			total += len;
-		}
+	for_each_set_bit(cid, indio_dev->active_scan_mask,
+			 sdata->num_data_channels) {
+		const struct iio_chan_spec *chan = &indio_dev->channels[cid];
+		int sb = chan->scan_type.storagebits / 8;
+		int rb = chan->scan_type.realbits / 8;
+		int err;
+
+		buf = PTR_ALIGN(buf, sb);
+		err = sdata->tf->read_multiple_byte(&sdata->tb, sdata->dev,
+						    chan->address, rb, buf,
+						    sdata->multiread_bit);
+		if (err != rb)
+			return (err < 0) ? err : -EIO;
+
+		buf += sb;
 	}
 
-	return total;
+	return 0;
 }
 EXPORT_SYMBOL(st_sensors_get_buffer_element);
 
drivers/iio/common/st_sensors/st_sensors_core.c +0 -0
index dffe006..6901c7f 100644

drivers/iio/common/st_sensors/st_sensors_core.c static int st_sensors_read_axis_data(struct iio_dev *indio_dev, · line 469

Quoted 8 lines
drivers/iio/common/st_sensors/st_sensors_core.c +1 -1
@@ -463,7 +463,7 @@ static int st_sensors_read_axis_data(struct iio_dev *indio_dev,
 	int err;
 	u8 *outdata;
 	struct st_sensor_data *sdata = iio_priv(indio_dev);
-	unsigned int byte_for_channel = ch->scan_type.storagebits >> 3;
+	unsigned int byte_for_channel = ch->scan_type.realbits >> 3;
 
 	outdata = kmalloc(byte_for_channel, GFP_KERNEL);
 	if (!outdata)

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#14 lore raw

Re: [RFC PATCH v1 4/9] iio:st_sensors: align on storagebits boundaries

Quoted 8 lines · Jonathan Cameron
Quoted 7 lines · Jonathan Cameron
Quoted 6 lines · Gregor Boirie

Triggered buffering memory accesses are not aligned on per channel storagebits boundaries. Fix this by reading each channel individually to ensure proper alignment.
Note that this patch drops the earlier optimisation that packs multiple channels reading into a single data block transaction when their respective I2C register addresses are contiguous.

Quoted 8 lines
Quoted 3 lines

I wonder how much effect this change has on the read out rate and whether repacking in software would be significantly quicker than the multiple transfers...

Thinking more on this I'm really not happy with this solution - may well kill data rates on some chips, to deal with this one unaligned case. Gregor, can you give repacking in software a go?
Won't be that hard I think... Be cynical and just memove the data a byte if you hit a 24bit channel.

I'll give it a try, maybe next week as I'm quite busy right now.

Quoted 103 lines

We could do this in the core demux but then we'd have to do something a bit clever with the reported buffer element type so it looked different to the driver and the buffer.

Lars, IIRC you've messed with that corner of the code more recently than me. Do you think doing unaligned data smashing in the demux would be sensible?

Can see we'd have to:
* expand the 'does the demux have to do anything' case to catch
this one (not too hard).
* make the demux table builder force size constraints to power of two.
* either work out the shift to export to userspace (nasty) or make sure we packed the data right depending on endianness so there wasn't any change to the shift.
All this stuff occurs in the slow patch (table build) - it would look no worse than normal demux (some memcpys) in the fast path.

Good idea or not? What do people think.

I'm might mock this up in iio_dummy if I get bored and find out how bad it really is...

Jonathan

Quoted 80 lines
Quoted 67 lines
---
  drivers/iio/common/st_sensors/st_sensors_buffer.c | 38 ++++++++++-------------
  drivers/iio/common/st_sensors/st_sensors_core.c   |  2 +-
  2 files changed, 18 insertions(+), 22 deletions(-)
drivers/iio/common/st_sensors/st_sensors_buffer.c +17 -21
index c558985..a7f1ced 100644
@@ -21,33 +21,29 @@
 
  #include <linux/iio/common/st_sensors.h>
 
-
  int st_sensors_get_buffer_element(struct iio_dev *indio_dev, u8 *buf)
  {
-	int i, len;
-	int total = 0;
+	unsigned int cid;
  	struct st_sensor_data *sdata = iio_priv(indio_dev);
-	unsigned int num_data_channels = sdata->num_data_channels;
-
-	for (i = 0; i < num_data_channels; i++) {
-		unsigned int bytes_to_read;
-
-		if (test_bit(i, indio_dev->active_scan_mask)) {
-			bytes_to_read = indio_dev->channels[i].scan_type.storagebits >> 3;
-			len = sdata->tf->read_multiple_byte(&sdata->tb,
-				sdata->dev, indio_dev->channels[i].address,
-				bytes_to_read,
-				buf + total, sdata->multiread_bit);
-
-			if (len < bytes_to_read)
-				return -EIO;
 
-			/* Advance the buffer pointer */
-			total += len;
-		}
+	for_each_set_bit(cid, indio_dev->active_scan_mask,
+			 sdata->num_data_channels) {
+		const struct iio_chan_spec *chan = &indio_dev->channels[cid];
+		int sb = chan->scan_type.storagebits / 8;
+		int rb = chan->scan_type.realbits / 8;
+		int err;
+
+		buf = PTR_ALIGN(buf, sb);
+		err = sdata->tf->read_multiple_byte(&sdata->tb, sdata->dev,
+						    chan->address, rb, buf,
+						    sdata->multiread_bit);
+		if (err != rb)
+			return (err < 0) ? err : -EIO;
+
+		buf += sb;
  	}
 
-	return total;
+	return 0;
  }
  EXPORT_SYMBOL(st_sensors_get_buffer_element);
 
drivers/iio/common/st_sensors/st_sensors_core.c +0 -0
index dffe006..6901c7f 100644

drivers/iio/common/st_sensors/st_sensors_core.c static int st_sensors_read_axis_data(struct iio_dev *indio_dev, · line 469

Quoted 8 lines
drivers/iio/common/st_sensors/st_sensors_core.c +1 -1
@@ -463,7 +463,7 @@ static int st_sensors_read_axis_data(struct iio_dev *indio_dev,
  	int err;
  	u8 *outdata;
  	struct st_sensor_data *sdata = iio_priv(indio_dev);
-	unsigned int byte_for_channel = ch->scan_type.storagebits >> 3;
+	unsigned int byte_for_channel = ch->scan_type.realbits >> 3;
 
  	outdata = kmalloc(byte_for_channel, GFP_KERNEL);
  	if (!outdata)

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

maintainer
#15 lore raw

Re: [RFC PATCH v1 4/9] iio:st_sensors: align on storagebits boundaries

Quoted 14 lines · Gregor Boirie
Quoted 11 lines · Jonathan Cameron
Quoted 10 lines · Jonathan Cameron
Quoted 6 lines · Gregor Boirie

Triggered buffering memory accesses are not aligned on per channel storagebits boundaries. Fix this by reading each channel individually to ensure proper alignment.
Note that this patch drops the earlier optimisation that packs multiple channels reading into a single data block transaction when their respective I2C register addresses are contiguous.

I wonder how much effect this change has on the read out rate and whether repacking in software would be significantly quicker than the multiple transfers...

Quoted 6 lines
Quoted 5 lines

Thinking more on this I'm really not happy with this solution - may well kill data rates on some chips, to deal with this one unaligned case. Gregor, can you give repacking in software a go?
Won't be that hard I think... Be cynical and just memove the data a byte if you hit a 24bit channel.

I'll give it a try, maybe next week as I'm quite busy right now.

Cool, I've pinged Denis to see if he will have time to review the rest of the series in the meantime.

Quoted 17 lines
Quoted 17 lines

We could do this in the core demux but then we'd have to do something a bit clever with the reported buffer element type so it looked different to the driver and the buffer.

Lars, IIRC you've messed with that corner of the code more recently than me. Do you think doing unaligned data smashing in the demux would be sensible?

Can see we'd have to:
* expand the 'does the demux have to do anything' case to catch
this one (not too hard).
* make the demux table builder force size constraints to power of two.
* either work out the shift to export to userspace (nasty) or make sure we packed the data right depending on endianness so there wasn't any change to the shift.
All this stuff occurs in the slow patch (table build) - it would look no worse than normal demux (some memcpys) in the fast path.

Quoted 4 lines
Quoted 4 lines

Good idea or not? What do people think.

I'm might mock this up in iio_dummy if I get bored and find out how bad it really is...

I didn't get bored enough (or more accurately my laptop battery ran flat before I got this far and there was no power on the remaining trains that week).

Quoted 81 lines
Quoted 76 lines

Jonathan

Quoted 74 lines
Quoted 69 lines
---
  drivers/iio/common/st_sensors/st_sensors_buffer.c | 38 ++++++++++-------------
  drivers/iio/common/st_sensors/st_sensors_core.c   |  2 +-
  2 files changed, 18 insertions(+), 22 deletions(-)
drivers/iio/common/st_sensors/st_sensors_buffer.c +17 -18
index c558985..a7f1ced 100644
@@ -21,33 +21,29 @@
    #include <linux/iio/common/st_sensors.h>
  -
  int st_sensors_get_buffer_element(struct iio_dev *indio_dev, u8 *buf)
  {
-    int i, len;
-    int total = 0;
+    unsigned int cid;
      struct st_sensor_data *sdata = iio_priv(indio_dev);
-    unsigned int num_data_channels = sdata->num_data_channels;
-
-    for (i = 0; i < num_data_channels; i++) {
-        unsigned int bytes_to_read;
-
-        if (test_bit(i, indio_dev->active_scan_mask)) {
-            bytes_to_read = indio_dev->channels[i].scan_type.storagebits >> 3;
-            len = sdata->tf->read_multiple_byte(&sdata->tb,
-                sdata->dev, indio_dev->channels[i].address,
-                bytes_to_read,
-                buf + total, sdata->multiread_bit);
-
-            if (len < bytes_to_read)
-                return -EIO;
  -            /* Advance the buffer pointer */
-            total += len;
-        }
+    for_each_set_bit(cid, indio_dev->active_scan_mask,
+             sdata->num_data_channels) {
+        const struct iio_chan_spec *chan = &indio_dev->channels[cid];
+        int sb = chan->scan_type.storagebits / 8;
+        int rb = chan->scan_type.realbits / 8;
+        int err;
+
+        buf = PTR_ALIGN(buf, sb);
+        err = sdata->tf->read_multiple_byte(&sdata->tb, sdata->dev,
+                            chan->address, rb, buf,
+                            sdata->multiread_bit);
+        if (err != rb)
+            return (err < 0) ? err : -EIO;
+
+        buf += sb;
      }
  -    return total;
+    return 0;
  }
  EXPORT_SYMBOL(st_sensors_get_buffer_element);
  diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c

index dffe006..6901c7f 100644

drivers/iio/common/st_sensors/st_sensors_core.c +1 -1
@@ -463,7 +463,7 @@ static int st_sensors_read_axis_data(struct iio_dev *indio_dev,
      int err;
      u8 *outdata;
      struct st_sensor_data *sdata = iio_priv(indio_dev);
-    unsigned int byte_for_channel = ch->scan_type.storagebits >> 3;
+    unsigned int byte_for_channel = ch->scan_type.realbits >> 3;
        outdata = kmalloc(byte_for_channel, GFP_KERNEL);
      if (!outdata)
 
Signature · 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Signature · 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#16 lore raw

Re: [RFC PATCH v1 4/9] iio:st_sensors: align on storagebits boundaries

In reply to Gregor Boirie
Quoted 7 lines · Gregor Boirie

Triggered buffering memory accesses are not aligned on per channel storagebits boundaries. Fix this by reading each channel individually to ensure proper alignment.
Note that this patch drops the earlier optimisation that packs multiple channels reading into a single data block transaction when their respective I2C register addresses are contiguous.

Both the 'before' and 'after' versions seem to be doing one bus transaction for each active channel in the scan mask. The optimization you're mentioning was dropped in an older patch:

iio: st_sensors: read each channel individually

At most it makes that optimization harder to restore, right?

Signature · 2 lines
Regards,
Leonard
#17 lore raw

RFC PATCH 5/9 iio:st_pressure: align storagebits on power of 2

In reply to Gregor Boirie

Sampled pressure data are 24 bits long and should be stored in a 32 bits word.

---
 drivers/iio/pressure/st_pressure_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/iio/pressure/st_pressure_core.c +2 -2
index 24754eed..13f1c2d 100644
@@ -167,7 +167,7 @@ static const struct iio_chan_spec st_press_1_channels[] = {
 		.scan_type = {
 			.sign = 'u',
 			.realbits = 24,
-			.storagebits = 24,
+			.storagebits = 32,
 			.endianness = IIO_LE,
 		},
 		.info_mask_separate =
@@ -239,7 +239,7 @@ static const struct iio_chan_spec st_press_lps22hb_channels[] = {
 		.scan_type = {
 			.sign = 'u',
 			.realbits = 24,
-			.storagebits = 24,
+			.storagebits = 32,
 			.endianness = IIO_LE,
 		},
 		.info_mask_separate =
Signature · 1 line
2.1.4
#18 lore raw

RFC PATCH 6/9 iio:st_pressure: temperature triggered buffering

In reply to Gregor Boirie

Enable support for triggered buffering of temperature samples.

---
 drivers/iio/pressure/st_pressure_core.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)
drivers/iio/pressure/st_pressure_core.c +9 -11
index 13f1c2d..bacdf6c 100644
@@ -39,8 +39,6 @@
 #define ST_PRESS_LSB_PER_CELSIUS		480UL
 #define ST_PRESS_MILLI_CELSIUS_OFFSET		42500UL
 
-#define ST_PRESS_NUMBER_DATA_CHANNELS		1
-
 /* FULLSCALE */
 #define ST_PRESS_FS_AVL_1100MB			1100
 #define ST_PRESS_FS_AVL_1260MB			1260
@@ -163,7 +161,7 @@ static const struct iio_chan_spec st_press_1_channels[] = {
 		.type = IIO_PRESSURE,
 		.channel2 = IIO_NO_MOD,
 		.address = ST_PRESS_1_OUT_XL_ADDR,
-		.scan_index = ST_SENSORS_SCAN_X,
+		.scan_index = 0,
 		.scan_type = {
 			.sign = 'u',
 			.realbits = 24,
@@ -178,7 +176,7 @@ static const struct iio_chan_spec st_press_1_channels[] = {
 		.type = IIO_TEMP,
 		.channel2 = IIO_NO_MOD,
 		.address = ST_TEMP_1_OUT_L_ADDR,
-		.scan_index = -1,
+		.scan_index = 1,
 		.scan_type = {
 			.sign = 'u',
 			.realbits = 16,
@@ -191,7 +189,7 @@ static const struct iio_chan_spec st_press_1_channels[] = {
 			BIT(IIO_CHAN_INFO_OFFSET),
 		.modified = 0,
 	},
-	IIO_CHAN_SOFT_TIMESTAMP(1)
+	IIO_CHAN_SOFT_TIMESTAMP(2)
 };
 
 static const struct iio_chan_spec st_press_lps001wp_channels[] = {
@@ -199,7 +197,7 @@ static const struct iio_chan_spec st_press_lps001wp_channels[] = {
 		.type = IIO_PRESSURE,
 		.channel2 = IIO_NO_MOD,
 		.address = ST_PRESS_LPS001WP_OUT_L_ADDR,
-		.scan_index = ST_SENSORS_SCAN_X,
+		.scan_index = 0,
 		.scan_type = {
 			.sign = 'u',
 			.realbits = 16,
@@ -215,7 +213,7 @@ static const struct iio_chan_spec st_press_lps001wp_channels[] = {
 		.type = IIO_TEMP,
 		.channel2 = IIO_NO_MOD,
 		.address = ST_TEMP_LPS001WP_OUT_L_ADDR,
-		.scan_index = -1,
+		.scan_index = 1,
 		.scan_type = {
 			.sign = 'u',
 			.realbits = 16,
@@ -227,7 +225,7 @@ static const struct iio_chan_spec st_press_lps001wp_channels[] = {
 			BIT(IIO_CHAN_INFO_SCALE),
 		.modified = 0,
 	},
-	IIO_CHAN_SOFT_TIMESTAMP(1)
+	IIO_CHAN_SOFT_TIMESTAMP(2)
 };
 
 static const struct iio_chan_spec st_press_lps22hb_channels[] = {
@@ -252,7 +250,7 @@ static const struct iio_chan_spec st_press_lps22hb_channels[] = {
 		.type = IIO_TEMP,
 		.channel2 = IIO_NO_MOD,
 		.address = ST_TEMP_1_OUT_L_ADDR,
-		.scan_index = -1,
+		.scan_index = 1,
 		.scan_type = {
 			.sign = 'u',
 			.realbits = 16,
@@ -265,7 +263,7 @@ static const struct iio_chan_spec st_press_lps22hb_channels[] = {
 		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
 		.modified = 0,
 	},
-	IIO_CHAN_SOFT_TIMESTAMP(1)
+	IIO_CHAN_SOFT_TIMESTAMP(2)
 };
 
 static const struct st_sensor_settings st_press_sensors_settings[] = {
@@ -598,7 +596,7 @@ int st_press_common_probe(struct iio_dev *indio_dev)
 	if (err < 0)
 		return err;
 
-	press_data->num_data_channels = ST_PRESS_NUMBER_DATA_CHANNELS;
+	press_data->num_data_channels = press_data->sensor_settings->num_ch - 1;
 	press_data->multiread_bit = press_data->sensor_settings->multi_read_bit;
 	indio_dev->channels = press_data->sensor_settings->ch;
 	indio_dev->num_channels = press_data->sensor_settings->num_ch;
Signature · 1 line
2.1.4
maintainer
#19 lore raw

Re: [RFC PATCH v1 6/9] iio:st_pressure: temperature triggered buffering

Quoted 3 lines · Gregor Boirie

Enable support for triggered buffering of temperature samples.

I was a little curious as to why this wasn't done previously!

Anyhow, again ideally would like an Ack from Denis.

Jonathan

Quoted 99 lines
---
 drivers/iio/pressure/st_pressure_core.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)
drivers/iio/pressure/st_pressure_core.c +9 -11
index 13f1c2d..bacdf6c 100644
@@ -39,8 +39,6 @@
 #define ST_PRESS_LSB_PER_CELSIUS		480UL
 #define ST_PRESS_MILLI_CELSIUS_OFFSET		42500UL
 
-#define ST_PRESS_NUMBER_DATA_CHANNELS		1
-
 /* FULLSCALE */
 #define ST_PRESS_FS_AVL_1100MB			1100
 #define ST_PRESS_FS_AVL_1260MB			1260
@@ -163,7 +161,7 @@ static const struct iio_chan_spec st_press_1_channels[] = {
 		.type = IIO_PRESSURE,
 		.channel2 = IIO_NO_MOD,
 		.address = ST_PRESS_1_OUT_XL_ADDR,
-		.scan_index = ST_SENSORS_SCAN_X,
+		.scan_index = 0,
 		.scan_type = {
 			.sign = 'u',
 			.realbits = 24,
@@ -178,7 +176,7 @@ static const struct iio_chan_spec st_press_1_channels[] = {
 		.type = IIO_TEMP,
 		.channel2 = IIO_NO_MOD,
 		.address = ST_TEMP_1_OUT_L_ADDR,
-		.scan_index = -1,
+		.scan_index = 1,
 		.scan_type = {
 			.sign = 'u',
 			.realbits = 16,
@@ -191,7 +189,7 @@ static const struct iio_chan_spec st_press_1_channels[] = {
 			BIT(IIO_CHAN_INFO_OFFSET),
 		.modified = 0,
 	},
-	IIO_CHAN_SOFT_TIMESTAMP(1)
+	IIO_CHAN_SOFT_TIMESTAMP(2)
 };
 
 static const struct iio_chan_spec st_press_lps001wp_channels[] = {
@@ -199,7 +197,7 @@ static const struct iio_chan_spec st_press_lps001wp_channels[] = {
 		.type = IIO_PRESSURE,
 		.channel2 = IIO_NO_MOD,
 		.address = ST_PRESS_LPS001WP_OUT_L_ADDR,
-		.scan_index = ST_SENSORS_SCAN_X,
+		.scan_index = 0,
 		.scan_type = {
 			.sign = 'u',
 			.realbits = 16,
@@ -215,7 +213,7 @@ static const struct iio_chan_spec st_press_lps001wp_channels[] = {
 		.type = IIO_TEMP,
 		.channel2 = IIO_NO_MOD,
 		.address = ST_TEMP_LPS001WP_OUT_L_ADDR,
-		.scan_index = -1,
+		.scan_index = 1,
 		.scan_type = {
 			.sign = 'u',
 			.realbits = 16,
@@ -227,7 +225,7 @@ static const struct iio_chan_spec st_press_lps001wp_channels[] = {
 			BIT(IIO_CHAN_INFO_SCALE),
 		.modified = 0,
 	},
-	IIO_CHAN_SOFT_TIMESTAMP(1)
+	IIO_CHAN_SOFT_TIMESTAMP(2)
 };
 
 static const struct iio_chan_spec st_press_lps22hb_channels[] = {
@@ -252,7 +250,7 @@ static const struct iio_chan_spec st_press_lps22hb_channels[] = {
 		.type = IIO_TEMP,
 		.channel2 = IIO_NO_MOD,
 		.address = ST_TEMP_1_OUT_L_ADDR,
-		.scan_index = -1,
+		.scan_index = 1,
 		.scan_type = {
 			.sign = 'u',
 			.realbits = 16,
@@ -265,7 +263,7 @@ static const struct iio_chan_spec st_press_lps22hb_channels[] = {
 		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
 		.modified = 0,
 	},
-	IIO_CHAN_SOFT_TIMESTAMP(1)
+	IIO_CHAN_SOFT_TIMESTAMP(2)
 };
 
 static const struct st_sensor_settings st_press_sensors_settings[] = {
@@ -598,7 +596,7 @@ int st_press_common_probe(struct iio_dev *indio_dev)
 	if (err < 0)
 		return err;
 
-	press_data->num_data_channels = ST_PRESS_NUMBER_DATA_CHANNELS;
+	press_data->num_data_channels = press_data->sensor_settings->num_ch - 1;
 	press_data->multiread_bit = press_data->sensor_settings->multi_read_bit;
 	indio_dev->channels = press_data->sensor_settings->ch;
 	indio_dev->num_channels = press_data->sensor_settings->num_ch;
maintainer
#20 lore raw

Re: [RFC PATCH v1 6/9] iio:st_pressure: temperature triggered buffering

Quoted 5 lines · Jonathan Cameron
Quoted 3 lines · Gregor Boirie

Enable support for triggered buffering of temperature samples.

I was a little curious as to why this wasn't done previously!

Reading this series more closely, precisely because of the 24bit alignment of the pressure channels. We need your earlier patches for this one to work.

Anyhow, this is clearly dependant on a new version of patch 4 so it's stalled for now.

Jonathan

Quoted 109 lines

Anyhow, again ideally would like an Ack from Denis.

Jonathan

Quoted 91 lines
---
 drivers/iio/pressure/st_pressure_core.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)
drivers/iio/pressure/st_pressure_core.c +8 -10
index 13f1c2d..bacdf6c 100644
@@ -39,8 +39,6 @@
 #define ST_PRESS_LSB_PER_CELSIUS		480UL
 #define ST_PRESS_MILLI_CELSIUS_OFFSET		42500UL
 
-#define ST_PRESS_NUMBER_DATA_CHANNELS		1
-
 /* FULLSCALE */
 #define ST_PRESS_FS_AVL_1100MB			1100
 #define ST_PRESS_FS_AVL_1260MB			1260
@@ -163,7 +161,7 @@ static const struct iio_chan_spec st_press_1_channels[] = {
 		.type = IIO_PRESSURE,
 		.channel2 = IIO_NO_MOD,
 		.address = ST_PRESS_1_OUT_XL_ADDR,
-		.scan_index = ST_SENSORS_SCAN_X,
+		.scan_index = 0,
 		.scan_type = {
 			.sign = 'u',
 			.realbits = 24,
@@ -178,7 +176,7 @@ static const struct iio_chan_spec st_press_1_channels[] = {
 		.type = IIO_TEMP,
 		.channel2 = IIO_NO_MOD,
 		.address = ST_TEMP_1_OUT_L_ADDR,
-		.scan_index = -1,
+		.scan_index = 1,
 		.scan_type = {
 			.sign = 'u',
 			.realbits = 16,
@@ -191,7 +189,7 @@ static const struct iio_chan_spec st_press_1_channels[] = {
 			BIT(IIO_CHAN_INFO_OFFSET),
 		.modified = 0,
 	},
-	IIO_CHAN_SOFT_TIMESTAMP(1)
+	IIO_CHAN_SOFT_TIMESTAMP(2)
 };
 
 static const struct iio_chan_spec st_press_lps001wp_channels[] = {
@@ -199,7 +197,7 @@ static const struct iio_chan_spec st_press_lps001wp_channels[] = {
 		.type = IIO_PRESSURE,
 		.channel2 = IIO_NO_MOD,
 		.address = ST_PRESS_LPS001WP_OUT_L_ADDR,
-		.scan_index = ST_SENSORS_SCAN_X,
+		.scan_index = 0,
 		.scan_type = {
 			.sign = 'u',
 			.realbits = 16,
@@ -215,7 +213,7 @@ static const struct iio_chan_spec st_press_lps001wp_channels[] = {
 		.type = IIO_TEMP,
 		.channel2 = IIO_NO_MOD,
 		.address = ST_TEMP_LPS001WP_OUT_L_ADDR,
-		.scan_index = -1,
+		.scan_index = 1,
 		.scan_type = {
 			.sign = 'u',
 			.realbits = 16,
@@ -227,7 +225,7 @@ static const struct iio_chan_spec st_press_lps001wp_channels[] = {
 			BIT(IIO_CHAN_INFO_SCALE),
 		.modified = 0,
 	},
-	IIO_CHAN_SOFT_TIMESTAMP(1)
+	IIO_CHAN_SOFT_TIMESTAMP(2)
 };
 
 static const struct iio_chan_spec st_press_lps22hb_channels[] = {
@@ -252,7 +250,7 @@ static const struct iio_chan_spec st_press_lps22hb_channels[] = {
 		.type = IIO_TEMP,
 		.channel2 = IIO_NO_MOD,
 		.address = ST_TEMP_1_OUT_L_ADDR,
-		.scan_index = -1,
+		.scan_index = 1,
 		.scan_type = {
 			.sign = 'u',
 			.realbits = 16,
@@ -265,7 +263,7 @@ static const struct iio_chan_spec st_press_lps22hb_channels[] = {
 		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
 		.modified = 0,
 	},
-	IIO_CHAN_SOFT_TIMESTAMP(1)
+	IIO_CHAN_SOFT_TIMESTAMP(2)
 };
 
 static const struct st_sensor_settings st_press_sensors_settings[] = {

drivers/iio/pressure/st_pressure_core.c int st_press_common_probe(struct iio_dev *indio_dev) · line 602

Quoted 8 lines
drivers/iio/pressure/st_pressure_core.c +1 -1
@@ -598,7 +596,7 @@ int st_press_common_probe(struct iio_dev *indio_dev)
 	if (err < 0)
 		return err;
 
-	press_data->num_data_channels = ST_PRESS_NUMBER_DATA_CHANNELS;
+	press_data->num_data_channels = press_data->sensor_settings->num_ch - 1;
 	press_data->multiread_bit = press_data->sensor_settings->multi_read_bit;
 	indio_dev->channels = press_data->sensor_settings->ch;
 	indio_dev->num_channels = press_data->sensor_settings->num_ch;

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#21 lore raw

RFC PATCH 7/9 iio:st_sensors: unexport st_sensors_get_buffer_element

In reply to Gregor Boirie

Remove st_sensors_get_buffer_element symbol export since not explicitly used outside of st_sensors driver.

---
 drivers/iio/common/st_sensors/st_sensors_buffer.c | 3 +--
 include/linux/iio/common/st_sensors.h             | 2 --
 2 files changed, 1 insertion(+), 4 deletions(-)
drivers/iio/common/st_sensors/st_sensors_buffer.c +1 -2
index a7f1ced..9343d74 100644
@@ -21,7 +21,7 @@
 
 #include <linux/iio/common/st_sensors.h>
 
-int st_sensors_get_buffer_element(struct iio_dev *indio_dev, u8 *buf)
+static int st_sensors_get_buffer_element(struct iio_dev *indio_dev, u8 *buf)
 {
 	unsigned int cid;
 	struct st_sensor_data *sdata = iio_priv(indio_dev);
@@ -45,7 +45,6 @@ int st_sensors_get_buffer_element(struct iio_dev *indio_dev, u8 *buf)
 
 	return 0;
 }
-EXPORT_SYMBOL(st_sensors_get_buffer_element);
 
 irqreturn_t st_sensors_trigger_handler(int irq, void *p)
 {
include/linux/iio/common/st_sensors.h +0 -2
index d029ffa..78a1934 100644
@@ -251,8 +251,6 @@ struct st_sensor_data {
 
 #ifdef CONFIG_IIO_BUFFER
 irqreturn_t st_sensors_trigger_handler(int irq, void *p);
-
-int st_sensors_get_buffer_element(struct iio_dev *indio_dev, u8 *buf);
 #endif
 
 #ifdef CONFIG_IIO_TRIGGER
Signature · 1 line
2.1.4
maintainer Applied
#22 lore raw

Re: [RFC PATCH v1 7/9] iio:st_sensors: unexport st_sensors_get_buffer_element

Quoted 4 lines · Gregor Boirie

Remove st_sensors_get_buffer_element symbol export since not explicitly used outside of st_sensors driver.

Good spot. It applied with a little fuzz to the togreg branch of iio.git. Lets at least cut down on how many patches we have queued up for this driver by taking the simple ones ;)

Applied to the togreg branch of iio.git

Thanks,

Jonathan.

Quoted 40 lines
---
 drivers/iio/common/st_sensors/st_sensors_buffer.c | 3 +--
 include/linux/iio/common/st_sensors.h             | 2 --
 2 files changed, 1 insertion(+), 4 deletions(-)
drivers/iio/common/st_sensors/st_sensors_buffer.c +1 -2
index a7f1ced..9343d74 100644
@@ -21,7 +21,7 @@
 
 #include <linux/iio/common/st_sensors.h>
 
-int st_sensors_get_buffer_element(struct iio_dev *indio_dev, u8 *buf)
+static int st_sensors_get_buffer_element(struct iio_dev *indio_dev, u8 *buf)
 {
 	unsigned int cid;
 	struct st_sensor_data *sdata = iio_priv(indio_dev);
@@ -45,7 +45,6 @@ int st_sensors_get_buffer_element(struct iio_dev *indio_dev, u8 *buf)
 
 	return 0;
 }
-EXPORT_SYMBOL(st_sensors_get_buffer_element);
 
 irqreturn_t st_sensors_trigger_handler(int irq, void *p)
 {
include/linux/iio/common/st_sensors.h +0 -2
index d029ffa..78a1934 100644
@@ -251,8 +251,6 @@ struct st_sensor_data {
 
 #ifdef CONFIG_IIO_BUFFER
 irqreturn_t st_sensors_trigger_handler(int irq, void *p);
-
-int st_sensors_get_buffer_element(struct iio_dev *indio_dev, u8 *buf);
 #endif
 
 #ifdef CONFIG_IIO_TRIGGER
#23 lore raw

RFC PATCH 8/9 iio:st_sensors: emulate SMBus block read if needed

In reply to Gregor Boirie

Use SMBus "block read" protocol only when supported by adapter.

---
 drivers/iio/common/st_sensors/st_sensors_i2c.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/iio/common/st_sensors/st_sensors_i2c.c +2 -2
index 98cfee29..b43aa36 100644
@@ -48,8 +48,8 @@ static int st_sensors_i2c_read_multiple_byte(
 	if (multiread_bit)
 		reg_addr |= ST_SENSORS_I2C_MULTIREAD;
 
-	return i2c_smbus_read_i2c_block_data(to_i2c_client(dev),
-							reg_addr, len, data);
+	return i2c_smbus_read_i2c_block_data_or_emulated(to_i2c_client(dev),
+							 reg_addr, len, data);
 }
 
 static int st_sensors_i2c_write_byte(struct st_sensor_transfer_buffer *tb,
Signature · 1 line
2.1.4
maintainer Applied
#24 lore raw

Re: [RFC PATCH v1 8/9] iio:st_sensors: emulate SMBus block read if needed

Quoted 3 lines · Gregor Boirie

Use SMBus "block read" protocol only when supported by adapter.

Applied to the togreg branch of iio.git.

We will need to be a little careful with this if we start reading fifo buffers in these drivers as suddenly emulation is going to read different things.

Right now I don't believe we are using that hardware but worth keeping in mind.

Thanks,

Jonathan

Quoted 20 lines
---
 drivers/iio/common/st_sensors/st_sensors_i2c.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/iio/common/st_sensors/st_sensors_i2c.c +2 -2
index 98cfee29..b43aa36 100644
@@ -48,8 +48,8 @@ static int st_sensors_i2c_read_multiple_byte(
 	if (multiread_bit)
 		reg_addr |= ST_SENSORS_I2C_MULTIREAD;
 
-	return i2c_smbus_read_i2c_block_data(to_i2c_client(dev),
-							reg_addr, len, data);
+	return i2c_smbus_read_i2c_block_data_or_emulated(to_i2c_client(dev),
+							 reg_addr, len, data);
 }
 
 static int st_sensors_i2c_write_byte(struct st_sensor_transfer_buffer *tb,
#25 lore raw

RFC PATCH 9/9 iio:st_sensors: fix power regulator usage

In reply to Gregor Boirie

Ensure failure to enable power regulators is properly handled.

---
 drivers/iio/accel/st_accel_core.c               | 12 ++++++----
 drivers/iio/common/st_sensors/st_sensors_core.c | 29 ++++++++++++++++++++-----
 drivers/iio/gyro/st_gyro_core.c                 | 12 ++++++----
 drivers/iio/magnetometer/st_magn_core.c         | 12 ++++++----
 drivers/iio/pressure/st_pressure_core.c         | 12 ++++++----
 include/linux/iio/common/st_sensors.h           |  2 +-
 6 files changed, 57 insertions(+), 22 deletions(-)
drivers/iio/accel/st_accel_core.c +8 -4
index dc73f2d..b8d3c3c 100644
@@ -757,13 +757,15 @@ int st_accel_common_probe(struct iio_dev *indio_dev)
 	indio_dev->info = &accel_info;
 	mutex_init(&adata->tb.buf_lock);
 
-	st_sensors_power_enable(indio_dev);
+	err = st_sensors_power_enable(indio_dev);
+	if (err)
+		return err;
 
 	err = st_sensors_check_device_support(indio_dev,
 					ARRAY_SIZE(st_accel_sensors_settings),
 					st_accel_sensors_settings);
 	if (err < 0)
-		return err;
+		goto st_accel_power_off;
 
 	adata->num_data_channels = ST_ACCEL_NUMBER_DATA_CHANNELS;
 	adata->multiread_bit = adata->sensor_settings->multi_read_bit;
@@ -780,11 +782,11 @@ int st_accel_common_probe(struct iio_dev *indio_dev)
 
 	err = st_sensors_init_sensor(indio_dev, adata->dev->platform_data);
 	if (err < 0)
-		return err;
+		goto st_accel_power_off;
 
 	err = st_accel_allocate_ring(indio_dev);
 	if (err < 0)
-		return err;
+		goto st_accel_power_off;
 
 	if (irq > 0) {
 		err = st_sensors_allocate_trigger(indio_dev,
@@ -807,6 +809,8 @@ st_accel_device_register_error:
 		st_sensors_deallocate_trigger(indio_dev);
 st_accel_probe_trigger_error:
 	st_accel_deallocate_ring(indio_dev);
+st_accel_power_off:
+	st_sensors_power_disable(indio_dev);
 
 	return err;
 }
drivers/iio/common/st_sensors/st_sensors_core.c +24 -5
index 6901c7f..286f28c 100644
@@ -228,7 +228,7 @@ int st_sensors_set_axis_enable(struct iio_dev *indio_dev, u8 axis_enable)
 }
 EXPORT_SYMBOL(st_sensors_set_axis_enable);
 
-void st_sensors_power_enable(struct iio_dev *indio_dev)
+int st_sensors_power_enable(struct iio_dev *indio_dev)
 {
 	struct st_sensor_data *pdata = iio_priv(indio_dev);
 	int err;
@@ -237,18 +237,37 @@ void st_sensors_power_enable(struct iio_dev *indio_dev)
 	pdata->vdd = devm_regulator_get_optional(indio_dev->dev.parent, "vdd");
 	if (!IS_ERR(pdata->vdd)) {
 		err = regulator_enable(pdata->vdd);
-		if (err != 0)
+		if (err != 0) {
 			dev_warn(&indio_dev->dev,
 				 "Failed to enable specified Vdd supply\n");
+			return err;
+		}
+	} else {
+		err = PTR_ERR(pdata->vdd);
+		if (err != -ENODEV)
+			return err;
 	}
 
 	pdata->vdd_io = devm_regulator_get_optional(indio_dev->dev.parent, "vddio");
 	if (!IS_ERR(pdata->vdd_io)) {
 		err = regulator_enable(pdata->vdd_io);
-		if (err != 0)
+		if (err != 0) {
 			dev_warn(&indio_dev->dev,
 				 "Failed to enable specified Vdd_IO supply\n");
+			goto st_sensors_disable_vdd;
+		}
+	} else {
+		err = PTR_ERR(pdata->vdd_io);
+		if (err != -ENODEV)
+			goto st_sensors_disable_vdd;
 	}
+
+	return 0;
+
+st_sensors_disable_vdd:
+	if (!IS_ERR_OR_NULL(pdata->vdd))
+		regulator_disable(pdata->vdd);
+	return err;
 }
 EXPORT_SYMBOL(st_sensors_power_enable);
 
@@ -256,10 +275,10 @@ void st_sensors_power_disable(struct iio_dev *indio_dev)
 {
 	struct st_sensor_data *pdata = iio_priv(indio_dev);
 
-	if (!IS_ERR(pdata->vdd))
+	if (!IS_ERR_OR_NULL(pdata->vdd))
 		regulator_disable(pdata->vdd);
 
-	if (!IS_ERR(pdata->vdd_io))
+	if (!IS_ERR_OR_NULL(pdata->vdd_io))
 		regulator_disable(pdata->vdd_io);
 }
 EXPORT_SYMBOL(st_sensors_power_disable);
drivers/iio/gyro/st_gyro_core.c +8 -4
index be9057e..f07f105 100644
@@ -424,13 +424,15 @@ int st_gyro_common_probe(struct iio_dev *indio_dev)
 	indio_dev->info = &gyro_info;
 	mutex_init(&gdata->tb.buf_lock);
 
-	st_sensors_power_enable(indio_dev);
+	err = st_sensors_power_enable(indio_dev);
+	if (err)
+		return err;
 
 	err = st_sensors_check_device_support(indio_dev,
 					ARRAY_SIZE(st_gyro_sensors_settings),
 					st_gyro_sensors_settings);
 	if (err < 0)
-		return err;
+		goto st_gyro_power_off;
 
 	gdata->num_data_channels = ST_GYRO_NUMBER_DATA_CHANNELS;
 	gdata->multiread_bit = gdata->sensor_settings->multi_read_bit;
@@ -444,11 +446,11 @@ int st_gyro_common_probe(struct iio_dev *indio_dev)
 	err = st_sensors_init_sensor(indio_dev,
 				(struct st_sensors_platform_data *)&gyro_pdata);
 	if (err < 0)
-		return err;
+		goto st_gyro_power_off;
 
 	err = st_gyro_allocate_ring(indio_dev);
 	if (err < 0)
-		return err;
+		goto st_gyro_power_off;
 
 	if (irq > 0) {
 		err = st_sensors_allocate_trigger(indio_dev,
@@ -471,6 +473,8 @@ st_gyro_device_register_error:
 		st_sensors_deallocate_trigger(indio_dev);
 st_gyro_probe_trigger_error:
 	st_gyro_deallocate_ring(indio_dev);
+st_gyro_power_off:
+	st_sensors_power_disable(indio_dev);
 
 	return err;
 }
drivers/iio/magnetometer/st_magn_core.c +8 -4
index 62036d2..7c94adc 100644
@@ -588,13 +588,15 @@ int st_magn_common_probe(struct iio_dev *indio_dev)
 	indio_dev->info = &magn_info;
 	mutex_init(&mdata->tb.buf_lock);
 
-	st_sensors_power_enable(indio_dev);
+	err = st_sensors_power_enable(indio_dev);
+	if (err)
+		return err;
 
 	err = st_sensors_check_device_support(indio_dev,
 					ARRAY_SIZE(st_magn_sensors_settings),
 					st_magn_sensors_settings);
 	if (err < 0)
-		return err;
+		goto st_magn_power_off;
 
 	mdata->num_data_channels = ST_MAGN_NUMBER_DATA_CHANNELS;
 	mdata->multiread_bit = mdata->sensor_settings->multi_read_bit;
@@ -607,11 +609,11 @@ int st_magn_common_probe(struct iio_dev *indio_dev)
 
 	err = st_sensors_init_sensor(indio_dev, NULL);
 	if (err < 0)
-		return err;
+		goto st_magn_power_off;
 
 	err = st_magn_allocate_ring(indio_dev);
 	if (err < 0)
-		return err;
+		goto st_magn_power_off;
 
 	if (irq > 0) {
 		err = st_sensors_allocate_trigger(indio_dev,
@@ -634,6 +636,8 @@ st_magn_device_register_error:
 		st_sensors_deallocate_trigger(indio_dev);
 st_magn_probe_trigger_error:
 	st_magn_deallocate_ring(indio_dev);
+st_magn_power_off:
+	st_sensors_power_disable(indio_dev);
 
 	return err;
 }
drivers/iio/pressure/st_pressure_core.c +8 -4
index bacdf6c..4b01b19 100644
@@ -588,13 +588,15 @@ int st_press_common_probe(struct iio_dev *indio_dev)
 	indio_dev->info = &press_info;
 	mutex_init(&press_data->tb.buf_lock);
 
-	st_sensors_power_enable(indio_dev);
+	err = st_sensors_power_enable(indio_dev);
+	if (err)
+		return err;
 
 	err = st_sensors_check_device_support(indio_dev,
 					ARRAY_SIZE(st_press_sensors_settings),
 					st_press_sensors_settings);
 	if (err < 0)
-		return err;
+		goto st_press_power_off;
 
 	press_data->num_data_channels = press_data->sensor_settings->num_ch - 1;
 	press_data->multiread_bit = press_data->sensor_settings->multi_read_bit;
@@ -615,11 +617,11 @@ int st_press_common_probe(struct iio_dev *indio_dev)
 
 	err = st_sensors_init_sensor(indio_dev, press_data->dev->platform_data);
 	if (err < 0)
-		return err;
+		goto st_press_power_off;
 
 	err = st_press_allocate_ring(indio_dev);
 	if (err < 0)
-		return err;
+		goto st_press_power_off;
 
 	if (irq > 0) {
 		err = st_sensors_allocate_trigger(indio_dev,
@@ -642,6 +644,8 @@ st_press_device_register_error:
 		st_sensors_deallocate_trigger(indio_dev);
 st_press_probe_trigger_error:
 	st_press_deallocate_ring(indio_dev);
+st_press_power_off:
+	st_sensors_power_disable(indio_dev);
 
 	return err;
 }
include/linux/iio/common/st_sensors.h +1 -1
index 78a1934..91d5f68 100644
@@ -278,7 +278,7 @@ int st_sensors_set_enable(struct iio_dev *indio_dev, bool enable);
 
 int st_sensors_set_axis_enable(struct iio_dev *indio_dev, u8 axis_enable);
 
-void st_sensors_power_enable(struct iio_dev *indio_dev);
+int st_sensors_power_enable(struct iio_dev *indio_dev);
 
 void st_sensors_power_disable(struct iio_dev *indio_dev);
 
Signature · 1 line
2.1.4
maintainer
#26 lore raw

Re: [RFC PATCH v1 9/9] iio:st_sensors: fix power regulator usage

Quoted 3 lines · Gregor Boirie

Ensure failure to enable power regulators is properly handled.

All looks sensible to me. I'm happy with the whole series, so will just let this sit until Denis / others have had plenty of time to look at it.

Jonathan

Quoted 269 lines
---
 drivers/iio/accel/st_accel_core.c               | 12 ++++++----
 drivers/iio/common/st_sensors/st_sensors_core.c | 29 ++++++++++++++++++++-----
 drivers/iio/gyro/st_gyro_core.c                 | 12 ++++++----
 drivers/iio/magnetometer/st_magn_core.c         | 12 ++++++----
 drivers/iio/pressure/st_pressure_core.c         | 12 ++++++----
 include/linux/iio/common/st_sensors.h           |  2 +-
 6 files changed, 57 insertions(+), 22 deletions(-)
drivers/iio/accel/st_accel_core.c +8 -4
index dc73f2d..b8d3c3c 100644
@@ -757,13 +757,15 @@ int st_accel_common_probe(struct iio_dev *indio_dev)
 	indio_dev->info = &accel_info;
 	mutex_init(&adata->tb.buf_lock);
 
-	st_sensors_power_enable(indio_dev);
+	err = st_sensors_power_enable(indio_dev);
+	if (err)
+		return err;
 
 	err = st_sensors_check_device_support(indio_dev,
 					ARRAY_SIZE(st_accel_sensors_settings),
 					st_accel_sensors_settings);
 	if (err < 0)
-		return err;
+		goto st_accel_power_off;
 
 	adata->num_data_channels = ST_ACCEL_NUMBER_DATA_CHANNELS;
 	adata->multiread_bit = adata->sensor_settings->multi_read_bit;
@@ -780,11 +782,11 @@ int st_accel_common_probe(struct iio_dev *indio_dev)
 
 	err = st_sensors_init_sensor(indio_dev, adata->dev->platform_data);
 	if (err < 0)
-		return err;
+		goto st_accel_power_off;
 
 	err = st_accel_allocate_ring(indio_dev);
 	if (err < 0)
-		return err;
+		goto st_accel_power_off;
 
 	if (irq > 0) {
 		err = st_sensors_allocate_trigger(indio_dev,
@@ -807,6 +809,8 @@ st_accel_device_register_error:
 		st_sensors_deallocate_trigger(indio_dev);
 st_accel_probe_trigger_error:
 	st_accel_deallocate_ring(indio_dev);
+st_accel_power_off:
+	st_sensors_power_disable(indio_dev);
 
 	return err;
 }
drivers/iio/common/st_sensors/st_sensors_core.c +24 -5
index 6901c7f..286f28c 100644
@@ -228,7 +228,7 @@ int st_sensors_set_axis_enable(struct iio_dev *indio_dev, u8 axis_enable)
 }
 EXPORT_SYMBOL(st_sensors_set_axis_enable);
 
-void st_sensors_power_enable(struct iio_dev *indio_dev)
+int st_sensors_power_enable(struct iio_dev *indio_dev)
 {
 	struct st_sensor_data *pdata = iio_priv(indio_dev);
 	int err;
@@ -237,18 +237,37 @@ void st_sensors_power_enable(struct iio_dev *indio_dev)
 	pdata->vdd = devm_regulator_get_optional(indio_dev->dev.parent, "vdd");
 	if (!IS_ERR(pdata->vdd)) {
 		err = regulator_enable(pdata->vdd);
-		if (err != 0)
+		if (err != 0) {
 			dev_warn(&indio_dev->dev,
 				 "Failed to enable specified Vdd supply\n");
+			return err;
+		}
+	} else {
+		err = PTR_ERR(pdata->vdd);
+		if (err != -ENODEV)
+			return err;
 	}
 
 	pdata->vdd_io = devm_regulator_get_optional(indio_dev->dev.parent, "vddio");
 	if (!IS_ERR(pdata->vdd_io)) {
 		err = regulator_enable(pdata->vdd_io);
-		if (err != 0)
+		if (err != 0) {
 			dev_warn(&indio_dev->dev,
 				 "Failed to enable specified Vdd_IO supply\n");
+			goto st_sensors_disable_vdd;
+		}
+	} else {
+		err = PTR_ERR(pdata->vdd_io);
+		if (err != -ENODEV)
+			goto st_sensors_disable_vdd;
 	}
+
+	return 0;
+
+st_sensors_disable_vdd:
+	if (!IS_ERR_OR_NULL(pdata->vdd))
+		regulator_disable(pdata->vdd);
+	return err;
 }
 EXPORT_SYMBOL(st_sensors_power_enable);
 
@@ -256,10 +275,10 @@ void st_sensors_power_disable(struct iio_dev *indio_dev)
 {
 	struct st_sensor_data *pdata = iio_priv(indio_dev);
 
-	if (!IS_ERR(pdata->vdd))
+	if (!IS_ERR_OR_NULL(pdata->vdd))
 		regulator_disable(pdata->vdd);
 
-	if (!IS_ERR(pdata->vdd_io))
+	if (!IS_ERR_OR_NULL(pdata->vdd_io))
 		regulator_disable(pdata->vdd_io);
 }
 EXPORT_SYMBOL(st_sensors_power_disable);
drivers/iio/gyro/st_gyro_core.c +8 -4
index be9057e..f07f105 100644
@@ -424,13 +424,15 @@ int st_gyro_common_probe(struct iio_dev *indio_dev)
 	indio_dev->info = &gyro_info;
 	mutex_init(&gdata->tb.buf_lock);
 
-	st_sensors_power_enable(indio_dev);
+	err = st_sensors_power_enable(indio_dev);
+	if (err)
+		return err;
 
 	err = st_sensors_check_device_support(indio_dev,
 					ARRAY_SIZE(st_gyro_sensors_settings),
 					st_gyro_sensors_settings);
 	if (err < 0)
-		return err;
+		goto st_gyro_power_off;
 
 	gdata->num_data_channels = ST_GYRO_NUMBER_DATA_CHANNELS;
 	gdata->multiread_bit = gdata->sensor_settings->multi_read_bit;
@@ -444,11 +446,11 @@ int st_gyro_common_probe(struct iio_dev *indio_dev)
 	err = st_sensors_init_sensor(indio_dev,
 				(struct st_sensors_platform_data *)&gyro_pdata);
 	if (err < 0)
-		return err;
+		goto st_gyro_power_off;
 
 	err = st_gyro_allocate_ring(indio_dev);
 	if (err < 0)
-		return err;
+		goto st_gyro_power_off;
 
 	if (irq > 0) {
 		err = st_sensors_allocate_trigger(indio_dev,
@@ -471,6 +473,8 @@ st_gyro_device_register_error:
 		st_sensors_deallocate_trigger(indio_dev);
 st_gyro_probe_trigger_error:
 	st_gyro_deallocate_ring(indio_dev);
+st_gyro_power_off:
+	st_sensors_power_disable(indio_dev);
 
 	return err;
 }
drivers/iio/magnetometer/st_magn_core.c +8 -4
index 62036d2..7c94adc 100644
@@ -588,13 +588,15 @@ int st_magn_common_probe(struct iio_dev *indio_dev)
 	indio_dev->info = &magn_info;
 	mutex_init(&mdata->tb.buf_lock);
 
-	st_sensors_power_enable(indio_dev);
+	err = st_sensors_power_enable(indio_dev);
+	if (err)
+		return err;
 
 	err = st_sensors_check_device_support(indio_dev,
 					ARRAY_SIZE(st_magn_sensors_settings),
 					st_magn_sensors_settings);
 	if (err < 0)
-		return err;
+		goto st_magn_power_off;
 
 	mdata->num_data_channels = ST_MAGN_NUMBER_DATA_CHANNELS;
 	mdata->multiread_bit = mdata->sensor_settings->multi_read_bit;
@@ -607,11 +609,11 @@ int st_magn_common_probe(struct iio_dev *indio_dev)
 
 	err = st_sensors_init_sensor(indio_dev, NULL);
 	if (err < 0)
-		return err;
+		goto st_magn_power_off;
 
 	err = st_magn_allocate_ring(indio_dev);
 	if (err < 0)
-		return err;
+		goto st_magn_power_off;
 
 	if (irq > 0) {
 		err = st_sensors_allocate_trigger(indio_dev,
@@ -634,6 +636,8 @@ st_magn_device_register_error:
 		st_sensors_deallocate_trigger(indio_dev);
 st_magn_probe_trigger_error:
 	st_magn_deallocate_ring(indio_dev);
+st_magn_power_off:
+	st_sensors_power_disable(indio_dev);
 
 	return err;
 }
drivers/iio/pressure/st_pressure_core.c +8 -4
index bacdf6c..4b01b19 100644
@@ -588,13 +588,15 @@ int st_press_common_probe(struct iio_dev *indio_dev)
 	indio_dev->info = &press_info;
 	mutex_init(&press_data->tb.buf_lock);
 
-	st_sensors_power_enable(indio_dev);
+	err = st_sensors_power_enable(indio_dev);
+	if (err)
+		return err;
 
 	err = st_sensors_check_device_support(indio_dev,
 					ARRAY_SIZE(st_press_sensors_settings),
 					st_press_sensors_settings);
 	if (err < 0)
-		return err;
+		goto st_press_power_off;
 
 	press_data->num_data_channels = press_data->sensor_settings->num_ch - 1;
 	press_data->multiread_bit = press_data->sensor_settings->multi_read_bit;
@@ -615,11 +617,11 @@ int st_press_common_probe(struct iio_dev *indio_dev)
 
 	err = st_sensors_init_sensor(indio_dev, press_data->dev->platform_data);
 	if (err < 0)
-		return err;
+		goto st_press_power_off;
 
 	err = st_press_allocate_ring(indio_dev);
 	if (err < 0)
-		return err;
+		goto st_press_power_off;
 
 	if (irq > 0) {
 		err = st_sensors_allocate_trigger(indio_dev,
@@ -642,6 +644,8 @@ st_press_device_register_error:
 		st_sensors_deallocate_trigger(indio_dev);
 st_press_probe_trigger_error:
 	st_press_deallocate_ring(indio_dev);
+st_press_power_off:
+	st_sensors_power_disable(indio_dev);
 
 	return err;
 }
include/linux/iio/common/st_sensors.h +1 -1
index 78a1934..91d5f68 100644
@@ -278,7 +278,7 @@ int st_sensors_set_enable(struct iio_dev *indio_dev, bool enable);
 
 int st_sensors_set_axis_enable(struct iio_dev *indio_dev, u8 axis_enable);
 
-void st_sensors_power_enable(struct iio_dev *indio_dev);
+int st_sensors_power_enable(struct iio_dev *indio_dev);
 
 void st_sensors_power_disable(struct iio_dev *indio_dev);
 
maintainer
#27 lore raw

Re: [RFC PATCH v1 9/9] iio:st_sensors: fix power regulator usage

Quoted 7 lines · Jonathan Cameron
Quoted 3 lines · Gregor Boirie

Ensure failure to enable power regulators is properly handled.

All looks sensible to me. I'm happy with the whole series, so will just let this sit until Denis / others have had plenty of time to look at it.

Other than patch 4 that is! Forgot about that one :)

Quoted 276 lines

Jonathan

Quoted 261 lines
---
 drivers/iio/accel/st_accel_core.c               | 12 ++++++----
 drivers/iio/common/st_sensors/st_sensors_core.c | 29 ++++++++++++++++++++-----
 drivers/iio/gyro/st_gyro_core.c                 | 12 ++++++----
 drivers/iio/magnetometer/st_magn_core.c         | 12 ++++++----
 drivers/iio/pressure/st_pressure_core.c         | 12 ++++++----
 include/linux/iio/common/st_sensors.h           |  2 +-
 6 files changed, 57 insertions(+), 22 deletions(-)
drivers/iio/accel/st_accel_core.c +8 -4
index dc73f2d..b8d3c3c 100644
@@ -757,13 +757,15 @@ int st_accel_common_probe(struct iio_dev *indio_dev)
 	indio_dev->info = &accel_info;
 	mutex_init(&adata->tb.buf_lock);
 
-	st_sensors_power_enable(indio_dev);
+	err = st_sensors_power_enable(indio_dev);
+	if (err)
+		return err;
 
 	err = st_sensors_check_device_support(indio_dev,
 					ARRAY_SIZE(st_accel_sensors_settings),
 					st_accel_sensors_settings);
 	if (err < 0)
-		return err;
+		goto st_accel_power_off;
 
 	adata->num_data_channels = ST_ACCEL_NUMBER_DATA_CHANNELS;
 	adata->multiread_bit = adata->sensor_settings->multi_read_bit;
@@ -780,11 +782,11 @@ int st_accel_common_probe(struct iio_dev *indio_dev)
 
 	err = st_sensors_init_sensor(indio_dev, adata->dev->platform_data);
 	if (err < 0)
-		return err;
+		goto st_accel_power_off;
 
 	err = st_accel_allocate_ring(indio_dev);
 	if (err < 0)
-		return err;
+		goto st_accel_power_off;
 
 	if (irq > 0) {
 		err = st_sensors_allocate_trigger(indio_dev,
@@ -807,6 +809,8 @@ st_accel_device_register_error:
 		st_sensors_deallocate_trigger(indio_dev);
 st_accel_probe_trigger_error:
 	st_accel_deallocate_ring(indio_dev);
+st_accel_power_off:
+	st_sensors_power_disable(indio_dev);
 
 	return err;
 }
drivers/iio/common/st_sensors/st_sensors_core.c +24 -5
index 6901c7f..286f28c 100644
@@ -228,7 +228,7 @@ int st_sensors_set_axis_enable(struct iio_dev *indio_dev, u8 axis_enable)
 }
 EXPORT_SYMBOL(st_sensors_set_axis_enable);
 
-void st_sensors_power_enable(struct iio_dev *indio_dev)
+int st_sensors_power_enable(struct iio_dev *indio_dev)
 {
 	struct st_sensor_data *pdata = iio_priv(indio_dev);
 	int err;
@@ -237,18 +237,37 @@ void st_sensors_power_enable(struct iio_dev *indio_dev)
 	pdata->vdd = devm_regulator_get_optional(indio_dev->dev.parent, "vdd");
 	if (!IS_ERR(pdata->vdd)) {
 		err = regulator_enable(pdata->vdd);
-		if (err != 0)
+		if (err != 0) {
 			dev_warn(&indio_dev->dev,
 				 "Failed to enable specified Vdd supply\n");
+			return err;
+		}
+	} else {
+		err = PTR_ERR(pdata->vdd);
+		if (err != -ENODEV)
+			return err;
 	}
 
 	pdata->vdd_io = devm_regulator_get_optional(indio_dev->dev.parent, "vddio");
 	if (!IS_ERR(pdata->vdd_io)) {
 		err = regulator_enable(pdata->vdd_io);
-		if (err != 0)
+		if (err != 0) {
 			dev_warn(&indio_dev->dev,
 				 "Failed to enable specified Vdd_IO supply\n");
+			goto st_sensors_disable_vdd;
+		}
+	} else {
+		err = PTR_ERR(pdata->vdd_io);
+		if (err != -ENODEV)
+			goto st_sensors_disable_vdd;
 	}
+
+	return 0;
+
+st_sensors_disable_vdd:
+	if (!IS_ERR_OR_NULL(pdata->vdd))
+		regulator_disable(pdata->vdd);
+	return err;
 }
 EXPORT_SYMBOL(st_sensors_power_enable);
 
@@ -256,10 +275,10 @@ void st_sensors_power_disable(struct iio_dev *indio_dev)
 {
 	struct st_sensor_data *pdata = iio_priv(indio_dev);
 
-	if (!IS_ERR(pdata->vdd))
+	if (!IS_ERR_OR_NULL(pdata->vdd))
 		regulator_disable(pdata->vdd);
 
-	if (!IS_ERR(pdata->vdd_io))
+	if (!IS_ERR_OR_NULL(pdata->vdd_io))
 		regulator_disable(pdata->vdd_io);
 }
 EXPORT_SYMBOL(st_sensors_power_disable);
drivers/iio/gyro/st_gyro_core.c +8 -4
index be9057e..f07f105 100644
@@ -424,13 +424,15 @@ int st_gyro_common_probe(struct iio_dev *indio_dev)
 	indio_dev->info = &gyro_info;
 	mutex_init(&gdata->tb.buf_lock);
 
-	st_sensors_power_enable(indio_dev);
+	err = st_sensors_power_enable(indio_dev);
+	if (err)
+		return err;
 
 	err = st_sensors_check_device_support(indio_dev,
 					ARRAY_SIZE(st_gyro_sensors_settings),
 					st_gyro_sensors_settings);
 	if (err < 0)
-		return err;
+		goto st_gyro_power_off;
 
 	gdata->num_data_channels = ST_GYRO_NUMBER_DATA_CHANNELS;
 	gdata->multiread_bit = gdata->sensor_settings->multi_read_bit;
@@ -444,11 +446,11 @@ int st_gyro_common_probe(struct iio_dev *indio_dev)
 	err = st_sensors_init_sensor(indio_dev,
 				(struct st_sensors_platform_data *)&gyro_pdata);
 	if (err < 0)
-		return err;
+		goto st_gyro_power_off;
 
 	err = st_gyro_allocate_ring(indio_dev);
 	if (err < 0)
-		return err;
+		goto st_gyro_power_off;
 
 	if (irq > 0) {
 		err = st_sensors_allocate_trigger(indio_dev,
@@ -471,6 +473,8 @@ st_gyro_device_register_error:
 		st_sensors_deallocate_trigger(indio_dev);
 st_gyro_probe_trigger_error:
 	st_gyro_deallocate_ring(indio_dev);
+st_gyro_power_off:
+	st_sensors_power_disable(indio_dev);
 
 	return err;
 }
drivers/iio/magnetometer/st_magn_core.c +8 -4
index 62036d2..7c94adc 100644
@@ -588,13 +588,15 @@ int st_magn_common_probe(struct iio_dev *indio_dev)
 	indio_dev->info = &magn_info;
 	mutex_init(&mdata->tb.buf_lock);
 
-	st_sensors_power_enable(indio_dev);
+	err = st_sensors_power_enable(indio_dev);
+	if (err)
+		return err;
 
 	err = st_sensors_check_device_support(indio_dev,
 					ARRAY_SIZE(st_magn_sensors_settings),
 					st_magn_sensors_settings);
 	if (err < 0)
-		return err;
+		goto st_magn_power_off;
 
 	mdata->num_data_channels = ST_MAGN_NUMBER_DATA_CHANNELS;
 	mdata->multiread_bit = mdata->sensor_settings->multi_read_bit;
@@ -607,11 +609,11 @@ int st_magn_common_probe(struct iio_dev *indio_dev)
 
 	err = st_sensors_init_sensor(indio_dev, NULL);
 	if (err < 0)
-		return err;
+		goto st_magn_power_off;
 
 	err = st_magn_allocate_ring(indio_dev);
 	if (err < 0)
-		return err;
+		goto st_magn_power_off;
 
 	if (irq > 0) {
 		err = st_sensors_allocate_trigger(indio_dev,
@@ -634,6 +636,8 @@ st_magn_device_register_error:
 		st_sensors_deallocate_trigger(indio_dev);
 st_magn_probe_trigger_error:
 	st_magn_deallocate_ring(indio_dev);
+st_magn_power_off:
+	st_sensors_power_disable(indio_dev);
 
 	return err;
 }
drivers/iio/pressure/st_pressure_core.c +8 -4
index bacdf6c..4b01b19 100644
@@ -588,13 +588,15 @@ int st_press_common_probe(struct iio_dev *indio_dev)
 	indio_dev->info = &press_info;
 	mutex_init(&press_data->tb.buf_lock);
 
-	st_sensors_power_enable(indio_dev);
+	err = st_sensors_power_enable(indio_dev);
+	if (err)
+		return err;
 
 	err = st_sensors_check_device_support(indio_dev,
 					ARRAY_SIZE(st_press_sensors_settings),
 					st_press_sensors_settings);
 	if (err < 0)
-		return err;
+		goto st_press_power_off;
 
 	press_data->num_data_channels = press_data->sensor_settings->num_ch - 1;
 	press_data->multiread_bit = press_data->sensor_settings->multi_read_bit;
@@ -615,11 +617,11 @@ int st_press_common_probe(struct iio_dev *indio_dev)
 
 	err = st_sensors_init_sensor(indio_dev, press_data->dev->platform_data);
 	if (err < 0)
-		return err;
+		goto st_press_power_off;
 
 	err = st_press_allocate_ring(indio_dev);
 	if (err < 0)
-		return err;
+		goto st_press_power_off;
 
 	if (irq > 0) {
 		err = st_sensors_allocate_trigger(indio_dev,
@@ -642,6 +644,8 @@ st_press_device_register_error:
 		st_sensors_deallocate_trigger(indio_dev);
 st_press_probe_trigger_error:
 	st_press_deallocate_ring(indio_dev);
+st_press_power_off:
+	st_sensors_power_disable(indio_dev);
 
 	return err;
 }
include/linux/iio/common/st_sensors.h +0 -0
index 78a1934..91d5f68 100644

include/linux/iio/common/st_sensors.h int st_sensors_set_enable(struct iio_dev *indio_dev, bool enable); · line 284

Quoted 8 lines
include/linux/iio/common/st_sensors.h +1 -1
@@ -278,7 +278,7 @@ int st_sensors_set_enable(struct iio_dev *indio_dev, bool enable);
 
 int st_sensors_set_axis_enable(struct iio_dev *indio_dev, u8 axis_enable);
 
-void st_sensors_power_enable(struct iio_dev *indio_dev);
+int st_sensors_power_enable(struct iio_dev *indio_dev);
 
 void st_sensors_power_disable(struct iio_dev *indio_dev);
 

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

maintainer
#28 lore raw

Re: [RFC PATCH v1 9/9] iio:st_sensors: fix power regulator usage

In reply to Gregor Boirie
Quoted 3 lines · Gregor Boirie

Ensure failure to enable power regulators is properly handled.

This looks good to me so I've applied it to the togreg branch of iio.git

Again, this caused a fair bit of fuzz but all looks straight forward.

If you do a series like this in future adding new part support, please put that new stuff at the end and any fixes / core reworks like this up at the top of the series.

Whilst better error handling is always good to have, this one doesn't strike me as suitable material for stable or even necessary to fast track it into the current cycle. It's more of a case of making sure we tidy up loose ends than a 'bug' fix.

Thanks and sorry for the delay with this seris.

Jonathan

Quoted 269 lines
---
 drivers/iio/accel/st_accel_core.c               | 12 ++++++----
 drivers/iio/common/st_sensors/st_sensors_core.c | 29 ++++++++++++++++++++-----
 drivers/iio/gyro/st_gyro_core.c                 | 12 ++++++----
 drivers/iio/magnetometer/st_magn_core.c         | 12 ++++++----
 drivers/iio/pressure/st_pressure_core.c         | 12 ++++++----
 include/linux/iio/common/st_sensors.h           |  2 +-
 6 files changed, 57 insertions(+), 22 deletions(-)
drivers/iio/accel/st_accel_core.c +8 -4
index dc73f2d..b8d3c3c 100644
@@ -757,13 +757,15 @@ int st_accel_common_probe(struct iio_dev *indio_dev)
 	indio_dev->info = &accel_info;
 	mutex_init(&adata->tb.buf_lock);
 
-	st_sensors_power_enable(indio_dev);
+	err = st_sensors_power_enable(indio_dev);
+	if (err)
+		return err;
 
 	err = st_sensors_check_device_support(indio_dev,
 					ARRAY_SIZE(st_accel_sensors_settings),
 					st_accel_sensors_settings);
 	if (err < 0)
-		return err;
+		goto st_accel_power_off;
 
 	adata->num_data_channels = ST_ACCEL_NUMBER_DATA_CHANNELS;
 	adata->multiread_bit = adata->sensor_settings->multi_read_bit;
@@ -780,11 +782,11 @@ int st_accel_common_probe(struct iio_dev *indio_dev)
 
 	err = st_sensors_init_sensor(indio_dev, adata->dev->platform_data);
 	if (err < 0)
-		return err;
+		goto st_accel_power_off;
 
 	err = st_accel_allocate_ring(indio_dev);
 	if (err < 0)
-		return err;
+		goto st_accel_power_off;
 
 	if (irq > 0) {
 		err = st_sensors_allocate_trigger(indio_dev,
@@ -807,6 +809,8 @@ st_accel_device_register_error:
 		st_sensors_deallocate_trigger(indio_dev);
 st_accel_probe_trigger_error:
 	st_accel_deallocate_ring(indio_dev);
+st_accel_power_off:
+	st_sensors_power_disable(indio_dev);
 
 	return err;
 }
drivers/iio/common/st_sensors/st_sensors_core.c +24 -5
index 6901c7f..286f28c 100644
@@ -228,7 +228,7 @@ int st_sensors_set_axis_enable(struct iio_dev *indio_dev, u8 axis_enable)
 }
 EXPORT_SYMBOL(st_sensors_set_axis_enable);
 
-void st_sensors_power_enable(struct iio_dev *indio_dev)
+int st_sensors_power_enable(struct iio_dev *indio_dev)
 {
 	struct st_sensor_data *pdata = iio_priv(indio_dev);
 	int err;
@@ -237,18 +237,37 @@ void st_sensors_power_enable(struct iio_dev *indio_dev)
 	pdata->vdd = devm_regulator_get_optional(indio_dev->dev.parent, "vdd");
 	if (!IS_ERR(pdata->vdd)) {
 		err = regulator_enable(pdata->vdd);
-		if (err != 0)
+		if (err != 0) {
 			dev_warn(&indio_dev->dev,
 				 "Failed to enable specified Vdd supply\n");
+			return err;
+		}
+	} else {
+		err = PTR_ERR(pdata->vdd);
+		if (err != -ENODEV)
+			return err;
 	}
 
 	pdata->vdd_io = devm_regulator_get_optional(indio_dev->dev.parent, "vddio");
 	if (!IS_ERR(pdata->vdd_io)) {
 		err = regulator_enable(pdata->vdd_io);
-		if (err != 0)
+		if (err != 0) {
 			dev_warn(&indio_dev->dev,
 				 "Failed to enable specified Vdd_IO supply\n");
+			goto st_sensors_disable_vdd;
+		}
+	} else {
+		err = PTR_ERR(pdata->vdd_io);
+		if (err != -ENODEV)
+			goto st_sensors_disable_vdd;
 	}
+
+	return 0;
+
+st_sensors_disable_vdd:
+	if (!IS_ERR_OR_NULL(pdata->vdd))
+		regulator_disable(pdata->vdd);
+	return err;
 }
 EXPORT_SYMBOL(st_sensors_power_enable);
 
@@ -256,10 +275,10 @@ void st_sensors_power_disable(struct iio_dev *indio_dev)
 {
 	struct st_sensor_data *pdata = iio_priv(indio_dev);
 
-	if (!IS_ERR(pdata->vdd))
+	if (!IS_ERR_OR_NULL(pdata->vdd))
 		regulator_disable(pdata->vdd);
 
-	if (!IS_ERR(pdata->vdd_io))
+	if (!IS_ERR_OR_NULL(pdata->vdd_io))
 		regulator_disable(pdata->vdd_io);
 }
 EXPORT_SYMBOL(st_sensors_power_disable);
drivers/iio/gyro/st_gyro_core.c +8 -4
index be9057e..f07f105 100644
@@ -424,13 +424,15 @@ int st_gyro_common_probe(struct iio_dev *indio_dev)
 	indio_dev->info = &gyro_info;
 	mutex_init(&gdata->tb.buf_lock);
 
-	st_sensors_power_enable(indio_dev);
+	err = st_sensors_power_enable(indio_dev);
+	if (err)
+		return err;
 
 	err = st_sensors_check_device_support(indio_dev,
 					ARRAY_SIZE(st_gyro_sensors_settings),
 					st_gyro_sensors_settings);
 	if (err < 0)
-		return err;
+		goto st_gyro_power_off;
 
 	gdata->num_data_channels = ST_GYRO_NUMBER_DATA_CHANNELS;
 	gdata->multiread_bit = gdata->sensor_settings->multi_read_bit;
@@ -444,11 +446,11 @@ int st_gyro_common_probe(struct iio_dev *indio_dev)
 	err = st_sensors_init_sensor(indio_dev,
 				(struct st_sensors_platform_data *)&gyro_pdata);
 	if (err < 0)
-		return err;
+		goto st_gyro_power_off;
 
 	err = st_gyro_allocate_ring(indio_dev);
 	if (err < 0)
-		return err;
+		goto st_gyro_power_off;
 
 	if (irq > 0) {
 		err = st_sensors_allocate_trigger(indio_dev,
@@ -471,6 +473,8 @@ st_gyro_device_register_error:
 		st_sensors_deallocate_trigger(indio_dev);
 st_gyro_probe_trigger_error:
 	st_gyro_deallocate_ring(indio_dev);
+st_gyro_power_off:
+	st_sensors_power_disable(indio_dev);
 
 	return err;
 }
drivers/iio/magnetometer/st_magn_core.c +8 -4
index 62036d2..7c94adc 100644
@@ -588,13 +588,15 @@ int st_magn_common_probe(struct iio_dev *indio_dev)
 	indio_dev->info = &magn_info;
 	mutex_init(&mdata->tb.buf_lock);
 
-	st_sensors_power_enable(indio_dev);
+	err = st_sensors_power_enable(indio_dev);
+	if (err)
+		return err;
 
 	err = st_sensors_check_device_support(indio_dev,
 					ARRAY_SIZE(st_magn_sensors_settings),
 					st_magn_sensors_settings);
 	if (err < 0)
-		return err;
+		goto st_magn_power_off;
 
 	mdata->num_data_channels = ST_MAGN_NUMBER_DATA_CHANNELS;
 	mdata->multiread_bit = mdata->sensor_settings->multi_read_bit;
@@ -607,11 +609,11 @@ int st_magn_common_probe(struct iio_dev *indio_dev)
 
 	err = st_sensors_init_sensor(indio_dev, NULL);
 	if (err < 0)
-		return err;
+		goto st_magn_power_off;
 
 	err = st_magn_allocate_ring(indio_dev);
 	if (err < 0)
-		return err;
+		goto st_magn_power_off;
 
 	if (irq > 0) {
 		err = st_sensors_allocate_trigger(indio_dev,
@@ -634,6 +636,8 @@ st_magn_device_register_error:
 		st_sensors_deallocate_trigger(indio_dev);
 st_magn_probe_trigger_error:
 	st_magn_deallocate_ring(indio_dev);
+st_magn_power_off:
+	st_sensors_power_disable(indio_dev);
 
 	return err;
 }
drivers/iio/pressure/st_pressure_core.c +8 -4
index bacdf6c..4b01b19 100644
@@ -588,13 +588,15 @@ int st_press_common_probe(struct iio_dev *indio_dev)
 	indio_dev->info = &press_info;
 	mutex_init(&press_data->tb.buf_lock);
 
-	st_sensors_power_enable(indio_dev);
+	err = st_sensors_power_enable(indio_dev);
+	if (err)
+		return err;
 
 	err = st_sensors_check_device_support(indio_dev,
 					ARRAY_SIZE(st_press_sensors_settings),
 					st_press_sensors_settings);
 	if (err < 0)
-		return err;
+		goto st_press_power_off;
 
 	press_data->num_data_channels = press_data->sensor_settings->num_ch - 1;
 	press_data->multiread_bit = press_data->sensor_settings->multi_read_bit;
@@ -615,11 +617,11 @@ int st_press_common_probe(struct iio_dev *indio_dev)
 
 	err = st_sensors_init_sensor(indio_dev, press_data->dev->platform_data);
 	if (err < 0)
-		return err;
+		goto st_press_power_off;
 
 	err = st_press_allocate_ring(indio_dev);
 	if (err < 0)
-		return err;
+		goto st_press_power_off;
 
 	if (irq > 0) {
 		err = st_sensors_allocate_trigger(indio_dev,
@@ -642,6 +644,8 @@ st_press_device_register_error:
 		st_sensors_deallocate_trigger(indio_dev);
 st_press_probe_trigger_error:
 	st_press_deallocate_ring(indio_dev);
+st_press_power_off:
+	st_sensors_power_disable(indio_dev);
 
 	return err;
 }
include/linux/iio/common/st_sensors.h +1 -1
index 78a1934..91d5f68 100644
@@ -278,7 +278,7 @@ int st_sensors_set_enable(struct iio_dev *indio_dev, bool enable);
 
 int st_sensors_set_axis_enable(struct iio_dev *indio_dev, u8 axis_enable);
 
-void st_sensors_power_enable(struct iio_dev *indio_dev);
+int st_sensors_power_enable(struct iio_dev *indio_dev);
 
 void st_sensors_power_disable(struct iio_dev *indio_dev);
 
Tested-by on the v1 series
#29 lore raw
In reply to Gregor Boirie
Quoted 3 lines · Gregor Boirie

This preliminary patch series adds support for a new ST LPS22HB pressure sensor and introduce a few fixes related to st_pressure core and st_sensors triggered buffering.

Very cool.

Quoted 5 lines

Patch 2 makes st_pressure sensors compliant with ABI and fixes a few missing sampling gains. Scale / offset computation is modified to address all gains currently possible. It impacts LPS331AP, LPS001WP and LPS25H sensors. Please please please ! If anyone owning one of these could run some tests, I'd be glad to get some feedback since I have none of them.

I applied all nine patches and tested with the LPS001WP on the Ux500.

Before the patch:

$ cat in_pressure_raw && cat in_temp_raw
15217
1763
$ cat in_pressure_raw && cat in_temp_raw
15217
1762
$ cat in_pressure_raw && cat in_temp_raw
15215
1763

After the patch:

$ cat in_pressure_raw && cat in_temp_raw
15232
1652
$ cat in_pressure_raw && cat in_temp_raw
15233
1656
$ cat in_pressure_raw && cat in_temp_raw
15234
1657

Looks reasonable.

For the series:

BTW: I cannot use buffered interval sampling using generic_buffer with triggerless sensors like this one, I suspect that is totally possible to fix, I just never got around to look into it :(

I'll look at the individual patches too, but I suspect Jonathan already gave good feedback on them.

Yours,
Linus Walleij

#30 lore raw
Quoted 11 lines · Linus Walleij

After the patch:

$ cat in_pressure_raw && cat in_temp_raw
15232
1652
$ cat in_pressure_raw && cat in_temp_raw
15233
1656
$ cat in_pressure_raw && cat in_temp_raw
15234
1657

And now I also have in_pressure_scale and in_temp_scale, which yields these values if I multiply each tuple with the scale factor:

95.2
25812.5
95.2
25875
95.2
25890

So ~95 kPa, 25.8 degrees celsius.
Seems like my office environment.
An atmosphere is ~100 kPa IIRC.

Yours,
Linus Walleij

#31 lore raw

Many thanks for taking time to test this. ~95 kPa seems rather a small value to me.

For reference, it seems 950 hPa at sea level would be found under a... hurricane ! (may vary with location)

Does it match with your local weather forecast / report ? grégor

Quoted 30 lines · Linus Walleij
Quoted 11 lines · Linus Walleij

After the patch:

$ cat in_pressure_raw && cat in_temp_raw
15232
1652
$ cat in_pressure_raw && cat in_temp_raw
15233
1656
$ cat in_pressure_raw && cat in_temp_raw
15234
1657

And now I also have in_pressure_scale and in_temp_scale, which yields these values if I multiply each tuple with the scale factor:

95.2
25812.5
95.2
25875
95.2
25890

So ~95 kPa, 25.8 degrees celsius.
Seems like my office environment.
An atmosphere is ~100 kPa IIRC.

Yours,
Linus Walleij

#32 lore raw
Quoted 8 lines · Gregor Boirie

Many thanks for taking time to test this. ~95 kPa seems rather a small value to me.

For reference, it seems 950 hPa at sea level would be found under a... hurricane !
(may vary with location)

Does it match with your local weather forecast / report ?

Hmmmm no :)

Checking with local weather site it should be ~1006 hPa.

Do you think the sensor is just badly calibrated?

I tested now with another sensor on the Snowball board. It gives (after scaling):

101.4
28.8
101.9
28.8
101.9
29.5

This sensor seems more accurate wrt pressure. However here instead the temperature seems bananas, this is a Swedish office, we don't have ~30degrees here... And it's not the board is just turned on so I don't think it's that warm. One sensor gives believeable pressure, the other gives believeable temperature...

The data sheet says this sensor is calibrated for two pressure+temperature combinations at manufacturing. I wonder if there are "secret registers" in the device that can update the calibration...

Anyways, the measures are not totally off scale so I guess they are in the right ballpark. And these patches makes it work better for sure.

Yours,
Linus Walleij

#33 lore raw
Quoted 9 lines · Linus Walleij
Quoted 7 lines · Gregor Boirie

Many thanks for taking time to test this. ~95 kPa seems rather a small value to me.

For reference, it seems 950 hPa at sea level would be found under a... hurricane !
(may vary with location)

Quoted 6 lines
Quoted 1 line

Does it match with your local weather forecast / report ?

Hmmmm no :)

Checking with local weather site it should be ~1006 hPa.

Do you think the sensor is just badly calibrated?

Amongst significant error sources, we regularly struggle against:
* soldering (mechanical, overheat)
* pcb bending and vibrations
* bad device lots, bad storage (esd, wrong calibration, nearby mechanical vibration sources like ultra-sound...)
* electrical noise (power supply, ground...)
* light intensity if not protected by a cover
* temperature, air flow
* probably others we are not aware of yet

All comes with various effects in terms of constant offset, noise and drift. Some are destructive, some are not. Welcome to world of MEMS :)

Quoted 10 lines

I tested now with another sensor on the Snowball board. It gives (after scaling):

101.4
28.8
101.9
28.8
101.9
29.5

Quoted 5 lines

This sensor seems more accurate wrt pressure. However here instead the temperature seems bananas, this is a Swedish office, we don't have ~30degrees here... And it's not the board is just turned on so I don't think it's that warm.

For pressure, LPS001WP may sample with typical absolute error up to +/-25 mBar and resolution of one 16th mBar : not consistent with your first results of ~95 kPa, but should comply with pressure sampling gain modified in patch 4.
No typical accuracy is given for temperature, and resolution is one 64th of degree Celsius. I suppose temperature gain applied in patch 4 is also compliant with all your results.
As a reference for comparison, temperature accuracy for LPS22HB, which is much more precise, is +/-1.5 degree Celsius...

So I simply suggest your erroneous results are not induced by wrong software sampling gains, which was my primary fear. No more no less. Maybe Denis may be of any help once he has some time to review the whole patch series.

Quoted 7 lines

One sensor gives believeable pressure, the other
gives believeable temperature...

The data sheet says this sensor is calibrated for two pressure+temperature combinations at manufacturing. I wonder if there are "secret registers" in the device that can update the calibration...

As this device is deprecated I suppose we won't get much support from ST... Maybe Denis may help if this really worth the pain :)

Quoted 7 lines

Anyways, the measures are not totally off scale so I guess they are in the right ballpark. And these patches makes it work better for sure.

Yours,
Linus Walleij

I do greatly appreciate your feedback. Many thanks. Regards,
greg

#34 lore raw
In reply to Linus Walleij
Quoted 9 lines · Linus Walleij
Quoted 7 lines · Gregor Boirie

Many thanks for taking time to test this. ~95 kPa seems rather a small value to me.

For reference, it seems 950 hPa at sea level would be found under a... hurricane !
(may vary with location)

Quoted 6 lines
Quoted 1 line

Does it match with your local weather forecast / report ?

Hmmmm no :)

Checking with local weather site it should be ~1006 hPa.

Do you think the sensor is just badly calibrated?

Amongst significant error sources, we regularly struggle against:
* soldering (mechanical, overheat)
* pcb bending and vibrations
* bad device lots, bad storage (esd, wrong calibration, nearby mechanical vibration sources like ultra-sound...)
* electrical noise (power supply, ground...)
* light intensity if not protected by a cover
* temperature, air flow
* probably others we are not aware of yet

All comes with various effects in terms of constant offset, noise and drift. Some are destructive, some are not. Welcome to world of MEMS :)

Quoted 10 lines

I tested now with another sensor on the Snowball board. It gives (after scaling):

101.4
28.8
101.9
28.8
101.9
29.5

Quoted 5 lines

This sensor seems more accurate wrt pressure. However here instead the temperature seems bananas, this is a Swedish office, we don't have ~30degrees here... And it's not the board is just turned on so I don't think it's that warm.

For pressure, LPS001WP may sample with typical absolute error up to +/-25 mBar and resolution of one 16th mBar : not consistent with your first results of ~95 kPa, but should comply with pressure sampling gain modified in patch 4.
No typical accuracy is given for temperature, and resolution is one 64th of degree Celsius. I suppose temperature gain applied in patch 4 is also compliant with all your results.
As a reference for comparison, temperature accuracy for LPS22HB, which is much more precise, is +/-1.5 degree Celsius...

So I simply suggest your erroneous results are not induced by wrong software sampling gains, which was my primary fear. No more no less. Maybe Denis may be of any help once he has some time to review the whole patch series.

Quoted 7 lines

One sensor gives believeable pressure, the other
gives believeable temperature...

The data sheet says this sensor is calibrated for two pressure+temperature combinations at manufacturing. I wonder if there are "secret registers" in the device that can update the calibration...

As this device is deprecated I suppose we won't get much support from ST... Maybe Denis may help if this really worth the pain :)

Quoted 6 lines

Anyways, the measures are not totally off scale so I guess they are in the right ballpark. And these patches makes it work better for sure.

Yours,
Linus Walleij

I do greatly appreciate your feedback. Many thanks. Regards,
greg

maintainer
#35 lore raw
In reply to Gregor Boirie
Quoted 30 lines · Gregor Boirie

This preliminary patch series adds support for a new ST LPS22HB pressure sensor and introduce a few fixes related to st_pressure core and st_sensors triggered buffering.
It is not meant to be reviewed for definitive inclusion as it touches too many drivers / devices I cannot test with.
Note that as a few minor and more controversial patches (7, 8 and 9) might also be candidate for a seperate series.

Patch 2 makes st_pressure sensors compliant with ABI and fixes a few missing sampling gains. Scale / offset computation is modified to address all gains currently possible. It impacts LPS331AP, LPS001WP and LPS25H sensors. Please please please ! If anyone owning one of these could run some tests, I'd be glad to get some feedback since I have none of them.

Patch 4 is a rework of the way st_sensors samples are stored in memory to comply with IIO expected alignment contraints (some st_pressure samples are 24 bits long). It is heavily based upon Linux Walleij' approach where each channel is captured individually. See http://www.spinics.net/lists/linux-iio/msg24028.html
and http://www.spinics.net/lists/linux-iio/msg23598.html threads for more infos. This patch impacts all st_sensors.
Please please please ! If anyone owning one of these could run some tests...

Patch 5 enforces 32 bits storage alignment for 24 bits long st_pressure sampling channels.
Patch 6 enables triggered buffering for st_pressure temperature channels. We need temperature samples to control on-board device temperature (noise and drift removal).
Both patches impact st_pressure sensors mentionned above. Please please please ! If anyone owning one of these could run some tests...

Quoted 3 lines

Regards,
gregor.

Hi Gregor,

This series has now drifted out of my active list. Hope you get time to respond to the various minor issues as would be great to get the remainder of these in place!

Not to worry if it will be a while.

Thanks,

Jonathan

Quoted 26 lines

Gregor Boirie (9):
iio:st_pressure:initial lps22hb sensor support iio:st_pressure: fix sampling gains iio:st_pressure: lps22hb temperature support iio:st_sensors: align on storagebits boundaries iio:st_pressure: align storagebits on power of 2 iio:st_pressure: temperature triggered buffering iio:st_sensors: unexport st_sensors_get_buffer_element iio:st_sensors: emulate SMBus block read if needed iio:st_sensors: fix power regulator usage

 .../devicetree/bindings/iio/st-sensors.txt         |   1 +
 drivers/iio/accel/st_accel_core.c                  |  12 +-
 drivers/iio/common/st_sensors/st_sensors_buffer.c  |  41 ++--
 drivers/iio/common/st_sensors/st_sensors_core.c    |  31 ++-
 drivers/iio/common/st_sensors/st_sensors_i2c.c     |   4 +-
 drivers/iio/gyro/st_gyro_core.c                    |  12 +-
 drivers/iio/magnetometer/st_magn_core.c            |  12 +-
 drivers/iio/pressure/Kconfig                       |   2 +-
 drivers/iio/pressure/st_pressure.h                 |   1 +
 drivers/iio/pressure/st_pressure_core.c            | 250 ++++++++++++++++-----
 drivers/iio/pressure/st_pressure_i2c.c             |   4 +
 drivers/iio/pressure/st_pressure_spi.c             |   1 +
 include/linux/iio/common/st_sensors.h              |   4 +-
 13 files changed, 278 insertions(+), 97 deletions(-)
#36 lore raw
Quoted 31 lines · Jonathan Cameron
Quoted 30 lines · Gregor Boirie

This preliminary patch series adds support for a new ST LPS22HB pressure sensor and introduce a few fixes related to st_pressure core and st_sensors triggered buffering.
It is not meant to be reviewed for definitive inclusion as it touches too many drivers / devices I cannot test with.
Note that as a few minor and more controversial patches (7, 8 and 9) might also be candidate for a seperate series.

Patch 2 makes st_pressure sensors compliant with ABI and fixes a few missing sampling gains. Scale / offset computation is modified to address all gains currently possible. It impacts LPS331AP, LPS001WP and LPS25H sensors. Please please please ! If anyone owning one of these could run some tests, I'd be glad to get some feedback since I have none of them.

Patch 4 is a rework of the way st_sensors samples are stored in memory to comply with IIO expected alignment contraints (some st_pressure samples are 24 bits long). It is heavily based upon Linux Walleij' approach where each channel is captured individually. See http://www.spinics.net/lists/linux-iio/msg24028.html
and http://www.spinics.net/lists/linux-iio/msg23598.html threads for more infos. This patch impacts all st_sensors.
Please please please ! If anyone owning one of these could run some tests...

Patch 5 enforces 32 bits storage alignment for 24 bits long st_pressure sampling channels.
Patch 6 enables triggered buffering for st_pressure temperature channels. We need temperature samples to control on-board device temperature (noise and drift removal).
Both patches impact st_pressure sensors mentionned above. Please please please ! If anyone owning one of these could run some tests...

Quoted 4 lines
Quoted 3 lines

Regards,
gregor.

Hi Gregor,

Hi everybody,

getting back to life...

Quoted 5 lines

This series has now drifted out of my active list. Hope you get time to respond to the various minor issues as would be great to get the remainder of these in place!

Not to worry if it will be a while.

I'll have some time to rebase the whole stuff and take your comments into account next week. Anyway, the good point is that the delay prevented me from conflicting with Linus' upstreaming work on triggered buffering machinery :)
Stay tuned.

Regards,
Grégor

Quoted 30 lines

Thanks,

Jonathan

Quoted 26 lines

Gregor Boirie (9):
iio:st_pressure:initial lps22hb sensor support iio:st_pressure: fix sampling gains iio:st_pressure: lps22hb temperature support iio:st_sensors: align on storagebits boundaries iio:st_pressure: align storagebits on power of 2 iio:st_pressure: temperature triggered buffering iio:st_sensors: unexport st_sensors_get_buffer_element iio:st_sensors: emulate SMBus block read if needed iio:st_sensors: fix power regulator usage

  .../devicetree/bindings/iio/st-sensors.txt         |   1 +
  drivers/iio/accel/st_accel_core.c                  |  12 +-
  drivers/iio/common/st_sensors/st_sensors_buffer.c  |  41 ++--
  drivers/iio/common/st_sensors/st_sensors_core.c    |  31 ++-
  drivers/iio/common/st_sensors/st_sensors_i2c.c     |   4 +-
  drivers/iio/gyro/st_gyro_core.c                    |  12 +-
  drivers/iio/magnetometer/st_magn_core.c            |  12 +-
  drivers/iio/pressure/Kconfig                       |   2 +-
  drivers/iio/pressure/st_pressure.h                 |   1 +
  drivers/iio/pressure/st_pressure_core.c            | 250 ++++++++++++++++-----
  drivers/iio/pressure/st_pressure_i2c.c             |   4 +
  drivers/iio/pressure/st_pressure_spi.c             |   1 +
  include/linux/iio/common/st_sensors.h              |   4 +-
  13 files changed, 278 insertions(+), 97 deletions(-)