diff mbox

[U-Boot,03/13] apalis/colibri_t20/t30: integrate recovery mode detection

Message ID 84d93469cfb940421e8e84b822b441fbad8ca29c.1436170106.git.marcel.ziswiler@toradex.com
State Superseded
Delegated to: Tom Warren
Headers show

Commit Message

Marcel Ziswiler July 6, 2015, 8:20 a.m. UTC
From: Marcel Ziswiler <marcel.ziswiler@toradex.com>

Allow detecting whether or not U-Boot was launched through the
recovery mode of the resp. NVIDIA SoC.

Make use of a board specific arch_misc_init() and enable the same via
CONFIG_ARCH_MISC_INIT configuration option.

While at it also sort the include files alphabetically (while
leaving common.h on top of course).

While at it also streamline some comments in the configuration files
and fix the spacing from using spaces to tabs.

Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
---
 board/toradex/apalis_t30/apalis_t30.c   | 15 ++++++++++++++-
 board/toradex/colibri_t20/colibri_t20.c | 12 ++++++++++++
 board/toradex/colibri_t30/colibri_t30.c | 18 +++++++++++++++---
 include/configs/apalis_t30.h            | 10 +++++++---
 include/configs/colibri_t20.h           | 14 +++++++++-----
 include/configs/colibri_t30.h           | 10 +++++++---
 6 files changed, 64 insertions(+), 15 deletions(-)

Comments

Simon Glass July 6, 2015, 4:38 p.m. UTC | #1
Hi Marcel,

On 6 July 2015 at 02:20, Marcel Ziswiler <marcel@ziswiler.com> wrote:
> From: Marcel Ziswiler <marcel.ziswiler@toradex.com>
>
> Allow detecting whether or not U-Boot was launched through the
> recovery mode of the resp. NVIDIA SoC.

Out of interest, is this just a message for the user? Why is it useful?

>
> Make use of a board specific arch_misc_init() and enable the same via
> CONFIG_ARCH_MISC_INIT configuration option.
>
> While at it also sort the include files alphabetically (while
> leaving common.h on top of course).
>
> While at it also streamline some comments in the configuration files
> and fix the spacing from using spaces to tabs.

Changes look fine, but clean-up should be in a separate patch as it is
unrelated.

>
> Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> ---
>  board/toradex/apalis_t30/apalis_t30.c   | 15 ++++++++++++++-
>  board/toradex/colibri_t20/colibri_t20.c | 12 ++++++++++++
>  board/toradex/colibri_t30/colibri_t30.c | 18 +++++++++++++++---
>  include/configs/apalis_t30.h            | 10 +++++++---
>  include/configs/colibri_t20.h           | 14 +++++++++-----
>  include/configs/colibri_t30.h           | 10 +++++++---
>  6 files changed, 64 insertions(+), 15 deletions(-)
>

Regards,
Simon
Marcel Ziswiler July 7, 2015, 6:04 a.m. UTC | #2
On 6 July 2015 18:38:21 CEST, Simon Glass <sjg@chromium.org> wrote:

>Out of interest, is this just a message for the user? Why is it useful?

Well, we or customers might do other interesting things on entering rcm like stopping autoboot or automatically start ums.

>Changes look fine, but clean-up should be in a separate patch as it is
>unrelated.

Will send a separate cleanup patch in the V2 round.

Thanks Simon!
Stephen Warren July 8, 2015, 8:57 p.m. UTC | #3
On 07/07/2015 12:04 AM, Marcel Ziswiler wrote:
>
>
> On 6 July 2015 18:38:21 CEST, Simon Glass <sjg@chromium.org> wrote:
>
>> Out of interest, is this just a message for the user? Why is it useful?
>
> Well, we or customers might do other interesting things on entering rcm like stopping autoboot or automatically start ums.

But that's not what this patch does; it simply prints a message. It 
doesn't seem terribly useful.
Marcel Ziswiler July 9, 2015, 1:02 p.m. UTC | #4
On Wed, 2015-07-08 at 14:57 -0600, Stephen Warren wrote:

> But that's not what this patch does; it simply prints a message. It 
> doesn't seem terribly useful.

Agreed but I look at it more like a documented entry point.

In our downstream U-Boot we use it to stop autoboot to allow for manual
recovery handling of our BSP demo images but unfortunately that does not
play that well with the tegra-uboot-flasher as it stands so I decided to
just have that message printed for now.
Stefan Agner July 9, 2015, 3:02 p.m. UTC | #5
On 08.07.2015 22:57, Stephen Warren wrote:
> On 07/07/2015 12:04 AM, Marcel Ziswiler wrote:
>>
>>
>> On 6 July 2015 18:38:21 CEST, Simon Glass <sjg@chromium.org> wrote:
>>
>>> Out of interest, is this just a message for the user? Why is it useful?
>>
>> Well, we or customers might do other interesting things on entering
>> rcm like stopping autoboot or automatically start ums.
>
> But that's not what this patch does; it simply prints a message. It
> doesn't seem terribly useful.
>
IMO, it really is useful: The boot ROM takes other initialization steps
when using RCM recovery mode vs. full NAND/eMMC boot. We have had issues
in the past where the Linux kernel hangs or shows issues just because
something (e.g. USB, NAND or eMMC) was/or was not initialized by the
boot ROM first. Of course those are bugs, and need to be resolved in the
end. But it can be helpful to reproduce issues when one sees whether the
recovery mode has been used or not...

--
Stefan
Stephen Warren July 9, 2015, 3:51 p.m. UTC | #6
On 07/09/2015 09:02 AM, Stefan Agner wrote:
> On 08.07.2015 22:57, Stephen Warren wrote:
>> On 07/07/2015 12:04 AM, Marcel Ziswiler wrote:
>>>
>>>
>>> On 6 July 2015 18:38:21 CEST, Simon Glass <sjg@chromium.org> wrote:
>>>
>>>> Out of interest, is this just a message for the user? Why is it useful?
>>>
>>> Well, we or customers might do other interesting things on entering
>>> rcm like stopping autoboot or automatically start ums.
>>
>> But that's not what this patch does; it simply prints a message. It
>> doesn't seem terribly useful.
>>
> IMO, it really is useful: The boot ROM takes other initialization steps
> when using RCM recovery mode vs. full NAND/eMMC boot. We have had issues
> in the past where the Linux kernel hangs or shows issues just because
> something (e.g. USB, NAND or eMMC) was/or was not initialized by the
> boot ROM first. Of course those are bugs, and need to be resolved in the
> end. But it can be helpful to reproduce issues when one sees whether the
> recovery mode has been used or not...

Surely you can remember whether you pressed the reset button or used 
tegrarcm/... to push U-Boot onto the device in recovery mode?

If it really is that useful, then I'd suggest not making this 
board-specific, since it's a SoC-defined concept, not a board-defined 
concept.
diff mbox

Patch

diff --git a/board/toradex/apalis_t30/apalis_t30.c b/board/toradex/apalis_t30/apalis_t30.c
index 6244214..879006f 100644
--- a/board/toradex/apalis_t30/apalis_t30.c
+++ b/board/toradex/apalis_t30/apalis_t30.c
@@ -6,10 +6,13 @@ 
  */
 
 #include <common.h>
-#include <dm.h>
 #include <asm/arch/gp_padctrl.h>
 #include <asm/arch/pinmux.h>
+#include <asm/arch-tegra/ap.h>
+#include <asm/arch-tegra/tegra.h>
 #include <asm/gpio.h>
+#include <asm/io.h>
+#include <dm.h>
 #include <i2c.h>
 #include <netdev.h>
 
@@ -18,6 +21,15 @@ 
 #define PMU_I2C_ADDRESS		0x2D
 #define MAX_I2C_RETRY		3
 
+int arch_misc_init(void)
+{
+	if (readl(NV_PA_BASE_SRAM + NVBOOTINFOTABLE_BOOTTYPE) ==
+	    NVBOOTTYPE_RECOVERY)
+		printf("USB recovery mode\n");
+
+	return 0;
+}
+
 /*
  * Routine: pinmux_init
  * Description: Do individual peripheral pinmux configs
@@ -47,6 +59,7 @@  int tegra_pcie_board_init(void)
 		debug("%s: Cannot find PMIC I2C chip\n", __func__);
 		return err;
 	}
+
 	/* TPS659110: VDD2_OP_REG = 1.05V */
 	data[0] = 0x27;
 	addr = 0x25;
diff --git a/board/toradex/colibri_t20/colibri_t20.c b/board/toradex/colibri_t20/colibri_t20.c
index 8ae9ccf..7210a8a 100644
--- a/board/toradex/colibri_t20/colibri_t20.c
+++ b/board/toradex/colibri_t20/colibri_t20.c
@@ -8,8 +8,20 @@ 
 #include <asm/arch/clock.h>
 #include <asm/arch/funcmux.h>
 #include <asm/arch/pinmux.h>
+#include <asm/arch-tegra/ap.h>
 #include <asm/arch-tegra/board.h>
+#include <asm/arch-tegra/tegra.h>
 #include <asm/gpio.h>
+#include <asm/io.h>
+
+int arch_misc_init(void)
+{
+	if (readl(NV_PA_BASE_SRAM + NVBOOTINFOTABLE_BOOTTYPE) ==
+	    NVBOOTTYPE_RECOVERY)
+		printf("USB recovery mode\n");
+
+	return 0;
+}
 
 #ifdef CONFIG_TEGRA_MMC
 /*
diff --git a/board/toradex/colibri_t30/colibri_t30.c b/board/toradex/colibri_t30/colibri_t30.c
index f4bc7d8..44b5beb 100644
--- a/board/toradex/colibri_t30/colibri_t30.c
+++ b/board/toradex/colibri_t30/colibri_t30.c
@@ -6,11 +6,23 @@ 
  */
 
 #include <common.h>
-#include <asm/arch/pinmux.h>
 #include <asm/arch/gp_padctrl.h>
-#include "pinmux-config-colibri_t30.h"
-#include <i2c.h>
+#include <asm/arch/pinmux.h>
+#include <asm/arch-tegra/ap.h>
+#include <asm/arch-tegra/tegra.h>
 #include <asm/gpio.h>
+#include <asm/io.h>
+#include <i2c.h>
+#include "pinmux-config-colibri_t30.h"
+
+int arch_misc_init(void)
+{
+	if (readl(NV_PA_BASE_SRAM + NVBOOTINFOTABLE_BOOTTYPE) ==
+	    NVBOOTTYPE_RECOVERY)
+		printf("USB recovery mode\n");
+
+	return 0;
+}
 
 /*
  * Routine: pinmux_init
diff --git a/include/configs/apalis_t30.h b/include/configs/apalis_t30.h
index aba9ba6..bb58936 100644
--- a/include/configs/apalis_t30.h
+++ b/include/configs/apalis_t30.h
@@ -1,6 +1,8 @@ 
 /*
  * Copyright (c) 2014-2015 Marcel Ziswiler
  *
+ * Configuration settings for the Toradex Apalis T30 modules.
+ *
  * SPDX-License-Identifier:	GPL-2.0+
  */
 
@@ -11,6 +13,8 @@ 
 
 #include "tegra30-common.h"
 
+#define CONFIG_ARCH_MISC_INIT
+
 /* High-level configuration options */
 #define V_PROMPT			"Apalis T30 # "
 #define CONFIG_TEGRA_BOARD_STRING	"Toradex Apalis T30"
@@ -26,7 +30,7 @@ 
 #define CONFIG_SYS_I2C_TEGRA
 #define CONFIG_CMD_I2C
 
-/* SD/MMC */
+/* SD/MMC support */
 #define CONFIG_MMC
 #define CONFIG_GENERIC_MMC
 #define CONFIG_TEGRA_MMC
@@ -38,10 +42,10 @@ 
 #define CONFIG_SYS_MMC_ENV_DEV		0
 #define CONFIG_SYS_MMC_ENV_PART		2
 
-/* USB Host support */
+/* USB host support */
 #define CONFIG_USB_EHCI
 #define CONFIG_USB_EHCI_TEGRA
-#define CONFIG_USB_MAX_CONTROLLER_COUNT 3
+#define CONFIG_USB_MAX_CONTROLLER_COUNT	3
 #define CONFIG_USB_STORAGE
 #define CONFIG_CMD_USB
 
diff --git a/include/configs/colibri_t20.h b/include/configs/colibri_t20.h
index a3f27e3..b22e82a 100644
--- a/include/configs/colibri_t20.h
+++ b/include/configs/colibri_t20.h
@@ -1,6 +1,8 @@ 
 /*
  * Copyright (C) 2012 Lucas Stach
  *
+ * Configuration settings for the Toradex Colibri T20 modules.
+ *
  * SPDX-License-Identifier:	GPL-2.0+
  */
 
@@ -9,6 +11,8 @@ 
 
 #include "tegra20-common.h"
 
+#define CONFIG_ARCH_MISC_INIT
+
 /* High-level configuration options */
 #define V_PROMPT			"Colibri T20 # "
 #define CONFIG_TEGRA_BOARD_STRING	"Toradex Colibri T20"
@@ -32,7 +36,7 @@ 
 #define CONFIG_USB_EHCI_TEGRA
 #define CONFIG_USB_ULPI
 #define CONFIG_USB_ULPI_VIEWPORT
-#define CONFIG_USB_MAX_CONTROLLER_COUNT 3
+#define CONFIG_USB_MAX_CONTROLLER_COUNT	3
 #define CONFIG_USB_STORAGE
 #define CONFIG_CMD_USB
 
@@ -46,13 +50,13 @@ 
 /* NAND support */
 #define CONFIG_CMD_NAND
 #define CONFIG_TEGRA_NAND
-#define CONFIG_SYS_MAX_NAND_DEVICE     1
+#define CONFIG_SYS_MAX_NAND_DEVICE	1
 
 /* Environment in NAND, 64K is a bit excessive but erase block is 512K anyway */
 #define CONFIG_ENV_IS_IN_NAND
-#define CONFIG_ENV_OFFSET              (SZ_2M)
-#undef  CONFIG_ENV_SIZE /* undef size from tegra20-common.h */
-#define CONFIG_ENV_SIZE                (SZ_64K)
+#define CONFIG_ENV_OFFSET		(SZ_2M)
+#undef CONFIG_ENV_SIZE	/* undef size from tegra20-common.h */
+#define CONFIG_ENV_SIZE			(SZ_64K)
 
 /* Debug commands */
 #define CONFIG_CMD_BDI
diff --git a/include/configs/colibri_t30.h b/include/configs/colibri_t30.h
index 4655668..7710786 100644
--- a/include/configs/colibri_t30.h
+++ b/include/configs/colibri_t30.h
@@ -1,6 +1,8 @@ 
 /*
  * Copyright (c) 2013-2015 Stefan Agner
  *
+ * Configuration settings for the Toradex Colibri T30 modules.
+ *
  * SPDX-License-Identifier:	GPL-2.0+
  */
 
@@ -11,6 +13,8 @@ 
 
 #include "tegra30-common.h"
 
+#define CONFIG_ARCH_MISC_INIT
+
 /* High-level configuration options */
 #define V_PROMPT			"Colibri T30 # "
 #define CONFIG_TEGRA_BOARD_STRING	"Toradex Colibri T30"
@@ -26,7 +30,7 @@ 
 #define CONFIG_SYS_I2C_TEGRA
 #define CONFIG_CMD_I2C
 
-/* SD/MMC */
+/* SD/MMC support */
 #define CONFIG_MMC
 #define CONFIG_GENERIC_MMC
 #define CONFIG_TEGRA_MMC
@@ -38,10 +42,10 @@ 
 #define CONFIG_SYS_MMC_ENV_DEV		0
 #define CONFIG_SYS_MMC_ENV_PART		2
 
-/* USB Host support */
+/* USB host support */
 #define CONFIG_USB_EHCI
 #define CONFIG_USB_EHCI_TEGRA
-#define CONFIG_USB_MAX_CONTROLLER_COUNT 3
+#define CONFIG_USB_MAX_CONTROLLER_COUNT	3
 #define CONFIG_USB_STORAGE
 #define CONFIG_CMD_USB