Attachment 224756 Details for Bug 237666 – usb: uhub return error from uhub_explore

[patch] usb: uhub return error from uhub_explore

20210507-01-usb_hub-err.diff (text/plain), 5.05 KB, created by Bjoern A. Zeeb on 2021-05-07 22:11:38 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Bjoern A. Zeeb

Created: 2021-05-07 22:11:38 UTC

Size: 5.05 KB

patch

obsolete

>!
>! usb: uhub return error from uhub_explore
>!
>! uhub_explore would always return no-error even if
>! and error occured in the loop over all ports.
>! The loop properly terminates in case of error
>! (or continues to the next port in which case the error will
>! be reset).
>! Propagate the possible error to the caller.
>! This seems to fix the continous
>!   "uhub_reattach_port: giving up port reset - device vanished"
>! messages people are seeing (in theory and based on my own testing).
>! XXX-BZ ^^^^^^^^^^^^^^^^^^^^ remove for final commit message.
>!
>! PR:		237666
>! Submitted-by:	phk
>! Tested-by:	bz, ...
>! Reviewed-by:	...
>! Diff-Re..:
>!
>diff --git sys/dev/usb/usb_hub.c sys/dev/usb/usb_hub.c
>index 6ed30b64b1e..2e2f153bbe5 100644
>--- sys/dev/usb/usb_hub.c
>+++ sys/dev/usb/usb_hub.c
>@@ -980,175 +980,176 @@ uhub_is_too_deep(struct usb_device *udev)
> 		break;
> 	}
> 	return (0);
> }
> 
> /*------------------------------------------------------------------------*
>  *	uhub_explore
>  *
>  * Returns:
>  *     0: Success
>  *  Else: Failure
>  *------------------------------------------------------------------------*/
> static usb_error_t
> uhub_explore(struct usb_device *udev)
> {
> 	struct usb_hub *hub;
> 	struct uhub_softc *sc;
> 	struct usb_port *up;
> 	usb_error_t err;
> 	uint8_t portno;
> 	uint8_t x;
> 	uint8_t do_unlock;
> 
> 	hub = udev->hub;
> 	sc = hub->hubsoftc;
> 
> 	DPRINTFN(11, "udev=%p addr=%d\n", udev, udev->address);
> 
> 	/* ignore devices that are too deep */
> 	if (uhub_is_too_deep(udev))
> 		return (USB_ERR_TOO_DEEP);
> 
> 	/* check if device is suspended */
> 	if (udev->flags.self_suspended) {
> 		/* need to wait until the child signals resume */
> 		DPRINTF("Device is suspended!\n");
> 		return (0);
> 	}
> 
> 	/*
> 	 * Make sure we don't race against user-space applications
> 	 * like LibUSB:
> 	 */
> 	do_unlock = usbd_enum_lock(udev);
> 
>+	err = USB_ERR_NORMAL_COMPLETION;
> 	for (x = 0; x != hub->nports; x++) {
> 		up = hub->ports + x;
> 		portno = x + 1;
> 
> 		err = uhub_read_port_status(sc, portno);
> 		if (err) {
> 			/* most likely the HUB is gone */
> 			break;
> 		}
> 		if (sc->sc_st.port_change & UPS_C_OVERCURRENT_INDICATOR) {
> 			DPRINTF("Overcurrent on port %u.\n", portno);
> 			err = usbd_req_clear_port_feature(
> 			    udev, NULL, portno, UHF_C_PORT_OVER_CURRENT);
> 			if (err) {
> 				/* most likely the HUB is gone */
> 				break;
> 			}
> 		}
> 		if (!(sc->sc_flags & UHUB_FLAG_DID_EXPLORE)) {
> 			/*
> 			 * Fake a connect status change so that the
> 			 * status gets checked initially!
> 			 */
> 			sc->sc_st.port_change |=
> 			    UPS_C_CONNECT_STATUS;
> 		}
> 		if (sc->sc_st.port_change & UPS_C_PORT_ENABLED) {
> 			err = usbd_req_clear_port_feature(
> 			    udev, NULL, portno, UHF_C_PORT_ENABLE);
> 			if (err) {
> 				/* most likely the HUB is gone */
> 				break;
> 			}
> 			if (sc->sc_st.port_change & UPS_C_CONNECT_STATUS) {
> 				/*
> 				 * Ignore the port error if the device
> 				 * has vanished !
> 				 */
> 			} else if (sc->sc_st.port_status & UPS_PORT_ENABLED) {
> 				DPRINTFN(0, "illegal enable change, "
> 				    "port %d\n", portno);
> 			} else {
> 				if (up->restartcnt == USB_RESTART_MAX) {
> 					/* XXX could try another speed ? */
> 					DPRINTFN(0, "port error, giving up "
> 					    "port %d\n", portno);
> 				} else {
> 					sc->sc_st.port_change |=
> 					    UPS_C_CONNECT_STATUS;
> 					up->restartcnt++;
> 				}
> 			}
> 		}
> 		if (sc->sc_st.port_change & UPS_C_CONNECT_STATUS) {
> 			err = uhub_reattach_port(sc, portno);
> 			if (err) {
> 				/* most likely the HUB is gone */
> 				break;
> 			}
> 		}
> 		if (sc->sc_st.port_change & (UPS_C_SUSPEND |
> 		    UPS_C_PORT_LINK_STATE)) {
> 			err = uhub_suspend_resume_port(sc, portno);
> 			if (err) {
> 				/* most likely the HUB is gone */
> 				break;
> 			}
> 		}
> 		err = uhub_explore_sub(sc, up);
> 		if (err) {
> 			/* no device(s) present */
> 			continue;
> 		}
> 		/* explore succeeded - reset restart counter */
> 		up->restartcnt = 0;
> 	}
> 
> 	if (do_unlock)
> 		usbd_enum_unlock(udev);
> 
> 	/* initial status checked */
> 	sc->sc_flags |= UHUB_FLAG_DID_EXPLORE;
> 
> 	/* return success */
>-	return (USB_ERR_NORMAL_COMPLETION);
>+	return (err);
> }
> 
> int
> uhub_probe(device_t dev)
> {
> 	struct usb_attach_arg *uaa = device_get_ivars(dev);
> 
> 	if (uaa->usb_mode != USB_MODE_HOST)
> 		return (ENXIO);
> 
> 	/*
> 	 * The subclass for USB HUBs is currently ignored because it
> 	 * is 0 for some and 1 for others.
> 	 */
> 	if (uaa->info.bConfigIndex == 0 &&
> 	    uaa->info.bDeviceClass == UDCLASS_HUB)
> 		return (BUS_PROBE_DEFAULT);
> 
> 	return (ENXIO);
> }
> 
> /* NOTE: The information returned by this function can be wrong. */
> usb_error_t
> uhub_query_info(struct usb_device *udev, uint8_t *pnports, uint8_t *ptt)
> {
> 	struct usb_hub_descriptor hubdesc20;
> 	struct usb_hub_ss_descriptor hubdesc30;
> 	usb_error_t err;
> 	uint8_t nports;
> 	uint8_t tt;
> 
> 	if (udev->ddesc.bDeviceClass != UDCLASS_HUB)
> 		return (USB_ERR_INVAL);
> 
> 	nports = 0;
> 	tt = 0;
> 
> 	switch (udev->speed) {
> 	case USB_SPEED_LOW:
> 	case USB_SPEED_FULL:
> 	case USB_SPEED_HIGH:
> 		/* assuming that there is one port */
> 		err = usbd_req_get_hub_descriptor(udev, NULL, &hubdesc20, 1);
> 		if (err) {
> 			DPRINTFN(0, "getting USB 2.0 HUB descriptor failed,"

Actions: View | Diff

Attachments on bug 237666: 204107 | 204110 | 204111 | 204382 | 204388 | 206324 | 206399 | 206443 | 209346 | 209793 | 210170 | 213055 | 213822 | 220423 | 224756 | 242272