Skip to content
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

Jump to bottom

feat: Add support for inverting reset and boot pins (ESF-177) #120

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: Add support for inverting reset and boot pins (ESF-177) #120

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 16 additions & 0 deletions Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,20 @@ menu "ESP serial flasher"
int "Number of retries when writing blocks either to target flash or RAM"
default 3

config SERIAL_FLASHER_RESET_INVERT
bool "Invert reset signal"
default n
depends on SERIAL_FLASHER_INTERFACE_UART
help
Enable this option if there is an inverting connection between
the output of the serial-flasher and the reset pin of the ESP chip.

config SERIAL_FLASHER_BOOT_INVERT
bool "Invert boot signal"
default n
depends on SERIAL_FLASHER_INTERFACE_UART
help
Enable this option if there is an inverting connection between
the output of the serial-flasher and the boot pin of the ESP chip.
Comment on lines +41 to +55
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
config SERIAL_FLASHER_RESET_INVERT
bool "Invert reset signal"
default n
depends on SERIAL_FLASHER_INTERFACE_UART
help
Enable this option if there is an inverting connection between
the output of the serial-flasher and the reset pin of the ESP chip.
config SERIAL_FLASHER_BOOT_INVERT
bool "Invert boot signal"
default n
depends on SERIAL_FLASHER_INTERFACE_UART
help
Enable this option if there is an inverting connection between
the output of the serial-flasher and the boot pin of the ESP chip.
config SERIAL_FLASHER_RESET_ACTIVE_LEVEL
int "Set the active level of the reset pin"
range 0 1
default 0
depends on SERIAL_FLASHER_INTERFACE_UART
help
Set the active level of the reset pin. 0 for low active, 1 for high active.
config SERIAL_FLASHER_BOOT_ACTIVE_LEVEL
int "Set the active level of the boot pin"
range 0 1
default 0
depends on SERIAL_FLASHER_INTERFACE_UART
help
Set the active level of the boot pin. 0 for low active, 1 for high active.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I actually don't see a problem with the current naming, but this is fine as well.


endmenu
13 changes: 13 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,19 @@ This is the time for which the boot pin is asserted when doing a hard reset in m

Default: 50

* `SERIAL_FLASHER_RESET_INVERT`

This inverts the output of the reset gpio pin. Useful if the hardware has inverting connection
between the host and the target reset pin. Implemented only for UART interface.

Default: n

* `SERIAL_FLASHER_BOOT_INVERT`
This inverts the output of the boot (IO0) gpio pin. Useful if the hardware has inverting connection
between the host and the target boot pin. Implemented only for UART interface.

Default: n

Comment on lines +68 to +80
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rewrite appropriately.

Configuration can be passed to `cmake` via command line:

```
Expand Down
8 changes: 4 additions & 4 deletions port/esp32_port.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,18 +142,18 @@ esp_loader_error_t loader_port_read(uint8_t *data, uint16_t size, uint32_t timeo
// assert reset pin for 50 milliseconds.
void loader_port_enter_bootloader(void)
{
gpio_set_level(s_gpio0_trigger_pin, 0);
gpio_set_level(s_gpio0_trigger_pin, SERIAL_FLASHER_BOOT_INVERT ? 1 : 0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
gpio_set_level(s_gpio0_trigger_pin, SERIAL_FLASHER_BOOT_INVERT ? 1 : 0);
gpio_set_level(s_gpio0_trigger_pin, CONFIG_SERIAL_FLASHER_BOOT_ACTIVE_LEVEL);

Copy link
Author

Choose a reason for hiding this comment

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

esp32_port.c refers to SERIAL_FLASHER_RESET_HOLD_TIME_MS as such without CONFIG_ prefix.
Is there something I don't understand why this new option would behave differently?

This same question goes for the other comments too.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @shiona, this is due to the fact that KConfig options do not get simply passed as defines to the build in ESP-IDF. KConfig options are processed and then CMake variables are created from them, which have to be handled in the build system.

These CMake variables will have the CONFIG prefix, unlike CMake variables for non-esp ports.

You can see that we have created a convenience macro for adding options which handles those differences and also the n and y booleans from KConfig, please use this to add the option.

loader_port_reset_target();
loader_port_delay_ms(SERIAL_FLASHER_BOOT_HOLD_TIME_MS);
gpio_set_level(s_gpio0_trigger_pin, 1);
gpio_set_level(s_gpio0_trigger_pin, SERIAL_FLASHER_BOOT_INVERT ? 0 : 1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
gpio_set_level(s_gpio0_trigger_pin, SERIAL_FLASHER_BOOT_INVERT ? 0 : 1);
gpio_set_level(s_gpio0_trigger_pin, !CONFIG_SERIAL_FLASHER_BOOT_ACTIVE_LEVEL);

}


void loader_port_reset_target(void)
{
gpio_set_level(s_reset_trigger_pin, 0);
gpio_set_level(s_reset_trigger_pin, SERIAL_FLASHER_RESET_INVERT ? 1 : 0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
gpio_set_level(s_reset_trigger_pin, SERIAL_FLASHER_RESET_INVERT ? 1 : 0);
gpio_set_level(s_reset_trigger_pin, CONFIG_SERIAL_FLASHER_RESET_ACTIVE_LEVEL);

Copy link
Collaborator

@DNedic DNedic Nov 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I actually believe the initial approach is more readable and future-proof, as well as being consistent with the rest of the ports.

loader_port_delay_ms(SERIAL_FLASHER_RESET_HOLD_TIME_MS);
gpio_set_level(s_reset_trigger_pin, 1);
gpio_set_level(s_reset_trigger_pin, SERIAL_FLASHER_RESET_INVERT ? 0 : 1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

gpio_set_level(s_reset_trigger_pin, !CONFIG_SERIAL_FLASHER_RESET_ACTIVE_LEVEL);

}


Expand Down
8 changes: 4 additions & 4 deletions port/pi_pico_port.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,20 +148,20 @@ void loader_port_pi_pico_deinit(void)

void loader_port_enter_bootloader(void)
{
gpio_put(s_boot_pin_num, 0);
gpio_put(s_boot_pin_num, SERIAL_FLASHER_BOOT_INVERT ? 1 : 0);

loader_port_reset_target();
loader_port_delay_ms(SERIAL_FLASHER_BOOT_HOLD_TIME_MS);

gpio_put(s_boot_pin_num, 1);
gpio_put(s_boot_pin_num, SERIAL_FLASHER_BOOT_INVERT ? 0 : 1);
}


void loader_port_reset_target(void)
{
gpio_put(s_reset_trigger_pin_num, 0);
gpio_put(s_reset_trigger_pin_num, SERIAL_FLASHER_RESET_INVERT ? 1 : 0);
loader_port_delay_ms(SERIAL_FLASHER_RESET_HOLD_TIME_MS);
gpio_put(s_reset_trigger_pin_num, 1);
gpio_put(s_reset_trigger_pin_num, SERIAL_FLASHER_RESET_INVERT ? 0 : 1);
}


Expand Down
8 changes: 4 additions & 4 deletions port/raspberry_port.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,18 +262,18 @@ esp_loader_error_t loader_port_read(uint8_t *data, uint16_t size, uint32_t timeo
// Set GPIO0 LOW, then assert reset pin for 50 milliseconds.
void loader_port_enter_bootloader(void)
{
gpioWrite(s_gpio0_trigger_pin, 0);
gpioWrite(s_gpio0_trigger_pin, SERIAL_FLASHER_BOOT_INVERT ? 1 : 0);
loader_port_reset_target();
loader_port_delay_ms(SERIAL_FLASHER_BOOT_HOLD_TIME_MS);
gpioWrite(s_gpio0_trigger_pin, 1);
gpioWrite(s_gpio0_trigger_pin, SERIAL_FLASHER_BOOT_INVERT ? 0 : 1);
}


void loader_port_reset_target(void)
{
gpioWrite(s_reset_trigger_pin, 0);
gpioWrite(s_reset_trigger_pin, SERIAL_FLASHER_RESET_INVERT ? 1 : 0);
loader_port_delay_ms(SERIAL_FLASHER_RESET_HOLD_TIME_MS);
gpioWrite(s_reset_trigger_pin, 1);
gpioWrite(s_reset_trigger_pin, SERIAL_FLASHER_RESET_INVERT ? 0 : 1);
}


Expand Down
8 changes: 4 additions & 4 deletions port/stm32_port.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,18 +89,18 @@ void loader_port_stm32_init(loader_stm32_config_t *config)
// assert reset pin for 100 milliseconds.
void loader_port_enter_bootloader(void)
{
HAL_GPIO_WritePin(gpio_port_io0, gpio_num_io0, GPIO_PIN_RESET);
HAL_GPIO_WritePin(gpio_port_io0, gpio_num_io0, SERIAL_FLASHER_BOOT_INVERT ? GPIO_PIN_SET : GPIO_PIN_RESET);
loader_port_reset_target();
HAL_Delay(SERIAL_FLASHER_BOOT_HOLD_TIME_MS);
HAL_GPIO_WritePin(gpio_port_io0, gpio_num_io0, GPIO_PIN_SET);
HAL_GPIO_WritePin(gpio_port_io0, gpio_num_io0, SERIAL_FLASHER_BOOT_INVERT ? GPIO_PIN_RESET : GPIO_PIN_SET);
}


void loader_port_reset_target(void)
{
HAL_GPIO_WritePin(gpio_port_rst, gpio_num_rst, GPIO_PIN_RESET);
HAL_GPIO_WritePin(gpio_port_rst, gpio_num_rst, SERIAL_FLASHER_RESET_INVERT ? GPIO_PIN_SET : GPIO_PIN_RESET);
HAL_Delay(SERIAL_FLASHER_RESET_HOLD_TIME_MS);
HAL_GPIO_WritePin(gpio_port_rst, gpio_num_rst, GPIO_PIN_SET);
HAL_GPIO_WritePin(gpio_port_rst, gpio_num_rst, SERIAL_FLASHER_RESET_INVERT ? GPIO_PIN_RESET : GPIO_PIN_SET);
}


Expand Down
8 changes: 4 additions & 4 deletions port/zephyr_port.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,17 +117,17 @@ esp_loader_error_t loader_port_zephyr_init(const loader_zephyr_config_t *config)

void loader_port_reset_target(void)
{
gpio_pin_set_dt(&enable_spec, false);
gpio_pin_set_dt(&enable_spec, CONFIG_SERIAL_FLASHER_RESET_INVERT ? true : false);
loader_port_delay_ms(CONFIG_SERIAL_FLASHER_RESET_HOLD_TIME_MS);
gpio_pin_set_dt(&enable_spec, true);
gpio_pin_set_dt(&enable_spec, CONFIG_SERIAL_FLASHER_RESET_INVERT ? false : true);
}

void loader_port_enter_bootloader(void)
{
gpio_pin_set_dt(&boot_spec, false);
gpio_pin_set_dt(&boot_spec, CONFIG_SERIAL_FLASHER_BOOT_INVERT ? true : false);
loader_port_reset_target();
loader_port_delay_ms(CONFIG_SERIAL_FLASHER_BOOT_HOLD_TIME_MS);
gpio_pin_set_dt(&boot_spec, true);
gpio_pin_set_dt(&boot_spec, CONFIG_SERIAL_FLASHER_BOOT_INVERT ? false : true);
}

void loader_port_delay_ms(uint32_t ms)
Expand Down