On Sat, 2015-03-28 at 21:58 +0100, Hartmut Knaack wrote: > Srinivas Pandruvada schrieb am 17.01.2015 um 01:39: > > Some hid sensor feature report can contain more than one reports. > > This API can now support receiving multiple values from the feature > > report. > > Also update the parameters in the users of this API. > > Hi Hartmut, > Hi Srinivas, > I noticed this series got applied already to linux-input. Yet I was wondering > if it wouldn't be better to have size variables like buffer_size, report_size > and remaining_bytes (from the next patch) defined as unsigned. > Also a minor indentation issue spotted inline, in case you get to work on that > section again some time. > This module will go further enhancements soon, then I will take care of this.
Thanks, Srinivas > > Signed-off-by: Srinivas Pandruvada <[email protected]> > > --- > > drivers/hid/hid-sensor-hub.c | 15 +++++++++++++-- > > drivers/iio/common/hid-sensors/hid-sensor-attributes.c | 13 +++++++------ > > drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 4 ++-- > > include/linux/hid-sensor-hub.h | 8 +++++--- > > 4 files changed, 27 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c > > index e54ce10..4d22bd4 100644 > > --- a/drivers/hid/hid-sensor-hub.c > > +++ b/drivers/hid/hid-sensor-hub.c > > @@ -232,10 +232,11 @@ done_proc: > > EXPORT_SYMBOL_GPL(sensor_hub_set_feature); > > > > int sensor_hub_get_feature(struct hid_sensor_hub_device *hsdev, u32 > > report_id, > > - u32 field_index, s32 *value) > > + u32 field_index, int buffer_size, void *buffer) > > { > > struct hid_report *report; > > struct sensor_hub_data *data = hid_get_drvdata(hsdev->hdev); > > + int report_size; > > int ret = 0; > > > > mutex_lock(&data->mutex); > > @@ -247,7 +248,17 @@ int sensor_hub_get_feature(struct > > hid_sensor_hub_device *hsdev, u32 report_id, > > } > > hid_hw_request(hsdev->hdev, report, HID_REQ_GET_REPORT); > > hid_hw_wait(hsdev->hdev); > > - *value = report->field[field_index]->value[0]; > > + > > + /* calculate number of bytes required to read this field */ > > + report_size = DIV_ROUND_UP(report->field[field_index]->report_size, > > + 8) * > > + report->field[field_index]->report_count; > The last line should not be indented with the opening parenthesis any more. > What do you think about this solution: > > report_size = DIV_ROUND_UP(report->field[field_index]->report_size, 8); > report_size *= report->field[field_index]->report_count; > > > + if (!report_size) { > > + ret = -EINVAL; > > + goto done_proc; > > + } > > + ret = min(report_size, buffer_size); > > + memcpy(buffer, report->field[field_index]->value, ret); > > > > done_proc: > > mutex_unlock(&data->mutex); > > diff --git a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c > > b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c > > index 25b01e1..e1435e9 100644 > > --- a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c > > +++ b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c > > @@ -153,8 +153,8 @@ s32 hid_sensor_read_poll_value(struct hid_sensor_common > > *st) > > int ret; > > > > ret = sensor_hub_get_feature(st->hsdev, > > - st->poll.report_id, > > - st->poll.index, &value); > > + st->poll.report_id, > > + st->poll.index, sizeof(value), &value); > > > > if (ret < 0 || value < 0) { > > return -EINVAL; > > @@ -174,8 +174,8 @@ int hid_sensor_read_samp_freq_value(struct > > hid_sensor_common *st, > > int ret; > > > > ret = sensor_hub_get_feature(st->hsdev, > > - st->poll.report_id, > > - st->poll.index, &value); > > + st->poll.report_id, > > + st->poll.index, sizeof(value), &value); > > if (ret < 0 || value < 0) { > > *val1 = *val2 = 0; > > return -EINVAL; > > @@ -229,8 +229,9 @@ int hid_sensor_read_raw_hyst_value(struct > > hid_sensor_common *st, > > int ret; > > > > ret = sensor_hub_get_feature(st->hsdev, > > - st->sensitivity.report_id, > > - st->sensitivity.index, &value); > > + st->sensitivity.report_id, > > + st->sensitivity.index, sizeof(value), > > + &value); > > if (ret < 0 || value < 0) { > > *val1 = *val2 = 0; > > return -EINVAL; > > diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c > > b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c > > index 92068cd..ef0c495 100644 > > --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c > > +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c > > @@ -76,8 +76,8 @@ int hid_sensor_power_state(struct hid_sensor_common *st, > > bool state) > > } > > > > sensor_hub_get_feature(st->hsdev, st->power_state.report_id, > > - st->power_state.index, > > - &state_val); > > + st->power_state.index, > > + sizeof(state_val), &state_val); > > return 0; > > } > > EXPORT_SYMBOL(hid_sensor_power_state); > > diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h > > index 4173a8f..dfdf6ef 100644 > > --- a/include/linux/hid-sensor-hub.h > > +++ b/include/linux/hid-sensor-hub.h > > @@ -179,13 +179,15 @@ int sensor_hub_set_feature(struct > > hid_sensor_hub_device *hsdev, u32 report_id, > > * @hsdev: Hub device instance. > > * @report_id: Report id to look for > > * @field_index: Field index inside a report > > -* @value: Place holder for return value > > +* @buffer_size: size of the buffer > > +* @buffer: buffer to copy output > > * > > * Used to get a field in feature report. For example this can get polling > > -* interval, sensitivity, activate/deactivate state. > > +* interval, sensitivity, activate/deactivate state. On success it returns > > +* number of bytes copied to buffer. On failure, it returns value < 0. > > */ > > int sensor_hub_get_feature(struct hid_sensor_hub_device *hsdev, u32 > > report_id, > > - u32 field_index, s32 *value); > > + u32 field_index, int buffer_size, void *buffer); > > > > /* hid-sensor-attributes */ > > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
