From 0021ee2f5d515885c5f7affaf438129fbaf7c8a4 Mon Sep 17 00:00:00 2001 From: Vincent Fazio Date: Fri, 2 Jun 2023 18:22:19 +0000 Subject: [PATCH] jtag/drivers/bcm2835gpio: Support all 54 GPIO pins Previously, only the first 32 GPIO were supported on the BCM2835. Performance was cited as being the primary justification for not supporting all 54 pins, notably: 1. There is overhead for calculating the memory offset for the pin 2. GPIO values cannot be written in bulk if pins span memory offsets Now, all 54 GPIO pins are supported by the driver. Since pins may use different offsets, multiple pins cannot be toggled with one memory store. Multiple stores now need to occur when one sufficed before. To offset some of the performance overhead for the additional stores, memory addresses, masks, and shift bits are calculated once and cached into struct. Calculating these once reduces the number of instructions a function needs to run in order to manipulate a given GPIO. The following functions have been updated to leverage the new struct as they represent some of the hottest paths: bcm2835_swdio_drive bcm2835_swdio_read bcm2835gpio_swd_write_fast bcm2835gpio_read bcm2835gpio_write For `bcm2835gpio_swd_write_fast`, performance should be roughly the same as the number of memory stores hasn't changed. For `bcm2835_write`, there is a slight performance degradation since TMS/TDI/TCK are set separately which incurs an additional memory store. Instruction counts across the above functions are reduced by ~10-40%. Macros to access registers have been reworked into inline functions to support access to all pins and to avoid checkpatch headaches. The `initial_gpio_state.output_level` member has been retyped to bool to better align with the expected values. Support for adjusting pads for the expanded pin range has been left out as support for manipulating these settings should be moved out of this driver and into its own utility. Change-Id: I18853d1a2c86776658630326c71a6bf236fcc6da Signed-off-by: Vincent Fazio Reviewed-on: https://review.openocd.org/c/openocd/+/7732 Reviewed-by: Tomas Vanek Tested-by: jenkins --- doc/openocd.texi | 3 +- src/jtag/drivers/bcm2835gpio.c | 214 +++++++++++++++++++++++---------- 2 files changed, 153 insertions(+), 64 deletions(-) diff --git a/doc/openocd.texi b/doc/openocd.texi index f2ffa3a75..63d07533e 100644 --- a/doc/openocd.texi +++ b/doc/openocd.texi @@ -3472,8 +3472,7 @@ able to coexist nicely with both sysfs bitbanging and various peripherals' kernel drivers. The driver restores the previous configuration on exit. -GPIO numbers >= 32 can't be used for performance reasons. GPIO configuration is -handled by the generic command @ref{adapter gpio, @command{adapter gpio}}. +GPIO configuration is handled by the generic command @ref{adapter gpio, @command{adapter gpio}}. See @file{interface/raspberrypi-native.cfg} for a sample config and @file{interface/raspberrypi-gpio-connector.cfg} for pinout. diff --git a/src/jtag/drivers/bcm2835gpio.c b/src/jtag/drivers/bcm2835gpio.c index 1a105aac4..eccbe06b8 100644 --- a/src/jtag/drivers/bcm2835gpio.c +++ b/src/jtag/drivers/bcm2835gpio.c @@ -31,28 +31,22 @@ static off_t bcm2835_peri_base = 0x20000000; #define BCM2835_GPIO_MODE_OUTPUT 1 /* GPIO setup macros */ -#define MODE_GPIO(_g) ({ \ - typeof(_g) g = (_g); \ - *(pio_base + (g / 10)) >> ((g % 10) * 3) & 7; \ -}) +#define BCM2835_GPIO_REG_READ(offset) \ + (*(pio_base + (offset))) -#define INP_GPIO(_g) do { \ - typeof(_g) g1 = (_g); \ - *(pio_base + (g1 / 10)) &= ~(7 << ((g1 % 10) * 3)); \ -} while (0) +#define BCM2835_GPIO_REG_WRITE(offset, value) \ + (*(pio_base + (offset)) = (value)) -#define SET_MODE_GPIO(_g, m) do { \ - typeof(_g) g = (_g); \ - /* clear the mode bits first, then set as necessary */ \ - INP_GPIO(g); \ - *(pio_base + (g / 10)) |= ((m) << ((g % 10) * 3)); \ -} while (0) +#define BCM2835_GPIO_SET_REG_BITS(offset, bit_mask) \ + (*(pio_base + (offset)) |= (bit_mask)) -#define OUT_GPIO(g) SET_MODE_GPIO(g, BCM2835_GPIO_MODE_OUTPUT) +#define BCM2835_GPIO_CLEAR_REG_BITS(offset, bit_mask) \ + (*(pio_base + (offset)) &= ~(bit_mask)) -#define GPIO_SET (*(pio_base + 7)) /* sets bits which are 1, ignores bits which are 0 */ -#define GPIO_CLR (*(pio_base + 10)) /* clears bits which are 1, ignores bits which are 0 */ -#define GPIO_LEV (*(pio_base + 13)) /* current level of the pin */ +#define BCM2835_GPIO_MODE_ADDR(gpio_pin_num) (pio_base + ((gpio_pin_num) / 10)) +#define BCM2835_GPIO_SET_ADDR(gpio_pin_num) (pio_base + 7 + ((gpio_pin_num) / 32)) +#define BCM2835_GPIO_CLR_ADDR(gpio_pin_num) (pio_base + 10 + ((gpio_pin_num) / 32)) +#define BCM2835_GPIO_LEVEL_ADDR(gpio_pin_num) (pio_base + 13 + ((gpio_pin_num) / 32)) static int dev_mem_fd; static volatile uint32_t *pio_base = MAP_FAILED; @@ -66,10 +60,75 @@ static unsigned int jtag_delay; static const struct adapter_gpio_config *adapter_gpio_config; static struct initial_gpio_state { unsigned int mode; - unsigned int output_level; + bool output_level; } initial_gpio_state[ADAPTER_GPIO_IDX_NUM]; static uint32_t initial_drive_strength_etc; +static struct { + volatile uint32_t *swdio_clr_set_addr[2]; + uint32_t swdio_mask; + volatile uint32_t *swdio_read_level_addr; + uint32_t swdio_level_shift_bits; + bool swdio_active_low; + volatile uint32_t *swdio_mode_addr; + uint32_t swdio_mode_input_mask; + uint32_t swdio_mode_output_mask; + volatile uint32_t *swclk_clr_set_addr[2]; + uint32_t swclk_mask; + volatile uint32_t *tdi_clr_set_addr[2]; + uint32_t tdi_mask; + volatile uint32_t *tms_clr_set_addr[2]; + uint32_t tms_mask; + volatile uint32_t *tck_clr_set_addr[2]; + uint32_t tck_mask; + volatile uint32_t *tdo_read_level_addr; + uint32_t tdo_level_shift_bits; + bool tdo_active_low; +} gpio_control; + +/* GPFSEL[0,5], read the function select bits of specified GPIO pin number */ +static inline unsigned int bcm2835_get_mode(unsigned int gpio_pin_num) +{ + return (BCM2835_GPIO_REG_READ((gpio_pin_num / 10)) >> ((gpio_pin_num % 10) * 3) & 7); +} + +/* GPLEV[13,14], current level of the pin */ +static inline bool bcm2835_get_level(unsigned int gpio_pin_num) +{ + return ((BCM2835_GPIO_REG_READ((13 + (gpio_pin_num / 32))) >> (gpio_pin_num % 32)) & 1); +} + +/* set GPIO pin as input */ +static inline void bcm2835_set_input(unsigned int gpio_pin_num) +{ + BCM2835_GPIO_CLEAR_REG_BITS((gpio_pin_num / 10), (7 << ((gpio_pin_num % 10) * 3))); +} + +/* clear the mode bits first, then set as necessary */ +static inline void bcm2835_set_mode(unsigned int gpio_pin_num, unsigned char mode) +{ + bcm2835_set_input(gpio_pin_num); + BCM2835_GPIO_SET_REG_BITS((gpio_pin_num / 10), (mode << ((gpio_pin_num % 10) * 3))); +} + +/* set GPIO pin as output */ +static inline void bcm2835_set_output(unsigned int gpio_pin_num) +{ + bcm2835_set_mode(gpio_pin_num, BCM2835_GPIO_MODE_OUTPUT); +} + +/* GPSET[7,8], sets bits which are 1, ignores bits which are 0 */ +static inline void bcm2835_gpio_set(unsigned int gpio_pin_num) +{ + BCM2835_GPIO_REG_WRITE((7 + (gpio_pin_num / 32)), (1 << (gpio_pin_num % 32))); +} + +/* GPCLR[10,11], clears bits which are 1, ignores bits which are 0 */ +static inline void bcm2835_gpio_clear(unsigned int gpio_pin_num) +{ + BCM2835_GPIO_REG_WRITE((10 + (gpio_pin_num / 32)), (1 << (gpio_pin_num % 32))); +} + static inline const char *bcm2835_get_mem_dev(void) { if (bcm2835_peri_mem_dev) @@ -96,7 +155,7 @@ static inline void bcm2835_delay(void) static bool is_gpio_config_valid(enum adapter_gpio_config_index idx) { /* Only chip 0 is supported, accept unset value (-1) too */ - return adapter_gpio_config[idx].gpio_num <= 31; + return adapter_gpio_config[idx].gpio_num <= 53; } static void set_gpio_value(const struct adapter_gpio_config *gpio_config, int value) @@ -105,27 +164,27 @@ static void set_gpio_value(const struct adapter_gpio_config *gpio_config, int va switch (gpio_config->drive) { case ADAPTER_GPIO_DRIVE_MODE_PUSH_PULL: if (value) - GPIO_SET = 1 << gpio_config->gpio_num; + bcm2835_gpio_set(gpio_config->gpio_num); else - GPIO_CLR = 1 << gpio_config->gpio_num; + bcm2835_gpio_clear(gpio_config->gpio_num); /* For performance reasons assume the GPIO is already set as an output * and therefore the call can be omitted here. */ break; case ADAPTER_GPIO_DRIVE_MODE_OPEN_DRAIN: if (value) { - INP_GPIO(gpio_config->gpio_num); + bcm2835_set_input(gpio_config->gpio_num); } else { - GPIO_CLR = 1 << gpio_config->gpio_num; - OUT_GPIO(gpio_config->gpio_num); + bcm2835_gpio_clear(gpio_config->gpio_num); + bcm2835_set_output(gpio_config->gpio_num); } break; case ADAPTER_GPIO_DRIVE_MODE_OPEN_SOURCE: if (value) { - GPIO_SET = 1 << gpio_config->gpio_num; - OUT_GPIO(gpio_config->gpio_num); + bcm2835_gpio_set(gpio_config->gpio_num); + bcm2835_set_output(gpio_config->gpio_num); } else { - INP_GPIO(gpio_config->gpio_num); + bcm2835_set_input(gpio_config->gpio_num); } break; } @@ -135,12 +194,12 @@ static void set_gpio_value(const struct adapter_gpio_config *gpio_config, int va static void restore_gpio(enum adapter_gpio_config_index idx) { if (is_gpio_config_valid(idx)) { - SET_MODE_GPIO(adapter_gpio_config[idx].gpio_num, initial_gpio_state[idx].mode); + bcm2835_set_mode(adapter_gpio_config[idx].gpio_num, initial_gpio_state[idx].mode); if (initial_gpio_state[idx].mode == BCM2835_GPIO_MODE_OUTPUT) { if (initial_gpio_state[idx].output_level) - GPIO_SET = 1 << adapter_gpio_config[idx].gpio_num; + bcm2835_gpio_set(adapter_gpio_config[idx].gpio_num); else - GPIO_CLR = 1 << adapter_gpio_config[idx].gpio_num; + bcm2835_gpio_clear(adapter_gpio_config[idx].gpio_num); } } bcm2835_gpio_synchronize(); @@ -151,9 +210,8 @@ static void initialize_gpio(enum adapter_gpio_config_index idx) if (!is_gpio_config_valid(idx)) return; - initial_gpio_state[idx].mode = MODE_GPIO(adapter_gpio_config[idx].gpio_num); - unsigned int shift = adapter_gpio_config[idx].gpio_num; - initial_gpio_state[idx].output_level = (GPIO_LEV >> shift) & 1; + initial_gpio_state[idx].mode = bcm2835_get_mode(adapter_gpio_config[idx].gpio_num); + initial_gpio_state[idx].output_level = bcm2835_get_level(adapter_gpio_config[idx].gpio_num); LOG_DEBUG("saved GPIO mode for %s (GPIO %d %d): %d", adapter_gpio_get_name(idx), adapter_gpio_config[idx].chip_num, adapter_gpio_config[idx].gpio_num, initial_gpio_state[idx].mode); @@ -171,35 +229,29 @@ static void initialize_gpio(enum adapter_gpio_config_index idx) set_gpio_value(&adapter_gpio_config[idx], 1); break; case ADAPTER_GPIO_INIT_STATE_INPUT: - INP_GPIO(adapter_gpio_config[idx].gpio_num); + bcm2835_set_input(adapter_gpio_config[idx].gpio_num); break; } /* Direction for non push-pull is already set by set_gpio_value() */ if (adapter_gpio_config[idx].drive == ADAPTER_GPIO_DRIVE_MODE_PUSH_PULL && adapter_gpio_config[idx].init_state != ADAPTER_GPIO_INIT_STATE_INPUT) - OUT_GPIO(adapter_gpio_config[idx].gpio_num); + bcm2835_set_output(adapter_gpio_config[idx].gpio_num); bcm2835_gpio_synchronize(); } static enum bb_value bcm2835gpio_read(void) { - unsigned int shift = adapter_gpio_config[ADAPTER_GPIO_IDX_TDO].gpio_num; - uint32_t value = (GPIO_LEV >> shift) & 1; - return value ^ (adapter_gpio_config[ADAPTER_GPIO_IDX_TDO].active_low ? BB_HIGH : BB_LOW); + bool value = ((*gpio_control.tdo_read_level_addr >> gpio_control.tdo_level_shift_bits) & 1); + return (value ^ gpio_control.tdo_active_low) ? BB_HIGH : BB_LOW; } static int bcm2835gpio_write(int tck, int tms, int tdi) { - uint32_t set = tck << adapter_gpio_config[ADAPTER_GPIO_IDX_TCK].gpio_num | - tms << adapter_gpio_config[ADAPTER_GPIO_IDX_TMS].gpio_num | - tdi << adapter_gpio_config[ADAPTER_GPIO_IDX_TDI].gpio_num; - uint32_t clear = !tck << adapter_gpio_config[ADAPTER_GPIO_IDX_TCK].gpio_num | - !tms << adapter_gpio_config[ADAPTER_GPIO_IDX_TMS].gpio_num | - !tdi << adapter_gpio_config[ADAPTER_GPIO_IDX_TDI].gpio_num; + *gpio_control.tdi_clr_set_addr[tdi] = gpio_control.tdi_mask; + *gpio_control.tms_clr_set_addr[tms] = gpio_control.tms_mask; + *gpio_control.tck_clr_set_addr[tck] = gpio_control.tck_mask; /* Write clock last */ - GPIO_SET = set; - GPIO_CLR = clear; bcm2835_gpio_synchronize(); bcm2835_delay(); @@ -210,16 +262,9 @@ static int bcm2835gpio_write(int tck, int tms, int tdi) /* Requires push-pull drive mode for swclk and swdio */ static int bcm2835gpio_swd_write_fast(int swclk, int swdio) { - swclk = swclk ^ (adapter_gpio_config[ADAPTER_GPIO_IDX_SWCLK].active_low ? 1 : 0); - swdio = swdio ^ (adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].active_low ? 1 : 0); + *gpio_control.swdio_clr_set_addr[swdio] = gpio_control.swdio_mask; + *gpio_control.swclk_clr_set_addr[swclk] = gpio_control.swclk_mask; /* Write clock last */ - uint32_t set = swclk << adapter_gpio_config[ADAPTER_GPIO_IDX_SWCLK].gpio_num | - swdio << adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].gpio_num; - uint32_t clear = !swclk << adapter_gpio_config[ADAPTER_GPIO_IDX_SWCLK].gpio_num | - !swdio << adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].gpio_num; - - GPIO_SET = set; - GPIO_CLR = clear; bcm2835_gpio_synchronize(); bcm2835_delay(); @@ -266,9 +311,11 @@ static void bcm2835_swdio_drive(bool is_output) if (is_output) { if (is_gpio_config_valid(ADAPTER_GPIO_IDX_SWDIO_DIR)) set_gpio_value(&adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO_DIR], 1); - OUT_GPIO(adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].gpio_num); + /* per bcm2835_set_output, clear mode bits then set the pin to output */ + *gpio_control.swdio_mode_addr = (gpio_control.swdio_mode_output_mask | + (*gpio_control.swdio_mode_addr & gpio_control.swdio_mode_input_mask)); } else { - INP_GPIO(adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].gpio_num); + *gpio_control.swdio_mode_addr &= gpio_control.swdio_mode_input_mask; if (is_gpio_config_valid(ADAPTER_GPIO_IDX_SWDIO_DIR)) set_gpio_value(&adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO_DIR], 0); } @@ -277,9 +324,8 @@ static void bcm2835_swdio_drive(bool is_output) static int bcm2835_swdio_read(void) { - unsigned int shift = adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].gpio_num; - uint32_t value = (GPIO_LEV >> shift) & 1; - return value ^ (adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].active_low ? 1 : 0); + bool value = ((*gpio_control.swdio_read_level_addr >> gpio_control.swdio_level_shift_bits) & 1); + return (int)(value ^ gpio_control.swdio_active_low); } static int bcm2835gpio_khz(int khz, int *jtag_speed) @@ -515,6 +561,26 @@ LOG_INFO("pads conf set to %08x", pads_base[BCM2835_PADS_GPIO_0_27_OFFSET]); initialize_gpio(ADAPTER_GPIO_IDX_TMS); initialize_gpio(ADAPTER_GPIO_IDX_TCK); initialize_gpio(ADAPTER_GPIO_IDX_TRST); + + /* flip the addresses used when pins are active low */ + unsigned char idx = adapter_gpio_config[ADAPTER_GPIO_IDX_TMS].active_low ? 1 : 0; + gpio_control.tms_clr_set_addr[idx] = BCM2835_GPIO_CLR_ADDR(adapter_gpio_config[ADAPTER_GPIO_IDX_TMS].gpio_num); + gpio_control.tms_clr_set_addr[!idx] = BCM2835_GPIO_SET_ADDR(adapter_gpio_config[ADAPTER_GPIO_IDX_TMS].gpio_num); + gpio_control.tms_mask = (1 << (adapter_gpio_config[ADAPTER_GPIO_IDX_TMS].gpio_num % 32)); + + idx = adapter_gpio_config[ADAPTER_GPIO_IDX_TDI].active_low ? 1 : 0; + gpio_control.tdi_clr_set_addr[idx] = BCM2835_GPIO_CLR_ADDR(adapter_gpio_config[ADAPTER_GPIO_IDX_TDI].gpio_num); + gpio_control.tdi_clr_set_addr[!idx] = BCM2835_GPIO_SET_ADDR(adapter_gpio_config[ADAPTER_GPIO_IDX_TDI].gpio_num); + gpio_control.tdi_mask = (1 << (adapter_gpio_config[ADAPTER_GPIO_IDX_TDI].gpio_num % 32)); + + idx = adapter_gpio_config[ADAPTER_GPIO_IDX_TCK].active_low ? 1 : 0; + gpio_control.tck_clr_set_addr[idx] = BCM2835_GPIO_CLR_ADDR(adapter_gpio_config[ADAPTER_GPIO_IDX_TCK].gpio_num); + gpio_control.tck_clr_set_addr[!idx] = BCM2835_GPIO_SET_ADDR(adapter_gpio_config[ADAPTER_GPIO_IDX_TCK].gpio_num); + gpio_control.tck_mask = (1 << (adapter_gpio_config[ADAPTER_GPIO_IDX_TCK].gpio_num % 32)); + + gpio_control.tdo_read_level_addr = BCM2835_GPIO_LEVEL_ADDR(adapter_gpio_config[ADAPTER_GPIO_IDX_TDO].gpio_num); + gpio_control.tdo_level_shift_bits = (adapter_gpio_config[ADAPTER_GPIO_IDX_TDO].gpio_num % 32); + gpio_control.tdo_active_low = adapter_gpio_config[ADAPTER_GPIO_IDX_TDO].active_low; } const struct bitbang_interface *bcm2835gpio_bitbang = &bcm2835gpio_bitbang_swd_write_generic; @@ -538,6 +604,30 @@ LOG_INFO("pads conf set to %08x", pads_base[BCM2835_PADS_GPIO_0_27_OFFSET]); if (adapter_gpio_config[ADAPTER_GPIO_IDX_SWCLK].drive == ADAPTER_GPIO_DRIVE_MODE_PUSH_PULL && adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].drive == ADAPTER_GPIO_DRIVE_MODE_PUSH_PULL) { + /* flip the addresses used when pins are active low */ + unsigned char idx = adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].active_low ? 1 : 0; + gpio_control.swdio_clr_set_addr[idx] = + BCM2835_GPIO_CLR_ADDR(adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].gpio_num); + gpio_control.swdio_clr_set_addr[!idx] = + BCM2835_GPIO_SET_ADDR(adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].gpio_num); + gpio_control.swdio_mask = (1 << (adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].gpio_num % 32)); + gpio_control.swdio_read_level_addr = + BCM2835_GPIO_LEVEL_ADDR(adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].gpio_num); + gpio_control.swdio_level_shift_bits = (adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].gpio_num % 32); + gpio_control.swdio_active_low = adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].active_low; + gpio_control.swdio_mode_addr = BCM2835_GPIO_MODE_ADDR(adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].gpio_num); + gpio_control.swdio_mode_input_mask = + ~(7 << ((adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].gpio_num % 10) * 3)); + gpio_control.swdio_mode_output_mask = + (1 << ((adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].gpio_num % 10) * 3)); + + idx = adapter_gpio_config[ADAPTER_GPIO_IDX_SWCLK].active_low ? 1 : 0; + gpio_control.swclk_clr_set_addr[idx] = + BCM2835_GPIO_CLR_ADDR(adapter_gpio_config[ADAPTER_GPIO_IDX_SWCLK].gpio_num); + gpio_control.swclk_clr_set_addr[!idx] = + BCM2835_GPIO_SET_ADDR(adapter_gpio_config[ADAPTER_GPIO_IDX_SWCLK].gpio_num); + gpio_control.swclk_mask = (1 << (adapter_gpio_config[ADAPTER_GPIO_IDX_SWCLK].gpio_num % 32)); + LOG_DEBUG("BCM2835 GPIO using fast mode for SWD write"); bcm2835gpio_bitbang = &bcm2835gpio_bitbang_swd_write_fast; } else { @@ -570,7 +660,7 @@ static int bcm2835gpio_quit(void) * current and final states of swdio and swdio_dir do not have to be * considered to calculate the safe restoration order. */ - INP_GPIO(adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].gpio_num); + bcm2835_set_input(adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].gpio_num); restore_gpio(ADAPTER_GPIO_IDX_SWDIO_DIR); restore_gpio(ADAPTER_GPIO_IDX_SWDIO); restore_gpio(ADAPTER_GPIO_IDX_SWCLK);