This is an automated email from the ASF dual-hosted git repository.
xiaoxiang781216 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx.git
The following commit(s) were added to refs/heads/master by this push:
new 47568187442 usbmsc: fix composite-mode class requests and Hi>Di stall
behavior
47568187442 is described below
commit 475681874424979149c4441f78f33ef02df6caff
Author: Ricard Rosson <[email protected]>
AuthorDate: Wed Jul 15 21:01:19 2026 +0100
usbmsc: fix composite-mode class requests and Hi>Di stall behavior
Three defects that together prevented macOS from ever mounting a
composite USBMSC function (Linux was mostly unaffected because its
probe sequence and recovery timing never exercised these paths):
1. usbmsc_setup() compared the class-request wIndex against the
compile-time constant USBMSC_INTERFACEID (= CONFIG_USBMSC_IFNOBASE,
i.e. 0) instead of the composite-assigned priv->devinfo.ifnobase.
In composite mode the MSC interface number is nonzero, so
GET MAX LUN, Bulk-Only Mass Storage Reset, and GET/SET INTERFACE
all failed the index check and stalled EP0. Standalone MSC is
unaffected (ifnobase == 0), which is why this went unnoticed.
2. usbmsc_deferredresponse() has its entire body inside
#ifndef CONFIG_USBMSC_COMPOSITE, so the deferred EP0 status stage
for MSRESET/SETINTERFACE was never sent in composite mode and the
host's Bulk-Only reset timed out. (Unreachable before fix 1 --
MSRESET used to stall at the wrong-interface check.) Compile the
body in composite mode too, but suppress the worker's deferred
response for SETCONFIGURATION there: the composite driver answers
that request itself, and a duplicate zero-length packet corrupts
the EP0 state.
3. usbmsc_cmdfinishstate() stalled the bulk IN endpoint whenever a
device-to-host command left a residue, even when the response had
already been sent and terminated by a short packet (or ZLP). The
stall is BOT-legal (USB MSC BOT 6.7.2) but gratuitous: the short
packet already ended the data phase and the residue is reported in
dCSWDataResidue. Hosts such as macOS answer any bulk-IN halt during
device probing with a full Bulk-Only reset sequence, which costs
seconds per command or aborts the probe entirely (macOS probes
MODE SENSE(6) with allocation lengths that exceed the response;
Linux's probe does not). Only halt the endpoint when nothing
terminated the data phase.
Root-cause analysis and host traces in apache/nuttx#19435.
Validated on RP2350 silicon (Raspberry Pi Pico 2 W, composite
CDC-ACM + CDC-NCM + USBMSC): GET MAX LUN answers 1 LUN (previously
EP0 stall and a garbage LUN count on macOS), MSRESET completes 10/10
(previously ETIMEDOUT), MODE SENSE(6) alloc=0xC0 returns short data
plus a CSW with dCSWDataResidue and zero bulk-IN stalls across the
exact-length suite, and macOS now mounts the volume (together with the
companion DCD fixes).
Co-Authored-By: Claude Fable 5 <[email protected]>
Signed-off-by: Ricard Rosson <[email protected]>
---
drivers/usbdev/usbmsc.c | 10 ++++------
drivers/usbdev/usbmsc_scsi.c | 42 ++++++++++++++++++++++++++++++++++++++++--
2 files changed, 44 insertions(+), 8 deletions(-)
diff --git a/drivers/usbdev/usbmsc.c b/drivers/usbdev/usbmsc.c
index bfac6ff03f9..f668f702da6 100644
--- a/drivers/usbdev/usbmsc.c
+++ b/drivers/usbdev/usbmsc.c
@@ -741,7 +741,7 @@ static int usbmsc_setup(FAR struct usbdevclass_driver_s
*driver,
if (ctrl->type == USB_REQ_RECIPIENT_INTERFACE)
{
if (priv->config == USBMSC_CONFIGID &&
- index == USBMSC_INTERFACEID &&
+ index == priv->devinfo.ifnobase &&
value == USBMSC_ALTINTERFACEID)
{
/* Signal to instantiate the interface change */
@@ -764,7 +764,7 @@ static int usbmsc_setup(FAR struct usbdevclass_driver_s
*driver,
if (ctrl->type == (USB_DIR_IN | USB_REQ_RECIPIENT_INTERFACE) &&
priv->config == USBMSC_CONFIGIDNONE)
{
- if (index != USBMSC_INTERFACEID)
+ if (index != priv->devinfo.ifnobase)
{
ret = -EDOM;
}
@@ -805,7 +805,7 @@ static int usbmsc_setup(FAR struct usbdevclass_driver_s
*driver,
{
/* Only one interface is supported */
- if (index != USBMSC_INTERFACEID)
+ if (index != priv->devinfo.ifnobase)
{
usbtrace(TRACE_CLSERROR(USBMSC_TRACEERR_MSRESETNDX),
index);
@@ -836,7 +836,7 @@ static int usbmsc_setup(FAR struct usbdevclass_driver_s
*driver,
{
/* Only one interface is supported */
- if (index != USBMSC_INTERFACEID)
+ if (index != priv->devinfo.ifnobase)
{
usbtrace(TRACE_CLSERROR(USBMSC_TRACEERR_GETMAXLUNNDX),
index);
@@ -1280,7 +1280,6 @@ void usbmsc_rdcomplete(FAR struct usbdev_ep_s *ep,
void usbmsc_deferredresponse(FAR struct usbmsc_dev_s *priv, bool failed)
{
-#ifndef CONFIG_USBMSC_COMPOSITE
FAR struct usbdev_s *dev;
FAR struct usbdev_req_s *ctrlreq;
int ret;
@@ -1322,7 +1321,6 @@ void usbmsc_deferredresponse(FAR struct usbmsc_dev_s
*priv, bool failed)
usbtrace(TRACE_CLSERROR(USBMSC_TRACEERR_DEFERREDRESPSTALLED), 0);
EP_STALL(dev->ep0);
}
-#endif
}
/****************************************************************************
diff --git a/drivers/usbdev/usbmsc_scsi.c b/drivers/usbdev/usbmsc_scsi.c
index d63478be224..0b2f217d77b 100644
--- a/drivers/usbdev/usbmsc_scsi.c
+++ b/drivers/usbdev/usbmsc_scsi.c
@@ -2614,6 +2614,8 @@ static int usbmsc_cmdfinishstate(FAR struct usbmsc_dev_s
*priv)
case USBMSC_FLAGS_DIRDEVICE2HOST:
if (priv->cbwlen > 0)
{
+ bool terminated = false;
+
/* On most commands (the exception is outgoing, write commands),
* the data has not yet been sent.
*/
@@ -2645,11 +2647,35 @@ static int usbmsc_cmdfinishstate(FAR struct
usbmsc_dev_s *priv)
usbtrace(TRACE_CLSERROR(USBMSC_TRACEERR_CMDFINISHSUBMIT),
(uint16_t)-ret);
}
+ else
+ {
+ /* The request was submitted with USBDEV_REQFLAGS_NULLPKT,
+ * so the transfer ends with a short packet (or a ZLP if
+ * the response length is an exact multiple of the packet
+ * size). Either way the host's Data-In phase terminates
+ * cleanly when this request completes.
+ */
+
+ terminated = true;
+ }
}
- /* Stall the BULK In endpoint if there is a residue */
+ /* If there is a residue, the host expected more data than we
+ * sent. If the data phase was already terminated by a short
+ * packet (or ZLP), nothing more is needed: the host's transfer
+ * has completed and the residue is reported in the CSW
+ * (dCSWDataResidue). Stalling here as well is permitted by BOT
+ * (USB MSC BOT 6.7.2), but it is gratuitous and some hosts
+ * (macOS) respond with a full Bulk-Only reset sequence to any
+ * bulk-IN halt during device probing, which costs seconds per
+ * command or aborts the probe entirely.
+ *
+ * Only halt the endpoint when nothing terminated the data
+ * phase - otherwise the host would mistake the following CSW
+ * for transfer data.
+ */
- if (priv->residue > 0)
+ if (priv->residue > 0 && !terminated)
{
#ifndef CONFIG_USBMSC_NOT_STALL_BULKEP
usbtrace(TRACE_CLSERROR(USBMSC_TRACEERR_CMDFINISHRESIDUE),
@@ -2975,8 +3001,20 @@ int usbmsc_scsi_main(int argc, FAR char *argv[])
* response
*/
+#ifdef CONFIG_USBMSC_COMPOSITE
+ /* In composite mode the composite driver itself responds to
+ * SETCONFIGURATION (see composite_ep0submit); only the deferred
+ * responses to MSRESET and SETINTERFACE are owed by this class.
+ * Submitting a second response for SETCONFIGURATION would corrupt
+ * the EP0 state.
+ */
+
+ if ((eventset & (USBMSC_EVENT_RESET |
+ USBMSC_EVENT_IFCHANGE)) != 0)
+#else
if ((eventset & (USBMSC_EVENT_RESET | USBMSC_EVENT_CFGCHANGE |
USBMSC_EVENT_IFCHANGE)) != 0)
+#endif
{
usbmsc_deferredresponse(priv, false);
}
