| Message ID | 20210614144558.12606-1-marek.behun@nic.cz |
|---|---|
| State | Accepted |
| Commit | 029bb91e806ed36dd48f0f967429bd5107464979 |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | [u-boot-marvell] arm: mvebu: turris_{omnia, mox}: ensure running bootcmd_rescue always works | expand |
Hi Stefan, Pali discovered this issue with the bootcmd_rescue code for Omnia & MOX. The patch is a therefore a fix. Is is still possible to send this for v2021.07 ? Sorry for the inconvenience. Marek
Hi Marek, On 14.06.21 16:49, Marek Behun wrote: > Hi Stefan, > > Pali discovered this issue with the bootcmd_rescue code for Omnia & MOX. > The patch is a therefore a fix. Is is still possible to send this for > v2021.07 ? Sorry for the inconvenience. No problem. I'll integrate it soon and will send a new pull request. Should be possible. Thanks, Stefan
On 14.06.21 16:45, Marek Behún wrote: > From: Pali Rohár <pali@kernel.org> > > One of the points of putting the rescue boot command into default > environment is that user can invoke it without physical access to the > board (without having to press the factory reset button), by running > run bootcmd_rescue > in U-Boot's console. > > Therefore we have to ensure that bootcmd_rescue is always set to default > value, regardless of whether the factory reset button was pressed. > Otherwise the variable will be empty for example after upgrade from > previous U-Boot. > > Fixes: ec3784d62646 ("arm: mvebu: turris_mox: add support for board rescue mode") > Fixes: 176c3e7760a2 ("arm: mvebu: turris_omnia: support invoking rescue boot from console") > Signed-off-by: Pali Rohár <pali@kernel.org> > Signed-off-by: Marek Behún <marek.behun@nic.cz> Reviewed-by: Stefan Roese <sr@denx.de> Thanks, Stefan > --- > board/CZ.NIC/turris_mox/turris_mox.c | 14 +++++++++++--- > board/CZ.NIC/turris_omnia/turris_omnia.c | 13 ++++++++++--- > 2 files changed, 21 insertions(+), 6 deletions(-) > > diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c > index 44c272c7cb..428cd23a19 100644 > --- a/board/CZ.NIC/turris_mox/turris_mox.c > +++ b/board/CZ.NIC/turris_mox/turris_mox.c > @@ -440,10 +440,18 @@ static bool read_reset_button(void) > > static void handle_reset_button(void) > { > + const char * const vars[1] = { "bootcmd_rescue", }; > + > + /* > + * Ensure that bootcmd_rescue has always stock value, so that running > + * run bootcmd_rescue > + * always works correctly. > + */ > + env_set_default_vars(1, (char * const *)vars, 0); > + > if (read_reset_button()) { > - const char * const vars[3] = { > + const char * const vars[2] = { > "bootcmd", > - "bootcmd_rescue", > "distro_bootcmd", > }; > > @@ -451,7 +459,7 @@ static void handle_reset_button(void) > * Set the above envs to their default values, in case the user > * managed to break them. > */ > - env_set_default_vars(3, (char * const *)vars, 0); > + env_set_default_vars(2, (char * const *)vars, 0); > > /* Ensure bootcmd_rescue is used by distroboot */ > env_set("boot_targets", "rescue"); > diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c > index ade923f599..8b2f94f959 100644 > --- a/board/CZ.NIC/turris_omnia/turris_omnia.c > +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c > @@ -339,9 +339,17 @@ static int set_regdomain(void) > > static void handle_reset_button(void) > { > + const char * const vars[1] = { "bootcmd_rescue", }; > int ret; > u8 reset_status; > > + /* > + * Ensure that bootcmd_rescue has always stock value, so that running > + * run bootcmd_rescue > + * always works correctly. > + */ > + env_set_default_vars(1, (char * const *)vars, 0); > + > ret = omnia_mcu_read(CMD_GET_RESET, &reset_status, 1); > if (ret) { > printf("omnia_mcu_read failed: %i, reset status unknown!\n", > @@ -352,9 +360,8 @@ static void handle_reset_button(void) > env_set_ulong("omnia_reset", reset_status); > > if (reset_status) { > - const char * const vars[3] = { > + const char * const vars[2] = { > "bootcmd", > - "bootcmd_rescue", > "distro_bootcmd", > }; > > @@ -362,7 +369,7 @@ static void handle_reset_button(void) > * Set the above envs to their default values, in case the user > * managed to break them. > */ > - env_set_default_vars(3, (char * const *)vars, 0); > + env_set_default_vars(2, (char * const *)vars, 0); > > /* Ensure bootcmd_rescue is used by distroboot */ > env_set("boot_targets", "rescue"); > Viele Grüße, Stefan
On 14.06.21 16:45, Marek Behún wrote: > From: Pali Rohár <pali@kernel.org> > > One of the points of putting the rescue boot command into default > environment is that user can invoke it without physical access to the > board (without having to press the factory reset button), by running > run bootcmd_rescue > in U-Boot's console. > > Therefore we have to ensure that bootcmd_rescue is always set to default > value, regardless of whether the factory reset button was pressed. > Otherwise the variable will be empty for example after upgrade from > previous U-Boot. > > Fixes: ec3784d62646 ("arm: mvebu: turris_mox: add support for board rescue mode") > Fixes: 176c3e7760a2 ("arm: mvebu: turris_omnia: support invoking rescue boot from console") > Signed-off-by: Pali Rohár <pali@kernel.org> > Signed-off-by: Marek Behún <marek.behun@nic.cz> Applied to u-boot-marvell/master Thanks, Stefan > --- > board/CZ.NIC/turris_mox/turris_mox.c | 14 +++++++++++--- > board/CZ.NIC/turris_omnia/turris_omnia.c | 13 ++++++++++--- > 2 files changed, 21 insertions(+), 6 deletions(-) > > diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c > index 44c272c7cb..428cd23a19 100644 > --- a/board/CZ.NIC/turris_mox/turris_mox.c > +++ b/board/CZ.NIC/turris_mox/turris_mox.c > @@ -440,10 +440,18 @@ static bool read_reset_button(void) > > static void handle_reset_button(void) > { > + const char * const vars[1] = { "bootcmd_rescue", }; > + > + /* > + * Ensure that bootcmd_rescue has always stock value, so that running > + * run bootcmd_rescue > + * always works correctly. > + */ > + env_set_default_vars(1, (char * const *)vars, 0); > + > if (read_reset_button()) { > - const char * const vars[3] = { > + const char * const vars[2] = { > "bootcmd", > - "bootcmd_rescue", > "distro_bootcmd", > }; > > @@ -451,7 +459,7 @@ static void handle_reset_button(void) > * Set the above envs to their default values, in case the user > * managed to break them. > */ > - env_set_default_vars(3, (char * const *)vars, 0); > + env_set_default_vars(2, (char * const *)vars, 0); > > /* Ensure bootcmd_rescue is used by distroboot */ > env_set("boot_targets", "rescue"); > diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c > index ade923f599..8b2f94f959 100644 > --- a/board/CZ.NIC/turris_omnia/turris_omnia.c > +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c > @@ -339,9 +339,17 @@ static int set_regdomain(void) > > static void handle_reset_button(void) > { > + const char * const vars[1] = { "bootcmd_rescue", }; > int ret; > u8 reset_status; > > + /* > + * Ensure that bootcmd_rescue has always stock value, so that running > + * run bootcmd_rescue > + * always works correctly. > + */ > + env_set_default_vars(1, (char * const *)vars, 0); > + > ret = omnia_mcu_read(CMD_GET_RESET, &reset_status, 1); > if (ret) { > printf("omnia_mcu_read failed: %i, reset status unknown!\n", > @@ -352,9 +360,8 @@ static void handle_reset_button(void) > env_set_ulong("omnia_reset", reset_status); > > if (reset_status) { > - const char * const vars[3] = { > + const char * const vars[2] = { > "bootcmd", > - "bootcmd_rescue", > "distro_bootcmd", > }; > > @@ -362,7 +369,7 @@ static void handle_reset_button(void) > * Set the above envs to their default values, in case the user > * managed to break them. > */ > - env_set_default_vars(3, (char * const *)vars, 0); > + env_set_default_vars(2, (char * const *)vars, 0); > > /* Ensure bootcmd_rescue is used by distroboot */ > env_set("boot_targets", "rescue"); > Viele Grüße, Stefan
diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c index 44c272c7cb..428cd23a19 100644 --- a/board/CZ.NIC/turris_mox/turris_mox.c +++ b/board/CZ.NIC/turris_mox/turris_mox.c @@ -440,10 +440,18 @@ static bool read_reset_button(void) static void handle_reset_button(void) { + const char * const vars[1] = { "bootcmd_rescue", }; + + /* + * Ensure that bootcmd_rescue has always stock value, so that running + * run bootcmd_rescue + * always works correctly. + */ + env_set_default_vars(1, (char * const *)vars, 0); + if (read_reset_button()) { - const char * const vars[3] = { + const char * const vars[2] = { "bootcmd", - "bootcmd_rescue", "distro_bootcmd", }; @@ -451,7 +459,7 @@ static void handle_reset_button(void) * Set the above envs to their default values, in case the user * managed to break them. */ - env_set_default_vars(3, (char * const *)vars, 0); + env_set_default_vars(2, (char * const *)vars, 0); /* Ensure bootcmd_rescue is used by distroboot */ env_set("boot_targets", "rescue"); diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c index ade923f599..8b2f94f959 100644 --- a/board/CZ.NIC/turris_omnia/turris_omnia.c +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c @@ -339,9 +339,17 @@ static int set_regdomain(void) static void handle_reset_button(void) { + const char * const vars[1] = { "bootcmd_rescue", }; int ret; u8 reset_status; + /* + * Ensure that bootcmd_rescue has always stock value, so that running + * run bootcmd_rescue + * always works correctly. + */ + env_set_default_vars(1, (char * const *)vars, 0); + ret = omnia_mcu_read(CMD_GET_RESET, &reset_status, 1); if (ret) { printf("omnia_mcu_read failed: %i, reset status unknown!\n", @@ -352,9 +360,8 @@ static void handle_reset_button(void) env_set_ulong("omnia_reset", reset_status); if (reset_status) { - const char * const vars[3] = { + const char * const vars[2] = { "bootcmd", - "bootcmd_rescue", "distro_bootcmd", }; @@ -362,7 +369,7 @@ static void handle_reset_button(void) * Set the above envs to their default values, in case the user * managed to break them. */ - env_set_default_vars(3, (char * const *)vars, 0); + env_set_default_vars(2, (char * const *)vars, 0); /* Ensure bootcmd_rescue is used by distroboot */ env_set("boot_targets", "rescue");