Re: [EXT] Re: [PATCH 3/8] usb: chipidea: host: add suspend/resume support for host controller

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Oct 10, 2022 at 02:54:02AM +0000, Xu Yang wrote:
> Hi Greg,
> 
> > -----Original Message-----
> > From: Greg KH <gregkh@linuxfoundation.org>
> > Sent: Monday, October 10, 2022 2:36 AM
> > To: Xu Yang <xu.yang_2@nxp.com>
> > Cc: peter.chen@kernel.org; Jun Li <jun.li@nxp.com>; linux-usb@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> > Subject: [EXT] Re: [PATCH 3/8] usb: chipidea: host: add suspend/resume support for host controller
> > 
> > Caution: EXT Email
> > 
> > On Mon, Oct 10, 2022 at 12:08:08AM +0800, Xu Yang wrote:
> > > The controller's power may be powered off during system suspend. This
> > > will add suspend/resume support when the controller suffers power lost.
> > >
> > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > > ---
> > >  drivers/usb/chipidea/host.c | 12 ++++++++++++
> > >  1 file changed, 12 insertions(+)
> > >
> > > diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> > > index bc3634a54c6b..dc033272c31e 100644
> > > --- a/drivers/usb/chipidea/host.c
> > > +++ b/drivers/usb/chipidea/host.c
> > > @@ -459,6 +459,16 @@ static void ci_hdrc_unmap_urb_for_dma(struct usb_hcd *hcd, struct urb *urb)
> > >       ci_hdrc_free_dma_aligned_buffer(urb);
> > >  }
> > >
> > > +static void ci_hdrc_host_suspend(struct ci_hdrc *ci)
> > > +{
> > > +     ehci_suspend(ci->hcd, device_may_wakeup(ci->dev));
> > 
> > Why is the chipidea driver somehow now tied to the ehci driver?
> 
> The chipidea host controller driver has been tied to ehci driver from the 
> beginning. Here, this patch will enhance the host controller by using ehci
> suspend/resume functions.
> 
> > 
> > Are you sure you want this?  Why?  Is hcd always a ehci device?
> 
> As the host controller may suffer power lost after system suspend, there is
> a need to restore it correctlly after system resume. ehci_suspend/resume()
> will handle the generic parts of controller suspend/resume, at the same time,
> ehci_resume() could handle power lost case by reporting this event to root
> hub and reset host controller in a normal way. So it's exactly a good choice
> to use them. 
> 
> As per chipidea RM: The USB 2.0 Controller is an instantiation of an 
> EHCI-compatible core which supports high-, full-, and low-speed operation.
> So this hcd will be a ehci device all the time.

Ah, sorry, you are right.  But the build breaks so you need to fix
something up to so that this dependancy really is there.

thanks,

greg k-h