On 2017/07/24 14:35, Martin Pieuchot wrote:
> On 21/07/17(Fri) 07:57, sc dying wrote:
>> On 2017/07/20 08:31, Martin Pieuchot wrote:
>>> On 18/07/17(Tue) 13:43, sc dying wrote:
>>>> On 2017/07/18 09:12, Martin Pieuchot wrote:
>>>>> On 17/07/17(Mon) 15:24, sc dying wrote:
>>>>>> On 2017/07/17 08:24, Martin Pieuchot wrote:
>>>>>>> On 15/07/17(Sat) 21:16, sc dying wrote:
>>>>>>>> - Call usbd_set_config before configuring endpoints in ure_init to fix
>>>>>>>>    an error when re-opening pipes.  I grabbed the code from if_kue.c.
>>>>>>>
>>>>>>> Which error? Calling usbd_set_config() should be avoid as much as
>>>>>>> possible in driver code.
>>>>>>
>>>>>> Without patch, ure on usb-3 port says this error
>>>>>>
>>>>>> ure0: usb errors on rx: IOERROR
>>>>>>
>>>>>> when down the interface and it up.
>>>>>
>>>>> This is certainly a bug in the way pipe are closed, or more likely in
>>>>> xhci(4).  Can you reproduce the problem on ehci(4)?  Do you find
>>>>> anything useful if you define XHCI_DEBUG in your kernel?
>>>>
>>>> This problem is not seen on ehci.
>>>>
>>>> On xhci with XHCI_DEBUG, curiously, it does not happen.
>>>> I'll look into this.
>>>
>>> Great, I committed your diff without this chunk.  I'd be glad to hear
>>> from you if you find the problem with xhci(4).
>>
>> Thank you for applying the patch.
>>
>> About IOERROR, it happens even if XHCI_DEBUG is defined.
>> It happens on usb 3 mode port, but does not on usb 2
>> even if it is xhci's port.
>>
>> All I did for test are
>> 1) Plug-in the device
>> 2) ifconfig ure0 inet6 fe80::1
>> 3) ping6 from other host to fe80::1
>> 4) ifconfig ure0 down
>> 5) ifconfig ure0 up
>> 6) ping6 again
>>
>> I forgot step 3) to check if the device replies to ping.
>>
>> If the device does not receive any packets, it can be re-up
>> without errors and works correctly after interface down and up.
>> However, once it receives a packet, re-opening bulk-in pipe for RX
>> immediately fails with TXERR (event_xfer translates it to IOERROR).
>>
>> xhci 4.6.6 says closing all pipes transitions the device state
>> from Configured to Addressed.
>> I thought it is needed to issue configure_ep and SET_CONFIG to transition
>> the device to Configured state correcly again as described in 4.6.6,
>> but this does not explain why the device works on usb 2.
>
> Here's a diff to try and play with.  I'd guess the problem is in
> pipe_close().  Which state has the EP you're closing?  Do we close
> the EP correctly?

EP 3 is RX pipe, EP 4 is TX pipe.
Looks good to me.

# ifconfig ure0 inet6 fe80::1
xhci_pipe_init: pipe=0xffff8000003c6000 addr=2 depth=1 port=1 speed=4
dev 1 dci 3 (epAddr=0x81)
xhci1: xhci_cmd_configure_ep dev 1
xhci_pipe_init: pipe=0xffff8000003c7000 addr=2 depth=1 port=1 speed=4
dev 1 dci 4 (epAddr=0x2)
xhci1: xhci_cmd_configure_ep dev 1
# ifconfig ure0 down
xhci_abort_xfer: xfer=0xffffff011e4a21e0 status=IN_PROGRESS
err=CANCELLED actlen=0 len=16384 idx=4
xhci1: xhci_cmd_stop_ep dev 1 dci 3
xhci_event_xfer: stopped xfer=0xffffff011e4a21e0
xhci1: xhci_cmd_set_tr_deq_async dev 1 dci 3
xhci1: EP 3 state=3
xhci1: xhci_cmd_configure_ep dev 1
xhci1: EP 4 state=1
xhci1: xhci_cmd_stop_ep dev 1 dci 4
xhci1: NULL xfer pointer
xhci1: xhci_cmd_configure_ep dev 1
# ifconfig ure0 up
xhci_pipe_init: pipe=0xffff8000003c6000 addr=2 depth=1 port=1 speed=4
dev 1 dci 3 (epAddr=0x81)
xhci1: xhci_cmd_configure_ep dev 1
xhci_pipe_init: pipe=0xffff8000003c7000 addr=2 depth=1 port=1 speed=4
dev 1 dci 4 (epAddr=0x2)
xhci1: xhci_cmd_configure_ep dev 1
ure0: usb errors on rx: IOERROR
# ifconfig ure0 down
xhci_abort_xfer: xfer=0xffffff011e4a22d0 status=NOT_STARTED
err=CANCELLED actlen=0 len=16384 idx=-1
xhci1: EP 3 state=2
xhci1: xhci_cmd_stop_ep dev 1 dci 3
xhci1: event error code=19, result=33
trb=0xffff8000221ae990 (0x00000000dc3e70c0 0x13000000 0x1008400)
xhci1: error stopping ep (3)
xhci1: xhci_cmd_configure_ep dev 1
xhci1: EP 4 state=1
xhci1: xhci_cmd_stop_ep dev 1 dci 4
xhci1: NULL xfer pointer
xhci1: xhci_cmd_configure_ep dev 1


>
> Index: xhci.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/xhci.c,v
> retrieving revision 1.73
> diff -u -p -r1.73 xhci.c
> --- xhci.c 22 Jun 2017 02:44:37 -0000 1.73
> +++ xhci.c 24 Jul 2017 14:32:40 -0000
> @@ -1251,13 +1251,22 @@ xhci_pipe_close(struct usbd_pipe *pipe)
>   struct xhci_softc *sc = (struct xhci_softc *)pipe->device->bus;
>   struct xhci_pipe *lxp, *xp = (struct xhci_pipe *)pipe;
>   struct xhci_soft_dev *sdev = &sc->sc_sdevs[xp->slot];
> + uint32_t state;
>   int i;
>
>   /* Root Hub */
>   if (pipe->device->depth == 0)
>   return;
>
> - /* Mask the endpoint */
> + /* Stop the endpoint */
> + bus_dmamap_sync(sdev->ictx_dma.tag, sdev->ictx_dma.map, 0,
> +    sc->sc_pagesize, BUS_DMASYNC_POSTREAD);
> + state = letoh32(XHCI_EPCTX_STATE(sdev->ep_ctx[xp->dci - 1]->info_lo));

Do you wanna see output context (device context, 6.2.1)?

bus_dmamap_sync(sdev->octx_dma.tag, sdev->octx_dma.map, 0,
    sc->sc_pagesize, BUS_DMASYNC_POSTREAD);
state = XHCI_EPCTX_STATE(letoh32(((struct xhci_epctx *)
    (sdev->octx_dma.vaddr + sc->sc_ctxsize))[xp->dci - 1].info_lo));

> + printf("%s: EP %d state=%x\n", DEVNAME(sc), xp->dci, state);
> + if (state != XHCI_EP_STOPPED && xhci_cmd_stop_ep(sc, xp->slot, xp->dci))
> + DPRINTF(("%s: error stopping ep (%d)\n", DEVNAME(sc), xp->dci));
> +
> + /* Mask it */
>   sdev->input_ctx->drop_flags = htole32(XHCI_INCTX_MASK_DCI(xp->dci));
>   sdev->input_ctx->add_flags = 0;
>
>

Reply via email to