* [RFC v2 PATCH 01/14] iio: st_common: New interrupt interface
2013-09-27 16:32 [RFC v2 PATCH 00/14] iio: STMicroelectronics DT and event support Lukasz Czerwinski
@ 2013-09-27 16:32 ` Lukasz Czerwinski
2013-10-01 15:34 ` Jonathan Cameron
2013-10-21 11:48 ` Mark Rutland
2013-09-27 16:32 ` [RFC v2 PATCH 02/14] iio: st_accel: Add dt bindings Lukasz Czerwinski
` (12 subsequent siblings)
13 siblings, 2 replies; 30+ messages in thread
From: Lukasz Czerwinski @ 2013-09-27 16:32 UTC (permalink / raw)
To: jic23; +Cc: denis.ciocca, linux-iio, lee.jones, devicetree
This patch adds two interrupts handling by the st_common library.
Additional second interrupt is used to indetify enabled event support for
chosen ST device. It supports board files and dt.
For dt interface multiple interrupts are passed through interrupt-names
property.
Read drdy_int_pin value is moved to the st_sensors_parse_platform_data()
in the st_sensors_i2c.c and st_sensors_spi.c Now drdy_int_pin can be
also configured via dt.
Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/iio/common/st_sensors/st_sensors_core.c | 36 +----------
drivers/iio/common/st_sensors/st_sensors_i2c.c | 75 +++++++++++++++++++++-
drivers/iio/common/st_sensors/st_sensors_spi.c | 77 ++++++++++++++++++++++-
include/linux/iio/common/st_sensors.h | 13 +++-
include/linux/platform_data/st_sensors_pdata.h | 2 +
5 files changed, 160 insertions(+), 43 deletions(-)
diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
index 7ba1ef2..697b16d 100644
--- a/drivers/iio/common/st_sensors/st_sensors_core.c
+++ b/drivers/iio/common/st_sensors/st_sensors_core.c
@@ -198,47 +198,13 @@ int st_sensors_set_axis_enable(struct iio_dev *indio_dev, u8 axis_enable)
}
EXPORT_SYMBOL(st_sensors_set_axis_enable);
-static int st_sensors_set_drdy_int_pin(struct iio_dev *indio_dev,
- struct st_sensors_platform_data *pdata)
-{
- struct st_sensor_data *sdata = iio_priv(indio_dev);
-
- switch (pdata->drdy_int_pin) {
- case 1:
- if (sdata->sensor->drdy_irq.mask_int1 == 0) {
- dev_err(&indio_dev->dev,
- "DRDY on INT1 not available.\n");
- return -EINVAL;
- }
- sdata->drdy_int_pin = 1;
- break;
- case 2:
- if (sdata->sensor->drdy_irq.mask_int2 == 0) {
- dev_err(&indio_dev->dev,
- "DRDY on INT2 not available.\n");
- return -EINVAL;
- }
- sdata->drdy_int_pin = 2;
- break;
- default:
- dev_err(&indio_dev->dev, "DRDY on pdata not valid.\n");
- return -EINVAL;
- }
-
- return 0;
-}
-
-int st_sensors_init_sensor(struct iio_dev *indio_dev,
- struct st_sensors_platform_data *pdata)
+int st_sensors_init_sensor(struct iio_dev *indio_dev)
{
struct st_sensor_data *sdata = iio_priv(indio_dev);
int err = 0;
mutex_init(&sdata->tb.buf_lock);
- if (pdata)
- err = st_sensors_set_drdy_int_pin(indio_dev, pdata);
-
err = st_sensors_set_enable(indio_dev, false);
if (err < 0)
return err;
diff --git a/drivers/iio/common/st_sensors/st_sensors_i2c.c b/drivers/iio/common/st_sensors/st_sensors_i2c.c
index 38af944..79a9e9e 100644
--- a/drivers/iio/common/st_sensors/st_sensors_i2c.c
+++ b/drivers/iio/common/st_sensors/st_sensors_i2c.c
@@ -12,17 +12,82 @@
#include <linux/module.h>
#include <linux/slab.h>
#include <linux/iio/iio.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
#include <linux/iio/common/st_sensors_i2c.h>
#define ST_SENSORS_I2C_MULTIREAD 0x80
-static unsigned int st_sensors_i2c_get_irq(struct iio_dev *indio_dev)
+static unsigned int st_sensors_i2c_get_data_rdy_irq(struct iio_dev *indio_dev)
{
struct st_sensor_data *sdata = iio_priv(indio_dev);
- return to_i2c_client(sdata->dev)->irq;
+ return sdata->irq_map[ST_SENSORS_INT1];
+}
+EXPORT_SYMBOL(st_sensors_i2c_get_data_rdy_irq);
+
+static unsigned int st_sensors_i2c_get_event_irq(struct iio_dev *indio_dev)
+{
+ struct st_sensor_data *sdata = iio_priv(indio_dev);
+
+ return sdata->irq_map[ST_SENSORS_INT2];
+}
+EXPORT_SYMBOL(st_sensors_i2c_get_event_irq);
+
+static void st_sensors_parse_platform_data(struct i2c_client *client,
+ struct iio_dev *indio_dev)
+{
+ struct st_sensor_data *sdata = iio_priv(indio_dev);
+ struct device_node *np = client->dev.of_node;
+ struct st_sensors_platform_data *pdata = client->dev.platform_data;
+
+ if (pdata)
+ sdata->drdy_int_pin = pdata->drdy_int_pin;
+ else if (np)
+ of_property_read_u8(np, "st,drdy-int-pin",
+ (u8 *)&sdata->drdy_int_pin);
+}
+
+static unsigned int st_sensors_i2c_map_irq(struct i2c_client *client,
+ unsigned int irq_num)
+{
+ struct device_node *np = client->dev.of_node;
+ struct st_sensors_platform_data *pdata = client->dev.platform_data;
+ int index = 0;
+
+ if (pdata)
+ return pdata->irqs[irq_num];
+ else if (np) {
+ switch (irq_num) {
+ case ST_SENSORS_INT1:
+ index = of_property_match_string(np,
+ "interrupt-names",
+ ST_SENSORS_DRDY_IRQ_NAME);
+ break;
+ case ST_SENSORS_INT2:
+ index = of_property_match_string(np,
+ "interrupt-names",
+ ST_SENSORS_EVENT_IRQ_NAME);
+ default:
+ break;
+ }
+ return index < 0 ? 0 : irq_of_parse_and_map(np, index);
+ }
+ return 0;
+}
+
+static void st_sensors_i2c_map_irqs(struct i2c_client *client,
+ struct iio_dev *indio_dev)
+{
+ struct st_sensor_data *sdata = iio_priv(indio_dev);
+
+ sdata->irq_map[ST_SENSORS_INT1] =
+ st_sensors_i2c_map_irq(client, ST_SENSORS_INT1);
+
+ sdata->irq_map[ST_SENSORS_INT2] =
+ st_sensors_i2c_map_irq(client, ST_SENSORS_INT2);
}
static int st_sensors_i2c_read_byte(struct st_sensor_transfer_buffer *tb,
@@ -71,8 +136,12 @@ void st_sensors_i2c_configure(struct iio_dev *indio_dev,
indio_dev->dev.parent = &client->dev;
indio_dev->name = client->name;
+ st_sensors_parse_platform_data(client, indio_dev);
+
sdata->tf = &st_sensors_tf_i2c;
- sdata->get_irq_data_ready = st_sensors_i2c_get_irq;
+ st_sensors_i2c_map_irqs(client, indio_dev);
+ sdata->get_irq_data_ready = st_sensors_i2c_get_data_rdy_irq;
+ sdata->get_irq_event = st_sensors_i2c_get_event_irq;
}
EXPORT_SYMBOL(st_sensors_i2c_configure);
diff --git a/drivers/iio/common/st_sensors/st_sensors_spi.c b/drivers/iio/common/st_sensors/st_sensors_spi.c
index 251baf6..fc04cfc 100644
--- a/drivers/iio/common/st_sensors/st_sensors_spi.c
+++ b/drivers/iio/common/st_sensors/st_sensors_spi.c
@@ -8,10 +8,14 @@
* Licensed under the GPL-2.
*/
+#include <linux/of.h>
+#include <linux/of_irq.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/slab.h>
#include <linux/iio/iio.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
#include <linux/iio/common/st_sensors_spi.h>
@@ -19,11 +23,74 @@
#define ST_SENSORS_SPI_MULTIREAD 0xc0
#define ST_SENSORS_SPI_READ 0x80
-static unsigned int st_sensors_spi_get_irq(struct iio_dev *indio_dev)
+static unsigned int st_sensors_spi_get_data_rdy_irq(struct iio_dev *indio_dev)
{
struct st_sensor_data *sdata = iio_priv(indio_dev);
- return to_spi_device(sdata->dev)->irq;
+ return sdata->irq_map[ST_SENSORS_INT1];
+}
+EXPORT_SYMBOL(st_sensors_spi_get_data_rdy_irq);
+
+static unsigned int st_sensors_spi_get_event_irq(struct iio_dev *indio_dev)
+{
+ struct st_sensor_data *sdata = iio_priv(indio_dev);
+
+ return sdata->irq_map[ST_SENSORS_INT2];
+}
+EXPORT_SYMBOL(st_sensors_spi_get_event_irq);
+
+static void st_sensors_parse_platform_data(struct spi_device *spi,
+ struct iio_dev *indio_dev)
+{
+ struct st_sensor_data *sdata = iio_priv(indio_dev);
+ struct device_node *np = spi->dev.of_node;
+ struct st_sensors_platform_data *pdata = spi->dev.platform_data;
+
+ if (pdata)
+ sdata->drdy_int_pin = pdata->drdy_int_pin;
+ else if (np)
+ of_property_read_u8(np, "drdy-int-pin",
+ (u8 *)&sdata->drdy_int_pin);
+}
+
+static unsigned int st_sensors_spi_map_irq(struct spi_device *spi,
+ unsigned int irq_num)
+{
+ struct device_node *np = spi->dev.of_node;
+ struct st_sensors_platform_data *pdata = spi->dev.platform_data;
+ int index = 0;
+
+ if (pdata)
+ return pdata->irqs[irq_num];
+ else if (np) {
+ switch (irq_num) {
+ case ST_SENSORS_INT1:
+ index = of_property_match_string(np,
+ "interrupt-names",
+ ST_SENSORS_DRDY_IRQ_NAME);
+ break;
+ case ST_SENSORS_INT2:
+ index = of_property_match_string(np,
+ "interrupt-names",
+ ST_SENSORS_EVENT_IRQ_NAME);
+ default:
+ break;
+ }
+ return index < 0 ? 0 : irq_of_parse_and_map(np, index);
+ }
+ return 0;
+}
+
+static void st_sensors_spi_map_irqs(struct spi_device *spi,
+ struct iio_dev *indio_dev)
+{
+ struct st_sensor_data *sdata = iio_priv(indio_dev);
+
+ sdata->irq_map[ST_SENSORS_INT1] =
+ st_sensors_spi_map_irq(spi, ST_SENSORS_INT1);
+
+ sdata->irq_map[ST_SENSORS_INT2] =
+ st_sensors_spi_map_irq(spi, ST_SENSORS_INT2);
}
static int st_sensors_spi_read(struct st_sensor_transfer_buffer *tb,
@@ -111,8 +178,12 @@ void st_sensors_spi_configure(struct iio_dev *indio_dev,
indio_dev->dev.parent = &spi->dev;
indio_dev->name = spi->modalias;
+ st_sensors_parse_platform_data(spi, indio_dev);
+
sdata->tf = &st_sensors_tf_spi;
- sdata->get_irq_data_ready = st_sensors_spi_get_irq;
+ st_sensors_spi_map_irqs(spi, indio_dev);
+ sdata->get_irq_data_ready = st_sensors_spi_get_data_rdy_irq;
+ sdata->get_irq_event = st_sensors_spi_get_event_irq;
}
EXPORT_SYMBOL(st_sensors_spi_configure);
diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
index 3c005eb..3f4a0f7 100644
--- a/include/linux/iio/common/st_sensors.h
+++ b/include/linux/iio/common/st_sensors.h
@@ -41,6 +41,12 @@
#define ST_SENSORS_MAX_NAME 17
#define ST_SENSORS_MAX_4WAI 7
+#define ST_SENSORS_INT_MAX 2
+#define ST_SENSORS_INT1 0
+#define ST_SENSORS_INT2 1
+#define ST_SENSORS_DRDY_IRQ_NAME "drdy-int"
+#define ST_SENSORS_EVENT_IRQ_NAME "event-int"
+
#define ST_SENSORS_LSM_CHANNELS(device_type, mask, index, mod, \
ch2, s, endian, rbits, sbits, addr) \
{ \
@@ -209,8 +215,10 @@ struct st_sensors {
* @buffer_data: Data used by buffer part.
* @odr: Output data rate of the sensor [Hz].
* num_data_channels: Number of data channels used in buffer.
+ * @irq_map: Container of mapped IRQs.
* @drdy_int_pin: Redirect DRDY on pin 1 (1) or pin 2 (2).
* @get_irq_data_ready: Function to get the IRQ used for data ready signal.
+ * @get_irq_event: Function to get the IRQ used for event signal.
* @tf: Transfer function structure used by I/O operations.
* @tb: Transfer buffers and mutex used by I/O operations.
*/
@@ -229,10 +237,12 @@ struct st_sensor_data {
unsigned int odr;
unsigned int num_data_channels;
+ unsigned int irq_map[ST_SENSORS_INT_MAX];
u8 drdy_int_pin;
unsigned int (*get_irq_data_ready) (struct iio_dev *indio_dev);
+ unsigned int (*get_irq_event) (struct iio_dev *indio_dev);
const struct st_sensor_transfer_function *tf;
struct st_sensor_transfer_buffer tb;
@@ -262,8 +272,7 @@ static inline void st_sensors_deallocate_trigger(struct iio_dev *indio_dev)
}
#endif
-int st_sensors_init_sensor(struct iio_dev *indio_dev,
- struct st_sensors_platform_data *pdata);
+int st_sensors_init_sensor(struct iio_dev *indio_dev);
int st_sensors_set_enable(struct iio_dev *indio_dev, bool enable);
diff --git a/include/linux/platform_data/st_sensors_pdata.h b/include/linux/platform_data/st_sensors_pdata.h
index 7538391..991db72 100644
--- a/include/linux/platform_data/st_sensors_pdata.h
+++ b/include/linux/platform_data/st_sensors_pdata.h
@@ -19,6 +19,8 @@
*/
struct st_sensors_platform_data {
u8 drdy_int_pin;
+
+ unsigned int irqs[2];
};
#endif /* ST_SENSORS_PDATA_H */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [RFC v2 PATCH 01/14] iio: st_common: New interrupt interface
2013-09-27 16:32 ` [RFC v2 PATCH 01/14] iio: st_common: New interrupt interface Lukasz Czerwinski
@ 2013-10-01 15:34 ` Jonathan Cameron
2013-10-01 16:22 ` Jonathan Cameron
2013-10-21 11:59 ` Mark Rutland
2013-10-21 11:48 ` Mark Rutland
1 sibling, 2 replies; 30+ messages in thread
From: Jonathan Cameron @ 2013-10-01 15:34 UTC (permalink / raw)
To: Lukasz Czerwinski, jic23; +Cc: denis.ciocca, linux-iio, lee.jones, devicetree
On 09/27/13 17:32, Lukasz Czerwinski wrote:
> This patch adds two interrupts handling by the st_common library.
> Additional second interrupt is used to indetify enabled event support for
> chosen ST device. It supports board files and dt.
>
> For dt interface multiple interrupts are passed through interrupt-names
> property.
>
> Read drdy_int_pin value is moved to the st_sensors_parse_platform_data()
> in the st_sensors_i2c.c and st_sensors_spi.c Now drdy_int_pin can be
> also configured via dt.
>
> Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
The code here is absolutely fine, though I would like some comments
on this from Denis and/or Lee (as they are much more familiar with the driver
than I am!
The one element that is problematic is the use of interrupt names in specifying
the two interrupts. Unforunately, for reasons that I think can be sumarized
as historical precedence and avoiding complexity of passing in other OSes,
the device tree powers that be are very anti doing multiple optional interrupts
this way and consider interrupt-names to be merely for information rather than
to be used to allow 'optional' interrupts.
One thread on this is:
http://www.spinics.net/lists/linux-iio/msg09646.html
Obviously this effects your later device tree bindings patches.
> ---
> drivers/iio/common/st_sensors/st_sensors_core.c | 36 +----------
> drivers/iio/common/st_sensors/st_sensors_i2c.c | 75 +++++++++++++++++++++-
> drivers/iio/common/st_sensors/st_sensors_spi.c | 77 ++++++++++++++++++++++-
> include/linux/iio/common/st_sensors.h | 13 +++-
> include/linux/platform_data/st_sensors_pdata.h | 2 +
> 5 files changed, 160 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
> index 7ba1ef2..697b16d 100644
> --- a/drivers/iio/common/st_sensors/st_sensors_core.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
> @@ -198,47 +198,13 @@ int st_sensors_set_axis_enable(struct iio_dev *indio_dev, u8 axis_enable)
> }
> EXPORT_SYMBOL(st_sensors_set_axis_enable);
>
> -static int st_sensors_set_drdy_int_pin(struct iio_dev *indio_dev,
> - struct st_sensors_platform_data *pdata)
> -{
> - struct st_sensor_data *sdata = iio_priv(indio_dev);
> -
> - switch (pdata->drdy_int_pin) {
> - case 1:
> - if (sdata->sensor->drdy_irq.mask_int1 == 0) {
> - dev_err(&indio_dev->dev,
> - "DRDY on INT1 not available.\n");
> - return -EINVAL;
> - }
> - sdata->drdy_int_pin = 1;
> - break;
> - case 2:
> - if (sdata->sensor->drdy_irq.mask_int2 == 0) {
> - dev_err(&indio_dev->dev,
> - "DRDY on INT2 not available.\n");
> - return -EINVAL;
> - }
> - sdata->drdy_int_pin = 2;
> - break;
> - default:
> - dev_err(&indio_dev->dev, "DRDY on pdata not valid.\n");
> - return -EINVAL;
> - }
> -
> - return 0;
> -}
> -
> -int st_sensors_init_sensor(struct iio_dev *indio_dev,
> - struct st_sensors_platform_data *pdata)
> +int st_sensors_init_sensor(struct iio_dev *indio_dev)
> {
> struct st_sensor_data *sdata = iio_priv(indio_dev);
> int err = 0;
>
> mutex_init(&sdata->tb.buf_lock);
>
> - if (pdata)
> - err = st_sensors_set_drdy_int_pin(indio_dev, pdata);
> -
> err = st_sensors_set_enable(indio_dev, false);
> if (err < 0)
> return err;
> diff --git a/drivers/iio/common/st_sensors/st_sensors_i2c.c b/drivers/iio/common/st_sensors/st_sensors_i2c.c
> index 38af944..79a9e9e 100644
> --- a/drivers/iio/common/st_sensors/st_sensors_i2c.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_i2c.c
> @@ -12,17 +12,82 @@
> #include <linux/module.h>
> #include <linux/slab.h>
> #include <linux/iio/iio.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
>
> #include <linux/iio/common/st_sensors_i2c.h>
>
>
> #define ST_SENSORS_I2C_MULTIREAD 0x80
>
> -static unsigned int st_sensors_i2c_get_irq(struct iio_dev *indio_dev)
> +static unsigned int st_sensors_i2c_get_data_rdy_irq(struct iio_dev *indio_dev)
> {
> struct st_sensor_data *sdata = iio_priv(indio_dev);
>
> - return to_i2c_client(sdata->dev)->irq;
> + return sdata->irq_map[ST_SENSORS_INT1];
> +}
> +EXPORT_SYMBOL(st_sensors_i2c_get_data_rdy_irq);
> +
> +static unsigned int st_sensors_i2c_get_event_irq(struct iio_dev *indio_dev)
> +{
> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> +
> + return sdata->irq_map[ST_SENSORS_INT2];
> +}
> +EXPORT_SYMBOL(st_sensors_i2c_get_event_irq);
> +
> +static void st_sensors_parse_platform_data(struct i2c_client *client,
> + struct iio_dev *indio_dev)
> +{
> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> + struct device_node *np = client->dev.of_node;
> + struct st_sensors_platform_data *pdata = client->dev.platform_data;
> +
> + if (pdata)
> + sdata->drdy_int_pin = pdata->drdy_int_pin;
> + else if (np)
> + of_property_read_u8(np, "st,drdy-int-pin",
> + (u8 *)&sdata->drdy_int_pin);
> +}
> +
> +static unsigned int st_sensors_i2c_map_irq(struct i2c_client *client,
> + unsigned int irq_num)
> +{
> + struct device_node *np = client->dev.of_node;
> + struct st_sensors_platform_data *pdata = client->dev.platform_data;
> + int index = 0;
> +
> + if (pdata)
> + return pdata->irqs[irq_num];
> + else if (np) {
> + switch (irq_num) {
> + case ST_SENSORS_INT1:
> + index = of_property_match_string(np,
> + "interrupt-names",
> + ST_SENSORS_DRDY_IRQ_NAME);
> + break;
> + case ST_SENSORS_INT2:
> + index = of_property_match_string(np,
> + "interrupt-names",
> + ST_SENSORS_EVENT_IRQ_NAME);
> + default:
> + break;
> + }
> + return index < 0 ? 0 : irq_of_parse_and_map(np, index);
> + }
> + return 0;
> +}
> +
> +static void st_sensors_i2c_map_irqs(struct i2c_client *client,
> + struct iio_dev *indio_dev)
> +{
> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> +
> + sdata->irq_map[ST_SENSORS_INT1] =
> + st_sensors_i2c_map_irq(client, ST_SENSORS_INT1);
> +
> + sdata->irq_map[ST_SENSORS_INT2] =
> + st_sensors_i2c_map_irq(client, ST_SENSORS_INT2);
> }
>
> static int st_sensors_i2c_read_byte(struct st_sensor_transfer_buffer *tb,
> @@ -71,8 +136,12 @@ void st_sensors_i2c_configure(struct iio_dev *indio_dev,
> indio_dev->dev.parent = &client->dev;
> indio_dev->name = client->name;
>
> + st_sensors_parse_platform_data(client, indio_dev);
> +
> sdata->tf = &st_sensors_tf_i2c;
> - sdata->get_irq_data_ready = st_sensors_i2c_get_irq;
> + st_sensors_i2c_map_irqs(client, indio_dev);
> + sdata->get_irq_data_ready = st_sensors_i2c_get_data_rdy_irq;
> + sdata->get_irq_event = st_sensors_i2c_get_event_irq;
> }
> EXPORT_SYMBOL(st_sensors_i2c_configure);
>
> diff --git a/drivers/iio/common/st_sensors/st_sensors_spi.c b/drivers/iio/common/st_sensors/st_sensors_spi.c
> index 251baf6..fc04cfc 100644
> --- a/drivers/iio/common/st_sensors/st_sensors_spi.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_spi.c
> @@ -8,10 +8,14 @@
> * Licensed under the GPL-2.
> */
>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/slab.h>
> #include <linux/iio/iio.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
>
> #include <linux/iio/common/st_sensors_spi.h>
>
> @@ -19,11 +23,74 @@
> #define ST_SENSORS_SPI_MULTIREAD 0xc0
> #define ST_SENSORS_SPI_READ 0x80
>
> -static unsigned int st_sensors_spi_get_irq(struct iio_dev *indio_dev)
> +static unsigned int st_sensors_spi_get_data_rdy_irq(struct iio_dev *indio_dev)
> {
> struct st_sensor_data *sdata = iio_priv(indio_dev);
>
> - return to_spi_device(sdata->dev)->irq;
> + return sdata->irq_map[ST_SENSORS_INT1];
> +}
> +EXPORT_SYMBOL(st_sensors_spi_get_data_rdy_irq);
> +
> +static unsigned int st_sensors_spi_get_event_irq(struct iio_dev *indio_dev)
> +{
> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> +
> + return sdata->irq_map[ST_SENSORS_INT2];
> +}
> +EXPORT_SYMBOL(st_sensors_spi_get_event_irq);
> +
> +static void st_sensors_parse_platform_data(struct spi_device *spi,
> + struct iio_dev *indio_dev)
> +{
> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> + struct device_node *np = spi->dev.of_node;
> + struct st_sensors_platform_data *pdata = spi->dev.platform_data;
> +
> + if (pdata)
> + sdata->drdy_int_pin = pdata->drdy_int_pin;
> + else if (np)
> + of_property_read_u8(np, "drdy-int-pin",
> + (u8 *)&sdata->drdy_int_pin);
> +}
> +
> +static unsigned int st_sensors_spi_map_irq(struct spi_device *spi,
> + unsigned int irq_num)
> +{
> + struct device_node *np = spi->dev.of_node;
> + struct st_sensors_platform_data *pdata = spi->dev.platform_data;
> + int index = 0;
> +
> + if (pdata)
> + return pdata->irqs[irq_num];
> + else if (np) {
> + switch (irq_num) {
> + case ST_SENSORS_INT1:
> + index = of_property_match_string(np,
> + "interrupt-names",
> + ST_SENSORS_DRDY_IRQ_NAME);
> + break;
> + case ST_SENSORS_INT2:
> + index = of_property_match_string(np,
> + "interrupt-names",
> + ST_SENSORS_EVENT_IRQ_NAME);
> + default:
> + break;
> + }
> + return index < 0 ? 0 : irq_of_parse_and_map(np, index);
> + }
> + return 0;
> +}
> +
> +static void st_sensors_spi_map_irqs(struct spi_device *spi,
> + struct iio_dev *indio_dev)
> +{
> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> +
> + sdata->irq_map[ST_SENSORS_INT1] =
> + st_sensors_spi_map_irq(spi, ST_SENSORS_INT1);
> +
> + sdata->irq_map[ST_SENSORS_INT2] =
> + st_sensors_spi_map_irq(spi, ST_SENSORS_INT2);
> }
>
> static int st_sensors_spi_read(struct st_sensor_transfer_buffer *tb,
> @@ -111,8 +178,12 @@ void st_sensors_spi_configure(struct iio_dev *indio_dev,
> indio_dev->dev.parent = &spi->dev;
> indio_dev->name = spi->modalias;
>
> + st_sensors_parse_platform_data(spi, indio_dev);
> +
> sdata->tf = &st_sensors_tf_spi;
> - sdata->get_irq_data_ready = st_sensors_spi_get_irq;
> + st_sensors_spi_map_irqs(spi, indio_dev);
> + sdata->get_irq_data_ready = st_sensors_spi_get_data_rdy_irq;
> + sdata->get_irq_event = st_sensors_spi_get_event_irq;
> }
> EXPORT_SYMBOL(st_sensors_spi_configure);
>
> diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
> index 3c005eb..3f4a0f7 100644
> --- a/include/linux/iio/common/st_sensors.h
> +++ b/include/linux/iio/common/st_sensors.h
> @@ -41,6 +41,12 @@
> #define ST_SENSORS_MAX_NAME 17
> #define ST_SENSORS_MAX_4WAI 7
>
> +#define ST_SENSORS_INT_MAX 2
> +#define ST_SENSORS_INT1 0
> +#define ST_SENSORS_INT2 1
> +#define ST_SENSORS_DRDY_IRQ_NAME "drdy-int"
> +#define ST_SENSORS_EVENT_IRQ_NAME "event-int"
> +
> #define ST_SENSORS_LSM_CHANNELS(device_type, mask, index, mod, \
> ch2, s, endian, rbits, sbits, addr) \
> { \
> @@ -209,8 +215,10 @@ struct st_sensors {
> * @buffer_data: Data used by buffer part.
> * @odr: Output data rate of the sensor [Hz].
> * num_data_channels: Number of data channels used in buffer.
> + * @irq_map: Container of mapped IRQs.
> * @drdy_int_pin: Redirect DRDY on pin 1 (1) or pin 2 (2).
> * @get_irq_data_ready: Function to get the IRQ used for data ready signal.
> + * @get_irq_event: Function to get the IRQ used for event signal.
> * @tf: Transfer function structure used by I/O operations.
> * @tb: Transfer buffers and mutex used by I/O operations.
> */
> @@ -229,10 +237,12 @@ struct st_sensor_data {
>
> unsigned int odr;
> unsigned int num_data_channels;
> + unsigned int irq_map[ST_SENSORS_INT_MAX];
>
> u8 drdy_int_pin;
>
> unsigned int (*get_irq_data_ready) (struct iio_dev *indio_dev);
> + unsigned int (*get_irq_event) (struct iio_dev *indio_dev);
>
> const struct st_sensor_transfer_function *tf;
> struct st_sensor_transfer_buffer tb;
> @@ -262,8 +272,7 @@ static inline void st_sensors_deallocate_trigger(struct iio_dev *indio_dev)
> }
> #endif
>
> -int st_sensors_init_sensor(struct iio_dev *indio_dev,
> - struct st_sensors_platform_data *pdata);
> +int st_sensors_init_sensor(struct iio_dev *indio_dev);
>
> int st_sensors_set_enable(struct iio_dev *indio_dev, bool enable);
>
> diff --git a/include/linux/platform_data/st_sensors_pdata.h b/include/linux/platform_data/st_sensors_pdata.h
> index 7538391..991db72 100644
> --- a/include/linux/platform_data/st_sensors_pdata.h
> +++ b/include/linux/platform_data/st_sensors_pdata.h
> @@ -19,6 +19,8 @@
> */
> struct st_sensors_platform_data {
> u8 drdy_int_pin;
> +
> + unsigned int irqs[2];
> };
>
> #endif /* ST_SENSORS_PDATA_H */
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC v2 PATCH 01/14] iio: st_common: New interrupt interface
2013-10-01 15:34 ` Jonathan Cameron
@ 2013-10-01 16:22 ` Jonathan Cameron
2013-10-21 11:59 ` Mark Rutland
1 sibling, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2013-10-01 16:22 UTC (permalink / raw)
To: Lukasz Czerwinski, jic23; +Cc: denis.ciocca, linux-iio, lee.jones, devicetree
On 10/01/13 16:34, Jonathan Cameron wrote:
> On 09/27/13 17:32, Lukasz Czerwinski wrote:
>> This patch adds two interrupts handling by the st_common library.
>> Additional second interrupt is used to indetify enabled event support for
>> chosen ST device. It supports board files and dt.
>>
>> For dt interface multiple interrupts are passed through interrupt-names
>> property.
>>
>> Read drdy_int_pin value is moved to the st_sensors_parse_platform_data()
>> in the st_sensors_i2c.c and st_sensors_spi.c Now drdy_int_pin can be
>> also configured via dt.
>>
>> Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>
> The code here is absolutely fine, though I would like some comments
> on this from Denis and/or Lee (as they are much more familiar with the driver
> than I am!
>
> The one element that is problematic is the use of interrupt names in specifying
> the two interrupts. Unforunately, for reasons that I think can be sumarized
> as historical precedence and avoiding complexity of passing in other OSes,
> the device tree powers that be are very anti doing multiple optional interrupts
> this way and consider interrupt-names to be merely for information rather than
> to be used to allow 'optional' interrupts.
>
>
> One thread on this is:
>
> http://www.spinics.net/lists/linux-iio/msg09646.html
Now I'm confused. You refer to that one clearly in your patch introduction.
Is this not still relying on the names field to work out which is which?
>
>
> Obviously this effects your later device tree bindings patches.
>
>> ---
>> drivers/iio/common/st_sensors/st_sensors_core.c | 36 +----------
>> drivers/iio/common/st_sensors/st_sensors_i2c.c | 75 +++++++++++++++++++++-
>> drivers/iio/common/st_sensors/st_sensors_spi.c | 77 ++++++++++++++++++++++-
>> include/linux/iio/common/st_sensors.h | 13 +++-
>> include/linux/platform_data/st_sensors_pdata.h | 2 +
>> 5 files changed, 160 insertions(+), 43 deletions(-)
>>
>> diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
>> index 7ba1ef2..697b16d 100644
>> --- a/drivers/iio/common/st_sensors/st_sensors_core.c
>> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
>> @@ -198,47 +198,13 @@ int st_sensors_set_axis_enable(struct iio_dev *indio_dev, u8 axis_enable)
>> }
>> EXPORT_SYMBOL(st_sensors_set_axis_enable);
>>
>> -static int st_sensors_set_drdy_int_pin(struct iio_dev *indio_dev,
>> - struct st_sensors_platform_data *pdata)
>> -{
>> - struct st_sensor_data *sdata = iio_priv(indio_dev);
>> -
>> - switch (pdata->drdy_int_pin) {
>> - case 1:
>> - if (sdata->sensor->drdy_irq.mask_int1 == 0) {
>> - dev_err(&indio_dev->dev,
>> - "DRDY on INT1 not available.\n");
>> - return -EINVAL;
>> - }
>> - sdata->drdy_int_pin = 1;
>> - break;
>> - case 2:
>> - if (sdata->sensor->drdy_irq.mask_int2 == 0) {
>> - dev_err(&indio_dev->dev,
>> - "DRDY on INT2 not available.\n");
>> - return -EINVAL;
>> - }
>> - sdata->drdy_int_pin = 2;
>> - break;
>> - default:
>> - dev_err(&indio_dev->dev, "DRDY on pdata not valid.\n");
>> - return -EINVAL;
>> - }
>> -
>> - return 0;
>> -}
>> -
>> -int st_sensors_init_sensor(struct iio_dev *indio_dev,
>> - struct st_sensors_platform_data *pdata)
>> +int st_sensors_init_sensor(struct iio_dev *indio_dev)
>> {
>> struct st_sensor_data *sdata = iio_priv(indio_dev);
>> int err = 0;
>>
>> mutex_init(&sdata->tb.buf_lock);
>>
>> - if (pdata)
>> - err = st_sensors_set_drdy_int_pin(indio_dev, pdata);
>> -
>> err = st_sensors_set_enable(indio_dev, false);
>> if (err < 0)
>> return err;
>> diff --git a/drivers/iio/common/st_sensors/st_sensors_i2c.c b/drivers/iio/common/st_sensors/st_sensors_i2c.c
>> index 38af944..79a9e9e 100644
>> --- a/drivers/iio/common/st_sensors/st_sensors_i2c.c
>> +++ b/drivers/iio/common/st_sensors/st_sensors_i2c.c
>> @@ -12,17 +12,82 @@
>> #include <linux/module.h>
>> #include <linux/slab.h>
>> #include <linux/iio/iio.h>
>> +#include <linux/of.h>
>> +#include <linux/of_irq.h>
>>
>> #include <linux/iio/common/st_sensors_i2c.h>
>>
>>
>> #define ST_SENSORS_I2C_MULTIREAD 0x80
>>
>> -static unsigned int st_sensors_i2c_get_irq(struct iio_dev *indio_dev)
>> +static unsigned int st_sensors_i2c_get_data_rdy_irq(struct iio_dev *indio_dev)
>> {
>> struct st_sensor_data *sdata = iio_priv(indio_dev);
>>
>> - return to_i2c_client(sdata->dev)->irq;
>> + return sdata->irq_map[ST_SENSORS_INT1];
>> +}
>> +EXPORT_SYMBOL(st_sensors_i2c_get_data_rdy_irq);
>> +
>> +static unsigned int st_sensors_i2c_get_event_irq(struct iio_dev *indio_dev)
>> +{
>> + struct st_sensor_data *sdata = iio_priv(indio_dev);
>> +
>> + return sdata->irq_map[ST_SENSORS_INT2];
>> +}
>> +EXPORT_SYMBOL(st_sensors_i2c_get_event_irq);
>> +
>> +static void st_sensors_parse_platform_data(struct i2c_client *client,
>> + struct iio_dev *indio_dev)
>> +{
>> + struct st_sensor_data *sdata = iio_priv(indio_dev);
>> + struct device_node *np = client->dev.of_node;
>> + struct st_sensors_platform_data *pdata = client->dev.platform_data;
>> +
>> + if (pdata)
>> + sdata->drdy_int_pin = pdata->drdy_int_pin;
>> + else if (np)
>> + of_property_read_u8(np, "st,drdy-int-pin",
>> + (u8 *)&sdata->drdy_int_pin);
>> +}
>> +
>> +static unsigned int st_sensors_i2c_map_irq(struct i2c_client *client,
>> + unsigned int irq_num)
>> +{
>> + struct device_node *np = client->dev.of_node;
>> + struct st_sensors_platform_data *pdata = client->dev.platform_data;
>> + int index = 0;
>> +
>> + if (pdata)
>> + return pdata->irqs[irq_num];
>> + else if (np) {
>> + switch (irq_num) {
>> + case ST_SENSORS_INT1:
>> + index = of_property_match_string(np,
>> + "interrupt-names",
>> + ST_SENSORS_DRDY_IRQ_NAME);
>> + break;
>> + case ST_SENSORS_INT2:
>> + index = of_property_match_string(np,
>> + "interrupt-names",
>> + ST_SENSORS_EVENT_IRQ_NAME);
>> + default:
>> + break;
>> + }
>> + return index < 0 ? 0 : irq_of_parse_and_map(np, index);
>> + }
>> + return 0;
>> +}
>> +
>> +static void st_sensors_i2c_map_irqs(struct i2c_client *client,
>> + struct iio_dev *indio_dev)
>> +{
>> + struct st_sensor_data *sdata = iio_priv(indio_dev);
>> +
>> + sdata->irq_map[ST_SENSORS_INT1] =
>> + st_sensors_i2c_map_irq(client, ST_SENSORS_INT1);
>> +
>> + sdata->irq_map[ST_SENSORS_INT2] =
>> + st_sensors_i2c_map_irq(client, ST_SENSORS_INT2);
>> }
>>
>> static int st_sensors_i2c_read_byte(struct st_sensor_transfer_buffer *tb,
>> @@ -71,8 +136,12 @@ void st_sensors_i2c_configure(struct iio_dev *indio_dev,
>> indio_dev->dev.parent = &client->dev;
>> indio_dev->name = client->name;
>>
>> + st_sensors_parse_platform_data(client, indio_dev);
>> +
>> sdata->tf = &st_sensors_tf_i2c;
>> - sdata->get_irq_data_ready = st_sensors_i2c_get_irq;
>> + st_sensors_i2c_map_irqs(client, indio_dev);
>> + sdata->get_irq_data_ready = st_sensors_i2c_get_data_rdy_irq;
>> + sdata->get_irq_event = st_sensors_i2c_get_event_irq;
>> }
>> EXPORT_SYMBOL(st_sensors_i2c_configure);
>>
>> diff --git a/drivers/iio/common/st_sensors/st_sensors_spi.c b/drivers/iio/common/st_sensors/st_sensors_spi.c
>> index 251baf6..fc04cfc 100644
>> --- a/drivers/iio/common/st_sensors/st_sensors_spi.c
>> +++ b/drivers/iio/common/st_sensors/st_sensors_spi.c
>> @@ -8,10 +8,14 @@
>> * Licensed under the GPL-2.
>> */
>>
>> +#include <linux/of.h>
>> +#include <linux/of_irq.h>
>> #include <linux/kernel.h>
>> #include <linux/module.h>
>> #include <linux/slab.h>
>> #include <linux/iio/iio.h>
>> +#include <linux/of.h>
>> +#include <linux/of_irq.h>
>>
>> #include <linux/iio/common/st_sensors_spi.h>
>>
>> @@ -19,11 +23,74 @@
>> #define ST_SENSORS_SPI_MULTIREAD 0xc0
>> #define ST_SENSORS_SPI_READ 0x80
>>
>> -static unsigned int st_sensors_spi_get_irq(struct iio_dev *indio_dev)
>> +static unsigned int st_sensors_spi_get_data_rdy_irq(struct iio_dev *indio_dev)
>> {
>> struct st_sensor_data *sdata = iio_priv(indio_dev);
>>
>> - return to_spi_device(sdata->dev)->irq;
>> + return sdata->irq_map[ST_SENSORS_INT1];
>> +}
>> +EXPORT_SYMBOL(st_sensors_spi_get_data_rdy_irq);
>> +
>> +static unsigned int st_sensors_spi_get_event_irq(struct iio_dev *indio_dev)
>> +{
>> + struct st_sensor_data *sdata = iio_priv(indio_dev);
>> +
>> + return sdata->irq_map[ST_SENSORS_INT2];
>> +}
>> +EXPORT_SYMBOL(st_sensors_spi_get_event_irq);
>> +
>> +static void st_sensors_parse_platform_data(struct spi_device *spi,
>> + struct iio_dev *indio_dev)
>> +{
>> + struct st_sensor_data *sdata = iio_priv(indio_dev);
>> + struct device_node *np = spi->dev.of_node;
>> + struct st_sensors_platform_data *pdata = spi->dev.platform_data;
>> +
>> + if (pdata)
>> + sdata->drdy_int_pin = pdata->drdy_int_pin;
>> + else if (np)
>> + of_property_read_u8(np, "drdy-int-pin",
>> + (u8 *)&sdata->drdy_int_pin);
>> +}
>> +
>> +static unsigned int st_sensors_spi_map_irq(struct spi_device *spi,
>> + unsigned int irq_num)
>> +{
>> + struct device_node *np = spi->dev.of_node;
>> + struct st_sensors_platform_data *pdata = spi->dev.platform_data;
>> + int index = 0;
>> +
>> + if (pdata)
>> + return pdata->irqs[irq_num];
>> + else if (np) {
>> + switch (irq_num) {
>> + case ST_SENSORS_INT1:
>> + index = of_property_match_string(np,
>> + "interrupt-names",
>> + ST_SENSORS_DRDY_IRQ_NAME);
>> + break;
>> + case ST_SENSORS_INT2:
>> + index = of_property_match_string(np,
>> + "interrupt-names",
>> + ST_SENSORS_EVENT_IRQ_NAME);
>> + default:
>> + break;
>> + }
>> + return index < 0 ? 0 : irq_of_parse_and_map(np, index);
>> + }
>> + return 0;
>> +}
>> +
>> +static void st_sensors_spi_map_irqs(struct spi_device *spi,
>> + struct iio_dev *indio_dev)
>> +{
>> + struct st_sensor_data *sdata = iio_priv(indio_dev);
>> +
>> + sdata->irq_map[ST_SENSORS_INT1] =
>> + st_sensors_spi_map_irq(spi, ST_SENSORS_INT1);
>> +
>> + sdata->irq_map[ST_SENSORS_INT2] =
>> + st_sensors_spi_map_irq(spi, ST_SENSORS_INT2);
>> }
>>
>> static int st_sensors_spi_read(struct st_sensor_transfer_buffer *tb,
>> @@ -111,8 +178,12 @@ void st_sensors_spi_configure(struct iio_dev *indio_dev,
>> indio_dev->dev.parent = &spi->dev;
>> indio_dev->name = spi->modalias;
>>
>> + st_sensors_parse_platform_data(spi, indio_dev);
>> +
>> sdata->tf = &st_sensors_tf_spi;
>> - sdata->get_irq_data_ready = st_sensors_spi_get_irq;
>> + st_sensors_spi_map_irqs(spi, indio_dev);
>> + sdata->get_irq_data_ready = st_sensors_spi_get_data_rdy_irq;
>> + sdata->get_irq_event = st_sensors_spi_get_event_irq;
>> }
>> EXPORT_SYMBOL(st_sensors_spi_configure);
>>
>> diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
>> index 3c005eb..3f4a0f7 100644
>> --- a/include/linux/iio/common/st_sensors.h
>> +++ b/include/linux/iio/common/st_sensors.h
>> @@ -41,6 +41,12 @@
>> #define ST_SENSORS_MAX_NAME 17
>> #define ST_SENSORS_MAX_4WAI 7
>>
>> +#define ST_SENSORS_INT_MAX 2
>> +#define ST_SENSORS_INT1 0
>> +#define ST_SENSORS_INT2 1
>> +#define ST_SENSORS_DRDY_IRQ_NAME "drdy-int"
>> +#define ST_SENSORS_EVENT_IRQ_NAME "event-int"
>> +
>> #define ST_SENSORS_LSM_CHANNELS(device_type, mask, index, mod, \
>> ch2, s, endian, rbits, sbits, addr) \
>> { \
>> @@ -209,8 +215,10 @@ struct st_sensors {
>> * @buffer_data: Data used by buffer part.
>> * @odr: Output data rate of the sensor [Hz].
>> * num_data_channels: Number of data channels used in buffer.
>> + * @irq_map: Container of mapped IRQs.
>> * @drdy_int_pin: Redirect DRDY on pin 1 (1) or pin 2 (2).
>> * @get_irq_data_ready: Function to get the IRQ used for data ready signal.
>> + * @get_irq_event: Function to get the IRQ used for event signal.
>> * @tf: Transfer function structure used by I/O operations.
>> * @tb: Transfer buffers and mutex used by I/O operations.
>> */
>> @@ -229,10 +237,12 @@ struct st_sensor_data {
>>
>> unsigned int odr;
>> unsigned int num_data_channels;
>> + unsigned int irq_map[ST_SENSORS_INT_MAX];
>>
>> u8 drdy_int_pin;
>>
>> unsigned int (*get_irq_data_ready) (struct iio_dev *indio_dev);
>> + unsigned int (*get_irq_event) (struct iio_dev *indio_dev);
>>
>> const struct st_sensor_transfer_function *tf;
>> struct st_sensor_transfer_buffer tb;
>> @@ -262,8 +272,7 @@ static inline void st_sensors_deallocate_trigger(struct iio_dev *indio_dev)
>> }
>> #endif
>>
>> -int st_sensors_init_sensor(struct iio_dev *indio_dev,
>> - struct st_sensors_platform_data *pdata);
>> +int st_sensors_init_sensor(struct iio_dev *indio_dev);
>>
>> int st_sensors_set_enable(struct iio_dev *indio_dev, bool enable);
>>
>> diff --git a/include/linux/platform_data/st_sensors_pdata.h b/include/linux/platform_data/st_sensors_pdata.h
>> index 7538391..991db72 100644
>> --- a/include/linux/platform_data/st_sensors_pdata.h
>> +++ b/include/linux/platform_data/st_sensors_pdata.h
>> @@ -19,6 +19,8 @@
>> */
>> struct st_sensors_platform_data {
>> u8 drdy_int_pin;
>> +
>> + unsigned int irqs[2];
>> };
>>
>> #endif /* ST_SENSORS_PDATA_H */
>>
> --
> 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] 30+ messages in thread* Re: [RFC v2 PATCH 01/14] iio: st_common: New interrupt interface
2013-10-01 15:34 ` Jonathan Cameron
2013-10-01 16:22 ` Jonathan Cameron
@ 2013-10-21 11:59 ` Mark Rutland
1 sibling, 0 replies; 30+ messages in thread
From: Mark Rutland @ 2013-10-21 11:59 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lukasz Czerwinski, jic23@cam.ac.uk, denis.ciocca@st.com,
linux-iio@vger.kernel.org, lee.jones@linaro.org,
devicetree@vger.kernel.org
On Tue, Oct 01, 2013 at 04:34:47PM +0100, Jonathan Cameron wrote:
> On 09/27/13 17:32, Lukasz Czerwinski wrote:
> > This patch adds two interrupts handling by the st_common library.
> > Additional second interrupt is used to indetify enabled event support for
> > chosen ST device. It supports board files and dt.
> >
> > For dt interface multiple interrupts are passed through interrupt-names
> > property.
> >
> > Read drdy_int_pin value is moved to the st_sensors_parse_platform_data()
> > in the st_sensors_i2c.c and st_sensors_spi.c Now drdy_int_pin can be
> > also configured via dt.
> >
> > Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>
> The code here is absolutely fine, though I would like some comments
> on this from Denis and/or Lee (as they are much more familiar with the driver
> than I am!
>
> The one element that is problematic is the use of interrupt names in specifying
> the two interrupts. Unforunately, for reasons that I think can be sumarized
> as historical precedence and avoiding complexity of passing in other OSes,
> the device tree powers that be are very anti doing multiple optional interrupts
> this way and consider interrupt-names to be merely for information rather than
> to be used to allow 'optional' interrupts.
There is some disagreement on this point. I think that using interrupt-names to
describe the set of interrupts you have makes a great deal of sense given how
common it is for only an arbitrary subset of interupts to be wired up.
There is an obvious problem in adding *-names properties to existing bindings
in that an existing OS will not know of the *-names property and may parse a DT
in a way that's not in accordance with the intended meaning. Depending on the
device this could mean a failure to probe, a failure to function, or a system
deadlock. This is an unfortunate problem, but one that we can work around on a
case-by-case basis.
For new bindings, I see no reason to not have interrupt-names from the start,
as a required component of the binding.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [RFC v2 PATCH 01/14] iio: st_common: New interrupt interface
2013-09-27 16:32 ` [RFC v2 PATCH 01/14] iio: st_common: New interrupt interface Lukasz Czerwinski
2013-10-01 15:34 ` Jonathan Cameron
@ 2013-10-21 11:48 ` Mark Rutland
1 sibling, 0 replies; 30+ messages in thread
From: Mark Rutland @ 2013-10-21 11:48 UTC (permalink / raw)
To: Lukasz Czerwinski
Cc: jic23@cam.ac.uk, denis.ciocca@st.com, linux-iio@vger.kernel.org,
lee.jones@linaro.org, devicetree@vger.kernel.org
On Fri, Sep 27, 2013 at 05:32:05PM +0100, Lukasz Czerwinski wrote:
> This patch adds two interrupts handling by the st_common library.
> Additional second interrupt is used to indetify enabled event support for
> chosen ST device. It supports board files and dt.
>
> For dt interface multiple interrupts are passed through interrupt-names
> property.
>
> Read drdy_int_pin value is moved to the st_sensors_parse_platform_data()
> in the st_sensors_i2c.c and st_sensors_spi.c Now drdy_int_pin can be
> also configured via dt.
>
> Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> drivers/iio/common/st_sensors/st_sensors_core.c | 36 +----------
> drivers/iio/common/st_sensors/st_sensors_i2c.c | 75 +++++++++++++++++++++-
> drivers/iio/common/st_sensors/st_sensors_spi.c | 77 ++++++++++++++++++++++-
> include/linux/iio/common/st_sensors.h | 13 +++-
> include/linux/platform_data/st_sensors_pdata.h | 2 +
> 5 files changed, 160 insertions(+), 43 deletions(-)
[...]
> +static void st_sensors_parse_platform_data(struct i2c_client *client,
> + struct iio_dev *indio_dev)
> +{
> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> + struct device_node *np = client->dev.of_node;
> + struct st_sensors_platform_data *pdata = client->dev.platform_data;
> +
> + if (pdata)
> + sdata->drdy_int_pin = pdata->drdy_int_pin;
> + else if (np)
> + of_property_read_u8(np, "st,drdy-int-pin",
> + (u8 *)&sdata->drdy_int_pin);
Why the cast?
What are valid values for this propertyy and what does it mean?
No sanity checking? Are there 256 pins?
> +}
> +
> +static unsigned int st_sensors_i2c_map_irq(struct i2c_client *client,
> + unsigned int irq_num)
> +{
> + struct device_node *np = client->dev.of_node;
> + struct st_sensors_platform_data *pdata = client->dev.platform_data;
> + int index = 0;
> +
> + if (pdata)
> + return pdata->irqs[irq_num];
> + else if (np) {
> + switch (irq_num) {
> + case ST_SENSORS_INT1:
> + index = of_property_match_string(np,
> + "interrupt-names",
> + ST_SENSORS_DRDY_IRQ_NAME);
> + break;
> + case ST_SENSORS_INT2:
> + index = of_property_match_string(np,
> + "interrupt-names",
> + ST_SENSORS_EVENT_IRQ_NAME);
> + default:
> + break;
> + }
> + return index < 0 ? 0 : irq_of_parse_and_map(np, index);
> + }
> + return 0;
> +}
Is there no platform_get_irq_byname equivalent for i2c devices? It seems like
naming the irq resources and using that would be a better solution.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC v2 PATCH 02/14] iio: st_accel: Add dt bindings
2013-09-27 16:32 [RFC v2 PATCH 00/14] iio: STMicroelectronics DT and event support Lukasz Czerwinski
2013-09-27 16:32 ` [RFC v2 PATCH 01/14] iio: st_common: New interrupt interface Lukasz Czerwinski
@ 2013-09-27 16:32 ` Lukasz Czerwinski
2013-10-01 15:40 ` Jonathan Cameron
2013-09-27 16:32 ` [RFC v2 PATCH 03/14] iio: st_gyro: " Lukasz Czerwinski
` (11 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Lukasz Czerwinski @ 2013-09-27 16:32 UTC (permalink / raw)
To: jic23; +Cc: denis.ciocca, linux-iio, lee.jones, devicetree
Add OF support for the st_accel.
Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/iio/accel/st_accel.h | 19 +++++++++----------
drivers/iio/accel/st_accel_core.c | 10 ++++------
drivers/iio/accel/st_accel_i2c.c | 19 ++++++++++++++++++-
drivers/iio/accel/st_accel_spi.c | 19 ++++++++++++++++++-
4 files changed, 49 insertions(+), 18 deletions(-)
diff --git a/drivers/iio/accel/st_accel.h b/drivers/iio/accel/st_accel.h
index c387763..08a0eb7 100644
--- a/drivers/iio/accel/st_accel.h
+++ b/drivers/iio/accel/st_accel.h
@@ -14,16 +14,16 @@
#include <linux/types.h>
#include <linux/iio/common/st_sensors.h>
-#define LSM303DLHC_ACCEL_DEV_NAME "lsm303dlhc_accel"
+#define LSM303DLHC_ACCEL_DEV_NAME "lsm303dlhc-accel"
#define LIS3DH_ACCEL_DEV_NAME "lis3dh"
-#define LSM330D_ACCEL_DEV_NAME "lsm330d_accel"
-#define LSM330DL_ACCEL_DEV_NAME "lsm330dl_accel"
-#define LSM330DLC_ACCEL_DEV_NAME "lsm330dlc_accel"
+#define LSM330D_ACCEL_DEV_NAME "lsm330d-accel"
+#define LSM330DL_ACCEL_DEV_NAME "lsm330dl-accel"
+#define LSM330DLC_ACCEL_DEV_NAME "lsm330dlc-accel"
#define LIS331DLH_ACCEL_DEV_NAME "lis331dlh"
-#define LSM303DL_ACCEL_DEV_NAME "lsm303dl_accel"
-#define LSM303DLH_ACCEL_DEV_NAME "lsm303dlh_accel"
-#define LSM303DLM_ACCEL_DEV_NAME "lsm303dlm_accel"
-#define LSM330_ACCEL_DEV_NAME "lsm330_accel"
+#define LSM303DL_ACCEL_DEV_NAME "lsm303dl-accel"
+#define LSM303DLH_ACCEL_DEV_NAME "lsm303dlh-accel"
+#define LSM303DLM_ACCEL_DEV_NAME "lsm303dlm-accel"
+#define LSM330_ACCEL_DEV_NAME "lsm330-accel"
/**
* struct st_sensors_platform_data - default accel platform data
@@ -33,8 +33,7 @@ static const struct st_sensors_platform_data default_accel_pdata = {
.drdy_int_pin = 1,
};
-int st_accel_common_probe(struct iio_dev *indio_dev,
- struct st_sensors_platform_data *pdata);
+int st_accel_common_probe(struct iio_dev *indio_dev);
void st_accel_common_remove(struct iio_dev *indio_dev);
#ifdef CONFIG_IIO_BUFFER
diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
index 38caedc..57e63bf 100644
--- a/drivers/iio/accel/st_accel_core.c
+++ b/drivers/iio/accel/st_accel_core.c
@@ -449,8 +449,7 @@ static const struct iio_trigger_ops st_accel_trigger_ops = {
#define ST_ACCEL_TRIGGER_OPS NULL
#endif
-int st_accel_common_probe(struct iio_dev *indio_dev,
- struct st_sensors_platform_data *plat_data)
+int st_accel_common_probe(struct iio_dev *indio_dev)
{
struct st_sensor_data *adata = iio_priv(indio_dev);
int irq = adata->get_irq_data_ready(indio_dev);
@@ -473,11 +472,10 @@ int st_accel_common_probe(struct iio_dev *indio_dev,
&adata->sensor->fs.fs_avl[0];
adata->odr = adata->sensor->odr.odr_avl[0].hz;
- if (!plat_data)
- plat_data =
- (struct st_sensors_platform_data *)&default_accel_pdata;
+ if (!adata->dev->platform_data && !adata->dev->of_node)
+ adata->drdy_int_pin = default_accel_pdata.drdy_int_pin;
- err = st_sensors_init_sensor(indio_dev, plat_data);
+ err = st_sensors_init_sensor(indio_dev);
if (err < 0)
return err;
diff --git a/drivers/iio/accel/st_accel_i2c.c b/drivers/iio/accel/st_accel_i2c.c
index d7bedbd..6c27ad44 100644
--- a/drivers/iio/accel/st_accel_i2c.c
+++ b/drivers/iio/accel/st_accel_i2c.c
@@ -34,7 +34,7 @@ static int st_accel_i2c_probe(struct i2c_client *client,
st_sensors_i2c_configure(indio_dev, client, adata);
- err = st_accel_common_probe(indio_dev, client->dev.platform_data);
+ err = st_accel_common_probe(indio_dev);
if (err < 0)
return err;
@@ -63,10 +63,27 @@ static const struct i2c_device_id st_accel_id_table[] = {
};
MODULE_DEVICE_TABLE(i2c, st_accel_id_table);
+#ifdef CONFIG_OF
+static struct of_device_id st_accel_dt_match[] = {
+ { .compatible = "st,lsm330dlhc_accel" },
+ { .compatible = "st,lis3dh" },
+ { .compatible = "st,lsm330d-accel" },
+ { .compatible = "st,lsm330dl-accel" },
+ { .compatible = "st,lsm330dlc-accel" },
+ { .compatible = "st,lsm331dlh" },
+ { .compatible = "st,lsm303dl-accel" },
+ { .compatible = "st,lsm303dlh-accel" },
+ { .compatible = "st,lsm303dlm-accel" },
+ { .compatible = "st,lsm330-accel" },
+ { }
+};
+#endif
+
static struct i2c_driver st_accel_driver = {
.driver = {
.owner = THIS_MODULE,
.name = "st-accel-i2c",
+ .of_match_table = of_match_ptr(st_accel_dt_match),
},
.probe = st_accel_i2c_probe,
.remove = st_accel_i2c_remove,
diff --git a/drivers/iio/accel/st_accel_spi.c b/drivers/iio/accel/st_accel_spi.c
index 1956396..e66548e 100644
--- a/drivers/iio/accel/st_accel_spi.c
+++ b/drivers/iio/accel/st_accel_spi.c
@@ -33,7 +33,7 @@ static int st_accel_spi_probe(struct spi_device *spi)
st_sensors_spi_configure(indio_dev, spi, adata);
- err = st_accel_common_probe(indio_dev, spi->dev.platform_data);
+ err = st_accel_common_probe(indio_dev);
if (err < 0)
return err;
@@ -62,10 +62,27 @@ static const struct spi_device_id st_accel_id_table[] = {
};
MODULE_DEVICE_TABLE(spi, st_accel_id_table);
+#ifdef CONFIG_OF
+static struct of_device_id st_accel_dt_match[] = {
+ { .compatible = "st,lsm330dlhc-accel" },
+ { .compatible = "st,lis3dh" },
+ { .compatible = "st,lsm330d-accel" },
+ { .compatible = "st,lsm330dl-accel" },
+ { .compatible = "st,lsm330dlc-accel" },
+ { .compatible = "st,lsm331dlh" },
+ { .compatible = "st,lsm303dl-accel" },
+ { .compatible = "st,lsm303dlh-accel" },
+ { .compatible = "st,lsm303dlm-accel" },
+ { .compatible = "st,lsm330-accel" },
+ { }
+};
+#endif
+
static struct spi_driver st_accel_driver = {
.driver = {
.owner = THIS_MODULE,
.name = "st-accel-spi",
+ .of_match_table = of_match_ptr(st_accel_dt_match),
},
.probe = st_accel_spi_probe,
.remove = st_accel_spi_remove,
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [RFC v2 PATCH 02/14] iio: st_accel: Add dt bindings
2013-09-27 16:32 ` [RFC v2 PATCH 02/14] iio: st_accel: Add dt bindings Lukasz Czerwinski
@ 2013-10-01 15:40 ` Jonathan Cameron
2013-10-21 11:37 ` Mark Rutland
0 siblings, 1 reply; 30+ messages in thread
From: Jonathan Cameron @ 2013-10-01 15:40 UTC (permalink / raw)
To: Lukasz Czerwinski, jic23; +Cc: denis.ciocca, linux-iio, lee.jones, devicetree
On 09/27/13 17:32, Lukasz Czerwinski wrote:
> Add OF support for the st_accel.
>
> Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
The dev name changes result in an ABI change. This was
being discussed deep in a thread related to Lee's patches
but I don't think has yet been resolved.
The issue is that they are exposed to userspace via sysfs
by both the i2c subsystem and IIO.
My gut feeling is we are stuck with the underscores but no one has
definitively come down and said we are find to leave them be
and break with DT conventions.
We can't even play games with having different naming in the device
tree and exposed to usespace because there are already DT files
using the implicit i2c bindings.
Otherwise this all looks fine to me.
> ---
> drivers/iio/accel/st_accel.h | 19 +++++++++----------
> drivers/iio/accel/st_accel_core.c | 10 ++++------
> drivers/iio/accel/st_accel_i2c.c | 19 ++++++++++++++++++-
> drivers/iio/accel/st_accel_spi.c | 19 ++++++++++++++++++-
> 4 files changed, 49 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/iio/accel/st_accel.h b/drivers/iio/accel/st_accel.h
> index c387763..08a0eb7 100644
> --- a/drivers/iio/accel/st_accel.h
> +++ b/drivers/iio/accel/st_accel.h
> @@ -14,16 +14,16 @@
> #include <linux/types.h>
> #include <linux/iio/common/st_sensors.h>
>
> -#define LSM303DLHC_ACCEL_DEV_NAME "lsm303dlhc_accel"
> +#define LSM303DLHC_ACCEL_DEV_NAME "lsm303dlhc-accel"
> #define LIS3DH_ACCEL_DEV_NAME "lis3dh"
> -#define LSM330D_ACCEL_DEV_NAME "lsm330d_accel"
> -#define LSM330DL_ACCEL_DEV_NAME "lsm330dl_accel"
> -#define LSM330DLC_ACCEL_DEV_NAME "lsm330dlc_accel"
> +#define LSM330D_ACCEL_DEV_NAME "lsm330d-accel"
> +#define LSM330DL_ACCEL_DEV_NAME "lsm330dl-accel"
> +#define LSM330DLC_ACCEL_DEV_NAME "lsm330dlc-accel"
> #define LIS331DLH_ACCEL_DEV_NAME "lis331dlh"
> -#define LSM303DL_ACCEL_DEV_NAME "lsm303dl_accel"
> -#define LSM303DLH_ACCEL_DEV_NAME "lsm303dlh_accel"
> -#define LSM303DLM_ACCEL_DEV_NAME "lsm303dlm_accel"
> -#define LSM330_ACCEL_DEV_NAME "lsm330_accel"
> +#define LSM303DL_ACCEL_DEV_NAME "lsm303dl-accel"
> +#define LSM303DLH_ACCEL_DEV_NAME "lsm303dlh-accel"
> +#define LSM303DLM_ACCEL_DEV_NAME "lsm303dlm-accel"
> +#define LSM330_ACCEL_DEV_NAME "lsm330-accel"
>
> /**
> * struct st_sensors_platform_data - default accel platform data
> @@ -33,8 +33,7 @@ static const struct st_sensors_platform_data default_accel_pdata = {
> .drdy_int_pin = 1,
> };
>
> -int st_accel_common_probe(struct iio_dev *indio_dev,
> - struct st_sensors_platform_data *pdata);
> +int st_accel_common_probe(struct iio_dev *indio_dev);
> void st_accel_common_remove(struct iio_dev *indio_dev);
>
> #ifdef CONFIG_IIO_BUFFER
> diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
> index 38caedc..57e63bf 100644
> --- a/drivers/iio/accel/st_accel_core.c
> +++ b/drivers/iio/accel/st_accel_core.c
> @@ -449,8 +449,7 @@ static const struct iio_trigger_ops st_accel_trigger_ops = {
> #define ST_ACCEL_TRIGGER_OPS NULL
> #endif
>
> -int st_accel_common_probe(struct iio_dev *indio_dev,
> - struct st_sensors_platform_data *plat_data)
> +int st_accel_common_probe(struct iio_dev *indio_dev)
> {
> struct st_sensor_data *adata = iio_priv(indio_dev);
> int irq = adata->get_irq_data_ready(indio_dev);
> @@ -473,11 +472,10 @@ int st_accel_common_probe(struct iio_dev *indio_dev,
> &adata->sensor->fs.fs_avl[0];
> adata->odr = adata->sensor->odr.odr_avl[0].hz;
>
> - if (!plat_data)
> - plat_data =
> - (struct st_sensors_platform_data *)&default_accel_pdata;
> + if (!adata->dev->platform_data && !adata->dev->of_node)
> + adata->drdy_int_pin = default_accel_pdata.drdy_int_pin;
>
> - err = st_sensors_init_sensor(indio_dev, plat_data);
> + err = st_sensors_init_sensor(indio_dev);
> if (err < 0)
> return err;
>
> diff --git a/drivers/iio/accel/st_accel_i2c.c b/drivers/iio/accel/st_accel_i2c.c
> index d7bedbd..6c27ad44 100644
> --- a/drivers/iio/accel/st_accel_i2c.c
> +++ b/drivers/iio/accel/st_accel_i2c.c
> @@ -34,7 +34,7 @@ static int st_accel_i2c_probe(struct i2c_client *client,
>
> st_sensors_i2c_configure(indio_dev, client, adata);
>
> - err = st_accel_common_probe(indio_dev, client->dev.platform_data);
> + err = st_accel_common_probe(indio_dev);
> if (err < 0)
> return err;
>
> @@ -63,10 +63,27 @@ static const struct i2c_device_id st_accel_id_table[] = {
> };
> MODULE_DEVICE_TABLE(i2c, st_accel_id_table);
>
> +#ifdef CONFIG_OF
> +static struct of_device_id st_accel_dt_match[] = {
> + { .compatible = "st,lsm330dlhc_accel" },
> + { .compatible = "st,lis3dh" },
> + { .compatible = "st,lsm330d-accel" },
> + { .compatible = "st,lsm330dl-accel" },
> + { .compatible = "st,lsm330dlc-accel" },
> + { .compatible = "st,lsm331dlh" },
> + { .compatible = "st,lsm303dl-accel" },
> + { .compatible = "st,lsm303dlh-accel" },
> + { .compatible = "st,lsm303dlm-accel" },
> + { .compatible = "st,lsm330-accel" },
> + { }
> +};
> +#endif
> +
> static struct i2c_driver st_accel_driver = {
> .driver = {
> .owner = THIS_MODULE,
> .name = "st-accel-i2c",
> + .of_match_table = of_match_ptr(st_accel_dt_match),
> },
> .probe = st_accel_i2c_probe,
> .remove = st_accel_i2c_remove,
> diff --git a/drivers/iio/accel/st_accel_spi.c b/drivers/iio/accel/st_accel_spi.c
> index 1956396..e66548e 100644
> --- a/drivers/iio/accel/st_accel_spi.c
> +++ b/drivers/iio/accel/st_accel_spi.c
> @@ -33,7 +33,7 @@ static int st_accel_spi_probe(struct spi_device *spi)
>
> st_sensors_spi_configure(indio_dev, spi, adata);
>
> - err = st_accel_common_probe(indio_dev, spi->dev.platform_data);
> + err = st_accel_common_probe(indio_dev);
> if (err < 0)
> return err;
>
> @@ -62,10 +62,27 @@ static const struct spi_device_id st_accel_id_table[] = {
> };
> MODULE_DEVICE_TABLE(spi, st_accel_id_table);
>
> +#ifdef CONFIG_OF
> +static struct of_device_id st_accel_dt_match[] = {
> + { .compatible = "st,lsm330dlhc-accel" },
> + { .compatible = "st,lis3dh" },
> + { .compatible = "st,lsm330d-accel" },
> + { .compatible = "st,lsm330dl-accel" },
> + { .compatible = "st,lsm330dlc-accel" },
> + { .compatible = "st,lsm331dlh" },
> + { .compatible = "st,lsm303dl-accel" },
> + { .compatible = "st,lsm303dlh-accel" },
> + { .compatible = "st,lsm303dlm-accel" },
> + { .compatible = "st,lsm330-accel" },
> + { }
> +};
> +#endif
> +
> static struct spi_driver st_accel_driver = {
> .driver = {
> .owner = THIS_MODULE,
> .name = "st-accel-spi",
> + .of_match_table = of_match_ptr(st_accel_dt_match),
> },
> .probe = st_accel_spi_probe,
> .remove = st_accel_spi_remove,
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC v2 PATCH 02/14] iio: st_accel: Add dt bindings
2013-10-01 15:40 ` Jonathan Cameron
@ 2013-10-21 11:37 ` Mark Rutland
0 siblings, 0 replies; 30+ messages in thread
From: Mark Rutland @ 2013-10-21 11:37 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lukasz Czerwinski, jic23@cam.ac.uk, denis.ciocca@st.com,
linux-iio@vger.kernel.org, lee.jones@linaro.org,
devicetree@vger.kernel.org
On Tue, Oct 01, 2013 at 04:40:58PM +0100, Jonathan Cameron wrote:
> On 09/27/13 17:32, Lukasz Czerwinski wrote:
> > Add OF support for the st_accel.
> >
> > Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>
> The dev name changes result in an ABI change. This was
> being discussed deep in a thread related to Lee's patches
> but I don't think has yet been resolved.
>
> The issue is that they are exposed to userspace via sysfs
> by both the i2c subsystem and IIO.
>
> My gut feeling is we are stuck with the underscores but no one has
> definitively come down and said we are find to leave them be
> and break with DT conventions.
I would very much prefer to not have Linux infrastructure problems affect the
device tree.
>
> We can't even play games with having different naming in the device
> tree and exposed to usespace because there are already DT files
> using the implicit i2c bindings.
We could fix this up my adding explicit (but marked legacy-only/deprecated)
of_device_id table entries for the implicit compatible strings already in use.
Once that's done we can try to clean up the implict match logic. We could at
least prevent implicit matches for drivers with of_match_id tables.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC v2 PATCH 03/14] iio: st_gyro: Add dt bindings
2013-09-27 16:32 [RFC v2 PATCH 00/14] iio: STMicroelectronics DT and event support Lukasz Czerwinski
2013-09-27 16:32 ` [RFC v2 PATCH 01/14] iio: st_common: New interrupt interface Lukasz Czerwinski
2013-09-27 16:32 ` [RFC v2 PATCH 02/14] iio: st_accel: Add dt bindings Lukasz Czerwinski
@ 2013-09-27 16:32 ` Lukasz Czerwinski
2013-09-27 16:32 ` [RFC v2 PATCH 04/14] iio: st_mang: " Lukasz Czerwinski
` (10 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: Lukasz Czerwinski @ 2013-09-27 16:32 UTC (permalink / raw)
To: jic23; +Cc: denis.ciocca, linux-iio, lee.jones, devicetree
Add OF support for the st_gyro.
Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/iio/gyro/st_gyro.h | 15 +++++++--------
drivers/iio/gyro/st_gyro_core.c | 8 +++++---
drivers/iio/gyro/st_gyro_i2c.c | 18 ++++++++++++++++--
drivers/iio/gyro/st_gyro_spi.c | 18 ++++++++++++++++--
4 files changed, 44 insertions(+), 15 deletions(-)
diff --git a/drivers/iio/gyro/st_gyro.h b/drivers/iio/gyro/st_gyro.h
index f8f2bf8..81e5ec3 100644
--- a/drivers/iio/gyro/st_gyro.h
+++ b/drivers/iio/gyro/st_gyro.h
@@ -15,24 +15,23 @@
#include <linux/iio/common/st_sensors.h>
#define L3G4200D_GYRO_DEV_NAME "l3g4200d"
-#define LSM330D_GYRO_DEV_NAME "lsm330d_gyro"
-#define LSM330DL_GYRO_DEV_NAME "lsm330dl_gyro"
-#define LSM330DLC_GYRO_DEV_NAME "lsm330dlc_gyro"
+#define LSM330D_GYRO_DEV_NAME "lsm330d-gyro"
+#define LSM330DL_GYRO_DEV_NAME "lsm330dl-gyro"
+#define LSM330DLC_GYRO_DEV_NAME "lsm330dlc-gyro"
#define L3GD20_GYRO_DEV_NAME "l3gd20"
#define L3GD20H_GYRO_DEV_NAME "l3gd20h"
-#define L3G4IS_GYRO_DEV_NAME "l3g4is_ui"
-#define LSM330_GYRO_DEV_NAME "lsm330_gyro"
+#define L3G4IS_GYRO_DEV_NAME "l3g4is-ui"
+#define LSM330_GYRO_DEV_NAME "lsm330-gyro"
/**
* struct st_sensors_platform_data - gyro platform data
* @drdy_int_pin: DRDY on gyros is available only on INT2 pin.
*/
-static const struct st_sensors_platform_data gyro_pdata = {
+static const struct st_sensors_platform_data default_gyro_pdata = {
.drdy_int_pin = 2,
};
-int st_gyro_common_probe(struct iio_dev *indio_dev,
- struct st_sensors_platform_data *pdata);
+int st_gyro_common_probe(struct iio_dev *indio_dev);
void st_gyro_common_remove(struct iio_dev *indio_dev);
#ifdef CONFIG_IIO_BUFFER
diff --git a/drivers/iio/gyro/st_gyro_core.c b/drivers/iio/gyro/st_gyro_core.c
index d53d91a..42f45af 100644
--- a/drivers/iio/gyro/st_gyro_core.c
+++ b/drivers/iio/gyro/st_gyro_core.c
@@ -302,8 +302,7 @@ static const struct iio_trigger_ops st_gyro_trigger_ops = {
#define ST_GYRO_TRIGGER_OPS NULL
#endif
-int st_gyro_common_probe(struct iio_dev *indio_dev,
- struct st_sensors_platform_data *pdata)
+int st_gyro_common_probe(struct iio_dev *indio_dev)
{
struct st_sensor_data *gdata = iio_priv(indio_dev);
int irq = gdata->get_irq_data_ready(indio_dev);
@@ -326,7 +325,10 @@ int st_gyro_common_probe(struct iio_dev *indio_dev,
&gdata->sensor->fs.fs_avl[0];
gdata->odr = gdata->sensor->odr.odr_avl[0].hz;
- err = st_sensors_init_sensor(indio_dev, pdata);
+ if (!gdata->dev->platform_data && !gdata->dev->of_node)
+ gdata->drdy_int_pin = default_gyro_pdata.drdy_int_pin;
+
+ err = st_sensors_init_sensor(indio_dev);
if (err < 0)
return err;
diff --git a/drivers/iio/gyro/st_gyro_i2c.c b/drivers/iio/gyro/st_gyro_i2c.c
index 16b8b8d..9c4245b 100644
--- a/drivers/iio/gyro/st_gyro_i2c.c
+++ b/drivers/iio/gyro/st_gyro_i2c.c
@@ -34,8 +34,8 @@ static int st_gyro_i2c_probe(struct i2c_client *client,
st_sensors_i2c_configure(indio_dev, client, gdata);
- err = st_gyro_common_probe(indio_dev,
- (struct st_sensors_platform_data *)&gyro_pdata);
+ err = st_gyro_common_probe(indio_dev);
+
if (err < 0)
return err;
@@ -62,10 +62,24 @@ static const struct i2c_device_id st_gyro_id_table[] = {
};
MODULE_DEVICE_TABLE(i2c, st_gyro_id_table);
+#ifdef CONFIG_OF
+static struct of_device_id st_gyro_dt_match[] = {
+ { .compatible = "st,l3g4200d" },
+ { .compatible = "st,lsm330d-gyro" },
+ { .compatible = "st,lsm330dl-gyro" },
+ { .compatible = "st,lsm330dlc-gyro" },
+ { .compatible = "st,l3gd20" },
+ { .compatible = "st,l3gd20h" },
+ { .compatible = "st,l3g4is-ui" },
+ { .compatible = "st,lsm330-gyro" },
+};
+#endif
+
static struct i2c_driver st_gyro_driver = {
.driver = {
.owner = THIS_MODULE,
.name = "st-gyro-i2c",
+ .of_match_table = of_match_ptr(st_gyro_dt_match),
},
.probe = st_gyro_i2c_probe,
.remove = st_gyro_i2c_remove,
diff --git a/drivers/iio/gyro/st_gyro_spi.c b/drivers/iio/gyro/st_gyro_spi.c
index 94763e2..1287af3 100644
--- a/drivers/iio/gyro/st_gyro_spi.c
+++ b/drivers/iio/gyro/st_gyro_spi.c
@@ -33,8 +33,8 @@ static int st_gyro_spi_probe(struct spi_device *spi)
st_sensors_spi_configure(indio_dev, spi, gdata);
- err = st_gyro_common_probe(indio_dev,
- (struct st_sensors_platform_data *)&gyro_pdata);
+ err = st_gyro_common_probe(indio_dev);
+
if (err < 0)
return err;
@@ -61,10 +61,24 @@ static const struct spi_device_id st_gyro_id_table[] = {
};
MODULE_DEVICE_TABLE(spi, st_gyro_id_table);
+#ifdef CONFIG_OF
+static struct of_device_id st_gyro_dt_match[] = {
+ { .compatible = "st,l3g4200d" },
+ { .compatible = "st,lsm330d-gyro" },
+ { .compatible = "st,lsm330dl-gyro" },
+ { .compatible = "st,lsm330dlc-gyro" },
+ { .compatible = "st,l3gd20" },
+ { .compatible = "st,l3gd20h" },
+ { .compatible = "st,l3g4is-ui" },
+ { .compatible = "st,lsm330-gyro" },
+};
+#endif
+
static struct spi_driver st_gyro_driver = {
.driver = {
.owner = THIS_MODULE,
.name = "st-gyro-spi",
+ .of_match_table = of_match_ptr(st_gyro_dt_match),
},
.probe = st_gyro_spi_probe,
.remove = st_gyro_spi_remove,
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC v2 PATCH 04/14] iio: st_mang: Add dt bindings
2013-09-27 16:32 [RFC v2 PATCH 00/14] iio: STMicroelectronics DT and event support Lukasz Czerwinski
` (2 preceding siblings ...)
2013-09-27 16:32 ` [RFC v2 PATCH 03/14] iio: st_gyro: " Lukasz Czerwinski
@ 2013-09-27 16:32 ` Lukasz Czerwinski
2013-09-27 16:32 ` [RFC v2 PATCH 05/14] iio: st_pressure: " Lukasz Czerwinski
` (9 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: Lukasz Czerwinski @ 2013-09-27 16:32 UTC (permalink / raw)
To: jic23; +Cc: denis.ciocca, linux-iio, lee.jones, devicetree
Add OF support for the st_magn.
Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/iio/magnetometer/st_magn.h | 3 +--
drivers/iio/magnetometer/st_magn_core.c | 5 ++---
drivers/iio/magnetometer/st_magn_i2c.c | 12 +++++++++++-
drivers/iio/magnetometer/st_magn_spi.c | 12 +++++++++++-
4 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/drivers/iio/magnetometer/st_magn.h b/drivers/iio/magnetometer/st_magn.h
index 694e33e..7e81d00 100644
--- a/drivers/iio/magnetometer/st_magn.h
+++ b/drivers/iio/magnetometer/st_magn.h
@@ -18,8 +18,7 @@
#define LSM303DLM_MAGN_DEV_NAME "lsm303dlm_magn"
#define LIS3MDL_MAGN_DEV_NAME "lis3mdl"
-int st_magn_common_probe(struct iio_dev *indio_dev,
- struct st_sensors_platform_data *pdata);
+int st_magn_common_probe(struct iio_dev *indio_dev);
void st_magn_common_remove(struct iio_dev *indio_dev);
#ifdef CONFIG_IIO_BUFFER
diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c
index 82461ef..4460075 100644
--- a/drivers/iio/magnetometer/st_magn_core.c
+++ b/drivers/iio/magnetometer/st_magn_core.c
@@ -345,8 +345,7 @@ static const struct iio_info magn_info = {
.write_raw = &st_magn_write_raw,
};
-int st_magn_common_probe(struct iio_dev *indio_dev,
- struct st_sensors_platform_data *pdata)
+int st_magn_common_probe(struct iio_dev *indio_dev)
{
struct st_sensor_data *mdata = iio_priv(indio_dev);
int irq = mdata->get_irq_data_ready(indio_dev);
@@ -369,7 +368,7 @@ int st_magn_common_probe(struct iio_dev *indio_dev,
&mdata->sensor->fs.fs_avl[0];
mdata->odr = mdata->sensor->odr.odr_avl[0].hz;
- err = st_sensors_init_sensor(indio_dev, pdata);
+ err = st_sensors_init_sensor(indio_dev);
if (err < 0)
return err;
diff --git a/drivers/iio/magnetometer/st_magn_i2c.c b/drivers/iio/magnetometer/st_magn_i2c.c
index 892e0fe..72a7cc2 100644
--- a/drivers/iio/magnetometer/st_magn_i2c.c
+++ b/drivers/iio/magnetometer/st_magn_i2c.c
@@ -34,7 +34,7 @@ static int st_magn_i2c_probe(struct i2c_client *client,
st_sensors_i2c_configure(indio_dev, client, mdata);
- err = st_magn_common_probe(indio_dev, NULL);
+ err = st_magn_common_probe(indio_dev);
if (err < 0)
return err;
@@ -57,10 +57,20 @@ static const struct i2c_device_id st_magn_id_table[] = {
};
MODULE_DEVICE_TABLE(i2c, st_magn_id_table);
+#ifdef CONFIG_OF
+static struct of_device_id st_magn_dt_match[] = {
+ { .compatible = "st,lsm303dlhc_magn" },
+ { .compatible = "st,lsm303dlm_magn" },
+ { .compatible = "st,lis3mdl" },
+ { }
+};
+#endif
+
static struct i2c_driver st_magn_driver = {
.driver = {
.owner = THIS_MODULE,
.name = "st-magn-i2c",
+ .of_match_table = of_match_ptr(st_magn_dt_match),
},
.probe = st_magn_i2c_probe,
.remove = st_magn_i2c_remove,
diff --git a/drivers/iio/magnetometer/st_magn_spi.c b/drivers/iio/magnetometer/st_magn_spi.c
index a6143ea..55c9691 100644
--- a/drivers/iio/magnetometer/st_magn_spi.c
+++ b/drivers/iio/magnetometer/st_magn_spi.c
@@ -33,7 +33,7 @@ static int st_magn_spi_probe(struct spi_device *spi)
st_sensors_spi_configure(indio_dev, spi, mdata);
- err = st_magn_common_probe(indio_dev, NULL);
+ err = st_magn_common_probe(indio_dev);
if (err < 0)
return err;
@@ -56,10 +56,20 @@ static const struct spi_device_id st_magn_id_table[] = {
};
MODULE_DEVICE_TABLE(spi, st_magn_id_table);
+#ifdef CONFIG_OF
+static struct of_device_id st_magn_dt_match[] = {
+ { .compatible = "st,lsm303dlhc_magn" },
+ { .compatible = "st,lsm303dlm_magn" },
+ { .compatible = "st,lis3mdl" },
+ { }
+};
+#endif
+
static struct spi_driver st_magn_driver = {
.driver = {
.owner = THIS_MODULE,
.name = "st-magn-spi",
+ .of_match_table = of_match_ptr(st_magn_dt_match),
},
.probe = st_magn_spi_probe,
.remove = st_magn_spi_remove,
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC v2 PATCH 05/14] iio: st_pressure: Add dt bindings
2013-09-27 16:32 [RFC v2 PATCH 00/14] iio: STMicroelectronics DT and event support Lukasz Czerwinski
` (3 preceding siblings ...)
2013-09-27 16:32 ` [RFC v2 PATCH 04/14] iio: st_mang: " Lukasz Czerwinski
@ 2013-09-27 16:32 ` Lukasz Czerwinski
2013-09-27 16:32 ` [RFC v2 PATCH 06/14] iio: st_common: Add threshold events support Lukasz Czerwinski
` (8 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: Lukasz Czerwinski @ 2013-09-27 16:32 UTC (permalink / raw)
To: jic23; +Cc: denis.ciocca, linux-iio, lee.jones, devicetree
Add OF support for the st_pressure.
Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/iio/pressure/st_pressure.h | 3 +--
drivers/iio/pressure/st_pressure_core.c | 10 ++++------
drivers/iio/pressure/st_pressure_i2c.c | 11 ++++++++++-
drivers/iio/pressure/st_pressure_spi.c | 11 ++++++++++-
4 files changed, 25 insertions(+), 10 deletions(-)
diff --git a/drivers/iio/pressure/st_pressure.h b/drivers/iio/pressure/st_pressure.h
index 049c21a..3b9e2fd 100644
--- a/drivers/iio/pressure/st_pressure.h
+++ b/drivers/iio/pressure/st_pressure.h
@@ -25,8 +25,7 @@ static const struct st_sensors_platform_data default_press_pdata = {
.drdy_int_pin = 1,
};
-int st_press_common_probe(struct iio_dev *indio_dev,
- struct st_sensors_platform_data *pdata);
+int st_press_common_probe(struct iio_dev *indio_dev);
void st_press_common_remove(struct iio_dev *indio_dev);
#ifdef CONFIG_IIO_BUFFER
diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
index 58083f9..f78bd33 100644
--- a/drivers/iio/pressure/st_pressure_core.c
+++ b/drivers/iio/pressure/st_pressure_core.c
@@ -348,8 +348,7 @@ static void st_press_power_disable(struct iio_dev *indio_dev)
regulator_disable(pdata->vdd_io);
}
-int st_press_common_probe(struct iio_dev *indio_dev,
- struct st_sensors_platform_data *plat_data)
+int st_press_common_probe(struct iio_dev *indio_dev)
{
struct st_sensor_data *pdata = iio_priv(indio_dev);
int irq = pdata->get_irq_data_ready(indio_dev);
@@ -378,11 +377,10 @@ int st_press_common_probe(struct iio_dev *indio_dev,
pdata->odr = pdata->sensor->odr.odr_avl[0].hz;
/* Some devices don't support a data ready pin. */
- if (!plat_data && pdata->sensor->drdy_irq.addr)
- plat_data =
- (struct st_sensors_platform_data *)&default_press_pdata;
+ if (!pdata->dev->platform_data && !pdata->dev->of_node)
+ pdata->drdy_int_pin = default_press_pdata.drdy_int_pin;
- err = st_sensors_init_sensor(indio_dev, plat_data);
+ err = st_sensors_init_sensor(indio_dev);
if (err < 0)
return err;
diff --git a/drivers/iio/pressure/st_pressure_i2c.c b/drivers/iio/pressure/st_pressure_i2c.c
index 51eab7f..6983a6d 100644
--- a/drivers/iio/pressure/st_pressure_i2c.c
+++ b/drivers/iio/pressure/st_pressure_i2c.c
@@ -34,7 +34,7 @@ static int st_press_i2c_probe(struct i2c_client *client,
st_sensors_i2c_configure(indio_dev, client, pdata);
- err = st_press_common_probe(indio_dev, client->dev.platform_data);
+ err = st_press_common_probe(indio_dev);
if (err < 0)
return err;
@@ -55,10 +55,19 @@ static const struct i2c_device_id st_press_id_table[] = {
};
MODULE_DEVICE_TABLE(i2c, st_press_id_table);
+#ifdef CONFIG_OF
+static struct of_device_id st_pressure_dt_match[] = {
+ { .compatible = "st,lps331ap" },
+ { .compatible = "st,lps001wp" },
+ { },
+};
+#endif
+
static struct i2c_driver st_press_driver = {
.driver = {
.owner = THIS_MODULE,
.name = "st-press-i2c",
+ .of_match_table = of_match_ptr(st_pressure_dt_match),
},
.probe = st_press_i2c_probe,
.remove = st_press_i2c_remove,
diff --git a/drivers/iio/pressure/st_pressure_spi.c b/drivers/iio/pressure/st_pressure_spi.c
index 399a29b..4045988 100644
--- a/drivers/iio/pressure/st_pressure_spi.c
+++ b/drivers/iio/pressure/st_pressure_spi.c
@@ -33,7 +33,7 @@ static int st_press_spi_probe(struct spi_device *spi)
st_sensors_spi_configure(indio_dev, spi, pdata);
- err = st_press_common_probe(indio_dev, spi->dev.platform_data);
+ err = st_press_common_probe(indio_dev);
if (err < 0)
return err;
@@ -53,10 +53,19 @@ static const struct spi_device_id st_press_id_table[] = {
};
MODULE_DEVICE_TABLE(spi, st_press_id_table);
+#ifdef CONFIG_OF
+static struct of_device_id st_pressure_dt_match[] = {
+ { .compatible = "st,lps331ap" },
+ { .compatible = "st,lps001wp" },
+ { },
+};
+#endif
+
static struct spi_driver st_press_driver = {
.driver = {
.owner = THIS_MODULE,
.name = "st-press-spi",
+ .of_match_table = of_match_ptr(st_pressure_dt_match),
},
.probe = st_press_spi_probe,
.remove = st_press_spi_remove,
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC v2 PATCH 06/14] iio: st_common: Add threshold events support
2013-09-27 16:32 [RFC v2 PATCH 00/14] iio: STMicroelectronics DT and event support Lukasz Czerwinski
` (4 preceding siblings ...)
2013-09-27 16:32 ` [RFC v2 PATCH 05/14] iio: st_pressure: " Lukasz Czerwinski
@ 2013-09-27 16:32 ` Lukasz Czerwinski
2013-10-01 16:09 ` Jonathan Cameron
2013-09-27 16:32 ` [RFC v2 PATCH 07/14] iio: st_accel: Add event subsystem to st_accel driver Lukasz Czerwinski
` (7 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Lukasz Czerwinski @ 2013-09-27 16:32 UTC (permalink / raw)
To: jic23; +Cc: denis.ciocca, linux-iio, lee.jones, devicetree
This patch adds threshold events support for the ST common
library.
Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/iio/common/st_sensors/st_sensors_core.c | 209 ++++++++++++++++++++++-
include/linux/iio/common/st_sensors.h | 74 ++++++++
2 files changed, 282 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
index 697b16d..98fb425 100644
--- a/drivers/iio/common/st_sensors/st_sensors_core.c
+++ b/drivers/iio/common/st_sensors/st_sensors_core.c
@@ -13,7 +13,9 @@
#include <linux/slab.h>
#include <linux/delay.h>
#include <linux/iio/iio.h>
+#include <linux/iio/events.h>
#include <asm/unaligned.h>
+#include <linux/interrupt.h>
#include <linux/iio/common/st_sensors.h>
@@ -339,7 +341,12 @@ int st_sensors_read_info_raw(struct iio_dev *indio_dev,
*val = *val >> ch->scan_type.shift;
- err = st_sensors_set_enable(indio_dev, false);
+ /*
+ * When events are enabled sensor should be always enabled.
+ * It prevents unnecessary sensor off.
+ */
+ if (!sdata->events_flag)
+ err = st_sensors_set_enable(indio_dev, false);
}
out:
mutex_unlock(&indio_dev->mlock);
@@ -465,6 +472,206 @@ ssize_t st_sensors_sysfs_scale_avail(struct device *dev,
}
EXPORT_SYMBOL(st_sensors_sysfs_scale_avail);
+static struct st_sensor_event *st_sensor_find_event_data(struct
+ st_sensor_data * sdata, u64 event_code)
+{
+ int mod = IIO_EVENT_CODE_EXTRACT_MODIFIER(event_code);
+ int chan_type = IIO_EVENT_CODE_EXTRACT_CHAN_TYPE(event_code);
+ int type = IIO_EVENT_CODE_EXTRACT_TYPE(event_code);
+ int dir = IIO_EVENT_CODE_EXTRACT_DIR(event_code);
+ struct st_sensor_event_irq *irq = &sdata->sensor->event_irq;
+ struct st_sensor_event *event;
+ int i;
+
+ if (!irq)
+ return NULL;
+
+ if (irq->event_count == 0)
+ return NULL;
+
+ for (i = 0; i < irq->event_count; i++) {
+ event = &irq->events[i];
+
+ if (event->modifier == mod &&
+ event->chan_type == chan_type &&
+ event->event_type == type &&
+ event->direction == dir)
+ return event;
+ }
+ return NULL;
+}
+
+int st_sensors_read_event_config(struct iio_dev *indio_dev,
+ u64 event_code)
+{
+ struct st_sensor_data *sdata = iio_priv(indio_dev);
+ struct st_sensor_event *event =
+ st_sensor_find_event_data(sdata, event_code);
+
+ if (!event || !sdata->events_enabled)
+ return -EINVAL;
+
+ return (bool)(sdata->events_flag & (1 << event->bit));
+}
+EXPORT_SYMBOL(st_sensors_read_event_config);
+
+int st_sensors_write_event_config(struct iio_dev *indio_dev,
+ u64 event_code,
+ int state)
+{
+ struct st_sensor_data *sdata = iio_priv(indio_dev);
+ struct st_sensor_event *event =
+ st_sensor_find_event_data(sdata, event_code);
+ int unsigned mask;
+ int err;
+
+ if (!event | !sdata->events_enabled)
+ return -EINVAL;
+
+ mutex_lock(&indio_dev->mlock);
+
+ mask = (1 << event->bit);
+ err = st_sensors_write_data_with_mask(indio_dev,
+ sdata->sensor->event_irq.ctrl_reg.addr,
+ mask, (bool)state);
+ if (err < 0)
+ goto exit;
+
+ if (state)
+ sdata->events_flag |= mask;
+ else
+ sdata->events_flag &= ~mask;
+
+ err = st_sensors_set_enable(indio_dev, (bool)sdata->events_flag);
+
+exit:
+ mutex_unlock(&indio_dev->mlock);
+ return err;
+}
+EXPORT_SYMBOL(st_sensors_write_event_config);
+
+int st_sensors_read_event_value(struct iio_dev *indio_dev,
+ u64 event_code,
+ int *val)
+{
+ struct st_sensor_data *sdata = iio_priv(indio_dev);
+ struct st_sensor_event *event =
+ st_sensor_find_event_data(sdata, event_code);
+ u8 byte;
+ int err;
+
+ if (!event | !sdata->events_enabled)
+ return -EINVAL;
+
+ mutex_lock(&indio_dev->mlock);
+
+ err = sdata->tf->read_byte(&sdata->tb, sdata->dev,
+ event->event_ths_reg.addr, &byte);
+ if (!err)
+ *val = byte;
+
+ mutex_unlock(&indio_dev->mlock);
+ return err;
+}
+EXPORT_SYMBOL(st_sensors_read_event_value);
+
+int st_sensors_write_event_value(struct iio_dev *indio_dev,
+ u64 event_code,
+ int val)
+{
+ struct st_sensor_data *sdata = iio_priv(indio_dev);
+ struct st_sensor_event *event =
+ st_sensor_find_event_data(sdata, event_code);
+ int err;
+
+ if (!event | !sdata->events_enabled)
+ return -EINVAL;
+
+ mutex_lock(&indio_dev->mlock);
+
+ err = st_sensors_write_data_with_mask(indio_dev,
+ event->event_ths_reg.addr,
+ event->event_ths_reg.mask,
+ (u8)val);
+
+ mutex_unlock(&indio_dev->mlock);
+
+ return err;
+}
+EXPORT_SYMBOL(st_sensors_write_event_value);
+
+static irqreturn_t st_sensor_event_handler(int irq, void *private)
+{
+ struct iio_dev *indio_dev = private;
+ struct st_sensor_data *sdata = iio_priv(indio_dev);
+ struct st_sensor_event_irq *irq_data =
+ &sdata->sensor->event_irq;
+ struct st_sensor_event *event;
+ s64 timestamp = iio_get_time_ns();
+ u8 status, mask, i;
+ int err = -EIO;
+
+ if (sdata)
+ err = sdata->tf->read_byte(&sdata->tb,
+ sdata->dev,
+ irq_data->status_reg.addr,
+ &status);
+
+ if (err < 0)
+ goto exit;
+
+ for (i = 0; i < irq_data->event_count; i++) {
+ event = &irq_data->events[i];
+ mask = (1 << event->bit);
+ if (status & mask)
+ iio_push_event(indio_dev,
+ IIO_MOD_EVENT_CODE(event->chan_type,
+ 0,
+ event->modifier,
+ event->event_type,
+ event->direction),
+ timestamp);
+ }
+
+exit:
+ return IRQ_HANDLED;
+}
+
+int st_sensors_request_event_irq(struct iio_dev *indio_dev)
+{
+ struct st_sensor_data *sdata = iio_priv(indio_dev);
+
+ return request_threaded_irq(sdata->get_irq_event(indio_dev),
+ NULL,
+ st_sensor_event_handler,
+ IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+ dev_name(&indio_dev->dev),
+ indio_dev);
+}
+
+int st_sensors_enable_events(struct iio_dev *indio_dev)
+{
+ int err;
+ struct st_sensor_data *sdata = iio_priv(indio_dev);
+ struct st_sensor_event_irq *irq = &sdata->sensor->event_irq;
+
+ err = st_sensors_write_data_with_mask(indio_dev,
+ irq->addr,
+ irq->mask,
+ 1);
+
+ if (err < 0)
+ goto error;
+
+ err = st_sensors_write_data_with_mask(indio_dev,
+ irq->ctrl_reg.addr,
+ irq->ctrl_reg.mask,
+ irq->ctrl_reg.val);
+error:
+ return err;
+}
+EXPORT_SYMBOL(st_sensors_enable_events);
+
MODULE_AUTHOR("Denis Ciocca <denis.ciocca@st.com>");
MODULE_DESCRIPTION("STMicroelectronics ST-sensors core");
MODULE_LICENSE("GPL v2");
diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
index 3f4a0f7..178dec8 100644
--- a/include/linux/iio/common/st_sensors.h
+++ b/include/linux/iio/common/st_sensors.h
@@ -14,6 +14,7 @@
#include <linux/i2c.h>
#include <linux/spi/spi.h>
#include <linux/irqreturn.h>
+#include <linux/iio/events.h>
#include <linux/iio/trigger.h>
#include <linux/bitops.h>
#include <linux/regulator/consumer.h>
@@ -25,6 +26,7 @@
#define ST_SENSORS_ODR_LIST_MAX 10
#define ST_SENSORS_FULLSCALE_AVL_MAX 10
+#define ST_SENSORS_EVENTS_MAX 10
#define ST_SENSORS_NUMBER_ALL_CHANNELS 4
#define ST_SENSORS_ENABLE_ALL_AXIS 0x07
@@ -47,6 +49,10 @@
#define ST_SENSORS_DRDY_IRQ_NAME "drdy-int"
#define ST_SENSORS_EVENT_IRQ_NAME "event-int"
+#define ST_SENSORS_LSM_EVENT_MASK \
+ (IIO_EV_BIT(IIO_EV_TYPE_THRESH, IIO_EV_DIR_RISING) | \
+ IIO_EV_BIT(IIO_EV_TYPE_THRESH, IIO_EV_DIR_FALLING))
+
#define ST_SENSORS_LSM_CHANNELS(device_type, mask, index, mod, \
ch2, s, endian, rbits, sbits, addr) \
{ \
@@ -63,6 +69,7 @@
.storagebits = sbits, \
.endianness = endian, \
}, \
+ .event_mask = ST_SENSORS_LSM_EVENT_MASK, \
}
#define ST_SENSOR_DEV_ATTR_SAMP_FREQ() \
@@ -114,6 +121,12 @@ struct st_sensor_fullscale {
struct st_sensor_fullscale_avl fs_avl[ST_SENSORS_FULLSCALE_AVL_MAX];
};
+struct st_sensor_register {
+ u8 addr;
+ u8 mask;
+ u8 val;
+};
+
/**
* struct st_sensor_bdu - ST sensor device block data update
* @addr: address of the register.
@@ -144,6 +157,46 @@ struct st_sensor_data_ready_irq {
};
/**
+ * struct st_sensor_event - event data.
+ * @bit: position of bit in status register related to event
+ * @chan: channel number.
+ * @chan_type: channel type.
+ * @modifier: modifier for the channel.
+ * @event_type: type of the event.
+ * @direction: direction of the event.
+ * @event_ths_reg: represents the threshold
+ * register of event.
+ */
+struct st_sensor_event {
+ u8 bit;
+ enum iio_chan_type chan_type;
+ enum iio_modifier modifier;
+ enum iio_event_type event_type;
+ enum iio_event_direction direction;
+ struct st_sensor_register event_ths_reg;
+};
+
+/**
+ * struct st_sensor_event_irq -ST sensor event interrupt.
+ * @addr: address of the interrupt register.
+ * @mask: mask to write on/off value.
+ * @event_count: number of events declared in @events array.
+ * @ctrl_reg: represents the control register
+ * of event system
+ * @status_reg: status register of event subsystem.
+ * @events array: driver events declared by user
+ */
+struct st_sensor_event_irq {
+ u8 addr;
+ u8 mask;
+ u8 event_count;
+ struct st_sensor_register ctrl_reg;
+ struct st_sensor_register status_reg;
+ struct st_sensor_event events[ST_SENSORS_EVENTS_MAX];
+};
+
+
+/**
* struct st_sensor_transfer_buffer - ST sensor device I/O buffer
* @buf_lock: Mutex to protect rx and tx buffers.
* @tx_buf: Buffer used by SPI transfer function to send data to the sensors.
@@ -184,6 +237,7 @@ struct st_sensor_transfer_function {
* @fs: Full scale register and full scale list available.
* @bdu: Block data update register.
* @drdy_irq: Data ready register of the sensor.
+ * @event_irq: Event register of the sensor.
* @multi_read_bit: Use or not particular bit for [I2C/SPI] multi-read.
* @bootime: samples to discard when sensor passing from power-down to power-up.
*/
@@ -198,6 +252,7 @@ struct st_sensors {
struct st_sensor_fullscale fs;
struct st_sensor_bdu bdu;
struct st_sensor_data_ready_irq drdy_irq;
+ struct st_sensor_event_irq event_irq;
bool multi_read_bit;
unsigned int bootime;
};
@@ -212,9 +267,11 @@ struct st_sensors {
* @vdd_io: Pointer to sensor's Vdd-IO power supply
* @enabled: Status of the sensor (false->off, true->on).
* @multiread_bit: Use or not particular bit for [I2C/SPI] multiread.
+ * @events_enable: Status of the sensor events (false->off, true->on).
* @buffer_data: Data used by buffer part.
* @odr: Output data rate of the sensor [Hz].
* num_data_channels: Number of data channels used in buffer.
+ * @events_flag: Data used by event part.
* @irq_map: Container of mapped IRQs.
* @drdy_int_pin: Redirect DRDY on pin 1 (1) or pin 2 (2).
* @get_irq_data_ready: Function to get the IRQ used for data ready signal.
@@ -232,11 +289,13 @@ struct st_sensor_data {
bool enabled;
bool multiread_bit;
+ bool events_enabled;
char *buffer_data;
unsigned int odr;
unsigned int num_data_channels;
+ unsigned int events_flag;
unsigned int irq_map[ST_SENSORS_INT_MAX];
u8 drdy_int_pin;
@@ -302,4 +361,19 @@ ssize_t st_sensors_sysfs_sampling_frequency_avail(struct device *dev,
ssize_t st_sensors_sysfs_scale_avail(struct device *dev,
struct device_attribute *attr, char *buf);
+int st_sensors_read_event_config(struct iio_dev *indio_dev,
+ u64 event_code);
+
+int st_sensors_write_event_config(struct iio_dev *indio_dev,
+ u64 event_code, int state);
+
+int st_sensors_read_event_value(struct iio_dev *indio_dev,
+ u64 event_code, int *val);
+
+int st_sensors_write_event_value(struct iio_dev *indio_dev,
+ u64 event_code, int val);
+
+int st_sensors_request_event_irq(struct iio_dev *indio_dev);
+
+int st_sensors_enable_events(struct iio_dev *indio_dev);
#endif /* ST_SENSORS_H */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [RFC v2 PATCH 06/14] iio: st_common: Add threshold events support
2013-09-27 16:32 ` [RFC v2 PATCH 06/14] iio: st_common: Add threshold events support Lukasz Czerwinski
@ 2013-10-01 16:09 ` Jonathan Cameron
0 siblings, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2013-10-01 16:09 UTC (permalink / raw)
To: Lukasz Czerwinski, jic23; +Cc: denis.ciocca, linux-iio, lee.jones, devicetree
On 09/27/13 17:32, Lukasz Czerwinski wrote:
> This patch adds threshold events support for the ST common
> library.
>
> Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Hi Lukasz,
Note that we are moving over to a more adaptable way of event registration.
This driver will need updating to match that, but this review is conducted
against the old method.
If you are happy to do the update, that's great. If not such is life and
someone else will have to pick it up before we can drop the old method.
A few minor bits inline.
> ---
> drivers/iio/common/st_sensors/st_sensors_core.c | 209 ++++++++++++++++++++++-
> include/linux/iio/common/st_sensors.h | 74 ++++++++
> 2 files changed, 282 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c
> index 697b16d..98fb425 100644
> --- a/drivers/iio/common/st_sensors/st_sensors_core.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_core.c
> @@ -13,7 +13,9 @@
> #include <linux/slab.h>
> #include <linux/delay.h>
> #include <linux/iio/iio.h>
> +#include <linux/iio/events.h>
> #include <asm/unaligned.h>
> +#include <linux/interrupt.h>
>
> #include <linux/iio/common/st_sensors.h>
>
> @@ -339,7 +341,12 @@ int st_sensors_read_info_raw(struct iio_dev *indio_dev,
>
> *val = *val >> ch->scan_type.shift;
>
> - err = st_sensors_set_enable(indio_dev, false);
> + /*
> + * When events are enabled sensor should be always enabled.
> + * It prevents unnecessary sensor off.
> + */
> + if (!sdata->events_flag)
> + err = st_sensors_set_enable(indio_dev, false);
> }
> out:
> mutex_unlock(&indio_dev->mlock);
> @@ -465,6 +472,206 @@ ssize_t st_sensors_sysfs_scale_avail(struct device *dev,
> }
> EXPORT_SYMBOL(st_sensors_sysfs_scale_avail);
>
This should get simpler after Lars' series but will still be a little
ugly. Ah well, only occurs in relatively slow paths.
> +static struct st_sensor_event *st_sensor_find_event_data(struct
> + st_sensor_data * sdata, u64 event_code)
> +{
> + int mod = IIO_EVENT_CODE_EXTRACT_MODIFIER(event_code);
> + int chan_type = IIO_EVENT_CODE_EXTRACT_CHAN_TYPE(event_code);
> + int type = IIO_EVENT_CODE_EXTRACT_TYPE(event_code);
> + int dir = IIO_EVENT_CODE_EXTRACT_DIR(event_code);
> + struct st_sensor_event_irq *irq = &sdata->sensor->event_irq;
> + struct st_sensor_event *event;
> + int i;
> +
If either of these two tests fails and you get here, hasn't
something gone horribly wrong?
> + if (!irq)
> + return NULL;
> +
> + if (irq->event_count == 0)
> + return NULL;
> +
> + for (i = 0; i < irq->event_count; i++) {
> + event = &irq->events[i];
> +
> + if (event->modifier == mod &&
> + event->chan_type == chan_type &&
> + event->event_type == type &&
> + event->direction == dir)
> + return event;
> + }
> + return NULL;
> +}
> +
> +int st_sensors_read_event_config(struct iio_dev *indio_dev,
> + u64 event_code)
> +{
> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> + struct st_sensor_event *event =
> + st_sensor_find_event_data(sdata, event_code);
> +
> + if (!event || !sdata->events_enabled)
> + return -EINVAL;
> +
> + return (bool)(sdata->events_flag & (1 << event->bit));
> +}
> +EXPORT_SYMBOL(st_sensors_read_event_config);
> +
> +int st_sensors_write_event_config(struct iio_dev *indio_dev,
> + u64 event_code,
> + int state)
> +{
> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> + struct st_sensor_event *event =
> + st_sensor_find_event_data(sdata, event_code);
> + int unsigned mask;
> + int err;
> +
> + if (!event | !sdata->events_enabled)
> + return -EINVAL;
> +
> + mutex_lock(&indio_dev->mlock);
> +
> + mask = (1 << event->bit);
> + err = st_sensors_write_data_with_mask(indio_dev,
> + sdata->sensor->event_irq.ctrl_reg.addr,
> + mask, (bool)state);
> + if (err < 0)
> + goto exit;
> +
> + if (state)
> + sdata->events_flag |= mask;
> + else
> + sdata->events_flag &= ~mask;
> +
> + err = st_sensors_set_enable(indio_dev, (bool)sdata->events_flag);
> +
> +exit:
> + mutex_unlock(&indio_dev->mlock);
> + return err;
> +}
> +EXPORT_SYMBOL(st_sensors_write_event_config);
> +
> +int st_sensors_read_event_value(struct iio_dev *indio_dev,
> + u64 event_code,
> + int *val)
> +{
> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> + struct st_sensor_event *event =
> + st_sensor_find_event_data(sdata, event_code);
> + u8 byte;
> + int err;
> +
> + if (!event | !sdata->events_enabled)
> + return -EINVAL;
> +
> + mutex_lock(&indio_dev->mlock);
> +
> + err = sdata->tf->read_byte(&sdata->tb, sdata->dev,
> + event->event_ths_reg.addr, &byte);
> + if (!err)
> + *val = byte;
> +
> + mutex_unlock(&indio_dev->mlock);
> + return err;
> +}
> +EXPORT_SYMBOL(st_sensors_read_event_value);
> +
> +int st_sensors_write_event_value(struct iio_dev *indio_dev,
> + u64 event_code,
> + int val)
> +{
> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> + struct st_sensor_event *event =
> + st_sensor_find_event_data(sdata, event_code);
> + int err;
> +
> + if (!event | !sdata->events_enabled)
> + return -EINVAL;
> +
> + mutex_lock(&indio_dev->mlock);
> +
> + err = st_sensors_write_data_with_mask(indio_dev,
> + event->event_ths_reg.addr,
> + event->event_ths_reg.mask,
> + (u8)val);
> +
> + mutex_unlock(&indio_dev->mlock);
> +
> + return err;
> +}
> +EXPORT_SYMBOL(st_sensors_write_event_value);
> +
> +static irqreturn_t st_sensor_event_handler(int irq, void *private)
> +{
> + struct iio_dev *indio_dev = private;
> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> + struct st_sensor_event_irq *irq_data =
> + &sdata->sensor->event_irq;
> + struct st_sensor_event *event;
> + s64 timestamp = iio_get_time_ns();
> + u8 status, mask, i;
> + int err = -EIO;
> +
I'm a little worried by this. How would we ever get here
without sdata pointing anywhere? If this is dropped,
remember to drop the -EIO above as well.
> + if (sdata)
> + err = sdata->tf->read_byte(&sdata->tb,
> + sdata->dev,
> + irq_data->status_reg.addr,
> + &status);
> +
> + if (err < 0)
return IRQ_HANDLED;
> + goto exit;
> +
> + for (i = 0; i < irq_data->event_count; i++) {
> + event = &irq_data->events[i];
> + mask = (1 << event->bit);
> + if (status & mask)
> + iio_push_event(indio_dev,
> + IIO_MOD_EVENT_CODE(event->chan_type,
> + 0,
> + event->modifier,
> + event->event_type,
> + event->direction),
> + timestamp);
> + }
> +
> +exit:
> + return IRQ_HANDLED;
> +}
> +
> +int st_sensors_request_event_irq(struct iio_dev *indio_dev)
> +{
> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> +
> + return request_threaded_irq(sdata->get_irq_event(indio_dev),
> + NULL,
> + st_sensor_event_handler,
> + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
Naming wise, the device name is a bit generic, perhaps
have -event on the end?
Also, for symmetry I'd normally expect to see a matching
st_sensors_free_event_irq.
Clearly it would be trivial, but it make the code more
'obviously' right which is always a good thing.
> + dev_name(&indio_dev->dev),
> + indio_dev);
> +}
> +
> +int st_sensors_enable_events(struct iio_dev *indio_dev)
> +{
> + int err;
> + struct st_sensor_data *sdata = iio_priv(indio_dev);
> + struct st_sensor_event_irq *irq = &sdata->sensor->event_irq;
> +
> + err = st_sensors_write_data_with_mask(indio_dev,
> + irq->addr,
> + irq->mask,
> + 1);
> +
> + if (err < 0)
return err; is cleaner here rather than the goto given
there is no cleaning up to do.
> + goto error;
> +
> + err = st_sensors_write_data_with_mask(indio_dev,
> + irq->ctrl_reg.addr,
> + irq->ctrl_reg.mask,
> + irq->ctrl_reg.val);
> +error:
> + return err;
> +}
> +EXPORT_SYMBOL(st_sensors_enable_events);
> +
> MODULE_AUTHOR("Denis Ciocca <denis.ciocca@st.com>");
> MODULE_DESCRIPTION("STMicroelectronics ST-sensors core");
> MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/iio/common/st_sensors.h b/include/linux/iio/common/st_sensors.h
> index 3f4a0f7..178dec8 100644
> --- a/include/linux/iio/common/st_sensors.h
> +++ b/include/linux/iio/common/st_sensors.h
> @@ -14,6 +14,7 @@
> #include <linux/i2c.h>
> #include <linux/spi/spi.h>
> #include <linux/irqreturn.h>
> +#include <linux/iio/events.h>
> #include <linux/iio/trigger.h>
> #include <linux/bitops.h>
> #include <linux/regulator/consumer.h>
> @@ -25,6 +26,7 @@
>
> #define ST_SENSORS_ODR_LIST_MAX 10
> #define ST_SENSORS_FULLSCALE_AVL_MAX 10
> +#define ST_SENSORS_EVENTS_MAX 10
>
> #define ST_SENSORS_NUMBER_ALL_CHANNELS 4
> #define ST_SENSORS_ENABLE_ALL_AXIS 0x07
> @@ -47,6 +49,10 @@
> #define ST_SENSORS_DRDY_IRQ_NAME "drdy-int"
> #define ST_SENSORS_EVENT_IRQ_NAME "event-int"
>
> +#define ST_SENSORS_LSM_EVENT_MASK \
> + (IIO_EV_BIT(IIO_EV_TYPE_THRESH, IIO_EV_DIR_RISING) | \
> + IIO_EV_BIT(IIO_EV_TYPE_THRESH, IIO_EV_DIR_FALLING))
> +
> #define ST_SENSORS_LSM_CHANNELS(device_type, mask, index, mod, \
> ch2, s, endian, rbits, sbits, addr) \
> { \
> @@ -63,6 +69,7 @@
> .storagebits = sbits, \
> .endianness = endian, \
> }, \
> + .event_mask = ST_SENSORS_LSM_EVENT_MASK, \
> }
>
> #define ST_SENSOR_DEV_ATTR_SAMP_FREQ() \
> @@ -114,6 +121,12 @@ struct st_sensor_fullscale {
> struct st_sensor_fullscale_avl fs_avl[ST_SENSORS_FULLSCALE_AVL_MAX];
> };
>
> +struct st_sensor_register {
> + u8 addr;
> + u8 mask;
> + u8 val;
> +};
> +
> /**
> * struct st_sensor_bdu - ST sensor device block data update
> * @addr: address of the register.
> @@ -144,6 +157,46 @@ struct st_sensor_data_ready_irq {
> };
>
> /**
> + * struct st_sensor_event - event data.
> + * @bit: position of bit in status register related to event
> + * @chan: channel number.
> + * @chan_type: channel type.
> + * @modifier: modifier for the channel.
> + * @event_type: type of the event.
> + * @direction: direction of the event.
> + * @event_ths_reg: represents the threshold
> + * register of event.
> + */
> +struct st_sensor_event {
> + u8 bit;
The approach used in Lars' new approach to the
event callbacks makes this simpler by using
a pointer to the relevant channel. Otherwise
this is very similar which should make lifte
easy when converting over.
> + enum iio_chan_type chan_type;
> + enum iio_modifier modifier;
> + enum iio_event_type event_type;
> + enum iio_event_direction direction;
> + struct st_sensor_register event_ths_reg;
> +};
> +
> +/**
> + * struct st_sensor_event_irq -ST sensor event interrupt.
> + * @addr: address of the interrupt register.
> + * @mask: mask to write on/off value.
> + * @event_count: number of events declared in @events array.
> + * @ctrl_reg: represents the control register
> + * of event system
> + * @status_reg: status register of event subsystem.
> + * @events array: driver events declared by user
> + */
> +struct st_sensor_event_irq {
> + u8 addr;
> + u8 mask;
> + u8 event_count;
> + struct st_sensor_register ctrl_reg;
> + struct st_sensor_register status_reg;
> + struct st_sensor_event events[ST_SENSORS_EVENTS_MAX];
> +};
> +
One blank line is all that is needed here.
> +
> +/**
> * struct st_sensor_transfer_buffer - ST sensor device I/O buffer
> * @buf_lock: Mutex to protect rx and tx buffers.
> * @tx_buf: Buffer used by SPI transfer function to send data to the sensors.
> @@ -184,6 +237,7 @@ struct st_sensor_transfer_function {
> * @fs: Full scale register and full scale list available.
> * @bdu: Block data update register.
> * @drdy_irq: Data ready register of the sensor.
> + * @event_irq: Event register of the sensor.
> * @multi_read_bit: Use or not particular bit for [I2C/SPI] multi-read.
> * @bootime: samples to discard when sensor passing from power-down to power-up.
> */
> @@ -198,6 +252,7 @@ struct st_sensors {
> struct st_sensor_fullscale fs;
> struct st_sensor_bdu bdu;
> struct st_sensor_data_ready_irq drdy_irq;
> + struct st_sensor_event_irq event_irq;
> bool multi_read_bit;
> unsigned int bootime;
> };
> @@ -212,9 +267,11 @@ struct st_sensors {
> * @vdd_io: Pointer to sensor's Vdd-IO power supply
> * @enabled: Status of the sensor (false->off, true->on).
> * @multiread_bit: Use or not particular bit for [I2C/SPI] multiread.
> + * @events_enable: Status of the sensor events (false->off, true->on).
> * @buffer_data: Data used by buffer part.
> * @odr: Output data rate of the sensor [Hz].
> * num_data_channels: Number of data channels used in buffer.
> + * @events_flag: Data used by event part.
> * @irq_map: Container of mapped IRQs.
> * @drdy_int_pin: Redirect DRDY on pin 1 (1) or pin 2 (2).
> * @get_irq_data_ready: Function to get the IRQ used for data ready signal.
> @@ -232,11 +289,13 @@ struct st_sensor_data {
>
> bool enabled;
> bool multiread_bit;
> + bool events_enabled;
>
> char *buffer_data;
>
> unsigned int odr;
> unsigned int num_data_channels;
> + unsigned int events_flag;
> unsigned int irq_map[ST_SENSORS_INT_MAX];
>
> u8 drdy_int_pin;
> @@ -302,4 +361,19 @@ ssize_t st_sensors_sysfs_sampling_frequency_avail(struct device *dev,
> ssize_t st_sensors_sysfs_scale_avail(struct device *dev,
> struct device_attribute *attr, char *buf);
>
> +int st_sensors_read_event_config(struct iio_dev *indio_dev,
> + u64 event_code);
> +
> +int st_sensors_write_event_config(struct iio_dev *indio_dev,
> + u64 event_code, int state);
> +
> +int st_sensors_read_event_value(struct iio_dev *indio_dev,
> + u64 event_code, int *val);
> +
> +int st_sensors_write_event_value(struct iio_dev *indio_dev,
> + u64 event_code, int val);
> +
> +int st_sensors_request_event_irq(struct iio_dev *indio_dev);
> +
> +int st_sensors_enable_events(struct iio_dev *indio_dev);
> #endif /* ST_SENSORS_H */
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC v2 PATCH 07/14] iio: st_accel: Add event subsystem to st_accel driver
2013-09-27 16:32 [RFC v2 PATCH 00/14] iio: STMicroelectronics DT and event support Lukasz Czerwinski
` (5 preceding siblings ...)
2013-09-27 16:32 ` [RFC v2 PATCH 06/14] iio: st_common: Add threshold events support Lukasz Czerwinski
@ 2013-09-27 16:32 ` Lukasz Czerwinski
2013-10-01 16:17 ` Jonathan Cameron
2013-09-27 16:32 ` [RFC v2 PATCH 08/14] iio: iio_magn: Add event ops Lukasz Czerwinski
` (6 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Lukasz Czerwinski @ 2013-09-27 16:32 UTC (permalink / raw)
To: jic23; +Cc: denis.ciocca, linux-iio, lee.jones, devicetree
This patch adds event support for iio st_accel driver.
Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/iio/accel/st_accel_core.c | 96 ++++++++++++++++++++++++++++++++++---
1 file changed, 90 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
index 57e63bf..ec5f7ed 100644
--- a/drivers/iio/accel/st_accel_core.c
+++ b/drivers/iio/accel/st_accel_core.c
@@ -19,6 +19,7 @@
#include <linux/gpio.h>
#include <linux/irq.h>
#include <linux/iio/iio.h>
+#include <linux/iio/events.h>
#include <linux/iio/sysfs.h>
#include <linux/iio/trigger.h>
#include <linux/iio/buffer.h>
@@ -68,6 +69,20 @@
#define ST_ACCEL_1_DRDY_IRQ_INT1_MASK 0x10
#define ST_ACCEL_1_DRDY_IRQ_INT2_MASK 0x08
#define ST_ACCEL_1_MULTIREAD_BIT true
+#define ST_ACCEL_1_EVENT_IRQ_ADDR 0x22
+#define ST_ACCEL_1_EVENT_IRQ_MASK 0x40
+#define ST_ACCEL_1_EVENT_CTRL_REG 0x30
+#define ST_ACCEL_1_EVENT_CTRL_MASK 0xc0
+#define ST_ACCEL_1_EVENT_CTRL_DEFAULT_VAL 0x01
+#define ST_ACCEL_1_THS_REG 0x32
+#define ST_ACCEL_1_EVENT_STATUS_REG 0x31
+#define ST_ACCEL_1_EVENT_STATUS_REG_MASK 0x7f
+#define ST_ACCEL_1_INT1_XL 0
+#define ST_ACCEL_1_INT1_XH 1
+#define ST_ACCEL_1_INT1_YL 2
+#define ST_ACCEL_1_INT1_YH 3
+#define ST_ACCEL_1_INT1_ZL 4
+#define ST_ACCEL_1_INT1_ZH 5
/* CUSTOM VALUES FOR SENSOR 2 */
#define ST_ACCEL_2_WAI_EXP 0x32
@@ -129,6 +144,19 @@
#define ST_ACCEL_3_IG1_EN_MASK 0x08
#define ST_ACCEL_3_MULTIREAD_BIT false
+#define ST_SENSORS_LSM_ACCEL_EVENT(mod, dir, bit_pos, ths_reg) \
+{ \
+ .bit = bit_pos,\
+ .chan_type = IIO_ACCEL, \
+ .modifier = mod, \
+ .event_type = IIO_EV_TYPE_THRESH, \
+ .direction = dir, \
+ .event_ths_reg = { \
+ .addr = ths_reg, \
+ .mask = 0x7f, \
+ }, \
+}
+
static const struct iio_chan_spec st_accel_12bit_channels[] = {
ST_SENSORS_LSM_CHANNELS(IIO_ACCEL,
BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
@@ -230,6 +258,46 @@ static const struct st_sensors st_accel_sensors[] = {
.mask_int1 = ST_ACCEL_1_DRDY_IRQ_INT1_MASK,
.mask_int2 = ST_ACCEL_1_DRDY_IRQ_INT2_MASK,
},
+ .event_irq = {
+ .addr = ST_ACCEL_1_EVENT_IRQ_ADDR,
+ .mask = ST_ACCEL_1_EVENT_IRQ_MASK,
+ .event_count = 6,
+ .ctrl_reg = {
+ .addr = ST_ACCEL_1_EVENT_CTRL_REG,
+ .mask = ST_ACCEL_1_EVENT_CTRL_MASK,
+ .val = ST_ACCEL_1_EVENT_CTRL_DEFAULT_VAL,
+ },
+ .status_reg = {
+ .addr = ST_ACCEL_1_EVENT_STATUS_REG,
+ .mask = ST_ACCEL_1_EVENT_STATUS_REG_MASK,
+ },
+ .events = {
+ ST_SENSORS_LSM_ACCEL_EVENT(IIO_MOD_X,
+ IIO_EV_DIR_FALLING,
+ ST_ACCEL_1_INT1_XL,
+ ST_ACCEL_1_THS_REG),
+ ST_SENSORS_LSM_ACCEL_EVENT(IIO_MOD_X,
+ IIO_EV_DIR_RISING,
+ ST_ACCEL_1_INT1_XH,
+ ST_ACCEL_1_THS_REG),
+ ST_SENSORS_LSM_ACCEL_EVENT(IIO_MOD_Y,
+ IIO_EV_DIR_FALLING,
+ ST_ACCEL_1_INT1_YL,
+ ST_ACCEL_1_THS_REG),
+ ST_SENSORS_LSM_ACCEL_EVENT(IIO_MOD_Y,
+ IIO_EV_DIR_RISING,
+ ST_ACCEL_1_INT1_YH,
+ ST_ACCEL_1_THS_REG),
+ ST_SENSORS_LSM_ACCEL_EVENT(IIO_MOD_Z,
+ IIO_EV_DIR_FALLING,
+ ST_ACCEL_1_INT1_ZL,
+ ST_ACCEL_1_THS_REG),
+ ST_SENSORS_LSM_ACCEL_EVENT(IIO_MOD_Z,
+ IIO_EV_DIR_RISING,
+ ST_ACCEL_1_INT1_ZH,
+ ST_ACCEL_1_THS_REG),
+ },
+ },
.multi_read_bit = ST_ACCEL_1_MULTIREAD_BIT,
.bootime = 2,
},
@@ -437,6 +505,10 @@ static const struct iio_info accel_info = {
.attrs = &st_accel_attribute_group,
.read_raw = &st_accel_read_raw,
.write_raw = &st_accel_write_raw,
+ .read_event_value = &st_sensors_read_event_value,
+ .write_event_value = &st_sensors_write_event_value,
+ .read_event_config = &st_sensors_read_event_config,
+ .write_event_config = &st_sensors_write_event_config,
};
#ifdef CONFIG_IIO_TRIGGER
@@ -452,7 +524,6 @@ static const struct iio_trigger_ops st_accel_trigger_ops = {
int st_accel_common_probe(struct iio_dev *indio_dev)
{
struct st_sensor_data *adata = iio_priv(indio_dev);
- int irq = adata->get_irq_data_ready(indio_dev);
int err;
indio_dev->modes = INDIO_DIRECT_MODE;
@@ -483,13 +554,24 @@ int st_accel_common_probe(struct iio_dev *indio_dev)
if (err < 0)
return err;
- if (irq > 0) {
+ if (adata->get_irq_data_ready(indio_dev) > 0) {
err = st_sensors_allocate_trigger(indio_dev,
ST_ACCEL_TRIGGER_OPS);
if (err < 0)
goto st_accel_probe_trigger_error;
}
+ if (adata->get_irq_event(indio_dev) > 0) {
+ adata->events_enabled = true;
+ err = st_sensors_request_event_irq(indio_dev);
+ if (err < 0)
+ goto st_accel_request_irq_event_error;
+
+ err = st_sensors_enable_events(indio_dev);
+ if (err < 0)
+ goto st_accel_enable_events_error;
+ }
+
err = iio_device_register(indio_dev);
if (err)
goto st_accel_device_register_error;
@@ -497,9 +579,11 @@ int st_accel_common_probe(struct iio_dev *indio_dev)
return 0;
st_accel_device_register_error:
- if (irq > 0)
+st_accel_enable_events_error:
+ if (adata->get_irq_data_ready(indio_dev) > 0)
st_sensors_deallocate_trigger(indio_dev);
st_accel_probe_trigger_error:
+st_accel_request_irq_event_error:
st_accel_deallocate_ring(indio_dev);
return err;
@@ -511,10 +595,10 @@ void st_accel_common_remove(struct iio_dev *indio_dev)
struct st_sensor_data *adata = iio_priv(indio_dev);
iio_device_unregister(indio_dev);
- if (adata->get_irq_data_ready(indio_dev) > 0)
+ if (adata->get_irq_data_ready(indio_dev) > 0) {
st_sensors_deallocate_trigger(indio_dev);
-
- st_accel_deallocate_ring(indio_dev);
+ st_accel_deallocate_ring(indio_dev);
+ }
}
EXPORT_SYMBOL(st_accel_common_remove);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [RFC v2 PATCH 07/14] iio: st_accel: Add event subsystem to st_accel driver
2013-09-27 16:32 ` [RFC v2 PATCH 07/14] iio: st_accel: Add event subsystem to st_accel driver Lukasz Czerwinski
@ 2013-10-01 16:17 ` Jonathan Cameron
0 siblings, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2013-10-01 16:17 UTC (permalink / raw)
To: Lukasz Czerwinski, jic23; +Cc: denis.ciocca, linux-iio, lee.jones, devicetree
On 09/27/13 17:32, Lukasz Czerwinski wrote:
> This patch adds event support for iio st_accel driver.
>
> Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
One bug introduced...
There is no need to have a dataready interrupt to make use of triggers
(i.e. to have a buffer). It is obviously needed to provide a trigger, but
there are plenty of others available (sysfs, soon to be high resolution timer,
general interrupt - usually gpio, and lots of driver provided ones).
Here the buffer is created whatever, but it is only freed if there is a
dataready interrupt. Clearly that doesn't make sense.
I'm guessing this might be a merge issue rather than being deliberate!
...
> @@ -511,10 +595,10 @@ void st_accel_common_remove(struct iio_dev *indio_dev)
> struct st_sensor_data *adata = iio_priv(indio_dev);
>
> iio_device_unregister(indio_dev);
> - if (adata->get_irq_data_ready(indio_dev) > 0)
> + if (adata->get_irq_data_ready(indio_dev) > 0) {
> st_sensors_deallocate_trigger(indio_dev);
> -
> - st_accel_deallocate_ring(indio_dev);
> + st_accel_deallocate_ring(indio_dev);
deallocating the ring is not dependent on the data_ready irq.
> + }
> }
> EXPORT_SYMBOL(st_accel_common_remove);
>
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC v2 PATCH 08/14] iio: iio_magn: Add event ops
2013-09-27 16:32 [RFC v2 PATCH 00/14] iio: STMicroelectronics DT and event support Lukasz Czerwinski
` (6 preceding siblings ...)
2013-09-27 16:32 ` [RFC v2 PATCH 07/14] iio: st_accel: Add event subsystem to st_accel driver Lukasz Czerwinski
@ 2013-09-27 16:32 ` Lukasz Czerwinski
2013-10-01 16:21 ` Jonathan Cameron
2013-09-27 16:32 ` [RFC v2 PATCH 09/14] iio: st_gyro: " Lukasz Czerwinski
` (5 subsequent siblings)
13 siblings, 1 reply; 30+ messages in thread
From: Lukasz Czerwinski @ 2013-09-27 16:32 UTC (permalink / raw)
To: jic23; +Cc: denis.ciocca, linux-iio, lee.jones, devicetree
Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/iio/magnetometer/st_magn_core.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c
index 4460075..1e212ad 100644
--- a/drivers/iio/magnetometer/st_magn_core.c
+++ b/drivers/iio/magnetometer/st_magn_core.c
@@ -343,6 +343,10 @@ static const struct iio_info magn_info = {
.attrs = &st_magn_attribute_group,
.read_raw = &st_magn_read_raw,
.write_raw = &st_magn_write_raw,
+ .read_event_value = &st_sensors_read_event_value,
+ .write_event_value = &st_sensors_write_event_value,
+ .read_event_config = &st_sensors_read_event_config,
+ .write_event_config = &st_sensors_write_event_config,
};
int st_magn_common_probe(struct iio_dev *indio_dev)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [RFC v2 PATCH 08/14] iio: iio_magn: Add event ops
2013-09-27 16:32 ` [RFC v2 PATCH 08/14] iio: iio_magn: Add event ops Lukasz Czerwinski
@ 2013-10-01 16:21 ` Jonathan Cameron
0 siblings, 0 replies; 30+ messages in thread
From: Jonathan Cameron @ 2013-10-01 16:21 UTC (permalink / raw)
To: Lukasz Czerwinski, jic23; +Cc: denis.ciocca, linux-iio, lee.jones, devicetree
On 09/27/13 17:32, Lukasz Czerwinski wrote:
> Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Why? The won't be used by the drivers as none have had the support
added to any of the parts yet.
Hopefully Denis, or someone else with access to a lot of parts
will do some of them (or at least test them for someone else).
> ---
> drivers/iio/magnetometer/st_magn_core.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c
> index 4460075..1e212ad 100644
> --- a/drivers/iio/magnetometer/st_magn_core.c
> +++ b/drivers/iio/magnetometer/st_magn_core.c
> @@ -343,6 +343,10 @@ static const struct iio_info magn_info = {
> .attrs = &st_magn_attribute_group,
> .read_raw = &st_magn_read_raw,
> .write_raw = &st_magn_write_raw,
> + .read_event_value = &st_sensors_read_event_value,
> + .write_event_value = &st_sensors_write_event_value,
> + .read_event_config = &st_sensors_read_event_config,
> + .write_event_config = &st_sensors_write_event_config,
> };
>
> int st_magn_common_probe(struct iio_dev *indio_dev)
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC v2 PATCH 09/14] iio: st_gyro: Add event ops
2013-09-27 16:32 [RFC v2 PATCH 00/14] iio: STMicroelectronics DT and event support Lukasz Czerwinski
` (7 preceding siblings ...)
2013-09-27 16:32 ` [RFC v2 PATCH 08/14] iio: iio_magn: Add event ops Lukasz Czerwinski
@ 2013-09-27 16:32 ` Lukasz Czerwinski
2013-09-27 16:32 ` [RFC v2 PATCH 10/14] iio: iio_press: " Lukasz Czerwinski
` (4 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: Lukasz Czerwinski @ 2013-09-27 16:32 UTC (permalink / raw)
To: jic23; +Cc: denis.ciocca, linux-iio, lee.jones, devicetree
Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/iio/gyro/st_gyro_core.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/iio/gyro/st_gyro_core.c b/drivers/iio/gyro/st_gyro_core.c
index 42f45af..6a41836 100644
--- a/drivers/iio/gyro/st_gyro_core.c
+++ b/drivers/iio/gyro/st_gyro_core.c
@@ -290,6 +290,10 @@ static const struct iio_info gyro_info = {
.attrs = &st_gyro_attribute_group,
.read_raw = &st_gyro_read_raw,
.write_raw = &st_gyro_write_raw,
+ .read_event_value = &st_sensors_read_event_value,
+ .write_event_value = &st_sensors_write_event_value,
+ .read_event_config = &st_sensors_read_event_config,
+ .write_event_config = &st_sensors_write_event_config,
};
#ifdef CONFIG_IIO_TRIGGER
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC v2 PATCH 10/14] iio: iio_press: Add event ops
2013-09-27 16:32 [RFC v2 PATCH 00/14] iio: STMicroelectronics DT and event support Lukasz Czerwinski
` (8 preceding siblings ...)
2013-09-27 16:32 ` [RFC v2 PATCH 09/14] iio: st_gyro: " Lukasz Czerwinski
@ 2013-09-27 16:32 ` Lukasz Czerwinski
2013-09-27 16:32 ` [RFC v2 PATCH 11/14] Documentation: Add st_magn binding documentation Lukasz Czerwinski
` (3 subsequent siblings)
13 siblings, 0 replies; 30+ messages in thread
From: Lukasz Czerwinski @ 2013-09-27 16:32 UTC (permalink / raw)
To: jic23; +Cc: denis.ciocca, linux-iio, lee.jones, devicetree
Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/iio/pressure/st_pressure_core.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/iio/pressure/st_pressure_core.c b/drivers/iio/pressure/st_pressure_core.c
index f78bd33..4d90ddf 100644
--- a/drivers/iio/pressure/st_pressure_core.c
+++ b/drivers/iio/pressure/st_pressure_core.c
@@ -302,6 +302,10 @@ static const struct iio_info press_info = {
.driver_module = THIS_MODULE,
.attrs = &st_press_attribute_group,
.read_raw = &st_press_read_raw,
+ .read_event_value = &st_sensors_read_event_value,
+ .write_event_value = &st_sensors_write_event_value,
+ .read_event_config = &st_sensors_read_event_config,
+ .write_event_config = &st_sensors_write_event_config,
};
#ifdef CONFIG_IIO_TRIGGER
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* [RFC v2 PATCH 11/14] Documentation: Add st_magn binding documentation
2013-09-27 16:32 [RFC v2 PATCH 00/14] iio: STMicroelectronics DT and event support Lukasz Czerwinski
` (9 preceding siblings ...)
2013-09-27 16:32 ` [RFC v2 PATCH 10/14] iio: iio_press: " Lukasz Czerwinski
@ 2013-09-27 16:32 ` Lukasz Czerwinski
2013-10-01 16:29 ` Jonathan Cameron
2013-10-21 11:16 ` Mark Rutland
2013-09-27 16:32 ` [RFC v2 PATCH 12/14] Documentation: Add st_gyro " Lukasz Czerwinski
` (2 subsequent siblings)
13 siblings, 2 replies; 30+ messages in thread
From: Lukasz Czerwinski @ 2013-09-27 16:32 UTC (permalink / raw)
To: jic23; +Cc: denis.ciocca, linux-iio, lee.jones, devicetree
This patch adds the document for STMicroeletronics Magnetic Sensors driver under
Documentation/devicetree/bindings/iio/.
Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
.../bindings/iio/magnetometer/st_magnometer.txt | 33 ++++++++++++++++++++
1 file changed, 33 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/st_magnometer.txt
diff --git a/Documentation/devicetree/bindings/iio/magnetometer/st_magnometer.txt b/Documentation/devicetree/bindings/iio/magnetometer/st_magnometer.txt
new file mode 100644
index 0000000..fb4f473
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/magnetometer/st_magnometer.txt
@@ -0,0 +1,33 @@
+STMicroelectronics Magnetic Sensors
+
+Required properties:
+
+ - compatible : value should be one of the following:
+ (a) "st,lsm303dlhc" for magnetometer in LSM330DLHC
+ (b) "st,lsm303dlm" for magnetometer in LIS3DH
+ (c) "st,lis3mdl" for magnetometer in LSM330
+
+ - reg : the I2C address of the magnetometer
+
+Optional properties:
+
+ - st,drdy-int-pin : redirect DRDY on pin INT1 (1) or pin INT2 (2) (u8)
+
+ - interrupts : Interrupt numbers for the ST accelerometers, as an array
+ in case the magnetometer have more interrupt lines:
+ <DataReady irq>,
+ <Event irq>;
+
+ - interrupt-names : Array of strings associated with the interrupt numbers
+
+Example:
+
+lis3mdl@1c {
+ compatible = "st,lis3mdl";
+ reg = <0x1C>;
+
+ st,drdy-int-pin = /bits/ 8 <1>;
+ interrupt-parent = <&gpf0>;
+ interrupts = <5 0>, <6 0>;
+ interrupt-names = "drdy-int", "event-int";
+};
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [RFC v2 PATCH 11/14] Documentation: Add st_magn binding documentation
2013-09-27 16:32 ` [RFC v2 PATCH 11/14] Documentation: Add st_magn binding documentation Lukasz Czerwinski
@ 2013-10-01 16:29 ` Jonathan Cameron
2013-10-01 15:49 ` Rob Herring
2013-10-21 11:16 ` Mark Rutland
1 sibling, 1 reply; 30+ messages in thread
From: Jonathan Cameron @ 2013-10-01 16:29 UTC (permalink / raw)
To: Lukasz Czerwinski, jic23
Cc: denis.ciocca, linux-iio, Lee Jones, devicetree, Mark Rutland,
Rob Herring, Pawel Moll, Stephen Warren, Ian Campbell
On 09/27/13 17:32, Lukasz Czerwinski wrote:
> This patch adds the document for STMicroeletronics Magnetic Sensors driver under
> Documentation/devicetree/bindings/iio/.
>
> Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
So right now we have two different approaches to documenting these bindings. Lee
proposed doing it on a per device name, here you are proposing a single file
per type.
What is the prefered approach?
Lee/Lukasz why would you two favour one way or the other?
Device tree maintainers, which is preferred?
Do we assume that any sane person looking up bindings will deploy
grep to find where they are documented, or does it want to more
or less be named after the part (in which case we'll get a lot
of repetition).
Also, the question of whether _'s are acceptable in names
to avoid breaking existing ABI for i2c devices is still open?
I'm assuming that given the current situation and the fact
that the word 'prefered' was used, we are better sticking
with _'s? (that doesn't actually effect this patch, but does
on the next two in this series and I'm being lazy).
> .../bindings/iio/magnetometer/st_magnometer.txt | 33 ++++++++++++++++++++
> 1 file changed, 33 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/st_magnometer.txt
>
> diff --git a/Documentation/devicetree/bindings/iio/magnetometer/st_magnometer.txt b/Documentation/devicetree/bindings/iio/magnetometer/st_magnometer.txt
> new file mode 100644
> index 0000000..fb4f473
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/magnetometer/st_magnometer.txt
> @@ -0,0 +1,33 @@
> +STMicroelectronics Magnetic Sensors
> +
> +Required properties:
> +
> + - compatible : value should be one of the following:
> + (a) "st,lsm303dlhc" for magnetometer in LSM330DLHC
> + (b) "st,lsm303dlm" for magnetometer in LIS3DH
> + (c) "st,lis3mdl" for magnetometer in LSM330
> +
> + - reg : the I2C address of the magnetometer
> +
> +Optional properties:
> +
> + - st,drdy-int-pin : redirect DRDY on pin INT1 (1) or pin INT2 (2) (u8)
> +
> + - interrupts : Interrupt numbers for the ST accelerometers, as an array
> + in case the magnetometer have more interrupt lines:
> + <DataReady irq>,
> + <Event irq>;
> +
> + - interrupt-names : Array of strings associated with the interrupt numbers
> +
> +Example:
> +
> +lis3mdl@1c {
> + compatible = "st,lis3mdl";
> + reg = <0x1C>;
> +
> + st,drdy-int-pin = /bits/ 8 <1>;
> + interrupt-parent = <&gpf0>;
> + interrupts = <5 0>, <6 0>;
> + interrupt-names = "drdy-int", "event-int";
> +};
>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC v2 PATCH 11/14] Documentation: Add st_magn binding documentation
2013-10-01 16:29 ` Jonathan Cameron
@ 2013-10-01 15:49 ` Rob Herring
2013-10-22 6:55 ` Lee Jones
0 siblings, 1 reply; 30+ messages in thread
From: Rob Herring @ 2013-10-01 15:49 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lukasz Czerwinski, jic23, denis.ciocca, linux-iio, Lee Jones,
devicetree, Mark Rutland, Pawel Moll, Stephen Warren,
Ian Campbell
On 10/01/2013 11:29 AM, Jonathan Cameron wrote:
> On 09/27/13 17:32, Lukasz Czerwinski wrote:
>> This patch adds the document for STMicroeletronics Magnetic Sensors driver under
>> Documentation/devicetree/bindings/iio/.
>>
>> Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>
> So right now we have two different approaches to documenting these bindings. Lee
> proposed doing it on a per device name, here you are proposing a single file
> per type.
>
> What is the prefered approach?
>
> Lee/Lukasz why would you two favour one way or the other?
>
> Device tree maintainers, which is preferred?
If the only difference is compatible string, then I think one doc is
fine and preferred by me.
Rob
>
> Do we assume that any sane person looking up bindings will deploy
> grep to find where they are documented, or does it want to more
> or less be named after the part (in which case we'll get a lot
> of repetition).
>
> Also, the question of whether _'s are acceptable in names
> to avoid breaking existing ABI for i2c devices is still open?
> I'm assuming that given the current situation and the fact
> that the word 'prefered' was used, we are better sticking
> with _'s? (that doesn't actually effect this patch, but does
> on the next two in this series and I'm being lazy).
>
>
>> .../bindings/iio/magnetometer/st_magnometer.txt | 33 ++++++++++++++++++++
>> 1 file changed, 33 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/st_magnometer.txt
>>
>> diff --git a/Documentation/devicetree/bindings/iio/magnetometer/st_magnometer.txt b/Documentation/devicetree/bindings/iio/magnetometer/st_magnometer.txt
>> new file mode 100644
>> index 0000000..fb4f473
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/magnetometer/st_magnometer.txt
>> @@ -0,0 +1,33 @@
>> +STMicroelectronics Magnetic Sensors
>> +
>> +Required properties:
>> +
>> + - compatible : value should be one of the following:
>> + (a) "st,lsm303dlhc" for magnetometer in LSM330DLHC
>> + (b) "st,lsm303dlm" for magnetometer in LIS3DH
>> + (c) "st,lis3mdl" for magnetometer in LSM330
>> +
>> + - reg : the I2C address of the magnetometer
>> +
>> +Optional properties:
>> +
>> + - st,drdy-int-pin : redirect DRDY on pin INT1 (1) or pin INT2 (2) (u8)
>> +
>> + - interrupts : Interrupt numbers for the ST accelerometers, as an array
>> + in case the magnetometer have more interrupt lines:
>> + <DataReady irq>,
>> + <Event irq>;
>> +
>> + - interrupt-names : Array of strings associated with the interrupt numbers
>> +
>> +Example:
>> +
>> +lis3mdl@1c {
>> + compatible = "st,lis3mdl";
>> + reg = <0x1C>;
>> +
>> + st,drdy-int-pin = /bits/ 8 <1>;
>> + interrupt-parent = <&gpf0>;
>> + interrupts = <5 0>, <6 0>;
>> + interrupt-names = "drdy-int", "event-int";
>> +};
>>
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [RFC v2 PATCH 11/14] Documentation: Add st_magn binding documentation
2013-10-01 15:49 ` Rob Herring
@ 2013-10-22 6:55 ` Lee Jones
0 siblings, 0 replies; 30+ messages in thread
From: Lee Jones @ 2013-10-22 6:55 UTC (permalink / raw)
To: Rob Herring
Cc: Jonathan Cameron, Lukasz Czerwinski, jic23, Denis Ciocca,
linux-iio, devicetree@vger.kernel.org, Mark Rutland, Pawel Moll,
Stephen Warren, Ian Campbell
On 1 October 2013 16:49, Rob Herring <robherring2@gmail.com> wrote:
> On 10/01/2013 11:29 AM, Jonathan Cameron wrote:
>> On 09/27/13 17:32, Lukasz Czerwinski wrote:
>>> This patch adds the document for STMicroeletronics Magnetic Sensors driver under
>>> Documentation/devicetree/bindings/iio/.
>>>
>>> Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
>>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>>> ---
>>
>> So right now we have two different approaches to documenting these bindings. Lee
>> proposed doing it on a per device name, here you are proposing a single file
>> per type.
>>
>> What is the prefered approach?
>>
>> Lee/Lukasz why would you two favour one way or the other?
>>
>> Device tree maintainers, which is preferred?
>
> If the only difference is compatible string, then I think one doc is
> fine and preferred by me.
I have no strong feelings either way.
--
Lee Jones
Linaro ST Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [RFC v2 PATCH 11/14] Documentation: Add st_magn binding documentation
2013-09-27 16:32 ` [RFC v2 PATCH 11/14] Documentation: Add st_magn binding documentation Lukasz Czerwinski
2013-10-01 16:29 ` Jonathan Cameron
@ 2013-10-21 11:16 ` Mark Rutland
1 sibling, 0 replies; 30+ messages in thread
From: Mark Rutland @ 2013-10-21 11:16 UTC (permalink / raw)
To: Lukasz Czerwinski
Cc: jic23@cam.ac.uk, denis.ciocca@st.com, linux-iio@vger.kernel.org,
lee.jones@linaro.org, devicetree@vger.kernel.org
On Fri, Sep 27, 2013 at 05:32:15PM +0100, Lukasz Czerwinski wrote:
> This patch adds the document for STMicroeletronics Magnetic Sensors driver under
> Documentation/devicetree/bindings/iio/.
>
> Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> .../bindings/iio/magnetometer/st_magnometer.txt | 33 ++++++++++++++++++++
> 1 file changed, 33 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/st_magnometer.txt
>
> diff --git a/Documentation/devicetree/bindings/iio/magnetometer/st_magnometer.txt b/Documentation/devicetree/bindings/iio/magnetometer/st_magnometer.txt
> new file mode 100644
> index 0000000..fb4f473
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/magnetometer/st_magnometer.txt
> @@ -0,0 +1,33 @@
> +STMicroelectronics Magnetic Sensors
> +
> +Required properties:
> +
> + - compatible : value should be one of the following:
s/should be/should contain/ -- we might have a future variant that's compatible.
> + (a) "st,lsm303dlhc" for magnetometer in LSM330DLHC
> + (b) "st,lsm303dlm" for magnetometer in LIS3DH
> + (c) "st,lis3mdl" for magnetometer in LSM330
I'd drop the (a), (b), (c) here, it'll just make it more painful to add future
variants. It's probably better to use "*" instead.
> +
> + - reg : the I2C address of the magnetometer
> +
> +Optional properties:
> +
> + - st,drdy-int-pin : redirect DRDY on pin INT1 (1) or pin INT2 (2) (u8)
Could you elaborate on this?
What does this property mena?
What values are valid?
> +
> + - interrupts : Interrupt numbers for the ST accelerometers, as an array
> + in case the magnetometer have more interrupt lines:
> + <DataReady irq>,
> + <Event irq>;
> +
> + - interrupt-names : Array of strings associated with the interrupt numbers
Nit: Interrupts are described by interrupt-specifiers.
Please describe the names explicitly, otherwise there's no point having them at
all, as no-one knows what they are...
How about something like:
- interrupts: a list of interrupt-specifiers, one for each entry in interrupt-names
- interrupt names: a list of strings. Should contain
* "drdy" for the ???? interrupt
* "event" for the ???? interrupt
> +
> +Example:
> +
> +lis3mdl@1c {
> + compatible = "st,lis3mdl";
> + reg = <0x1C>;
> +
> + st,drdy-int-pin = /bits/ 8 <1>;
The use of /bits/ is remarkably uncommon, why do you need it here?
Thanks,
Mark.
> + interrupt-parent = <&gpf0>;
> + interrupts = <5 0>, <6 0>;
> + interrupt-names = "drdy-int", "event-int";
> +};
> --
> 1.7.9.5
>
> --
> 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] 30+ messages in thread
* [RFC v2 PATCH 12/14] Documentation: Add st_gyro binding documentation
2013-09-27 16:32 [RFC v2 PATCH 00/14] iio: STMicroelectronics DT and event support Lukasz Czerwinski
` (10 preceding siblings ...)
2013-09-27 16:32 ` [RFC v2 PATCH 11/14] Documentation: Add st_magn binding documentation Lukasz Czerwinski
@ 2013-09-27 16:32 ` Lukasz Czerwinski
2013-10-21 11:19 ` Mark Rutland
2013-09-27 16:32 ` [RFC v2 PATCH 13/14] Documentation: Add st_pressure " Lukasz Czerwinski
2013-09-27 16:32 ` [RFC v2 PATCH 14/14] Documentation: Add st_accel " Lukasz Czerwinski
13 siblings, 1 reply; 30+ messages in thread
From: Lukasz Czerwinski @ 2013-09-27 16:32 UTC (permalink / raw)
To: jic23; +Cc: denis.ciocca, linux-iio, lee.jones, devicetree
This patch adds the document for STMicroeletronics Angular Velocity Sensors
driver under Documentation/devicetree/bindings/iio/.
Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
.../bindings/iio/gyroscope/st_gyroscope.txt | 38 ++++++++++++++++++++
1 file changed, 38 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/gyroscope/st_gyroscope.txt
diff --git a/Documentation/devicetree/bindings/iio/gyroscope/st_gyroscope.txt b/Documentation/devicetree/bindings/iio/gyroscope/st_gyroscope.txt
new file mode 100644
index 0000000..8a6a9be
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/gyroscope/st_gyroscope.txt
@@ -0,0 +1,38 @@
+STMicroelectronics Angular Velocity Sensors
+
+Required properties:
+
+ - compatible : value should be one of the following:
+ (a) "st,l3g4200d" for gyroscope in L3G4200D
+ (b) "st,lsm330d-gyro" for gyroscope in LSM330D
+ (c) "st,lsm330dl-gyro" for gyroscope in LSM330DL
+ (d) "st,lsm330dlc-gyro" for gyroscope in LSM330DLC
+ (e) "st,l3gd20" for gyroscope in L3GD20
+ (f) "st,l3gd20h" for gyroscope in L3GD20H
+ (g) "st,l3g4is-ui" for gyroscope in L3G4IS
+ (h) "st,lsm330-gyro" for gyroscope in LSM330
+
+ - reg : the I2C address of the gyroscope
+
+Optional properties:
+
+ - st,drdy-int-pin : redirect DRDY on pin INT1 (1) or pin INT2 (2) (u8)
+
+ - interrupts : Interrupt numbers for the ST gyroscopes, as an array
+ in case the gyroscope have more interrupt lines:
+ <DataReady irq>,
+ <Event irq>;
+
+ - interrupt-names : Array of strings associated with the interrupt numbers
+
+Example:
+
+lsm330dlc_gyro@6b {
+ compatible = "st,lsm330dlc-gyro";
+ reg = <0x6b>;
+
+ st,drdy-int-pin = /bits/ 8 <2>;
+ interrupt-parent = <&gpf0>;
+ interrupts = <3 0>, <4 0>;
+ interrupt-names = "drdy-int", "event-int";
+};
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [RFC v2 PATCH 12/14] Documentation: Add st_gyro binding documentation
2013-09-27 16:32 ` [RFC v2 PATCH 12/14] Documentation: Add st_gyro " Lukasz Czerwinski
@ 2013-10-21 11:19 ` Mark Rutland
0 siblings, 0 replies; 30+ messages in thread
From: Mark Rutland @ 2013-10-21 11:19 UTC (permalink / raw)
To: Lukasz Czerwinski
Cc: jic23@cam.ac.uk, denis.ciocca@st.com, linux-iio@vger.kernel.org,
lee.jones@linaro.org, devicetree@vger.kernel.org
On Fri, Sep 27, 2013 at 05:32:16PM +0100, Lukasz Czerwinski wrote:
> This patch adds the document for STMicroeletronics Angular Velocity Sensors
> driver under Documentation/devicetree/bindings/iio/.
>
> Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> .../bindings/iio/gyroscope/st_gyroscope.txt | 38 ++++++++++++++++++++
> 1 file changed, 38 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/gyroscope/st_gyroscope.txt
>
> diff --git a/Documentation/devicetree/bindings/iio/gyroscope/st_gyroscope.txt b/Documentation/devicetree/bindings/iio/gyroscope/st_gyroscope.txt
> new file mode 100644
> index 0000000..8a6a9be
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/gyroscope/st_gyroscope.txt
> @@ -0,0 +1,38 @@
> +STMicroelectronics Angular Velocity Sensors
> +
> +Required properties:
> +
> + - compatible : value should be one of the following:
> + (a) "st,l3g4200d" for gyroscope in L3G4200D
> + (b) "st,lsm330d-gyro" for gyroscope in LSM330D
> + (c) "st,lsm330dl-gyro" for gyroscope in LSM330DL
> + (d) "st,lsm330dlc-gyro" for gyroscope in LSM330DLC
> + (e) "st,l3gd20" for gyroscope in L3GD20
> + (f) "st,l3gd20h" for gyroscope in L3GD20H
> + (g) "st,l3g4is-ui" for gyroscope in L3G4IS
> + (h) "st,lsm330-gyro" for gyroscope in LSM330
> +
> + - reg : the I2C address of the gyroscope
> +
> +Optional properties:
> +
> + - st,drdy-int-pin : redirect DRDY on pin INT1 (1) or pin INT2 (2) (u8)
> +
> + - interrupts : Interrupt numbers for the ST gyroscopes, as an array
> + in case the gyroscope have more interrupt lines:
> + <DataReady irq>,
> + <Event irq>;
> +
> + - interrupt-names : Array of strings associated with the interrupt numbers
All my comments from patch 11 apply here too.
Mark.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC v2 PATCH 13/14] Documentation: Add st_pressure binding documentation
2013-09-27 16:32 [RFC v2 PATCH 00/14] iio: STMicroelectronics DT and event support Lukasz Czerwinski
` (11 preceding siblings ...)
2013-09-27 16:32 ` [RFC v2 PATCH 12/14] Documentation: Add st_gyro " Lukasz Czerwinski
@ 2013-09-27 16:32 ` Lukasz Czerwinski
2013-10-21 11:23 ` Mark Rutland
2013-09-27 16:32 ` [RFC v2 PATCH 14/14] Documentation: Add st_accel " Lukasz Czerwinski
13 siblings, 1 reply; 30+ messages in thread
From: Lukasz Czerwinski @ 2013-09-27 16:32 UTC (permalink / raw)
To: jic23; +Cc: denis.ciocca, linux-iio, lee.jones, devicetree
This patch adds the document for STMicroeletronics Pressure Sensors
driver under Documentation/devicetree/bindings/iio/.
Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
.../bindings/iio/barometer/st_barometer.txt | 38 ++++++++++++++++++++
1 file changed, 38 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/barometer/st_barometer.txt
diff --git a/Documentation/devicetree/bindings/iio/barometer/st_barometer.txt b/Documentation/devicetree/bindings/iio/barometer/st_barometer.txt
new file mode 100644
index 0000000..5ce4bfa
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/barometer/st_barometer.txt
@@ -0,0 +1,38 @@
+STMicroelectronics Pressure Sensors
+
+Required properties:
+
+ - compatible : value should be:
+ (a) "st,lps331ap" for barometer in LPS331AP
+ (b) "st,lps001wp" for barometer in LPS001WP
+
+ - reg : the I2C address of the barometer
+
+Optional properties:
+
+ - st,drdy-int-pin : redirect DRDY on pin INT1 (1) or pin INT2 (2) (u8)
+
+ - interrupts : Interrupt numbers for the ST barometer, as an array
+ in case the barometer have more interrupt lines:
+ <DataReady irq>,
+ <Event irq>;
+
+ - interrupt-names : Array of strings associated with the interrupt numbers
+
+ - vdd-supply: Phandle to the Vdd supply regulator
+
+ - vddio-supply: Phandle to the Vdd-IO supply regulator
+
+Example:
+
+lps331ap5d@5d {
+ compatible = "st,lps331ap";
+ reg = <0x5d>;
+
+ st,drdy-int-pin = /bits/ 8 <1>;
+ interrupt-parent = <&gpf0>;
+ interrupts = <5 0>, <6 0>;
+ interrupt-names = "drdy-int", "event-int";
+ vdd-supply = <&ab8500_ldo_aux1_reg>;
+ vddio-supply = <&db8500_vsmps2_reg>;
+};
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [RFC v2 PATCH 13/14] Documentation: Add st_pressure binding documentation
2013-09-27 16:32 ` [RFC v2 PATCH 13/14] Documentation: Add st_pressure " Lukasz Czerwinski
@ 2013-10-21 11:23 ` Mark Rutland
0 siblings, 0 replies; 30+ messages in thread
From: Mark Rutland @ 2013-10-21 11:23 UTC (permalink / raw)
To: Lukasz Czerwinski
Cc: jic23@cam.ac.uk, denis.ciocca@st.com, linux-iio@vger.kernel.org,
lee.jones@linaro.org, devicetree@vger.kernel.org
On Fri, Sep 27, 2013 at 05:32:17PM +0100, Lukasz Czerwinski wrote:
> This patch adds the document for STMicroeletronics Pressure Sensors
> driver under Documentation/devicetree/bindings/iio/.
>
> Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> .../bindings/iio/barometer/st_barometer.txt | 38 ++++++++++++++++++++
> 1 file changed, 38 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/barometer/st_barometer.txt
>
> diff --git a/Documentation/devicetree/bindings/iio/barometer/st_barometer.txt b/Documentation/devicetree/bindings/iio/barometer/st_barometer.txt
> new file mode 100644
> index 0000000..5ce4bfa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/barometer/st_barometer.txt
> @@ -0,0 +1,38 @@
> +STMicroelectronics Pressure Sensors
> +
> +Required properties:
> +
> + - compatible : value should be:
> + (a) "st,lps331ap" for barometer in LPS331AP
When you say "for the barometer in LPS331AP", is the LPS331AP a multi function
device? Or only a barometer?
> + (b) "st,lps001wp" for barometer in LPS001WP
> +
> + - reg : the I2C address of the barometer
> +
> +Optional properties:
> +
> + - st,drdy-int-pin : redirect DRDY on pin INT1 (1) or pin INT2 (2) (u8)
> +
> + - interrupts : Interrupt numbers for the ST barometer, as an array
> + in case the barometer have more interrupt lines:
> + <DataReady irq>,
> + <Event irq>;
> +
> + - interrupt-names : Array of strings associated with the interrupt numbers
The comments from patch 11 apply here too.
> +
> + - vdd-supply: Phandle to the Vdd supply regulator
> +
> + - vddio-supply: Phandle to the Vdd-IO supply regulator
These seem fine.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [RFC v2 PATCH 14/14] Documentation: Add st_accel binding documentation
2013-09-27 16:32 [RFC v2 PATCH 00/14] iio: STMicroelectronics DT and event support Lukasz Czerwinski
` (12 preceding siblings ...)
2013-09-27 16:32 ` [RFC v2 PATCH 13/14] Documentation: Add st_pressure " Lukasz Czerwinski
@ 2013-09-27 16:32 ` Lukasz Czerwinski
13 siblings, 0 replies; 30+ messages in thread
From: Lukasz Czerwinski @ 2013-09-27 16:32 UTC (permalink / raw)
To: jic23; +Cc: denis.ciocca, linux-iio, lee.jones, devicetree
This patch adds the document for STMicroeletronics Acceleration Sensors
driver under Documentation/devicetree/bindings/iio/.
Signed-off-by: Lukasz Czerwinski <l.czerwinski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
.../iio/accelerometer/st_accelerometer.txt | 40 ++++++++++++++++++++
1 file changed, 40 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/accelerometer/st_accelerometer.txt
diff --git a/Documentation/devicetree/bindings/iio/accelerometer/st_accelerometer.txt b/Documentation/devicetree/bindings/iio/accelerometer/st_accelerometer.txt
new file mode 100644
index 0000000..af70f29
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/accelerometer/st_accelerometer.txt
@@ -0,0 +1,40 @@
+STMicroelectronics Acceleration Sensors
+
+Required properties:
+
+ - compatible : value should be one of the following:
+ (a) "st,lsm330dlhc-accel" for accelerometer in LSM330DLHC
+ (b) "st,lis3dh" for accelerometer in LIS3DH
+ (c) "st,lsm330d-accel" for accelerometer in LSM330
+ (d) "st,lsm330dl-accel" for accelerometer in LSM330DL
+ (e) "st,lsm330dlc-accel" for accelerometer in LSM330DLC
+ (f) "st,lsm331dlh" for accelerometer in LSM331DLH
+ (g) "st,lsm303dl-accel" for accelerometer in LSM303DL
+ (h) "st,lsm303dlh-accel" for accelerometer in LSM303DLH
+ (i) "st,lsm303dlm-accel" for accelerometer in LSM303DLM
+ (j) "st,lsm330-accel" for accelerometer in LMS330
+
+ - reg : the I2C address of the accelerometer
+
+Optional properties:
+
+ - st,drdy-int-pin : redirect DRDY on pin INT1 (1) or pin INT2 (2) (u8)
+
+ - interrupts : Interrupt numbers for the ST accelerometers, as an array
+ in case the accelerometer have more interrupt lines:
+ <DataReady irq>,
+ <Event irq>;
+
+ - interrupt-names : Array of strings associated with the interrupt numbers
+
+Example:
+
+lsm330dlc_accel@19 {
+ compatible = "st,lsm330dlc-accel";
+ reg = <0x19>;
+
+ st,drdy-int-pin = /bits/ 8 <1>;
+ interrupt-parent = <&gpx0>;
+ interrupts= <0 0>, <1 0>;
+ interrupt-names = "event-int". "drdy-int";\
+};
--
1.7.9.5
^ permalink raw reply related [flat|nested] 30+ messages in thread