From ecc4313a340056a9427758c6d3a0200acb3f3bc3 Mon Sep 17 00:00:00 2001 From: Jan Pochyla Date: Tue, 7 Jan 2020 18:42:23 +0100 Subject: [PATCH] core/usb: avoid naks in hid/webusb rx interfaces --- core/embed/trezorhal/usb.c | 14 ----------- core/embed/trezorhal/usb_hid-impl.h | 34 +++++++++++--------------- core/embed/trezorhal/usb_webusb-impl.h | 34 +++++++++++--------------- 3 files changed, 28 insertions(+), 54 deletions(-) diff --git a/core/embed/trezorhal/usb.c b/core/embed/trezorhal/usb.c index 9e62c73e1..0183a70ce 100644 --- a/core/embed/trezorhal/usb.c +++ b/core/embed/trezorhal/usb.c @@ -179,20 +179,6 @@ static void usb_desc_add_iface(size_t desc_len) { usb_config_desc->wTotalLength); } -static uint8_t usb_ep_set_nak(USBD_HandleTypeDef *dev, uint8_t ep_num) { - PCD_HandleTypeDef *hpcd = dev->pData; - USB_OTG_GlobalTypeDef *USBx = hpcd->Instance; - USBx_OUTEP(ep_num)->DOEPCTL |= USB_OTG_DOEPCTL_SNAK; - return USBD_OK; -} - -static uint8_t usb_ep_clear_nak(USBD_HandleTypeDef *dev, uint8_t ep_num) { - PCD_HandleTypeDef *hpcd = dev->pData; - USB_OTG_GlobalTypeDef *USBx = hpcd->Instance; - USBx_OUTEP(ep_num)->DOEPCTL |= USB_OTG_DOEPCTL_CNAK; - return USBD_OK; -} - /* * USB interface implementations */ diff --git a/core/embed/trezorhal/usb_hid-impl.h b/core/embed/trezorhal/usb_hid-impl.h index b893a5548..d08be2a78 100644 --- a/core/embed/trezorhal/usb_hid-impl.h +++ b/core/embed/trezorhal/usb_hid-impl.h @@ -158,20 +158,23 @@ int usb_hid_read(uint8_t iface_num, uint8_t *buf, uint32_t len) { if (iface->type != USB_IFACE_TYPE_HID) { return -2; // Invalid interface type } - usb_hid_state_t *state = &iface->hid; + volatile usb_hid_state_t *state = &iface->hid; - // Copy maximum possible amount of data and truncate the buffer length - if (len < state->last_read_len) { + // Copy maximum possible amount of data + uint32_t last_read_len = state->last_read_len; + if (len < last_read_len) { return 0; // Not enough data in the read buffer } - len = state->last_read_len; + memcpy(buf, state->rx_buffer, last_read_len); + + // Reset the length to indicate we are ready to read next packet state->last_read_len = 0; - memcpy(buf, state->rx_buffer, len); - // Clear NAK to indicate we are ready to read more data - usb_ep_clear_nak(&usb_dev_handle, state->ep_out); + // Prepare the OUT EP to receive next packet + USBD_LL_PrepareReceive(&usb_dev_handle, state->ep_out, state->rx_buffer, + state->max_packet_len); - return len; + return last_read_len; } int usb_hid_write(uint8_t iface_num, const uint8_t *buf, uint32_t len) { @@ -182,7 +185,7 @@ int usb_hid_write(uint8_t iface_num, const uint8_t *buf, uint32_t len) { if (iface->type != USB_IFACE_TYPE_HID) { return -2; // Invalid interface type } - usb_hid_state_t *state = &iface->hid; + volatile usb_hid_state_t *state = &iface->hid; if (state->ep_in_is_idle == 0) { return 0; // Last transmission is not over yet @@ -344,17 +347,8 @@ static void usb_hid_class_data_in(USBD_HandleTypeDef *dev, static void usb_hid_class_data_out(USBD_HandleTypeDef *dev, usb_hid_state_t *state, uint8_t ep_num) { if (ep_num == state->ep_out) { + // Save the report length to indicate we have read something, but don't + // schedule next reading until user reads this one state->last_read_len = USBD_LL_GetRxDataSize(dev, ep_num); - - // Prepare the OUT EP to receive next packet - // User should provide state->rx_buffer that is big enough for - // state->max_packet_len bytes - USBD_LL_PrepareReceive(dev, ep_num, state->rx_buffer, - state->max_packet_len); - - if (state->last_read_len > 0) { - // Block the OUT EP until we process received data - usb_ep_set_nak(dev, ep_num); - } } } diff --git a/core/embed/trezorhal/usb_webusb-impl.h b/core/embed/trezorhal/usb_webusb-impl.h index e50ae866d..cfc08d4fa 100644 --- a/core/embed/trezorhal/usb_webusb-impl.h +++ b/core/embed/trezorhal/usb_webusb-impl.h @@ -134,20 +134,23 @@ int usb_webusb_read(uint8_t iface_num, uint8_t *buf, uint32_t len) { if (iface->type != USB_IFACE_TYPE_WEBUSB) { return -2; // Invalid interface type } - usb_webusb_state_t *state = &iface->webusb; + volatile usb_webusb_state_t *state = &iface->webusb; - // Copy maximum possible amount of data and truncate the buffer length - if (len < state->last_read_len) { + // Copy maximum possible amount of data + uint32_t last_read_len = state->last_read_len; + if (len < last_read_len) { return 0; // Not enough data in the read buffer } - len = state->last_read_len; + memcpy(buf, state->rx_buffer, last_read_len); + + // Reset the length to indicate we are ready to read next packet state->last_read_len = 0; - memcpy(buf, state->rx_buffer, len); - // Clear NAK to indicate we are ready to read more data - usb_ep_clear_nak(&usb_dev_handle, state->ep_out); + // Prepare the OUT EP to receive next packet + USBD_LL_PrepareReceive(&usb_dev_handle, state->ep_out, state->rx_buffer, + state->max_packet_len); - return len; + return last_read_len; } int usb_webusb_write(uint8_t iface_num, const uint8_t *buf, uint32_t len) { @@ -158,7 +161,7 @@ int usb_webusb_write(uint8_t iface_num, const uint8_t *buf, uint32_t len) { if (iface->type != USB_IFACE_TYPE_WEBUSB) { return -2; // Invalid interface type } - usb_webusb_state_t *state = &iface->webusb; + volatile usb_webusb_state_t *state = &iface->webusb; state->ep_in_is_idle = 0; USBD_LL_Transmit(&usb_dev_handle, state->ep_in, UNCONST(buf), (uint16_t)len); @@ -268,17 +271,8 @@ static void usb_webusb_class_data_out(USBD_HandleTypeDef *dev, usb_webusb_state_t *state, uint8_t ep_num) { if (ep_num == state->ep_out) { + // Save the report length to indicate we have read something, but don't + // schedule next reading until user reads this one state->last_read_len = USBD_LL_GetRxDataSize(dev, ep_num); - - // Prepare the OUT EP to receive next packet - // User should provide state->rx_buffer that is big enough for - // state->max_packet_len bytes - USBD_LL_PrepareReceive(dev, ep_num, state->rx_buffer, - state->max_packet_len); - - if (state->last_read_len > 0) { - // Block the OUT EP until we process received data - usb_ep_set_nak(dev, ep_num); - } } }