Re: [PATCH v4 5/7] iio: hid-sensors: use asynchronous resume

3 messages, 2 authors, 2016-08-15 · open the first message on its own page

Re: [PATCH v4 5/7] iio: hid-sensors: use asynchronous resume

From: Srinivas Pandruvada <hidden>
Date: 2016-08-15 17:29:23

On Mon, 2016-08-15 at 10:14 -0700, Dmitry Torokhov wrote:
On Mon, Aug 15, 2016 at 9:42 AM, Srinivas Pandruvada
[off-list ref] wrote:
quoted
On Mon, 2016-08-15 at 08:45 -0700, Dmitry Torokhov wrote:
quoted
On Mon, Aug 15, 2016 at 7:52 AM, Jonathan Cameron <jic23@kernel.o
rg>
wrote:
quoted

On 15/08/16 15:07, Jonathan Cameron wrote:
quoted

On 07/08/16 11:15, Jiri Kosina wrote:
quoted

On Sun, 7 Aug 2016, Srinivas Pandruvada wrote:
quoted

Some platforms power off sensor hubs during S3 suspend,
which
will require
longer time to resume. This hurts system resume time, so
resume
asynchronously.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@l
inux
.intel.com>
Jonathan, are you going to cherry-pick this patch from the
series?
Alternatively, if you're okay with it, I can pull it in
together with the
whole set with your Acked-by or Reviewed-by.
I'll take it via IIO. Got a bit of catching up to do (been on
holiday)
Applied to the togreg branch of iio.git - initially pushed out
as
testing for the autobuilders to play with it.
This one is not really connected to the others so makes sense
to
take it separately.

I'm out of my depth on the rest of the patches in this series
and don't have time to learn enough to follow them! Sorry I
can't help on that front.
About this patch: me sees a new work, me does not see new calls
to
cancel_work_sync() or flush_work() anywhere, me gets worried.
This work is scheduled during resume and is not delayed call. Only
time
really we need to cancel or flush if module is unloaded before
resume
work, not sure if this case realistic. Do you see any other case
possible?
Runtime resume can happen at any time, I can unload module or unbind
it at any time. I also wasn't aware that our implementation goal for
locking rules/lifetime rules/etc was "realistic" instead of
"correct".
This is not for runtime_resume, this is for regular S3 suspend. But I
agree, I will submit a patch for correctness.

Thanks,
Srinivas
Thanks.

Re: [PATCH v4 5/7] iio: hid-sensors: use asynchronous resume

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2016-08-15 17:53:25

On Mon, Aug 15, 2016 at 10:29:23AM -0700, Srinivas Pandruvada wrote:
On Mon, 2016-08-15 at 10:14 -0700, Dmitry Torokhov wrote:
quoted
On Mon, Aug 15, 2016 at 9:42 AM, Srinivas Pandruvada
[off-list ref] wrote:
quoted
On Mon, 2016-08-15 at 08:45 -0700, Dmitry Torokhov wrote:
quoted
On Mon, Aug 15, 2016 at 7:52 AM, Jonathan Cameron <jic23@kernel.o
rg>
wrote:
quoted

On 15/08/16 15:07, Jonathan Cameron wrote:
quoted

On 07/08/16 11:15, Jiri Kosina wrote:
quoted

On Sun, 7 Aug 2016, Srinivas Pandruvada wrote:
quoted

Some platforms power off sensor hubs during S3 suspend,
which
will require
longer time to resume. This hurts system resume time, so
resume
asynchronously.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@l
inux
.intel.com>
Jonathan, are you going to cherry-pick this patch from the
series?
Alternatively, if you're okay with it, I can pull it in
together with the
whole set with your Acked-by or Reviewed-by.
I'll take it via IIO. Got a bit of catching up to do (been on
holiday)
Applied to the togreg branch of iio.git - initially pushed out
as
testing for the autobuilders to play with it.
This one is not really connected to the others so makes sense
to
take it separately.

I'm out of my depth on the rest of the patches in this series
and don't have time to learn enough to follow them! Sorry I
can't help on that front.
About this patch: me sees a new work, me does not see new calls
to
cancel_work_sync() or flush_work() anywhere, me gets worried.
This work is scheduled during resume and is not delayed call. Only
time
really we need to cancel or flush if module is unloaded before
resume
work, not sure if this case realistic. Do you see any other case
possible?
Runtime resume can happen at any time, I can unload module or unbind
it at any time. I also wasn't aware that our implementation goal for
locking rules/lifetime rules/etc was "realistic" instead of
"correct".
This is not for runtime_resume, this is for regular S3 suspend. But I
agree, I will submit a patch for correctness.
Thinking about it some more: if you are off-loading powering up the hub
to work structure it means that the hub is not actually powered up when
the rest of the system thinks it is, which may cause subsequent requests
to it fail.

You need to make sure noone is actually interacting with the device
until you are done powering it up.

Thanks.

-- 
Dmitry

Re: [PATCH v4 5/7] iio: hid-sensors: use asynchronous resume

From: Srinivas Pandruvada <hidden>
Date: 2016-08-15 18:24:00

On Mon, 2016-08-15 at 10:53 -0700, Dmitry Torokhov wrote:
On Mon, Aug 15, 2016 at 10:29:23AM -0700, Srinivas Pandruvada wrote:
quoted
On Mon, 2016-08-15 at 10:14 -0700, Dmitry Torokhov wrote:
quoted
On Mon, Aug 15, 2016 at 9:42 AM, Srinivas Pandruvada
[off-list ref] wrote:
quoted

On Mon, 2016-08-15 at 08:45 -0700, Dmitry Torokhov wrote:
quoted

On Mon, Aug 15, 2016 at 7:52 AM, Jonathan Cameron <jic23@kern
el.o
rg>
wrote:
quoted


On 15/08/16 15:07, Jonathan Cameron wrote:
quoted


On 07/08/16 11:15, Jiri Kosina wrote:
quoted


On Sun, 7 Aug 2016, Srinivas Pandruvada wrote:
quoted


Some platforms power off sensor hubs during S3
suspend,
which
will require
longer time to resume. This hurts system resume time,
so
resume
asynchronously.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruva
da@l
inux
.intel.com>
Jonathan, are you going to cherry-pick this patch from
the
series?
Alternatively, if you're okay with it, I can pull it in
together with the
whole set with your Acked-by or Reviewed-by.
I'll take it via IIO. Got a bit of catching up to do
(been on
holiday)
Applied to the togreg branch of iio.git - initially pushed
out
as
testing for the autobuilders to play with it.
This one is not really connected to the others so makes
sense
to
take it separately.

I'm out of my depth on the rest of the patches in this
series
and don't have time to learn enough to follow them! Sorry I
can't help on that front.
About this patch: me sees a new work, me does not see new
calls
to
cancel_work_sync() or flush_work() anywhere, me gets worried.
This work is scheduled during resume and is not delayed call.
Only
time
really we need to cancel or flush if module is unloaded before
resume
work, not sure if this case realistic. Do you see any other
case
possible?
Runtime resume can happen at any time, I can unload module or
unbind
it at any time. I also wasn't aware that our implementation goal
for
locking rules/lifetime rules/etc was "realistic" instead of
"correct".
This is not for runtime_resume, this is for regular S3 suspend. But
I
agree, I will submit a patch for correctness.
Thinking about it some more: if you are off-loading powering up the
hub
to work structure it means that the hub is not actually powered up
when
the rest of the system thinks it is, which may cause subsequent
requests
to it fail.

You need to make sure noone is actually interacting with the device
until you are done powering it up.
This will happen as the input request will be waiting till sensor is
powered up and response is received.

Thanks,
Srinivas

Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help