* iio: adis16400: Set timestamp as the last element in
@ 2014-01-24 11:24 Marcus Folkesson
2014-01-24 11:24 ` [PATCH] iio: adis16400: Set timestamp as the last element in chan_spec Marcus Folkesson
0 siblings, 1 reply; 4+ messages in thread
From: Marcus Folkesson @ 2014-01-24 11:24 UTC (permalink / raw)
To: Jonathan Cameron, Lars-Peter Clausen, Marcus Folkesson, Sachin Kamat
Cc: linux-iio, linux-kernel
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] iio: adis16400: Set timestamp as the last element in chan_spec
2014-01-24 11:24 iio: adis16400: Set timestamp as the last element in Marcus Folkesson
@ 2014-01-24 11:24 ` Marcus Folkesson
2014-01-24 13:46 ` Lars-Peter Clausen
0 siblings, 1 reply; 4+ messages in thread
From: Marcus Folkesson @ 2014-01-24 11:24 UTC (permalink / raw)
To: Jonathan Cameron, Lars-Peter Clausen, Marcus Folkesson, Sachin Kamat
Cc: linux-iio, linux-kernel
This is necessary since timestamp is calculated as the last element
in iio_compute_scan_bytes().
Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
drivers/iio/imu/adis16400.h | 1 +
drivers/iio/imu/adis16400_core.c | 10 +++++-----
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/iio/imu/adis16400.h b/drivers/iio/imu/adis16400.h
index 2f8f9d6..0916bf6 100644
--- a/drivers/iio/imu/adis16400.h
+++ b/drivers/iio/imu/adis16400.h
@@ -189,6 +189,7 @@ enum {
ADIS16300_SCAN_INCLI_X,
ADIS16300_SCAN_INCLI_Y,
ADIS16400_SCAN_ADC,
+ ADIS16400_SCAN_TIMESTAMP,
};
#ifdef CONFIG_IIO_BUFFER
diff --git a/drivers/iio/imu/adis16400_core.c b/drivers/iio/imu/adis16400_core.c
index 3fb7757..7f98657 100644
--- a/drivers/iio/imu/adis16400_core.c
+++ b/drivers/iio/imu/adis16400_core.c
@@ -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[] = {
--
1.7.10.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] iio: adis16400: Set timestamp as the last element in chan_spec
2014-01-24 11:24 ` [PATCH] iio: adis16400: Set timestamp as the last element in chan_spec Marcus Folkesson
@ 2014-01-24 13:46 ` Lars-Peter Clausen
2014-02-08 11:25 ` Jonathan Cameron
0 siblings, 1 reply; 4+ messages in thread
From: Lars-Peter Clausen @ 2014-01-24 13:46 UTC (permalink / raw)
To: Marcus Folkesson; +Cc: Jonathan Cameron, Sachin Kamat, linux-iio, linux-kernel
On 01/24/2014 12:24 PM, Marcus Folkesson wrote:
> This is necessary since timestamp is calculated as the last element
> in iio_compute_scan_bytes().
>
> Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
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.
> ---
> drivers/iio/imu/adis16400.h | 1 +
> drivers/iio/imu/adis16400_core.c | 10 +++++-----
> 2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/imu/adis16400.h b/drivers/iio/imu/adis16400.h
> index 2f8f9d6..0916bf6 100644
> --- a/drivers/iio/imu/adis16400.h
> +++ b/drivers/iio/imu/adis16400.h
> @@ -189,6 +189,7 @@ enum {
> ADIS16300_SCAN_INCLI_X,
> ADIS16300_SCAN_INCLI_Y,
> ADIS16400_SCAN_ADC,
> + ADIS16400_SCAN_TIMESTAMP,
> };
>
> #ifdef CONFIG_IIO_BUFFER
> diff --git a/drivers/iio/imu/adis16400_core.c b/drivers/iio/imu/adis16400_core.c
> index 3fb7757..7f98657 100644
> --- a/drivers/iio/imu/adis16400_core.c
> +++ b/drivers/iio/imu/adis16400_core.c
> @@ -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[] = {
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] iio: adis16400: Set timestamp as the last element in chan_spec
2014-01-24 13:46 ` Lars-Peter Clausen
@ 2014-02-08 11:25 ` Jonathan Cameron
0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2014-02-08 11:25 UTC (permalink / raw)
To: Lars-Peter Clausen, Marcus Folkesson
Cc: Sachin Kamat, linux-iio, linux-kernel
On 24/01/14 13:46, Lars-Peter Clausen wrote:
> On 01/24/2014 12:24 PM, Marcus Folkesson wrote:
>> This is necessary since timestamp is calculated as the last element
>> in iio_compute_scan_bytes().
>>
>> Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
>
> Looks good, thanks.
>
> Acked-by: Lars-Peter Clausen
Applied with some fuzz to fixes-togreg. Also marked for stable.
Thanks
Jonathan
>
> 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.
>
>> ---
>> drivers/iio/imu/adis16400.h | 1 +
>> drivers/iio/imu/adis16400_core.c | 10 +++++-----
>> 2 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/iio/imu/adis16400.h b/drivers/iio/imu/adis16400.h
>> index 2f8f9d6..0916bf6 100644
>> --- a/drivers/iio/imu/adis16400.h
>> +++ b/drivers/iio/imu/adis16400.h
>> @@ -189,6 +189,7 @@ enum {
>> ADIS16300_SCAN_INCLI_X,
>> ADIS16300_SCAN_INCLI_Y,
>> ADIS16400_SCAN_ADC,
>> + ADIS16400_SCAN_TIMESTAMP,
>> };
>>
>> #ifdef CONFIG_IIO_BUFFER
>> diff --git a/drivers/iio/imu/adis16400_core.c b/drivers/iio/imu/adis16400_core.c
>> index 3fb7757..7f98657 100644
>> --- a/drivers/iio/imu/adis16400_core.c
>> +++ b/drivers/iio/imu/adis16400_core.c
>> @@ -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[] = {
>>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-02-08 11:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-24 11:24 iio: adis16400: Set timestamp as the last element in Marcus Folkesson
2014-01-24 11:24 ` [PATCH] iio: adis16400: Set timestamp as the last element in chan_spec Marcus Folkesson
2014-01-24 13:46 ` Lars-Peter Clausen
2014-02-08 11:25 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).