| Message ID | 20180731061721.15831-2-kai.heng.feng@canonical.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Keep rtsx_usb suspended when there's no card | expand |
Hi Alan, Ulf, at 14:17, Kai-Heng Feng <kai.heng.feng@canonical.com> wrote: > Although rtsx_usb doesn't support card removal detection, card insertion > will resume rtsx_usb by USB remote wakeup signaling. > > When rtsx_usb gets resumed, also resumes its child devices, > rtsx_usb_sdmmc and rtsx_usb_ms, to notify them there's a card in its > slot. > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > --- > drivers/misc/cardreader/rtsx_usb.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/misc/cardreader/rtsx_usb.c > b/drivers/misc/cardreader/rtsx_usb.c > index b97903ff1a72..f7a66f614085 100644 > --- a/drivers/misc/cardreader/rtsx_usb.c > +++ b/drivers/misc/cardreader/rtsx_usb.c > @@ -723,8 +723,15 @@ static int rtsx_usb_suspend(struct usb_interface > *intf, pm_message_t message) > return 0; > } > > +static int rtsx_usb_resume_child(struct device *dev, void *data) > +{ > + pm_request_resume(dev); > + return 0; > +} > + > static int rtsx_usb_resume(struct usb_interface *intf) > { > + device_for_each_child(&intf->dev, NULL, rtsx_usb_resume_child); > return 0; > } > > @@ -734,6 +741,7 @@ static int rtsx_usb_reset_resume(struct usb_interface > *intf) > (struct rtsx_ucr *)usb_get_intfdata(intf); > > rtsx_usb_reset_chip(ucr); > + device_for_each_child(&intf->dev, NULL, rtsx_usb_resume_child); > return 0; > } I am working on the next version of this series, and the last missing puzzle is to differentiate system-wide resume from runtime resume in usb_driver's resume() and reset_resume() callback. The parent device, rtsx_usb, has two child devices, rtsx_usb_ms and rtsx_usb_sdmmc. pm_request_resume() is used in rtsx_usb's resume() to facilitate USB remote wakeup signaling, so we don't need to poll the card slot status. But this has a side effect: during system resume the rtsx_usb calls pm_request_resume() in its resume(), so child devices calls their runtime_resume() instead of resume() callback. So, is it reasonable to pass pm_message_t to resume() and reset_resume() callbacks and use PMSG_IS_AUTO() to differentiate them? Kai-Heng > > -- > 2.17.1
On Thu, 13 Sep 2018, Kai-Heng Feng wrote: > I am working on the next version of this series, and the last missing > puzzle is to differentiate system-wide resume from runtime resume in > usb_driver's resume() and reset_resume() callback. > > The parent device, rtsx_usb, has two child devices, rtsx_usb_ms and > rtsx_usb_sdmmc. > pm_request_resume() is used in rtsx_usb's resume() to facilitate USB remote > wakeup signaling, so we don't need to poll the card slot status. > But this has a side effect: during system resume the rtsx_usb calls > pm_request_resume() in its resume(), so child devices calls their > runtime_resume() instead of resume() callback. Have you actually observed this? It shouldn't happen. pm_request_resume() schedules a runtime resume on the PM work queue, but this work queue is frozen during system sleep. It doesn't unfreeze until after all the devices have been restored to full power. Thus the child device's resume() callback should be invoked before runtime_resume(). > So, is it reasonable to pass pm_message_t to resume() and reset_resume() > callbacks and use PMSG_IS_AUTO() to differentiate them? It should not be necessary to do this. Alan Stern
at 21:41, Alan Stern <stern@rowland.harvard.edu> wrote: > On Thu, 13 Sep 2018, Kai-Heng Feng wrote: > >> I am working on the next version of this series, and the last missing >> puzzle is to differentiate system-wide resume from runtime resume in >> usb_driver's resume() and reset_resume() callback. >> >> The parent device, rtsx_usb, has two child devices, rtsx_usb_ms and >> rtsx_usb_sdmmc. >> pm_request_resume() is used in rtsx_usb's resume() to facilitate USB >> remote >> wakeup signaling, so we don't need to poll the card slot status. >> But this has a side effect: during system resume the rtsx_usb calls >> pm_request_resume() in its resume(), so child devices calls their >> runtime_resume() instead of resume() callback. > > Have you actually observed this? It shouldn't happen. > pm_request_resume() schedules a runtime resume on the PM work queue, > but this work queue is frozen during system sleep. It doesn't unfreeze > until after all the devices have been restored to full power. Thus the > child device's resume() callback should be invoked before > runtime_resume(). You are right, I read the trace wrong. It happens right before system-wide suspension, when the PM core resumes it to fully functional state. > >> So, is it reasonable to pass pm_message_t to resume() and reset_resume() >> callbacks and use PMSG_IS_AUTO() to differentiate them? > > It should not be necessary to do this. Understood, thanks. Kai-Heng > > Alan Stern
diff --git a/drivers/misc/cardreader/rtsx_usb.c b/drivers/misc/cardreader/rtsx_usb.c index b97903ff1a72..f7a66f614085 100644 --- a/drivers/misc/cardreader/rtsx_usb.c +++ b/drivers/misc/cardreader/rtsx_usb.c @@ -723,8 +723,15 @@ static int rtsx_usb_suspend(struct usb_interface *intf, pm_message_t message) return 0; } +static int rtsx_usb_resume_child(struct device *dev, void *data) +{ + pm_request_resume(dev); + return 0; +} + static int rtsx_usb_resume(struct usb_interface *intf) { + device_for_each_child(&intf->dev, NULL, rtsx_usb_resume_child); return 0; } @@ -734,6 +741,7 @@ static int rtsx_usb_reset_resume(struct usb_interface *intf) (struct rtsx_ucr *)usb_get_intfdata(intf); rtsx_usb_reset_chip(ucr); + device_for_each_child(&intf->dev, NULL, rtsx_usb_resume_child); return 0; }
Although rtsx_usb doesn't support card removal detection, card insertion will resume rtsx_usb by USB remote wakeup signaling. When rtsx_usb gets resumed, also resumes its child devices, rtsx_usb_sdmmc and rtsx_usb_ms, to notify them there's a card in its slot. Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> --- drivers/misc/cardreader/rtsx_usb.c | 8 ++++++++ 1 file changed, 8 insertions(+)