Skip to content

Allow any gpio pin to be used (not just the first 32) #117

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: sdk-2.0.0
Choose a base branch
from
Open

Allow any gpio pin to be used (not just the first 32) #117

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c20d184
gitignore
georgeharker Oct 2, 2024
5aa7b5d
try and allow gpio > 32
georgeharker Sep 13, 2024
1987b08
Merge branch 'sdk-2.0.0' of github.com:georgeharker/openocd into sdk-…
georgeharker Oct 10, 2024
5261b66
Address PR comments
georgeharker Oct 10, 2024
8a4fecf
Update to reflect new unset constants
georgeharker Oct 10, 2024
55916b0
Replace bit check with >= 32
georgeharker Oct 10, 2024
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
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
*.lo
*.la
*.in
confdefs.h

# generated source files
src/jtag/minidriver_imp.h
Expand Down
106 changes: 74 additions & 32 deletions src/jtag/drivers/bcm2835gpio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ static off_t bcm2835_peri_base = 0x20000000;
*(pio_base+((g)/10)) |= ((m)<<(((g)%10)*3)); } while (0)
#define OUT_GPIO(g) SET_MODE_GPIO(g, BCM2835_GPIO_MODE_OUTPUT)

#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 GPIO_SET(g) (*(pio_base+7+((g&32)>>5))) /* sets bits which are 1, ignores bits which are 0 */
#define GPIO_CLR(g) (*(pio_base+10+((g&32)>>5))) /* clears bits which are 1, ignores bits which are 0 */
#define GPIO_LEV(g) (*(pio_base+13+((g&32)>>5))) /* current level of the pin */

static int dev_mem_fd;
static volatile uint32_t *pio_base = MAP_FAILED;
Expand Down Expand Up @@ -83,8 +83,11 @@ 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;
/* Only chip 0 is supported, accept unset value (-1) too, allow
* any gpio up to including 45 */
return (adapter_gpio_config[idx].chip_num == 0 ||
adapter_gpio_config[idx].chip_num == ADAPTER_GPIO_NOT_SET)
&& adapter_gpio_config[idx].gpio_num <= 45;
}

static void set_gpio_value(const struct adapter_gpio_config *gpio_config, int value)
Expand All @@ -93,9 +96,9 @@ 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;
GPIO_SET(gpio_config->gpio_num) = 1 << (gpio_config->gpio_num & 0x1f);
Copy link

@lurch lurch Oct 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO it looks a bit weird seeing gpio_config->gpio_num repeated on both sides of the = sign? But perhaps "fixing" that would be too much of a departure from the way the existing code was working? 🤷

Copy link
Author

@georgeharker georgeharker Oct 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree - could rewrite entirely if you'd like and make set an entire macro?

Copy link

@lurch lurch Oct 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really my call to make. What does @P33M think?

Copy link
Author

@georgeharker georgeharker Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@P33M any thoughts?

else
GPIO_CLR = 1 << gpio_config->gpio_num;
GPIO_CLR(gpio_config->gpio_num) = 1 << (gpio_config->gpio_num & 0x1f);
/* For performance reasons assume the GPIO is already set as an output
* and therefore the call can be omitted here.
*/
Expand All @@ -104,13 +107,13 @@ static void set_gpio_value(const struct adapter_gpio_config *gpio_config, int va
if (value) {
INP_GPIO(gpio_config->gpio_num);
} else {
GPIO_CLR = 1 << gpio_config->gpio_num;
GPIO_CLR(gpio_config->gpio_num) = 1 << (gpio_config->gpio_num & 0x1f);
OUT_GPIO(gpio_config->gpio_num);
}
break;
case ADAPTER_GPIO_DRIVE_MODE_OPEN_SOURCE:
if (value) {
GPIO_SET = 1 << gpio_config->gpio_num;
GPIO_SET(gpio_config->gpio_num) = 1 << (gpio_config->gpio_num & 0x1f);
OUT_GPIO(gpio_config->gpio_num);
} else {
INP_GPIO(gpio_config->gpio_num);
Expand All @@ -126,9 +129,9 @@ static void restore_gpio(enum adapter_gpio_config_index idx)
SET_MODE_GPIO(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;
GPIO_SET(adapter_gpio_config[idx].gpio_num) = 1 << (adapter_gpio_config[idx].gpio_num & 0x1f);
else
GPIO_CLR = 1 << adapter_gpio_config[idx].gpio_num;
GPIO_CLR(adapter_gpio_config[idx].gpio_num) = 1 << (adapter_gpio_config[idx].gpio_num & 0x1f);
}
}
bcm2835_gpio_synchronize();
Expand All @@ -140,8 +143,8 @@ static void initialize_gpio(enum adapter_gpio_config_index 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;
unsigned int shift = adapter_gpio_config[idx].gpio_num & 0x1f;
initial_gpio_state[idx].output_level = (GPIO_LEV(adapter_gpio_config[idx].gpio_num) >> shift) & 1;
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);
Expand Down Expand Up @@ -172,23 +175,46 @@ static void initialize_gpio(enum adapter_gpio_config_index idx)

static bb_value_t bcm2835gpio_read(void)
{
unsigned int shift = adapter_gpio_config[ADAPTER_GPIO_IDX_TDO].gpio_num;
uint32_t value = (GPIO_LEV >> shift) & 1;
unsigned int shift = adapter_gpio_config[ADAPTER_GPIO_IDX_TDO].gpio_num & 0x1f;
uint32_t value = (GPIO_LEV(adapter_gpio_config[ADAPTER_GPIO_IDX_TDO].gpio_num) >> shift) & 1;
return value ^ (adapter_gpio_config[ADAPTER_GPIO_IDX_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_SET = set;
GPIO_CLR = clear;
if ((adapter_gpio_config[ADAPTER_GPIO_IDX_TCK].gpio_num >= 32) ==
(adapter_gpio_config[ADAPTER_GPIO_IDX_TMS].gpio_num >= 32) &&
(adapter_gpio_config[ADAPTER_GPIO_IDX_TMS].gpio_num >= 32) ==
(adapter_gpio_config[ADAPTER_GPIO_IDX_TDI].gpio_num >= 32)) {
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_SET(adapter_gpio_config[ADAPTER_GPIO_IDX_TCK].gpio_num) = set;
GPIO_CLR(adapter_gpio_config[ADAPTER_GPIO_IDX_TCK].gpio_num) = clear;
} else {
uint32_t set = tck << adapter_gpio_config[ADAPTER_GPIO_IDX_TCK].gpio_num;
uint32_t clear = !tck << adapter_gpio_config[ADAPTER_GPIO_IDX_TCK].gpio_num;

GPIO_SET(adapter_gpio_config[ADAPTER_GPIO_IDX_TCK].gpio_num) = set;
GPIO_CLR(adapter_gpio_config[ADAPTER_GPIO_IDX_TCK].gpio_num) = clear;

set = tms << adapter_gpio_config[ADAPTER_GPIO_IDX_TMS].gpio_num;
clear = !tms << adapter_gpio_config[ADAPTER_GPIO_IDX_TMS].gpio_num;

GPIO_SET(adapter_gpio_config[ADAPTER_GPIO_IDX_TMS].gpio_num) = set;
GPIO_CLR(adapter_gpio_config[ADAPTER_GPIO_IDX_TMS].gpio_num) = clear;

set = tdi << adapter_gpio_config[ADAPTER_GPIO_IDX_TDI].gpio_num;
clear = !tdi << adapter_gpio_config[ADAPTER_GPIO_IDX_TDI].gpio_num;

GPIO_SET(adapter_gpio_config[ADAPTER_GPIO_IDX_TDI].gpio_num) = set;
GPIO_CLR(adapter_gpio_config[ADAPTER_GPIO_IDX_TDI].gpio_num) = clear;
}
bcm2835_gpio_synchronize();

bcm2835_delay();
Expand All @@ -202,13 +228,29 @@ 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);

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;
if ((adapter_gpio_config[ADAPTER_GPIO_IDX_SWCLK].gpio_num & 32) ==
(adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].gpio_num & 32)) {

GPIO_SET = set;
GPIO_CLR = clear;
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(adapter_gpio_config[ADAPTER_GPIO_IDX_SWCLK].gpio_num) = set;
GPIO_CLR(adapter_gpio_config[ADAPTER_GPIO_IDX_SWCLK].gpio_num) = clear;
} else {
uint32_t set = swclk << adapter_gpio_config[ADAPTER_GPIO_IDX_SWCLK].gpio_num;
uint32_t clear = !swclk << adapter_gpio_config[ADAPTER_GPIO_IDX_SWCLK].gpio_num;

GPIO_SET(adapter_gpio_config[ADAPTER_GPIO_IDX_SWCLK].gpio_num) = set;
GPIO_CLR(adapter_gpio_config[ADAPTER_GPIO_IDX_SWCLK].gpio_num) = clear;

set = swdio << adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].gpio_num;
clear = !swdio << adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].gpio_num;

GPIO_SET(adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].gpio_num) = set;
GPIO_CLR(adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].gpio_num) = clear;
}
bcm2835_gpio_synchronize();

bcm2835_delay();
Expand Down Expand Up @@ -266,8 +308,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;
unsigned int shift = adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].gpio_num & 0x1f;
uint32_t value = (GPIO_LEV(adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].gpio_num) >> shift) & 1;
return value ^ (adapter_gpio_config[ADAPTER_GPIO_IDX_SWDIO].active_low ? 1 : 0);
}

Expand Down