Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/45674 )
Change subject: realtek_mst_i2c_spi.c: Introduce MCU reset param ......................................................................
Patch Set 10:
(4 comments)
https://review.coreboot.org/c/flashrom/+/45674/6/realtek_mst_i2c_spi.c File realtek_mst_i2c_spi.c:
https://review.coreboot.org/c/flashrom/+/45674/6/realtek_mst_i2c_spi.c@a429 PS6, Line 429: ret = 0;
Without 'reset-mcu' now on the following else statement we specifically set ret to 0. […]
The only thing to change around here is to move the `goto` statement to the other branch of the if-block.
https://review.coreboot.org/c/flashrom/+/45674/10/realtek_mst_i2c_spi.c File realtek_mst_i2c_spi.c:
https://review.coreboot.org/c/flashrom/+/45674/10/realtek_mst_i2c_spi.c@444 PS10, Line 444: if else if
https://review.coreboot.org/c/flashrom/+/45674/10/realtek_mst_i2c_spi.c@452 PS10, Line 452: /* It's fine to not set this parameter. */ Please initialize `*reset` here.
https://review.coreboot.org/c/flashrom/+/45674/10/realtek_mst_i2c_spi.c@476 PS10, Line 476: if (reset) { I see that the MPU is reset here and during programmer shutdown. This means there should be 4 cases to handle:
- Never reset - Reset before flashing - Reset after flashing - Reset both before and after flashing
The current approach only lets one choose between the first and the last option. Should we account for the two central options, too? This can use flexible syntax, too:
if reset_str contains "pre" then reset |= RESET_PRE; end if; if reset_str contains "post" then reset |= RESET_POST; end if; if reset_str contains "both" then reset |= RESET_PRE | RESET_POST; end if;
Something like this would allow one to request a reset before and after flashing using either "pre,post" or "both". It needs more refining, though: "preposterous" would work as well 😄