From c5422a5c48984f4ea2a5fb6b22f6778183002f68 Mon Sep 17 00:00:00 2001 From: Peter Lawrence <12226419+majbthrd@users.noreply.github.com> Date: Tue, 23 Feb 2021 12:06:41 -0600 Subject: [PATCH 1/2] rp2040: use TU endpoint conventions and remove redundant variables --- src/portable/raspberrypi/rp2040/dcd_rp2040.c | 31 ++++++++++---------- src/portable/raspberrypi/rp2040/rp2040_usb.c | 12 ++++---- src/portable/raspberrypi/rp2040/rp2040_usb.h | 4 --- 3 files changed, 21 insertions(+), 26 deletions(-) diff --git a/src/portable/raspberrypi/rp2040/dcd_rp2040.c b/src/portable/raspberrypi/rp2040/dcd_rp2040.c index 95b0963a..1e67726a 100644 --- a/src/portable/raspberrypi/rp2040/dcd_rp2040.c +++ b/src/portable/raspberrypi/rp2040/dcd_rp2040.c @@ -48,20 +48,21 @@ // Init these in dcd_init static uint8_t *next_buffer_ptr; -// Endpoints 0-15, direction 0 for out and 1 for in. +// Endpoints 0-15, direction TUSB_DIR_OUT for out and TUSB_DIR_IN for in. static struct hw_endpoint hw_endpoints[16][2] = {0}; -static inline struct hw_endpoint *hw_endpoint_get_by_num(uint8_t num, uint8_t in) +static inline struct hw_endpoint *hw_endpoint_get_by_num(uint8_t num, tusb_dir_t dir) { - return &hw_endpoints[num][in]; + return &hw_endpoints[num][dir]; } static struct hw_endpoint *hw_endpoint_get_by_addr(uint8_t ep_addr) { uint8_t num = tu_edpt_number(ep_addr); - uint8_t in = (ep_addr & TUSB_DIR_IN_MASK) ? 1 : 0; - return hw_endpoint_get_by_num(num, in); + tusb_dir_t dir = tu_edpt_dir(ep_addr); + return hw_endpoint_get_by_num(num, dir); } + static void _hw_endpoint_alloc(struct hw_endpoint *ep) { uint size = TU_MIN(64, ep->wMaxPacketSize); @@ -102,12 +103,10 @@ static void _hw_endpoint_alloc(struct hw_endpoint *ep) static void _hw_endpoint_init(struct hw_endpoint *ep, uint8_t ep_addr, uint wMaxPacketSize, uint8_t transfer_type) { uint8_t num = tu_edpt_number(ep_addr); - bool in = ep_addr & TUSB_DIR_IN_MASK; + tusb_dir_t dir = tu_edpt_dir(ep_addr); ep->ep_addr = ep_addr; - ep->in = in; // For device, IN is a tx transfer and OUT is an rx transfer - ep->rx = in == false; - ep->num = num; + ep->rx = (dir == TUSB_DIR_OUT); // Response to a setup packet on EP0 starts with pid of 1 ep->next_pid = num == 0 ? 1u : 0u; @@ -131,7 +130,7 @@ static void _hw_endpoint_init(struct hw_endpoint *ep, uint8_t ep_addr, uint wMax ep->transfer_type = transfer_type; // Every endpoint has a buffer control register in dpram - if (ep->in) + if (tu_edpt_dir(ep->ep_addr) == TUSB_DIR_IN) { ep->buffer_control = &usb_dpram->ep_buf_ctrl[num].in; } @@ -143,7 +142,7 @@ static void _hw_endpoint_init(struct hw_endpoint *ep, uint8_t ep_addr, uint wMax // Clear existing buffer control state *ep->buffer_control = 0; - if (ep->num == 0) + if (tu_edpt_number(ep->ep_addr) == 0) { // EP0 has no endpoint control register because // the buffer offsets are fixed @@ -155,7 +154,7 @@ static void _hw_endpoint_init(struct hw_endpoint *ep, uint8_t ep_addr, uint wMax else { // Set the endpoint control register (starts at EP1, hence num-1) - if (in) + if (dir == TUSB_DIR_IN) { ep->endpoint_control = &usb_dpram->ep_ctrl[num-1].in; } @@ -259,10 +258,10 @@ static void ep0_0len_status(void) static void _hw_endpoint_stall(struct hw_endpoint *ep) { assert(!ep->stalled); - if (ep->num == 0) + if (tu_edpt_number(ep->ep_addr) == 0) { // A stall on EP0 has to be armed so it can be cleared on the next setup packet - usb_hw_set->ep_stall_arm = ep->in ? USB_EP_STALL_ARM_EP0_IN_BITS : USB_EP_STALL_ARM_EP0_OUT_BITS; + usb_hw_set->ep_stall_arm = (tu_edpt_dir(ep->ep_addr) == TUSB_DIR_IN) ? USB_EP_STALL_ARM_EP0_IN_BITS : USB_EP_STALL_ARM_EP0_OUT_BITS; } _hw_endpoint_buffer_control_set_mask32(ep, USB_BUF_CTRL_STALL); ep->stalled = true; @@ -276,10 +275,10 @@ static void hw_endpoint_stall(uint8_t ep_addr) static void _hw_endpoint_clear_stall(struct hw_endpoint *ep) { - if (ep->num == 0) + if (tu_edpt_number(ep->ep_addr) == 0) { // Probably already been cleared but no harm - usb_hw_clear->ep_stall_arm = ep->in ? USB_EP_STALL_ARM_EP0_IN_BITS : USB_EP_STALL_ARM_EP0_OUT_BITS; + usb_hw_clear->ep_stall_arm = (tu_edpt_dir(ep->ep_addr) == TUSB_DIR_IN) ? USB_EP_STALL_ARM_EP0_IN_BITS : USB_EP_STALL_ARM_EP0_OUT_BITS; } _hw_endpoint_buffer_control_clear_mask32(ep, USB_BUF_CTRL_STALL); ep->stalled = false; diff --git a/src/portable/raspberrypi/rp2040/rp2040_usb.c b/src/portable/raspberrypi/rp2040/rp2040_usb.c index 339297ad..6c6911f2 100644 --- a/src/portable/raspberrypi/rp2040/rp2040_usb.c +++ b/src/portable/raspberrypi/rp2040/rp2040_usb.c @@ -84,7 +84,7 @@ void _hw_endpoint_buffer_control_update32(struct hw_endpoint *ep, uint32_t and_m value |= or_mask; if (or_mask & USB_BUF_CTRL_AVAIL) { if (*ep->buffer_control & USB_BUF_CTRL_AVAIL) { - panic("ep %d %s was already available", ep->num, ep_dir_string[ep->in]); + panic("ep %d %s was already available", tu_edpt_number(ep->ep_addr), ep_dir_string[tu_edpt_dir(ep->ep_addr)]); } *ep->buffer_control = value & ~USB_BUF_CTRL_AVAIL; // 12 cycle delay.. (should be good for 48*12Mhz = 576Mhz) @@ -141,11 +141,11 @@ void _hw_endpoint_start_next_buffer(struct hw_endpoint *ep) void _hw_endpoint_xfer_start(struct hw_endpoint *ep, uint8_t *buffer, uint16_t total_len) { _hw_endpoint_lock_update(ep, 1); - pico_trace("Start transfer of total len %d on ep %d %s\n", total_len, ep->num, ep_dir_string[ep->in]); + pico_trace("Start transfer of total len %d on ep %d %s\n", total_len, tu_edpt_number(ep->ep_addr), ep_dir_string[tu_edpt_dir(ep->ep_addr)]); if (ep->active) { // TODO: Is this acceptable for interrupt packets? - pico_warn("WARN: starting new transfer on already active ep %d %s\n", ep->num, ep_dir_string[ep->in]); + pico_warn("WARN: starting new transfer on already active ep %d %s\n", tu_edpt_number(ep->ep_addr), ep_dir_string[tu_edpt_dir(ep->ep_addr)]); hw_endpoint_reset_transfer(ep); } @@ -223,7 +223,7 @@ bool _hw_endpoint_xfer_continue(struct hw_endpoint *ep) // Part way through a transfer if (!ep->active) { - panic("Can't continue xfer on inactive ep %d %s", ep->num, ep_dir_string); + panic("Can't continue xfer on inactive ep %d %s", tu_edpt_number(ep->ep_addr), ep_dir_string); } // Update EP struct from hardware state @@ -244,7 +244,7 @@ bool _hw_endpoint_xfer_continue(struct hw_endpoint *ep) if (ep->len == ep->total_len) { pico_trace("Completed transfer of %d bytes on ep %d %s\n", - ep->len, ep->num, ep_dir_string[ep->in]); + ep->len, tu_edpt_number(ep->ep_addr), ep_dir_string[tu_edpt_dir(ep->ep_addr)]); // Notify caller we are done so it can notify the tinyusb // stack _hw_endpoint_lock_update(ep, -1); @@ -263,7 +263,7 @@ bool _hw_endpoint_xfer_continue(struct hw_endpoint *ep) void _hw_endpoint_xfer(struct hw_endpoint *ep, uint8_t *buffer, uint16_t total_len, bool start) { // Trace - pico_trace("hw_endpoint_xfer ep %d %s", ep->num, ep_dir_string[ep->in]); + pico_trace("hw_endpoint_xfer ep %d %s", tu_edpt_number(ep->ep_addr), ep_dir_string[tu_edpt_dir(ep->ep_addr)]); pico_trace(" total_len %d, start=%d\n", total_len, start); assert(ep->configured); diff --git a/src/portable/raspberrypi/rp2040/rp2040_usb.h b/src/portable/raspberrypi/rp2040/rp2040_usb.h index 7c3234e8..40b845a8 100644 --- a/src/portable/raspberrypi/rp2040/rp2040_usb.h +++ b/src/portable/raspberrypi/rp2040/rp2040_usb.h @@ -42,10 +42,6 @@ struct hw_endpoint { // Is this a valid struct bool configured; - // EP direction - bool in; - // EP num (not including direction) - uint8_t num; // Transfer direction (i.e. IN is rx for host but tx for device) // allows us to common up transfer functions From eb44b6f7db33e89c0d1dc369d914f18388deff53 Mon Sep 17 00:00:00 2001 From: Peter Lawrence <12226419+majbthrd@users.noreply.github.com> Date: Fri, 26 Feb 2021 11:07:34 -0600 Subject: [PATCH 2/2] rp2040: improve _hw_endpoint_init() --- src/portable/raspberrypi/rp2040/dcd_rp2040.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/portable/raspberrypi/rp2040/dcd_rp2040.c b/src/portable/raspberrypi/rp2040/dcd_rp2040.c index 787add15..d3287043 100644 --- a/src/portable/raspberrypi/rp2040/dcd_rp2040.c +++ b/src/portable/raspberrypi/rp2040/dcd_rp2040.c @@ -102,8 +102,8 @@ static void _hw_endpoint_alloc(struct hw_endpoint *ep) static void _hw_endpoint_init(struct hw_endpoint *ep, uint8_t ep_addr, uint16_t wMaxPacketSize, uint8_t transfer_type) { - uint8_t num = tu_edpt_number(ep_addr); - tusb_dir_t dir = tu_edpt_dir(ep_addr); + const uint8_t num = tu_edpt_number(ep_addr); + const tusb_dir_t dir = tu_edpt_dir(ep_addr); ep->ep_addr = ep_addr; // For device, IN is a tx transfer and OUT is an rx transfer ep->rx = (dir == TUSB_DIR_OUT); @@ -130,7 +130,7 @@ static void _hw_endpoint_init(struct hw_endpoint *ep, uint8_t ep_addr, uint16_t ep->transfer_type = transfer_type; // Every endpoint has a buffer control register in dpram - if (tu_edpt_dir(ep->ep_addr) == TUSB_DIR_IN) + if (dir == TUSB_DIR_IN) { ep->buffer_control = &usb_dpram->ep_buf_ctrl[num].in; } @@ -142,7 +142,7 @@ static void _hw_endpoint_init(struct hw_endpoint *ep, uint8_t ep_addr, uint16_t // Clear existing buffer control state *ep->buffer_control = 0; - if (tu_edpt_number(ep->ep_addr) == 0) + if (num == 0) { // EP0 has no endpoint control register because // the buffer offsets are fixed