Hi Paul,
thanks for sending this fix.
Basically i can now bring up my board with UART.
Further i want to discuss the whole thing a bit, before we can finish.
On 25.08.2015 17:40, Paul Kocialkowski wrote:
The SYS_BOOT-based fallback shouldn't only check for one of the conditions of
use and then let the switch/case handle each boot device without enforcing the
conditions for each type of boot device again.
For instance, this behaviour would trigger the fallback for UART when
BOOT_DEVICE_UART is defined, CONFIG_SPL_YMODEM_SUPPORT is enabled (which should
be a show-stopper) and e.g. BOOT_DEVICE_USB is enabled and not
CONFIG_SPL_USB_SUPPORT.
Separating the logic for USB and UART is a first step to solve this.
In addition, a similar problematic behaviour took place when BOOT_DEVICE_USBETH,
BOOT_DEVICE_USB and CONFIG_SPL_USBETH_SUPPORT were enabled and not
CONFIG_SPL_USB_SUPPORT.
Since BOOT_DEVICE_USBETH is only a problem when it's defined to the same value
as BOOT_DEVICE_USB, we can check that BOOT_DEVICE_USBETH and BOOT_DEVICE_USB are
different and if not, that CONFIG_SPL_USBETH_SUPPORT is not enabled to enable
the SYS_BOOT-based fallback mechanism.
Signed-off-by: Paul Kocialkowski <cont...@paulk.fr>
---
arch/arm/cpu/armv7/omap-common/boot-common.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/arch/arm/cpu/armv7/omap-common/boot-common.c
b/arch/arm/cpu/armv7/omap-common/boot-common.c
index 5ec46fa..41f65c0 100644
--- a/arch/arm/cpu/armv7/omap-common/boot-common.c
+++ b/arch/arm/cpu/armv7/omap-common/boot-common.c
@@ -30,6 +30,7 @@ void save_omap_boot_params(void)
{
u32 boot_params = *((u32 *)OMAP_SRAM_SCRATCH_BOOT_PARAMS);
struct omap_boot_parameters *omap_boot_params;
+ int sys_boot_device = 0;
the name of sys_boot_device variable is a bit confusing to me.
It would be more readable if you name it for example "boot_device_invalid".
u32 boot_device;
u32 boot_mode;
@@ -64,31 +65,31 @@ 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:
+#if defined(BOOT_DEVICE_UART) && !defined(CONFIG_SPL_YMODEM_SUPPORT)
+ if (boot_device == BOOT_DEVICE_UART)
+ sys_boot_device = 1;
#endif
A more readable approach could be:
/* detect a inoperable bootdevice passed from ROM-code */
int boot_device_invalid = 0;
switch (boot_device) {
#if !defined(CONFIG_SPL_YMODEM_SUPPORT) && defined(BOOT_DEVICE_UART)
case BOOT_DEVICE_UART:
boot_device_invalid = 1;
break;
#endif
#if !defined(CONFIG_SPL_USBETH_SUPPORT) && defined(BOOT_DEVICE_USBETH)
case BOOT_DEVICE_USBETH:
boot_device_invalid = 1;
break;
#endif
#if !defined(CONFIG_SPL_USB_SUPPORT) && defined(BOOT_DEVICE_USB)
case BOOT_DEVICE_USB:
boot_device_invalid = 1;
break;
#endif
}
if (boot_device_invalid)
boot_device = omap_sys_boot_device();
-#ifdef BOOT_DEVICE_USB
- case BOOT_DEVICE_USB:
+#if defined(BOOT_DEVICE_USB) && !defined(CONFIG_SPL_USB_SUPPORT) && \
+ (!defined(BOOT_DEVICE_USBETH) || \
+ ((BOOT_DEVICE_USBETH != BOOT_DEVICE_USB) || \
+ !defined(CONFIG_SPL_USBETH_SUPPORT)))
+ if (boot_device == BOOT_DEVICE_USB)
+ sys_boot_device = 1;
#endif
I don't see the need of testing "BOOT_DEVICE_USBETH != BOOT_DEVICE_USB",
because they are always different defined in spl.h.
BOOT_DEVICE_USBETH = 0x44
BOOT_DEVICE_USB = 0x45
maybe i'm missing something here.
+
+ if (sys_boot_device) {
boot_device = omap_sys_boot_device();
would it be a good idea to pass the current boot_device to the fallback
function omap_sys_boot_device.
So the plattform fallback could figure out "the next best".
best regards,
Hannes
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
