* [PATCH 1/9] iio: humidity: hts221: refactor write_with_mask code
2017-07-09 16:56 ` Lorenzo Bianconi
@ 2017-07-09 16:56 ` Lorenzo Bianconi
-1 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-09 16:56 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o
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
^ permalink raw reply related [flat|nested] 64+ messages in thread* [PATCH 1/9] iio: humidity: hts221: refactor write_with_mask code
@ 2017-07-09 16:56 ` Lorenzo Bianconi
0 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-09 16:56 UTC (permalink / raw)
To: jic23; +Cc: linux-iio, devicetree, 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@st.com>
---
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
^ permalink raw reply related [flat|nested] 64+ messages in thread[parent not found: <20170709165704.26311-2-lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>]
* Re: [PATCH 1/9] iio: humidity: hts221: refactor write_with_mask code
2017-07-09 16:56 ` Lorenzo Bianconi
@ 2017-07-09 18:28 ` Jonathan Cameron
-1 siblings, 0 replies; 64+ messages in thread
From: Jonathan Cameron @ 2017-07-09 18:28 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o
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;
>
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [PATCH 1/9] iio: humidity: hts221: refactor write_with_mask code
@ 2017-07-09 18:28 ` Jonathan Cameron
0 siblings, 0 replies; 64+ messages in thread
From: Jonathan Cameron @ 2017-07-09 18:28 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: linux-iio, devicetree, lorenzo.bianconi
On Sun, 9 Jul 2017 18:56:56 +0200
Lorenzo Bianconi <lorenzo.bianconi83@gmail.com> 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@st.com>
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;
>
^ permalink raw reply [flat|nested] 64+ messages in thread[parent not found: <20170709192827.2c343108-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* Re: [PATCH 1/9] iio: humidity: hts221: refactor write_with_mask code
2017-07-09 18:28 ` Jonathan Cameron
@ 2017-07-09 21:18 ` Lorenzo Bianconi
-1 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-09 21:18 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, 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
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [PATCH 1/9] iio: humidity: hts221: refactor write_with_mask code
@ 2017-07-09 21:18 ` Lorenzo Bianconi
0 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-09 21:18 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio, devicetree, Lorenzo BIANCONI
> On Sun, 9 Jul 2017 18:56:56 +0200
> Lorenzo Bianconi <lorenzo.bianconi83@gmail.com> 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@st.com>
> 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
^ permalink raw reply [flat|nested] 64+ messages in thread[parent not found: <CAA2SeNJmUFp7WaOsAEq+yLMSKrzQVkhmyMhB2OVWsbBAmv4nCg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 1/9] iio: humidity: hts221: refactor write_with_mask code
2017-07-09 21:18 ` Lorenzo Bianconi
@ 2017-07-10 20:47 ` Jonathan Cameron
-1 siblings, 0 replies; 64+ messages in thread
From: Jonathan Cameron @ 2017-07-10 20:47 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Lorenzo BIANCONI
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
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [PATCH 1/9] iio: humidity: hts221: refactor write_with_mask code
@ 2017-07-10 20:47 ` Jonathan Cameron
0 siblings, 0 replies; 64+ messages in thread
From: Jonathan Cameron @ 2017-07-10 20:47 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: linux-iio, devicetree, Lorenzo BIANCONI
On Sun, 9 Jul 2017 23:18:47 +0200
Lorenzo Bianconi <lorenzo.bianconi83@gmail.com> wrote:
> > On Sun, 9 Jul 2017 18:56:56 +0200
> > Lorenzo Bianconi <lorenzo.bianconi83@gmail.com> 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@st.com>
> > 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
>
>
>
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH 2/9] iio: humidity: hts221: move BDU configuration in probe routine
2017-07-09 16:56 ` Lorenzo Bianconi
@ 2017-07-09 16:56 ` Lorenzo Bianconi
-1 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-09 16:56 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o
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
^ permalink raw reply related [flat|nested] 64+ messages in thread* [PATCH 2/9] iio: humidity: hts221: move BDU configuration in probe routine
@ 2017-07-09 16:56 ` Lorenzo Bianconi
0 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-09 16:56 UTC (permalink / raw)
To: jic23; +Cc: linux-iio, devicetree, 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@st.com>
---
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
^ permalink raw reply related [flat|nested] 64+ messages in thread[parent not found: <20170709165704.26311-3-lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>]
* Re: [PATCH 2/9] iio: humidity: hts221: move BDU configuration in probe routine
2017-07-09 16:56 ` Lorenzo Bianconi
@ 2017-07-09 18:29 ` Jonathan Cameron
-1 siblings, 0 replies; 64+ messages in thread
From: Jonathan Cameron @ 2017-07-09 18:29 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o
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
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [PATCH 2/9] iio: humidity: hts221: move BDU configuration in probe routine
@ 2017-07-09 18:29 ` Jonathan Cameron
0 siblings, 0 replies; 64+ messages in thread
From: Jonathan Cameron @ 2017-07-09 18:29 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: linux-iio, devicetree, lorenzo.bianconi
On Sun, 9 Jul 2017 18:56:57 +0200
Lorenzo Bianconi <lorenzo.bianconi83@gmail.com> 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@st.com>
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) {
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH 3/9] iio: humidity: hts221: do not overwrite reserved data during power-down
2017-07-09 16:56 ` Lorenzo Bianconi
@ 2017-07-09 16:56 ` Lorenzo Bianconi
-1 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-09 16:56 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o
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
^ permalink raw reply related [flat|nested] 64+ messages in thread* [PATCH 3/9] iio: humidity: hts221: do not overwrite reserved data during power-down
@ 2017-07-09 16:56 ` Lorenzo Bianconi
0 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-09 16:56 UTC (permalink / raw)
To: jic23; +Cc: linux-iio, devicetree, 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@st.com>
---
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
^ permalink raw reply related [flat|nested] 64+ messages in thread[parent not found: <20170709165704.26311-4-lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>]
* Re: [PATCH 3/9] iio: humidity: hts221: do not overwrite reserved data during power-down
2017-07-09 16:56 ` Lorenzo Bianconi
@ 2017-07-09 18:30 ` Jonathan Cameron
-1 siblings, 0 replies; 64+ messages in thread
From: Jonathan Cameron @ 2017-07-09 18:30 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o
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;
> }
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [PATCH 3/9] iio: humidity: hts221: do not overwrite reserved data during power-down
@ 2017-07-09 18:30 ` Jonathan Cameron
0 siblings, 0 replies; 64+ messages in thread
From: Jonathan Cameron @ 2017-07-09 18:30 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: linux-iio, devicetree, lorenzo.bianconi
On Sun, 9 Jul 2017 18:56:58 +0200
Lorenzo Bianconi <lorenzo.bianconi83@gmail.com> 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@st.com>
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;
> }
^ permalink raw reply [flat|nested] 64+ messages in thread[parent not found: <20170709193042.330fcd60-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* Re: [PATCH 3/9] iio: humidity: hts221: do not overwrite reserved data during power-down
2017-07-09 18:30 ` Jonathan Cameron
@ 2017-07-09 21:26 ` Lorenzo Bianconi
-1 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-09 21:26 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, 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
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [PATCH 3/9] iio: humidity: hts221: do not overwrite reserved data during power-down
@ 2017-07-09 21:26 ` Lorenzo Bianconi
0 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-09 21:26 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio, devicetree, Lorenzo BIANCONI
> On Sun, 9 Jul 2017 18:56:58 +0200
> Lorenzo Bianconi <lorenzo.bianconi83@gmail.com> 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@st.com>
> 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
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH 4/9] iio: humidity: hts221: avoid useless ODR reconfiguration
2017-07-09 16:56 ` Lorenzo Bianconi
@ 2017-07-09 16:56 ` Lorenzo Bianconi
-1 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-09 16:56 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o
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
^ permalink raw reply related [flat|nested] 64+ messages in thread* [PATCH 4/9] iio: humidity: hts221: avoid useless ODR reconfiguration
@ 2017-07-09 16:56 ` Lorenzo Bianconi
0 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-09 16:56 UTC (permalink / raw)
To: jic23; +Cc: linux-iio, devicetree, 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@st.com>
---
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
^ permalink raw reply related [flat|nested] 64+ messages in thread[parent not found: <20170709165704.26311-5-lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>]
* Re: [PATCH 4/9] iio: humidity: hts221: avoid useless ODR reconfiguration
2017-07-09 16:56 ` Lorenzo Bianconi
@ 2017-07-09 18:32 ` Jonathan Cameron
-1 siblings, 0 replies; 64+ messages in thread
From: Jonathan Cameron @ 2017-07-09 18:32 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o
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;
> }
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [PATCH 4/9] iio: humidity: hts221: avoid useless ODR reconfiguration
@ 2017-07-09 18:32 ` Jonathan Cameron
0 siblings, 0 replies; 64+ messages in thread
From: Jonathan Cameron @ 2017-07-09 18:32 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: linux-iio, devicetree, lorenzo.bianconi
On Sun, 9 Jul 2017 18:56:59 +0200
Lorenzo Bianconi <lorenzo.bianconi83@gmail.com> 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@st.com>
> ---
> 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;
> }
^ permalink raw reply [flat|nested] 64+ messages in thread[parent not found: <20170709193208.6864b158-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* Re: [PATCH 4/9] iio: humidity: hts221: avoid useless ODR reconfiguration
2017-07-09 18:32 ` Jonathan Cameron
@ 2017-07-09 21:27 ` Lorenzo Bianconi
-1 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-09 21:27 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, 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
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [PATCH 4/9] iio: humidity: hts221: avoid useless ODR reconfiguration
@ 2017-07-09 21:27 ` Lorenzo Bianconi
0 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-09 21:27 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio, devicetree, Lorenzo BIANCONI
> On Sun, 9 Jul 2017 18:56:59 +0200
> Lorenzo Bianconi <lorenzo.bianconi83@gmail.com> 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@st.com>
>> ---
>> 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
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH 5/9] iio: humidity: hts221: support active-low interrupts
2017-07-09 16:56 ` Lorenzo Bianconi
@ 2017-07-09 16:57 ` Lorenzo Bianconi
-1 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-09 16:57 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o
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
^ permalink raw reply related [flat|nested] 64+ messages in thread* [PATCH 5/9] iio: humidity: hts221: support active-low interrupts
@ 2017-07-09 16:57 ` Lorenzo Bianconi
0 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-09 16:57 UTC (permalink / raw)
To: jic23; +Cc: linux-iio, devicetree, 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@st.com>
---
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
^ permalink raw reply related [flat|nested] 64+ messages in thread[parent not found: <20170709165704.26311-6-lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>]
* Re: [PATCH 5/9] iio: humidity: hts221: support active-low interrupts
2017-07-09 16:57 ` Lorenzo Bianconi
@ 2017-07-09 18:39 ` Jonathan Cameron
-1 siblings, 0 replies; 64+ messages in thread
From: Jonathan Cameron @ 2017-07-09 18:39 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o,
Linus Walleij, Thomas Gleixner, Jason Cooper, Marc Zyngier
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;
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [PATCH 5/9] iio: humidity: hts221: support active-low interrupts
@ 2017-07-09 18:39 ` Jonathan Cameron
0 siblings, 0 replies; 64+ messages in thread
From: Jonathan Cameron @ 2017-07-09 18:39 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: linux-iio, devicetree, lorenzo.bianconi, Linus Walleij,
Thomas Gleixner, Jason Cooper, Marc Zyngier
On Sun, 9 Jul 2017 18:57:00 +0200
Lorenzo Bianconi <lorenzo.bianconi83@gmail.com> 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@st.com>
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;
^ permalink raw reply [flat|nested] 64+ messages in thread[parent not found: <20170709193951.14caff79-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* Re: [PATCH 5/9] iio: humidity: hts221: support active-low interrupts
2017-07-09 18:39 ` Jonathan Cameron
@ 2017-07-10 9:36 ` Marc Zyngier
-1 siblings, 0 replies; 64+ messages in thread
From: Marc Zyngier @ 2017-07-10 9:36 UTC (permalink / raw)
To: Jonathan Cameron, Lorenzo Bianconi
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o,
Linus Walleij, Thomas Gleixner, Jason Cooper
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...
^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH 5/9] iio: humidity: hts221: support active-low interrupts
@ 2017-07-10 9:36 ` Marc Zyngier
0 siblings, 0 replies; 64+ messages in thread
From: Marc Zyngier @ 2017-07-10 9:36 UTC (permalink / raw)
To: Jonathan Cameron, Lorenzo Bianconi
Cc: linux-iio, devicetree, lorenzo.bianconi, Linus Walleij,
Thomas Gleixner, Jason Cooper
Hi Jonathan,
On 09/07/17 19:39, Jonathan Cameron wrote:
> On Sun, 9 Jul 2017 18:57:00 +0200
> Lorenzo Bianconi <lorenzo.bianconi83@gmail.com> 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@st.com>
> 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...
^ permalink raw reply [flat|nested] 64+ messages in thread
[parent not found: <ceef5860-91ee-a922-0dd7-8f2698192b9b-5wv7dgnIgG8@public.gmane.org>]
* Re: [PATCH 5/9] iio: humidity: hts221: support active-low interrupts
2017-07-10 9:36 ` Marc Zyngier
@ 2017-07-11 18:52 ` Jonathan Cameron
-1 siblings, 0 replies; 64+ messages in thread
From: Jonathan Cameron @ 2017-07-11 18:52 UTC (permalink / raw)
To: Marc Zyngier
Cc: Lorenzo Bianconi, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o,
Linus Walleij, Thomas Gleixner, Jason Cooper
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
^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH 5/9] iio: humidity: hts221: support active-low interrupts
@ 2017-07-11 18:52 ` Jonathan Cameron
0 siblings, 0 replies; 64+ messages in thread
From: Jonathan Cameron @ 2017-07-11 18:52 UTC (permalink / raw)
To: Marc Zyngier
Cc: Lorenzo Bianconi, linux-iio, devicetree, lorenzo.bianconi,
Linus Walleij, Thomas Gleixner, Jason Cooper
On Mon, 10 Jul 2017 10:36:47 +0100
Marc Zyngier <marc.zyngier@arm.com> 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@gmail.com> 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@st.com>
> > 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
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH 6/9] dt-bindings: iio: humidity: hts221: support active-low interrupts
2017-07-09 16:56 ` Lorenzo Bianconi
@ 2017-07-09 16:57 ` Lorenzo Bianconi
-1 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-09 16:57 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o
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
^ permalink raw reply related [flat|nested] 64+ messages in thread
* [PATCH 6/9] dt-bindings: iio: humidity: hts221: support active-low interrupts
@ 2017-07-09 16:57 ` Lorenzo Bianconi
0 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-09 16:57 UTC (permalink / raw)
To: jic23; +Cc: linux-iio, devicetree, 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@st.com>
---
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
^ permalink raw reply related [flat|nested] 64+ messages in thread
[parent not found: <20170709165704.26311-7-lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>]
* Re: [PATCH 6/9] dt-bindings: iio: humidity: hts221: support active-low interrupts
2017-07-09 16:57 ` Lorenzo Bianconi
@ 2017-07-11 2:49 ` Rob Herring
-1 siblings, 0 replies; 64+ messages in thread
From: Rob Herring @ 2017-07-11 2:49 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: jic23-DgEjT+Ai2ygdnm+yROfE0A, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o
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>
^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH 6/9] dt-bindings: iio: humidity: hts221: support active-low interrupts
@ 2017-07-11 2:49 ` Rob Herring
0 siblings, 0 replies; 64+ messages in thread
From: Rob Herring @ 2017-07-11 2:49 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: jic23, linux-iio, devicetree, lorenzo.bianconi
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@st.com>
> ---
> Documentation/devicetree/bindings/iio/humidity/hts221.txt | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH 7/9] iio: humidity: hts221: support open drain mode
2017-07-09 16:56 ` Lorenzo Bianconi
@ 2017-07-09 16:57 ` Lorenzo Bianconi
-1 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-09 16:57 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o
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
^ permalink raw reply related [flat|nested] 64+ messages in thread* [PATCH 7/9] iio: humidity: hts221: support open drain mode
@ 2017-07-09 16:57 ` Lorenzo Bianconi
0 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-09 16:57 UTC (permalink / raw)
To: jic23; +Cc: linux-iio, devicetree, 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@st.com>
---
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
^ permalink raw reply related [flat|nested] 64+ messages in thread
* [PATCH 8/9] dt-bindings: iio: humidity: hts221: support open drain mode
2017-07-09 16:56 ` Lorenzo Bianconi
@ 2017-07-09 16:57 ` Lorenzo Bianconi
-1 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-09 16:57 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o
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
^ permalink raw reply related [flat|nested] 64+ messages in thread
* [PATCH 8/9] dt-bindings: iio: humidity: hts221: support open drain mode
@ 2017-07-09 16:57 ` Lorenzo Bianconi
0 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-09 16:57 UTC (permalink / raw)
To: jic23; +Cc: linux-iio, devicetree, lorenzo.bianconi
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@st.com>
---
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
^ permalink raw reply related [flat|nested] 64+ messages in thread
[parent not found: <20170709165704.26311-9-lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>]
* Re: [PATCH 8/9] dt-bindings: iio: humidity: hts221: support open drain mode
2017-07-09 16:57 ` Lorenzo Bianconi
@ 2017-07-11 2:51 ` Rob Herring
-1 siblings, 0 replies; 64+ messages in thread
From: Rob Herring @ 2017-07-11 2:51 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: jic23-DgEjT+Ai2ygdnm+yROfE0A, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o
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
^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH 8/9] dt-bindings: iio: humidity: hts221: support open drain mode
@ 2017-07-11 2:51 ` Rob Herring
0 siblings, 0 replies; 64+ messages in thread
From: Rob Herring @ 2017-07-11 2:51 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: jic23, linux-iio, devicetree, lorenzo.bianconi
On Sun, Jul 09, 2017 at 06:57:03PM +0200, Lorenzo Bianconi wrote:
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@st.com>
> ---
> 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH 8/9] dt-bindings: iio: humidity: hts221: support open drain mode
2017-07-11 2:51 ` Rob Herring
@ 2017-07-11 18:26 ` Jonathan Cameron
-1 siblings, 0 replies; 64+ messages in thread
From: Jonathan Cameron @ 2017-07-11 18:26 UTC (permalink / raw)
To: Rob Herring
Cc: Lorenzo Bianconi, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o
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
^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH 8/9] dt-bindings: iio: humidity: hts221: support open drain mode
@ 2017-07-11 18:26 ` Jonathan Cameron
0 siblings, 0 replies; 64+ messages in thread
From: Jonathan Cameron @ 2017-07-11 18:26 UTC (permalink / raw)
To: Rob Herring; +Cc: Lorenzo Bianconi, linux-iio, devicetree, lorenzo.bianconi
On Mon, 10 Jul 2017 21:51:58 -0500
Rob Herring <robh@kernel.org> wrote:
> On Sun, Jul 09, 2017 at 06:57:03PM +0200, Lorenzo Bianconi wrote:
> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@st.com>
> > ---
> > 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@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 64+ messages in thread
[parent not found: <20170711192656.7fd08a1c-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* Re: [PATCH 8/9] dt-bindings: iio: humidity: hts221: support open drain mode
2017-07-11 18:26 ` Jonathan Cameron
@ 2017-07-11 18:42 ` Rob Herring
-1 siblings, 0 replies; 64+ messages in thread
From: Rob Herring @ 2017-07-11 18:42 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lorenzo Bianconi, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o
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
^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH 8/9] dt-bindings: iio: humidity: hts221: support open drain mode
@ 2017-07-11 18:42 ` Rob Herring
0 siblings, 0 replies; 64+ messages in thread
From: Rob Herring @ 2017-07-11 18:42 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lorenzo Bianconi, linux-iio, devicetree, lorenzo.bianconi
On Tue, Jul 11, 2017 at 1:26 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On Mon, 10 Jul 2017 21:51:58 -0500
> Rob Herring <robh@kernel.org> wrote:
>
>> On Sun, Jul 09, 2017 at 06:57:03PM +0200, Lorenzo Bianconi wrote:
>> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@st.com>
>> > ---
>> > 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
^ permalink raw reply [flat|nested] 64+ messages in thread
[parent not found: <CAL_JsqJa7JeofpG=66AnVrTP4XtVrp9B4-QoLTyK08GzF4L5uQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 8/9] dt-bindings: iio: humidity: hts221: support open drain mode
2017-07-11 18:42 ` Rob Herring
@ 2017-07-11 18:56 ` Jonathan Cameron
-1 siblings, 0 replies; 64+ messages in thread
From: Jonathan Cameron @ 2017-07-11 18:56 UTC (permalink / raw)
To: Rob Herring, Jonathan Cameron
Cc: Lorenzo Bianconi, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o
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.
^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH 8/9] dt-bindings: iio: humidity: hts221: support open drain mode
@ 2017-07-11 18:56 ` Jonathan Cameron
0 siblings, 0 replies; 64+ messages in thread
From: Jonathan Cameron @ 2017-07-11 18:56 UTC (permalink / raw)
To: Rob Herring, Jonathan Cameron
Cc: Lorenzo Bianconi, linux-iio, devicetree, lorenzo.bianconi
On 11 July 2017 19:42:54 BST, Rob Herring <robh@kernel=2Eorg> wrote:
>On Tue, Jul 11, 2017 at 1:26 PM, Jonathan Cameron <jic23@kernel=2Eorg>
>wrote:
>> On Mon, 10 Jul 2017 21:51:58 -0500
>> Rob Herring <robh@kernel=2Eorg> wrote:
>>
>>> On Sun, Jul 09, 2017 at 06:57:03PM +0200, Lorenzo Bianconi wrote:
>>> > Signed-off-by: Lorenzo Bianconi <lorenzo=2Ebianconi@st=2Ecom>
>>> > ---
>>> > Documentation/devicetree/bindings/iio/humidity/hts221=2Etxt | 8
>++++++++
>>> > 1 file changed, 8 insertions(+)
>>> >
>>> > diff --git
>a/Documentation/devicetree/bindings/iio/humidity/hts221=2Etxt
>b/Documentation/devicetree/bindings/iio/humidity/hts221=2Etxt
>>> > index fca263e13400=2E=2E732b83c06c08 100644
>>> > --- a/Documentation/devicetree/bindings/iio/humidity/hts221=2Etxt
>>> > +++ b/Documentation/devicetree/bindings/iio/humidity/hts221=2Etxt
>>> > @@ -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=2E
>> This is straight out of the pinctrl bindings and has been used in a
>few
>> other places as well=2E I can see it should really be more obviously
>> associated with the drdy pin though=2E A few IIO drivers already
>> use this binding (st sensors and the lsm6dsx)=2E If we are going
>> to decide it's a bad idea we should deprecate those bindings and
>> add whatever we do here as well=2E
>
>Ah, okay=2E Then the description should have a "see pinctrl=2Etxt" for
>the description=2E
Agreed=2E=20
>
>Rob
>--
>To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>the body of a message to majordomo@vger=2Ekernel=2Eorg
>More majordomo info at http://vger=2Ekernel=2Eorg/majordomo-info=2Ehtml
--=20
Sent from my Android device with K-9 Mail=2E Please excuse my brevity=2E
^ permalink raw reply [flat|nested] 64+ messages in thread
[parent not found: <0462CB71-6762-48F2-99B3-EA62FF81C280-tko9wxEg+fIOOJlXag/Snyp2UmYkHbXO@public.gmane.org>]
* Re: [PATCH 8/9] dt-bindings: iio: humidity: hts221: support open drain mode
2017-07-11 18:56 ` Jonathan Cameron
@ 2017-07-15 14:32 ` Lorenzo Bianconi
-1 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-15 14:32 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Rob Herring, Jonathan Cameron, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, 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
^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH 8/9] dt-bindings: iio: humidity: hts221: support open drain mode
@ 2017-07-15 14:32 ` Lorenzo Bianconi
0 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-15 14:32 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Rob Herring, Jonathan Cameron, linux-iio, devicetree, Lorenzo BIANCONI
>
>
> On 11 July 2017 19:42:54 BST, Rob Herring <robh@kernel.org> wrote:
>>On Tue, Jul 11, 2017 at 1:26 PM, Jonathan Cameron <jic23@kernel.org>
>>wrote:
>>> On Mon, 10 Jul 2017 21:51:58 -0500
>>> Rob Herring <robh@kernel.org> wrote:
>>>
>>>> On Sun, Jul 09, 2017 at 06:57:03PM +0200, Lorenzo Bianconi wrote:
>>>> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@st.com>
>>>> > ---
>>>> > 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@vger.kernel.org
>>More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> Sent from my Android device with K-9 Mail. Please excuse my brevity.
--
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
^ permalink raw reply [flat|nested] 64+ messages in thread
[parent not found: <CAA2SeN+2T2-G1khRNpNzccesX=cip0ConT11RJRV-HB8Ojk0pA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 8/9] dt-bindings: iio: humidity: hts221: support open drain mode
2017-07-15 14:32 ` Lorenzo Bianconi
@ 2017-07-17 11:49 ` Jonathan Cameron
-1 siblings, 0 replies; 64+ messages in thread
From: Jonathan Cameron @ 2017-07-17 11:49 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: Jonathan Cameron, Rob Herring, Jonathan Cameron,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Lorenzo BIANCONI
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.
>
>
>
^ permalink raw reply [flat|nested] 64+ messages in thread
* Re: [PATCH 8/9] dt-bindings: iio: humidity: hts221: support open drain mode
@ 2017-07-17 11:49 ` Jonathan Cameron
0 siblings, 0 replies; 64+ messages in thread
From: Jonathan Cameron @ 2017-07-17 11:49 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: Jonathan Cameron, Rob Herring, Jonathan Cameron, linux-iio,
devicetree, Lorenzo BIANCONI
On Sat, 15 Jul 2017 16:32:35 +0200
Lorenzo Bianconi <lorenzo.bianconi83@gmail.com> wrote:
> >
> >
> > On 11 July 2017 19:42:54 BST, Rob Herring <robh@kernel.org> wrote:
> >>On Tue, Jul 11, 2017 at 1:26 PM, Jonathan Cameron <jic23@kernel.org>
> >>wrote:
> >>> On Mon, 10 Jul 2017 21:51:58 -0500
> >>> Rob Herring <robh@kernel.org> wrote:
> >>>
> >>>> On Sun, Jul 09, 2017 at 06:57:03PM +0200, Lorenzo Bianconi wrote:
> >>>> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@st.com>
> >>>> > ---
> >>>> > 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@vger.kernel.org
> >>More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> > --
> > Sent from my Android device with K-9 Mail. Please excuse my brevity.
>
>
>
^ permalink raw reply [flat|nested] 64+ messages in thread
* [PATCH 9/9] iio: humidity: hts221: move drdy enable logic in hts221_trig_set_state()
2017-07-09 16:56 ` Lorenzo Bianconi
@ 2017-07-09 16:57 ` Lorenzo Bianconi
-1 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-09 16:57 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o
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
^ permalink raw reply related [flat|nested] 64+ messages in thread* [PATCH 9/9] iio: humidity: hts221: move drdy enable logic in hts221_trig_set_state()
@ 2017-07-09 16:57 ` Lorenzo Bianconi
0 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-09 16:57 UTC (permalink / raw)
To: jic23; +Cc: linux-iio, devicetree, 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@st.com>
---
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
^ permalink raw reply related [flat|nested] 64+ messages in thread[parent not found: <20170709165704.26311-10-lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>]
* Re: [PATCH 9/9] iio: humidity: hts221: move drdy enable logic in hts221_trig_set_state()
2017-07-09 16:57 ` Lorenzo Bianconi
@ 2017-07-09 18:41 ` Jonathan Cameron
-1 siblings, 0 replies; 64+ messages in thread
From: Jonathan Cameron @ 2017-07-09 18:41 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o
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
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [PATCH 9/9] iio: humidity: hts221: move drdy enable logic in hts221_trig_set_state()
@ 2017-07-09 18:41 ` Jonathan Cameron
0 siblings, 0 replies; 64+ messages in thread
From: Jonathan Cameron @ 2017-07-09 18:41 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: linux-iio, devicetree, lorenzo.bianconi
On Sun, 9 Jul 2017 18:57:04 +0200
Lorenzo Bianconi <lorenzo.bianconi83@gmail.com> wrote:
> Move data-ready configuration in hts221_buffer.c since it is only related
> to trigger logic
>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@st.com>
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;
^ permalink raw reply [flat|nested] 64+ messages in thread[parent not found: <20170709194123.39fd6914-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* Re: [PATCH 9/9] iio: humidity: hts221: move drdy enable logic in hts221_trig_set_state()
2017-07-09 18:41 ` Jonathan Cameron
@ 2017-07-09 21:28 ` Lorenzo Bianconi
-1 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-09 21:28 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, 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
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [PATCH 9/9] iio: humidity: hts221: move drdy enable logic in hts221_trig_set_state()
@ 2017-07-09 21:28 ` Lorenzo Bianconi
0 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-09 21:28 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio, devicetree, Lorenzo BIANCONI
> On Sun, 9 Jul 2017 18:57:04 +0200
> Lorenzo Bianconi <lorenzo.bianconi83@gmail.com> wrote:
>
>> Move data-ready configuration in hts221_buffer.c since it is only related
>> to trigger logic
>>
>> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@st.com>
> 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
^ permalink raw reply [flat|nested] 64+ messages in thread[parent not found: <CAA2SeNLhBbTbMGDHyXGN02V4n0RHrwK-Bg0D_q9phpcEa3e7Rw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 9/9] iio: humidity: hts221: move drdy enable logic in hts221_trig_set_state()
2017-07-09 21:28 ` Lorenzo Bianconi
@ 2017-07-16 9:45 ` Lorenzo Bianconi
-1 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-16 9:45 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, 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
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [PATCH 9/9] iio: humidity: hts221: move drdy enable logic in hts221_trig_set_state()
@ 2017-07-16 9:45 ` Lorenzo Bianconi
0 siblings, 0 replies; 64+ messages in thread
From: Lorenzo Bianconi @ 2017-07-16 9:45 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio, devicetree, Lorenzo BIANCONI
>> On Sun, 9 Jul 2017 18:57:04 +0200
>> Lorenzo Bianconi <lorenzo.bianconi83@gmail.com> wrote:
>>
>>> Move data-ready configuration in hts221_buffer.c since it is only related
>>> to trigger logic
>>>
>>> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@st.com>
>> 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
^ permalink raw reply [flat|nested] 64+ messages in thread[parent not found: <CAA2SeNK6dd=myMk59j1To9GtvBaj1T8XXwcaqrtO8V5s-bM5WQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH 9/9] iio: humidity: hts221: move drdy enable logic in hts221_trig_set_state()
2017-07-16 9:45 ` Lorenzo Bianconi
@ 2017-07-16 19:50 ` Jonathan Cameron
-1 siblings, 0 replies; 64+ messages in thread
From: Jonathan Cameron @ 2017-07-16 19:50 UTC (permalink / raw)
To: Lorenzo Bianconi, Jonathan Cameron
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Lorenzo BIANCONI
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
^ permalink raw reply [flat|nested] 64+ messages in thread* Re: [PATCH 9/9] iio: humidity: hts221: move drdy enable logic in hts221_trig_set_state()
@ 2017-07-16 19:50 ` Jonathan Cameron
0 siblings, 0 replies; 64+ messages in thread
From: Jonathan Cameron @ 2017-07-16 19:50 UTC (permalink / raw)
To: Lorenzo Bianconi, Jonathan Cameron
Cc: linux-iio, devicetree, Lorenzo BIANCONI
On 16 July 2017 10:45:15 BST, Lorenzo Bianconi <lorenzo.bianconi83@gmail.com> wrote:
>>> On Sun, 9 Jul 2017 18:57:04 +0200
>>> Lorenzo Bianconi <lorenzo.bianconi83@gmail.com> wrote:
>>>
>>>> Move data-ready configuration in hts221_buffer.c since it is only
>related
>>>> to trigger logic
>>>>
>>>> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@st.com>
>>> 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.
^ permalink raw reply [flat|nested] 64+ messages in thread