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@xxxxxxxxxxx> > Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx> 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 */ > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html