Message-ID: <Pine.LNX.4.44L0.1505121129360.1400-100000@iolanthe.rowland.org>
Date:	Tue, 12 May 2015 11:54:07 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	"Lu, Baolu" <baolu.lu@...ux.intel.com>
cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Mathias Nyman <mathias.nyman@...el.com>,
	<linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/3] usb: notify hcd when USB device suspend or resume

On Tue, 12 May 2015, Lu, Baolu wrote:

> I'm sorry that I confused you.
> 
> FSC is a different thing from what this patch series does.

I know that.  The patch series, in its current form, is fine.  Now I'm
trying to understand what you originally wanted to do.

> > Let's see if I understand it correctly:
> >
> > 	When the controller goes into suspend, you want the cache to
> > 	be pushed into memory so that the cache can be powered down,
> > 	thereby saving additional energy.
> 
> Not the controller goes into suspend, but USB devices.

Suppose a USB device goes into runtime suspend, and suppose the
hardware pushes the cached information for the endpoints on that device
into memory.  The cache will still contain information for other,
non-suspended devices.  Consequently the cache _can't_ be powered down
at this time, right?  You seem to be saying:

	When a USB device goes into suspend, you want the cache to
	be pushed into memory so that the cache can be powered down,
	thereby saving additional energy.

But that is just wrong!  The only time you know the hardware can safely
power-down the cache is when the _controller_ goes into suspend.  At
that time you _know_ all the attached devices are suspended, so the
cache isn't needed.

Therefore, when the controller goes into suspend, you want to make sure 
that the cache has been pushed into memory.  The actual push operation 
can occur earlier, during xhci_device_suspend, but the time when the 
information's location matters is during xhci_suspend.

> In order to talking to USB devices, xHCI has an endpoint state for each
> endpoint of a device. When a USB device goes into suspend, xHC driver
> could ask the hardware to push the state from cache to the memory.
> Thereby saving additional energy. This is the intention of this patch 
> series.

You don't save any energy when the _device_ goes into suspend.  You
save energy only when the _controller_ goes into suspend, because 
that's the only time when the cache can be powered down.  Right?

> > 	If the hardware supports FSC, this will happen automatically.
> >
> > 	If the hardware doesn't support FSC, the cached data won't get
> > 	pushed to memory unless the driver tells the controller to do
> > 	so at the time the device is suspended.  But this will slow
> > 	things down, so the driver should avoid doing it when it's not
> > 	needed.
> >
> > 	During system suspend you know in advance that the controller
> > 	will be suspended.  Therefore the driver should push the cache
> > 	to memory if the hardware doesn't support FSC.  During runtime
> > 	suspend you don't know in advance whether the controller will
> > 	be suspended, so the driver should not push the cache to
> > 	memory.
> >
> > 	But what happens in the case where all the devices have gone
> > 	into runtime suspend, so the controller also goes into runtime
> > 	suspend, and the hardware doesn't support FSC?  It seems that
> > 	in this case the cache would have to remain powered on.
> 
> FSC is not part of this patch series. It was introduced when I tried to
> explain the reason why I kept 'msg' parameter in the callback.
> 
> If the hardware support FSC, another operation named "save context
> operation" will push everything in hardware cache into memory. So when
> a USB device is going to suspend and xhci_device_suspend() callback is
> being called, software can do an optimization. That is, if "save context
> operation" will push everything in hardware cache to memory later,
> xhci_device_suspend() could skip asking hardware for cache operation
> and let "save context operation" do it later.

That's more or less the same as what I wrote above, except that I said 
it would happen automatically if the hardware supports FSC.  It isn't 
automatic; it requires the driver to issue a "save context operation" 
command.

> One example of this situation is system suspend. During system suspend,
> below process will be done for a USB3 fabric.
> 
> 1) all USB devices suspend
> 2) root hub suspend
> 3) host controller suspend
> 
> In 1), xhci_device_suspend() call back will be called for each device 
> suspend.
> In 3) "save context operation" will be executed.
> 
> In this case, if FSC is supported, xhci_device_suspend() could skip asking
> host hardware for cache operation.

That's basically what I said.

But now why should "msg" matter?  It seems that xhci_device_suspend() 
should skip asking for the cache operation whenever FSC is supported, 
regardless of whether you're talking about runtime suspend or system 
suspend.

> > Also, it's still not clear what the driver needs to do differently in
> > order to push out the cached data.  You have managed to imply both:
> >
> > 	Issuing Stop Endpoint with SP set is mandatory whenever
> > 	a device is suspended, and
> >
> > 	The SP bit is what tells the controller to push the endpoint
> > 	state to memory.
> >
> > This doesn't make sense.
> 
> Software doesn't handle the cache directly. XHC driver just tell hardware
> that a device is going to suspend. XHC driver does this through stop 
> endpoint
> command by setting the SP (suspend) bit in the command trb.
> 
> During xHC handles stop endpoint command, it will check SP bit, if it's set,
> xHC knows that the endpoint state could be pushed out to memory now,
> since device is going to suspend. The operation of cache is very hardware
> implementation dependent.

Does that mean the driver has no control over whether "stop endpoint 
with SP set" will push the cached information into memory?  In that 
case, how can xhci_device_suspend skip asking the hardware for the 
cache operation?

> > One last question: How much extra time does it take to push the cached
> > data to memory?  Bear in mind that suspending a device is not a rapid
> > operation; if pushing the data takes no more than a few microseconds, I
> > think it would be worthwhile to do it always, even when it's not
> > necessary.
> 
> It depends on how much time software spends in xhci_device_suspend().
> I don't have data in hand, but I agree with you that if this time is no more
> than a few microseconds, we can let do it always.

Above you seem to be saying that there's no way to prevent it!  Your
exact words are: "The operation of cache is very hardware
implementation dependent."

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists