[PATCH 0/9] hts221: add new features and fix power-off procedure

[PATCH 0/9] hts221: add new features and fix power-off procedure

[Lorenzo Bianconi]

Patches 1-3: fix a bug in power-down/suspend procedure
Patch 4: avoid useless sample rate configurations
Patches 5-8: add active low and open drain support with corresponding
dt-binding
Patch 9: improve code readability

Lorenzo Bianconi (9):
  iio: humidity: hts221: refactor write_with_mask code
  iio: humidity: hts221: move BDU configuration in probe routine
  iio: humidity: hts221: do not overwrite reserved data during
    power-down
  iio: humidity: hts221: avoid useless ODR reconfiguration
  iio: humidity: hts221: support active-low interrupts
  dt-bindings: iio: humidity: hts221: support active-low interrupts
  iio: humidity: hts221: support open drain mode
  dt-bindings: iio: humidity: hts221: support open drain mode
  iio: humidity: hts221: move drdy enable logic in
    hts221_trig_set_state()

 .../devicetree/bindings/iio/humidity/hts221.txt    |  11 +-
 drivers/iio/humidity/hts221.h                      |   5 +-
 drivers/iio/humidity/hts221_buffer.c               |  43 ++++++-
 drivers/iio/humidity/hts221_core.c                 | 124 +++++++--------------
 4 files changed, 93 insertions(+), 90 deletions(-)

-- 
2.13.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/9] iio: humidity: hts221: refactor write_with_mask code

[Lorenzo Bianconi]

Move bit-shift in hts221_write_with_mask() instead of coding
the shift depth in the configured value. That change will be necessary
to fix an issue in device power-down procedure

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
---
 drivers/iio/humidity/hts221_core.c | 74 +++++++++++++++++---------------------
 1 file changed, 32 insertions(+), 42 deletions(-)

diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
index a56da3999e00..f5181e4e1eff 100644
--- a/drivers/iio/humidity/hts221_core.c
+++ b/drivers/iio/humidity/hts221_core.c
@@ -32,30 +32,12 @@
 #define HTS221_HUMIDITY_AVG_MASK	0x07
 #define HTS221_TEMP_AVG_MASK		0x38
 
-#define HTS221_ODR_MASK			0x87
+#define HTS221_ODR_MASK			0x03
 #define HTS221_BDU_MASK			BIT(2)
+#define HTS221_ENABLE_MASK		BIT(7)
 
 #define HTS221_DRDY_MASK		BIT(2)
 
-#define HTS221_ENABLE_SENSOR		BIT(7)
-
-#define HTS221_HUMIDITY_AVG_4		0x00 /* 0.4 %RH */
-#define HTS221_HUMIDITY_AVG_8		0x01 /* 0.3 %RH */
-#define HTS221_HUMIDITY_AVG_16		0x02 /* 0.2 %RH */
-#define HTS221_HUMIDITY_AVG_32		0x03 /* 0.15 %RH */
-#define HTS221_HUMIDITY_AVG_64		0x04 /* 0.1 %RH */
-#define HTS221_HUMIDITY_AVG_128		0x05 /* 0.07 %RH */
-#define HTS221_HUMIDITY_AVG_256		0x06 /* 0.05 %RH */
-#define HTS221_HUMIDITY_AVG_512		0x07 /* 0.03 %RH */
-
-#define HTS221_TEMP_AVG_2		0x00 /* 0.08 degC */
-#define HTS221_TEMP_AVG_4		0x08 /* 0.05 degC */
-#define HTS221_TEMP_AVG_8		0x10 /* 0.04 degC */
-#define HTS221_TEMP_AVG_16		0x18 /* 0.03 degC */
-#define HTS221_TEMP_AVG_32		0x20 /* 0.02 degC */
-#define HTS221_TEMP_AVG_64		0x28 /* 0.015 degC */
-#define HTS221_TEMP_AVG_128		0x30 /* 0.01 degC */
-#define HTS221_TEMP_AVG_256		0x38 /* 0.007 degC */
 
 /* calibration registers */
 #define HTS221_REG_0RH_CAL_X_H		0x36
@@ -90,28 +72,28 @@ static const struct hts221_avg hts221_avg_list[] = {
 		.addr = HTS221_REG_AVG_ADDR,
 		.mask = HTS221_HUMIDITY_AVG_MASK,
 		.avg_avl = {
-			{ 4, HTS221_HUMIDITY_AVG_4 },
-			{ 8, HTS221_HUMIDITY_AVG_8 },
-			{ 16, HTS221_HUMIDITY_AVG_16 },
-			{ 32, HTS221_HUMIDITY_AVG_32 },
-			{ 64, HTS221_HUMIDITY_AVG_64 },
-			{ 128, HTS221_HUMIDITY_AVG_128 },
-			{ 256, HTS221_HUMIDITY_AVG_256 },
-			{ 512, HTS221_HUMIDITY_AVG_512 },
+			{   4, 0x0 }, /* 0.4 %RH */
+			{   8, 0x1 }, /* 0.3 %RH */
+			{  16, 0x2 }, /* 0.2 %RH */
+			{  32, 0x3 }, /* 0.15 %RH */
+			{  64, 0x4 }, /* 0.1 %RH */
+			{ 128, 0x5 }, /* 0.07 %RH */
+			{ 256, 0x6 }, /* 0.05 %RH */
+			{ 512, 0x7 }, /* 0.03 %RH */
 		},
 	},
 	{
 		.addr = HTS221_REG_AVG_ADDR,
 		.mask = HTS221_TEMP_AVG_MASK,
 		.avg_avl = {
-			{ 2, HTS221_TEMP_AVG_2 },
-			{ 4, HTS221_TEMP_AVG_4 },
-			{ 8, HTS221_TEMP_AVG_8 },
-			{ 16, HTS221_TEMP_AVG_16 },
-			{ 32, HTS221_TEMP_AVG_32 },
-			{ 64, HTS221_TEMP_AVG_64 },
-			{ 128, HTS221_TEMP_AVG_128 },
-			{ 256, HTS221_TEMP_AVG_256 },
+			{   2, 0x0 }, /* 0.08 degC */
+			{   4, 0x1 }, /* 0.05 degC */
+			{   8, 0x2 }, /* 0.04 degC */
+			{  16, 0x3 }, /* 0.03 degC */
+			{  32, 0x4 }, /* 0.02 degC */
+			{  64, 0x5 }, /* 0.015 degC */
+			{ 128, 0x6 }, /* 0.01 degC */
+			{ 256, 0x7 }, /* 0.007 degC */
 		},
 	},
 };
@@ -166,7 +148,7 @@ static int hts221_write_with_mask(struct hts221_hw *hw, u8 addr, u8 mask,
 		goto unlock;
 	}
 
-	data = (data & ~mask) | (val & mask);
+	data = (data & ~mask) | ((val << __ffs(mask)) & mask);
 
 	err = hw->tf->write(hw->dev, addr, sizeof(data), &data);
 	if (err < 0)
@@ -201,11 +183,10 @@ static int hts221_check_whoami(struct hts221_hw *hw)
 
 int hts221_config_drdy(struct hts221_hw *hw, bool enable)
 {
-	u8 val = enable ? BIT(2) : 0;
 	int err;
 
 	err = hts221_write_with_mask(hw, HTS221_REG_CNTRL3_ADDR,
-				     HTS221_DRDY_MASK, val);
+				     HTS221_DRDY_MASK, enable);
 
 	return err < 0 ? err : 0;
 }
@@ -213,7 +194,6 @@ int hts221_config_drdy(struct hts221_hw *hw, bool enable)
 static int hts221_update_odr(struct hts221_hw *hw, u8 odr)
 {
 	int i, err;
-	u8 val;
 
 	for (i = 0; i < ARRAY_SIZE(hts221_odr_table); i++)
 		if (hts221_odr_table[i].hz == odr)
@@ -222,9 +202,19 @@ static int hts221_update_odr(struct hts221_hw *hw, u8 odr)
 	if (i == ARRAY_SIZE(hts221_odr_table))
 		return -EINVAL;
 
-	val = HTS221_ENABLE_SENSOR | HTS221_BDU_MASK | hts221_odr_table[i].val;
+	/* enable Block Data Update */
+	err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
+				     HTS221_BDU_MASK, 1);
+	if (err < 0)
+		return err;
+
+	err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
+				     HTS221_ODR_MASK, hts221_odr_table[i].val);
+	if (err < 0)
+		return err;
+
 	err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
-				     HTS221_ODR_MASK, val);
+				     HTS221_ENABLE_MASK, 1);
 	if (err < 0)
 		return err;
 
-- 
2.13.1

Re: [PATCH 1/9] iio: humidity: hts221: refactor write_with_mask code

[Jonathan Cameron]

On Sun,  9 Jul 2017 18:56:56 +0200
Lorenzo Bianconi <lorenzo.bianconi83-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> Move bit-shift in hts221_write_with_mask() instead of coding
> the shift depth in the configured value. That change will be necessary
> to fix an issue in device power-down procedure
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
A could of questions inline.  

Jonathan
> ---
>  drivers/iio/humidity/hts221_core.c | 74 +++++++++++++++++---------------------
>  1 file changed, 32 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
> index a56da3999e00..f5181e4e1eff 100644
> --- a/drivers/iio/humidity/hts221_core.c
> +++ b/drivers/iio/humidity/hts221_core.c
> @@ -32,30 +32,12 @@
>  #define HTS221_HUMIDITY_AVG_MASK	0x07
>  #define HTS221_TEMP_AVG_MASK		0x38
>  
> -#define HTS221_ODR_MASK			0x87
> +#define HTS221_ODR_MASK			0x03
>  #define HTS221_BDU_MASK			BIT(2)
> +#define HTS221_ENABLE_MASK		BIT(7)
>  
>  #define HTS221_DRDY_MASK		BIT(2)
>  
> -#define HTS221_ENABLE_SENSOR		BIT(7)
> -
> -#define HTS221_HUMIDITY_AVG_4		0x00 /* 0.4 %RH */
> -#define HTS221_HUMIDITY_AVG_8		0x01 /* 0.3 %RH */
> -#define HTS221_HUMIDITY_AVG_16		0x02 /* 0.2 %RH */
> -#define HTS221_HUMIDITY_AVG_32		0x03 /* 0.15 %RH */
> -#define HTS221_HUMIDITY_AVG_64		0x04 /* 0.1 %RH */
> -#define HTS221_HUMIDITY_AVG_128		0x05 /* 0.07 %RH */
> -#define HTS221_HUMIDITY_AVG_256		0x06 /* 0.05 %RH */
> -#define HTS221_HUMIDITY_AVG_512		0x07 /* 0.03 %RH */
> -
> -#define HTS221_TEMP_AVG_2		0x00 /* 0.08 degC */
> -#define HTS221_TEMP_AVG_4		0x08 /* 0.05 degC */
> -#define HTS221_TEMP_AVG_8		0x10 /* 0.04 degC */
> -#define HTS221_TEMP_AVG_16		0x18 /* 0.03 degC */
> -#define HTS221_TEMP_AVG_32		0x20 /* 0.02 degC */
> -#define HTS221_TEMP_AVG_64		0x28 /* 0.015 degC */
> -#define HTS221_TEMP_AVG_128		0x30 /* 0.01 degC */
> -#define HTS221_TEMP_AVG_256		0x38 /* 0.007 degC */
>  
>  /* calibration registers */
>  #define HTS221_REG_0RH_CAL_X_H		0x36
> @@ -90,28 +72,28 @@ static const struct hts221_avg hts221_avg_list[] = {
>  		.addr = HTS221_REG_AVG_ADDR,
>  		.mask = HTS221_HUMIDITY_AVG_MASK,
>  		.avg_avl = {
> -			{ 4, HTS221_HUMIDITY_AVG_4 },
> -			{ 8, HTS221_HUMIDITY_AVG_8 },
> -			{ 16, HTS221_HUMIDITY_AVG_16 },
> -			{ 32, HTS221_HUMIDITY_AVG_32 },
> -			{ 64, HTS221_HUMIDITY_AVG_64 },
> -			{ 128, HTS221_HUMIDITY_AVG_128 },
> -			{ 256, HTS221_HUMIDITY_AVG_256 },
> -			{ 512, HTS221_HUMIDITY_AVG_512 },
> +			{   4, 0x0 }, /* 0.4 %RH */
> +			{   8, 0x1 }, /* 0.3 %RH */
> +			{  16, 0x2 }, /* 0.2 %RH */
> +			{  32, 0x3 }, /* 0.15 %RH */
> +			{  64, 0x4 }, /* 0.1 %RH */
> +			{ 128, 0x5 }, /* 0.07 %RH */
> +			{ 256, 0x6 }, /* 0.05 %RH */
> +			{ 512, 0x7 }, /* 0.03 %RH */
>  		},
>  	},
>  	{
>  		.addr = HTS221_REG_AVG_ADDR,
>  		.mask = HTS221_TEMP_AVG_MASK,
>  		.avg_avl = {
> -			{ 2, HTS221_TEMP_AVG_2 },
> -			{ 4, HTS221_TEMP_AVG_4 },
> -			{ 8, HTS221_TEMP_AVG_8 },
> -			{ 16, HTS221_TEMP_AVG_16 },
> -			{ 32, HTS221_TEMP_AVG_32 },
> -			{ 64, HTS221_TEMP_AVG_64 },
> -			{ 128, HTS221_TEMP_AVG_128 },
> -			{ 256, HTS221_TEMP_AVG_256 },
> +			{   2, 0x0 }, /* 0.08 degC */
> +			{   4, 0x1 }, /* 0.05 degC */
> +			{   8, 0x2 }, /* 0.04 degC */
> +			{  16, 0x3 }, /* 0.03 degC */
> +			{  32, 0x4 }, /* 0.02 degC */
> +			{  64, 0x5 }, /* 0.015 degC */
> +			{ 128, 0x6 }, /* 0.01 degC */
> +			{ 256, 0x7 }, /* 0.007 degC */
Could we potentially use the index directly here rather than
having to explicitly store it?  Would be more elegant perhaps..
>  		},
>  	},
>  };
> @@ -166,7 +148,7 @@ static int hts221_write_with_mask(struct hts221_hw *hw, u8 addr, u8 mask,
>  		goto unlock;
>  	}
>  
> -	data = (data & ~mask) | (val & mask);
> +	data = (data & ~mask) | ((val << __ffs(mask)) & mask);
>  
>  	err = hw->tf->write(hw->dev, addr, sizeof(data), &data);
>  	if (err < 0)
> @@ -201,11 +183,10 @@ static int hts221_check_whoami(struct hts221_hw *hw)
>  
>  int hts221_config_drdy(struct hts221_hw *hw, bool enable)
>  {
> -	u8 val = enable ? BIT(2) : 0;
>  	int err;
>  
>  	err = hts221_write_with_mask(hw, HTS221_REG_CNTRL3_ADDR,
> -				     HTS221_DRDY_MASK, val);
> +				     HTS221_DRDY_MASK, enable);
>  
>  	return err < 0 ? err : 0;
>  }
> @@ -213,7 +194,6 @@ int hts221_config_drdy(struct hts221_hw *hw, bool enable)
>  static int hts221_update_odr(struct hts221_hw *hw, u8 odr)
>  {
>  	int i, err;
> -	u8 val;
>  
>  	for (i = 0; i < ARRAY_SIZE(hts221_odr_table); i++)
>  		if (hts221_odr_table[i].hz == odr)
> @@ -222,9 +202,19 @@ static int hts221_update_odr(struct hts221_hw *hw, u8 odr)
>  	if (i == ARRAY_SIZE(hts221_odr_table))
>  		return -EINVAL;
>  
> -	val = HTS221_ENABLE_SENSOR | HTS221_BDU_MASK | hts221_odr_table[i].val;
> +	/* enable Block Data Update */
> +	err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
> +				     HTS221_BDU_MASK, 1);
> +	if (err < 0)
> +		return err;
> +
> +	err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
> +				     HTS221_ODR_MASK, hts221_odr_table[i].val);
> +	if (err < 0)
> +		return err;
> +
>  	err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
> -				     HTS221_ODR_MASK, val);
This original code looks like a bug given the odr mask should only
cover the odr bits, not the enable and bdu.

Am I missing something?

Taking one write and changing it to 3 isn't that nice, but I guess
we aren't in a fast path here so fair enough given the cleaner
resulting code.
> +				     HTS221_ENABLE_MASK, 1);
>  	if (err < 0)
>  		return err;
>  

Re: [PATCH 1/9] iio: humidity: hts221: refactor write_with_mask code

[Lorenzo Bianconi]

> On Sun,  9 Jul 2017 18:56:56 +0200
> Lorenzo Bianconi <lorenzo.bianconi83-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> Move bit-shift in hts221_write_with_mask() instead of coding
>> the shift depth in the configured value. That change will be necessary
>> to fix an issue in device power-down procedure
>>
>> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
> A could of questions inline.

Hi Jonathan,

Thanks for quick review :)

>
> Jonathan
>> ---
>>  drivers/iio/humidity/hts221_core.c | 74 +++++++++++++++++---------------------
>>  1 file changed, 32 insertions(+), 42 deletions(-)
>>
>> diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
>> index a56da3999e00..f5181e4e1eff 100644
>> --- a/drivers/iio/humidity/hts221_core.c
>> +++ b/drivers/iio/humidity/hts221_core.c
>> @@ -32,30 +32,12 @@
>>  #define HTS221_HUMIDITY_AVG_MASK     0x07
>>  #define HTS221_TEMP_AVG_MASK         0x38
>>
>> -#define HTS221_ODR_MASK                      0x87
>> +#define HTS221_ODR_MASK                      0x03
>>  #define HTS221_BDU_MASK                      BIT(2)
>> +#define HTS221_ENABLE_MASK           BIT(7)
>>
>>  #define HTS221_DRDY_MASK             BIT(2)
>>
>> -#define HTS221_ENABLE_SENSOR         BIT(7)
>> -
>> -#define HTS221_HUMIDITY_AVG_4                0x00 /* 0.4 %RH */
>> -#define HTS221_HUMIDITY_AVG_8                0x01 /* 0.3 %RH */
>> -#define HTS221_HUMIDITY_AVG_16               0x02 /* 0.2 %RH */
>> -#define HTS221_HUMIDITY_AVG_32               0x03 /* 0.15 %RH */
>> -#define HTS221_HUMIDITY_AVG_64               0x04 /* 0.1 %RH */
>> -#define HTS221_HUMIDITY_AVG_128              0x05 /* 0.07 %RH */
>> -#define HTS221_HUMIDITY_AVG_256              0x06 /* 0.05 %RH */
>> -#define HTS221_HUMIDITY_AVG_512              0x07 /* 0.03 %RH */
>> -
>> -#define HTS221_TEMP_AVG_2            0x00 /* 0.08 degC */
>> -#define HTS221_TEMP_AVG_4            0x08 /* 0.05 degC */
>> -#define HTS221_TEMP_AVG_8            0x10 /* 0.04 degC */
>> -#define HTS221_TEMP_AVG_16           0x18 /* 0.03 degC */
>> -#define HTS221_TEMP_AVG_32           0x20 /* 0.02 degC */
>> -#define HTS221_TEMP_AVG_64           0x28 /* 0.015 degC */
>> -#define HTS221_TEMP_AVG_128          0x30 /* 0.01 degC */
>> -#define HTS221_TEMP_AVG_256          0x38 /* 0.007 degC */
>>
>>  /* calibration registers */
>>  #define HTS221_REG_0RH_CAL_X_H               0x36
>> @@ -90,28 +72,28 @@ static const struct hts221_avg hts221_avg_list[] = {
>>               .addr = HTS221_REG_AVG_ADDR,
>>               .mask = HTS221_HUMIDITY_AVG_MASK,
>>               .avg_avl = {
>> -                     { 4, HTS221_HUMIDITY_AVG_4 },
>> -                     { 8, HTS221_HUMIDITY_AVG_8 },
>> -                     { 16, HTS221_HUMIDITY_AVG_16 },
>> -                     { 32, HTS221_HUMIDITY_AVG_32 },
>> -                     { 64, HTS221_HUMIDITY_AVG_64 },
>> -                     { 128, HTS221_HUMIDITY_AVG_128 },
>> -                     { 256, HTS221_HUMIDITY_AVG_256 },
>> -                     { 512, HTS221_HUMIDITY_AVG_512 },
>> +                     {   4, 0x0 }, /* 0.4 %RH */
>> +                     {   8, 0x1 }, /* 0.3 %RH */
>> +                     {  16, 0x2 }, /* 0.2 %RH */
>> +                     {  32, 0x3 }, /* 0.15 %RH */
>> +                     {  64, 0x4 }, /* 0.1 %RH */
>> +                     { 128, 0x5 }, /* 0.07 %RH */
>> +                     { 256, 0x6 }, /* 0.05 %RH */
>> +                     { 512, 0x7 }, /* 0.03 %RH */
>>               },
>>       },
>>       {
>>               .addr = HTS221_REG_AVG_ADDR,
>>               .mask = HTS221_TEMP_AVG_MASK,
>>               .avg_avl = {
>> -                     { 2, HTS221_TEMP_AVG_2 },
>> -                     { 4, HTS221_TEMP_AVG_4 },
>> -                     { 8, HTS221_TEMP_AVG_8 },
>> -                     { 16, HTS221_TEMP_AVG_16 },
>> -                     { 32, HTS221_TEMP_AVG_32 },
>> -                     { 64, HTS221_TEMP_AVG_64 },
>> -                     { 128, HTS221_TEMP_AVG_128 },
>> -                     { 256, HTS221_TEMP_AVG_256 },
>> +                     {   2, 0x0 }, /* 0.08 degC */
>> +                     {   4, 0x1 }, /* 0.05 degC */
>> +                     {   8, 0x2 }, /* 0.04 degC */
>> +                     {  16, 0x3 }, /* 0.03 degC */
>> +                     {  32, 0x4 }, /* 0.02 degC */
>> +                     {  64, 0x5 }, /* 0.015 degC */
>> +                     { 128, 0x6 }, /* 0.01 degC */
>> +                     { 256, 0x7 }, /* 0.007 degC */
> Could we potentially use the index directly here rather than
> having to explicitly store it?  Would be more elegant perhaps..

Ack, will do in v2

>>               },
>>       },
>>  };
>> @@ -166,7 +148,7 @@ static int hts221_write_with_mask(struct hts221_hw *hw, u8 addr, u8 mask,
>>               goto unlock;
>>       }
>>
>> -     data = (data & ~mask) | (val & mask);
>> +     data = (data & ~mask) | ((val << __ffs(mask)) & mask);
>>
>>       err = hw->tf->write(hw->dev, addr, sizeof(data), &data);
>>       if (err < 0)
>> @@ -201,11 +183,10 @@ static int hts221_check_whoami(struct hts221_hw *hw)
>>
>>  int hts221_config_drdy(struct hts221_hw *hw, bool enable)
>>  {
>> -     u8 val = enable ? BIT(2) : 0;
>>       int err;
>>
>>       err = hts221_write_with_mask(hw, HTS221_REG_CNTRL3_ADDR,
>> -                                  HTS221_DRDY_MASK, val);
>> +                                  HTS221_DRDY_MASK, enable);
>>
>>       return err < 0 ? err : 0;
>>  }
>> @@ -213,7 +194,6 @@ int hts221_config_drdy(struct hts221_hw *hw, bool enable)
>>  static int hts221_update_odr(struct hts221_hw *hw, u8 odr)
>>  {
>>       int i, err;
>> -     u8 val;
>>
>>       for (i = 0; i < ARRAY_SIZE(hts221_odr_table); i++)
>>               if (hts221_odr_table[i].hz == odr)
>> @@ -222,9 +202,19 @@ static int hts221_update_odr(struct hts221_hw *hw, u8 odr)
>>       if (i == ARRAY_SIZE(hts221_odr_table))
>>               return -EINVAL;
>>
>> -     val = HTS221_ENABLE_SENSOR | HTS221_BDU_MASK | hts221_odr_table[i].val;
>> +     /* enable Block Data Update */
>> +     err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
>> +                                  HTS221_BDU_MASK, 1);
>> +     if (err < 0)
>> +             return err;
>> +
>> +     err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
>> +                                  HTS221_ODR_MASK, hts221_odr_table[i].val);
>> +     if (err < 0)
>> +             return err;
>> +
>>       err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
>> -                                  HTS221_ODR_MASK, val);
> This original code looks like a bug given the odr mask should only
> cover the odr bits, not the enable and bdu.
>
> Am I missing something?

Base on hts221_write_with_mask() code the original mask covers odr,
bdu and enable fields in CNTRL1 register.

>
> Taking one write and changing it to 3 isn't that nice, but I guess
> we aren't in a fast path here so fair enough given the cleaner
> resulting code.
>> +                                  HTS221_ENABLE_MASK, 1);
>>       if (err < 0)
>>               return err;
>>
>

Regards,
Lorenzo



-- 
UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch;
unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp;
umount; make clean; sleep

Re: [PATCH 1/9] iio: humidity: hts221: refactor write_with_mask code

[Jonathan Cameron]

On Sun, 9 Jul 2017 23:18:47 +0200
Lorenzo Bianconi <lorenzo.bianconi83-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> > On Sun,  9 Jul 2017 18:56:56 +0200
> > Lorenzo Bianconi <lorenzo.bianconi83-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >  
> >> Move bit-shift in hts221_write_with_mask() instead of coding
> >> the shift depth in the configured value. That change will be necessary
> >> to fix an issue in device power-down procedure
> >>
> >> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>  
> > A could of questions inline.  
> 
> Hi Jonathan,
> 
> Thanks for quick review :)
> 
> >
> > Jonathan  
> >> ---
> >>  drivers/iio/humidity/hts221_core.c | 74 +++++++++++++++++---------------------
> >>  1 file changed, 32 insertions(+), 42 deletions(-)
> >>
> >> diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
> >> index a56da3999e00..f5181e4e1eff 100644
> >> --- a/drivers/iio/humidity/hts221_core.c
> >> +++ b/drivers/iio/humidity/hts221_core.c
> >> @@ -32,30 +32,12 @@
> >>  #define HTS221_HUMIDITY_AVG_MASK     0x07
> >>  #define HTS221_TEMP_AVG_MASK         0x38
> >>
> >> -#define HTS221_ODR_MASK                      0x87
> >> +#define HTS221_ODR_MASK                      0x03
> >>  #define HTS221_BDU_MASK                      BIT(2)
> >> +#define HTS221_ENABLE_MASK           BIT(7)
> >>
> >>  #define HTS221_DRDY_MASK             BIT(2)
> >>
> >> -#define HTS221_ENABLE_SENSOR         BIT(7)
> >> -
> >> -#define HTS221_HUMIDITY_AVG_4                0x00 /* 0.4 %RH */
> >> -#define HTS221_HUMIDITY_AVG_8                0x01 /* 0.3 %RH */
> >> -#define HTS221_HUMIDITY_AVG_16               0x02 /* 0.2 %RH */
> >> -#define HTS221_HUMIDITY_AVG_32               0x03 /* 0.15 %RH */
> >> -#define HTS221_HUMIDITY_AVG_64               0x04 /* 0.1 %RH */
> >> -#define HTS221_HUMIDITY_AVG_128              0x05 /* 0.07 %RH */
> >> -#define HTS221_HUMIDITY_AVG_256              0x06 /* 0.05 %RH */
> >> -#define HTS221_HUMIDITY_AVG_512              0x07 /* 0.03 %RH */
> >> -
> >> -#define HTS221_TEMP_AVG_2            0x00 /* 0.08 degC */
> >> -#define HTS221_TEMP_AVG_4            0x08 /* 0.05 degC */
> >> -#define HTS221_TEMP_AVG_8            0x10 /* 0.04 degC */
> >> -#define HTS221_TEMP_AVG_16           0x18 /* 0.03 degC */
> >> -#define HTS221_TEMP_AVG_32           0x20 /* 0.02 degC */
> >> -#define HTS221_TEMP_AVG_64           0x28 /* 0.015 degC */
> >> -#define HTS221_TEMP_AVG_128          0x30 /* 0.01 degC */
> >> -#define HTS221_TEMP_AVG_256          0x38 /* 0.007 degC */
> >>
> >>  /* calibration registers */
> >>  #define HTS221_REG_0RH_CAL_X_H               0x36
> >> @@ -90,28 +72,28 @@ static const struct hts221_avg hts221_avg_list[] = {
> >>               .addr = HTS221_REG_AVG_ADDR,
> >>               .mask = HTS221_HUMIDITY_AVG_MASK,
> >>               .avg_avl = {
> >> -                     { 4, HTS221_HUMIDITY_AVG_4 },
> >> -                     { 8, HTS221_HUMIDITY_AVG_8 },
> >> -                     { 16, HTS221_HUMIDITY_AVG_16 },
> >> -                     { 32, HTS221_HUMIDITY_AVG_32 },
> >> -                     { 64, HTS221_HUMIDITY_AVG_64 },
> >> -                     { 128, HTS221_HUMIDITY_AVG_128 },
> >> -                     { 256, HTS221_HUMIDITY_AVG_256 },
> >> -                     { 512, HTS221_HUMIDITY_AVG_512 },
> >> +                     {   4, 0x0 }, /* 0.4 %RH */
> >> +                     {   8, 0x1 }, /* 0.3 %RH */
> >> +                     {  16, 0x2 }, /* 0.2 %RH */
> >> +                     {  32, 0x3 }, /* 0.15 %RH */
> >> +                     {  64, 0x4 }, /* 0.1 %RH */
> >> +                     { 128, 0x5 }, /* 0.07 %RH */
> >> +                     { 256, 0x6 }, /* 0.05 %RH */
> >> +                     { 512, 0x7 }, /* 0.03 %RH */
> >>               },
> >>       },
> >>       {
> >>               .addr = HTS221_REG_AVG_ADDR,
> >>               .mask = HTS221_TEMP_AVG_MASK,
> >>               .avg_avl = {
> >> -                     { 2, HTS221_TEMP_AVG_2 },
> >> -                     { 4, HTS221_TEMP_AVG_4 },
> >> -                     { 8, HTS221_TEMP_AVG_8 },
> >> -                     { 16, HTS221_TEMP_AVG_16 },
> >> -                     { 32, HTS221_TEMP_AVG_32 },
> >> -                     { 64, HTS221_TEMP_AVG_64 },
> >> -                     { 128, HTS221_TEMP_AVG_128 },
> >> -                     { 256, HTS221_TEMP_AVG_256 },
> >> +                     {   2, 0x0 }, /* 0.08 degC */
> >> +                     {   4, 0x1 }, /* 0.05 degC */
> >> +                     {   8, 0x2 }, /* 0.04 degC */
> >> +                     {  16, 0x3 }, /* 0.03 degC */
> >> +                     {  32, 0x4 }, /* 0.02 degC */
> >> +                     {  64, 0x5 }, /* 0.015 degC */
> >> +                     { 128, 0x6 }, /* 0.01 degC */
> >> +                     { 256, 0x7 }, /* 0.007 degC */  
> > Could we potentially use the index directly here rather than
> > having to explicitly store it?  Would be more elegant perhaps..  
> 
> Ack, will do in v2
> 
> >>               },
> >>       },
> >>  };
> >> @@ -166,7 +148,7 @@ static int hts221_write_with_mask(struct hts221_hw *hw, u8 addr, u8 mask,
> >>               goto unlock;
> >>       }
> >>
> >> -     data = (data & ~mask) | (val & mask);
> >> +     data = (data & ~mask) | ((val << __ffs(mask)) & mask);
> >>
> >>       err = hw->tf->write(hw->dev, addr, sizeof(data), &data);
> >>       if (err < 0)
> >> @@ -201,11 +183,10 @@ static int hts221_check_whoami(struct hts221_hw *hw)
> >>
> >>  int hts221_config_drdy(struct hts221_hw *hw, bool enable)
> >>  {
> >> -     u8 val = enable ? BIT(2) : 0;
> >>       int err;
> >>
> >>       err = hts221_write_with_mask(hw, HTS221_REG_CNTRL3_ADDR,
> >> -                                  HTS221_DRDY_MASK, val);
> >> +                                  HTS221_DRDY_MASK, enable);
> >>
> >>       return err < 0 ? err : 0;
> >>  }
> >> @@ -213,7 +194,6 @@ int hts221_config_drdy(struct hts221_hw *hw, bool enable)
> >>  static int hts221_update_odr(struct hts221_hw *hw, u8 odr)
> >>  {
> >>       int i, err;
> >> -     u8 val;
> >>
> >>       for (i = 0; i < ARRAY_SIZE(hts221_odr_table); i++)
> >>               if (hts221_odr_table[i].hz == odr)
> >> @@ -222,9 +202,19 @@ static int hts221_update_odr(struct hts221_hw *hw, u8 odr)
> >>       if (i == ARRAY_SIZE(hts221_odr_table))
> >>               return -EINVAL;
> >>
> >> -     val = HTS221_ENABLE_SENSOR | HTS221_BDU_MASK | hts221_odr_table[i].val;
> >> +     /* enable Block Data Update */
> >> +     err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
> >> +                                  HTS221_BDU_MASK, 1);
> >> +     if (err < 0)
> >> +             return err;
> >> +
> >> +     err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
> >> +                                  HTS221_ODR_MASK, hts221_odr_table[i].val);
> >> +     if (err < 0)
> >> +             return err;
> >> +
> >>       err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
> >> -                                  HTS221_ODR_MASK, val);  
> > This original code looks like a bug given the odr mask should only
> > cover the odr bits, not the enable and bdu.
> >
> > Am I missing something?  
> 
> Base on hts221_write_with_mask() code the original mask covers odr,
> bdu and enable fields in CNTRL1 register.
Ah.  That makes sense even if it's rather less elegant than the new
code!

Thanks for clearing that up.

Jonathan
> 
> >
> > Taking one write and changing it to 3 isn't that nice, but I guess
> > we aren't in a fast path here so fair enough given the cleaner
> > resulting code.  
> >> +                                  HTS221_ENABLE_MASK, 1);
> >>       if (err < 0)
> >>               return err;
> >>  
> >  
> 
> Regards,
> Lorenzo
> 
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/9] iio: humidity: hts221: move BDU configuration in probe routine

[Lorenzo Bianconi]

Enable Block Data Update in hts221_probe() in order to avoid to reconfigure
it every time the sensor is enabled

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
---
 drivers/iio/humidity/hts221_core.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
index f5181e4e1eff..9546b0ea6283 100644
--- a/drivers/iio/humidity/hts221_core.c
+++ b/drivers/iio/humidity/hts221_core.c
@@ -202,12 +202,6 @@ static int hts221_update_odr(struct hts221_hw *hw, u8 odr)
 	if (i == ARRAY_SIZE(hts221_odr_table))
 		return -EINVAL;
 
-	/* enable Block Data Update */
-	err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
-				     HTS221_BDU_MASK, 1);
-	if (err < 0)
-		return err;
-
 	err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
 				     HTS221_ODR_MASK, hts221_odr_table[i].val);
 	if (err < 0)
@@ -644,6 +638,12 @@ int hts221_probe(struct iio_dev *iio_dev)
 	iio_dev->name = HTS221_DEV_NAME;
 	iio_dev->info = &hts221_info;
 
+	/* enable Block Data Update */
+	err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
+				     HTS221_BDU_MASK, 1);
+	if (err < 0)
+		return err;
+
 	/* configure humidity sensor */
 	err = hts221_parse_rh_caldata(hw);
 	if (err < 0) {
-- 
2.13.1

Re: [PATCH 2/9] iio: humidity: hts221: move BDU configuration in probe routine

[Jonathan Cameron]

On Sun,  9 Jul 2017 18:56:57 +0200
Lorenzo Bianconi <lorenzo.bianconi83-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> Enable Block Data Update in hts221_probe() in order to avoid to reconfigure
> it every time the sensor is enabled
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
Well there goes part of my concern about extra writes.

Patch looks good

Jonathan
> ---
>  drivers/iio/humidity/hts221_core.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
> index f5181e4e1eff..9546b0ea6283 100644
> --- a/drivers/iio/humidity/hts221_core.c
> +++ b/drivers/iio/humidity/hts221_core.c
> @@ -202,12 +202,6 @@ static int hts221_update_odr(struct hts221_hw *hw, u8 odr)
>  	if (i == ARRAY_SIZE(hts221_odr_table))
>  		return -EINVAL;
>  
> -	/* enable Block Data Update */
> -	err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
> -				     HTS221_BDU_MASK, 1);
> -	if (err < 0)
> -		return err;
> -
>  	err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
>  				     HTS221_ODR_MASK, hts221_odr_table[i].val);
>  	if (err < 0)
> @@ -644,6 +638,12 @@ int hts221_probe(struct iio_dev *iio_dev)
>  	iio_dev->name = HTS221_DEV_NAME;
>  	iio_dev->info = &hts221_info;
>  
> +	/* enable Block Data Update */
> +	err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
> +				     HTS221_BDU_MASK, 1);
> +	if (err < 0)
> +		return err;
> +
>  	/* configure humidity sensor */
>  	err = hts221_parse_rh_caldata(hw);
>  	if (err < 0) {

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/9] iio: humidity: hts221: do not overwrite reserved data during power-down

[Lorenzo Bianconi]

Fixes: e4a70e3e7d84 (iio: humidity: add support to hts221 rh/temp device)
Fixes: b7079eeac5da (iio: humidity: hts221: add power management support)
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
---
 drivers/iio/humidity/hts221_core.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
index 9546b0ea6283..b36734b8070e 100644
--- a/drivers/iio/humidity/hts221_core.c
+++ b/drivers/iio/humidity/hts221_core.c
@@ -305,11 +305,10 @@ int hts221_power_on(struct hts221_hw *hw)
 
 int hts221_power_off(struct hts221_hw *hw)
 {
-	__le16 data = 0;
 	int err;
 
-	err = hw->tf->write(hw->dev, HTS221_REG_CNTRL1_ADDR, sizeof(data),
-			    (u8 *)&data);
+	err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
+				     HTS221_ENABLE_MASK, false);
 	if (err < 0)
 		return err;
 
@@ -692,11 +691,10 @@ static int __maybe_unused hts221_suspend(struct device *dev)
 {
 	struct iio_dev *iio_dev = dev_get_drvdata(dev);
 	struct hts221_hw *hw = iio_priv(iio_dev);
-	__le16 data = 0;
 	int err;
 
-	err = hw->tf->write(hw->dev, HTS221_REG_CNTRL1_ADDR, sizeof(data),
-			    (u8 *)&data);
+	err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
+				     HTS221_ENABLE_MASK, false);
 
 	return err < 0 ? err : 0;
 }
-- 
2.13.1

Re: [PATCH 3/9] iio: humidity: hts221: do not overwrite reserved data during power-down

[Jonathan Cameron]

On Sun,  9 Jul 2017 18:56:58 +0200
Lorenzo Bianconi <lorenzo.bianconi83-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> Fixes: e4a70e3e7d84 (iio: humidity: add support to hts221 rh/temp device)
> Fixes: b7079eeac5da (iio: humidity: hts221: add power management support)
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
This needs more description.  What is the effect of this?

Which bits of data are we writing that we shouldn't be touching?


> ---
>  drivers/iio/humidity/hts221_core.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
> index 9546b0ea6283..b36734b8070e 100644
> --- a/drivers/iio/humidity/hts221_core.c
> +++ b/drivers/iio/humidity/hts221_core.c
> @@ -305,11 +305,10 @@ int hts221_power_on(struct hts221_hw *hw)
>  
>  int hts221_power_off(struct hts221_hw *hw)
>  {
> -	__le16 data = 0;
>  	int err;
>  
> -	err = hw->tf->write(hw->dev, HTS221_REG_CNTRL1_ADDR, sizeof(data),
> -			    (u8 *)&data);
> +	err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
> +				     HTS221_ENABLE_MASK, false);
>  	if (err < 0)
>  		return err;
>  
> @@ -692,11 +691,10 @@ static int __maybe_unused hts221_suspend(struct device *dev)
>  {
>  	struct iio_dev *iio_dev = dev_get_drvdata(dev);
>  	struct hts221_hw *hw = iio_priv(iio_dev);
> -	__le16 data = 0;
>  	int err;
>  
> -	err = hw->tf->write(hw->dev, HTS221_REG_CNTRL1_ADDR, sizeof(data),
> -			    (u8 *)&data);
> +	err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
> +				     HTS221_ENABLE_MASK, false);
>  
>  	return err < 0 ? err : 0;
>  }

Re: [PATCH 3/9] iio: humidity: hts221: do not overwrite reserved data during power-down

[Lorenzo Bianconi]

> On Sun,  9 Jul 2017 18:56:58 +0200
> Lorenzo Bianconi <lorenzo.bianconi83-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> Fixes: e4a70e3e7d84 (iio: humidity: add support to hts221 rh/temp device)
>> Fixes: b7079eeac5da (iio: humidity: hts221: add power management support)
>> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
> This needs more description.  What is the effect of this?
>
> Which bits of data are we writing that we shouldn't be touching?

GENMASK(6, 3) in CTRL_REG1 and GENMASK(6, 2) in CTRL_REG2 are marked
reserved so it is better to not changed the original value (not
declared in the datasheet).
I will add a comment in v2

Regards,
Lorenzo

>
>
>> ---
>>  drivers/iio/humidity/hts221_core.c | 10 ++++------
>>  1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
>> index 9546b0ea6283..b36734b8070e 100644
>> --- a/drivers/iio/humidity/hts221_core.c
>> +++ b/drivers/iio/humidity/hts221_core.c
>> @@ -305,11 +305,10 @@ int hts221_power_on(struct hts221_hw *hw)
>>
>>  int hts221_power_off(struct hts221_hw *hw)
>>  {
>> -     __le16 data = 0;
>>       int err;
>>
>> -     err = hw->tf->write(hw->dev, HTS221_REG_CNTRL1_ADDR, sizeof(data),
>> -                         (u8 *)&data);
>> +     err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
>> +                                  HTS221_ENABLE_MASK, false);
>>       if (err < 0)
>>               return err;
>>
>> @@ -692,11 +691,10 @@ static int __maybe_unused hts221_suspend(struct device *dev)
>>  {
>>       struct iio_dev *iio_dev = dev_get_drvdata(dev);
>>       struct hts221_hw *hw = iio_priv(iio_dev);
>> -     __le16 data = 0;
>>       int err;
>>
>> -     err = hw->tf->write(hw->dev, HTS221_REG_CNTRL1_ADDR, sizeof(data),
>> -                         (u8 *)&data);
>> +     err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
>> +                                  HTS221_ENABLE_MASK, false);
>>
>>       return err < 0 ? err : 0;
>>  }
>



-- 
UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch;
unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp;
umount; make clean; sleep

[PATCH 4/9] iio: humidity: hts221: avoid useless ODR reconfiguration

[Lorenzo Bianconi]

Configure sensor ODR just in hts221_write_raw() in order to avoid
to set device sample rate multiple times.
Squash hts221_power_off()/hts221_power_on() in hts221_set_enable()

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
---
 drivers/iio/humidity/hts221.h        |  3 +--
 drivers/iio/humidity/hts221_buffer.c |  4 ++--
 drivers/iio/humidity/hts221_core.c   | 37 +++++++++++-------------------------
 3 files changed, 14 insertions(+), 30 deletions(-)

diff --git a/drivers/iio/humidity/hts221.h b/drivers/iio/humidity/hts221.h
index 94510266e0a5..7b5691a6df53 100644
--- a/drivers/iio/humidity/hts221.h
+++ b/drivers/iio/humidity/hts221.h
@@ -68,8 +68,7 @@ extern const struct dev_pm_ops hts221_pm_ops;
 
 int hts221_config_drdy(struct hts221_hw *hw, bool enable);
 int hts221_probe(struct iio_dev *iio_dev);
-int hts221_power_on(struct hts221_hw *hw);
-int hts221_power_off(struct hts221_hw *hw);
+int hts221_set_enable(struct hts221_hw *hw, bool enable);
 int hts221_allocate_buffers(struct hts221_hw *hw);
 int hts221_allocate_trigger(struct hts221_hw *hw);
 
diff --git a/drivers/iio/humidity/hts221_buffer.c b/drivers/iio/humidity/hts221_buffer.c
index 7d19a3da7ab7..c4ed153ad397 100644
--- a/drivers/iio/humidity/hts221_buffer.c
+++ b/drivers/iio/humidity/hts221_buffer.c
@@ -109,12 +109,12 @@ int hts221_allocate_trigger(struct hts221_hw *hw)
 
 static int hts221_buffer_preenable(struct iio_dev *iio_dev)
 {
-	return hts221_power_on(iio_priv(iio_dev));
+	return hts221_set_enable(iio_priv(iio_dev), true);
 }
 
 static int hts221_buffer_postdisable(struct iio_dev *iio_dev)
 {
-	return hts221_power_off(iio_priv(iio_dev));
+	return hts221_set_enable(iio_priv(iio_dev), false);
 }
 
 static const struct iio_buffer_setup_ops hts221_buffer_ops = {
diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
index b36734b8070e..7d30e2594a58 100644
--- a/drivers/iio/humidity/hts221_core.c
+++ b/drivers/iio/humidity/hts221_core.c
@@ -207,11 +207,6 @@ static int hts221_update_odr(struct hts221_hw *hw, u8 odr)
 	if (err < 0)
 		return err;
 
-	err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
-				     HTS221_ENABLE_MASK, 1);
-	if (err < 0)
-		return err;
-
 	hw->odr = odr;
 
 	return 0;
@@ -290,29 +285,16 @@ hts221_sysfs_temp_oversampling_avail(struct device *dev,
 	return len;
 }
 
-int hts221_power_on(struct hts221_hw *hw)
-{
-	int err;
-
-	err = hts221_update_odr(hw, hw->odr);
-	if (err < 0)
-		return err;
-
-	hw->enabled = true;
-
-	return 0;
-}
-
-int hts221_power_off(struct hts221_hw *hw)
+int hts221_set_enable(struct hts221_hw *hw, bool enable)
 {
 	int err;
 
 	err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
-				     HTS221_ENABLE_MASK, false);
+				     HTS221_ENABLE_MASK, enable);
 	if (err < 0)
 		return err;
 
-	hw->enabled = false;
+	hw->enabled = enable;
 
 	return 0;
 }
@@ -467,7 +449,7 @@ static int hts221_read_oneshot(struct hts221_hw *hw, u8 addr, int *val)
 	u8 data[HTS221_DATA_SIZE];
 	int err;
 
-	err = hts221_power_on(hw);
+	err = hts221_set_enable(hw, true);
 	if (err < 0)
 		return err;
 
@@ -477,7 +459,7 @@ static int hts221_read_oneshot(struct hts221_hw *hw, u8 addr, int *val)
 	if (err < 0)
 		return err;
 
-	hts221_power_off(hw);
+	hts221_set_enable(hw, false);
 
 	*val = (s16)get_unaligned_le16(data);
 
@@ -627,8 +609,6 @@ int hts221_probe(struct iio_dev *iio_dev)
 	if (err < 0)
 		return err;
 
-	hw->odr = hts221_odr_table[0].hz;
-
 	iio_dev->modes = INDIO_DIRECT_MODE;
 	iio_dev->dev.parent = hw->dev;
 	iio_dev->available_scan_masks = hts221_scan_masks;
@@ -643,6 +623,10 @@ int hts221_probe(struct iio_dev *iio_dev)
 	if (err < 0)
 		return err;
 
+	err = hts221_update_odr(hw, hts221_odr_table[0].hz);
+	if (err < 0)
+		return err;
+
 	/* configure humidity sensor */
 	err = hts221_parse_rh_caldata(hw);
 	if (err < 0) {
@@ -706,7 +690,8 @@ static int __maybe_unused hts221_resume(struct device *dev)
 	int err = 0;
 
 	if (hw->enabled)
-		err = hts221_update_odr(hw, hw->odr);
+		err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
+					     HTS221_ENABLE_MASK, true);
 
 	return err;
 }
-- 
2.13.1

Re: [PATCH 4/9] iio: humidity: hts221: avoid useless ODR reconfiguration

[Jonathan Cameron]

On Sun,  9 Jul 2017 18:56:59 +0200
Lorenzo Bianconi <lorenzo.bianconi83-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> Configure sensor ODR just in hts221_write_raw() in order to avoid
> to set device sample rate multiple times.
> Squash hts221_power_off()/hts221_power_on() in hts221_set_enable()
Two changes = two patches please.

Actual contents for each looks fine.
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
> ---
>  drivers/iio/humidity/hts221.h        |  3 +--
>  drivers/iio/humidity/hts221_buffer.c |  4 ++--
>  drivers/iio/humidity/hts221_core.c   | 37 +++++++++++-------------------------
>  3 files changed, 14 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/iio/humidity/hts221.h b/drivers/iio/humidity/hts221.h
> index 94510266e0a5..7b5691a6df53 100644
> --- a/drivers/iio/humidity/hts221.h
> +++ b/drivers/iio/humidity/hts221.h
> @@ -68,8 +68,7 @@ extern const struct dev_pm_ops hts221_pm_ops;
>  
>  int hts221_config_drdy(struct hts221_hw *hw, bool enable);
>  int hts221_probe(struct iio_dev *iio_dev);
> -int hts221_power_on(struct hts221_hw *hw);
> -int hts221_power_off(struct hts221_hw *hw);
> +int hts221_set_enable(struct hts221_hw *hw, bool enable);
>  int hts221_allocate_buffers(struct hts221_hw *hw);
>  int hts221_allocate_trigger(struct hts221_hw *hw);
>  
> diff --git a/drivers/iio/humidity/hts221_buffer.c b/drivers/iio/humidity/hts221_buffer.c
> index 7d19a3da7ab7..c4ed153ad397 100644
> --- a/drivers/iio/humidity/hts221_buffer.c
> +++ b/drivers/iio/humidity/hts221_buffer.c
> @@ -109,12 +109,12 @@ int hts221_allocate_trigger(struct hts221_hw *hw)
>  
>  static int hts221_buffer_preenable(struct iio_dev *iio_dev)
>  {
> -	return hts221_power_on(iio_priv(iio_dev));
> +	return hts221_set_enable(iio_priv(iio_dev), true);
>  }
>  
>  static int hts221_buffer_postdisable(struct iio_dev *iio_dev)
>  {
> -	return hts221_power_off(iio_priv(iio_dev));
> +	return hts221_set_enable(iio_priv(iio_dev), false);
>  }
>  
>  static const struct iio_buffer_setup_ops hts221_buffer_ops = {
> diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
> index b36734b8070e..7d30e2594a58 100644
> --- a/drivers/iio/humidity/hts221_core.c
> +++ b/drivers/iio/humidity/hts221_core.c
> @@ -207,11 +207,6 @@ static int hts221_update_odr(struct hts221_hw *hw, u8 odr)
>  	if (err < 0)
>  		return err;
>  
> -	err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
> -				     HTS221_ENABLE_MASK, 1);
> -	if (err < 0)
> -		return err;
> -
>  	hw->odr = odr;
>  
>  	return 0;
> @@ -290,29 +285,16 @@ hts221_sysfs_temp_oversampling_avail(struct device *dev,
>  	return len;
>  }
>  
> -int hts221_power_on(struct hts221_hw *hw)
> -{
> -	int err;
> -
> -	err = hts221_update_odr(hw, hw->odr);
> -	if (err < 0)
> -		return err;
> -
> -	hw->enabled = true;
> -
> -	return 0;
> -}
> -
> -int hts221_power_off(struct hts221_hw *hw)
> +int hts221_set_enable(struct hts221_hw *hw, bool enable)
>  {
>  	int err;
>  
>  	err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
> -				     HTS221_ENABLE_MASK, false);
> +				     HTS221_ENABLE_MASK, enable);
>  	if (err < 0)
>  		return err;
>  
> -	hw->enabled = false;
> +	hw->enabled = enable;
>  
>  	return 0;
>  }
> @@ -467,7 +449,7 @@ static int hts221_read_oneshot(struct hts221_hw *hw, u8 addr, int *val)
>  	u8 data[HTS221_DATA_SIZE];
>  	int err;
>  
> -	err = hts221_power_on(hw);
> +	err = hts221_set_enable(hw, true);
>  	if (err < 0)
>  		return err;
>  
> @@ -477,7 +459,7 @@ static int hts221_read_oneshot(struct hts221_hw *hw, u8 addr, int *val)
>  	if (err < 0)
>  		return err;
>  
> -	hts221_power_off(hw);
> +	hts221_set_enable(hw, false);
>  
>  	*val = (s16)get_unaligned_le16(data);
>  
> @@ -627,8 +609,6 @@ int hts221_probe(struct iio_dev *iio_dev)
>  	if (err < 0)
>  		return err;
>  
> -	hw->odr = hts221_odr_table[0].hz;
> -
>  	iio_dev->modes = INDIO_DIRECT_MODE;
>  	iio_dev->dev.parent = hw->dev;
>  	iio_dev->available_scan_masks = hts221_scan_masks;
> @@ -643,6 +623,10 @@ int hts221_probe(struct iio_dev *iio_dev)
>  	if (err < 0)
>  		return err;
>  
> +	err = hts221_update_odr(hw, hts221_odr_table[0].hz);
> +	if (err < 0)
> +		return err;
> +
>  	/* configure humidity sensor */
>  	err = hts221_parse_rh_caldata(hw);
>  	if (err < 0) {
> @@ -706,7 +690,8 @@ static int __maybe_unused hts221_resume(struct device *dev)
>  	int err = 0;
>  
>  	if (hw->enabled)
> -		err = hts221_update_odr(hw, hw->odr);
> +		err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
> +					     HTS221_ENABLE_MASK, true);
>  
>  	return err;
>  }

Re: [PATCH 4/9] iio: humidity: hts221: avoid useless ODR reconfiguration

[Lorenzo Bianconi]

> On Sun,  9 Jul 2017 18:56:59 +0200
> Lorenzo Bianconi <lorenzo.bianconi83-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> Configure sensor ODR just in hts221_write_raw() in order to avoid
>> to set device sample rate multiple times.
>> Squash hts221_power_off()/hts221_power_on() in hts221_set_enable()
> Two changes = two patches please.
>
> Actual contents for each looks fine.

Ack, will do in v2
Regards,
Lorenzo

>>
>> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
>> ---
>>  drivers/iio/humidity/hts221.h        |  3 +--
>>  drivers/iio/humidity/hts221_buffer.c |  4 ++--
>>  drivers/iio/humidity/hts221_core.c   | 37 +++++++++++-------------------------
>>  3 files changed, 14 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/iio/humidity/hts221.h b/drivers/iio/humidity/hts221.h
>> index 94510266e0a5..7b5691a6df53 100644
>> --- a/drivers/iio/humidity/hts221.h
>> +++ b/drivers/iio/humidity/hts221.h
>> @@ -68,8 +68,7 @@ extern const struct dev_pm_ops hts221_pm_ops;
>>
>>  int hts221_config_drdy(struct hts221_hw *hw, bool enable);
>>  int hts221_probe(struct iio_dev *iio_dev);
>> -int hts221_power_on(struct hts221_hw *hw);
>> -int hts221_power_off(struct hts221_hw *hw);
>> +int hts221_set_enable(struct hts221_hw *hw, bool enable);
>>  int hts221_allocate_buffers(struct hts221_hw *hw);
>>  int hts221_allocate_trigger(struct hts221_hw *hw);
>>
>> diff --git a/drivers/iio/humidity/hts221_buffer.c b/drivers/iio/humidity/hts221_buffer.c
>> index 7d19a3da7ab7..c4ed153ad397 100644
>> --- a/drivers/iio/humidity/hts221_buffer.c
>> +++ b/drivers/iio/humidity/hts221_buffer.c
>> @@ -109,12 +109,12 @@ int hts221_allocate_trigger(struct hts221_hw *hw)
>>
>>  static int hts221_buffer_preenable(struct iio_dev *iio_dev)
>>  {
>> -     return hts221_power_on(iio_priv(iio_dev));
>> +     return hts221_set_enable(iio_priv(iio_dev), true);
>>  }
>>
>>  static int hts221_buffer_postdisable(struct iio_dev *iio_dev)
>>  {
>> -     return hts221_power_off(iio_priv(iio_dev));
>> +     return hts221_set_enable(iio_priv(iio_dev), false);
>>  }
>>
>>  static const struct iio_buffer_setup_ops hts221_buffer_ops = {
>> diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
>> index b36734b8070e..7d30e2594a58 100644
>> --- a/drivers/iio/humidity/hts221_core.c
>> +++ b/drivers/iio/humidity/hts221_core.c
>> @@ -207,11 +207,6 @@ static int hts221_update_odr(struct hts221_hw *hw, u8 odr)
>>       if (err < 0)
>>               return err;
>>
>> -     err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
>> -                                  HTS221_ENABLE_MASK, 1);
>> -     if (err < 0)
>> -             return err;
>> -
>>       hw->odr = odr;
>>
>>       return 0;
>> @@ -290,29 +285,16 @@ hts221_sysfs_temp_oversampling_avail(struct device *dev,
>>       return len;
>>  }
>>
>> -int hts221_power_on(struct hts221_hw *hw)
>> -{
>> -     int err;
>> -
>> -     err = hts221_update_odr(hw, hw->odr);
>> -     if (err < 0)
>> -             return err;
>> -
>> -     hw->enabled = true;
>> -
>> -     return 0;
>> -}
>> -
>> -int hts221_power_off(struct hts221_hw *hw)
>> +int hts221_set_enable(struct hts221_hw *hw, bool enable)
>>  {
>>       int err;
>>
>>       err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
>> -                                  HTS221_ENABLE_MASK, false);
>> +                                  HTS221_ENABLE_MASK, enable);
>>       if (err < 0)
>>               return err;
>>
>> -     hw->enabled = false;
>> +     hw->enabled = enable;
>>
>>       return 0;
>>  }
>> @@ -467,7 +449,7 @@ static int hts221_read_oneshot(struct hts221_hw *hw, u8 addr, int *val)
>>       u8 data[HTS221_DATA_SIZE];
>>       int err;
>>
>> -     err = hts221_power_on(hw);
>> +     err = hts221_set_enable(hw, true);
>>       if (err < 0)
>>               return err;
>>
>> @@ -477,7 +459,7 @@ static int hts221_read_oneshot(struct hts221_hw *hw, u8 addr, int *val)
>>       if (err < 0)
>>               return err;
>>
>> -     hts221_power_off(hw);
>> +     hts221_set_enable(hw, false);
>>
>>       *val = (s16)get_unaligned_le16(data);
>>
>> @@ -627,8 +609,6 @@ int hts221_probe(struct iio_dev *iio_dev)
>>       if (err < 0)
>>               return err;
>>
>> -     hw->odr = hts221_odr_table[0].hz;
>> -
>>       iio_dev->modes = INDIO_DIRECT_MODE;
>>       iio_dev->dev.parent = hw->dev;
>>       iio_dev->available_scan_masks = hts221_scan_masks;
>> @@ -643,6 +623,10 @@ int hts221_probe(struct iio_dev *iio_dev)
>>       if (err < 0)
>>               return err;
>>
>> +     err = hts221_update_odr(hw, hts221_odr_table[0].hz);
>> +     if (err < 0)
>> +             return err;
>> +
>>       /* configure humidity sensor */
>>       err = hts221_parse_rh_caldata(hw);
>>       if (err < 0) {
>> @@ -706,7 +690,8 @@ static int __maybe_unused hts221_resume(struct device *dev)
>>       int err = 0;
>>
>>       if (hw->enabled)
>> -             err = hts221_update_odr(hw, hw->odr);
>> +             err = hts221_write_with_mask(hw, HTS221_REG_CNTRL1_ADDR,
>> +                                          HTS221_ENABLE_MASK, true);
>>
>>       return err;
>>  }
>



-- 
UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch;
unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp;
umount; make clean; sleep

[PATCH 5/9] iio: humidity: hts221: support active-low interrupts

[Lorenzo Bianconi]

Add support for active low interrupts (IRQF_TRIGGER_LOW and
IRQF_TRIGGER_FALLING). Configure the device as active high or low
according to the requested irq line.

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
---
 drivers/iio/humidity/hts221.h        |  1 +
 drivers/iio/humidity/hts221_buffer.c | 11 +++++++++++
 drivers/iio/humidity/hts221_core.c   |  3 +--
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/humidity/hts221.h b/drivers/iio/humidity/hts221.h
index 7b5691a6df53..0fbc89fe213d 100644
--- a/drivers/iio/humidity/hts221.h
+++ b/drivers/iio/humidity/hts221.h
@@ -67,6 +67,7 @@ struct hts221_hw {
 extern const struct dev_pm_ops hts221_pm_ops;
 
 int hts221_config_drdy(struct hts221_hw *hw, bool enable);
+int hts221_write_with_mask(struct hts221_hw *hw, u8 addr, u8 mask, u8 val);
 int hts221_probe(struct iio_dev *iio_dev);
 int hts221_set_enable(struct hts221_hw *hw, bool enable);
 int hts221_allocate_buffers(struct hts221_hw *hw);
diff --git a/drivers/iio/humidity/hts221_buffer.c b/drivers/iio/humidity/hts221_buffer.c
index c4ed153ad397..ad5222295b2c 100644
--- a/drivers/iio/humidity/hts221_buffer.c
+++ b/drivers/iio/humidity/hts221_buffer.c
@@ -22,6 +22,8 @@
 
 #include "hts221.h"
 
+#define HTS221_REG_DRDY_HL_ADDR		0x22
+#define HTS221_REG_DRDY_HL_MASK		BIT(7)
 #define HTS221_REG_STATUS_ADDR		0x27
 #define HTS221_RH_DRDY_MASK		BIT(1)
 #define HTS221_TEMP_DRDY_MASK		BIT(0)
@@ -67,6 +69,7 @@ static irqreturn_t hts221_trigger_handler_thread(int irq, void *private)
 int hts221_allocate_trigger(struct hts221_hw *hw)
 {
 	struct iio_dev *iio_dev = iio_priv_to_dev(hw);
+	bool irq_active_low = false;
 	unsigned long irq_type;
 	int err;
 
@@ -76,6 +79,10 @@ int hts221_allocate_trigger(struct hts221_hw *hw)
 	case IRQF_TRIGGER_HIGH:
 	case IRQF_TRIGGER_RISING:
 		break;
+	case IRQF_TRIGGER_LOW:
+	case IRQF_TRIGGER_FALLING:
+		irq_active_low = true;
+		break;
 	default:
 		dev_info(hw->dev,
 			 "mode %lx unsupported, using IRQF_TRIGGER_RISING\n",
@@ -84,6 +91,10 @@ int hts221_allocate_trigger(struct hts221_hw *hw)
 		break;
 	}
 
+	err = hts221_write_with_mask(hw, HTS221_REG_DRDY_HL_ADDR,
+				     HTS221_REG_DRDY_HL_MASK, irq_active_low);
+	if (err < 0)
+		return err;
 	err = devm_request_threaded_irq(hw->dev, hw->irq, NULL,
 					hts221_trigger_handler_thread,
 					irq_type | IRQF_ONESHOT,
diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
index 7d30e2594a58..dfacf64a6f2e 100644
--- a/drivers/iio/humidity/hts221_core.c
+++ b/drivers/iio/humidity/hts221_core.c
@@ -134,8 +134,7 @@ static const struct iio_chan_spec hts221_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(2),
 };
 
-static int hts221_write_with_mask(struct hts221_hw *hw, u8 addr, u8 mask,
-				  u8 val)
+int hts221_write_with_mask(struct hts221_hw *hw, u8 addr, u8 mask, u8 val)
 {
 	u8 data;
 	int err;
-- 
2.13.1

Re: [PATCH 5/9] iio: humidity: hts221: support active-low interrupts

[Jonathan Cameron]

On Sun,  9 Jul 2017 18:57:00 +0200
Lorenzo Bianconi <lorenzo.bianconi83-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> Add support for active low interrupts (IRQF_TRIGGER_LOW and
> IRQF_TRIGGER_FALLING). Configure the device as active high or low
> according to the requested irq line.
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
Hi Lorenzo,

Sorry, you are getting caught up in a more general question I'd like to 
pin down an answer for...

What should we be doing if we have a part that supports both high and
low interrupts and/or rising and falling?

We actually have more than one possible thing we are configuring with
one parameter.  If we are looking at shared interrupts or level
converters it's more than possible we have an inversion going on between
the two.   How is this represented to the two ends?

What's the right way of doing this?

I've added a few CCs that I think might chip in on this question!

My personal gut feeling is that the inverter should have explicit
representation in the kernel.  We should be able to instantiate an
irq_chip which is responsible for flipping it's sense.

You request an active high input on one side and it deals with
the active low that needs to be requested upstream.

Chances are I'm missing something and this is already well handled!

Jonathan
> ---
>  drivers/iio/humidity/hts221.h        |  1 +
>  drivers/iio/humidity/hts221_buffer.c | 11 +++++++++++
>  drivers/iio/humidity/hts221_core.c   |  3 +--
>  3 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/humidity/hts221.h b/drivers/iio/humidity/hts221.h
> index 7b5691a6df53..0fbc89fe213d 100644
> --- a/drivers/iio/humidity/hts221.h
> +++ b/drivers/iio/humidity/hts221.h
> @@ -67,6 +67,7 @@ struct hts221_hw {
>  extern const struct dev_pm_ops hts221_pm_ops;
>  
>  int hts221_config_drdy(struct hts221_hw *hw, bool enable);
> +int hts221_write_with_mask(struct hts221_hw *hw, u8 addr, u8 mask, u8 val);
>  int hts221_probe(struct iio_dev *iio_dev);
>  int hts221_set_enable(struct hts221_hw *hw, bool enable);
>  int hts221_allocate_buffers(struct hts221_hw *hw);
> diff --git a/drivers/iio/humidity/hts221_buffer.c b/drivers/iio/humidity/hts221_buffer.c
> index c4ed153ad397..ad5222295b2c 100644
> --- a/drivers/iio/humidity/hts221_buffer.c
> +++ b/drivers/iio/humidity/hts221_buffer.c
> @@ -22,6 +22,8 @@
>  
>  #include "hts221.h"
>  
> +#define HTS221_REG_DRDY_HL_ADDR		0x22
> +#define HTS221_REG_DRDY_HL_MASK		BIT(7)
>  #define HTS221_REG_STATUS_ADDR		0x27
>  #define HTS221_RH_DRDY_MASK		BIT(1)
>  #define HTS221_TEMP_DRDY_MASK		BIT(0)
> @@ -67,6 +69,7 @@ static irqreturn_t hts221_trigger_handler_thread(int irq, void *private)
>  int hts221_allocate_trigger(struct hts221_hw *hw)
>  {
>  	struct iio_dev *iio_dev = iio_priv_to_dev(hw);
> +	bool irq_active_low = false;
>  	unsigned long irq_type;
>  	int err;
>  
> @@ -76,6 +79,10 @@ int hts221_allocate_trigger(struct hts221_hw *hw)
>  	case IRQF_TRIGGER_HIGH:
>  	case IRQF_TRIGGER_RISING:
>  		break;
> +	case IRQF_TRIGGER_LOW:
> +	case IRQF_TRIGGER_FALLING:
> +		irq_active_low = true;
> +		break;
>  	default:
>  		dev_info(hw->dev,
>  			 "mode %lx unsupported, using IRQF_TRIGGER_RISING\n",
> @@ -84,6 +91,10 @@ int hts221_allocate_trigger(struct hts221_hw *hw)
>  		break;
>  	}
>  
> +	err = hts221_write_with_mask(hw, HTS221_REG_DRDY_HL_ADDR,
> +				     HTS221_REG_DRDY_HL_MASK, irq_active_low);
> +	if (err < 0)
> +		return err;
>  	err = devm_request_threaded_irq(hw->dev, hw->irq, NULL,
>  					hts221_trigger_handler_thread,
>  					irq_type | IRQF_ONESHOT,
> diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
> index 7d30e2594a58..dfacf64a6f2e 100644
> --- a/drivers/iio/humidity/hts221_core.c
> +++ b/drivers/iio/humidity/hts221_core.c
> @@ -134,8 +134,7 @@ static const struct iio_chan_spec hts221_channels[] = {
>  	IIO_CHAN_SOFT_TIMESTAMP(2),
>  };
>  
> -static int hts221_write_with_mask(struct hts221_hw *hw, u8 addr, u8 mask,
> -				  u8 val)
> +int hts221_write_with_mask(struct hts221_hw *hw, u8 addr, u8 mask, u8 val)
>  {
>  	u8 data;
>  	int err;

Re: [PATCH 5/9] iio: humidity: hts221: support active-low interrupts

[Marc Zyngier]

Hi Jonathan,

On 09/07/17 19:39, Jonathan Cameron wrote:
> On Sun,  9 Jul 2017 18:57:00 +0200
> Lorenzo Bianconi <lorenzo.bianconi83-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 
>> Add support for active low interrupts (IRQF_TRIGGER_LOW and
>> IRQF_TRIGGER_FALLING). Configure the device as active high or low
>> according to the requested irq line.
>>
>> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
> Hi Lorenzo,
> 
> Sorry, you are getting caught up in a more general question I'd like to 
> pin down an answer for...
> 
> What should we be doing if we have a part that supports both high and
> low interrupts and/or rising and falling?
> 
> We actually have more than one possible thing we are configuring with
> one parameter.  If we are looking at shared interrupts or level
> converters it's more than possible we have an inversion going on between
> the two.   How is this represented to the two ends?
> 
> What's the right way of doing this?
> 
> I've added a few CCs that I think might chip in on this question!
> 
> My personal gut feeling is that the inverter should have explicit
> representation in the kernel.  We should be able to instantiate an
> irq_chip which is responsible for flipping it's sense.
> 
> You request an active high input on one side and it deals with
> the active low that needs to be requested upstream.
> 
> Chances are I'm missing something and this is already well handled!

We already have things like that, such as
drivers/irqchip/irq-mtk-sysirq.c (whose sole purpose in life is to
invert interrupt polarity).

Hope this helps,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 5/9] iio: humidity: hts221: support active-low interrupts

[Jonathan Cameron]

On Mon, 10 Jul 2017 10:36:47 +0100
Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org> wrote:

> Hi Jonathan,
> 
> On 09/07/17 19:39, Jonathan Cameron wrote:
> > On Sun,  9 Jul 2017 18:57:00 +0200
> > Lorenzo Bianconi <lorenzo.bianconi83-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >   
> >> Add support for active low interrupts (IRQF_TRIGGER_LOW and
> >> IRQF_TRIGGER_FALLING). Configure the device as active high or low
> >> according to the requested irq line.
> >>
> >> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>  
> > Hi Lorenzo,
> > 
> > Sorry, you are getting caught up in a more general question I'd like to 
> > pin down an answer for...
> > 
> > What should we be doing if we have a part that supports both high and
> > low interrupts and/or rising and falling?
> > 
> > We actually have more than one possible thing we are configuring with
> > one parameter.  If we are looking at shared interrupts or level
> > converters it's more than possible we have an inversion going on between
> > the two.   How is this represented to the two ends?
> > 
> > What's the right way of doing this?
> > 
> > I've added a few CCs that I think might chip in on this question!
> > 
> > My personal gut feeling is that the inverter should have explicit
> > representation in the kernel.  We should be able to instantiate an
> > irq_chip which is responsible for flipping it's sense.
> > 
> > You request an active high input on one side and it deals with
> > the active low that needs to be requested upstream.
> > 
> > Chances are I'm missing something and this is already well handled!  
> 
> We already have things like that, such as
> drivers/irqchip/irq-mtk-sysirq.c (whose sole purpose in life is to
> invert interrupt polarity).
> 
> Hope this helps,
> 
> 	M.
Thanks Marc,

I think it would need generalising a fair bit, but in principle that
is doing exactly what is needed (just run with generic irq_chip
callbacks except for irq_set_type).

The registration logic seems rather convoluted, but I haven't yet
dug into why it is like that.  Certainly seems like we should be
able to get away with something rather less involved by not
trying to do it quite so efficiently.  For this particular
case I think we'd probably instantiate an irq chip per inverted
irq (there is no connection between them really).

Now the tricky bit as ever is going to be getting the bindings right.
The ones for the example you give are somewhat device specific.

Jonathan

[PATCH 6/9] dt-bindings: iio: humidity: hts221: support active-low interrupts

[Lorenzo Bianconi]

Update hts221 device binding with active-low interrupts support
(IRQ_TYPE_LEVEL_LOW and IRQ_TYPE_EDGE_FALLING).

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
---
 Documentation/devicetree/bindings/iio/humidity/hts221.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iio/humidity/hts221.txt b/Documentation/devicetree/bindings/iio/humidity/hts221.txt
index b20ab9c12080..fca263e13400 100644
--- a/Documentation/devicetree/bindings/iio/humidity/hts221.txt
+++ b/Documentation/devicetree/bindings/iio/humidity/hts221.txt
@@ -7,7 +7,8 @@ Required properties:
 Optional properties:
 - interrupt-parent: should be the phandle for the interrupt controller
 - interrupts: interrupt mapping for IRQ. It should be configured with
-  flags IRQ_TYPE_LEVEL_HIGH or IRQ_TYPE_EDGE_RISING.
+  flags IRQ_TYPE_LEVEL_HIGH, IRQ_TYPE_EDGE_RISING, IRQ_TYPE_LEVEL_LOW or
+  IRQ_TYPE_EDGE_FALLING.
 
   Refer to interrupt-controller/interrupts.txt for generic interrupt
   client node bindings.
-- 
2.13.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/9] dt-bindings: iio: humidity: hts221: support active-low interrupts

[Rob Herring]

On Sun, Jul 09, 2017 at 06:57:01PM +0200, Lorenzo Bianconi wrote:
> Update hts221 device binding with active-low interrupts support
> (IRQ_TYPE_LEVEL_LOW and IRQ_TYPE_EDGE_FALLING).
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/iio/humidity/hts221.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

[PATCH 7/9] iio: humidity: hts221: support open drain mode

[Lorenzo Bianconi]

Add open drain support in order to share requested IRQ line between
hts221 device and other peripherals

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
---
 drivers/iio/humidity/hts221_buffer.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/humidity/hts221_buffer.c b/drivers/iio/humidity/hts221_buffer.c
index ad5222295b2c..f29f01a22375 100644
--- a/drivers/iio/humidity/hts221_buffer.c
+++ b/drivers/iio/humidity/hts221_buffer.c
@@ -20,10 +20,14 @@
 #include <linux/iio/triggered_buffer.h>
 #include <linux/iio/buffer.h>
 
+#include <linux/platform_data/st_sensors_pdata.h>
+
 #include "hts221.h"
 
 #define HTS221_REG_DRDY_HL_ADDR		0x22
 #define HTS221_REG_DRDY_HL_MASK		BIT(7)
+#define HTS221_REG_DRDY_PP_OD_ADDR	0x22
+#define HTS221_REG_DRDY_PP_OD_MASK	BIT(6)
 #define HTS221_REG_STATUS_ADDR		0x27
 #define HTS221_RH_DRDY_MASK		BIT(1)
 #define HTS221_TEMP_DRDY_MASK		BIT(0)
@@ -69,7 +73,9 @@ static irqreturn_t hts221_trigger_handler_thread(int irq, void *private)
 int hts221_allocate_trigger(struct hts221_hw *hw)
 {
 	struct iio_dev *iio_dev = iio_priv_to_dev(hw);
-	bool irq_active_low = false;
+	bool irq_active_low = false, open_drain = false;
+	struct device_node *np = hw->dev->of_node;
+	struct st_sensors_platform_data *pdata;
 	unsigned long irq_type;
 	int err;
 
@@ -95,6 +101,20 @@ int hts221_allocate_trigger(struct hts221_hw *hw)
 				     HTS221_REG_DRDY_HL_MASK, irq_active_low);
 	if (err < 0)
 		return err;
+
+	pdata = (struct st_sensors_platform_data *)hw->dev->platform_data;
+	if ((np && of_property_read_bool(np, "drive-open-drain")) ||
+	    (pdata && pdata->open_drain)) {
+		irq_type |= IRQF_SHARED;
+		open_drain = true;
+	}
+
+	err = hts221_write_with_mask(hw, HTS221_REG_DRDY_PP_OD_ADDR,
+				     HTS221_REG_DRDY_PP_OD_MASK,
+				     open_drain);
+	if (err < 0)
+		return err;
+
 	err = devm_request_threaded_irq(hw->dev, hw->irq, NULL,
 					hts221_trigger_handler_thread,
 					irq_type | IRQF_ONESHOT,
-- 
2.13.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 8/9] dt-bindings: iio: humidity: hts221: support open drain mode

[Lorenzo Bianconi]

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
---
 Documentation/devicetree/bindings/iio/humidity/hts221.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/humidity/hts221.txt b/Documentation/devicetree/bindings/iio/humidity/hts221.txt
index fca263e13400..732b83c06c08 100644
--- a/Documentation/devicetree/bindings/iio/humidity/hts221.txt
+++ b/Documentation/devicetree/bindings/iio/humidity/hts221.txt
@@ -5,6 +5,14 @@ Required properties:
 - reg: i2c address of the sensor / spi cs line
 
 Optional properties:
+- drive-open-drain: the interrupt/data ready line will be configured
+  as open drain, which is useful if several sensors share the same
+  interrupt line. This is a boolean property.
+  (This binding is taken from pinctrl/pinctrl-bindings.txt)
+  If the requested interrupt is configured as IRQ_TYPE_LEVEL_HIGH or
+  IRQ_TYPE_EDGE_RISING a pull-down resistor is needed to drive the line
+  when it is not active, whereas a pull-up one is needed when interrupt
+  line is configured as IRQ_TYPE_LEVEL_LOW or IRQ_TYPE_EDGE_FALLING.
 - interrupt-parent: should be the phandle for the interrupt controller
 - interrupts: interrupt mapping for IRQ. It should be configured with
   flags IRQ_TYPE_LEVEL_HIGH, IRQ_TYPE_EDGE_RISING, IRQ_TYPE_LEVEL_LOW or
-- 
2.13.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 8/9] dt-bindings: iio: humidity: hts221: support open drain mode

[Rob Herring]

On Sun, Jul 09, 2017 at 06:57:03PM +0200, Lorenzo Bianconi wrote:
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/iio/humidity/hts221.txt | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/humidity/hts221.txt b/Documentation/devicetree/bindings/iio/humidity/hts221.txt
> index fca263e13400..732b83c06c08 100644
> --- a/Documentation/devicetree/bindings/iio/humidity/hts221.txt
> +++ b/Documentation/devicetree/bindings/iio/humidity/hts221.txt
> @@ -5,6 +5,14 @@ Required properties:
>  - reg: i2c address of the sensor / spi cs line
>  
>  Optional properties:
> +- drive-open-drain: the interrupt/data ready line will be configured

Needs a vendor prefix and "drdy-open-drain" would be more specific.

> +  as open drain, which is useful if several sensors share the same
> +  interrupt line. This is a boolean property.
> +  (This binding is taken from pinctrl/pinctrl-bindings.txt)
> +  If the requested interrupt is configured as IRQ_TYPE_LEVEL_HIGH or
> +  IRQ_TYPE_EDGE_RISING a pull-down resistor is needed to drive the line
> +  when it is not active, whereas a pull-up one is needed when interrupt
> +  line is configured as IRQ_TYPE_LEVEL_LOW or IRQ_TYPE_EDGE_FALLING.
>  - interrupt-parent: should be the phandle for the interrupt controller
>  - interrupts: interrupt mapping for IRQ. It should be configured with
>    flags IRQ_TYPE_LEVEL_HIGH, IRQ_TYPE_EDGE_RISING, IRQ_TYPE_LEVEL_LOW or
> -- 
> 2.13.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 8/9] dt-bindings: iio: humidity: hts221: support open drain mode

[Jonathan Cameron]

On Mon, 10 Jul 2017 21:51:58 -0500
Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:

> On Sun, Jul 09, 2017 at 06:57:03PM +0200, Lorenzo Bianconi wrote:
> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
> > ---
> >  Documentation/devicetree/bindings/iio/humidity/hts221.txt | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/humidity/hts221.txt b/Documentation/devicetree/bindings/iio/humidity/hts221.txt
> > index fca263e13400..732b83c06c08 100644
> > --- a/Documentation/devicetree/bindings/iio/humidity/hts221.txt
> > +++ b/Documentation/devicetree/bindings/iio/humidity/hts221.txt
> > @@ -5,6 +5,14 @@ Required properties:
> >  - reg: i2c address of the sensor / spi cs line
> >  
> >  Optional properties:
> > +- drive-open-drain: the interrupt/data ready line will be configured  
> 
> Needs a vendor prefix and "drdy-open-drain" would be more specific.
This is straight out of the pinctrl bindings and has been used in a few
other places as well.  I can see it should really be more obviously
associated with the drdy pin though.  A few IIO drivers already
use this binding (st sensors and the lsm6dsx).  If we are going
to decide it's a bad idea we should deprecate those bindings and
add whatever we do here as well.

Jonathan
> 
> > +  as open drain, which is useful if several sensors share the same
> > +  interrupt line. This is a boolean property.
> > +  (This binding is taken from pinctrl/pinctrl-bindings.txt)
> > +  If the requested interrupt is configured as IRQ_TYPE_LEVEL_HIGH or
> > +  IRQ_TYPE_EDGE_RISING a pull-down resistor is needed to drive the line
> > +  when it is not active, whereas a pull-up one is needed when interrupt
> > +  line is configured as IRQ_TYPE_LEVEL_LOW or IRQ_TYPE_EDGE_FALLING.
> >  - interrupt-parent: should be the phandle for the interrupt controller
> >  - interrupts: interrupt mapping for IRQ. It should be configured with
> >    flags IRQ_TYPE_LEVEL_HIGH, IRQ_TYPE_EDGE_RISING, IRQ_TYPE_LEVEL_LOW or
> > -- 
> > 2.13.1
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe devicetree" in
> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html  

Re: [PATCH 8/9] dt-bindings: iio: humidity: hts221: support open drain mode

[Rob Herring]

On Tue, Jul 11, 2017 at 1:26 PM, Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Mon, 10 Jul 2017 21:51:58 -0500
> Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>
>> On Sun, Jul 09, 2017 at 06:57:03PM +0200, Lorenzo Bianconi wrote:
>> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
>> > ---
>> >  Documentation/devicetree/bindings/iio/humidity/hts221.txt | 8 ++++++++
>> >  1 file changed, 8 insertions(+)
>> >
>> > diff --git a/Documentation/devicetree/bindings/iio/humidity/hts221.txt b/Documentation/devicetree/bindings/iio/humidity/hts221.txt
>> > index fca263e13400..732b83c06c08 100644
>> > --- a/Documentation/devicetree/bindings/iio/humidity/hts221.txt
>> > +++ b/Documentation/devicetree/bindings/iio/humidity/hts221.txt
>> > @@ -5,6 +5,14 @@ Required properties:
>> >  - reg: i2c address of the sensor / spi cs line
>> >
>> >  Optional properties:
>> > +- drive-open-drain: the interrupt/data ready line will be configured
>>
>> Needs a vendor prefix and "drdy-open-drain" would be more specific.
> This is straight out of the pinctrl bindings and has been used in a few
> other places as well.  I can see it should really be more obviously
> associated with the drdy pin though.  A few IIO drivers already
> use this binding (st sensors and the lsm6dsx).  If we are going
> to decide it's a bad idea we should deprecate those bindings and
> add whatever we do here as well.

Ah, okay. Then the description should have a  "see pinctrl.txt" for
the description.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 8/9] dt-bindings: iio: humidity: hts221: support open drain mode

[Jonathan Cameron]

On 11 July 2017 19:42:54 BST, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>On Tue, Jul 11, 2017 at 1:26 PM, Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>wrote:
>> On Mon, 10 Jul 2017 21:51:58 -0500
>> Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>
>>> On Sun, Jul 09, 2017 at 06:57:03PM +0200, Lorenzo Bianconi wrote:
>>> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
>>> > ---
>>> >  Documentation/devicetree/bindings/iio/humidity/hts221.txt | 8
>++++++++
>>> >  1 file changed, 8 insertions(+)
>>> >
>>> > diff --git
>a/Documentation/devicetree/bindings/iio/humidity/hts221.txt
>b/Documentation/devicetree/bindings/iio/humidity/hts221.txt
>>> > index fca263e13400..732b83c06c08 100644
>>> > --- a/Documentation/devicetree/bindings/iio/humidity/hts221.txt
>>> > +++ b/Documentation/devicetree/bindings/iio/humidity/hts221.txt
>>> > @@ -5,6 +5,14 @@ Required properties:
>>> >  - reg: i2c address of the sensor / spi cs line
>>> >
>>> >  Optional properties:
>>> > +- drive-open-drain: the interrupt/data ready line will be
>configured
>>>
>>> Needs a vendor prefix and "drdy-open-drain" would be more specific.
>> This is straight out of the pinctrl bindings and has been used in a
>few
>> other places as well.  I can see it should really be more obviously
>> associated with the drdy pin though.  A few IIO drivers already
>> use this binding (st sensors and the lsm6dsx).  If we are going
>> to decide it's a bad idea we should deprecate those bindings and
>> add whatever we do here as well.
>
>Ah, okay. Then the description should have a  "see pinctrl.txt" for
>the description.
Agreed. 
>
>Rob
>--
>To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Re: [PATCH 8/9] dt-bindings: iio: humidity: hts221: support open drain mode

[Lorenzo Bianconi]

>
>
> On 11 July 2017 19:42:54 BST, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>On Tue, Jul 11, 2017 at 1:26 PM, Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>wrote:
>>> On Mon, 10 Jul 2017 21:51:58 -0500
>>> Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>>
>>>> On Sun, Jul 09, 2017 at 06:57:03PM +0200, Lorenzo Bianconi wrote:
>>>> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
>>>> > ---
>>>> >  Documentation/devicetree/bindings/iio/humidity/hts221.txt | 8
>>++++++++
>>>> >  1 file changed, 8 insertions(+)
>>>> >
>>>> > diff --git
>>a/Documentation/devicetree/bindings/iio/humidity/hts221.txt
>>b/Documentation/devicetree/bindings/iio/humidity/hts221.txt
>>>> > index fca263e13400..732b83c06c08 100644
>>>> > --- a/Documentation/devicetree/bindings/iio/humidity/hts221.txt
>>>> > +++ b/Documentation/devicetree/bindings/iio/humidity/hts221.txt
>>>> > @@ -5,6 +5,14 @@ Required properties:
>>>> >  - reg: i2c address of the sensor / spi cs line
>>>> >
>>>> >  Optional properties:
>>>> > +- drive-open-drain: the interrupt/data ready line will be
>>configured
>>>>
>>>> Needs a vendor prefix and "drdy-open-drain" would be more specific.
>>> This is straight out of the pinctrl bindings and has been used in a
>>few
>>> other places as well.  I can see it should really be more obviously
>>> associated with the drdy pin though.  A few IIO drivers already
>>> use this binding (st sensors and the lsm6dsx).  If we are going
>>> to decide it's a bad idea we should deprecate those bindings and
>>> add whatever we do here as well.
>>
>>Ah, okay. Then the description should have a  "see pinctrl.txt" for
>>the description.
> Agreed.

This property has been defined in pinctrl/pinctrl-bindings.txt. Is it
fine to refer to that file?
Regards,
Lorenzo

>>
>>Rob
>>--
>>To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>>the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> Sent from my Android device with K-9 Mail. Please excuse my brevity.



-- 
UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch;
unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp;
umount; make clean; sleep

Re: [PATCH 8/9] dt-bindings: iio: humidity: hts221: support open drain mode

[Jonathan Cameron]

On Sat, 15 Jul 2017 16:32:35 +0200
Lorenzo Bianconi <lorenzo.bianconi83-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> >
> >
> > On 11 July 2017 19:42:54 BST, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:  
> >>On Tue, Jul 11, 2017 at 1:26 PM, Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> >>wrote:  
> >>> On Mon, 10 Jul 2017 21:51:58 -0500
> >>> Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> >>>  
> >>>> On Sun, Jul 09, 2017 at 06:57:03PM +0200, Lorenzo Bianconi wrote:  
> >>>> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
> >>>> > ---
> >>>> >  Documentation/devicetree/bindings/iio/humidity/hts221.txt | 8  
> >>++++++++  
> >>>> >  1 file changed, 8 insertions(+)
> >>>> >
> >>>> > diff --git  
> >>a/Documentation/devicetree/bindings/iio/humidity/hts221.txt
> >>b/Documentation/devicetree/bindings/iio/humidity/hts221.txt  
> >>>> > index fca263e13400..732b83c06c08 100644
> >>>> > --- a/Documentation/devicetree/bindings/iio/humidity/hts221.txt
> >>>> > +++ b/Documentation/devicetree/bindings/iio/humidity/hts221.txt
> >>>> > @@ -5,6 +5,14 @@ Required properties:
> >>>> >  - reg: i2c address of the sensor / spi cs line
> >>>> >
> >>>> >  Optional properties:
> >>>> > +- drive-open-drain: the interrupt/data ready line will be  
> >>configured  
> >>>>
> >>>> Needs a vendor prefix and "drdy-open-drain" would be more specific.  
> >>> This is straight out of the pinctrl bindings and has been used in a  
> >>few  
> >>> other places as well.  I can see it should really be more obviously
> >>> associated with the drdy pin though.  A few IIO drivers already
> >>> use this binding (st sensors and the lsm6dsx).  If we are going
> >>> to decide it's a bad idea we should deprecate those bindings and
> >>> add whatever we do here as well.  
> >>
> >>Ah, okay. Then the description should have a  "see pinctrl.txt" for
> >>the description.  
> > Agreed.  
> 
> This property has been defined in pinctrl/pinctrl-bindings.txt. Is it
> fine to refer to that file?
> Regards,
> Lorenzo
Yes, that's what Rob was effectively suggesting.
> 
> >>
> >>Rob
> >>--
> >>To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> >>the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >>More majordomo info at  http://vger.kernel.org/majordomo-info.html  
> >
> > --
> > Sent from my Android device with K-9 Mail. Please excuse my brevity.  
> 
> 
> 

[PATCH 9/9] iio: humidity: hts221: move drdy enable logic in hts221_trig_set_state()

[Lorenzo Bianconi]

Move data-ready configuration in hts221_buffer.c since it is only related
to trigger logic

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
---
 drivers/iio/humidity/hts221.h        |  1 -
 drivers/iio/humidity/hts221_buffer.c |  8 +++++++-
 drivers/iio/humidity/hts221_core.c   | 14 --------------
 3 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/drivers/iio/humidity/hts221.h b/drivers/iio/humidity/hts221.h
index 0fbc89fe213d..2b4e5246447a 100644
--- a/drivers/iio/humidity/hts221.h
+++ b/drivers/iio/humidity/hts221.h
@@ -66,7 +66,6 @@ struct hts221_hw {
 
 extern const struct dev_pm_ops hts221_pm_ops;
 
-int hts221_config_drdy(struct hts221_hw *hw, bool enable);
 int hts221_write_with_mask(struct hts221_hw *hw, u8 addr, u8 mask, u8 val);
 int hts221_probe(struct iio_dev *iio_dev);
 int hts221_set_enable(struct hts221_hw *hw, bool enable);
diff --git a/drivers/iio/humidity/hts221_buffer.c b/drivers/iio/humidity/hts221_buffer.c
index f29f01a22375..9690dfe9a844 100644
--- a/drivers/iio/humidity/hts221_buffer.c
+++ b/drivers/iio/humidity/hts221_buffer.c
@@ -28,6 +28,8 @@
 #define HTS221_REG_DRDY_HL_MASK		BIT(7)
 #define HTS221_REG_DRDY_PP_OD_ADDR	0x22
 #define HTS221_REG_DRDY_PP_OD_MASK	BIT(6)
+#define HTS221_REG_DRDY_EN_ADDR		0x22
+#define HTS221_REG_DRDY_EN_MASK		BIT(2)
 #define HTS221_REG_STATUS_ADDR		0x27
 #define HTS221_RH_DRDY_MASK		BIT(1)
 #define HTS221_TEMP_DRDY_MASK		BIT(0)
@@ -36,8 +38,12 @@ static int hts221_trig_set_state(struct iio_trigger *trig, bool state)
 {
 	struct iio_dev *iio_dev = iio_trigger_get_drvdata(trig);
 	struct hts221_hw *hw = iio_priv(iio_dev);
+	int err;
+
+	err = hts221_write_with_mask(hw, HTS221_REG_DRDY_EN_ADDR,
+				     HTS221_REG_DRDY_EN_MASK, state);
 
-	return hts221_config_drdy(hw, state);
+	return err < 0 ? err : 0;
 }
 
 static const struct iio_trigger_ops hts221_trigger_ops = {
diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
index dfacf64a6f2e..12c2bc5954e4 100644
--- a/drivers/iio/humidity/hts221_core.c
+++ b/drivers/iio/humidity/hts221_core.c
@@ -23,7 +23,6 @@
 
 #define HTS221_REG_CNTRL1_ADDR		0x20
 #define HTS221_REG_CNTRL2_ADDR		0x21
-#define HTS221_REG_CNTRL3_ADDR		0x22
 
 #define HTS221_REG_AVG_ADDR		0x10
 #define HTS221_REG_H_OUT_L		0x28
@@ -36,9 +35,6 @@
 #define HTS221_BDU_MASK			BIT(2)
 #define HTS221_ENABLE_MASK		BIT(7)
 
-#define HTS221_DRDY_MASK		BIT(2)
-
-
 /* calibration registers */
 #define HTS221_REG_0RH_CAL_X_H		0x36
 #define HTS221_REG_1RH_CAL_X_H		0x3a
@@ -180,16 +176,6 @@ static int hts221_check_whoami(struct hts221_hw *hw)
 	return 0;
 }
 
-int hts221_config_drdy(struct hts221_hw *hw, bool enable)
-{
-	int err;
-
-	err = hts221_write_with_mask(hw, HTS221_REG_CNTRL3_ADDR,
-				     HTS221_DRDY_MASK, enable);
-
-	return err < 0 ? err : 0;
-}
-
 static int hts221_update_odr(struct hts221_hw *hw, u8 odr)
 {
 	int i, err;
-- 
2.13.1

Re: [PATCH 9/9] iio: humidity: hts221: move drdy enable logic in hts221_trig_set_state()

[Jonathan Cameron]

On Sun,  9 Jul 2017 18:57:04 +0200
Lorenzo Bianconi <lorenzo.bianconi83-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> Move data-ready configuration in hts221_buffer.c since it is only related
> to trigger logic
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
The rest of the series looks good to me.  We are early in the cycle
so should have plenty of time to pin down the question on how to handle
irq line inversion visibility to the kernel.

Jonathan
> ---
>  drivers/iio/humidity/hts221.h        |  1 -
>  drivers/iio/humidity/hts221_buffer.c |  8 +++++++-
>  drivers/iio/humidity/hts221_core.c   | 14 --------------
>  3 files changed, 7 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/iio/humidity/hts221.h b/drivers/iio/humidity/hts221.h
> index 0fbc89fe213d..2b4e5246447a 100644
> --- a/drivers/iio/humidity/hts221.h
> +++ b/drivers/iio/humidity/hts221.h
> @@ -66,7 +66,6 @@ struct hts221_hw {
>  
>  extern const struct dev_pm_ops hts221_pm_ops;
>  
> -int hts221_config_drdy(struct hts221_hw *hw, bool enable);
>  int hts221_write_with_mask(struct hts221_hw *hw, u8 addr, u8 mask, u8 val);
>  int hts221_probe(struct iio_dev *iio_dev);
>  int hts221_set_enable(struct hts221_hw *hw, bool enable);
> diff --git a/drivers/iio/humidity/hts221_buffer.c b/drivers/iio/humidity/hts221_buffer.c
> index f29f01a22375..9690dfe9a844 100644
> --- a/drivers/iio/humidity/hts221_buffer.c
> +++ b/drivers/iio/humidity/hts221_buffer.c
> @@ -28,6 +28,8 @@
>  #define HTS221_REG_DRDY_HL_MASK		BIT(7)
>  #define HTS221_REG_DRDY_PP_OD_ADDR	0x22
>  #define HTS221_REG_DRDY_PP_OD_MASK	BIT(6)
> +#define HTS221_REG_DRDY_EN_ADDR		0x22
> +#define HTS221_REG_DRDY_EN_MASK		BIT(2)
>  #define HTS221_REG_STATUS_ADDR		0x27
>  #define HTS221_RH_DRDY_MASK		BIT(1)
>  #define HTS221_TEMP_DRDY_MASK		BIT(0)
> @@ -36,8 +38,12 @@ static int hts221_trig_set_state(struct iio_trigger *trig, bool state)
>  {
>  	struct iio_dev *iio_dev = iio_trigger_get_drvdata(trig);
>  	struct hts221_hw *hw = iio_priv(iio_dev);
> +	int err;
> +
> +	err = hts221_write_with_mask(hw, HTS221_REG_DRDY_EN_ADDR,
> +				     HTS221_REG_DRDY_EN_MASK, state);
>  
> -	return hts221_config_drdy(hw, state);
> +	return err < 0 ? err : 0;
>  }
>  
>  static const struct iio_trigger_ops hts221_trigger_ops = {
> diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
> index dfacf64a6f2e..12c2bc5954e4 100644
> --- a/drivers/iio/humidity/hts221_core.c
> +++ b/drivers/iio/humidity/hts221_core.c
> @@ -23,7 +23,6 @@
>  
>  #define HTS221_REG_CNTRL1_ADDR		0x20
>  #define HTS221_REG_CNTRL2_ADDR		0x21
> -#define HTS221_REG_CNTRL3_ADDR		0x22
>  
>  #define HTS221_REG_AVG_ADDR		0x10
>  #define HTS221_REG_H_OUT_L		0x28
> @@ -36,9 +35,6 @@
>  #define HTS221_BDU_MASK			BIT(2)
>  #define HTS221_ENABLE_MASK		BIT(7)
>  
> -#define HTS221_DRDY_MASK		BIT(2)
> -
> -
>  /* calibration registers */
>  #define HTS221_REG_0RH_CAL_X_H		0x36
>  #define HTS221_REG_1RH_CAL_X_H		0x3a
> @@ -180,16 +176,6 @@ static int hts221_check_whoami(struct hts221_hw *hw)
>  	return 0;
>  }
>  
> -int hts221_config_drdy(struct hts221_hw *hw, bool enable)
> -{
> -	int err;
> -
> -	err = hts221_write_with_mask(hw, HTS221_REG_CNTRL3_ADDR,
> -				     HTS221_DRDY_MASK, enable);
> -
> -	return err < 0 ? err : 0;
> -}
> -
>  static int hts221_update_odr(struct hts221_hw *hw, u8 odr)
>  {
>  	int i, err;

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 9/9] iio: humidity: hts221: move drdy enable logic in hts221_trig_set_state()

[Lorenzo Bianconi]

> On Sun,  9 Jul 2017 18:57:04 +0200
> Lorenzo Bianconi <lorenzo.bianconi83-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> Move data-ready configuration in hts221_buffer.c since it is only related
>> to trigger logic
>>
>> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
> The rest of the series looks good to me.  We are early in the cycle
> so should have plenty of time to pin down the question on how to handle
> irq line inversion visibility to the kernel.

Fine, pretty interesting stuff :)
Regards,
Lorenzo

>
> Jonathan
>> ---
>>  drivers/iio/humidity/hts221.h        |  1 -
>>  drivers/iio/humidity/hts221_buffer.c |  8 +++++++-
>>  drivers/iio/humidity/hts221_core.c   | 14 --------------
>>  3 files changed, 7 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/iio/humidity/hts221.h b/drivers/iio/humidity/hts221.h
>> index 0fbc89fe213d..2b4e5246447a 100644
>> --- a/drivers/iio/humidity/hts221.h
>> +++ b/drivers/iio/humidity/hts221.h
>> @@ -66,7 +66,6 @@ struct hts221_hw {
>>
>>  extern const struct dev_pm_ops hts221_pm_ops;
>>
>> -int hts221_config_drdy(struct hts221_hw *hw, bool enable);
>>  int hts221_write_with_mask(struct hts221_hw *hw, u8 addr, u8 mask, u8 val);
>>  int hts221_probe(struct iio_dev *iio_dev);
>>  int hts221_set_enable(struct hts221_hw *hw, bool enable);
>> diff --git a/drivers/iio/humidity/hts221_buffer.c b/drivers/iio/humidity/hts221_buffer.c
>> index f29f01a22375..9690dfe9a844 100644
>> --- a/drivers/iio/humidity/hts221_buffer.c
>> +++ b/drivers/iio/humidity/hts221_buffer.c
>> @@ -28,6 +28,8 @@
>>  #define HTS221_REG_DRDY_HL_MASK              BIT(7)
>>  #define HTS221_REG_DRDY_PP_OD_ADDR   0x22
>>  #define HTS221_REG_DRDY_PP_OD_MASK   BIT(6)
>> +#define HTS221_REG_DRDY_EN_ADDR              0x22
>> +#define HTS221_REG_DRDY_EN_MASK              BIT(2)
>>  #define HTS221_REG_STATUS_ADDR               0x27
>>  #define HTS221_RH_DRDY_MASK          BIT(1)
>>  #define HTS221_TEMP_DRDY_MASK                BIT(0)
>> @@ -36,8 +38,12 @@ static int hts221_trig_set_state(struct iio_trigger *trig, bool state)
>>  {
>>       struct iio_dev *iio_dev = iio_trigger_get_drvdata(trig);
>>       struct hts221_hw *hw = iio_priv(iio_dev);
>> +     int err;
>> +
>> +     err = hts221_write_with_mask(hw, HTS221_REG_DRDY_EN_ADDR,
>> +                                  HTS221_REG_DRDY_EN_MASK, state);
>>
>> -     return hts221_config_drdy(hw, state);
>> +     return err < 0 ? err : 0;
>>  }
>>
>>  static const struct iio_trigger_ops hts221_trigger_ops = {
>> diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
>> index dfacf64a6f2e..12c2bc5954e4 100644
>> --- a/drivers/iio/humidity/hts221_core.c
>> +++ b/drivers/iio/humidity/hts221_core.c
>> @@ -23,7 +23,6 @@
>>
>>  #define HTS221_REG_CNTRL1_ADDR               0x20
>>  #define HTS221_REG_CNTRL2_ADDR               0x21
>> -#define HTS221_REG_CNTRL3_ADDR               0x22
>>
>>  #define HTS221_REG_AVG_ADDR          0x10
>>  #define HTS221_REG_H_OUT_L           0x28
>> @@ -36,9 +35,6 @@
>>  #define HTS221_BDU_MASK                      BIT(2)
>>  #define HTS221_ENABLE_MASK           BIT(7)
>>
>> -#define HTS221_DRDY_MASK             BIT(2)
>> -
>> -
>>  /* calibration registers */
>>  #define HTS221_REG_0RH_CAL_X_H               0x36
>>  #define HTS221_REG_1RH_CAL_X_H               0x3a
>> @@ -180,16 +176,6 @@ static int hts221_check_whoami(struct hts221_hw *hw)
>>       return 0;
>>  }
>>
>> -int hts221_config_drdy(struct hts221_hw *hw, bool enable)
>> -{
>> -     int err;
>> -
>> -     err = hts221_write_with_mask(hw, HTS221_REG_CNTRL3_ADDR,
>> -                                  HTS221_DRDY_MASK, enable);
>> -
>> -     return err < 0 ? err : 0;
>> -}
>> -
>>  static int hts221_update_odr(struct hts221_hw *hw, u8 odr)
>>  {
>>       int i, err;
>



-- 
UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch;
unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp;
umount; make clean; sleep

Re: [PATCH 9/9] iio: humidity: hts221: move drdy enable logic in hts221_trig_set_state()

[Lorenzo Bianconi]

>> On Sun,  9 Jul 2017 18:57:04 +0200
>> Lorenzo Bianconi <lorenzo.bianconi83-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>
>>> Move data-ready configuration in hts221_buffer.c since it is only related
>>> to trigger logic
>>>
>>> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
>> The rest of the series looks good to me.  We are early in the cycle
>> so should have plenty of time to pin down the question on how to handle
>> irq line inversion visibility to the kernel.
>
> Fine, pretty interesting stuff :)
> Regards,
> Lorenzo

Hi Jonathan,

I added all suggested changes in v2. What is the best way to support
that possibility (irq line inversion visibility)?
Is it up to hts221?

Regards,
Lorenzo

>
>>
>> Jonathan
>>> ---
>>>  drivers/iio/humidity/hts221.h        |  1 -
>>>  drivers/iio/humidity/hts221_buffer.c |  8 +++++++-
>>>  drivers/iio/humidity/hts221_core.c   | 14 --------------
>>>  3 files changed, 7 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/iio/humidity/hts221.h b/drivers/iio/humidity/hts221.h
>>> index 0fbc89fe213d..2b4e5246447a 100644
>>> --- a/drivers/iio/humidity/hts221.h
>>> +++ b/drivers/iio/humidity/hts221.h
>>> @@ -66,7 +66,6 @@ struct hts221_hw {
>>>
>>>  extern const struct dev_pm_ops hts221_pm_ops;
>>>
>>> -int hts221_config_drdy(struct hts221_hw *hw, bool enable);
>>>  int hts221_write_with_mask(struct hts221_hw *hw, u8 addr, u8 mask, u8 val);
>>>  int hts221_probe(struct iio_dev *iio_dev);
>>>  int hts221_set_enable(struct hts221_hw *hw, bool enable);
>>> diff --git a/drivers/iio/humidity/hts221_buffer.c b/drivers/iio/humidity/hts221_buffer.c
>>> index f29f01a22375..9690dfe9a844 100644
>>> --- a/drivers/iio/humidity/hts221_buffer.c
>>> +++ b/drivers/iio/humidity/hts221_buffer.c
>>> @@ -28,6 +28,8 @@
>>>  #define HTS221_REG_DRDY_HL_MASK              BIT(7)
>>>  #define HTS221_REG_DRDY_PP_OD_ADDR   0x22
>>>  #define HTS221_REG_DRDY_PP_OD_MASK   BIT(6)
>>> +#define HTS221_REG_DRDY_EN_ADDR              0x22
>>> +#define HTS221_REG_DRDY_EN_MASK              BIT(2)
>>>  #define HTS221_REG_STATUS_ADDR               0x27
>>>  #define HTS221_RH_DRDY_MASK          BIT(1)
>>>  #define HTS221_TEMP_DRDY_MASK                BIT(0)
>>> @@ -36,8 +38,12 @@ static int hts221_trig_set_state(struct iio_trigger *trig, bool state)
>>>  {
>>>       struct iio_dev *iio_dev = iio_trigger_get_drvdata(trig);
>>>       struct hts221_hw *hw = iio_priv(iio_dev);
>>> +     int err;
>>> +
>>> +     err = hts221_write_with_mask(hw, HTS221_REG_DRDY_EN_ADDR,
>>> +                                  HTS221_REG_DRDY_EN_MASK, state);
>>>
>>> -     return hts221_config_drdy(hw, state);
>>> +     return err < 0 ? err : 0;
>>>  }
>>>
>>>  static const struct iio_trigger_ops hts221_trigger_ops = {
>>> diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
>>> index dfacf64a6f2e..12c2bc5954e4 100644
>>> --- a/drivers/iio/humidity/hts221_core.c
>>> +++ b/drivers/iio/humidity/hts221_core.c
>>> @@ -23,7 +23,6 @@
>>>
>>>  #define HTS221_REG_CNTRL1_ADDR               0x20
>>>  #define HTS221_REG_CNTRL2_ADDR               0x21
>>> -#define HTS221_REG_CNTRL3_ADDR               0x22
>>>
>>>  #define HTS221_REG_AVG_ADDR          0x10
>>>  #define HTS221_REG_H_OUT_L           0x28
>>> @@ -36,9 +35,6 @@
>>>  #define HTS221_BDU_MASK                      BIT(2)
>>>  #define HTS221_ENABLE_MASK           BIT(7)
>>>
>>> -#define HTS221_DRDY_MASK             BIT(2)
>>> -
>>> -
>>>  /* calibration registers */
>>>  #define HTS221_REG_0RH_CAL_X_H               0x36
>>>  #define HTS221_REG_1RH_CAL_X_H               0x3a
>>> @@ -180,16 +176,6 @@ static int hts221_check_whoami(struct hts221_hw *hw)
>>>       return 0;
>>>  }
>>>
>>> -int hts221_config_drdy(struct hts221_hw *hw, bool enable)
>>> -{
>>> -     int err;
>>> -
>>> -     err = hts221_write_with_mask(hw, HTS221_REG_CNTRL3_ADDR,
>>> -                                  HTS221_DRDY_MASK, enable);
>>> -
>>> -     return err < 0 ? err : 0;
>>> -}
>>> -
>>>  static int hts221_update_odr(struct hts221_hw *hw, u8 odr)
>>>  {
>>>       int i, err;
>>
>
>
>
> --
> UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch;
> unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp;
> umount; make clean; sleep



-- 
UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch;
unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp;
umount; make clean; sleep
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 9/9] iio: humidity: hts221: move drdy enable logic in hts221_trig_set_state()

[Jonathan Cameron]

On 16 July 2017 10:45:15 BST, Lorenzo Bianconi <lorenzo.bianconi83-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> On Sun,  9 Jul 2017 18:57:04 +0200
>>> Lorenzo Bianconi <lorenzo.bianconi83-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>
>>>> Move data-ready configuration in hts221_buffer.c since it is only
>related
>>>> to trigger logic
>>>>
>>>> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
>>> The rest of the series looks good to me.  We are early in the cycle
>>> so should have plenty of time to pin down the question on how to
>handle
>>> irq line inversion visibility to the kernel.
>>
>> Fine, pretty interesting stuff :)
>> Regards,
>> Lorenzo
>
>Hi Jonathan,
>
>I added all suggested changes in v2. What is the best way to support
>that possibility (irq line inversion visibility)?
>Is it up to hts221?
I think the only real conclusion so far is that there are irqchip drivers dealing with this
already. Hence on basis of precedence alone, not an issue for the ultimate client driver.

Which I thought would be the case but wanted to get more info. Hence ignore whole issue in
this driver.

J
>
>Regards,
>Lorenzo
>
>>
>>>
>>> Jonathan
>>>> ---
>>>>  drivers/iio/humidity/hts221.h        |  1 -
>>>>  drivers/iio/humidity/hts221_buffer.c |  8 +++++++-
>>>>  drivers/iio/humidity/hts221_core.c   | 14 --------------
>>>>  3 files changed, 7 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/drivers/iio/humidity/hts221.h
>b/drivers/iio/humidity/hts221.h
>>>> index 0fbc89fe213d..2b4e5246447a 100644
>>>> --- a/drivers/iio/humidity/hts221.h
>>>> +++ b/drivers/iio/humidity/hts221.h
>>>> @@ -66,7 +66,6 @@ struct hts221_hw {
>>>>
>>>>  extern const struct dev_pm_ops hts221_pm_ops;
>>>>
>>>> -int hts221_config_drdy(struct hts221_hw *hw, bool enable);
>>>>  int hts221_write_with_mask(struct hts221_hw *hw, u8 addr, u8 mask,
>u8 val);
>>>>  int hts221_probe(struct iio_dev *iio_dev);
>>>>  int hts221_set_enable(struct hts221_hw *hw, bool enable);
>>>> diff --git a/drivers/iio/humidity/hts221_buffer.c
>b/drivers/iio/humidity/hts221_buffer.c
>>>> index f29f01a22375..9690dfe9a844 100644
>>>> --- a/drivers/iio/humidity/hts221_buffer.c
>>>> +++ b/drivers/iio/humidity/hts221_buffer.c
>>>> @@ -28,6 +28,8 @@
>>>>  #define HTS221_REG_DRDY_HL_MASK              BIT(7)
>>>>  #define HTS221_REG_DRDY_PP_OD_ADDR   0x22
>>>>  #define HTS221_REG_DRDY_PP_OD_MASK   BIT(6)
>>>> +#define HTS221_REG_DRDY_EN_ADDR              0x22
>>>> +#define HTS221_REG_DRDY_EN_MASK              BIT(2)
>>>>  #define HTS221_REG_STATUS_ADDR               0x27
>>>>  #define HTS221_RH_DRDY_MASK          BIT(1)
>>>>  #define HTS221_TEMP_DRDY_MASK                BIT(0)
>>>> @@ -36,8 +38,12 @@ static int hts221_trig_set_state(struct
>iio_trigger *trig, bool state)
>>>>  {
>>>>       struct iio_dev *iio_dev = iio_trigger_get_drvdata(trig);
>>>>       struct hts221_hw *hw = iio_priv(iio_dev);
>>>> +     int err;
>>>> +
>>>> +     err = hts221_write_with_mask(hw, HTS221_REG_DRDY_EN_ADDR,
>>>> +                                  HTS221_REG_DRDY_EN_MASK, state);
>>>>
>>>> -     return hts221_config_drdy(hw, state);
>>>> +     return err < 0 ? err : 0;
>>>>  }
>>>>
>>>>  static const struct iio_trigger_ops hts221_trigger_ops = {
>>>> diff --git a/drivers/iio/humidity/hts221_core.c
>b/drivers/iio/humidity/hts221_core.c
>>>> index dfacf64a6f2e..12c2bc5954e4 100644
>>>> --- a/drivers/iio/humidity/hts221_core.c
>>>> +++ b/drivers/iio/humidity/hts221_core.c
>>>> @@ -23,7 +23,6 @@
>>>>
>>>>  #define HTS221_REG_CNTRL1_ADDR               0x20
>>>>  #define HTS221_REG_CNTRL2_ADDR               0x21
>>>> -#define HTS221_REG_CNTRL3_ADDR               0x22
>>>>
>>>>  #define HTS221_REG_AVG_ADDR          0x10
>>>>  #define HTS221_REG_H_OUT_L           0x28
>>>> @@ -36,9 +35,6 @@
>>>>  #define HTS221_BDU_MASK                      BIT(2)
>>>>  #define HTS221_ENABLE_MASK           BIT(7)
>>>>
>>>> -#define HTS221_DRDY_MASK             BIT(2)
>>>> -
>>>> -
>>>>  /* calibration registers */
>>>>  #define HTS221_REG_0RH_CAL_X_H               0x36
>>>>  #define HTS221_REG_1RH_CAL_X_H               0x3a
>>>> @@ -180,16 +176,6 @@ static int hts221_check_whoami(struct
>hts221_hw *hw)
>>>>       return 0;
>>>>  }
>>>>
>>>> -int hts221_config_drdy(struct hts221_hw *hw, bool enable)
>>>> -{
>>>> -     int err;
>>>> -
>>>> -     err = hts221_write_with_mask(hw, HTS221_REG_CNTRL3_ADDR,
>>>> -                                  HTS221_DRDY_MASK, enable);
>>>> -
>>>> -     return err < 0 ? err : 0;
>>>> -}
>>>> -
>>>>  static int hts221_update_odr(struct hts221_hw *hw, u8 odr)
>>>>  {
>>>>       int i, err;
>>>
>>
>>
>>
>> --
>> UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch;
>> unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes;
>gasp;
>> umount; make clean; sleep

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html