Re: [PATCH v3 2/3] usb: xhci: Add the suspend/resume functionality

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Thu, Oct 18, 2012 at 07:59:56AM -0700, Sarah Sharp wrote:
> On Tue, Oct 16, 2012 at 12:59:33PM +0300, Felipe Balbi wrote:
> > Hi,
> > 
> > > +#ifdef CONFIG_PM_SLEEP
> > > +static int xhci_plat_suspend(struct device *dev)
> > > +{
> > > +	struct usb_hcd	*hcd	= dev_get_drvdata(dev);
> > > +	struct xhci_hcd	*xhci	= hcd_to_xhci(hcd);
> > > +
> > > +	/* Make sure that the HCD Core has set state to HC_STATE_SUSPENDED */
> > > +	if (hcd->state != HC_STATE_SUSPENDED ||
> > > +		xhci->shared_hcd->state != HC_STATE_SUSPENDED)
> > > +		return -EINVAL;
> > > +
> > > +	return xhci_suspend(xhci);
> > 
> > this is pretty much what xhci_pci_suspend() is doing. Sarah, would you
> > be ok with a patch such as:
> 
> Yes, that patch looks fine.
> 
> > usb: host: xhci: move HC_STATE_SUSPENDED check to xhci_suspend()
> > 
> > [ STILL NEED TO WRITE A PROPER COMMIT LOG ]
> > 
> > NYET-Signed-off-by: Felipe Balbi <balbi@xxxxxx>
> > ---
> > 
> > diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> > index 8345d7c..aeb3973 100644
> > --- a/drivers/usb/host/xhci-pci.c
> > +++ b/drivers/usb/host/xhci-pci.c
> > @@ -218,15 +218,8 @@ static void xhci_pci_remove(struct pci_dev *dev)
> >  static int xhci_pci_suspend(struct usb_hcd *hcd, bool do_wakeup)
> >  {
> >  	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
> > -	int	retval = 0;
> >  
> > -	if (hcd->state != HC_STATE_SUSPENDED ||
> > -			xhci->shared_hcd->state != HC_STATE_SUSPENDED)
> > -		return -EINVAL;
> > -
> > -	retval = xhci_suspend(xhci);
> > -
> > -	return retval;
> > +	return xhci_suspend(xhci);
> >  }
> >  
> >  static int xhci_pci_resume(struct usb_hcd *hcd, bool hibernated)
> > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> > index 8d7fcbb..b85029e 100644
> > --- a/drivers/usb/host/xhci.c
> > +++ b/drivers/usb/host/xhci.c
> > @@ -879,6 +879,10 @@ int xhci_suspend(struct xhci_hcd *xhci)
> >  	struct usb_hcd		*hcd = xhci_to_hcd(xhci);
> >  	u32			command;
> >  
> > +	if (hcd->state != HC_STATE_SUSPENDED ||
> > +			xhci->shared_hcd->state != HC_STATE_SUSPENDED)
> > +		return -EINVAL;
> > +
> >  	spin_lock_irq(&xhci->lock);
> >  	clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
> >  	clear_bit(HCD_FLAG_HW_ACCESSIBLE, &xhci->shared_hcd->flags);
> > 
> > do you think there is any reason to keep replicating the
> > HC_STATE_SUSPENDED test all over the place ?
> 
> Nope, no reason I can see.  I'm pretty sure I only check the suspended
> state in case the code in the USB core to handle the host controller
> suspend ever breaks for the corner case of the host hardware needing two
> roothubs (USB 2.0 and USB 3.0 in this case).  It doesn't matter if we
> check that in the PCI/platform suspend or in xhci_suspend, as long as it
> gets checked.

cool, I'll send a proper patch tomorrow ;-)

cheers

-- 
balbi

Attachment: signature.asc
Description: Digital signature


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux