From 0d3d4c981ac77b600ce95c9ea6f1cdb280127342 Mon Sep 17 00:00:00 2001 From: Vincent Fazio Date: Wed, 31 Jan 2024 09:06:20 -0600 Subject: [PATCH] jtag/adapter: retype adapter_gpio_config.{gpio,chip}_num Previously, the gpio_num and chip_num members of adapter_gpio_config were typed as 'int' and a sentinel value of -1 was used to denote unconfigured values. Now, these members are typed as 'unsigned int' to better reflect their expected value range. The sentinel value now maps to UINT_MAX as all adapters either define an upper bound for these members or, in the case of bcm2835gpio, only operate on a specific chip, in which case the value doesn't matter. Format specifiers have been left as %d since, when configured, valid values are within the positive range of 'int'. This allows unconfigured values to display as a more readable value of -1 instead of UINT_MAX. Change-Id: Ieb20e5327b2e2e443a8e43d8689cb29538a5c9c1 Signed-off-by: Vincent Fazio Reviewed-on: https://review.openocd.org/c/openocd/+/8124 Tested-by: jenkins Reviewed-by: Tomas Vanek --- src/jtag/adapter.c | 22 ++++++++++++---------- src/jtag/adapter.h | 7 +++++-- src/jtag/drivers/am335xgpio.c | 15 ++++++++------- src/jtag/drivers/bcm2835gpio.c | 16 ++++++++-------- src/jtag/drivers/linuxgpiod.c | 4 +--- 5 files changed, 34 insertions(+), 30 deletions(-) diff --git a/src/jtag/adapter.c b/src/jtag/adapter.c index e70f4a1e8..bbf1cb3d2 100644 --- a/src/jtag/adapter.c +++ b/src/jtag/adapter.c @@ -94,8 +94,9 @@ static void adapter_driver_gpios_init(void) return; for (int i = 0; i < ADAPTER_GPIO_IDX_NUM; ++i) { - adapter_config.gpios[i].gpio_num = -1; - adapter_config.gpios[i].chip_num = -1; + /* Use ADAPTER_GPIO_NOT_SET as the sentinel 'unset' value. */ + adapter_config.gpios[i].gpio_num = ADAPTER_GPIO_NOT_SET; + adapter_config.gpios[i].chip_num = ADAPTER_GPIO_NOT_SET; if (gpio_map[i].direction == ADAPTER_GPIO_DIRECTION_INPUT) adapter_config.gpios[i].init_state = ADAPTER_GPIO_INIT_STATE_INPUT; } @@ -848,6 +849,11 @@ static COMMAND_HELPER(helper_adapter_gpio_print_config, enum adapter_gpio_config const char *pull = ""; const char *init_state = ""; + if (gpio_config->gpio_num == ADAPTER_GPIO_NOT_SET) { + command_print(CMD, "adapter gpio %s: not configured", gpio_map[gpio_idx].name); + return ERROR_OK; + } + switch (gpio_map[gpio_idx].direction) { case ADAPTER_GPIO_DIRECTION_INPUT: dir = "input"; @@ -900,8 +906,8 @@ static COMMAND_HELPER(helper_adapter_gpio_print_config, enum adapter_gpio_config } } - command_print(CMD, "adapter gpio %s (%s): num %d, chip %d, active-%s%s%s%s", - gpio_map[gpio_idx].name, dir, gpio_config->gpio_num, gpio_config->chip_num, active_state, + command_print(CMD, "adapter gpio %s (%s): num %u, chip %d, active-%s%s%s%s", + gpio_map[gpio_idx].name, dir, gpio_config->gpio_num, (int)gpio_config->chip_num, active_state, drive, pull, init_state); return ERROR_OK; @@ -942,9 +948,7 @@ COMMAND_HANDLER(adapter_gpio_config_handler) LOG_DEBUG("Processing %s", CMD_ARGV[i]); if (isdigit(*CMD_ARGV[i])) { - int gpio_num; /* Use a meaningful output parameter for more helpful error messages */ - COMMAND_PARSE_NUMBER(int, CMD_ARGV[i], gpio_num); - gpio_config->gpio_num = gpio_num; + COMMAND_PARSE_NUMBER(uint, CMD_ARGV[i], gpio_config->gpio_num); ++i; continue; } @@ -955,9 +959,7 @@ COMMAND_HANDLER(adapter_gpio_config_handler) return ERROR_FAIL; } LOG_DEBUG("-chip arg is %s", CMD_ARGV[i + 1]); - int chip_num; /* Use a meaningful output parameter for more helpful error messages */ - COMMAND_PARSE_NUMBER(int, CMD_ARGV[i + 1], chip_num); - gpio_config->chip_num = chip_num; + COMMAND_PARSE_NUMBER(uint, CMD_ARGV[i + 1], gpio_config->chip_num); i += 2; continue; } diff --git a/src/jtag/adapter.h b/src/jtag/adapter.h index 682fc10ae..23ffe2cc5 100644 --- a/src/jtag/adapter.h +++ b/src/jtag/adapter.h @@ -10,6 +10,7 @@ #include #include #include +#include /** Supported output drive modes for adaptor GPIO */ enum adapter_gpio_drive_mode { @@ -56,8 +57,8 @@ enum adapter_gpio_config_index { /** Configuration options for a single GPIO */ struct adapter_gpio_config { - int gpio_num; - int chip_num; + unsigned int gpio_num; + unsigned int chip_num; enum adapter_gpio_drive_mode drive; /* For outputs only */ enum adapter_gpio_init_state init_state; bool active_low; @@ -121,4 +122,6 @@ const char *adapter_gpio_get_name(enum adapter_gpio_config_index idx); */ const struct adapter_gpio_config *adapter_gpio_get_config(void); +#define ADAPTER_GPIO_NOT_SET UINT_MAX + #endif /* OPENOCD_JTAG_ADAPTER_H */ diff --git a/src/jtag/drivers/am335xgpio.c b/src/jtag/drivers/am335xgpio.c index 29d410118..cfe41c3be 100644 --- a/src/jtag/drivers/am335xgpio.c +++ b/src/jtag/drivers/am335xgpio.c @@ -86,9 +86,7 @@ static const struct adapter_gpio_config *adapter_gpio_config; static bool is_gpio_config_valid(const struct adapter_gpio_config *gpio_config) { - return gpio_config->chip_num >= 0 - && gpio_config->chip_num < AM335XGPIO_NUM_GPIO_CHIPS - && gpio_config->gpio_num >= 0 + return gpio_config->chip_num < AM335XGPIO_NUM_GPIO_CHIPS && gpio_config->gpio_num < AM335XGPIO_NUM_GPIO_PER_CHIP; } @@ -249,10 +247,13 @@ static int am335xgpio_reset(int trst, int srst) if (is_gpio_config_valid(&adapter_gpio_config[ADAPTER_GPIO_IDX_TRST])) set_gpio_value(&adapter_gpio_config[ADAPTER_GPIO_IDX_TRST], trst); - LOG_DEBUG("am335xgpio_reset(%d, %d), trst_gpio: %d %d, srst_gpio: %d %d", - trst, srst, - adapter_gpio_config[ADAPTER_GPIO_IDX_TRST].chip_num, adapter_gpio_config[ADAPTER_GPIO_IDX_TRST].gpio_num, - adapter_gpio_config[ADAPTER_GPIO_IDX_SRST].chip_num, adapter_gpio_config[ADAPTER_GPIO_IDX_SRST].gpio_num); + LOG_DEBUG("trst %d gpio: %d %d, srst %d gpio: %d %d", + trst, + (int)adapter_gpio_config[ADAPTER_GPIO_IDX_TRST].chip_num, + (int)adapter_gpio_config[ADAPTER_GPIO_IDX_TRST].gpio_num, + srst, + (int)adapter_gpio_config[ADAPTER_GPIO_IDX_SRST].chip_num, + (int)adapter_gpio_config[ADAPTER_GPIO_IDX_SRST].gpio_num); return ERROR_OK; } diff --git a/src/jtag/drivers/bcm2835gpio.c b/src/jtag/drivers/bcm2835gpio.c index 7fd7f3894..ff10b0a78 100644 --- a/src/jtag/drivers/bcm2835gpio.c +++ b/src/jtag/drivers/bcm2835gpio.c @@ -84,10 +84,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].chip_num >= -1 - && adapter_gpio_config[idx].chip_num <= 0 - && adapter_gpio_config[idx].gpio_num >= 0 - && adapter_gpio_config[idx].gpio_num <= 31; + return adapter_gpio_config[idx].gpio_num <= 31; } static void set_gpio_value(const struct adapter_gpio_config *gpio_config, int value) @@ -243,10 +240,13 @@ static int bcm2835gpio_reset(int trst, int srst) if (is_gpio_config_valid(ADAPTER_GPIO_IDX_TRST)) set_gpio_value(&adapter_gpio_config[ADAPTER_GPIO_IDX_TRST], trst); - LOG_DEBUG("BCM2835 GPIO: bcm2835gpio_reset(%d, %d), trst_gpio: %d %d, srst_gpio: %d %d", - trst, srst, - adapter_gpio_config[ADAPTER_GPIO_IDX_TRST].chip_num, adapter_gpio_config[ADAPTER_GPIO_IDX_TRST].gpio_num, - adapter_gpio_config[ADAPTER_GPIO_IDX_SRST].chip_num, adapter_gpio_config[ADAPTER_GPIO_IDX_SRST].gpio_num); + LOG_DEBUG("trst %d gpio: %d %d, srst %d gpio: %d %d", + trst, + (int)adapter_gpio_config[ADAPTER_GPIO_IDX_TRST].chip_num, + (int)adapter_gpio_config[ADAPTER_GPIO_IDX_TRST].gpio_num, + srst, + (int)adapter_gpio_config[ADAPTER_GPIO_IDX_SRST].chip_num, + (int)adapter_gpio_config[ADAPTER_GPIO_IDX_SRST].gpio_num); return ERROR_OK; } diff --git a/src/jtag/drivers/linuxgpiod.c b/src/jtag/drivers/linuxgpiod.c index d1a88c88d..942883788 100644 --- a/src/jtag/drivers/linuxgpiod.c +++ b/src/jtag/drivers/linuxgpiod.c @@ -37,9 +37,7 @@ static const struct adapter_gpio_config *adapter_gpio_config; */ static bool is_gpio_config_valid(enum adapter_gpio_config_index idx) { - return adapter_gpio_config[idx].chip_num >= 0 - && adapter_gpio_config[idx].chip_num < 1000 - && adapter_gpio_config[idx].gpio_num >= 0 + return adapter_gpio_config[idx].chip_num < 1000 && adapter_gpio_config[idx].gpio_num < 10000; }