Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx> wrote: >On 10/24/2013 09:20 AM, Jonathan Cameron wrote: >> On 10/24/13 16:17, Srinivas Pandruvada wrote: >>> Hi Jonathan, >>> On 10/24/2013 04:18 AM, Jonathan Cameron wrote: >>>> On 10/23/13 20:11, Srinivas Pandruvada wrote: >>>>> A quaternion is composed of four components: a vector with x, y, z >>>>> coordinates and a w rotation. Added channel modifiers for >exporting >>>>> these four components via user space. >>>>> >>>>> Signed-off-by: Srinivas Pandruvada ><srinivas.pandruvada@xxxxxxxxxxxxxxx> >>>> Quaternions are one of the 'interesting' cases that make me wonder >if we need >>>> a slightly more generic form of read_raw. The quaternion only has >meaning >>>> with an associated scale. Hence you really have to guarantee that >you can grab all >>>> 4 components together. >What if we add two members in iio_chan_info: >IIO_CHAN_INFO_TRIG_RAW_SYNC (WR): To trigger a read of and store >sample >in internal memory >(As per HID sensor spec you have to read/write all quaternion >components >together, data is also packed together. ) >IIO_CHAN_INFO_RAW_SYNC (RD): To return individual channel raw data >captured during IIO_CHAN_INFO_TRIG_RAW_SYNC >In this way four reads will give all component data but they are >related >to each other. That is a bit ugly. I would rather consider a single channel that outputs all 4 components and change the handling to cope. If people want synchronized data across multiple normal channels then they should be using the buffer... >>>> >>>> Do we need to rethink how the read callback works? >>>> >>>> Mind you, if we do that then we need to extend the description of >the buffer element >>>> in the scan_mask. We could define a compound description for a >channel, but that is 'interesting'. >For scan elements we have to specify a way so that the scan_elem >doesn't >contain per channel enable/disable. >We create one global channel enable sysfs entry. This is what you are >thinking? No. Either we have a single channel which contains all 4 components or just handle them separately. If anyone requests only some of them from user space it does not matter to us even if the data will be useless. > >Thanks, >Srinivas > > >>>> I also don't like this being defined as type IIO_ROT - the units >(such as exist are different). >>>> See next patch for what I'm referring to here. >>> I didn't receive your next patch with comments. Can you please >resend? >>> Thanks, >> Sorry for not being clear, I simply meant the IIO_ROT bit was in the >next batch, >> but as it is connected to my comment here, didn't want to start >another discussion >> in reply to that patch. >> >> (hence I never replied to that patch) >>> Srinivas >>>> I've cc'd a few people who might have views on this. >>>> >>>> >>>> >>>>> --- >>>>> drivers/iio/industrialio-core.c | 4 ++++ >>>>> include/linux/iio/types.h | 4 ++++ >>>>> 2 files changed, 8 insertions(+) >>>>> >>>>> diff --git a/drivers/iio/industrialio-core.c >b/drivers/iio/industrialio-core.c >>>>> index f95c697..4ffaead 100644 >>>>> --- a/drivers/iio/industrialio-core.c >>>>> +++ b/drivers/iio/industrialio-core.c >>>>> @@ -80,6 +80,10 @@ static const char * const iio_modifier_names[] >= { >>>>> [IIO_MOD_LIGHT_RED] = "red", >>>>> [IIO_MOD_LIGHT_GREEN] = "green", >>>>> [IIO_MOD_LIGHT_BLUE] = "blue", >>>>> + [IIO_MOD_QUATERNION_X] = "quat_x", >>>>> + [IIO_MOD_QUATERNION_Y] = "quat_y", >>>>> + [IIO_MOD_QUATERNION_Z] = "quat_z", >>>>> + [IIO_MOD_QUATERNION_W] = "quat_w", >>>>> }; >>>>> /* relies on pairs of these shared then separate */ >>>>> diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h >>>>> index 88bf0f0..ac2345c 100644 >>>>> --- a/include/linux/iio/types.h >>>>> +++ b/include/linux/iio/types.h >>>>> @@ -52,6 +52,10 @@ enum iio_modifier { >>>>> IIO_MOD_LIGHT_RED, >>>>> IIO_MOD_LIGHT_GREEN, >>>>> IIO_MOD_LIGHT_BLUE, >>>>> + IIO_MOD_QUATERNION_X, >>>>> + IIO_MOD_QUATERNION_Y, >>>>> + IIO_MOD_QUATERNION_Z, >>>>> + IIO_MOD_QUATERNION_W, >>>>> }; >>>>> #define IIO_VAL_INT 1 >>>>> >>> -- >>> 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 > >-- >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-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html