iio: adis16400: Set timestamp as the last element in

PATCH In review Marcus Folkesson 3 participants 4 messages Jan 24, 2014 – Feb 8, 2014

Subsystems (2)

IIO Analog Devices IIO drivers

lore

Share & tools

lore

Marcus Folkesson

author

2014-01-24 11:24 UTC #0 lore raw

Hi,

This patch make sure that the timestamp element is the last element in the iio_chan_spec.
The index values for the timestamp was hardcoded and
in conflict with other elements (at least on adis16448).

This is a problem in iio_compute_scan_bytes(). The function is searching for the first element with the timestamp index. If the found element has a lesser number of storage bits, this is a potential buffer overflow issue.

Marcus Folkesson

author

2014-01-24 11:24 UTC #1 lore raw

PATCH 1/1 iio: adis16400: Set timestamp as the last element in chan_spec

This is necessary since timestamp is calculated as the last element in iio_compute_scan_bytes().

Signed-off-by Marcus Folkesson <[email protected]>

---
 drivers/iio/imu/adis16400.h      |    1 +
 drivers/iio/imu/adis16400_core.c |   10 +++++-----
 2 files changed, 6 insertions(+), 5 deletions(-)

drivers/iio/imu/adis16400.h +1 -0

index 2f8f9d6..0916bf6 100644
@@ -189,6 +189,7 @@ enum {
 	ADIS16300_SCAN_INCLI_X,
 	ADIS16300_SCAN_INCLI_Y,
 	ADIS16400_SCAN_ADC,
+	ADIS16400_SCAN_TIMESTAMP,
 };
 
 #ifdef CONFIG_IIO_BUFFER

drivers/iio/imu/adis16400_core.c +5 -5

index 3fb7757..7f98657 100644
@@ -632,7 +632,7 @@ static const struct iio_chan_spec adis16400_channels[] = {
 	ADIS16400_MAGN_CHAN(Z, ADIS16400_ZMAGN_OUT, 14),
 	ADIS16400_TEMP_CHAN(ADIS16400_TEMP_OUT, 12),
 	ADIS16400_AUX_ADC_CHAN(ADIS16400_AUX_ADC, 12),
-	IIO_CHAN_SOFT_TIMESTAMP(12)
+	IIO_CHAN_SOFT_TIMESTAMP(ADIS16400_SCAN_TIMESTAMP),
 };
 
 static const struct iio_chan_spec adis16448_channels[] = {

@@ -654,7 +654,7 @@ static const struct iio_chan_spec adis16448_channels[] = {
 		.scan_type = IIO_ST('s', 16, 16, 0),
 	},
 	ADIS16400_TEMP_CHAN(ADIS16448_TEMP_OUT, 12),
-	IIO_CHAN_SOFT_TIMESTAMP(11)
+	IIO_CHAN_SOFT_TIMESTAMP(ADIS16400_SCAN_TIMESTAMP),
 };
 
 static const struct iio_chan_spec adis16350_channels[] = {

@@ -672,7 +672,7 @@ static const struct iio_chan_spec adis16350_channels[] = {
 	ADIS16400_MOD_TEMP_CHAN(X, ADIS16350_XTEMP_OUT, 12),
 	ADIS16400_MOD_TEMP_CHAN(Y, ADIS16350_YTEMP_OUT, 12),
 	ADIS16400_MOD_TEMP_CHAN(Z, ADIS16350_ZTEMP_OUT, 12),
-	IIO_CHAN_SOFT_TIMESTAMP(11)
+	IIO_CHAN_SOFT_TIMESTAMP(ADIS16400_SCAN_TIMESTAMP),
 };
 
 static const struct iio_chan_spec adis16300_channels[] = {

@@ -685,7 +685,7 @@ static const struct iio_chan_spec adis16300_channels[] = {
 	ADIS16400_AUX_ADC_CHAN(ADIS16300_AUX_ADC, 12),
 	ADIS16400_INCLI_CHAN(X, ADIS16300_PITCH_OUT, 13),
 	ADIS16400_INCLI_CHAN(Y, ADIS16300_ROLL_OUT, 13),
-	IIO_CHAN_SOFT_TIMESTAMP(14)
+	IIO_CHAN_SOFT_TIMESTAMP(ADIS16400_SCAN_TIMESTAMP),
 };
 
 static const struct iio_chan_spec adis16334_channels[] = {

@@ -696,7 +696,7 @@ static const struct iio_chan_spec adis16334_channels[] = {
 	ADIS16400_ACCEL_CHAN(Y, ADIS16400_YACCL_OUT, 14),
 	ADIS16400_ACCEL_CHAN(Z, ADIS16400_ZACCL_OUT, 14),
 	ADIS16400_TEMP_CHAN(ADIS16350_XTEMP_OUT, 12),
-	IIO_CHAN_SOFT_TIMESTAMP(8)
+	IIO_CHAN_SOFT_TIMESTAMP(ADIS16400_SCAN_TIMESTAMP),
 };
 
 static struct attribute *adis16400_attributes[] = {

Signature · 1 line

1.7.10.4

Lars-Peter Clausen

Acked-by on v1 1/1

2014-01-24 13:46 UTC #2 lore raw

Re: [PATCH] iio: adis16400: Set timestamp as the last element in chan_spec

Quoted 4 lines · Marcus Folkesson

This is necessary since timestamp is calculated as the last element in iio_compute_scan_bytes().
Signed-off-by Marcus Folkesson <[email protected]>

Looks good, thanks.

Acked-by Lars-Peter Clausen

That said, I think we should try to modify the core code so that the scan index for the timestamp is auto assigned to be always the last one.

Quoted 67 lines

---
 drivers/iio/imu/adis16400.h      |    1 +
 drivers/iio/imu/adis16400_core.c |   10 +++++-----
 2 files changed, 6 insertions(+), 5 deletions(-)

drivers/iio/imu/adis16400.h +1 -0

index 2f8f9d6..0916bf6 100644
@@ -189,6 +189,7 @@ enum {
 	ADIS16300_SCAN_INCLI_X,
 	ADIS16300_SCAN_INCLI_Y,
 	ADIS16400_SCAN_ADC,
+	ADIS16400_SCAN_TIMESTAMP,
 };
 
 #ifdef CONFIG_IIO_BUFFER

drivers/iio/imu/adis16400_core.c +5 -5

index 3fb7757..7f98657 100644
@@ -632,7 +632,7 @@ static const struct iio_chan_spec adis16400_channels[] = {
 	ADIS16400_MAGN_CHAN(Z, ADIS16400_ZMAGN_OUT, 14),
 	ADIS16400_TEMP_CHAN(ADIS16400_TEMP_OUT, 12),
 	ADIS16400_AUX_ADC_CHAN(ADIS16400_AUX_ADC, 12),
-	IIO_CHAN_SOFT_TIMESTAMP(12)
+	IIO_CHAN_SOFT_TIMESTAMP(ADIS16400_SCAN_TIMESTAMP),
 };
 
 static const struct iio_chan_spec adis16448_channels[] = {

@@ -654,7 +654,7 @@ static const struct iio_chan_spec adis16448_channels[] = {
 		.scan_type = IIO_ST('s', 16, 16, 0),
 	},
 	ADIS16400_TEMP_CHAN(ADIS16448_TEMP_OUT, 12),
-	IIO_CHAN_SOFT_TIMESTAMP(11)
+	IIO_CHAN_SOFT_TIMESTAMP(ADIS16400_SCAN_TIMESTAMP),
 };
 
 static const struct iio_chan_spec adis16350_channels[] = {

@@ -672,7 +672,7 @@ static const struct iio_chan_spec adis16350_channels[] = {
 	ADIS16400_MOD_TEMP_CHAN(X, ADIS16350_XTEMP_OUT, 12),
 	ADIS16400_MOD_TEMP_CHAN(Y, ADIS16350_YTEMP_OUT, 12),
 	ADIS16400_MOD_TEMP_CHAN(Z, ADIS16350_ZTEMP_OUT, 12),
-	IIO_CHAN_SOFT_TIMESTAMP(11)
+	IIO_CHAN_SOFT_TIMESTAMP(ADIS16400_SCAN_TIMESTAMP),
 };
 
 static const struct iio_chan_spec adis16300_channels[] = {

@@ -685,7 +685,7 @@ static const struct iio_chan_spec adis16300_channels[] = {
 	ADIS16400_AUX_ADC_CHAN(ADIS16300_AUX_ADC, 12),
 	ADIS16400_INCLI_CHAN(X, ADIS16300_PITCH_OUT, 13),
 	ADIS16400_INCLI_CHAN(Y, ADIS16300_ROLL_OUT, 13),
-	IIO_CHAN_SOFT_TIMESTAMP(14)
+	IIO_CHAN_SOFT_TIMESTAMP(ADIS16400_SCAN_TIMESTAMP),
 };
 
 static const struct iio_chan_spec adis16334_channels[] = {

@@ -696,7 +696,7 @@ static const struct iio_chan_spec adis16334_channels[] = {
 	ADIS16400_ACCEL_CHAN(Y, ADIS16400_YACCL_OUT, 14),
 	ADIS16400_ACCEL_CHAN(Z, ADIS16400_ZACCL_OUT, 14),
 	ADIS16400_TEMP_CHAN(ADIS16350_XTEMP_OUT, 12),
-	IIO_CHAN_SOFT_TIMESTAMP(8)
+	IIO_CHAN_SOFT_TIMESTAMP(ADIS16400_SCAN_TIMESTAMP),
 };
 
 static struct attribute *adis16400_attributes[] = {

Jonathan Cameron

maintainer

2014-02-08 11:25 UTC #3 lore raw

Re: [PATCH] iio: adis16400: Set timestamp as the last element in chan_spec

Quoted 9 lines · Lars-Peter Clausen

Quoted 4 lines · Marcus Folkesson
This is necessary since timestamp is calculated as the last element in iio_compute_scan_bytes().
Signed-off-by Marcus Folkesson <[email protected]>
Looks good, thanks.
Acked-by Lars-Peter Clausen

Applied with some fuzz to fixes-togreg. Also marked for stable.

Thanks

Jonathan

Quoted 3 lines

That said, I think we should try to modify the core code so that the scan index for the timestamp is auto assigned to be always the last one.

I'd be happy with such a change. Note we need to be careful not to block other positions of timestamps coming from hardware timestamp generators, but for software time stamps it would make sense just to automate always having them at the end.

Quoted 69 lines

Quoted 68 lines

---
  drivers/iio/imu/adis16400.h      |    1 +
  drivers/iio/imu/adis16400_core.c |   10 +++++-----
  2 files changed, 6 insertions(+), 5 deletions(-)

drivers/iio/imu/adis16400.h +1 -0

index 2f8f9d6..0916bf6 100644
@@ -189,6 +189,7 @@ enum {
  	ADIS16300_SCAN_INCLI_X,
  	ADIS16300_SCAN_INCLI_Y,
  	ADIS16400_SCAN_ADC,
+	ADIS16400_SCAN_TIMESTAMP,
  };
 
  #ifdef CONFIG_IIO_BUFFER

drivers/iio/imu/adis16400_core.c +5 -5

index 3fb7757..7f98657 100644
@@ -632,7 +632,7 @@ static const struct iio_chan_spec adis16400_channels[] = {
  	ADIS16400_MAGN_CHAN(Z, ADIS16400_ZMAGN_OUT, 14),
  	ADIS16400_TEMP_CHAN(ADIS16400_TEMP_OUT, 12),
  	ADIS16400_AUX_ADC_CHAN(ADIS16400_AUX_ADC, 12),
-	IIO_CHAN_SOFT_TIMESTAMP(12)
+	IIO_CHAN_SOFT_TIMESTAMP(ADIS16400_SCAN_TIMESTAMP),
  };
 
  static const struct iio_chan_spec adis16448_channels[] = {

@@ -654,7 +654,7 @@ static const struct iio_chan_spec adis16448_channels[] = {
  		.scan_type = IIO_ST('s', 16, 16, 0),
  	},
  	ADIS16400_TEMP_CHAN(ADIS16448_TEMP_OUT, 12),
-	IIO_CHAN_SOFT_TIMESTAMP(11)
+	IIO_CHAN_SOFT_TIMESTAMP(ADIS16400_SCAN_TIMESTAMP),
  };
 
  static const struct iio_chan_spec adis16350_channels[] = {

@@ -672,7 +672,7 @@ static const struct iio_chan_spec adis16350_channels[] = {
  	ADIS16400_MOD_TEMP_CHAN(X, ADIS16350_XTEMP_OUT, 12),
  	ADIS16400_MOD_TEMP_CHAN(Y, ADIS16350_YTEMP_OUT, 12),
  	ADIS16400_MOD_TEMP_CHAN(Z, ADIS16350_ZTEMP_OUT, 12),
-	IIO_CHAN_SOFT_TIMESTAMP(11)
+	IIO_CHAN_SOFT_TIMESTAMP(ADIS16400_SCAN_TIMESTAMP),
  };
 
  static const struct iio_chan_spec adis16300_channels[] = {

@@ -685,7 +685,7 @@ static const struct iio_chan_spec adis16300_channels[] = {
  	ADIS16400_AUX_ADC_CHAN(ADIS16300_AUX_ADC, 12),
  	ADIS16400_INCLI_CHAN(X, ADIS16300_PITCH_OUT, 13),
  	ADIS16400_INCLI_CHAN(Y, ADIS16300_ROLL_OUT, 13),
-	IIO_CHAN_SOFT_TIMESTAMP(14)
+	IIO_CHAN_SOFT_TIMESTAMP(ADIS16400_SCAN_TIMESTAMP),
  };
 
  static const struct iio_chan_spec adis16334_channels[] = {

@@ -696,7 +696,7 @@ static const struct iio_chan_spec adis16334_channels[] = {
  	ADIS16400_ACCEL_CHAN(Y, ADIS16400_YACCL_OUT, 14),
  	ADIS16400_ACCEL_CHAN(Z, ADIS16400_ZACCL_OUT, 14),
  	ADIS16400_TEMP_CHAN(ADIS16350_XTEMP_OUT, 12),
-	IIO_CHAN_SOFT_TIMESTAMP(8)
+	IIO_CHAN_SOFT_TIMESTAMP(ADIS16400_SCAN_TIMESTAMP),
  };
 
  static struct attribute *adis16400_attributes[] = {