All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] iio: hid-sensor-trigger: Change get poll value function order to avoid sensor properties losing after resume from S3
  2017-02-22  9:17 [PATCH] iio: hid-sensor-trigger: Change get poll value function order to avoid sensor properties losing after resume from S3 Song Hongyan
@ 2017-02-22  5:40     ` Pandruvada, Srinivas
  0 siblings, 0 replies; 10+ messages in thread
From: Pandruvada, Srinivas @ 2017-02-22  5:40 UTC (permalink / raw)
  To: linux-input-u79uwXL29TY76Z2rM5mHXA, Song, Hongyan,
	linux-iio-u79uwXL29TY76Z2rM5mHXA
  Cc: jikos-DgEjT+Ai2ygdnm+yROfE0A, jic23-DgEjT+Ai2ygdnm+yROfE0A

On Wed, 2017-02-22 at 17:17 +0800, Song Hongyan wrote:
> In function _hid_sensor_power_state(), when
> hid_sensor_read_poll_value()
> is called, sensor's all properties will be updated by the value from
> sensor hardware/firmware.
> In some implementation, sensor hardware/firmware will do a power
> cycle
> during S3. In this case, after resume, once
> hid_sensor_read_poll_value()
> is called, sensor's all properties which are kept by driver during S3
> will be changed to default value.
> But instead, if a set feature function is called first, sensor
> hardware/firmware will be recovered to the last status. So change the
> sensor_hub_set_feature() calling order to behind of set feature
> function
> to avoid sensor properties lose.
> 
> Signed-off-by: Song Hongyan <hongyan.song@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

> ---
>  drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> index a3cce3a..ecf592d 100644
> --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> @@ -51,8 +51,6 @@ static int _hid_sensor_power_state(struct
> hid_sensor_common *st, bool state)
>  			st->report_state.report_id,
>  			st->report_state.index,
>  			HID_USAGE_SENSOR_PROP_REPORTING_STATE_ALL_EV
> ENTS_ENUM);
> -
> -		poll_value = hid_sensor_read_poll_value(st);
>  	} else {
>  		int val;
>  
> @@ -89,7 +87,9 @@ static 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,
>  			       sizeof(state_val), &state_val);
> -	if (state && poll_value)
> +	if (state)
> +		poll_value = hid_sensor_read_poll_value(st);
> +	if (poll_value > 0)
>  		msleep_interruptible(poll_value * 2);
>  
>  	return 0;

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] iio: hid-sensor-trigger: Change get poll value function order to avoid sensor properties losing after resume from S3
@ 2017-02-22  5:40     ` Pandruvada, Srinivas
  0 siblings, 0 replies; 10+ messages in thread
From: Pandruvada, Srinivas @ 2017-02-22  5:40 UTC (permalink / raw)
  To: linux-input, Song, Hongyan, linux-iio; +Cc: jikos, jic23

T24gV2VkLCAyMDE3LTAyLTIyIGF0IDE3OjE3ICswODAwLCBTb25nIEhvbmd5YW4gd3JvdGU6DQo+
IEluIGZ1bmN0aW9uIF9oaWRfc2Vuc29yX3Bvd2VyX3N0YXRlKCksIHdoZW4NCj4gaGlkX3NlbnNv
cl9yZWFkX3BvbGxfdmFsdWUoKQ0KPiBpcyBjYWxsZWQsIHNlbnNvcidzIGFsbCBwcm9wZXJ0aWVz
IHdpbGwgYmUgdXBkYXRlZCBieSB0aGUgdmFsdWUgZnJvbQ0KPiBzZW5zb3IgaGFyZHdhcmUvZmly
bXdhcmUuDQo+IEluIHNvbWUgaW1wbGVtZW50YXRpb24sIHNlbnNvciBoYXJkd2FyZS9maXJtd2Fy
ZSB3aWxsIGRvIGEgcG93ZXINCj4gY3ljbGUNCj4gZHVyaW5nIFMzLiBJbiB0aGlzIGNhc2UsIGFm
dGVyIHJlc3VtZSwgb25jZQ0KPiBoaWRfc2Vuc29yX3JlYWRfcG9sbF92YWx1ZSgpDQo+IGlzIGNh
bGxlZCwgc2Vuc29yJ3MgYWxsIHByb3BlcnRpZXMgd2hpY2ggYXJlIGtlcHQgYnkgZHJpdmVyIGR1
cmluZyBTMw0KPiB3aWxsIGJlIGNoYW5nZWQgdG8gZGVmYXVsdCB2YWx1ZS4NCj4gQnV0IGluc3Rl
YWQsIGlmIGEgc2V0IGZlYXR1cmUgZnVuY3Rpb24gaXMgY2FsbGVkIGZpcnN0LCBzZW5zb3INCj4g
aGFyZHdhcmUvZmlybXdhcmUgd2lsbCBiZSByZWNvdmVyZWQgdG8gdGhlIGxhc3Qgc3RhdHVzLiBT
byBjaGFuZ2UgdGhlDQo+IHNlbnNvcl9odWJfc2V0X2ZlYXR1cmUoKSBjYWxsaW5nIG9yZGVyIHRv
IGJlaGluZCBvZiBzZXQgZmVhdHVyZQ0KPiBmdW5jdGlvbg0KPiB0byBhdm9pZCBzZW5zb3IgcHJv
cGVydGllcyBsb3NlLg0KPiANCj4gU2lnbmVkLW9mZi1ieTogU29uZyBIb25neWFuIDxob25neWFu
LnNvbmdAaW50ZWwuY29tPg0KQWNrZWQtYnk6IFNyaW5pdmFzIFBhbmRydXZhZGEgPHNyaW5pdmFz
LnBhbmRydXZhZGFAbGludXguaW50ZWwuY29tPg0KDQo+IC0tLQ0KPiDCoGRyaXZlcnMvaWlvL2Nv
bW1vbi9oaWQtc2Vuc29ycy9oaWQtc2Vuc29yLXRyaWdnZXIuYyB8IDYgKysrLS0tDQo+IMKgMSBm
aWxlIGNoYW5nZWQsIDMgaW5zZXJ0aW9ucygrKSwgMyBkZWxldGlvbnMoLSkNCj4gDQo+IGRpZmYg
LS1naXQgYS9kcml2ZXJzL2lpby9jb21tb24vaGlkLXNlbnNvcnMvaGlkLXNlbnNvci10cmlnZ2Vy
LmMNCj4gYi9kcml2ZXJzL2lpby9jb21tb24vaGlkLXNlbnNvcnMvaGlkLXNlbnNvci10cmlnZ2Vy
LmMNCj4gaW5kZXggYTNjY2UzYS4uZWNmNTkyZCAxMDA2NDQNCj4gLS0tIGEvZHJpdmVycy9paW8v
Y29tbW9uL2hpZC1zZW5zb3JzL2hpZC1zZW5zb3ItdHJpZ2dlci5jDQo+ICsrKyBiL2RyaXZlcnMv
aWlvL2NvbW1vbi9oaWQtc2Vuc29ycy9oaWQtc2Vuc29yLXRyaWdnZXIuYw0KPiBAQCAtNTEsOCAr
NTEsNiBAQCBzdGF0aWMgaW50IF9oaWRfc2Vuc29yX3Bvd2VyX3N0YXRlKHN0cnVjdA0KPiBoaWRf
c2Vuc29yX2NvbW1vbiAqc3QsIGJvb2wgc3RhdGUpDQo+IMKgCQkJc3QtPnJlcG9ydF9zdGF0ZS5y
ZXBvcnRfaWQsDQo+IMKgCQkJc3QtPnJlcG9ydF9zdGF0ZS5pbmRleCwNCj4gwqAJCQlISURfVVNB
R0VfU0VOU09SX1BST1BfUkVQT1JUSU5HX1NUQVRFX0FMTF9FVg0KPiBFTlRTX0VOVU0pOw0KPiAt
DQo+IC0JCXBvbGxfdmFsdWUgPSBoaWRfc2Vuc29yX3JlYWRfcG9sbF92YWx1ZShzdCk7DQo+IMKg
CX0gZWxzZSB7DQo+IMKgCQlpbnQgdmFsOw0KPiDCoA0KPiBAQCAtODksNyArODcsOSBAQCBzdGF0
aWMgaW50IF9oaWRfc2Vuc29yX3Bvd2VyX3N0YXRlKHN0cnVjdA0KPiBoaWRfc2Vuc29yX2NvbW1v
biAqc3QsIGJvb2wgc3RhdGUpDQo+IMKgCXNlbnNvcl9odWJfZ2V0X2ZlYXR1cmUoc3QtPmhzZGV2
LCBzdC0+cG93ZXJfc3RhdGUucmVwb3J0X2lkLA0KPiDCoAkJCcKgwqDCoMKgwqDCoMKgc3QtPnBv
d2VyX3N0YXRlLmluZGV4LA0KPiDCoAkJCcKgwqDCoMKgwqDCoMKgc2l6ZW9mKHN0YXRlX3ZhbCks
ICZzdGF0ZV92YWwpOw0KPiAtCWlmIChzdGF0ZSAmJiBwb2xsX3ZhbHVlKQ0KPiArCWlmIChzdGF0
ZSkNCj4gKwkJcG9sbF92YWx1ZSA9IGhpZF9zZW5zb3JfcmVhZF9wb2xsX3ZhbHVlKHN0KTsNCj4g
KwlpZiAocG9sbF92YWx1ZSA+IDApDQo+IMKgCQltc2xlZXBfaW50ZXJydXB0aWJsZShwb2xsX3Zh
bHVlICogMik7DQo+IMKgDQo+IMKgCXJldHVybiAwOw==

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] iio: hid-sensor-trigger: Change get poll value function order to avoid sensor properties losing after resume from S3
@ 2017-02-22  9:17 Song Hongyan
       [not found] ` <1487755058-31310-1-git-send-email-hongyan.song-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Song Hongyan @ 2017-02-22  9:17 UTC (permalink / raw)
  To: linux-input, linux-iio; +Cc: jikos, jic23, srinivas.pandruvada, Song Hongyan

In function _hid_sensor_power_state(), when hid_sensor_read_poll_value()
is called, sensor's all properties will be updated by the value from
sensor hardware/firmware.
In some implementation, sensor hardware/firmware will do a power cycle
during S3. In this case, after resume, once hid_sensor_read_poll_value()
is called, sensor's all properties which are kept by driver during S3
will be changed to default value.
But instead, if a set feature function is called first, sensor
hardware/firmware will be recovered to the last status. So change the
sensor_hub_set_feature() calling order to behind of set feature function
to avoid sensor properties lose.

Signed-off-by: Song Hongyan <hongyan.song@intel.com>
---
 drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
index a3cce3a..ecf592d 100644
--- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
+++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
@@ -51,8 +51,6 @@ static int _hid_sensor_power_state(struct hid_sensor_common *st, bool state)
 			st->report_state.report_id,
 			st->report_state.index,
 			HID_USAGE_SENSOR_PROP_REPORTING_STATE_ALL_EVENTS_ENUM);
-
-		poll_value = hid_sensor_read_poll_value(st);
 	} else {
 		int val;
 
@@ -89,7 +87,9 @@ static 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,
 			       sizeof(state_val), &state_val);
-	if (state && poll_value)
+	if (state)
+		poll_value = hid_sensor_read_poll_value(st);
+	if (poll_value > 0)
 		msleep_interruptible(poll_value * 2);
 
 	return 0;
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] iio: hid-sensor-trigger: Change get poll value function order to avoid sensor properties losing after resume from S3
  2017-02-22  5:40     ` Pandruvada, Srinivas
@ 2017-02-25 16:51         ` Jonathan Cameron
  -1 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2017-02-25 16:51 UTC (permalink / raw)
  To: Pandruvada, Srinivas, linux-input-u79uwXL29TY76Z2rM5mHXA, Song,
	Hongyan, linux-iio-u79uwXL29TY76Z2rM5mHXA
  Cc: jikos-DgEjT+Ai2ygdnm+yROfE0A

On 22/02/17 05:40, Pandruvada, Srinivas wrote:
> On Wed, 2017-02-22 at 17:17 +0800, Song Hongyan wrote:
>> In function _hid_sensor_power_state(), when
>> hid_sensor_read_poll_value()
>> is called, sensor's all properties will be updated by the value from
>> sensor hardware/firmware.
>> In some implementation, sensor hardware/firmware will do a power
>> cycle
>> during S3. In this case, after resume, once
>> hid_sensor_read_poll_value()
>> is called, sensor's all properties which are kept by driver during S3
>> will be changed to default value.
>> But instead, if a set feature function is called first, sensor
>> hardware/firmware will be recovered to the last status. So change the
>> sensor_hub_set_feature() calling order to behind of set feature
>> function
>> to avoid sensor properties lose.
>>
>> Signed-off-by: Song Hongyan <hongyan.song-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Hi Song,

The patch message, whilst excellent on the technical detail, gives me
no sense of urgency on this. Is this a fix we want to have heading for
stable ASAP or are we looking at something seen in some developmental
hardware for example?
Links to bug reports, or examples of hardware suffering from the issue
are always helpful as well.

Also for something like this a 'fixes' tag is very helpful when people
are looking to back port it.

Anyhow, I'm going to hold off on taking this one until I have more
information. Right now it makes little difference as the next fixes
pull request from me will probably not be until next weekend.

Thanks,

Jonathan
> 
>> ---
>>  drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
>> b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
>> index a3cce3a..ecf592d 100644
>> --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
>> +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
>> @@ -51,8 +51,6 @@ static int _hid_sensor_power_state(struct
>> hid_sensor_common *st, bool state)
>>  			st->report_state.report_id,
>>  			st->report_state.index,
>>  			HID_USAGE_SENSOR_PROP_REPORTING_STATE_ALL_EV
>> ENTS_ENUM);
>> -
>> -		poll_value = hid_sensor_read_poll_value(st);
>>  	} else {
>>  		int val;
>>  
>> @@ -89,7 +87,9 @@ static 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,
>>  			       sizeof(state_val), &state_val);
>> -	if (state && poll_value)
>> +	if (state)
>> +		poll_value = hid_sensor_read_poll_value(st);
>> +	if (poll_value > 0)
>>  		msleep_interruptible(poll_value * 2);
>>  
>>  	return 0;N�����r��y���b�X��ǧv�^�)޺{.n�+����{��*"��^n�r���z�\x1a��h����&��\x1e�G���h�\x03(�階�ݢj"��\x1a�^[m�����z�ޖ���f���h���~�mml==

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] iio: hid-sensor-trigger: Change get poll value function order to avoid sensor properties losing after resume from S3
@ 2017-02-25 16:51         ` Jonathan Cameron
  0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2017-02-25 16:51 UTC (permalink / raw)
  To: Pandruvada, Srinivas, linux-input, Song, Hongyan, linux-iio; +Cc: jikos

On 22/02/17 05:40, Pandruvada, Srinivas wrote:
> On Wed, 2017-02-22 at 17:17 +0800, Song Hongyan wrote:
>> In function _hid_sensor_power_state(), when
>> hid_sensor_read_poll_value()
>> is called, sensor's all properties will be updated by the value from
>> sensor hardware/firmware.
>> In some implementation, sensor hardware/firmware will do a power
>> cycle
>> during S3. In this case, after resume, once
>> hid_sensor_read_poll_value()
>> is called, sensor's all properties which are kept by driver during S3
>> will be changed to default value.
>> But instead, if a set feature function is called first, sensor
>> hardware/firmware will be recovered to the last status. So change the
>> sensor_hub_set_feature() calling order to behind of set feature
>> function
>> to avoid sensor properties lose.
>>
>> Signed-off-by: Song Hongyan <hongyan.song@intel.com>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Hi Song,

The patch message, whilst excellent on the technical detail, gives me
no sense of urgency on this. Is this a fix we want to have heading for
stable ASAP or are we looking at something seen in some developmental
hardware for example?
Links to bug reports, or examples of hardware suffering from the issue
are always helpful as well.

Also for something like this a 'fixes' tag is very helpful when people
are looking to back port it.

Anyhow, I'm going to hold off on taking this one until I have more
information. Right now it makes little difference as the next fixes
pull request from me will probably not be until next weekend.

Thanks,

Jonathan
> 
>> ---
>>  drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
>> b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
>> index a3cce3a..ecf592d 100644
>> --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
>> +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
>> @@ -51,8 +51,6 @@ static int _hid_sensor_power_state(struct
>> hid_sensor_common *st, bool state)
>>  			st->report_state.report_id,
>>  			st->report_state.index,
>>  			HID_USAGE_SENSOR_PROP_REPORTING_STATE_ALL_EV
>> ENTS_ENUM);
>> -
>> -		poll_value = hid_sensor_read_poll_value(st);
>>  	} else {
>>  		int val;
>>  
>> @@ -89,7 +87,9 @@ static 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,
>>  			       sizeof(state_val), &state_val);
>> -	if (state && poll_value)
>> +	if (state)
>> +		poll_value = hid_sensor_read_poll_value(st);
>> +	if (poll_value > 0)
>>  		msleep_interruptible(poll_value * 2);
>>  
>>  	return 0;N�����r��y���b�X��ǧv�^�)޺{.n�+����{��*"��^n�r���z�\x1a��h����&��\x1e�G���h�\x03(�階�ݢj"��\x1a�^[m�����z�ޖ���f���h���~�mml==


^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH] iio: hid-sensor-trigger: Change get poll value function order to avoid sensor properties losing after resume from S3
  2017-02-25 16:51         ` Jonathan Cameron
@ 2017-02-27  1:00             ` Song, Hongyan
  -1 siblings, 0 replies; 10+ messages in thread
From: Song, Hongyan @ 2017-02-27  1:00 UTC (permalink / raw)
  To: Jonathan Cameron, Pandruvada, Srinivas,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA
  Cc: jikos-DgEjT+Ai2ygdnm+yROfE0A

Hi Jonathan,
	I add more information about the patch:
	
Bug found: 
When we cat sensor incli sensor data with hysteresis value is 0, there always have streamed sensor data output,
But when system enter S3 and resume back, the streaming thread is still on but there is not streaming sensor data output.
Test data as below:
279: 2017-01-24 01:16:00.279385 *5* incli_3d: Min Delta: 99.779ms, Max Delta: 100.316ms, Received: 10/s, Overall: 170/16s; Sample values ,0.010546,-0.006712,2.797579
280: 2017-01-24 01:16:41.211385 *5* incli_3d: Min Delta: 100.16ms, Max Delta: 40924.6ms, Received: 2/s, Overall: 172/19s; Sample values ,0.010546,-0.006712,2.797579
281: 2017-01-24 01:16:42.219892 *5* incli_3d: Min Delta: 0.023ms, Max Delta: 543.28ms, Received: 2/s, Overall: 174/20s; Sample values ,0.010546,-0.006712,2.797579
282: 2017-01-24 01:16:42.755548 *4* CIIOSensorInform::sensor_wait_event poll timeout
283: 2017-01-24 01:16:43.223062 *5* incli_3d: Min Delta: --, Max Delta: --, Received: 0/s, Overall: 174/21s
284: 2017-01-24 01:16:43.756980 *4* CIIOSensorInform::sensor_wait_event poll timeout
285: 2017-01-24 01:16:44.225527 *5* incli_3d: Min Delta: --, Max Delta: --, Received: 0/s, Overall: 174/22s
286: 2017-01-24 01:16:44.758467 *4* CIIOSensorInform::sensor_wait_event poll timeout
287: 2017-01-24 01:16:45.228082 *5* incli_3d: Min Delta: --, Max Delta: --, Received: 0/s, Overall: 174/23s
288: 2017-01-24 01:16:45.759958 *4* CIIOSensorInform::sensor_wait_event poll timeout

Root cause: 
when sensor resume back from S3, the sensor property user set is lost.
For example, if we set sensor hysteresis value to be 0, then let system enter S3, when resume back, the hysteresis value will be not 0 but the default value 1.

this is not what we want, resume back from S3 should not lose the properties which is set before S3, just because the property setting sequence lead to this lose.

This is patch is a fix patch and needed for stable.

Thanks a lot!

BR
Song Hongyan


-----Original Message-----
From: Jonathan Cameron [mailto:jic23@kernel.org] 
Sent: Sunday, February 26, 2017 12:52 AM
To: Pandruvada, Srinivas <srinivas.pandruvada@intel.com>; linux-input@vger.kernel.org; Song, Hongyan <hongyan.song@intel.com>; linux-iio@vger.kernel.org
Cc: jikos@kernel.org
Subject: Re: [PATCH] iio: hid-sensor-trigger: Change get poll value function order to avoid sensor properties losing after resume from S3

On 22/02/17 05:40, Pandruvada, Srinivas wrote:
> On Wed, 2017-02-22 at 17:17 +0800, Song Hongyan wrote:
>> In function _hid_sensor_power_state(), when
>> hid_sensor_read_poll_value()
>> is called, sensor's all properties will be updated by the value from 
>> sensor hardware/firmware.
>> In some implementation, sensor hardware/firmware will do a power 
>> cycle during S3. In this case, after resume, once
>> hid_sensor_read_poll_value()
>> is called, sensor's all properties which are kept by driver during S3 
>> will be changed to default value.
>> But instead, if a set feature function is called first, sensor 
>> hardware/firmware will be recovered to the last status. So change the
>> sensor_hub_set_feature() calling order to behind of set feature 
>> function to avoid sensor properties lose.
>>
>> Signed-off-by: Song Hongyan <hongyan.song@intel.com>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Hi Song,

The patch message, whilst excellent on the technical detail, gives me no sense of urgency on this. Is this a fix we want to have heading for stable ASAP or are we looking at something seen in some developmental hardware for example?
Links to bug reports, or examples of hardware suffering from the issue are always helpful as well.

Also for something like this a 'fixes' tag is very helpful when people are looking to back port it.

Anyhow, I'm going to hold off on taking this one until I have more information. Right now it makes little difference as the next fixes pull request from me will probably not be until next weekend.

Thanks,

Jonathan
> 
>> ---
>>  drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
>> b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
>> index a3cce3a..ecf592d 100644
>> --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
>> +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
>> @@ -51,8 +51,6 @@ static int _hid_sensor_power_state(struct 
>> hid_sensor_common *st, bool state)
>>  			st->report_state.report_id,
>>  			st->report_state.index,
>>  			HID_USAGE_SENSOR_PROP_REPORTING_STATE_ALL_EV
>> ENTS_ENUM);
>> -
>> -		poll_value = hid_sensor_read_poll_value(st);
>>  	} else {
>>  		int val;
>>  
>> @@ -89,7 +87,9 @@ static 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,
>>  			       sizeof(state_val), &state_val);
>> -	if (state && poll_value)
>> +	if (state)
>> +		poll_value = hid_sensor_read_poll_value(st);
>> +	if (poll_value > 0)
>>  		msleep_interruptible(poll_value * 2);
>>  
>>  	return 
>> 0;N     r  y   b X  ǧv ^ )޺{.n +    {  *"  ^n r   z \x1a  h    &  \x1e G   
>> h \x03( 階 ݢj"  \x1a ^[m     z ޖ   f   h   ~ mml==


^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH] iio: hid-sensor-trigger: Change get poll value function order to avoid sensor properties losing after resume from S3
@ 2017-02-27  1:00             ` Song, Hongyan
  0 siblings, 0 replies; 10+ messages in thread
From: Song, Hongyan @ 2017-02-27  1:00 UTC (permalink / raw)
  To: Jonathan Cameron, Pandruvada, Srinivas, linux-input, linux-iio; +Cc: jikos

SGkgSm9uYXRoYW4sDQoJSSBhZGQgbW9yZSBpbmZvcm1hdGlvbiBhYm91dCB0aGUgcGF0Y2g6DQoJ
DQpCdWcgZm91bmQ6IA0KV2hlbiB3ZSBjYXQgc2Vuc29yIGluY2xpIHNlbnNvciBkYXRhIHdpdGgg
aHlzdGVyZXNpcyB2YWx1ZSBpcyAwLCB0aGVyZSBhbHdheXMgaGF2ZSBzdHJlYW1lZCBzZW5zb3Ig
ZGF0YSBvdXRwdXQsDQpCdXQgd2hlbiBzeXN0ZW0gZW50ZXIgUzMgYW5kIHJlc3VtZSBiYWNrLCB0
aGUgc3RyZWFtaW5nIHRocmVhZCBpcyBzdGlsbCBvbiBidXQgdGhlcmUgaXMgbm90IHN0cmVhbWlu
ZyBzZW5zb3IgZGF0YSBvdXRwdXQuDQpUZXN0IGRhdGEgYXMgYmVsb3c6DQoyNzk6IDIwMTctMDEt
MjQgMDE6MTY6MDAuMjc5Mzg1ICo1KiBpbmNsaV8zZDogTWluIERlbHRhOiA5OS43NzltcywgTWF4
IERlbHRhOiAxMDAuMzE2bXMsIFJlY2VpdmVkOiAxMC9zLCBPdmVyYWxsOiAxNzAvMTZzOyBTYW1w
bGUgdmFsdWVzICwwLjAxMDU0NiwtMC4wMDY3MTIsMi43OTc1NzkNCjI4MDogMjAxNy0wMS0yNCAw
MToxNjo0MS4yMTEzODUgKjUqIGluY2xpXzNkOiBNaW4gRGVsdGE6IDEwMC4xNm1zLCBNYXggRGVs
dGE6IDQwOTI0LjZtcywgUmVjZWl2ZWQ6IDIvcywgT3ZlcmFsbDogMTcyLzE5czsgU2FtcGxlIHZh
bHVlcyAsMC4wMTA1NDYsLTAuMDA2NzEyLDIuNzk3NTc5DQoyODE6IDIwMTctMDEtMjQgMDE6MTY6
NDIuMjE5ODkyICo1KiBpbmNsaV8zZDogTWluIERlbHRhOiAwLjAyM21zLCBNYXggRGVsdGE6IDU0
My4yOG1zLCBSZWNlaXZlZDogMi9zLCBPdmVyYWxsOiAxNzQvMjBzOyBTYW1wbGUgdmFsdWVzICww
LjAxMDU0NiwtMC4wMDY3MTIsMi43OTc1NzkNCjI4MjogMjAxNy0wMS0yNCAwMToxNjo0Mi43NTU1
NDggKjQqIENJSU9TZW5zb3JJbmZvcm06OnNlbnNvcl93YWl0X2V2ZW50IHBvbGwgdGltZW91dA0K
MjgzOiAyMDE3LTAxLTI0IDAxOjE2OjQzLjIyMzA2MiAqNSogaW5jbGlfM2Q6IE1pbiBEZWx0YTog
LS0sIE1heCBEZWx0YTogLS0sIFJlY2VpdmVkOiAwL3MsIE92ZXJhbGw6IDE3NC8yMXMNCjI4NDog
MjAxNy0wMS0yNCAwMToxNjo0My43NTY5ODAgKjQqIENJSU9TZW5zb3JJbmZvcm06OnNlbnNvcl93
YWl0X2V2ZW50IHBvbGwgdGltZW91dA0KMjg1OiAyMDE3LTAxLTI0IDAxOjE2OjQ0LjIyNTUyNyAq
NSogaW5jbGlfM2Q6IE1pbiBEZWx0YTogLS0sIE1heCBEZWx0YTogLS0sIFJlY2VpdmVkOiAwL3Ms
IE92ZXJhbGw6IDE3NC8yMnMNCjI4NjogMjAxNy0wMS0yNCAwMToxNjo0NC43NTg0NjcgKjQqIENJ
SU9TZW5zb3JJbmZvcm06OnNlbnNvcl93YWl0X2V2ZW50IHBvbGwgdGltZW91dA0KMjg3OiAyMDE3
LTAxLTI0IDAxOjE2OjQ1LjIyODA4MiAqNSogaW5jbGlfM2Q6IE1pbiBEZWx0YTogLS0sIE1heCBE
ZWx0YTogLS0sIFJlY2VpdmVkOiAwL3MsIE92ZXJhbGw6IDE3NC8yM3MNCjI4ODogMjAxNy0wMS0y
NCAwMToxNjo0NS43NTk5NTggKjQqIENJSU9TZW5zb3JJbmZvcm06OnNlbnNvcl93YWl0X2V2ZW50
IHBvbGwgdGltZW91dA0KDQpSb290IGNhdXNlOiANCndoZW4gc2Vuc29yIHJlc3VtZSBiYWNrIGZy
b20gUzMsIHRoZSBzZW5zb3IgcHJvcGVydHkgdXNlciBzZXQgaXMgbG9zdC4NCkZvciBleGFtcGxl
LCBpZiB3ZSBzZXQgc2Vuc29yIGh5c3RlcmVzaXMgdmFsdWUgdG8gYmUgMCwgdGhlbiBsZXQgc3lz
dGVtIGVudGVyIFMzLCB3aGVuIHJlc3VtZSBiYWNrLCB0aGUgaHlzdGVyZXNpcyB2YWx1ZSB3aWxs
IGJlIG5vdCAwIGJ1dCB0aGUgZGVmYXVsdCB2YWx1ZSAxLg0KDQp0aGlzIGlzIG5vdCB3aGF0IHdl
IHdhbnQsIHJlc3VtZSBiYWNrIGZyb20gUzMgc2hvdWxkIG5vdCBsb3NlIHRoZSBwcm9wZXJ0aWVz
IHdoaWNoIGlzIHNldCBiZWZvcmUgUzMsIGp1c3QgYmVjYXVzZSB0aGUgcHJvcGVydHkgc2V0dGlu
ZyBzZXF1ZW5jZSBsZWFkIHRvIHRoaXMgbG9zZS4NCg0KVGhpcyBpcyBwYXRjaCBpcyBhIGZpeCBw
YXRjaCBhbmQgbmVlZGVkIGZvciBzdGFibGUuDQoNClRoYW5rcyBhIGxvdCENCg0KQlINClNvbmcg
SG9uZ3lhbg0KDQoNCi0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQpGcm9tOiBKb25hdGhhbiBD
YW1lcm9uIFttYWlsdG86amljMjNAa2VybmVsLm9yZ10gDQpTZW50OiBTdW5kYXksIEZlYnJ1YXJ5
IDI2LCAyMDE3IDEyOjUyIEFNDQpUbzogUGFuZHJ1dmFkYSwgU3Jpbml2YXMgPHNyaW5pdmFzLnBh
bmRydXZhZGFAaW50ZWwuY29tPjsgbGludXgtaW5wdXRAdmdlci5rZXJuZWwub3JnOyBTb25nLCBI
b25neWFuIDxob25neWFuLnNvbmdAaW50ZWwuY29tPjsgbGludXgtaWlvQHZnZXIua2VybmVsLm9y
Zw0KQ2M6IGppa29zQGtlcm5lbC5vcmcNClN1YmplY3Q6IFJlOiBbUEFUQ0hdIGlpbzogaGlkLXNl
bnNvci10cmlnZ2VyOiBDaGFuZ2UgZ2V0IHBvbGwgdmFsdWUgZnVuY3Rpb24gb3JkZXIgdG8gYXZv
aWQgc2Vuc29yIHByb3BlcnRpZXMgbG9zaW5nIGFmdGVyIHJlc3VtZSBmcm9tIFMzDQoNCk9uIDIy
LzAyLzE3IDA1OjQwLCBQYW5kcnV2YWRhLCBTcmluaXZhcyB3cm90ZToNCj4gT24gV2VkLCAyMDE3
LTAyLTIyIGF0IDE3OjE3ICswODAwLCBTb25nIEhvbmd5YW4gd3JvdGU6DQo+PiBJbiBmdW5jdGlv
biBfaGlkX3NlbnNvcl9wb3dlcl9zdGF0ZSgpLCB3aGVuDQo+PiBoaWRfc2Vuc29yX3JlYWRfcG9s
bF92YWx1ZSgpDQo+PiBpcyBjYWxsZWQsIHNlbnNvcidzIGFsbCBwcm9wZXJ0aWVzIHdpbGwgYmUg
dXBkYXRlZCBieSB0aGUgdmFsdWUgZnJvbSANCj4+IHNlbnNvciBoYXJkd2FyZS9maXJtd2FyZS4N
Cj4+IEluIHNvbWUgaW1wbGVtZW50YXRpb24sIHNlbnNvciBoYXJkd2FyZS9maXJtd2FyZSB3aWxs
IGRvIGEgcG93ZXIgDQo+PiBjeWNsZSBkdXJpbmcgUzMuIEluIHRoaXMgY2FzZSwgYWZ0ZXIgcmVz
dW1lLCBvbmNlDQo+PiBoaWRfc2Vuc29yX3JlYWRfcG9sbF92YWx1ZSgpDQo+PiBpcyBjYWxsZWQs
IHNlbnNvcidzIGFsbCBwcm9wZXJ0aWVzIHdoaWNoIGFyZSBrZXB0IGJ5IGRyaXZlciBkdXJpbmcg
UzMgDQo+PiB3aWxsIGJlIGNoYW5nZWQgdG8gZGVmYXVsdCB2YWx1ZS4NCj4+IEJ1dCBpbnN0ZWFk
LCBpZiBhIHNldCBmZWF0dXJlIGZ1bmN0aW9uIGlzIGNhbGxlZCBmaXJzdCwgc2Vuc29yIA0KPj4g
aGFyZHdhcmUvZmlybXdhcmUgd2lsbCBiZSByZWNvdmVyZWQgdG8gdGhlIGxhc3Qgc3RhdHVzLiBT
byBjaGFuZ2UgdGhlDQo+PiBzZW5zb3JfaHViX3NldF9mZWF0dXJlKCkgY2FsbGluZyBvcmRlciB0
byBiZWhpbmQgb2Ygc2V0IGZlYXR1cmUgDQo+PiBmdW5jdGlvbiB0byBhdm9pZCBzZW5zb3IgcHJv
cGVydGllcyBsb3NlLg0KPj4NCj4+IFNpZ25lZC1vZmYtYnk6IFNvbmcgSG9uZ3lhbiA8aG9uZ3lh
bi5zb25nQGludGVsLmNvbT4NCj4gQWNrZWQtYnk6IFNyaW5pdmFzIFBhbmRydXZhZGEgPHNyaW5p
dmFzLnBhbmRydXZhZGFAbGludXguaW50ZWwuY29tPg0KSGkgU29uZywNCg0KVGhlIHBhdGNoIG1l
c3NhZ2UsIHdoaWxzdCBleGNlbGxlbnQgb24gdGhlIHRlY2huaWNhbCBkZXRhaWwsIGdpdmVzIG1l
IG5vIHNlbnNlIG9mIHVyZ2VuY3kgb24gdGhpcy4gSXMgdGhpcyBhIGZpeCB3ZSB3YW50IHRvIGhh
dmUgaGVhZGluZyBmb3Igc3RhYmxlIEFTQVAgb3IgYXJlIHdlIGxvb2tpbmcgYXQgc29tZXRoaW5n
IHNlZW4gaW4gc29tZSBkZXZlbG9wbWVudGFsIGhhcmR3YXJlIGZvciBleGFtcGxlPw0KTGlua3Mg
dG8gYnVnIHJlcG9ydHMsIG9yIGV4YW1wbGVzIG9mIGhhcmR3YXJlIHN1ZmZlcmluZyBmcm9tIHRo
ZSBpc3N1ZSBhcmUgYWx3YXlzIGhlbHBmdWwgYXMgd2VsbC4NCg0KQWxzbyBmb3Igc29tZXRoaW5n
IGxpa2UgdGhpcyBhICdmaXhlcycgdGFnIGlzIHZlcnkgaGVscGZ1bCB3aGVuIHBlb3BsZSBhcmUg
bG9va2luZyB0byBiYWNrIHBvcnQgaXQuDQoNCkFueWhvdywgSSdtIGdvaW5nIHRvIGhvbGQgb2Zm
IG9uIHRha2luZyB0aGlzIG9uZSB1bnRpbCBJIGhhdmUgbW9yZSBpbmZvcm1hdGlvbi4gUmlnaHQg
bm93IGl0IG1ha2VzIGxpdHRsZSBkaWZmZXJlbmNlIGFzIHRoZSBuZXh0IGZpeGVzIHB1bGwgcmVx
dWVzdCBmcm9tIG1lIHdpbGwgcHJvYmFibHkgbm90IGJlIHVudGlsIG5leHQgd2Vla2VuZC4NCg0K
VGhhbmtzLA0KDQpKb25hdGhhbg0KPiANCj4+IC0tLQ0KPj4gIGRyaXZlcnMvaWlvL2NvbW1vbi9o
aWQtc2Vuc29ycy9oaWQtc2Vuc29yLXRyaWdnZXIuYyB8IDYgKysrLS0tDQo+PiAgMSBmaWxlIGNo
YW5nZWQsIDMgaW5zZXJ0aW9ucygrKSwgMyBkZWxldGlvbnMoLSkNCj4+DQo+PiBkaWZmIC0tZ2l0
IGEvZHJpdmVycy9paW8vY29tbW9uL2hpZC1zZW5zb3JzL2hpZC1zZW5zb3ItdHJpZ2dlci5jDQo+
PiBiL2RyaXZlcnMvaWlvL2NvbW1vbi9oaWQtc2Vuc29ycy9oaWQtc2Vuc29yLXRyaWdnZXIuYw0K
Pj4gaW5kZXggYTNjY2UzYS4uZWNmNTkyZCAxMDA2NDQNCj4+IC0tLSBhL2RyaXZlcnMvaWlvL2Nv
bW1vbi9oaWQtc2Vuc29ycy9oaWQtc2Vuc29yLXRyaWdnZXIuYw0KPj4gKysrIGIvZHJpdmVycy9p
aW8vY29tbW9uL2hpZC1zZW5zb3JzL2hpZC1zZW5zb3ItdHJpZ2dlci5jDQo+PiBAQCAtNTEsOCAr
NTEsNiBAQCBzdGF0aWMgaW50IF9oaWRfc2Vuc29yX3Bvd2VyX3N0YXRlKHN0cnVjdCANCj4+IGhp
ZF9zZW5zb3JfY29tbW9uICpzdCwgYm9vbCBzdGF0ZSkNCj4+ICAJCQlzdC0+cmVwb3J0X3N0YXRl
LnJlcG9ydF9pZCwNCj4+ICAJCQlzdC0+cmVwb3J0X3N0YXRlLmluZGV4LA0KPj4gIAkJCUhJRF9V
U0FHRV9TRU5TT1JfUFJPUF9SRVBPUlRJTkdfU1RBVEVfQUxMX0VWDQo+PiBFTlRTX0VOVU0pOw0K
Pj4gLQ0KPj4gLQkJcG9sbF92YWx1ZSA9IGhpZF9zZW5zb3JfcmVhZF9wb2xsX3ZhbHVlKHN0KTsN
Cj4+ICAJfSBlbHNlIHsNCj4+ICAJCWludCB2YWw7DQo+PiAgDQo+PiBAQCAtODksNyArODcsOSBA
QCBzdGF0aWMgaW50IF9oaWRfc2Vuc29yX3Bvd2VyX3N0YXRlKHN0cnVjdCANCj4+IGhpZF9zZW5z
b3JfY29tbW9uICpzdCwgYm9vbCBzdGF0ZSkNCj4+ICAJc2Vuc29yX2h1Yl9nZXRfZmVhdHVyZShz
dC0+aHNkZXYsIHN0LT5wb3dlcl9zdGF0ZS5yZXBvcnRfaWQsDQo+PiAgCQkJICAgICAgIHN0LT5w
b3dlcl9zdGF0ZS5pbmRleCwNCj4+ICAJCQkgICAgICAgc2l6ZW9mKHN0YXRlX3ZhbCksICZzdGF0
ZV92YWwpOw0KPj4gLQlpZiAoc3RhdGUgJiYgcG9sbF92YWx1ZSkNCj4+ICsJaWYgKHN0YXRlKQ0K
Pj4gKwkJcG9sbF92YWx1ZSA9IGhpZF9zZW5zb3JfcmVhZF9wb2xsX3ZhbHVlKHN0KTsNCj4+ICsJ
aWYgKHBvbGxfdmFsdWUgPiAwKQ0KPj4gIAkJbXNsZWVwX2ludGVycnVwdGlibGUocG9sbF92YWx1
ZSAqIDIpOw0KPj4gIA0KPj4gIAlyZXR1cm4gDQo+PiAwO04gICAgIHIgIHkgICBiIFggIMendiBe
ICneunsubiArICAgIHsgICoiICBebiByICAgeiAaICBoICAgICYgIB4gRyAgIA0KPj4gaCADKCDp
mo4g3aJqIiAgGiAbbSAgICAgeiDeliAgIGYgICBoICAgfiBtbWw9PQ0KDQo=

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] iio: hid-sensor-trigger: Change get poll value function order to avoid sensor properties losing after resume from S3
  2017-02-27  1:00             ` Song, Hongyan
@ 2017-02-27  1:37                 ` Pandruvada, Srinivas
  -1 siblings, 0 replies; 10+ messages in thread
From: Pandruvada, Srinivas @ 2017-02-27  1:37 UTC (permalink / raw)
  To: linux-input-u79uwXL29TY76Z2rM5mHXA, Song, Hongyan,
	linux-iio-u79uwXL29TY76Z2rM5mHXA, jic23-DgEjT+Ai2ygdnm+yROfE0A
  Cc: jikos-DgEjT+Ai2ygdnm+yROfE0A

On Mon, 2017-02-27 at 01:00 +0000, Song, Hongyan wrote:
> Hi Jonathan,
> 	I add more information about the patch:
> 	
> Bug found: 
> When we cat sensor incli sensor data with hysteresis value is 0,
> there always have streamed sensor data output,
> But when system enter S3 and resume back, the streaming thread is
> still on but there is not streaming sensor data output.
> Test data as below:
> 279: 2017-01-24 01:16:00.279385 *5* incli_3d: Min Delta: 99.779ms,
> Max Delta: 100.316ms, Received: 10/s, Overall: 170/16s; Sample values
> ,0.010546,-0.006712,2.797579
> 280: 2017-01-24 01:16:41.211385 *5* incli_3d: Min Delta: 100.16ms,
> Max Delta: 40924.6ms, Received: 2/s, Overall: 172/19s; Sample values
> ,0.010546,-0.006712,2.797579
> 281: 2017-01-24 01:16:42.219892 *5* incli_3d: Min Delta: 0.023ms, Max
> Delta: 543.28ms, Received: 2/s, Overall: 174/20s; Sample values
> ,0.010546,-0.006712,2.797579
> 282: 2017-01-24 01:16:42.755548 *4*
> CIIOSensorInform::sensor_wait_event poll timeout
> 283: 2017-01-24 01:16:43.223062 *5* incli_3d: Min Delta: --, Max
> Delta: --, Received: 0/s, Overall: 174/21s
> 284: 2017-01-24 01:16:43.756980 *4*
> CIIOSensorInform::sensor_wait_event poll timeout
> 285: 2017-01-24 01:16:44.225527 *5* incli_3d: Min Delta: --, Max
> Delta: --, Received: 0/s, Overall: 174/22s
> 286: 2017-01-24 01:16:44.758467 *4*
> CIIOSensorInform::sensor_wait_event poll timeout
> 287: 2017-01-24 01:16:45.228082 *5* incli_3d: Min Delta: --, Max
> Delta: --, Received: 0/s, Overall: 174/23s
> 288: 2017-01-24 01:16:45.759958 *4*
> CIIOSensorInform::sensor_wait_event poll timeout
> 
> Root cause: 
> when sensor resume back from S3, the sensor property user set is
> lost.
> For example, if we set sensor hysteresis value to be 0, then let
> system enter S3, when resume back, the hysteresis value will be not 0
> but the default value 1.
> 
> this is not what we want, resume back from S3 should not lose the
> properties which is set before S3, just because the property setting
> sequence lead to this lose.
> 
> This is patch is a fix patch and needed for stable.
But not urgent.

Thanks,
Srinivas

> 
> Thanks a lot!
> 
> BR
> Song Hongyan
> 
> 
> -----Original Message-----
> From: Jonathan Cameron [mailto:jic23@kernel.org] 
> Sent: Sunday, February 26, 2017 12:52 AM
> To: Pandruvada, Srinivas <srinivas.pandruvada@intel.com>; linux-input
> @vger.kernel.org; Song, Hongyan <hongyan.song@intel.com>; linux-iio@v
> ger.kernel.org
> Cc: jikos@kernel.org
> Subject: Re: [PATCH] iio: hid-sensor-trigger: Change get poll value
> function order to avoid sensor properties losing after resume from S3
> 
> On 22/02/17 05:40, Pandruvada, Srinivas wrote:
> > On Wed, 2017-02-22 at 17:17 +0800, Song Hongyan wrote:
> > > In function _hid_sensor_power_state(), when
> > > hid_sensor_read_poll_value()
> > > is called, sensor's all properties will be updated by the value
> > > from 
> > > sensor hardware/firmware.
> > > In some implementation, sensor hardware/firmware will do a power 
> > > cycle during S3. In this case, after resume, once
> > > hid_sensor_read_poll_value()
> > > is called, sensor's all properties which are kept by driver
> > > during S3 
> > > will be changed to default value.
> > > But instead, if a set feature function is called first, sensor 
> > > hardware/firmware will be recovered to the last status. So change
> > > the
> > > sensor_hub_set_feature() calling order to behind of set feature 
> > > function to avoid sensor properties lose.
> > > 
> > > Signed-off-by: Song Hongyan <hongyan.song@intel.com>
> > 
> > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> 
> Hi Song,
> 
> The patch message, whilst excellent on the technical detail, gives me
> no sense of urgency on this. Is this a fix we want to have heading
> for stable ASAP or are we looking at something seen in some
> developmental hardware for example?
> Links to bug reports, or examples of hardware suffering from the
> issue are always helpful as well.
> 
> Also for something like this a 'fixes' tag is very helpful when
> people are looking to back port it.
> 
> Anyhow, I'm going to hold off on taking this one until I have more
> information. Right now it makes little difference as the next fixes
> pull request from me will probably not be until next weekend.
> 
> Thanks,
> 
> Jonathan
> > 
> > > ---
> > >  drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> > > b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> > > index a3cce3a..ecf592d 100644
> > > --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> > > +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> > > @@ -51,8 +51,6 @@ static int _hid_sensor_power_state(struct 
> > > hid_sensor_common *st, bool state)
> > >  			st->report_state.report_id,
> > >  			st->report_state.index,
> > >  			HID_USAGE_SENSOR_PROP_REPORTING_STATE_AL
> > > L_EV
> > > ENTS_ENUM);
> > > -
> > > -		poll_value = hid_sensor_read_poll_value(st);
> > >  	} else {
> > >  		int val;
> > >  
> > > @@ -89,7 +87,9 @@ static 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,
> > >  			       sizeof(state_val), &state_val);
> > > -	if (state && poll_value)
> > > +	if (state)
> > > +		poll_value = hid_sensor_read_poll_value(st);
> > > +	if (poll_value > 0)
> > >  		msleep_interruptible(poll_value * 2);
> > >  
> > >  	return 
> > > 0;N     r  y   b X  ǧv ^ )޺{.n +    {  *"  ^n r   z \x1a  h    &  \x1e
> > > G   
> > > h \x03( 階 ݢj"  \x1a ^[m     z ޖ   f   h   ~ mml==
> 
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] iio: hid-sensor-trigger: Change get poll value function order to avoid sensor properties losing after resume from S3
@ 2017-02-27  1:37                 ` Pandruvada, Srinivas
  0 siblings, 0 replies; 10+ messages in thread
From: Pandruvada, Srinivas @ 2017-02-27  1:37 UTC (permalink / raw)
  To: linux-input, Song, Hongyan, linux-iio, jic23; +Cc: jikos

T24gTW9uLCAyMDE3LTAyLTI3IGF0IDAxOjAwICswMDAwLCBTb25nLCBIb25neWFuIHdyb3RlOg0K
PiBIaSBKb25hdGhhbiwNCj4gCUkgYWRkIG1vcmUgaW5mb3JtYXRpb24gYWJvdXQgdGhlIHBhdGNo
Og0KPiAJDQo+IEJ1ZyBmb3VuZDrCoA0KPiBXaGVuIHdlIGNhdCBzZW5zb3IgaW5jbGkgc2Vuc29y
IGRhdGEgd2l0aCBoeXN0ZXJlc2lzIHZhbHVlIGlzIDAsDQo+IHRoZXJlIGFsd2F5cyBoYXZlIHN0
cmVhbWVkIHNlbnNvciBkYXRhIG91dHB1dCwNCj4gQnV0IHdoZW4gc3lzdGVtIGVudGVyIFMzIGFu
ZCByZXN1bWUgYmFjaywgdGhlIHN0cmVhbWluZyB0aHJlYWQgaXMNCj4gc3RpbGwgb24gYnV0IHRo
ZXJlIGlzIG5vdCBzdHJlYW1pbmcgc2Vuc29yIGRhdGEgb3V0cHV0Lg0KPiBUZXN0IGRhdGEgYXMg
YmVsb3c6DQo+IDI3OTogMjAxNy0wMS0yNCAwMToxNjowMC4yNzkzODUgKjUqIGluY2xpXzNkOiBN
aW4gRGVsdGE6IDk5Ljc3OW1zLA0KPiBNYXggRGVsdGE6IDEwMC4zMTZtcywgUmVjZWl2ZWQ6IDEw
L3MsIE92ZXJhbGw6IDE3MC8xNnM7IFNhbXBsZSB2YWx1ZXMNCj4gLDAuMDEwNTQ2LC0wLjAwNjcx
MiwyLjc5NzU3OQ0KPiAyODA6IDIwMTctMDEtMjQgMDE6MTY6NDEuMjExMzg1ICo1KiBpbmNsaV8z
ZDogTWluIERlbHRhOiAxMDAuMTZtcywNCj4gTWF4IERlbHRhOiA0MDkyNC42bXMsIFJlY2VpdmVk
OiAyL3MsIE92ZXJhbGw6IDE3Mi8xOXM7IFNhbXBsZSB2YWx1ZXMNCj4gLDAuMDEwNTQ2LC0wLjAw
NjcxMiwyLjc5NzU3OQ0KPiAyODE6IDIwMTctMDEtMjQgMDE6MTY6NDIuMjE5ODkyICo1KiBpbmNs
aV8zZDogTWluIERlbHRhOiAwLjAyM21zLCBNYXgNCj4gRGVsdGE6IDU0My4yOG1zLCBSZWNlaXZl
ZDogMi9zLCBPdmVyYWxsOiAxNzQvMjBzOyBTYW1wbGUgdmFsdWVzDQo+ICwwLjAxMDU0NiwtMC4w
MDY3MTIsMi43OTc1NzkNCj4gMjgyOiAyMDE3LTAxLTI0IDAxOjE2OjQyLjc1NTU0OCAqNCoNCj4g
Q0lJT1NlbnNvckluZm9ybTo6c2Vuc29yX3dhaXRfZXZlbnQgcG9sbCB0aW1lb3V0DQo+IDI4Mzog
MjAxNy0wMS0yNCAwMToxNjo0My4yMjMwNjIgKjUqIGluY2xpXzNkOiBNaW4gRGVsdGE6IC0tLCBN
YXgNCj4gRGVsdGE6IC0tLCBSZWNlaXZlZDogMC9zLCBPdmVyYWxsOiAxNzQvMjFzDQo+IDI4NDog
MjAxNy0wMS0yNCAwMToxNjo0My43NTY5ODAgKjQqDQo+IENJSU9TZW5zb3JJbmZvcm06OnNlbnNv
cl93YWl0X2V2ZW50IHBvbGwgdGltZW91dA0KPiAyODU6IDIwMTctMDEtMjQgMDE6MTY6NDQuMjI1
NTI3ICo1KiBpbmNsaV8zZDogTWluIERlbHRhOiAtLSwgTWF4DQo+IERlbHRhOiAtLSwgUmVjZWl2
ZWQ6IDAvcywgT3ZlcmFsbDogMTc0LzIycw0KPiAyODY6IDIwMTctMDEtMjQgMDE6MTY6NDQuNzU4
NDY3ICo0Kg0KPiBDSUlPU2Vuc29ySW5mb3JtOjpzZW5zb3Jfd2FpdF9ldmVudCBwb2xsIHRpbWVv
dXQNCj4gMjg3OiAyMDE3LTAxLTI0IDAxOjE2OjQ1LjIyODA4MiAqNSogaW5jbGlfM2Q6IE1pbiBE
ZWx0YTogLS0sIE1heA0KPiBEZWx0YTogLS0sIFJlY2VpdmVkOiAwL3MsIE92ZXJhbGw6IDE3NC8y
M3MNCj4gMjg4OiAyMDE3LTAxLTI0IDAxOjE2OjQ1Ljc1OTk1OCAqNCoNCj4gQ0lJT1NlbnNvcklu
Zm9ybTo6c2Vuc29yX3dhaXRfZXZlbnQgcG9sbCB0aW1lb3V0DQo+IA0KPiBSb290IGNhdXNlOsKg
DQo+IHdoZW4gc2Vuc29yIHJlc3VtZSBiYWNrIGZyb20gUzMsIHRoZSBzZW5zb3IgcHJvcGVydHkg
dXNlciBzZXQgaXMNCj4gbG9zdC4NCj4gRm9yIGV4YW1wbGUsIGlmIHdlIHNldCBzZW5zb3IgaHlz
dGVyZXNpcyB2YWx1ZSB0byBiZSAwLCB0aGVuIGxldA0KPiBzeXN0ZW0gZW50ZXIgUzMsIHdoZW4g
cmVzdW1lIGJhY2ssIHRoZSBoeXN0ZXJlc2lzIHZhbHVlIHdpbGwgYmUgbm90IDANCj4gYnV0IHRo
ZSBkZWZhdWx0IHZhbHVlIDEuDQo+IA0KPiB0aGlzIGlzIG5vdCB3aGF0IHdlIHdhbnQsIHJlc3Vt
ZSBiYWNrIGZyb20gUzMgc2hvdWxkIG5vdCBsb3NlIHRoZQ0KPiBwcm9wZXJ0aWVzIHdoaWNoIGlz
IHNldCBiZWZvcmUgUzMsIGp1c3QgYmVjYXVzZSB0aGUgcHJvcGVydHkgc2V0dGluZw0KPiBzZXF1
ZW5jZSBsZWFkIHRvIHRoaXMgbG9zZS4NCj4gDQo+IFRoaXMgaXMgcGF0Y2ggaXMgYSBmaXggcGF0
Y2ggYW5kIG5lZWRlZCBmb3Igc3RhYmxlLg0KQnV0IG5vdCB1cmdlbnQuDQoNClRoYW5rcywNClNy
aW5pdmFzDQoNCj4gDQo+IFRoYW5rcyBhIGxvdCENCj4gDQo+IEJSDQo+IFNvbmcgSG9uZ3lhbg0K
PiANCj4gDQo+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+IEZyb206IEpvbmF0aGFuIENh
bWVyb24gW21haWx0bzpqaWMyM0BrZXJuZWwub3JnXcKgDQo+IFNlbnQ6IFN1bmRheSwgRmVicnVh
cnkgMjYsIDIwMTcgMTI6NTIgQU0NCj4gVG86IFBhbmRydXZhZGEsIFNyaW5pdmFzIDxzcmluaXZh
cy5wYW5kcnV2YWRhQGludGVsLmNvbT47IGxpbnV4LWlucHV0DQo+IEB2Z2VyLmtlcm5lbC5vcmc7
IFNvbmcsIEhvbmd5YW4gPGhvbmd5YW4uc29uZ0BpbnRlbC5jb20+OyBsaW51eC1paW9Adg0KPiBn
ZXIua2VybmVsLm9yZw0KPiBDYzogamlrb3NAa2VybmVsLm9yZw0KPiBTdWJqZWN0OiBSZTogW1BB
VENIXSBpaW86IGhpZC1zZW5zb3ItdHJpZ2dlcjogQ2hhbmdlIGdldCBwb2xsIHZhbHVlDQo+IGZ1
bmN0aW9uIG9yZGVyIHRvIGF2b2lkIHNlbnNvciBwcm9wZXJ0aWVzIGxvc2luZyBhZnRlciByZXN1
bWUgZnJvbSBTMw0KPiANCj4gT24gMjIvMDIvMTcgMDU6NDAsIFBhbmRydXZhZGEsIFNyaW5pdmFz
IHdyb3RlOg0KPiA+IE9uIFdlZCwgMjAxNy0wMi0yMiBhdCAxNzoxNyArMDgwMCwgU29uZyBIb25n
eWFuIHdyb3RlOg0KPiA+ID4gSW4gZnVuY3Rpb24gX2hpZF9zZW5zb3JfcG93ZXJfc3RhdGUoKSwg
d2hlbg0KPiA+ID4gaGlkX3NlbnNvcl9yZWFkX3BvbGxfdmFsdWUoKQ0KPiA+ID4gaXMgY2FsbGVk
LCBzZW5zb3IncyBhbGwgcHJvcGVydGllcyB3aWxsIGJlIHVwZGF0ZWQgYnkgdGhlIHZhbHVlDQo+
ID4gPiBmcm9twqANCj4gPiA+IHNlbnNvciBoYXJkd2FyZS9maXJtd2FyZS4NCj4gPiA+IEluIHNv
bWUgaW1wbGVtZW50YXRpb24sIHNlbnNvciBoYXJkd2FyZS9maXJtd2FyZSB3aWxsIGRvIGEgcG93
ZXLCoA0KPiA+ID4gY3ljbGUgZHVyaW5nIFMzLiBJbiB0aGlzIGNhc2UsIGFmdGVyIHJlc3VtZSwg
b25jZQ0KPiA+ID4gaGlkX3NlbnNvcl9yZWFkX3BvbGxfdmFsdWUoKQ0KPiA+ID4gaXMgY2FsbGVk
LCBzZW5zb3IncyBhbGwgcHJvcGVydGllcyB3aGljaCBhcmUga2VwdCBieSBkcml2ZXINCj4gPiA+
IGR1cmluZyBTM8KgDQo+ID4gPiB3aWxsIGJlIGNoYW5nZWQgdG8gZGVmYXVsdCB2YWx1ZS4NCj4g
PiA+IEJ1dCBpbnN0ZWFkLCBpZiBhIHNldCBmZWF0dXJlIGZ1bmN0aW9uIGlzIGNhbGxlZCBmaXJz
dCwgc2Vuc29ywqANCj4gPiA+IGhhcmR3YXJlL2Zpcm13YXJlIHdpbGwgYmUgcmVjb3ZlcmVkIHRv
IHRoZSBsYXN0IHN0YXR1cy4gU28gY2hhbmdlDQo+ID4gPiB0aGUNCj4gPiA+IHNlbnNvcl9odWJf
c2V0X2ZlYXR1cmUoKSBjYWxsaW5nIG9yZGVyIHRvIGJlaGluZCBvZiBzZXQgZmVhdHVyZcKgDQo+
ID4gPiBmdW5jdGlvbiB0byBhdm9pZCBzZW5zb3IgcHJvcGVydGllcyBsb3NlLg0KPiA+ID4gDQo+
ID4gPiBTaWduZWQtb2ZmLWJ5OiBTb25nIEhvbmd5YW4gPGhvbmd5YW4uc29uZ0BpbnRlbC5jb20+
DQo+ID4gDQo+ID4gQWNrZWQtYnk6IFNyaW5pdmFzIFBhbmRydXZhZGEgPHNyaW5pdmFzLnBhbmRy
dXZhZGFAbGludXguaW50ZWwuY29tPg0KPiANCj4gSGkgU29uZywNCj4gDQo+IFRoZSBwYXRjaCBt
ZXNzYWdlLCB3aGlsc3QgZXhjZWxsZW50IG9uIHRoZSB0ZWNobmljYWwgZGV0YWlsLCBnaXZlcyBt
ZQ0KPiBubyBzZW5zZSBvZiB1cmdlbmN5IG9uIHRoaXMuIElzIHRoaXMgYSBmaXggd2Ugd2FudCB0
byBoYXZlIGhlYWRpbmcNCj4gZm9yIHN0YWJsZSBBU0FQIG9yIGFyZSB3ZSBsb29raW5nIGF0IHNv
bWV0aGluZyBzZWVuIGluIHNvbWUNCj4gZGV2ZWxvcG1lbnRhbCBoYXJkd2FyZSBmb3IgZXhhbXBs
ZT8NCj4gTGlua3MgdG8gYnVnIHJlcG9ydHMsIG9yIGV4YW1wbGVzIG9mIGhhcmR3YXJlIHN1ZmZl
cmluZyBmcm9tIHRoZQ0KPiBpc3N1ZSBhcmUgYWx3YXlzIGhlbHBmdWwgYXMgd2VsbC4NCj4gDQo+
IEFsc28gZm9yIHNvbWV0aGluZyBsaWtlIHRoaXMgYSAnZml4ZXMnIHRhZyBpcyB2ZXJ5IGhlbHBm
dWwgd2hlbg0KPiBwZW9wbGUgYXJlIGxvb2tpbmcgdG8gYmFjayBwb3J0IGl0Lg0KPiANCj4gQW55
aG93LCBJJ20gZ29pbmcgdG8gaG9sZCBvZmYgb24gdGFraW5nIHRoaXMgb25lIHVudGlsIEkgaGF2
ZSBtb3JlDQo+IGluZm9ybWF0aW9uLiBSaWdodCBub3cgaXQgbWFrZXMgbGl0dGxlIGRpZmZlcmVu
Y2UgYXMgdGhlIG5leHQgZml4ZXMNCj4gcHVsbCByZXF1ZXN0IGZyb20gbWUgd2lsbCBwcm9iYWJs
eSBub3QgYmUgdW50aWwgbmV4dCB3ZWVrZW5kLg0KPiANCj4gVGhhbmtzLA0KPiANCj4gSm9uYXRo
YW4NCj4gPiANCj4gPiA+IC0tLQ0KPiA+ID4gwqBkcml2ZXJzL2lpby9jb21tb24vaGlkLXNlbnNv
cnMvaGlkLXNlbnNvci10cmlnZ2VyLmMgfCA2ICsrKy0tLQ0KPiA+ID4gwqAxIGZpbGUgY2hhbmdl
ZCwgMyBpbnNlcnRpb25zKCspLCAzIGRlbGV0aW9ucygtKQ0KPiA+ID4gDQo+ID4gPiBkaWZmIC0t
Z2l0IGEvZHJpdmVycy9paW8vY29tbW9uL2hpZC1zZW5zb3JzL2hpZC1zZW5zb3ItdHJpZ2dlci5j
DQo+ID4gPiBiL2RyaXZlcnMvaWlvL2NvbW1vbi9oaWQtc2Vuc29ycy9oaWQtc2Vuc29yLXRyaWdn
ZXIuYw0KPiA+ID4gaW5kZXggYTNjY2UzYS4uZWNmNTkyZCAxMDA2NDQNCj4gPiA+IC0tLSBhL2Ry
aXZlcnMvaWlvL2NvbW1vbi9oaWQtc2Vuc29ycy9oaWQtc2Vuc29yLXRyaWdnZXIuYw0KPiA+ID4g
KysrIGIvZHJpdmVycy9paW8vY29tbW9uL2hpZC1zZW5zb3JzL2hpZC1zZW5zb3ItdHJpZ2dlci5j
DQo+ID4gPiBAQCAtNTEsOCArNTEsNiBAQCBzdGF0aWMgaW50IF9oaWRfc2Vuc29yX3Bvd2VyX3N0
YXRlKHN0cnVjdMKgDQo+ID4gPiBoaWRfc2Vuc29yX2NvbW1vbiAqc3QsIGJvb2wgc3RhdGUpDQo+
ID4gPiDCoAkJCXN0LT5yZXBvcnRfc3RhdGUucmVwb3J0X2lkLA0KPiA+ID4gwqAJCQlzdC0+cmVw
b3J0X3N0YXRlLmluZGV4LA0KPiA+ID4gwqAJCQlISURfVVNBR0VfU0VOU09SX1BST1BfUkVQT1JU
SU5HX1NUQVRFX0FMDQo+ID4gPiBMX0VWDQo+ID4gPiBFTlRTX0VOVU0pOw0KPiA+ID4gLQ0KPiA+
ID4gLQkJcG9sbF92YWx1ZSA9IGhpZF9zZW5zb3JfcmVhZF9wb2xsX3ZhbHVlKHN0KTsNCj4gPiA+
IMKgCX0gZWxzZSB7DQo+ID4gPiDCoAkJaW50IHZhbDsNCj4gPiA+IMKgDQo+ID4gPiBAQCAtODks
NyArODcsOSBAQCBzdGF0aWMgaW50IF9oaWRfc2Vuc29yX3Bvd2VyX3N0YXRlKHN0cnVjdMKgDQo+
ID4gPiBoaWRfc2Vuc29yX2NvbW1vbiAqc3QsIGJvb2wgc3RhdGUpDQo+ID4gPiDCoAlzZW5zb3Jf
aHViX2dldF9mZWF0dXJlKHN0LT5oc2Rldiwgc3QtDQo+ID4gPiA+cG93ZXJfc3RhdGUucmVwb3J0
X2lkLA0KPiA+ID4gwqAJCQnCoMKgwqDCoMKgwqDCoHN0LT5wb3dlcl9zdGF0ZS5pbmRleCwNCj4g
PiA+IMKgCQkJwqDCoMKgwqDCoMKgwqBzaXplb2Yoc3RhdGVfdmFsKSwgJnN0YXRlX3ZhbCk7DQo+
ID4gPiAtCWlmIChzdGF0ZSAmJiBwb2xsX3ZhbHVlKQ0KPiA+ID4gKwlpZiAoc3RhdGUpDQo+ID4g
PiArCQlwb2xsX3ZhbHVlID0gaGlkX3NlbnNvcl9yZWFkX3BvbGxfdmFsdWUoc3QpOw0KPiA+ID4g
KwlpZiAocG9sbF92YWx1ZSA+IDApDQo+ID4gPiDCoAkJbXNsZWVwX2ludGVycnVwdGlibGUocG9s
bF92YWx1ZSAqIDIpOw0KPiA+ID4gwqANCj4gPiA+IMKgCXJldHVybsKgDQo+ID4gPiAwO07CoMKg
wqDCoMKgcsKgwqB5wqDCoMKgYiBYwqDCoMendiBeICneunsubiArwqDCoMKgwqB7wqDCoCoiwqDC
oF5uIHLCoMKgwqB6IBrCoMKgaMKgwqDCoMKgJsKgwqAeDQo+ID4gPiBHwqDCoMKgDQo+ID4gPiBo
IAMoIOmajiDdomoiwqDCoBogG23CoMKgwqDCoMKgeiDelsKgwqDCoGbCoMKgwqBowqDCoMKgfiBt
bWw9PQ0KPiANCj4g

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] iio: hid-sensor-trigger: Change get poll value function order to avoid sensor properties losing after resume from S3
  2017-02-27  1:37                 ` Pandruvada, Srinivas
  (?)
@ 2017-03-05 10:29                 ` Jonathan Cameron
  -1 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2017-03-05 10:29 UTC (permalink / raw)
  To: Pandruvada, Srinivas, linux-input, Song, Hongyan, linux-iio; +Cc: jikos

On 27/02/17 01:37, Pandruvada, Srinivas wrote:
> On Mon, 2017-02-27 at 01:00 +0000, Song, Hongyan wrote:
>> Hi Jonathan,
>> 	I add more information about the patch:
>> 	
>> Bug found: 
>> When we cat sensor incli sensor data with hysteresis value is 0,
>> there always have streamed sensor data output,
>> But when system enter S3 and resume back, the streaming thread is
>> still on but there is not streaming sensor data output.
>> Test data as below:
>> 279: 2017-01-24 01:16:00.279385 *5* incli_3d: Min Delta: 99.779ms,
>> Max Delta: 100.316ms, Received: 10/s, Overall: 170/16s; Sample values
>> ,0.010546,-0.006712,2.797579
>> 280: 2017-01-24 01:16:41.211385 *5* incli_3d: Min Delta: 100.16ms,
>> Max Delta: 40924.6ms, Received: 2/s, Overall: 172/19s; Sample values
>> ,0.010546,-0.006712,2.797579
>> 281: 2017-01-24 01:16:42.219892 *5* incli_3d: Min Delta: 0.023ms, Max
>> Delta: 543.28ms, Received: 2/s, Overall: 174/20s; Sample values
>> ,0.010546,-0.006712,2.797579
>> 282: 2017-01-24 01:16:42.755548 *4*
>> CIIOSensorInform::sensor_wait_event poll timeout
>> 283: 2017-01-24 01:16:43.223062 *5* incli_3d: Min Delta: --, Max
>> Delta: --, Received: 0/s, Overall: 174/21s
>> 284: 2017-01-24 01:16:43.756980 *4*
>> CIIOSensorInform::sensor_wait_event poll timeout
>> 285: 2017-01-24 01:16:44.225527 *5* incli_3d: Min Delta: --, Max
>> Delta: --, Received: 0/s, Overall: 174/22s
>> 286: 2017-01-24 01:16:44.758467 *4*
>> CIIOSensorInform::sensor_wait_event poll timeout
>> 287: 2017-01-24 01:16:45.228082 *5* incli_3d: Min Delta: --, Max
>> Delta: --, Received: 0/s, Overall: 174/23s
>> 288: 2017-01-24 01:16:45.759958 *4*
>> CIIOSensorInform::sensor_wait_event poll timeout
>>
>> Root cause: 
>> when sensor resume back from S3, the sensor property user set is
>> lost.
>> For example, if we set sensor hysteresis value to be 0, then let
>> system enter S3, when resume back, the hysteresis value will be not 0
>> but the default value 1.
>>
>> this is not what we want, resume back from S3 should not lose the
>> properties which is set before S3, just because the property setting
>> sequence lead to this lose.
>>
>> This is patch is a fix patch and needed for stable.
> But not urgent.
Thanks for the info.

Applied to the fixes-togreg branch of iio.git.

Thanks,

Jonathan
> 
> Thanks,
> Srinivas
> 
>>
>> Thanks a lot!
>>
>> BR
>> Song Hongyan
>>
>>
>> -----Original Message-----
>> From: Jonathan Cameron [mailto:jic23@kernel.org] 
>> Sent: Sunday, February 26, 2017 12:52 AM
>> To: Pandruvada, Srinivas <srinivas.pandruvada@intel.com>; linux-input
>> @vger.kernel.org; Song, Hongyan <hongyan.song@intel.com>; linux-iio@v
>> ger.kernel.org
>> Cc: jikos@kernel.org
>> Subject: Re: [PATCH] iio: hid-sensor-trigger: Change get poll value
>> function order to avoid sensor properties losing after resume from S3
>>
>> On 22/02/17 05:40, Pandruvada, Srinivas wrote:
>>> On Wed, 2017-02-22 at 17:17 +0800, Song Hongyan wrote:
>>>> In function _hid_sensor_power_state(), when
>>>> hid_sensor_read_poll_value()
>>>> is called, sensor's all properties will be updated by the value
>>>> from 
>>>> sensor hardware/firmware.
>>>> In some implementation, sensor hardware/firmware will do a power 
>>>> cycle during S3. In this case, after resume, once
>>>> hid_sensor_read_poll_value()
>>>> is called, sensor's all properties which are kept by driver
>>>> during S3 
>>>> will be changed to default value.
>>>> But instead, if a set feature function is called first, sensor 
>>>> hardware/firmware will be recovered to the last status. So change
>>>> the
>>>> sensor_hub_set_feature() calling order to behind of set feature 
>>>> function to avoid sensor properties lose.
>>>>
>>>> Signed-off-by: Song Hongyan <hongyan.song@intel.com>
>>>
>>> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
>>
>> Hi Song,
>>
>> The patch message, whilst excellent on the technical detail, gives me
>> no sense of urgency on this. Is this a fix we want to have heading
>> for stable ASAP or are we looking at something seen in some
>> developmental hardware for example?
>> Links to bug reports, or examples of hardware suffering from the
>> issue are always helpful as well.
>>
>> Also for something like this a 'fixes' tag is very helpful when
>> people are looking to back port it.
>>
>> Anyhow, I'm going to hold off on taking this one until I have more
>> information. Right now it makes little difference as the next fixes
>> pull request from me will probably not be until next weekend.
>>
>> Thanks,
>>
>> Jonathan
>>>
>>>> ---
>>>>  drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 6 +++---
>>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
>>>> b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
>>>> index a3cce3a..ecf592d 100644
>>>> --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
>>>> +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
>>>> @@ -51,8 +51,6 @@ static int _hid_sensor_power_state(struct 
>>>> hid_sensor_common *st, bool state)
>>>>  			st->report_state.report_id,
>>>>  			st->report_state.index,
>>>>  			HID_USAGE_SENSOR_PROP_REPORTING_STATE_AL
>>>> L_EV
>>>> ENTS_ENUM);
>>>> -
>>>> -		poll_value = hid_sensor_read_poll_value(st);
>>>>  	} else {
>>>>  		int val;
>>>>  
>>>> @@ -89,7 +87,9 @@ static 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,
>>>>  			       sizeof(state_val), &state_val);
>>>> -	if (state && poll_value)
>>>> +	if (state)
>>>> +		poll_value = hid_sensor_read_poll_value(st);
>>>> +	if (poll_value > 0)
>>>>  		msleep_interruptible(poll_value * 2);
>>>>  
>>>>  	return 
>>>> 0;N     r  y   b X  ǧv ^ )޺{.n +    {  *"  ^n r   z \x1a  h    &  \x1e
>>>> G   
>>>> h \x03( 階 ݢj"  \x1a ^[m     z ޖ   f   h   ~ mml==
>>
>> N�����r��y���b�X��ǧv�^�)޺{.n�+����{��*"��^n�r���z�\x1a��h����&��\x1e�G���h�\x03(�階�ݢj"��\x1a�^[m�����z�ޖ���f���h���~�mml==


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-03-05 10:32 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-22  9:17 [PATCH] iio: hid-sensor-trigger: Change get poll value function order to avoid sensor properties losing after resume from S3 Song Hongyan
     [not found] ` <1487755058-31310-1-git-send-email-hongyan.song-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2017-02-22  5:40   ` Pandruvada, Srinivas
2017-02-22  5:40     ` Pandruvada, Srinivas
     [not found]     ` <1487741996.19408.0.camel-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2017-02-25 16:51       ` Jonathan Cameron
2017-02-25 16:51         ` Jonathan Cameron
     [not found]         ` <1f73506c-2bdd-c527-cd13-76594a1a7375-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2017-02-27  1:00           ` Song, Hongyan
2017-02-27  1:00             ` Song, Hongyan
     [not found]             ` <AE3E3DFA698D6144A7445C92D1D41E2F10BE0440-0J0gbvR4kTggGBtAFL8yw7fspsVTdybXVpNB7YpNyf8@public.gmane.org>
2017-02-27  1:37               ` Pandruvada, Srinivas
2017-02-27  1:37                 ` Pandruvada, Srinivas
2017-03-05 10:29                 ` Jonathan Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.