usb: add a hcd notify entry in hc_driver
Subsystems (2)
cover usb: add a hcd notify entry in hc_driver
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.
Lu Baolu (3):
usb: add a hcd notify entry in hc_driver
usb: xhci: implement hc_driver notify entry
usb: xhci: cleanup unnecessary stop device and ring doorbell
operations
drivers/usb/core/generic.c | 10 +++++++-- drivers/usb/core/hcd.c | 23 +++++++++++++++++++++ drivers/usb/host/xhci-hub.c | 49 +-------------------------------------------- drivers/usb/host/xhci.c | 28 ++++++++++++++++++++++++++ drivers/usb/host/xhci.h | 3 +++ include/linux/usb/hcd.h | 11 +++++++++- 6 files changed, 73 insertions(+), 51 deletions(-)
Signature · 1 line
2.1.0
RFC PATCH 1/3 usb: add a hcd notify entry in hc_driver
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.
--- 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(-)
drivers/usb/core/generic.c +8 -2
@@ -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; }
drivers/usb/core/hcd.c +23 -0
@@ -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); +#endif /* CONFIG_PM */ /*-------------------------------------------------------------------------*/
include/linux/usb/hcd.h +10 -1
@@ -76,6 +76,11 @@ struct giveback_urb_bh {struct usb_host_endpoint *completing_ep; }; +enum hcd_notification_type { +HCD_MSG_DEV_SUSPEND =0 ,/* USB device suspend */ +HCD_MSG_DEV_RESUME ,/* USB device resume */ +}; +struct usb_hcd {/*
@@ -383,7 +388,9 @@ struct hc_driver {int (*find_raw_port_number)(struct usb_hcd *,int );/* Call for power on/off the port if necessary */ int (*port_power)(struct usb_hcd *hcd,int portnum,bool enable); - +/* Call to notify hcd when it is necessary. */ +void (*notify)(struct usb_hcd *,struct usb_device *, +enum hcd_notification_type ,void *); };static inline int hcd_giveback_urb_in_bh (struct usb_hcd *hcd)
@@ -632,6 +639,8 @@ extern void usb_root_hub_lost_power(struct usb_device *rhdev);extern int hcd_bus_suspend (struct usb_device *rhdev,pm_message_t msg);extern int hcd_bus_resume (struct usb_device *rhdev,pm_message_t msg);extern void usb_hcd_resume_root_hub (struct usb_hcd *hcd); +extern void hcd_notify (struct usb_device *udev, +enum hcd_notification_type type,void *data);#else static inline void usb_hcd_resume_root_hub (struct usb_hcd *hcd) {
Signature · 1 line
2.1.0
Quoted 4 lines · Lu Baolu
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.
"Notify" is too generic. It's better to make the callback routine specific to the activity in question. So this patch series should add "device_suspend" and "device_resume" callbacks, not a general "notify" callback.
Quoted 51 lines
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 <[email protected]>--- 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(-)drivers/usb/core/generic.c +5
-1@@ -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; }
drivers/usb/core/generic.c static int generic_resume(struct usb_device *udev, pm_message_t msg) · line 238
Quoted 8 lines
drivers/usb/core/generic.c +3
-1@@ -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); + }
Don't you want to tell the HCD about the resume _before_ it happens? After all, the devices endpoint will be used as soon as the resume takes place.
And besides, this should be symmetrical with the code above, where you tell the HCD about a suspend _after_ it happens.
Alan Stern
Hi Alan,
Thanks for your review comments. Below has my response inline.
Quoted 10 lines · Alan Stern
Quoted 4 lines · Lu Baolu
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."Notify" is too generic. It's better to make the callback routine specific to the activity in question. So this patch series should add "
device_suspend" and "device_resume" callbacks, not a general "notify" callback.
Fair enough. I will make this change in v2 if there is no objection.
Quoted 49 lines
Quoted 49 lines
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 <[email protected]>--- 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(-)drivers/usb/core/generic.c +5
-1@@ -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); + }
Quoted 14 lines
drivers/usb/core/generic.c static int generic_resume(struct usb_device *udev, pm_message_t msg) · line 238
Quoted 11 lines
drivers/usb/core/generic.c +3
-1return 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); + }Don't you want to tell the HCD about the resume _before_ it happens? After all, the devices endpoint will be used as soon as the resume takes place.
The order that software should do during device suspend/resume is defined in 4.15.1.1 of xHCI spec 1.1.
Spec 4.15.1.1:
Software shall stop all endpoints of a device using the Stop Endpoint Command and setting the Suspend (SP) flag to ‘1’ prior to selectively suspending a device. After the device is resumed software shall ring an endpoint’s doorbell to restart it.
--end--
So the order looks like:
tell hcd device suspendusb_port_suspend()
usb_port_resume()
tell hcd device resume
Quoted 3 lines
And besides, this should be symmetrical with the code above, where you tell the HCD about a suspend _after_ it happens.
This was done in above change in generic_suspend():
@@ -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;
}
If usb_port_suspend() returns error, we should roll back HCD to the previous state.
Quoted 2 lines
Alan Stern
Thanks again,
Baolu
Quoted 8 lines · Baolu
The order that software should do during device suspend/resume is defined in 4.15.1.1 of xHCI spec 1.1.
Spec 4.15.1.1:
Software shall stop all endpoints of a device using the Stop Endpoint Command and setting the Suspend
(SP) flag to �1� prior to selectively suspending a device.
But _after_ all the URBs sent to the device have completed, right?
Quoted 3 lines
After the
device is resumed software shall ring an
endpoint�s doorbell to restart it.
The driver would ring the endpoint's doorbell anyway when a new URB is submitted, wouldn't it? Which means the resume callback doesn't actually have to do anything.
Quoted 6 lines
--end--
So the order looks like:
tell hcd device suspend
usb_port_suspend()
You have forgotten that usb_port_suspend() can send URBs to the device (to enable remote wakeup, for example). Therefore you shouldn't notify the HCD until usb_port_suspend() is partly or totally finished.
Quoted 2 lines
usb_port_resume()
tell hcd device resume
You have also forgotten that usb_port_resume() calls various functions that send URBs to ep0 on the device. Therefore if the HCD's device_resume callback needs to do something (like ringing ep0's doorbell), you had better invoke the callback _before_ calling usb_port_resume(). Or maybe you had better do this _within_ usb_port_resume().
Alan Stern
Quoted 11 lines · Alan Stern
Quoted 8 lines · Baolu
The order that software should do during device suspend/resume is defined in 4.15.1.1 of xHCI spec 1.1.
Spec 4.15.1.1:
Software shall stop all endpoints of a device using the Stop Endpoint Command and setting the Suspend
(SP) flag to 1 prior to selectively suspending a device.But _after_ all the URBs sent to the device have completed, right?
Yes, that's right.
Quoted 7 lines
Quoted 4 lines
After the
device is resumed software shall ring an
endpoint's doorbell to restart it.The driver would ring the endpoint's doorbell anyway when a new URB is submitted, wouldn't it? Which means the resume callback doesn't actually have to do anything.
The value of ringing door bell after resume is that hcd can fetch endpoint state from memory to cache and get ready for transfer as early as possible.
Quoted 10 lines
Quoted 7 lines
--end--
So the order looks like:
tell hcd device suspend
usb_port_suspend()You have forgotten that
usb_port_suspend()can send URBs to the device (to enable remote wakeup, for example). Therefore you shouldn't notify the HCD untilusb_port_suspend()is partly or totally finished.
Yes, I agree with you. I will move the notification into usb_port_suspend(), just before sending suspend request to parent hub.
Quoted 9 lines
Quoted 3 lines
usb_port_resume()
tell hcd device resumeYou have also forgotten that
usb_port_resume()calls various functions that send URBs to ep0 on the device. Therefore if the HCD'sdevice_resumecallback needs to do something (like ringing ep0's doorbell), you had better invoke the callback _before_ callingusb_port_resume(). Or maybe you had better do this _within_usb_port_resume().
Agree. I will do this within usb_port_resume() just after completing sending clear port feature.
Quoted 2 lines
Alan Stern
Thank you for the comments. I will send the v2 patch series with all your comments addressed.
Thanks,
Baolu
Quoted 6 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Greg Kroah-Hartman
maintainerQuoted 87 lines · Lu Baolu
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 <[email protected]>--- 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(-)drivers/usb/core/generic.c +8
-2@@ -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; }drivers/usb/core/hcd.c +12
-0@@ -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;
drivers/usb/core/hcd.c void usb_hcd_resume_root_hub (struct usb_hcd *hcd) · line 2313
Quoted 10 lines
drivers/usb/core/hcd.c +10
-0+ +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?
Quoted 91 lines · Greg Kroah-Hartman
Quoted 90 lines · Lu Baolu
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 <[email protected]>--- 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(-)drivers/usb/core/generic.c +8
-2@@ -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; }drivers/usb/core/hcd.c +15
-0@@ -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 ;
Quoted 8 lines
drivers/usb/core/hcd.c void usb_hcd_resume_root_hub (struct usb_hcd *hcd) · line 2313
Quoted 7 lines
drivers/usb/core/hcd.c +7
-0+ + 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.
Quoted 2 lines
RFC PATCH 2/3 usb: xhci: implement hc_driver notify entry
In reply to Lu BaoluThis patch implements the notify entry defined in hc_driver for xHC driver. For HCD_MSG_DEV_SUSPEND message, it will issue stop endpoint command for each endpoint in the device. The Suspend(SP) bit in the command TRB will be set, which gives xHC a hint about the suspend. For HCD_MSG_DEV_RESUME, it will ring doorbells for all endpoints unconditionally. XHC may use these hints to optimize its operation on endpoint state cashes.
--- drivers/usb/host/xhci-hub.c | 2 +- drivers/usb/host/xhci.c | 28 ++++++++++++++++++++++++++++ drivers/usb/host/xhci.h | 3 +++ 3 files changed, 32 insertions(+), 1 deletion(-)
drivers/usb/host/xhci-hub.c +1 -1
@@ -266,7 +266,7 @@ int xhci_find_slot_id_by_port(struct usb_hcd *hcd, struct xhci_hcd *xhci,* to complete. * suspend will set to 1, if suspend bit need to set in command. */ -static int xhci_stop_device (struct xhci_hcd *xhci,int slot_id,int suspend) +int xhci_stop_device (struct xhci_hcd *xhci,int slot_id,int suspend) {struct xhci_virt_device *virt_dev;struct xhci_command *cmd;
drivers/usb/host/xhci.c +28 -0
@@ -4917,6 +4917,29 @@ error: }EXPORT_SYMBOL_GPL (xhci_gen_setup); +void xhci_notify (struct usb_hcd *hcd,struct usb_device *udev, +enum hcd_notification_type type,void *data) +{ +struct xhci_hcd *xhci; + + xhci =hcd_to_xhci (hcd); +if (!xhci || !xhci->devs[udev->slot_id]) +return ; + +switch (type) { +case HCD_MSG_DEV_SUSPEND : +xhci_stop_device (xhci, udev->slot_id,1 ); +break ; +case HCD_MSG_DEV_RESUME : +xhci_ring_device (xhci, udev->slot_id); +break ; +default : +xhci_dbg (xhci,"unknown notification message %d.\n" , type); +break ; + } + +} +static const struct hc_driver xhci_hc_driver = { .description ="xhci-hcd" , .product_desc ="xHCI Host Controller" ,
@@ -4976,6 +4999,11 @@ static const struct hc_driver xhci_hc_driver = { .enable_usb3_lpm_timeout = xhci_enable_usb3_lpm_timeout, .disable_usb3_lpm_timeout = xhci_disable_usb3_lpm_timeout, .find_raw_port_number = xhci_find_raw_port_number, + +/* +* notification call back +*/ + .notify = xhci_notify, };void xhci_init_driver (struct hc_driver *drv,int (*setup_fn)(struct usb_hcd *))
drivers/usb/host/xhci.h +3 -0
@@ -1867,10 +1867,13 @@ u32 xhci_port_state_to_neutral(u32 state);int xhci_find_slot_id_by_port (struct usb_hcd *hcd,struct xhci_hcd *xhci,u16 port);void xhci_ring_device (struct xhci_hcd *xhci,int slot_id); +int xhci_stop_device (struct xhci_hcd *xhci,int slot_id,int suspend);/* xHCI contexts */ struct xhci_input_control_ctx *xhci_get_input_control_ctx (struct xhci_container_ctx *ctx);struct xhci_slot_ctx *xhci_get_slot_ctx (struct xhci_hcd *xhci,struct xhci_container_ctx *ctx);struct xhci_ep_ctx *xhci_get_ep_ctx (struct xhci_hcd *xhci,struct xhci_container_ctx *ctx,unsigned int ep_index); +void xhci_notify (struct usb_hcd *hcd,struct usb_device *udev, +enum hcd_notification_type type,void *data);#endif /* __LINUX_XHCI_HCD_H */
Signature · 1 line
2.1.0
RFC PATCH 3/3 usb: xhci: cleanup unnecessary stop device and ring doorbell operations
In reply to Lu BaoluThere is no need to call xhci_stop_device() and xhci_ring_device() in hub control and bus suspend functions since all device suspend and resume have been notified through the new notify entry in hc_driver.
--- drivers/usb/host/xhci-hub.c | 47 --------------------------------------------- 1 file changed, 47 deletions(-)
drivers/usb/host/xhci-hub.c +0 -47
@@ -704,7 +704,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,u32 temp, status;int retval =0 ;__le32 __iomem **port_array; -int slot_id;struct xhci_bus_state *bus_state;u16 link_state =0 ;u16 wake_mask =0 ;
@@ -818,17 +817,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, goto error; } - slot_id =xhci_find_slot_id_by_port (hcd, xhci, - wIndex +1 ); -if (!slot_id) { -xhci_warn (xhci,"slot_id is zero\n" ); - goto error; - } -/* unlock to execute stop endpoint commands */ -spin_unlock_irqrestore (&xhci->lock, flags); -xhci_stop_device (xhci, slot_id,1 ); -spin_lock_irqsave (&xhci->lock, flags); -xhci_set_link_state (xhci, port_array, wIndex,XDEV_U3 );spin_unlock_irqrestore (&xhci->lock, flags);
@@ -876,19 +864,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, goto error; } -if (link_state ==USB_SS_PORT_LS_U3 ) { - slot_id =xhci_find_slot_id_by_port (hcd, xhci, - wIndex +1 ); -if (slot_id) { -/* unlock to execute stop endpoint -* commands */ -spin_unlock_irqrestore (&xhci->lock, - flags); -xhci_stop_device (xhci, slot_id,1 ); -spin_lock_irqsave (&xhci->lock, flags); - } - } -xhci_set_link_state (xhci, port_array, wIndex, link_state);
@@ -994,14 +969,6 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, XDEV_U0); } bus_state->port_c_suspend |=1 << wIndex; - - slot_id =xhci_find_slot_id_by_port (hcd, xhci, - wIndex +1 ); -if (!slot_id) { -xhci_dbg (xhci,"slot_id is zero\n" ); - goto error; - } -xhci_ring_device (xhci, slot_id);break ;case USB_PORT_FEAT_C_SUSPEND : bus_state->port_c_suspend &= ~(1 << wIndex);
@@ -1133,20 +1100,12 @@ int xhci_bus_suspend(struct usb_hcd *hcd)while (port_index--) {/* suspend the port if the port is not suspended */ u32 t1, t2; -int slot_id; t1 =readl (port_array[port_index]); t2 =xhci_port_state_to_neutral (t1);if ((t1 &PORT_PE ) && !(t1 &PORT_PLS_MASK )) {xhci_dbg (xhci,"port %d not suspended\n" , port_index); - slot_id =xhci_find_slot_id_by_port (hcd, xhci, - port_index +1 ); -if (slot_id) { -spin_unlock_irqrestore (&xhci->lock, flags); -xhci_stop_device (xhci, slot_id,1 ); -spin_lock_irqsave (&xhci->lock, flags); - } t2 &= ~PORT_PLS_MASK ; t2 |=PORT_LINK_STROBE |XDEV_U3 ;set_bit (port_index, &bus_state->bus_suspended);
@@ -1207,7 +1166,6 @@ int xhci_bus_resume(struct usb_hcd *hcd)/* Check whether need resume ports. If needed resume port and disable remote wakeup */ u32 temp; -int slot_id; temp =readl (port_array[port_index]);if (DEV_SUPERSPEED (temp))
@@ -1240,11 +1198,6 @@ int xhci_bus_resume(struct usb_hcd *hcd)/* Clear PLC */ xhci_test_and_clear_bit (xhci, port_array, port_index,PORT_PLC ); - - slot_id =xhci_find_slot_id_by_port (hcd, - xhci, port_index +1 ); -if (slot_id) -xhci_ring_device (xhci, slot_id); }else writel (temp, port_array[port_index]); }
Signature · 1 line
2.1.0