[PATCH 1/3] HID: Delay opening HID device

[PATCH 1/3] HID: Delay opening HID device

[Srinivas Pandruvada]

Don't call hid_open_device till there is actually an user. This saves
power by not opening underlying transport for HID. Also close device
if there are no active mfd client using HID sensor hub.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/hid/hid-sensor-hub.c   | 45 ++++++++++++++++++++++++++++++++----------
 include/linux/hid-sensor-hub.h | 18 +++++++++++++++++
 2 files changed, 53 insertions(+), 10 deletions(-)

diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 10e1581..8effdee 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -465,6 +465,39 @@ static int sensor_hub_raw_event(struct hid_device *hdev,
 	return 1;
 }
 
+int sensor_hub_device_open(struct hid_sensor_hub_device *hsdev)
+{
+	int ret;
+	struct sensor_hub_data *data =  hid_get_drvdata(hsdev->hdev);
+
+	mutex_lock(&data->mutex);
+	if (!hsdev->ref_cnt) {
+		ret = hid_hw_open(hsdev->hdev);
+		if (ret) {
+			hid_err(hsdev->hdev, "failed to open hid device\n");
+			mutex_unlock(&data->mutex);
+			return ret;
+		}
+	}
+	hsdev->ref_cnt++;
+	mutex_unlock(&data->mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(sensor_hub_device_open);
+
+void sensor_hub_device_close(struct hid_sensor_hub_device *hsdev)
+{
+	struct sensor_hub_data *data =  hid_get_drvdata(hsdev->hdev);
+
+	mutex_lock(&data->mutex);
+	hsdev->ref_cnt--;
+	if (!hsdev->ref_cnt)
+		hid_hw_close(hsdev->hdev);
+	mutex_unlock(&data->mutex);
+}
+EXPORT_SYMBOL_GPL(sensor_hub_device_close);
+
 static int sensor_hub_probe(struct hid_device *hdev,
 				const struct hid_device_id *id)
 {
@@ -506,12 +539,6 @@ static int sensor_hub_probe(struct hid_device *hdev,
 		hid_err(hdev, "hw start failed\n");
 		return ret;
 	}
-	ret = hid_hw_open(hdev);
-	if (ret) {
-		hid_err(hdev, "failed to open input interrupt pipe\n");
-		goto err_stop_hw;
-	}
-
 	INIT_LIST_HEAD(&sd->dyn_callback_list);
 	sd->hid_sensor_client_cnt = 0;
 	report_enum = &hdev->report_enum[HID_INPUT_REPORT];
@@ -520,7 +547,7 @@ static int sensor_hub_probe(struct hid_device *hdev,
 	if (dev_cnt > HID_MAX_PHY_DEVICES) {
 		hid_err(hdev, "Invalid Physical device count\n");
 		ret = -EINVAL;
-		goto err_close;
+		goto err_stop_hw;
 	}
 	sd->hid_sensor_hub_client_devs = kzalloc(dev_cnt *
 						sizeof(struct mfd_cell),
@@ -528,7 +555,7 @@ static int sensor_hub_probe(struct hid_device *hdev,
 	if (sd->hid_sensor_hub_client_devs == NULL) {
 		hid_err(hdev, "Failed to allocate memory for mfd cells\n");
 			ret = -ENOMEM;
-			goto err_close;
+			goto err_stop_hw;
 	}
 	list_for_each_entry(report, &report_enum->report_list, list) {
 		hid_dbg(hdev, "Report id:%x\n", report->id);
@@ -565,8 +592,6 @@ err_free_names:
 	for (i = 0; i < sd->hid_sensor_client_cnt ; ++i)
 		kfree(sd->hid_sensor_hub_client_devs[i].name);
 	kfree(sd->hid_sensor_hub_client_devs);
-err_close:
-	hid_hw_close(hdev);
 err_stop_hw:
 	hid_hw_stop(hdev);
 
diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h
index 32ba451..a265af2 100644
--- a/include/linux/hid-sensor-hub.h
+++ b/include/linux/hid-sensor-hub.h
@@ -47,11 +47,13 @@ struct hid_sensor_hub_attribute_info {
  * @hdev:		Stores the hid instance.
  * @vendor_id:		Vendor id of hub device.
  * @product_id:		Product id of hub device.
+ * @ref_cnt:		Number of MFD clients have opened this device
  */
 struct hid_sensor_hub_device {
 	struct hid_device *hdev;
 	u32 vendor_id;
 	u32 product_id;
+	int ref_cnt;
 };
 
 /**
@@ -74,6 +76,22 @@ struct hid_sensor_hub_callbacks {
 			 void *priv);
 };
 
+/**
+* sensor_hub_device_open() - Open hub device
+* @hsdev:	Hub device instance.
+*
+* Used to open hid device for sensor hub.
+*/
+int sensor_hub_device_open(struct hid_sensor_hub_device *hsdev);
+
+/**
+* sensor_hub_device_clode() - Close hub device
+* @hsdev:	Hub device instance.
+*
+* Used to clode hid device for sensor hub.
+*/
+void sensor_hub_device_close(struct hid_sensor_hub_device *hsdev);
+
 /* Registration functions */
 
 /**
-- 
1.8.3.1

Re: [PATCH 1/3] HID: Delay opening HID device

[Jonathan Cameron]

On 09/10/13 21:03, Srinivas Pandruvada wrote:
> Don't call hid_open_device till there is actually an user. This saves
> power by not opening underlying transport for HID. Also close device
> if there are no active mfd client using HID sensor hub.
> 
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Buglet inline.
> ---
>  drivers/hid/hid-sensor-hub.c   | 45 ++++++++++++++++++++++++++++++++----------
>  include/linux/hid-sensor-hub.h | 18 +++++++++++++++++
>  2 files changed, 53 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
> index 10e1581..8effdee 100644
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c
> @@ -465,6 +465,39 @@ static int sensor_hub_raw_event(struct hid_device *hdev,
>  	return 1;
>  }
>  
> +int sensor_hub_device_open(struct hid_sensor_hub_device *hsdev)
> +{
> +	int ret;
> +	struct sensor_hub_data *data =  hid_get_drvdata(hsdev->hdev);
> +
> +	mutex_lock(&data->mutex);
> +	if (!hsdev->ref_cnt) {
> +		ret = hid_hw_open(hsdev->hdev);
> +		if (ret) {
> +			hid_err(hsdev->hdev, "failed to open hid device\n");
> +			mutex_unlock(&data->mutex);
> +			return ret;
> +		}
> +	}
> +	hsdev->ref_cnt++;
> +	mutex_unlock(&data->mutex);
> +
> +	return ret;
Not initialized.

> +}
> +EXPORT_SYMBOL_GPL(sensor_hub_device_open);
> +
> +void sensor_hub_device_close(struct hid_sensor_hub_device *hsdev)
> +{
> +	struct sensor_hub_data *data =  hid_get_drvdata(hsdev->hdev);
> +
> +	mutex_lock(&data->mutex);
> +	hsdev->ref_cnt--;
> +	if (!hsdev->ref_cnt)
> +		hid_hw_close(hsdev->hdev);
> +	mutex_unlock(&data->mutex);
> +}
> +EXPORT_SYMBOL_GPL(sensor_hub_device_close);
> +
>  static int sensor_hub_probe(struct hid_device *hdev,
>  				const struct hid_device_id *id)
>  {
> @@ -506,12 +539,6 @@ static int sensor_hub_probe(struct hid_device *hdev,
>  		hid_err(hdev, "hw start failed\n");
>  		return ret;
>  	}
> -	ret = hid_hw_open(hdev);
> -	if (ret) {
> -		hid_err(hdev, "failed to open input interrupt pipe\n");
> -		goto err_stop_hw;
> -	}
> -
>  	INIT_LIST_HEAD(&sd->dyn_callback_list);
>  	sd->hid_sensor_client_cnt = 0;
>  	report_enum = &hdev->report_enum[HID_INPUT_REPORT];
> @@ -520,7 +547,7 @@ static int sensor_hub_probe(struct hid_device *hdev,
>  	if (dev_cnt > HID_MAX_PHY_DEVICES) {
>  		hid_err(hdev, "Invalid Physical device count\n");
>  		ret = -EINVAL;
> -		goto err_close;
> +		goto err_stop_hw;
>  	}
>  	sd->hid_sensor_hub_client_devs = kzalloc(dev_cnt *
>  						sizeof(struct mfd_cell),
> @@ -528,7 +555,7 @@ static int sensor_hub_probe(struct hid_device *hdev,
>  	if (sd->hid_sensor_hub_client_devs == NULL) {
>  		hid_err(hdev, "Failed to allocate memory for mfd cells\n");
>  			ret = -ENOMEM;
> -			goto err_close;
> +			goto err_stop_hw;
>  	}
>  	list_for_each_entry(report, &report_enum->report_list, list) {
>  		hid_dbg(hdev, "Report id:%x\n", report->id);
> @@ -565,8 +592,6 @@ err_free_names:
>  	for (i = 0; i < sd->hid_sensor_client_cnt ; ++i)
>  		kfree(sd->hid_sensor_hub_client_devs[i].name);
>  	kfree(sd->hid_sensor_hub_client_devs);
> -err_close:
> -	hid_hw_close(hdev);
>  err_stop_hw:
>  	hid_hw_stop(hdev);
>  
> diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h
> index 32ba451..a265af2 100644
> --- a/include/linux/hid-sensor-hub.h
> +++ b/include/linux/hid-sensor-hub.h
> @@ -47,11 +47,13 @@ struct hid_sensor_hub_attribute_info {
>   * @hdev:		Stores the hid instance.
>   * @vendor_id:		Vendor id of hub device.
>   * @product_id:		Product id of hub device.
> + * @ref_cnt:		Number of MFD clients have opened this device
>   */
>  struct hid_sensor_hub_device {
>  	struct hid_device *hdev;
>  	u32 vendor_id;
>  	u32 product_id;
> +	int ref_cnt;
>  };
>  
>  /**
> @@ -74,6 +76,22 @@ struct hid_sensor_hub_callbacks {
>  			 void *priv);
>  };
>  
> +/**
> +* sensor_hub_device_open() - Open hub device
> +* @hsdev:	Hub device instance.
> +*
> +* Used to open hid device for sensor hub.
> +*/
> +int sensor_hub_device_open(struct hid_sensor_hub_device *hsdev);
> +
> +/**
> +* sensor_hub_device_clode() - Close hub device
> +* @hsdev:	Hub device instance.
> +*
> +* Used to clode hid device for sensor hub.
> +*/
> +void sensor_hub_device_close(struct hid_sensor_hub_device *hsdev);
> +
>  /* Registration functions */
>  
>  /**
> 

Re: [PATCH 1/3] HID: Delay opening HID device

[Srinivas Pandruvada]

On 09/15/2013 03:15 AM, Jonathan Cameron wrote:
> On 09/10/13 21:03, Srinivas Pandruvada wrote:
>> Don't call hid_open_device till there is actually an user. This saves
>> power by not opening underlying transport for HID. Also close device
>> if there are no active mfd client using HID sensor hub.
>>
>> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Buglet inline.
>> ---
>>   drivers/hid/hid-sensor-hub.c   | 45 ++++++++++++++++++++++++++++++++----------
>>   include/linux/hid-sensor-hub.h | 18 +++++++++++++++++
>>   2 files changed, 53 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
>> index 10e1581..8effdee 100644
>> --- a/drivers/hid/hid-sensor-hub.c
>> +++ b/drivers/hid/hid-sensor-hub.c
>> @@ -465,6 +465,39 @@ static int sensor_hub_raw_event(struct hid_device *hdev,
>>   	return 1;
>>   }
>>   
>> +int sensor_hub_device_open(struct hid_sensor_hub_device *hsdev)
>> +{
>> +	int ret;
>> +	struct sensor_hub_data *data =  hid_get_drvdata(hsdev->hdev);
>> +
>> +	mutex_lock(&data->mutex);
>> +	if (!hsdev->ref_cnt) {
>> +		ret = hid_hw_open(hsdev->hdev);
>> +		if (ret) {
>> +			hid_err(hsdev->hdev, "failed to open hid device\n");
>> +			mutex_unlock(&data->mutex);
>> +			return ret;
>> +		}
>> +	}
>> +	hsdev->ref_cnt++;
>> +	mutex_unlock(&data->mutex);
>> +
>> +	return ret;
> Not initialized.
Good catch.
>> +}
>> +EXPORT_SYMBOL_GPL(sensor_hub_device_open);
>> +
>> +void sensor_hub_device_close(struct hid_sensor_hub_device *hsdev)
>> +{
>> +	struct sensor_hub_data *data =  hid_get_drvdata(hsdev->hdev);
>> +
>> +	mutex_lock(&data->mutex);
>> +	hsdev->ref_cnt--;
>> +	if (!hsdev->ref_cnt)
>> +		hid_hw_close(hsdev->hdev);
>> +	mutex_unlock(&data->mutex);
>> +}
>> +EXPORT_SYMBOL_GPL(sensor_hub_device_close);
>> +
>>   static int sensor_hub_probe(struct hid_device *hdev,
>>   				const struct hid_device_id *id)
>>   {
>> @@ -506,12 +539,6 @@ static int sensor_hub_probe(struct hid_device *hdev,
>>   		hid_err(hdev, "hw start failed\n");
>>   		return ret;
>>   	}
>> -	ret = hid_hw_open(hdev);
>> -	if (ret) {
>> -		hid_err(hdev, "failed to open input interrupt pipe\n");
>> -		goto err_stop_hw;
>> -	}
>> -
>>   	INIT_LIST_HEAD(&sd->dyn_callback_list);
>>   	sd->hid_sensor_client_cnt = 0;
>>   	report_enum = &hdev->report_enum[HID_INPUT_REPORT];
>> @@ -520,7 +547,7 @@ static int sensor_hub_probe(struct hid_device *hdev,
>>   	if (dev_cnt > HID_MAX_PHY_DEVICES) {
>>   		hid_err(hdev, "Invalid Physical device count\n");
>>   		ret = -EINVAL;
>> -		goto err_close;
>> +		goto err_stop_hw;
>>   	}
>>   	sd->hid_sensor_hub_client_devs = kzalloc(dev_cnt *
>>   						sizeof(struct mfd_cell),
>> @@ -528,7 +555,7 @@ static int sensor_hub_probe(struct hid_device *hdev,
>>   	if (sd->hid_sensor_hub_client_devs == NULL) {
>>   		hid_err(hdev, "Failed to allocate memory for mfd cells\n");
>>   			ret = -ENOMEM;
>> -			goto err_close;
>> +			goto err_stop_hw;
>>   	}
>>   	list_for_each_entry(report, &report_enum->report_list, list) {
>>   		hid_dbg(hdev, "Report id:%x\n", report->id);
>> @@ -565,8 +592,6 @@ err_free_names:
>>   	for (i = 0; i < sd->hid_sensor_client_cnt ; ++i)
>>   		kfree(sd->hid_sensor_hub_client_devs[i].name);
>>   	kfree(sd->hid_sensor_hub_client_devs);
>> -err_close:
>> -	hid_hw_close(hdev);
>>   err_stop_hw:
>>   	hid_hw_stop(hdev);
>>   
>> diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h
>> index 32ba451..a265af2 100644
>> --- a/include/linux/hid-sensor-hub.h
>> +++ b/include/linux/hid-sensor-hub.h
>> @@ -47,11 +47,13 @@ struct hid_sensor_hub_attribute_info {
>>    * @hdev:		Stores the hid instance.
>>    * @vendor_id:		Vendor id of hub device.
>>    * @product_id:		Product id of hub device.
>> + * @ref_cnt:		Number of MFD clients have opened this device
>>    */
>>   struct hid_sensor_hub_device {
>>   	struct hid_device *hdev;
>>   	u32 vendor_id;
>>   	u32 product_id;
>> +	int ref_cnt;
>>   };
>>   
>>   /**
>> @@ -74,6 +76,22 @@ struct hid_sensor_hub_callbacks {
>>   			 void *priv);
>>   };
>>   
>> +/**
>> +* sensor_hub_device_open() - Open hub device
>> +* @hsdev:	Hub device instance.
>> +*
>> +* Used to open hid device for sensor hub.
>> +*/
>> +int sensor_hub_device_open(struct hid_sensor_hub_device *hsdev);
>> +
>> +/**
>> +* sensor_hub_device_clode() - Close hub device
>> +* @hsdev:	Hub device instance.
>> +*
>> +* Used to clode hid device for sensor hub.
>> +*/
>> +void sensor_hub_device_close(struct hid_sensor_hub_device *hsdev);
>> +
>>   /* Registration functions */
>>   
>>   /**
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Thanks,
Srinivas

[PATCH 1/3] HID: Delay opening HID device

[Srinivas Pandruvada]

Don't call hid_open_device till there is actually an user. This saves
power by not opening underlying transport for HID. Also close device
if there are no active mfd client using HID sensor hub.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/hid/hid-sensor-hub.c   | 45 ++++++++++++++++++++++++++++++++----------
 include/linux/hid-sensor-hub.h | 18 +++++++++++++++++
 2 files changed, 53 insertions(+), 10 deletions(-)

diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 10e1581..88fc5ae 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -465,6 +465,39 @@ static int sensor_hub_raw_event(struct hid_device *hdev,
 	return 1;
 }
 
+int sensor_hub_device_open(struct hid_sensor_hub_device *hsdev)
+{
+	int ret = 0;
+	struct sensor_hub_data *data =  hid_get_drvdata(hsdev->hdev);
+
+	mutex_lock(&data->mutex);
+	if (!hsdev->ref_cnt) {
+		ret = hid_hw_open(hsdev->hdev);
+		if (ret) {
+			hid_err(hsdev->hdev, "failed to open hid device\n");
+			mutex_unlock(&data->mutex);
+			return ret;
+		}
+	}
+	hsdev->ref_cnt++;
+	mutex_unlock(&data->mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(sensor_hub_device_open);
+
+void sensor_hub_device_close(struct hid_sensor_hub_device *hsdev)
+{
+	struct sensor_hub_data *data =  hid_get_drvdata(hsdev->hdev);
+
+	mutex_lock(&data->mutex);
+	hsdev->ref_cnt--;
+	if (!hsdev->ref_cnt)
+		hid_hw_close(hsdev->hdev);
+	mutex_unlock(&data->mutex);
+}
+EXPORT_SYMBOL_GPL(sensor_hub_device_close);
+
 static int sensor_hub_probe(struct hid_device *hdev,
 				const struct hid_device_id *id)
 {
@@ -506,12 +539,6 @@ static int sensor_hub_probe(struct hid_device *hdev,
 		hid_err(hdev, "hw start failed\n");
 		return ret;
 	}
-	ret = hid_hw_open(hdev);
-	if (ret) {
-		hid_err(hdev, "failed to open input interrupt pipe\n");
-		goto err_stop_hw;
-	}
-
 	INIT_LIST_HEAD(&sd->dyn_callback_list);
 	sd->hid_sensor_client_cnt = 0;
 	report_enum = &hdev->report_enum[HID_INPUT_REPORT];
@@ -520,7 +547,7 @@ static int sensor_hub_probe(struct hid_device *hdev,
 	if (dev_cnt > HID_MAX_PHY_DEVICES) {
 		hid_err(hdev, "Invalid Physical device count\n");
 		ret = -EINVAL;
-		goto err_close;
+		goto err_stop_hw;
 	}
 	sd->hid_sensor_hub_client_devs = kzalloc(dev_cnt *
 						sizeof(struct mfd_cell),
@@ -528,7 +555,7 @@ static int sensor_hub_probe(struct hid_device *hdev,
 	if (sd->hid_sensor_hub_client_devs == NULL) {
 		hid_err(hdev, "Failed to allocate memory for mfd cells\n");
 			ret = -ENOMEM;
-			goto err_close;
+			goto err_stop_hw;
 	}
 	list_for_each_entry(report, &report_enum->report_list, list) {
 		hid_dbg(hdev, "Report id:%x\n", report->id);
@@ -565,8 +592,6 @@ err_free_names:
 	for (i = 0; i < sd->hid_sensor_client_cnt ; ++i)
 		kfree(sd->hid_sensor_hub_client_devs[i].name);
 	kfree(sd->hid_sensor_hub_client_devs);
-err_close:
-	hid_hw_close(hdev);
 err_stop_hw:
 	hid_hw_stop(hdev);
 
diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h
index 32ba451..a265af2 100644
--- a/include/linux/hid-sensor-hub.h
+++ b/include/linux/hid-sensor-hub.h
@@ -47,11 +47,13 @@ struct hid_sensor_hub_attribute_info {
  * @hdev:		Stores the hid instance.
  * @vendor_id:		Vendor id of hub device.
  * @product_id:		Product id of hub device.
+ * @ref_cnt:		Number of MFD clients have opened this device
  */
 struct hid_sensor_hub_device {
 	struct hid_device *hdev;
 	u32 vendor_id;
 	u32 product_id;
+	int ref_cnt;
 };
 
 /**
@@ -74,6 +76,22 @@ struct hid_sensor_hub_callbacks {
 			 void *priv);
 };
 
+/**
+* sensor_hub_device_open() - Open hub device
+* @hsdev:	Hub device instance.
+*
+* Used to open hid device for sensor hub.
+*/
+int sensor_hub_device_open(struct hid_sensor_hub_device *hsdev);
+
+/**
+* sensor_hub_device_clode() - Close hub device
+* @hsdev:	Hub device instance.
+*
+* Used to clode hid device for sensor hub.
+*/
+void sensor_hub_device_close(struct hid_sensor_hub_device *hsdev);
+
 /* Registration functions */
 
 /**
-- 
1.8.3.1

[PATCH 2/3] IIO: call sensor hub open close function

[Srinivas Pandruvada]

Call hid_sensor_hub_device_open when user space opens device and call
hid_sensor_hub_device_close when device is closed. This helps in
saving power.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
index 87419c4..b6e77e0 100644
--- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
+++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
@@ -34,6 +34,12 @@ static int hid_sensor_data_rdy_trigger_set_state(struct iio_trigger *trig,
 	struct hid_sensor_common *st = iio_trigger_get_drvdata(trig);
 	int state_val;
 
+	if (state) {
+		if (sensor_hub_device_open(st->hsdev))
+			return -EIO;
+	} else
+		sensor_hub_device_close(st->hsdev);
+
 	state_val = state ? 1 : 0;
 	if (IS_ENABLED(CONFIG_HID_SENSOR_ENUM_BASE_QUIRKS))
 		++state_val;
-- 
1.8.3.1

[PATCH 3/3] HID RTC: Open sensor hub open close

[Srinivas Pandruvada]

Open sensor hub when module is loaded and close when module is removed.
This helps saving power by opening HID transport only when there is an
user.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Acked-by: Alessandro Zummo <a.zummo-BfzFCNDTiLLj+vYz1yj4TQ@public.gmane.org>
---
 drivers/rtc/rtc-hid-sensor-time.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-hid-sensor-time.c b/drivers/rtc/rtc-hid-sensor-time.c
index 7273b01..1fe170c 100644
--- a/drivers/rtc/rtc-hid-sensor-time.c
+++ b/drivers/rtc/rtc-hid-sensor-time.c
@@ -279,15 +279,28 @@ static int hid_time_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	ret = sensor_hub_device_open(hsdev);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to open sensor hub device!\n");
+		goto err_open;
+	}
+
 	time_state->rtc = devm_rtc_device_register(&pdev->dev,
 					"hid-sensor-time", &hid_time_rtc_ops,
 					THIS_MODULE);
 
 	if (IS_ERR(time_state->rtc)) {
 		dev_err(&pdev->dev, "rtc device register failed!\n");
-		return PTR_ERR(time_state->rtc);
+		ret = PTR_ERR(time_state->rtc);
+		goto err_rtc;
 	}
 
+	return 0;
+
+err_rtc:
+	sensor_hub_device_close(hsdev);
+err_open:
+	sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_TIME);
 	return ret;
 }
 
@@ -295,6 +308,7 @@ static int hid_time_remove(struct platform_device *pdev)
 {
 	struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
 
+	sensor_hub_device_close(hsdev);
 	sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_TIME);
 
 	return 0;
-- 
1.8.3.1

Re: [PATCH 1/3] HID: Delay opening HID device

[Jonathan Cameron]

On 09/18/13 18:13, Srinivas Pandruvada wrote:
> Don't call hid_open_device till there is actually an user. This saves
> power by not opening underlying transport for HID. Also close device
> if there are no active mfd client using HID sensor hub.
> 
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

All look fine to me.  Ideally I'd like an Ack from Jiri to cover the bit
under drivers/hid/ before taking it through IIO.

Jonathan
> ---
>  drivers/hid/hid-sensor-hub.c   | 45 ++++++++++++++++++++++++++++++++----------
>  include/linux/hid-sensor-hub.h | 18 +++++++++++++++++
>  2 files changed, 53 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
> index 10e1581..88fc5ae 100644
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c
> @@ -465,6 +465,39 @@ static int sensor_hub_raw_event(struct hid_device *hdev,
>  	return 1;
>  }
>  
> +int sensor_hub_device_open(struct hid_sensor_hub_device *hsdev)
> +{
> +	int ret = 0;
> +	struct sensor_hub_data *data =  hid_get_drvdata(hsdev->hdev);
> +
> +	mutex_lock(&data->mutex);
> +	if (!hsdev->ref_cnt) {
> +		ret = hid_hw_open(hsdev->hdev);
> +		if (ret) {
> +			hid_err(hsdev->hdev, "failed to open hid device\n");
> +			mutex_unlock(&data->mutex);
> +			return ret;
> +		}
> +	}
> +	hsdev->ref_cnt++;
> +	mutex_unlock(&data->mutex);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(sensor_hub_device_open);
> +
> +void sensor_hub_device_close(struct hid_sensor_hub_device *hsdev)
> +{
> +	struct sensor_hub_data *data =  hid_get_drvdata(hsdev->hdev);
> +
> +	mutex_lock(&data->mutex);
> +	hsdev->ref_cnt--;
> +	if (!hsdev->ref_cnt)
> +		hid_hw_close(hsdev->hdev);
> +	mutex_unlock(&data->mutex);
> +}
> +EXPORT_SYMBOL_GPL(sensor_hub_device_close);
> +
>  static int sensor_hub_probe(struct hid_device *hdev,
>  				const struct hid_device_id *id)
>  {
> @@ -506,12 +539,6 @@ static int sensor_hub_probe(struct hid_device *hdev,
>  		hid_err(hdev, "hw start failed\n");
>  		return ret;
>  	}
> -	ret = hid_hw_open(hdev);
> -	if (ret) {
> -		hid_err(hdev, "failed to open input interrupt pipe\n");
> -		goto err_stop_hw;
> -	}
> -
>  	INIT_LIST_HEAD(&sd->dyn_callback_list);
>  	sd->hid_sensor_client_cnt = 0;
>  	report_enum = &hdev->report_enum[HID_INPUT_REPORT];
> @@ -520,7 +547,7 @@ static int sensor_hub_probe(struct hid_device *hdev,
>  	if (dev_cnt > HID_MAX_PHY_DEVICES) {
>  		hid_err(hdev, "Invalid Physical device count\n");
>  		ret = -EINVAL;
> -		goto err_close;
> +		goto err_stop_hw;
>  	}
>  	sd->hid_sensor_hub_client_devs = kzalloc(dev_cnt *
>  						sizeof(struct mfd_cell),
> @@ -528,7 +555,7 @@ static int sensor_hub_probe(struct hid_device *hdev,
>  	if (sd->hid_sensor_hub_client_devs == NULL) {
>  		hid_err(hdev, "Failed to allocate memory for mfd cells\n");
>  			ret = -ENOMEM;
> -			goto err_close;
> +			goto err_stop_hw;
>  	}
>  	list_for_each_entry(report, &report_enum->report_list, list) {
>  		hid_dbg(hdev, "Report id:%x\n", report->id);
> @@ -565,8 +592,6 @@ err_free_names:
>  	for (i = 0; i < sd->hid_sensor_client_cnt ; ++i)
>  		kfree(sd->hid_sensor_hub_client_devs[i].name);
>  	kfree(sd->hid_sensor_hub_client_devs);
> -err_close:
> -	hid_hw_close(hdev);
>  err_stop_hw:
>  	hid_hw_stop(hdev);
>  
> diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h
> index 32ba451..a265af2 100644
> --- a/include/linux/hid-sensor-hub.h
> +++ b/include/linux/hid-sensor-hub.h
> @@ -47,11 +47,13 @@ struct hid_sensor_hub_attribute_info {
>   * @hdev:		Stores the hid instance.
>   * @vendor_id:		Vendor id of hub device.
>   * @product_id:		Product id of hub device.
> + * @ref_cnt:		Number of MFD clients have opened this device
>   */
>  struct hid_sensor_hub_device {
>  	struct hid_device *hdev;
>  	u32 vendor_id;
>  	u32 product_id;
> +	int ref_cnt;
>  };
>  
>  /**
> @@ -74,6 +76,22 @@ struct hid_sensor_hub_callbacks {
>  			 void *priv);
>  };
>  
> +/**
> +* sensor_hub_device_open() - Open hub device
> +* @hsdev:	Hub device instance.
> +*
> +* Used to open hid device for sensor hub.
> +*/
> +int sensor_hub_device_open(struct hid_sensor_hub_device *hsdev);
> +
> +/**
> +* sensor_hub_device_clode() - Close hub device
> +* @hsdev:	Hub device instance.
> +*
> +* Used to clode hid device for sensor hub.
> +*/
> +void sensor_hub_device_close(struct hid_sensor_hub_device *hsdev);
> +
>  /* Registration functions */
>  
>  /**
> 

Re: [PATCH 1/3] HID: Delay opening HID device

[Jiri Kosina]

On Wed, 18 Sep 2013, Srinivas Pandruvada wrote:

> Don't call hid_open_device till there is actually an user. This saves
> power by not opening underlying transport for HID. Also close device
> if there are no active mfd client using HID sensor hub.
> 
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Signed-off-by: Jiri Kosina <jkosina-AlSwsSmVLrQ@public.gmane.org>

Jonathan, I'd be good if I pass this to you to apply the whole lot.

Thanks.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 1/3] HID: Delay opening HID device

[Jonathan Cameron]

On 09/24/13 10:32, Jiri Kosina wrote:
> On Wed, 18 Sep 2013, Srinivas Pandruvada wrote:
> 
>> Don't call hid_open_device till there is actually an user. This saves
>> power by not opening underlying transport for HID. Also close device
>> if there are no active mfd client using HID sensor hub.
>>
>> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> 
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> 
> Jonathan, I'd be good if I pass this to you to apply the whole lot.
Applied to the togreg branch of iio.git

Thanks,

Re: [PATCH 2/3] IIO: call sensor hub open close function

[Jonathan Cameron]

On 09/18/13 18:13, Srinivas Pandruvada wrote:
> Call hid_sensor_hub_device_open when user space opens device and call
> hid_sensor_hub_device_close when device is closed. This helps in
> saving power.
> 
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Applied to the togreg branch of iio.git

Thanks,
> ---
>  drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> index 87419c4..b6e77e0 100644
> --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> @@ -34,6 +34,12 @@ static int hid_sensor_data_rdy_trigger_set_state(struct iio_trigger *trig,
>  	struct hid_sensor_common *st = iio_trigger_get_drvdata(trig);
>  	int state_val;
>  
> +	if (state) {
> +		if (sensor_hub_device_open(st->hsdev))
> +			return -EIO;
> +	} else
> +		sensor_hub_device_close(st->hsdev);
> +
>  	state_val = state ? 1 : 0;
>  	if (IS_ENABLED(CONFIG_HID_SENSOR_ENUM_BASE_QUIRKS))
>  		++state_val;
> 

Re: [PATCH 3/3] HID RTC: Open sensor hub open close

[Jonathan Cameron]

On 09/18/13 18:13, Srinivas Pandruvada wrote:
> Open sensor hub when module is loaded and close when module is removed.
> This helps saving power by opening HID transport only when there is an
> user.
> 
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Acked-by: Alessandro Zummo <a.zummo-BfzFCNDTiLLj+vYz1yj4TQ@public.gmane.org>
Hi,

It looks like there has been a fair bit of reorganising going on in this driver
that is making this a little fiddly to merge.

Could you send me an updated patch against what is currently in the togreg
branch of iio.git.  As we haven't touched this recently, that should be
the same as mainline.

Jonathan

> ---
>  drivers/rtc/rtc-hid-sensor-time.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/rtc/rtc-hid-sensor-time.c b/drivers/rtc/rtc-hid-sensor-time.c
> index 7273b01..1fe170c 100644
> --- a/drivers/rtc/rtc-hid-sensor-time.c
> +++ b/drivers/rtc/rtc-hid-sensor-time.c
> @@ -279,15 +279,28 @@ static int hid_time_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> +	ret = sensor_hub_device_open(hsdev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to open sensor hub device!\n");
> +		goto err_open;
> +	}
> +
>  	time_state->rtc = devm_rtc_device_register(&pdev->dev,
>  					"hid-sensor-time", &hid_time_rtc_ops,
>  					THIS_MODULE);
>  
>  	if (IS_ERR(time_state->rtc)) {
>  		dev_err(&pdev->dev, "rtc device register failed!\n");
> -		return PTR_ERR(time_state->rtc);
> +		ret = PTR_ERR(time_state->rtc);
> +		goto err_rtc;
>  	}
>  
> +	return 0;
> +
> +err_rtc:
> +	sensor_hub_device_close(hsdev);
> +err_open:
> +	sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_TIME);
>  	return ret;
>  }
>  
> @@ -295,6 +308,7 @@ static int hid_time_remove(struct platform_device *pdev)
>  {
>  	struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
>  
> +	sensor_hub_device_close(hsdev);
>  	sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_TIME);
>  
>  	return 0;
>