On Thu, 5 Mar 2020 11:39:36 +0100 Lars-Peter Clausen <lars@xxxxxxxxxx> wrote: > On 3/5/20 10:58 AM, Sa, Nuno wrote: > > On Wed, 2020-03-04 at 17:59 +0000, Sa, Nuno wrote: > >> [External] > >> > >> On Tue, 2020-03-03 at 21:08 +0000, Jonathan Cameron wrote: > >>> [External] > >>> > >>> On Tue, 25 Feb 2020 13:41:51 +0100 > >>> Nuno Sá <nuno.sa@xxxxxxxxxx> wrote: > >>> > >>>> Support ADIS16475 and similar IMU devices. These devices are > >>>> a precision, miniature MEMS inertial measurement unit (IMU) that > >>>> includes a triaxial gyroscope and a triaxial accelerometer. Each > >>>> inertial sensor combines with signal conditioning that optimizes > >>>> dynamic performance. > >>>> > >>>> The driver adds support for the following devices: > >>>> * adis16470, adis16475, adis16477, adis16465, adis16467, > >>>> adis16500, > >>>> adis16505, adis16507. > >>>> > >>>> Signed-off-by: Nuno Sá <nuno.sa@xxxxxxxxxx> > >>> A few bits and pieces inline. > >>> > >>> Thanks, > >>> > >>> Jonathan > >>> > >>> > >>>> --- > >>>> .../ABI/testing/sysfs-bus-iio-imu-adis16475 | 7 + > >>>> MAINTAINERS | 8 + > >>>> drivers/iio/imu/Kconfig | 13 + > >>>> drivers/iio/imu/Makefile | 1 + > >>>> drivers/iio/imu/adis16475.c | 1304 > >>>> +++++++++++++++++ > >>>> 5 files changed, 1333 insertions(+) > >>>> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-imu- > >>>> adis16475 > >>>> create mode 100644 drivers/iio/imu/adis16475.c > >>>> > >>>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-imu- > >>>> adis16475 > >>>> b/Documentation/ABI/testing/sysfs-bus-iio-imu-adis16475 > >>>> new file mode 100644 > >>>> index 000000000000..e2c3776035ea > >>>> --- /dev/null > >>>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-imu-adis16475 > >>>> @@ -0,0 +1,7 @@ > >>>> +What: /sys/bus/iio/devices/iio:deviceX/burst_mode_ena > >>>> ble > >>>> +KernelVersion: > >>>> +Contact: linux-iio@xxxxxxxxxxxxxxx > >>>> +Description: > >>>> + Use the device burst read mode when reading buffered > >>>> + data. This mode provides a way to read a batch of > >>>> + output data registers, using a continuous stream of > >>>> bits. > >>> See comment on this below. I'm not keen on this being exposed to > >>> userspace > >>> because it will rarely have any idea how to set it. > >>> > >>>> diff --git a/MAINTAINERS b/MAINTAINERS > >>>> index 8fa40c3eb72a..f11262f1f3bb 100644 > >>>> --- a/MAINTAINERS > >>>> +++ b/MAINTAINERS > >>>> @@ -1008,6 +1008,14 @@ W: > >>>> http://ez.analog.com/community/linux-device-drivers > >>>> F: drivers/iio/imu/adis16460.c > >>>> F: Documentation/devicetree/bindings/iio/imu/adi,adis16460 > >>>> .yaml > >>>> > >>>> +ANALOG DEVICES INC ADIS16475 DRIVER > >>>> +M: Nuno Sa <nuno.sa@xxxxxxxxxx> > >>>> +L: linux-iio@xxxxxxxxxxxxxxx > >>>> +W: http://ez.analog.com/community/linux-device-drivers > >>>> +S: Supported > >>>> +F: drivers/iio/imu/adis16475.c > >>>> +F: Documentation/ABI/testing/sysfs-bus-iio-imu-adis16475 > >>>> + > >>>> ANALOG DEVICES INC ADM1177 DRIVER > >>>> M: Beniamin Bia <beniamin.bia@xxxxxxxxxx> > >>>> M: Michael Hennerich <Michael.Hennerich@xxxxxxxxxx> > >>>> diff --git a/drivers/iio/imu/Kconfig b/drivers/iio/imu/Kconfig > >>>> index 60bb1029e759..fc4123d518bc 100644 > >>>> --- a/drivers/iio/imu/Kconfig > >>>> +++ b/drivers/iio/imu/Kconfig > >>>> @@ -29,6 +29,19 @@ config ADIS16460 > >>>> To compile this driver as a module, choose M here: the module > >>>> will be > >>>> called adis16460. > >>>> > >>>> +config ADIS16475 > >>>> + tristate "Analog Devices ADIS16475 and similar IMU driver" > >>>> + depends on SPI > >>>> + select IIO_ADIS_LIB > >>>> + select IIO_ADIS_LIB_BUFFER if IIO_BUFFER > >>>> + help > >>>> + Say yes here to build support for Analog Devices ADIS16470, > >>>> ADIS16475, > >>>> + ADIS16477, ADIS16465, ADIS16467, ADIS16500, ADIS16505, > >>>> ADIS16507 inertial > >>>> + sensors. > >>>> + > >>>> + To compile this driver as a module, choose M here: the module > >>>> will be > >>>> + called adis16475. > >>>> + > >>>> config ADIS16480 > >>>> tristate "Analog Devices ADIS16480 and similar IMU driver" > >>>> depends on SPI > >>>> diff --git a/drivers/iio/imu/Makefile b/drivers/iio/imu/Makefile > >>>> index 5237fd4bc384..88b2c4555230 100644 > >>>> --- a/drivers/iio/imu/Makefile > >>>> +++ b/drivers/iio/imu/Makefile > >>>> @@ -6,6 +6,7 @@ > >>>> # When adding new entries keep the list in alphabetical order > >>>> obj-$(CONFIG_ADIS16400) += adis16400.o > >>>> obj-$(CONFIG_ADIS16460) += adis16460.o > >>>> +obj-$(CONFIG_ADIS16475) += adis16475.o > >>>> obj-$(CONFIG_ADIS16480) += adis16480.o > >>>> > >>>> adis_lib-y += adis.o > >>>> diff --git a/drivers/iio/imu/adis16475.c > >>>> b/drivers/iio/imu/adis16475.c > >>>> new file mode 100644 > >>>> index 000000000000..f7c637734ec8 > >>>> --- /dev/null > >>>> +++ b/drivers/iio/imu/adis16475.c > >>>> @@ -0,0 +1,1304 @@ > >>>> +// SPDX-License-Identifier: GPL-2.0 > >>>> +/* > >>>> + * ADIS16475 IMU driver > >>>> + * > >>>> + * Copyright 2019 Analog Devices Inc. > >>>> + */ > >>>> +#include <asm/unaligned.h> > >>>> +#include <linux/bitfield.h> > >>>> +#include <linux/bitops.h> > >>>> +#include <linux/clk.h> > >>>> +#include <linux/debugfs.h> > >>>> +#include <linux/delay.h> > >>>> +#include <linux/device.h> > >>>> +#include <linux/kernel.h> > >>>> +#include <linux/iio/buffer.h> > >>>> +#include <linux/iio/iio.h> > >>>> +#include <linux/iio/imu/adis.h> > >>>> +#include <linux/iio/sysfs.h> > >>>> +#include <linux/iio/trigger_consumer.h> > >>>> +#include <linux/irq.h> > >>>> +#include <linux/module.h> > >>>> +#include <linux/spi/spi.h> > >>>> + > >>>> +#define ADIS16475_REG_DIAG_STAT 0x02 > >>>> +#define ADIS16475_REG_X_GYRO_L 0x04 > >>>> +#define ADIS16475_REG_Y_GYRO_L 0x08 > >>>> +#define ADIS16475_REG_Z_GYRO_L 0x0C > >>>> +#define ADIS16475_REG_X_ACCEL_L 0x10 > >>>> +#define ADIS16475_REG_Y_ACCEL_L 0x14 > >>>> +#define ADIS16475_REG_Z_ACCEL_L 0x18 > >>>> +#define ADIS16475_REG_TEMP_OUT 0x1c > >>>> +#define ADIS16475_REG_X_GYRO_BIAS_L 0x40 > >>>> +#define ADIS16475_REG_Y_GYRO_BIAS_L 0x44 > >>>> +#define ADIS16475_REG_Z_GYRO_BIAS_L 0x48 > >>>> +#define ADIS16475_REG_X_ACCEL_BIAS_L 0x4c > >>>> +#define ADIS16475_REG_Y_ACCEL_BIAS_L 0x50 > >>>> +#define ADIS16475_REG_Z_ACCEL_BIAS_L 0x54 > >>>> +#define ADIS16475_REG_FILT_CTRL 0x5c > >>>> +#define ADIS16475_FILT_CTRL_MASK GENMASK(2, 0) > >>>> +#define ADIS16475_FILT_CTRL(x) FIELD_PREP(ADIS16475_FI > >>>> LT_CTRL_MASK, x) > >>>> +#define ADIS16475_REG_MSG_CTRL 0x60 > >>>> +#define ADIS16475_MSG_CTRL_DR_POL_MASK BIT(0) > >>>> +#define ADIS16475_MSG_CTRL_DR_POL(x) \ > >>>> + FIELD_PREP(ADIS16475_MSG_CTRL_DR_POL_MA > >>>> SK, x) > >>>> +#define ADIS16475_EXT_CLK_MASK GENMASK(4, 2) > >>>> +#define ADIS16475_EXT_CLK(x) FIELD_PREP(ADIS16475_EX > >>>> T_CLK_MASK, x) > >>>> +#define ADIS16475_REG_UP_SCALE 0x62 > >>>> +#define ADIS16475_REG_DEC_RATE 0x64 > >>>> +#define ADIS16475_REG_GLOB_CMD 0x68 > >>>> +#define ADIS16475_REG_FIRM_REV 0x6c > >>>> +#define ADIS16475_REG_FIRM_DM 0x6e > >>>> +#define ADIS16475_REG_FIRM_Y 0x70 > >>>> +#define ADIS16475_REG_PROD_ID 0x72 > >>>> +#define ADIS16475_REG_SERIAL_NUM 0x74 > >>>> +#define ADIS16475_REG_FLASH_CNT 0x7c > >>>> +#define ADIS16500_BURST32_MASK BIT(9) > >>>> +#define ADIS16500_BURST32(x) FIELD_PREP(ADIS16500_BU > >>>> RST32_MASK, x) > >>>> +/* number of data elements in burst mode */ > >>>> +#define ADIS16475_BURST_MAX_DATA 10 > >>>> +#define ADIS16475_MAX_SCAN_DATA 15 > >>>> + > >>>> +enum clk_mode { > >>>> + ADIS16475_CLK_DIRECT = 1, > >>>> + ADIS16475_CLK_SCALED, > >>>> + ADIS16475_CLK_OUTPUT, > >>>> + ADIS16475_CLK_PULSE = 5, > >>>> +}; > >>>> + > >>>> +struct adis16475_clks { > >>>> + const char *name; > >>>> + enum clk_mode clk_mode; > >>>> + u16 min_rate; > >>>> + u16 max_rate; > >>>> +}; > >>>> + > >>>> +struct adis16475_chip_info { > >>>> + const struct iio_chan_spec *channels; > >>>> + const struct adis16475_clks *clks; > >>>> + const struct adis_data adis_data; > >>>> + u32 num_channels; > >>>> + u32 gyro_max_val; > >>>> + u32 gyro_max_scale; > >>>> + u32 accel_max_val; > >>>> + u32 accel_max_scale; > >>>> + u32 temp_scale; > >>>> + u32 int_clk; > >>>> + u16 max_dec; > >>>> + u8 num_clks; > >>>> + bool has_burst32; > >>>> +}; > >>>> + > >>>> +struct adis16475 { > >>>> + const struct adis16475_chip_info *info; > >>>> + struct adis adis; > >>>> + u32 clk_freq; > >>>> + u32 cached_spi_speed_hz; > >>>> + bool burst32; > >>>> +}; > >>>> + > >>>> +enum { > >>>> + ADIS16475_SCAN_GYRO_X, > >>>> + ADIS16475_SCAN_GYRO_Y, > >>>> + ADIS16475_SCAN_GYRO_Z, > >>>> + ADIS16475_SCAN_ACCEL_X, > >>>> + ADIS16475_SCAN_ACCEL_Y, > >>>> + ADIS16475_SCAN_ACCEL_Z, > >>>> + ADIS16475_SCAN_TEMP, > >>>> + ADIS16475_SCAN_DIAG_S_FLAGS, > >>>> + ADIS16475_SCAN_CRC_FAILURE, > >>>> +}; > >>>> + > >>>> +#ifdef CONFIG_DEBUG_FS > >>>> +static ssize_t adis16475_show_firmware_revision(struct file > >>>> *file, > >>>> + char __user *userbuf, > >>>> + size_t count, loff_t > >>>> *ppos) > >>>> +{ > >>>> + struct adis16475 *st = file->private_data; > >>>> + char buf[7]; > >>>> + size_t len; > >>>> + u16 rev; > >>>> + int ret; > >>>> + > >>>> + ret = adis_read_reg_16(&st->adis, ADIS16475_REG_FIRM_REV, > >>>> &rev); > >>>> + if (ret) > >>>> + return ret; > >>>> + > >>>> + len = scnprintf(buf, sizeof(buf), "%x.%x\n", rev >> 8, rev & > >>>> 0xff); > >>>> + > >>>> + return simple_read_from_buffer(userbuf, count, ppos, buf, len); > >>>> +} > >>>> + > >>>> +static const struct file_operations > >>>> adis16475_firmware_revision_fops = { > >>>> + .open = simple_open, > >>>> + .read = adis16475_show_firmware_revision, > >>>> + .llseek = default_llseek, > >>>> + .owner = THIS_MODULE, > >>>> +}; > >>>> + > >>>> +static ssize_t adis16475_show_firmware_date(struct file *file, > >>>> + char __user *userbuf, > >>>> + size_t count, loff_t *ppos) > >>>> +{ > >>>> + struct adis16475 *st = file->private_data; > >>>> + u16 md, year; > >>>> + char buf[12]; > >>>> + size_t len; > >>>> + int ret; > >>>> + > >>>> + ret = adis_read_reg_16(&st->adis, ADIS16475_REG_FIRM_Y, &year); > >>>> + if (ret) > >>>> + return ret; > >>>> + > >>>> + ret = adis_read_reg_16(&st->adis, ADIS16475_REG_FIRM_DM, &md); > >>>> + if (ret) > >>>> + return ret; > >>>> + > >>>> + len = snprintf(buf, sizeof(buf), "%.2x-%.2x-%.4x\n", md >> 8, > >>>> md & 0xff, > >>>> + year); > >>>> + > >>>> + return simple_read_from_buffer(userbuf, count, ppos, buf, len); > >>>> +} > >>>> + > >>>> +static const struct file_operations adis16475_firmware_date_fops > >>>> = > >>>> { > >>>> + .open = simple_open, > >>>> + .read = adis16475_show_firmware_date, > >>>> + .llseek = default_llseek, > >>>> + .owner = THIS_MODULE, > >>>> +}; > >>>> + > >>>> +static int adis16475_show_serial_number(void *arg, u64 *val) > >>>> +{ > >>>> + struct adis16475 *st = arg; > >>>> + u16 serial; > >>>> + int ret; > >>>> + > >>>> + ret = adis_read_reg_16(&st->adis, ADIS16475_REG_SERIAL_NUM, > >>>> &serial); > >>>> + if (ret) > >>>> + return ret; > >>>> + > >>>> + *val = serial; > >>>> + > >>>> + return 0; > >>>> +} > >>>> +DEFINE_SIMPLE_ATTRIBUTE(adis16475_serial_number_fops, > >>>> + adis16475_show_serial_number, NULL, > >>>> "0x%.4llx\n"); > >>>> + > >>>> +static int adis16475_show_product_id(void *arg, u64 *val) > >>>> +{ > >>>> + struct adis16475 *st = arg; > >>>> + u16 prod_id; > >>>> + int ret; > >>>> + > >>>> + ret = adis_read_reg_16(&st->adis, ADIS16475_REG_PROD_ID, > >>>> &prod_id); > >>>> + if (ret) > >>>> + return ret; > >>>> + > >>>> + *val = prod_id; > >>>> + > >>>> + return 0; > >>>> +} > >>>> +DEFINE_SIMPLE_ATTRIBUTE(adis16475_product_id_fops, > >>>> + adis16475_show_product_id, NULL, "%llu\n"); > >>>> + > >>>> +static int adis16475_show_flash_count(void *arg, u64 *val) > >>>> +{ > >>>> + struct adis16475 *st = arg; > >>>> + u32 flash_count; > >>>> + int ret; > >>>> + > >>>> + ret = adis_read_reg_32(&st->adis, ADIS16475_REG_FLASH_CNT, > >>>> + &flash_count); > >>>> + if (ret) > >>>> + return ret; > >>>> + > >>>> + *val = flash_count; > >>>> + > >>>> + return 0; > >>>> +} > >>>> +DEFINE_SIMPLE_ATTRIBUTE(adis16475_flash_count_fops, > >>>> + adis16475_show_flash_count, NULL, "%lld\n"); > >>>> + > >>>> +static int adis16475_debugfs_init(struct iio_dev *indio_dev) > >>>> +{ > >>>> + struct adis16475 *st = iio_priv(indio_dev); > >>>> + > >>>> + debugfs_create_file("serial_number", 0400, indio_dev- > >>>>> debugfs_dentry, > >>>> + st, &adis16475_serial_number_fops); > >>>> + debugfs_create_file("product_id", 0400, indio_dev- > >>>>> debugfs_dentry, st, > >>>> + &adis16475_product_id_fops); > >>>> + debugfs_create_file("flash_count", 0400, indio_dev- > >>>>> debugfs_dentry, st, > >>>> + &adis16475_flash_count_fops); > >>>> + debugfs_create_file("firmware_revision", 0400, > >>>> + indio_dev->debugfs_dentry, st, > >>>> + &adis16475_firmware_revision_fops); > >>>> + debugfs_create_file("firmware_date", 0400, indio_dev- > >>>>> debugfs_dentry, > >>>> + st, &adis16475_firmware_date_fops); > >>>> + return 0; > >>>> +} > >>>> +#else > >>>> +static int adis16475_debugfs_init(struct iio_dev *indio_dev) > >>>> +{ > >>>> + return 0; > >>>> +} > >>>> +#endif > >>>> + > >>>> +static ssize_t adis16475_burst_mode_enable_get(struct device > >>>> *dev, > >>>> + struct device_attribute > >>>> *attr, > >>>> + char *buf) > >>>> +{ > >>>> + struct adis16475 *st = iio_priv(dev_to_iio_dev(dev)); > >>>> + > >>>> + return sprintf(buf, "%d\n", st->adis.burst->en); > >>>> +} > >>>> + > >>>> +static ssize_t adis16475_burst_mode_enable_set(struct device > >>>> *dev, > >>>> + struct device_attribute > >>>> *attr, > >>>> + const char *buf, size_t > >>>> len) > >>>> +{ > >>>> + struct adis16475 *st = iio_priv(dev_to_iio_dev(dev)); > >>>> + bool val; > >>>> + int ret; > >>>> + > >>>> + ret = kstrtobool(buf, &val); > >>>> + if (ret) > >>>> + return ret; > >>>> + > >>>> + if (val) > >>>> + /* 1MHz max in burst mode */ > >>>> + st->adis.spi->max_speed_hz = 1000000; > >>>> + else > >>>> + st->adis.spi->max_speed_hz = st->cached_spi_speed_hz; > >>>> + > >>>> + st->adis.burst->en = val; > >>>> + > >>>> + return len; > >>>> +} > >>>> + > >>>> +static IIO_DEVICE_ATTR(burst_mode_enable, 0644, > >>>> + adis16475_burst_mode_enable_get, > >>>> + adis16475_burst_mode_enable_set, 0); > >>>> + > >>>> +static struct attribute *adis16475_attributes[] = { > >>>> + &iio_dev_attr_burst_mode_enable.dev_attr.attr, > >>> Hmm. Normally we try to avoid exposing this and make the decision > >>> automatically based on which channels are enabled. > >> Hmm. In that case, the decision would probably have to go to the > >> library since it is there that the "preparations" for buffered mode > >> are > >> done. Mostly, the data we are interested in the burst data is gyro, > >> accel and temp. So to make the decision based on which channels are > >> enabled is not that straight. Should we enable it only when all > >> channels are enabled? Some of them (and which)? > > I thought a little bit more about it and we could do some math in the > > lib to know when using burst mode becomes faster (depending on enabled > > scan elements). The library would need to know about the burst max spi > > frequency and also some minor changes in how burst_len is being > > calculated (drivers should explicitly set this rather than the > > library). The question is, is it worth it to go with this extra work > > :)? > > You can leave it out of the initial version and always use burst mode > and then add support for this in a follow up patch if it is worth it. > > - Lars > Agreed. No one tends to buy an expensive IMU to just sample one channel quickly! Hence I doubt anyone will ever run it in a mode where burst mode doesn't make sense. Thanks, Jonathan
