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@xxxxxxxxxx] ; > Sent: Sunday, February 26, 2017 12:52 AM > To: Pandruvada, Srinivas <srinivas.pandruvada@xxxxxxxxx>; linux-input > @vger.kernel.org; Song, Hongyan <hongyan.song@xxxxxxxxx>; linux-iio@v > ger.kernel.org > Cc: jikos@xxxxxxxxxx > 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@xxxxxxxxx> > > > > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx> > > 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 h & > > > G > > > h ( 階 ݢj" m z ޖ f h ~ mml== > > ��.n��������+%������w��{.n�����{��(��)�� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
