Re: [Patch v3 5/6] iio: hid-sensors: Added device rotation support

From: Jonathan Cameron
Date: Mon Apr 14 2014 - 04:56:21 EST




On April 14, 2014 2:48:13 AM GMT+01:00, Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx> wrote:
>
>On 04/12/2014 10:21 AM, Jonathan Cameron wrote:
>> On 09/04/14 01:56, Srinivas Pandruvada wrote:
>>> Added usage id processing for device rotation. This uses IIO
>>> interfaces for triggered buffer to present data to user
>>> mode.This uses HID sensor framework for registering callback
>>> events from the sensor hub.
>>> Data is exported to user space in the form of quaternion rotation
>>> format.
>>>
>>> Signed-off-by: Srinivas Pandruvada
><srinivas.pandruvada@xxxxxxxxxxxxxxx>
>> Very nice. Happy to take this the moment we have a go ahead on
>> the devm_kmemdup and perhaps those minor tweaks I asked for in the
>> multi value handling patch.
>>
>> Actually, may seem a random question, but what the heck are the scale
>> units you can read for the quaternion? Firstly it's an integer, so
>> would only make the value bigger, and secondly we are dealing with
>> a quaternion, which is inherently scale free (when used for rotation
>> anyway).
>> I suppose these units might be meant to transform to a unit
>quaternion
>> (though this can be easily established form the quaternion itself).
>>
>Good point. For quaternion there is no scale exported in channel spec.
>I
>have in switch case, which was left over, I will remove it. The offset
>field mostly will be 0, but as you pointed out below they are unit
>exponent.
>
>
>> Looking quickly at the other HID drivers, I am a little confused as
>> to what is going on in general. Could you talk me through this stuff?
>> I have a vague recollection we went through this before, but can't
>> recall the result of those discussions.
>>
>> At first glance, it looks like scale is being used to indicate the
>> base units (via magic numbers) and offset to contain exponents
>> to be applied also as magic numbers. Neither of these is anywhere
>> near our ABI which is going to cause issues for any 'standard'
>> userspace library..
>>
>We had this discussion long time back. Initially introduced separate
>attributes for units ans scale but suggestion was to use offset and
>scale.
>
>Currently these drivers are used only by Android kernel, which has
>corresponding user space to interpret.
>But now since after Win8, they became available in many devices.
>I have received questions on this from user space developers now, so I
>have to either document them or make complaint to others. I am working
>on it. I have many units to convert to standard format. I will submit a
>
>change for this.

Good to know you have this in hand.

Thanks

Jonathan
>
>Thanks,
>Srinivas
>
>> Thanks.
>>
>> Jonathan
>>> ---
>>> drivers/iio/orientation/Kconfig | 12 +
>>> drivers/iio/orientation/Makefile | 1 +
>>> drivers/iio/orientation/hid-sensor-rotation.c | 359
>>> ++++++++++++++++++++++++++
>>> include/linux/hid-sensor-ids.h | 1 +
>>> 4 files changed, 373 insertions(+)
>>> create mode 100644 drivers/iio/orientation/hid-sensor-rotation.c
>>>
>>> diff --git a/drivers/iio/orientation/Kconfig
>>> b/drivers/iio/orientation/Kconfig
>>> index 58c62c8..e3aa1e5 100644
>>> --- a/drivers/iio/orientation/Kconfig
>>> +++ b/drivers/iio/orientation/Kconfig
>>> @@ -16,4 +16,16 @@ config HID_SENSOR_INCLINOMETER_3D
>>> Say yes here to build support for the HID SENSOR
>>> Inclinometer 3D.
>>>
>>> +config HID_SENSOR_DEVICE_ROTATION
>>> + depends on HID_SENSOR_HUB
>>> + select IIO_BUFFER
>>> + select IIO_TRIGGERED_BUFFER
>>> + select HID_SENSOR_IIO_COMMON
>>> + select HID_SENSOR_IIO_TRIGGER
>>> + tristate "HID Device Rotation"
>>> + help
>>> + Say yes here to build support for the HID SENSOR
>>> + device rotation. The output of a device rotation sensor
>>> + is presented using quaternion format.
>>> +
>>> endmenu
>>> diff --git a/drivers/iio/orientation/Makefile
>>> b/drivers/iio/orientation/Makefile
>>> index 2c97572..4734dab 100644
>>> --- a/drivers/iio/orientation/Makefile
>>> +++ b/drivers/iio/orientation/Makefile
>>> @@ -4,3 +4,4 @@
>>>
>>> # When adding new entries keep the list in alphabetical order
>>> obj-$(CONFIG_HID_SENSOR_INCLINOMETER_3D) += hid-sensor-incl-3d.o
>>> +obj-$(CONFIG_HID_SENSOR_DEVICE_ROTATION) += hid-sensor-rotation.o
>>> diff --git a/drivers/iio/orientation/hid-sensor-rotation.c
>>> b/drivers/iio/orientation/hid-sensor-rotation.c
>>> new file mode 100644
>>> index 0000000..5c7d558
>>> --- /dev/null
>>> +++ b/drivers/iio/orientation/hid-sensor-rotation.c
>>> @@ -0,0 +1,359 @@
>>> +/*
>>> + * HID Sensors Driver
>>> + * Copyright (c) 2014, Intel Corporation.
>>> + *
>>> + * This program is free software; you can redistribute it and/or
>>> modify it
>>> + * under the terms and conditions of the GNU General Public
>License,
>>> + * version 2, as published by the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope it will be useful, but
>>> WITHOUT
>>> + * ANY WARRANTY; without even the implied warranty of
>>> MERCHANTABILITY or
>>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public
>>> License for
>>> + * more details.
>>> + */
>>> +
>>> +#include <linux/device.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/module.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/irq.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/hid-sensor-hub.h>
>>> +#include <linux/iio/iio.h>
>>> +#include <linux/iio/sysfs.h>
>>> +#include <linux/iio/buffer.h>
>>> +#include <linux/iio/trigger_consumer.h>
>>> +#include <linux/iio/triggered_buffer.h>
>>> +#include "../common/hid-sensors/hid-sensor-trigger.h"
>>> +
>>> +struct dev_rot_state {
>>> + struct hid_sensor_hub_callbacks callbacks;
>>> + struct hid_sensor_common common_attributes;
>>> + struct hid_sensor_hub_attribute_info quaternion;
>>> + u32 sampled_vals[4];
>>> +};
>>> +
>>> +/* Channel definitions */
>>> +static const struct iio_chan_spec dev_rot_channels[] = {
>>> + {
>>> + .type = IIO_ROT,
>>> + .modified = 1,
>>> + .channel2 = IIO_MOD_QUATERNION,
>>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>>> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
>>> + BIT(IIO_CHAN_INFO_SAMP_FREQ) |
>>> + BIT(IIO_CHAN_INFO_HYSTERESIS) |
>>> + BIT(IIO_CHAN_INFO_RAW),
>>> + }
>>> +};
>>> +
>>> +/* Adjust channel real bits based on report descriptor */
>>> +static void dev_rot_adjust_channel_bit_mask(struct iio_chan_spec
>*chan,
>>> + int size)
>>> +{
>>> + chan->scan_type.sign = 's';
>>> + /* Real storage bits will change based on the report desc. */
>>> + chan->scan_type.realbits = size * 8;
>>> + /* Maximum size of a sample to capture is u32 */
>>> + chan->scan_type.storagebits = sizeof(u32) * 8;
>>> + chan->scan_type.repeat = 4;
>>> +}
>>> +
>>> +/* Channel read_raw handler */
>>> +static int dev_rot_read_raw(struct iio_dev *indio_dev,
>>> + struct iio_chan_spec const *chan,
>>> + int size, int *vals, int *val_len,
>>> + long mask)
>>> +{
>>> + struct dev_rot_state *rot_state = iio_priv(indio_dev);
>>> + int ret_type;
>>> + int i;
>>> +
>>> + vals[0] = 0;
>>> + vals[1] = 0;
>>> +
>>> + switch (mask) {
>>> + case IIO_CHAN_INFO_RAW:
>>> + if (size >= 4) {
>>> + for (i = 0; i < 4; ++i)
>>> + vals[i] = rot_state->sampled_vals[i];
>>> + ret_type = IIO_VAL_INT_MULTIPLE;
>>> + *val_len = 4;
>>> + } else
>>> + ret_type = -EINVAL;
>>> + break;
>>> + case IIO_CHAN_INFO_SCALE:
>>> + vals[0] = rot_state->quaternion.units;
>>> + ret_type = IIO_VAL_INT;
>>> + break;
>>> + case IIO_CHAN_INFO_OFFSET:
>>> + vals[1] = hid_sensor_convert_exponent(
>>> + rot_state->quaternion.unit_expo);
>>> + ret_type = IIO_VAL_INT;
>>> + break;
>>> + case IIO_CHAN_INFO_SAMP_FREQ:
>>> + ret_type = hid_sensor_read_samp_freq_value(
>>> + &rot_state->common_attributes, &vals[0], &vals[1]);
>>> + break;
>>> + case IIO_CHAN_INFO_HYSTERESIS:
>>> + ret_type = hid_sensor_read_raw_hyst_value(
>>> + &rot_state->common_attributes, &vals[0], &vals[1]);
>>> + break;
>>> + default:
>>> + ret_type = -EINVAL;
>>> + break;
>>> + }
>>> +
>>> + return ret_type;
>>> +}
>>> +
>>> +/* Channel write_raw handler */
>>> +static int dev_rot_write_raw(struct iio_dev *indio_dev,
>>> + struct iio_chan_spec const *chan,
>>> + int val,
>>> + int val2,
>>> + long mask)
>>> +{
>>> + struct dev_rot_state *rot_state = iio_priv(indio_dev);
>>> + int ret;
>>> +
>>> + switch (mask) {
>>> + case IIO_CHAN_INFO_SAMP_FREQ:
>>> + ret = hid_sensor_write_samp_freq_value(
>>> + &rot_state->common_attributes, val, val2);
>>> + break;
>>> + case IIO_CHAN_INFO_HYSTERESIS:
>>> + ret = hid_sensor_write_raw_hyst_value(
>>> + &rot_state->common_attributes, val, val2);
>>> + break;
>>> + default:
>>> + ret = -EINVAL;
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static const struct iio_info dev_rot_info = {
>>> + .driver_module = THIS_MODULE,
>>> + .read_raw_multi = &dev_rot_read_raw,
>>> + .write_raw = &dev_rot_write_raw,
>>> +};
>>> +
>>> +/* Function to push data to buffer */
>>> +static void hid_sensor_push_data(struct iio_dev *indio_dev, u8
>>> *data, int len)
>>> +{
>>> + dev_dbg(&indio_dev->dev, "hid_sensor_push_data >>\n");
>>> + iio_push_to_buffers(indio_dev, (u8 *)data);
>>> + dev_dbg(&indio_dev->dev, "hid_sensor_push_data <<\n");
>>> +
>>> +}
>>> +
>>> +/* Callback handler to send event after all samples are received
>and
>>> captured */
>>> +static int dev_rot_proc_event(struct hid_sensor_hub_device *hsdev,
>>> + unsigned usage_id,
>>> + void *priv)
>>> +{
>>> + struct iio_dev *indio_dev = platform_get_drvdata(priv);
>>> + struct dev_rot_state *rot_state = iio_priv(indio_dev);
>>> +
>>> + dev_dbg(&indio_dev->dev, "dev_rot_proc_event [%d]\n",
>>> + rot_state->common_attributes.data_ready);
>>> +
>>> + if (rot_state->common_attributes.data_ready)
>>> + hid_sensor_push_data(indio_dev,
>>> + (u8 *)rot_state->sampled_vals,
>>> + sizeof(rot_state->sampled_vals));
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/* Capture samples in local storage */
>>> +static int dev_rot_capture_sample(struct hid_sensor_hub_device
>*hsdev,
>>> + unsigned usage_id,
>>> + size_t raw_len, char *raw_data,
>>> + void *priv)
>>> +{
>>> + struct iio_dev *indio_dev = platform_get_drvdata(priv);
>>> + struct dev_rot_state *rot_state = iio_priv(indio_dev);
>>> +
>>> + if (usage_id == HID_USAGE_SENSOR_ORIENT_QUATERNION) {
>>> + memcpy(rot_state->sampled_vals, raw_data,
>>> + sizeof(rot_state->sampled_vals));
>>> + dev_dbg(&indio_dev->dev, "Recd Quat len:%zu::%zu\n",
>raw_len,
>>> + sizeof(rot_state->sampled_vals));
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/* Parse report which is specific to an usage id*/
>>> +static int dev_rot_parse_report(struct platform_device *pdev,
>>> + struct hid_sensor_hub_device *hsdev,
>>> + struct iio_chan_spec *channels,
>>> + unsigned usage_id,
>>> + struct dev_rot_state *st)
>>> +{
>>> + int ret;
>>> +
>>> + ret = sensor_hub_input_get_attribute_info(hsdev,
>>> + HID_INPUT_REPORT,
>>> + usage_id,
>>> + HID_USAGE_SENSOR_ORIENT_QUATERNION,
>>> + &st->quaternion);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + dev_rot_adjust_channel_bit_mask(&channels[0],
>>> + st->quaternion.size / 4);
>>> +
>>> + dev_dbg(&pdev->dev, "dev_rot %x:%x\n", st->quaternion.index,
>>> + st->quaternion.report_id);
>>> +
>>> + dev_dbg(&pdev->dev, "dev_rot: attrib size %d\n",
>>> + st->quaternion.size);
>>> +
>>> + /* Set Sensitivity field ids, when there is no individual
>>> modifier */
>>> + if (st->common_attributes.sensitivity.index < 0) {
>>> + sensor_hub_input_get_attribute_info(hsdev,
>>> + HID_FEATURE_REPORT, usage_id,
>>> + HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS |
>>> + HID_USAGE_SENSOR_DATA_ORIENTATION,
>>> + &st->common_attributes.sensitivity);
>>> + dev_dbg(&pdev->dev, "Sensitivity index:report %d:%d\n",
>>> + st->common_attributes.sensitivity.index,
>>> + st->common_attributes.sensitivity.report_id);
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/* Function to initialize the processing for usage id */
>>> +static int hid_dev_rot_probe(struct platform_device *pdev)
>>> +{
>>> + int ret;
>>> + static char *name = "dev_rotation";
>>> + struct iio_dev *indio_dev;
>>> + struct dev_rot_state *rot_state;
>>> + struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
>>> + struct iio_chan_spec *channels;
>>> +
>>> + indio_dev = devm_iio_device_alloc(&pdev->dev,
>>> + sizeof(struct dev_rot_state));
>>> + if (indio_dev == NULL)
>>> + return -ENOMEM;
>>> +
>>> + platform_set_drvdata(pdev, indio_dev);
>>> +
>>> + rot_state = iio_priv(indio_dev);
>>> + rot_state->common_attributes.hsdev = hsdev;
>>> + rot_state->common_attributes.pdev = pdev;
>>> +
>>> + ret = hid_sensor_parse_common_attributes(hsdev,
>>> + HID_USAGE_SENSOR_DEVICE_ORIENTATION,
>>> + &rot_state->common_attributes);
>>> + if (ret) {
>>> + dev_err(&pdev->dev, "failed to setup common attributes\n");
>>> + return ret;
>>> + }
>>> +
>>> + channels = devm_kmemdup(&pdev->dev, dev_rot_channels,
>>> sizeof(dev_rot_channels),
>>> + GFP_KERNEL);
>>> + if (!channels) {
>>> + dev_err(&pdev->dev, "failed to duplicate channels\n");
>>> + return -ENOMEM;
>>> + }
>>> +
>>> + ret = dev_rot_parse_report(pdev, hsdev, channels,
>>> + HID_USAGE_SENSOR_DEVICE_ORIENTATION, rot_state);
>>> + if (ret) {
>>> + dev_err(&pdev->dev, "failed to setup attributes\n");
>>> + return ret;
>>> + }
>>> +
>>> + indio_dev->channels = channels;
>>> + indio_dev->num_channels = ARRAY_SIZE(dev_rot_channels);
>>> + indio_dev->dev.parent = &pdev->dev;
>>> + indio_dev->info = &dev_rot_info;
>>> + indio_dev->name = name;
>>> + indio_dev->modes = INDIO_DIRECT_MODE;
>>> +
>>> + ret = iio_triggered_buffer_setup(indio_dev,
>>> &iio_pollfunc_store_time,
>>> + NULL, NULL);
>>> + if (ret) {
>>> + dev_err(&pdev->dev, "failed to initialize trigger
>buffer\n");
>>> + return ret;
>>> + }
>>> + rot_state->common_attributes.data_ready = false;
>>> + ret = hid_sensor_setup_trigger(indio_dev, name,
>>> + &rot_state->common_attributes);
>>> + if (ret) {
>>> + dev_err(&pdev->dev, "trigger setup failed\n");
>>> + goto error_unreg_buffer_funcs;
>>> + }
>>> +
>>> + ret = iio_device_register(indio_dev);
>>> + if (ret) {
>>> + dev_err(&pdev->dev, "device register failed\n");
>>> + goto error_remove_trigger;
>>> + }
>>> +
>>> + rot_state->callbacks.send_event = dev_rot_proc_event;
>>> + rot_state->callbacks.capture_sample = dev_rot_capture_sample;
>>> + rot_state->callbacks.pdev = pdev;
>>> + ret = sensor_hub_register_callback(hsdev,
>>> + HID_USAGE_SENSOR_DEVICE_ORIENTATION,
>>> + &rot_state->callbacks);
>>> + if (ret) {
>>> + dev_err(&pdev->dev, "callback reg failed\n");
>>> + goto error_iio_unreg;
>>> + }
>>> +
>>> + return 0;
>>> +
>>> +error_iio_unreg:
>>> + iio_device_unregister(indio_dev);
>>> +error_remove_trigger:
>>> + hid_sensor_remove_trigger(&rot_state->common_attributes);
>>> +error_unreg_buffer_funcs:
>>> + iio_triggered_buffer_cleanup(indio_dev);
>>> + return ret;
>>> +}
>>> +
>>> +/* Function to deinitialize the processing for usage id */
>>> +static int hid_dev_rot_remove(struct platform_device *pdev)
>>> +{
>>> + struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
>>> + struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>>> + struct dev_rot_state *rot_state = iio_priv(indio_dev);
>>> +
>>> + sensor_hub_remove_callback(hsdev,
>>> HID_USAGE_SENSOR_DEVICE_ORIENTATION);
>>> + iio_device_unregister(indio_dev);
>>> + hid_sensor_remove_trigger(&rot_state->common_attributes);
>>> + iio_triggered_buffer_cleanup(indio_dev);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static struct platform_device_id hid_dev_rot_ids[] = {
>>> + {
>>> + /* Format: HID-SENSOR-usage_id_in_hex_lowercase */
>>> + .name = "HID-SENSOR-20008a",
>>> + },
>>> + { /* sentinel */ }
>>> +};
>>> +MODULE_DEVICE_TABLE(platform, hid_dev_rot_ids);
>>> +
>>> +static struct platform_driver hid_dev_rot_platform_driver = {
>>> + .id_table = hid_dev_rot_ids,
>>> + .driver = {
>>> + .name = KBUILD_MODNAME,
>>> + .owner = THIS_MODULE,
>>> + },
>>> + .probe = hid_dev_rot_probe,
>>> + .remove = hid_dev_rot_remove,
>>> +};
>>> +module_platform_driver(hid_dev_rot_platform_driver);
>>> +
>>> +MODULE_DESCRIPTION("HID Sensor Device Rotation");
>>> +MODULE_AUTHOR("Srinivas Pandruvada
>>> <srinivas.pandruvada@xxxxxxxxxxxxxxx>");
>>> +MODULE_LICENSE("GPL");
>>> diff --git a/include/linux/hid-sensor-ids.h
>>> b/include/linux/hid-sensor-ids.h
>>> index 14ead9e..109f0e6 100644
>>> --- a/include/linux/hid-sensor-ids.h
>>> +++ b/include/linux/hid-sensor-ids.h
>>> @@ -76,6 +76,7 @@
>>> #define HID_USAGE_SENSOR_ORIENT_TILT_Y 0x200480
>>> #define HID_USAGE_SENSOR_ORIENT_TILT_Z 0x200481
>>>
>>> +#define HID_USAGE_SENSOR_DEVICE_ORIENTATION 0x20008A
>>> #define HID_USAGE_SENSOR_ORIENT_ROTATION_MATRIX 0x200482
>>> #define HID_USAGE_SENSOR_ORIENT_QUATERNION 0x200483
>>> #define HID_USAGE_SENSOR_ORIENT_MAGN_FLUX 0x200484
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio"
>in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>

--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/