Re: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages
On June 10, 2014 6:33:25 PM GMT+01:00, Reyad Attiyat
wrote:
>Hey Jonathan,
>
>I will make the changes you suggested thanks for reviewing this. Any
>conclusions on the naming, did you want me to change the sysfs name to
>"in_rot_from_north_true/magnetic"?
Yes. I think that is the cleanest option from those we have considered!
>
>On Mon, Jun 9, 2014 at 2:55 PM, Jonathan Cameron
>wrote:
>> On 03/06/14 00:14, Reyad Attiyat wrote:
>>>
>>> Updated magn_3d_channel enum for all possible north channels
>>>
>>> Added functions to setup iio_chan_spec array depending on which hid
>usage
>>> reports are found
>>>
>>> Renamed magn_val to iio_val to differentiate the index being used
>>>
>>> Updated magn_3d_state struct to hold pointer array (magn_val_addr[])
>which
>>> points to iio_val[]
>>>
>>> Updated magn_3d_parse_report to scan for all compass usages and
>create iio
>>> channels for each
>>>
>>> Signed-off-by: Reyad Attiyat
>>
>> Hi Reyad,
>>
>> I've taken a rather quick look at this. Will probably want to take
>> one more close look at a newer version.
>>
>> Various bits and bobs inline - mostly fine!
>> Jonathan
>>
>>> ---
>>> drivers/iio/magnetometer/hid-sensor-magn-3d.c | 271
>>> +-
>>> 1 file changed, 177 insertions(+), 94 deletions(-)
>>>
>>> diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
>>> b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
>>> index 6d162b7..32f4d90 100644
>>> --- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
>>> +++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
>>> @@ -34,63 +34,54 @@ enum magn_3d_channel {
>>> CHANNEL_SCAN_INDEX_X,
>>> CHANNEL_SCAN_INDEX_Y,
>>> CHANNEL_SCAN_INDEX_Z,
>>> + CHANNEL_SCAN_INDEX_NORTH_MAGN,
>>> + CHANNEL_SCAN_INDEX_NORTH_TRUE,
>>> + CHANNEL_SCAN_INDEX_NORTH_TILT_COMP,
>>> + CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP,
>>> MAGN_3D_CHANNEL_MAX,
>>> };
>>>
>>> +#define IIO_CHANNEL_MAX MAGN_3D_CHANNEL_MAX
>>
>> Please don't define a generic sounding name for a local
>> constant. Why not use MAGN_3D_CHANNEL_MAX everywhere?
>>
>>> +
>>> struct magn_3d_state {
>>> struct hid_sensor_hub_callbacks callbacks;
>>> struct hid_sensor_common common_attributes;
>>> struct hid_sensor_hub_attribute_info
>magn[MAGN_3D_CHANNEL_MAX];
>>> - u32 magn_val[MAGN_3D_CHANNEL_MAX];
>>> -};
>>> + u32 *magn_val_addr[MAGN_3D_CHANNEL_MAX];
>>>
>>> -static const u32 magn_3d_addresses[MAGN_3D_CHANNEL_MAX] = {
>>> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS,
>>> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS,
>>> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS
>>> + u32 iio_val[IIO_CHANNEL_MAX];
>>> + int num_iio_channels;
>>> };
>>>
>>> -/* Channel definitions */
>>> -static const struct iio_chan_spec magn_3d_channels[] = {
>>> - {
>>> - .type = IIO_MAGN,
>>> - .modified = 1,
>>> - .channel2 = IIO_MOD_X,
>>> - .info_mask_shared_by_type =
>BIT(IIO_CHAN_INFO_OFFSET) |
>>> - BIT(IIO_CHAN_INFO_SCALE) |
>>> - BIT(IIO_CHAN_INFO_SAMP_FREQ) |
>>> - BIT(IIO_CHAN_INFO_HYSTERESIS),
>>> - .scan_index = CHANNEL_SCAN_INDEX_X,
>>> - }, {
>>> - .type = IIO_MAGN,
>>> - .modified = 1,
>>> - .channel2 = IIO_MOD_Y,
>>> - .info_mask_shared_by_type =
>BIT(IIO_CHAN_INFO_OFFSET) |
>>> - BIT(IIO_CHAN_INFO_SCALE) |
>>> - BIT(IIO_CHAN_INFO_SAMP_FREQ) |
>>> - BIT(IIO_CHAN_INFO_HYSTERESIS),
>>> - .scan_index = CHANNEL_SCAN_INDEX_Y,
>>> - }, {
>>> - .type = IIO_MAGN,
>>> - .modified = 1,
>>> - .channel2 = IIO_MOD_Z,
>>> - .info_mask_shared_by_type =
>BIT(IIO_CHAN_INFO_OFFSET) |
>>> - BIT(IIO_CHAN_INFO_SCALE) |
>>> - BIT(IIO_CHAN_INFO_SAMP_FREQ) |
>>> - BIT(IIO_CHAN_INFO_HYSTERESIS),
>>> - .scan_index = CHANNEL_SCAN_INDEX_Z,
>>> +/* Find index into magn_3d_state magn[] and magn_val_addr[] from
>HID
>>> Usage */
>>> +static int magn_3d_usage_id_to_chan_index(unsigned usage_id){
>>> + int offset = -1;
>>
>> I'd personally prefer offset = -EINVAL and to have it assigned as
>> a default element in the switch statement rather that here.
>>
>>> +
>>> + switch (usage_id) {
>>> + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS:
>>> + offset = CHANNEL_SCAN_INDEX_X;
>>> + break;
>>> + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS:
>>> + offset = CHANNEL_SCAN_INDEX_Y;
>>> + break;
>>> + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS:
>>> + offset = CHANNEL_SCAN_INDEX_Z;
>>> + break;
>>> + case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH:
>>> + offset = CHANNEL_SCAN_INDEX_NORTH_TILT_COMP;
>>> +
Re: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages
Hey Jonathan,
I will make the changes you suggested thanks for reviewing this. Any
conclusions on the naming, did you want me to change the sysfs name to
"in_rot_from_north_true/magnetic"?
On Mon, Jun 9, 2014 at 2:55 PM, Jonathan Cameron wrote:
> On 03/06/14 00:14, Reyad Attiyat wrote:
>>
>> Updated magn_3d_channel enum for all possible north channels
>>
>> Added functions to setup iio_chan_spec array depending on which hid usage
>> reports are found
>>
>> Renamed magn_val to iio_val to differentiate the index being used
>>
>> Updated magn_3d_state struct to hold pointer array (magn_val_addr[]) which
>> points to iio_val[]
>>
>> Updated magn_3d_parse_report to scan for all compass usages and create iio
>> channels for each
>>
>> Signed-off-by: Reyad Attiyat
>
> Hi Reyad,
>
> I've taken a rather quick look at this. Will probably want to take
> one more close look at a newer version.
>
> Various bits and bobs inline - mostly fine!
> Jonathan
>
>> ---
>> drivers/iio/magnetometer/hid-sensor-magn-3d.c | 271
>> +-
>> 1 file changed, 177 insertions(+), 94 deletions(-)
>>
>> diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
>> b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
>> index 6d162b7..32f4d90 100644
>> --- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
>> +++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
>> @@ -34,63 +34,54 @@ enum magn_3d_channel {
>> CHANNEL_SCAN_INDEX_X,
>> CHANNEL_SCAN_INDEX_Y,
>> CHANNEL_SCAN_INDEX_Z,
>> + CHANNEL_SCAN_INDEX_NORTH_MAGN,
>> + CHANNEL_SCAN_INDEX_NORTH_TRUE,
>> + CHANNEL_SCAN_INDEX_NORTH_TILT_COMP,
>> + CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP,
>> MAGN_3D_CHANNEL_MAX,
>> };
>>
>> +#define IIO_CHANNEL_MAX MAGN_3D_CHANNEL_MAX
>
> Please don't define a generic sounding name for a local
> constant. Why not use MAGN_3D_CHANNEL_MAX everywhere?
>
>> +
>> struct magn_3d_state {
>> struct hid_sensor_hub_callbacks callbacks;
>> struct hid_sensor_common common_attributes;
>> struct hid_sensor_hub_attribute_info magn[MAGN_3D_CHANNEL_MAX];
>> - u32 magn_val[MAGN_3D_CHANNEL_MAX];
>> -};
>> + u32 *magn_val_addr[MAGN_3D_CHANNEL_MAX];
>>
>> -static const u32 magn_3d_addresses[MAGN_3D_CHANNEL_MAX] = {
>> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS,
>> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS,
>> - HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS
>> + u32 iio_val[IIO_CHANNEL_MAX];
>> + int num_iio_channels;
>> };
>>
>> -/* Channel definitions */
>> -static const struct iio_chan_spec magn_3d_channels[] = {
>> - {
>> - .type = IIO_MAGN,
>> - .modified = 1,
>> - .channel2 = IIO_MOD_X,
>> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
>> - BIT(IIO_CHAN_INFO_SCALE) |
>> - BIT(IIO_CHAN_INFO_SAMP_FREQ) |
>> - BIT(IIO_CHAN_INFO_HYSTERESIS),
>> - .scan_index = CHANNEL_SCAN_INDEX_X,
>> - }, {
>> - .type = IIO_MAGN,
>> - .modified = 1,
>> - .channel2 = IIO_MOD_Y,
>> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
>> - BIT(IIO_CHAN_INFO_SCALE) |
>> - BIT(IIO_CHAN_INFO_SAMP_FREQ) |
>> - BIT(IIO_CHAN_INFO_HYSTERESIS),
>> - .scan_index = CHANNEL_SCAN_INDEX_Y,
>> - }, {
>> - .type = IIO_MAGN,
>> - .modified = 1,
>> - .channel2 = IIO_MOD_Z,
>> - .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
>> - BIT(IIO_CHAN_INFO_SCALE) |
>> - BIT(IIO_CHAN_INFO_SAMP_FREQ) |
>> - BIT(IIO_CHAN_INFO_HYSTERESIS),
>> - .scan_index = CHANNEL_SCAN_INDEX_Z,
>> +/* Find index into magn_3d_state magn[] and magn_val_addr[] from HID
>> Usage */
>> +static int magn_3d_usage_id_to_chan_index(unsigned usage_id){
>> + int offset = -1;
>
> I'd personally prefer offset = -EINVAL and to have it assigned as
> a default element in the switch statement rather that here.
>
>> +
>> + switch (usage_id) {
>> + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS:
>> + offset = CHANNEL_SCAN_INDEX_X;
>> + break;
>> + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS:
>> + offset = CHANNEL_SCAN_INDEX_Y;
>> + break;
>> + case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS:
>> + offset = CHANNEL_SCAN_INDEX_Z;
>> + break;
>> + case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH:
>> + offset = CHANNEL_SCAN_INDEX_NORTH_TILT_COMP;
>> + break;
>> + case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH:
>> + offset = CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP;
>> + break;
>> + case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH:
>> + offset =
Re: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages
Hey Jonathan,
I will make the changes you suggested thanks for reviewing this. Any
conclusions on the naming, did you want me to change the sysfs name to
in_rot_from_north_true/magnetic?
On Mon, Jun 9, 2014 at 2:55 PM, Jonathan Cameron ji...@kernel.org wrote:
On 03/06/14 00:14, Reyad Attiyat wrote:
Updated magn_3d_channel enum for all possible north channels
Added functions to setup iio_chan_spec array depending on which hid usage
reports are found
Renamed magn_val to iio_val to differentiate the index being used
Updated magn_3d_state struct to hold pointer array (magn_val_addr[]) which
points to iio_val[]
Updated magn_3d_parse_report to scan for all compass usages and create iio
channels for each
Signed-off-by: Reyad Attiyat reyad.atti...@gmail.com
Hi Reyad,
I've taken a rather quick look at this. Will probably want to take
one more close look at a newer version.
Various bits and bobs inline - mostly fine!
Jonathan
---
drivers/iio/magnetometer/hid-sensor-magn-3d.c | 271
+-
1 file changed, 177 insertions(+), 94 deletions(-)
diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
index 6d162b7..32f4d90 100644
--- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
+++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
@@ -34,63 +34,54 @@ enum magn_3d_channel {
CHANNEL_SCAN_INDEX_X,
CHANNEL_SCAN_INDEX_Y,
CHANNEL_SCAN_INDEX_Z,
+ CHANNEL_SCAN_INDEX_NORTH_MAGN,
+ CHANNEL_SCAN_INDEX_NORTH_TRUE,
+ CHANNEL_SCAN_INDEX_NORTH_TILT_COMP,
+ CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP,
MAGN_3D_CHANNEL_MAX,
};
+#define IIO_CHANNEL_MAX MAGN_3D_CHANNEL_MAX
Please don't define a generic sounding name for a local
constant. Why not use MAGN_3D_CHANNEL_MAX everywhere?
+
struct magn_3d_state {
struct hid_sensor_hub_callbacks callbacks;
struct hid_sensor_common common_attributes;
struct hid_sensor_hub_attribute_info magn[MAGN_3D_CHANNEL_MAX];
- u32 magn_val[MAGN_3D_CHANNEL_MAX];
-};
+ u32 *magn_val_addr[MAGN_3D_CHANNEL_MAX];
-static const u32 magn_3d_addresses[MAGN_3D_CHANNEL_MAX] = {
- HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS,
- HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS,
- HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS
+ u32 iio_val[IIO_CHANNEL_MAX];
+ int num_iio_channels;
};
-/* Channel definitions */
-static const struct iio_chan_spec magn_3d_channels[] = {
- {
- .type = IIO_MAGN,
- .modified = 1,
- .channel2 = IIO_MOD_X,
- .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
- BIT(IIO_CHAN_INFO_SCALE) |
- BIT(IIO_CHAN_INFO_SAMP_FREQ) |
- BIT(IIO_CHAN_INFO_HYSTERESIS),
- .scan_index = CHANNEL_SCAN_INDEX_X,
- }, {
- .type = IIO_MAGN,
- .modified = 1,
- .channel2 = IIO_MOD_Y,
- .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
- BIT(IIO_CHAN_INFO_SCALE) |
- BIT(IIO_CHAN_INFO_SAMP_FREQ) |
- BIT(IIO_CHAN_INFO_HYSTERESIS),
- .scan_index = CHANNEL_SCAN_INDEX_Y,
- }, {
- .type = IIO_MAGN,
- .modified = 1,
- .channel2 = IIO_MOD_Z,
- .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
- BIT(IIO_CHAN_INFO_SCALE) |
- BIT(IIO_CHAN_INFO_SAMP_FREQ) |
- BIT(IIO_CHAN_INFO_HYSTERESIS),
- .scan_index = CHANNEL_SCAN_INDEX_Z,
+/* Find index into magn_3d_state magn[] and magn_val_addr[] from HID
Usage */
+static int magn_3d_usage_id_to_chan_index(unsigned usage_id){
+ int offset = -1;
I'd personally prefer offset = -EINVAL and to have it assigned as
a default element in the switch statement rather that here.
+
+ switch (usage_id) {
+ case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS:
+ offset = CHANNEL_SCAN_INDEX_X;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS:
+ offset = CHANNEL_SCAN_INDEX_Y;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS:
+ offset = CHANNEL_SCAN_INDEX_Z;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH:
+ offset = CHANNEL_SCAN_INDEX_NORTH_TILT_COMP;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH:
+ offset = CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH:
+ offset = CHANNEL_SCAN_INDEX_NORTH_MAGN;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH:
+ offset = CHANNEL_SCAN_INDEX_NORTH_TRUE;
+ break;
}
-};
-/* Adjust
Re: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages
On June 10, 2014 6:33:25 PM GMT+01:00, Reyad Attiyat reyad.atti...@gmail.com
wrote:
Hey Jonathan,
I will make the changes you suggested thanks for reviewing this. Any
conclusions on the naming, did you want me to change the sysfs name to
in_rot_from_north_true/magnetic?
Yes. I think that is the cleanest option from those we have considered!
On Mon, Jun 9, 2014 at 2:55 PM, Jonathan Cameron ji...@kernel.org
wrote:
On 03/06/14 00:14, Reyad Attiyat wrote:
Updated magn_3d_channel enum for all possible north channels
Added functions to setup iio_chan_spec array depending on which hid
usage
reports are found
Renamed magn_val to iio_val to differentiate the index being used
Updated magn_3d_state struct to hold pointer array (magn_val_addr[])
which
points to iio_val[]
Updated magn_3d_parse_report to scan for all compass usages and
create iio
channels for each
Signed-off-by: Reyad Attiyat reyad.atti...@gmail.com
Hi Reyad,
I've taken a rather quick look at this. Will probably want to take
one more close look at a newer version.
Various bits and bobs inline - mostly fine!
Jonathan
---
drivers/iio/magnetometer/hid-sensor-magn-3d.c | 271
+-
1 file changed, 177 insertions(+), 94 deletions(-)
diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
index 6d162b7..32f4d90 100644
--- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
+++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
@@ -34,63 +34,54 @@ enum magn_3d_channel {
CHANNEL_SCAN_INDEX_X,
CHANNEL_SCAN_INDEX_Y,
CHANNEL_SCAN_INDEX_Z,
+ CHANNEL_SCAN_INDEX_NORTH_MAGN,
+ CHANNEL_SCAN_INDEX_NORTH_TRUE,
+ CHANNEL_SCAN_INDEX_NORTH_TILT_COMP,
+ CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP,
MAGN_3D_CHANNEL_MAX,
};
+#define IIO_CHANNEL_MAX MAGN_3D_CHANNEL_MAX
Please don't define a generic sounding name for a local
constant. Why not use MAGN_3D_CHANNEL_MAX everywhere?
+
struct magn_3d_state {
struct hid_sensor_hub_callbacks callbacks;
struct hid_sensor_common common_attributes;
struct hid_sensor_hub_attribute_info
magn[MAGN_3D_CHANNEL_MAX];
- u32 magn_val[MAGN_3D_CHANNEL_MAX];
-};
+ u32 *magn_val_addr[MAGN_3D_CHANNEL_MAX];
-static const u32 magn_3d_addresses[MAGN_3D_CHANNEL_MAX] = {
- HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS,
- HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS,
- HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS
+ u32 iio_val[IIO_CHANNEL_MAX];
+ int num_iio_channels;
};
-/* Channel definitions */
-static const struct iio_chan_spec magn_3d_channels[] = {
- {
- .type = IIO_MAGN,
- .modified = 1,
- .channel2 = IIO_MOD_X,
- .info_mask_shared_by_type =
BIT(IIO_CHAN_INFO_OFFSET) |
- BIT(IIO_CHAN_INFO_SCALE) |
- BIT(IIO_CHAN_INFO_SAMP_FREQ) |
- BIT(IIO_CHAN_INFO_HYSTERESIS),
- .scan_index = CHANNEL_SCAN_INDEX_X,
- }, {
- .type = IIO_MAGN,
- .modified = 1,
- .channel2 = IIO_MOD_Y,
- .info_mask_shared_by_type =
BIT(IIO_CHAN_INFO_OFFSET) |
- BIT(IIO_CHAN_INFO_SCALE) |
- BIT(IIO_CHAN_INFO_SAMP_FREQ) |
- BIT(IIO_CHAN_INFO_HYSTERESIS),
- .scan_index = CHANNEL_SCAN_INDEX_Y,
- }, {
- .type = IIO_MAGN,
- .modified = 1,
- .channel2 = IIO_MOD_Z,
- .info_mask_shared_by_type =
BIT(IIO_CHAN_INFO_OFFSET) |
- BIT(IIO_CHAN_INFO_SCALE) |
- BIT(IIO_CHAN_INFO_SAMP_FREQ) |
- BIT(IIO_CHAN_INFO_HYSTERESIS),
- .scan_index = CHANNEL_SCAN_INDEX_Z,
+/* Find index into magn_3d_state magn[] and magn_val_addr[] from
HID
Usage */
+static int magn_3d_usage_id_to_chan_index(unsigned usage_id){
+ int offset = -1;
I'd personally prefer offset = -EINVAL and to have it assigned as
a default element in the switch statement rather that here.
+
+ switch (usage_id) {
+ case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS:
+ offset = CHANNEL_SCAN_INDEX_X;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS:
+ offset = CHANNEL_SCAN_INDEX_Y;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS:
+ offset = CHANNEL_SCAN_INDEX_Z;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH:
+ offset = CHANNEL_SCAN_INDEX_NORTH_TILT_COMP;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH:
+ offset = CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH:
+ offset = CHANNEL_SCAN_INDEX_NORTH_MAGN;
+ break;
+
Re: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages
On 03/06/14 00:14, Reyad Attiyat wrote:
Updated magn_3d_channel enum for all possible north channels
Added functions to setup iio_chan_spec array depending on which hid usage
reports are found
Renamed magn_val to iio_val to differentiate the index being used
Updated magn_3d_state struct to hold pointer array (magn_val_addr[]) which
points to iio_val[]
Updated magn_3d_parse_report to scan for all compass usages and create iio
channels for each
Signed-off-by: Reyad Attiyat
Hi Reyad,
I've taken a rather quick look at this. Will probably want to take
one more close look at a newer version.
Various bits and bobs inline - mostly fine!
Jonathan
---
drivers/iio/magnetometer/hid-sensor-magn-3d.c | 271 +-
1 file changed, 177 insertions(+), 94 deletions(-)
diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
index 6d162b7..32f4d90 100644
--- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
+++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
@@ -34,63 +34,54 @@ enum magn_3d_channel {
CHANNEL_SCAN_INDEX_X,
CHANNEL_SCAN_INDEX_Y,
CHANNEL_SCAN_INDEX_Z,
+ CHANNEL_SCAN_INDEX_NORTH_MAGN,
+ CHANNEL_SCAN_INDEX_NORTH_TRUE,
+ CHANNEL_SCAN_INDEX_NORTH_TILT_COMP,
+ CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP,
MAGN_3D_CHANNEL_MAX,
};
+#define IIO_CHANNEL_MAX MAGN_3D_CHANNEL_MAX
Please don't define a generic sounding name for a local
constant. Why not use MAGN_3D_CHANNEL_MAX everywhere?
+
struct magn_3d_state {
struct hid_sensor_hub_callbacks callbacks;
struct hid_sensor_common common_attributes;
struct hid_sensor_hub_attribute_info magn[MAGN_3D_CHANNEL_MAX];
- u32 magn_val[MAGN_3D_CHANNEL_MAX];
-};
+ u32 *magn_val_addr[MAGN_3D_CHANNEL_MAX];
-static const u32 magn_3d_addresses[MAGN_3D_CHANNEL_MAX] = {
- HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS,
- HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS,
- HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS
+ u32 iio_val[IIO_CHANNEL_MAX];
+ int num_iio_channels;
};
-/* Channel definitions */
-static const struct iio_chan_spec magn_3d_channels[] = {
- {
- .type = IIO_MAGN,
- .modified = 1,
- .channel2 = IIO_MOD_X,
- .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
- BIT(IIO_CHAN_INFO_SCALE) |
- BIT(IIO_CHAN_INFO_SAMP_FREQ) |
- BIT(IIO_CHAN_INFO_HYSTERESIS),
- .scan_index = CHANNEL_SCAN_INDEX_X,
- }, {
- .type = IIO_MAGN,
- .modified = 1,
- .channel2 = IIO_MOD_Y,
- .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
- BIT(IIO_CHAN_INFO_SCALE) |
- BIT(IIO_CHAN_INFO_SAMP_FREQ) |
- BIT(IIO_CHAN_INFO_HYSTERESIS),
- .scan_index = CHANNEL_SCAN_INDEX_Y,
- }, {
- .type = IIO_MAGN,
- .modified = 1,
- .channel2 = IIO_MOD_Z,
- .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
- BIT(IIO_CHAN_INFO_SCALE) |
- BIT(IIO_CHAN_INFO_SAMP_FREQ) |
- BIT(IIO_CHAN_INFO_HYSTERESIS),
- .scan_index = CHANNEL_SCAN_INDEX_Z,
+/* Find index into magn_3d_state magn[] and magn_val_addr[] from HID Usage */
+static int magn_3d_usage_id_to_chan_index(unsigned usage_id){
+ int offset = -1;
I'd personally prefer offset = -EINVAL and to have it assigned as
a default element in the switch statement rather that here.
+
+ switch (usage_id) {
+ case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS:
+ offset = CHANNEL_SCAN_INDEX_X;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS:
+ offset = CHANNEL_SCAN_INDEX_Y;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS:
+ offset = CHANNEL_SCAN_INDEX_Z;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH:
+ offset = CHANNEL_SCAN_INDEX_NORTH_TILT_COMP;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH:
+ offset = CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH:
+ offset = CHANNEL_SCAN_INDEX_NORTH_MAGN;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH:
+ offset = CHANNEL_SCAN_INDEX_NORTH_TRUE;
+ break;
}
-};
-/* Adjust channel real bits based on report descriptor */
-static void magn_3d_adjust_channel_bit_mask(struct iio_chan_spec *channels,
- int channel, int size)
-{
- channels[channel].scan_type.sign = 's';
- /* Real storage bits will change based on the report desc. */
- channels[channel].scan_type.realbits = size * 8;
- /* Maximum
Re: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages
On 04/06/14 16:42, Srinivas Pandruvada wrote: On 06/04/2014 08:23 AM, Reyad Attiyat wrote: Hey Srinivas, On Tue, Jun 3, 2014 at 12:43 PM, Srinivas Pandruvada wrote: May be we should have a field in const struct iio_chan_spec, so that we can dynamically enable disable channels. In this way we can statically define channels, but can be enabled/disabled dynamically. Would this require changing iio subsystem to create sysfs entries only when enabled? Would we need to add functions to disable and enable later on? This is just a thought. You don't have to change it. I am sure Jonathan will have some opinion this. Replied to earlier message. If there is some common code we can factor out into the core then I'm happy to do so, but I don't see an advantage in using a field, rather than just tailoring the copy in the first place. e.g. 1) Pass whatever is needed to figure out how many channels are present. 2) Allocate space for that many channels. 3) Copy said channels from predefined versions, perhaps amending as necessary. This is a reasonably common pattern in complex parts or those with hugely variable numbers of channels... I think we need to present angle in radians. Are you basing change present in linux-next? This will automatically do in this function. I'll look into this. What function should I use to make the iio chanel to report radians? I think it will work if the existing sequence is maintained st->scale_precision = hid_sensor_format_scale( HID_USAGE_SENSOR_COMPASS_3D, >magn[CHANNEL_SCAN_INDEX_X], >scale_pre_decml, >scale_post_decml); So as long as you call this function, the scale value to user space will be returned correctly. I don't see kfree. Try devm_kcalloc? I changed kmemdup to kcalloc so there is still a kfree in the exsiting code. I can change this to devm_kcalloc but only if we don't go with static channels that are enabled as found. Since you are changing this part, devm_ calls are preferred, I think. As long as it doesn't add complexity or possibility of race conditions, devm calls are usually a good idea. Thanks, Srinivas Thanks, Reyad Attiyat -- 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: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages
On 03/06/14 18:43, Srinivas Pandruvada wrote:
On 06/02/2014 04:14 PM, Reyad Attiyat wrote:
Updated magn_3d_channel enum for all possible north channels
Added functions to setup iio_chan_spec array depending on which hid usage
reports are found
Renamed magn_val to iio_val to differentiate the index being used
Updated magn_3d_state struct to hold pointer array (magn_val_addr[]) which
points to iio_val[]
Updated magn_3d_parse_report to scan for all compass usages and create iio
channels for each
Signed-off-by: Reyad Attiyat
---
drivers/iio/magnetometer/hid-sensor-magn-3d.c | 271 +-
1 file changed, 177 insertions(+), 94 deletions(-)
diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
index 6d162b7..32f4d90 100644
--- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
+++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
@@ -34,63 +34,54 @@ enum magn_3d_channel {
CHANNEL_SCAN_INDEX_X,
CHANNEL_SCAN_INDEX_Y,
CHANNEL_SCAN_INDEX_Z,
+CHANNEL_SCAN_INDEX_NORTH_MAGN,
+CHANNEL_SCAN_INDEX_NORTH_TRUE,
+CHANNEL_SCAN_INDEX_NORTH_TILT_COMP,
+CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP,
MAGN_3D_CHANNEL_MAX,
};
+#define IIO_CHANNEL_MAX MAGN_3D_CHANNEL_MAX
+
struct magn_3d_state {
struct hid_sensor_hub_callbacks callbacks;
struct hid_sensor_common common_attributes;
struct hid_sensor_hub_attribute_info magn[MAGN_3D_CHANNEL_MAX];
-u32 magn_val[MAGN_3D_CHANNEL_MAX];
-};
+u32 *magn_val_addr[MAGN_3D_CHANNEL_MAX];
-static const u32 magn_3d_addresses[MAGN_3D_CHANNEL_MAX] = {
-HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS,
-HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS,
-HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS
+u32 iio_val[IIO_CHANNEL_MAX];
+int num_iio_channels;
};
-/* Channel definitions */
-static const struct iio_chan_spec magn_3d_channels[] = {
-{
-.type = IIO_MAGN,
-.modified = 1,
-.channel2 = IIO_MOD_X,
-.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
-BIT(IIO_CHAN_INFO_SCALE) |
-BIT(IIO_CHAN_INFO_SAMP_FREQ) |
-BIT(IIO_CHAN_INFO_HYSTERESIS),
-.scan_index = CHANNEL_SCAN_INDEX_X,
-}, {
-.type = IIO_MAGN,
-.modified = 1,
-.channel2 = IIO_MOD_Y,
-.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
-BIT(IIO_CHAN_INFO_SCALE) |
-BIT(IIO_CHAN_INFO_SAMP_FREQ) |
-BIT(IIO_CHAN_INFO_HYSTERESIS),
-.scan_index = CHANNEL_SCAN_INDEX_Y,
-}, {
-.type = IIO_MAGN,
-.modified = 1,
-.channel2 = IIO_MOD_Z,
-.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
-BIT(IIO_CHAN_INFO_SCALE) |
-BIT(IIO_CHAN_INFO_SAMP_FREQ) |
-BIT(IIO_CHAN_INFO_HYSTERESIS),
-.scan_index = CHANNEL_SCAN_INDEX_Z,
May be we should have a field in const struct iio_chan_spec, so that we
can dynamically enable disable channels. In this way we can statically define
channels, but can be enabled/disabled dynamically.
But then it's not const to you have to take a per instance copy.
Once you are doing that you might as well just pull out into the copy
those channels that are actually present, at which point the field
doesn't have any purpose.
+/* Find index into magn_3d_state magn[] and magn_val_addr[] from HID Usage */
+static int magn_3d_usage_id_to_chan_index(unsigned usage_id){
+int offset = -1;
+
+switch (usage_id) {
+case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS:
+offset = CHANNEL_SCAN_INDEX_X;
+break;
+case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS:
+offset = CHANNEL_SCAN_INDEX_Y;
+break;
+case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS:
+offset = CHANNEL_SCAN_INDEX_Z;
+break;
+case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH:
+offset = CHANNEL_SCAN_INDEX_NORTH_TILT_COMP;
+break;
+case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH:
+offset = CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP;
+break;
+case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH:
+offset = CHANNEL_SCAN_INDEX_NORTH_MAGN;
+break;
+case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH:
+offset = CHANNEL_SCAN_INDEX_NORTH_TRUE;
+break;
}
-};
-/* Adjust channel real bits based on report descriptor */
-static void magn_3d_adjust_channel_bit_mask(struct iio_chan_spec *channels,
-int channel, int size)
-{
-channels[channel].scan_type.sign = 's';
-/* Real storage bits will change based on the report desc. */
-channels[channel].scan_type.realbits = size * 8;
-/* Maximum size of a sample to capture is u32 */
-channels[channel].scan_type.storagebits = sizeof(u32) * 8;
+return offset;
}
/* Channel read_raw handler */
@@ -101,21 +92,31 @@ static int magn_3d_read_raw(struct iio_dev *indio_dev,
{
struct magn_3d_state *magn_state = iio_priv(indio_dev);
int
Re: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages
On 03/06/14 18:43, Srinivas Pandruvada wrote:
On 06/02/2014 04:14 PM, Reyad Attiyat wrote:
Updated magn_3d_channel enum for all possible north channels
Added functions to setup iio_chan_spec array depending on which hid usage
reports are found
Renamed magn_val to iio_val to differentiate the index being used
Updated magn_3d_state struct to hold pointer array (magn_val_addr[]) which
points to iio_val[]
Updated magn_3d_parse_report to scan for all compass usages and create iio
channels for each
Signed-off-by: Reyad Attiyat reyad.atti...@gmail.com
---
drivers/iio/magnetometer/hid-sensor-magn-3d.c | 271 +-
1 file changed, 177 insertions(+), 94 deletions(-)
diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
index 6d162b7..32f4d90 100644
--- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
+++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
@@ -34,63 +34,54 @@ enum magn_3d_channel {
CHANNEL_SCAN_INDEX_X,
CHANNEL_SCAN_INDEX_Y,
CHANNEL_SCAN_INDEX_Z,
+CHANNEL_SCAN_INDEX_NORTH_MAGN,
+CHANNEL_SCAN_INDEX_NORTH_TRUE,
+CHANNEL_SCAN_INDEX_NORTH_TILT_COMP,
+CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP,
MAGN_3D_CHANNEL_MAX,
};
+#define IIO_CHANNEL_MAX MAGN_3D_CHANNEL_MAX
+
struct magn_3d_state {
struct hid_sensor_hub_callbacks callbacks;
struct hid_sensor_common common_attributes;
struct hid_sensor_hub_attribute_info magn[MAGN_3D_CHANNEL_MAX];
-u32 magn_val[MAGN_3D_CHANNEL_MAX];
-};
+u32 *magn_val_addr[MAGN_3D_CHANNEL_MAX];
-static const u32 magn_3d_addresses[MAGN_3D_CHANNEL_MAX] = {
-HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS,
-HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS,
-HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS
+u32 iio_val[IIO_CHANNEL_MAX];
+int num_iio_channels;
};
-/* Channel definitions */
-static const struct iio_chan_spec magn_3d_channels[] = {
-{
-.type = IIO_MAGN,
-.modified = 1,
-.channel2 = IIO_MOD_X,
-.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
-BIT(IIO_CHAN_INFO_SCALE) |
-BIT(IIO_CHAN_INFO_SAMP_FREQ) |
-BIT(IIO_CHAN_INFO_HYSTERESIS),
-.scan_index = CHANNEL_SCAN_INDEX_X,
-}, {
-.type = IIO_MAGN,
-.modified = 1,
-.channel2 = IIO_MOD_Y,
-.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
-BIT(IIO_CHAN_INFO_SCALE) |
-BIT(IIO_CHAN_INFO_SAMP_FREQ) |
-BIT(IIO_CHAN_INFO_HYSTERESIS),
-.scan_index = CHANNEL_SCAN_INDEX_Y,
-}, {
-.type = IIO_MAGN,
-.modified = 1,
-.channel2 = IIO_MOD_Z,
-.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
-BIT(IIO_CHAN_INFO_SCALE) |
-BIT(IIO_CHAN_INFO_SAMP_FREQ) |
-BIT(IIO_CHAN_INFO_HYSTERESIS),
-.scan_index = CHANNEL_SCAN_INDEX_Z,
May be we should have a field in const struct iio_chan_spec, so that we
can dynamically enable disable channels. In this way we can statically define
channels, but can be enabled/disabled dynamically.
But then it's not const to you have to take a per instance copy.
Once you are doing that you might as well just pull out into the copy
those channels that are actually present, at which point the field
doesn't have any purpose.
+/* Find index into magn_3d_state magn[] and magn_val_addr[] from HID Usage */
+static int magn_3d_usage_id_to_chan_index(unsigned usage_id){
+int offset = -1;
+
+switch (usage_id) {
+case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS:
+offset = CHANNEL_SCAN_INDEX_X;
+break;
+case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS:
+offset = CHANNEL_SCAN_INDEX_Y;
+break;
+case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS:
+offset = CHANNEL_SCAN_INDEX_Z;
+break;
+case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH:
+offset = CHANNEL_SCAN_INDEX_NORTH_TILT_COMP;
+break;
+case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH:
+offset = CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP;
+break;
+case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH:
+offset = CHANNEL_SCAN_INDEX_NORTH_MAGN;
+break;
+case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH:
+offset = CHANNEL_SCAN_INDEX_NORTH_TRUE;
+break;
}
-};
-/* Adjust channel real bits based on report descriptor */
-static void magn_3d_adjust_channel_bit_mask(struct iio_chan_spec *channels,
-int channel, int size)
-{
-channels[channel].scan_type.sign = 's';
-/* Real storage bits will change based on the report desc. */
-channels[channel].scan_type.realbits = size * 8;
-/* Maximum size of a sample to capture is u32 */
-channels[channel].scan_type.storagebits = sizeof(u32) * 8;
+return offset;
}
/* Channel read_raw handler */
@@ -101,21 +92,31 @@ static int magn_3d_read_raw(struct iio_dev *indio_dev,
{
struct magn_3d_state *magn_state =
Re: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages
On 04/06/14 16:42, Srinivas Pandruvada wrote: On 06/04/2014 08:23 AM, Reyad Attiyat wrote: Hey Srinivas, On Tue, Jun 3, 2014 at 12:43 PM, Srinivas Pandruvada srinivas.pandruv...@linux.intel.com wrote: May be we should have a field in const struct iio_chan_spec, so that we can dynamically enable disable channels. In this way we can statically define channels, but can be enabled/disabled dynamically. Would this require changing iio subsystem to create sysfs entries only when enabled? Would we need to add functions to disable and enable later on? This is just a thought. You don't have to change it. I am sure Jonathan will have some opinion this. Replied to earlier message. If there is some common code we can factor out into the core then I'm happy to do so, but I don't see an advantage in using a field, rather than just tailoring the copy in the first place. e.g. 1) Pass whatever is needed to figure out how many channels are present. 2) Allocate space for that many channels. 3) Copy said channels from predefined versions, perhaps amending as necessary. This is a reasonably common pattern in complex parts or those with hugely variable numbers of channels... I think we need to present angle in radians. Are you basing change present in linux-next? This will automatically do in this function. I'll look into this. What function should I use to make the iio chanel to report radians? I think it will work if the existing sequence is maintained st-scale_precision = hid_sensor_format_scale( HID_USAGE_SENSOR_COMPASS_3D, st-magn[CHANNEL_SCAN_INDEX_X], st-scale_pre_decml, st-scale_post_decml); So as long as you call this function, the scale value to user space will be returned correctly. I don't see kfree. Try devm_kcalloc? I changed kmemdup to kcalloc so there is still a kfree in the exsiting code. I can change this to devm_kcalloc but only if we don't go with static channels that are enabled as found. Since you are changing this part, devm_ calls are preferred, I think. As long as it doesn't add complexity or possibility of race conditions, devm calls are usually a good idea. Thanks, Srinivas Thanks, Reyad Attiyat -- 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: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages
On 03/06/14 00:14, Reyad Attiyat wrote:
Updated magn_3d_channel enum for all possible north channels
Added functions to setup iio_chan_spec array depending on which hid usage
reports are found
Renamed magn_val to iio_val to differentiate the index being used
Updated magn_3d_state struct to hold pointer array (magn_val_addr[]) which
points to iio_val[]
Updated magn_3d_parse_report to scan for all compass usages and create iio
channels for each
Signed-off-by: Reyad Attiyat reyad.atti...@gmail.com
Hi Reyad,
I've taken a rather quick look at this. Will probably want to take
one more close look at a newer version.
Various bits and bobs inline - mostly fine!
Jonathan
---
drivers/iio/magnetometer/hid-sensor-magn-3d.c | 271 +-
1 file changed, 177 insertions(+), 94 deletions(-)
diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
index 6d162b7..32f4d90 100644
--- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
+++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
@@ -34,63 +34,54 @@ enum magn_3d_channel {
CHANNEL_SCAN_INDEX_X,
CHANNEL_SCAN_INDEX_Y,
CHANNEL_SCAN_INDEX_Z,
+ CHANNEL_SCAN_INDEX_NORTH_MAGN,
+ CHANNEL_SCAN_INDEX_NORTH_TRUE,
+ CHANNEL_SCAN_INDEX_NORTH_TILT_COMP,
+ CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP,
MAGN_3D_CHANNEL_MAX,
};
+#define IIO_CHANNEL_MAX MAGN_3D_CHANNEL_MAX
Please don't define a generic sounding name for a local
constant. Why not use MAGN_3D_CHANNEL_MAX everywhere?
+
struct magn_3d_state {
struct hid_sensor_hub_callbacks callbacks;
struct hid_sensor_common common_attributes;
struct hid_sensor_hub_attribute_info magn[MAGN_3D_CHANNEL_MAX];
- u32 magn_val[MAGN_3D_CHANNEL_MAX];
-};
+ u32 *magn_val_addr[MAGN_3D_CHANNEL_MAX];
-static const u32 magn_3d_addresses[MAGN_3D_CHANNEL_MAX] = {
- HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS,
- HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS,
- HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS
+ u32 iio_val[IIO_CHANNEL_MAX];
+ int num_iio_channels;
};
-/* Channel definitions */
-static const struct iio_chan_spec magn_3d_channels[] = {
- {
- .type = IIO_MAGN,
- .modified = 1,
- .channel2 = IIO_MOD_X,
- .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
- BIT(IIO_CHAN_INFO_SCALE) |
- BIT(IIO_CHAN_INFO_SAMP_FREQ) |
- BIT(IIO_CHAN_INFO_HYSTERESIS),
- .scan_index = CHANNEL_SCAN_INDEX_X,
- }, {
- .type = IIO_MAGN,
- .modified = 1,
- .channel2 = IIO_MOD_Y,
- .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
- BIT(IIO_CHAN_INFO_SCALE) |
- BIT(IIO_CHAN_INFO_SAMP_FREQ) |
- BIT(IIO_CHAN_INFO_HYSTERESIS),
- .scan_index = CHANNEL_SCAN_INDEX_Y,
- }, {
- .type = IIO_MAGN,
- .modified = 1,
- .channel2 = IIO_MOD_Z,
- .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
- BIT(IIO_CHAN_INFO_SCALE) |
- BIT(IIO_CHAN_INFO_SAMP_FREQ) |
- BIT(IIO_CHAN_INFO_HYSTERESIS),
- .scan_index = CHANNEL_SCAN_INDEX_Z,
+/* Find index into magn_3d_state magn[] and magn_val_addr[] from HID Usage */
+static int magn_3d_usage_id_to_chan_index(unsigned usage_id){
+ int offset = -1;
I'd personally prefer offset = -EINVAL and to have it assigned as
a default element in the switch statement rather that here.
+
+ switch (usage_id) {
+ case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS:
+ offset = CHANNEL_SCAN_INDEX_X;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS:
+ offset = CHANNEL_SCAN_INDEX_Y;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS:
+ offset = CHANNEL_SCAN_INDEX_Z;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH:
+ offset = CHANNEL_SCAN_INDEX_NORTH_TILT_COMP;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH:
+ offset = CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH:
+ offset = CHANNEL_SCAN_INDEX_NORTH_MAGN;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH:
+ offset = CHANNEL_SCAN_INDEX_NORTH_TRUE;
+ break;
}
-};
-/* Adjust channel real bits based on report descriptor */
-static void magn_3d_adjust_channel_bit_mask(struct iio_chan_spec *channels,
- int channel, int size)
-{
- channels[channel].scan_type.sign = 's';
- /* Real storage bits will change based on the report desc. */
- channels[channel].scan_type.realbits = size *
Re: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages
On 06/04/2014 08:23 AM, Reyad Attiyat wrote: Hey Srinivas, On Tue, Jun 3, 2014 at 12:43 PM, Srinivas Pandruvada wrote: May be we should have a field in const struct iio_chan_spec, so that we can dynamically enable disable channels. In this way we can statically define channels, but can be enabled/disabled dynamically. Would this require changing iio subsystem to create sysfs entries only when enabled? Would we need to add functions to disable and enable later on? This is just a thought. You don't have to change it. I am sure Jonathan will have some opinion this. I think we need to present angle in radians. Are you basing change present in linux-next? This will automatically do in this function. I'll look into this. What function should I use to make the iio chanel to report radians? I think it will work if the existing sequence is maintained st->scale_precision = hid_sensor_format_scale( HID_USAGE_SENSOR_COMPASS_3D, >magn[CHANNEL_SCAN_INDEX_X], >scale_pre_decml, >scale_post_decml); So as long as you call this function, the scale value to user space will be returned correctly. I don't see kfree. Try devm_kcalloc? I changed kmemdup to kcalloc so there is still a kfree in the exsiting code. I can change this to devm_kcalloc but only if we don't go with static channels that are enabled as found. Since you are changing this part, devm_ calls are preferred, I think. Thanks, Srinivas Thanks, Reyad Attiyat -- 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: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages
Hey Srinivas, On Tue, Jun 3, 2014 at 12:43 PM, Srinivas Pandruvada wrote: > > May be we should have a field in const struct iio_chan_spec, so that we > can dynamically enable disable channels. In this way we can statically > define channels, but can be enabled/disabled dynamically. > Would this require changing iio subsystem to create sysfs entries only when enabled? Would we need to add functions to disable and enable later on? > > I think we need to present angle in radians. Are you basing change present > in linux-next? This will automatically do in this function. > I'll look into this. What function should I use to make the iio chanel to report radians? > > > I don't see kfree. Try devm_kcalloc? > I changed kmemdup to kcalloc so there is still a kfree in the exsiting code. I can change this to devm_kcalloc but only if we don't go with static channels that are enabled as found. Thanks, Reyad Attiyat -- 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: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages
Hey Srinivas, On Tue, Jun 3, 2014 at 12:43 PM, Srinivas Pandruvada srinivas.pandruv...@linux.intel.com wrote: May be we should have a field in const struct iio_chan_spec, so that we can dynamically enable disable channels. In this way we can statically define channels, but can be enabled/disabled dynamically. Would this require changing iio subsystem to create sysfs entries only when enabled? Would we need to add functions to disable and enable later on? I think we need to present angle in radians. Are you basing change present in linux-next? This will automatically do in this function. I'll look into this. What function should I use to make the iio chanel to report radians? I don't see kfree. Try devm_kcalloc? I changed kmemdup to kcalloc so there is still a kfree in the exsiting code. I can change this to devm_kcalloc but only if we don't go with static channels that are enabled as found. Thanks, Reyad Attiyat -- 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: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages
On 06/04/2014 08:23 AM, Reyad Attiyat wrote: Hey Srinivas, On Tue, Jun 3, 2014 at 12:43 PM, Srinivas Pandruvada srinivas.pandruv...@linux.intel.com wrote: May be we should have a field in const struct iio_chan_spec, so that we can dynamically enable disable channels. In this way we can statically define channels, but can be enabled/disabled dynamically. Would this require changing iio subsystem to create sysfs entries only when enabled? Would we need to add functions to disable and enable later on? This is just a thought. You don't have to change it. I am sure Jonathan will have some opinion this. I think we need to present angle in radians. Are you basing change present in linux-next? This will automatically do in this function. I'll look into this. What function should I use to make the iio chanel to report radians? I think it will work if the existing sequence is maintained st-scale_precision = hid_sensor_format_scale( HID_USAGE_SENSOR_COMPASS_3D, st-magn[CHANNEL_SCAN_INDEX_X], st-scale_pre_decml, st-scale_post_decml); So as long as you call this function, the scale value to user space will be returned correctly. I don't see kfree. Try devm_kcalloc? I changed kmemdup to kcalloc so there is still a kfree in the exsiting code. I can change this to devm_kcalloc but only if we don't go with static channels that are enabled as found. Since you are changing this part, devm_ calls are preferred, I think. Thanks, Srinivas Thanks, Reyad Attiyat -- 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: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages
On 06/02/2014 04:14 PM, Reyad Attiyat wrote:
Updated magn_3d_channel enum for all possible north channels
Added functions to setup iio_chan_spec array depending on which hid usage
reports are found
Renamed magn_val to iio_val to differentiate the index being used
Updated magn_3d_state struct to hold pointer array (magn_val_addr[]) which
points to iio_val[]
Updated magn_3d_parse_report to scan for all compass usages and create iio
channels for each
Signed-off-by: Reyad Attiyat
---
drivers/iio/magnetometer/hid-sensor-magn-3d.c | 271 +-
1 file changed, 177 insertions(+), 94 deletions(-)
diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
index 6d162b7..32f4d90 100644
--- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
+++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
@@ -34,63 +34,54 @@ enum magn_3d_channel {
CHANNEL_SCAN_INDEX_X,
CHANNEL_SCAN_INDEX_Y,
CHANNEL_SCAN_INDEX_Z,
+ CHANNEL_SCAN_INDEX_NORTH_MAGN,
+ CHANNEL_SCAN_INDEX_NORTH_TRUE,
+ CHANNEL_SCAN_INDEX_NORTH_TILT_COMP,
+ CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP,
MAGN_3D_CHANNEL_MAX,
};
+#define IIO_CHANNEL_MAX MAGN_3D_CHANNEL_MAX
+
struct magn_3d_state {
struct hid_sensor_hub_callbacks callbacks;
struct hid_sensor_common common_attributes;
struct hid_sensor_hub_attribute_info magn[MAGN_3D_CHANNEL_MAX];
- u32 magn_val[MAGN_3D_CHANNEL_MAX];
-};
+ u32 *magn_val_addr[MAGN_3D_CHANNEL_MAX];
-static const u32 magn_3d_addresses[MAGN_3D_CHANNEL_MAX] = {
- HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS,
- HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS,
- HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS
+ u32 iio_val[IIO_CHANNEL_MAX];
+ int num_iio_channels;
};
-/* Channel definitions */
-static const struct iio_chan_spec magn_3d_channels[] = {
- {
- .type = IIO_MAGN,
- .modified = 1,
- .channel2 = IIO_MOD_X,
- .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
- BIT(IIO_CHAN_INFO_SCALE) |
- BIT(IIO_CHAN_INFO_SAMP_FREQ) |
- BIT(IIO_CHAN_INFO_HYSTERESIS),
- .scan_index = CHANNEL_SCAN_INDEX_X,
- }, {
- .type = IIO_MAGN,
- .modified = 1,
- .channel2 = IIO_MOD_Y,
- .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
- BIT(IIO_CHAN_INFO_SCALE) |
- BIT(IIO_CHAN_INFO_SAMP_FREQ) |
- BIT(IIO_CHAN_INFO_HYSTERESIS),
- .scan_index = CHANNEL_SCAN_INDEX_Y,
- }, {
- .type = IIO_MAGN,
- .modified = 1,
- .channel2 = IIO_MOD_Z,
- .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
- BIT(IIO_CHAN_INFO_SCALE) |
- BIT(IIO_CHAN_INFO_SAMP_FREQ) |
- BIT(IIO_CHAN_INFO_HYSTERESIS),
- .scan_index = CHANNEL_SCAN_INDEX_Z,
May be we should have a field in const struct iio_chan_spec, so that we
can dynamically enable disable channels. In this way we can statically
define channels, but can be enabled/disabled dynamically.
+/* Find index into magn_3d_state magn[] and magn_val_addr[] from HID Usage */
+static int magn_3d_usage_id_to_chan_index(unsigned usage_id){
+ int offset = -1;
+
+ switch (usage_id) {
+ case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS:
+ offset = CHANNEL_SCAN_INDEX_X;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS:
+ offset = CHANNEL_SCAN_INDEX_Y;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS:
+ offset = CHANNEL_SCAN_INDEX_Z;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH:
+ offset = CHANNEL_SCAN_INDEX_NORTH_TILT_COMP;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH:
+ offset = CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH:
+ offset = CHANNEL_SCAN_INDEX_NORTH_MAGN;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH:
+ offset = CHANNEL_SCAN_INDEX_NORTH_TRUE;
+ break;
}
-};
-/* Adjust channel real bits based on report descriptor */
-static void magn_3d_adjust_channel_bit_mask(struct iio_chan_spec *channels,
- int channel, int size)
-{
- channels[channel].scan_type.sign = 's';
- /* Real storage bits will change based on the report desc. */
- channels[channel].scan_type.realbits = size * 8;
- /* Maximum size of a sample to capture is u32 */
- channels[channel].scan_type.storagebits = sizeof(u32) * 8;
+ return offset;
}
/* Channel read_raw handler */
@@ -101,21 +92,31 @@ static int
Re: [PATCHv2 3/3] IIO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages
On 06/02/2014 04:14 PM, Reyad Attiyat wrote:
Updated magn_3d_channel enum for all possible north channels
Added functions to setup iio_chan_spec array depending on which hid usage
reports are found
Renamed magn_val to iio_val to differentiate the index being used
Updated magn_3d_state struct to hold pointer array (magn_val_addr[]) which
points to iio_val[]
Updated magn_3d_parse_report to scan for all compass usages and create iio
channels for each
Signed-off-by: Reyad Attiyat reyad.atti...@gmail.com
---
drivers/iio/magnetometer/hid-sensor-magn-3d.c | 271 +-
1 file changed, 177 insertions(+), 94 deletions(-)
diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
index 6d162b7..32f4d90 100644
--- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
+++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
@@ -34,63 +34,54 @@ enum magn_3d_channel {
CHANNEL_SCAN_INDEX_X,
CHANNEL_SCAN_INDEX_Y,
CHANNEL_SCAN_INDEX_Z,
+ CHANNEL_SCAN_INDEX_NORTH_MAGN,
+ CHANNEL_SCAN_INDEX_NORTH_TRUE,
+ CHANNEL_SCAN_INDEX_NORTH_TILT_COMP,
+ CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP,
MAGN_3D_CHANNEL_MAX,
};
+#define IIO_CHANNEL_MAX MAGN_3D_CHANNEL_MAX
+
struct magn_3d_state {
struct hid_sensor_hub_callbacks callbacks;
struct hid_sensor_common common_attributes;
struct hid_sensor_hub_attribute_info magn[MAGN_3D_CHANNEL_MAX];
- u32 magn_val[MAGN_3D_CHANNEL_MAX];
-};
+ u32 *magn_val_addr[MAGN_3D_CHANNEL_MAX];
-static const u32 magn_3d_addresses[MAGN_3D_CHANNEL_MAX] = {
- HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS,
- HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS,
- HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS
+ u32 iio_val[IIO_CHANNEL_MAX];
+ int num_iio_channels;
};
-/* Channel definitions */
-static const struct iio_chan_spec magn_3d_channels[] = {
- {
- .type = IIO_MAGN,
- .modified = 1,
- .channel2 = IIO_MOD_X,
- .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
- BIT(IIO_CHAN_INFO_SCALE) |
- BIT(IIO_CHAN_INFO_SAMP_FREQ) |
- BIT(IIO_CHAN_INFO_HYSTERESIS),
- .scan_index = CHANNEL_SCAN_INDEX_X,
- }, {
- .type = IIO_MAGN,
- .modified = 1,
- .channel2 = IIO_MOD_Y,
- .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
- BIT(IIO_CHAN_INFO_SCALE) |
- BIT(IIO_CHAN_INFO_SAMP_FREQ) |
- BIT(IIO_CHAN_INFO_HYSTERESIS),
- .scan_index = CHANNEL_SCAN_INDEX_Y,
- }, {
- .type = IIO_MAGN,
- .modified = 1,
- .channel2 = IIO_MOD_Z,
- .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
- BIT(IIO_CHAN_INFO_SCALE) |
- BIT(IIO_CHAN_INFO_SAMP_FREQ) |
- BIT(IIO_CHAN_INFO_HYSTERESIS),
- .scan_index = CHANNEL_SCAN_INDEX_Z,
May be we should have a field in const struct iio_chan_spec, so that we
can dynamically enable disable channels. In this way we can statically
define channels, but can be enabled/disabled dynamically.
+/* Find index into magn_3d_state magn[] and magn_val_addr[] from HID Usage */
+static int magn_3d_usage_id_to_chan_index(unsigned usage_id){
+ int offset = -1;
+
+ switch (usage_id) {
+ case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_X_AXIS:
+ offset = CHANNEL_SCAN_INDEX_X;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Y_AXIS:
+ offset = CHANNEL_SCAN_INDEX_Y;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_MAGN_FLUX_Z_AXIS:
+ offset = CHANNEL_SCAN_INDEX_Z;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH:
+ offset = CHANNEL_SCAN_INDEX_NORTH_TILT_COMP;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_COMP_TRUE_NORTH:
+ offset = CHANNEL_SCAN_INDEX_NORTH_TRUE_TILT_COMP;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_MAGN_NORTH:
+ offset = CHANNEL_SCAN_INDEX_NORTH_MAGN;
+ break;
+ case HID_USAGE_SENSOR_ORIENT_TRUE_NORTH:
+ offset = CHANNEL_SCAN_INDEX_NORTH_TRUE;
+ break;
}
-};
-/* Adjust channel real bits based on report descriptor */
-static void magn_3d_adjust_channel_bit_mask(struct iio_chan_spec *channels,
- int channel, int size)
-{
- channels[channel].scan_type.sign = 's';
- /* Real storage bits will change based on the report desc. */
- channels[channel].scan_type.realbits = size * 8;
- /* Maximum size of a sample to capture is u32 */
- channels[channel].scan_type.storagebits = sizeof(u32) * 8;
+ return offset;
}
/* Channel read_raw handler */
@@ -101,21
