On 07/01/15 18:24, Srinivas Pandruvada wrote: > Current implementation only allows one outstanding synchronous read. > This is a performance hit when user mode is requesting raw reads > of sensor attributes on multiple sensors together. > This change changes the mutex lock to per hid sensor hub device instead > of global lock. Although request to hid sensor hub is serialized, there > can be multiple outstanding read requests pending for responses via > hid reports. > > Signed-off-by: Srinivas Pandruvada <[email protected]> Seems sensible to me. I don't think this has any cross dependencies with the IIO patches...
Acked-by: Jonathan Cameron <[email protected]> > --- > drivers/hid/hid-sensor-hub.c | 90 > +++++++++++++++++++----------------------- > include/linux/hid-sensor-hub.h | 24 +++++++++++ > 2 files changed, 65 insertions(+), 49 deletions(-) > > diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c > index e54ce10..865cd56 100644 > --- a/drivers/hid/hid-sensor-hub.c > +++ b/drivers/hid/hid-sensor-hub.c > @@ -29,29 +29,10 @@ > #define HID_SENSOR_HUB_ENUM_QUIRK 0x01 > > /** > - * struct sensor_hub_pending - Synchronous read pending information > - * @status: Pending status true/false. > - * @ready: Completion synchronization data. > - * @usage_id: Usage id for physical device, E.g. Gyro usage > id. > - * @attr_usage_id: Usage Id of a field, E.g. X-AXIS for a gyro. > - * @raw_size: Response size for a read request. > - * @raw_data: Place holder for received response. > - */ > -struct sensor_hub_pending { > - bool status; > - struct completion ready; > - u32 usage_id; > - u32 attr_usage_id; > - int raw_size; > - u8 *raw_data; > -}; > - > -/** > * struct sensor_hub_data - Hold a instance data for a HID hub device > * @hsdev: Stored hid instance for current hub device. > * @mutex: Mutex to serialize synchronous request. > * @lock: Spin lock to protect pending request structure. > - * @pending: Holds information of pending sync read request. > * @dyn_callback_list: Holds callback function > * @dyn_callback_lock: spin lock to protect callback list > * @hid_sensor_hub_client_devs: Stores all MFD cells for a hub instance. > @@ -61,7 +42,6 @@ struct sensor_hub_pending { > struct sensor_hub_data { > struct mutex mutex; > spinlock_t lock; > - struct sensor_hub_pending pending; > struct list_head dyn_callback_list; > spinlock_t dyn_callback_lock; > struct mfd_cell *hid_sensor_hub_client_devs; > @@ -266,40 +246,42 @@ int sensor_hub_input_attr_get_raw_value(struct > hid_sensor_hub_device *hsdev, > struct hid_report *report; > int ret_val = 0; > > - mutex_lock(&data->mutex); > - memset(&data->pending, 0, sizeof(data->pending)); > - init_completion(&data->pending.ready); > - data->pending.usage_id = usage_id; > - data->pending.attr_usage_id = attr_usage_id; > - data->pending.raw_size = 0; > + mutex_lock(&hsdev->mutex); > + memset(&hsdev->pending, 0, sizeof(hsdev->pending)); > + init_completion(&hsdev->pending.ready); > + hsdev->pending.usage_id = usage_id; > + hsdev->pending.attr_usage_id = attr_usage_id; > + hsdev->pending.raw_size = 0; > > spin_lock_irqsave(&data->lock, flags); > - data->pending.status = true; > + hsdev->pending.status = true; > spin_unlock_irqrestore(&data->lock, flags); > report = sensor_hub_report(report_id, hsdev->hdev, HID_INPUT_REPORT); > if (!report) > goto err_free; > > + mutex_lock(&data->mutex); > hid_hw_request(hsdev->hdev, report, HID_REQ_GET_REPORT); > - wait_for_completion_interruptible_timeout(&data->pending.ready, HZ*5); > - switch (data->pending.raw_size) { > + mutex_unlock(&data->mutex); > + wait_for_completion_interruptible_timeout(&hsdev->pending.ready, HZ*5); > + switch (hsdev->pending.raw_size) { > case 1: > - ret_val = *(u8 *)data->pending.raw_data; > + ret_val = *(u8 *)hsdev->pending.raw_data; > break; > case 2: > - ret_val = *(u16 *)data->pending.raw_data; > + ret_val = *(u16 *)hsdev->pending.raw_data; > break; > case 4: > - ret_val = *(u32 *)data->pending.raw_data; > + ret_val = *(u32 *)hsdev->pending.raw_data; > break; > default: > ret_val = 0; > } > - kfree(data->pending.raw_data); > + kfree(hsdev->pending.raw_data); > > err_free: > - data->pending.status = false; > - mutex_unlock(&data->mutex); > + hsdev->pending.status = false; > + mutex_unlock(&hsdev->mutex); > > return ret_val; > } > @@ -455,16 +437,6 @@ static int sensor_hub_raw_event(struct hid_device *hdev, > report->field[i]->report_count)/8); > sz = (report->field[i]->report_size * > report->field[i]->report_count)/8; > - if (pdata->pending.status && pdata->pending.attr_usage_id == > - report->field[i]->usage->hid) { > - hid_dbg(hdev, "data was pending ...\n"); > - pdata->pending.raw_data = kmemdup(ptr, sz, GFP_ATOMIC); > - if (pdata->pending.raw_data) > - pdata->pending.raw_size = sz; > - else > - pdata->pending.raw_size = 0; > - complete(&pdata->pending.ready); > - } > collection = &hdev->collection[ > report->field[i]->usage->collection_index]; > hid_dbg(hdev, "collection->usage %x\n", > @@ -474,8 +446,21 @@ static int sensor_hub_raw_event(struct hid_device *hdev, > report->field[i]->physical, > report->field[i]->usage[0].collection_index, > &hsdev, &priv); > - > - if (callback && callback->capture_sample) { > + if (!callback) { > + ptr += sz; > + continue; > + } > + if (hsdev->pending.status && hsdev->pending.attr_usage_id == > + report->field[i]->usage->hid) { > + hid_dbg(hdev, "data was pending ...\n"); > + hsdev->pending.raw_data = kmemdup(ptr, sz, GFP_ATOMIC); > + if (hsdev->pending.raw_data) > + hsdev->pending.raw_size = sz; > + else > + hsdev->pending.raw_size = 0; > + complete(&hsdev->pending.ready); > + } > + if (callback->capture_sample) { > if (report->field[i]->logical) > callback->capture_sample(hsdev, > report->field[i]->logical, sz, ptr, > @@ -630,6 +615,8 @@ static int sensor_hub_probe(struct hid_device *hdev, > hsdev->hdev = hdev; > hsdev->vendor_id = hdev->vendor; > hsdev->product_id = hdev->product; > + hsdev->usage = collection->usage; > + mutex_init(&hsdev->mutex); > hsdev->start_collection_index = i; > if (last_hsdev) > last_hsdev->end_collection_index = i; > @@ -676,13 +663,18 @@ static void sensor_hub_remove(struct hid_device *hdev) > { > struct sensor_hub_data *data = hid_get_drvdata(hdev); > unsigned long flags; > + int i; > > hid_dbg(hdev, " hardware removed\n"); > hid_hw_close(hdev); > hid_hw_stop(hdev); > spin_lock_irqsave(&data->lock, flags); > - if (data->pending.status) > - complete(&data->pending.ready); > + for (i = 0; i < data->hid_sensor_client_cnt; ++i) { > + struct hid_sensor_hub_device *hsdev = > + data->hid_sensor_hub_client_devs[i].platform_data; > + if (hsdev->pending.status) > + complete(&hsdev->pending.ready); > + } > spin_unlock_irqrestore(&data->lock, flags); > mfd_remove_devices(&hdev->dev); > hid_set_drvdata(hdev, NULL); > diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h > index 4173a8f..0b21c4f 100644 > --- a/include/linux/hid-sensor-hub.h > +++ b/include/linux/hid-sensor-hub.h > @@ -49,19 +49,43 @@ struct hid_sensor_hub_attribute_info { > }; > > /** > + * struct sensor_hub_pending - Synchronous read pending information > + * @status: Pending status true/false. > + * @ready: Completion synchronization data. > + * @usage_id: Usage id for physical device, E.g. Gyro usage > id. > + * @attr_usage_id: Usage Id of a field, E.g. X-AXIS for a gyro. > + * @raw_size: Response size for a read request. > + * @raw_data: Place holder for received response. > + */ > +struct sensor_hub_pending { > + bool status; > + struct completion ready; > + u32 usage_id; > + u32 attr_usage_id; > + int raw_size; > + u8 *raw_data; > +}; > + > +/** > * struct hid_sensor_hub_device - Stores the hub instance data > * @hdev: Stores the hid instance. > * @vendor_id: Vendor id of hub device. > * @product_id: Product id of hub device. > + * @usage: Usage id for this hub device instance. > * @start_collection_index: Starting index for a phy type collection > * @end_collection_index: Last index for a phy type collection > + * @mutex: synchronizing mutex. > + * @pending: Holds information of pending sync read request. > */ > struct hid_sensor_hub_device { > struct hid_device *hdev; > u32 vendor_id; > u32 product_id; > + u32 usage; > int start_collection_index; > int end_collection_index; > + struct mutex mutex; > + struct sensor_hub_pending pending; > }; > > /** > -- 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
