From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Message-ID: <5366E298.8050409@linux.intel.com> Date: Sun, 04 May 2014 18:00:08 -0700 From: Srinivas Pandruvada MIME-Version: 1.0 To: Jonathan Cameron CC: linux-iio@vger.kernel.org Subject: Re: [PATCH 01/16] iio: hid-sensors: Convert units and exponent References: <1397863356-2470-1-git-send-email-srinivas.pandruvada@linux.intel.com> <535AA9C9.10002@kernel.org> <535C0A2D.60207@linux.intel.com> <5365425F.9040309@kernel.org> <53665382.9030906@linux.intel.com> <4102328d-88e0-441f-87c9-0b57cbc33ca5@email.android.com> In-Reply-To: <4102328d-88e0-441f-87c9-0b57cbc33ca5@email.android.com> Content-Type: multipart/alternative; boundary="------------080304010807030700020202" List-ID: This is a multi-part message in MIME format. --------------080304010807030700020202 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hi Jonathan, I have just sent you one patch, which can be applied to iio git tree on top of the currently applied patches. Thanks, Srinivas On 05/04/2014 07:56 AM, Jonathan Cameron wrote: > Cool. I will hold them for now then. Had a bored moment and not much > else pending so jumped the gun! > > On May 4, 2014 3:49:38 PM GMT+01:00, Srinivas Pandruvada > wrote: > > Hi Jonathan, > > Somebody pointed me some error in magnetometer calculation, I have a > correction, > I will submit as another patch. I was about to push second version of > these patches. > > Thanks, > Srinivas > > On 05/03/2014 12:24 PM, Jonathan Cameron wrote: > > On 26/04/14 20:34, Srinivas Pandruvada wrote: > > On 04/25/2014 11:30 AM, Jonathan Cameron wrote: > > On 19/04/14 00:22, Srinivas Pandruvada wrote: > > HID sensor hub specify a default unit and > alternative units. This along with unit exponent > can be used adjust scale. This change change HID > sensor data units to IIO defined units for each > sensor type. So in this way user space can use a > simply use: "(data + offset) * scale" to get final > result. Signed-off-by: Srinivas Pandruvada > > > see inline. > > Applied to the togreg branch of iio.git with a tweaks to > comments as mentioned (please check I didn't mess them up!) > > --- > .../iio/common/hid-sensors/hid-sensor-attributes.c > | 114 +++++++++++++++++++++ > include/linux/hid-sensor-hub.h | 4 + 2 files > changed, 118 insertions(+) diff --git > a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c > b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c > index 75b5473..451a95b 100644 --- > a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c > +++ > b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c > @@ -26,6 +26,40 @@ #include > #include +struct { + u32 > usage_id; + int unit; /* 0 for default others from > HID sensor spec */ + int scale_val0; /* scale, > whole number */ + int scale_val1; /* scale, > fraction in micros */ +} unit_conversion[] = { + > {HID_USAGE_SENSOR_ACCEL_3D, 0, 9, 806650}, + > {HID_USAGE_SENSOR_ACCEL_3D, + > HID_USAGE_SENSOR_UNITS_METERS_PER_SEC_SQRD, 1, 0}, > + {HID_USAGE_SENSOR_ACCEL_3D, + > HID_USAGE_SENSOR_UNITS_G, 9, 806650}, + + > {HID_USAGE_SENSOR_GYRO_3D, 0, 0, 17453}, + > {HID_USAGE_SENSOR_GYRO_3D, + > HID_USAGE_SENSOR_UNITS_RADIANS_PER_SECOND, 1, 0}, > + {HID_USAGE_SENSOR_GYRO_3D, + > HID_USAGE_SENSOR_UNITS_DEGREES_PER_SECOND, 0, > 17453}, + + {HID_USAGE_SENSOR_COMPASS_3D, 0, 1000, > 0}, + {HID_USAGE_SENSOR_COMPASS_3D, > HID_USAGE_SENSOR_UNITS_GAUSS, 1, 0}, + + > {HID_USAGE_SENSOR_INCLINOMETER_3D, 0, 17453, 0}, + > {HID_USAGE_SENSOR_INCLINOMETER_3D, + > HID_USAGE_SENSOR_UNITS_DEGREES, 0, 17453}, + > {HID_USAGE_SENSOR_INCLINOMETER_3D, + > HID_USAGE_SENSOR_UNITS_RADIANS, 1, 0}, + + > {HID_USAGE_SENSOR_ALS, 0, 1, 0}, + > {HID_USAGE_SENSOR_ALS, HID_USAGE_SENSOR_UNITS_LUX, > 1, 0}, + + {HID_USAGE_SENSOR_PRESSURE, 0, 100000, > 0}, + {HID_USAGE_SENSOR_PRESSURE, > HID_USAGE_SENSOR_UNITS_PASCAL, 1, 0}, +}; + static > int pow_10(unsigned power) { int i; @@ -209,6 > +243,86 @@ int > hid_sensor_write_raw_hyst_value(struct > hid_sensor_common *st, } > EXPORT_SYMBOL(hid_sensor_write_raw_hyst_value); > +/* + * This fuction applies the unit exponent to > the scale. + * For example: > > Function > > + * 9.806650 ->exp:2-> val0[980]val1[6650] + * > 9.000806 ->exp:2-> val0[900]val1[806] + * 0.174535 > ->exp:2-> val0[17]val1[4535] + * 1.001745 > ->exp:0-> val0[1]val1[1745] + * 1.001745 ->exp:2-> > val0[100]val1[1745] + * 1.001745 ->exp:4-> > val0[10017]val1[45] + * 9.806650 ->exp:-2-> > val0[0]val1[98066] > > It took me a while to get what you meant here, perhaps > for the examples it is worth showing the actual value > directly as well. Maybe. 9.806650 exp:2->980.6650 -> > val0[980]val1[665000] (note the above is I think > incorrect due to lack of trailing zeros on val1. > > + */ +static void adjust_exponent_micro(int *val0, > int *val1, int scale0, + int scale1, int exp) +{ + > int i; + int x; + int res; + int rem; + + if (exp > > 0) { + *val0 = scale0 * pow_10(exp); + res = 0; > + if (exp > 6) { + *val1 = 0; + return; + } > > so if example 1 above.. val0 = 900 as desired. i = 0; > sca1e1 = 806650 x = 806650/100000 = 8; res = 80; > scale1 = 06650 i = 1 x = 06650/10000 = 0; res = 80; so > 980. Good. > > + for (i = 0; i < exp; ++i) { + x = scale1 / > pow_10(5 - i); + res += (pow_10(exp - 1 - i) * x); > + scale1 = scale1 % pow_10(5 - i); + } + *val0 += > res; + *val1 = scale1 * pow_10(exp); > > val1 = 665000 which is correct with the trailing zeros > in val2. So your code is good but examples need those > zeros! > > Impressive, Absolutely great review. I will correct > comments in the next version. > > Given this was the only issue in the patch set I'll just fix > it up as I merge it. It's been a couple of weeks an noone else > has chipped in on this series, so I'm taking it now. > > Thanks, Srinivas > > + } else if (exp < 0) { + exp = abs(exp); + if > (exp > 6) { + *val0 = *val1 = 0; + return; + } + > *val0 = scale0 / pow_10(exp); + rem = scale0 % > pow_10(exp); + res = 0; + for (i = 0; i < (6 - > exp); ++i) { + x = scale1 / pow_10(5 - i); + res > += (pow_10(5 - exp - i) * x); + scale1 = scale1 % > pow_10(5 - i); + } + *val1 = rem * pow_10(6 - exp) > + res; + } else { + *val0 = scale0; + *val1 = > scale1; + } +} + +int hid_sensor_format_scale(u32 > usage_id, + struct hid_sensor_hub_attribute_info > *attr_info, + int *val0, int *val1) +{ + int i; + > int exp; + + *val0 = 1; + *val1 = 0; + + for (i = > 0; ARRAY_SIZE(unit_conversion); ++i) { + if > (unit_conversion[i].usage_id == usage_id && + > unit_conversion[i].unit == attr_info->units) { + > exp = hid_sensor_convert_exponent( + > attr_info->unit_expo); + > adjust_exponent_micro(val0, val1, + > unit_conversion[i].scale_val0, + > unit_conversion[i].scale_val1, exp); + break; + } > + } + + return IIO_VAL_INT_PLUS_MICRO; +} > +EXPORT_SYMBOL(hid_sensor_format_scale); + int > hid_sensor_parse_common_attributes(struct > hid_sensor_hub_device *hsdev, u32 usage_id, struct > hid_sensor_common *st) diff --git > a/include/linux/hid-sensor-hub.h > b/include/linux/hid-sensor-hub.h index > b70cfd7..89626b2 100644 --- > a/include/linux/hid-sensor-hub.h +++ > b/include/linux/hid-sensor-hub.h @@ -223,4 +223,8 > @@ int hid_sensor_read_samp_freq_value(struct > hid_sensor_common *st, int > hid_sensor_get_usage_index(struct > hid_sensor_hub_device *hsdev, u32 report_id, int > field_index, u32 usage_id); +int > hid_sensor_format_scale(u32 usage_id, + struct > hid_sensor_hub_attribute_info *attr_info, + int > *val0, int *val1); + #endif > > -- To unsubscribe from this list: send the line > "unsubscribe linux-iio" in the body of a message to > majordomo@vger.kernel.org More majordomo info at > http://vger.kernel.org/majordomo-info.html > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info athttp://vger.kernel.org/majordomo-info.html > > > -- > Sent from my Android phone with K-9 Mail. Please excuse my brevity. --------------080304010807030700020202 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit Hi Jonathan,

I have just sent you one patch, which can be applied to iio git tree on top of the currently applied patches.

Thanks,
Srinivas

On 05/04/2014 07:56 AM, Jonathan Cameron wrote:
Cool. I will hold them for now then. Had a bored moment and not much else pending so jumped the gun!

On May 4, 2014 3:49:38 PM GMT+01:00, Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote: 
Hi Jonathan,

Somebody pointed me some error in magnetometer calculation, I have a 
correction,
I will submit as another patch. I was about to push second version of 
these patches.

Thanks,
Srinivas

On 05/03/2014 12:24 PM, Jonathan Cameron wrote:

 On 26/04/14 20:34, Srinivas Pandruvada wrote:


 On 04/25/2014 11:30 AM, Jonathan Cameron wrote:

 On 19/04/14 00:22, Srinivas Pandruvada wrote:

 HID sensor hub specify a default
unit and alternative units. This
 along with unit exponent can be used adjust scale. This change
 change HID sensor data units to IIO defined units for each
 sensor type. So in this way user space can use a simply use:
 "(data + offset) * scale" to get final result.

 Signed-off-by: Srinivas Pandruvada 
 <srinivas.pandruvada@linux.intel.com>

 see inline.

 Applied to the togreg branch of iio.git with a tweaks to comments as 
 mentioned
 (please check I didn't mess them up!)


 ---
  
.../iio/common/hid-sensors/hid-sensor-attributes.c | 114 
 +++++++++++++++++++++
   include/linux/hid-sensor-hub.h                     |   4 +
   2 files changed, 118 insertions(+)

 diff --git a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c 
 b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
 index 75b5473..451a95b 100644
 --- a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
 +++ b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
 @@ -26,6 +26,40 @@
   #include <linux/iio/iio.h>
   #include <linux/iio/sysfs.h>

 +struct {
 +    u32 usage_id;
 +    int unit; /* 0 for default others from HID sensor spec */
 +    int scale_val0; /* scale, whole number */
 +    int scale_val1; /* scale, fraction in micros */
 +} unit_conversion[] = {
 +    {HID_USAGE_SENSOR_ACCEL_3D, 0, 9, 806650},
 +    {HID_USAGE_SENSOR_ACCEL_3D,
 +       
HID_USAGE_SENSOR_UNITS_METERS_PER_SEC_SQRD, 1, 0},
 +    {HID_USAGE_SENSOR_ACCEL_3D,
 +        HID_USAGE_SENSOR_UNITS_G, 9, 806650},
 +
 +    {HID_USAGE_SENSOR_GYRO_3D, 0, 0, 17453},
 +    {HID_USAGE_SENSOR_GYRO_3D,
 +        HID_USAGE_SENSOR_UNITS_RADIANS_PER_SECOND, 1, 0},
 +    {HID_USAGE_SENSOR_GYRO_3D,
 +        HID_USAGE_SENSOR_UNITS_DEGREES_PER_SECOND, 0, 17453},
 +
 +    {HID_USAGE_SENSOR_COMPASS_3D, 0, 1000, 0},
 +    {HID_USAGE_SENSOR_COMPASS_3D, HID_USAGE_SENSOR_UNITS_GAUSS, 1, 
 0},
 +
 +    {HID_USAGE_SENSOR_INCLINOMETER_3D, 0, 17453, 0},
 +    {HID_USAGE_SENSOR_INCLINOMETER_3D,
 +        HID_USAGE_SENSOR_UNITS_DEGREES, 0, 17453},
 +    {HID_USAGE_SENSOR_INCLINOMETER_3D,
 +        HID_USAGE_SENSOR_UNITS_RADIANS, 1, 0},
 +
 +    {HID_USAGE_SENSOR_ALS, 0, 1, 0},
 +    {HID_USAGE_SENSOR_ALS, HID_USAGE_SENSOR_UNITS_LUX, 1, 0},
 +
 +   
{HID_USAGE_SENSOR_PRESSURE, 0, 100000, 0},
 +    {HID_USAGE_SENSOR_PRESSURE, HID_USAGE_SENSOR_UNITS_PASCAL, 1, 0},
 +};
 +
   static int pow_10(unsigned power)
   {
       int i;
 @@ -209,6 +243,86 @@ int hid_sensor_write_raw_hyst_value(struct 
 hid_sensor_common *st,
   }
   EXPORT_SYMBOL(hid_sensor_write_raw_hyst_value);

 +/*
 + * This fuction applies the unit exponent to the scale.
 + * For example:

 Function

 + * 9.806650 ->exp:2-> val0[980]val1[6650]
 + * 9.000806 ->exp:2-> val0[900]val1[806]
 + * 0.174535 ->exp:2-> val0[17]val1[4535]
 + * 1.001745 ->exp:0-> val0[1]val1[1745]
 + * 1.001745 ->exp:2-> val0[100]val1[1745]
 + * 1.001745 ->exp:4-> val0[10017]val1[45]
 + * 9.806650 ->exp:-2->
val0[0]val1[98066]

 It took me a while to get what you meant here, perhaps for the 
 examples it
 is worth showing the actual value directly as well. Maybe.
 9.806650 exp:2->980.6650 -> val0[980]val1[665000] (note the above is 
 I think
 incorrect due to lack of trailing zeros on val1.

 + */
 +static void adjust_exponent_micro(int *val0, int *val1, int scale0,
 +                  int scale1, int exp)
 +{
 +    int i;
 +    int x;
 +    int res;
 +    int rem;
 +
 +    if (exp > 0) {
 +        *val0 = scale0 * pow_10(exp);
 +        res = 0;
 +        if (exp > 6) {
 +            *val1 = 0;
 +            return;
 +        }

 so if example 1 above..
 val0 = 900 as desired.
 i  = 0;
 sca1e1 = 806650

x = 806650/100000 = 8;
 res = 80;
 scale1 = 06650
 i = 1
 x = 06650/10000 = 0;
 res = 80;
  so 980. Good.

 +        for (i = 0; i < exp; ++i) {
 +            x = scale1 / pow_10(5 - i);
 +            res += (pow_10(exp - 1 - i) * x);
 +            scale1 = scale1 % pow_10(5 - i);
 +        }
 +        *val0 += res;
 +            *val1 = scale1 * pow_10(exp);

 val1 = 665000 which is correct with the trailing zeros in val2.
 So your code is good but examples need those zeros!

 Impressive, Absolutely great review. I will correct comments in the 
 next version.

 Given this was the only issue in the patch set I'll just fix it up as 
 I merge
 it.

 It's been a couple of weeks an noone else has chipped in on this 

series, so
 I'm taking it now.




 Thanks,
 Srinivas



 +    } else if (exp < 0) {
 +        exp = abs(exp);
 +        if (exp > 6) {
 +            *val0 = *val1 = 0;
 +            return;
 +        }
 +        *val0 = scale0 / pow_10(exp);
 +        rem = scale0 % pow_10(exp);
 +        res = 0;
 +        for (i = 0; i < (6 - exp); ++i) {
 +            x = scale1 / pow_10(5 - i);
 +            res += (pow_10(5 - exp - i) * x);
 +            scale1 = scale1 % pow_10(5 - i);
 +        }
 +        *val1 = rem * pow_10(6 -
exp) + res;
 +    } else {
 +        *val0 = scale0;
 +        *val1 = scale1;
 +    }
 +}
 +
 +int hid_sensor_format_scale(u32 usage_id,
 +            struct hid_sensor_hub_attribute_info *attr_info,
 +            int *val0, int *val1)
 +{
 +    int i;
 +    int exp;
 +
 +    *val0 = 1;
 +    *val1 = 0;
 +
 +    for (i = 0; ARRAY_SIZE(unit_conversion); ++i) {
 +        if (unit_conversion[i].usage_id == usage_id &&
 +            unit_conversion[i].unit == attr_info->units) {
 +            exp  = hid_sensor_convert_exponent(
 +                        attr_info->unit_expo);
 +            adjust_exponent_micro(val0, val1,
 +                    unit_conversion[i].scale_val0,
 +                    unit_conversion[i].scale_val1, exp);
 +            break;
 +        }
 +    }
 +
 +    return IIO_VAL_INT_PLUS_MICRO;
 +}

+EXPORT_SYMBOL(hid_sensor_format_scale);
 +
   int hid_sensor_parse_common_attributes(struct 
 hid_sensor_hub_device *hsdev,
                       u32 usage_id,
                       struct hid_sensor_common *st)
 diff --git a/include/linux/hid-sensor-hub.h 
 b/include/linux/hid-sensor-hub.h
 index b70cfd7..89626b2 100644
 --- a/include/linux/hid-sensor-hub.h
 +++ b/include/linux/hid-sensor-hub.h
 @@ -223,4 +223,8 @@ int hid_sensor_read_samp_freq_value(struct 
 hid_sensor_common *st,
   int hid_sensor_get_usage_index(struct hid_sensor_hub_device *hsdev,
                   u32 report_id, int field_index, u32 usage_id);

 +int hid_sensor_format_scale(u32 usage_id,
 +                struct hid_sensor_hub_attribute_info *attr_info,
 +                int *val0, int *val1);
 +
   #endif


 -- 
 To unsubscribe from this list: send the line "unsubscribe linux-iio"
in
 the body of a message to majordomo@vger.kernel.org
 More majordomo info at http://vger.kernel.org/majordomo-info.html





--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

--------------080304010807030700020202--