Re: [PATCH 09/10] iio: imu: kmx61: Drop odr_bits from kmx61_samp_freq_table

2015-01-04 Thread Hartmut Knaack 
Jonathan Cameron schrieb am 04.01.2015 um 11:55:
> On 01/01/15 13:53, Hartmut Knaack wrote:
>> Daniel Baluta schrieb am 23.12.2014 um 14:22:
>>> odr_bits values are between 0 and 11, so we can use the index
>>> in kmx61_samp_freq_table instead of odr_bits structure member.
>> Basically looking good, but I would feel more comfortable to check
>> against the boundaries of the table. Please see inline.
>>
>>>
>>> Signed-off-by: Daniel Baluta 
>>> ---
>>>  drivers/iio/imu/kmx61.c | 64 
>>> -
>>>  1 file changed, 26 insertions(+), 38 deletions(-)
>>>
>>> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
>>> index eb3900e..98369eb 100644
>>> --- a/drivers/iio/imu/kmx61.c
>>> +++ b/drivers/iio/imu/kmx61.c
>>> @@ -169,19 +169,18 @@ u16 kmx61_uscale_table[] = {9582, 19163, 38326};
>>>  static const struct {
>>> int val;
>>> int val2;
>>> -   u8 odr_bits;
>>> -} kmx61_samp_freq_table[] = { {12, 50, 0x00},
>>> -   {25, 0, 0x01},
>>> -   {50, 0, 0x02},
>>> -   {100, 0, 0x03},
>>> -   {200, 0, 0x04},
>>> -   {400, 0, 0x05},
>>> -   {800, 0, 0x06},
>>> -   {1600, 0, 0x07},
>>> -   {0, 781000, 0x08},
>>> -   {1, 563000, 0x09},
>>> -   {3, 125000, 0x0A},
>>> -   {6, 25, 0x0B} };
>>> +} kmx61_samp_freq_table[] = { {12, 50},
>>> +   {25, 0},
>>> +   {50, 0},
>>> +   {100, 0},
>>> +   {200, 0},
>>> +   {400, 0},
>>> +   {800, 0},
>>> +   {1600, 0},
>>> +   {0, 781000},
>>> +   {1, 563000},
>>> +   {3, 125000},
>>> +   {6, 25} };
>>>  
>>>  static const struct {
>>> int val;
>>> @@ -302,24 +301,10 @@ static int kmx61_convert_freq_to_bit(int val, int 
>>> val2)
>>> for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
>>> if (val == kmx61_samp_freq_table[i].val &&
>>> val2 == kmx61_samp_freq_table[i].val2)
>>> -   return kmx61_samp_freq_table[i].odr_bits;
>>> -   return -EINVAL;
>>> -}
>>> -
>>> -static int kmx61_convert_bit_to_freq(u8 odr_bits, int *val, int *val2)
>>> -{
>>> -   int i;
>>> -
>>> -   for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
>>> -   if (odr_bits == kmx61_samp_freq_table[i].odr_bits) {
>>> -   *val = kmx61_samp_freq_table[i].val;
>>> -   *val2 = kmx61_samp_freq_table[i].val2;
>>> -   return 0;
>>> -   }
>>> +   return i;
>>> return -EINVAL;
>>>  }
>>>  
>>> -
>>>  static int kmx61_convert_wake_up_odr_to_bit(int val, int val2)
>>>  {
>>> int i;
>>> @@ -478,7 +463,7 @@ static int kmx61_set_odr(struct kmx61_data *data, int 
>>> val, int val2, u8 device)
>>>  
>>>  static int kmx61_get_odr(struct kmx61_data *data, int *val, int *val2,
>>>  u8 device)
>>> -{  int i;
>>> +{
>>> u8 lodr_bits;
>>>  
>>> if (device & KMX61_ACC)
>>> @@ -490,13 +475,13 @@ static int kmx61_get_odr(struct kmx61_data *data, int 
>>> *val, int *val2,
>>> else
>>> return -EINVAL;
>>>  
>>> -   for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
>>> -   if (lodr_bits == kmx61_samp_freq_table[i].odr_bits) {
>>> -   *val = kmx61_samp_freq_table[i].val;
>>> -   *val2 = kmx61_samp_freq_table[i].val2;
>>> -   return 0;
>>> -   }
>>> -   return -EINVAL;
>>> +   if (lodr_bits > 0xB)
>> Since we use it as an index, I regard it safer to check against
>> ARRAY_SIZE(kmx61_samp_freq_table) rather than a fix value.
> Makes sense to me as well - though obviously 
> ARRAY_SIZE(kmx61_samp_freq_table) is
> 0xC (12) rather than 0xB (11) so you'll need a minus 1
> 
Instead of subtracting, I would favor this one:
if !(lodr_bits < ARRAY_SIZE(...))

Or this one:
if (lodr_bits >= ARRAY_SIZE(...))

> I'll leave this one for a new spin.
>>
>>> +   return -EINVAL;
>>> +
>>> +   *val = kmx61_samp_freq_table[lodr_bits].val;
>>> +   *val2 = kmx61_samp_freq_table[lodr_bits].val2;
>>> +
>>> +   return 0;
>>>  }
>>>  
>>>  static int kmx61_set_range(struct kmx61_data *data, u8 range)
>>> @@ -580,8 +565,11 @@ static int kmx61_chip_init(struct kmx61_data *data)
>>> }
>>> data->odr_bits = ret;
>>>  
>>> -   /* set output data rate for wake up (motion detection) function */
>>> -   ret = kmx61_convert_bit_to_freq(data->odr_bits, &val, &val2);
>>> +   /*
>>> +* set output data rate for wake up (motion detection) function
>>> +* to match data rate for accelerometer sampling
>>> +*/
>>> +   ret = kmx61_get_odr(data, &val, &val2, KMX61_ACC);
>>> if (ret < 0)
>>> return ret;
>>>  
>>>
>>
> 
> --
> To unsubscribe from this list: send

Re: [PATCH 09/10] iio: imu: kmx61: Drop odr_bits from kmx61_samp_freq_table

2015-01-04 Thread Jonathan Cameron 
On 01/01/15 13:53, Hartmut Knaack wrote:
> Daniel Baluta schrieb am 23.12.2014 um 14:22:
>> odr_bits values are between 0 and 11, so we can use the index
>> in kmx61_samp_freq_table instead of odr_bits structure member.
> Basically looking good, but I would feel more comfortable to check
> against the boundaries of the table. Please see inline.
> 
>>
>> Signed-off-by: Daniel Baluta 
>> ---
>>  drivers/iio/imu/kmx61.c | 64 
>> -
>>  1 file changed, 26 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
>> index eb3900e..98369eb 100644
>> --- a/drivers/iio/imu/kmx61.c
>> +++ b/drivers/iio/imu/kmx61.c
>> @@ -169,19 +169,18 @@ u16 kmx61_uscale_table[] = {9582, 19163, 38326};
>>  static const struct {
>>  int val;
>>  int val2;
>> -u8 odr_bits;
>> -} kmx61_samp_freq_table[] = { {12, 50, 0x00},
>> -{25, 0, 0x01},
>> -{50, 0, 0x02},
>> -{100, 0, 0x03},
>> -{200, 0, 0x04},
>> -{400, 0, 0x05},
>> -{800, 0, 0x06},
>> -{1600, 0, 0x07},
>> -{0, 781000, 0x08},
>> -{1, 563000, 0x09},
>> -{3, 125000, 0x0A},
>> -{6, 25, 0x0B} };
>> +} kmx61_samp_freq_table[] = { {12, 50},
>> +{25, 0},
>> +{50, 0},
>> +{100, 0},
>> +{200, 0},
>> +{400, 0},
>> +{800, 0},
>> +{1600, 0},
>> +{0, 781000},
>> +{1, 563000},
>> +{3, 125000},
>> +{6, 25} };
>>  
>>  static const struct {
>>  int val;
>> @@ -302,24 +301,10 @@ static int kmx61_convert_freq_to_bit(int val, int val2)
>>  for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
>>  if (val == kmx61_samp_freq_table[i].val &&
>>  val2 == kmx61_samp_freq_table[i].val2)
>> -return kmx61_samp_freq_table[i].odr_bits;
>> -return -EINVAL;
>> -}
>> -
>> -static int kmx61_convert_bit_to_freq(u8 odr_bits, int *val, int *val2)
>> -{
>> -int i;
>> -
>> -for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
>> -if (odr_bits == kmx61_samp_freq_table[i].odr_bits) {
>> -*val = kmx61_samp_freq_table[i].val;
>> -*val2 = kmx61_samp_freq_table[i].val2;
>> -return 0;
>> -}
>> +return i;
>>  return -EINVAL;
>>  }
>>  
>> -
>>  static int kmx61_convert_wake_up_odr_to_bit(int val, int val2)
>>  {
>>  int i;
>> @@ -478,7 +463,7 @@ static int kmx61_set_odr(struct kmx61_data *data, int 
>> val, int val2, u8 device)
>>  
>>  static int kmx61_get_odr(struct kmx61_data *data, int *val, int *val2,
>>   u8 device)
>> -{   int i;
>> +{
>>  u8 lodr_bits;
>>  
>>  if (device & KMX61_ACC)
>> @@ -490,13 +475,13 @@ static int kmx61_get_odr(struct kmx61_data *data, int 
>> *val, int *val2,
>>  else
>>  return -EINVAL;
>>  
>> -for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
>> -if (lodr_bits == kmx61_samp_freq_table[i].odr_bits) {
>> -*val = kmx61_samp_freq_table[i].val;
>> -*val2 = kmx61_samp_freq_table[i].val2;
>> -return 0;
>> -}
>> -return -EINVAL;
>> +if (lodr_bits > 0xB)
> Since we use it as an index, I regard it safer to check against
> ARRAY_SIZE(kmx61_samp_freq_table) rather than a fix value.
Makes sense to me as well - though obviously ARRAY_SIZE(kmx61_samp_freq_table) 
is
0xC (12) rather than 0xB (11) so you'll need a minus 1

I'll leave this one for a new spin.
> 
>> +return -EINVAL;
>> +
>> +*val = kmx61_samp_freq_table[lodr_bits].val;
>> +*val2 = kmx61_samp_freq_table[lodr_bits].val2;
>> +
>> +return 0;
>>  }
>>  
>>  static int kmx61_set_range(struct kmx61_data *data, u8 range)
>> @@ -580,8 +565,11 @@ static int kmx61_chip_init(struct kmx61_data *data)
>>  }
>>  data->odr_bits = ret;
>>  
>> -/* set output data rate for wake up (motion detection) function */
>> -ret = kmx61_convert_bit_to_freq(data->odr_bits, &val, &val2);
>> +/*
>> + * set output data rate for wake up (motion detection) function
>> + * to match data rate for accelerometer sampling
>> + */
>> +ret = kmx61_get_odr(data, &val, &val2, KMX61_ACC);
>>  if (ret < 0)
>>  return ret;
>>  
>>
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 09/10] iio: imu: kmx61: Drop odr_bits from kmx61_samp_freq_table

2015-01-01 Thread Hartmut Knaack 
Daniel Baluta schrieb am 23.12.2014 um 14:22:
> odr_bits values are between 0 and 11, so we can use the index
> in kmx61_samp_freq_table instead of odr_bits structure member.
Basically looking good, but I would feel more comfortable to check
against the boundaries of the table. Please see inline.

> 
> Signed-off-by: Daniel Baluta 
> ---
>  drivers/iio/imu/kmx61.c | 64 
> -
>  1 file changed, 26 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
> index eb3900e..98369eb 100644
> --- a/drivers/iio/imu/kmx61.c
> +++ b/drivers/iio/imu/kmx61.c
> @@ -169,19 +169,18 @@ u16 kmx61_uscale_table[] = {9582, 19163, 38326};
>  static const struct {
>   int val;
>   int val2;
> - u8 odr_bits;
> -} kmx61_samp_freq_table[] = { {12, 50, 0x00},
> - {25, 0, 0x01},
> - {50, 0, 0x02},
> - {100, 0, 0x03},
> - {200, 0, 0x04},
> - {400, 0, 0x05},
> - {800, 0, 0x06},
> - {1600, 0, 0x07},
> - {0, 781000, 0x08},
> - {1, 563000, 0x09},
> - {3, 125000, 0x0A},
> - {6, 25, 0x0B} };
> +} kmx61_samp_freq_table[] = { {12, 50},
> + {25, 0},
> + {50, 0},
> + {100, 0},
> + {200, 0},
> + {400, 0},
> + {800, 0},
> + {1600, 0},
> + {0, 781000},
> + {1, 563000},
> + {3, 125000},
> + {6, 25} };
>  
>  static const struct {
>   int val;
> @@ -302,24 +301,10 @@ static int kmx61_convert_freq_to_bit(int val, int val2)
>   for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
>   if (val == kmx61_samp_freq_table[i].val &&
>   val2 == kmx61_samp_freq_table[i].val2)
> - return kmx61_samp_freq_table[i].odr_bits;
> - return -EINVAL;
> -}
> -
> -static int kmx61_convert_bit_to_freq(u8 odr_bits, int *val, int *val2)
> -{
> - int i;
> -
> - for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
> - if (odr_bits == kmx61_samp_freq_table[i].odr_bits) {
> - *val = kmx61_samp_freq_table[i].val;
> - *val2 = kmx61_samp_freq_table[i].val2;
> - return 0;
> - }
> + return i;
>   return -EINVAL;
>  }
>  
> -
>  static int kmx61_convert_wake_up_odr_to_bit(int val, int val2)
>  {
>   int i;
> @@ -478,7 +463,7 @@ static int kmx61_set_odr(struct kmx61_data *data, int 
> val, int val2, u8 device)
>  
>  static int kmx61_get_odr(struct kmx61_data *data, int *val, int *val2,
>u8 device)
> -{int i;
> +{
>   u8 lodr_bits;
>  
>   if (device & KMX61_ACC)
> @@ -490,13 +475,13 @@ static int kmx61_get_odr(struct kmx61_data *data, int 
> *val, int *val2,
>   else
>   return -EINVAL;
>  
> - for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
> - if (lodr_bits == kmx61_samp_freq_table[i].odr_bits) {
> - *val = kmx61_samp_freq_table[i].val;
> - *val2 = kmx61_samp_freq_table[i].val2;
> - return 0;
> - }
> - return -EINVAL;
> + if (lodr_bits > 0xB)
Since we use it as an index, I regard it safer to check against
ARRAY_SIZE(kmx61_samp_freq_table) rather than a fix value.

> + return -EINVAL;
> +
> + *val = kmx61_samp_freq_table[lodr_bits].val;
> + *val2 = kmx61_samp_freq_table[lodr_bits].val2;
> +
> + return 0;
>  }
>  
>  static int kmx61_set_range(struct kmx61_data *data, u8 range)
> @@ -580,8 +565,11 @@ static int kmx61_chip_init(struct kmx61_data *data)
>   }
>   data->odr_bits = ret;
>  
> - /* set output data rate for wake up (motion detection) function */
> - ret = kmx61_convert_bit_to_freq(data->odr_bits, &val, &val2);
> + /*
> +  * set output data rate for wake up (motion detection) function
> +  * to match data rate for accelerometer sampling
> +  */
> + ret = kmx61_get_odr(data, &val, &val2, KMX61_ACC);
>   if (ret < 0)
>   return ret;
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 09/10] iio: imu: kmx61: Drop odr_bits from kmx61_samp_freq_table

2014-12-23 Thread Daniel Baluta 
odr_bits values are between 0 and 11, so we can use the index
in kmx61_samp_freq_table instead of odr_bits structure member.

Signed-off-by: Daniel Baluta 
---
 drivers/iio/imu/kmx61.c | 64 -
 1 file changed, 26 insertions(+), 38 deletions(-)

diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
index eb3900e..98369eb 100644
--- a/drivers/iio/imu/kmx61.c
+++ b/drivers/iio/imu/kmx61.c
@@ -169,19 +169,18 @@ u16 kmx61_uscale_table[] = {9582, 19163, 38326};
 static const struct {
int val;
int val2;
-   u8 odr_bits;
-} kmx61_samp_freq_table[] = { {12, 50, 0x00},
-   {25, 0, 0x01},
-   {50, 0, 0x02},
-   {100, 0, 0x03},
-   {200, 0, 0x04},
-   {400, 0, 0x05},
-   {800, 0, 0x06},
-   {1600, 0, 0x07},
-   {0, 781000, 0x08},
-   {1, 563000, 0x09},
-   {3, 125000, 0x0A},
-   {6, 25, 0x0B} };
+} kmx61_samp_freq_table[] = { {12, 50},
+   {25, 0},
+   {50, 0},
+   {100, 0},
+   {200, 0},
+   {400, 0},
+   {800, 0},
+   {1600, 0},
+   {0, 781000},
+   {1, 563000},
+   {3, 125000},
+   {6, 25} };
 
 static const struct {
int val;
@@ -302,24 +301,10 @@ static int kmx61_convert_freq_to_bit(int val, int val2)
for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
if (val == kmx61_samp_freq_table[i].val &&
val2 == kmx61_samp_freq_table[i].val2)
-   return kmx61_samp_freq_table[i].odr_bits;
-   return -EINVAL;
-}
-
-static int kmx61_convert_bit_to_freq(u8 odr_bits, int *val, int *val2)
-{
-   int i;
-
-   for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
-   if (odr_bits == kmx61_samp_freq_table[i].odr_bits) {
-   *val = kmx61_samp_freq_table[i].val;
-   *val2 = kmx61_samp_freq_table[i].val2;
-   return 0;
-   }
+   return i;
return -EINVAL;
 }
 
-
 static int kmx61_convert_wake_up_odr_to_bit(int val, int val2)
 {
int i;
@@ -478,7 +463,7 @@ static int kmx61_set_odr(struct kmx61_data *data, int val, 
int val2, u8 device)
 
 static int kmx61_get_odr(struct kmx61_data *data, int *val, int *val2,
 u8 device)
-{  int i;
+{
u8 lodr_bits;
 
if (device & KMX61_ACC)
@@ -490,13 +475,13 @@ static int kmx61_get_odr(struct kmx61_data *data, int 
*val, int *val2,
else
return -EINVAL;
 
-   for (i = 0; i < ARRAY_SIZE(kmx61_samp_freq_table); i++)
-   if (lodr_bits == kmx61_samp_freq_table[i].odr_bits) {
-   *val = kmx61_samp_freq_table[i].val;
-   *val2 = kmx61_samp_freq_table[i].val2;
-   return 0;
-   }
-   return -EINVAL;
+   if (lodr_bits > 0xB)
+   return -EINVAL;
+
+   *val = kmx61_samp_freq_table[lodr_bits].val;
+   *val2 = kmx61_samp_freq_table[lodr_bits].val2;
+
+   return 0;
 }
 
 static int kmx61_set_range(struct kmx61_data *data, u8 range)
@@ -580,8 +565,11 @@ static int kmx61_chip_init(struct kmx61_data *data)
}
data->odr_bits = ret;
 
-   /* set output data rate for wake up (motion detection) function */
-   ret = kmx61_convert_bit_to_freq(data->odr_bits, &val, &val2);
+   /*
+* set output data rate for wake up (motion detection) function
+* to match data rate for accelerometer sampling
+*/
+   ret = kmx61_get_odr(data, &val, &val2, KMX61_ACC);
if (ret < 0)
return ret;
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/