[RFC PATCH v1 0/9] iio:st_sensors: fixes and lps22hb pressure sensor

[RFC PATCH v1 0/9] iio:st_sensors: fixes and lps22hb pressure sensor

[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...

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(-)

-- 
2.1.4

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

[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

Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
---
 .../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(-)

diff --git a/Documentation/devicetree/bindings/iio/st-sensors.txt b/Documentation/devicetree/bindings/iio/st-sensors.txt
index 637e283..2bb8931 100644
--- a/Documentation/devicetree/bindings/iio/st-sensors.txt
+++ b/Documentation/devicetree/bindings/iio/st-sensors.txt
@@ -63,3 +63,4 @@ Pressure sensors:
 - st,lps001wp-press
 - st,lps25h-press
 - st,lps331ap-press
+- st,lps22hb-press
diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
index 8de0192..d63d280 100644
--- a/drivers/iio/pressure/Kconfig
+++ b/drivers/iio/pressure/Kconfig
@@ -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:
diff --git a/drivers/iio/pressure/st_pressure.h b/drivers/iio/pressure/st_pressure.h
index f5f4149..903a21e 100644
--- a/drivers/iio/pressure/st_pressure.h
+++ b/drivers/iio/pressure/st_pressure.h
@@ -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
diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
index 9e9b72a..c0ff3bf 100644
--- a/drivers/iio/pressure/st_pressure_core.c
+++ b/drivers/iio/pressure/st_pressure_core.c
@@ -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;
 
diff --git a/drivers/iio/pressure/st_pressure_i2c.c b/drivers/iio/pressure/st_pressure_i2c.c
index 8fcf976..ed18701 100644
--- a/drivers/iio/pressure/st_pressure_i2c.c
+++ b/drivers/iio/pressure/st_pressure_i2c.c
@@ -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);
diff --git a/drivers/iio/pressure/st_pressure_spi.c b/drivers/iio/pressure/st_pressure_spi.c
index 40c0692..5505080 100644
--- a/drivers/iio/pressure/st_pressure_spi.c
+++ b/drivers/iio/pressure/st_pressure_spi.c
@@ -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);
-- 
2.1.4

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

[Jonathan Cameron]

On 19/04/16 10:18, Gregor Boirie wrote:
> 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
> 
> Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
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
> ---
>  .../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(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/st-sensors.txt b/Documentation/devicetree/bindings/iio/st-sensors.txt
> index 637e283..2bb8931 100644
> --- a/Documentation/devicetree/bindings/iio/st-sensors.txt
> +++ b/Documentation/devicetree/bindings/iio/st-sensors.txt
> @@ -63,3 +63,4 @@ Pressure sensors:
>  - st,lps001wp-press
>  - st,lps25h-press
>  - st,lps331ap-press
> +- st,lps22hb-press
> diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
> index 8de0192..d63d280 100644
> --- a/drivers/iio/pressure/Kconfig
> +++ b/drivers/iio/pressure/Kconfig
> @@ -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:
> diff --git a/drivers/iio/pressure/st_pressure.h b/drivers/iio/pressure/st_pressure.h
> index f5f4149..903a21e 100644
> --- a/drivers/iio/pressure/st_pressure.h
> +++ b/drivers/iio/pressure/st_pressure.h
> @@ -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
> diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
> index 9e9b72a..c0ff3bf 100644
> --- a/drivers/iio/pressure/st_pressure_core.c
> +++ b/drivers/iio/pressure/st_pressure_core.c
> @@ -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;
>  
> diff --git a/drivers/iio/pressure/st_pressure_i2c.c b/drivers/iio/pressure/st_pressure_i2c.c
> index 8fcf976..ed18701 100644
> --- a/drivers/iio/pressure/st_pressure_i2c.c
> +++ b/drivers/iio/pressure/st_pressure_i2c.c
> @@ -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);
> diff --git a/drivers/iio/pressure/st_pressure_spi.c b/drivers/iio/pressure/st_pressure_spi.c
> index 40c0692..5505080 100644
> --- a/drivers/iio/pressure/st_pressure_spi.c
> +++ b/drivers/iio/pressure/st_pressure_spi.c
> @@ -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);
> 

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

[Jonathan Cameron]

On 24/04/16 10:29, Jonathan Cameron wrote:
> On 19/04/16 10:18, Gregor Boirie wrote:
>> 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
>>
>> Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
> 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
> 
> Jonathan
>> ---
>>  .../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(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/st-sensors.txt b/Documentation/devicetree/bindings/iio/st-sensors.txt
>> index 637e283..2bb8931 100644
>> --- a/Documentation/devicetree/bindings/iio/st-sensors.txt
>> +++ b/Documentation/devicetree/bindings/iio/st-sensors.txt
>> @@ -63,3 +63,4 @@ Pressure sensors:
>>  - st,lps001wp-press
>>  - st,lps25h-press
>>  - st,lps331ap-press
>> +- st,lps22hb-press
>> diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
>> index 8de0192..d63d280 100644
>> --- a/drivers/iio/pressure/Kconfig
>> +++ b/drivers/iio/pressure/Kconfig
>> @@ -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:
>> diff --git a/drivers/iio/pressure/st_pressure.h b/drivers/iio/pressure/st_pressure.h
>> index f5f4149..903a21e 100644
>> --- a/drivers/iio/pressure/st_pressure.h
>> +++ b/drivers/iio/pressure/st_pressure.h
>> @@ -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
>> diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
>> index 9e9b72a..c0ff3bf 100644
>> --- a/drivers/iio/pressure/st_pressure_core.c
>> +++ b/drivers/iio/pressure/st_pressure_core.c
>> @@ -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;
>>  
>> diff --git a/drivers/iio/pressure/st_pressure_i2c.c b/drivers/iio/pressure/st_pressure_i2c.c
>> index 8fcf976..ed18701 100644
>> --- a/drivers/iio/pressure/st_pressure_i2c.c
>> +++ b/drivers/iio/pressure/st_pressure_i2c.c
>> @@ -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);
>> diff --git a/drivers/iio/pressure/st_pressure_spi.c b/drivers/iio/pressure/st_pressure_spi.c
>> index 40c0692..5505080 100644
>> --- a/drivers/iio/pressure/st_pressure_spi.c
>> +++ b/drivers/iio/pressure/st_pressure_spi.c
>> @@ -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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

[Jonathan Cameron]

On 01/05/16 20:28, Jonathan Cameron wrote:
> On 24/04/16 10:29, Jonathan Cameron wrote:
>> On 19/04/16 10:18, Gregor Boirie wrote:
>>> 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
>>>
>>> Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
>> 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
> 
> Jonathan
>>
>> Jonathan
>>> ---
>>>  .../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(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/st-sensors.txt b/Documentation/devicetree/bindings/iio/st-sensors.txt
>>> index 637e283..2bb8931 100644
>>> --- a/Documentation/devicetree/bindings/iio/st-sensors.txt
>>> +++ b/Documentation/devicetree/bindings/iio/st-sensors.txt
>>> @@ -63,3 +63,4 @@ Pressure sensors:
>>>  - st,lps001wp-press
>>>  - st,lps25h-press
>>>  - st,lps331ap-press
>>> +- st,lps22hb-press
>>> diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
>>> index 8de0192..d63d280 100644
>>> --- a/drivers/iio/pressure/Kconfig
>>> +++ b/drivers/iio/pressure/Kconfig
>>> @@ -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:
>>> diff --git a/drivers/iio/pressure/st_pressure.h b/drivers/iio/pressure/st_pressure.h
>>> index f5f4149..903a21e 100644
>>> --- a/drivers/iio/pressure/st_pressure.h
>>> +++ b/drivers/iio/pressure/st_pressure.h
>>> @@ -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
>>> diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
>>> index 9e9b72a..c0ff3bf 100644
>>> --- a/drivers/iio/pressure/st_pressure_core.c
>>> +++ b/drivers/iio/pressure/st_pressure_core.c
>>> @@ -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;
>>>  
>>> diff --git a/drivers/iio/pressure/st_pressure_i2c.c b/drivers/iio/pressure/st_pressure_i2c.c
>>> index 8fcf976..ed18701 100644
>>> --- a/drivers/iio/pressure/st_pressure_i2c.c
>>> +++ b/drivers/iio/pressure/st_pressure_i2c.c
>>> @@ -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);
>>> diff --git a/drivers/iio/pressure/st_pressure_spi.c b/drivers/iio/pressure/st_pressure_spi.c
>>> index 40c0692..5505080 100644
>>> --- a/drivers/iio/pressure/st_pressure_spi.c
>>> +++ b/drivers/iio/pressure/st_pressure_spi.c
>>> @@ -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 majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

[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.

Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
---
 drivers/iio/pressure/st_pressure_core.c | 109 ++++++++++++++++++++++----------
 1 file changed, 75 insertions(+), 34 deletions(-)

diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
index c0ff3bf..b8087b9 100644
--- a/drivers/iio/pressure/st_pressure_core.c
+++ b/drivers/iio/pressure/st_pressure_core.c
@@ -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;
-- 
2.1.4

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

[Jonathan Cameron]

On 19/04/16 10:18, Gregor Boirie wrote:
> 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.
> 
> Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
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

> ---
>  drivers/iio/pressure/st_pressure_core.c | 109 ++++++++++++++++++++++----------
>  1 file changed, 75 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
> index c0ff3bf..b8087b9 100644
> --- a/drivers/iio/pressure/st_pressure_core.c
> +++ b/drivers/iio/pressure/st_pressure_core.c
> @@ -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
They seem to have changed this link..
> + */
> +
> +/* 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)
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.

>  #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
So these are dropped because it isn't controllable?
> -#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;
> 

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

[Linus Walleij]

On Sun, May 29, 2016 at 5:14 PM, Jonathan Cameron <jic23@kernel.org> wrote:

> 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

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

[Jonathan Cameron]

On 30/05/16 09:17, Linus Walleij wrote:
> On Sun, May 29, 2016 at 5:14 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> 
>> 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
> 
> Yours,
> Linus Walleij
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

[Gregor Boirie]

Implement lps22hb temperature sampling channel.

Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
---
 drivers/iio/pressure/st_pressure_core.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
index b8087b9..24754eed 100644
--- a/drivers/iio/pressure/st_pressure_core.c
+++ b/drivers/iio/pressure/st_pressure_core.c
@@ -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,
 				},
 			},
 		},
-- 
2.1.4

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

[Jonathan Cameron]

On 19/04/16 10:18, Gregor Boirie wrote:
> Implement lps22hb temperature sampling channel.
> 
> Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
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
> ---
>  drivers/iio/pressure/st_pressure_core.c | 26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
> index b8087b9..24754eed 100644
> --- a/drivers/iio/pressure/st_pressure_core.c
> +++ b/drivers/iio/pressure/st_pressure_core.c
> @@ -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,
>  				},
>  			},
>  		},
> 

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

[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.

Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
---
 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(-)

diff --git a/drivers/iio/common/st_sensors/st_sensors_buffer.c b/drivers/iio/common/st_sensors/st_sensors_buffer.c
index c558985..a7f1ced 100644
--- a/drivers/iio/common/st_sensors/st_sensors_buffer.c
+++ b/drivers/iio/common/st_sensors/st_sensors_buffer.c
@@ -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
--- a/drivers/iio/common/st_sensors/st_sensors_core.c
+++ b/drivers/iio/common/st_sensors/st_sensors_core.c
@@ -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)
-- 
2.1.4

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

[Jonathan Cameron]

On 19/04/16 10:18, Gregor Boirie wrote:
> 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...


> 
> Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
> ---
>  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(-)
> 
> diff --git a/drivers/iio/common/st_sensors/st_sensors_buffer.c b/drivers/iio/common/st_sensors/st_sensors_buffer.c
> index c558985..a7f1ced 100644
> --- a/drivers/iio/common/st_sensors/st_sensors_buffer.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_buffer.c
> @@ -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
> --- a/drivers/iio/common/st_sensors/st_sensors_core.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
> @@ -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)
> 

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

[Jonathan Cameron]

On 24/04/16 10:35, Jonathan Cameron wrote:
> On 19/04/16 10:18, Gregor Boirie wrote:
>> 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

> 
>>
>> Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
>> ---
>>  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(-)
>>
>> diff --git a/drivers/iio/common/st_sensors/st_sensors_buffer.c b/drivers/iio/common/st_sensors/st_sensors_buffer.c
>> index c558985..a7f1ced 100644
>> --- a/drivers/iio/common/st_sensors/st_sensors_buffer.c
>> +++ b/drivers/iio/common/st_sensors/st_sensors_buffer.c
>> @@ -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
>> --- a/drivers/iio/common/st_sensors/st_sensors_core.c
>> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
>> @@ -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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

[Gregor Boirie]

On 05/01/2016 09:27 PM, Jonathan Cameron wrote:
> On 24/04/16 10:35, Jonathan Cameron wrote:
>> On 19/04/16 10:18, Gregor Boirie wrote:
>>> 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.
I'll give it a try, maybe next week as I'm quite busy right now.
>
> 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
>
>>> Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
>>> ---
>>>   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(-)
>>>
>>> diff --git a/drivers/iio/common/st_sensors/st_sensors_buffer.c b/drivers/iio/common/st_sensors/st_sensors_buffer.c
>>> index c558985..a7f1ced 100644
>>> --- a/drivers/iio/common/st_sensors/st_sensors_buffer.c
>>> +++ b/drivers/iio/common/st_sensors/st_sensors_buffer.c
>>> @@ -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
>>> --- a/drivers/iio/common/st_sensors/st_sensors_core.c
>>> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
>>> @@ -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 majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>

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

[Jonathan Cameron]

On 02/05/16 09:19, Gregor Boirie wrote:
> 
> 
> On 05/01/2016 09:27 PM, Jonathan Cameron wrote:
>> On 24/04/16 10:35, Jonathan Cameron wrote:
>>> On 19/04/16 10:18, Gregor Boirie wrote:
>>>> 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.
> 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.
>>
>> 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...
>>
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).
>> Jonathan
>>
>>>> Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
>>>> ---
>>>>   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(-)
>>>>
>>>> diff --git a/drivers/iio/common/st_sensors/st_sensors_buffer.c b/drivers/iio/common/st_sensors/st_sensors_buffer.c
>>>> index c558985..a7f1ced 100644
>>>> --- a/drivers/iio/common/st_sensors/st_sensors_buffer.c
>>>> +++ b/drivers/iio/common/st_sensors/st_sensors_buffer.c
>>>> @@ -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
>>>> --- a/drivers/iio/common/st_sensors/st_sensors_core.c
>>>> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
>>>> @@ -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 majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Crestez Dan Leonard]

On 04/19/2016 12:18 PM, Gregor Boirie wrote:
> 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?

-- 
Regards,
Leonard

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

[Gregor Boirie]

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

Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
---
 drivers/iio/pressure/st_pressure_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
index 24754eed..13f1c2d 100644
--- a/drivers/iio/pressure/st_pressure_core.c
+++ b/drivers/iio/pressure/st_pressure_core.c
@@ -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 =
-- 
2.1.4

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

[Gregor Boirie]

Enable support for triggered buffering of temperature samples.

Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
---
 drivers/iio/pressure/st_pressure_core.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
index 13f1c2d..bacdf6c 100644
--- a/drivers/iio/pressure/st_pressure_core.c
+++ b/drivers/iio/pressure/st_pressure_core.c
@@ -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;
-- 
2.1.4

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

[Jonathan Cameron]

On 19/04/16 10:18, Gregor Boirie wrote:
> Enable support for triggered buffering of temperature samples.
> 
> Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
I was a little curious as to why this wasn't done previously!

Anyhow, again ideally would like an Ack from Denis.

Jonathan
> ---
>  drivers/iio/pressure/st_pressure_core.c | 20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
> index 13f1c2d..bacdf6c 100644
> --- a/drivers/iio/pressure/st_pressure_core.c
> +++ b/drivers/iio/pressure/st_pressure_core.c
> @@ -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;
> 

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

[Jonathan Cameron]

On 24/04/16 11:58, Jonathan Cameron wrote:
> On 19/04/16 10:18, Gregor Boirie wrote:
>> Enable support for triggered buffering of temperature samples.
>>
>> Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
> 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
> 
> Anyhow, again ideally would like an Ack from Denis.
> 
> Jonathan
>> ---
>>  drivers/iio/pressure/st_pressure_core.c | 20 +++++++++-----------
>>  1 file changed, 9 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
>> index 13f1c2d..bacdf6c 100644
>> --- a/drivers/iio/pressure/st_pressure_core.c
>> +++ b/drivers/iio/pressure/st_pressure_core.c
>> @@ -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;
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

[Gregor Boirie]

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

Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
---
 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(-)

diff --git a/drivers/iio/common/st_sensors/st_sensors_buffer.c b/drivers/iio/common/st_sensors/st_sensors_buffer.c
index a7f1ced..9343d74 100644
--- a/drivers/iio/common/st_sensors/st_sensors_buffer.c
+++ b/drivers/iio/common/st_sensors/st_sensors_buffer.c
@@ -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)
 {
diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
index d029ffa..78a1934 100644
--- a/include/linux/iio/common/st_sensors.h
+++ b/include/linux/iio/common/st_sensors.h
@@ -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
-- 
2.1.4

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

[Jonathan Cameron]

On 19/04/16 10:18, Gregor Boirie wrote:
> Remove st_sensors_get_buffer_element symbol export since not explicitly
> used outside of st_sensors driver.
> 
> Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
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.

> ---
>  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(-)
> 
> diff --git a/drivers/iio/common/st_sensors/st_sensors_buffer.c b/drivers/iio/common/st_sensors/st_sensors_buffer.c
> index a7f1ced..9343d74 100644
> --- a/drivers/iio/common/st_sensors/st_sensors_buffer.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_buffer.c
> @@ -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)
>  {
> diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
> index d029ffa..78a1934 100644
> --- a/include/linux/iio/common/st_sensors.h
> +++ b/include/linux/iio/common/st_sensors.h
> @@ -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
> 

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

[Gregor Boirie]

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

Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
---
 drivers/iio/common/st_sensors/st_sensors_i2c.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/common/st_sensors/st_sensors_i2c.c b/drivers/iio/common/st_sensors/st_sensors_i2c.c
index 98cfee29..b43aa36 100644
--- a/drivers/iio/common/st_sensors/st_sensors_i2c.c
+++ b/drivers/iio/common/st_sensors/st_sensors_i2c.c
@@ -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,
-- 
2.1.4

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

[Jonathan Cameron]

On 19/04/16 10:18, Gregor Boirie wrote:
> Use SMBus "block read" protocol only when supported by adapter.
> 
> Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
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
> ---
>  drivers/iio/common/st_sensors/st_sensors_i2c.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/common/st_sensors/st_sensors_i2c.c b/drivers/iio/common/st_sensors/st_sensors_i2c.c
> index 98cfee29..b43aa36 100644
> --- a/drivers/iio/common/st_sensors/st_sensors_i2c.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_i2c.c
> @@ -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,
> 

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

[Gregor Boirie]

Ensure failure to enable power regulators is properly handled.

Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
---
 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(-)

diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
index dc73f2d..b8d3c3c 100644
--- a/drivers/iio/accel/st_accel_core.c
+++ b/drivers/iio/accel/st_accel_core.c
@@ -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;
 }
diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
index 6901c7f..286f28c 100644
--- a/drivers/iio/common/st_sensors/st_sensors_core.c
+++ b/drivers/iio/common/st_sensors/st_sensors_core.c
@@ -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);
diff --git a/drivers/iio/gyro/st_gyro_core.c b/drivers/iio/gyro/st_gyro_core.c
index be9057e..f07f105 100644
--- a/drivers/iio/gyro/st_gyro_core.c
+++ b/drivers/iio/gyro/st_gyro_core.c
@@ -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;
 }
diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c
index 62036d2..7c94adc 100644
--- a/drivers/iio/magnetometer/st_magn_core.c
+++ b/drivers/iio/magnetometer/st_magn_core.c
@@ -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;
 }
diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
index bacdf6c..4b01b19 100644
--- a/drivers/iio/pressure/st_pressure_core.c
+++ b/drivers/iio/pressure/st_pressure_core.c
@@ -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;
 }
diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
index 78a1934..91d5f68 100644
--- a/include/linux/iio/common/st_sensors.h
+++ b/include/linux/iio/common/st_sensors.h
@@ -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);
 
-- 
2.1.4

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

[Jonathan Cameron]

On 19/04/16 10:18, Gregor Boirie wrote:
> Ensure failure to enable power regulators is properly handled.
> 
> Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
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
> ---
>  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(-)
> 
> diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
> index dc73f2d..b8d3c3c 100644
> --- a/drivers/iio/accel/st_accel_core.c
> +++ b/drivers/iio/accel/st_accel_core.c
> @@ -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;
>  }
> diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
> index 6901c7f..286f28c 100644
> --- a/drivers/iio/common/st_sensors/st_sensors_core.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
> @@ -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);
> diff --git a/drivers/iio/gyro/st_gyro_core.c b/drivers/iio/gyro/st_gyro_core.c
> index be9057e..f07f105 100644
> --- a/drivers/iio/gyro/st_gyro_core.c
> +++ b/drivers/iio/gyro/st_gyro_core.c
> @@ -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;
>  }
> diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c
> index 62036d2..7c94adc 100644
> --- a/drivers/iio/magnetometer/st_magn_core.c
> +++ b/drivers/iio/magnetometer/st_magn_core.c
> @@ -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;
>  }
> diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
> index bacdf6c..4b01b19 100644
> --- a/drivers/iio/pressure/st_pressure_core.c
> +++ b/drivers/iio/pressure/st_pressure_core.c
> @@ -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;
>  }
> diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
> index 78a1934..91d5f68 100644
> --- a/include/linux/iio/common/st_sensors.h
> +++ b/include/linux/iio/common/st_sensors.h
> @@ -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);
>  
> 

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

[Jonathan Cameron]

On 24/04/16 12:01, Jonathan Cameron wrote:
> On 19/04/16 10:18, Gregor Boirie wrote:
>> Ensure failure to enable power regulators is properly handled.
>>
>> Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
> 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 :)
> Jonathan
>> ---
>>  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(-)
>>
>> diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
>> index dc73f2d..b8d3c3c 100644
>> --- a/drivers/iio/accel/st_accel_core.c
>> +++ b/drivers/iio/accel/st_accel_core.c
>> @@ -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;
>>  }
>> diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
>> index 6901c7f..286f28c 100644
>> --- a/drivers/iio/common/st_sensors/st_sensors_core.c
>> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
>> @@ -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);
>> diff --git a/drivers/iio/gyro/st_gyro_core.c b/drivers/iio/gyro/st_gyro_core.c
>> index be9057e..f07f105 100644
>> --- a/drivers/iio/gyro/st_gyro_core.c
>> +++ b/drivers/iio/gyro/st_gyro_core.c
>> @@ -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;
>>  }
>> diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c
>> index 62036d2..7c94adc 100644
>> --- a/drivers/iio/magnetometer/st_magn_core.c
>> +++ b/drivers/iio/magnetometer/st_magn_core.c
>> @@ -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;
>>  }
>> diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
>> index bacdf6c..4b01b19 100644
>> --- a/drivers/iio/pressure/st_pressure_core.c
>> +++ b/drivers/iio/pressure/st_pressure_core.c
>> @@ -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;
>>  }
>> diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
>> index 78a1934..91d5f68 100644
>> --- a/include/linux/iio/common/st_sensors.h
>> +++ b/include/linux/iio/common/st_sensors.h
>> @@ -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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

[Jonathan Cameron]

On 19/04/16 10:18, Gregor Boirie wrote:
> Ensure failure to enable power regulators is properly handled.
> 
> Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
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
> ---
>  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(-)
> 
> diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
> index dc73f2d..b8d3c3c 100644
> --- a/drivers/iio/accel/st_accel_core.c
> +++ b/drivers/iio/accel/st_accel_core.c
> @@ -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;
>  }
> diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
> index 6901c7f..286f28c 100644
> --- a/drivers/iio/common/st_sensors/st_sensors_core.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
> @@ -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);
> diff --git a/drivers/iio/gyro/st_gyro_core.c b/drivers/iio/gyro/st_gyro_core.c
> index be9057e..f07f105 100644
> --- a/drivers/iio/gyro/st_gyro_core.c
> +++ b/drivers/iio/gyro/st_gyro_core.c
> @@ -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;
>  }
> diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c
> index 62036d2..7c94adc 100644
> --- a/drivers/iio/magnetometer/st_magn_core.c
> +++ b/drivers/iio/magnetometer/st_magn_core.c
> @@ -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;
>  }
> diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
> index bacdf6c..4b01b19 100644
> --- a/drivers/iio/pressure/st_pressure_core.c
> +++ b/drivers/iio/pressure/st_pressure_core.c
> @@ -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;
>  }
> diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
> index 78a1934..91d5f68 100644
> --- a/include/linux/iio/common/st_sensors.h
> +++ b/include/linux/iio/common/st_sensors.h
> @@ -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);
>  
> 

Re: [RFC PATCH v1 0/9] iio:st_sensors: fixes and lps22hb pressure sensor

[Linus Walleij]

On Tue, Apr 19, 2016 at 11:18 AM, Gregor Boirie
<gregor.boirie@parrot.com> wrote:

> 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.

> 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:
Tested-by: Linus Walleij <linus.walleij@linaro.org>

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

Re: [RFC PATCH v1 0/9] iio:st_sensors: fixes and lps22hb pressure sensor

[Linus Walleij]

On Wed, Apr 27, 2016 at 1:26 PM, Linus Walleij <linus.walleij@linaro.org> wrote:

> 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

Re: [RFC PATCH v1 0/9] iio:st_sensors: fixes and lps22hb pressure sensor

[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 ?
grégor

On 04/27/2016 02:02 PM, Linus Walleij wrote:
> On Wed, Apr 27, 2016 at 1:26 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
>
>> 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

Re: [RFC PATCH v1 0/9] iio:st_sensors: fixes and lps22hb pressure sensor

[Linus Walleij]

On Wed, Apr 27, 2016 at 3:08 PM, Gregor Boirie <gregor.boirie@parrot.com> wrote:

> 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

Re: [RFC PATCH v1 0/9] iio:st_sensors: fixes and lps22hb pressure sensor

[Gregor Boirie]

[-- Attachment #1: Type: text/plain, Size: 2882 bytes --]

On 04/28/2016 09:47 AM, Linus Walleij wrote:
> On Wed, Apr 27, 2016 at 3:08 PM, Gregor Boirie <gregor.boirie@parrot.com> wrote:
>
>> 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?

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 :)

> 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.
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.

>   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 :)
>
> 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

[-- Attachment #2: Type: text/html, Size: 4146 bytes --]

Re: [RFC PATCH v1 0/9] iio:st_sensors: fixes and lps22hb pressure sensor

[Gregor Boirie]

On 04/28/2016 09:47 AM, Linus Walleij wrote:
> On Wed, Apr 27, 2016 at 3:08 PM, Gregor Boirie<gregor.boirie@parrot.com>  wrote:
>
>> 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?

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 :)

> 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.
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.

>   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 :)
> 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

Re: [RFC PATCH v1 0/9] iio:st_sensors: fixes and lps22hb pressure sensor

[Jonathan Cameron]

On 19/04/16 10:18, Gregor Boirie wrote:
> 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.
> 
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
> 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(-)
> 

Re: [RFC PATCH v1 0/9] iio:st_sensors: fixes and lps22hb pressure sensor

[Gregor Boirie]

On 06/11/2016 07:36 PM, Jonathan Cameron wrote:
> On 19/04/16 10:18, Gregor Boirie wrote:
>> 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.
>>
> Hi Gregor,
Hi everybody,

getting back to life...
> 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
>
> Thanks,
>
> Jonathan
>> 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(-)
>>