2023-07-19 12:40:50

by Ramona Bolboaca

[permalink] [raw]
Subject: [PATCH v2 0/3] Add new channels for adis16475

changes in v2:
new patches: 1,2
patch3: added buffer support for delta angle and delta velocity channels
If a device supports burst32, it also supports burst mode for reading delta
angle and delta velocity measurements, so the same has_burst32 flag was used
to handle the logic for reading delta angle and delta velocity channels.

Ramona Bolboaca (3):
iio: imu: adis16475.c: Remove unused enum elements
iio: imu: adis16475.c: Add has_burst32 flag to adis16477 devices
iio: imu: adis16475.c: Add delta angle and delta velocity channels

drivers/iio/imu/adis16475.c | 215 +++++++++++++++++++++++++++++++-----
1 file changed, 187 insertions(+), 28 deletions(-)

--
2.25.1



2023-07-19 12:41:01

by Ramona Bolboaca

[permalink] [raw]
Subject: [PATCH v2 2/3] iio: imu: adis16475.c: Add has_burst32 flag to adis16477 devices

adis16477 devices support burst32 function, thus has_burst32
flag should be set to true.

Signed-off-by: Ramona Bolboaca <[email protected]>
---
drivers/iio/imu/adis16475.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/iio/imu/adis16475.c b/drivers/iio/imu/adis16475.c
index 243f0a91fdf9..17275a53ca2c 100644
--- a/drivers/iio/imu/adis16475.c
+++ b/drivers/iio/imu/adis16475.c
@@ -726,6 +726,7 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.max_dec = 1999,
.sync = adis16475_sync_mode,
.num_sync = ARRAY_SIZE(adis16475_sync_mode),
+ .has_burst32 = true,
.adis_data = ADIS16475_DATA(16477, &adis16475_timeouts),
},
[ADIS16477_2] = {
@@ -741,6 +742,7 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.max_dec = 1999,
.sync = adis16475_sync_mode,
.num_sync = ARRAY_SIZE(adis16475_sync_mode),
+ .has_burst32 = true,
.adis_data = ADIS16475_DATA(16477, &adis16475_timeouts),
},
[ADIS16477_3] = {
@@ -756,6 +758,7 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.max_dec = 1999,
.sync = adis16475_sync_mode,
.num_sync = ARRAY_SIZE(adis16475_sync_mode),
+ .has_burst32 = true,
.adis_data = ADIS16475_DATA(16477, &adis16475_timeouts),
},
[ADIS16465_1] = {
--
2.25.1


2023-07-19 12:41:48

by Ramona Bolboaca

[permalink] [raw]
Subject: [PATCH v2 1/3] iio: imu: adis16475.c: Remove unused enum elements

Remove unused enum elements ADIS16475_SCAN_DIAG_S_FLAGS and
ADIS16475_SCAN_CRC_FAILURE.

Signed-off-by: Ramona Bolboaca <[email protected]>
---
drivers/iio/imu/adis16475.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/drivers/iio/imu/adis16475.c b/drivers/iio/imu/adis16475.c
index 3abffb01ba31..243f0a91fdf9 100644
--- a/drivers/iio/imu/adis16475.c
+++ b/drivers/iio/imu/adis16475.c
@@ -115,8 +115,6 @@ enum {
ADIS16475_SCAN_ACCEL_Y,
ADIS16475_SCAN_ACCEL_Z,
ADIS16475_SCAN_TEMP,
- ADIS16475_SCAN_DIAG_S_FLAGS,
- ADIS16475_SCAN_CRC_FAILURE,
};

static bool low_rate_allow;
--
2.25.1


2023-07-19 12:42:01

by Ramona Bolboaca

[permalink] [raw]
Subject: [PATCH v2 3/3] iio: imu: adis16475.c: Add delta angle and delta velocity channels

Add support for delta angle and delta velocity raw and buffer
readings to adis16475 driver.

Signed-off-by: Ramona Bolboaca <[email protected]>
---
drivers/iio/imu/adis16475.c | 210 +++++++++++++++++++++++++++++++-----
1 file changed, 184 insertions(+), 26 deletions(-)

diff --git a/drivers/iio/imu/adis16475.c b/drivers/iio/imu/adis16475.c
index 17275a53ca2c..e92b42974ee6 100644
--- a/drivers/iio/imu/adis16475.c
+++ b/drivers/iio/imu/adis16475.c
@@ -31,6 +31,12 @@
#define ADIS16475_REG_Y_ACCEL_L 0x14
#define ADIS16475_REG_Z_ACCEL_L 0x18
#define ADIS16475_REG_TEMP_OUT 0x1c
+#define ADIS16475_REG_X_DELTANG_L 0x24
+#define ADIS16475_REG_Y_DELTANG_L 0x28
+#define ADIS16475_REG_Z_DELTANG_L 0x2C
+#define ADIS16475_REG_X_DELTVEL_L 0x30
+#define ADIS16475_REG_Y_DELTVEL_L 0x34
+#define ADIS16475_REG_Z_DELTVEL_L 0x38
#define ADIS16475_REG_X_GYRO_BIAS_L 0x40
#define ADIS16475_REG_Y_GYRO_BIAS_L 0x44
#define ADIS16475_REG_Z_GYRO_BIAS_L 0x48
@@ -55,6 +61,8 @@
#define ADIS16475_REG_PROD_ID 0x72
#define ADIS16475_REG_SERIAL_NUM 0x74
#define ADIS16475_REG_FLASH_CNT 0x7c
+#define ADIS16500_BURST_DATA_SEL_MASK BIT(8)
+#define ADIS16500_BURST_DATA_SEL(x) FIELD_PREP(ADIS16500_BURST_DATA_SEL_MASK, x)
#define ADIS16500_BURST32_MASK BIT(9)
#define ADIS16500_BURST32(x) FIELD_PREP(ADIS16500_BURST32_MASK, x)
/* number of data elements in burst mode */
@@ -65,6 +73,8 @@
#define ADIS16475_BURST_MAX_SPEED 1000000
#define ADIS16475_LSB_DEC_MASK BIT(0)
#define ADIS16475_LSB_FIR_MASK BIT(1)
+#define ADIS16500_BURST_DATA_SEL_0_CHN_MASK (GENMASK(6, 0) | BIT(13))
+#define ADIS16500_BURST_DATA_SEL_1_CHN_MASK GENMASK(13, 6)

enum {
ADIS16475_SYNC_DIRECT = 1,
@@ -90,6 +100,8 @@ struct adis16475_chip_info {
u32 accel_max_val;
u32 accel_max_scale;
u32 temp_scale;
+ u32 deltang_max_val;
+ u32 deltvel_max_val;
u32 int_clk;
u16 max_dec;
u8 num_sync;
@@ -115,6 +127,12 @@ enum {
ADIS16475_SCAN_ACCEL_Y,
ADIS16475_SCAN_ACCEL_Z,
ADIS16475_SCAN_TEMP,
+ ADIS16475_SCAN_DELTANG_X,
+ ADIS16475_SCAN_DELTANG_Y,
+ ADIS16475_SCAN_DELTANG_Z,
+ ADIS16475_SCAN_DELTVEL_X,
+ ADIS16475_SCAN_DELTVEL_Y,
+ ADIS16475_SCAN_DELTVEL_Z,
};

static bool low_rate_allow;
@@ -451,6 +469,14 @@ static int adis16475_read_raw(struct iio_dev *indio_dev,
case IIO_TEMP:
*val = st->info->temp_scale;
return IIO_VAL_INT;
+ case IIO_ROT:
+ *val = st->info->deltang_max_val;
+ *val2 = 31;
+ return IIO_VAL_FRACTIONAL_LOG2;
+ case IIO_VELOCITY:
+ *val = st->info->deltvel_max_val;
+ *val2 = 31;
+ return IIO_VAL_FRACTIONAL_LOG2;
default:
return -EINVAL;
}
@@ -551,6 +577,40 @@ static int adis16475_write_raw(struct iio_dev *indio_dev,
}, \
}

+#define ADIS16475_MOD_CHAN_DELTA(_type, _mod, _address, _si, _r_bits, _s_bits) { \
+ .type = (_type), \
+ .modified = 1, \
+ .channel2 = (_mod), \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+ .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
+ BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY), \
+ .address = (_address), \
+ .scan_index = _si, \
+ .scan_type = { \
+ .sign = 's', \
+ .realbits = (_r_bits), \
+ .storagebits = (_s_bits), \
+ .endianness = IIO_BE, \
+ }, \
+ }
+
+#define ADIS16475_DELTANG_CHAN(_mod) \
+ ADIS16475_MOD_CHAN_DELTA(IIO_ROT, IIO_MOD_ ## _mod, \
+ ADIS16475_REG_ ## _mod ## _DELTANG_L, ADIS16475_SCAN_DELTANG_ ## _mod, 32, 32)
+
+#define ADIS16475_DELTVEL_CHAN(_mod) \
+ ADIS16475_MOD_CHAN_DELTA(IIO_VELOCITY, IIO_MOD_ ## _mod, \
+ ADIS16475_REG_ ## _mod ## _DELTVEL_L, ADIS16475_SCAN_DELTVEL_ ## _mod, 32, 32)
+
+#define ADIS16475_DELTANG_CHAN_NO_SCAN(_mod) \
+ ADIS16475_MOD_CHAN_DELTA(IIO_ROT, IIO_MOD_ ## _mod, \
+ ADIS16475_REG_ ## _mod ## _DELTANG_L, -1, 32, 32)
+
+#define ADIS16475_DELTVEL_CHAN_NO_SCAN(_mod) \
+ ADIS16475_MOD_CHAN_DELTA(IIO_VELOCITY, IIO_MOD_ ## _mod, \
+ ADIS16475_REG_ ## _mod ## _DELTVEL_L, -1, 32, 32)
+
static const struct iio_chan_spec adis16475_channels[] = {
ADIS16475_GYRO_CHANNEL(X),
ADIS16475_GYRO_CHANNEL(Y),
@@ -559,7 +619,30 @@ static const struct iio_chan_spec adis16475_channels[] = {
ADIS16475_ACCEL_CHANNEL(Y),
ADIS16475_ACCEL_CHANNEL(Z),
ADIS16475_TEMP_CHANNEL(),
- IIO_CHAN_SOFT_TIMESTAMP(7)
+ IIO_CHAN_SOFT_TIMESTAMP(7),
+ ADIS16475_DELTANG_CHAN_NO_SCAN(X),
+ ADIS16475_DELTANG_CHAN_NO_SCAN(Y),
+ ADIS16475_DELTANG_CHAN_NO_SCAN(Z),
+ ADIS16475_DELTVEL_CHAN_NO_SCAN(X),
+ ADIS16475_DELTVEL_CHAN_NO_SCAN(Y),
+ ADIS16475_DELTVEL_CHAN_NO_SCAN(Z)
+};
+
+static const struct iio_chan_spec adis16500_channels[] = {
+ ADIS16475_GYRO_CHANNEL(X),
+ ADIS16475_GYRO_CHANNEL(Y),
+ ADIS16475_GYRO_CHANNEL(Z),
+ ADIS16475_ACCEL_CHANNEL(X),
+ ADIS16475_ACCEL_CHANNEL(Y),
+ ADIS16475_ACCEL_CHANNEL(Z),
+ ADIS16475_TEMP_CHANNEL(),
+ ADIS16475_DELTANG_CHAN(X),
+ ADIS16475_DELTANG_CHAN(Y),
+ ADIS16475_DELTANG_CHAN(Z),
+ ADIS16475_DELTVEL_CHAN(X),
+ ADIS16475_DELTVEL_CHAN(Y),
+ ADIS16475_DELTVEL_CHAN(Z),
+ IIO_CHAN_SOFT_TIMESTAMP(13)
};

enum adis16475_variant {
@@ -662,6 +745,8 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.accel_max_val = 1,
.accel_max_scale = IIO_M_S_2_TO_G(800 << 16),
.temp_scale = 100,
+ .deltang_max_val = 2160,
+ .deltvel_max_val = 400,
.int_clk = 2000,
.max_dec = 1999,
.sync = adis16475_sync_mode,
@@ -677,6 +762,8 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.accel_max_val = 1,
.accel_max_scale = IIO_M_S_2_TO_G(4000 << 16),
.temp_scale = 100,
+ .deltang_max_val = 360,
+ .deltvel_max_val = 100,
.int_clk = 2000,
.max_dec = 1999,
.sync = adis16475_sync_mode,
@@ -692,6 +779,8 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.accel_max_val = 1,
.accel_max_scale = IIO_M_S_2_TO_G(4000 << 16),
.temp_scale = 100,
+ .deltang_max_val = 720,
+ .deltvel_max_val = 100,
.int_clk = 2000,
.max_dec = 1999,
.sync = adis16475_sync_mode,
@@ -707,6 +796,8 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.accel_max_val = 1,
.accel_max_scale = IIO_M_S_2_TO_G(4000 << 16),
.temp_scale = 100,
+ .deltang_max_val = 2160,
+ .deltvel_max_val = 100,
.int_clk = 2000,
.max_dec = 1999,
.sync = adis16475_sync_mode,
@@ -715,13 +806,15 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
},
[ADIS16477_1] = {
.name = "adis16477-1",
- .num_channels = ARRAY_SIZE(adis16475_channels),
- .channels = adis16475_channels,
+ .num_channels = ARRAY_SIZE(adis16500_channels),
+ .channels = adis16500_channels,
.gyro_max_val = 1,
.gyro_max_scale = IIO_RAD_TO_DEGREE(160 << 16),
.accel_max_val = 1,
.accel_max_scale = IIO_M_S_2_TO_G(800 << 16),
.temp_scale = 100,
+ .deltang_max_val = 360,
+ .deltvel_max_val = 400,
.int_clk = 2000,
.max_dec = 1999,
.sync = adis16475_sync_mode,
@@ -731,13 +824,15 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
},
[ADIS16477_2] = {
.name = "adis16477-2",
- .num_channels = ARRAY_SIZE(adis16475_channels),
- .channels = adis16475_channels,
+ .num_channels = ARRAY_SIZE(adis16500_channels),
+ .channels = adis16500_channels,
.gyro_max_val = 1,
.gyro_max_scale = IIO_RAD_TO_DEGREE(40 << 16),
.accel_max_val = 1,
.accel_max_scale = IIO_M_S_2_TO_G(800 << 16),
.temp_scale = 100,
+ .deltang_max_val = 720,
+ .deltvel_max_val = 400,
.int_clk = 2000,
.max_dec = 1999,
.sync = adis16475_sync_mode,
@@ -747,13 +842,15 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
},
[ADIS16477_3] = {
.name = "adis16477-3",
- .num_channels = ARRAY_SIZE(adis16475_channels),
- .channels = adis16475_channels,
+ .num_channels = ARRAY_SIZE(adis16500_channels),
+ .channels = adis16500_channels,
.gyro_max_val = 1,
.gyro_max_scale = IIO_RAD_TO_DEGREE(10 << 16),
.accel_max_val = 1,
.accel_max_scale = IIO_M_S_2_TO_G(800 << 16),
.temp_scale = 100,
+ .deltang_max_val = 2160,
+ .deltvel_max_val = 400,
.int_clk = 2000,
.max_dec = 1999,
.sync = adis16475_sync_mode,
@@ -770,6 +867,8 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.accel_max_val = 1,
.accel_max_scale = IIO_M_S_2_TO_G(4000 << 16),
.temp_scale = 100,
+ .deltang_max_val = 360,
+ .deltvel_max_val = 100,
.int_clk = 2000,
.max_dec = 1999,
.sync = adis16475_sync_mode,
@@ -785,6 +884,8 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.accel_max_val = 1,
.accel_max_scale = IIO_M_S_2_TO_G(4000 << 16),
.temp_scale = 100,
+ .deltang_max_val = 720,
+ .deltvel_max_val = 100,
.int_clk = 2000,
.max_dec = 1999,
.sync = adis16475_sync_mode,
@@ -800,6 +901,8 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.accel_max_val = 1,
.accel_max_scale = IIO_M_S_2_TO_G(4000 << 16),
.temp_scale = 100,
+ .deltang_max_val = 2160,
+ .deltvel_max_val = 100,
.int_clk = 2000,
.max_dec = 1999,
.sync = adis16475_sync_mode,
@@ -815,6 +918,8 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.accel_max_val = 1,
.accel_max_scale = IIO_M_S_2_TO_G(800 << 16),
.temp_scale = 100,
+ .deltang_max_val = 360,
+ .deltvel_max_val = 400,
.int_clk = 2000,
.max_dec = 1999,
.sync = adis16475_sync_mode,
@@ -830,6 +935,8 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.accel_max_val = 1,
.accel_max_scale = IIO_M_S_2_TO_G(800 << 16),
.temp_scale = 100,
+ .deltang_max_val = 720,
+ .deltvel_max_val = 400,
.int_clk = 2000,
.max_dec = 1999,
.sync = adis16475_sync_mode,
@@ -845,6 +952,8 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
.accel_max_val = 1,
.accel_max_scale = IIO_M_S_2_TO_G(800 << 16),
.temp_scale = 100,
+ .deltang_max_val = 2160,
+ .deltvel_max_val = 400,
.int_clk = 2000,
.max_dec = 1999,
.sync = adis16475_sync_mode,
@@ -853,13 +962,15 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
},
[ADIS16500] = {
.name = "adis16500",
- .num_channels = ARRAY_SIZE(adis16475_channels),
- .channels = adis16475_channels,
+ .num_channels = ARRAY_SIZE(adis16500_channels),
+ .channels = adis16500_channels,
.gyro_max_val = 1,
.gyro_max_scale = IIO_RAD_TO_DEGREE(10 << 16),
.accel_max_val = 392,
.accel_max_scale = 32000 << 16,
.temp_scale = 100,
+ .deltang_max_val = 2160,
+ .deltvel_max_val = 400,
.int_clk = 2000,
.max_dec = 1999,
.sync = adis16475_sync_mode,
@@ -870,13 +981,15 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
},
[ADIS16505_1] = {
.name = "adis16505-1",
- .num_channels = ARRAY_SIZE(adis16475_channels),
- .channels = adis16475_channels,
+ .num_channels = ARRAY_SIZE(adis16500_channels),
+ .channels = adis16500_channels,
.gyro_max_val = 1,
.gyro_max_scale = IIO_RAD_TO_DEGREE(160 << 16),
.accel_max_val = 78,
.accel_max_scale = 32000 << 16,
.temp_scale = 100,
+ .deltang_max_val = 360,
+ .deltvel_max_val = 100,
.int_clk = 2000,
.max_dec = 1999,
.sync = adis16475_sync_mode,
@@ -887,13 +1000,15 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
},
[ADIS16505_2] = {
.name = "adis16505-2",
- .num_channels = ARRAY_SIZE(adis16475_channels),
- .channels = adis16475_channels,
+ .num_channels = ARRAY_SIZE(adis16500_channels),
+ .channels = adis16500_channels,
.gyro_max_val = 1,
.gyro_max_scale = IIO_RAD_TO_DEGREE(40 << 16),
.accel_max_val = 78,
.accel_max_scale = 32000 << 16,
.temp_scale = 100,
+ .deltang_max_val = 720,
+ .deltvel_max_val = 100,
.int_clk = 2000,
.max_dec = 1999,
.sync = adis16475_sync_mode,
@@ -904,13 +1019,15 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
},
[ADIS16505_3] = {
.name = "adis16505-3",
- .num_channels = ARRAY_SIZE(adis16475_channels),
- .channels = adis16475_channels,
+ .num_channels = ARRAY_SIZE(adis16500_channels),
+ .channels = adis16500_channels,
.gyro_max_val = 1,
.gyro_max_scale = IIO_RAD_TO_DEGREE(10 << 16),
.accel_max_val = 78,
.accel_max_scale = 32000 << 16,
.temp_scale = 100,
+ .deltang_max_val = 2160,
+ .deltvel_max_val = 100,
.int_clk = 2000,
.max_dec = 1999,
.sync = adis16475_sync_mode,
@@ -921,13 +1038,15 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
},
[ADIS16507_1] = {
.name = "adis16507-1",
- .num_channels = ARRAY_SIZE(adis16475_channels),
- .channels = adis16475_channels,
+ .num_channels = ARRAY_SIZE(adis16500_channels),
+ .channels = adis16500_channels,
.gyro_max_val = 1,
.gyro_max_scale = IIO_RAD_TO_DEGREE(160 << 16),
.accel_max_val = 392,
.accel_max_scale = 32000 << 16,
.temp_scale = 100,
+ .deltang_max_val = 360,
+ .deltvel_max_val = 400,
.int_clk = 2000,
.max_dec = 1999,
.sync = adis16475_sync_mode,
@@ -938,13 +1057,15 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
},
[ADIS16507_2] = {
.name = "adis16507-2",
- .num_channels = ARRAY_SIZE(adis16475_channels),
- .channels = adis16475_channels,
+ .num_channels = ARRAY_SIZE(adis16500_channels),
+ .channels = adis16500_channels,
.gyro_max_val = 1,
.gyro_max_scale = IIO_RAD_TO_DEGREE(40 << 16),
.accel_max_val = 392,
.accel_max_scale = 32000 << 16,
.temp_scale = 100,
+ .deltang_max_val = 720,
+ .deltvel_max_val = 400,
.int_clk = 2000,
.max_dec = 1999,
.sync = adis16475_sync_mode,
@@ -955,13 +1076,15 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
},
[ADIS16507_3] = {
.name = "adis16507-3",
- .num_channels = ARRAY_SIZE(adis16475_channels),
- .channels = adis16475_channels,
+ .num_channels = ARRAY_SIZE(adis16500_channels),
+ .channels = adis16500_channels,
.gyro_max_val = 1,
.gyro_max_scale = IIO_RAD_TO_DEGREE(10 << 16),
.accel_max_val = 392,
.accel_max_scale = 32000 << 16,
.temp_scale = 100,
+ .deltang_max_val = 2160,
+ .deltvel_max_val = 400,
.int_clk = 2000,
.max_dec = 1999,
.sync = adis16475_sync_mode,
@@ -972,10 +1095,40 @@ static const struct adis16475_chip_info adis16475_chip_info[] = {
},
};

+static const unsigned long adis16475_avail_scan_masks[] = {
+ ADIS16500_BURST_DATA_SEL_0_CHN_MASK,
+ ADIS16500_BURST_DATA_SEL_1_CHN_MASK,
+ 0
+};
+
+static int adis16475_update_scan_mode(struct iio_dev *indio_dev,
+ const unsigned long *scan_mask)
+{
+ u16 en;
+ int ret;
+ struct adis16475 *st = iio_priv(indio_dev);
+
+ if (st->info->has_burst32) {
+ if (*scan_mask & ADIS16500_BURST_DATA_SEL_0_CHN_MASK)
+ en = ADIS16500_BURST_DATA_SEL(0);
+ else if (*scan_mask & ADIS16500_BURST_DATA_SEL_1_CHN_MASK)
+ en = ADIS16500_BURST_DATA_SEL(1);
+ else
+ return -EINVAL;
+
+ ret = __adis_update_bits(&st->adis, ADIS16475_REG_MSG_CTRL,
+ ADIS16500_BURST_DATA_SEL_MASK, en);
+ if (ret)
+ return ret;
+ }
+
+ return adis_update_scan_mode(indio_dev, scan_mask);
+}
+
static const struct iio_info adis16475_info = {
.read_raw = &adis16475_read_raw,
.write_raw = &adis16475_write_raw,
- .update_scan_mode = adis_update_scan_mode,
+ .update_scan_mode = adis16475_update_scan_mode,
.debugfs_reg_access = adis_debugfs_reg_access,
};

@@ -1044,7 +1197,7 @@ static irqreturn_t adis16475_trigger_handler(int irq, void *p)
struct iio_dev *indio_dev = pf->indio_dev;
struct adis16475 *st = iio_priv(indio_dev);
struct adis *adis = &st->adis;
- int ret, bit, i = 0;
+ int ret, bit, buff_offset = 0, i = 0;
__be16 *buffer;
u16 crc;
bool valid;
@@ -1074,6 +1227,9 @@ static irqreturn_t adis16475_trigger_handler(int irq, void *p)
case ADIS16475_SCAN_TEMP:
st->data[i++] = buffer[offset];
break;
+ case ADIS16475_SCAN_DELTANG_X ... ADIS16475_SCAN_DELTVEL_Z:
+ buff_offset = ADIS16475_SCAN_DELTANG_X;
+ fallthrough;
case ADIS16475_SCAN_GYRO_X ... ADIS16475_SCAN_ACCEL_Z:
/*
* The first 2 bytes on the received data are the
@@ -1081,11 +1237,11 @@ static irqreturn_t adis16475_trigger_handler(int irq, void *p)
*/
if (st->burst32) {
/* upper 16 */
- st->data[i++] = buffer[bit * 2 + 2];
+ st->data[i++] = buffer[(bit - buff_offset) * 2 + 2];
/* lower 16 */
- st->data[i++] = buffer[bit * 2 + 1];
+ st->data[i++] = buffer[(bit - buff_offset) * 2 + 1];
} else {
- st->data[i++] = buffer[bit + 1];
+ st->data[i++] = buffer[(bit - buff_offset) + 1];
/*
* Don't bother in doing the manual read if the
* device supports burst32. burst32 will be
@@ -1314,6 +1470,8 @@ static int adis16475_probe(struct spi_device *spi)
indio_dev->num_channels = st->info->num_channels;
indio_dev->info = &adis16475_info;
indio_dev->modes = INDIO_DIRECT_MODE;
+ if (st->info->has_burst32)
+ indio_dev->available_scan_masks = adis16475_avail_scan_masks;

ret = __adis_initial_startup(&st->adis);
if (ret)
--
2.25.1


2023-07-20 07:44:50

by Nuno Sá

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] iio: imu: adis16475.c: Add has_burst32 flag to adis16477 devices

On Wed, 2023-07-19 at 15:31 +0300, Ramona Bolboaca wrote:
> adis16477 devices support burst32 function, thus has_burst32
> flag should be set to true.
>
> Signed-off-by: Ramona Bolboaca <[email protected]>
> ---

Ups...

Reviewed-by: Nuno Sa <[email protected]>

>  drivers/iio/imu/adis16475.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/iio/imu/adis16475.c b/drivers/iio/imu/adis16475.c
> index 243f0a91fdf9..17275a53ca2c 100644
> --- a/drivers/iio/imu/adis16475.c
> +++ b/drivers/iio/imu/adis16475.c
> @@ -726,6 +726,7 @@ static const struct adis16475_chip_info
> adis16475_chip_info[] = {
>                 .max_dec = 1999,
>                 .sync = adis16475_sync_mode,
>                 .num_sync = ARRAY_SIZE(adis16475_sync_mode),
> +               .has_burst32 = true,
>                 .adis_data = ADIS16475_DATA(16477, &adis16475_timeouts),
>         },
>         [ADIS16477_2] = {
> @@ -741,6 +742,7 @@ static const struct adis16475_chip_info
> adis16475_chip_info[] = {
>                 .max_dec = 1999,
>                 .sync = adis16475_sync_mode,
>                 .num_sync = ARRAY_SIZE(adis16475_sync_mode),
> +               .has_burst32 = true,
>                 .adis_data = ADIS16475_DATA(16477, &adis16475_timeouts),
>         },
>         [ADIS16477_3] = {
> @@ -756,6 +758,7 @@ static const struct adis16475_chip_info
> adis16475_chip_info[] = {
>                 .max_dec = 1999,
>                 .sync = adis16475_sync_mode,
>                 .num_sync = ARRAY_SIZE(adis16475_sync_mode),
> +               .has_burst32 = true,
>                 .adis_data = ADIS16475_DATA(16477, &adis16475_timeouts),
>         },
>         [ADIS16465_1] = {

2023-07-20 07:59:36

by Nuno Sá

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] iio: imu: adis16475.c: Remove unused enum elements

On Wed, 2023-07-19 at 15:31 +0300, Ramona Bolboaca wrote:
> Remove unused enum elements ADIS16475_SCAN_DIAG_S_FLAGS and
> ADIS16475_SCAN_CRC_FAILURE.
>
> Signed-off-by: Ramona Bolboaca <[email protected]>
> ---

Reviewed-by: Nuno Sa <[email protected]>

>  drivers/iio/imu/adis16475.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/iio/imu/adis16475.c b/drivers/iio/imu/adis16475.c
> index 3abffb01ba31..243f0a91fdf9 100644
> --- a/drivers/iio/imu/adis16475.c
> +++ b/drivers/iio/imu/adis16475.c
> @@ -115,8 +115,6 @@ enum {
>         ADIS16475_SCAN_ACCEL_Y,
>         ADIS16475_SCAN_ACCEL_Z,
>         ADIS16475_SCAN_TEMP,
> -       ADIS16475_SCAN_DIAG_S_FLAGS,
> -       ADIS16475_SCAN_CRC_FAILURE,
>  };
>  
>  static bool low_rate_allow;


2023-07-20 09:42:14

by Nuno Sá

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] iio: imu: adis16475.c: Add delta angle and delta velocity channels

On Wed, 2023-07-19 at 15:31 +0300, Ramona Bolboaca wrote:
> Add support for delta angle and delta velocity raw and buffer
> readings to adis16475 driver.
>
> Signed-off-by: Ramona Bolboaca <[email protected]>
> ---
>  drivers/iio/imu/adis16475.c | 210 +++++++++++++++++++++++++++++++-----
>  1 file changed, 184 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/iio/imu/adis16475.c b/drivers/iio/imu/adis16475.c
> index 17275a53ca2c..e92b42974ee6 100644
> --- a/drivers/iio/imu/adis16475.c
> +++ b/drivers/iio/imu/adis16475.c
> @@ -31,6 +31,12 @@
>  #define ADIS16475_REG_Y_ACCEL_L                0x14
>  #define ADIS16475_REG_Z_ACCEL_L                0x18
>  #define ADIS16475_REG_TEMP_OUT         0x1c
> +#define ADIS16475_REG_X_DELTANG_L      0x24
> +#define ADIS16475_REG_Y_DELTANG_L      0x28
> +#define ADIS16475_REG_Z_DELTANG_L      0x2C
> +#define ADIS16475_REG_X_DELTVEL_L      0x30
> +#define ADIS16475_REG_Y_DELTVEL_L      0x34
> +#define ADIS16475_REG_Z_DELTVEL_L      0x38
>  #define ADIS16475_REG_X_GYRO_BIAS_L    0x40
>  #define ADIS16475_REG_Y_GYRO_BIAS_L    0x44
>  #define ADIS16475_REG_Z_GYRO_BIAS_L    0x48
> @@ -55,6 +61,8 @@
>  #define ADIS16475_REG_PROD_ID          0x72
>  #define ADIS16475_REG_SERIAL_NUM       0x74
>  #define ADIS16475_REG_FLASH_CNT                0x7c
> +#define ADIS16500_BURST_DATA_SEL_MASK  BIT(8)
> +#define
> ADIS16500_BURST_DATA_SEL(x)    FIELD_PREP(ADIS16500_BURST_DATA_SEL_MASK, x)
>  #define ADIS16500_BURST32_MASK         BIT(9)
>  #define ADIS16500_BURST32(x)           FIELD_PREP(ADIS16500_BURST32_MASK, x)
>  /* number of data elements in burst mode */
> @@ -65,6 +73,8 @@
>  #define ADIS16475_BURST_MAX_SPEED      1000000
>  #define ADIS16475_LSB_DEC_MASK         BIT(0)
>  #define ADIS16475_LSB_FIR_MASK         BIT(1)
> +#define ADIS16500_BURST_DATA_SEL_0_CHN_MASK    (GENMASK(6, 0) | BIT(13))
> +#define ADIS16500_BURST_DATA_SEL_1_CHN_MASK    GENMASK(13, 6)
>  
>  enum {
>         ADIS16475_SYNC_DIRECT = 1,
> @@ -90,6 +100,8 @@ struct adis16475_chip_info {
>         u32 accel_max_val;
>         u32 accel_max_scale;
>         u32 temp_scale;
> +       u32 deltang_max_val;
> +       u32 deltvel_max_val;
>         u32 int_clk;
>         u16 max_dec;
>         u8 num_sync;
> @@ -115,6 +127,12 @@ enum {
>         ADIS16475_SCAN_ACCEL_Y,
>         ADIS16475_SCAN_ACCEL_Z,
>         ADIS16475_SCAN_TEMP,
> +       ADIS16475_SCAN_DELTANG_X,
> +       ADIS16475_SCAN_DELTANG_Y,
> +       ADIS16475_SCAN_DELTANG_Z,
> +       ADIS16475_SCAN_DELTVEL_X,
> +       ADIS16475_SCAN_DELTVEL_Y,
> +       ADIS16475_SCAN_DELTVEL_Z,
>  };
>  
>  static bool low_rate_allow;
> @@ -451,6 +469,14 @@ static int adis16475_read_raw(struct iio_dev *indio_dev,
>                 case IIO_TEMP:
>                         *val = st->info->temp_scale;
>                         return IIO_VAL_INT;
> +               case IIO_ROT:
> +                       *val = st->info->deltang_max_val;
> +                       *val2 = 31;
> +                       return IIO_VAL_FRACTIONAL_LOG2;
> +               case IIO_VELOCITY:
> +                       *val = st->info->deltvel_max_val;
> +                       *val2 = 31;
> +                       return IIO_VAL_FRACTIONAL_LOG2;
>                 default:
>                         return -EINVAL;
>                 }
> @@ -551,6 +577,40 @@ static int adis16475_write_raw(struct iio_dev *indio_dev,
>                 }, \
>         }
>  
> +#define ADIS16475_MOD_CHAN_DELTA(_type, _mod, _address, _si, _r_bits,
> _s_bits) { \
> +               .type = (_type), \
> +               .modified = 1, \
> +               .channel2 = (_mod), \
> +               .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> +               .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> +               .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
> +                       BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY), \
> +               .address = (_address), \
> +               .scan_index = _si, \
> +               .scan_type = { \
> +                       .sign = 's', \
> +                       .realbits = (_r_bits), \
> +                       .storagebits = (_s_bits), \
> +                       .endianness = IIO_BE, \
> +               }, \
> +       }
> +
> +#define ADIS16475_DELTANG_CHAN(_mod) \
> +       ADIS16475_MOD_CHAN_DELTA(IIO_ROT, IIO_MOD_ ## _mod, \
> +                          ADIS16475_REG_ ## _mod ## _DELTANG_L,
> ADIS16475_SCAN_DELTANG_ ## _mod, 32, 32)
> +
> +#define ADIS16475_DELTVEL_CHAN(_mod) \
> +       ADIS16475_MOD_CHAN_DELTA(IIO_VELOCITY, IIO_MOD_ ## _mod, \
> +                          ADIS16475_REG_ ## _mod ## _DELTVEL_L,
> ADIS16475_SCAN_DELTVEL_ ## _mod, 32, 32)
> +
> +#define ADIS16475_DELTANG_CHAN_NO_SCAN(_mod) \
> +       ADIS16475_MOD_CHAN_DELTA(IIO_ROT, IIO_MOD_ ## _mod, \
> +                          ADIS16475_REG_ ## _mod ## _DELTANG_L, -1, 32, 32)
> +
> +#define ADIS16475_DELTVEL_CHAN_NO_SCAN(_mod) \
> +       ADIS16475_MOD_CHAN_DELTA(IIO_VELOCITY, IIO_MOD_ ## _mod, \
> +                          ADIS16475_REG_ ## _mod ## _DELTVEL_L, -1, 32, 32)
> +
>  static const struct iio_chan_spec adis16475_channels[] = {
>         ADIS16475_GYRO_CHANNEL(X),
>         ADIS16475_GYRO_CHANNEL(Y),
> @@ -559,7 +619,30 @@ static const struct iio_chan_spec adis16475_channels[] =
> {
>         ADIS16475_ACCEL_CHANNEL(Y),
>         ADIS16475_ACCEL_CHANNEL(Z),
>         ADIS16475_TEMP_CHANNEL(),
> -       IIO_CHAN_SOFT_TIMESTAMP(7)
> +       IIO_CHAN_SOFT_TIMESTAMP(7),
> +       ADIS16475_DELTANG_CHAN_NO_SCAN(X),
> +       ADIS16475_DELTANG_CHAN_NO_SCAN(Y),
> +       ADIS16475_DELTANG_CHAN_NO_SCAN(Z),
> +       ADIS16475_DELTVEL_CHAN_NO_SCAN(X),
> +       ADIS16475_DELTVEL_CHAN_NO_SCAN(Y),
> +       ADIS16475_DELTVEL_CHAN_NO_SCAN(Z)
> +};
> +
> +static const struct iio_chan_spec adis16500_channels[] = {
> +       ADIS16475_GYRO_CHANNEL(X),
> +       ADIS16475_GYRO_CHANNEL(Y),
> +       ADIS16475_GYRO_CHANNEL(Z),
> +       ADIS16475_ACCEL_CHANNEL(X),
> +       ADIS16475_ACCEL_CHANNEL(Y),
> +       ADIS16475_ACCEL_CHANNEL(Z),
> +       ADIS16475_TEMP_CHANNEL(),
> +       ADIS16475_DELTANG_CHAN(X),
> +       ADIS16475_DELTANG_CHAN(Y),
> +       ADIS16475_DELTANG_CHAN(Z),
> +       ADIS16475_DELTVEL_CHAN(X),
> +       ADIS16475_DELTVEL_CHAN(Y),
> +       ADIS16475_DELTVEL_CHAN(Z),
> +       IIO_CHAN_SOFT_TIMESTAMP(13)
>  };

Hmm, IIRC I suggested this to you so apologizes. But I'm not sure we should do
this. In theory, I think we provide all the interfaces so an userspace app would
still work by changing the timestamp index but who knows what apps might be
assuming... So maybe, it's just safer to keep timestamp at index 7.

I guess it will also make the available scan_masks neater.

>  
>  enum adis16475_variant {
> @@ -662,6 +745,8 @@ static const struct adis16475_chip_info
> adis16475_chip_info[] = {
>                 .accel_max_val = 1,
>                 .accel_max_scale = IIO_M_S_2_TO_G(800 << 16),
>                 .temp_scale = 100,
> +               .deltang_max_val = 2160,
> +               .deltvel_max_val = 400,
>                 .int_clk = 2000,
>                 .max_dec = 1999,
>                 .sync = adis16475_sync_mode,
> @@ -677,6 +762,8 @@ static const struct adis16475_chip_info
> adis16475_chip_info[] = {
>                 .accel_max_val = 1,
>                 .accel_max_scale = IIO_M_S_2_TO_G(4000 << 16),
>                 .temp_scale = 100,
> +               .deltang_max_val = 360,
> +               .deltvel_max_val = 100,
>                 .int_clk = 2000,
>                 .max_dec = 1999,
>                 .sync = adis16475_sync_mode,
> @@ -692,6 +779,8 @@ static const struct adis16475_chip_info
> adis16475_chip_info[] = {
>                 .accel_max_val = 1,
>                 .accel_max_scale = IIO_M_S_2_TO_G(4000 << 16),
>                 .temp_scale = 100,
> +               .deltang_max_val = 720,
> +               .deltvel_max_val = 100,
>                 .int_clk = 2000,
>                 .max_dec = 1999,
>                 .sync = adis16475_sync_mode,
> @@ -707,6 +796,8 @@ static const struct adis16475_chip_info
> adis16475_chip_info[] = {
>                 .accel_max_val = 1,
>                 .accel_max_scale = IIO_M_S_2_TO_G(4000 << 16),
>                 .temp_scale = 100,
> +               .deltang_max_val = 2160,
> +               .deltvel_max_val = 100,
>                 .int_clk = 2000,
>                 .max_dec = 1999,
>                 .sync = adis16475_sync_mode,
> @@ -715,13 +806,15 @@ static const struct adis16475_chip_info
> adis16475_chip_info[] = {
>         },
>         [ADIS16477_1] = {
>                 .name = "adis16477-1",
> -               .num_channels = ARRAY_SIZE(adis16475_channels),
> -               .channels = adis16475_channels,
> +               .num_channels = ARRAY_SIZE(adis16500_channels),
> +               .channels = adis16500_channels,
>                 .gyro_max_val = 1,
>                 .gyro_max_scale = IIO_RAD_TO_DEGREE(160 << 16),
>                 .accel_max_val = 1,
>                 .accel_max_scale = IIO_M_S_2_TO_G(800 << 16),
>                 .temp_scale = 100,
> +               .deltang_max_val = 360,
> +               .deltvel_max_val = 400,
>                 .int_clk = 2000,
>                 .max_dec = 1999,
>                 .sync = adis16475_sync_mode,
> @@ -731,13 +824,15 @@ static const struct adis16475_chip_info
> adis16475_chip_info[] = {
>         },
>         [ADIS16477_2] = {
>                 .name = "adis16477-2",
> -               .num_channels = ARRAY_SIZE(adis16475_channels),
> -               .channels = adis16475_channels,
> +               .num_channels = ARRAY_SIZE(adis16500_channels),
> +               .channels = adis16500_channels,
>                 .gyro_max_val = 1,
>                 .gyro_max_scale = IIO_RAD_TO_DEGREE(40 << 16),
>                 .accel_max_val = 1,
>                 .accel_max_scale = IIO_M_S_2_TO_G(800 << 16),
>                 .temp_scale = 100,
> +               .deltang_max_val = 720,
> +               .deltvel_max_val = 400,
>                 .int_clk = 2000,
>                 .max_dec = 1999,
>                 .sync = adis16475_sync_mode,
> @@ -747,13 +842,15 @@ static const struct adis16475_chip_info
> adis16475_chip_info[] = {
>         },
>         [ADIS16477_3] = {
>                 .name = "adis16477-3",
> -               .num_channels = ARRAY_SIZE(adis16475_channels),
> -               .channels = adis16475_channels,
> +               .num_channels = ARRAY_SIZE(adis16500_channels),
> +               .channels = adis16500_channels,
>                 .gyro_max_val = 1,
>                 .gyro_max_scale = IIO_RAD_TO_DEGREE(10 << 16),
>                 .accel_max_val = 1,
>                 .accel_max_scale = IIO_M_S_2_TO_G(800 << 16),
>                 .temp_scale = 100,
> +               .deltang_max_val = 2160,
> +               .deltvel_max_val = 400,
>                 .int_clk = 2000,
>                 .max_dec = 1999,
>                 .sync = adis16475_sync_mode,
> @@ -770,6 +867,8 @@ static const struct adis16475_chip_info
> adis16475_chip_info[] = {
>                 .accel_max_val = 1,
>                 .accel_max_scale = IIO_M_S_2_TO_G(4000 << 16),
>                 .temp_scale = 100,
> +               .deltang_max_val = 360,
> +               .deltvel_max_val = 100,
>                 .int_clk = 2000,
>                 .max_dec = 1999,
>                 .sync = adis16475_sync_mode,
> @@ -785,6 +884,8 @@ static const struct adis16475_chip_info
> adis16475_chip_info[] = {
>                 .accel_max_val = 1,
>                 .accel_max_scale = IIO_M_S_2_TO_G(4000 << 16),
>                 .temp_scale = 100,
> +               .deltang_max_val = 720,
> +               .deltvel_max_val = 100,
>                 .int_clk = 2000,
>                 .max_dec = 1999,
>                 .sync = adis16475_sync_mode,
> @@ -800,6 +901,8 @@ static const struct adis16475_chip_info
> adis16475_chip_info[] = {
>                 .accel_max_val = 1,
>                 .accel_max_scale = IIO_M_S_2_TO_G(4000 << 16),
>                 .temp_scale = 100,
> +               .deltang_max_val = 2160,
> +               .deltvel_max_val = 100,
>                 .int_clk = 2000,
>                 .max_dec = 1999,
>                 .sync = adis16475_sync_mode,
> @@ -815,6 +918,8 @@ static const struct adis16475_chip_info
> adis16475_chip_info[] = {
>                 .accel_max_val = 1,
>                 .accel_max_scale = IIO_M_S_2_TO_G(800 << 16),
>                 .temp_scale = 100,
> +               .deltang_max_val = 360,
> +               .deltvel_max_val = 400,
>                 .int_clk = 2000,
>                 .max_dec = 1999,
>                 .sync = adis16475_sync_mode,
> @@ -830,6 +935,8 @@ static const struct adis16475_chip_info
> adis16475_chip_info[] = {
>                 .accel_max_val = 1,
>                 .accel_max_scale = IIO_M_S_2_TO_G(800 << 16),
>                 .temp_scale = 100,
> +               .deltang_max_val = 720,
> +               .deltvel_max_val = 400,
>                 .int_clk = 2000,
>                 .max_dec = 1999,
>                 .sync = adis16475_sync_mode,
> @@ -845,6 +952,8 @@ static const struct adis16475_chip_info
> adis16475_chip_info[] = {
>                 .accel_max_val = 1,
>                 .accel_max_scale = IIO_M_S_2_TO_G(800 << 16),
>                 .temp_scale = 100,
> +               .deltang_max_val = 2160,
> +               .deltvel_max_val = 400,
>                 .int_clk = 2000,
>                 .max_dec = 1999,
>                 .sync = adis16475_sync_mode,
> @@ -853,13 +962,15 @@ static const struct adis16475_chip_info
> adis16475_chip_info[] = {
>         },
>         [ADIS16500] = {
>                 .name = "adis16500",
> -               .num_channels = ARRAY_SIZE(adis16475_channels),
> -               .channels = adis16475_channels,
> +               .num_channels = ARRAY_SIZE(adis16500_channels),
> +               .channels = adis16500_channels,
>                 .gyro_max_val = 1,
>                 .gyro_max_scale = IIO_RAD_TO_DEGREE(10 << 16),
>                 .accel_max_val = 392,
>                 .accel_max_scale = 32000 << 16,
>                 .temp_scale = 100,
> +               .deltang_max_val = 2160,
> +               .deltvel_max_val = 400,
>                 .int_clk = 2000,
>                 .max_dec = 1999,
>                 .sync = adis16475_sync_mode,
> @@ -870,13 +981,15 @@ static const struct adis16475_chip_info
> adis16475_chip_info[] = {
>         },
>         [ADIS16505_1] = {
>                 .name = "adis16505-1",
> -               .num_channels = ARRAY_SIZE(adis16475_channels),
> -               .channels = adis16475_channels,
> +               .num_channels = ARRAY_SIZE(adis16500_channels),
> +               .channels = adis16500_channels,
>                 .gyro_max_val = 1,
>                 .gyro_max_scale = IIO_RAD_TO_DEGREE(160 << 16),
>                 .accel_max_val = 78,
>                 .accel_max_scale = 32000 << 16,
>                 .temp_scale = 100,
> +               .deltang_max_val = 360,
> +               .deltvel_max_val = 100,
>                 .int_clk = 2000,
>                 .max_dec = 1999,
>                 .sync = adis16475_sync_mode,
> @@ -887,13 +1000,15 @@ static const struct adis16475_chip_info
> adis16475_chip_info[] = {
>         },
>         [ADIS16505_2] = {
>                 .name = "adis16505-2",
> -               .num_channels = ARRAY_SIZE(adis16475_channels),
> -               .channels = adis16475_channels,
> +               .num_channels = ARRAY_SIZE(adis16500_channels),
> +               .channels = adis16500_channels,
>                 .gyro_max_val = 1,
>                 .gyro_max_scale = IIO_RAD_TO_DEGREE(40 << 16),
>                 .accel_max_val = 78,
>                 .accel_max_scale = 32000 << 16,
>                 .temp_scale = 100,
> +               .deltang_max_val = 720,
> +               .deltvel_max_val = 100,
>                 .int_clk = 2000,
>                 .max_dec = 1999,
>                 .sync = adis16475_sync_mode,
> @@ -904,13 +1019,15 @@ static const struct adis16475_chip_info
> adis16475_chip_info[] = {
>         },
>         [ADIS16505_3] = {
>                 .name = "adis16505-3",
> -               .num_channels = ARRAY_SIZE(adis16475_channels),
> -               .channels = adis16475_channels,
> +               .num_channels = ARRAY_SIZE(adis16500_channels),
> +               .channels = adis16500_channels,
>                 .gyro_max_val = 1,
>                 .gyro_max_scale = IIO_RAD_TO_DEGREE(10 << 16),
>                 .accel_max_val = 78,
>                 .accel_max_scale = 32000 << 16,
>                 .temp_scale = 100,
> +               .deltang_max_val = 2160,
> +               .deltvel_max_val = 100,
>                 .int_clk = 2000,
>                 .max_dec = 1999,
>                 .sync = adis16475_sync_mode,
> @@ -921,13 +1038,15 @@ static const struct adis16475_chip_info
> adis16475_chip_info[] = {
>         },
>         [ADIS16507_1] = {
>                 .name = "adis16507-1",
> -               .num_channels = ARRAY_SIZE(adis16475_channels),
> -               .channels = adis16475_channels,
> +               .num_channels = ARRAY_SIZE(adis16500_channels),
> +               .channels = adis16500_channels,
>                 .gyro_max_val = 1,
>                 .gyro_max_scale = IIO_RAD_TO_DEGREE(160 << 16),
>                 .accel_max_val = 392,
>                 .accel_max_scale = 32000 << 16,
>                 .temp_scale = 100,
> +               .deltang_max_val = 360,
> +               .deltvel_max_val = 400,
>                 .int_clk = 2000,
>                 .max_dec = 1999,
>                 .sync = adis16475_sync_mode,
> @@ -938,13 +1057,15 @@ static const struct adis16475_chip_info
> adis16475_chip_info[] = {
>         },
>         [ADIS16507_2] = {
>                 .name = "adis16507-2",
> -               .num_channels = ARRAY_SIZE(adis16475_channels),
> -               .channels = adis16475_channels,
> +               .num_channels = ARRAY_SIZE(adis16500_channels),
> +               .channels = adis16500_channels,
>                 .gyro_max_val = 1,
>                 .gyro_max_scale = IIO_RAD_TO_DEGREE(40 << 16),
>                 .accel_max_val = 392,
>                 .accel_max_scale = 32000 << 16,
>                 .temp_scale = 100,
> +               .deltang_max_val = 720,
> +               .deltvel_max_val = 400,
>                 .int_clk = 2000,
>                 .max_dec = 1999,
>                 .sync = adis16475_sync_mode,
> @@ -955,13 +1076,15 @@ static const struct adis16475_chip_info
> adis16475_chip_info[] = {
>         },
>         [ADIS16507_3] = {
>                 .name = "adis16507-3",
> -               .num_channels = ARRAY_SIZE(adis16475_channels),
> -               .channels = adis16475_channels,
> +               .num_channels = ARRAY_SIZE(adis16500_channels),
> +               .channels = adis16500_channels,
>                 .gyro_max_val = 1,
>                 .gyro_max_scale = IIO_RAD_TO_DEGREE(10 << 16),
>                 .accel_max_val = 392,
>                 .accel_max_scale = 32000 << 16,
>                 .temp_scale = 100,
> +               .deltang_max_val = 2160,
> +               .deltvel_max_val = 400,
>                 .int_clk = 2000,
>                 .max_dec = 1999,
>                 .sync = adis16475_sync_mode,
> @@ -972,10 +1095,40 @@ static const struct adis16475_chip_info
> adis16475_chip_info[] = {
>         },
>  };
>  
> +static const unsigned long adis16475_avail_scan_masks[] = {
> +       ADIS16500_BURST_DATA_SEL_0_CHN_MASK,
> +       ADIS16500_BURST_DATA_SEL_1_CHN_MASK,
> +       0
> +};
> +

Hmm I guess this ok if we don't have strict scan_mask match. AFAICT, that's only
the case for HARDWARE buffering and not likely to change... right? Jonathan?

> +static int adis16475_update_scan_mode(struct iio_dev *indio_dev,
> +                                     const unsigned long *scan_mask)
> +{
> +       u16 en;
> +       int ret;
> +       struct adis16475 *st = iio_priv(indio_dev);
> +
> +       if (st->info->has_burst32) {

Hmm, I get that buffering these channels are only available in devices that have
burst32. But I would not assume that as there's no guarantee that will always be
the case. Given that's straight to just have another flag for the presence of
these channels in the burst data, I would prefer to have one right now.

As a nit I would flip the logic in here:

if (!st->info->has_burst32)
return adis_update_scan_mode(indio_dev, scan_mask);

Then you could keep indentation and even save some lines of code. Anyways, just
my personal preference.

> +               if (*scan_mask & ADIS16500_BURST_DATA_SEL_0_CHN_MASK)
> +                       en = ADIS16500_BURST_DATA_SEL(0);
> +               else if (*scan_mask & ADIS16500_BURST_DATA_SEL_1_CHN_MASK)
> +                       en = ADIS16500_BURST_DATA_SEL(1);
> +               else
> +                       return -EINVAL;
> +

I don't think the above is ever possible since you're guaranteed that a subset
of the available_scan_masks is set. So, if() .. else should be enough.

Moreover will this actually work with BIT(13) or BIT(6) set? Won't we always
fall in the first case if one of those bits are set even if we want
ADIS16500_BURST_DATA_SEL(1)? So I guess you should explicitly check against
GENMASK(5, 0) for normal accel/gyro or GENMASK(13, 8) for delta stuff. Right?

- Nuno Sá

2023-07-23 13:04:18

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] iio: imu: adis16475.c: Remove unused enum elements

On Thu, 20 Jul 2023 09:38:50 +0200
Nuno Sá <[email protected]> wrote:

> On Wed, 2023-07-19 at 15:31 +0300, Ramona Bolboaca wrote:
> > Remove unused enum elements ADIS16475_SCAN_DIAG_S_FLAGS and
> > ADIS16475_SCAN_CRC_FAILURE.
> >
> > Signed-off-by: Ramona Bolboaca <[email protected]>
> > ---
>
> Reviewed-by: Nuno Sa <[email protected]>
Applied to the togreg branch of iio.git and pushed out as testing for the
autobuilders to poke at it.

Thanks,

Jonathan

>
> >  drivers/iio/imu/adis16475.c | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/drivers/iio/imu/adis16475.c b/drivers/iio/imu/adis16475.c
> > index 3abffb01ba31..243f0a91fdf9 100644
> > --- a/drivers/iio/imu/adis16475.c
> > +++ b/drivers/iio/imu/adis16475.c
> > @@ -115,8 +115,6 @@ enum {
> >         ADIS16475_SCAN_ACCEL_Y,
> >         ADIS16475_SCAN_ACCEL_Z,
> >         ADIS16475_SCAN_TEMP,
> > -       ADIS16475_SCAN_DIAG_S_FLAGS,
> > -       ADIS16475_SCAN_CRC_FAILURE,
> >  };
> >  
> >  static bool low_rate_allow;
>


2023-07-23 13:19:51

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] iio: imu: adis16475.c: Add delta angle and delta velocity channels

On Wed, 19 Jul 2023 15:31:52 +0300
Ramona Bolboaca <[email protected]> wrote:

> Add support for delta angle and delta velocity raw and buffer
> readings to adis16475 driver.
>
> Signed-off-by: Ramona Bolboaca <[email protected]>
As noted in my reply to Nuno's review, the question of channel type
is still unresolved.

https://lore.kernel.org/all/20230720193457.272f02a9@jic23-huawei/

Thanks,

Jonathan

2023-07-23 13:22:10

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] iio: imu: adis16475.c: Add delta angle and delta velocity channels

On Thu, 20 Jul 2023 10:58:51 +0200
Nuno Sá <[email protected]> wrote:

> On Wed, 2023-07-19 at 15:31 +0300, Ramona Bolboaca wrote:
> > Add support for delta angle and delta velocity raw and buffer
> > readings to adis16475 driver.
> >
> > Signed-off-by: Ramona Bolboaca <[email protected]>
> > ---
Some follow up comments..

> > +
> > +#define ADIS16475_DELTANG_CHAN(_mod) \
> > +       ADIS16475_MOD_CHAN_DELTA(IIO_ROT, IIO_MOD_ ## _mod, \
> > +                          ADIS16475_REG_ ## _mod ## _DELTANG_L,
> > ADIS16475_SCAN_DELTANG_ ## _mod, 32, 32)
> > +
> > +#define ADIS16475_DELTVEL_CHAN(_mod) \
> > +       ADIS16475_MOD_CHAN_DELTA(IIO_VELOCITY, IIO_MOD_ ## _mod, \
> > +                          ADIS16475_REG_ ## _mod ## _DELTVEL_L,
> > ADIS16475_SCAN_DELTVEL_ ## _mod, 32, 32)
> > +
> > +#define ADIS16475_DELTANG_CHAN_NO_SCAN(_mod) \
> > +       ADIS16475_MOD_CHAN_DELTA(IIO_ROT, IIO_MOD_ ## _mod, \
> > +                          ADIS16475_REG_ ## _mod ## _DELTANG_L, -1, 32, 32)
> > +
> > +#define ADIS16475_DELTVEL_CHAN_NO_SCAN(_mod) \
> > +       ADIS16475_MOD_CHAN_DELTA(IIO_VELOCITY, IIO_MOD_ ## _mod, \
> > +                          ADIS16475_REG_ ## _mod ## _DELTVEL_L, -1, 32, 32)

I don't think we've come to a conclusion on whether IIO_ROT and IIO_VELOCITY
are valid descriptions of these channels..

> > +
> >  static const struct iio_chan_spec adis16475_channels[] = {
> >         ADIS16475_GYRO_CHANNEL(X),
> >         ADIS16475_GYRO_CHANNEL(Y),
> > @@ -559,7 +619,30 @@ static const struct iio_chan_spec adis16475_channels[] =
> > {
> >         ADIS16475_ACCEL_CHANNEL(Y),
> >         ADIS16475_ACCEL_CHANNEL(Z),
> >         ADIS16475_TEMP_CHANNEL(),
> > -       IIO_CHAN_SOFT_TIMESTAMP(7)
> > +       IIO_CHAN_SOFT_TIMESTAMP(7),
> > +       ADIS16475_DELTANG_CHAN_NO_SCAN(X),
> > +       ADIS16475_DELTANG_CHAN_NO_SCAN(Y),
> > +       ADIS16475_DELTANG_CHAN_NO_SCAN(Z),
> > +       ADIS16475_DELTVEL_CHAN_NO_SCAN(X),
> > +       ADIS16475_DELTVEL_CHAN_NO_SCAN(Y),
> > +       ADIS16475_DELTVEL_CHAN_NO_SCAN(Z)
> > +};
> > +
> > +static const struct iio_chan_spec adis16500_channels[] = {
> > +       ADIS16475_GYRO_CHANNEL(X),
> > +       ADIS16475_GYRO_CHANNEL(Y),
> > +       ADIS16475_GYRO_CHANNEL(Z),
> > +       ADIS16475_ACCEL_CHANNEL(X),
> > +       ADIS16475_ACCEL_CHANNEL(Y),
> > +       ADIS16475_ACCEL_CHANNEL(Z),
> > +       ADIS16475_TEMP_CHANNEL(),
> > +       ADIS16475_DELTANG_CHAN(X),
> > +       ADIS16475_DELTANG_CHAN(Y),
> > +       ADIS16475_DELTANG_CHAN(Z),
> > +       ADIS16475_DELTVEL_CHAN(X),
> > +       ADIS16475_DELTVEL_CHAN(Y),
> > +       ADIS16475_DELTVEL_CHAN(Z),
> > +       IIO_CHAN_SOFT_TIMESTAMP(13)
> >  };
>
> Hmm, IIRC I suggested this to you so apologizes. But I'm not sure we should do
> this. In theory, I think we provide all the interfaces so an userspace app would
> still work by changing the timestamp index but who knows what apps might be
> assuming... So maybe, it's just safer to keep timestamp at index 7.

That stops you using iio_push_to_buffers_with_timestamp()

This is in the category of cross our fingers no one notices the ABI change.
So I'd go ahead with moving the timestamp channel and hope no one has hand rolled
code that assume the channel won't change index.

>
> I guess it will also make the available scan_masks neater.
>

...


> > @@ -972,10 +1095,40 @@ static const struct adis16475_chip_info
> > adis16475_chip_info[] = {
> >         },
> >  };
> >  
> > +static const unsigned long adis16475_avail_scan_masks[] = {
> > +       ADIS16500_BURST_DATA_SEL_0_CHN_MASK,
> > +       ADIS16500_BURST_DATA_SEL_1_CHN_MASK,
> > +       0
> > +};
> > +
>
> Hmm I guess this ok if we don't have strict scan_mask match. AFAICT, that's only
> the case for HARDWARE buffering and not likely to change... right? Jonathan?

Should be. The core will deal with demuxing the data otherwise.

>
> > +static int adis16475_update_scan_mode(struct iio_dev *indio_dev,
> > +                                     const unsigned long *scan_mask)
> > +{
> > +       u16 en;
> > +       int ret;
> > +       struct adis16475 *st = iio_priv(indio_dev);
> > +
> > +       if (st->info->has_burst32) {
>
> Hmm, I get that buffering these channels are only available in devices that have
> burst32. But I would not assume that as there's no guarantee that will always be
> the case. Given that's straight to just have another flag for the presence of
> these channels in the burst data, I would prefer to have one right now.

Agreed.

>
> As a nit I would flip the logic in here:
>
> if (!st->info->has_burst32)
> return adis_update_scan_mode(indio_dev, scan_mask);
>
> Then you could keep indentation and even save some lines of code. Anyways, just
> my personal preference.
>
> > +               if (*scan_mask & ADIS16500_BURST_DATA_SEL_0_CHN_MASK)
> > +                       en = ADIS16500_BURST_DATA_SEL(0);
> > +               else if (*scan_mask & ADIS16500_BURST_DATA_SEL_1_CHN_MASK)
> > +                       en = ADIS16500_BURST_DATA_SEL(1);
> > +               else
> > +                       return -EINVAL;
> > +
>
> I don't think the above is ever possible since you're guaranteed that a subset
> of the available_scan_masks is set. So, if() .. else should be enough.

I'm not 100% sure you can't just have a software timestamp even with available_scan_masks.
Definitely worth checking if you are going to drop the else.

>
> Moreover will this actually work with BIT(13) or BIT(6) set? Won't we always
> fall in the first case if one of those bits are set even if we want
> ADIS16500_BURST_DATA_SEL(1)? So I guess you should explicitly check against
> GENMASK(5, 0) for normal accel/gyro or GENMASK(13, 8) for delta stuff. Right?
>
> - Nuno Sá
>


2023-07-23 13:23:50

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] iio: imu: adis16475.c: Add has_burst32 flag to adis16477 devices

On Thu, 20 Jul 2023 09:39:27 +0200
Nuno Sá <[email protected]> wrote:

> On Wed, 2023-07-19 at 15:31 +0300, Ramona Bolboaca wrote:
> > adis16477 devices support burst32 function, thus has_burst32
> > flag should be set to true.
> >
> > Signed-off-by: Ramona Bolboaca <[email protected]>
> > ---
>
> Ups...
>
> Reviewed-by: Nuno Sa <[email protected]>
Applied.

Thanks,

Jonathan

>
> >  drivers/iio/imu/adis16475.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/iio/imu/adis16475.c b/drivers/iio/imu/adis16475.c
> > index 243f0a91fdf9..17275a53ca2c 100644
> > --- a/drivers/iio/imu/adis16475.c
> > +++ b/drivers/iio/imu/adis16475.c
> > @@ -726,6 +726,7 @@ static const struct adis16475_chip_info
> > adis16475_chip_info[] = {
> >                 .max_dec = 1999,
> >                 .sync = adis16475_sync_mode,
> >                 .num_sync = ARRAY_SIZE(adis16475_sync_mode),
> > +               .has_burst32 = true,
> >                 .adis_data = ADIS16475_DATA(16477, &adis16475_timeouts),
> >         },
> >         [ADIS16477_2] = {
> > @@ -741,6 +742,7 @@ static const struct adis16475_chip_info
> > adis16475_chip_info[] = {
> >                 .max_dec = 1999,
> >                 .sync = adis16475_sync_mode,
> >                 .num_sync = ARRAY_SIZE(adis16475_sync_mode),
> > +               .has_burst32 = true,
> >                 .adis_data = ADIS16475_DATA(16477, &adis16475_timeouts),
> >         },
> >         [ADIS16477_3] = {
> > @@ -756,6 +758,7 @@ static const struct adis16475_chip_info
> > adis16475_chip_info[] = {
> >                 .max_dec = 1999,
> >                 .sync = adis16475_sync_mode,
> >                 .num_sync = ARRAY_SIZE(adis16475_sync_mode),
> > +               .has_burst32 = true,
> >                 .adis_data = ADIS16475_DATA(16477, &adis16475_timeouts),
> >         },
> >         [ADIS16465_1] = {
>


2023-07-26 12:16:27

by Ramona Bolboaca

[permalink] [raw]
Subject: RE: [PATCH v2 3/3] iio: imu: adis16475.c: Add delta angle and delta velocity channels


> On Wed, 19 Jul 2023 15:31:52 +0300
> Ramona Bolboaca <[email protected]> wrote:
>
> > Add support for delta angle and delta velocity raw and buffer
> > readings to adis16475 driver.
> >
> > Signed-off-by: Ramona Bolboaca <[email protected]>
> As noted in my reply to Nuno's review, the question of channel type
> is still unresolved.
>

Hi Jonathan,

I think Nuno already offered the definition from the data-sheet, but let me
mention it here again for context:

Delta angle:
In addition to the angular rate of rotation (gyroscope) measurements around
each axis (x, y, and z), the ADIS16505 also provides delta angle measurements
that represent a calculation of angular displacement between each sample update.

Delta velocity:
In addition to the linear acceleration measurements along each axis (x, y, and
z), the ADIS16505 also provides delta velocity measurements that represent a
calculation of linear velocity change between each sample update.

Seeing how we cannot find something more suitable, do you think it would make
sense to add these new channels to IIO: something like IIO_DELTA_VELOCITY and
IIO_DELTA_ANGLE and specify in ABI docs that these measurements are relative
to the previous sample? Or do you have something else in mind?

Thank you,
Ramona Bolboaca

> https://urldefense.com/v3/__https://lore.kernel.org/all/20230720193457.27
> 2f02a9@jic23-
> huawei/__;!!A3Ni8CS0y2Y!52Dhq_Yj9srV_lE8JgPysp2b3Lbg9U2w94VIGTMa8
> 3hepJgv-NCnFWomV83Ud5C3TGp3tMMI0IDVRYZV5N8$
>
> Thanks,
>
> Jonathan

2023-07-29 15:26:10

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] iio: imu: adis16475.c: Add delta angle and delta velocity channels

On Wed, 26 Jul 2023 10:23:02 +0000
"Bolboaca, Ramona" <[email protected]> wrote:

> > On Wed, 19 Jul 2023 15:31:52 +0300
> > Ramona Bolboaca <[email protected]> wrote:
> >
> > > Add support for delta angle and delta velocity raw and buffer
> > > readings to adis16475 driver.
> > >
> > > Signed-off-by: Ramona Bolboaca <[email protected]>
> > As noted in my reply to Nuno's review, the question of channel type
> > is still unresolved.
> >
>
> Hi Jonathan,
>
> I think Nuno already offered the definition from the data-sheet, but let me
> mention it here again for context:
>
> Delta angle:
> In addition to the angular rate of rotation (gyroscope) measurements around
> each axis (x, y, and z), the ADIS16505 also provides delta angle measurements
> that represent a calculation of angular displacement between each sample update.
>
> Delta velocity:
> In addition to the linear acceleration measurements along each axis (x, y, and
> z), the ADIS16505 also provides delta velocity measurements that represent a
> calculation of linear velocity change between each sample update.
>
> Seeing how we cannot find something more suitable, do you think it would make
> sense to add these new channels to IIO: something like IIO_DELTA_VELOCITY and
> IIO_DELTA_ANGLE and specify in ABI docs that these measurements are relative
> to the previous sample? Or do you have something else in mind?

Yes - I think we need these new channel types to avoid inconsistent use of the
existing types. In the documentation I would give a little context, particularly
around the common use of these being to simply sum them over time.

Jonathan


>
> Thank you,
> Ramona Bolboaca
>
> > https://urldefense.com/v3/__https://lore.kernel.org/all/20230720193457.27
> > 2f02a9@jic23-
> > huawei/__;!!A3Ni8CS0y2Y!52Dhq_Yj9srV_lE8JgPysp2b3Lbg9U2w94VIGTMa8
> > 3hepJgv-NCnFWomV83Ud5C3TGp3tMMI0IDVRYZV5N8$
> >
> > Thanks,
> >
> > Jonathan