From e61aa0cd37d39531ca54175d17a9dd2cfce11637 Mon Sep 17 00:00:00 2001 From: Marcin Bukat Date: Wed, 7 Oct 2026 08:02:00 +0200 Subject: [PATCH] usb_storage: allocate the buffer at the storage handover The host configures the device before Rockbox has handed the storage over, and mass storage allocated its buffer right then. On the recording screen the recording buffer still holds all free memory at that point and cannot shrink, so plugging in USB there panicked with "usb_storage_init_connection(): OOM". Recording closes, and frees its buffer, only on SYS_USB_CONNECTED, which the handover broadcasts. Playback does not hit this: its buffer gives memory up on request. The buffer is only needed to run commands, and commands already wait for the handover. So it is allocated, and the endpoint primed for the first command, in the notify event that both handover paths in usb.c send, or at once when the storage is already handed over. Until then the host's first command waits at the endpoint, NAKed. GET_MAX_LUN, which comes before the handover, is answered from the core's control buffer instead of the transfer buffer. Targets with static USB buffers are unchanged. Tested on a Samsung YP-CP3: USB plugged in on the recording screen, at boot and from the main menu, mounts, and files copied both ways keep their checksums. Co-Authored-By: Claude Opus 5.5 Change-Id: Icadc24fd52a07d03125234e492dab0ede9e339c9 --- firmware/usbstack/usb_storage.c | 43 ++++++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 6 deletions(-) diff --git a/firmware/usbstack/usb_storage.c b/firmware/usbstack/usb_storage.c index 2598a3d46d..a262770e4a 100644 --- a/firmware/usbstack/usb_storage.c +++ b/firmware/usbstack/usb_storage.c @@ -449,9 +449,13 @@ static int usb_storage_get_config_descriptor(unsigned char *dest,int max_packet_ static int usb_handle = 0; #endif -static int usb_storage_init_connection(void) +/* Sets up the buffers and primes the rx endpoint for the first command. + * The buffers are allocated only once Rockbox has handed the storage over: + * the host configures the device before every thread has acknowledged the + * connect, while for example the recording buffer still holds all free + * memory. Until then the host's first command waits at the endpoint. */ +static void start_receiving_commands(void) { - logf("ums: set config"); /* prime rx endpoint. We only need room for commands */ state = WAITING_FOR_COMMAND; @@ -470,6 +474,9 @@ static int usb_storage_init_connection(void) #else unsigned char * buffer; + if (usb_handle > 0) + return; /* already receiving */ + // Add 31 to handle worst-case misalignment usb_handle = core_alloc_ex(ALLOCATE_BUFFER_SIZE + MAX_CBW_SIZE + 31, &buflib_ops_locked); @@ -489,6 +496,25 @@ static int usb_storage_init_connection(void) #endif #endif usb_drv_recv_nonblocking(EP_OUT, cbw_buffer, MAX_CBW_SIZE); +} + +static bool receiving_commands(void) +{ +#ifdef USB_STATIC_ALLOC + return true; +#else + return usb_handle > 0; +#endif +} + +static int usb_storage_init_connection(void) +{ + logf("ums: set config"); + state = WAITING_FOR_COMMAND; +#ifndef USB_STATIC_ALLOC + if(usb_exclusive_storage()) +#endif + start_receiving_commands(); int i; for(i=0;ibRequest) { case USB_BULK_GET_MAX_LUN: { - *tb.max_lun = storage_num_drives() - 1; + /* comes before the handover, without the transfer buffer */ + reqdata[0] = storage_num_drives() - 1; #if defined(HAVE_MULTIDRIVE) - if(skip_first) (*tb.max_lun) --; + if(skip_first) reqdata[0]--; #endif logf("ums: getmaxlun"); - usb_core_control_response(USB_CONTROL_ACK, tb.max_lun, 1); + usb_core_control_response(USB_CONTROL_ACK, reqdata, 1); handled = true; break; } @@ -1450,6 +1476,11 @@ static void handle_scsi(struct command_block_wrapper* cbw) static void usb_storage_notify_event(intptr_t data) { (void)data; + if(!receiving_commands()) { + if(usb_exclusive_storage()) + start_receiving_commands(); + return; + } if(state == WAITING_FOR_STORAGE && usb_exclusive_storage()) handle_scsi((struct command_block_wrapper*)cbw_buffer); }