From d28932934482cd5df5871d8276adc1cc812e9fc4 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Tue, 28 Mar 2023 11:02:17 -0400 Subject: [PATCH 1/3] Handle HID OUT reports with no report ID --- shared-module/usb_hid/Device.c | 44 +++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/shared-module/usb_hid/Device.c b/shared-module/usb_hid/Device.c index 680bb83893dfe..6f530bfef8aff 100644 --- a/shared-module/usb_hid/Device.c +++ b/shared-module/usb_hid/Device.c @@ -293,24 +293,40 @@ uint16_t tud_hid_get_report_cb(uint8_t itf, uint8_t report_id, hid_report_type_t // Callback invoked when we receive Set_Report request through control endpoint void tud_hid_set_report_cb(uint8_t itf, uint8_t report_id, hid_report_type_t report_type, uint8_t const *buffer, uint16_t bufsize) { (void)itf; - if (report_type == HID_REPORT_TYPE_INVALID) { - report_id = buffer[0]; - buffer++; - bufsize--; + + usb_hid_device_obj_t *hid_device = NULL; + size_t id_idx; + + if (report_id == 0 && report_type == HID_REPORT_TYPE_INVALID) { + // This could be a report with a non-zero report ID in the first byte, or + // it could be for report ID 0. + // Heuristic: see if there's a device with report ID 0, and if its report length matches + // the size of the incoming buffer. In that case, assume the first byte is not the report ID, + // but is data. Otherwise use the first byte as the report id. + if (usb_hid_get_device_with_report_id(0, &hid_device, &id_idx) && + hid_device && + hid_device->out_report_buffers[id_idx] && + hid_device->out_report_lengths[id_idx] == bufsize) { + // Use as is, with report_id 0. + } else { + // No matching report ID 0, so use the first byte as the report ID. + report_id = buffer[0]; + buffer++; + bufsize--; + } } else if (report_type != HID_REPORT_TYPE_OUTPUT && report_type != HID_REPORT_TYPE_FEATURE) { return; } - usb_hid_device_obj_t *hid_device; - size_t id_idx; - // Find device with this report id, and get the report id index. - if (usb_hid_get_device_with_report_id(report_id, &hid_device, &id_idx)) { + // report_id might be changed due to parsing above, so test again. + if ((report_id == 0 && report_type == HID_REPORT_TYPE_INVALID) || + // Fetch the matching device if we don't already have the report_id 0 device. + (usb_hid_get_device_with_report_id(report_id, &hid_device, &id_idx) && + hid_device && + hid_device->out_report_buffers[id_idx] && + hid_device->out_report_lengths[id_idx] == bufsize)) { // If a report of the correct size has been read, save it in the proper OUT report buffer. - if (hid_device && - hid_device->out_report_buffers[id_idx] && - hid_device->out_report_lengths[id_idx] >= bufsize) { - memcpy(hid_device->out_report_buffers[id_idx], buffer, bufsize); - hid_device->out_report_buffers_updated[id_idx] = true; - } + memcpy(hid_device->out_report_buffers[id_idx], buffer, bufsize); + hid_device->out_report_buffers_updated[id_idx] = true; } } From 11082435f1d6c916d6274e77e842c0d10cbbff6a Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Tue, 28 Mar 2023 16:05:17 -0400 Subject: [PATCH 2/3] shrink SAMD21 builds by a few hundred bytes --- ports/atmel-samd/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ports/atmel-samd/Makefile b/ports/atmel-samd/Makefile index 343dcf7d63427..31e69f36c983a 100644 --- a/ports/atmel-samd/Makefile +++ b/ports/atmel-samd/Makefile @@ -116,12 +116,12 @@ else # -finline-limit=80 or so is similar to not having it on. # There is no simple default value, though. ifeq ($(SHRINK_BUILD), 1) - CFLAGS += -finline-limit=45 + CFLAGS += -finline-limit=45 --param max-inline-insns-auto=110 endif # We used to do this but it seems to not reduce space any more, at least in gcc 11. # Leave it here, commented out, just for reference. - # --param inline-unit-growth=15 --param max-inline-insns-auto=20 + # --param inline-unit-growth=15 ifdef CFLAGS_BOARD CFLAGS += $(CFLAGS_BOARD) From b2535496b08849d93cec582d5894a748b60cbb96 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Tue, 28 Mar 2023 16:28:31 -0400 Subject: [PATCH 3/3] use max-inline-insns-auto=110 only on SAMD21 --- ports/atmel-samd/Makefile | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/ports/atmel-samd/Makefile b/ports/atmel-samd/Makefile index 31e69f36c983a..ca906ab98f862 100644 --- a/ports/atmel-samd/Makefile +++ b/ports/atmel-samd/Makefile @@ -104,21 +104,18 @@ else CFLAGS += -DNDEBUG # Do a default shrink for small builds, including all SAMD21 builds. + # -finline-limit can shrink the image size. + # -finline-limit=80 or so is similar to not having it on. + # There is no simple default value, though. ifeq ($(CIRCUITPY_FULL_BUILD),0) - SHRINK_BUILD = 1 + CFLAGS += -finline-limit=45 else ifeq ($(CHIP_FAMILY), samd21) - SHRINK_BUILD = 1 + # max-inline-insns-auto increases the size of SAMD51 builds. + CFLAGS += -finline-limit=45 --param max-inline-insns-auto=110 endif endif - # -finline-limit can shrink the image size. - # -finline-limit=80 or so is similar to not having it on. - # There is no simple default value, though. - ifeq ($(SHRINK_BUILD), 1) - CFLAGS += -finline-limit=45 --param max-inline-insns-auto=110 - endif - # We used to do this but it seems to not reduce space any more, at least in gcc 11. # Leave it here, commented out, just for reference. # --param inline-unit-growth=15