On Fri, 22 Mar 2019, Tony Lindgren wrote:
> * Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> [190322 20:03]:
> > On Fri, 22 Mar 2019, Tony Lindgren wrote:
> >
> > > * Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> [190322 18:37]:
> > > > How about putting these runtime PM additions into
> > > > usb_hcd_platform_shutdown instead, so they will apply to all platform
> > > > controller drivers?
> > >
> > > OK let's do that then.
> > >
> > > > Also, are you certain you want the pm_runtime_put_sync at the end? If
> > > > the system is shutting down anyway, why waste time doing an extra
> > > > runtime suspend?
> > >
> > > Well mostly to keep the calls paired. But maybe there are
> > > also kexec reboot cases where we'd want to have things
> > > properly disabled for PM before kexec.
> >
> > I'm not sure that makes sense. You can't actually disable anything for
> > runtime PM from within a driver; all you can do is tell the runtime PM
> > core that _you're_ not using the device any more. But if some other
> > part of the system is still using it, it will remain at full power.
>
> Right, it's just a usecount and whatever happens after that is
> ouf of the driver control. And in the shutdown case PM runtime
> does not have much of a chance to do anything here :)
>
> Not much point checking the pm_runtime_get_sync() for errors
> either then.. We can just add a comment there.
>
> If the patch below looks OK to you I'll post with an updated
> description.
>
> Regards,
>
> Tony
>
> 8< -------------------
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -3017,6 +3017,9 @@ usb_hcd_platform_shutdown(struct platform_device *dev)
> {
> struct usb_hcd *hcd = platform_get_drvdata(dev);
>
> + /* No need for pm_runtime_put(), we're shutting down */
> + pm_runtime_get_sync(&dev->dev);
> +
> if (hcd->driver->shutdown)
> hcd->driver->shutdown(hcd);
> }
Yes, that will be okay. Assuming it fixes your problem, of course. :-)
Alan Stern
