Date: Sun, 8 Mar 2020 13:39:37 +0100 From: Hans Petter Selasky <hps@selasky.org> To: Shichun.Ma@dell.com, freebsd-usb@freebsd.org Cc: Shunchao.Hu@dell.com Subject: Re: Error bit using of wPortStatus when need usbd_req_warm_reset_port Message-ID: <8b8ae413-29d2-1b0b-1037-cf3b49c1baf7@selasky.org> In-Reply-To: <13174e356c0640ab84fd697a1ae28e55@KULX13MDC130.APAC.DELL.COM> References: <13174e356c0640ab84fd697a1ae28e55@KULX13MDC130.APAC.DELL.COM>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------C50C2C89E61F94EB24D1116F
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
On 2020-03-08 12:15, Shichun.Ma@dell.com wrote:
> Link info need 4 bits and they take bit 5 to bit 9, while UPS_PORT_POWER takes bit 9.
> So this will give wrong link state info in function uhub_suspend_resume_port when it check if need usbd_req_warm_reset_port.
> Please review and kindly suggest how to fix this issue.
Hi,
You are absolutely right. Good catch.
I think we will just remove the POWER bit from the super speed RH. It is
currently not needed for anything. Then the LINK state values won't be
clobbered like you found.
Does the attached patch work for you?
--HPS
--------------C50C2C89E61F94EB24D1116F
Content-Type: text/x-patch; charset=UTF-8;
name="usb_power.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="usb_power.diff"
Index: sys/dev/usb/controller/xhci.c
===================================================================
--- sys/dev/usb/controller/xhci.c (revision 358693)
+++ sys/dev/usb/controller/xhci.c (working copy)
@@ -3591,13 +3591,10 @@
i |= UPS_OVERCURRENT_INDICATOR;
if (v & XHCI_PS_PR)
i |= UPS_RESET;
- if (v & XHCI_PS_PP) {
- /*
- * The USB 3.0 RH is using the
- * USB 2.0's power bit
- */
- i |= UPS_PORT_POWER;
- }
+#if 0
+ if (v & XHCI_PS_PP)
+ /* XXX undefined */
+#endif
USETW(sc->sc_hub_desc.ps.wPortStatus, i);
i = 0;
Index: sys/dev/usb/usb_hub.c
===================================================================
--- sys/dev/usb/usb_hub.c (revision 358693)
+++ sys/dev/usb/usb_hub.c (working copy)
@@ -660,7 +660,7 @@
break;
case USB_SPEED_SUPER:
if (udev->parent_hub == NULL)
- power_mask = UPS_PORT_POWER;
+ power_mask = 0; /* XXX undefined */
else
power_mask = UPS_PORT_POWER_SS;
break;
@@ -668,7 +668,7 @@
power_mask = 0;
break;
}
- if (!(sc->sc_st.port_status & power_mask)) {
+ if ((sc->sc_st.port_status & power_mask) != power_mask) {
DPRINTF("WARNING: strange, connected port %d "
"has no power\n", portno);
}
--------------C50C2C89E61F94EB24D1116F--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?8b8ae413-29d2-1b0b-1037-cf3b49c1baf7>