Date: Tue, 31 Dec 2019 11:53:55 +0100 From: Hans Petter Selasky <hps@selasky.org> To: Shichun.Ma@dell.com, freebsd-usb@freebsd.org Cc: Shunchao.Hu@dell.com Subject: Re: can not receive xfer interrupt after stop xfer is called intel XHCI Gemini Lake SOC Message-ID: <df8e2391-58a1-969f-d2ec-7101df6764aa@selasky.org> In-Reply-To: <caff3f2cab964507b12df3841d443000@KULX13MDC130.APAC.DELL.COM> References: <1577408331523.24347@Dell.com> <acc5ccc7-d76c-d9ff-f9d5-f63ac40227d6@selasky.org> <1577411424906.21267@Dell.com> <db23ff3a-df66-e060-4409-5eccb214d3cf@selasky.org> <caff3f2cab964507b12df3841d443000@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.
--------------F7341B54021F63A4FCBB01F8
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
On 2019-12-31 01:23, Shichun.Ma@dell.com wrote:
> Dell Customer Communication - Confidential
>
> Hi HPS,
>
> It's already in stopped status. I am also confusing on the root cause.
> I attached the test application and my patch for the xhci stop xfer workaround solution.
> The confusing points:
> 1. cancel xfer error can be reproduced on all CCID smart readers (I have tested three different model of readers);
> 2. keyboard has similar endpoint attribution, while I can't reproduce similar problem on the keyboard;
>
Hi,
I suspect it is the newer XHCI hardware which has some additional
checks. Can you verify if the XHCI controller in your computer accept
multiple configure_ep() commands? The state diagram in the XHCI
specification does not say you cannot do this, but I imagine this might
be causing it.
The XHCI driver in FreeBSD configure one and one endpoint and not all at
the same time.
Can you try the attached patch instead of yours?
--HPS
--------------F7341B54021F63A4FCBB01F8
Content-Type: text/x-patch; charset=UTF-8;
name="xhci.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="xhci.diff"
Index: sys/dev/usb/controller/xhci.c
===================================================================
--- sys/dev/usb/controller/xhci.c (revision 356220)
+++ sys/dev/usb/controller/xhci.c (working copy)
@@ -3838,6 +3838,7 @@
struct usb_page_cache *pcinp;
usb_error_t err;
usb_stream_t stream_id;
+ uint32_t mask;
uint8_t index;
uint8_t epno;
@@ -3903,16 +3904,20 @@
* endpoint context state diagram in the XHCI specification:
*/
- xhci_configure_mask(udev, (1U << epno) | 1U, 0);
+ mask = (1U << epno);
+ xhci_configure_mask(udev, mask | 1U, 0);
- if (epno > 1)
+ if (!(sc->sc_hw.devs[index].ep_configured & mask)) {
+ sc->sc_hw.devs[index].ep_configured |= mask;
err = xhci_cmd_configure_ep(sc, buf_inp.physaddr, 0, index);
- else
+ } else {
err = xhci_cmd_evaluate_ctx(sc, buf_inp.physaddr, index);
+ }
- if (err != 0)
- DPRINTF("Could not configure endpoint %u\n", epno);
-
+ if (err != 0) {
+ DPRINTF("Could not configure "
+ "endpoint %u at slot %u.\n", epno, index);
+ }
XHCI_CMD_UNLOCK(sc);
return (0);
@@ -4273,6 +4278,7 @@
/* set default state */
sc->sc_hw.devs[index].state = XHCI_ST_DEFAULT;
+ sc->sc_hw.devs[index].ep_configured = 3U;
/* reset number of contexts */
sc->sc_hw.devs[index].context_num = 0;
@@ -4290,6 +4296,7 @@
break;
sc->sc_hw.devs[index].state = XHCI_ST_ADDRESSED;
+ sc->sc_hw.devs[index].ep_configured = 3U;
/* set configure mask to slot only */
xhci_configure_mask(udev, 1, 0);
@@ -4304,11 +4311,19 @@
break;
case USB_STATE_CONFIGURED:
- if (sc->sc_hw.devs[index].state == XHCI_ST_CONFIGURED)
- break;
+ if (sc->sc_hw.devs[index].state == XHCI_ST_CONFIGURED) {
+ /* deconfigure all endpoints, except EP0 */
+ err = xhci_cmd_configure_ep(sc, 0, 1, index);
+ if (err) {
+ DPRINTF("Failed to deconfigure "
+ "slot %u.\n", index);
+ }
+ }
+
/* set configured state */
sc->sc_hw.devs[index].state = XHCI_ST_CONFIGURED;
+ sc->sc_hw.devs[index].ep_configured = 3U;
/* reset number of contexts */
sc->sc_hw.devs[index].context_num = 0;
Index: sys/dev/usb/controller/xhci.h
===================================================================
--- sys/dev/usb/controller/xhci.h (revision 356220)
+++ sys/dev/usb/controller/xhci.h (working copy)
@@ -408,6 +408,8 @@
struct xhci_endpoint_ext endp[XHCI_MAX_ENDPOINTS];
+ uint32_t ep_configured;
+
uint8_t state;
uint8_t nports;
uint8_t tt;
--------------F7341B54021F63A4FCBB01F8--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?df8e2391-58a1-969f-d2ec-7101df6764aa>