| Message ID | 20190121112336.23489-5-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | A3700 USB S2RAM support | expand |
On Mon, 21 Jan 2019, Miquel Raynal wrote: > Add suspend/resume callbacks to reset the host controller properly > during S2RAM operation. > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > --- > drivers/usb/host/ehci-orion.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c > index 3109f082949e..dab22aa57c0b 100644 > --- a/drivers/usb/host/ehci-orion.c > +++ b/drivers/usb/host/ehci-orion.c > @@ -182,6 +182,30 @@ static int ehci_orion_drv_reset(struct usb_hcd *hcd) > return ret; > } > > +static int __maybe_unused ehci_orion_drv_suspend(struct device *dev) > +{ > + struct usb_hcd *hcd = dev_get_drvdata(dev); > + struct ehci_hcd *ehci = hcd_to_ehci(hcd); > + > + ehci_prepare_ports_for_controller_suspend(ehci, > + device_may_wakeup(dev)); > + > + return ehci_suspend(hcd, false); This is a little odd. Why not just call ehci_suspend(hcd, device_may_wakeup(dev)); directly and let it take care of calling ehci_prepare_ports_for_controller_suspend() for you? If you have a good reason for doing things this way, please mention that reason in the Changelog. > +} > + > +static int __maybe_unused ehci_orion_drv_resume(struct device *dev) > +{ > + struct usb_hcd *hcd = dev_get_drvdata(dev); > + struct ehci_hcd *ehci = hcd_to_ehci(hcd); > + > + ehci_prepare_ports_for_controller_resume(ehci); > + > + return ehci_resume(hcd, false); Same thing here. > +} > + > +static SIMPLE_DEV_PM_OPS(ehci_orion_pm_ops, ehci_orion_drv_suspend, > + ehci_orion_drv_resume); > + > static const struct ehci_driver_overrides orion_overrides __initconst = { > .extra_priv_size = sizeof(struct orion_ehci_hcd), > .reset = ehci_orion_drv_reset, > @@ -334,6 +358,7 @@ static struct platform_driver ehci_orion_driver = { > .driver = { > .name = "orion-ehci", > .of_match_table = ehci_orion_dt_ids, > + .pm = &ehci_orion_pm_ops, > }, > }; Alan Stern
Hi Alan, Alan Stern <stern@rowland.harvard.edu> wrote on Wed, 23 Jan 2019 14:58:44 -0500 (EST): > On Mon, 21 Jan 2019, Miquel Raynal wrote: > > > Add suspend/resume callbacks to reset the host controller properly > > during S2RAM operation. > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > --- > > drivers/usb/host/ehci-orion.c | 25 +++++++++++++++++++++++++ > > 1 file changed, 25 insertions(+) > > > > diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c > > index 3109f082949e..dab22aa57c0b 100644 > > --- a/drivers/usb/host/ehci-orion.c > > +++ b/drivers/usb/host/ehci-orion.c > > @@ -182,6 +182,30 @@ static int ehci_orion_drv_reset(struct usb_hcd *hcd) > > return ret; > > } > > > > +static int __maybe_unused ehci_orion_drv_suspend(struct device *dev) > > +{ > > + struct usb_hcd *hcd = dev_get_drvdata(dev); > > + struct ehci_hcd *ehci = hcd_to_ehci(hcd); > > + > > + ehci_prepare_ports_for_controller_suspend(ehci, > > + device_may_wakeup(dev)); > > + > > + return ehci_suspend(hcd, false); > > This is a little odd. Why not just call > > ehci_suspend(hcd, device_may_wakeup(dev)); > > directly and let it take care of calling > ehci_prepare_ports_for_controller_suspend() for you? If you have a > good reason for doing things this way, please mention that reason in > the Changelog. There is no reason, I think I got the inspiration from an other driver calling manually ehci_prepare_ports_for_controller_suspend(), but I did not check it was already taken care of by ehci_suspend/resume(). Call removed in both paths, thanks! Miquèl
diff --git a/drivers/usb/host/ehci-orion.c b/drivers/usb/host/ehci-orion.c index 3109f082949e..dab22aa57c0b 100644 --- a/drivers/usb/host/ehci-orion.c +++ b/drivers/usb/host/ehci-orion.c @@ -182,6 +182,30 @@ static int ehci_orion_drv_reset(struct usb_hcd *hcd) return ret; } +static int __maybe_unused ehci_orion_drv_suspend(struct device *dev) +{ + struct usb_hcd *hcd = dev_get_drvdata(dev); + struct ehci_hcd *ehci = hcd_to_ehci(hcd); + + ehci_prepare_ports_for_controller_suspend(ehci, + device_may_wakeup(dev)); + + return ehci_suspend(hcd, false); +} + +static int __maybe_unused ehci_orion_drv_resume(struct device *dev) +{ + struct usb_hcd *hcd = dev_get_drvdata(dev); + struct ehci_hcd *ehci = hcd_to_ehci(hcd); + + ehci_prepare_ports_for_controller_resume(ehci); + + return ehci_resume(hcd, false); +} + +static SIMPLE_DEV_PM_OPS(ehci_orion_pm_ops, ehci_orion_drv_suspend, + ehci_orion_drv_resume); + static const struct ehci_driver_overrides orion_overrides __initconst = { .extra_priv_size = sizeof(struct orion_ehci_hcd), .reset = ehci_orion_drv_reset, @@ -334,6 +358,7 @@ static struct platform_driver ehci_orion_driver = { .driver = { .name = "orion-ehci", .of_match_table = ehci_orion_dt_ids, + .pm = &ehci_orion_pm_ops, }, };
Add suspend/resume callbacks to reset the host controller properly during S2RAM operation. Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- drivers/usb/host/ehci-orion.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)