usb: ehci: remember init mode

PATCH New Stephen Warren 2 participants 6 messages Aug 20, 2015 – Aug 27, 2015

Subsystems (2)

USB USB ehci driver

lore

Share & tools

lore

Stephen Warren

author

2015-08-20 23:38 UTC #0 lore raw

PATCH 1/1 usb: ehci: remember init mode

From: Stephen Warren <[email protected]>

When an EHCI device is registered in device mode, the HW isn't actually initialized at all, and hence isn't left in a running state. Consequently, when the device is deregistered, ehci_shutdown() will fail, since the HW bits it expects to see set in response to its shutdown requests will not be sent, and the message "EHCI failed to shut down host controller." will be printed.

Fix ehci-hcd.c to remember whether the device was registered in host or device mode, and only call ehci_shutdown() for host mode registrations.

Signed-off-by Stephen Warren <[email protected]>

---
 drivers/usb/host/ehci-hcd.c | 7 ++++++-
 drivers/usb/host/ehci.h     | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)

drivers/usb/host/ehci-hcd.c +6 -1

index 3a0d32ee2ba1..88b670b7f8a7 100644
@@ -1645,8 +1645,10 @@ int ehci_register(struct udevice *dev, struct ehci_hccr *hccr,
 	ctrl->hcor = hcor;
 	ctrl->priv = ctrl;
 
-	if (init == USB_INIT_DEVICE)
+	ctrl->init = init;
+	if (ctrl->init == USB_INIT_DEVICE)
 		goto done;
+
 	ret = ehci_reset(ctrl);
 	if (ret)
 		goto err;

@@ -1666,6 +1668,9 @@ int ehci_deregister(struct udevice *dev)
 {
 	struct ehci_ctrl *ctrl = dev_get_priv(dev);
 
+	if (ctrl->init == USB_INIT_DEVICE)
+		return 0;
+
 	ehci_shutdown(ctrl);
 
 	return 0;

drivers/usb/host/ehci.h +1 -0

index 3379c293c4df..b41c04a8b30a 100644
@@ -242,6 +242,7 @@ struct ehci_ops {
 };
 
 struct ehci_ctrl {
+	enum usb_init_type init;
 	struct ehci_hccr *hccr;	/* R/O registers, not need for volatile */
 	struct ehci_hcor *hcor;
 	int rootdev;

Signature · 1 line

1.9.1

Marek Vasut

2015-08-21 05:34 UTC #1 lore raw

PATCH 1/1 usb: ehci: remember init mode

Quoted 11 lines · Stephen Warren

From: Stephen Warren <[email protected]>
When an EHCI device is registered in device mode, the HW isn't actually initialized at all, and hence isn't left in a running state. Consequently, when the device is deregistered, ehci_shutdown() will fail, since the HW bits it expects to see set in response to its shutdown requests will not be sent, and the message "EHCI failed to shut down host controller." will be printed.
Fix ehci-hcd.c to remember whether the device was registered in host or device mode, and only call ehci_shutdown() for host mode registrations.

Hi,

Shouldn't you instead check if the controller is running or not and shut it down based on that ?

Best regards,
Marek Vasut

Stephen Warren

author

2015-08-21 21:58 UTC #2 lore raw

PATCH 1/1 usb: ehci: remember init mode

Quoted 10 lines · Marek Vasut

Quoted 9 lines · Stephen Warren
From: Stephen Warren <[email protected]>
When an EHCI device is registered in device mode, the HW isn't actually initialized at all, and hence isn't left in a running state. Consequently, when the device is deregistered, ehci_shutdown() will fail, since the HW bits it expects to see set in response to its shutdown requests will not be sent, and the message "EHCI failed to shut down host controller." will be printed.

Quoted 7 lines

Quoted 2 lines
Fix ehci-hcd.c to remember whether the device was registered in host or device mode, and only call ehci_shutdown() for host mode registrations.
Hi,
Shouldn't you instead check if the controller is running or not and shut it down based on that ?

Why would the two states be different? How can the code check if the controller is running; is there some HW bit that should be checked for that?

Marek Vasut

2015-08-24 03:47 UTC #3 lore raw

PATCH 1/1 usb: ehci: remember init mode

Quoted 14 lines · Stephen Warren

Quoted 13 lines · Marek Vasut
Quoted 11 lines · Stephen Warren
From: Stephen Warren <[email protected]>
When an EHCI device is registered in device mode, the HW isn't actually initialized at all, and hence isn't left in a running state. Consequently, when the device is deregistered, ehci_shutdown() will fail, since the HW bits it expects to see set in response to its shutdown requests will not be sent, and the message "EHCI failed to shut down host controller." will be printed.
Fix ehci-hcd.c to remember whether the device was registered in host or device mode, and only call ehci_shutdown() for host mode registrations.

Quoted 8 lines

Quoted 4 lines
Hi,
Shouldn't you instead check if the controller is running or not and shut it down based on that ?
Why would the two states be different? How can the code check if the controller is running; is there some HW bit that should be checked for that?

Hi,

Hrm, I am not aware of any, sorry :-( But the driver can track whether the controller was previously inited or not, right ? I mean, this is DM, so we should be able to have a private data bit for that, no ?

Does it make sense to track this sort of thing in private data please ?

Best regards,
Marek Vasut

Stephen Warren

author

2015-08-24 18:57 UTC #4 lore raw

PATCH 1/1 usb: ehci: remember init mode

Quoted 24 lines · Marek Vasut

Quoted 14 lines · Stephen Warren
Quoted 13 lines · Marek Vasut
Quoted 11 lines · Stephen Warren
From: Stephen Warren <[email protected]>
When an EHCI device is registered in device mode, the HW isn't actually initialized at all, and hence isn't left in a running state. Consequently, when the device is deregistered, ehci_shutdown() will fail, since the HW bits it expects to see set in response to its shutdown requests will not be sent, and the message "EHCI failed to shut down host controller." will be printed.
Fix ehci-hcd.c to remember whether the device was registered in host or device mode, and only call ehci_shutdown() for host mode registrations.
Quoted 8 lines
Quoted 4 lines
Hi,
Shouldn't you instead check if the controller is running or not and shut it down based on that ?
Why would the two states be different? How can the code check if the controller is running; is there some HW bit that should be checked for that?

Quoted 5 lines

Hi,
Hrm, I am not aware of any, sorry :-( But the driver can track whether the controller was previously inited or not, right ? I mean, this is DM, so we should be able to have a private data bit for that, no ?

I believe that's exactly what this patch is doing. I added a variable to the driver's data structure indicating whether the last initialization call was for host mode (-> EHCI is running, and should be shut down in remove()) or device mode (-> EHCI is not running, so should not be shut down in remove()).

Quoted 5 lines

Does it make sense to track this sort of thing in private data please ?
Best regards,
Marek Vasut

Marek Vasut

2015-08-27 10:45 UTC #5 lore raw

PATCH 1/1 usb: ehci: remember init mode

Quoted 32 lines · Stephen Warren

Quoted 25 lines · Marek Vasut
Quoted 15 lines · Stephen Warren
Quoted 14 lines · Marek Vasut
Quoted 12 lines · Stephen Warren
From: Stephen Warren <[email protected]>
When an EHCI device is registered in device mode, the HW isn't actually initialized at all, and hence isn't left in a running state. Consequently, when the device is deregistered, ehci_shutdown() will fail, since the HW bits it expects to see set in response to its shutdown requests will not be sent, and the message "EHCI failed to shut down host controller." will be printed.
Fix ehci-hcd.c to remember whether the device was registered in host or device mode, and only call ehci_shutdown() for host mode registrations.
Quoted 8 lines
Quoted 4 lines
Hi,
Shouldn't you instead check if the controller is running or not and shut it down based on that ?
Why would the two states be different? How can the code check if the controller is running; is there some HW bit that should be checked for that?
Quoted 5 lines
Hi,
Hrm, I am not aware of any, sorry :-( But the driver can track whether the controller was previously inited or not, right ? I mean, this is DM, so we should be able to have a private data bit for that, no ?

Quoted 5 lines

I believe that's exactly what this patch is doing. I added a variable to the driver's data structure indicating whether the last initialization call was for host mode (-> EHCI is running, and should be shut down in remove()) or device mode (-> EHCI is not running, so should not be shut down in remove()).

Uh, right, sorry. Applied.

Best regards,
Marek Vasut