From b0ce6febbea0259d99473000ace5a8fb231c4114 Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Thu, 7 Apr 2022 09:38:28 -0500 Subject: [PATCH] Duplicate GD32 `uint16_t*` cast workaround from KeyboardioHID This is slightly different than the original to make it easier to revert once the bug is fixed. I used block comments so that the original lines could be preserved as-is, and reverting the change only requires the removal of the lines added for the workaround. Signed-off-by: Michael Richters --- .../usb/BootKeyboard/BootKeyboard.cpp | 15 +++++++++++ .../driver/hid/keyboardio/usb/HID.cpp | 25 +++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/src/kaleidoscope/driver/hid/keyboardio/usb/BootKeyboard/BootKeyboard.cpp b/src/kaleidoscope/driver/hid/keyboardio/usb/BootKeyboard/BootKeyboard.cpp index 37720546..f3a77ff9 100644 --- a/src/kaleidoscope/driver/hid/keyboardio/usb/BootKeyboard/BootKeyboard.cpp +++ b/src/kaleidoscope/driver/hid/keyboardio/usb/BootKeyboard/BootKeyboard.cpp @@ -194,7 +194,22 @@ bool BootKeyboard_::setup(USBSetup& setup) { if (setup.wValueH == HID_REPORT_TYPE_OUTPUT) { if (length == sizeof(leds)) { + // ------------------------------------------------------------ + // Workaround for a bug in the GD32 core: + // + // On GD32, when we call `USV_RecvControl`, it casts the (void*) pointer + // we give it to `uint16_t*`, which means that it will alway write an even + // number of bytes to that pointer. Because we don't want to overwrite + // the next byte in memory past `leds`, we use a temporary array that is + // guaranteed to be big enough, and copy the data from that: + uint8_t raw_report_data[2]; + USB_RecvControl(&raw_report_data, length); + leds = raw_report_data[0]; + // Once the GD32 core bug is fixed, we can replace the above code with the + // original code below: + /* ------------------------------------------------------------ USB_RecvControl(&leds, length); + // ------------------------------------------------------------ */ return true; } } diff --git a/src/kaleidoscope/driver/hid/keyboardio/usb/HID.cpp b/src/kaleidoscope/driver/hid/keyboardio/usb/HID.cpp index f3e2c730..27dbb168 100644 --- a/src/kaleidoscope/driver/hid/keyboardio/usb/HID.cpp +++ b/src/kaleidoscope/driver/hid/keyboardio/usb/HID.cpp @@ -158,12 +158,37 @@ bool HID_::setup(USBSetup& setup) { if (request == HID_SET_REPORT) { uint16_t length = setup.wLength; + // ------------------------------------------------------------ + // Workaround for a bug in the GD32 core: + // + // On GD32, when we call `USV_RecvControl`, it casts the (void*) pointer + // we give it to `uint16_t*`, which means that it will alway write an even + // number of bytes to that pointer. Because we might be trying to just + // read the `leds` byte, and we don't want to overwrite the next byte in + // memory, instead of giving it the pointer to the `setReportData` member + // variable directly, we have it write into to temporary `raw_report_data` + // array that's guaranteed to be big enough, then copy the data from that + // array into `setReportData`: + uint8_t raw_report_data[sizeof(setReportData) + 1]; + if (length == sizeof(setReportData)) { + USB_RecvControl(&raw_report_data, length); + setReportData.reportId = raw_report_data[0]; + setReportData.leds = raw_report_data[1]; + } else if (length == sizeof(setReportData.leds)) { + USB_RecvControl(&raw_report_data, length); + setReportData.reportId = 0; + setReportData.leds = raw_report_data[0]; + } + // Once the GD32 core bug is fixed, we can replace the above code with the + // original code below: + /* ------------------------------------------------------------ if (length == sizeof(setReportData)) { USB_RecvControl(&setReportData, length); } else if (length == sizeof(setReportData.leds)) { USB_RecvControl(&setReportData.leds, length); setReportData.reportId = 0; } + // ------------------------------------------------------------ */ } }