| Message ID | 1470561939-14278-6-git-send-email-srinivas.pandruvada@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, 7 Aug 2016, Srinivas Pandruvada wrote: > 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@linux.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. Thanks,
On 07/08/16 11:15, Jiri Kosina wrote: > On Sun, 7 Aug 2016, Srinivas Pandruvada wrote: > >> 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@linux.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) Jonathan > Thanks, > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 15/08/16 15:07, Jonathan Cameron wrote: > On 07/08/16 11:15, Jiri Kosina wrote: >> On Sun, 7 Aug 2016, Srinivas Pandruvada wrote: >> >>> 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@linux.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. Jonathan > > Jonathan >> Thanks, >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Aug 15, 2016 at 7:52 AM, Jonathan Cameron <jic23@kernel.org> wrote: > On 15/08/16 15:07, Jonathan Cameron wrote: >> On 07/08/16 11:15, Jiri Kosina wrote: >>> On Sun, 7 Aug 2016, Srinivas Pandruvada wrote: >>> >>>> 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@linux.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. Thanks.
On 15/08/16 16:45, Dmitry Torokhov wrote: > On Mon, Aug 15, 2016 at 7:52 AM, Jonathan Cameron <jic23@kernel.org> wrote: >> On 15/08/16 15:07, Jonathan Cameron wrote: >>> On 07/08/16 11:15, Jiri Kosina wrote: >>>> On Sun, 7 Aug 2016, Srinivas Pandruvada wrote: >>>> >>>>> 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@linux.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. > > Thanks. Good point. Backed out for now... Jonathan > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2016-08-15 at 08:45 -0700, Dmitry Torokhov wrote: > On Mon, Aug 15, 2016 at 7:52 AM, Jonathan Cameron <jic23@kernel.org> > wrote: > > > > On 15/08/16 15:07, Jonathan Cameron wrote: > > > > > > On 07/08/16 11:15, Jiri Kosina wrote: > > > > > > > > On Sun, 7 Aug 2016, Srinivas Pandruvada wrote: > > > > > > > > > > > > > > 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@linux > > > > > .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? Thanks, Srinivas > > Thanks. > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Aug 15, 2016 at 9:42 AM, Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote: > On Mon, 2016-08-15 at 08:45 -0700, Dmitry Torokhov wrote: >> On Mon, Aug 15, 2016 at 7:52 AM, Jonathan Cameron <jic23@kernel.org> >> wrote: >> > >> > On 15/08/16 15:07, Jonathan Cameron wrote: >> > > >> > > On 07/08/16 11:15, Jiri Kosina wrote: >> > > > >> > > > On Sun, 7 Aug 2016, Srinivas Pandruvada wrote: >> > > > >> > > > > >> > > > > 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@linux >> > > > > .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". Thanks.
On Mon, 2016-08-15 at 10:14 -0700, Dmitry Torokhov wrote: > On Mon, Aug 15, 2016 at 9:42 AM, Srinivas Pandruvada > <srinivas.pandruvada@linux.intel.com> wrote: > > > > On Mon, 2016-08-15 at 08:45 -0700, Dmitry Torokhov wrote: > > > > > > On Mon, Aug 15, 2016 at 7:52 AM, Jonathan Cameron <jic23@kernel.o > > > rg> > > > wrote: > > > > > > > > > > > > On 15/08/16 15:07, Jonathan Cameron wrote: > > > > > > > > > > > > > > > On 07/08/16 11:15, Jiri Kosina wrote: > > > > > > > > > > > > > > > > > > On Sun, 7 Aug 2016, Srinivas Pandruvada wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > 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. > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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: > > On Mon, Aug 15, 2016 at 9:42 AM, Srinivas Pandruvada > > <srinivas.pandruvada@linux.intel.com> wrote: > > > > > > On Mon, 2016-08-15 at 08:45 -0700, Dmitry Torokhov wrote: > > > > > > > > On Mon, Aug 15, 2016 at 7:52 AM, Jonathan Cameron <jic23@kernel.o > > > > rg> > > > > wrote: > > > > > > > > > > > > > > > On 15/08/16 15:07, Jonathan Cameron wrote: > > > > > > > > > > > > > > > > > > On 07/08/16 11:15, Jiri Kosina wrote: > > > > > > > > > > > > > > > > > > > > > On Sun, 7 Aug 2016, Srinivas Pandruvada wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 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.
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: > > > > On Mon, 2016-08-15 at 10:14 -0700, Dmitry Torokhov wrote: > > > > > > On Mon, Aug 15, 2016 at 9:42 AM, Srinivas Pandruvada > > > <srinivas.pandruvada@linux.intel.com> wrote: > > > > > > > > > > > > On Mon, 2016-08-15 at 08:45 -0700, Dmitry Torokhov wrote: > > > > > > > > > > > > > > > On Mon, Aug 15, 2016 at 7:52 AM, Jonathan Cameron <jic23@kern > > > > > el.o > > > > > rg> > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > On 15/08/16 15:07, Jonathan Cameron wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > On 07/08/16 11:15, Jiri Kosina wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Sun, 7 Aug 2016, Srinivas Pandruvada wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 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. > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c index 5b41f9d..9784ae3 100644 --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c @@ -122,6 +122,14 @@ int hid_sensor_power_state(struct hid_sensor_common *st, bool state) #endif } +static void hid_sensor_set_power_work(struct work_struct *work) +{ + struct hid_sensor_common *attrb = container_of(work, + struct hid_sensor_common, + work); + _hid_sensor_power_state(attrb, true); +} + static int hid_sensor_data_rdy_trigger_set_state(struct iio_trigger *trig, bool state) { @@ -170,6 +178,9 @@ int hid_sensor_setup_trigger(struct iio_dev *indio_dev, const char *name, goto error_unreg_trigger; iio_device_set_drvdata(indio_dev, attrb); + + INIT_WORK(&attrb->work, hid_sensor_set_power_work); + pm_suspend_ignore_children(&attrb->pdev->dev, true); pm_runtime_enable(&attrb->pdev->dev); /* Default to 3 seconds, but can be changed from sysfs */ @@ -202,7 +213,15 @@ static int hid_sensor_resume(struct device *dev) struct platform_device *pdev = to_platform_device(dev); struct iio_dev *indio_dev = platform_get_drvdata(pdev); struct hid_sensor_common *attrb = iio_device_get_drvdata(indio_dev); + schedule_work(&attrb->work); + return 0; +} +static int hid_sensor_runtime_resume(struct device *dev) +{ + struct platform_device *pdev = to_platform_device(dev); + struct iio_dev *indio_dev = platform_get_drvdata(pdev); + struct hid_sensor_common *attrb = iio_device_get_drvdata(indio_dev); return _hid_sensor_power_state(attrb, true); } @@ -211,7 +230,7 @@ static int hid_sensor_resume(struct device *dev) const struct dev_pm_ops hid_sensor_pm_ops = { SET_SYSTEM_SLEEP_PM_OPS(hid_sensor_suspend, hid_sensor_resume) SET_RUNTIME_PM_OPS(hid_sensor_suspend, - hid_sensor_resume, NULL) + hid_sensor_runtime_resume, NULL) }; EXPORT_SYMBOL(hid_sensor_pm_ops); diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h index c02b5ce..dd85f35 100644 --- a/include/linux/hid-sensor-hub.h +++ b/include/linux/hid-sensor-hub.h @@ -236,6 +236,7 @@ struct hid_sensor_common { struct hid_sensor_hub_attribute_info report_state; struct hid_sensor_hub_attribute_info power_state; struct hid_sensor_hub_attribute_info sensitivity; + struct work_struct work; }; /* Convert from hid unit expo to regular exponent */
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@linux.intel.com> --- drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 21 ++++++++++++++++++++- include/linux/hid-sensor-hub.h | 1 + 2 files changed, 21 insertions(+), 1 deletion(-)