| Message ID | 20180725152515.1833-3-kai.heng.feng@canonical.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Keep rtsx_usb suspended when there's no card | expand |
On Wed, 25 Jul 2018, Kai-Heng Feng wrote: > We can use MEMSTICK_POWER_{ON,OFF} along with pm_runtime_{get,put} > helpers to let memstick host support runtime pm. > > There's a small window between memstick_detect_change() and its queued > work, memstick_check(). In this window the rpm count may go down to zero > before the memstick host powers on, so the host can be inadvertently > suspended. > > Increment rpm count before calling memstick_check(), and decrement rpm > count afterward, as now we are sure the memstick host should be > suspended or not. > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > --- > drivers/memstick/core/memstick.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/memstick/core/memstick.c b/drivers/memstick/core/memstick.c > index 76382c858c35..944fe0c93fa7 100644 > --- a/drivers/memstick/core/memstick.c > +++ b/drivers/memstick/core/memstick.c > @@ -18,6 +18,7 @@ > #include <linux/delay.h> > #include <linux/slab.h> > #include <linux/module.h> > +#include <linux/pm_runtime.h> > > #define DRIVER_NAME "memstick" > > @@ -209,6 +210,7 @@ static int memstick_dummy_check(struct memstick_dev *card) > */ > void memstick_detect_change(struct memstick_host *host) > { > + pm_runtime_get_noresume(host->dev.parent); > queue_work(workqueue, &host->media_checker); > } > EXPORT_SYMBOL(memstick_detect_change); > @@ -479,6 +481,8 @@ static void memstick_check(struct work_struct *work) > host->set_param(host, MEMSTICK_POWER, MEMSTICK_POWER_OFF); > > mutex_unlock(&host->lock); > + > + pm_runtime_put_noidle(host->dev.parent); > dev_dbg(&host->dev, "memstick_check finished\n"); > } This should be pm_runtime_put(), not _put_noidle(). The reason is because if this call causes the PM runtime usage count to drop to 0, you do want the device to go into runtime suspend. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On 2018Jul26, at 01:56, Alan Stern <stern@rowland.harvard.edu> wrote: > > On Wed, 25 Jul 2018, Kai-Heng Feng wrote: > >> We can use MEMSTICK_POWER_{ON,OFF} along with pm_runtime_{get,put} >> helpers to let memstick host support runtime pm. >> >> There's a small window between memstick_detect_change() and its queued >> work, memstick_check(). In this window the rpm count may go down to zero >> before the memstick host powers on, so the host can be inadvertently >> suspended. >> >> Increment rpm count before calling memstick_check(), and decrement rpm >> count afterward, as now we are sure the memstick host should be >> suspended or not. >> >> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> >> --- >> drivers/memstick/core/memstick.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/memstick/core/memstick.c b/drivers/memstick/core/memstick.c >> index 76382c858c35..944fe0c93fa7 100644 >> --- a/drivers/memstick/core/memstick.c >> +++ b/drivers/memstick/core/memstick.c >> @@ -18,6 +18,7 @@ >> #include <linux/delay.h> >> #include <linux/slab.h> >> #include <linux/module.h> >> +#include <linux/pm_runtime.h> >> >> #define DRIVER_NAME "memstick" >> >> @@ -209,6 +210,7 @@ static int memstick_dummy_check(struct memstick_dev *card) >> */ >> void memstick_detect_change(struct memstick_host *host) >> { >> + pm_runtime_get_noresume(host->dev.parent); >> queue_work(workqueue, &host->media_checker); >> } >> EXPORT_SYMBOL(memstick_detect_change); >> @@ -479,6 +481,8 @@ static void memstick_check(struct work_struct *work) >> host->set_param(host, MEMSTICK_POWER, MEMSTICK_POWER_OFF); >> >> mutex_unlock(&host->lock); >> + >> + pm_runtime_put_noidle(host->dev.parent); >> dev_dbg(&host->dev, "memstick_check finished\n"); >> } > > This should be pm_runtime_put(), not _put_noidle(). The reason is > because if this call causes the PM runtime usage count to drop to 0, > you do want the device to go into runtime suspend. Will update in next version, thanks! Kai-Heng > > Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" 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/memstick/core/memstick.c b/drivers/memstick/core/memstick.c index 76382c858c35..944fe0c93fa7 100644 --- a/drivers/memstick/core/memstick.c +++ b/drivers/memstick/core/memstick.c @@ -18,6 +18,7 @@ #include <linux/delay.h> #include <linux/slab.h> #include <linux/module.h> +#include <linux/pm_runtime.h> #define DRIVER_NAME "memstick" @@ -209,6 +210,7 @@ static int memstick_dummy_check(struct memstick_dev *card) */ void memstick_detect_change(struct memstick_host *host) { + pm_runtime_get_noresume(host->dev.parent); queue_work(workqueue, &host->media_checker); } EXPORT_SYMBOL(memstick_detect_change); @@ -479,6 +481,8 @@ static void memstick_check(struct work_struct *work) host->set_param(host, MEMSTICK_POWER, MEMSTICK_POWER_OFF); mutex_unlock(&host->lock); + + pm_runtime_put_noidle(host->dev.parent); dev_dbg(&host->dev, "memstick_check finished\n"); }
We can use MEMSTICK_POWER_{ON,OFF} along with pm_runtime_{get,put} helpers to let memstick host support runtime pm. There's a small window between memstick_detect_change() and its queued work, memstick_check(). In this window the rpm count may go down to zero before the memstick host powers on, so the host can be inadvertently suspended. Increment rpm count before calling memstick_check(), and decrement rpm count afterward, as now we are sure the memstick host should be suspended or not. Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> --- drivers/memstick/core/memstick.c | 4 ++++ 1 file changed, 4 insertions(+)