[PATCH 4/4] iio: st_sensors: support open drain mode

[PATCH 4/4] iio: st_sensors: support open drain mode

[Linus Walleij]

Some types of ST Sensors can be connected to the same IRQ line
as other peripherals using open drain. Add a device tree binding
and a sensor data property to flip the right bit in the interrupt
control register to enable open drain mode on the INT line.

If the line is set to be open drain, also tag on IRQF_SHARED
to the IRQ flags when requesting the interrupt, as the whole
point of using open drain interrupt lines is to share them with
more than one peripheral (wire-or).

Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Giuseppe Barba <giuseppe.barba-qxv4g6HH51o@public.gmane.org>
Cc: Denis Ciocca <denis.ciocca-qxv4g6HH51o@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
ChangeLog v2->v3:
- Rebase on top of the patches fixing the other issues (handling
  IRQ status check and channel reading bug).
ChangeLog v1->v2:
- Rebased to fit the new patch order.
---
 Documentation/devicetree/bindings/iio/st-sensors.txt |  3 +++
 drivers/iio/accel/st_accel_core.c                    |  8 ++++++++
 drivers/iio/common/st_sensors/st_sensors_core.c      | 20 ++++++++++++++++++++
 drivers/iio/common/st_sensors/st_sensors_trigger.c   | 13 +++++++++++++
 drivers/iio/pressure/st_pressure_core.c              |  8 ++++++++
 include/linux/iio/common/st_sensors.h                |  6 ++++++
 include/linux/platform_data/st_sensors_pdata.h       |  2 ++
 7 files changed, 60 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/st-sensors.txt b/Documentation/devicetree/bindings/iio/st-sensors.txt
index d4b87cc1e446..accb5681fbbb 100644
--- a/Documentation/devicetree/bindings/iio/st-sensors.txt
+++ b/Documentation/devicetree/bindings/iio/st-sensors.txt
@@ -16,6 +16,9 @@ Optional properties:
 - st,drdy-int-pin: the pin on the package that will be used to signal
   "data ready" (valid values: 1 or 2). This property is not configurable
   on all sensors.
+- st,int-pin-open-drain: the interrupt/data ready line will be configured
+  as open drain, which is useful if several sensors share the same
+  interrupt line.
 
 Sensors may also have applicable pin control settings, those use the
 standard bindings from pinctrl/pinctrl-bindings.txt.
diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
index 4df4c278af23..f06f4329db5b 100644
--- a/drivers/iio/accel/st_accel_core.c
+++ b/drivers/iio/accel/st_accel_core.c
@@ -96,6 +96,8 @@
 #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_OD_IRQ_ADDR			0x22
+#define ST_ACCEL_2_OD_IRQ_MASK			0x40
 #define ST_ACCEL_2_MULTIREAD_BIT		true
 
 /* CUSTOM VALUES FOR SENSOR 3 */
@@ -177,6 +179,8 @@
 #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_OD_IRQ_ADDR			0x22
+#define ST_ACCEL_5_OD_IRQ_MASK			0x40
 #define ST_ACCEL_5_IG1_EN_ADDR			0x21
 #define ST_ACCEL_5_IG1_EN_MASK			0x08
 #define ST_ACCEL_5_MULTIREAD_BIT		false
@@ -368,6 +372,8 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = {
 			.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,
+			.addr_od = ST_ACCEL_2_OD_IRQ_ADDR,
+			.mask_od = ST_ACCEL_2_OD_IRQ_MASK,
 			.addr_stat_drdy = ST_SENSORS_DEFAULT_STAT_ADDR,
 		},
 		.multi_read_bit = ST_ACCEL_2_MULTIREAD_BIT,
@@ -557,6 +563,8 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = {
 			.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,
+			.addr_od = ST_ACCEL_5_OD_IRQ_ADDR,
+			.mask_od = ST_ACCEL_5_OD_IRQ_MASK,
 			.addr_stat_drdy = ST_SENSORS_DEFAULT_STAT_ADDR,
 		},
 		.multi_read_bit = ST_ACCEL_5_MULTIREAD_BIT,
diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
index f5a2d445d0c0..2e03c1220d1f 100644
--- a/drivers/iio/common/st_sensors/st_sensors_core.c
+++ b/drivers/iio/common/st_sensors/st_sensors_core.c
@@ -301,6 +301,14 @@ static int st_sensors_set_drdy_int_pin(struct iio_dev *indio_dev,
 		return -EINVAL;
 	}
 
+	if (pdata->open_drain) {
+		if (!sdata->sensor_settings->drdy_irq.addr_od)
+			dev_err(&indio_dev->dev,
+				"open drain requested but unsupported.\n");
+		else
+			sdata->int_pin_open_drain = true;
+	}
+
 	return 0;
 }
 
@@ -321,6 +329,8 @@ static struct st_sensors_platform_data *st_sensors_of_probe(struct device *dev,
 	else
 		pdata->drdy_int_pin = defdata ? defdata->drdy_int_pin : 0;
 
+	pdata->open_drain = of_property_read_bool(np, "st,int-pin-open-drain");
+
 	return pdata;
 }
 #else
@@ -374,6 +384,16 @@ int st_sensors_init_sensor(struct iio_dev *indio_dev,
 			return err;
 	}
 
+	if (sdata->int_pin_open_drain) {
+		dev_info(&indio_dev->dev,
+			 "set interrupt line to open drain mode\n");
+		err = st_sensors_write_data_with_mask(indio_dev,
+				sdata->sensor_settings->drdy_irq.addr_od,
+				sdata->sensor_settings->drdy_irq.mask_od, 1);
+		if (err < 0)
+			return err;
+	}
+
 	err = st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
 
 	return err;
diff --git a/drivers/iio/common/st_sensors/st_sensors_trigger.c b/drivers/iio/common/st_sensors/st_sensors_trigger.c
index 6a8c98327945..da72279fcf99 100644
--- a/drivers/iio/common/st_sensors/st_sensors_trigger.c
+++ b/drivers/iio/common/st_sensors/st_sensors_trigger.c
@@ -64,6 +64,19 @@ int st_sensors_allocate_trigger(struct iio_dev *indio_dev,
 			"rising edge\n", irq_trig);
 		irq_trig = IRQF_TRIGGER_RISING;
 	}
+
+	/*
+	 * If the interrupt pin is Open Drain, by definition this
+	 * means that the interrupt line may be shared with other
+	 * peripherals. But to do this we also need to have a status
+	 * register and mask to figure out if this sensor was firing
+	 * the IRQ or not, so we can tell the interrupt handle that
+	 * it was "our" interrupt.
+	 */
+	if (sdata->int_pin_open_drain &&
+	    sdata->sensor_settings->drdy_irq.addr_stat_drdy)
+		irq_trig |= IRQF_SHARED;
+
 	err = request_threaded_irq(irq,
 			iio_trigger_generic_data_rdy_poll,
 			NULL,
diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
index 1cd37eaa4a57..9e9b72a8f18f 100644
--- a/drivers/iio/pressure/st_pressure_core.c
+++ b/drivers/iio/pressure/st_pressure_core.c
@@ -64,6 +64,8 @@
 #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_OD_IRQ_ADDR		0x22
+#define ST_PRESS_LPS331AP_OD_IRQ_MASK		0x40
 #define ST_PRESS_LPS331AP_MULTIREAD_BIT		true
 #define ST_PRESS_LPS331AP_TEMP_OFFSET		42500
 
@@ -104,6 +106,8 @@
 #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_OD_IRQ_ADDR		0x22
+#define ST_PRESS_LPS25H_OD_IRQ_MASK		0x40
 #define ST_PRESS_LPS25H_MULTIREAD_BIT		true
 #define ST_PRESS_LPS25H_TEMP_OFFSET		42500
 #define ST_PRESS_LPS25H_OUT_XL_ADDR		0x28
@@ -226,6 +230,8 @@ static const struct st_sensor_settings st_press_sensors_settings[] = {
 			.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,
+			.addr_od = ST_PRESS_LPS331AP_OD_IRQ_ADDR,
+			.mask_od = ST_PRESS_LPS331AP_OD_IRQ_MASK,
 			.addr_stat_drdy = ST_SENSORS_DEFAULT_STAT_ADDR,
 		},
 		.multi_read_bit = ST_PRESS_LPS331AP_MULTIREAD_BIT,
@@ -313,6 +319,8 @@ static const struct st_sensor_settings st_press_sensors_settings[] = {
 			.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,
+			.addr_od = ST_PRESS_LPS25H_OD_IRQ_ADDR,
+			.mask_od = ST_PRESS_LPS25H_OD_IRQ_MASK,
 			.addr_stat_drdy = ST_SENSORS_DEFAULT_STAT_ADDR,
 		},
 		.multi_read_bit = ST_PRESS_LPS25H_MULTIREAD_BIT,
diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
index d8da075bfda0..d029ffac0d69 100644
--- a/include/linux/iio/common/st_sensors.h
+++ b/include/linux/iio/common/st_sensors.h
@@ -122,6 +122,8 @@ struct st_sensor_bdu {
  * @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.
+ * @addr_od: address to enable/disable Open Drain on the INT lines.
+ * @mask_od: mask to enable/disable Open Drain on the INT lines.
  * @addr_stat_drdy: address to read status of DRDY (data ready) interrupt
  * struct ig1 - represents the Interrupt Generator 1 of sensors.
  * @en_addr: address of the enable ig1 register.
@@ -133,6 +135,8 @@ struct st_sensor_data_ready_irq {
 	u8 mask_int2;
 	u8 addr_ihl;
 	u8 mask_ihl;
+	u8 addr_od;
+	u8 mask_od;
 	u8 addr_stat_drdy;
 	struct {
 		u8 en_addr;
@@ -215,6 +219,7 @@ struct st_sensor_settings {
  * @odr: Output data rate of the sensor [Hz].
  * num_data_channels: Number of data channels used in buffer.
  * @drdy_int_pin: Redirect DRDY on pin 1 (1) or pin 2 (2).
+ * @int_pin_open_drain: Set the interrupt/DRDY to open drain.
  * @get_irq_data_ready: Function to get the IRQ used for data ready signal.
  * @tf: Transfer function structure used by I/O operations.
  * @tb: Transfer buffers and mutex used by I/O operations.
@@ -236,6 +241,7 @@ struct st_sensor_data {
 	unsigned int num_data_channels;
 
 	u8 drdy_int_pin;
+	bool int_pin_open_drain;
 
 	unsigned int (*get_irq_data_ready) (struct iio_dev *indio_dev);
 
diff --git a/include/linux/platform_data/st_sensors_pdata.h b/include/linux/platform_data/st_sensors_pdata.h
index 753839187ba0..79b0e4cdb814 100644
--- a/include/linux/platform_data/st_sensors_pdata.h
+++ b/include/linux/platform_data/st_sensors_pdata.h
@@ -16,9 +16,11 @@
  * @drdy_int_pin: Redirect DRDY on pin 1 (1) or pin 2 (2).
  *	Available only for accelerometer and pressure sensors.
  *	Accelerometer DRDY on LSM330 available only on pin 1 (see datasheet).
+ * @open_drain: set the interrupt line to be open drain if possible.
  */
 struct st_sensors_platform_data {
 	u8 drdy_int_pin;
+	bool open_drain;
 };
 
 #endif /* ST_SENSORS_PDATA_H */
-- 
2.4.3

Re: [PATCH 4/4] iio: st_sensors: support open drain mode

[Jonathan Cameron]

On 24/03/16 13:18, Linus Walleij wrote:
> Some types of ST Sensors can be connected to the same IRQ line
> as other peripherals using open drain. Add a device tree binding
> and a sensor data property to flip the right bit in the interrupt
> control register to enable open drain mode on the INT line.
> 
> If the line is set to be open drain, also tag on IRQF_SHARED
> to the IRQ flags when requesting the interrupt, as the whole
> point of using open drain interrupt lines is to share them with
> more than one peripheral (wire-or).
> 
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: Giuseppe Barba <giuseppe.barba-qxv4g6HH51o@public.gmane.org>
> Cc: Denis Ciocca <denis.ciocca-qxv4g6HH51o@public.gmane.org>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Few comments inline but looks fine to me.  Ideally again an
ack from Denis would be good. Mostly this one is stalled behind the earlier
ones.

Jonathan
> ---
> ChangeLog v2->v3:
> - Rebase on top of the patches fixing the other issues (handling
>   IRQ status check and channel reading bug).
> ChangeLog v1->v2:
> - Rebased to fit the new patch order.
> ---
>  Documentation/devicetree/bindings/iio/st-sensors.txt |  3 +++
>  drivers/iio/accel/st_accel_core.c                    |  8 ++++++++
>  drivers/iio/common/st_sensors/st_sensors_core.c      | 20 ++++++++++++++++++++
>  drivers/iio/common/st_sensors/st_sensors_trigger.c   | 13 +++++++++++++
>  drivers/iio/pressure/st_pressure_core.c              |  8 ++++++++
>  include/linux/iio/common/st_sensors.h                |  6 ++++++
>  include/linux/platform_data/st_sensors_pdata.h       |  2 ++
>  7 files changed, 60 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/st-sensors.txt b/Documentation/devicetree/bindings/iio/st-sensors.txt
> index d4b87cc1e446..accb5681fbbb 100644
> --- a/Documentation/devicetree/bindings/iio/st-sensors.txt
> +++ b/Documentation/devicetree/bindings/iio/st-sensors.txt
> @@ -16,6 +16,9 @@ Optional properties:
>  - st,drdy-int-pin: the pin on the package that will be used to signal
>    "data ready" (valid values: 1 or 2). This property is not configurable
>    on all sensors.
> +- st,int-pin-open-drain: the interrupt/data ready line will be configured
> +  as open drain, which is useful if several sensors share the same
> +  interrupt line.
Really feels like this one ought to be more generic.. Mind you so does
drdy-int-pin!

Hmm. Looks like most open drain controls are chip specific - other than
your pinctrl ones and that doesn't really map to here I guess.


>  
>  Sensors may also have applicable pin control settings, those use the
>  standard bindings from pinctrl/pinctrl-bindings.txt.
> diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
> index 4df4c278af23..f06f4329db5b 100644
> --- a/drivers/iio/accel/st_accel_core.c
> +++ b/drivers/iio/accel/st_accel_core.c
> @@ -96,6 +96,8 @@
>  #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_OD_IRQ_ADDR			0x22
> +#define ST_ACCEL_2_OD_IRQ_MASK			0x40
>  #define ST_ACCEL_2_MULTIREAD_BIT		true
>  
>  /* CUSTOM VALUES FOR SENSOR 3 */
> @@ -177,6 +179,8 @@
>  #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_OD_IRQ_ADDR			0x22
> +#define ST_ACCEL_5_OD_IRQ_MASK			0x40
>  #define ST_ACCEL_5_IG1_EN_ADDR			0x21
>  #define ST_ACCEL_5_IG1_EN_MASK			0x08
>  #define ST_ACCEL_5_MULTIREAD_BIT		false
> @@ -368,6 +372,8 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = {
>  			.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,
> +			.addr_od = ST_ACCEL_2_OD_IRQ_ADDR,
> +			.mask_od = ST_ACCEL_2_OD_IRQ_MASK,
>  			.addr_stat_drdy = ST_SENSORS_DEFAULT_STAT_ADDR,
>  		},
>  		.multi_read_bit = ST_ACCEL_2_MULTIREAD_BIT,
> @@ -557,6 +563,8 @@ static const struct st_sensor_settings st_accel_sensors_settings[] = {
>  			.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,
> +			.addr_od = ST_ACCEL_5_OD_IRQ_ADDR,
> +			.mask_od = ST_ACCEL_5_OD_IRQ_MASK,
>  			.addr_stat_drdy = ST_SENSORS_DEFAULT_STAT_ADDR,
>  		},
>  		.multi_read_bit = ST_ACCEL_5_MULTIREAD_BIT,
> diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
> index f5a2d445d0c0..2e03c1220d1f 100644
> --- a/drivers/iio/common/st_sensors/st_sensors_core.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
> @@ -301,6 +301,14 @@ static int st_sensors_set_drdy_int_pin(struct iio_dev *indio_dev,
>  		return -EINVAL;
>  	}
>  
> +	if (pdata->open_drain) {
> +		if (!sdata->sensor_settings->drdy_irq.addr_od)
> +			dev_err(&indio_dev->dev,
> +				"open drain requested but unsupported.\n");
> +		else
> +			sdata->int_pin_open_drain = true;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -321,6 +329,8 @@ static struct st_sensors_platform_data *st_sensors_of_probe(struct device *dev,
>  	else
>  		pdata->drdy_int_pin = defdata ? defdata->drdy_int_pin : 0;
>  
> +	pdata->open_drain = of_property_read_bool(np, "st,int-pin-open-drain");
> +
>  	return pdata;
>  }
>  #else
> @@ -374,6 +384,16 @@ int st_sensors_init_sensor(struct iio_dev *indio_dev,
>  			return err;
>  	}
>  
> +	if (sdata->int_pin_open_drain) {
> +		dev_info(&indio_dev->dev,
> +			 "set interrupt line to open drain mode\n");
Hmm. Curriously I have a board with an lis3l02dq on it where the interrupt
is shared, but doesn't explicitly support open drain. In that case it
is done with some external circuitry.

Anyhow - I guess when I actually add support for that I'll just add a sanity
check in here that addr_od and mask_od are specified :)


> +		err = st_sensors_write_data_with_mask(indio_dev,
> +				sdata->sensor_settings->drdy_irq.addr_od,
> +				sdata->sensor_settings->drdy_irq.mask_od, 1);
> +		if (err < 0)
> +			return err;
> +	}
> +
>  	err = st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
>  
>  	return err;
> diff --git a/drivers/iio/common/st_sensors/st_sensors_trigger.c b/drivers/iio/common/st_sensors/st_sensors_trigger.c
> index 6a8c98327945..da72279fcf99 100644
> --- a/drivers/iio/common/st_sensors/st_sensors_trigger.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_trigger.c
> @@ -64,6 +64,19 @@ int st_sensors_allocate_trigger(struct iio_dev *indio_dev,
>  			"rising edge\n", irq_trig);
>  		irq_trig = IRQF_TRIGGER_RISING;
>  	}
> +
> +	/*
> +	 * If the interrupt pin is Open Drain, by definition this
> +	 * means that the interrupt line may be shared with other
> +	 * peripherals. But to do this we also need to have a status
> +	 * register and mask to figure out if this sensor was firing
> +	 * the IRQ or not, so we can tell the interrupt handle that
> +	 * it was "our" interrupt.
> +	 */
> +	if (sdata->int_pin_open_drain &&
> +	    sdata->sensor_settings->drdy_irq.addr_stat_drdy)
> +		irq_trig |= IRQF_SHARED;
> +
>  	err = request_threaded_irq(irq,
>  			iio_trigger_generic_data_rdy_poll,
>  			NULL,
> diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
> index 1cd37eaa4a57..9e9b72a8f18f 100644
> --- a/drivers/iio/pressure/st_pressure_core.c
> +++ b/drivers/iio/pressure/st_pressure_core.c
> @@ -64,6 +64,8 @@
>  #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_OD_IRQ_ADDR		0x22
> +#define ST_PRESS_LPS331AP_OD_IRQ_MASK		0x40
>  #define ST_PRESS_LPS331AP_MULTIREAD_BIT		true
>  #define ST_PRESS_LPS331AP_TEMP_OFFSET		42500
>  
> @@ -104,6 +106,8 @@
>  #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_OD_IRQ_ADDR		0x22
> +#define ST_PRESS_LPS25H_OD_IRQ_MASK		0x40
>  #define ST_PRESS_LPS25H_MULTIREAD_BIT		true
>  #define ST_PRESS_LPS25H_TEMP_OFFSET		42500
>  #define ST_PRESS_LPS25H_OUT_XL_ADDR		0x28
> @@ -226,6 +230,8 @@ static const struct st_sensor_settings st_press_sensors_settings[] = {
>  			.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,
> +			.addr_od = ST_PRESS_LPS331AP_OD_IRQ_ADDR,
> +			.mask_od = ST_PRESS_LPS331AP_OD_IRQ_MASK,
>  			.addr_stat_drdy = ST_SENSORS_DEFAULT_STAT_ADDR,
>  		},
>  		.multi_read_bit = ST_PRESS_LPS331AP_MULTIREAD_BIT,
> @@ -313,6 +319,8 @@ static const struct st_sensor_settings st_press_sensors_settings[] = {
>  			.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,
> +			.addr_od = ST_PRESS_LPS25H_OD_IRQ_ADDR,
> +			.mask_od = ST_PRESS_LPS25H_OD_IRQ_MASK,
>  			.addr_stat_drdy = ST_SENSORS_DEFAULT_STAT_ADDR,
>  		},
>  		.multi_read_bit = ST_PRESS_LPS25H_MULTIREAD_BIT,
> diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
> index d8da075bfda0..d029ffac0d69 100644
> --- a/include/linux/iio/common/st_sensors.h
> +++ b/include/linux/iio/common/st_sensors.h
> @@ -122,6 +122,8 @@ struct st_sensor_bdu {
>   * @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.
> + * @addr_od: address to enable/disable Open Drain on the INT lines.
> + * @mask_od: mask to enable/disable Open Drain on the INT lines.
>   * @addr_stat_drdy: address to read status of DRDY (data ready) interrupt
>   * struct ig1 - represents the Interrupt Generator 1 of sensors.
>   * @en_addr: address of the enable ig1 register.
> @@ -133,6 +135,8 @@ struct st_sensor_data_ready_irq {
>  	u8 mask_int2;
>  	u8 addr_ihl;
>  	u8 mask_ihl;
> +	u8 addr_od;
> +	u8 mask_od;
>  	u8 addr_stat_drdy;
>  	struct {
>  		u8 en_addr;
> @@ -215,6 +219,7 @@ struct st_sensor_settings {
>   * @odr: Output data rate of the sensor [Hz].
>   * num_data_channels: Number of data channels used in buffer.
>   * @drdy_int_pin: Redirect DRDY on pin 1 (1) or pin 2 (2).
> + * @int_pin_open_drain: Set the interrupt/DRDY to open drain.
>   * @get_irq_data_ready: Function to get the IRQ used for data ready signal.
>   * @tf: Transfer function structure used by I/O operations.
>   * @tb: Transfer buffers and mutex used by I/O operations.
> @@ -236,6 +241,7 @@ struct st_sensor_data {
>  	unsigned int num_data_channels;
>  
>  	u8 drdy_int_pin;
> +	bool int_pin_open_drain;
>  
>  	unsigned int (*get_irq_data_ready) (struct iio_dev *indio_dev);
>  
> diff --git a/include/linux/platform_data/st_sensors_pdata.h b/include/linux/platform_data/st_sensors_pdata.h
> index 753839187ba0..79b0e4cdb814 100644
> --- a/include/linux/platform_data/st_sensors_pdata.h
> +++ b/include/linux/platform_data/st_sensors_pdata.h
> @@ -16,9 +16,11 @@
>   * @drdy_int_pin: Redirect DRDY on pin 1 (1) or pin 2 (2).
>   *	Available only for accelerometer and pressure sensors.
>   *	Accelerometer DRDY on LSM330 available only on pin 1 (see datasheet).
> + * @open_drain: set the interrupt line to be open drain if possible.
>   */
>  struct st_sensors_platform_data {
>  	u8 drdy_int_pin;
> +	bool open_drain;
>  };
>  
>  #endif /* ST_SENSORS_PDATA_H */
> 

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

Re: [PATCH 4/4] iio: st_sensors: support open drain mode

[Linus Walleij]

On Mon, Mar 28, 2016 at 11:12 AM, Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On 24/03/16 13:18, Linus Walleij wrote:

>> +- st,int-pin-open-drain: the interrupt/data ready line will be configured
>> +  as open drain, which is useful if several sensors share the same
>> +  interrupt line.
>
> Really feels like this one ought to be more generic.. Mind you so does
> drdy-int-pin!

Sorry about drdy-int-pin, it wasn't me ;)

That one is a mux kind of thing: interrupt goes out on IRQ line
INT1 or INT2, select one...

> Hmm. Looks like most open drain controls are chip specific - other than
> your pinctrl ones and that doesn't really map to here I guess.

Both the muxing and the OD setting can be done with pin control,
it's just that it is like swatting a fly with a skyscraper - vast overkill
for two interrupt lines.

I came to the conclusion that with small chips or IP blocks just
randomly doing some muxing or biasing of a few pins isn't necessarily
worth all the trouble of implementing pin control, the latter was
conceived for real big arrays of pins really. There are already a
few of these all over the kernel, e.g. the ST Multi-Purpose expander
drivers/mfd/stmpe.c - could be pin control muxed but it'd be overkill.

So as pin control maintainer when this kind of hardware comes up
I usually just "think it over" and then "OK then".

>> +     if (sdata->int_pin_open_drain) {
>> +             dev_info(&indio_dev->dev,
>> +                      "set interrupt line to open drain mode\n");
>
> Hmm. Curriously I have a board with an lis3l02dq on it where the interrupt
> is shared, but doesn't explicitly support open drain. In that case it
> is done with some external circuitry.

That should run fine with just the status read patch from earlier
in the series I guess?

> Anyhow - I guess when I actually add support for that I'll just add a sanity
> check in here that addr_od and mask_od are specified :)

Hm, yeah. Otherwise just patch out my enforcement of the
OD flag for a shared line and add a comment in.

Yours,
Linus Walleij

Re: [PATCH 4/4] iio: st_sensors: support open drain mode

[Jonathan Cameron]

On 31/03/16 09:15, Linus Walleij wrote:
> On Mon, Mar 28, 2016 at 11:12 AM, Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> On 24/03/16 13:18, Linus Walleij wrote:
> 
>>> +- st,int-pin-open-drain: the interrupt/data ready line will be configured
>>> +  as open drain, which is useful if several sensors share the same
>>> +  interrupt line.
>>
>> Really feels like this one ought to be more generic.. Mind you so does
>> drdy-int-pin!
> 
> Sorry about drdy-int-pin, it wasn't me ;)
> 
> That one is a mux kind of thing: interrupt goes out on IRQ line
> INT1 or INT2, select one...
> 
>> Hmm. Looks like most open drain controls are chip specific - other than
>> your pinctrl ones and that doesn't really map to here I guess.
> 
> Both the muxing and the OD setting can be done with pin control,
> it's just that it is like swatting a fly with a skyscraper - vast overkill
> for two interrupt lines.
> 
> I came to the conclusion that with small chips or IP blocks just
> randomly doing some muxing or biasing of a few pins isn't necessarily
> worth all the trouble of implementing pin control, the latter was
> conceived for real big arrays of pins really. There are already a
> few of these all over the kernel, e.g. the ST Multi-Purpose expander
> drivers/mfd/stmpe.c - could be pin control muxed but it'd be overkill.
> 
> So as pin control maintainer when this kind of hardware comes up
> I usually just "think it over" and then "OK then".
I can indeed see the advantages of that flexibility, but from an IIO point
of view, we are going to have a number of very similar bindings.  I can't
see a reason not to generalize the form of the binding away from a single
driver.

So lets drop the vendor prefix from this one and have
int-pin-open-drain (and drdy-int-pin ideally).
> 
>>> +     if (sdata->int_pin_open_drain) {
>>> +             dev_info(&indio_dev->dev,
>>> +                      "set interrupt line to open drain mode\n");
>>
>> Hmm. Curriously I have a board with an lis3l02dq on it where the interrupt
>> is shared, but doesn't explicitly support open drain. In that case it
>> is done with some external circuitry.
> 
> That should run fine with just the status read patch from earlier
> in the series I guess?
I'll go with a 'probably'.  Getting that part to run at all is 'interesting'
as it's minimum rate is 440Hz and my board can't keep up.
> 
>> Anyhow - I guess when I actually add support for that I'll just add a sanity
>> check in here that addr_od and mask_od are specified :)
> 
> Hm, yeah. Otherwise just patch out my enforcement of the
> OD flag for a shared line and add a comment in.
Will figure it out when / if I ever actually do those patches...

J
> 
> Yours,
> Linus Walleij
> --
> 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
>