From f13aa06442258275093addec9cb9ec907d82b2b5 Mon Sep 17 00:00:00 2001 From: Nuno Sá Date: Tue, 15 Sep 2020 14:02:53 +0200 Subject: iio: adis16400: Use Managed device functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the adis managed device functions to setup the buffer and the trigger. The ultimate goal will be to completely drop the non devm version from the lib. Since we are here, drop the `.remove` callback by further using devm functions. Signed-off-by: Nuno Sá Link: https://lore.kernel.org/r/20200915120258.161587-6-nuno.sa@analog.com Signed-off-by: Jonathan Cameron --- drivers/iio/imu/adis16400.c | 38 +++++++++++++++----------------------- 1 file changed, 15 insertions(+), 23 deletions(-) (limited to 'drivers/iio/imu') diff --git a/drivers/iio/imu/adis16400.c b/drivers/iio/imu/adis16400.c index 1ebe3e50d3e6..889c8c2a19f4 100644 --- a/drivers/iio/imu/adis16400.c +++ b/drivers/iio/imu/adis16400.c @@ -1164,6 +1164,12 @@ static void adis16400_setup_chan_mask(struct adis16400_state *st) st->avail_scan_mask[0] |= BIT(ch->scan_index); } } + +static void adis16400_stop(void *data) +{ + adis16400_stop_device(data); +} + static int adis16400_probe(struct spi_device *spi) { struct adis16400_state *st; @@ -1201,37 +1207,24 @@ static int adis16400_probe(struct spi_device *spi) if (ret) return ret; - ret = adis_setup_buffer_and_trigger(&st->adis, indio_dev, - adis16400_trigger_handler); + ret = devm_adis_setup_buffer_and_trigger(&st->adis, indio_dev, adis16400_trigger_handler); if (ret) return ret; /* Get the device into a sane initial state */ ret = adis16400_initial_setup(indio_dev); if (ret) - goto error_cleanup_buffer; - ret = iio_device_register(indio_dev); - if (ret) - goto error_cleanup_buffer; - - adis16400_debugfs_init(indio_dev); - return 0; - -error_cleanup_buffer: - adis_cleanup_buffer_and_trigger(&st->adis, indio_dev); - return ret; -} - -static int adis16400_remove(struct spi_device *spi) -{ - struct iio_dev *indio_dev = spi_get_drvdata(spi); - struct adis16400_state *st = iio_priv(indio_dev); + return ret; - iio_device_unregister(indio_dev); - adis16400_stop_device(indio_dev); + ret = devm_add_action_or_reset(&spi->dev, adis16400_stop, indio_dev); + if (ret) + return ret; - adis_cleanup_buffer_and_trigger(&st->adis, indio_dev); + ret = devm_iio_device_register(&spi->dev, indio_dev); + if (ret) + return ret; + adis16400_debugfs_init(indio_dev); return 0; } @@ -1261,7 +1254,6 @@ static struct spi_driver adis16400_driver = { }, .id_table = adis16400_id, .probe = adis16400_probe, - .remove = adis16400_remove, }; module_spi_driver(adis16400_driver); -- cgit v1.2.3 From fa623cddc040930207b57fa0c4767bbb8f7ac8a1 Mon Sep 17 00:00:00 2001 From: Nuno Sá Date: Tue, 15 Sep 2020 14:02:54 +0200 Subject: iio: adis16460: Use Managed device functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the adis managed device functions to setup the buffer and the trigger. The ultimate goal will be to completely drop the non devm version from the lib. Since we are here, drop the `.remove` callback by further using devm functions. Signed-off-by: Nuno Sá Link: https://lore.kernel.org/r/20200915120258.161587-7-nuno.sa@analog.com Signed-off-by: Jonathan Cameron --- drivers/iio/imu/adis16460.c | 25 ++++--------------------- 1 file changed, 4 insertions(+), 21 deletions(-) (limited to 'drivers/iio/imu') diff --git a/drivers/iio/imu/adis16460.c b/drivers/iio/imu/adis16460.c index b26a5f1bc51a..74a161e39733 100644 --- a/drivers/iio/imu/adis16460.c +++ b/drivers/iio/imu/adis16460.c @@ -403,7 +403,7 @@ static int adis16460_probe(struct spi_device *spi) if (ret) return ret; - ret = adis_setup_buffer_and_trigger(&st->adis, indio_dev, NULL); + ret = devm_adis_setup_buffer_and_trigger(&st->adis, indio_dev, NULL); if (ret) return ret; @@ -411,30 +411,14 @@ static int adis16460_probe(struct spi_device *spi) ret = __adis_initial_startup(&st->adis); if (ret) - goto error_cleanup_buffer; + return ret; - ret = iio_device_register(indio_dev); + ret = devm_iio_device_register(&spi->dev, indio_dev); if (ret) - goto error_cleanup_buffer; + return ret; adis16460_debugfs_init(indio_dev); - return 0; - -error_cleanup_buffer: - adis_cleanup_buffer_and_trigger(&st->adis, indio_dev); - return ret; -} - -static int adis16460_remove(struct spi_device *spi) -{ - struct iio_dev *indio_dev = spi_get_drvdata(spi); - struct adis16460 *st = iio_priv(indio_dev); - - iio_device_unregister(indio_dev); - - adis_cleanup_buffer_and_trigger(&st->adis, indio_dev); - return 0; } @@ -457,7 +441,6 @@ static struct spi_driver adis16460_driver = { }, .id_table = adis16460_ids, .probe = adis16460_probe, - .remove = adis16460_remove, }; module_spi_driver(adis16460_driver); -- cgit v1.2.3 From 514f641b3b2af794df90d61f4346e5858177665d Mon Sep 17 00:00:00 2001 From: Nuno Sá Date: Tue, 15 Sep 2020 14:02:55 +0200 Subject: iio: adis16480: Use Managed device functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the adis managed device functions to setup the buffer and the trigger. The ultimate goal will be to completely drop the non devm version from the lib. Since we are here, drop the `.remove` callback by further using devm functions. Signed-off-by: Nuno Sá Link: https://lore.kernel.org/r/20200915120258.161587-8-nuno.sa@analog.com Signed-off-by: Jonathan Cameron --- drivers/iio/imu/adis16480.c | 55 +++++++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 30 deletions(-) (limited to 'drivers/iio/imu') diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c index 1eb4f98076f1..dfe86c589325 100644 --- a/drivers/iio/imu/adis16480.c +++ b/drivers/iio/imu/adis16480.c @@ -1212,6 +1212,16 @@ static int adis16480_get_ext_clocks(struct adis16480 *st) return 0; } +static void adis16480_stop(void *data) +{ + adis16480_stop_device(data); +} + +static void adis16480_clk_disable(void *data) +{ + clk_disable_unprepare(data); +} + static int adis16480_probe(struct spi_device *spi) { const struct spi_device_id *id = spi_get_device_id(spi); @@ -1245,18 +1255,26 @@ static int adis16480_probe(struct spi_device *spi) if (ret) return ret; + ret = devm_add_action_or_reset(&spi->dev, adis16480_stop, indio_dev); + if (ret) + return ret; + ret = adis16480_config_irq_pin(spi->dev.of_node, st); if (ret) - goto error_stop_device; + return ret; ret = adis16480_get_ext_clocks(st); if (ret) - goto error_stop_device; + return ret; if (!IS_ERR_OR_NULL(st->ext_clk)) { ret = adis16480_ext_clk_config(st, spi->dev.of_node, true); if (ret) - goto error_stop_device; + return ret; + + ret = devm_add_action_or_reset(&spi->dev, adis16480_clk_disable, st->ext_clk); + if (ret) + return ret; st->clk_freq = clk_get_rate(st->ext_clk); st->clk_freq *= 1000; /* micro */ @@ -1264,38 +1282,16 @@ static int adis16480_probe(struct spi_device *spi) st->clk_freq = st->chip_info->int_clk; } - ret = adis_setup_buffer_and_trigger(&st->adis, indio_dev, NULL); + ret = devm_adis_setup_buffer_and_trigger(&st->adis, indio_dev, NULL); if (ret) - goto error_clk_disable_unprepare; + return ret; - ret = iio_device_register(indio_dev); + ret = devm_iio_device_register(&spi->dev, indio_dev); if (ret) - goto error_cleanup_buffer; + return ret; adis16480_debugfs_init(indio_dev); - return 0; - -error_cleanup_buffer: - adis_cleanup_buffer_and_trigger(&st->adis, indio_dev); -error_clk_disable_unprepare: - clk_disable_unprepare(st->ext_clk); -error_stop_device: - adis16480_stop_device(indio_dev); - return ret; -} - -static int adis16480_remove(struct spi_device *spi) -{ - struct iio_dev *indio_dev = spi_get_drvdata(spi); - struct adis16480 *st = iio_priv(indio_dev); - - iio_device_unregister(indio_dev); - adis16480_stop_device(indio_dev); - - adis_cleanup_buffer_and_trigger(&st->adis, indio_dev); - clk_disable_unprepare(st->ext_clk); - return 0; } @@ -1338,7 +1334,6 @@ static struct spi_driver adis16480_driver = { }, .id_table = adis16480_ids, .probe = adis16480_probe, - .remove = adis16480_remove, }; module_spi_driver(adis16480_driver); -- cgit v1.2.3 From d8f0cd76684e679d4a8ec4929fcdf6c3a030a007 Mon Sep 17 00:00:00 2001 From: Nuno Sá Date: Tue, 15 Sep 2020 14:02:58 +0200 Subject: iio: adis: Drop non Managed device functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop `adis_setup_buffer_and_trigger()`. All users were updated to use the devm version of this function. This avoids having almost the same code repeated. Signed-off-by: Nuno Sá Link: https://lore.kernel.org/r/20200915120258.161587-11-nuno.sa@analog.com Signed-off-by: Jonathan Cameron --- drivers/iio/imu/adis_buffer.c | 64 +++--------------------------------------- drivers/iio/imu/adis_trigger.c | 60 --------------------------------------- include/linux/iio/imu/adis.h | 27 ------------------ 3 files changed, 4 insertions(+), 147 deletions(-) (limited to 'drivers/iio/imu') diff --git a/drivers/iio/imu/adis_buffer.c b/drivers/iio/imu/adis_buffer.c index 5b4225ee09b9..df6144285470 100644 --- a/drivers/iio/imu/adis_buffer.c +++ b/drivers/iio/imu/adis_buffer.c @@ -169,48 +169,6 @@ static void adis_buffer_cleanup(void *arg) kfree(adis->xfer); } -/** - * adis_setup_buffer_and_trigger() - Sets up buffer and trigger for the adis device - * @adis: The adis device. - * @indio_dev: The IIO device. - * @trigger_handler: Optional trigger handler, may be NULL. - * - * Returns 0 on success, a negative error code otherwise. - * - * This function sets up the buffer and trigger for a adis devices. If - * 'trigger_handler' is NULL the default trigger handler will be used. The - * default trigger handler will simply read the registers assigned to the - * currently active channels. - * - * adis_cleanup_buffer_and_trigger() should be called to free the resources - * allocated by this function. - */ -int adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev *indio_dev, - irqreturn_t (*trigger_handler)(int, void *)) -{ - int ret; - - if (!trigger_handler) - trigger_handler = adis_trigger_handler; - - ret = iio_triggered_buffer_setup(indio_dev, &iio_pollfunc_store_time, - trigger_handler, NULL); - if (ret) - return ret; - - if (adis->spi->irq) { - ret = adis_probe_trigger(adis, indio_dev); - if (ret) - goto error_buffer_cleanup; - } - return 0; - -error_buffer_cleanup: - iio_triggered_buffer_cleanup(indio_dev); - return ret; -} -EXPORT_SYMBOL_GPL(adis_setup_buffer_and_trigger); - /** * devm_adis_setup_buffer_and_trigger() - Sets up buffer and trigger for * the managed adis device @@ -220,7 +178,10 @@ EXPORT_SYMBOL_GPL(adis_setup_buffer_and_trigger); * * Returns 0 on success, a negative error code otherwise. * - * This function perfoms exactly the same as adis_setup_buffer_and_trigger() + * This function sets up the buffer and trigger for a adis devices. If + * 'trigger_handler' is NULL the default trigger handler will be used. The + * default trigger handler will simply read the registers assigned to the + * currently active channels. */ int devm_adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev *indio_dev, @@ -248,20 +209,3 @@ devm_adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev *indio_dev, } EXPORT_SYMBOL_GPL(devm_adis_setup_buffer_and_trigger); -/** - * adis_cleanup_buffer_and_trigger() - Free buffer and trigger resources - * @adis: The adis device. - * @indio_dev: The IIO device. - * - * Frees resources allocated by adis_setup_buffer_and_trigger() - */ -void adis_cleanup_buffer_and_trigger(struct adis *adis, - struct iio_dev *indio_dev) -{ - if (adis->spi->irq) - adis_remove_trigger(adis); - kfree(adis->buffer); - kfree(adis->xfer); - iio_triggered_buffer_cleanup(indio_dev); -} -EXPORT_SYMBOL_GPL(adis_cleanup_buffer_and_trigger); diff --git a/drivers/iio/imu/adis_trigger.c b/drivers/iio/imu/adis_trigger.c index 8afe71947c00..64e0ba51cb18 100644 --- a/drivers/iio/imu/adis_trigger.c +++ b/drivers/iio/imu/adis_trigger.c @@ -55,53 +55,6 @@ static int adis_validate_irq_flag(struct adis *adis) return 0; } -/** - * adis_probe_trigger() - Sets up trigger for a adis device - * @adis: The adis device - * @indio_dev: The IIO device - * - * Returns 0 on success or a negative error code - * - * adis_remove_trigger() should be used to free the trigger. - */ -int adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev) -{ - int ret; - - adis->trig = iio_trigger_alloc("%s-dev%d", indio_dev->name, - indio_dev->id); - if (adis->trig == NULL) - return -ENOMEM; - - adis_trigger_setup(adis); - - ret = adis_validate_irq_flag(adis); - if (ret) - return ret; - - ret = request_irq(adis->spi->irq, - &iio_trigger_generic_data_rdy_poll, - adis->irq_flag, - indio_dev->name, - adis->trig); - if (ret) - goto error_free_trig; - - ret = iio_trigger_register(adis->trig); - - indio_dev->trig = iio_trigger_get(adis->trig); - if (ret) - goto error_free_irq; - - return 0; - -error_free_irq: - free_irq(adis->spi->irq, adis->trig); -error_free_trig: - iio_trigger_free(adis->trig); - return ret; -} -EXPORT_SYMBOL_GPL(adis_probe_trigger); /** * devm_adis_probe_trigger() - Sets up trigger for a managed adis device @@ -137,16 +90,3 @@ int devm_adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev) } EXPORT_SYMBOL_GPL(devm_adis_probe_trigger); -/** - * adis_remove_trigger() - Remove trigger for a adis devices - * @adis: The adis device - * - * Removes the trigger previously registered with adis_probe_trigger(). - */ -void adis_remove_trigger(struct adis *adis) -{ - iio_trigger_unregister(adis->trig); - free_irq(adis->spi->irq, adis->trig); - iio_trigger_free(adis->trig); -} -EXPORT_SYMBOL_GPL(adis_remove_trigger); diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h index 2df67448f0d1..01ba691da2f3 100644 --- a/include/linux/iio/imu/adis.h +++ b/include/linux/iio/imu/adis.h @@ -517,14 +517,8 @@ struct adis_burst { int devm_adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev *indio_dev, irq_handler_t trigger_handler); -int adis_setup_buffer_and_trigger(struct adis *adis, - struct iio_dev *indio_dev, irqreturn_t (*trigger_handler)(int, void *)); -void adis_cleanup_buffer_and_trigger(struct adis *adis, - struct iio_dev *indio_dev); int devm_adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev); -int adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev); -void adis_remove_trigger(struct adis *adis); int adis_update_scan_mode(struct iio_dev *indio_dev, const unsigned long *scan_mask); @@ -538,33 +532,12 @@ devm_adis_setup_buffer_and_trigger(struct adis *adis, struct iio_dev *indio_dev, return 0; } -static inline int adis_setup_buffer_and_trigger(struct adis *adis, - struct iio_dev *indio_dev, irqreturn_t (*trigger_handler)(int, void *)) -{ - return 0; -} - -static inline void adis_cleanup_buffer_and_trigger(struct adis *adis, - struct iio_dev *indio_dev) -{ -} - static inline int devm_adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev) { return 0; } -static inline int adis_probe_trigger(struct adis *adis, - struct iio_dev *indio_dev) -{ - return 0; -} - -static inline void adis_remove_trigger(struct adis *adis) -{ -} - #define adis_update_scan_mode NULL #endif /* CONFIG_IIO_BUFFER */ -- cgit v1.2.3 From d563d4d834641bae50564bd1e6e48187b630c563 Mon Sep 17 00:00:00 2001 From: Jonathan Cameron Date: Sun, 13 Sep 2020 14:21:13 +0100 Subject: iio:imu:adis16400: Sort out missing kernel doc. I'd like to be enable W=1 for all IIO builds as it catches real issues as well as more minor documentation issues such as this (also good to fix though!) drivers/iio/imu/adis16400.c:183: warning: Function parameter or member 'avail_scan_mask' not described in 'adis16400_state' Signed-off-by: Jonathan Cameron Acked-by: Alexandru Ardelean Cc: Alexandru Ardelean Link: https://lore.kernel.org/r/20200913132115.800131-2-jic23@kernel.org --- drivers/iio/imu/adis16400.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/iio/imu') diff --git a/drivers/iio/imu/adis16400.c b/drivers/iio/imu/adis16400.c index 889c8c2a19f4..73433f7feb31 100644 --- a/drivers/iio/imu/adis16400.c +++ b/drivers/iio/imu/adis16400.c @@ -173,6 +173,8 @@ struct adis16400_chip_info { * @variant: chip variant info * @filt_int: integer part of requested filter frequency * @adis: adis device + * @avail_scan_mask: NULL terminated array of bitmaps of channels + * that must be enabled together **/ struct adis16400_state { struct adis16400_chip_info *variant; -- cgit v1.2.3 From 36e322ec5dd24f1d0840061ffe406458669bccf1 Mon Sep 17 00:00:00 2001 From: Nuno Sá Date: Thu, 17 Sep 2020 17:52:20 +0200 Subject: iio: adis: Move burst mode into adis_data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add burst mode variables in the per device specific data structure. As some drivers support multiple devices with different burst sizes it makes sense this data to be in `adis_data`. While moving the variables, there are two main differences: 1. The `en`variable is dropped. If a device supports burst mode, it will just use it as it will has better performance for almost all real use cases. 2. Replace `extra_len` by `burst_len`. Users should now explicitly define the length of the burst buffer as it is typically constant. This also allows to remove the following line from the library: ``` /* All but the timestamp channel */ burst_length = (indio_dev->num_channels - 1) * sizeof(u16); ``` The library should not assume that a timestamp channel is defined. Moreover, most parts also include some diagnostic data, crc, etc.. in the burst buffer that needed to be included in an `extra_len` variable which is not that nice. On top of this, some devices already start to have some 32bit size channels ... This patch is also a move to completely drop the `struct adis_burst` from the library. Signed-off-by: Nuno Sá Signed-off-by: Jonathan Cameron Link: https://lore.kernel.org/r/20200917155223.218500-2-nuno.sa@analog.com --- drivers/iio/imu/adis_buffer.c | 12 +++++------- include/linux/iio/imu/adis.h | 9 +++++++++ 2 files changed, 14 insertions(+), 7 deletions(-) (limited to 'drivers/iio/imu') diff --git a/drivers/iio/imu/adis_buffer.c b/drivers/iio/imu/adis_buffer.c index df6144285470..ac354321f63a 100644 --- a/drivers/iio/imu/adis_buffer.c +++ b/drivers/iio/imu/adis_buffer.c @@ -26,12 +26,10 @@ static int adis_update_scan_mode_burst(struct iio_dev *indio_dev, unsigned int burst_length, burst_max_length; u8 *tx; - /* All but the timestamp channel */ - burst_length = (indio_dev->num_channels - 1) * sizeof(u16); - burst_length += adis->burst->extra_len + adis->burst_extra_len; + burst_length = adis->data->burst_len + adis->burst_extra_len; - if (adis->burst->burst_max_len) - burst_max_length = adis->burst->burst_max_len; + if (adis->data->burst_max_len) + burst_max_length = adis->data->burst_max_len; else burst_max_length = burst_length; @@ -47,7 +45,7 @@ static int adis_update_scan_mode_burst(struct iio_dev *indio_dev, } tx = adis->buffer + burst_max_length; - tx[0] = ADIS_READ_REG(adis->burst->reg_cmd); + tx[0] = ADIS_READ_REG(adis->data->burst_reg_cmd); tx[1] = 0; adis->xfer[0].tx_buf = tx; @@ -76,7 +74,7 @@ int adis_update_scan_mode(struct iio_dev *indio_dev, kfree(adis->xfer); kfree(adis->buffer); - if (adis->burst && adis->burst->en) + if (adis->data->burst_len) return adis_update_scan_mode_burst(indio_dev, scan_mask); scan_count = indio_dev->scan_bytes / 2; diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h index 01ba691da2f3..c502ea3b9199 100644 --- a/include/linux/iio/imu/adis.h +++ b/include/linux/iio/imu/adis.h @@ -51,6 +51,11 @@ struct adis_timeout { * @timeouts: Chip specific delays * @enable_irq: Hook for ADIS devices that have a special IRQ enable/disable * @has_paging: True if ADIS device has paged registers + * @burst_reg_cmd: Register command that triggers burst + * @burst_len: Burst size in the SPI RX buffer. If @burst_max_len is defined, + * this should be the minimum size supported by the device. + * @burst_max_len: Holds the maximum burst size when the device supports + * more than one burst mode with different sizes */ struct adis_data { unsigned int read_delay; @@ -75,6 +80,10 @@ struct adis_data { int (*enable_irq)(struct adis *adis, bool enable); bool has_paging; + + unsigned int burst_reg_cmd; + unsigned int burst_len; + unsigned int burst_max_len; }; /** -- cgit v1.2.3 From f81d053bb4614fd2b71d774521f9016b6e8548e7 Mon Sep 17 00:00:00 2001 From: Nuno Sá Date: Thu, 17 Sep 2020 17:52:21 +0200 Subject: iio: adis16400: Drop adis_burst usage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Burst mode variables are now part of the `adis_data` struct. The driver also has now to explicitly define the length of the burst buffer. Signed-off-by: Nuno Sá Signed-off-by: Jonathan Cameron Link: https://lore.kernel.org/r/20200917155223.218500-3-nuno.sa@analog.com --- drivers/iio/imu/adis16400.c | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-) (limited to 'drivers/iio/imu') diff --git a/drivers/iio/imu/adis16400.c b/drivers/iio/imu/adis16400.c index 73433f7feb31..54af2ed664f6 100644 --- a/drivers/iio/imu/adis16400.c +++ b/drivers/iio/imu/adis16400.c @@ -319,11 +319,6 @@ enum adis16400_chip_variant { ADIS16448, }; -static struct adis_burst adis16400_burst = { - .en = true, - .reg_cmd = ADIS16400_GLOB_CMD, -}; - static int adis16334_get_freq(struct adis16400_state *st) { int ret; @@ -949,7 +944,7 @@ static const char * const adis16400_status_error_msgs[] = { [ADIS16400_DIAG_STAT_POWER_LOW] = "Power supply below 4.75V", }; -#define ADIS16400_DATA(_timeouts) \ +#define ADIS16400_DATA(_timeouts, _burst_len) \ { \ .msc_ctrl_reg = ADIS16400_MSC_CTRL, \ .glob_cmd_reg = ADIS16400_GLOB_CMD, \ @@ -975,6 +970,8 @@ static const char * const adis16400_status_error_msgs[] = { BIT(ADIS16400_DIAG_STAT_POWER_HIGH) | \ BIT(ADIS16400_DIAG_STAT_POWER_LOW), \ .timeouts = (_timeouts), \ + .burst_reg_cmd = ADIS16400_GLOB_CMD, \ + .burst_len = (_burst_len) \ } static const struct adis_timeout adis16300_timeouts = { @@ -1025,7 +1022,7 @@ static struct adis16400_chip_info adis16400_chips[] = { .temp_offset = 25000000 / 140000, /* 25 C = 0x00 */ .set_freq = adis16400_set_freq, .get_freq = adis16400_get_freq, - .adis_data = ADIS16400_DATA(&adis16300_timeouts), + .adis_data = ADIS16400_DATA(&adis16300_timeouts, 18), }, [ADIS16334] = { .channels = adis16334_channels, @@ -1038,7 +1035,7 @@ static struct adis16400_chip_info adis16400_chips[] = { .temp_offset = 25000000 / 67850, /* 25 C = 0x00 */ .set_freq = adis16334_set_freq, .get_freq = adis16334_get_freq, - .adis_data = ADIS16400_DATA(&adis16334_timeouts), + .adis_data = ADIS16400_DATA(&adis16334_timeouts, 0), }, [ADIS16350] = { .channels = adis16350_channels, @@ -1050,7 +1047,7 @@ static struct adis16400_chip_info adis16400_chips[] = { .flags = ADIS16400_NO_BURST | ADIS16400_HAS_SLOW_MODE, .set_freq = adis16400_set_freq, .get_freq = adis16400_get_freq, - .adis_data = ADIS16400_DATA(&adis16300_timeouts), + .adis_data = ADIS16400_DATA(&adis16300_timeouts, 0), }, [ADIS16360] = { .channels = adis16350_channels, @@ -1063,7 +1060,7 @@ static struct adis16400_chip_info adis16400_chips[] = { .temp_offset = 25000000 / 136000, /* 25 C = 0x00 */ .set_freq = adis16400_set_freq, .get_freq = adis16400_get_freq, - .adis_data = ADIS16400_DATA(&adis16300_timeouts), + .adis_data = ADIS16400_DATA(&adis16300_timeouts, 28), }, [ADIS16362] = { .channels = adis16350_channels, @@ -1076,7 +1073,7 @@ static struct adis16400_chip_info adis16400_chips[] = { .temp_offset = 25000000 / 136000, /* 25 C = 0x00 */ .set_freq = adis16400_set_freq, .get_freq = adis16400_get_freq, - .adis_data = ADIS16400_DATA(&adis16362_timeouts), + .adis_data = ADIS16400_DATA(&adis16362_timeouts, 28), }, [ADIS16364] = { .channels = adis16350_channels, @@ -1089,7 +1086,7 @@ static struct adis16400_chip_info adis16400_chips[] = { .temp_offset = 25000000 / 136000, /* 25 C = 0x00 */ .set_freq = adis16400_set_freq, .get_freq = adis16400_get_freq, - .adis_data = ADIS16400_DATA(&adis16362_timeouts), + .adis_data = ADIS16400_DATA(&adis16362_timeouts, 28), }, [ADIS16367] = { .channels = adis16350_channels, @@ -1102,7 +1099,7 @@ static struct adis16400_chip_info adis16400_chips[] = { .temp_offset = 25000000 / 136000, /* 25 C = 0x00 */ .set_freq = adis16400_set_freq, .get_freq = adis16400_get_freq, - .adis_data = ADIS16400_DATA(&adis16300_timeouts), + .adis_data = ADIS16400_DATA(&adis16300_timeouts, 28), }, [ADIS16400] = { .channels = adis16400_channels, @@ -1114,7 +1111,7 @@ static struct adis16400_chip_info adis16400_chips[] = { .temp_offset = 25000000 / 140000, /* 25 C = 0x00 */ .set_freq = adis16400_set_freq, .get_freq = adis16400_get_freq, - .adis_data = ADIS16400_DATA(&adis16400_timeouts), + .adis_data = ADIS16400_DATA(&adis16400_timeouts, 24), }, [ADIS16445] = { .channels = adis16445_channels, @@ -1128,7 +1125,7 @@ static struct adis16400_chip_info adis16400_chips[] = { .temp_offset = 31000000 / 73860, /* 31 C = 0x00 */ .set_freq = adis16334_set_freq, .get_freq = adis16334_get_freq, - .adis_data = ADIS16400_DATA(&adis16445_timeouts), + .adis_data = ADIS16400_DATA(&adis16445_timeouts, 16), }, [ADIS16448] = { .channels = adis16448_channels, @@ -1142,7 +1139,7 @@ static struct adis16400_chip_info adis16400_chips[] = { .temp_offset = 31000000 / 73860, /* 31 C = 0x00 */ .set_freq = adis16334_set_freq, .get_freq = adis16334_get_freq, - .adis_data = ADIS16400_DATA(&adis16448_timeouts), + .adis_data = ADIS16400_DATA(&adis16448_timeouts, 24), } }; @@ -1198,9 +1195,6 @@ static int adis16400_probe(struct spi_device *spi) if (!(st->variant->flags & ADIS16400_NO_BURST)) { adis16400_setup_chan_mask(st); indio_dev->available_scan_masks = st->avail_scan_mask; - st->adis.burst = &adis16400_burst; - if (st->variant->flags & ADIS16400_BURST_DIAG_STAT) - st->adis.burst_extra_len = sizeof(u16); } adis16400_data = &st->variant->adis_data; -- cgit v1.2.3 From 0dfaa465fc734fec8554f9a700594ea28242dc75 Mon Sep 17 00:00:00 2001 From: Nuno Sá Date: Thu, 17 Sep 2020 17:52:22 +0200 Subject: iio: adis16475: Drop adis_burst usage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Burst mode variables are now part of the `adis_data` struct. The driver also has now to explicitly define the length of the burst buffer. Signed-off-by: Nuno Sá Signed-off-by: Jonathan Cameron Link: https://lore.kernel.org/r/20200917155223.218500-4-nuno.sa@analog.com --- drivers/iio/imu/adis16475.c | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) (limited to 'drivers/iio/imu') diff --git a/drivers/iio/imu/adis16475.c b/drivers/iio/imu/adis16475.c index 35d10ccb66c2..197d48240991 100644 --- a/drivers/iio/imu/adis16475.c +++ b/drivers/iio/imu/adis16475.c @@ -565,6 +565,9 @@ static int adis16475_enable_irq(struct adis *adis, bool enable) BIT(ADIS16475_DIAG_STAT_CLK), \ .enable_irq = adis16475_enable_irq, \ .timeouts = (_timeouts), \ + .burst_reg_cmd = ADIS16475_REG_GLOB_CMD, \ + .burst_len = ADIS16475_BURST_MAX_DATA, \ + .burst_max_len = ADIS16475_BURST32_MAX_DATA \ } static const struct adis16475_sync adis16475_sync_mode[] = { @@ -910,20 +913,6 @@ static const struct iio_info adis16475_info = { .debugfs_reg_access = adis_debugfs_reg_access, }; -static struct adis_burst adis16475_burst = { - .en = true, - .reg_cmd = ADIS16475_REG_GLOB_CMD, - /* - * adis_update_scan_mode_burst() sets the burst length in respect with - * the number of channels and allocates 16 bits for each. However, - * adis1647x devices also need space for DIAG_STAT, DATA_CNTR or - * TIME_STAMP (depending on the clock mode but for us these bytes are - * don't care...) and CRC. - */ - .extra_len = 3 * sizeof(u16), - .burst_max_len = ADIS16475_BURST32_MAX_DATA, -}; - static bool adis16475_validate_crc(const u8 *buffer, u16 crc, const bool burst32) { @@ -1279,7 +1268,6 @@ static int adis16475_probe(struct spi_device *spi) st = iio_priv(indio_dev); spi_set_drvdata(spi, indio_dev); - st->adis.burst = &adis16475_burst; st->info = device_get_match_data(&spi->dev); if (!st->info) -- cgit v1.2.3 From 44a76de8ca4d13b157c49a16d0ef47ccff91a2e3 Mon Sep 17 00:00:00 2001 From: Mario Tesi Date: Thu, 17 Sep 2020 18:47:16 +0200 Subject: iio: imu: st_lsm6dsx: Scaling factor type set to IIO_VAL_INT_PLUS_NANO MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scaling factor values for Acc lead to an unacceptable rounding of the full scale (FS) calculated by some SensorHAL on Android devices. For examples setting FS to 4g the in_accel_x_scale, in_accel_y_scale and in_accel_z_scale are 0.001196 on 6 decimal digits and the FS is 0.001196 × ((2^15) − 1) ~= 39.1893 m/s^2. Android CTS R10 SensorParameterRangeTest test expects a value greater than 39.20 m/s^2 so this test fails (ACCELEROMETER_MAX_RANGE = 4 * 9.80). Using 9 decimal digits the new scale factor is 0.001196411 and the FS now is 0.001196411 × ((2^15)−1) ~= 39.2028 m/s^2. This patch extends to IIO_VAL_INT_PLUS_NANO type the scaling factor to all IMU devices where SensorParameterRangeTest CTS test fails. Signed-off-by: Mario Tesi Acked-by: Lorenzo Bianconi Signed-off-by: Jonathan Cameron Link: https://lore.kernel.org/r/1600361236-2285-1-git-send-email-martepisa@gmail.com --- drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 134 +++++++++++++++------------ 1 file changed, 77 insertions(+), 57 deletions(-) (limited to 'drivers/iio/imu') diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c index 346c24281d26..42f485634d04 100644 --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c @@ -157,10 +157,10 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = { .addr = 0x20, .mask = GENMASK(4, 3), }, - .fs_avl[0] = { IIO_G_TO_M_S_2(61), 0x0 }, - .fs_avl[1] = { IIO_G_TO_M_S_2(122), 0x2 }, - .fs_avl[2] = { IIO_G_TO_M_S_2(244), 0x3 }, - .fs_avl[3] = { IIO_G_TO_M_S_2(732), 0x1 }, + .fs_avl[0] = { IIO_G_TO_M_S_2(61000), 0x0 }, + .fs_avl[1] = { IIO_G_TO_M_S_2(122000), 0x2 }, + .fs_avl[2] = { IIO_G_TO_M_S_2(244000), 0x3 }, + .fs_avl[3] = { IIO_G_TO_M_S_2(732000), 0x1 }, .fs_len = 4, }, [ST_LSM6DSX_ID_GYRO] = { @@ -169,9 +169,9 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = { .mask = GENMASK(4, 3), }, - .fs_avl[0] = { IIO_DEGREE_TO_RAD(8750), 0x0 }, - .fs_avl[1] = { IIO_DEGREE_TO_RAD(17500), 0x1 }, - .fs_avl[2] = { IIO_DEGREE_TO_RAD(70000), 0x3 }, + .fs_avl[0] = { IIO_DEGREE_TO_RAD(8750000), 0x0 }, + .fs_avl[1] = { IIO_DEGREE_TO_RAD(17500000), 0x1 }, + .fs_avl[2] = { IIO_DEGREE_TO_RAD(70000000), 0x3 }, .fs_len = 3, }, }, @@ -259,10 +259,10 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = { .addr = 0x10, .mask = GENMASK(3, 2), }, - .fs_avl[0] = { IIO_G_TO_M_S_2(61), 0x0 }, - .fs_avl[1] = { IIO_G_TO_M_S_2(122), 0x2 }, - .fs_avl[2] = { IIO_G_TO_M_S_2(244), 0x3 }, - .fs_avl[3] = { IIO_G_TO_M_S_2(488), 0x1 }, + .fs_avl[0] = { IIO_G_TO_M_S_2(61000), 0x0 }, + .fs_avl[1] = { IIO_G_TO_M_S_2(122000), 0x2 }, + .fs_avl[2] = { IIO_G_TO_M_S_2(244000), 0x3 }, + .fs_avl[3] = { IIO_G_TO_M_S_2(488000), 0x1 }, .fs_len = 4, }, [ST_LSM6DSX_ID_GYRO] = { @@ -270,10 +270,10 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = { .addr = 0x11, .mask = GENMASK(3, 2), }, - .fs_avl[0] = { IIO_DEGREE_TO_RAD(8750), 0x0 }, - .fs_avl[1] = { IIO_DEGREE_TO_RAD(17500), 0x1 }, - .fs_avl[2] = { IIO_DEGREE_TO_RAD(35000), 0x2 }, - .fs_avl[3] = { IIO_DEGREE_TO_RAD(70000), 0x3 }, + .fs_avl[0] = { IIO_DEGREE_TO_RAD(8750000), 0x0 }, + .fs_avl[1] = { IIO_DEGREE_TO_RAD(17500000), 0x1 }, + .fs_avl[2] = { IIO_DEGREE_TO_RAD(35000000), 0x2 }, + .fs_avl[3] = { IIO_DEGREE_TO_RAD(70000000), 0x3 }, .fs_len = 4, }, }, @@ -425,10 +425,10 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = { .addr = 0x10, .mask = GENMASK(3, 2), }, - .fs_avl[0] = { IIO_G_TO_M_S_2(61), 0x0 }, - .fs_avl[1] = { IIO_G_TO_M_S_2(122), 0x2 }, - .fs_avl[2] = { IIO_G_TO_M_S_2(244), 0x3 }, - .fs_avl[3] = { IIO_G_TO_M_S_2(488), 0x1 }, + .fs_avl[0] = { IIO_G_TO_M_S_2(61000), 0x0 }, + .fs_avl[1] = { IIO_G_TO_M_S_2(122000), 0x2 }, + .fs_avl[2] = { IIO_G_TO_M_S_2(244000), 0x3 }, + .fs_avl[3] = { IIO_G_TO_M_S_2(488000), 0x1 }, .fs_len = 4, }, [ST_LSM6DSX_ID_GYRO] = { @@ -436,10 +436,10 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = { .addr = 0x11, .mask = GENMASK(3, 2), }, - .fs_avl[0] = { IIO_DEGREE_TO_RAD(8750), 0x0 }, - .fs_avl[1] = { IIO_DEGREE_TO_RAD(17500), 0x1 }, - .fs_avl[2] = { IIO_DEGREE_TO_RAD(35000), 0x2 }, - .fs_avl[3] = { IIO_DEGREE_TO_RAD(70000), 0x3 }, + .fs_avl[0] = { IIO_DEGREE_TO_RAD(8750000), 0x0 }, + .fs_avl[1] = { IIO_DEGREE_TO_RAD(17500000), 0x1 }, + .fs_avl[2] = { IIO_DEGREE_TO_RAD(35000000), 0x2 }, + .fs_avl[3] = { IIO_DEGREE_TO_RAD(70000000), 0x3 }, .fs_len = 4, }, }, @@ -600,10 +600,10 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = { .addr = 0x10, .mask = GENMASK(3, 2), }, - .fs_avl[0] = { IIO_G_TO_M_S_2(61), 0x0 }, - .fs_avl[1] = { IIO_G_TO_M_S_2(122), 0x2 }, - .fs_avl[2] = { IIO_G_TO_M_S_2(244), 0x3 }, - .fs_avl[3] = { IIO_G_TO_M_S_2(488), 0x1 }, + .fs_avl[0] = { IIO_G_TO_M_S_2(61000), 0x0 }, + .fs_avl[1] = { IIO_G_TO_M_S_2(122000), 0x2 }, + .fs_avl[2] = { IIO_G_TO_M_S_2(244000), 0x3 }, + .fs_avl[3] = { IIO_G_TO_M_S_2(488000), 0x1 }, .fs_len = 4, }, [ST_LSM6DSX_ID_GYRO] = { @@ -611,10 +611,10 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = { .addr = 0x11, .mask = GENMASK(3, 2), }, - .fs_avl[0] = { IIO_DEGREE_TO_RAD(8750), 0x0 }, - .fs_avl[1] = { IIO_DEGREE_TO_RAD(17500), 0x1 }, - .fs_avl[2] = { IIO_DEGREE_TO_RAD(35000), 0x2 }, - .fs_avl[3] = { IIO_DEGREE_TO_RAD(70000), 0x3 }, + .fs_avl[0] = { IIO_DEGREE_TO_RAD(8750000), 0x0 }, + .fs_avl[1] = { IIO_DEGREE_TO_RAD(17500000), 0x1 }, + .fs_avl[2] = { IIO_DEGREE_TO_RAD(35000000), 0x2 }, + .fs_avl[3] = { IIO_DEGREE_TO_RAD(70000000), 0x3 }, .fs_len = 4, }, }, @@ -816,10 +816,10 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = { .addr = 0x10, .mask = GENMASK(3, 2), }, - .fs_avl[0] = { IIO_G_TO_M_S_2(61), 0x0 }, - .fs_avl[1] = { IIO_G_TO_M_S_2(122), 0x2 }, - .fs_avl[2] = { IIO_G_TO_M_S_2(244), 0x3 }, - .fs_avl[3] = { IIO_G_TO_M_S_2(488), 0x1 }, + .fs_avl[0] = { IIO_G_TO_M_S_2(61000), 0x0 }, + .fs_avl[1] = { IIO_G_TO_M_S_2(122000), 0x2 }, + .fs_avl[2] = { IIO_G_TO_M_S_2(244000), 0x3 }, + .fs_avl[3] = { IIO_G_TO_M_S_2(488000), 0x1 }, .fs_len = 4, }, [ST_LSM6DSX_ID_GYRO] = { @@ -827,10 +827,10 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = { .addr = 0x11, .mask = GENMASK(3, 2), }, - .fs_avl[0] = { IIO_DEGREE_TO_RAD(8750), 0x0 }, - .fs_avl[1] = { IIO_DEGREE_TO_RAD(17500), 0x1 }, - .fs_avl[2] = { IIO_DEGREE_TO_RAD(35000), 0x2 }, - .fs_avl[3] = { IIO_DEGREE_TO_RAD(70000), 0x3 }, + .fs_avl[0] = { IIO_DEGREE_TO_RAD(8750000), 0x0 }, + .fs_avl[1] = { IIO_DEGREE_TO_RAD(17500000), 0x1 }, + .fs_avl[2] = { IIO_DEGREE_TO_RAD(35000000), 0x2 }, + .fs_avl[3] = { IIO_DEGREE_TO_RAD(70000000), 0x3 }, .fs_len = 4, }, }, @@ -1021,10 +1021,10 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = { .addr = 0x10, .mask = GENMASK(3, 2), }, - .fs_avl[0] = { IIO_G_TO_M_S_2(61), 0x0 }, - .fs_avl[1] = { IIO_G_TO_M_S_2(122), 0x2 }, - .fs_avl[2] = { IIO_G_TO_M_S_2(244), 0x3 }, - .fs_avl[3] = { IIO_G_TO_M_S_2(488), 0x1 }, + .fs_avl[0] = { IIO_G_TO_M_S_2(61000), 0x0 }, + .fs_avl[1] = { IIO_G_TO_M_S_2(122000), 0x2 }, + .fs_avl[2] = { IIO_G_TO_M_S_2(244000), 0x3 }, + .fs_avl[3] = { IIO_G_TO_M_S_2(488000), 0x1 }, .fs_len = 4, }, [ST_LSM6DSX_ID_GYRO] = { @@ -1032,10 +1032,10 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = { .addr = 0x11, .mask = GENMASK(3, 2), }, - .fs_avl[0] = { IIO_DEGREE_TO_RAD(8750), 0x0 }, - .fs_avl[1] = { IIO_DEGREE_TO_RAD(17500), 0x1 }, - .fs_avl[2] = { IIO_DEGREE_TO_RAD(35000), 0x2 }, - .fs_avl[3] = { IIO_DEGREE_TO_RAD(70000), 0x3 }, + .fs_avl[0] = { IIO_DEGREE_TO_RAD(8750000), 0x0 }, + .fs_avl[1] = { IIO_DEGREE_TO_RAD(17500000), 0x1 }, + .fs_avl[2] = { IIO_DEGREE_TO_RAD(35000000), 0x2 }, + .fs_avl[3] = { IIO_DEGREE_TO_RAD(70000000), 0x3 }, .fs_len = 4, }, }, @@ -1200,10 +1200,10 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = { .addr = 0x10, .mask = GENMASK(3, 2), }, - .fs_avl[0] = { IIO_G_TO_M_S_2(61), 0x0 }, - .fs_avl[1] = { IIO_G_TO_M_S_2(122), 0x2 }, - .fs_avl[2] = { IIO_G_TO_M_S_2(244), 0x3 }, - .fs_avl[3] = { IIO_G_TO_M_S_2(488), 0x1 }, + .fs_avl[0] = { IIO_G_TO_M_S_2(61000), 0x0 }, + .fs_avl[1] = { IIO_G_TO_M_S_2(122000), 0x2 }, + .fs_avl[2] = { IIO_G_TO_M_S_2(244000), 0x3 }, + .fs_avl[3] = { IIO_G_TO_M_S_2(488000), 0x1 }, .fs_len = 4, }, [ST_LSM6DSX_ID_GYRO] = { @@ -1211,10 +1211,10 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = { .addr = 0x11, .mask = GENMASK(3, 2), }, - .fs_avl[0] = { IIO_DEGREE_TO_RAD(8750), 0x0 }, - .fs_avl[1] = { IIO_DEGREE_TO_RAD(17500), 0x1 }, - .fs_avl[2] = { IIO_DEGREE_TO_RAD(35000), 0x2 }, - .fs_avl[3] = { IIO_DEGREE_TO_RAD(70000), 0x3 }, + .fs_avl[0] = { IIO_DEGREE_TO_RAD(8750000), 0x0 }, + .fs_avl[1] = { IIO_DEGREE_TO_RAD(17500000), 0x1 }, + .fs_avl[2] = { IIO_DEGREE_TO_RAD(35000000), 0x2 }, + .fs_avl[3] = { IIO_DEGREE_TO_RAD(70000000), 0x3 }, .fs_len = 4, }, }, @@ -1598,7 +1598,7 @@ static int st_lsm6dsx_read_raw(struct iio_dev *iio_dev, case IIO_CHAN_INFO_SCALE: *val = 0; *val2 = sensor->gain; - ret = IIO_VAL_INT_PLUS_MICRO; + ret = IIO_VAL_INT_PLUS_NANO; break; default: ret = -EINVAL; @@ -1836,13 +1836,31 @@ static ssize_t st_lsm6dsx_sysfs_scale_avail(struct device *dev, fs_table = &hw->settings->fs_table[sensor->id]; for (i = 0; i < fs_table->fs_len; i++) - len += scnprintf(buf + len, PAGE_SIZE - len, "0.%06u ", + len += scnprintf(buf + len, PAGE_SIZE - len, "0.%09u ", fs_table->fs_avl[i].gain); buf[len - 1] = '\n'; return len; } +static int st_lsm6dsx_write_raw_get_fmt(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, + long mask) +{ + switch (mask) { + case IIO_CHAN_INFO_SCALE: + switch (chan->type) { + case IIO_ANGL_VEL: + case IIO_ACCEL: + return IIO_VAL_INT_PLUS_NANO; + default: + return IIO_VAL_INT_PLUS_MICRO; + } + default: + return IIO_VAL_INT_PLUS_MICRO; + } +} + static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(st_lsm6dsx_sysfs_sampling_frequency_avail); static IIO_DEVICE_ATTR(in_accel_scale_available, 0444, st_lsm6dsx_sysfs_scale_avail, NULL, 0); @@ -1868,6 +1886,7 @@ static const struct iio_info st_lsm6dsx_acc_info = { .read_event_config = st_lsm6dsx_read_event_config, .write_event_config = st_lsm6dsx_write_event_config, .hwfifo_set_watermark = st_lsm6dsx_set_watermark, + .write_raw_get_fmt = st_lsm6dsx_write_raw_get_fmt, }; static struct attribute *st_lsm6dsx_gyro_attributes[] = { @@ -1885,6 +1904,7 @@ static const struct iio_info st_lsm6dsx_gyro_info = { .read_raw = st_lsm6dsx_read_raw, .write_raw = st_lsm6dsx_write_raw, .hwfifo_set_watermark = st_lsm6dsx_set_watermark, + .write_raw_get_fmt = st_lsm6dsx_write_raw_get_fmt, }; static int st_lsm6dsx_get_drdy_pin(struct st_lsm6dsx_hw *hw, int *drdy_pin) -- cgit v1.2.3 From f71e41e23e129640f620b65fc362a6da02580310 Mon Sep 17 00:00:00 2001 From: Tom Rix Date: Sun, 9 Aug 2020 10:55:51 -0700 Subject: iio:imu:st_lsm6dsx: check st_lsm6dsx_shub_read_output return Potential error return is not checked. This can lead to use of undefined data. Detected by clang static analysis. st_lsm6dsx_shub.c:540:8: warning: Assigned value is garbage or undefined *val = (s16)le16_to_cpu(*((__le16 *)data)); ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Fixes: c91c1c844ebd ("iio: imu: st_lsm6dsx: add i2c embedded controller support") Signed-off-by: Tom Rix Reviewed-by: Andy Shevchenko Link: https://lore.kernel.org/r/20200809175551.6794-1-trix@redhat.com Signed-off-by: Jonathan Cameron --- drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/iio/imu') diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c index ed83471dc7dd..8c8d8870ca07 100644 --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c @@ -313,6 +313,8 @@ st_lsm6dsx_shub_read(struct st_lsm6dsx_sensor *sensor, u8 addr, err = st_lsm6dsx_shub_read_output(hw, data, len & ST_LS6DSX_READ_OP_MASK); + if (err < 0) + return err; st_lsm6dsx_shub_master_enable(sensor, false); -- cgit v1.2.3 From c14edb4d0bdc53f969ea84c7f384472c28b1a9f8 Mon Sep 17 00:00:00 2001 From: Jonathan Cameron Date: Wed, 22 Jul 2020 16:50:52 +0100 Subject: iio:imu:st_lsm6dsx Fix alignment and data leak issues One of a class of bugs pointed out by Lars in a recent review. iio_push_to_buffers_with_timestamp assumes the buffer used is aligned to the size of the timestamp (8 bytes). This is not guaranteed in this driver which uses an array of smaller elements on the stack. As Lars also noted this anti pattern can involve a leak of data to userspace and that indeed can happen here. We close both issues by moving to an array of suitable structures in the iio_priv() data. This data is allocated with kzalloc so no data can leak apart from previous readings. For the tagged path the data is aligned by using __aligned(8) for the buffer on the stack. There has been a lot of churn in this driver, so likely backports may be needed for stable. Fixes: 290a6ce11d93 ("iio: imu: add support to lsm6dsx driver") Reported-by: Lars-Peter Clausen Signed-off-by: Jonathan Cameron Reviewed-by: Andy Shevchenko Cc: Lorenzo Bianconi Cc: Link: https://lore.kernel.org/r/20200722155103.979802-17-jic23@kernel.org --- drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 6 ++++ drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 42 ++++++++++++++++---------- 2 files changed, 32 insertions(+), 16 deletions(-) (limited to 'drivers/iio/imu') diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h index d80ba2e688ed..9275346a9cc1 100644 --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h @@ -383,6 +383,7 @@ struct st_lsm6dsx_sensor { * @iio_devs: Pointers to acc/gyro iio_dev instances. * @settings: Pointer to the specific sensor settings in use. * @orientation: sensor chip orientation relative to main hardware. + * @scan: Temporary buffers used to align data before iio_push_to_buffers() */ struct st_lsm6dsx_hw { struct device *dev; @@ -411,6 +412,11 @@ struct st_lsm6dsx_hw { const struct st_lsm6dsx_settings *settings; struct iio_mount_matrix orientation; + /* Ensure natural alignment of buffer elements */ + struct { + __le16 channels[3]; + s64 ts __aligned(8); + } scan[3]; }; static __maybe_unused const struct iio_event_spec st_lsm6dsx_event = { diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c index 7de10bd636ea..12ed0a2e55e4 100644 --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c @@ -353,9 +353,6 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw) int err, sip, acc_sip, gyro_sip, ts_sip, ext_sip, read_len, offset; u16 fifo_len, pattern_len = hw->sip * ST_LSM6DSX_SAMPLE_SIZE; u16 fifo_diff_mask = hw->settings->fifo_ops.fifo_diff.mask; - u8 gyro_buff[ST_LSM6DSX_IIO_BUFF_SIZE]; - u8 acc_buff[ST_LSM6DSX_IIO_BUFF_SIZE]; - u8 ext_buff[ST_LSM6DSX_IIO_BUFF_SIZE]; bool reset_ts = false; __le16 fifo_status; s64 ts = 0; @@ -416,19 +413,22 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw) while (acc_sip > 0 || gyro_sip > 0 || ext_sip > 0) { if (gyro_sip > 0 && !(sip % gyro_sensor->decimator)) { - memcpy(gyro_buff, &hw->buff[offset], - ST_LSM6DSX_SAMPLE_SIZE); - offset += ST_LSM6DSX_SAMPLE_SIZE; + memcpy(hw->scan[ST_LSM6DSX_ID_GYRO].channels, + &hw->buff[offset], + sizeof(hw->scan[ST_LSM6DSX_ID_GYRO].channels)); + offset += sizeof(hw->scan[ST_LSM6DSX_ID_GYRO].channels); } if (acc_sip > 0 && !(sip % acc_sensor->decimator)) { - memcpy(acc_buff, &hw->buff[offset], - ST_LSM6DSX_SAMPLE_SIZE); - offset += ST_LSM6DSX_SAMPLE_SIZE; + memcpy(hw->scan[ST_LSM6DSX_ID_ACC].channels, + &hw->buff[offset], + sizeof(hw->scan[ST_LSM6DSX_ID_ACC].channels)); + offset += sizeof(hw->scan[ST_LSM6DSX_ID_ACC].channels); } if (ext_sip > 0 && !(sip % ext_sensor->decimator)) { - memcpy(ext_buff, &hw->buff[offset], - ST_LSM6DSX_SAMPLE_SIZE); - offset += ST_LSM6DSX_SAMPLE_SIZE; + memcpy(hw->scan[ST_LSM6DSX_ID_EXT0].channels, + &hw->buff[offset], + sizeof(hw->scan[ST_LSM6DSX_ID_EXT0].channels)); + offset += sizeof(hw->scan[ST_LSM6DSX_ID_EXT0].channels); } if (ts_sip-- > 0) { @@ -458,19 +458,22 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw) if (gyro_sip > 0 && !(sip % gyro_sensor->decimator)) { iio_push_to_buffers_with_timestamp( hw->iio_devs[ST_LSM6DSX_ID_GYRO], - gyro_buff, gyro_sensor->ts_ref + ts); + &hw->scan[ST_LSM6DSX_ID_GYRO], + gyro_sensor->ts_ref + ts); gyro_sip--; } if (acc_sip > 0 && !(sip % acc_sensor->decimator)) { iio_push_to_buffers_with_timestamp( hw->iio_devs[ST_LSM6DSX_ID_ACC], - acc_buff, acc_sensor->ts_ref + ts); + &hw->scan[ST_LSM6DSX_ID_ACC], + acc_sensor->ts_ref + ts); acc_sip--; } if (ext_sip > 0 && !(sip % ext_sensor->decimator)) { iio_push_to_buffers_with_timestamp( hw->iio_devs[ST_LSM6DSX_ID_EXT0], - ext_buff, ext_sensor->ts_ref + ts); + &hw->scan[ST_LSM6DSX_ID_EXT0], + ext_sensor->ts_ref + ts); ext_sip--; } sip++; @@ -555,7 +558,14 @@ int st_lsm6dsx_read_tagged_fifo(struct st_lsm6dsx_hw *hw) { u16 pattern_len = hw->sip * ST_LSM6DSX_TAGGED_SAMPLE_SIZE; u16 fifo_len, fifo_diff_mask; - u8 iio_buff[ST_LSM6DSX_IIO_BUFF_SIZE], tag; + /* + * Alignment needed as this can ultimately be passed to a + * call to iio_push_to_buffers_with_timestamp() which + * must be passed a buffer that is aligned to 8 bytes so + * as to allow insertion of a naturally aligned timestamp. + */ + u8 iio_buff[ST_LSM6DSX_IIO_BUFF_SIZE] __aligned(8); + u8 tag; bool reset_ts = false; int i, err, read_len; __le16 fifo_status; -- cgit v1.2.3 From 6b0cc5dce0725ae8f1a2883514da731c55eeb35e Mon Sep 17 00:00:00 2001 From: Jonathan Cameron Date: Wed, 22 Jul 2020 16:50:53 +0100 Subject: iio:imu:inv_mpu6050 Fix dma and ts alignment and data leak issues. This case is a bit different to the rest of the series. The driver was doing a regmap_bulk_read into a buffer that wasn't dma safe as it was on the stack with no guarantee of it being in a cacheline on it's own. Fixing that also dealt with the data leak and alignment issues that Lars-Peter pointed out. Also removed some unaligned handling as we are now aligned. Fixes tag is for the dma safe buffer issue. Potentially we would need to backport timestamp alignment futher but that is a totally different patch. Fixes: fd64df16f40e ("iio: imu: inv_mpu6050: Add SPI support for MPU6000") Reported-by: Lars-Peter Clausen Signed-off-by: Jonathan Cameron Reviewed-by: Andy Shevchenko Reviewed-by: Jean-Baptiste Maneyrol Cc: Link: https://lore.kernel.org/r/20200722155103.979802-18-jic23@kernel.org --- drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 12 +++++++++--- drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 12 +++++------- 2 files changed, 14 insertions(+), 10 deletions(-) (limited to 'drivers/iio/imu') diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h index cd38b3fccc7b..eb522b38acf3 100644 --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h @@ -122,6 +122,13 @@ struct inv_mpu6050_chip_config { u8 user_ctrl; }; +/* + * Maximum of 6 + 6 + 2 + 7 (for MPU9x50) = 21 round up to 24 and plus 8. + * May be less if fewer channels are enabled, as long as the timestamp + * remains 8 byte aligned + */ +#define INV_MPU6050_OUTPUT_DATA_SIZE 32 + /** * struct inv_mpu6050_hw - Other important hardware information. * @whoami: Self identification byte from WHO_AM_I register @@ -165,6 +172,7 @@ struct inv_mpu6050_hw { * @magn_raw_to_gauss: coefficient to convert mag raw value to Gauss. * @magn_orient: magnetometer sensor chip orientation if available. * @suspended_sensors: sensors mask of sensors turned off for suspend + * @data: dma safe buffer used for bulk reads. */ struct inv_mpu6050_state { struct mutex lock; @@ -190,6 +198,7 @@ struct inv_mpu6050_state { s32 magn_raw_to_gauss[3]; struct iio_mount_matrix magn_orient; unsigned int suspended_sensors; + u8 data[INV_MPU6050_OUTPUT_DATA_SIZE] ____cacheline_aligned; }; /*register and associated bit definition*/ @@ -334,9 +343,6 @@ struct inv_mpu6050_state { #define INV_ICM20608_TEMP_OFFSET 8170 #define INV_ICM20608_TEMP_SCALE 3059976 -/* 6 + 6 + 2 + 7 (for MPU9x50) = 21 round up to 24 and plus 8 */ -#define INV_MPU6050_OUTPUT_DATA_SIZE 32 - #define INV_MPU6050_REG_INT_PIN_CFG 0x37 #define INV_MPU6050_ACTIVE_HIGH 0x00 #define INV_MPU6050_ACTIVE_LOW 0x80 diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c index b533fa2dad0a..d8e6b88ddffc 100644 --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c @@ -13,7 +13,6 @@ #include #include #include -#include #include "inv_mpu_iio.h" /** @@ -121,7 +120,6 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p) struct inv_mpu6050_state *st = iio_priv(indio_dev); size_t bytes_per_datum; int result; - u8 data[INV_MPU6050_OUTPUT_DATA_SIZE]; u16 fifo_count; s64 timestamp; int int_status; @@ -160,11 +158,11 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p) * read fifo_count register to know how many bytes are inside the FIFO * right now */ - result = regmap_bulk_read(st->map, st->reg->fifo_count_h, data, - INV_MPU6050_FIFO_COUNT_BYTE); + result = regmap_bulk_read(st->map, st->reg->fifo_count_h, + st->data, INV_MPU6050_FIFO_COUNT_BYTE); if (result) goto end_session; - fifo_count = get_unaligned_be16(&data[0]); + fifo_count = be16_to_cpup((__be16 *)&st->data[0]); /* * Handle fifo overflow by resetting fifo. @@ -182,7 +180,7 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p) inv_mpu6050_update_period(st, pf->timestamp, nb); for (i = 0; i < nb; ++i) { result = regmap_bulk_read(st->map, st->reg->fifo_r_w, - data, bytes_per_datum); + st->data, bytes_per_datum); if (result) goto flush_fifo; /* skip first samples if needed */ @@ -191,7 +189,7 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p) continue; } timestamp = inv_mpu6050_get_timestamp(st); - iio_push_to_buffers_with_timestamp(indio_dev, data, timestamp); + iio_push_to_buffers_with_timestamp(indio_dev, st->data, timestamp); } end_session: -- cgit v1.2.3 From da83df66265b79c956f556b97554a0f23431c11f Mon Sep 17 00:00:00 2001 From: Jonathan Cameron Date: Wed, 22 Jul 2020 16:50:54 +0100 Subject: iio:imu:inv_mpu6050: Use regmap_noinc_read for fifo reads. We should not be assuming that we are reading a sequence of registers as here we are doing a read of a lot of data from a single register address. Not marked for stable as by coincidence it being wrong doesn't make any difference. Suggested-by: Jean-Baptiste Maneyrol Signed-off-by: Jonathan Cameron Reviewed-by: Jean-Baptiste Maneyrol Link: https://lore.kernel.org/r/20200722155103.979802-19-jic23@kernel.org --- drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/iio/imu') diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c index d8e6b88ddffc..45c37525c2f1 100644 --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c @@ -179,8 +179,8 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p) nb = fifo_count / bytes_per_datum; inv_mpu6050_update_period(st, pf->timestamp, nb); for (i = 0; i < nb; ++i) { - result = regmap_bulk_read(st->map, st->reg->fifo_r_w, - st->data, bytes_per_datum); + result = regmap_noinc_read(st->map, st->reg->fifo_r_w, + st->data, bytes_per_datum); if (result) goto flush_fifo; /* skip first samples if needed */ -- cgit v1.2.3