Re: [RFC][PATCH 1/3] usb: add a hcd notify entry in hc_driver

[Lu, Baolu]

On 05/04/2015 04:14 PM, Greg Kroah-Hartman wrote:
> On Mon, May 04, 2015 at 11:15:30AM +0800, Lu Baolu wrote:
> > This patch adds a new entry pointer in hc_driver. With this new entry,
> > USB core can notify host driver when something happens and host driver
> > is willing to be notified. One use case of this interface comes from
> > xHCI compatible host controller driver.
> >
> > The xHCI spec is designed to allow an xHC implementation to cache the
> > endpoint state. Caching endpoint state allows an xHC to reduce latency
> > when handling ERDYs and other USB asynchronous events. However holding
> > this state in xHC consumes resources and power. The xHCI spec designs
> > some methods through which host controller driver can hint xHC about
> > how to optimize its operation, e.g. to determine when it holds state
> > internally or pushes it out to memory, when to power down logic, etc.
> >
> > When a USB device is going to suspend, states of all endpoints cached
> > in the xHC should be pushed out to memory to save power and resources.
> > Vice versa, when a USB device resumes, those states should be brought
> > back to the cache. USB core suspends or resumes a USB device by sending
> > set/clear port feature requests to the parent hub where the USB device
> > is connected. Unfortunately, these operations are transparent to xHCI
> > driver unless the USB device is plugged in a root port. This patch
> > utilizes the notify interface to notify xHCI driver whenever a USB
> > device's power state is changed.
> >
> > It is harmless if a USB devices under USB 3.0 host controller suspends
> > or resumes without a notification to hcd driver. However there may be
> > less opportunities for power savings and there may be increased latency
> > for restarting an endpoint. The precise impact will be different for
> > each xHC implementation. It all depends on what an implementation does
> > with the hints.
> >
> > Signed-off-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> > ---
> >   drivers/usb/core/generic.c | 10 ++++++++--
> >   drivers/usb/core/hcd.c     | 23 +++++++++++++++++++++++
> >   include/linux/usb/hcd.h    | 11 ++++++++++-
> >   3 files changed, 41 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c
> > index 358ca8d..92bee33 100644
> > --- a/drivers/usb/core/generic.c
> > +++ b/drivers/usb/core/generic.c
> > @@ -211,8 +211,12 @@ static int generic_suspend(struct usb_device *udev, pm_message_t msg)
> >   	/* Non-root devices don't need to do anything for FREEZE or PRETHAW */
> >   	else if (msg.event == PM_EVENT_FREEZE || msg.event == PM_EVENT_PRETHAW)
> >   		rc = 0;
> > -	else
> > +	else {
> > +		hcd_notify(udev, HCD_MSG_DEV_SUSPEND, &msg);
> >   		rc = usb_port_suspend(udev, msg);
> > +		if (rc)
> > +			hcd_notify(udev, HCD_MSG_DEV_RESUME, &msg);
> > +	}
> >   
> >   	return rc;
> >   }
> > @@ -228,8 +232,10 @@ static int generic_resume(struct usb_device *udev, pm_message_t msg)
> >   	 */
> >   	if (!udev->parent)
> >   		rc = hcd_bus_resume(udev, msg);
> > -	else
> > +	else {
> >   		rc = usb_port_resume(udev, msg);
> > +		hcd_notify(udev, HCD_MSG_DEV_RESUME, &msg);
> > +	}
> >   	return rc;
> >   }
> >   
> > diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> > index 45a915c..725d611 100644
> > --- a/drivers/usb/core/hcd.c
> > +++ b/drivers/usb/core/hcd.c
> > @@ -2289,6 +2289,29 @@ void usb_hcd_resume_root_hub (struct usb_hcd *hcd)
> >   }
> >   EXPORT_SYMBOL_GPL(usb_hcd_resume_root_hub);
> >   
> > +/**
> > + * hcd_notify - notify hcd driver with a message
> > + * @udev: USB device
> > + * @type: message type of this notification
> > + * @data: message type specific data
> > + *
> > + * Call back to hcd driver to notify an event.
> > + */
> > +void hcd_notify(struct usb_device *udev,
> > +		enum hcd_notification_type type, void *data)
> > +{
> > +	struct usb_hcd *hcd;
> > +
> > +	if (!udev)
> > +		return;
> > +
> > +	hcd = bus_to_hcd(udev->bus);
> > +
> > +	if (hcd->driver->notify)
> > +		hcd->driver->notify(hcd, udev, type, data);
> > +}
> > +EXPORT_SYMBOL_GPL(hcd_notify);
> Why does this have to be exported?
It doesn't have to be exported. I will remove it in v2.

>

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