Re: [PATCH 3/3] IO: hid-sensor-magn-3d: Add in support for True/Magnetic North HID usages
From: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Date: 2014-05-28 21:52:53
Also in:
linux-iio, lkml
Hi Reyad, On 05/28/2014 02:35 PM, Reyad Attiyat wrote:
Hey Srinivas, Well I could use sensor_hub_input_get_attribute_info() for each usage attribute. I was just thinking that since each usage attribute is found in a row, one for each field I think, it'd be easier to create iio channels that way. This would eliminate running the for loop search for usage id each time.
It shouldn't be an issue for few more attributes. The idea is that we don't expose the report level parsing information to the client drivers. The client shouldn't worry about which collection or report it belongs to. In this way if we have to enhance the parse function for newer FW changes or quirks, it is only done at one place. Clients are not affected at all. Alternatively we can enhance API, which takes multiple usage ids and fills information in a single call. What do you think about this? Thanks, Srinivas
I realize that my parse_report function is quite ugly espically ending in four closing parenthesis and copying code from sensor_hub_input_get_attribute_info(). I will change it if you don't think it will slow down the driver or have any negative effects. Thanks, Reyad On Wed, May 28, 2014 at 4:25 PM, Srinivas Pandruvada [off-list ref] wrote:quoted
On 05/28/2014 02:15 PM, Reyad Attiyat wrote:quoted
quoted
+static void sensor_hub_fill_attr_info( + struct hid_sensor_hub_attribute_info *info, + s32 index, s32 report_id, struct hid_field *field) +{ + info->index = index; + info->report_id = report_id; + info->units = field->unit; + info->unit_expo = field->unit_exponent; + info->size = (field->report_size * field->report_count)/8; + info->logical_minimum = field->logical_minimum; + info->logical_maximum = field->logical_maximum; }I copied this function from hid/hid-sensor-hub.c as it is marked static in that file. I use it to fill attributes as I find them. Should I create an another patch to make it non-static?I didn't check your implementation. But sensor_hub_input_get_attribute_info() function is not enough? We are already using to get other attributes for x, y and Z. Thanks, Srinivasquoted
quoted
+ list_for_each_entry(report, &report_enum->report_list, list) { + for (i = 0; i < report->maxfield; ++i) { + field = report->field[i]; + + for (j = 0; j < field->maxusage; j++) { + usage = &(field->usage[j]); +This is how I mange to find all possible hid reports in the parse reports function. I noticed that in the other function that was used, sensor_hub_input_get_attribute_info(), it only uses field->usage[0]. Is there a reason for this and should I change my current implementation?