Date: Tue, 31 Dec 2019 00:23:16 +0000 From: <Shichun.Ma@dell.com> To: <hps@selasky.org>, <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: <caff3f2cab964507b12df3841d443000@KULX13MDC130.APAC.DELL.COM> In-Reply-To: <db23ff3a-df66-e060-4409-5eccb214d3cf@selasky.org> References: <1577408331523.24347@Dell.com> <acc5ccc7-d76c-d9ff-f9d5-f63ac40227d6@selasky.org> <1577411424906.21267@Dell.com> <db23ff3a-df66-e060-4409-5eccb214d3cf@selasky.org>
index | next in thread | previous in thread | raw e-mail
[-- Attachment #1 --]
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;
Do xfer and cancel xfer can be called umagic_sysctl_doxfer/umagic_sysctl_cancelxfer in umagic.c
Regards,
Horse Ma (Shichun Ma)
Software Engineer
Dell | Cloud client-computing - Wyse
office +86 10 82862579, Mobile +86 13241851528
See our products at www.dell.com/wyse
-----Original Message-----
From: Hans Petter Selasky <hps@selasky.org>
Sent: Friday, December 27, 2019 4:28 PM
To: Ma, Horse; freebsd-usb@freebsd.org
Cc: Hu, Shunchao
Subject: Re: can not receive xfer interrupt after stop xfer is called intel XHCI Gemini Lake SOC
[EXTERNAL EMAIL]
On 2019-12-27 02:50, Shichun.Ma@dell.com wrote:
> Hi HPS,
> XHCI spec just tells:
> The configure Endpoint command TRB evalutes the bandwidth and resouce reqirement of the endpoints selected by the command.
> So I don't think this command can be call to one endpoint several times.
Hi,
When you have alternate interface settings for the same endpoint, how can you then update the max packet size, if the endpoint can't be re-configured?
Are we missing some XHCI command before configure, like stop endpoint?
> How, I tried to call configure endpoint with DC (disable configure), it can't help.
>
>>From XHCI spec's view, "reset Endpoint command" is enough to stop xfer.
>
In this piece of code, does it help to stop the endpoint before you configure?
> /* configure endpoint */
>
> err = xhci_configure_endpoint_by_xfer(xfer);
>
> if (err != 0) {
> XHCI_CMD_UNLOCK(sc);
> return (err);
> }
>
> /*
> * Get the endpoint into the stopped state according to the
> * endpoint context state diagram in the XHCI specification:
> */
>
> err = xhci_cmd_stop_ep(sc, 0, epno, index);
>
> if (err != 0)
> DPRINTF("Could not stop endpoint %u\n", epno);
Thanks for notifying us about this issue.
--HPS
[-- Attachment #2 --]
/*
* DELL PROPRIETARY INFORMATION
*
* This software is confidential. Dell Inc., or one of its subsidiaries, has
* supplied this software to you under the terms of a license agreement,
* nondisclosure agreement or both. You may not copy, disclose, or use this
* software except in accordance with those terms.
*
* Copyright 2019 Dell Inc. or its subsidiaries. All Rights Reserved.
*
* DELL INC. MAKES NO REPRESENTATIONS OR WARRANTIES
* ABOUT THE SUITABILITY OF THE SOFTWARE, EITHER EXPRESS
* OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE IMPLIED
* WARRANTIES OF MERCHANTABILITY, FITNESS FOR A
* PARTICULAR PURPOSE, OR NON-INFRINGEMENT. DELL SHALL
* NOT BE LIABLE FOR ANY DAMAGES SUFFERED BY LICENSEE
* AS A RESULT OF USING, MODIFYING OR DISTRIBUTING
* THIS SOFTWARE OR ITS DERIVATIVES.
*
*/
#include <sys/cdefs.h>
#include <sys/stdint.h>
#include <sys/stddef.h>
#include <sys/param.h>
#include <sys/queue.h>
#include <sys/types.h>
#include <sys/systm.h>
#include <sys/kernel.h>
#include <sys/bus.h>
#include <sys/module.h>
#include <sys/lock.h>
#include <sys/mutex.h>
#include <sys/condvar.h>
#include <sys/sysctl.h>
#include <sys/sx.h>
#include <sys/unistd.h>
#include <sys/callout.h>
#include <sys/malloc.h>
#include <sys/priv.h>
#include <sys/conf.h>
#include <dev/usb/usb.h>
#include <dev/usb/usbdi.h>
#include <dev/usb/usbdi_util.h>
#include <dev/usb/usb_ioctl.h>
#include <dev/usb/usb_dev.h>
#include <dev/usb/usb_mbuf.h>
#include "usbdevs.h"
#include <dev/usb/usb_core.h>
#include <dev/usb/usb_debug.h>
#include <dev/usb/usb_process.h>
#include <dev/usb/usb_device.h>
#include <dev/usb/usbfs.h>
#include <dev/usb/usb_busdma.h>
#include <dev/usb/usb_transfer.h>
#include <dev/usb/usb_request.h>
#include <dev/usb/usb_dynamic.h>
#include <dev/usb/usb_hub.h>
#include <dev/usb/usb_util.h>
#include <dev/usb/usb_msctest.h>
#include <dev/usb/quirk/usb_quirk.h>
#include <dev/usb/usb_controller.h>
#include <dev/usb/usb_bus.h>
#define umagic_INNER 0
#define umagic_OUTER 1
#define umagic_MAX_SENSORS 2
#define umagic_CMD_DATA 0x80
#define umagic_CMD_INIT 0x82
struct umagic_softc;
enum {
UMAGIC_INTR_DT,
UMAGIC_N_TRANSFER,
};
struct umagic_softc {
struct usb_device *sc_udev;
struct usb_xfer *sc_xfer[UMAGIC_N_TRANSFER];
struct mtx sc_mtx;
uint8_t sc_iface_index[2];
};
/* prototypes */
static device_probe_t umagic_probe;
static device_attach_t umagic_attach;
static device_detach_t umagic_detach;
static usb_callback_t umagic_intr_callback;
static devclass_t umagic_devclass;
static device_method_t umagic_methods[] = {
DEVMETHOD(device_probe, umagic_probe),
DEVMETHOD(device_attach, umagic_attach),
DEVMETHOD(device_detach, umagic_detach),
DEVMETHOD_END
};
static driver_t umagic_driver = {
.name = "umagic",
.methods = umagic_methods,
.size = sizeof(struct umagic_softc),
};
static const STRUCT_USB_HOST_ID umagic_devs[] = {
{USB_VPI(0x8E6, 0x3437, 0)}, //IDBridge CT30
//{USB_VPI(0x4f2, 0x112, 0)}, //wyse keyboard
};
DRIVER_MODULE(umagic, uhub, umagic_driver, umagic_devclass, NULL, NULL);
MODULE_DEPEND(umagic, usb, 1, 1, 1);
MODULE_VERSION(umagic, 1);
USB_PNP_HOST_INFO(umagic_devs);
static const struct usb_config umagic_config[UMAGIC_N_TRANSFER] = {
[UMAGIC_INTR_DT] = {
.type = UE_INTERRUPT,
.endpoint = UE_ADDR_ANY,
.direction = UE_DIR_IN,
.flags = {.pipe_bof = 1, .short_xfer_ok = 1,},
.bufsize = 0, /* use wMaxPacketSize */
.callback = &umagic_intr_callback,
.if_index = 1,
},
};
static int
umagic_probe(device_t dev)
{
struct usb_attach_arg *uaa;
uaa = device_get_ivars(dev);
return usbd_lookup_id_by_uaa(umagic_devs, sizeof(umagic_devs), uaa);
}
static int
umagic_sysctl_cancelxfer(SYSCTL_HANDLER_ARGS)
{
device_t dev;
struct umagic_softc *sc;
int error, hsmode;
struct usb_xfer *xfer;
dev = oidp->oid_arg1;
sc = device_get_softc(dev);
if (sc == NULL)
return (EINVAL);
error = sysctl_handle_int(oidp, &hsmode, 0, req);
if (error != 0 || req->newptr == NULL) {
//KLOGN(0, "error %d\n", error);
return error;
}
KLOGN(KDBG, "hsmode %d\n", hsmode);
if (hsmode != 3)
return 0;
xfer = sc->sc_xfer[UMAGIC_INTR_DT];
KLOGN(KDBG, "++stop xfer %p\n", xfer);
USB_XFER_LOCK(xfer);
usbd_transfer_stop(xfer);
USB_XFER_UNLOCK(xfer);
KLOGN(KDBG, "--stop xfer %p\n", xfer);
return 0;
}
static int
umagic_sysctl_doxfer(SYSCTL_HANDLER_ARGS)
{
device_t dev;
struct umagic_softc *sc;
int error, hsmode;
struct usb_xfer *xfer;
dev = oidp->oid_arg1;
sc = device_get_softc(dev);
if (sc == NULL)
return (EINVAL);
error = sysctl_handle_int(oidp, &hsmode, 0, req);
if (error != 0 || req->newptr == NULL) {
//KLOGN(0, "error %d\n", error);
return error;
}
KLOGN(KDBG, "hsmode %d\n", hsmode);
if (hsmode != 3)
return 0;
xfer = sc->sc_xfer[UMAGIC_INTR_DT];
KLOGN(KDBG, "++do xfer %p\n", xfer);
USB_XFER_LOCK(xfer);
usbd_transfer_start(xfer);
USB_XFER_UNLOCK(xfer);
KLOGN(KDBG, "--do xfer %p\n", xfer);
return 0;
}
static int
umagic_attach(device_t dev)
{
struct umagic_softc *sc = device_get_softc(dev);
struct usb_attach_arg *uaa = device_get_ivars(dev);
int error;
int i;
struct sysctl_oid *oid_node;
sc->sc_udev = uaa->device;
sc->sc_iface_index[0] = uaa->info.bIfaceIndex;
device_set_usb_desc(dev);
mtx_init(&sc->sc_mtx, "umagic lock", NULL, MTX_DEF | MTX_RECURSE);
oid_node = SYSCTL_ADD_NODE(device_get_sysctl_ctx(dev),
SYSCTL_CHILDREN(device_get_sysctl_tree(dev)), OID_AUTO, "sensors",
CTLFLAG_RD, NULL, "");
if (oid_node == NULL) {
KLOGN(0, "\n");
error = ENOMEM;
goto detach;
}
SYSCTL_ADD_PROC(device_get_sysctl_ctx(dev), SYSCTL_CHILDREN(oid_node),
OID_AUTO, "cancelxfer", CTLTYPE_INT | CTLFLAG_RW | CTLFLAG_ANYBODY,
dev, 0, umagic_sysctl_cancelxfer, "I", "cancel");
SYSCTL_ADD_PROC(device_get_sysctl_ctx(dev), SYSCTL_CHILDREN(oid_node),
OID_AUTO, "doxfer", CTLTYPE_INT | CTLFLAG_RW | CTLFLAG_ANYBODY,
dev, 0, umagic_sysctl_doxfer, "I", "doxfer");
/* grab all interfaces from other drivers */
for (i = 0;; i++) {
if (i == uaa->info.bIfaceIndex)
continue;
if (usbd_get_iface(uaa->device, i) == NULL)
break;
usbd_set_parent_iface(uaa->device, i, uaa->info.bIfaceIndex);
}
error = usbd_transfer_setup(uaa->device,
sc->sc_iface_index, sc->sc_xfer, umagic_config,
UMAGIC_N_TRANSFER, sc, &sc->sc_mtx);
if (error)
goto detach;
if (sc->sc_udev->desc->idVendor == 0x4f2) {
error = usbd_req_set_idle(sc->sc_udev, NULL,
sc->sc_iface_index[0], 0, 0);
KLOGN(0, "dev set idle %d\n", error);
}
#if 0
mtx_lock(&sc->sc_mtx);
usbd_transfer_start(sc->sc_xfer[UMAGIC_INTR_DT]);
mtx_unlock(&sc->sc_mtx);
#endif
KLOGN(0, "\n");
return 0;
detach:
KLOGN(0, "error=%s\n", usbd_errstr(error));
umagic_detach(dev);
return error;
}
static int
umagic_detach(device_t dev)
{
struct umagic_softc *sc = device_get_softc(dev);
usbd_transfer_unsetup(sc->sc_xfer, UMAGIC_N_TRANSFER);
KLOGN(0, "masc!!!!!!!!!!!!!!!!!!\n");
mtx_destroy(&sc->sc_mtx);
KLOGN(0, "masc!!!!!!!!!!!!!!!!!!\n");
KLOGN(0, "\n");
return 0;
}
static void
umagic_intr_callback(struct usb_xfer *xfer, usb_error_t error)
{
struct umagic_softc *sc = usbd_xfer_softc(xfer);
struct usb_page_cache *pc;
uint8_t buf[8];
int len;
usbd_xfer_status(xfer, &len, NULL, NULL, NULL);
KLOGN(0, "%p, state %d, error %d\n", sc, USB_GET_STATE(xfer), error);
switch (USB_GET_STATE(xfer)) {
case USB_ST_TRANSFERRED:
memset(buf, 0, sizeof(buf));
pc = usbd_xfer_get_frame(xfer, 0);
len = MIN(len, sizeof(buf));
usbd_copy_out(pc, 0, buf, len);
hexdump(buf, len, "intr", 0);
//break;
case USB_ST_SETUP:
tr_setup:
usbd_xfer_set_frame_len(xfer, 0, usbd_xfer_max_len(xfer));
usbd_transfer_submit(xfer);
KLOGN(0, "sc %p\n", sc);
break;
default: /* Error */
if (error != USB_ERR_CANCELLED) {
/* try clear stall first */
usbd_xfer_set_stall(xfer);
goto tr_setup;
}
xfer->error = 0;
break;
}
}
[-- Attachment #3 --]
diff --git a/sys/dev/usb/controller/xhci.c b/sys/dev/usb/controller/xhci.c
index 8c6a221ca9b..cade9111a3c 100644
--- a/sys/dev/usb/controller/xhci.c
+++ b/sys/dev/usb/controller/xhci.c
@@ -3905,9 +3905,10 @@ xhci_configure_reset_endpoint(struct usb_xfer *xfer)
xhci_configure_mask(udev, (1U << epno) | 1U, 0);
- if (epno > 1)
- err = xhci_cmd_configure_ep(sc, buf_inp.physaddr, 0, index);
- else
+ if (epno > 1) {
+ if (!pepext->stop_xfer_fixed)
+ err = xhci_cmd_configure_ep(sc, buf_inp.physaddr, 0, index);
+ } else
err = xhci_cmd_evaluate_ctx(sc, buf_inp.physaddr, index);
if (err != 0)
@@ -4020,11 +4021,87 @@ xhci_configure_msg(struct usb_proc_msg *pm)
}
}
+static int
+epdesc2epno(struct usb_endpoint_descriptor *edesc)
+{
+ int epno;
+
+ epno = edesc->bEndpointAddress;
+ if ((edesc->bmAttributes & UE_XFERTYPE) == UE_CONTROL)
+ epno |= UE_DIR_IN;
+ epno = XHCI_EPNO2EPID(epno);
+ return epno;
+}
+
+static inline int
+udev_max_packet(struct usb_device *udev)
+{
+ int mps = 8;
+
+ if (udev->speed == USB_SPEED_LOW) {
+ mps = 8;
+ } else if (udev->speed == USB_SPEED_HIGH) {
+ mps = 64;
+ } else if (udev->speed == USB_SPEED_SUPER) {
+ mps = 512;
+ } else {
+ if (udev->desc->bMaxPacketSize)
+ mps = udev->desc->bMaxPacketSize;
+ else
+ mps = 8; //defalt value on linux: 64,
+ }
+
+ return mps;
+}
+
+static inline int
+set_endpt_para(struct usb_device *udev, const struct usb_endpoint_descriptor *edesc,
+ struct xhci_endpoint_ext *pepext)
+{
+ int mult;
+ int max_packet;
+ int max_burst;
+ int max_frame;
+ int ep_type = edesc->bmAttributes & 0x3;
+
+ mult = 1;
+ max_packet = UGETW(edesc->wMaxPacketSize) & 0x7ff;
+ if (ep_type == UE_CONTROL) {
+ max_burst = 1;
+ max_packet = udev_max_packet(udev);
+ } else if ((udev->speed == USB_SPEED_SUPER) && (ep_type == UE_ISOCHRONOUS)) {
+ max_burst = (UGETW(edesc->wMaxPacketSize) & 0x1800) >> 11;
+ KLOGN(KERR, "unsupported ep\n");
+ } else {
+ if ((udev->speed == USB_SPEED_HIGH) &&
+ ((ep_type == UE_ISOCHRONOUS) || (ep_type == UE_INTERRUPT))) {
+ max_burst = (UGETW(edesc->wMaxPacketSize) & 0x1800) >> 11;
+ if (max_burst > 2)
+ max_burst = 3;
+ else
+ (max_burst)++;
+ } else {
+ max_burst = 1;
+ }
+ }
+ max_frame = max_burst * max_packet;
+ pepext->max_packet_count = mult;
+ pepext->fps_shift = 0;
+ pepext->max_packet_size = max_packet;
+ //pepext->max_burst_size = max_burst;
+ pepext->max_frame_size = max_frame;
+ return 0;
+}
+
static void
xhci_ep_init(struct usb_device *udev, struct usb_endpoint_descriptor *edesc,
struct usb_endpoint *ep)
{
struct xhci_endpoint_ext *pepext;
+ int epno;
+ int err;
+ struct usb_page_search buf_inp;
+ int ep_type = edesc->bmAttributes & 0x3;
DPRINTFN(2, "endpoint=%p, addr=%d, endpt=%d, mode=%d\n",
ep, udev->address, edesc->bEndpointAddress, udev->flags.usb_mode);
@@ -4038,9 +4115,41 @@ xhci_ep_init(struct usb_device *udev, struct usb_endpoint_descriptor *edesc,
pepext = xhci_get_endpoint_ext(udev, edesc);
+ epno = epdesc2epno(edesc);
+ if ((epno != 1) && (ep_type == UE_INTERRUPT)) {
+ struct xhci_softc *sc = XHCI_BUS2SC(udev->bus);
+ int index = udev->controller_slot_id;
+
+ usbd_get_page(&sc->sc_hw.devs[index].input_pc, 0, &buf_inp);
+ XHCI_CMD_LOCK(sc);
+ set_endpt_para(udev, edesc, pepext);
+ err = xhci_configure_endpoint(udev, edesc, pepext, 0,
+ pepext->max_packet_count,
+ 1, pepext->fps_shift, pepext->max_packet_size,
+ pepext->max_frame_size, ep->ep_mode);
+ if (err)
+ KLOGN(KERR, "soft configure ep %d\n", err);
+ xhci_configure_mask(udev, (1U << epno) | 1U, 0);
+ err = xhci_cmd_configure_ep(sc, buf_inp.physaddr, 0, index);
+ if (err)
+ KLOGN(KERR, "configure ep %d\n", err);
+ XHCI_CMD_UNLOCK(sc);
+ }
USB_BUS_LOCK(udev->bus);
- pepext->trb_halted = 1;
- pepext->trb_running = 0;
+ if (epno == 1) {
+ pepext->trb_halted = 1;
+ pepext->trb_running = 0;
+ } else {
+ if (err) {
+ KLOGN(KERR, "configure ep err %d\n", err);
+ pepext->trb_halted = 1;
+ pepext->trb_running = 0;
+ } else {
+ pepext->trb_halted = 0;
+ pepext->trb_running = 1;
+ pepext->stop_xfer_fixed = 1;
+ }
+ }
USB_BUS_UNLOCK(udev->bus);
}
home |
help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?caff3f2cab964507b12df3841d443000>