* [PATCH v2 0/2] ms5611: Add triggered buffer support @ 2016-01-26 16:57 Daniel Baluta 2016-01-26 16:57 ` [PATCH v2 1/2] iio: pressure: ms5611: Add IIO_CHAN_INFO_SCALE to mask Daniel Baluta 2016-01-26 16:57 ` [PATCH v2 2/2] iio: pressure: ms5611: Add triggered buffer support Daniel Baluta 0 siblings, 2 replies; 4+ messages in thread From: Daniel Baluta @ 2016-01-26 16:57 UTC (permalink / raw) To: jic23 Cc: knaack.h, lars, ludovic.tancerel, tduszyns, akinobu.mita, daniel.baluta, k.kozlowski, afd, broonie, linux-iio, linux-kernel, lucas.demarchi First patch reverts 7cb46c2a066 ("iio: pressure: ms5611: remove IIO_CHAN_INFO_SCALE from mask") because we need the scale to correctly transform "raw" data from buffer into correct measurement units. Second patch adds the actual buffer support. Changes since v1: * addressed comments from Peter * explain why the need for allocating a buffer of 4 s32 elements * correctly handle data copy into buffer when not all channels are enabled * fixed timestamp channel index (2 instead of 3) * added some new lines for readability Daniel Baluta (2): iio: pressure: ms5611: Add IIO_CHAN_INFO_SCALE to mask iio: pressure: ms5611: Add triggered buffer support drivers/iio/pressure/Kconfig | 1 + drivers/iio/pressure/ms5611.h | 1 + drivers/iio/pressure/ms5611_core.c | 95 ++++++++++++++++++++++++++++++++++++-- drivers/iio/pressure/ms5611_i2c.c | 7 +++ drivers/iio/pressure/ms5611_spi.c | 8 ++++ 5 files changed, 108 insertions(+), 4 deletions(-) -- 2.5.0 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 1/2] iio: pressure: ms5611: Add IIO_CHAN_INFO_SCALE to mask 2016-01-26 16:57 [PATCH v2 0/2] ms5611: Add triggered buffer support Daniel Baluta @ 2016-01-26 16:57 ` Daniel Baluta 2016-01-26 16:57 ` [PATCH v2 2/2] iio: pressure: ms5611: Add triggered buffer support Daniel Baluta 1 sibling, 0 replies; 4+ messages in thread From: Daniel Baluta @ 2016-01-26 16:57 UTC (permalink / raw) To: jic23 Cc: knaack.h, lars, ludovic.tancerel, tduszyns, akinobu.mita, daniel.baluta, k.kozlowski, afd, broonie, linux-iio, linux-kernel, lucas.demarchi This allows data exported via buffer interface to be converted to standard units in userspace. Signed-off-by: Daniel Baluta <daniel.baluta@intel.com> --- drivers/iio/pressure/ms5611_core.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/drivers/iio/pressure/ms5611_core.c b/drivers/iio/pressure/ms5611_core.c index 2f3d9b4..6cf0e55 100644 --- a/drivers/iio/pressure/ms5611_core.c +++ b/drivers/iio/pressure/ms5611_core.c @@ -201,6 +201,18 @@ static int ms5611_read_raw(struct iio_dev *indio_dev, default: return -EINVAL; } + case IIO_CHAN_INFO_SCALE: + switch (chan->type) { + case IIO_TEMP: + *val = 10; + return IIO_VAL_INT; + case IIO_PRESSURE: + *val = 0; + *val2 = 1000; + return IIO_VAL_INT_PLUS_MICRO; + default: + return -EINVAL; + } } return -EINVAL; @@ -218,11 +230,13 @@ static struct ms5611_chip_info chip_info_tbl[] = { static const struct iio_chan_spec ms5611_channels[] = { { .type = IIO_PRESSURE, - .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) | + BIT(IIO_CHAN_INFO_SCALE), }, { .type = IIO_TEMP, - .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) | + BIT(IIO_CHAN_INFO_SCALE), } }; -- 2.5.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2 2/2] iio: pressure: ms5611: Add triggered buffer support 2016-01-26 16:57 [PATCH v2 0/2] ms5611: Add triggered buffer support Daniel Baluta 2016-01-26 16:57 ` [PATCH v2 1/2] iio: pressure: ms5611: Add IIO_CHAN_INFO_SCALE to mask Daniel Baluta @ 2016-01-26 16:57 ` Daniel Baluta 2016-01-30 17:09 ` Jonathan Cameron 1 sibling, 1 reply; 4+ messages in thread From: Daniel Baluta @ 2016-01-26 16:57 UTC (permalink / raw) To: jic23 Cc: knaack.h, lars, ludovic.tancerel, tduszyns, akinobu.mita, daniel.baluta, k.kozlowski, afd, broonie, linux-iio, linux-kernel, lucas.demarchi This will be used together with an external trigger (e.g hrtimer based software trigger). Signed-off-by: Daniel Baluta <daniel.baluta@intel.com> --- drivers/iio/pressure/Kconfig | 1 + drivers/iio/pressure/ms5611.h | 1 + drivers/iio/pressure/ms5611_core.c | 77 +++++++++++++++++++++++++++++++++++++- drivers/iio/pressure/ms5611_i2c.c | 7 ++++ drivers/iio/pressure/ms5611_spi.c | 8 ++++ 5 files changed, 92 insertions(+), 2 deletions(-) diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig index e8f60db..48c3741 100644 --- a/drivers/iio/pressure/Kconfig +++ b/drivers/iio/pressure/Kconfig @@ -69,6 +69,7 @@ config MPL3115 config MS5611 tristate "Measurement Specialties MS5611 pressure sensor driver" + select IIO_TRIGGERED_BUFFER help Say Y here to build support for the Measurement Specialties MS5611, MS5607 pressure and temperature sensors. diff --git a/drivers/iio/pressure/ms5611.h b/drivers/iio/pressure/ms5611.h index 23b93c7..2d70dd6 100644 --- a/drivers/iio/pressure/ms5611.h +++ b/drivers/iio/pressure/ms5611.h @@ -52,5 +52,6 @@ struct ms5611_state { }; int ms5611_probe(struct iio_dev *indio_dev, struct device *dev, int type); +int ms5611_remove(struct iio_dev *indio_dev); #endif /* _MS5611_H */ diff --git a/drivers/iio/pressure/ms5611_core.c b/drivers/iio/pressure/ms5611_core.c index 6cf0e55..7ce58cb 100644 --- a/drivers/iio/pressure/ms5611_core.c +++ b/drivers/iio/pressure/ms5611_core.c @@ -17,6 +17,9 @@ #include <linux/iio/iio.h> #include <linux/delay.h> +#include <linux/iio/buffer.h> +#include <linux/iio/triggered_buffer.h> +#include <linux/iio/trigger_consumer.h> #include "ms5611.h" static bool ms5611_prom_is_valid(u16 *prom, size_t len) @@ -173,6 +176,34 @@ static int ms5611_reset(struct iio_dev *indio_dev) return 0; } +static irqreturn_t ms5611_trigger_handler(int irq, void *p) +{ + struct iio_poll_func *pf = p; + struct iio_dev *indio_dev = pf->indio_dev; + struct ms5611_state *st = iio_priv(indio_dev); + s32 buf[4]; /* s32 (pressure) + s32 (temp) + 2 * s32 (timestamp) */ + s32 pressure, temp; + int ret, i = 0; + + mutex_lock(&st->lock); + ret = ms5611_read_temp_and_pressure(indio_dev, &temp, &pressure); + mutex_unlock(&st->lock); + if (ret < 0) + goto err; + + if (test_bit(0, indio_dev->active_scan_mask)) + buf[i++] = pressure; + if (test_bit(1, indio_dev->active_scan_mask)) + buf[i] = temp; + + iio_push_to_buffers_with_timestamp(indio_dev, buf, pf->timestamp); + +err: + iio_trigger_notify_done(indio_dev->trig); + + return IRQ_HANDLED; +} + static int ms5611_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, int *val, int *val2, long mask) @@ -232,12 +263,27 @@ static const struct iio_chan_spec ms5611_channels[] = { .type = IIO_PRESSURE, .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) | BIT(IIO_CHAN_INFO_SCALE), + .scan_index = 0, + .scan_type = { + .sign = 's', + .realbits = 32, + .storagebits = 32, + .endianness = IIO_CPU, + }, }, { .type = IIO_TEMP, .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) | BIT(IIO_CHAN_INFO_SCALE), - } + .scan_index = 1, + .scan_type = { + .sign = 's', + .realbits = 32, + .storagebits = 32, + .endianness = IIO_CPU, + }, + }, + IIO_CHAN_SOFT_TIMESTAMP(2), }; static const struct iio_info ms5611_info = { @@ -274,10 +320,37 @@ int ms5611_probe(struct iio_dev *indio_dev, struct device *dev, int type) if (ret < 0) return ret; - return devm_iio_device_register(dev, indio_dev); + ret = iio_triggered_buffer_setup(indio_dev, &iio_pollfunc_store_time, + ms5611_trigger_handler, NULL); + if (ret < 0) { + dev_err(dev, "iio triggered buffer setup failed\n"); + return ret; + } + + ret = iio_device_register(indio_dev); + if (ret < 0) { + dev_err(dev, "unable to register iio device\n"); + goto err_buffer_cleanup; + } + + return 0; + +err_buffer_cleanup: + iio_triggered_buffer_cleanup(indio_dev); + + return ret; } EXPORT_SYMBOL(ms5611_probe); +int ms5611_remove(struct iio_dev *indio_dev) +{ + iio_device_unregister(indio_dev); + iio_triggered_buffer_cleanup(indio_dev); + + return 0; +} +EXPORT_SYMBOL(ms5611_remove); + MODULE_AUTHOR("Tomasz Duszynski <tduszyns@gmail.com>"); MODULE_DESCRIPTION("MS5611 core driver"); MODULE_LICENSE("GPL v2"); diff --git a/drivers/iio/pressure/ms5611_i2c.c b/drivers/iio/pressure/ms5611_i2c.c index 245797d..42706a8 100644 --- a/drivers/iio/pressure/ms5611_i2c.c +++ b/drivers/iio/pressure/ms5611_i2c.c @@ -99,6 +99,7 @@ static int ms5611_i2c_probe(struct i2c_client *client, return -ENOMEM; st = iio_priv(indio_dev); + i2c_set_clientdata(client, indio_dev); st->reset = ms5611_i2c_reset; st->read_prom_word = ms5611_i2c_read_prom_word; st->read_adc_temp_and_pressure = ms5611_i2c_read_adc_temp_and_pressure; @@ -107,6 +108,11 @@ static int ms5611_i2c_probe(struct i2c_client *client, return ms5611_probe(indio_dev, &client->dev, id->driver_data); } +static int ms5611_i2c_remove(struct i2c_client *client) +{ + return ms5611_remove(i2c_get_clientdata(client)); +} + static const struct i2c_device_id ms5611_id[] = { { "ms5611", MS5611 }, { "ms5607", MS5607 }, @@ -120,6 +126,7 @@ static struct i2c_driver ms5611_driver = { }, .id_table = ms5611_id, .probe = ms5611_i2c_probe, + .remove = ms5611_i2c_remove, }; module_i2c_driver(ms5611_driver); diff --git a/drivers/iio/pressure/ms5611_spi.c b/drivers/iio/pressure/ms5611_spi.c index aaa0c4b..c4bf4e8 100644 --- a/drivers/iio/pressure/ms5611_spi.c +++ b/drivers/iio/pressure/ms5611_spi.c @@ -90,6 +90,8 @@ static int ms5611_spi_probe(struct spi_device *spi) if (!indio_dev) return -ENOMEM; + spi_set_drvdata(spi, indio_dev); + spi->mode = SPI_MODE_0; spi->max_speed_hz = 20000000; spi->bits_per_word = 8; @@ -107,6 +109,11 @@ static int ms5611_spi_probe(struct spi_device *spi) spi_get_device_id(spi)->driver_data); } +static int ms5611_spi_remove(struct spi_device *spi) +{ + return ms5611_remove(spi_get_drvdata(spi)); +} + static const struct spi_device_id ms5611_id[] = { { "ms5611", MS5611 }, { "ms5607", MS5607 }, @@ -120,6 +127,7 @@ static struct spi_driver ms5611_driver = { }, .id_table = ms5611_id, .probe = ms5611_spi_probe, + .remove = ms5611_spi_remove, }; module_spi_driver(ms5611_driver); -- 2.5.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 2/2] iio: pressure: ms5611: Add triggered buffer support 2016-01-26 16:57 ` [PATCH v2 2/2] iio: pressure: ms5611: Add triggered buffer support Daniel Baluta @ 2016-01-30 17:09 ` Jonathan Cameron 0 siblings, 0 replies; 4+ messages in thread From: Jonathan Cameron @ 2016-01-30 17:09 UTC (permalink / raw) To: Daniel Baluta Cc: knaack.h, lars, ludovic.tancerel, tduszyns, akinobu.mita, k.kozlowski, afd, broonie, linux-iio, linux-kernel, lucas.demarchi On 26/01/16 16:57, Daniel Baluta wrote: > This will be used together with an external trigger (e.g hrtimer > based software trigger). If so, don't use iio_pollfunc_store_time (pass NULL instead). That'll tell you when your trigger fired, whereas more useful is when you actually grab the data (a bit later). Also, I'd let the core demux handle the duties of working out which elements have been requested by userspace. Makes for simpler code in here and should anyone ever hook up a second client buffer then that stuff will be running in anger anyway. Otherwise, looks good to me. Jonathan > > Signed-off-by: Daniel Baluta <daniel.baluta@intel.com> > --- > drivers/iio/pressure/Kconfig | 1 + > drivers/iio/pressure/ms5611.h | 1 + > drivers/iio/pressure/ms5611_core.c | 77 +++++++++++++++++++++++++++++++++++++- > drivers/iio/pressure/ms5611_i2c.c | 7 ++++ > drivers/iio/pressure/ms5611_spi.c | 8 ++++ > 5 files changed, 92 insertions(+), 2 deletions(-) > > diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig > index e8f60db..48c3741 100644 > --- a/drivers/iio/pressure/Kconfig > +++ b/drivers/iio/pressure/Kconfig > @@ -69,6 +69,7 @@ config MPL3115 > > config MS5611 > tristate "Measurement Specialties MS5611 pressure sensor driver" > + select IIO_TRIGGERED_BUFFER > help > Say Y here to build support for the Measurement Specialties > MS5611, MS5607 pressure and temperature sensors. > diff --git a/drivers/iio/pressure/ms5611.h b/drivers/iio/pressure/ms5611.h > index 23b93c7..2d70dd6 100644 > --- a/drivers/iio/pressure/ms5611.h > +++ b/drivers/iio/pressure/ms5611.h > @@ -52,5 +52,6 @@ struct ms5611_state { > }; > > int ms5611_probe(struct iio_dev *indio_dev, struct device *dev, int type); > +int ms5611_remove(struct iio_dev *indio_dev); > > #endif /* _MS5611_H */ > diff --git a/drivers/iio/pressure/ms5611_core.c b/drivers/iio/pressure/ms5611_core.c > index 6cf0e55..7ce58cb 100644 > --- a/drivers/iio/pressure/ms5611_core.c > +++ b/drivers/iio/pressure/ms5611_core.c > @@ -17,6 +17,9 @@ > #include <linux/iio/iio.h> > #include <linux/delay.h> > > +#include <linux/iio/buffer.h> > +#include <linux/iio/triggered_buffer.h> > +#include <linux/iio/trigger_consumer.h> > #include "ms5611.h" > > static bool ms5611_prom_is_valid(u16 *prom, size_t len) > @@ -173,6 +176,34 @@ static int ms5611_reset(struct iio_dev *indio_dev) > return 0; > } > > +static irqreturn_t ms5611_trigger_handler(int irq, void *p) > +{ > + struct iio_poll_func *pf = p; > + struct iio_dev *indio_dev = pf->indio_dev; > + struct ms5611_state *st = iio_priv(indio_dev); > + s32 buf[4]; /* s32 (pressure) + s32 (temp) + 2 * s32 (timestamp) */ > + s32 pressure, temp; > + int ret, i = 0; > + > + mutex_lock(&st->lock); > + ret = ms5611_read_temp_and_pressure(indio_dev, &temp, &pressure); > + mutex_unlock(&st->lock); > + if (ret < 0) > + goto err; > + > + if (test_bit(0, indio_dev->active_scan_mask)) > + buf[i++] = pressure; > + if (test_bit(1, indio_dev->active_scan_mask)) > + buf[i] = temp; I'd leave this to the demux in the core - so specify available scan masks = 0x3,0 and push them both at all times - gives you simpler code here as well as you can just call ms5611_read_temp_and_pressure(indio_dev, &buf[0], &buf[1]), check ret and push to the buffer. > + > + iio_push_to_buffers_with_timestamp(indio_dev, buf, pf->timestamp); Normally the pf->timestamp is only sensible if we are dealing with a data ready trigger. In this case, I'd prefer to see the timestamp being as close to the data capture as possible. e.g. Just read it directly in here and drop the top half of the pollfunc. > + > +err: > + iio_trigger_notify_done(indio_dev->trig); > + > + return IRQ_HANDLED; > +} > + > static int ms5611_read_raw(struct iio_dev *indio_dev, > struct iio_chan_spec const *chan, > int *val, int *val2, long mask) > @@ -232,12 +263,27 @@ static const struct iio_chan_spec ms5611_channels[] = { > .type = IIO_PRESSURE, > .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) | > BIT(IIO_CHAN_INFO_SCALE), > + .scan_index = 0, > + .scan_type = { > + .sign = 's', > + .realbits = 32, > + .storagebits = 32, > + .endianness = IIO_CPU, > + }, > }, > { > .type = IIO_TEMP, > .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) | > BIT(IIO_CHAN_INFO_SCALE), > - } > + .scan_index = 1, > + .scan_type = { > + .sign = 's', > + .realbits = 32, > + .storagebits = 32, > + .endianness = IIO_CPU, > + }, > + }, > + IIO_CHAN_SOFT_TIMESTAMP(2), > }; > > static const struct iio_info ms5611_info = { > @@ -274,10 +320,37 @@ int ms5611_probe(struct iio_dev *indio_dev, struct device *dev, int type) > if (ret < 0) > return ret; > > - return devm_iio_device_register(dev, indio_dev); > + ret = iio_triggered_buffer_setup(indio_dev, &iio_pollfunc_store_time, > + ms5611_trigger_handler, NULL); > + if (ret < 0) { > + dev_err(dev, "iio triggered buffer setup failed\n"); > + return ret; > + } > + > + ret = iio_device_register(indio_dev); > + if (ret < 0) { > + dev_err(dev, "unable to register iio device\n"); > + goto err_buffer_cleanup; > + } > + > + return 0; > + > +err_buffer_cleanup: > + iio_triggered_buffer_cleanup(indio_dev); > + > + return ret; > } > EXPORT_SYMBOL(ms5611_probe); > > +int ms5611_remove(struct iio_dev *indio_dev) > +{ > + iio_device_unregister(indio_dev); > + iio_triggered_buffer_cleanup(indio_dev); > + > + return 0; > +} > +EXPORT_SYMBOL(ms5611_remove); > + > MODULE_AUTHOR("Tomasz Duszynski <tduszyns@gmail.com>"); > MODULE_DESCRIPTION("MS5611 core driver"); > MODULE_LICENSE("GPL v2"); > diff --git a/drivers/iio/pressure/ms5611_i2c.c b/drivers/iio/pressure/ms5611_i2c.c > index 245797d..42706a8 100644 > --- a/drivers/iio/pressure/ms5611_i2c.c > +++ b/drivers/iio/pressure/ms5611_i2c.c > @@ -99,6 +99,7 @@ static int ms5611_i2c_probe(struct i2c_client *client, > return -ENOMEM; > > st = iio_priv(indio_dev); > + i2c_set_clientdata(client, indio_dev); > st->reset = ms5611_i2c_reset; > st->read_prom_word = ms5611_i2c_read_prom_word; > st->read_adc_temp_and_pressure = ms5611_i2c_read_adc_temp_and_pressure; > @@ -107,6 +108,11 @@ static int ms5611_i2c_probe(struct i2c_client *client, > return ms5611_probe(indio_dev, &client->dev, id->driver_data); > } > > +static int ms5611_i2c_remove(struct i2c_client *client) > +{ > + return ms5611_remove(i2c_get_clientdata(client)); > +} > + > static const struct i2c_device_id ms5611_id[] = { > { "ms5611", MS5611 }, > { "ms5607", MS5607 }, > @@ -120,6 +126,7 @@ static struct i2c_driver ms5611_driver = { > }, > .id_table = ms5611_id, > .probe = ms5611_i2c_probe, > + .remove = ms5611_i2c_remove, > }; > module_i2c_driver(ms5611_driver); > > diff --git a/drivers/iio/pressure/ms5611_spi.c b/drivers/iio/pressure/ms5611_spi.c > index aaa0c4b..c4bf4e8 100644 > --- a/drivers/iio/pressure/ms5611_spi.c > +++ b/drivers/iio/pressure/ms5611_spi.c > @@ -90,6 +90,8 @@ static int ms5611_spi_probe(struct spi_device *spi) > if (!indio_dev) > return -ENOMEM; > > + spi_set_drvdata(spi, indio_dev); > + > spi->mode = SPI_MODE_0; > spi->max_speed_hz = 20000000; > spi->bits_per_word = 8; > @@ -107,6 +109,11 @@ static int ms5611_spi_probe(struct spi_device *spi) > spi_get_device_id(spi)->driver_data); > } > > +static int ms5611_spi_remove(struct spi_device *spi) > +{ > + return ms5611_remove(spi_get_drvdata(spi)); > +} > + > static const struct spi_device_id ms5611_id[] = { > { "ms5611", MS5611 }, > { "ms5607", MS5607 }, > @@ -120,6 +127,7 @@ static struct spi_driver ms5611_driver = { > }, > .id_table = ms5611_id, > .probe = ms5611_spi_probe, > + .remove = ms5611_spi_remove, > }; > module_spi_driver(ms5611_driver); > > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-01-30 17:09 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-01-26 16:57 [PATCH v2 0/2] ms5611: Add triggered buffer support Daniel Baluta 2016-01-26 16:57 ` [PATCH v2 1/2] iio: pressure: ms5611: Add IIO_CHAN_INFO_SCALE to mask Daniel Baluta 2016-01-26 16:57 ` [PATCH v2 2/2] iio: pressure: ms5611: Add triggered buffer support Daniel Baluta 2016-01-30 17:09 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).