Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(hid): Eliminate data race in USB pathway causing dropped keys #2257

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions app/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,15 @@ config ZMK_SETTINGS_SAVE_DEBOUNCE
#SETTINGS
endif

config ZMK_WORKAROUND_COPY_USB_TX_DATA_BUFFER
bool "Enable workaround for USB driver not copying data before transmit"
help
Work around some hardware USB drivers not copying the passed USB TX buffer
contents before beginning a transaction, causing corruption of the USB
message, by copying the TX data into a temporary buffer first. For
example, the NRFx family of Zephyr drivers suffer this issue.
default y if SOC_SERIES_NRF52X

config ZMK_BATTERY_REPORT_INTERVAL
depends on ZMK_BATTERY_REPORTING
int "Battery level report interval in seconds"
Expand Down
24 changes: 24 additions & 0 deletions app/include/zmk/hid.h
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,30 @@ struct zmk_hid_mouse_report {

#endif // IS_ENABLED(CONFIG_ZMK_MOUSE)

struct zmk_hid_report_body {
union {
#if IS_ENABLED(CONFIG_ZMK_MOUSE)
struct zmk_hid_mouse_report_body mouse;
#endif

struct zmk_hid_keyboard_report_body keyboard;
struct zmk_hid_consumer_report_body consumer;
} __packed;
} __packed;

struct zmk_hid_report {
union {
#if IS_ENABLED(CONFIG_ZMK_USB_BOOT)
zmk_hid_boot_report_t boot;
#endif

struct {
uint8_t report_id;
struct zmk_hid_report_body body;
} __packed;
} __packed;
} __packed;

zmk_mod_flags_t zmk_hid_get_explicit_mods(void);
int zmk_hid_register_mod(zmk_mod_t modifier);
int zmk_hid_unregister_mod(zmk_mod_t modifier);
Expand Down
16 changes: 15 additions & 1 deletion app/src/usb_hid.c
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
* SPDX-License-Identifier: MIT
*/

#include <zephyr/sys/util.h>

#include <zephyr/device.h>
#include <zephyr/init.h>

Expand All @@ -24,6 +26,17 @@ static const struct device *hid_dev;

static K_SEM_DEFINE(hid_sem, 1, 1);

#if IS_ENABLED(CONFIG_ZMK_WORKAROUND_COPY_USB_TX_DATA_BUFFER)
static uint8_t hid_ep_write_buf[sizeof(struct zmk_hid_report)];

static const uint8_t *prepare_report_buf(const uint8_t *report, size_t len) {
memcpy(hid_ep_write_buf, report, len);
return hid_ep_write_buf;
}
#else
static const uint8_t *prepare_report_buf(const uint8_t *report, size_t len) { return report; }
#endif

static void in_ready_cb(const struct device *dev) { k_sem_give(&hid_sem); }

#define HID_GET_REPORT_TYPE_MASK 0xff00
Expand Down Expand Up @@ -137,7 +150,8 @@ static int zmk_usb_hid_send_report(const uint8_t *report, size_t len) {
return -ENODEV;
default:
k_sem_take(&hid_sem, K_MSEC(30));
int err = hid_int_ep_write(hid_dev, report, len, NULL);
const uint8_t *buf = prepare_report_buf(report, len);
int err = hid_int_ep_write(hid_dev, buf, len, NULL);

if (err) {
k_sem_give(&hid_sem);
Expand Down