xiaoxiang781216 commented on code in PR #19861:
URL: https://github.com/apache/nuttx/pull/19861#discussion_r3806313304
##########
drivers/usbhost/usbhost_xhci.c:
##########
@@ -2811,6 +2853,9 @@ static void xhci_portsc_work(FAR void *arg)
usbhost_vtrace2(XHCI_VTRACE2_PORTSC_DISCONND,
rhpndx + 1, priv->pscwait);
+ syslog(LOG_INFO, "%s: port %d: device removed\n",
Review Comment:
let's add debug macro in include/debug.h like others
##########
drivers/usbhost/usbhost_xhci.c:
##########
@@ -4569,6 +4615,19 @@ static int xhci_asynch(FAR struct usbhost_driver_s
*drvr, usbhost_ep_t ep,
DEBUGASSERT(priv && rhport && epinfo && buffer && buflen > 0);
+ /* An asynchronous transfer has no caller to come back to, so a buffer
+ * needing a stand-in cannot be used: the copy back out of it would have
+ * to happen in the completion handler, which runs in a work queue thread
+ * where a caller's address means nothing. The callers of this are class
+ * drivers using kernel memory, which needs no stand-in.
+ */
+
+ if (!xhci_dma_direct(priv, buffer, buflen))
Review Comment:
do you need change other similar check to xhci_dma_direct
##########
drivers/usbhost/usbhost_xhci.c:
##########
@@ -433,10 +435,6 @@ static ssize_t xhci_transfer_wait(FAR struct
usbhost_xhci_s *priv,
static bool xhci_dmacapable(FAR struct usbhost_xhci_s *priv,
FAR uint8_t *buffer, size_t buflen);
static uint32_t xhci_speed_id(uint8_t speed);
-#ifdef CONFIG_USBHOST_ASYNCH
-static bool xhci_dma_direct(FAR struct usbhost_xhci_s *priv,
Review Comment:
why add in the previous patch, but remove here? please review AI generated
patch carefully and remove the intermediate change
##########
drivers/usbhost/usbhost_xhci.c:
##########
@@ -1496,6 +1498,16 @@ static int xhci_port_enable(FAR struct usbhost_xhci_s
*priv,
}
}
+ /* Say what turned up, now that the port can answer.
+ *
+ * The speed field only means anything once the port has been reset and
+ * enabled. A USB2 port reports the reset default, full speed, until
+ * then.
+ */
+
+ syslog(LOG_INFO, "%s: port %d: device attached at %s\n",
Review Comment:
ditto
##########
drivers/usbhost/usbhost_xhci.c:
##########
@@ -160,6 +160,16 @@ struct xhci_epinfo_s
size_t dmacopy; /* Length to copy back out of a stand-in */
bool dmain; /* Direction this buffer was prepared for */
sem_t iocsem; /* Semaphore used to wait for transfer
completion */
+
+ /* One transfer at a time on an endpoint. The controller lock below is
Review Comment:
why the second transfer can't append the transfer to the hardware link list
##########
drivers/usbhost/usbhost_xhci.c:
##########
@@ -3831,6 +3831,68 @@ static int xhci_ep0configure(FAR struct usbhost_driver_s
*drvr,
return ret;
}
+/****************************************************************************
+ * Name: xhci_interval
+ *
+ * Description:
+ * Work out the Interval an endpoint context wants.
+ *
+ * The field is an exponent: the controller services the endpoint every
+ * 2^Interval microframes. An endpoint descriptor does not say it that
+ * way, and what it does say depends on how fast the device is, so the
+ * number cannot simply be copied across.
+ *
+ * A low or full speed interrupt endpoint counts in frames, so its period
+ * is bInterval milliseconds, or bInterval * 8 microframes, and the
+ * exponent is the position of the highest bit of that. Everything else
+ * that is periodic already states an exponent, one greater than the one
+ * wanted here. Control and bulk endpoints are not periodic and the field
+ * means nothing to them.
+ *
+ ****************************************************************************/
+
+static uint8_t xhci_interval(uint8_t speed, uint8_t xfrtype,
Review Comment:
where you call this function
##########
drivers/usbhost/usbhost_xhci.c:
##########
@@ -3971,16 +3971,31 @@ static int xhci_epalloc(FAR struct usbhost_driver_s
*drvr,
epinfo->epno = epdesc->addr;
#ifndef CONFIG_USBHOST_INT_DISABLE
- epinfo->interval = epdesc->interval;
+ epinfo->interval = xhci_interval(hport->speed, epdesc->xfrtype,
Review Comment:
squash into previous patch
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
