From 490fba90a90eb7b741f57fefd2bcf2c1e11eb471 Mon Sep 17 00:00:00 2001 From: Michael Nosthoff Date: Fri, 9 Mar 2018 16:13:52 +0100 Subject: iio: ad7793: implement IIO_CHAN_INFO_SAMP_FREQ This commit is a follow-up to changes made to ad_sigma_delta.h in staging: iio: ad7192: implement IIO_CHAN_INFO_SAMP_FREQ which broke ad7793 as it was not altered to match those changes. This driver predates the availability of IIO_CHAN_INFO_SAMP_FREQ attribute wherein usage has some advantages like it can be accessed by in-kernel consumers as well as reduces the code size. Therefore, use IIO_CHAN_INFO_SAMP_FREQ to implement the sampling_frequency attribute instead of using IIO_DEV_ATTR_SAMP_FREQ() macro. Move code from the functions associated with IIO_DEV_ATTR_SAMP_FREQ() into respective read and write hooks with the mask set to IIO_CHAN_INFO_SAMP_FREQ. Fixes: a13e831fcaa7 ("staging: iio: ad7192: implement IIO_CHAN_INFO_SAMP_FREQ") Signed-off-by: Michael Nosthoff Cc: Signed-off-by: Jonathan Cameron --- drivers/iio/adc/ad7793.c | 75 ++++++++++++++++-------------------------------- 1 file changed, 24 insertions(+), 51 deletions(-) (limited to 'drivers') diff --git a/drivers/iio/adc/ad7793.c b/drivers/iio/adc/ad7793.c index 801afb61310b..d4bbe5b53318 100644 --- a/drivers/iio/adc/ad7793.c +++ b/drivers/iio/adc/ad7793.c @@ -348,55 +348,6 @@ static const u16 ad7793_sample_freq_avail[16] = {0, 470, 242, 123, 62, 50, 39, static const u16 ad7797_sample_freq_avail[16] = {0, 0, 0, 123, 62, 50, 0, 33, 0, 17, 16, 12, 10, 8, 6, 4}; -static ssize_t ad7793_read_frequency(struct device *dev, - struct device_attribute *attr, - char *buf) -{ - struct iio_dev *indio_dev = dev_to_iio_dev(dev); - struct ad7793_state *st = iio_priv(indio_dev); - - return sprintf(buf, "%d\n", - st->chip_info->sample_freq_avail[AD7793_MODE_RATE(st->mode)]); -} - -static ssize_t ad7793_write_frequency(struct device *dev, - struct device_attribute *attr, - const char *buf, - size_t len) -{ - struct iio_dev *indio_dev = dev_to_iio_dev(dev); - struct ad7793_state *st = iio_priv(indio_dev); - long lval; - int i, ret; - - ret = kstrtol(buf, 10, &lval); - if (ret) - return ret; - - if (lval == 0) - return -EINVAL; - - for (i = 0; i < 16; i++) - if (lval == st->chip_info->sample_freq_avail[i]) - break; - if (i == 16) - return -EINVAL; - - ret = iio_device_claim_direct_mode(indio_dev); - if (ret) - return ret; - st->mode &= ~AD7793_MODE_RATE(-1); - st->mode |= AD7793_MODE_RATE(i); - ad_sd_write_reg(&st->sd, AD7793_REG_MODE, sizeof(st->mode), st->mode); - iio_device_release_direct_mode(indio_dev); - - return len; -} - -static IIO_DEV_ATTR_SAMP_FREQ(S_IWUSR | S_IRUGO, - ad7793_read_frequency, - ad7793_write_frequency); - static IIO_CONST_ATTR_SAMP_FREQ_AVAIL( "470 242 123 62 50 39 33 19 17 16 12 10 8 6 4"); @@ -424,7 +375,6 @@ static IIO_DEVICE_ATTR_NAMED(in_m_in_scale_available, ad7793_show_scale_available, NULL, 0); static struct attribute *ad7793_attributes[] = { - &iio_dev_attr_sampling_frequency.dev_attr.attr, &iio_const_attr_sampling_frequency_available.dev_attr.attr, &iio_dev_attr_in_m_in_scale_available.dev_attr.attr, NULL @@ -435,7 +385,6 @@ static const struct attribute_group ad7793_attribute_group = { }; static struct attribute *ad7797_attributes[] = { - &iio_dev_attr_sampling_frequency.dev_attr.attr, &iio_const_attr_sampling_frequency_available_ad7797.dev_attr.attr, NULL }; @@ -505,6 +454,10 @@ static int ad7793_read_raw(struct iio_dev *indio_dev, *val -= offset; } return IIO_VAL_INT; + case IIO_CHAN_INFO_SAMP_FREQ: + *val = st->chip_info + ->sample_freq_avail[AD7793_MODE_RATE(st->mode)]; + return IIO_VAL_INT; } return -EINVAL; } @@ -542,6 +495,26 @@ static int ad7793_write_raw(struct iio_dev *indio_dev, break; } break; + case IIO_CHAN_INFO_SAMP_FREQ: + if (!val) { + ret = -EINVAL; + break; + } + + for (i = 0; i < 16; i++) + if (val == st->chip_info->sample_freq_avail[i]) + break; + + if (i == 16) { + ret = -EINVAL; + break; + } + + st->mode &= ~AD7793_MODE_RATE(-1); + st->mode |= AD7793_MODE_RATE(i); + ad_sd_write_reg(&st->sd, AD7793_REG_MODE, sizeof(st->mode), + st->mode); + break; default: ret = -EINVAL; } -- cgit v1.2.3-59-g8ed1b From 7531cf59bfa082769887ec70c2029838ea139f11 Mon Sep 17 00:00:00 2001 From: Fabrice Gasnier Date: Tue, 13 Mar 2018 15:23:05 +0100 Subject: iio: adc: stm32-dfsdm: fix successive oversampling settings When doing successive oversampling settings, it may fail to update filter parameters silently: - First time oversampling is being set, it will be successful, as fl->res is 0 initially. - Next attempts with various oversamp value may return 0 (success), but keep previous filter parameters, due to 'res' never reaches above or equal current 'fl->res'. This is particularly true when setting sampling frequency (that relies on oversamp). Typical failure without error: - run 1st test @16kHz samp freq will succeed - run new test @8kHz will succeed as well - run new test @16kHz (again): sample rate will remain 8kHz without error Fixes: e2e6771c6462 ("IIO: ADC: add STM32 DFSDM sigma delta ADC support") Signed-off-by: Fabrice Gasnier Acked-by: Arnaud Pouliquen Cc: Signed-off-by: Jonathan Cameron --- drivers/iio/adc/stm32-dfsdm-adc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c index 01422d11753c..dc911b6fedd6 100644 --- a/drivers/iio/adc/stm32-dfsdm-adc.c +++ b/drivers/iio/adc/stm32-dfsdm-adc.c @@ -144,6 +144,7 @@ static int stm32_dfsdm_set_osrs(struct stm32_dfsdm_filter *fl, * Leave as soon as if exact resolution if reached. * Otherwise the higher resolution below 32 bits is kept. */ + fl->res = 0; for (fosr = 1; fosr <= DFSDM_MAX_FL_OVERSAMPLING; fosr++) { for (iosr = 1; iosr <= DFSDM_MAX_INT_OVERSAMPLING; iosr++) { if (fast) @@ -193,7 +194,7 @@ static int stm32_dfsdm_set_osrs(struct stm32_dfsdm_filter *fl, } } - if (!fl->fosr) + if (!fl->res) return -EINVAL; return 0; -- cgit v1.2.3-59-g8ed1b From d58109dcf37fc9baec354385ec9fdcd8878d174d Mon Sep 17 00:00:00 2001 From: Fabrice Gasnier Date: Tue, 13 Mar 2018 15:23:06 +0100 Subject: iio: adc: stm32-dfsdm: fix sample rate for div2 spi clock When channel clk source is set to "CLKOUT_F" or "CLKOUT_R" (e.g. div2), sample rate is currently set to half the requested value. Fixes: eca949800d2d ("IIO: ADC: add stm32 DFSDM support for PDM microphone") Signed-off-by: Fabrice Gasnier Acked-by: Arnaud Pouliquen Cc: Signed-off-by: Jonathan Cameron --- drivers/iio/adc/stm32-dfsdm-adc.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c index dc911b6fedd6..b28a716a23b2 100644 --- a/drivers/iio/adc/stm32-dfsdm-adc.c +++ b/drivers/iio/adc/stm32-dfsdm-adc.c @@ -771,7 +771,7 @@ static int stm32_dfsdm_write_raw(struct iio_dev *indio_dev, struct stm32_dfsdm_adc *adc = iio_priv(indio_dev); struct stm32_dfsdm_filter *fl = &adc->dfsdm->fl_list[adc->fl_id]; struct stm32_dfsdm_channel *ch = &adc->dfsdm->ch_list[chan->channel]; - unsigned int spi_freq = adc->spi_freq; + unsigned int spi_freq; int ret = -EINVAL; switch (mask) { @@ -785,8 +785,18 @@ static int stm32_dfsdm_write_raw(struct iio_dev *indio_dev, case IIO_CHAN_INFO_SAMP_FREQ: if (!val) return -EINVAL; - if (ch->src != DFSDM_CHANNEL_SPI_CLOCK_EXTERNAL) + + switch (ch->src) { + case DFSDM_CHANNEL_SPI_CLOCK_INTERNAL: spi_freq = adc->dfsdm->spi_master_freq; + break; + case DFSDM_CHANNEL_SPI_CLOCK_INTERNAL_DIV2_FALLING: + case DFSDM_CHANNEL_SPI_CLOCK_INTERNAL_DIV2_RISING: + spi_freq = adc->dfsdm->spi_master_freq / 2; + break; + default: + spi_freq = adc->spi_freq; + } if (spi_freq % val) dev_warn(&indio_dev->dev, -- cgit v1.2.3-59-g8ed1b From c043ec1ca5baae63726aae32abbe003192bc6eec Mon Sep 17 00:00:00 2001 From: Martin Kelly Date: Mon, 26 Mar 2018 14:27:51 -0700 Subject: iio:buffer: make length types match kfifo types Currently, we use int for buffer length and bytes_per_datum. However, kfifo uses unsigned int for length and size_t for element size. We need to make sure these matches or we will have bugs related to overflow (in the range between INT_MAX and UINT_MAX for length, for example). In addition, set_bytes_per_datum uses size_t while bytes_per_datum is an int, which would cause bugs for large values of bytes_per_datum. Change buffer length to use unsigned int and bytes_per_datum to use size_t. Signed-off-by: Martin Kelly Cc: Signed-off-by: Jonathan Cameron --- drivers/iio/buffer/industrialio-buffer-dma.c | 2 +- drivers/iio/buffer/kfifo_buf.c | 4 ++-- include/linux/iio/buffer_impl.h | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) (limited to 'drivers') diff --git a/drivers/iio/buffer/industrialio-buffer-dma.c b/drivers/iio/buffer/industrialio-buffer-dma.c index 05e0c353e089..b32bf57910ca 100644 --- a/drivers/iio/buffer/industrialio-buffer-dma.c +++ b/drivers/iio/buffer/industrialio-buffer-dma.c @@ -587,7 +587,7 @@ EXPORT_SYMBOL_GPL(iio_dma_buffer_set_bytes_per_datum); * Should be used as the set_length callback for iio_buffer_access_ops * struct for DMA buffers. */ -int iio_dma_buffer_set_length(struct iio_buffer *buffer, int length) +int iio_dma_buffer_set_length(struct iio_buffer *buffer, unsigned int length) { /* Avoid an invalid state */ if (length < 2) diff --git a/drivers/iio/buffer/kfifo_buf.c b/drivers/iio/buffer/kfifo_buf.c index 047fe757ab97..ac622edf2486 100644 --- a/drivers/iio/buffer/kfifo_buf.c +++ b/drivers/iio/buffer/kfifo_buf.c @@ -22,7 +22,7 @@ struct iio_kfifo { #define iio_to_kfifo(r) container_of(r, struct iio_kfifo, buffer) static inline int __iio_allocate_kfifo(struct iio_kfifo *buf, - int bytes_per_datum, int length) + size_t bytes_per_datum, unsigned int length) { if ((length == 0) || (bytes_per_datum == 0)) return -EINVAL; @@ -67,7 +67,7 @@ static int iio_set_bytes_per_datum_kfifo(struct iio_buffer *r, size_t bpd) return 0; } -static int iio_set_length_kfifo(struct iio_buffer *r, int length) +static int iio_set_length_kfifo(struct iio_buffer *r, unsigned int length) { /* Avoid an invalid state */ if (length < 2) diff --git a/include/linux/iio/buffer_impl.h b/include/linux/iio/buffer_impl.h index b9e22b7e2f28..d1171db23742 100644 --- a/include/linux/iio/buffer_impl.h +++ b/include/linux/iio/buffer_impl.h @@ -53,7 +53,7 @@ struct iio_buffer_access_funcs { int (*request_update)(struct iio_buffer *buffer); int (*set_bytes_per_datum)(struct iio_buffer *buffer, size_t bpd); - int (*set_length)(struct iio_buffer *buffer, int length); + int (*set_length)(struct iio_buffer *buffer, unsigned int length); int (*enable)(struct iio_buffer *buffer, struct iio_dev *indio_dev); int (*disable)(struct iio_buffer *buffer, struct iio_dev *indio_dev); @@ -72,10 +72,10 @@ struct iio_buffer_access_funcs { */ struct iio_buffer { /** @length: Number of datums in buffer. */ - int length; + unsigned int length; /** @bytes_per_datum: Size of individual datum including timestamp. */ - int bytes_per_datum; + size_t bytes_per_datum; /** * @access: Buffer access functions associated with the -- cgit v1.2.3-59-g8ed1b From 3d13de4b027d5f6276c0f9d3a264f518747d83f2 Mon Sep 17 00:00:00 2001 From: Martin Kelly Date: Mon, 26 Mar 2018 14:27:52 -0700 Subject: iio:kfifo_buf: check for uint overflow Currently, the following causes a kernel OOPS in memcpy: echo 1073741825 > buffer/length echo 1 > buffer/enable Note that using 1073741824 instead of 1073741825 causes "write error: Cannot allocate memory" but no OOPS. This is because 1073741824 == 2^30 and 1073741825 == 2^30+1. Since kfifo rounds up to the nearest power of 2, it will actually call kmalloc with roundup_pow_of_two(length) * bytes_per_datum. Using length == 1073741825 and bytes_per_datum == 2, we get: kmalloc(roundup_pow_of_two(1073741825) * 2 or kmalloc(2147483648 * 2) or kmalloc(4294967296) or kmalloc(UINT_MAX + 1) so this overflows to 0, causing kmalloc to return ZERO_SIZE_PTR and subsequent memcpy to fail once the device is enabled. Fix this by checking for overflow prior to allocating a kfifo. With this check added, the above code returns -EINVAL when enabling the buffer, rather than causing an OOPS. Signed-off-by: Martin Kelly cc: Signed-off-by: Jonathan Cameron --- drivers/iio/buffer/kfifo_buf.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'drivers') diff --git a/drivers/iio/buffer/kfifo_buf.c b/drivers/iio/buffer/kfifo_buf.c index ac622edf2486..70c302a93d7f 100644 --- a/drivers/iio/buffer/kfifo_buf.c +++ b/drivers/iio/buffer/kfifo_buf.c @@ -27,6 +27,13 @@ static inline int __iio_allocate_kfifo(struct iio_kfifo *buf, if ((length == 0) || (bytes_per_datum == 0)) return -EINVAL; + /* + * Make sure we don't overflow an unsigned int after kfifo rounds up to + * the next power of 2. + */ + if (roundup_pow_of_two(length) > UINT_MAX / bytes_per_datum) + return -EINVAL; + return __kfifo_alloc((struct __kfifo *)&buf->kf, length, bytes_per_datum, GFP_KERNEL); } -- cgit v1.2.3-59-g8ed1b From f0c8d1f6dc8eac5a1fbf441c8e080721a7b6c0ff Mon Sep 17 00:00:00 2001 From: Eugen Hristev Date: Tue, 10 Apr 2018 11:57:47 +0300 Subject: iio: adc: at91-sama5d2_adc: fix channel configuration for differential channels When iterating through the channels, the index in the array is not the scan index. Added an xlate function to translate to the proper index. The result of the bug is that the channel array is indexed with a wrong index, thus instead of the proper channel, we access invalid memory, which may lead to invalid results and/or corruption. This will be used also for devicetree channel xlate. Fixes: 5e1a1da0f ("iio: adc: at91-sama5d2_adc: add hw trigger and buffer support") Fixes: 073c66201 ("iio: adc: at91-sama5d2_adc: add support for DMA") Signed-off-by: Eugen Hristev Cc: Signed-off-by: Jonathan Cameron --- drivers/iio/adc/at91-sama5d2_adc.c | 41 ++++++++++++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c index 4eff8351ce29..8729d6524b4d 100644 --- a/drivers/iio/adc/at91-sama5d2_adc.c +++ b/drivers/iio/adc/at91-sama5d2_adc.c @@ -333,6 +333,27 @@ static const struct iio_chan_spec at91_adc_channels[] = { + AT91_SAMA5D2_DIFF_CHAN_CNT + 1), }; +static int at91_adc_chan_xlate(struct iio_dev *indio_dev, int chan) +{ + int i; + + for (i = 0; i < indio_dev->num_channels; i++) { + if (indio_dev->channels[i].scan_index == chan) + return i; + } + return -EINVAL; +} + +static inline struct iio_chan_spec const * +at91_adc_chan_get(struct iio_dev *indio_dev, int chan) +{ + int index = at91_adc_chan_xlate(indio_dev, chan); + + if (index < 0) + return NULL; + return indio_dev->channels + index; +} + static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state) { struct iio_dev *indio = iio_trigger_get_drvdata(trig); @@ -350,8 +371,10 @@ static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state) at91_adc_writel(st, AT91_SAMA5D2_TRGR, status); for_each_set_bit(bit, indio->active_scan_mask, indio->num_channels) { - struct iio_chan_spec const *chan = indio->channels + bit; + struct iio_chan_spec const *chan = at91_adc_chan_get(indio, bit); + if (!chan) + continue; if (state) { at91_adc_writel(st, AT91_SAMA5D2_CHER, BIT(chan->channel)); @@ -448,7 +471,11 @@ static int at91_adc_dma_start(struct iio_dev *indio_dev) for_each_set_bit(bit, indio_dev->active_scan_mask, indio_dev->num_channels) { - struct iio_chan_spec const *chan = indio_dev->channels + bit; + struct iio_chan_spec const *chan = + at91_adc_chan_get(indio_dev, bit); + + if (!chan) + continue; st->dma_st.rx_buf_sz += chan->scan_type.storagebits / 8; } @@ -526,8 +553,11 @@ static int at91_adc_buffer_predisable(struct iio_dev *indio_dev) */ for_each_set_bit(bit, indio_dev->active_scan_mask, indio_dev->num_channels) { - struct iio_chan_spec const *chan = indio_dev->channels + bit; + struct iio_chan_spec const *chan = + at91_adc_chan_get(indio_dev, bit); + if (!chan) + continue; if (st->dma_st.dma_chan) at91_adc_readl(st, chan->address); } @@ -587,8 +617,11 @@ static void at91_adc_trigger_handler_nodma(struct iio_dev *indio_dev, for_each_set_bit(bit, indio_dev->active_scan_mask, indio_dev->num_channels) { - struct iio_chan_spec const *chan = indio_dev->channels + bit; + struct iio_chan_spec const *chan = + at91_adc_chan_get(indio_dev, bit); + if (!chan) + continue; st->buffer[i] = at91_adc_readl(st, chan->address); i++; } -- cgit v1.2.3-59-g8ed1b From 6f92253024d9d947a4f454654840ce479e251376 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 14 Apr 2018 17:09:09 +0200 Subject: iio: hid-sensor-trigger: Fix sometimes not powering up the sensor after resume hid_sensor_set_power_work() powers the sensors back up after a resume based on the user_requested_state atomic_t. But hid_sensor_power_state() treats this as a boolean flag, leading to the following problematic scenario: 1) Some app starts using the iio-sensor in buffered / triggered mode, hid_sensor_data_rdy_trigger_set_state(true) gets called, setting user_requested_state to 1. 2) Something directly accesses a _raw value through sysfs, leading to a call to hid_sensor_power_state(true) followed by hid_sensor_power_state(false) call, this sets user_requested_state to 1 followed by setting it to 0. 3) Suspend/resume the machine, hid_sensor_set_power_work() now does NOT power the sensor back up because user_requested_state (wrongly) is 0. Which stops the app using the sensor in buffered mode from receiving any new values. This commit changes user_requested_state to a counter tracking how many times hid_sensor_power_state(true) was called instead, fixing this. Cc: Bastien Nocera Cc: Srinivas Pandruvada Signed-off-by: Hans de Goede Acked-by: Srinivas Pandruvada Cc: Signed-off-by: Jonathan Cameron --- drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c index cfb6588565ba..4905a997a7ec 100644 --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c @@ -178,14 +178,14 @@ int hid_sensor_power_state(struct hid_sensor_common *st, bool state) #ifdef CONFIG_PM int ret; - atomic_set(&st->user_requested_state, state); - if (atomic_add_unless(&st->runtime_pm_enable, 1, 1)) pm_runtime_enable(&st->pdev->dev); - if (state) + if (state) { + atomic_inc(&st->user_requested_state); ret = pm_runtime_get_sync(&st->pdev->dev); - else { + } else { + atomic_dec(&st->user_requested_state); pm_runtime_mark_last_busy(&st->pdev->dev); pm_runtime_use_autosuspend(&st->pdev->dev); ret = pm_runtime_put_autosuspend(&st->pdev->dev); -- cgit v1.2.3-59-g8ed1b From 76974ef9d1bf397b7bb97892a3b3bc516a1fc2c2 Mon Sep 17 00:00:00 2001 From: Eugen Hristev Date: Mon, 16 Apr 2018 09:54:03 +0300 Subject: iio: adc: select buffer for at91-sama5d2_adc We need to select the buffer code, otherwise we get build errors with undefined functions on the trigger and buffer, if we select just IIO and then AT91_SAMA5D2_ADC from menuconfig This adds a Kconfig 'select' statement like other ADC drivers have it already. Fixes: 5e1a1da0f8c9 ("iio: adc: at91-sama5d2_adc: add hw trigger and buffer support") Signed-off-by: Eugen Hristev Cc: Signed-off-by: Jonathan Cameron --- drivers/iio/adc/Kconfig | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers') diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig index 72bc2b71765a..47bbed3afc8f 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -159,6 +159,7 @@ config AT91_SAMA5D2_ADC depends on ARCH_AT91 || COMPILE_TEST depends on HAS_IOMEM depends on HAS_DMA + select IIO_BUFFER select IIO_TRIGGERED_BUFFER help Say yes here to build support for Atmel SAMA5D2 ADC which is -- cgit v1.2.3-59-g8ed1b