* [PATCH 1/2] iio: st_sensors: support active-low interrupts
@ 2015-11-13 20:18 Linus Walleij
2015-11-15 10:31 ` Jonathan Cameron
2015-11-16 6:02 ` Denis Ciocca
0 siblings, 2 replies; 6+ messages in thread
From: Linus Walleij @ 2015-11-13 20:18 UTC (permalink / raw)
To: Jonathan Cameron, linux-iio; +Cc: Linus Walleij, Giuseppe Barba, Denis Ciocca
Most ST MEMS Sensors that support interrupts can also handle sending
an active low interrupt, i.e. going from high to low on data ready
(or other interrupt) and thus triggering on a falling edge to the
interrupt controller.
Set up logic to inspect the interrupt line we get for a sensor: if
it is triggering on rising edge, leave everything alone, but if it
triggers on falling edges, set up active low, and if unsupported
configurations appear: warn with errors and reconfigure the interrupt
to a rising edge, which all interrupt generating sensors support.
Create a local header for st_sensors_core.h to share functions
between the sensor core and the trigger setup code.
Cc: Giuseppe Barba <giuseppe.barba@st.com>
Cc: Denis Ciocca <denis.ciocca@st.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
This patch requires the previously sent patch to switch the
driver to request_any_context_irq().
I am missing a data sheet for the LSM303AGR sensor that Giuseppe
added recently, so I have no idea what bits to poke to get active
low IRQs (or open drain) on that sensor.
---
drivers/iio/accel/st_accel_core.c | 16 ++++++++
drivers/iio/common/st_sensors/st_sensors_core.c | 6 +--
drivers/iio/common/st_sensors/st_sensors_core.h | 8 ++++
drivers/iio/common/st_sensors/st_sensors_trigger.c | 44 ++++++++++++++++++++--
drivers/iio/gyro/st_gyro_core.c | 12 ++++++
drivers/iio/pressure/st_pressure_core.c | 8 ++++
include/linux/iio/common/st_sensors.h | 4 ++
7 files changed, 91 insertions(+), 7 deletions(-)
create mode 100644 drivers/iio/common/st_sensors/st_sensors_core.h
diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
index 197a08b4e2f3..1132224cbc10 100644
--- a/drivers/iio/accel/st_accel_core.c
+++ b/drivers/iio/accel/st_accel_core.c
@@ -67,6 +67,8 @@
#define ST_ACCEL_1_DRDY_IRQ_ADDR 0x22
#define ST_ACCEL_1_DRDY_IRQ_INT1_MASK 0x10
#define ST_ACCEL_1_DRDY_IRQ_INT2_MASK 0x08
+#define ST_ACCEL_1_IHL_IRQ_ADDR 0x25
+#define ST_ACCEL_1_IHL_IRQ_MASK 0x02
#define ST_ACCEL_1_MULTIREAD_BIT true
/* CUSTOM VALUES FOR SENSOR 2 */
@@ -92,6 +94,8 @@
#define ST_ACCEL_2_DRDY_IRQ_ADDR 0x22
#define ST_ACCEL_2_DRDY_IRQ_INT1_MASK 0x02
#define ST_ACCEL_2_DRDY_IRQ_INT2_MASK 0x10
+#define ST_ACCEL_2_IHL_IRQ_ADDR 0x22
+#define ST_ACCEL_2_IHL_IRQ_MASK 0x80
#define ST_ACCEL_2_MULTIREAD_BIT true
/* CUSTOM VALUES FOR SENSOR 3 */
@@ -125,6 +129,8 @@
#define ST_ACCEL_3_DRDY_IRQ_ADDR 0x23
#define ST_ACCEL_3_DRDY_IRQ_INT1_MASK 0x80
#define ST_ACCEL_3_DRDY_IRQ_INT2_MASK 0x00
+#define ST_ACCEL_3_IHL_IRQ_ADDR 0x23
+#define ST_ACCEL_3_IHL_IRQ_MASK 0x40
#define ST_ACCEL_3_IG1_EN_ADDR 0x23
#define ST_ACCEL_3_IG1_EN_MASK 0x08
#define ST_ACCEL_3_MULTIREAD_BIT false
@@ -169,6 +175,8 @@
#define ST_ACCEL_5_DRDY_IRQ_ADDR 0x22
#define ST_ACCEL_5_DRDY_IRQ_INT1_MASK 0x04
#define ST_ACCEL_5_DRDY_IRQ_INT2_MASK 0x20
+#define ST_ACCEL_5_IHL_IRQ_ADDR 0x22
+#define ST_ACCEL_5_IHL_IRQ_MASK 0x80
#define ST_ACCEL_5_IG1_EN_ADDR 0x21
#define ST_ACCEL_5_IG1_EN_MASK 0x08
#define ST_ACCEL_5_MULTIREAD_BIT false
@@ -291,6 +299,8 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = {
.addr = ST_ACCEL_1_DRDY_IRQ_ADDR,
.mask_int1 = ST_ACCEL_1_DRDY_IRQ_INT1_MASK,
.mask_int2 = ST_ACCEL_1_DRDY_IRQ_INT2_MASK,
+ .addr_ihl = ST_ACCEL_1_IHL_IRQ_ADDR,
+ .mask_ihl = ST_ACCEL_1_IHL_IRQ_MASK,
},
.multi_read_bit = ST_ACCEL_1_MULTIREAD_BIT,
.bootime = 2,
@@ -354,6 +364,8 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = {
.addr = ST_ACCEL_2_DRDY_IRQ_ADDR,
.mask_int1 = ST_ACCEL_2_DRDY_IRQ_INT1_MASK,
.mask_int2 = ST_ACCEL_2_DRDY_IRQ_INT2_MASK,
+ .addr_ihl = ST_ACCEL_2_IHL_IRQ_ADDR,
+ .mask_ihl = ST_ACCEL_2_IHL_IRQ_MASK,
},
.multi_read_bit = ST_ACCEL_2_MULTIREAD_BIT,
.bootime = 2,
@@ -429,6 +441,8 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = {
.addr = ST_ACCEL_3_DRDY_IRQ_ADDR,
.mask_int1 = ST_ACCEL_3_DRDY_IRQ_INT1_MASK,
.mask_int2 = ST_ACCEL_3_DRDY_IRQ_INT2_MASK,
+ .addr_ihl = ST_ACCEL_3_IHL_IRQ_ADDR,
+ .mask_ihl = ST_ACCEL_3_IHL_IRQ_MASK,
.ig1 = {
.en_addr = ST_ACCEL_3_IG1_EN_ADDR,
.en_mask = ST_ACCEL_3_IG1_EN_MASK,
@@ -536,6 +550,8 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = {
.addr = ST_ACCEL_5_DRDY_IRQ_ADDR,
.mask_int1 = ST_ACCEL_5_DRDY_IRQ_INT1_MASK,
.mask_int2 = ST_ACCEL_5_DRDY_IRQ_INT2_MASK,
+ .addr_ihl = ST_ACCEL_5_IHL_IRQ_ADDR,
+ .mask_ihl = ST_ACCEL_5_IHL_IRQ_MASK,
},
.multi_read_bit = ST_ACCEL_5_MULTIREAD_BIT,
.bootime = 2, /* guess */
diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
index 25258e2c1a82..ed6f54d5c932 100644
--- a/drivers/iio/common/st_sensors/st_sensors_core.c
+++ b/drivers/iio/common/st_sensors/st_sensors_core.c
@@ -17,7 +17,7 @@
#include <linux/of.h>
#include <asm/unaligned.h>
#include <linux/iio/common/st_sensors.h>
-
+#include "st_sensors_core.h"
#define ST_SENSORS_WAI_ADDRESS 0x0f
@@ -26,8 +26,8 @@ static inline u32 st_sensors_get_unaligned_le24(const u8 *p)
return (s32)((p[0] | p[1] << 8 | p[2] << 16) << 8) >> 8;
}
-static int st_sensors_write_data_with_mask(struct iio_dev *indio_dev,
- u8 reg_addr, u8 mask, u8 data)
+int st_sensors_write_data_with_mask(struct iio_dev *indio_dev,
+ u8 reg_addr, u8 mask, u8 data)
{
int err;
u8 new_data;
diff --git a/drivers/iio/common/st_sensors/st_sensors_core.h b/drivers/iio/common/st_sensors/st_sensors_core.h
new file mode 100644
index 000000000000..cd88098ff6f1
--- /dev/null
+++ b/drivers/iio/common/st_sensors/st_sensors_core.h
@@ -0,0 +1,8 @@
+/*
+ * Local functions in the ST Sensors core
+ */
+#ifndef __ST_SENSORS_CORE_H
+#define __ST_SENSORS_CORE_H
+int st_sensors_write_data_with_mask(struct iio_dev *indio_dev,
+ u8 reg_addr, u8 mask, u8 data);
+#endif
diff --git a/drivers/iio/common/st_sensors/st_sensors_trigger.c b/drivers/iio/common/st_sensors/st_sensors_trigger.c
index e33796cdd607..cb53d14aca94 100644
--- a/drivers/iio/common/st_sensors/st_sensors_trigger.c
+++ b/drivers/iio/common/st_sensors/st_sensors_trigger.c
@@ -14,15 +14,16 @@
#include <linux/iio/iio.h>
#include <linux/iio/trigger.h>
#include <linux/interrupt.h>
-
#include <linux/iio/common/st_sensors.h>
-
+#include "st_sensors_core.h"
int st_sensors_allocate_trigger(struct iio_dev *indio_dev,
const struct iio_trigger_ops *trigger_ops)
{
int err;
struct st_sensor_data *sdata = iio_priv(indio_dev);
+ unsigned long irq_trig;
+ int irq;
sdata->trig = iio_trigger_alloc("%s-dev%d",
indio_dev->name, indio_dev->id);
@@ -32,9 +33,43 @@ int st_sensors_allocate_trigger(struct iio_dev *indio_dev,
goto iio_trigger_alloc_error;
}
- err = request_any_context_irq(sdata->get_irq_data_ready(indio_dev),
+ irq = sdata->get_irq_data_ready(indio_dev);
+ irq_trig = irqd_get_trigger_type(irq_get_irq_data(irq));
+ /*
+ * If the IRQ is triggered on falling edge, we need to mark the
+ * interrupt as active low, if the hardware supports this.
+ */
+ if (irq_trig == IRQF_TRIGGER_FALLING) {
+ if (!sdata->sensor_settings->drdy_irq.addr_ihl) {
+ dev_err(&indio_dev->dev,
+ "falling edge specified for IRQ but hardware "
+ "only support rising edge, will request "
+ "rising edge\n");
+ irq_trig = IRQF_TRIGGER_RISING;
+ } else {
+ /* Set up INT active low i.e. falling edge */
+ err = st_sensors_write_data_with_mask(indio_dev,
+ sdata->sensor_settings->drdy_irq.addr_ihl,
+ sdata->sensor_settings->drdy_irq.mask_ihl, 1);
+ if (err < 0)
+ goto iio_trigger_edge_error;
+ dev_info(&indio_dev->dev,
+ "interrupts on the falling edge\n");
+ }
+ } else if (irq_trig == IRQF_TRIGGER_RISING) {
+ dev_info(&indio_dev->dev,
+ "interrupts on the rising edge\n");
+
+ } else {
+ dev_err(&indio_dev->dev,
+ "unsupported IRQ trigger specified (%lx), only "
+ "rising and falling edges supported, enforce "
+ "rising edge\n", irq_trig);
+ irq_trig = IRQF_TRIGGER_RISING;
+ }
+ err = request_any_context_irq(irq,
iio_trigger_generic_data_rdy_poll,
- IRQF_TRIGGER_RISING,
+ irq_trig,
sdata->trig->name,
sdata->trig);
if (err) {
@@ -59,6 +94,7 @@ iio_trigger_register_error:
free_irq(sdata->get_irq_data_ready(indio_dev), sdata->trig);
request_irq_error:
iio_trigger_free(sdata->trig);
+iio_trigger_edge_error:
iio_trigger_alloc_error:
return err;
}
diff --git a/drivers/iio/gyro/st_gyro_core.c b/drivers/iio/gyro/st_gyro_core.c
index 02eddcebeea3..260bfe021904 100644
--- a/drivers/iio/gyro/st_gyro_core.c
+++ b/drivers/iio/gyro/st_gyro_core.c
@@ -61,6 +61,8 @@
#define ST_GYRO_1_BDU_MASK 0x80
#define ST_GYRO_1_DRDY_IRQ_ADDR 0x22
#define ST_GYRO_1_DRDY_IRQ_INT2_MASK 0x08
+#define ST_GYRO_1_IHL_IRQ_ADDR 0x22
+#define ST_GYRO_1_IHL_IRQ_MASK 0x20
#define ST_GYRO_1_MULTIREAD_BIT true
/* CUSTOM VALUES FOR SENSOR 2 */
@@ -85,6 +87,8 @@
#define ST_GYRO_2_BDU_MASK 0x80
#define ST_GYRO_2_DRDY_IRQ_ADDR 0x22
#define ST_GYRO_2_DRDY_IRQ_INT2_MASK 0x08
+#define ST_GYRO_2_IHL_IRQ_ADDR 0x22
+#define ST_GYRO_2_IHL_IRQ_MASK 0x20
#define ST_GYRO_2_MULTIREAD_BIT true
/* CUSTOM VALUES FOR SENSOR 3 */
@@ -109,6 +113,8 @@
#define ST_GYRO_3_BDU_MASK 0x80
#define ST_GYRO_3_DRDY_IRQ_ADDR 0x22
#define ST_GYRO_3_DRDY_IRQ_INT2_MASK 0x08
+#define ST_GYRO_3_IHL_IRQ_ADDR 0x22
+#define ST_GYRO_3_IHL_IRQ_MASK 0x20
#define ST_GYRO_3_MULTIREAD_BIT true
@@ -185,6 +191,8 @@ static const struct st_sensor_settings st_gyro_sensors_settings[] = {
.drdy_irq = {
.addr = ST_GYRO_1_DRDY_IRQ_ADDR,
.mask_int2 = ST_GYRO_1_DRDY_IRQ_INT2_MASK,
+ .addr_ihl = ST_GYRO_1_IHL_IRQ_ADDR,
+ .mask_ihl = ST_GYRO_1_IHL_IRQ_MASK,
},
.multi_read_bit = ST_GYRO_1_MULTIREAD_BIT,
.bootime = 2,
@@ -248,6 +256,8 @@ static const struct st_sensor_settings st_gyro_sensors_settings[] = {
.drdy_irq = {
.addr = ST_GYRO_2_DRDY_IRQ_ADDR,
.mask_int2 = ST_GYRO_2_DRDY_IRQ_INT2_MASK,
+ .addr_ihl = ST_GYRO_2_IHL_IRQ_ADDR,
+ .mask_ihl = ST_GYRO_2_IHL_IRQ_MASK,
},
.multi_read_bit = ST_GYRO_2_MULTIREAD_BIT,
.bootime = 2,
@@ -307,6 +317,8 @@ static const struct st_sensor_settings st_gyro_sensors_settings[] = {
.drdy_irq = {
.addr = ST_GYRO_3_DRDY_IRQ_ADDR,
.mask_int2 = ST_GYRO_3_DRDY_IRQ_INT2_MASK,
+ .addr_ihl = ST_GYRO_3_IHL_IRQ_ADDR,
+ .mask_ihl = ST_GYRO_3_IHL_IRQ_MASK,
},
.multi_read_bit = ST_GYRO_3_MULTIREAD_BIT,
.bootime = 2,
diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
index b39a2fb0671c..172393ad34af 100644
--- a/drivers/iio/pressure/st_pressure_core.c
+++ b/drivers/iio/pressure/st_pressure_core.c
@@ -62,6 +62,8 @@
#define ST_PRESS_LPS331AP_DRDY_IRQ_ADDR 0x22
#define ST_PRESS_LPS331AP_DRDY_IRQ_INT1_MASK 0x04
#define ST_PRESS_LPS331AP_DRDY_IRQ_INT2_MASK 0x20
+#define ST_PRESS_LPS331AP_IHL_IRQ_ADDR 0x22
+#define ST_PRESS_LPS331AP_IHL_IRQ_MASK 0x80
#define ST_PRESS_LPS331AP_MULTIREAD_BIT true
#define ST_PRESS_LPS331AP_TEMP_OFFSET 42500
@@ -100,6 +102,8 @@
#define ST_PRESS_LPS25H_DRDY_IRQ_ADDR 0x23
#define ST_PRESS_LPS25H_DRDY_IRQ_INT1_MASK 0x01
#define ST_PRESS_LPS25H_DRDY_IRQ_INT2_MASK 0x10
+#define ST_PRESS_LPS25H_IHL_IRQ_ADDR 0x22
+#define ST_PRESS_LPS25H_IHL_IRQ_MASK 0x80
#define ST_PRESS_LPS25H_MULTIREAD_BIT true
#define ST_PRESS_LPS25H_TEMP_OFFSET 42500
#define ST_PRESS_LPS25H_OUT_XL_ADDR 0x28
@@ -220,6 +224,8 @@ static const struct st_sensor_settings st_press_sensors_settings[] = {
.addr = ST_PRESS_LPS331AP_DRDY_IRQ_ADDR,
.mask_int1 = ST_PRESS_LPS331AP_DRDY_IRQ_INT1_MASK,
.mask_int2 = ST_PRESS_LPS331AP_DRDY_IRQ_INT2_MASK,
+ .addr_ihl = ST_PRESS_LPS331AP_IHL_IRQ_ADDR,
+ .mask_ihl = ST_PRESS_LPS331AP_IHL_IRQ_MASK,
},
.multi_read_bit = ST_PRESS_LPS331AP_MULTIREAD_BIT,
.bootime = 2,
@@ -304,6 +310,8 @@ static const struct st_sensor_settings st_press_sensors_settings[] = {
.addr = ST_PRESS_LPS25H_DRDY_IRQ_ADDR,
.mask_int1 = ST_PRESS_LPS25H_DRDY_IRQ_INT1_MASK,
.mask_int2 = ST_PRESS_LPS25H_DRDY_IRQ_INT2_MASK,
+ .addr_ihl = ST_PRESS_LPS25H_IHL_IRQ_ADDR,
+ .mask_ihl = ST_PRESS_LPS25H_IHL_IRQ_MASK,
},
.multi_read_bit = ST_PRESS_LPS25H_MULTIREAD_BIT,
.bootime = 2,
diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
index 2fe939c73cd2..6670c3d25c58 100644
--- a/include/linux/iio/common/st_sensors.h
+++ b/include/linux/iio/common/st_sensors.h
@@ -119,6 +119,8 @@ struct st_sensor_bdu {
* @addr: address of the register.
* @mask_int1: mask to enable/disable IRQ on INT1 pin.
* @mask_int2: mask to enable/disable IRQ on INT2 pin.
+ * @addr_ihl: address to enable/disable active low on the INT lines.
+ * @mask_ihl: mask to enable/disable active low on the INT lines.
* struct ig1 - represents the Interrupt Generator 1 of sensors.
* @en_addr: address of the enable ig1 register.
* @en_mask: mask to write the on/off value for enable.
@@ -127,6 +129,8 @@ struct st_sensor_data_ready_irq {
u8 addr;
u8 mask_int1;
u8 mask_int2;
+ u8 addr_ihl;
+ u8 mask_ihl;
struct {
u8 en_addr;
u8 en_mask;
--
2.4.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] iio: st_sensors: support active-low interrupts
2015-11-13 20:18 [PATCH 1/2] iio: st_sensors: support active-low interrupts Linus Walleij
@ 2015-11-15 10:31 ` Jonathan Cameron
2015-11-15 16:26 ` Linus Walleij
2015-11-16 6:02 ` Denis Ciocca
1 sibling, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2015-11-15 10:31 UTC (permalink / raw)
To: Linus Walleij, linux-iio; +Cc: Giuseppe Barba, Denis Ciocca
On 13/11/15 20:18, Linus Walleij wrote:
> Most ST MEMS Sensors that support interrupts can also handle sending
> an active low interrupt, i.e. going from high to low on data ready
> (or other interrupt) and thus triggering on a falling edge to the
> interrupt controller.
>
> Set up logic to inspect the interrupt line we get for a sensor: if
> it is triggering on rising edge, leave everything alone, but if it
> triggers on falling edges, set up active low, and if unsupported
> configurations appear: warn with errors and reconfigure the interrupt
> to a rising edge, which all interrupt generating sensors support.
>
> Create a local header for st_sensors_core.h to share functions
> between the sensor core and the trigger setup code.
>
> Cc: Giuseppe Barba <giuseppe.barba@st.com>
> Cc: Denis Ciocca <denis.ciocca@st.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Looks good to me. Ideally would like an ack from Denis and
feedback from Giuseppe on the LSM303AGR. Also I guess
the lis2dh12 is probably the same as similar parts
but best to check as that got added yesterday.
> ---
> This patch requires the previously sent patch to switch the
> driver to request_any_context_irq().
Really requires or will create some fuzz without it? Looks like
the latter so if we end up discussing that one for a while
I might reorder. Mind you early in this cycle anyway so
no rush on this one!
Jonathan
p.s. If anyone fancies adding support for the venerable
lis3l02dq as well so we can kill off my ancient driver
in staging that would be great! (I know I should do it
myself probably but I can always hope someone else will
get there first ;)
>
> I am missing a data sheet for the LSM303AGR sensor that Giuseppe
> added recently, so I have no idea what bits to poke to get active
> low IRQs (or open drain) on that sensor.
> ---
> drivers/iio/accel/st_accel_core.c | 16 ++++++++
> drivers/iio/common/st_sensors/st_sensors_core.c | 6 +--
> drivers/iio/common/st_sensors/st_sensors_core.h | 8 ++++
> drivers/iio/common/st_sensors/st_sensors_trigger.c | 44 ++++++++++++++++++++--
> drivers/iio/gyro/st_gyro_core.c | 12 ++++++
> drivers/iio/pressure/st_pressure_core.c | 8 ++++
> include/linux/iio/common/st_sensors.h | 4 ++
> 7 files changed, 91 insertions(+), 7 deletions(-)
> create mode 100644 drivers/iio/common/st_sensors/st_sensors_core.h
>
> diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
> index 197a08b4e2f3..1132224cbc10 100644
> --- a/drivers/iio/accel/st_accel_core.c
> +++ b/drivers/iio/accel/st_accel_core.c
> @@ -67,6 +67,8 @@
> #define ST_ACCEL_1_DRDY_IRQ_ADDR 0x22
> #define ST_ACCEL_1_DRDY_IRQ_INT1_MASK 0x10
> #define ST_ACCEL_1_DRDY_IRQ_INT2_MASK 0x08
> +#define ST_ACCEL_1_IHL_IRQ_ADDR 0x25
> +#define ST_ACCEL_1_IHL_IRQ_MASK 0x02
> #define ST_ACCEL_1_MULTIREAD_BIT true
>
> /* CUSTOM VALUES FOR SENSOR 2 */
> @@ -92,6 +94,8 @@
> #define ST_ACCEL_2_DRDY_IRQ_ADDR 0x22
> #define ST_ACCEL_2_DRDY_IRQ_INT1_MASK 0x02
> #define ST_ACCEL_2_DRDY_IRQ_INT2_MASK 0x10
> +#define ST_ACCEL_2_IHL_IRQ_ADDR 0x22
> +#define ST_ACCEL_2_IHL_IRQ_MASK 0x80
> #define ST_ACCEL_2_MULTIREAD_BIT true
>
> /* CUSTOM VALUES FOR SENSOR 3 */
> @@ -125,6 +129,8 @@
> #define ST_ACCEL_3_DRDY_IRQ_ADDR 0x23
> #define ST_ACCEL_3_DRDY_IRQ_INT1_MASK 0x80
> #define ST_ACCEL_3_DRDY_IRQ_INT2_MASK 0x00
> +#define ST_ACCEL_3_IHL_IRQ_ADDR 0x23
> +#define ST_ACCEL_3_IHL_IRQ_MASK 0x40
> #define ST_ACCEL_3_IG1_EN_ADDR 0x23
> #define ST_ACCEL_3_IG1_EN_MASK 0x08
> #define ST_ACCEL_3_MULTIREAD_BIT false
> @@ -169,6 +175,8 @@
> #define ST_ACCEL_5_DRDY_IRQ_ADDR 0x22
> #define ST_ACCEL_5_DRDY_IRQ_INT1_MASK 0x04
> #define ST_ACCEL_5_DRDY_IRQ_INT2_MASK 0x20
> +#define ST_ACCEL_5_IHL_IRQ_ADDR 0x22
> +#define ST_ACCEL_5_IHL_IRQ_MASK 0x80
> #define ST_ACCEL_5_IG1_EN_ADDR 0x21
> #define ST_ACCEL_5_IG1_EN_MASK 0x08
> #define ST_ACCEL_5_MULTIREAD_BIT false
> @@ -291,6 +299,8 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = {
> .addr = ST_ACCEL_1_DRDY_IRQ_ADDR,
> .mask_int1 = ST_ACCEL_1_DRDY_IRQ_INT1_MASK,
> .mask_int2 = ST_ACCEL_1_DRDY_IRQ_INT2_MASK,
> + .addr_ihl = ST_ACCEL_1_IHL_IRQ_ADDR,
> + .mask_ihl = ST_ACCEL_1_IHL_IRQ_MASK,
> },
> .multi_read_bit = ST_ACCEL_1_MULTIREAD_BIT,
> .bootime = 2,
> @@ -354,6 +364,8 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = {
> .addr = ST_ACCEL_2_DRDY_IRQ_ADDR,
> .mask_int1 = ST_ACCEL_2_DRDY_IRQ_INT1_MASK,
> .mask_int2 = ST_ACCEL_2_DRDY_IRQ_INT2_MASK,
> + .addr_ihl = ST_ACCEL_2_IHL_IRQ_ADDR,
> + .mask_ihl = ST_ACCEL_2_IHL_IRQ_MASK,
> },
> .multi_read_bit = ST_ACCEL_2_MULTIREAD_BIT,
> .bootime = 2,
> @@ -429,6 +441,8 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = {
> .addr = ST_ACCEL_3_DRDY_IRQ_ADDR,
> .mask_int1 = ST_ACCEL_3_DRDY_IRQ_INT1_MASK,
> .mask_int2 = ST_ACCEL_3_DRDY_IRQ_INT2_MASK,
> + .addr_ihl = ST_ACCEL_3_IHL_IRQ_ADDR,
> + .mask_ihl = ST_ACCEL_3_IHL_IRQ_MASK,
> .ig1 = {
> .en_addr = ST_ACCEL_3_IG1_EN_ADDR,
> .en_mask = ST_ACCEL_3_IG1_EN_MASK,
> @@ -536,6 +550,8 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = {
> .addr = ST_ACCEL_5_DRDY_IRQ_ADDR,
> .mask_int1 = ST_ACCEL_5_DRDY_IRQ_INT1_MASK,
> .mask_int2 = ST_ACCEL_5_DRDY_IRQ_INT2_MASK,
> + .addr_ihl = ST_ACCEL_5_IHL_IRQ_ADDR,
> + .mask_ihl = ST_ACCEL_5_IHL_IRQ_MASK,
> },
> .multi_read_bit = ST_ACCEL_5_MULTIREAD_BIT,
> .bootime = 2, /* guess */
> diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
> index 25258e2c1a82..ed6f54d5c932 100644
> --- a/drivers/iio/common/st_sensors/st_sensors_core.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
> @@ -17,7 +17,7 @@
> #include <linux/of.h>
> #include <asm/unaligned.h>
> #include <linux/iio/common/st_sensors.h>
> -
> +#include "st_sensors_core.h"
>
> #define ST_SENSORS_WAI_ADDRESS 0x0f
>
> @@ -26,8 +26,8 @@ static inline u32 st_sensors_get_unaligned_le24(const u8 *p)
> return (s32)((p[0] | p[1] << 8 | p[2] << 16) << 8) >> 8;
> }
>
> -static int st_sensors_write_data_with_mask(struct iio_dev *indio_dev,
> - u8 reg_addr, u8 mask, u8 data)
> +int st_sensors_write_data_with_mask(struct iio_dev *indio_dev,
> + u8 reg_addr, u8 mask, u8 data)
> {
> int err;
> u8 new_data;
> diff --git a/drivers/iio/common/st_sensors/st_sensors_core.h b/drivers/iio/common/st_sensors/st_sensors_core.h
> new file mode 100644
> index 000000000000..cd88098ff6f1
> --- /dev/null
> +++ b/drivers/iio/common/st_sensors/st_sensors_core.h
> @@ -0,0 +1,8 @@
> +/*
> + * Local functions in the ST Sensors core
> + */
> +#ifndef __ST_SENSORS_CORE_H
> +#define __ST_SENSORS_CORE_H
> +int st_sensors_write_data_with_mask(struct iio_dev *indio_dev,
> + u8 reg_addr, u8 mask, u8 data);
> +#endif
> diff --git a/drivers/iio/common/st_sensors/st_sensors_trigger.c b/drivers/iio/common/st_sensors/st_sensors_trigger.c
> index e33796cdd607..cb53d14aca94 100644
> --- a/drivers/iio/common/st_sensors/st_sensors_trigger.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_trigger.c
> @@ -14,15 +14,16 @@
> #include <linux/iio/iio.h>
> #include <linux/iio/trigger.h>
> #include <linux/interrupt.h>
> -
> #include <linux/iio/common/st_sensors.h>
> -
> +#include "st_sensors_core.h"
>
> int st_sensors_allocate_trigger(struct iio_dev *indio_dev,
> const struct iio_trigger_ops *trigger_ops)
> {
> int err;
> struct st_sensor_data *sdata = iio_priv(indio_dev);
> + unsigned long irq_trig;
> + int irq;
>
> sdata->trig = iio_trigger_alloc("%s-dev%d",
> indio_dev->name, indio_dev->id);
> @@ -32,9 +33,43 @@ int st_sensors_allocate_trigger(struct iio_dev *indio_dev,
> goto iio_trigger_alloc_error;
> }
>
> - err = request_any_context_irq(sdata->get_irq_data_ready(indio_dev),
> + irq = sdata->get_irq_data_ready(indio_dev);
> + irq_trig = irqd_get_trigger_type(irq_get_irq_data(irq));
> + /*
> + * If the IRQ is triggered on falling edge, we need to mark the
> + * interrupt as active low, if the hardware supports this.
> + */
> + if (irq_trig == IRQF_TRIGGER_FALLING) {
> + if (!sdata->sensor_settings->drdy_irq.addr_ihl) {
> + dev_err(&indio_dev->dev,
> + "falling edge specified for IRQ but hardware "
> + "only support rising edge, will request "
> + "rising edge\n");
> + irq_trig = IRQF_TRIGGER_RISING;
> + } else {
> + /* Set up INT active low i.e. falling edge */
> + err = st_sensors_write_data_with_mask(indio_dev,
> + sdata->sensor_settings->drdy_irq.addr_ihl,
> + sdata->sensor_settings->drdy_irq.mask_ihl, 1);
> + if (err < 0)
> + goto iio_trigger_edge_error;
> + dev_info(&indio_dev->dev,
> + "interrupts on the falling edge\n");
> + }
> + } else if (irq_trig == IRQF_TRIGGER_RISING) {
> + dev_info(&indio_dev->dev,
> + "interrupts on the rising edge\n");
> +
> + } else {
> + dev_err(&indio_dev->dev,
> + "unsupported IRQ trigger specified (%lx), only "
> + "rising and falling edges supported, enforce "
> + "rising edge\n", irq_trig);
> + irq_trig = IRQF_TRIGGER_RISING;
> + }
> + err = request_any_context_irq(irq,
> iio_trigger_generic_data_rdy_poll,
> - IRQF_TRIGGER_RISING,
> + irq_trig,
> sdata->trig->name,
> sdata->trig);
> if (err) {
> @@ -59,6 +94,7 @@ iio_trigger_register_error:
> free_irq(sdata->get_irq_data_ready(indio_dev), sdata->trig);
> request_irq_error:
> iio_trigger_free(sdata->trig);
> +iio_trigger_edge_error:
> iio_trigger_alloc_error:
> return err;
> }
> diff --git a/drivers/iio/gyro/st_gyro_core.c b/drivers/iio/gyro/st_gyro_core.c
> index 02eddcebeea3..260bfe021904 100644
> --- a/drivers/iio/gyro/st_gyro_core.c
> +++ b/drivers/iio/gyro/st_gyro_core.c
> @@ -61,6 +61,8 @@
> #define ST_GYRO_1_BDU_MASK 0x80
> #define ST_GYRO_1_DRDY_IRQ_ADDR 0x22
> #define ST_GYRO_1_DRDY_IRQ_INT2_MASK 0x08
> +#define ST_GYRO_1_IHL_IRQ_ADDR 0x22
> +#define ST_GYRO_1_IHL_IRQ_MASK 0x20
> #define ST_GYRO_1_MULTIREAD_BIT true
>
> /* CUSTOM VALUES FOR SENSOR 2 */
> @@ -85,6 +87,8 @@
> #define ST_GYRO_2_BDU_MASK 0x80
> #define ST_GYRO_2_DRDY_IRQ_ADDR 0x22
> #define ST_GYRO_2_DRDY_IRQ_INT2_MASK 0x08
> +#define ST_GYRO_2_IHL_IRQ_ADDR 0x22
> +#define ST_GYRO_2_IHL_IRQ_MASK 0x20
> #define ST_GYRO_2_MULTIREAD_BIT true
>
> /* CUSTOM VALUES FOR SENSOR 3 */
> @@ -109,6 +113,8 @@
> #define ST_GYRO_3_BDU_MASK 0x80
> #define ST_GYRO_3_DRDY_IRQ_ADDR 0x22
> #define ST_GYRO_3_DRDY_IRQ_INT2_MASK 0x08
> +#define ST_GYRO_3_IHL_IRQ_ADDR 0x22
> +#define ST_GYRO_3_IHL_IRQ_MASK 0x20
> #define ST_GYRO_3_MULTIREAD_BIT true
>
>
> @@ -185,6 +191,8 @@ static const struct st_sensor_settings st_gyro_sensors_settings[] = {
> .drdy_irq = {
> .addr = ST_GYRO_1_DRDY_IRQ_ADDR,
> .mask_int2 = ST_GYRO_1_DRDY_IRQ_INT2_MASK,
> + .addr_ihl = ST_GYRO_1_IHL_IRQ_ADDR,
> + .mask_ihl = ST_GYRO_1_IHL_IRQ_MASK,
> },
> .multi_read_bit = ST_GYRO_1_MULTIREAD_BIT,
> .bootime = 2,
> @@ -248,6 +256,8 @@ static const struct st_sensor_settings st_gyro_sensors_settings[] = {
> .drdy_irq = {
> .addr = ST_GYRO_2_DRDY_IRQ_ADDR,
> .mask_int2 = ST_GYRO_2_DRDY_IRQ_INT2_MASK,
> + .addr_ihl = ST_GYRO_2_IHL_IRQ_ADDR,
> + .mask_ihl = ST_GYRO_2_IHL_IRQ_MASK,
> },
> .multi_read_bit = ST_GYRO_2_MULTIREAD_BIT,
> .bootime = 2,
> @@ -307,6 +317,8 @@ static const struct st_sensor_settings st_gyro_sensors_settings[] = {
> .drdy_irq = {
> .addr = ST_GYRO_3_DRDY_IRQ_ADDR,
> .mask_int2 = ST_GYRO_3_DRDY_IRQ_INT2_MASK,
> + .addr_ihl = ST_GYRO_3_IHL_IRQ_ADDR,
> + .mask_ihl = ST_GYRO_3_IHL_IRQ_MASK,
> },
> .multi_read_bit = ST_GYRO_3_MULTIREAD_BIT,
> .bootime = 2,
> diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
> index b39a2fb0671c..172393ad34af 100644
> --- a/drivers/iio/pressure/st_pressure_core.c
> +++ b/drivers/iio/pressure/st_pressure_core.c
> @@ -62,6 +62,8 @@
> #define ST_PRESS_LPS331AP_DRDY_IRQ_ADDR 0x22
> #define ST_PRESS_LPS331AP_DRDY_IRQ_INT1_MASK 0x04
> #define ST_PRESS_LPS331AP_DRDY_IRQ_INT2_MASK 0x20
> +#define ST_PRESS_LPS331AP_IHL_IRQ_ADDR 0x22
> +#define ST_PRESS_LPS331AP_IHL_IRQ_MASK 0x80
> #define ST_PRESS_LPS331AP_MULTIREAD_BIT true
> #define ST_PRESS_LPS331AP_TEMP_OFFSET 42500
>
> @@ -100,6 +102,8 @@
> #define ST_PRESS_LPS25H_DRDY_IRQ_ADDR 0x23
> #define ST_PRESS_LPS25H_DRDY_IRQ_INT1_MASK 0x01
> #define ST_PRESS_LPS25H_DRDY_IRQ_INT2_MASK 0x10
> +#define ST_PRESS_LPS25H_IHL_IRQ_ADDR 0x22
> +#define ST_PRESS_LPS25H_IHL_IRQ_MASK 0x80
> #define ST_PRESS_LPS25H_MULTIREAD_BIT true
> #define ST_PRESS_LPS25H_TEMP_OFFSET 42500
> #define ST_PRESS_LPS25H_OUT_XL_ADDR 0x28
> @@ -220,6 +224,8 @@ static const struct st_sensor_settings st_press_sensors_settings[] = {
> .addr = ST_PRESS_LPS331AP_DRDY_IRQ_ADDR,
> .mask_int1 = ST_PRESS_LPS331AP_DRDY_IRQ_INT1_MASK,
> .mask_int2 = ST_PRESS_LPS331AP_DRDY_IRQ_INT2_MASK,
> + .addr_ihl = ST_PRESS_LPS331AP_IHL_IRQ_ADDR,
> + .mask_ihl = ST_PRESS_LPS331AP_IHL_IRQ_MASK,
> },
> .multi_read_bit = ST_PRESS_LPS331AP_MULTIREAD_BIT,
> .bootime = 2,
> @@ -304,6 +310,8 @@ static const struct st_sensor_settings st_press_sensors_settings[] = {
> .addr = ST_PRESS_LPS25H_DRDY_IRQ_ADDR,
> .mask_int1 = ST_PRESS_LPS25H_DRDY_IRQ_INT1_MASK,
> .mask_int2 = ST_PRESS_LPS25H_DRDY_IRQ_INT2_MASK,
> + .addr_ihl = ST_PRESS_LPS25H_IHL_IRQ_ADDR,
> + .mask_ihl = ST_PRESS_LPS25H_IHL_IRQ_MASK,
> },
> .multi_read_bit = ST_PRESS_LPS25H_MULTIREAD_BIT,
> .bootime = 2,
> diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
> index 2fe939c73cd2..6670c3d25c58 100644
> --- a/include/linux/iio/common/st_sensors.h
> +++ b/include/linux/iio/common/st_sensors.h
> @@ -119,6 +119,8 @@ struct st_sensor_bdu {
> * @addr: address of the register.
> * @mask_int1: mask to enable/disable IRQ on INT1 pin.
> * @mask_int2: mask to enable/disable IRQ on INT2 pin.
> + * @addr_ihl: address to enable/disable active low on the INT lines.
> + * @mask_ihl: mask to enable/disable active low on the INT lines.
> * struct ig1 - represents the Interrupt Generator 1 of sensors.
> * @en_addr: address of the enable ig1 register.
> * @en_mask: mask to write the on/off value for enable.
> @@ -127,6 +129,8 @@ struct st_sensor_data_ready_irq {
> u8 addr;
> u8 mask_int1;
> u8 mask_int2;
> + u8 addr_ihl;
> + u8 mask_ihl;
> struct {
> u8 en_addr;
> u8 en_mask;
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] iio: st_sensors: support active-low interrupts
2015-11-15 10:31 ` Jonathan Cameron
@ 2015-11-15 16:26 ` Linus Walleij
2015-11-15 17:46 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Linus Walleij @ 2015-11-15 16:26 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio@vger.kernel.org, Giuseppe Barba, Denis Ciocca
On Sun, Nov 15, 2015 at 11:31 AM, Jonathan Cameron <jic23@kernel.org> wrote:
>> This patch requires the previously sent patch to switch the
>> driver to request_any_context_irq().
>
> Really requires or will create some fuzz without it? Looks like
> the latter so if we end up discussing that one for a while
> I might reorder. Mind you early in this cycle anyway so
> no rush on this one!
I can put all three patches in a series and put this before that
patch, no problem.
> p.s. If anyone fancies adding support for the venerable
> lis3l02dq as well so we can kill off my ancient driver
> in staging that would be great! (I know I should do it
> myself probably but I can always hope someone else will
> get there first ;)
They are all so similar that I could probably code up
support from the datasheet.
However I have no hardware to test on...
Who submitted that driver in the first place?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] iio: st_sensors: support active-low interrupts
2015-11-15 16:26 ` Linus Walleij
@ 2015-11-15 17:46 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2015-11-15 17:46 UTC (permalink / raw)
To: Linus Walleij; +Cc: linux-iio@vger.kernel.org, Giuseppe Barba, Denis Ciocca
On 15/11/15 16:26, Linus Walleij wrote:
> On Sun, Nov 15, 2015 at 11:31 AM, Jonathan Cameron <jic23@kernel.org> wrote:
>
>>> This patch requires the previously sent patch to switch the
>>> driver to request_any_context_irq().
>>
>> Really requires or will create some fuzz without it? Looks like
>> the latter so if we end up discussing that one for a while
>> I might reorder. Mind you early in this cycle anyway so
>> no rush on this one!
>
> I can put all three patches in a series and put this before that
> patch, no problem.
Might be wise as we can get this one sorted anyway whilst seeing
if we can get some input on the other one from others...
>
>> p.s. If anyone fancies adding support for the venerable
>> lis3l02dq as well so we can kill off my ancient driver
>> in staging that would be great! (I know I should do it
>> myself probably but I can always hope someone else will
>> get there first ;)
>
> They are all so similar that I could probably code up
> support from the datasheet.
>
> However I have no hardware to test on...
>
> Who submitted that driver in the first place?
<tries to look innocent>... That would be me ;)
Back at the dawn of time for IIO - this was one of our original
drivers (the sca3000 was to and that's still in staging too -
that one is a seriously odd beast).
Technically I still have a board with one lis3l02dq on it
somewhere around, but I haven't booted it in a few years and
it has darned silly connectors so wiring it up to something else
would be 'interesting'. Not to mention, if I recall correctly
this chip only gives level interrupts and the board only
has edge triggered ones. Were some nasty games to discover
if new data had turned up whilst you were reading the old stuff.
I'm almost tempted to drop the driver entirely on the basis
the only boards I know ever had one have been unavailable for
a long time and I'm the maintainer of those anyway and given
complete lack of patches for the last 5 years may have the only
ones that even work (probably) any more...
I can always bring it back up if I ever actually make the board
boot again.
p.s. Now it will turn out there are hundreds of people using
intel research stargate 2 imb400 sensor boards....
(they were also compatible and sold by CrossBow for their
imote 2 boards but those went all .net for no apparent reason.
It was interesting hardware. The camera board for instance
had no pull ups on the i2c bus so the tinyos driver that was
the only documentation effectively simply never read back or
checked for acks from the sensor. Gives you an idea of the
fun. Still these were very small boards that well predated
the gumstix etc.
Ah, the glory days when I regularly fired up actual hardware!
Jonathan
>
> Yours,
> Linus Walleij
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] iio: st_sensors: support active-low interrupts
2015-11-13 20:18 [PATCH 1/2] iio: st_sensors: support active-low interrupts Linus Walleij
2015-11-15 10:31 ` Jonathan Cameron
@ 2015-11-16 6:02 ` Denis Ciocca
2015-11-16 8:29 ` Linus Walleij
1 sibling, 1 reply; 6+ messages in thread
From: Denis Ciocca @ 2015-11-16 6:02 UTC (permalink / raw)
To: Linus Walleij; +Cc: Jonathan Cameron, linux-iio@vger.kernel.org, Giuseppe BARBA
Hi Linus,
looks good. Just some comments inline...
On Friday, November 13, 2015 09:18:16 PM Linus Walleij wrote:
> Most ST MEMS Sensors that support interrupts can also handle sending
> an active low interrupt, i.e. going from high to low on data ready
> (or other interrupt) and thus triggering on a falling edge to the
> interrupt controller.
>
> Set up logic to inspect the interrupt line we get for a sensor: if
> it is triggering on rising edge, leave everything alone, but if it
> triggers on falling edges, set up active low, and if unsupported
> configurations appear: warn with errors and reconfigure the interrupt
> to a rising edge, which all interrupt generating sensors support.
>
> Create a local header for st_sensors_core.h to share functions
> between the sensor core and the trigger setup code.
>
> Cc: Giuseppe Barba <giuseppe.barba@st.com>
> Cc: Denis Ciocca <denis.ciocca@st.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> This patch requires the previously sent patch to switch the
> driver to request_any_context_irq().
>
> I am missing a data sheet for the LSM303AGR sensor that Giuseppe
> added recently, so I have no idea what bits to poke to get active
> low IRQs (or open drain) on that sensor.
> ---
> drivers/iio/accel/st_accel_core.c | 16 ++++++++
> drivers/iio/common/st_sensors/st_sensors_core.c | 6 +--
> drivers/iio/common/st_sensors/st_sensors_core.h | 8 ++++
> drivers/iio/common/st_sensors/st_sensors_trigger.c | 44
> ++++++++++++++++++++-- drivers/iio/gyro/st_gyro_core.c |
> 12 ++++++
> drivers/iio/pressure/st_pressure_core.c | 8 ++++
> include/linux/iio/common/st_sensors.h | 4 ++
> 7 files changed, 91 insertions(+), 7 deletions(-)
> create mode 100644 drivers/iio/common/st_sensors/st_sensors_core.h
>
> diff --git a/drivers/iio/accel/st_accel_core.c
> b/drivers/iio/accel/st_accel_core.c index 197a08b4e2f3..1132224cbc10 100644
> --- a/drivers/iio/accel/st_accel_core.c
> +++ b/drivers/iio/accel/st_accel_core.c
> @@ -67,6 +67,8 @@
> #define ST_ACCEL_1_DRDY_IRQ_ADDR 0x22
> #define ST_ACCEL_1_DRDY_IRQ_INT1_MASK 0x10
> #define ST_ACCEL_1_DRDY_IRQ_INT2_MASK 0x08
> +#define ST_ACCEL_1_IHL_IRQ_ADDR 0x25
> +#define ST_ACCEL_1_IHL_IRQ_MASK 0x02
> #define ST_ACCEL_1_MULTIREAD_BIT true
>
> /* CUSTOM VALUES FOR SENSOR 2 */
> @@ -92,6 +94,8 @@
> #define ST_ACCEL_2_DRDY_IRQ_ADDR 0x22
> #define ST_ACCEL_2_DRDY_IRQ_INT1_MASK 0x02
> #define ST_ACCEL_2_DRDY_IRQ_INT2_MASK 0x10
> +#define ST_ACCEL_2_IHL_IRQ_ADDR 0x22
> +#define ST_ACCEL_2_IHL_IRQ_MASK 0x80
> #define ST_ACCEL_2_MULTIREAD_BIT true
>
> /* CUSTOM VALUES FOR SENSOR 3 */
> @@ -125,6 +129,8 @@
> #define ST_ACCEL_3_DRDY_IRQ_ADDR 0x23
> #define ST_ACCEL_3_DRDY_IRQ_INT1_MASK 0x80
> #define ST_ACCEL_3_DRDY_IRQ_INT2_MASK 0x00
> +#define ST_ACCEL_3_IHL_IRQ_ADDR 0x23
> +#define ST_ACCEL_3_IHL_IRQ_MASK 0x40
> #define ST_ACCEL_3_IG1_EN_ADDR 0x23
> #define ST_ACCEL_3_IG1_EN_MASK 0x08
> #define ST_ACCEL_3_MULTIREAD_BIT false
> @@ -169,6 +175,8 @@
> #define ST_ACCEL_5_DRDY_IRQ_ADDR 0x22
> #define ST_ACCEL_5_DRDY_IRQ_INT1_MASK 0x04
> #define ST_ACCEL_5_DRDY_IRQ_INT2_MASK 0x20
> +#define ST_ACCEL_5_IHL_IRQ_ADDR 0x22
> +#define ST_ACCEL_5_IHL_IRQ_MASK 0x80
> #define ST_ACCEL_5_IG1_EN_ADDR 0x21
> #define ST_ACCEL_5_IG1_EN_MASK 0x08
> #define ST_ACCEL_5_MULTIREAD_BIT false
> @@ -291,6 +299,8 @@ static const struct st_sensor_settings
> st_accel_sensors_settings[] = { .addr = ST_ACCEL_1_DRDY_IRQ_ADDR,
> .mask_int1 = ST_ACCEL_1_DRDY_IRQ_INT1_MASK,
> .mask_int2 = ST_ACCEL_1_DRDY_IRQ_INT2_MASK,
> + .addr_ihl = ST_ACCEL_1_IHL_IRQ_ADDR,
> + .mask_ihl = ST_ACCEL_1_IHL_IRQ_MASK,
> },
> .multi_read_bit = ST_ACCEL_1_MULTIREAD_BIT,
> .bootime = 2,
> @@ -354,6 +364,8 @@ static const struct st_sensor_settings
> st_accel_sensors_settings[] = { .addr = ST_ACCEL_2_DRDY_IRQ_ADDR,
> .mask_int1 = ST_ACCEL_2_DRDY_IRQ_INT1_MASK,
> .mask_int2 = ST_ACCEL_2_DRDY_IRQ_INT2_MASK,
> + .addr_ihl = ST_ACCEL_2_IHL_IRQ_ADDR,
> + .mask_ihl = ST_ACCEL_2_IHL_IRQ_MASK,
> },
> .multi_read_bit = ST_ACCEL_2_MULTIREAD_BIT,
> .bootime = 2,
> @@ -429,6 +441,8 @@ static const struct st_sensor_settings
> st_accel_sensors_settings[] = { .addr = ST_ACCEL_3_DRDY_IRQ_ADDR,
> .mask_int1 = ST_ACCEL_3_DRDY_IRQ_INT1_MASK,
> .mask_int2 = ST_ACCEL_3_DRDY_IRQ_INT2_MASK,
> + .addr_ihl = ST_ACCEL_3_IHL_IRQ_ADDR,
> + .mask_ihl = ST_ACCEL_3_IHL_IRQ_MASK,
> .ig1 = {
> .en_addr = ST_ACCEL_3_IG1_EN_ADDR,
> .en_mask = ST_ACCEL_3_IG1_EN_MASK,
> @@ -536,6 +550,8 @@ static const struct st_sensor_settings
> st_accel_sensors_settings[] = { .addr = ST_ACCEL_5_DRDY_IRQ_ADDR,
> .mask_int1 = ST_ACCEL_5_DRDY_IRQ_INT1_MASK,
> .mask_int2 = ST_ACCEL_5_DRDY_IRQ_INT2_MASK,
> + .addr_ihl = ST_ACCEL_5_IHL_IRQ_ADDR,
> + .mask_ihl = ST_ACCEL_5_IHL_IRQ_MASK,
> },
> .multi_read_bit = ST_ACCEL_5_MULTIREAD_BIT,
> .bootime = 2, /* guess */
> diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c
> b/drivers/iio/common/st_sensors/st_sensors_core.c index
> 25258e2c1a82..ed6f54d5c932 100644
> --- a/drivers/iio/common/st_sensors/st_sensors_core.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
> @@ -17,7 +17,7 @@
> #include <linux/of.h>
> #include <asm/unaligned.h>
> #include <linux/iio/common/st_sensors.h>
> -
> +#include "st_sensors_core.h"
>
> #define ST_SENSORS_WAI_ADDRESS 0x0f
>
> @@ -26,8 +26,8 @@ static inline u32 st_sensors_get_unaligned_le24(const u8
> *p) return (s32)((p[0] | p[1] << 8 | p[2] << 16) << 8) >> 8;
> }
>
> -static int st_sensors_write_data_with_mask(struct iio_dev *indio_dev,
> - u8 reg_addr, u8 mask, u8
> data) +int st_sensors_write_data_with_mask(struct iio_dev *indio_dev,
> + u8 reg_addr, u8 mask, u8 data)
> {
> int err;
> u8 new_data;
This function should include EXPORT_SYMBOL() if drivers are used as modules.
> diff --git a/drivers/iio/common/st_sensors/st_sensors_core.h
> b/drivers/iio/common/st_sensors/st_sensors_core.h new file mode 100644
> index 000000000000..cd88098ff6f1
> --- /dev/null
> +++ b/drivers/iio/common/st_sensors/st_sensors_core.h
> @@ -0,0 +1,8 @@
> +/*
> + * Local functions in the ST Sensors core
> + */
> +#ifndef __ST_SENSORS_CORE_H
> +#define __ST_SENSORS_CORE_H
> +int st_sensors_write_data_with_mask(struct iio_dev *indio_dev,
> + u8 reg_addr, u8 mask, u8 data);
> +#endif
> diff --git a/drivers/iio/common/st_sensors/st_sensors_trigger.c
> b/drivers/iio/common/st_sensors/st_sensors_trigger.c index
> e33796cdd607..cb53d14aca94 100644
> --- a/drivers/iio/common/st_sensors/st_sensors_trigger.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_trigger.c
> @@ -14,15 +14,16 @@
> #include <linux/iio/iio.h>
> #include <linux/iio/trigger.h>
> #include <linux/interrupt.h>
> -
> #include <linux/iio/common/st_sensors.h>
> -
> +#include "st_sensors_core.h"
>
> int st_sensors_allocate_trigger(struct iio_dev *indio_dev,
> const struct iio_trigger_ops *trigger_ops)
> {
> int err;
> struct st_sensor_data *sdata = iio_priv(indio_dev);
> + unsigned long irq_trig;
> + int irq;
This is pure style. You can use one line for err and irq variables.
>
> sdata->trig = iio_trigger_alloc("%s-dev%d",
> indio_dev->name, indio_dev->id);
> @@ -32,9 +33,43 @@ int st_sensors_allocate_trigger(struct iio_dev
> *indio_dev, goto iio_trigger_alloc_error;
> }
>
> - err = request_any_context_irq(sdata->get_irq_data_ready(indio_dev),
> + irq = sdata->get_irq_data_ready(indio_dev);
> + irq_trig = irqd_get_trigger_type(irq_get_irq_data(irq));
> + /*
> + * If the IRQ is triggered on falling edge, we need to mark the
> + * interrupt as active low, if the hardware supports this.
> + */
> + if (irq_trig == IRQF_TRIGGER_FALLING) {
> + if (!sdata->sensor_settings->drdy_irq.addr_ihl) {
> + dev_err(&indio_dev->dev,
> + "falling edge specified for IRQ but hardware
> " + "only support rising edge, will request "
> + "rising edge\n");
> + irq_trig = IRQF_TRIGGER_RISING;
> + } else {
> + /* Set up INT active low i.e. falling edge */
> + err = st_sensors_write_data_with_mask(indio_dev,
> + sdata->sensor_settings->drdy_irq.addr_ihl,
> + sdata->sensor_settings->drdy_irq.mask_ihl,
> 1); + if (err < 0)
> + goto iio_trigger_edge_error;
> + dev_info(&indio_dev->dev,
> + "interrupts on the falling edge\n");
> + }
> + } else if (irq_trig == IRQF_TRIGGER_RISING) {
> + dev_info(&indio_dev->dev,
> + "interrupts on the rising edge\n");
> +
> + } else {
> + dev_err(&indio_dev->dev,
> + "unsupported IRQ trigger specified (%lx), only "
> + "rising and falling edges supported, enforce "
> + "rising edge\n", irq_trig);
> + irq_trig = IRQF_TRIGGER_RISING;
> + }
> + err = request_any_context_irq(irq,
> iio_trigger_generic_data_rdy_poll,
> - IRQF_TRIGGER_RISING,
> + irq_trig,
> sdata->trig->name,
> sdata->trig);
> if (err) {
> @@ -59,6 +94,7 @@ iio_trigger_register_error:
> free_irq(sdata->get_irq_data_ready(indio_dev), sdata->trig);
> request_irq_error:
> iio_trigger_free(sdata->trig);
> +iio_trigger_edge_error:
This one should be one line before. When error happen, trigger should be
freed.
Anyway I think it is better to change the label into free_iio_trigger and use
only one instead of two.
If u can remove also iio_trigger_alloc_error and return immediately with the
first one I would be very happy. :)
> iio_trigger_alloc_error:
> return err;
> }
> diff --git a/drivers/iio/gyro/st_gyro_core.c
> b/drivers/iio/gyro/st_gyro_core.c index 02eddcebeea3..260bfe021904 100644
> --- a/drivers/iio/gyro/st_gyro_core.c
> +++ b/drivers/iio/gyro/st_gyro_core.c
> @@ -61,6 +61,8 @@
> #define ST_GYRO_1_BDU_MASK 0x80
> #define ST_GYRO_1_DRDY_IRQ_ADDR 0x22
> #define ST_GYRO_1_DRDY_IRQ_INT2_MASK 0x08
> +#define ST_GYRO_1_IHL_IRQ_ADDR 0x22
> +#define ST_GYRO_1_IHL_IRQ_MASK 0x20
> #define ST_GYRO_1_MULTIREAD_BIT true
>
> /* CUSTOM VALUES FOR SENSOR 2 */
> @@ -85,6 +87,8 @@
> #define ST_GYRO_2_BDU_MASK 0x80
> #define ST_GYRO_2_DRDY_IRQ_ADDR 0x22
> #define ST_GYRO_2_DRDY_IRQ_INT2_MASK 0x08
> +#define ST_GYRO_2_IHL_IRQ_ADDR 0x22
> +#define ST_GYRO_2_IHL_IRQ_MASK 0x20
> #define ST_GYRO_2_MULTIREAD_BIT true
>
> /* CUSTOM VALUES FOR SENSOR 3 */
> @@ -109,6 +113,8 @@
> #define ST_GYRO_3_BDU_MASK 0x80
> #define ST_GYRO_3_DRDY_IRQ_ADDR 0x22
> #define ST_GYRO_3_DRDY_IRQ_INT2_MASK 0x08
> +#define ST_GYRO_3_IHL_IRQ_ADDR 0x22
> +#define ST_GYRO_3_IHL_IRQ_MASK 0x20
> #define ST_GYRO_3_MULTIREAD_BIT true
>
>
> @@ -185,6 +191,8 @@ static const struct st_sensor_settings
> st_gyro_sensors_settings[] = { .drdy_irq = {
> .addr = ST_GYRO_1_DRDY_IRQ_ADDR,
> .mask_int2 = ST_GYRO_1_DRDY_IRQ_INT2_MASK,
> + .addr_ihl = ST_GYRO_1_IHL_IRQ_ADDR,
> + .mask_ihl = ST_GYRO_1_IHL_IRQ_MASK,
> },
> .multi_read_bit = ST_GYRO_1_MULTIREAD_BIT,
> .bootime = 2,
> @@ -248,6 +256,8 @@ static const struct st_sensor_settings
> st_gyro_sensors_settings[] = { .drdy_irq = {
> .addr = ST_GYRO_2_DRDY_IRQ_ADDR,
> .mask_int2 = ST_GYRO_2_DRDY_IRQ_INT2_MASK,
> + .addr_ihl = ST_GYRO_2_IHL_IRQ_ADDR,
> + .mask_ihl = ST_GYRO_2_IHL_IRQ_MASK,
> },
> .multi_read_bit = ST_GYRO_2_MULTIREAD_BIT,
> .bootime = 2,
> @@ -307,6 +317,8 @@ static const struct st_sensor_settings
> st_gyro_sensors_settings[] = { .drdy_irq = {
> .addr = ST_GYRO_3_DRDY_IRQ_ADDR,
> .mask_int2 = ST_GYRO_3_DRDY_IRQ_INT2_MASK,
> + .addr_ihl = ST_GYRO_3_IHL_IRQ_ADDR,
> + .mask_ihl = ST_GYRO_3_IHL_IRQ_MASK,
> },
> .multi_read_bit = ST_GYRO_3_MULTIREAD_BIT,
> .bootime = 2,
> diff --git a/drivers/iio/pressure/st_pressure_core.c
> b/drivers/iio/pressure/st_pressure_core.c index b39a2fb0671c..172393ad34af
> 100644
> --- a/drivers/iio/pressure/st_pressure_core.c
> +++ b/drivers/iio/pressure/st_pressure_core.c
> @@ -62,6 +62,8 @@
> #define ST_PRESS_LPS331AP_DRDY_IRQ_ADDR 0x22
> #define ST_PRESS_LPS331AP_DRDY_IRQ_INT1_MASK 0x04
> #define ST_PRESS_LPS331AP_DRDY_IRQ_INT2_MASK 0x20
> +#define ST_PRESS_LPS331AP_IHL_IRQ_ADDR 0x22
> +#define ST_PRESS_LPS331AP_IHL_IRQ_MASK 0x80
> #define ST_PRESS_LPS331AP_MULTIREAD_BIT true
> #define ST_PRESS_LPS331AP_TEMP_OFFSET 42500
>
> @@ -100,6 +102,8 @@
> #define ST_PRESS_LPS25H_DRDY_IRQ_ADDR 0x23
> #define ST_PRESS_LPS25H_DRDY_IRQ_INT1_MASK 0x01
> #define ST_PRESS_LPS25H_DRDY_IRQ_INT2_MASK 0x10
> +#define ST_PRESS_LPS25H_IHL_IRQ_ADDR 0x22
> +#define ST_PRESS_LPS25H_IHL_IRQ_MASK 0x80
> #define ST_PRESS_LPS25H_MULTIREAD_BIT true
> #define ST_PRESS_LPS25H_TEMP_OFFSET 42500
> #define ST_PRESS_LPS25H_OUT_XL_ADDR 0x28
> @@ -220,6 +224,8 @@ static const struct st_sensor_settings
> st_press_sensors_settings[] = { .addr = ST_PRESS_LPS331AP_DRDY_IRQ_ADDR,
> .mask_int1 = ST_PRESS_LPS331AP_DRDY_IRQ_INT1_MASK,
> .mask_int2 = ST_PRESS_LPS331AP_DRDY_IRQ_INT2_MASK,
> + .addr_ihl = ST_PRESS_LPS331AP_IHL_IRQ_ADDR,
> + .mask_ihl = ST_PRESS_LPS331AP_IHL_IRQ_MASK,
> },
> .multi_read_bit = ST_PRESS_LPS331AP_MULTIREAD_BIT,
> .bootime = 2,
> @@ -304,6 +310,8 @@ static const struct st_sensor_settings
> st_press_sensors_settings[] = { .addr = ST_PRESS_LPS25H_DRDY_IRQ_ADDR,
> .mask_int1 = ST_PRESS_LPS25H_DRDY_IRQ_INT1_MASK,
> .mask_int2 = ST_PRESS_LPS25H_DRDY_IRQ_INT2_MASK,
> + .addr_ihl = ST_PRESS_LPS25H_IHL_IRQ_ADDR,
> + .mask_ihl = ST_PRESS_LPS25H_IHL_IRQ_MASK,
> },
> .multi_read_bit = ST_PRESS_LPS25H_MULTIREAD_BIT,
> .bootime = 2,
> diff --git a/include/linux/iio/common/st_sensors.h
> b/include/linux/iio/common/st_sensors.h index 2fe939c73cd2..6670c3d25c58
> 100644
> --- a/include/linux/iio/common/st_sensors.h
> +++ b/include/linux/iio/common/st_sensors.h
> @@ -119,6 +119,8 @@ struct st_sensor_bdu {
> * @addr: address of the register.
> * @mask_int1: mask to enable/disable IRQ on INT1 pin.
> * @mask_int2: mask to enable/disable IRQ on INT2 pin.
> + * @addr_ihl: address to enable/disable active low on the INT lines.
> + * @mask_ihl: mask to enable/disable active low on the INT lines.
> * struct ig1 - represents the Interrupt Generator 1 of sensors.
> * @en_addr: address of the enable ig1 register.
> * @en_mask: mask to write the on/off value for enable.
> @@ -127,6 +129,8 @@ struct st_sensor_data_ready_irq {
> u8 addr;
> u8 mask_int1;
> u8 mask_int2;
> + u8 addr_ihl;
> + u8 mask_ihl;
> struct {
> u8 en_addr;
> u8 en_mask;
> --
> 2.4.3
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] iio: st_sensors: support active-low interrupts
2015-11-16 6:02 ` Denis Ciocca
@ 2015-11-16 8:29 ` Linus Walleij
0 siblings, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2015-11-16 8:29 UTC (permalink / raw)
To: Denis Ciocca; +Cc: Jonathan Cameron, linux-iio@vger.kernel.org, Giuseppe BARBA
On Mon, Nov 16, 2015 at 7:02 AM, Denis Ciocca <denis.ciocca@st.com> wrote:
>> -static int st_sensors_write_data_with_mask(struct iio_dev *indio_dev,
>> - u8 reg_addr, u8 mask, u8
>> data) +int st_sensors_write_data_with_mask(struct iio_dev *indio_dev,
>> + u8 reg_addr, u8 mask, u8 data)
>> {
>> int err;
>> u8 new_data;
>
> This function should include EXPORT_SYMBOL() if drivers are used as modules.
Not a problem:
st_sensors-y := st_sensors_core.o
st_sensors-$(CONFIG_IIO_BUFFER) += st_sensors_buffer.o
st_sensors-$(CONFIG_IIO_TRIGGER) += st_sensors_trigger.o
And CONFIG_IIO_TRIGGER is a bool.
So if it is used, it is always baked into st_sensors_core.o.
(And that is why I chose to create a local header for it.)
Fixing the rest of the problems, thanks Denis!
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-11-16 8:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-13 20:18 [PATCH 1/2] iio: st_sensors: support active-low interrupts Linus Walleij
2015-11-15 10:31 ` Jonathan Cameron
2015-11-15 16:26 ` Linus Walleij
2015-11-15 17:46 ` Jonathan Cameron
2015-11-16 6:02 ` Denis Ciocca
2015-11-16 8:29 ` Linus Walleij
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox