diff mbox

[U-Boot,v5,5/8] omap-common: SYS_BOOT-based fallback boot device selection for peripheral boot

Message ID 1436968946-16025-6-git-send-email-contact@paulk.fr
State Accepted
Delegated to: Tom Rini
Headers show

Commit Message

Paul Kocialkowski July 15, 2015, 2:02 p.m. UTC
OMAP devices might boot from peripheral devices, such as UART or USB.
When that happens, the U-Boot SPL tries to boot the next stage (complete U-Boot)
from that peripheral device, but in most cases, this is not a valid boot device.

This introduces a fallback option that reads the SYS_BOOT pins, that are used by
the bootrom to determine which device to boot from. It is intended for the
SYS_BOOT value to be interpreted in the memory-preferred scheme, so that the
U-Boot SPL can load the next stage from a valid location.

Practically, this options allows loading the U-Boot SPL through USB and have it
load the next stage according to the memory device selected by SYS_BOOT instead
of stalling.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 arch/arm/cpu/armv7/omap-common/boot-common.c | 52 ++++++++++++++++++++++++----
 arch/arm/include/asm/omap_common.h           |  4 +++
 2 files changed, 49 insertions(+), 7 deletions(-)

Comments

Tom Rini July 28, 2015, 2:59 p.m. UTC | #1
On Wed, Jul 15, 2015 at 04:02:23PM +0200, Paul Kocialkowski wrote:

> OMAP devices might boot from peripheral devices, such as UART or USB.
> When that happens, the U-Boot SPL tries to boot the next stage (complete U-Boot)
> from that peripheral device, but in most cases, this is not a valid boot device.
> 
> This introduces a fallback option that reads the SYS_BOOT pins, that are used by
> the bootrom to determine which device to boot from. It is intended for the
> SYS_BOOT value to be interpreted in the memory-preferred scheme, so that the
> U-Boot SPL can load the next stage from a valid location.
> 
> Practically, this options allows loading the U-Boot SPL through USB and have it
> load the next stage according to the memory device selected by SYS_BOOT instead
> of stalling.
> 
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>

Applied to u-boot/master, thanks!
Hannes Schmelzer Aug. 25, 2015, 8:40 a.m. UTC | #2
Hi Paul, Tom,

i am failing bring up my AM335x boards (tseries, kwb) with bare UART 
connection since introducing this change.

--
U-Boot SPL 2015.10-rc2-00079-g35f2192-dirty (Aug 25 2015 - 07:34:24)
boot device - 0
SPL: Unsupported Boot Device (0)!
### ERROR ### Please RESET the board ###
---

The source of my problem is the __weak function in omap-common/boot-common.c

__weak u32 omap_sys_boot_device(void)
{
     return BOOT_DEVICE_NONE;
}

which one is called on line #82

For my understanding this function should be called allways if chip has 
basically support for some BOOT_DEVICE_x  __AND__ there is no 
implementation for it - the function should prevent target from stalling 
with selecting another (hopefully working) boot-device.
Right ?

I am not sure at this time how to deal with the facts ... i see several 
possibilities:

a)
i have to implement some "omap_sys_boot_device" function in my boards - 
this would maybe sometimes comfortable but i think this is not inventors 
mind. It would be more convenient to implement it in some common place 
for AM335x or OMAP. But what do with the information about SYS_BOOT pins 
? they always represent a boot-order ... which boot-device should it take ?

b) and/or something is wrong with the #ifdef ... construct at line #67 -
In fact there is a problem with
/
defined(BOOT_DEVICE_USBETH) && !defined(CONFIG_SPL_USBETH_SUPPORT)/

basicaly BOOT_DEVICE_USBETH is defined in spl.h but in my configuration 
there is no #define for CONFIG_SPL_USBETH_SUPPORT and so the following 
switch/case calls in case of BOOT_DEVICE_UART this weak function.

further i think that this construct isn't complete yet, because it wants 
to handle all peripheral booting on AM335x or OMAP in general.

following peripherals are currently handled:

BOOT_DEVICE_UART
BOOT_DEVICE_USB
BOOT_DEVICE_USBETH

but there is also
BOOT_DEVICE_CPGMAC

Summary i think this changeset isn't complete.

How we want to deal with this facts ?

best regards,
Hannes

On 28.07.2015 16:59, Tom Rini wrote:
> On Wed, Jul 15, 2015 at 04:02:23PM +0200, Paul Kocialkowski wrote:
>
>> OMAP devices might boot from peripheral devices, such as UART or USB.
>> When that happens, the U-Boot SPL tries to boot the next stage (complete U-Boot)
>> from that peripheral device, but in most cases, this is not a valid boot device.
>>
>> This introduces a fallback option that reads the SYS_BOOT pins, that are used by
>> the bootrom to determine which device to boot from. It is intended for the
>> SYS_BOOT value to be interpreted in the memory-preferred scheme, so that the
>> U-Boot SPL can load the next stage from a valid location.
>>
>> Practically, this options allows loading the U-Boot SPL through USB and have it
>> load the next stage according to the memory device selected by SYS_BOOT instead
>> of stalling.
>>
>> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> Applied to u-boot/master, thanks!
>
>
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
Paul Kocialkowski Aug. 25, 2015, 12:27 p.m. UTC | #3
Le mardi 25 août 2015 à 10:40 +0200, Schmelzer Hannes a écrit :
> Hi Paul, Tom,
>
> i am failing bring up my AM335x boards (tseries, kwb) with bare UART
> connection since introducing this change.

Does this mean that you're trying to get the device to load the full
U-Boot binary over UART?

> For my understanding this function should be called allways if chip
> has basically support for some BOOT_DEVICE_x  __AND__ there is no
> implementation for it - the function should prevent target from
> stalling with selecting another (hopefully working) boot-device.
> Right ?

This is a fallback mechanism that allows selecting the boot device from
the SYS_BOOT pins when the U-Boot SPL was loaded from peripheral booting
(USB or UART) by the bootrom and that the U-Boot SPL has no support for
continuing boot through that same peripheral (USB or UART).

It does require omap_sys_boot_device to be implemented for each platform
(currently, only am33xx doesn't have a proper one). The point is that it
selects another *memory* (read, not peripheral) boot device that the
U-Boot SPL may be able to handle.

In any case, it offers a way to *maybe* put the U-Boot SPL on the right
track instead of being unable to boot at all.

> I am not sure at this time how to deal with the facts ... i see
> several possibilities:
>
> a) 
> i have to implement some "omap_sys_boot_device" function in my boards
> - this would maybe sometimes comfortable but i think this is not
> inventors mind. It would be more convenient to implement it in some
> common place for AM335x or OMAP. But what do with the information
> about SYS_BOOT pins ? they always represent a boot-order ... which
> boot-device should it take ?

That function is not supposed to be board-specific at all, but to be
platform-specific. This is not the way to select the proper boot device,
which is done by reading the boot info structure passed by the bootrom
(first thing in omap-common/lowlevel_init.S).

> b) and/or something is wrong with the #ifdef ... construct at line #67
> - 
> In fact there is a problem with 
> defined(BOOT_DEVICE_USBETH) && !defined(CONFIG_SPL_USBETH_SUPPORT)
>
> basicaly BOOT_DEVICE_USBETH is defined in spl.h but in my
> configuration there is no #define for CONFIG_SPL_USBETH_SUPPORT and so
> the following switch/case calls in case of BOOT_DEVICE_UART this weak
> function.

If I got everything right, the bootrom is passing BOOT_DEVICE_UART as
boot device, but you haven't selected CONFIG_SPL_YMODEM_SUPPORT. Thus,
it falls back to asking omap_sys_boot_device, which is not implemented.

The real problem here is that you have not enabled support for loading
the main U-Boot binary via UART, with CONFIG_SPL_YMODEM_SUPPORT.

UART booting is unrelated to CONFIG_SPL_USBETH_SUPPORT.

> further i think that this construct isn't complete yet, because it
> wants to handle all peripheral booting on AM335x or OMAP in general.
>
> following peripherals are currently handled:
>
> BOOT_DEVICE_UART
> BOOT_DEVICE_USB
> BOOT_DEVICE_USBETH
>
> but there is also 
> BOOT_DEVICE_CPGMAC
>
> Summary i think this changeset isn't complete.

Can the bootrom indicate that it booted from BOOT_DEVICE_CPGMAC?
I haven't seen that and don't really know what it corresponds to. But if
you think it is concerned by this fallback mechanism, you could add
support for it. I only made this for the omap devices I have (and I
don't have any am33xx board) and I didn't want to blindly implement too
much for am33xx.

> On 28.07.2015 16:59, Tom Rini wrote:
> 
> > On Wed, Jul 15, 2015 at 04:02:23PM +0200, Paul Kocialkowski wrote:
> > 
> > > OMAP devices might boot from peripheral devices, such as UART or USB.
> > > When that happens, the U-Boot SPL tries to boot the next stage (complete U-Boot)
> > > from that peripheral device, but in most cases, this is not a valid boot device.
> > > 
> > > This introduces a fallback option that reads the SYS_BOOT pins, that are used by
> > > the bootrom to determine which device to boot from. It is intended for the
> > > SYS_BOOT value to be interpreted in the memory-preferred scheme, so that the
> > > U-Boot SPL can load the next stage from a valid location.
> > > 
> > > Practically, this options allows loading the U-Boot SPL through USB and have it
> > > load the next stage according to the memory device selected by SYS_BOOT instead
> > > of stalling.
> > > 
> > > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > Applied to u-boot/master, thanks!
> > 
> > 
> > 
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot@lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
>
Hannes Schmelzer Aug. 25, 2015, 12:52 p.m. UTC | #4
Hi Paul,
thanks for answer.

On 25.08.2015 14:27, Paul Kocialkowski wrote:
> Le mardi 25 août 2015 à 10:40 +0200, Schmelzer Hannes a écrit :
>> Hi Paul, Tom,
>>
>> i am failing bring up my AM335x boards (tseries, kwb) with bare UART
>> connection since introducing this change.
> Does this mean that you're trying to get the device to load the full
> U-Boot binary over UART?
Yes i do so. On at least one BuR board this is the only possibility for 
very first startup.
Once U-Boot is loaded via UART we fetch the rest from TFTP and burn it 
into eMMC flash connected to MMC1.
>> For my understanding this function should be called allways if chip
>> has basically support for some BOOT_DEVICE_x  __AND__ there is no
>> implementation for it - the function should prevent target from
>> stalling with selecting another (hopefully working) boot-device.
>> Right ?
> This is a fallback mechanism that allows selecting the boot device from
> the SYS_BOOT pins when the U-Boot SPL was loaded from peripheral booting
> (USB or UART) by the bootrom and that the U-Boot SPL has no support for
> continuing boot through that same peripheral (USB or UART).
>
> It does require omap_sys_boot_device to be implemented for each platform
> (currently, only am33xx doesn't have a proper one). The point is that it
> selects another *memory* (read, not peripheral) boot device that the
> U-Boot SPL may be able to handle.
>
> In any case, it offers a way to *maybe* put the U-Boot SPL on the right
> track instead of being unable to boot at all.
Okay, i understand - this keeps track with my understanding.
>> I am not sure at this time how to deal with the facts ... i see
>> several possibilities:
>>
>> a)
>> i have to implement some "omap_sys_boot_device" function in my boards
>> - this would maybe sometimes comfortable but i think this is not
>> inventors mind. It would be more convenient to implement it in some
>> common place for AM335x or OMAP. But what do with the information
>> about SYS_BOOT pins ? they always represent a boot-order ... which
>> boot-device should it take ?
> That function is not supposed to be board-specific at all, but to be
> platform-specific. This is not the way to select the proper boot device,
> which is done by reading the boot info structure passed by the bootrom
> (first thing in omap-common/lowlevel_init.S).
>
>> b) and/or something is wrong with the #ifdef ... construct at line #67
>> -
>> In fact there is a problem with
>> defined(BOOT_DEVICE_USBETH) && !defined(CONFIG_SPL_USBETH_SUPPORT)
>>
>> basicaly BOOT_DEVICE_USBETH is defined in spl.h but in my
>> configuration there is no #define for CONFIG_SPL_USBETH_SUPPORT and so
>> the following switch/case calls in case of BOOT_DEVICE_UART this weak
>> function.
> If I got everything right, the bootrom is passing BOOT_DEVICE_UART as
> boot device, but you haven't selected CONFIG_SPL_YMODEM_SUPPORT. Thus,
> it falls back to asking omap_sys_boot_device, which is not implemented.
I don't think, there is everything right. Have a closer look to the #ifdef.

#if (defined(BOOT_DEVICE_UART) && !defined(CONFIG_SPL_YMODEM_SUPPORT)) || \
     (defined(BOOT_DEVICE_USB) && !defined(CONFIG_SPL_USB_SUPPORT)) || \
     (defined(BOOT_DEVICE_USBETH) && !defined(CONFIG_SPL_USBETH_SUPPORT))

I have enabled CONFIG_SPL_YMODEM_SUPPORT, look at bur_am335x_common.h.

> The real problem here is that you have not enabled support for loading
> the main U-Boot binary via UART, with CONFIG_SPL_YMODEM_SUPPORT.
>
> UART booting is unrelated to CONFIG_SPL_USBETH_SUPPORT.
No, due to the fact that defined(BOOT_DEVICE_USBETH) is allways true 
(spl.h) and i don't have
CONFIG_SPL_USBETH_SUPPORT enabled, the #ifdef above:

defined(BOOT_DEVICE_USBETH) && !defined(CONFIG_SPL_USBETH_SUPPORT)

becomes true and the followed switch/case does the rest.
>> further i think that this construct isn't complete yet, because it
>> wants to handle all peripheral booting on AM335x or OMAP in general.
>>
>> following peripherals are currently handled:
>>
>> BOOT_DEVICE_UART
>> BOOT_DEVICE_USB
>> BOOT_DEVICE_USBETH
>>
>> but there is also
>> BOOT_DEVICE_CPGMAC
>>
>> Summary i think this changeset isn't complete.
> Can the bootrom indicate that it booted from BOOT_DEVICE_CPGMAC?
> I haven't seen that and don't really know what it corresponds to. But if
> you think it is concerned by this fallback mechanism, you could add
> support for it. I only made this for the omap devices I have (and I
> don't have any am33xx board) and I didn't want to blindly implement too
> much for am33xx.
Yes, thats possible von AM335x.
I will have a close look if it is necessary to implement here some fallback.
But probably not, because the most likely case is that "full" U-Boot 
supports Ethernet and the SPL doesn't and not otherwise :-)


best regards,
Hannes
Paul Kocialkowski Aug. 25, 2015, 2:44 p.m. UTC | #5
> > If I got everything right, the bootrom is passing BOOT_DEVICE_UART as
> > boot device, but you haven't selected CONFIG_SPL_YMODEM_SUPPORT. Thus,
> > it falls back to asking omap_sys_boot_device, which is not implemented.
> I don't think, there is everything right. Have a closer look to the #ifdef.
> 
> #if (defined(BOOT_DEVICE_UART) && !defined(CONFIG_SPL_YMODEM_SUPPORT)) || \
>      (defined(BOOT_DEVICE_USB) && !defined(CONFIG_SPL_USB_SUPPORT)) || \
>      (defined(BOOT_DEVICE_USBETH) && !defined(CONFIG_SPL_USBETH_SUPPORT))
> 
> I have enabled CONFIG_SPL_YMODEM_SUPPORT, look at bur_am335x_common.h.
> 
> > The real problem here is that you have not enabled support for loading
> > the main U-Boot binary via UART, with CONFIG_SPL_YMODEM_SUPPORT.
> >
> > UART booting is unrelated to CONFIG_SPL_USBETH_SUPPORT.
> No, due to the fact that defined(BOOT_DEVICE_USBETH) is allways true 
> (spl.h) and i don't have
> CONFIG_SPL_USBETH_SUPPORT enabled, the #ifdef above:
> 
> defined(BOOT_DEVICE_USBETH) && !defined(CONFIG_SPL_USBETH_SUPPORT)
> 
> becomes true and the followed switch/case does the rest.

Oh you're right, I hadn't thought this through.

I'll think about how to solve this in the cleanest way (I wouldn't want
to duplicate the ifdef logic too much). Thanks for bringing this up.

> >> further i think that this construct isn't complete yet, because it
> >> wants to handle all peripheral booting on AM335x or OMAP in general.
> >>
> >> following peripherals are currently handled:
> >>
> >> BOOT_DEVICE_UART
> >> BOOT_DEVICE_USB
> >> BOOT_DEVICE_USBETH
> >>
> >> but there is also
> >> BOOT_DEVICE_CPGMAC
> >>
> >> Summary i think this changeset isn't complete.
> > Can the bootrom indicate that it booted from BOOT_DEVICE_CPGMAC?
> > I haven't seen that and don't really know what it corresponds to. But if
> > you think it is concerned by this fallback mechanism, you could add
> > support for it. I only made this for the omap devices I have (and I
> > don't have any am33xx board) and I didn't want to blindly implement too
> > much for am33xx.
> Yes, thats possible von AM335x.
> I will have a close look if it is necessary to implement here some fallback.
> But probably not, because the most likely case is that "full" U-Boot 
> supports Ethernet and the SPL doesn't and not otherwise :-)

Alright then, that's fine with me. It's okay if the fallback mechanism
doesn't handle all use cases, too.
diff mbox

Patch

diff --git a/arch/arm/cpu/armv7/omap-common/boot-common.c b/arch/arm/cpu/armv7/omap-common/boot-common.c
index f2a25dc..5ec46fa 100644
--- a/arch/arm/cpu/armv7/omap-common/boot-common.c
+++ b/arch/arm/cpu/armv7/omap-common/boot-common.c
@@ -21,6 +21,11 @@ 
 
 DECLARE_GLOBAL_DATA_PTR;
 
+__weak u32 omap_sys_boot_device(void)
+{
+	return BOOT_DEVICE_NONE;
+}
+
 void save_omap_boot_params(void)
 {
 	u32 boot_params = *((u32 *)OMAP_SRAM_SCRATCH_BOOT_PARAMS);
@@ -34,9 +39,10 @@  void save_omap_boot_params(void)
 
 	omap_boot_params = (struct omap_boot_parameters *)boot_params;
 
-	/* Boot device */
-
 	boot_device = omap_boot_params->boot_device;
+	boot_mode = MMCSD_MODE_UNDEFINED;
+
+	/* Boot device */
 
 #ifdef BOOT_DEVICE_NAND_I2C
 	/*
@@ -58,16 +64,39 @@  void save_omap_boot_params(void)
 	if (boot_device == BOOT_DEVICE_QSPI_4)
 		boot_device = BOOT_DEVICE_SPI;
 #endif
+#if (defined(BOOT_DEVICE_UART) && !defined(CONFIG_SPL_YMODEM_SUPPORT)) || \
+    (defined(BOOT_DEVICE_USB) && !defined(CONFIG_SPL_USB_SUPPORT)) || \
+    (defined(BOOT_DEVICE_USBETH) && !defined(CONFIG_SPL_USBETH_SUPPORT))
+	/*
+	 * When booting from peripheral booting, the boot device is not usable
+	 * as-is (unless there is support for it), so the boot device is instead
+	 * figured out using the SYS_BOOT pins.
+	 */
+	switch (boot_device) {
+#ifdef BOOT_DEVICE_UART
+	case BOOT_DEVICE_UART:
+#endif
+#ifdef BOOT_DEVICE_USB
+	case BOOT_DEVICE_USB:
+#endif
+		boot_device = omap_sys_boot_device();
+
+		/* MMC raw mode will fallback to FS mode. */
+		if ((boot_device >= MMC_BOOT_DEVICES_START) &&
+		    (boot_device <= MMC_BOOT_DEVICES_END))
+			boot_mode = MMCSD_MODE_RAW;
+
+		break;
+	}
+#endif
 
 	gd->arch.omap_boot_device = boot_device;
 
 	/* Boot mode */
 
-	boot_mode = MMCSD_MODE_UNDEFINED;
-
+#ifdef CONFIG_OMAP34XX
 	if ((boot_device >= MMC_BOOT_DEVICES_START) &&
 	    (boot_device <= MMC_BOOT_DEVICES_END)) {
-#ifdef CONFIG_OMAP34XX
 		switch (boot_device) {
 		case BOOT_DEVICE_MMC1:
 			boot_mode = MMCSD_MODE_FS;
@@ -76,7 +105,16 @@  void save_omap_boot_params(void)
 			boot_mode = MMCSD_MODE_RAW;
 			break;
 		}
+	}
 #else
+	/*
+	 * If the boot device was dynamically changed and doesn't match what
+	 * the bootrom initially booted, we cannot use the boot device
+	 * descriptor to figure out the boot mode.
+	 */
+	if ((boot_device == omap_boot_params->boot_device) &&
+	    (boot_device >= MMC_BOOT_DEVICES_START) &&
+	    (boot_device <= MMC_BOOT_DEVICES_END)) {
 		boot_params = omap_boot_params->boot_device_descriptor;
 		if ((boot_params < NON_SECURE_SRAM_START) ||
 		    (boot_params > NON_SECURE_SRAM_END))
@@ -92,12 +130,12 @@  void save_omap_boot_params(void)
 		if (boot_mode != MMCSD_MODE_FS &&
 		    boot_mode != MMCSD_MODE_RAW)
 #ifdef CONFIG_SUPPORT_EMMC_BOOT
-			boot_mode = MMCSD_MODE_EMMCBOOT
+			boot_mode = MMCSD_MODE_EMMCBOOT;
 #else
 			boot_mode = MMCSD_MODE_UNDEFINED;
 #endif
-#endif
 	}
+#endif
 
 	gd->arch.omap_boot_mode = boot_mode;
 
diff --git a/arch/arm/include/asm/omap_common.h b/arch/arm/include/asm/omap_common.h
index 084ea68..056affc 100644
--- a/arch/arm/include/asm/omap_common.h
+++ b/arch/arm/include/asm/omap_common.h
@@ -697,4 +697,8 @@  static inline u8 is_dra72x(void)
 #define CH_FLAGS_CHFLASH	(1 << 2)
 #define CH_FLAGS_CHMMCSD	(1 << 3)
 
+#ifndef __ASSEMBLY__
+u32 omap_sys_boot_device(void);
+#endif
+
 #endif /* _OMAP_COMMON_H_ */