[PATCH 00/11] iio: ST clean-ups and new pressure sensor device

[PATCH 00/11] iio: ST clean-ups and new pressure sensor device

[Lee Jones]

This patch-set includes a few clean-ups surrounding error handling and
non-mandatory functionality along with regulator support and the addition
of a new pressure/temperature sensor (LPS001WP). Everything has been
tested with Device Tree.

 arch/arm/boot/dts/ste-dbx5x0.dtsi               |   5 --
 arch/arm/boot/dts/ste-snowball.dts              |  10 ++++
 arch/arm/configs/u8500_defconfig                |   4 ++
 drivers/iio/common/st_sensors/st_sensors_core.c |  11 ++--
 drivers/iio/pressure/st_pressure.h              |   1 +
 drivers/iio/pressure/st_pressure_core.c         | 276 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------
 drivers/iio/pressure/st_pressure_i2c.c          |  18 +++----
 include/linux/iio/common/st_sensors.h           |   4 ++
 8 files changed, 230 insertions(+), 99 deletions(-)

[PATCH 01/11] ARM: ux500: Remove PrimeCell IDs from Nomadik I2C DT nodes

[Lee Jones]

Turns out that they're actually not required and the driver probes just
fine without them. The ID is incorrect at the moment anyway. They actually
currently specify the stn8815.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/ste-dbx5x0.dtsi | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/arm/boot/dts/ste-dbx5x0.dtsi b/arch/arm/boot/dts/ste-dbx5x0.dtsi
index a152945..9c01d66 100644
--- a/arch/arm/boot/dts/ste-dbx5x0.dtsi
+++ b/arch/arm/boot/dts/ste-dbx5x0.dtsi
@@ -531,7 +531,6 @@
 			compatible = "stericsson,db8500-i2c", "st,nomadik-i2c", "arm,primecell";
 			reg = <0x80004000 0x1000>;
 			interrupts = <0 21 IRQ_TYPE_LEVEL_HIGH>;
-			arm,primecell-periphid = <0x180024>;
 
 			#address-cells = <1>;
 			#size-cells = <0>;
@@ -544,7 +543,6 @@
 			compatible = "stericsson,db8500-i2c", "st,nomadik-i2c", "arm,primecell";
 			reg = <0x80122000 0x1000>;
 			interrupts = <0 22 IRQ_TYPE_LEVEL_HIGH>;
-			arm,primecell-periphid = <0x180024>;
 
 			#address-cells = <1>;
 			#size-cells = <0>;
@@ -557,7 +555,6 @@
 			compatible = "stericsson,db8500-i2c", "st,nomadik-i2c", "arm,primecell";
 			reg = <0x80128000 0x1000>;
 			interrupts = <0 55 IRQ_TYPE_LEVEL_HIGH>;
-			arm,primecell-periphid = <0x180024>;
 
 			#address-cells = <1>;
 			#size-cells = <0>;
@@ -570,7 +567,6 @@
 			compatible = "stericsson,db8500-i2c", "st,nomadik-i2c", "arm,primecell";
 			reg = <0x80110000 0x1000>;
 			interrupts = <0 12 IRQ_TYPE_LEVEL_HIGH>;
-			arm,primecell-periphid = <0x180024>;
 
 			#address-cells = <1>;
 			#size-cells = <0>;
@@ -583,7 +579,6 @@
 			compatible = "stericsson,db8500-i2c", "st,nomadik-i2c", "arm,primecell";
 			reg = <0x8012a000 0x1000>;
 			interrupts = <0 51 IRQ_TYPE_LEVEL_HIGH>;
-			arm,primecell-periphid = <0x180024>;
 
 			#address-cells = <1>;
 			#size-cells = <0>;
-- 
1.8.1.2

[PATCH 02/11] ARM: ux500: Enable the LPS001WP Pressure & Temperature sensor from DT

[Lee Jones]

After applying this node the LPS001WP sensor chip should probe
successfully once the driver support has also been applied.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/ste-snowball.dts | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/ste-snowball.dts b/arch/arm/boot/dts/ste-snowball.dts
index cf9b16e..aad8f54 100644
--- a/arch/arm/boot/dts/ste-snowball.dts
+++ b/arch/arm/boot/dts/ste-snowball.dts
@@ -153,6 +153,16 @@
 			status = "okay";
 		};
 
+		i2c@80128000 {
+			lps001wp@5c {
+				compatible = "lps001wp";
+				reg = <0x5c>;
+
+				vdd-supply = <&ab8500_ldo_aux1_reg>;
+				vms-supply = <&db8500_vsmps2_reg>;
+			};
+		};
+
 		uart@80120000 {
 			status = "okay";
 		};
-- 
1.8.1.2

Re: [PATCH 02/11] ARM: ux500: Enable the LPS001WP Pressure & Temperature sensor from DT

[Mark Rutland]

Hi Lee,

On Wed, Sep 04, 2013 at 10:31:34AM +0100, Lee Jones wrote:
> After applying this node the LPS001WP sensor chip should probe
> successfully once the driver support has also been applied.
> 
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  arch/arm/boot/dts/ste-snowball.dts | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/ste-snowball.dts b/arch/arm/boot/dts/ste-snowball.dts
> index cf9b16e..aad8f54 100644
> --- a/arch/arm/boot/dts/ste-snowball.dts
> +++ b/arch/arm/boot/dts/ste-snowball.dts
> @@ -153,6 +153,16 @@
>  			status = "okay";
>  		};
>  
> +		i2c@80128000 {
> +			lps001wp@5c {
> +				compatible = "lps001wp";
> +				reg = <0x5c>;
> +
> +				vdd-supply = <&ab8500_ldo_aux1_reg>;
> +				vms-supply = <&db8500_vsmps2_reg>;
> +			};
> +		};
> +

This appears to be missing a binding document. I couldn't see one
anywhere in this series, or in mainline already).

As far as I can see, the compatible string should be "st,lps001wp".

Please produce a binding document.

Is there any publicly available documentation on the chip?

Thanks,
Mark.

>  		uart@80120000 {
>  			status = "okay";
>  		};
> -- 
> 1.8.1.2
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

Re: [PATCH 02/11] ARM: ux500: Enable the LPS001WP Pressure & Temperature sensor from DT

[Lee Jones]

On Wed, 04 Sep 2013, Mark Rutland wrote:

> Hi Lee,
> 
> On Wed, Sep 04, 2013 at 10:31:34AM +0100, Lee Jones wrote:
> > After applying this node the LPS001WP sensor chip should probe
> > successfully once the driver support has also been applied.
> > 
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  arch/arm/boot/dts/ste-snowball.dts | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/ste-snowball.dts b/arch/arm/boot/dts/ste-snowball.dts
> > index cf9b16e..aad8f54 100644
> > --- a/arch/arm/boot/dts/ste-snowball.dts
> > +++ b/arch/arm/boot/dts/ste-snowball.dts
> > @@ -153,6 +153,16 @@
> >  			status = "okay";
> >  		};
> >  
> > +		i2c@80128000 {
> > +			lps001wp@5c {
> > +				compatible = "lps001wp";
> > +				reg = <0x5c>;
> > +
> > +				vdd-supply = <&ab8500_ldo_aux1_reg>;
> > +				vms-supply = <&db8500_vsmps2_reg>;
> > +			};
> > +		};
> > +
> 
> This appears to be missing a binding document. I couldn't see one
> anywhere in this series, or in mainline already).
> 
> As far as I can see, the compatible string should be "st,lps001wp".

The I2C subsystem doesn't actually care about vendor prefixes. They
are all stripped before use in all cases. However, I'm happy to
provide one if for no other reason than to stick to convention.

> Please produce a binding document.

Again, I'm happy to provide a boilerplate document, but there's
nothing special happening here.

> Is there any publicly available documentation on the chip?

If you Google the device name, it will be the first hit.

> >  		uart@80120000 {
> >  			status = "okay";
> >  		};

Kind regards,
Lee

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 02/11] ARM: ux500: Enable the LPS001WP Pressure & Temperature sensor from DT

[Mark Rutland]

On Wed, Sep 04, 2013 at 02:36:54PM +0100, Lee Jones wrote:
> On Wed, 04 Sep 2013, Mark Rutland wrote:
> 
> > Hi Lee,
> > 
> > On Wed, Sep 04, 2013 at 10:31:34AM +0100, Lee Jones wrote:
> > > After applying this node the LPS001WP sensor chip should probe
> > > successfully once the driver support has also been applied.
> > > 
> > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > > ---
> > >  arch/arm/boot/dts/ste-snowball.dts | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > > 
> > > diff --git a/arch/arm/boot/dts/ste-snowball.dts b/arch/arm/boot/dts/ste-snowball.dts
> > > index cf9b16e..aad8f54 100644
> > > --- a/arch/arm/boot/dts/ste-snowball.dts
> > > +++ b/arch/arm/boot/dts/ste-snowball.dts
> > > @@ -153,6 +153,16 @@
> > >  			status = "okay";
> > >  		};
> > >  
> > > +		i2c@80128000 {
> > > +			lps001wp@5c {
> > > +				compatible = "lps001wp";
> > > +				reg = <0x5c>;
> > > +
> > > +				vdd-supply = <&ab8500_ldo_aux1_reg>;
> > > +				vms-supply = <&db8500_vsmps2_reg>;
> > > +			};
> > > +		};
> > > +
> > 
> > This appears to be missing a binding document. I couldn't see one
> > anywhere in this series, or in mainline already).
> > 
> > As far as I can see, the compatible string should be "st,lps001wp".
> 
> The I2C subsystem doesn't actually care about vendor prefixes. They
> are all stripped before use in all cases. However, I'm happy to
> provide one if for no other reason than to stick to convention.

I'd prefer if we did document this. Just because Linux doesn't care
about this at the moment doesn't mean we (or another OS) might want to
handle it differently in future. It also gives us a semblance of
consistency.

> 
> > Please produce a binding document.
> 
> Again, I'm happy to provide a boilerplate document, but there's
> nothing special happening here.

That's not true. The names of the regulators should be documented. One
seems to be optional for some reason.

> 
> > Is there any publicly available documentation on the chip?
> 
> If you Google the device name, it will be the first hit.

Ok, I'll take a look.

> 
> > >  		uart@80120000 {
> > >  			status = "okay";
> > >  		};
> 
> Kind regards,
> Lee

Cheers,
Mark.

Re: [PATCH 02/11] ARM: ux500: Enable the LPS001WP Pressure & Temperature sensor from DT

[Lee Jones]

On Wed, 04 Sep 2013, Mark Rutland wrote:

> Hi Lee,
> 
> On Wed, Sep 04, 2013 at 10:31:34AM +0100, Lee Jones wrote:
> > After applying this node the LPS001WP sensor chip should probe
> > successfully once the driver support has also been applied.
> > 
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  arch/arm/boot/dts/ste-snowball.dts | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/ste-snowball.dts b/arch/arm/boot/dts/ste-snowball.dts
> > index cf9b16e..aad8f54 100644
> > --- a/arch/arm/boot/dts/ste-snowball.dts
> > +++ b/arch/arm/boot/dts/ste-snowball.dts
> > @@ -153,6 +153,16 @@
> >  			status = "okay";
> >  		};
> >  
> > +		i2c@80128000 {
> > +			lps001wp@5c {
> > +				compatible = "lps001wp";
> > +				reg = <0x5c>;
> > +
> > +				vdd-supply = <&ab8500_ldo_aux1_reg>;
> > +				vms-supply = <&db8500_vsmps2_reg>;
> > +			};
> > +		};
> > +
> 
> This appears to be missing a binding document. I couldn't see one
> anywhere in this series, or in mainline already).
> 
> As far as I can see, the compatible string should be "st,lps001wp".
> 
> Please produce a binding document.

Patch sent, most of you are on CC.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v2 02/11] ARM: ux500: Enable the LPS001WP Pressure & Temperature sensor from DT

[Lee Jones]

After applying this node the LPS001WP sensor chip should probe
successfully once the driver support has also been applied.
    
Signed-off-by: Lee Jones <lee.jones@linaro.org>

diff --git a/arch/arm/boot/dts/ste-snowball.dts b/arch/arm/boot/dts/ste-snowball.dts
index cf9b16e..dc556d1 100644
--- a/arch/arm/boot/dts/ste-snowball.dts
+++ b/arch/arm/boot/dts/ste-snowball.dts
@@ -153,6 +153,16 @@
                        status = "okay";
                };
 
+               i2c@80128000 {
+                       lps001wp@5c {
+                               compatible = "st,lps001wp";
+                               reg = <0x5c>;
+
+                               vdd-supply = <&ab8500_ldo_aux1_reg>;
+                               vms-supply = <&db8500_vsmps2_reg>;
+                       };
+               };
+
                uart@80120000 {
                        status = "okay";
                };

[PATCH 03/11] ARM: ux500: CONFIG: Enable ST's IIO Pressure Sensors by default

[Lee Jones]

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/configs/u8500_defconfig | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/configs/u8500_defconfig b/arch/arm/configs/u8500_defconfig
index a0025dc..d77aa57 100644
--- a/arch/arm/configs/u8500_defconfig
+++ b/arch/arm/configs/u8500_defconfig
@@ -37,6 +37,10 @@ CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug"
 CONFIG_BLK_DEV_RAM=y
 CONFIG_BLK_DEV_RAM_SIZE=65536
 CONFIG_SENSORS_BH1780=y
+CONFIG_IIO=y
+CONFIG_IIO_ST_PRESS=y
+CONFIG_IIO_ST_PRESS_I2C=y
+CONFIG_IIO_ST_SENSORS_I2C=y
 CONFIG_NETDEVICES=y
 CONFIG_SMSC911X=y
 CONFIG_SMSC_PHY=y
-- 
1.8.1.2

[PATCH 04/11] iio: pressure-i2c: st: Simplify error checking in probe()

[Lee Jones]

Strip out all the unnecessary gotos and check for NULL returns in the
usual manner.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/iio/pressure/st_pressure_i2c.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/pressure/st_pressure_i2c.c b/drivers/iio/pressure/st_pressure_i2c.c
index 7cebcc7..2ace770 100644
--- a/drivers/iio/pressure/st_pressure_i2c.c
+++ b/drivers/iio/pressure/st_pressure_i2c.c
@@ -26,10 +26,8 @@ static int st_press_i2c_probe(struct i2c_client *client,
 	int err;
 
 	indio_dev = iio_device_alloc(sizeof(*pdata));
-	if (indio_dev == NULL) {
-		err = -ENOMEM;
-		goto iio_device_alloc_error;
-	}
+	if (!indio_dev)
+		return -ENOMEM;
 
 	pdata = iio_priv(indio_dev);
 	pdata->dev = &client->dev;
@@ -37,15 +35,12 @@ static int st_press_i2c_probe(struct i2c_client *client,
 	st_sensors_i2c_configure(indio_dev, client, pdata);
 
 	err = st_press_common_probe(indio_dev);
-	if (err < 0)
-		goto st_press_common_probe_error;
+	if (err < 0) {
+		iio_device_free(indio_dev);
+		return err;
+	}
 
 	return 0;
-
-st_press_common_probe_error:
-	iio_device_free(indio_dev);
-iio_device_alloc_error:
-	return err;
 }
 
 static int st_press_i2c_remove(struct i2c_client *client)
-- 
1.8.1.2

Re: [PATCH 04/11] iio: pressure-i2c: st: Simplify error checking in probe()

[Jonathan Cameron]

Hi Lee. This won't apply as this driver has already been cleaned up using devm_iio_device_alloc.

Guessing you are basing on an old tree? 
Always use at least staging-next for IIO patches.

I think this one will hit mainline in next few days.

Jonathan

Lee Jones <lee.jones@linaro.org> wrote:
>Strip out all the unnecessary gotos and check for NULL returns in the
>usual manner.
>
>Signed-off-by: Lee Jones <lee.jones@linaro.org>
>---
> drivers/iio/pressure/st_pressure_i2c.c | 17 ++++++-----------
> 1 file changed, 6 insertions(+), 11 deletions(-)
>
>diff --git a/drivers/iio/pressure/st_pressure_i2c.c
>b/drivers/iio/pressure/st_pressure_i2c.c
>index 7cebcc7..2ace770 100644
>--- a/drivers/iio/pressure/st_pressure_i2c.c
>+++ b/drivers/iio/pressure/st_pressure_i2c.c
>@@ -26,10 +26,8 @@ static int st_press_i2c_probe(struct i2c_client
>*client,
> 	int err;
> 
> 	indio_dev = iio_device_alloc(sizeof(*pdata));
>-	if (indio_dev == NULL) {
>-		err = -ENOMEM;
>-		goto iio_device_alloc_error;
>-	}
>+	if (!indio_dev)
>+		return -ENOMEM;
> 
> 	pdata = iio_priv(indio_dev);
> 	pdata->dev = &client->dev;
>@@ -37,15 +35,12 @@ static int st_press_i2c_probe(struct i2c_client
>*client,
> 	st_sensors_i2c_configure(indio_dev, client, pdata);
> 
> 	err = st_press_common_probe(indio_dev);
>-	if (err < 0)
>-		goto st_press_common_probe_error;
>+	if (err < 0) {
>+		iio_device_free(indio_dev);
>+		return err;
>+	}
> 
> 	return 0;
>-
>-st_press_common_probe_error:
>-	iio_device_free(indio_dev);
>-iio_device_alloc_error:
>-	return err;
> }
> 
> static int st_press_i2c_remove(struct i2c_client *client)

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

Re: [PATCH 04/11] iio: pressure-i2c: st: Simplify error checking in probe()

[Lee Jones]

> Hi Lee. This won't apply as this driver has already been cleaned up using devm_iio_device_alloc.
> 
> Guessing you are basing on an old tree? 
> Always use at least staging-next for IIO patches.

Sure, I'll rebase and resubmit the set once all the other creases have
been ironed out.

> I think this one will hit mainline in next few days.

Okay, great.

> Lee Jones <lee.jones@linaro.org> wrote:
> >Strip out all the unnecessary gotos and check for NULL returns in the
> >usual manner.
> >
> >Signed-off-by: Lee Jones <lee.jones@linaro.org>
> >---
> > drivers/iio/pressure/st_pressure_i2c.c | 17 ++++++-----------
> > 1 file changed, 6 insertions(+), 11 deletions(-)
> >
> >diff --git a/drivers/iio/pressure/st_pressure_i2c.c
> >b/drivers/iio/pressure/st_pressure_i2c.c
> >index 7cebcc7..2ace770 100644
> >--- a/drivers/iio/pressure/st_pressure_i2c.c
> >+++ b/drivers/iio/pressure/st_pressure_i2c.c
> >@@ -26,10 +26,8 @@ static int st_press_i2c_probe(struct i2c_client
> >*client,
> > 	int err;
> > 
> > 	indio_dev = iio_device_alloc(sizeof(*pdata));
> >-	if (indio_dev == NULL) {
> >-		err = -ENOMEM;
> >-		goto iio_device_alloc_error;
> >-	}
> >+	if (!indio_dev)
> >+		return -ENOMEM;
> > 
> > 	pdata = iio_priv(indio_dev);
> > 	pdata->dev = &client->dev;
> >@@ -37,15 +35,12 @@ static int st_press_i2c_probe(struct i2c_client
> >*client,
> > 	st_sensors_i2c_configure(indio_dev, client, pdata);
> > 
> > 	err = st_press_common_probe(indio_dev);
> >-	if (err < 0)
> >-		goto st_press_common_probe_error;
> >+	if (err < 0) {
> >+		iio_device_free(indio_dev);
> >+		return err;
> >+	}
> > 
> > 	return 0;
> >-
> >-st_press_common_probe_error:
> >-	iio_device_free(indio_dev);
> >-iio_device_alloc_error:
> >-	return err;
> > }
> > 
> > static int st_press_i2c_remove(struct i2c_client *client)
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 05/11] iio: pressure-core: st: Describe LPS331AP defines by name

[Lee Jones]

They're currently named *_1_*, for 'Sensor 1', but the code will be much
more readable if we use the naming convention *_LPS331AP_* instead.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/iio/pressure/st_pressure_core.c | 90 ++++++++++++++++-----------------
 1 file changed, 44 insertions(+), 46 deletions(-)

diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
index 9c343b4..becfb25 100644
--- a/drivers/iio/pressure/st_pressure_core.c
+++ b/drivers/iio/pressure/st_pressure_core.c
@@ -31,92 +31,90 @@
 #define ST_PRESS_MBAR_TO_KPASCAL(x)		(x * 10)
 #define ST_PRESS_NUMBER_DATA_CHANNELS		1
 
-/* DEFAULT VALUE FOR SENSORS */
-#define ST_PRESS_DEFAULT_OUT_XL_ADDR		0x28
-#define ST_TEMP_DEFAULT_OUT_L_ADDR		0x2b
-
 /* FULLSCALE */
 #define ST_PRESS_FS_AVL_1260MB			1260
 
-/* CUSTOM VALUES FOR SENSOR 1 */
-#define ST_PRESS_1_WAI_EXP			0xbb
-#define ST_PRESS_1_ODR_ADDR			0x20
-#define ST_PRESS_1_ODR_MASK			0x70
-#define ST_PRESS_1_ODR_AVL_1HZ_VAL		0x01
-#define ST_PRESS_1_ODR_AVL_7HZ_VAL		0x05
-#define ST_PRESS_1_ODR_AVL_13HZ_VAL		0x06
-#define ST_PRESS_1_ODR_AVL_25HZ_VAL		0x07
-#define ST_PRESS_1_PW_ADDR			0x20
-#define ST_PRESS_1_PW_MASK			0x80
-#define ST_PRESS_1_FS_ADDR			0x23
-#define ST_PRESS_1_FS_MASK			0x30
-#define ST_PRESS_1_FS_AVL_1260_VAL		0x00
-#define ST_PRESS_1_FS_AVL_1260_GAIN		ST_PRESS_MBAR_TO_KPASCAL(244141)
-#define ST_PRESS_1_FS_AVL_TEMP_GAIN		2083000
-#define ST_PRESS_1_BDU_ADDR			0x20
-#define ST_PRESS_1_BDU_MASK			0x04
-#define ST_PRESS_1_DRDY_IRQ_ADDR		0x22
-#define ST_PRESS_1_DRDY_IRQ_MASK		0x04
-#define ST_PRESS_1_MULTIREAD_BIT		true
-#define ST_PRESS_1_TEMP_OFFSET			42500
+/* CUSTOM VALUES FOR LPS331AP SENSOR */
+#define ST_PRESS_LPS331AP_WAI_EXP		0xbb
+#define ST_PRESS_LPS331AP_ODR_ADDR		0x20
+#define ST_PRESS_LPS331AP_ODR_MASK		0x70
+#define ST_PRESS_LPS331AP_ODR_AVL_1HZ_VAL	0x01
+#define ST_PRESS_LPS331AP_ODR_AVL_7HZ_VAL	0x05
+#define ST_PRESS_LPS331AP_ODR_AVL_13HZ_VAL	0x06
+#define ST_PRESS_LPS331AP_ODR_AVL_25HZ_VAL	0x07
+#define ST_PRESS_LPS331AP_PW_ADDR		0x20
+#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_MBAR_TO_KPASCAL(244141)
+#define ST_PRESS_LPS331AP_FS_AVL_TEMP_GAIN	2083000
+#define ST_PRESS_LPS331AP_BDU_ADDR		0x20
+#define ST_PRESS_LPS331AP_BDU_MASK		0x04
+#define ST_PRESS_LPS331AP_DRDY_IRQ_ADDR		0x22
+#define ST_PRESS_LPS331AP_DRDY_IRQ_MASK		0x04
+#define ST_PRESS_LPS331AP_MULTIREAD_BIT		true
+#define ST_PRESS_LPS331AP_TEMP_OFFSET		42500
+#define ST_PRESS_LPS331AP_OUT_XL_ADDR		0x28
+#define ST_TEMP_LPS331AP_OUT_L_ADDR		0x2b
 
 static const struct iio_chan_spec st_press_channels[] = {
 	ST_SENSORS_LSM_CHANNELS(IIO_PRESSURE,
 			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
 			ST_SENSORS_SCAN_X, 0, IIO_NO_MOD, 'u', IIO_LE, 24, 24,
-			ST_PRESS_DEFAULT_OUT_XL_ADDR),
+			ST_PRESS_LPS331AP_OUT_XL_ADDR),
 	ST_SENSORS_LSM_CHANNELS(IIO_TEMP,
 			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE) |
 						BIT(IIO_CHAN_INFO_OFFSET),
 			-1, 0, IIO_NO_MOD, 's', IIO_LE, 16, 16,
-			ST_TEMP_DEFAULT_OUT_L_ADDR),
+			ST_TEMP_LPS331AP_OUT_L_ADDR),
 	IIO_CHAN_SOFT_TIMESTAMP(1)
 };
 
 static const struct st_sensors st_press_sensors[] = {
 	{
-		.wai = ST_PRESS_1_WAI_EXP,
+		.wai = ST_PRESS_LPS331AP_WAI_EXP,
 		.sensors_supported = {
 			[0] = LPS331AP_PRESS_DEV_NAME,
 		},
 		.ch = (struct iio_chan_spec *)st_press_channels,
 		.odr = {
-			.addr = ST_PRESS_1_ODR_ADDR,
-			.mask = ST_PRESS_1_ODR_MASK,
+			.addr = ST_PRESS_LPS331AP_ODR_ADDR,
+			.mask = ST_PRESS_LPS331AP_ODR_MASK,
 			.odr_avl = {
-				{ 1, ST_PRESS_1_ODR_AVL_1HZ_VAL, },
-				{ 7, ST_PRESS_1_ODR_AVL_7HZ_VAL, },
-				{ 13, ST_PRESS_1_ODR_AVL_13HZ_VAL, },
-				{ 25, ST_PRESS_1_ODR_AVL_25HZ_VAL, },
+				{ 1, ST_PRESS_LPS331AP_ODR_AVL_1HZ_VAL, },
+				{ 7, ST_PRESS_LPS331AP_ODR_AVL_7HZ_VAL, },
+				{ 13, ST_PRESS_LPS331AP_ODR_AVL_13HZ_VAL, },
+				{ 25, ST_PRESS_LPS331AP_ODR_AVL_25HZ_VAL, },
 			},
 		},
 		.pw = {
-			.addr = ST_PRESS_1_PW_ADDR,
-			.mask = ST_PRESS_1_PW_MASK,
+			.addr = ST_PRESS_LPS331AP_PW_ADDR,
+			.mask = ST_PRESS_LPS331AP_PW_MASK,
 			.value_on = ST_SENSORS_DEFAULT_POWER_ON_VALUE,
 			.value_off = ST_SENSORS_DEFAULT_POWER_OFF_VALUE,
 		},
 		.fs = {
-			.addr = ST_PRESS_1_FS_ADDR,
-			.mask = ST_PRESS_1_FS_MASK,
+			.addr = ST_PRESS_LPS331AP_FS_ADDR,
+			.mask = ST_PRESS_LPS331AP_FS_MASK,
 			.fs_avl = {
 				[0] = {
 					.num = ST_PRESS_FS_AVL_1260MB,
-					.value = ST_PRESS_1_FS_AVL_1260_VAL,
-					.gain = ST_PRESS_1_FS_AVL_1260_GAIN,
-					.gain2 = ST_PRESS_1_FS_AVL_TEMP_GAIN,
+					.value = ST_PRESS_LPS331AP_FS_AVL_1260_VAL,
+					.gain = ST_PRESS_LPS331AP_FS_AVL_1260_GAIN,
+					.gain2 = ST_PRESS_LPS331AP_FS_AVL_TEMP_GAIN,
 				},
 			},
 		},
 		.bdu = {
-			.addr = ST_PRESS_1_BDU_ADDR,
-			.mask = ST_PRESS_1_BDU_MASK,
+			.addr = ST_PRESS_LPS331AP_BDU_ADDR,
+			.mask = ST_PRESS_LPS331AP_BDU_MASK,
 		},
 		.drdy_irq = {
-			.addr = ST_PRESS_1_DRDY_IRQ_ADDR,
-			.mask = ST_PRESS_1_DRDY_IRQ_MASK,
+			.addr = ST_PRESS_LPS331AP_DRDY_IRQ_ADDR,
+			.mask = ST_PRESS_LPS331AP_DRDY_IRQ_MASK,
 		},
-		.multi_read_bit = ST_PRESS_1_MULTIREAD_BIT,
+		.multi_read_bit = ST_PRESS_LPS331AP_MULTIREAD_BIT,
 		.bootime = 2,
 	},
 };
-- 
1.8.1.2

Re: [PATCH 05/11] iio: pressure-core: st: Describe LPS331AP defines by name

[Denis CIOCCA]

Hi Lee,
> They're currently named *_1_*, for 'Sensor 1', but the code will be much
> more readable if we use the naming convention *_LPS331AP_* instead.
You are right, but the reason is to maintain the same structure of the=20
other sensors drivers (like accel, gyro and magn). Often some sensors=20
can use the same data and using 1,2,... I think it's more general.

Denis

> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>   drivers/iio/pressure/st_pressure_core.c | 90 ++++++++++++++++----------=
-------
>   1 file changed, 44 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressu=
re/st_pressure_core.c
> index 9c343b4..becfb25 100644
> --- a/drivers/iio/pressure/st_pressure_core.c
> +++ b/drivers/iio/pressure/st_pressure_core.c
> @@ -31,92 +31,90 @@
>   #define ST_PRESS_MBAR_TO_KPASCAL(x)		(x * 10)
>   #define ST_PRESS_NUMBER_DATA_CHANNELS		1
>  =20
> -/* DEFAULT VALUE FOR SENSORS */
> -#define ST_PRESS_DEFAULT_OUT_XL_ADDR		0x28
> -#define ST_TEMP_DEFAULT_OUT_L_ADDR		0x2b
> -
>   /* FULLSCALE */
>   #define ST_PRESS_FS_AVL_1260MB			1260
>  =20
> -/* CUSTOM VALUES FOR SENSOR 1 */
> -#define ST_PRESS_1_WAI_EXP			0xbb
> -#define ST_PRESS_1_ODR_ADDR			0x20
> -#define ST_PRESS_1_ODR_MASK			0x70
> -#define ST_PRESS_1_ODR_AVL_1HZ_VAL		0x01
> -#define ST_PRESS_1_ODR_AVL_7HZ_VAL		0x05
> -#define ST_PRESS_1_ODR_AVL_13HZ_VAL		0x06
> -#define ST_PRESS_1_ODR_AVL_25HZ_VAL		0x07
> -#define ST_PRESS_1_PW_ADDR			0x20
> -#define ST_PRESS_1_PW_MASK			0x80
> -#define ST_PRESS_1_FS_ADDR			0x23
> -#define ST_PRESS_1_FS_MASK			0x30
> -#define ST_PRESS_1_FS_AVL_1260_VAL		0x00
> -#define ST_PRESS_1_FS_AVL_1260_GAIN		ST_PRESS_MBAR_TO_KPASCAL(244141)
> -#define ST_PRESS_1_FS_AVL_TEMP_GAIN		2083000
> -#define ST_PRESS_1_BDU_ADDR			0x20
> -#define ST_PRESS_1_BDU_MASK			0x04
> -#define ST_PRESS_1_DRDY_IRQ_ADDR		0x22
> -#define ST_PRESS_1_DRDY_IRQ_MASK		0x04
> -#define ST_PRESS_1_MULTIREAD_BIT		true
> -#define ST_PRESS_1_TEMP_OFFSET			42500
> +/* CUSTOM VALUES FOR LPS331AP SENSOR */
> +#define ST_PRESS_LPS331AP_WAI_EXP		0xbb
> +#define ST_PRESS_LPS331AP_ODR_ADDR		0x20
> +#define ST_PRESS_LPS331AP_ODR_MASK		0x70
> +#define ST_PRESS_LPS331AP_ODR_AVL_1HZ_VAL	0x01
> +#define ST_PRESS_LPS331AP_ODR_AVL_7HZ_VAL	0x05
> +#define ST_PRESS_LPS331AP_ODR_AVL_13HZ_VAL	0x06
> +#define ST_PRESS_LPS331AP_ODR_AVL_25HZ_VAL	0x07
> +#define ST_PRESS_LPS331AP_PW_ADDR		0x20
> +#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_MBAR_TO_KPASCAL(2441=
41)
> +#define ST_PRESS_LPS331AP_FS_AVL_TEMP_GAIN	2083000
> +#define ST_PRESS_LPS331AP_BDU_ADDR		0x20
> +#define ST_PRESS_LPS331AP_BDU_MASK		0x04
> +#define ST_PRESS_LPS331AP_DRDY_IRQ_ADDR		0x22
> +#define ST_PRESS_LPS331AP_DRDY_IRQ_MASK		0x04
> +#define ST_PRESS_LPS331AP_MULTIREAD_BIT		true
> +#define ST_PRESS_LPS331AP_TEMP_OFFSET		42500
> +#define ST_PRESS_LPS331AP_OUT_XL_ADDR		0x28
> +#define ST_TEMP_LPS331AP_OUT_L_ADDR		0x2b
>  =20
>   static const struct iio_chan_spec st_press_channels[] =3D {
>   	ST_SENSORS_LSM_CHANNELS(IIO_PRESSURE,
>   			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>   			ST_SENSORS_SCAN_X, 0, IIO_NO_MOD, 'u', IIO_LE, 24, 24,
> -			ST_PRESS_DEFAULT_OUT_XL_ADDR),
> +			ST_PRESS_LPS331AP_OUT_XL_ADDR),
>   	ST_SENSORS_LSM_CHANNELS(IIO_TEMP,
>   			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE) |
>   						BIT(IIO_CHAN_INFO_OFFSET),
>   			-1, 0, IIO_NO_MOD, 's', IIO_LE, 16, 16,
> -			ST_TEMP_DEFAULT_OUT_L_ADDR),
> +			ST_TEMP_LPS331AP_OUT_L_ADDR),
>   	IIO_CHAN_SOFT_TIMESTAMP(1)
>   };
>  =20
>   static const struct st_sensors st_press_sensors[] =3D {
>   	{
> -		.wai =3D ST_PRESS_1_WAI_EXP,
> +		.wai =3D ST_PRESS_LPS331AP_WAI_EXP,
>   		.sensors_supported =3D {
>   			[0] =3D LPS331AP_PRESS_DEV_NAME,
>   		},
>   		.ch =3D (struct iio_chan_spec *)st_press_channels,
>   		.odr =3D {
> -			.addr =3D ST_PRESS_1_ODR_ADDR,
> -			.mask =3D ST_PRESS_1_ODR_MASK,
> +			.addr =3D ST_PRESS_LPS331AP_ODR_ADDR,
> +			.mask =3D ST_PRESS_LPS331AP_ODR_MASK,
>   			.odr_avl =3D {
> -				{ 1, ST_PRESS_1_ODR_AVL_1HZ_VAL, },
> -				{ 7, ST_PRESS_1_ODR_AVL_7HZ_VAL, },
> -				{ 13, ST_PRESS_1_ODR_AVL_13HZ_VAL, },
> -				{ 25, ST_PRESS_1_ODR_AVL_25HZ_VAL, },
> +				{ 1, ST_PRESS_LPS331AP_ODR_AVL_1HZ_VAL, },
> +				{ 7, ST_PRESS_LPS331AP_ODR_AVL_7HZ_VAL, },
> +				{ 13, ST_PRESS_LPS331AP_ODR_AVL_13HZ_VAL, },
> +				{ 25, ST_PRESS_LPS331AP_ODR_AVL_25HZ_VAL, },
>   			},
>   		},
>   		.pw =3D {
> -			.addr =3D ST_PRESS_1_PW_ADDR,
> -			.mask =3D ST_PRESS_1_PW_MASK,
> +			.addr =3D ST_PRESS_LPS331AP_PW_ADDR,
> +			.mask =3D ST_PRESS_LPS331AP_PW_MASK,
>   			.value_on =3D ST_SENSORS_DEFAULT_POWER_ON_VALUE,
>   			.value_off =3D ST_SENSORS_DEFAULT_POWER_OFF_VALUE,
>   		},
>   		.fs =3D {
> -			.addr =3D ST_PRESS_1_FS_ADDR,
> -			.mask =3D ST_PRESS_1_FS_MASK,
> +			.addr =3D ST_PRESS_LPS331AP_FS_ADDR,
> +			.mask =3D ST_PRESS_LPS331AP_FS_MASK,
>   			.fs_avl =3D {
>   				[0] =3D {
>   					.num =3D ST_PRESS_FS_AVL_1260MB,
> -					.value =3D ST_PRESS_1_FS_AVL_1260_VAL,
> -					.gain =3D ST_PRESS_1_FS_AVL_1260_GAIN,
> -					.gain2 =3D ST_PRESS_1_FS_AVL_TEMP_GAIN,
> +					.value =3D ST_PRESS_LPS331AP_FS_AVL_1260_VAL,
> +					.gain =3D ST_PRESS_LPS331AP_FS_AVL_1260_GAIN,
> +					.gain2 =3D ST_PRESS_LPS331AP_FS_AVL_TEMP_GAIN,
>   				},
>   			},
>   		},
>   		.bdu =3D {
> -			.addr =3D ST_PRESS_1_BDU_ADDR,
> -			.mask =3D ST_PRESS_1_BDU_MASK,
> +			.addr =3D ST_PRESS_LPS331AP_BDU_ADDR,
> +			.mask =3D ST_PRESS_LPS331AP_BDU_MASK,
>   		},
>   		.drdy_irq =3D {
> -			.addr =3D ST_PRESS_1_DRDY_IRQ_ADDR,
> -			.mask =3D ST_PRESS_1_DRDY_IRQ_MASK,
> +			.addr =3D ST_PRESS_LPS331AP_DRDY_IRQ_ADDR,
> +			.mask =3D ST_PRESS_LPS331AP_DRDY_IRQ_MASK,
>   		},
> -		.multi_read_bit =3D ST_PRESS_1_MULTIREAD_BIT,
> +		.multi_read_bit =3D ST_PRESS_LPS331AP_MULTIREAD_BIT,
>   		.bootime =3D 2,
>   	},
>   };

Re: [PATCH 05/11] iio: pressure-core: st: Describe LPS331AP defines by name

[Lee Jones]

> > They're currently named *_1_*, for 'Sensor 1', but the code will be much
> > more readable if we use the naming convention *_LPS331AP_* instead.
> You are right, but the reason is to maintain the same structure of the 
> other sensors drivers (like accel, gyro and magn). Often some sensors 
> can use the same data and using 1,2,... I think it's more general.

I've just coded this up for the pressure sensor [1] and in my opinion it
looks pretty ugly. We save 5 lines in this usecase, but we're
separating out some of the register addresses from the masks
etc.

Besides some code space #defines don't actually cost anything, so
unless the register set is almost identical I'd suggest putting them
in a header file out the way and separating them out like in the
original patch. I personally think it makes things so much clearer for
the reader.

I haven't looked at the other sensors yet, so perhaps we need to
evaluate this on a case by case basis, but certainly for this driver,
as we only support two sensors currently and for the sake of 5 lines I
think it's better to have them clearly defined per-sensor.

What do you think?

[1]:

/* GROUP VALUES SENSOR */
#define ST_PRESS_1_ODR_ADDR			0x20
#define ST_PRESS_1_ODR_AVL_1HZ_VAL		0x01
#define ST_PRESS_1_PW_ADDR			0x20
#define ST_PRESS_1_BDU_ADDR			0x20
#define ST_PRESS_1_MULTIREAD_BIT		true

/* CUSTOM VALUES FOR LPS331AP SENSOR */
#define ST_PRESS_LPS331AP_WAI_EXP		0xbb
#define ST_PRESS_LPS331AP_ODR_MASK		0x70
#define ST_PRESS_LPS331AP_ODR_AVL_7HZ_VAL	0x05
#define ST_PRESS_LPS331AP_ODR_AVL_13HZ_VAL	0x06
#define ST_PRESS_LPS331AP_ODR_AVL_25HZ_VAL	0x07
#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_MBAR_TO_KPASCAL(244141)
#define ST_PRESS_LPS331AP_FS_AVL_TEMP_GAIN	2083000
#define ST_PRESS_LPS331AP_BDU_MASK		0x04
#define ST_PRESS_LPS331AP_DRDY_IRQ_ADDR		0x22
#define ST_PRESS_LPS331AP_DRDY_IRQ_MASK		0x04
#define ST_PRESS_LPS331AP_TEMP_OFFSET		42500
#define ST_PRESS_LPS331AP_OUT_XL_ADDR		0x28
#define ST_TEMP_LPS331AP_OUT_L_ADDR		0x2b

/* CUSTOM VALUES FOR LPS001WP SENSOR */
#define ST_PRESS_LPS001WP_WAI_EXP		0xba
#define ST_PRESS_LPS001WP_ODR_MASK		0x30
#define ST_PRESS_LPS001WP_ODR_AVL_7HZ_VAL	0x02
#define ST_PRESS_LPS001WP_ODR_AVL_13HZ_VAL	0x03
#define ST_PRESS_LPS001WP_PW_MASK		0x40
#define ST_PRESS_LPS001WP_BDU_MASK		0x04
#define ST_PRESS_LPS001WP_OUT_L_ADDR		0x28
#define ST_TEMP_LPS001WP_OUT_L_ADDR		0x2a

static const struct iio_chan_spec st_press_lsp331ap_channels[] = {
	{
		.type = IIO_PRESSURE,
		.channel2 = IIO_NO_MOD,
		.address = ST_PRESS_LPS331AP_OUT_XL_ADDR,
		.scan_index = ST_SENSORS_SCAN_X,
		.scan_type = {
			.sign = 'u',
			.realbits = 24,
			.storagebits = 24,
			.endianness = IIO_LE,
		},
		.info_mask_separate =
			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
		.modified = 0,
	},
	{
		.type = IIO_TEMP,
		.channel2 = IIO_NO_MOD,
		.address = ST_TEMP_LPS331AP_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) |
			BIT(IIO_CHAN_INFO_OFFSET),
		.modified = 0,
	},
	IIO_CHAN_SOFT_TIMESTAMP(1)
};

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_type = {
			.sign = 'u',
			.realbits = 16,
			.storagebits = 16,
			.endianness = IIO_LE,
		},
		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
		.modified = 0,
	},
	{
		.type = IIO_TEMP,
		.channel2 = IIO_NO_MOD,
		.address = ST_TEMP_LPS001WP_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_OFFSET),
		.modified = 0,
	},
	IIO_CHAN_SOFT_TIMESTAMP(1)
};

static const struct st_sensors st_press_sensors[] = {
	{
		.wai = ST_PRESS_LPS331AP_WAI_EXP,
		.sensors_supported = {
			[0] = LPS331AP_PRESS_DEV_NAME,
		},
		.ch = (struct iio_chan_spec *)st_press_lsp331ap_channels,
		.num_ch = ARRAY_SIZE(st_press_lsp331ap_channels),
		.odr = {
			.addr = ST_PRESS_1_ODR_ADDR,
			.mask = ST_PRESS_LPS331AP_ODR_MASK,
			.odr_avl = {
				{ 1, ST_PRESS_1_ODR_AVL_1HZ_VAL, },
				{ 7, ST_PRESS_LPS331AP_ODR_AVL_7HZ_VAL, },
				{ 13, ST_PRESS_LPS331AP_ODR_AVL_13HZ_VAL, },
				{ 25, ST_PRESS_LPS331AP_ODR_AVL_25HZ_VAL, },
			},
		},
		.pw = {
			.addr = ST_PRESS_1_PW_ADDR,
			.mask = ST_PRESS_LPS331AP_PW_MASK,
			.value_on = ST_SENSORS_DEFAULT_POWER_ON_VALUE,
			.value_off = ST_SENSORS_DEFAULT_POWER_OFF_VALUE,
		},
		.fs = {
			.addr = ST_PRESS_LPS331AP_FS_ADDR,
			.mask = ST_PRESS_LPS331AP_FS_MASK,
			.fs_avl = {
				[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,
				},
			},
		},
		.bdu = {
			.addr = ST_PRESS_1_BDU_ADDR,
			.mask = ST_PRESS_LPS331AP_BDU_MASK,
		},
		.drdy_irq = {
			.addr = ST_PRESS_LPS331AP_DRDY_IRQ_ADDR,
			.mask = ST_PRESS_LPS331AP_DRDY_IRQ_MASK,
		},
		.multi_read_bit = ST_PRESS_1_MULTIREAD_BIT,
		.bootime = 2,
	},
	{
		.wai = ST_PRESS_LPS001WP_WAI_EXP,
		.sensors_supported = {
			[0] = LPS001WP_PRESS_DEV_NAME,
		},
		.ch = (struct iio_chan_spec *)st_press_lps001wp_channels,
		.num_ch = ARRAY_SIZE(st_press_lps001wp_channels),
		.odr = {
			.addr = ST_PRESS_1_ODR_ADDR,
			.mask = ST_PRESS_LPS001WP_ODR_MASK,
			.odr_avl = {
				{ 1, ST_PRESS_1_ODR_AVL_1HZ_VAL, },
				{ 7, ST_PRESS_LPS001WP_ODR_AVL_7HZ_VAL, },
				{ 13, ST_PRESS_LPS001WP_ODR_AVL_13HZ_VAL, },
			},
		},
		.pw = {
			.addr = ST_PRESS_1_PW_ADDR,
			.mask = ST_PRESS_LPS001WP_PW_MASK,
			.value_on = ST_SENSORS_DEFAULT_POWER_ON_VALUE,
			.value_off = ST_SENSORS_DEFAULT_POWER_OFF_VALUE,
		},
		.fs = {
			.addr = 0,
		},
		.bdu = {
			.addr = ST_PRESS_1_BDU_ADDR,
			.mask = ST_PRESS_LPS001WP_BDU_MASK,
		},
		.drdy_irq = {
			.addr = 0,
		},
		.multi_read_bit = ST_PRESS_1_MULTIREAD_BIT,
		.bootime = 2,
	},
};


> > -/* CUSTOM VALUES FOR SENSOR 1 */
> > -#define ST_PRESS_1_WAI_EXP			0xbb
> > -#define ST_PRESS_1_ODR_ADDR			0x20
> > -#define ST_PRESS_1_ODR_MASK			0x70
> > -#define ST_PRESS_1_ODR_AVL_1HZ_VAL		0x01
> > -#define ST_PRESS_1_ODR_AVL_7HZ_VAL		0x05
> > -#define ST_PRESS_1_ODR_AVL_13HZ_VAL		0x06
> > -#define ST_PRESS_1_ODR_AVL_25HZ_VAL		0x07
> > -#define ST_PRESS_1_PW_ADDR			0x20
> > -#define ST_PRESS_1_PW_MASK			0x80
> > -#define ST_PRESS_1_FS_ADDR			0x23
> > -#define ST_PRESS_1_FS_MASK			0x30
> > -#define ST_PRESS_1_FS_AVL_1260_VAL		0x00
> > -#define ST_PRESS_1_FS_AVL_1260_GAIN		ST_PRESS_MBAR_TO_KPASCAL(244141)
> > -#define ST_PRESS_1_FS_AVL_TEMP_GAIN		2083000
> > -#define ST_PRESS_1_BDU_ADDR			0x20
> > -#define ST_PRESS_1_BDU_MASK			0x04
> > -#define ST_PRESS_1_DRDY_IRQ_ADDR		0x22
> > -#define ST_PRESS_1_DRDY_IRQ_MASK		0x04
> > -#define ST_PRESS_1_MULTIREAD_BIT		true
> > -#define ST_PRESS_1_TEMP_OFFSET			42500
> > +/* CUSTOM VALUES FOR LPS331AP SENSOR */
> > +#define ST_PRESS_LPS331AP_WAI_EXP		0xbb
> > +#define ST_PRESS_LPS331AP_ODR_ADDR		0x20
> > +#define ST_PRESS_LPS331AP_ODR_MASK		0x70
> > +#define ST_PRESS_LPS331AP_ODR_AVL_1HZ_VAL	0x01
> > +#define ST_PRESS_LPS331AP_ODR_AVL_7HZ_VAL	0x05
> > +#define ST_PRESS_LPS331AP_ODR_AVL_13HZ_VAL	0x06
> > +#define ST_PRESS_LPS331AP_ODR_AVL_25HZ_VAL	0x07
> > +#define ST_PRESS_LPS331AP_PW_ADDR		0x20
> > +#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_MBAR_TO_KPASCAL(244141)
> > +#define ST_PRESS_LPS331AP_FS_AVL_TEMP_GAIN	2083000
> > +#define ST_PRESS_LPS331AP_BDU_ADDR		0x20
> > +#define ST_PRESS_LPS331AP_BDU_MASK		0x04
> > +#define ST_PRESS_LPS331AP_DRDY_IRQ_ADDR		0x22
> > +#define ST_PRESS_LPS331AP_DRDY_IRQ_MASK		0x04
> > +#define ST_PRESS_LPS331AP_MULTIREAD_BIT		true
> > +#define ST_PRESS_LPS331AP_TEMP_OFFSET		42500
> > +#define ST_PRESS_LPS331AP_OUT_XL_ADDR		0x28
> > +#define ST_TEMP_LPS331AP_OUT_L_ADDR		0x2b
> >   
> >   static const struct iio_chan_spec st_press_channels[] = {
> >   	ST_SENSORS_LSM_CHANNELS(IIO_PRESSURE,
> >   			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> >   			ST_SENSORS_SCAN_X, 0, IIO_NO_MOD, 'u', IIO_LE, 24, 24,
> > -			ST_PRESS_DEFAULT_OUT_XL_ADDR),
> > +			ST_PRESS_LPS331AP_OUT_XL_ADDR),
> >   	ST_SENSORS_LSM_CHANNELS(IIO_TEMP,
> >   			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE) |
> >   						BIT(IIO_CHAN_INFO_OFFSET),
> >   			-1, 0, IIO_NO_MOD, 's', IIO_LE, 16, 16,
> > -			ST_TEMP_DEFAULT_OUT_L_ADDR),
> > +			ST_TEMP_LPS331AP_OUT_L_ADDR),
> >   	IIO_CHAN_SOFT_TIMESTAMP(1)
> >   };
> >   
> >   static const struct st_sensors st_press_sensors[] = {
> >   	{
> > -		.wai = ST_PRESS_1_WAI_EXP,
> > +		.wai = ST_PRESS_LPS331AP_WAI_EXP,
> >   		.sensors_supported = {
> >   			[0] = LPS331AP_PRESS_DEV_NAME,
> >   		},
> >   		.ch = (struct iio_chan_spec *)st_press_channels,
> >   		.odr = {
> > -			.addr = ST_PRESS_1_ODR_ADDR,
> > -			.mask = ST_PRESS_1_ODR_MASK,
> > +			.addr = ST_PRESS_LPS331AP_ODR_ADDR,
> > +			.mask = ST_PRESS_LPS331AP_ODR_MASK,
> >   			.odr_avl = {
> > -				{ 1, ST_PRESS_1_ODR_AVL_1HZ_VAL, },
> > -				{ 7, ST_PRESS_1_ODR_AVL_7HZ_VAL, },
> > -				{ 13, ST_PRESS_1_ODR_AVL_13HZ_VAL, },
> > -				{ 25, ST_PRESS_1_ODR_AVL_25HZ_VAL, },
> > +				{ 1, ST_PRESS_LPS331AP_ODR_AVL_1HZ_VAL, },
> > +				{ 7, ST_PRESS_LPS331AP_ODR_AVL_7HZ_VAL, },
> > +				{ 13, ST_PRESS_LPS331AP_ODR_AVL_13HZ_VAL, },
> > +				{ 25, ST_PRESS_LPS331AP_ODR_AVL_25HZ_VAL, },
> >   			},
> >   		},
> >   		.pw = {
> > -			.addr = ST_PRESS_1_PW_ADDR,
> > -			.mask = ST_PRESS_1_PW_MASK,
> > +			.addr = ST_PRESS_LPS331AP_PW_ADDR,
> > +			.mask = ST_PRESS_LPS331AP_PW_MASK,
> >   			.value_on = ST_SENSORS_DEFAULT_POWER_ON_VALUE,
> >   			.value_off = ST_SENSORS_DEFAULT_POWER_OFF_VALUE,
> >   		},
> >   		.fs = {
> > -			.addr = ST_PRESS_1_FS_ADDR,
> > -			.mask = ST_PRESS_1_FS_MASK,
> > +			.addr = ST_PRESS_LPS331AP_FS_ADDR,
> > +			.mask = ST_PRESS_LPS331AP_FS_MASK,
> >   			.fs_avl = {
> >   				[0] = {
> >   					.num = ST_PRESS_FS_AVL_1260MB,
> > -					.value = ST_PRESS_1_FS_AVL_1260_VAL,
> > -					.gain = ST_PRESS_1_FS_AVL_1260_GAIN,
> > -					.gain2 = ST_PRESS_1_FS_AVL_TEMP_GAIN,
> > +					.value = ST_PRESS_LPS331AP_FS_AVL_1260_VAL,
> > +					.gain = ST_PRESS_LPS331AP_FS_AVL_1260_GAIN,
> > +					.gain2 = ST_PRESS_LPS331AP_FS_AVL_TEMP_GAIN,
> >   				},
> >   			},
> >   		},
> >   		.bdu = {
> > -			.addr = ST_PRESS_1_BDU_ADDR,
> > -			.mask = ST_PRESS_1_BDU_MASK,
> > +			.addr = ST_PRESS_LPS331AP_BDU_ADDR,
> > +			.mask = ST_PRESS_LPS331AP_BDU_MASK,
> >   		},
> >   		.drdy_irq = {
> > -			.addr = ST_PRESS_1_DRDY_IRQ_ADDR,
> > -			.mask = ST_PRESS_1_DRDY_IRQ_MASK,
> > +			.addr = ST_PRESS_LPS331AP_DRDY_IRQ_ADDR,
> > +			.mask = ST_PRESS_LPS331AP_DRDY_IRQ_MASK,
> >   		},
> > -		.multi_read_bit = ST_PRESS_1_MULTIREAD_BIT,
> > +		.multi_read_bit = ST_PRESS_LPS331AP_MULTIREAD_BIT,
> >   		.bootime = 2,
> >   	},
> >   };

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 06/11] iio: pressure-core: st: Expand and rename LPS331AP's channel descriptor

[Lee Jones]

Due to the MACRO used, the task of reading, understanding and maintaining
the LPS331AP's channel descriptor is substantially difficult. This patch
is based on the view that it's better to have easy to read, maintainable
code than to save a few lines here and there. For that reason we're
expanding the array so initialisation is completed in full.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/iio/pressure/st_pressure_core.c | 45 +++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
index becfb25..7ba9299 100644
--- a/drivers/iio/pressure/st_pressure_core.c
+++ b/drivers/iio/pressure/st_pressure_core.c
@@ -58,16 +58,39 @@
 #define ST_PRESS_LPS331AP_OUT_XL_ADDR		0x28
 #define ST_TEMP_LPS331AP_OUT_L_ADDR		0x2b
 
-static const struct iio_chan_spec st_press_channels[] = {
-	ST_SENSORS_LSM_CHANNELS(IIO_PRESSURE,
+static const struct iio_chan_spec st_press_lsp331ap_channels[] = {
+	{
+		.type = IIO_PRESSURE,
+		.channel2 = IIO_NO_MOD,
+		.address = ST_PRESS_LPS331AP_OUT_XL_ADDR,
+		.scan_index = ST_SENSORS_SCAN_X,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = 24,
+			.storagebits = 24,
+			.endianness = IIO_LE,
+		},
+		.info_mask_separate =
 			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
-			ST_SENSORS_SCAN_X, 0, IIO_NO_MOD, 'u', IIO_LE, 24, 24,
-			ST_PRESS_LPS331AP_OUT_XL_ADDR),
-	ST_SENSORS_LSM_CHANNELS(IIO_TEMP,
-			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE) |
-						BIT(IIO_CHAN_INFO_OFFSET),
-			-1, 0, IIO_NO_MOD, 's', IIO_LE, 16, 16,
-			ST_TEMP_LPS331AP_OUT_L_ADDR),
+		.modified = 0,
+	},
+	{
+		.type = IIO_TEMP,
+		.channel2 = IIO_NO_MOD,
+		.address = ST_TEMP_LPS331AP_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) |
+			BIT(IIO_CHAN_INFO_OFFSET),
+		.modified = 0,
+	},
 	IIO_CHAN_SOFT_TIMESTAMP(1)
 };
 
@@ -77,7 +100,7 @@ static const struct st_sensors st_press_sensors[] = {
 		.sensors_supported = {
 			[0] = LPS331AP_PRESS_DEV_NAME,
 		},
-		.ch = (struct iio_chan_spec *)st_press_channels,
+		.ch = (struct iio_chan_spec *)st_press_lsp331ap_channels,
 		.odr = {
 			.addr = ST_PRESS_LPS331AP_ODR_ADDR,
 			.mask = ST_PRESS_LPS331AP_ODR_MASK,
@@ -214,7 +237,7 @@ int st_press_common_probe(struct iio_dev *indio_dev)
 	pdata->num_data_channels = ST_PRESS_NUMBER_DATA_CHANNELS;
 	pdata->multiread_bit = pdata->sensor->multi_read_bit;
 	indio_dev->channels = pdata->sensor->ch;
-	indio_dev->num_channels = ARRAY_SIZE(st_press_channels);
+	indio_dev->num_channels = ARRAY_SIZE(st_press_lsp331ap_channels);
 
 	pdata->current_fullscale = (struct st_sensor_fullscale_avl *)
 						&pdata->sensor->fs.fs_avl[0];
-- 
1.8.1.2

Re: [PATCH 06/11] iio: pressure-core: st: Expand and rename LPS331AP's channel descriptor

[Denis CIOCCA]

> Due to the MACRO used, the task of reading, understanding and maintaining
> the LPS331AP's channel descriptor is substantially difficult. This patch
> is based on the view that it's better to have easy to read, maintainable
> code than to save a few lines here and there. For that reason we're
> expanding the array so initialisation is completed in full.
Also for this one, the channel names are general and can be shared=20
between different sensors. For the channel definition it's not a problem=20
for me, but I think it's not necessary adds all that code...

Denis

> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>   drivers/iio/pressure/st_pressure_core.c | 45 +++++++++++++++++++++++++-=
-------
>   1 file changed, 34 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressu=
re/st_pressure_core.c
> index becfb25..7ba9299 100644
> --- a/drivers/iio/pressure/st_pressure_core.c
> +++ b/drivers/iio/pressure/st_pressure_core.c
> @@ -58,16 +58,39 @@
>   #define ST_PRESS_LPS331AP_OUT_XL_ADDR		0x28
>   #define ST_TEMP_LPS331AP_OUT_L_ADDR		0x2b
>  =20
> -static const struct iio_chan_spec st_press_channels[] =3D {
> -	ST_SENSORS_LSM_CHANNELS(IIO_PRESSURE,
> +static const struct iio_chan_spec st_press_lsp331ap_channels[] =3D {
> +	{
> +		.type =3D IIO_PRESSURE,
> +		.channel2 =3D IIO_NO_MOD,
> +		.address =3D ST_PRESS_LPS331AP_OUT_XL_ADDR,
> +		.scan_index =3D ST_SENSORS_SCAN_X,
> +		.scan_type =3D {
> +			.sign =3D 'u',
> +			.realbits =3D 24,
> +			.storagebits =3D 24,
> +			.endianness =3D IIO_LE,
> +		},
> +		.info_mask_separate =3D
>   			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> -			ST_SENSORS_SCAN_X, 0, IIO_NO_MOD, 'u', IIO_LE, 24, 24,
> -			ST_PRESS_LPS331AP_OUT_XL_ADDR),
> -	ST_SENSORS_LSM_CHANNELS(IIO_TEMP,
> -			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE) |
> -						BIT(IIO_CHAN_INFO_OFFSET),
> -			-1, 0, IIO_NO_MOD, 's', IIO_LE, 16, 16,
> -			ST_TEMP_LPS331AP_OUT_L_ADDR),
> +		.modified =3D 0,
> +	},
> +	{
> +		.type =3D IIO_TEMP,
> +		.channel2 =3D IIO_NO_MOD,
> +		.address =3D ST_TEMP_LPS331AP_OUT_L_ADDR,
> +		.scan_index =3D -1,
> +		.scan_type =3D {
> +			.sign =3D 'u',
> +			.realbits =3D 16,
> +			.storagebits =3D 16,
> +			.endianness =3D IIO_LE,
> +		},
> +		.info_mask_separate =3D
> +			BIT(IIO_CHAN_INFO_RAW) |
> +			BIT(IIO_CHAN_INFO_SCALE) |
> +			BIT(IIO_CHAN_INFO_OFFSET),
> +		.modified =3D 0,
> +	},
>   	IIO_CHAN_SOFT_TIMESTAMP(1)
>   };
>  =20
> @@ -77,7 +100,7 @@ static const struct st_sensors st_press_sensors[] =3D =
{
>   		.sensors_supported =3D {
>   			[0] =3D LPS331AP_PRESS_DEV_NAME,
>   		},
> -		.ch =3D (struct iio_chan_spec *)st_press_channels,
> +		.ch =3D (struct iio_chan_spec *)st_press_lsp331ap_channels,
>   		.odr =3D {
>   			.addr =3D ST_PRESS_LPS331AP_ODR_ADDR,
>   			.mask =3D ST_PRESS_LPS331AP_ODR_MASK,
> @@ -214,7 +237,7 @@ int st_press_common_probe(struct iio_dev *indio_dev)
>   	pdata->num_data_channels =3D ST_PRESS_NUMBER_DATA_CHANNELS;
>   	pdata->multiread_bit =3D pdata->sensor->multi_read_bit;
>   	indio_dev->channels =3D pdata->sensor->ch;
> -	indio_dev->num_channels =3D ARRAY_SIZE(st_press_channels);
> +	indio_dev->num_channels =3D ARRAY_SIZE(st_press_lsp331ap_channels);
>  =20
>   	pdata->current_fullscale =3D (struct st_sensor_fullscale_avl *)
>   						&pdata->sensor->fs.fs_avl[0];

Re: [PATCH 06/11] iio: pressure-core: st: Expand and rename LPS331AP's channel descriptor

[Lee Jones]

On Wed, 04 Sep 2013, Denis CIOCCA wrote:

> > Due to the MACRO used, the task of reading, understanding and maintaining
> > the LPS331AP's channel descriptor is substantially difficult. This patch
> > is based on the view that it's better to have easy to read, maintainable
> > code than to save a few lines here and there. For that reason we're
> > expanding the array so initialisation is completed in full.
> Also for this one, the channel names are general and can be shared 
> between different sensors. For the channel definition it's not a problem 
> for me, but I think it's not necessary adds all that code...

I'm not sure what you mean by this. Would you be kind enough to
explain it in a different way please?

> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >   drivers/iio/pressure/st_pressure_core.c | 45 +++++++++++++++++++++++++--------
> >   1 file changed, 34 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
> > index becfb25..7ba9299 100644
> > --- a/drivers/iio/pressure/st_pressure_core.c
> > +++ b/drivers/iio/pressure/st_pressure_core.c
> > @@ -58,16 +58,39 @@
> >   #define ST_PRESS_LPS331AP_OUT_XL_ADDR		0x28
> >   #define ST_TEMP_LPS331AP_OUT_L_ADDR		0x2b
> >   
> > -static const struct iio_chan_spec st_press_channels[] = {
> > -	ST_SENSORS_LSM_CHANNELS(IIO_PRESSURE,
> > +static const struct iio_chan_spec st_press_lsp331ap_channels[] = {
> > +	{
> > +		.type = IIO_PRESSURE,
> > +		.channel2 = IIO_NO_MOD,
> > +		.address = ST_PRESS_LPS331AP_OUT_XL_ADDR,
> > +		.scan_index = ST_SENSORS_SCAN_X,
> > +		.scan_type = {
> > +			.sign = 'u',
> > +			.realbits = 24,
> > +			.storagebits = 24,
> > +			.endianness = IIO_LE,
> > +		},
> > +		.info_mask_separate =
> >   			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> > -			ST_SENSORS_SCAN_X, 0, IIO_NO_MOD, 'u', IIO_LE, 24, 24,
> > -			ST_PRESS_LPS331AP_OUT_XL_ADDR),
> > -	ST_SENSORS_LSM_CHANNELS(IIO_TEMP,
> > -			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE) |
> > -						BIT(IIO_CHAN_INFO_OFFSET),
> > -			-1, 0, IIO_NO_MOD, 's', IIO_LE, 16, 16,
> > -			ST_TEMP_LPS331AP_OUT_L_ADDR),
> > +		.modified = 0,
> > +	},
> > +	{
> > +		.type = IIO_TEMP,
> > +		.channel2 = IIO_NO_MOD,
> > +		.address = ST_TEMP_LPS331AP_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) |
> > +			BIT(IIO_CHAN_INFO_OFFSET),
> > +		.modified = 0,
> > +	},
> >   	IIO_CHAN_SOFT_TIMESTAMP(1)
> >   };
> >   
> > @@ -77,7 +100,7 @@ static const struct st_sensors st_press_sensors[] = {
> >   		.sensors_supported = {
> >   			[0] = LPS331AP_PRESS_DEV_NAME,
> >   		},
> > -		.ch = (struct iio_chan_spec *)st_press_channels,
> > +		.ch = (struct iio_chan_spec *)st_press_lsp331ap_channels,
> >   		.odr = {
> >   			.addr = ST_PRESS_LPS331AP_ODR_ADDR,
> >   			.mask = ST_PRESS_LPS331AP_ODR_MASK,
> > @@ -214,7 +237,7 @@ int st_press_common_probe(struct iio_dev *indio_dev)
> >   	pdata->num_data_channels = ST_PRESS_NUMBER_DATA_CHANNELS;
> >   	pdata->multiread_bit = pdata->sensor->multi_read_bit;
> >   	indio_dev->channels = pdata->sensor->ch;
> > -	indio_dev->num_channels = ARRAY_SIZE(st_press_channels);
> > +	indio_dev->num_channels = ARRAY_SIZE(st_press_lsp331ap_channels);
> >   
> >   	pdata->current_fullscale = (struct st_sensor_fullscale_avl *)
> >   						&pdata->sensor->fs.fs_avl[0];

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 06/11] iio: pressure-core: st: Expand and rename LPS331AP's channel descriptor

[Denis CIOCCA]

DQo+Pj4gRHVlIHRvIHRoZSBNQUNSTyB1c2VkLCB0aGUgdGFzayBvZiByZWFkaW5nLCB1bmRlcnN0
YW5kaW5nIGFuZCBtYWludGFpbmluZw0KPj4+IHRoZSBMUFMzMzFBUCdzIGNoYW5uZWwgZGVzY3Jp
cHRvciBpcyBzdWJzdGFudGlhbGx5IGRpZmZpY3VsdC4gVGhpcyBwYXRjaA0KPj4+IGlzIGJhc2Vk
IG9uIHRoZSB2aWV3IHRoYXQgaXQncyBiZXR0ZXIgdG8gaGF2ZSBlYXN5IHRvIHJlYWQsIG1haW50
YWluYWJsZQ0KPj4+IGNvZGUgdGhhbiB0byBzYXZlIGEgZmV3IGxpbmVzIGhlcmUgYW5kIHRoZXJl
LiBGb3IgdGhhdCByZWFzb24gd2UncmUNCj4+PiBleHBhbmRpbmcgdGhlIGFycmF5IHNvIGluaXRp
YWxpc2F0aW9uIGlzIGNvbXBsZXRlZCBpbiBmdWxsLg0KPj4gQWxzbyBmb3IgdGhpcyBvbmUsIHRo
ZSBjaGFubmVsIG5hbWVzIGFyZSBnZW5lcmFsIGFuZCBjYW4gYmUgc2hhcmVkDQo+PiBiZXR3ZWVu
IGRpZmZlcmVudCBzZW5zb3JzLiBGb3IgdGhlIGNoYW5uZWwgZGVmaW5pdGlvbiBpdCdzIG5vdCBh
IHByb2JsZW0NCj4+IGZvciBtZSwgYnV0IEkgdGhpbmsgaXQncyBub3QgbmVjZXNzYXJ5IGFkZHMg
YWxsIHRoYXQgY29kZS4uLg0KPiBJJ20gbm90IHN1cmUgd2hhdCB5b3UgbWVhbiBieSB0aGlzLiBX
b3VsZCB5b3UgYmUga2luZCBlbm91Z2ggdG8NCj4gZXhwbGFpbiBpdCBpbiBhIGRpZmZlcmVudCB3
YXkgcGxlYXNlPw0KVGhlIGNoYW5uZWwgbmFtZSAoaW4gdGhpcyBjYXNlIHN0X3ByZXNzX2NoYW5u
ZWxzKSBpcyBub3Qgb25seSBzcGVjaWZpYyANCmZvciBvbmUgc2Vuc29yIGJ1dCBjYW4gYmUgc2hh
cmVkLiBPayBpbiB0aGlzIGRyaXZlciBub3cgaXMgdXNlZCBvbmx5IGZvciANCnRoZSBscHMzMzFh
cCBidXQgZm9yIGV4YW1wbGUgaW4gYWNjZWxlcm9tZXRlciBkcml2ZXIgaXMgdXNlZCBieSBzZXZl
cmFsIA0Kc2Vuc29ycy4gSXQncyBwb3NzaWJsZSBpbiB0aGUgZnV0dXJlIGZvciBuZXcgcHJlc3N1
cmUgc2Vuc29ycyB1c2UgdGhlIA0Kc2FtZSBjaGFubmVscyBkZWZpbml0aW9uLg0KVGhlIGNoYW5u
ZWwgZGVmaW5pdGlvbiBpcyBpbnRlbmRlZCB0aGUgc3dpdGNoIGJ5IG1hY3JvIA0KU1RfU0VOU09S
U19MU01fQ0hBTk5FTFMgdG8gdGhlIGZ1bGwgZGVmaW5pdGlvbiwgZm9yIG1lIGlzIG5vdCBhIHBy
b2JsZW0gDQpidXQgSSB0aGluayBpdCdzIG5vdCBuZWNlc3NhcnkuDQoNCkRlbmlzDQoNCj4+PiBT
aWduZWQtb2ZmLWJ5OiBMZWUgSm9uZXMgPGxlZS5qb25lc0BsaW5hcm8ub3JnPg0KPj4+IC0tLQ0K
Pj4+ICAgIGRyaXZlcnMvaWlvL3ByZXNzdXJlL3N0X3ByZXNzdXJlX2NvcmUuYyB8IDQ1ICsrKysr
KysrKysrKysrKysrKysrKysrKystLS0tLS0tLQ0KPj4+ICAgIDEgZmlsZSBjaGFuZ2VkLCAzNCBp
bnNlcnRpb25zKCspLCAxMSBkZWxldGlvbnMoLSkNCj4+Pg0KPj4+IGRpZmYgLS1naXQgYS9kcml2
ZXJzL2lpby9wcmVzc3VyZS9zdF9wcmVzc3VyZV9jb3JlLmMgYi9kcml2ZXJzL2lpby9wcmVzc3Vy
ZS9zdF9wcmVzc3VyZV9jb3JlLmMNCj4+PiBpbmRleCBiZWNmYjI1Li43YmE5Mjk5IDEwMDY0NA0K
Pj4+IC0tLSBhL2RyaXZlcnMvaWlvL3ByZXNzdXJlL3N0X3ByZXNzdXJlX2NvcmUuYw0KPj4+ICsr
KyBiL2RyaXZlcnMvaWlvL3ByZXNzdXJlL3N0X3ByZXNzdXJlX2NvcmUuYw0KPj4+IEBAIC01OCwx
NiArNTgsMzkgQEANCj4+PiAgICAjZGVmaW5lIFNUX1BSRVNTX0xQUzMzMUFQX09VVF9YTF9BRERS
CQkweDI4DQo+Pj4gICAgI2RlZmluZSBTVF9URU1QX0xQUzMzMUFQX09VVF9MX0FERFIJCTB4MmIN
Cj4+PiAgICANCj4+PiAtc3RhdGljIGNvbnN0IHN0cnVjdCBpaW9fY2hhbl9zcGVjIHN0X3ByZXNz
X2NoYW5uZWxzW10gPSB7DQo+Pj4gLQlTVF9TRU5TT1JTX0xTTV9DSEFOTkVMUyhJSU9fUFJFU1NV
UkUsDQo+Pj4gK3N0YXRpYyBjb25zdCBzdHJ1Y3QgaWlvX2NoYW5fc3BlYyBzdF9wcmVzc19sc3Az
MzFhcF9jaGFubmVsc1tdID0gew0KPj4+ICsJew0KPj4+ICsJCS50eXBlID0gSUlPX1BSRVNTVVJF
LA0KPj4+ICsJCS5jaGFubmVsMiA9IElJT19OT19NT0QsDQo+Pj4gKwkJLmFkZHJlc3MgPSBTVF9Q
UkVTU19MUFMzMzFBUF9PVVRfWExfQUREUiwNCj4+PiArCQkuc2Nhbl9pbmRleCA9IFNUX1NFTlNP
UlNfU0NBTl9YLA0KPj4+ICsJCS5zY2FuX3R5cGUgPSB7DQo+Pj4gKwkJCS5zaWduID0gJ3UnLA0K
Pj4+ICsJCQkucmVhbGJpdHMgPSAyNCwNCj4+PiArCQkJLnN0b3JhZ2ViaXRzID0gMjQsDQo+Pj4g
KwkJCS5lbmRpYW5uZXNzID0gSUlPX0xFLA0KPj4+ICsJCX0sDQo+Pj4gKwkJLmluZm9fbWFza19z
ZXBhcmF0ZSA9DQo+Pj4gICAgCQkJQklUKElJT19DSEFOX0lORk9fUkFXKSB8IEJJVChJSU9fQ0hB
Tl9JTkZPX1NDQUxFKSwNCj4+PiAtCQkJU1RfU0VOU09SU19TQ0FOX1gsIDAsIElJT19OT19NT0Qs
ICd1JywgSUlPX0xFLCAyNCwgMjQsDQo+Pj4gLQkJCVNUX1BSRVNTX0xQUzMzMUFQX09VVF9YTF9B
RERSKSwNCj4+PiAtCVNUX1NFTlNPUlNfTFNNX0NIQU5ORUxTKElJT19URU1QLA0KPj4+IC0JCQlC
SVQoSUlPX0NIQU5fSU5GT19SQVcpIHwgQklUKElJT19DSEFOX0lORk9fU0NBTEUpIHwNCj4+PiAt
CQkJCQkJQklUKElJT19DSEFOX0lORk9fT0ZGU0VUKSwNCj4+PiAtCQkJLTEsIDAsIElJT19OT19N
T0QsICdzJywgSUlPX0xFLCAxNiwgMTYsDQo+Pj4gLQkJCVNUX1RFTVBfTFBTMzMxQVBfT1VUX0xf
QUREUiksDQo+Pj4gKwkJLm1vZGlmaWVkID0gMCwNCj4+PiArCX0sDQo+Pj4gKwl7DQo+Pj4gKwkJ
LnR5cGUgPSBJSU9fVEVNUCwNCj4+PiArCQkuY2hhbm5lbDIgPSBJSU9fTk9fTU9ELA0KPj4+ICsJ
CS5hZGRyZXNzID0gU1RfVEVNUF9MUFMzMzFBUF9PVVRfTF9BRERSLA0KPj4+ICsJCS5zY2FuX2lu
ZGV4ID0gLTEsDQo+Pj4gKwkJLnNjYW5fdHlwZSA9IHsNCj4+PiArCQkJLnNpZ24gPSAndScsDQo+
Pj4gKwkJCS5yZWFsYml0cyA9IDE2LA0KPj4+ICsJCQkuc3RvcmFnZWJpdHMgPSAxNiwNCj4+PiAr
CQkJLmVuZGlhbm5lc3MgPSBJSU9fTEUsDQo+Pj4gKwkJfSwNCj4+PiArCQkuaW5mb19tYXNrX3Nl
cGFyYXRlID0NCj4+PiArCQkJQklUKElJT19DSEFOX0lORk9fUkFXKSB8DQo+Pj4gKwkJCUJJVChJ
SU9fQ0hBTl9JTkZPX1NDQUxFKSB8DQo+Pj4gKwkJCUJJVChJSU9fQ0hBTl9JTkZPX09GRlNFVCks
DQo+Pj4gKwkJLm1vZGlmaWVkID0gMCwNCj4+PiArCX0sDQo+Pj4gICAgCUlJT19DSEFOX1NPRlRf
VElNRVNUQU1QKDEpDQo+Pj4gICAgfTsNCj4+PiAgICANCj4+PiBAQCAtNzcsNyArMTAwLDcgQEAg
c3RhdGljIGNvbnN0IHN0cnVjdCBzdF9zZW5zb3JzIHN0X3ByZXNzX3NlbnNvcnNbXSA9IHsNCj4+
PiAgICAJCS5zZW5zb3JzX3N1cHBvcnRlZCA9IHsNCj4+PiAgICAJCQlbMF0gPSBMUFMzMzFBUF9Q
UkVTU19ERVZfTkFNRSwNCj4+PiAgICAJCX0sDQo+Pj4gLQkJLmNoID0gKHN0cnVjdCBpaW9fY2hh
bl9zcGVjICopc3RfcHJlc3NfY2hhbm5lbHMsDQo+Pj4gKwkJLmNoID0gKHN0cnVjdCBpaW9fY2hh
bl9zcGVjICopc3RfcHJlc3NfbHNwMzMxYXBfY2hhbm5lbHMsDQo+Pj4gICAgCQkub2RyID0gew0K
Pj4+ICAgIAkJCS5hZGRyID0gU1RfUFJFU1NfTFBTMzMxQVBfT0RSX0FERFIsDQo+Pj4gICAgCQkJ
Lm1hc2sgPSBTVF9QUkVTU19MUFMzMzFBUF9PRFJfTUFTSywNCj4+PiBAQCAtMjE0LDcgKzIzNyw3
IEBAIGludCBzdF9wcmVzc19jb21tb25fcHJvYmUoc3RydWN0IGlpb19kZXYgKmluZGlvX2RldikN
Cj4+PiAgICAJcGRhdGEtPm51bV9kYXRhX2NoYW5uZWxzID0gU1RfUFJFU1NfTlVNQkVSX0RBVEFf
Q0hBTk5FTFM7DQo+Pj4gICAgCXBkYXRhLT5tdWx0aXJlYWRfYml0ID0gcGRhdGEtPnNlbnNvci0+
bXVsdGlfcmVhZF9iaXQ7DQo+Pj4gICAgCWluZGlvX2Rldi0+Y2hhbm5lbHMgPSBwZGF0YS0+c2Vu
c29yLT5jaDsNCj4+PiAtCWluZGlvX2Rldi0+bnVtX2NoYW5uZWxzID0gQVJSQVlfU0laRShzdF9w
cmVzc19jaGFubmVscyk7DQo+Pj4gKwlpbmRpb19kZXYtPm51bV9jaGFubmVscyA9IEFSUkFZX1NJ
WkUoc3RfcHJlc3NfbHNwMzMxYXBfY2hhbm5lbHMpOw0KPj4+ICAgIA0KPj4+ICAgIAlwZGF0YS0+
Y3VycmVudF9mdWxsc2NhbGUgPSAoc3RydWN0IHN0X3NlbnNvcl9mdWxsc2NhbGVfYXZsICopDQo+
Pj4gICAgCQkJCQkJJnBkYXRhLT5zZW5zb3ItPmZzLmZzX2F2bFswXTsNCg==

Re: [PATCH 06/11] iio: pressure-core: st: Expand and rename LPS331AP's channel descriptor

[Lee Jones]

On Thu, 05 Sep 2013, Denis CIOCCA wrote:

> 
> >>> Due to the MACRO used, the task of reading, understanding and maintaining
> >>> the LPS331AP's channel descriptor is substantially difficult. This patch
> >>> is based on the view that it's better to have easy to read, maintainable
> >>> code than to save a few lines here and there. For that reason we're
> >>> expanding the array so initialisation is completed in full.
> >> Also for this one, the channel names are general and can be shared
> >> between different sensors. For the channel definition it's not a problem
> >> for me, but I think it's not necessary adds all that code...
> > I'm not sure what you mean by this. Would you be kind enough to
> > explain it in a different way please?
> The channel name (in this case st_press_channels) is not only specific 
> for one sensor but can be shared. Ok in this driver now is used only for 
> the lps331ap but for example in accelerometer driver is used by several 
> sensors. It's possible in the future for new pressure sensors use the 
> same channels definition.

Ah yes I see what you mean. Well as you say, for the moment, as
they're separated, this naming convention seems the most
appropriate. If we add anymore devices which share the definition, we
can pick the best naming convention for the situation I think. For
instance, I like that you've split the channels up into the number of
bits they support in the gyro and accel cases, so something of that
nature could be utilised if other device support is added.

> The channel definition is intended the switch by macro 
> ST_SENSORS_LSM_CHANNELS to the full definition, for me is not a problem 
> but I think it's not necessary.

If you are familiar with the macro I guess you could get used to
working with it, but coming from in as a first time reader, adding a
new device was pretty difficult. I had to look up the macro in the
header file, then have the original struct open too and cross
reference in 3 different places. It's made even more difficult by the
macro being in a different order to the original struct.

Now I've had time to work with it, I could probably work with it as
well. I was just thinking about helping out any new person that comes
along and tries to add support for a new sensor.

> >>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> >>> ---
> >>>    drivers/iio/pressure/st_pressure_core.c | 45 +++++++++++++++++++++++++--------
> >>>    1 file changed, 34 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
> >>> index becfb25..7ba9299 100644
> >>> --- a/drivers/iio/pressure/st_pressure_core.c
> >>> +++ b/drivers/iio/pressure/st_pressure_core.c
> >>> @@ -58,16 +58,39 @@
> >>>    #define ST_PRESS_LPS331AP_OUT_XL_ADDR		0x28
> >>>    #define ST_TEMP_LPS331AP_OUT_L_ADDR		0x2b
> >>>    
> >>> -static const struct iio_chan_spec st_press_channels[] = {
> >>> -	ST_SENSORS_LSM_CHANNELS(IIO_PRESSURE,
> >>> +static const struct iio_chan_spec st_press_lsp331ap_channels[] = {
> >>> +	{
> >>> +		.type = IIO_PRESSURE,
> >>> +		.channel2 = IIO_NO_MOD,
> >>> +		.address = ST_PRESS_LPS331AP_OUT_XL_ADDR,
> >>> +		.scan_index = ST_SENSORS_SCAN_X,
> >>> +		.scan_type = {
> >>> +			.sign = 'u',
> >>> +			.realbits = 24,
> >>> +			.storagebits = 24,
> >>> +			.endianness = IIO_LE,
> >>> +		},
> >>> +		.info_mask_separate =
> >>>    			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> >>> -			ST_SENSORS_SCAN_X, 0, IIO_NO_MOD, 'u', IIO_LE, 24, 24,
> >>> -			ST_PRESS_LPS331AP_OUT_XL_ADDR),
> >>> -	ST_SENSORS_LSM_CHANNELS(IIO_TEMP,
> >>> -			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE) |
> >>> -						BIT(IIO_CHAN_INFO_OFFSET),
> >>> -			-1, 0, IIO_NO_MOD, 's', IIO_LE, 16, 16,
> >>> -			ST_TEMP_LPS331AP_OUT_L_ADDR),
> >>> +		.modified = 0,
> >>> +	},
> >>> +	{
> >>> +		.type = IIO_TEMP,
> >>> +		.channel2 = IIO_NO_MOD,
> >>> +		.address = ST_TEMP_LPS331AP_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) |
> >>> +			BIT(IIO_CHAN_INFO_OFFSET),
> >>> +		.modified = 0,
> >>> +	},
> >>>    	IIO_CHAN_SOFT_TIMESTAMP(1)
> >>>    };
> >>>    
> >>> @@ -77,7 +100,7 @@ static const struct st_sensors st_press_sensors[] = {
> >>>    		.sensors_supported = {
> >>>    			[0] = LPS331AP_PRESS_DEV_NAME,
> >>>    		},
> >>> -		.ch = (struct iio_chan_spec *)st_press_channels,
> >>> +		.ch = (struct iio_chan_spec *)st_press_lsp331ap_channels,
> >>>    		.odr = {
> >>>    			.addr = ST_PRESS_LPS331AP_ODR_ADDR,
> >>>    			.mask = ST_PRESS_LPS331AP_ODR_MASK,
> >>> @@ -214,7 +237,7 @@ int st_press_common_probe(struct iio_dev *indio_dev)
> >>>    	pdata->num_data_channels = ST_PRESS_NUMBER_DATA_CHANNELS;
> >>>    	pdata->multiread_bit = pdata->sensor->multi_read_bit;
> >>>    	indio_dev->channels = pdata->sensor->ch;
> >>> -	indio_dev->num_channels = ARRAY_SIZE(st_press_channels);
> >>> +	indio_dev->num_channels = ARRAY_SIZE(st_press_lsp331ap_channels);
> >>>    
> >>>    	pdata->current_fullscale = (struct st_sensor_fullscale_avl *)
> >>>    						&pdata->sensor->fs.fs_avl[0];

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 06/11] iio: pressure-core: st: Expand and rename LPS331AP's channel descriptor

[Denis CIOCCA]

TGVlLCBJIGdvdCB5b3VyIHBvaW50LiBGb3IgbWUgaXMgb2suLi4NCg0KRGVuaXMNCj4gT24gVGh1
LCAwNSBTZXAgMjAxMywgRGVuaXMgQ0lPQ0NBIHdyb3RlOg0KPg0KPj4+Pj4gRHVlIHRvIHRoZSBN
QUNSTyB1c2VkLCB0aGUgdGFzayBvZiByZWFkaW5nLCB1bmRlcnN0YW5kaW5nIGFuZCBtYWludGFp
bmluZw0KPj4+Pj4gdGhlIExQUzMzMUFQJ3MgY2hhbm5lbCBkZXNjcmlwdG9yIGlzIHN1YnN0YW50
aWFsbHkgZGlmZmljdWx0LiBUaGlzIHBhdGNoDQo+Pj4+PiBpcyBiYXNlZCBvbiB0aGUgdmlldyB0
aGF0IGl0J3MgYmV0dGVyIHRvIGhhdmUgZWFzeSB0byByZWFkLCBtYWludGFpbmFibGUNCj4+Pj4+
IGNvZGUgdGhhbiB0byBzYXZlIGEgZmV3IGxpbmVzIGhlcmUgYW5kIHRoZXJlLiBGb3IgdGhhdCBy
ZWFzb24gd2UncmUNCj4+Pj4+IGV4cGFuZGluZyB0aGUgYXJyYXkgc28gaW5pdGlhbGlzYXRpb24g
aXMgY29tcGxldGVkIGluIGZ1bGwuDQo+Pj4+IEFsc28gZm9yIHRoaXMgb25lLCB0aGUgY2hhbm5l
bCBuYW1lcyBhcmUgZ2VuZXJhbCBhbmQgY2FuIGJlIHNoYXJlZA0KPj4+PiBiZXR3ZWVuIGRpZmZl
cmVudCBzZW5zb3JzLiBGb3IgdGhlIGNoYW5uZWwgZGVmaW5pdGlvbiBpdCdzIG5vdCBhIHByb2Js
ZW0NCj4+Pj4gZm9yIG1lLCBidXQgSSB0aGluayBpdCdzIG5vdCBuZWNlc3NhcnkgYWRkcyBhbGwg
dGhhdCBjb2RlLi4uDQo+Pj4gSSdtIG5vdCBzdXJlIHdoYXQgeW91IG1lYW4gYnkgdGhpcy4gV291
bGQgeW91IGJlIGtpbmQgZW5vdWdoIHRvDQo+Pj4gZXhwbGFpbiBpdCBpbiBhIGRpZmZlcmVudCB3
YXkgcGxlYXNlPw0KPj4gVGhlIGNoYW5uZWwgbmFtZSAoaW4gdGhpcyBjYXNlIHN0X3ByZXNzX2No
YW5uZWxzKSBpcyBub3Qgb25seSBzcGVjaWZpYw0KPj4gZm9yIG9uZSBzZW5zb3IgYnV0IGNhbiBi
ZSBzaGFyZWQuIE9rIGluIHRoaXMgZHJpdmVyIG5vdyBpcyB1c2VkIG9ubHkgZm9yDQo+PiB0aGUg
bHBzMzMxYXAgYnV0IGZvciBleGFtcGxlIGluIGFjY2VsZXJvbWV0ZXIgZHJpdmVyIGlzIHVzZWQg
Ynkgc2V2ZXJhbA0KPj4gc2Vuc29ycy4gSXQncyBwb3NzaWJsZSBpbiB0aGUgZnV0dXJlIGZvciBu
ZXcgcHJlc3N1cmUgc2Vuc29ycyB1c2UgdGhlDQo+PiBzYW1lIGNoYW5uZWxzIGRlZmluaXRpb24u
DQo+IEFoIHllcyBJIHNlZSB3aGF0IHlvdSBtZWFuLiBXZWxsIGFzIHlvdSBzYXksIGZvciB0aGUg
bW9tZW50LCBhcw0KPiB0aGV5J3JlIHNlcGFyYXRlZCwgdGhpcyBuYW1pbmcgY29udmVudGlvbiBz
ZWVtcyB0aGUgbW9zdA0KPiBhcHByb3ByaWF0ZS4gSWYgd2UgYWRkIGFueW1vcmUgZGV2aWNlcyB3
aGljaCBzaGFyZSB0aGUgZGVmaW5pdGlvbiwgd2UNCj4gY2FuIHBpY2sgdGhlIGJlc3QgbmFtaW5n
IGNvbnZlbnRpb24gZm9yIHRoZSBzaXR1YXRpb24gSSB0aGluay4gRm9yDQo+IGluc3RhbmNlLCBJ
IGxpa2UgdGhhdCB5b3UndmUgc3BsaXQgdGhlIGNoYW5uZWxzIHVwIGludG8gdGhlIG51bWJlciBv
Zg0KPiBiaXRzIHRoZXkgc3VwcG9ydCBpbiB0aGUgZ3lybyBhbmQgYWNjZWwgY2FzZXMsIHNvIHNv
bWV0aGluZyBvZiB0aGF0DQo+IG5hdHVyZSBjb3VsZCBiZSB1dGlsaXNlZCBpZiBvdGhlciBkZXZp
Y2Ugc3VwcG9ydCBpcyBhZGRlZC4NCj4NCj4+IFRoZSBjaGFubmVsIGRlZmluaXRpb24gaXMgaW50
ZW5kZWQgdGhlIHN3aXRjaCBieSBtYWNybw0KPj4gU1RfU0VOU09SU19MU01fQ0hBTk5FTFMgdG8g
dGhlIGZ1bGwgZGVmaW5pdGlvbiwgZm9yIG1lIGlzIG5vdCBhIHByb2JsZW0NCj4+IGJ1dCBJIHRo
aW5rIGl0J3Mgbm90IG5lY2Vzc2FyeS4NCj4gSWYgeW91IGFyZSBmYW1pbGlhciB3aXRoIHRoZSBt
YWNybyBJIGd1ZXNzIHlvdSBjb3VsZCBnZXQgdXNlZCB0bw0KPiB3b3JraW5nIHdpdGggaXQsIGJ1
dCBjb21pbmcgZnJvbSBpbiBhcyBhIGZpcnN0IHRpbWUgcmVhZGVyLCBhZGRpbmcgYQ0KPiBuZXcg
ZGV2aWNlIHdhcyBwcmV0dHkgZGlmZmljdWx0LiBJIGhhZCB0byBsb29rIHVwIHRoZSBtYWNybyBp
biB0aGUNCj4gaGVhZGVyIGZpbGUsIHRoZW4gaGF2ZSB0aGUgb3JpZ2luYWwgc3RydWN0IG9wZW4g
dG9vIGFuZCBjcm9zcw0KPiByZWZlcmVuY2UgaW4gMyBkaWZmZXJlbnQgcGxhY2VzLiBJdCdzIG1h
ZGUgZXZlbiBtb3JlIGRpZmZpY3VsdCBieSB0aGUNCj4gbWFjcm8gYmVpbmcgaW4gYSBkaWZmZXJl
bnQgb3JkZXIgdG8gdGhlIG9yaWdpbmFsIHN0cnVjdC4NCj4NCj4gTm93IEkndmUgaGFkIHRpbWUg
dG8gd29yayB3aXRoIGl0LCBJIGNvdWxkIHByb2JhYmx5IHdvcmsgd2l0aCBpdCBhcw0KPiB3ZWxs
LiBJIHdhcyBqdXN0IHRoaW5raW5nIGFib3V0IGhlbHBpbmcgb3V0IGFueSBuZXcgcGVyc29uIHRo
YXQgY29tZXMNCj4gYWxvbmcgYW5kIHRyaWVzIHRvIGFkZCBzdXBwb3J0IGZvciBhIG5ldyBzZW5z
b3IuDQo+DQo+Pj4+PiBTaWduZWQtb2ZmLWJ5OiBMZWUgSm9uZXMgPGxlZS5qb25lc0BsaW5hcm8u
b3JnPg0KPj4+Pj4gLS0tDQo+Pj4+PiAgICAgZHJpdmVycy9paW8vcHJlc3N1cmUvc3RfcHJlc3N1
cmVfY29yZS5jIHwgNDUgKysrKysrKysrKysrKysrKysrKysrKysrKy0tLS0tLS0tDQo+Pj4+PiAg
ICAgMSBmaWxlIGNoYW5nZWQsIDM0IGluc2VydGlvbnMoKyksIDExIGRlbGV0aW9ucygtKQ0KPj4+
Pj4NCj4+Pj4+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2lpby9wcmVzc3VyZS9zdF9wcmVzc3VyZV9j
b3JlLmMgYi9kcml2ZXJzL2lpby9wcmVzc3VyZS9zdF9wcmVzc3VyZV9jb3JlLmMNCj4+Pj4+IGlu
ZGV4IGJlY2ZiMjUuLjdiYTkyOTkgMTAwNjQ0DQo+Pj4+PiAtLS0gYS9kcml2ZXJzL2lpby9wcmVz
c3VyZS9zdF9wcmVzc3VyZV9jb3JlLmMNCj4+Pj4+ICsrKyBiL2RyaXZlcnMvaWlvL3ByZXNzdXJl
L3N0X3ByZXNzdXJlX2NvcmUuYw0KPj4+Pj4gQEAgLTU4LDE2ICs1OCwzOSBAQA0KPj4+Pj4gICAg
ICNkZWZpbmUgU1RfUFJFU1NfTFBTMzMxQVBfT1VUX1hMX0FERFIJCTB4MjgNCj4+Pj4+ICAgICAj
ZGVmaW5lIFNUX1RFTVBfTFBTMzMxQVBfT1VUX0xfQUREUgkJMHgyYg0KPj4+Pj4gICAgIA0KPj4+
Pj4gLXN0YXRpYyBjb25zdCBzdHJ1Y3QgaWlvX2NoYW5fc3BlYyBzdF9wcmVzc19jaGFubmVsc1td
ID0gew0KPj4+Pj4gLQlTVF9TRU5TT1JTX0xTTV9DSEFOTkVMUyhJSU9fUFJFU1NVUkUsDQo+Pj4+
PiArc3RhdGljIGNvbnN0IHN0cnVjdCBpaW9fY2hhbl9zcGVjIHN0X3ByZXNzX2xzcDMzMWFwX2No
YW5uZWxzW10gPSB7DQo+Pj4+PiArCXsNCj4+Pj4+ICsJCS50eXBlID0gSUlPX1BSRVNTVVJFLA0K
Pj4+Pj4gKwkJLmNoYW5uZWwyID0gSUlPX05PX01PRCwNCj4+Pj4+ICsJCS5hZGRyZXNzID0gU1Rf
UFJFU1NfTFBTMzMxQVBfT1VUX1hMX0FERFIsDQo+Pj4+PiArCQkuc2Nhbl9pbmRleCA9IFNUX1NF
TlNPUlNfU0NBTl9YLA0KPj4+Pj4gKwkJLnNjYW5fdHlwZSA9IHsNCj4+Pj4+ICsJCQkuc2lnbiA9
ICd1JywNCj4+Pj4+ICsJCQkucmVhbGJpdHMgPSAyNCwNCj4+Pj4+ICsJCQkuc3RvcmFnZWJpdHMg
PSAyNCwNCj4+Pj4+ICsJCQkuZW5kaWFubmVzcyA9IElJT19MRSwNCj4+Pj4+ICsJCX0sDQo+Pj4+
PiArCQkuaW5mb19tYXNrX3NlcGFyYXRlID0NCj4+Pj4+ICAgICAJCQlCSVQoSUlPX0NIQU5fSU5G
T19SQVcpIHwgQklUKElJT19DSEFOX0lORk9fU0NBTEUpLA0KPj4+Pj4gLQkJCVNUX1NFTlNPUlNf
U0NBTl9YLCAwLCBJSU9fTk9fTU9ELCAndScsIElJT19MRSwgMjQsIDI0LA0KPj4+Pj4gLQkJCVNU
X1BSRVNTX0xQUzMzMUFQX09VVF9YTF9BRERSKSwNCj4+Pj4+IC0JU1RfU0VOU09SU19MU01fQ0hB
Tk5FTFMoSUlPX1RFTVAsDQo+Pj4+PiAtCQkJQklUKElJT19DSEFOX0lORk9fUkFXKSB8IEJJVChJ
SU9fQ0hBTl9JTkZPX1NDQUxFKSB8DQo+Pj4+PiAtCQkJCQkJQklUKElJT19DSEFOX0lORk9fT0ZG
U0VUKSwNCj4+Pj4+IC0JCQktMSwgMCwgSUlPX05PX01PRCwgJ3MnLCBJSU9fTEUsIDE2LCAxNiwN
Cj4+Pj4+IC0JCQlTVF9URU1QX0xQUzMzMUFQX09VVF9MX0FERFIpLA0KPj4+Pj4gKwkJLm1vZGlm
aWVkID0gMCwNCj4+Pj4+ICsJfSwNCj4+Pj4+ICsJew0KPj4+Pj4gKwkJLnR5cGUgPSBJSU9fVEVN
UCwNCj4+Pj4+ICsJCS5jaGFubmVsMiA9IElJT19OT19NT0QsDQo+Pj4+PiArCQkuYWRkcmVzcyA9
IFNUX1RFTVBfTFBTMzMxQVBfT1VUX0xfQUREUiwNCj4+Pj4+ICsJCS5zY2FuX2luZGV4ID0gLTEs
DQo+Pj4+PiArCQkuc2Nhbl90eXBlID0gew0KPj4+Pj4gKwkJCS5zaWduID0gJ3UnLA0KPj4+Pj4g
KwkJCS5yZWFsYml0cyA9IDE2LA0KPj4+Pj4gKwkJCS5zdG9yYWdlYml0cyA9IDE2LA0KPj4+Pj4g
KwkJCS5lbmRpYW5uZXNzID0gSUlPX0xFLA0KPj4+Pj4gKwkJfSwNCj4+Pj4+ICsJCS5pbmZvX21h
c2tfc2VwYXJhdGUgPQ0KPj4+Pj4gKwkJCUJJVChJSU9fQ0hBTl9JTkZPX1JBVykgfA0KPj4+Pj4g
KwkJCUJJVChJSU9fQ0hBTl9JTkZPX1NDQUxFKSB8DQo+Pj4+PiArCQkJQklUKElJT19DSEFOX0lO
Rk9fT0ZGU0VUKSwNCj4+Pj4+ICsJCS5tb2RpZmllZCA9IDAsDQo+Pj4+PiArCX0sDQo+Pj4+PiAg
ICAgCUlJT19DSEFOX1NPRlRfVElNRVNUQU1QKDEpDQo+Pj4+PiAgICAgfTsNCj4+Pj4+ICAgICAN
Cj4+Pj4+IEBAIC03Nyw3ICsxMDAsNyBAQCBzdGF0aWMgY29uc3Qgc3RydWN0IHN0X3NlbnNvcnMg
c3RfcHJlc3Nfc2Vuc29yc1tdID0gew0KPj4+Pj4gICAgIAkJLnNlbnNvcnNfc3VwcG9ydGVkID0g
ew0KPj4+Pj4gICAgIAkJCVswXSA9IExQUzMzMUFQX1BSRVNTX0RFVl9OQU1FLA0KPj4+Pj4gICAg
IAkJfSwNCj4+Pj4+IC0JCS5jaCA9IChzdHJ1Y3QgaWlvX2NoYW5fc3BlYyAqKXN0X3ByZXNzX2No
YW5uZWxzLA0KPj4+Pj4gKwkJLmNoID0gKHN0cnVjdCBpaW9fY2hhbl9zcGVjICopc3RfcHJlc3Nf
bHNwMzMxYXBfY2hhbm5lbHMsDQo+Pj4+PiAgICAgCQkub2RyID0gew0KPj4+Pj4gICAgIAkJCS5h
ZGRyID0gU1RfUFJFU1NfTFBTMzMxQVBfT0RSX0FERFIsDQo+Pj4+PiAgICAgCQkJLm1hc2sgPSBT
VF9QUkVTU19MUFMzMzFBUF9PRFJfTUFTSywNCj4+Pj4+IEBAIC0yMTQsNyArMjM3LDcgQEAgaW50
IHN0X3ByZXNzX2NvbW1vbl9wcm9iZShzdHJ1Y3QgaWlvX2RldiAqaW5kaW9fZGV2KQ0KPj4+Pj4g
ICAgIAlwZGF0YS0+bnVtX2RhdGFfY2hhbm5lbHMgPSBTVF9QUkVTU19OVU1CRVJfREFUQV9DSEFO
TkVMUzsNCj4+Pj4+ICAgICAJcGRhdGEtPm11bHRpcmVhZF9iaXQgPSBwZGF0YS0+c2Vuc29yLT5t
dWx0aV9yZWFkX2JpdDsNCj4+Pj4+ICAgICAJaW5kaW9fZGV2LT5jaGFubmVscyA9IHBkYXRhLT5z
ZW5zb3ItPmNoOw0KPj4+Pj4gLQlpbmRpb19kZXYtPm51bV9jaGFubmVscyA9IEFSUkFZX1NJWkUo
c3RfcHJlc3NfY2hhbm5lbHMpOw0KPj4+Pj4gKwlpbmRpb19kZXYtPm51bV9jaGFubmVscyA9IEFS
UkFZX1NJWkUoc3RfcHJlc3NfbHNwMzMxYXBfY2hhbm5lbHMpOw0KPj4+Pj4gICAgIA0KPj4+Pj4g
ICAgIAlwZGF0YS0+Y3VycmVudF9mdWxsc2NhbGUgPSAoc3RydWN0IHN0X3NlbnNvcl9mdWxsc2Nh
bGVfYXZsICopDQo+Pj4+PiAgICAgCQkJCQkJJnBkYXRhLT5zZW5zb3ItPmZzLmZzX2F2bFswXTsN
Cg==

[PATCH 07/11] iio: sensors-core: st: Allow full-scale to be an optional feature

[Lee Jones]

Some chips either don't support it or fail to provide adequate documentation,
so sometimes it's impossible to enable the feature even if it is supported.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/iio/common/st_sensors/st_sensors_core.c | 11 +++++++----
 drivers/iio/pressure/st_pressure_core.c         |  6 ++++--
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
index 865b178..fc9acb7 100644
--- a/drivers/iio/common/st_sensors/st_sensors_core.c
+++ b/drivers/iio/common/st_sensors/st_sensors_core.c
@@ -209,10 +209,13 @@ int st_sensors_init_sensor(struct iio_dev *indio_dev)
 	if (err < 0)
 		goto init_error;
 
-	err = st_sensors_set_fullscale(indio_dev,
-						sdata->current_fullscale->num);
-	if (err < 0)
-		goto init_error;
+	if (sdata->current_fullscale) {
+		err = st_sensors_set_fullscale(indio_dev,
+					       sdata->current_fullscale->num);
+		if (err < 0)
+			goto init_error;
+	} else
+		dev_info(&indio_dev->dev, "Full-scale not possible\n");
 
 	err = st_sensors_set_odr(indio_dev, sdata->odr);
 	if (err < 0)
diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
index 7ba9299..423ed6a 100644
--- a/drivers/iio/pressure/st_pressure_core.c
+++ b/drivers/iio/pressure/st_pressure_core.c
@@ -239,8 +239,10 @@ int st_press_common_probe(struct iio_dev *indio_dev)
 	indio_dev->channels = pdata->sensor->ch;
 	indio_dev->num_channels = ARRAY_SIZE(st_press_lsp331ap_channels);
 
-	pdata->current_fullscale = (struct st_sensor_fullscale_avl *)
-						&pdata->sensor->fs.fs_avl[0];
+	if (pdata->sensor->fs.addr != 0)
+		pdata->current_fullscale = (struct st_sensor_fullscale_avl *)
+			&pdata->sensor->fs.fs_avl[0];
+
 	pdata->odr = pdata->sensor->odr.odr_avl[0].hz;
 
 	err = st_sensors_init_sensor(indio_dev);
-- 
1.8.1.2

Re: [PATCH 07/11] iio: sensors-core: st: Allow full-scale to be an optional feature

[Denis CIOCCA]

Acked-by: Denis Ciocca <denis.ciocca@st.com>
> Some chips either don't support it or fail to provide adequate documentat=
ion,
> so sometimes it's impossible to enable the feature even if it is supporte=
d.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>   drivers/iio/common/st_sensors/st_sensors_core.c | 11 +++++++----
>   drivers/iio/pressure/st_pressure_core.c         |  6 ++++--
>   2 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/ii=
o/common/st_sensors/st_sensors_core.c
> index 865b178..fc9acb7 100644
> --- a/drivers/iio/common/st_sensors/st_sensors_core.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
> @@ -209,10 +209,13 @@ int st_sensors_init_sensor(struct iio_dev *indio_de=
v)
>   	if (err < 0)
>   		goto init_error;
>  =20
> -	err =3D st_sensors_set_fullscale(indio_dev,
> -						sdata->current_fullscale->num);
> -	if (err < 0)
> -		goto init_error;
> +	if (sdata->current_fullscale) {
> +		err =3D st_sensors_set_fullscale(indio_dev,
> +					       sdata->current_fullscale->num);
> +		if (err < 0)
> +			goto init_error;
> +	} else
> +		dev_info(&indio_dev->dev, "Full-scale not possible\n");
>  =20
>   	err =3D st_sensors_set_odr(indio_dev, sdata->odr);
>   	if (err < 0)
> diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressu=
re/st_pressure_core.c
> index 7ba9299..423ed6a 100644
> --- a/drivers/iio/pressure/st_pressure_core.c
> +++ b/drivers/iio/pressure/st_pressure_core.c
> @@ -239,8 +239,10 @@ int st_press_common_probe(struct iio_dev *indio_dev)
>   	indio_dev->channels =3D pdata->sensor->ch;
>   	indio_dev->num_channels =3D ARRAY_SIZE(st_press_lsp331ap_channels);
>  =20
> -	pdata->current_fullscale =3D (struct st_sensor_fullscale_avl *)
> -						&pdata->sensor->fs.fs_avl[0];
> +	if (pdata->sensor->fs.addr !=3D 0)
> +		pdata->current_fullscale =3D (struct st_sensor_fullscale_avl *)
> +			&pdata->sensor->fs.fs_avl[0];
> +
>   	pdata->odr =3D pdata->sensor->odr.odr_avl[0].hz;
>  =20
>   	err =3D st_sensors_init_sensor(indio_dev);

[PATCH 08/11] iio: pressure-core: st: Allow for number of channels to vary

[Lee Jones]

At the moment the number of channels specified is dictated by the first
sensor supported by the driver. As we add support for more sensors this
is likely to vary. Instead of using the ARRAY_SIZE() of the LPS331AP's
channel specifier we'll use a new adaptable 'struct st_sensors' element
instead.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/iio/pressure/st_pressure_core.c | 3 ++-
 include/linux/iio/common/st_sensors.h   | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
index 423ed6a..3cf54ed 100644
--- a/drivers/iio/pressure/st_pressure_core.c
+++ b/drivers/iio/pressure/st_pressure_core.c
@@ -101,6 +101,7 @@ static const struct st_sensors st_press_sensors[] = {
 			[0] = LPS331AP_PRESS_DEV_NAME,
 		},
 		.ch = (struct iio_chan_spec *)st_press_lsp331ap_channels,
+		.num_ch = ARRAY_SIZE(st_press_lsp331ap_channels),
 		.odr = {
 			.addr = ST_PRESS_LPS331AP_ODR_ADDR,
 			.mask = ST_PRESS_LPS331AP_ODR_MASK,
@@ -237,7 +238,7 @@ int st_press_common_probe(struct iio_dev *indio_dev)
 	pdata->num_data_channels = ST_PRESS_NUMBER_DATA_CHANNELS;
 	pdata->multiread_bit = pdata->sensor->multi_read_bit;
 	indio_dev->channels = pdata->sensor->ch;
-	indio_dev->num_channels = ARRAY_SIZE(st_press_lsp331ap_channels);
+	indio_dev->num_channels = pdata->sensor->num_ch;
 
 	if (pdata->sensor->fs.addr != 0)
 		pdata->current_fullscale = (struct st_sensor_fullscale_avl *)
diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
index 72b2694..4aef925 100644
--- a/include/linux/iio/common/st_sensors.h
+++ b/include/linux/iio/common/st_sensors.h
@@ -180,6 +180,7 @@ struct st_sensors {
 	u8 wai;
 	char sensors_supported[ST_SENSORS_MAX_4WAI][ST_SENSORS_MAX_NAME];
 	struct iio_chan_spec *ch;
+	int num_ch;
 	struct st_sensor_odr odr;
 	struct st_sensor_power pw;
 	struct st_sensor_axis enable_axis;
-- 
1.8.1.2

Re: [PATCH 08/11] iio: pressure-core: st: Allow for number of channels to vary

[Denis CIOCCA]

Acked-by: Denis Ciocca <denis.ciocca@st.com>
> At the moment the number of channels specified is dictated by the first
> sensor supported by the driver. As we add support for more sensors this
> is likely to vary. Instead of using the ARRAY_SIZE() of the LPS331AP's
> channel specifier we'll use a new adaptable 'struct st_sensors' element
> instead.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>   drivers/iio/pressure/st_pressure_core.c | 3 ++-
>   include/linux/iio/common/st_sensors.h   | 1 +
>   2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressu=
re/st_pressure_core.c
> index 423ed6a..3cf54ed 100644
> --- a/drivers/iio/pressure/st_pressure_core.c
> +++ b/drivers/iio/pressure/st_pressure_core.c
> @@ -101,6 +101,7 @@ static const struct st_sensors st_press_sensors[] =3D=
 {
>   			[0] =3D LPS331AP_PRESS_DEV_NAME,
>   		},
>   		.ch =3D (struct iio_chan_spec *)st_press_lsp331ap_channels,
> +		.num_ch =3D ARRAY_SIZE(st_press_lsp331ap_channels),
>   		.odr =3D {
>   			.addr =3D ST_PRESS_LPS331AP_ODR_ADDR,
>   			.mask =3D ST_PRESS_LPS331AP_ODR_MASK,
> @@ -237,7 +238,7 @@ int st_press_common_probe(struct iio_dev *indio_dev)
>   	pdata->num_data_channels =3D ST_PRESS_NUMBER_DATA_CHANNELS;
>   	pdata->multiread_bit =3D pdata->sensor->multi_read_bit;
>   	indio_dev->channels =3D pdata->sensor->ch;
> -	indio_dev->num_channels =3D ARRAY_SIZE(st_press_lsp331ap_channels);
> +	indio_dev->num_channels =3D pdata->sensor->num_ch;
>  =20
>   	if (pdata->sensor->fs.addr !=3D 0)
>   		pdata->current_fullscale =3D (struct st_sensor_fullscale_avl *)
> diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/co=
mmon/st_sensors.h
> index 72b2694..4aef925 100644
> --- a/include/linux/iio/common/st_sensors.h
> +++ b/include/linux/iio/common/st_sensors.h
> @@ -180,6 +180,7 @@ struct st_sensors {
>   	u8 wai;
>   	char sensors_supported[ST_SENSORS_MAX_4WAI][ST_SENSORS_MAX_NAME];
>   	struct iio_chan_spec *ch;
> +	int num_ch;
>   	struct st_sensor_odr odr;
>   	struct st_sensor_power pw;
>   	struct st_sensor_axis enable_axis;

[PATCH 09/11] iio: pressure-core: st: Clean-up error handling in probe function

[Lee Jones]

This patch contains some pretty basic clean-ups in probe() pertaining to
the simplification of error handling and a couple of readability adaptions.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/iio/pressure/st_pressure_core.c | 41 +++++++++++++++------------------
 1 file changed, 19 insertions(+), 22 deletions(-)

diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
index 3cf54ed..2893d08 100644
--- a/drivers/iio/pressure/st_pressure_core.c
+++ b/drivers/iio/pressure/st_pressure_core.c
@@ -224,21 +224,23 @@ static const struct iio_trigger_ops st_press_trigger_ops = {
 
 int st_press_common_probe(struct iio_dev *indio_dev)
 {
-	int err;
 	struct st_sensor_data *pdata = iio_priv(indio_dev);
+	int irq = pdata->get_irq_data_ready(indio_dev);
+	int err;
 
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->info = &press_info;
 
 	err = st_sensors_check_device_support(indio_dev,
-				ARRAY_SIZE(st_press_sensors), st_press_sensors);
+					      ARRAY_SIZE(st_press_sensors),
+					      st_press_sensors);
 	if (err < 0)
-		goto st_press_common_probe_error;
+		return err;
 
 	pdata->num_data_channels = ST_PRESS_NUMBER_DATA_CHANNELS;
-	pdata->multiread_bit = pdata->sensor->multi_read_bit;
-	indio_dev->channels = pdata->sensor->ch;
-	indio_dev->num_channels = pdata->sensor->num_ch;
+	pdata->multiread_bit     = pdata->sensor->multi_read_bit;
+	indio_dev->channels      = pdata->sensor->ch;
+	indio_dev->num_channels  = pdata->sensor->num_ch;
 
 	if (pdata->sensor->fs.addr != 0)
 		pdata->current_fullscale = (struct st_sensor_fullscale_avl *)
@@ -248,32 +250,27 @@ int st_press_common_probe(struct iio_dev *indio_dev)
 
 	err = st_sensors_init_sensor(indio_dev);
 	if (err < 0)
-		goto st_press_common_probe_error;
+		return err;
 
-	if (pdata->get_irq_data_ready(indio_dev) > 0) {
+	if (irq > 0) {
 		err = st_press_allocate_ring(indio_dev);
 		if (err < 0)
-			goto st_press_common_probe_error;
+			return err;
 
 		err = st_sensors_allocate_trigger(indio_dev,
-							ST_PRESS_TRIGGER_OPS);
-		if (err < 0)
-			goto st_press_probe_trigger_error;
+						  ST_PRESS_TRIGGER_OPS);
+		if (err < 0) {
+			st_press_deallocate_ring(indio_dev);
+			return err;
+		}
 	}
 
 	err = iio_device_register(indio_dev);
-	if (err)
-		goto st_press_device_register_error;
-
-	return err;
-
-st_press_device_register_error:
-	if (pdata->get_irq_data_ready(indio_dev) > 0)
+	if (err && irq > 0) {
 		st_sensors_deallocate_trigger(indio_dev);
-st_press_probe_trigger_error:
-	if (pdata->get_irq_data_ready(indio_dev) > 0)
 		st_press_deallocate_ring(indio_dev);
-st_press_common_probe_error:
+	}
+
 	return err;
 }
 EXPORT_SYMBOL(st_press_common_probe);
-- 
1.8.1.2

Re: [PATCH 09/11] iio: pressure-core: st: Clean-up error handling in probe function

[Jonathan Cameron]

Lee Jones <lee.jones@linaro.org> wrote:
>This patch contains some pretty basic clean-ups in probe() pertaining
>to
>the simplification of error handling and a couple of readability
>adaptions.
>
>Signed-off-by: Lee Jones <lee.jones@linaro.org>

Hi Lee

Most of this series look fine but I am not that keen on some of this one.

The white space changes are definitely a matter of taste and honestly I don't care either way.

The returns are fine when there is no cleanup to do.

GOTOs and a clean unwind of the function is to my eye easier to review than unwinding directly after each error.

If reviewing original driver I would probably not bother commenting but for a cleanup patch the cleanup wants to be a clear improvement!  

Out of time now so will have to get to the rest of the series sometime soon.  

>---
>drivers/iio/pressure/st_pressure_core.c | 41
>+++++++++++++++------------------
> 1 file changed, 19 insertions(+), 22 deletions(-)
>
>diff --git a/drivers/iio/pressure/st_pressure_core.c
>b/drivers/iio/pressure/st_pressure_core.c
>index 3cf54ed..2893d08 100644
>--- a/drivers/iio/pressure/st_pressure_core.c
>+++ b/drivers/iio/pressure/st_pressure_core.c
>@@ -224,21 +224,23 @@ static const struct iio_trigger_ops
>st_press_trigger_ops = {
> 
> int st_press_common_probe(struct iio_dev *indio_dev)
> {
>-	int err;
> 	struct st_sensor_data *pdata = iio_priv(indio_dev);
>+	int irq = pdata->get_irq_data_ready(indio_dev);
>+	int err;
> 
> 	indio_dev->modes = INDIO_DIRECT_MODE;
> 	indio_dev->info = &press_info;
> 
> 	err = st_sensors_check_device_support(indio_dev,
>-				ARRAY_SIZE(st_press_sensors), st_press_sensors);
>+					      ARRAY_SIZE(st_press_sensors),
>+					      st_press_sensors);
> 	if (err < 0)
>-		goto st_press_common_probe_error;
>+		return err;
> 
> 	pdata->num_data_channels = ST_PRESS_NUMBER_DATA_CHANNELS;
>-	pdata->multiread_bit = pdata->sensor->multi_read_bit;
>-	indio_dev->channels = pdata->sensor->ch;
>-	indio_dev->num_channels = pdata->sensor->num_ch;
>+	pdata->multiread_bit     = pdata->sensor->multi_read_bit;
>+	indio_dev->channels      = pdata->sensor->ch;
>+	indio_dev->num_channels  = pdata->sensor->num_ch;
> 
> 	if (pdata->sensor->fs.addr != 0)
> 		pdata->current_fullscale = (struct st_sensor_fullscale_avl *)
>@@ -248,32 +250,27 @@ int st_press_common_probe(struct iio_dev
>*indio_dev)
> 
> 	err = st_sensors_init_sensor(indio_dev);
> 	if (err < 0)
>-		goto st_press_common_probe_error;
>+		return err;
> 
>-	if (pdata->get_irq_data_ready(indio_dev) > 0) {
>+	if (irq > 0) {
> 		err = st_press_allocate_ring(indio_dev);
> 		if (err < 0)
>-			goto st_press_common_probe_error;
>+			return err;
> 
> 		err = st_sensors_allocate_trigger(indio_dev,
>-							ST_PRESS_TRIGGER_OPS);
>-		if (err < 0)
>-			goto st_press_probe_trigger_error;
>+						  ST_PRESS_TRIGGER_OPS);
>+		if (err < 0) {
>+			st_press_deallocate_ring(indio_dev);
>+			return err;
>+		}
> 	}
> 
> 	err = iio_device_register(indio_dev);
>-	if (err)
>-		goto st_press_device_register_error;
>-
>-	return err;
>-
>-st_press_device_register_error:
>-	if (pdata->get_irq_data_ready(indio_dev) > 0)
>+	if (err && irq > 0) {
> 		st_sensors_deallocate_trigger(indio_dev);
>-st_press_probe_trigger_error:
>-	if (pdata->get_irq_data_ready(indio_dev) > 0)
> 		st_press_deallocate_ring(indio_dev);
>-st_press_common_probe_error:
>+	}
>+
> 	return err;
> }
> EXPORT_SYMBOL(st_press_common_probe);

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

[PATCH 10/11] iio: pressure: st: Add support for new LPS001WP pressure sensor

[Lee Jones]

Here we use existing practices to introduce support for another
pressure/temperature sensor, the LPS001WP.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/iio/pressure/st_pressure.h      |  1 +
 drivers/iio/pressure/st_pressure_core.c | 84 +++++++++++++++++++++++++++++++++
 drivers/iio/pressure/st_pressure_i2c.c  |  1 +
 3 files changed, 86 insertions(+)

diff --git a/drivers/iio/pressure/st_pressure.h b/drivers/iio/pressure/st_pressure.h
index 414e45a..5b42aa2 100644
--- a/drivers/iio/pressure/st_pressure.h
+++ b/drivers/iio/pressure/st_pressure.h
@@ -15,6 +15,7 @@
 #include <linux/iio/common/st_sensors.h>
 
 #define LPS331AP_PRESS_DEV_NAME		"lps331ap"
+#define LPS001WP_PRESS_DEV_NAME		"lps001wp"
 
 int st_press_common_probe(struct iio_dev *indio_dev);
 void st_press_common_remove(struct iio_dev *indio_dev);
diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
index 2893d08..f452417 100644
--- a/drivers/iio/pressure/st_pressure_core.c
+++ b/drivers/iio/pressure/st_pressure_core.c
@@ -58,6 +58,21 @@
 #define ST_PRESS_LPS331AP_OUT_XL_ADDR		0x28
 #define ST_TEMP_LPS331AP_OUT_L_ADDR		0x2b
 
+/* CUSTOM VALUES FOR LPS001WP SENSOR */
+#define ST_PRESS_LPS001WP_WAI_EXP		0xba
+#define ST_PRESS_LPS001WP_ODR_ADDR		0x20
+#define ST_PRESS_LPS001WP_ODR_MASK		0x30
+#define ST_PRESS_LPS001WP_ODR_AVL_1HZ_VAL	0x01
+#define ST_PRESS_LPS001WP_ODR_AVL_7HZ_VAL	0x02
+#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_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
+
 static const struct iio_chan_spec st_press_lsp331ap_channels[] = {
 	{
 		.type = IIO_PRESSURE,
@@ -94,6 +109,40 @@ static const struct iio_chan_spec st_press_lsp331ap_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(1)
 };
 
+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_type = {
+			.sign = 'u',
+			.realbits = 16,
+			.storagebits = 16,
+			.endianness = IIO_LE,
+		},
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+		.modified = 0,
+	},
+	{
+		.type = IIO_TEMP,
+		.channel2 = IIO_NO_MOD,
+		.address = ST_TEMP_LPS001WP_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_OFFSET),
+		.modified = 0,
+	},
+	IIO_CHAN_SOFT_TIMESTAMP(1)
+};
+
 static const struct st_sensors st_press_sensors[] = {
 	{
 		.wai = ST_PRESS_LPS331AP_WAI_EXP,
@@ -141,6 +190,41 @@ static const struct st_sensors st_press_sensors[] = {
 		.multi_read_bit = ST_PRESS_LPS331AP_MULTIREAD_BIT,
 		.bootime = 2,
 	},
+	{
+		.wai = ST_PRESS_LPS001WP_WAI_EXP,
+		.sensors_supported = {
+			[0] = LPS001WP_PRESS_DEV_NAME,
+		},
+		.ch = (struct iio_chan_spec *)st_press_lps001wp_channels,
+		.num_ch = ARRAY_SIZE(st_press_lps001wp_channels),
+		.odr = {
+			.addr = ST_PRESS_LPS001WP_ODR_ADDR,
+			.mask = ST_PRESS_LPS001WP_ODR_MASK,
+			.odr_avl = {
+				{ 1, ST_PRESS_LPS001WP_ODR_AVL_1HZ_VAL, },
+				{ 7, ST_PRESS_LPS001WP_ODR_AVL_7HZ_VAL, },
+				{ 13, ST_PRESS_LPS001WP_ODR_AVL_13HZ_VAL, },
+			},
+		},
+		.pw = {
+			.addr = ST_PRESS_LPS001WP_PW_ADDR,
+			.mask = ST_PRESS_LPS001WP_PW_MASK,
+			.value_on = ST_SENSORS_DEFAULT_POWER_ON_VALUE,
+			.value_off = ST_SENSORS_DEFAULT_POWER_OFF_VALUE,
+		},
+		.fs = {
+			.addr = 0,
+		},
+		.bdu = {
+			.addr = ST_PRESS_LPS001WP_BDU_ADDR,
+			.mask = ST_PRESS_LPS001WP_BDU_MASK,
+		},
+		.drdy_irq = {
+			.addr = 0,
+		},
+		.multi_read_bit = ST_PRESS_LPS001WP_MULTIREAD_BIT,
+		.bootime = 2,
+	},
 };
 
 static int st_press_read_raw(struct iio_dev *indio_dev,
diff --git a/drivers/iio/pressure/st_pressure_i2c.c b/drivers/iio/pressure/st_pressure_i2c.c
index 2ace770..c5c96ac 100644
--- a/drivers/iio/pressure/st_pressure_i2c.c
+++ b/drivers/iio/pressure/st_pressure_i2c.c
@@ -52,6 +52,7 @@ static int st_press_i2c_remove(struct i2c_client *client)
 
 static const struct i2c_device_id st_press_id_table[] = {
 	{ LPS331AP_PRESS_DEV_NAME },
+	{ LPS001WP_PRESS_DEV_NAME },
 	{},
 };
 MODULE_DEVICE_TABLE(i2c, st_press_id_table);
-- 
1.8.1.2

[PATCH 11/11] iio: pressure-core: st: Provide correct regulator support

[Lee Jones]

The power to some of the sensors are controlled by regulators. In most
cases these are 'always on', but if not they will fail to work until
the regulator is enabled using the relevant APIs.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/iio/pressure/st_pressure_core.c | 13 +++++++++++++
 include/linux/iio/common/st_sensors.h   |  3 +++
 2 files changed, 16 insertions(+)

diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
index f452417..7beed89 100644
--- a/drivers/iio/pressure/st_pressure_core.c
+++ b/drivers/iio/pressure/st_pressure_core.c
@@ -23,6 +23,7 @@
 #include <linux/iio/sysfs.h>
 #include <linux/iio/trigger.h>
 #include <linux/iio/buffer.h>
+#include <linux/regulator/consumer.h>
 #include <asm/unaligned.h>
 
 #include <linux/iio/common/st_sensors.h>
@@ -315,6 +316,15 @@ int st_press_common_probe(struct iio_dev *indio_dev)
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->info = &press_info;
 
+	/* Regulator not mandatory, but if requested we should enable it. */
+	pdata->regulator = regulator_get(&indio_dev->dev, "vdd");
+	if (!IS_ERR_OR_NULL(pdata->regulator)) {
+		err = regulator_enable(pdata->regulator);
+		if (err != 0)
+			dev_warn(&indio_dev->dev,
+				 "Failed to enable specified vdd regulator\n");
+	}
+
 	err = st_sensors_check_device_support(indio_dev,
 					      ARRAY_SIZE(st_press_sensors),
 					      st_press_sensors);
@@ -363,6 +373,9 @@ void st_press_common_remove(struct iio_dev *indio_dev)
 {
 	struct st_sensor_data *pdata = iio_priv(indio_dev);
 
+	if (!IS_ERR_OR_NULL(pdata->regulator))
+		regulator_disable(pdata->regulator);
+
 	iio_device_unregister(indio_dev);
 	if (pdata->get_irq_data_ready(indio_dev) > 0) {
 		st_sensors_deallocate_trigger(indio_dev);
diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
index 4aef925..eb6ef3f 100644
--- a/include/linux/iio/common/st_sensors.h
+++ b/include/linux/iio/common/st_sensors.h
@@ -16,6 +16,7 @@
 #include <linux/irqreturn.h>
 #include <linux/iio/trigger.h>
 #include <linux/bitops.h>
+#include <linux/regulator/consumer.h>
 
 #define ST_SENSORS_TX_MAX_LENGTH		2
 #define ST_SENSORS_RX_MAX_LENGTH		6
@@ -197,6 +198,7 @@ struct st_sensors {
  * @trig: The trigger in use by the core driver.
  * @sensor: Pointer to the current sensor struct in use.
  * @current_fullscale: Maximum range of measure by the sensor.
+ * @regulator: Pointer to sensor's power supply
  * @enabled: Status of the sensor (false->off, true->on).
  * @multiread_bit: Use or not particular bit for [I2C/SPI] multiread.
  * @buffer_data: Data used by buffer part.
@@ -211,6 +213,7 @@ struct st_sensor_data {
 	struct iio_trigger *trig;
 	struct st_sensors *sensor;
 	struct st_sensor_fullscale_avl *current_fullscale;
+	struct regulator *regulator;
 
 	bool enabled;
 	bool multiread_bit;
-- 
1.8.1.2

Re: [PATCH 11/11] iio: pressure-core: st: Provide correct regulator support

[Mark Rutland]

Hi Lee,

On Wed, Sep 04, 2013 at 10:31:43AM +0100, Lee Jones wrote:
> The power to some of the sensors are controlled by regulators. In most
> cases these are 'always on', but if not they will fail to work until
> the regulator is enabled using the relevant APIs.
> 
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/iio/pressure/st_pressure_core.c | 13 +++++++++++++
>  include/linux/iio/common/st_sensors.h   |  3 +++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
> index f452417..7beed89 100644
> --- a/drivers/iio/pressure/st_pressure_core.c
> +++ b/drivers/iio/pressure/st_pressure_core.c
> @@ -23,6 +23,7 @@
>  #include <linux/iio/sysfs.h>
>  #include <linux/iio/trigger.h>
>  #include <linux/iio/buffer.h>
> +#include <linux/regulator/consumer.h>
>  #include <asm/unaligned.h>
>  
>  #include <linux/iio/common/st_sensors.h>
> @@ -315,6 +316,15 @@ int st_press_common_probe(struct iio_dev *indio_dev)
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  	indio_dev->info = &press_info;
>  
> +	/* Regulator not mandatory, but if requested we should enable it. */
> +	pdata->regulator = regulator_get(&indio_dev->dev, "vdd");
> +	if (!IS_ERR_OR_NULL(pdata->regulator)) {

Can regulator_get return NULL? As far as I can see, it either returns a
valid reulator pointer or an ERR_PTR value.

When you say "if requested", do you mean "if described in the dt"? If
so, the above doesn't distunguish between a regulator not being listed
and one failing to be got (e.g. if we got EPROBE_DEFER from
regulator_get).

I think this would be better handled with something like Mark Brown's
suggested regulator_get_optional [1,2].

Thanks,
Mark.

[1] https://lkml.org/lkml/2013/7/30/328 (cover)
[2] https://lkml.org/lkml/2013/7/30/334 (patches)

> +		err = regulator_enable(pdata->regulator);
> +		if (err != 0)
> +			dev_warn(&indio_dev->dev,
> +				 "Failed to enable specified vdd regulator\n");
> +	}
> +
>  	err = st_sensors_check_device_support(indio_dev,
>  					      ARRAY_SIZE(st_press_sensors),
>  					      st_press_sensors);
> @@ -363,6 +373,9 @@ void st_press_common_remove(struct iio_dev *indio_dev)
>  {
>  	struct st_sensor_data *pdata = iio_priv(indio_dev);
>  
> +	if (!IS_ERR_OR_NULL(pdata->regulator))
> +		regulator_disable(pdata->regulator);
> +
>  	iio_device_unregister(indio_dev);
>  	if (pdata->get_irq_data_ready(indio_dev) > 0) {
>  		st_sensors_deallocate_trigger(indio_dev);
> diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
> index 4aef925..eb6ef3f 100644
> --- a/include/linux/iio/common/st_sensors.h
> +++ b/include/linux/iio/common/st_sensors.h
> @@ -16,6 +16,7 @@
>  #include <linux/irqreturn.h>
>  #include <linux/iio/trigger.h>
>  #include <linux/bitops.h>
> +#include <linux/regulator/consumer.h>
>  
>  #define ST_SENSORS_TX_MAX_LENGTH		2
>  #define ST_SENSORS_RX_MAX_LENGTH		6
> @@ -197,6 +198,7 @@ struct st_sensors {
>   * @trig: The trigger in use by the core driver.
>   * @sensor: Pointer to the current sensor struct in use.
>   * @current_fullscale: Maximum range of measure by the sensor.
> + * @regulator: Pointer to sensor's power supply
>   * @enabled: Status of the sensor (false->off, true->on).
>   * @multiread_bit: Use or not particular bit for [I2C/SPI] multiread.
>   * @buffer_data: Data used by buffer part.
> @@ -211,6 +213,7 @@ struct st_sensor_data {
>  	struct iio_trigger *trig;
>  	struct st_sensors *sensor;
>  	struct st_sensor_fullscale_avl *current_fullscale;
> +	struct regulator *regulator;
>  
>  	bool enabled;
>  	bool multiread_bit;
> -- 
> 1.8.1.2
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

Re: [PATCH 11/11] iio: pressure-core: st: Provide correct regulator support

[Lars-Peter Clausen]

On 09/04/2013 03:11 PM, Mark Rutland wrote:
> Hi Lee,
> 
> On Wed, Sep 04, 2013 at 10:31:43AM +0100, Lee Jones wrote:
>> The power to some of the sensors are controlled by regulators. In most
>> cases these are 'always on', but if not they will fail to work until
>> the regulator is enabled using the relevant APIs.
>>
>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
>> ---
>>  drivers/iio/pressure/st_pressure_core.c | 13 +++++++++++++
>>  include/linux/iio/common/st_sensors.h   |  3 +++
>>  2 files changed, 16 insertions(+)
>>
>> diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
>> index f452417..7beed89 100644
>> --- a/drivers/iio/pressure/st_pressure_core.c
>> +++ b/drivers/iio/pressure/st_pressure_core.c
>> @@ -23,6 +23,7 @@
>>  #include <linux/iio/sysfs.h>
>>  #include <linux/iio/trigger.h>
>>  #include <linux/iio/buffer.h>
>> +#include <linux/regulator/consumer.h>
>>  #include <asm/unaligned.h>
>>  
>>  #include <linux/iio/common/st_sensors.h>
>> @@ -315,6 +316,15 @@ int st_press_common_probe(struct iio_dev *indio_dev)
>>  	indio_dev->modes = INDIO_DIRECT_MODE;
>>  	indio_dev->info = &press_info;
>>  
>> +	/* Regulator not mandatory, but if requested we should enable it. */
>> +	pdata->regulator = regulator_get(&indio_dev->dev, "vdd");
>> +	if (!IS_ERR_OR_NULL(pdata->regulator)) {
> 
> Can regulator_get return NULL? As far as I can see, it either returns a
> valid reulator pointer or an ERR_PTR value.
> 
> When you say "if requested", do you mean "if described in the dt"? If
> so, the above doesn't distunguish between a regulator not being listed
> and one failing to be got (e.g. if we got EPROBE_DEFER from
> regulator_get).
> 
> I think this would be better handled with something like Mark Brown's
> suggested regulator_get_optional [1,2].

It can return NULL, but NULL is actually a valid regulator in that case, so
the check should only be IS_ERR. And yes regulator_get_optional is what
should be used here.

- Lars

Re: [PATCH 11/11] iio: pressure-core: st: Provide correct regulator support

[Lee Jones]

On Wed, 04 Sep 2013, Lars-Peter Clausen wrote:

> On 09/04/2013 03:11 PM, Mark Rutland wrote:
> > Hi Lee,
> > 
> > On Wed, Sep 04, 2013 at 10:31:43AM +0100, Lee Jones wrote:
> >> The power to some of the sensors are controlled by regulators. In most
> >> cases these are 'always on', but if not they will fail to work until
> >> the regulator is enabled using the relevant APIs.
> >>
> >> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> >> ---
> >>  drivers/iio/pressure/st_pressure_core.c | 13 +++++++++++++
> >>  include/linux/iio/common/st_sensors.h   |  3 +++
> >>  2 files changed, 16 insertions(+)
> >>
> >> diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
> >> index f452417..7beed89 100644
> >> --- a/drivers/iio/pressure/st_pressure_core.c
> >> +++ b/drivers/iio/pressure/st_pressure_core.c
> >> @@ -23,6 +23,7 @@
> >>  #include <linux/iio/sysfs.h>
> >>  #include <linux/iio/trigger.h>
> >>  #include <linux/iio/buffer.h>
> >> +#include <linux/regulator/consumer.h>
> >>  #include <asm/unaligned.h>
> >>  
> >>  #include <linux/iio/common/st_sensors.h>
> >> @@ -315,6 +316,15 @@ int st_press_common_probe(struct iio_dev *indio_dev)
> >>  	indio_dev->modes = INDIO_DIRECT_MODE;
> >>  	indio_dev->info = &press_info;
> >>  
> >> +	/* Regulator not mandatory, but if requested we should enable it. */
> >> +	pdata->regulator = regulator_get(&indio_dev->dev, "vdd");
> >> +	if (!IS_ERR_OR_NULL(pdata->regulator)) {
> > 
> > Can regulator_get return NULL? As far as I can see, it either returns a
> > valid reulator pointer or an ERR_PTR value.
> > 
> > When you say "if requested", do you mean "if described in the dt"? If
> > so, the above doesn't distunguish between a regulator not being listed
> > and one failing to be got (e.g. if we got EPROBE_DEFER from
> > regulator_get).
> > 
> > I think this would be better handled with something like Mark Brown's
> > suggested regulator_get_optional [1,2].

Thanks Mark, I didn't know that existed.

> It can return NULL, but NULL is actually a valid regulator in that case, so
> the check should only be IS_ERR. And yes regulator_get_optional is what
> should be used here.

Okay, I'll use that instead.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 11/11] iio: pressure-core: st: Provide correct regulator support

[Mark Brown]

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

On Wed, Sep 04, 2013 at 02:26:38PM +0100, Lee Jones wrote:
> On Wed, 04 Sep 2013, Lars-Peter Clausen wrote:

> > > I think this would be better handled with something like Mark Brown's
> > > suggested regulator_get_optional [1,2].

> Thanks Mark, I didn't know that existed.

It's only just gone into Linus' tree this merge window so it's very new.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 11/11] iio: pressure-core: st: Provide correct regulator support

[Mark Brown]

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

On Wed, Sep 04, 2013 at 02:11:11PM +0100, Mark Rutland wrote:
> On Wed, Sep 04, 2013 at 10:31:43AM +0100, Lee Jones wrote:

> > +	/* Regulator not mandatory, but if requested we should enable it. */
> > +	pdata->regulator = regulator_get(&indio_dev->dev, "vdd");
> > +	if (!IS_ERR_OR_NULL(pdata->regulator)) {

> Can regulator_get return NULL? As far as I can see, it either returns a
> valid reulator pointer or an ERR_PTR value.

Yes, NULL is a valid regulator.

> When you say "if requested", do you mean "if described in the dt"? If
> so, the above doesn't distunguish between a regulator not being listed
> and one failing to be got (e.g. if we got EPROBE_DEFER from
> regulator_get).

> I think this would be better handled with something like Mark Brown's
> suggested regulator_get_optional [1,2].

If the regulator may genuinely be absent from the system then it should
be being requested using regulator_get_optional().  Otherwise it should
be being requested using regulator_get().  In both cases it is important 
that the driver pays attention to errors.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]