diff mbox series

[2/5] iio: imu: adis16480.c: Add delta angle and delta velocity channels

Message ID 20240423084210.191987-3-ramona.gradinariu@analog.com (mailing list archive)
State Changes Requested
Headers show
Series Add support for adis16545/47 | expand

Commit Message

Ramona Gradinariu April 23, 2024, 8:42 a.m. UTC 
Add support for delta angle and delta velocity raw readings to
adis16480 driver.

Signed-off-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
---
 drivers/iio/imu/adis16480.c | 78 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 76 insertions(+), 2 deletions(-)

Comments

Jonathan Cameron April 28, 2024, 3:04 p.m. UTC | #1 
On Tue, 23 Apr 2024 11:42:07 +0300
Ramona Gradinariu <ramona.bolboaca13@gmail.com> wrote:

> Add support for delta angle and delta velocity raw readings to
> adis16480 driver.

Why are these not allowed via the buffer interface?   Normally
my expectation of delta values is they are more or less useless
without buffered capture. The intent of providing those channels
is that they are gathered over time and summed up to give the
angle difference (for example) between start of capture and now.
Note the formula on the datasheet 
https://www.analog.com/media/en/technical-documentation/data-sheets/adis16545-16547.pdf
looks wrong (formula 3) as it's adding the signals at time
nD + d and at nD + D - 1 whereas for a delta you'd subtract those
(maybe I'm reading that wrong).

If we are providing these values as raw readings I'd expect them
to be presented as delta_angle / time (e.g. rate of change of angle) and
delta_velocity / time = acceleration (be it slightly distorted vs
the acceleration measured as a result of oversampling.).
So basically spot measurements of delta values are normally pretty
useless.

My guess is that you did this because the device either seems
to allow burst reads of the main channels or of these delta
values?

If so consider using available_scan_masks to allow one or the
other set of channels rather than not allowing capture of these
via the buffered interfaces.

Jonathan



> 
> Signed-off-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
> ---
>  drivers/iio/imu/adis16480.c | 78 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 76 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
> index bc6cbd00cd4b..4adc2244a4ef 100644
> --- a/drivers/iio/imu/adis16480.c
> +++ b/drivers/iio/imu/adis16480.c
> @@ -140,6 +140,8 @@ struct adis16480_chip_info {
>  	unsigned int accel_max_val;
>  	unsigned int accel_max_scale;
>  	unsigned int temp_scale;
> +	unsigned int deltang_max_val;
> +	unsigned int deltvel_max_val;
>  	unsigned int int_clk;
>  	unsigned int max_dec_rate;
>  	const unsigned int *filter_freqs;
> @@ -445,6 +447,12 @@ enum {
>  	ADIS16480_SCAN_MAGN_Z,
>  	ADIS16480_SCAN_BARO,
>  	ADIS16480_SCAN_TEMP,
> +	ADIS16480_SCAN_DELTANG_X,
> +	ADIS16480_SCAN_DELTANG_Y,
> +	ADIS16480_SCAN_DELTANG_Z,
> +	ADIS16480_SCAN_DELTVEL_X,
> +	ADIS16480_SCAN_DELTVEL_Y,
> +	ADIS16480_SCAN_DELTVEL_Z,
>  };
>  
>  static const unsigned int adis16480_calibbias_regs[] = {
> @@ -688,6 +696,14 @@ static int adis16480_read_raw(struct iio_dev *indio_dev,
>  			*val = 131; /* 1310mbar = 131 kPa */
>  			*val2 = 32767 << 16;
>  			return IIO_VAL_FRACTIONAL;
> +		case IIO_DELTA_ANGL:
> +			*val = st->chip_info->deltang_max_val;
> +			*val2 = 31;
> +			return IIO_VAL_FRACTIONAL_LOG2;
> +		case IIO_DELTA_VELOCITY:
> +			*val = st->chip_info->deltvel_max_val;
> +			*val2 = 31;
> +			return IIO_VAL_FRACTIONAL_LOG2;
>  		default:
>  			return -EINVAL;
>  		}
> @@ -761,6 +777,30 @@ static int adis16480_write_raw(struct iio_dev *indio_dev,
>  	BIT(IIO_CHAN_INFO_CALIBSCALE), \
>  	32)
>  
> +#define ADIS16480_DELTANG_CHANNEL(_mod) \
> +	ADIS16480_MOD_CHANNEL(IIO_DELTA_ANGL, IIO_MOD_ ## _mod, \
> +	ADIS16480_REG_ ## _mod ## _DELTAANG_OUT, ADIS16480_SCAN_DELTANG_ ## _mod, \
> +	0, \

Trivial but why this line wrap?  I'd push 32 onto the line above at least.

> +	32)
> +
> +#define ADIS16480_DELTANG_CHANNEL_NO_SCAN(_mod) \
> +	ADIS16480_MOD_CHANNEL(IIO_DELTA_ANGL, IIO_MOD_ ## _mod, \
> +	ADIS16480_REG_ ## _mod ## _DELTAANG_OUT, -1, \
> +	0, \
> +	32)
> +
> +#define ADIS16480_DELTVEL_CHANNEL(_mod) \
> +	ADIS16480_MOD_CHANNEL(IIO_DELTA_VELOCITY, IIO_MOD_ ## _mod, \
> +	ADIS16480_REG_ ## _mod ## _DELTAVEL_OUT, ADIS16480_SCAN_DELTVEL_ ## _mod, \
> +	0, \
> +	32)
> +
> +#define ADIS16480_DELTVEL_CHANNEL_NO_SCAN(_mod) \
> +	ADIS16480_MOD_CHANNEL(IIO_DELTA_VELOCITY, IIO_MOD_ ## _mod, \
> +	ADIS16480_REG_ ## _mod ## _DELTAVEL_OUT, -1, \
> +	0, \
> +	32)
Jonathan Cameron April 28, 2024, 3:07 p.m. UTC | #2 
On Sun, 28 Apr 2024 16:04:38 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Tue, 23 Apr 2024 11:42:07 +0300
> Ramona Gradinariu <ramona.bolboaca13@gmail.com> wrote:
> 
> > Add support for delta angle and delta velocity raw readings to
> > adis16480 driver.  
> 
> Why are these not allowed via the buffer interface?   Normally
> my expectation of delta values is they are more or less useless
> without buffered capture. The intent of providing those channels
> is that they are gathered over time and summed up to give the
> angle difference (for example) between start of capture and now.
> Note the formula on the datasheet 
> https://www.analog.com/media/en/technical-documentation/data-sheets/adis16545-16547.pdf
> looks wrong (formula 3) as it's adding the signals at time
> nD + d and at nD + D - 1 whereas for a delta you'd subtract those
> (maybe I'm reading that wrong).
> 
> If we are providing these values as raw readings I'd expect them
> to be presented as delta_angle / time (e.g. rate of change of angle) and
> delta_velocity / time = acceleration (be it slightly distorted vs
> the acceleration measured as a result of oversampling.).
> So basically spot measurements of delta values are normally pretty
> useless.
> 
> My guess is that you did this because the device either seems
> to allow burst reads of the main channels or of these delta
> values?
> 
> If so consider using available_scan_masks to allow one or the
> other set of channels rather than not allowing capture of these
> via the buffered interfaces.

I see from later patch that this was adding support to parts that
don't support grabbing these in burst mode.  Please make that clearer
in this patch description and list the parts to which this support
applied.

I'm a bit dubious of it being a useful feature, but maybe it is
worth supporting.

Jonathan

> 
> Jonathan
> 
> 
> 
> > 
> > Signed-off-by: Ramona Gradinariu <ramona.gradinariu@analog.com>
> > ---
> >  drivers/iio/imu/adis16480.c | 78 ++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 76 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
> > index bc6cbd00cd4b..4adc2244a4ef 100644
> > --- a/drivers/iio/imu/adis16480.c
> > +++ b/drivers/iio/imu/adis16480.c
> > @@ -140,6 +140,8 @@ struct adis16480_chip_info {
> >  	unsigned int accel_max_val;
> >  	unsigned int accel_max_scale;
> >  	unsigned int temp_scale;
> > +	unsigned int deltang_max_val;
> > +	unsigned int deltvel_max_val;
> >  	unsigned int int_clk;
> >  	unsigned int max_dec_rate;
> >  	const unsigned int *filter_freqs;
> > @@ -445,6 +447,12 @@ enum {
> >  	ADIS16480_SCAN_MAGN_Z,
> >  	ADIS16480_SCAN_BARO,
> >  	ADIS16480_SCAN_TEMP,
> > +	ADIS16480_SCAN_DELTANG_X,
> > +	ADIS16480_SCAN_DELTANG_Y,
> > +	ADIS16480_SCAN_DELTANG_Z,
> > +	ADIS16480_SCAN_DELTVEL_X,
> > +	ADIS16480_SCAN_DELTVEL_Y,
> > +	ADIS16480_SCAN_DELTVEL_Z,
> >  };
> >  
> >  static const unsigned int adis16480_calibbias_regs[] = {
> > @@ -688,6 +696,14 @@ static int adis16480_read_raw(struct iio_dev *indio_dev,
> >  			*val = 131; /* 1310mbar = 131 kPa */
> >  			*val2 = 32767 << 16;
> >  			return IIO_VAL_FRACTIONAL;
> > +		case IIO_DELTA_ANGL:
> > +			*val = st->chip_info->deltang_max_val;
> > +			*val2 = 31;
> > +			return IIO_VAL_FRACTIONAL_LOG2;
> > +		case IIO_DELTA_VELOCITY:
> > +			*val = st->chip_info->deltvel_max_val;
> > +			*val2 = 31;
> > +			return IIO_VAL_FRACTIONAL_LOG2;
> >  		default:
> >  			return -EINVAL;
> >  		}
> > @@ -761,6 +777,30 @@ static int adis16480_write_raw(struct iio_dev *indio_dev,
> >  	BIT(IIO_CHAN_INFO_CALIBSCALE), \
> >  	32)
> >  
> > +#define ADIS16480_DELTANG_CHANNEL(_mod) \
> > +	ADIS16480_MOD_CHANNEL(IIO_DELTA_ANGL, IIO_MOD_ ## _mod, \
> > +	ADIS16480_REG_ ## _mod ## _DELTAANG_OUT, ADIS16480_SCAN_DELTANG_ ## _mod, \
> > +	0, \  
> 
> Trivial but why this line wrap?  I'd push 32 onto the line above at least.
> 
> > +	32)
> > +
> > +#define ADIS16480_DELTANG_CHANNEL_NO_SCAN(_mod) \
> > +	ADIS16480_MOD_CHANNEL(IIO_DELTA_ANGL, IIO_MOD_ ## _mod, \
> > +	ADIS16480_REG_ ## _mod ## _DELTAANG_OUT, -1, \
> > +	0, \
> > +	32)
> > +
> > +#define ADIS16480_DELTVEL_CHANNEL(_mod) \
> > +	ADIS16480_MOD_CHANNEL(IIO_DELTA_VELOCITY, IIO_MOD_ ## _mod, \
> > +	ADIS16480_REG_ ## _mod ## _DELTAVEL_OUT, ADIS16480_SCAN_DELTVEL_ ## _mod, \
> > +	0, \
> > +	32)
> > +
> > +#define ADIS16480_DELTVEL_CHANNEL_NO_SCAN(_mod) \
> > +	ADIS16480_MOD_CHANNEL(IIO_DELTA_VELOCITY, IIO_MOD_ ## _mod, \
> > +	ADIS16480_REG_ ## _mod ## _DELTAVEL_OUT, -1, \
> > +	0, \
> > +	32)  
>
diff mbox series

Patch

diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
index bc6cbd00cd4b..4adc2244a4ef 100644
--- a/drivers/iio/imu/adis16480.c
+++ b/drivers/iio/imu/adis16480.c
@@ -140,6 +140,8 @@  struct adis16480_chip_info {
 	unsigned int accel_max_val;
 	unsigned int accel_max_scale;
 	unsigned int temp_scale;
+	unsigned int deltang_max_val;
+	unsigned int deltvel_max_val;
 	unsigned int int_clk;
 	unsigned int max_dec_rate;
 	const unsigned int *filter_freqs;
@@ -445,6 +447,12 @@  enum {
 	ADIS16480_SCAN_MAGN_Z,
 	ADIS16480_SCAN_BARO,
 	ADIS16480_SCAN_TEMP,
+	ADIS16480_SCAN_DELTANG_X,
+	ADIS16480_SCAN_DELTANG_Y,
+	ADIS16480_SCAN_DELTANG_Z,
+	ADIS16480_SCAN_DELTVEL_X,
+	ADIS16480_SCAN_DELTVEL_Y,
+	ADIS16480_SCAN_DELTVEL_Z,
 };
 
 static const unsigned int adis16480_calibbias_regs[] = {
@@ -688,6 +696,14 @@  static int adis16480_read_raw(struct iio_dev *indio_dev,
 			*val = 131; /* 1310mbar = 131 kPa */
 			*val2 = 32767 << 16;
 			return IIO_VAL_FRACTIONAL;
+		case IIO_DELTA_ANGL:
+			*val = st->chip_info->deltang_max_val;
+			*val2 = 31;
+			return IIO_VAL_FRACTIONAL_LOG2;
+		case IIO_DELTA_VELOCITY:
+			*val = st->chip_info->deltvel_max_val;
+			*val2 = 31;
+			return IIO_VAL_FRACTIONAL_LOG2;
 		default:
 			return -EINVAL;
 		}
@@ -761,6 +777,30 @@  static int adis16480_write_raw(struct iio_dev *indio_dev,
 	BIT(IIO_CHAN_INFO_CALIBSCALE), \
 	32)
 
+#define ADIS16480_DELTANG_CHANNEL(_mod) \
+	ADIS16480_MOD_CHANNEL(IIO_DELTA_ANGL, IIO_MOD_ ## _mod, \
+	ADIS16480_REG_ ## _mod ## _DELTAANG_OUT, ADIS16480_SCAN_DELTANG_ ## _mod, \
+	0, \
+	32)
+
+#define ADIS16480_DELTANG_CHANNEL_NO_SCAN(_mod) \
+	ADIS16480_MOD_CHANNEL(IIO_DELTA_ANGL, IIO_MOD_ ## _mod, \
+	ADIS16480_REG_ ## _mod ## _DELTAANG_OUT, -1, \
+	0, \
+	32)
+
+#define ADIS16480_DELTVEL_CHANNEL(_mod) \
+	ADIS16480_MOD_CHANNEL(IIO_DELTA_VELOCITY, IIO_MOD_ ## _mod, \
+	ADIS16480_REG_ ## _mod ## _DELTAVEL_OUT, ADIS16480_SCAN_DELTVEL_ ## _mod, \
+	0, \
+	32)
+
+#define ADIS16480_DELTVEL_CHANNEL_NO_SCAN(_mod) \
+	ADIS16480_MOD_CHANNEL(IIO_DELTA_VELOCITY, IIO_MOD_ ## _mod, \
+	ADIS16480_REG_ ## _mod ## _DELTAVEL_OUT, -1, \
+	0, \
+	32)
+
 #define ADIS16480_MAGN_CHANNEL(_mod) \
 	ADIS16480_MOD_CHANNEL(IIO_MAGN, IIO_MOD_ ## _mod, \
 	ADIS16480_REG_ ## _mod ## _MAGN_OUT, ADIS16480_SCAN_MAGN_ ## _mod, \
@@ -816,7 +856,13 @@  static const struct iio_chan_spec adis16480_channels[] = {
 	ADIS16480_MAGN_CHANNEL(Z),
 	ADIS16480_PRESSURE_CHANNEL(),
 	ADIS16480_TEMP_CHANNEL(),
-	IIO_CHAN_SOFT_TIMESTAMP(11)
+	IIO_CHAN_SOFT_TIMESTAMP(11),
+	ADIS16480_DELTANG_CHANNEL_NO_SCAN(X),
+	ADIS16480_DELTANG_CHANNEL_NO_SCAN(Y),
+	ADIS16480_DELTANG_CHANNEL_NO_SCAN(Z),
+	ADIS16480_DELTVEL_CHANNEL_NO_SCAN(X),
+	ADIS16480_DELTVEL_CHANNEL_NO_SCAN(Y),
+	ADIS16480_DELTVEL_CHANNEL_NO_SCAN(Z),
 };
 
 static const struct iio_chan_spec adis16485_channels[] = {
@@ -827,7 +873,13 @@  static const struct iio_chan_spec adis16485_channels[] = {
 	ADIS16480_ACCEL_CHANNEL(Y),
 	ADIS16480_ACCEL_CHANNEL(Z),
 	ADIS16480_TEMP_CHANNEL(),
-	IIO_CHAN_SOFT_TIMESTAMP(7)
+	IIO_CHAN_SOFT_TIMESTAMP(7),
+	ADIS16480_DELTANG_CHANNEL_NO_SCAN(X),
+	ADIS16480_DELTANG_CHANNEL_NO_SCAN(Y),
+	ADIS16480_DELTANG_CHANNEL_NO_SCAN(Z),
+	ADIS16480_DELTVEL_CHANNEL_NO_SCAN(X),
+	ADIS16480_DELTVEL_CHANNEL_NO_SCAN(Y),
+	ADIS16480_DELTVEL_CHANNEL_NO_SCAN(Z),
 };
 
 enum adis16480_variant {
@@ -938,6 +990,8 @@  static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.accel_max_val = IIO_M_S_2_TO_G(21973 << 16),
 		.accel_max_scale = 18,
 		.temp_scale = 5650, /* 5.65 milli degree Celsius */
+		.deltang_max_val = IIO_DEGREE_TO_RAD(180),
+		.deltvel_max_val = 100,
 		.int_clk = 2460000,
 		.max_dec_rate = 2048,
 		.has_sleep_cnt = true,
@@ -952,6 +1006,8 @@  static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.accel_max_val = IIO_M_S_2_TO_G(12500 << 16),
 		.accel_max_scale = 10,
 		.temp_scale = 5650, /* 5.65 milli degree Celsius */
+		.deltang_max_val = IIO_DEGREE_TO_RAD(720),
+		.deltvel_max_val = 200,
 		.int_clk = 2460000,
 		.max_dec_rate = 2048,
 		.has_sleep_cnt = true,
@@ -966,6 +1022,8 @@  static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.accel_max_val = IIO_M_S_2_TO_G(20000 << 16),
 		.accel_max_scale = 5,
 		.temp_scale = 5650, /* 5.65 milli degree Celsius */
+		.deltang_max_val = IIO_DEGREE_TO_RAD(720),
+		.deltvel_max_val = 50,
 		.int_clk = 2460000,
 		.max_dec_rate = 2048,
 		.has_sleep_cnt = true,
@@ -980,6 +1038,8 @@  static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.accel_max_val = IIO_M_S_2_TO_G(22500 << 16),
 		.accel_max_scale = 18,
 		.temp_scale = 5650, /* 5.65 milli degree Celsius */
+		.deltang_max_val = IIO_DEGREE_TO_RAD(720),
+		.deltvel_max_val = 200,
 		.int_clk = 2460000,
 		.max_dec_rate = 2048,
 		.has_sleep_cnt = true,
@@ -994,6 +1054,8 @@  static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.accel_max_val = IIO_M_S_2_TO_G(16000 << 16),
 		.accel_max_scale = 8,
 		.temp_scale = 14285, /* 14.285 milli degree Celsius */
+		.deltang_max_val = IIO_DEGREE_TO_RAD(720),
+		.deltvel_max_val = 200,
 		.int_clk = 4250000,
 		.max_dec_rate = 4250,
 		.filter_freqs = adis16495_def_filter_freqs,
@@ -1008,6 +1070,8 @@  static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.accel_max_val = IIO_M_S_2_TO_G(32000 << 16),
 		.accel_max_scale = 8,
 		.temp_scale = 12500, /* 12.5 milli degree Celsius */
+		.deltang_max_val = IIO_DEGREE_TO_RAD(360),
+		.deltvel_max_val = 100,
 		.int_clk = 4250000,
 		.max_dec_rate = 4250,
 		.filter_freqs = adis16495_def_filter_freqs,
@@ -1025,6 +1089,8 @@  static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.accel_max_val = IIO_M_S_2_TO_G(32000 << 16),
 		.accel_max_scale = 8,
 		.temp_scale = 12500, /* 12.5 milli degree Celsius */
+		.deltang_max_val = IIO_DEGREE_TO_RAD(720),
+		.deltvel_max_val = 100,
 		.int_clk = 4250000,
 		.max_dec_rate = 4250,
 		.filter_freqs = adis16495_def_filter_freqs,
@@ -1042,6 +1108,8 @@  static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.accel_max_val = IIO_M_S_2_TO_G(32000 << 16),
 		.accel_max_scale = 8,
 		.temp_scale = 12500, /* 12.5 milli degree Celsius */
+		.deltang_max_val = IIO_DEGREE_TO_RAD(2160),
+		.deltvel_max_val = 100,
 		.int_clk = 4250000,
 		.max_dec_rate = 4250,
 		.filter_freqs = adis16495_def_filter_freqs,
@@ -1059,6 +1127,8 @@  static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.accel_max_val = IIO_M_S_2_TO_G(32000 << 16),
 		.accel_max_scale = 40,
 		.temp_scale = 12500, /* 12.5 milli degree Celsius */
+		.deltang_max_val = IIO_DEGREE_TO_RAD(360),
+		.deltvel_max_val = 400,
 		.int_clk = 4250000,
 		.max_dec_rate = 4250,
 		.filter_freqs = adis16495_def_filter_freqs,
@@ -1076,6 +1146,8 @@  static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.accel_max_val = IIO_M_S_2_TO_G(32000 << 16),
 		.accel_max_scale = 40,
 		.temp_scale = 12500, /* 12.5 milli degree Celsius */
+		.deltang_max_val = IIO_DEGREE_TO_RAD(720),
+		.deltvel_max_val = 400,
 		.int_clk = 4250000,
 		.max_dec_rate = 4250,
 		.filter_freqs = adis16495_def_filter_freqs,
@@ -1093,6 +1165,8 @@  static const struct adis16480_chip_info adis16480_chip_info[] = {
 		.accel_max_val = IIO_M_S_2_TO_G(32000 << 16),
 		.accel_max_scale = 40,
 		.temp_scale = 12500, /* 12.5 milli degree Celsius */
+		.deltang_max_val = IIO_DEGREE_TO_RAD(2160),
+		.deltvel_max_val = 400,
 		.int_clk = 4250000,
 		.max_dec_rate = 4250,
 		.filter_freqs = adis16495_def_filter_freqs,