Re: [PATCH] usb: hub: fix state change check for not powercycled ports

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

 



Hi!

On Thu, Feb 11, 2021 at 01:03:54PM +0100, Ahmad Fatoum wrote:
Hello,

On 18.01.21 20:09, Michael Grzeschik wrote:
Since we don't power cycle the ports on start since patch "19bb0b2a usb:
hub: Do not power-cycle usb devices on init" it is possible that the
device on this port is already active from a previous enumeration. This
way barebox will never get any change in USB_PORT_STAT_C_CONNECTION bit
change.

Although the device will probably still work fine after the following
port reset, the current code will always miss reenumerating these still
plugged devices. This patch fixes this by ignoring the check for
STAT_C_CONNECTION bit and only go for USB_PORT_STAT_CONNECTION which
should be enough.

This breaks USB enumeration for me on an i.MX6Q. The USB flash driver behind
the hub is detected twice, which barebox doesn't like at all:

barebox@Embest MarS Board i.MX6Dual:/ usb -s
usb: USB: scanning bus for devices...
usb1: Bus 001 Device 001: ID 0000:0000 EHCI Host Controller
usb1-0: Bus 001 Device 002: ID 1a40:0101 USB 2.0 Hub [MTT]
usb1-0-1: Bus 001 Device 003: ID 058f:6387 Mass Storage
Using index 0 for the new disk
usb1-0-1: Bus 001 Device 004: ID 058f:6387 Mass Storage
ERROR: register_device: already registered usb1-0-1
ERROR: usb1-0-1: Failed to register device: Invalid argument
usb: 3 USB Device(s) found
Bus 001 Device 001: ID 0000:0000 EHCI Host Controller
Bus 001 Device 002: ID 1a40:0101 USB 2.0 Hub [MTT]
Bus 001 Device 003: ID 058f:6387 Mass Storage

This seems to corrupt some internal barebox state as reading
/dev/disk0.0 will hang. Reading /dev/disk0 is fine however.

With this patch reverted it however works:

usb: USB: scanning bus for devices...
usb1: Bus 001 Device 001: ID 0000:0000 EHCI Host Controller
usb1-0: Bus 001 Device 002: ID 1a40:0101 USB 2.0 Hub [MTT]
usb1-0-1: Bus 001 Device 003: ID 058f:6387 Mass Storage
Using index 0 for the new disk
usb: 3 USB Device(s) found
Bus 001 Device 001: ID 0000:0000 EHCI Host Controller
Bus 001 Device 002: ID 1a40:0101 USB 2.0 Hub [MTT]
Bus 001 Device 003: ID 058f:6387 Mass Storage

The USB drive is not self-powered. It doesn't make a difference
whether it's done directly after cold reset or:
 usb -s
 boot bnet
 usb -s

Cheers,
Ahmad

I have reproduced that. My latest findings are that, when I debugged
this, usb core on my board I was debugging was behaving uncommon. It was
not setting the USB_PORT_STAT_C_CONNECTION in the first place. But the
further patches I send did solve this.

So this whole issue was due to an uncorrect configured over-current
polarity setup. With my other patches applied I see this bit
USB_PORT_STAT_C_CONNECTION set even without the power-cycle.

My suggestion is to _revert_ this patch, as it is solving a
bogus problem not related to the whole usb-core state machine.

Thanks for testing!
Michael


Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
---
 drivers/usb/core/hub.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 01653d8c20..d1112248ee 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -336,8 +336,7 @@ static void usb_scan_port(struct usb_device_scan *usb_scan)
 	dev_dbg(&dev->dev, "port%d: Status 0x%04x Change 0x%04x\n",
 			port + 1, portstatus, portchange);

-	if (!(portchange & USB_PORT_STAT_C_CONNECTION) ||
-	    !(portstatus & USB_PORT_STAT_CONNECTION)) {
+	if (!(portstatus & USB_PORT_STAT_CONNECTION)) {
 		if (get_time_ns() >= hub->connect_timeout) {
 			dev_dbg(&dev->dev, "port%d: timeout\n", port + 1);
 			/* Remove this device from scanning list */


--
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Attachment: signature.asc
Description: PGP signature

_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox

[Index of Archives]     [Linux Embedded]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux