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@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 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@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- 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
