[U-Boot] [PATCH] omap-common: SYS_BOOT fallback logic correction

[U-Boot] [PATCH] omap-common: SYS_BOOT fallback logic correction

[Paul Kocialkowski]

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 <contact@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;
 	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
-#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
+
+	if (sys_boot_device) {
 		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;
 
-- 
1.9.1

[U-Boot] [PATCH] omap-common: SYS_BOOT fallback logic correction

[Tom Rini]

On Tue, Aug 25, 2015 at 05:40:53PM +0200, 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 <contact@paulk.fr>

Reviewed-by: Tom Rini <trini@konsulko.com>

But I'm going to wait until Hannes can report back with a Tested-by too,
thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150825/12ee2e2a/attachment.sig>

[U-Boot] [PATCH] omap-common: SYS_BOOT fallback logic correction

[Hannes Schmelzer]

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 <contact@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] [PATCH] omap-common: SYS_BOOT fallback logic correction

[Paul Kocialkowski]

Le mercredi 26 ao?t 2015 ? 08:24 +0200, Hannes Schmelzer a ?crit :
> 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 <contact@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".

sys_boot_device makes sense as in "take the boot device from sys_boot".
Like salted_chips, to indicate that the chips are salted.

The point is not to say that the boot device is "invalid", it's to say
that we have no support for it and better try something else instead.

> >   	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

On all OMAP platforms, USB peripheral loading is done via bulk USB and I
suspect it is the same on am33xx platforms.

However, people did define that as USBETH to allow loading the next
stage via USB ethernet, but it could just as well (and more rightfully)
have been SPL_USB_SUPPORT. The idea is that the platform's spl.h could
define both BOOT_DEVICE_USB and BOOT_DEVICE_USBETH to the same value and
let each board define whether to go with USBETH or simple USB given what
is enabled among CONFIG_SPL_USBETH_SUPPORT and CONFIG_SPL_USB_SUPPORT.

That still sounds like a relevant approach to me, even though it wasn't
implemented that way yet. Currently, am33xx either defines
BOOT_DEVICE_USB or BOOT_DEVICE_USBETH (but not both at the same time).

The point is, we have to be careful about the case where BOOT_DEVICE_USB
and BOOT_DEVICE_USBETH are defined to the same value, because one could
enable the fallback while the other one would still be valid. And in
that case, the fallback would take over, which is not what we want.

Since USBETH is never reported as such by the bootrom, I thought it
would make more sense to have platforms that want the fallback active to
define a proper BOOT_DEVICE_USB and have the boot-common code
double-check that it can legitimately be enabled. It doesn't make such a
big difference to also include USBETH as a proper boot device in that
logic, provided that we keep making sure it won't override USB when both
are defined to the same value (and vice-versa).

On the other hand CPGMAC is indeed network loading, which is peripheral
booting as well. If we agree that the fallback is designed to replace
unsupported peripheral booting devices by the requested *memory* booting
device from SYS_BOOT, it makes sense to add support for it, too.

> #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.

See above.

> > +
> > +	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".

No, the idea here is purely to read from the SYS_BOOT pins and take the
first memory booting device, nothing more. It seems like the most
reliable way to do things, because what's in the SYS_BOOT pins is
supposed to be a working setup.

-- 
Paul Kocialkowski, Replicant developer

Replicant is a fully free Android distribution running on several
devices, a free software mobile operating system putting the emphasis on
freedom and privacy/security.

Website: http://www.replicant.us/
Blog: http://blog.replicant.us/
Wiki/tracker/forums: http://redmine.replicant.us/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150826/f4718bd9/attachment.sig>

[U-Boot] [PATCH] omap-common: SYS_BOOT fallback logic correction

[Tom Rini]

On Wed, Aug 26, 2015 at 12:51:07PM +0200, Paul Kocialkowski wrote:

[snip]
> On all OMAP platforms, USB peripheral loading is done via bulk USB and I
> suspect it is the same on am33xx platforms.

Actually no! :(

On OMAP3/4/5 (so dra7xx/am57xx) it's the same bulk USB.  On
AM33xx/AM43xx it's (in ROM) USB RNDIS.  That's why on am33xx/am43xx we
continue on with USB RNDIS (which is what SPL_USBETH_SUPPORT implements,
not the other USB "ethernet" class).

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150826/e82c5e33/attachment.sig>

[U-Boot] [PATCH] omap-common: SYS_BOOT fallback logic correction

[Paul Kocialkowski]

Le mercredi 26 ao?t 2015 ? 09:23 -0400, Tom Rini a ?crit :
> On Wed, Aug 26, 2015 at 12:51:07PM +0200, Paul Kocialkowski wrote:
> 
> [snip]
> > On all OMAP platforms, USB peripheral loading is done via bulk USB and I
> > suspect it is the same on am33xx platforms.
> 
> Actually no! :(
> 
> On OMAP3/4/5 (so dra7xx/am57xx) it's the same bulk USB.  On
> AM33xx/AM43xx it's (in ROM) USB RNDIS.  That's why on am33xx/am43xx we
> continue on with USB RNDIS (which is what SPL_USBETH_SUPPORT implements,
> not the other USB "ethernet" class).

Interesting. So with this in mind, would it be acceptable to assume that
BOOT_DEVICE_USB and BOOT_DEVICE_USBETH are always two distinct boot
devices and thus not have to care about whether they're equal or not and
just assume they're not, like any other BOOT_DEVICE?

This would greatly simplify the logic and make everything more readable.

I can submit a new patch set in that direction tomorrow.

-- 
Paul Kocialkowski, Replicant developer

Replicant is a fully free Android distribution running on several
devices, a free software mobile operating system putting the emphasis on
freedom and privacy/security.

Website: http://www.replicant.us/
Blog: http://blog.replicant.us/
Wiki/tracker/forums: http://redmine.replicant.us/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150827/1d95d8cd/attachment.sig>

[U-Boot] [PATCH] omap-common: SYS_BOOT fallback logic correction

[Tom Rini]

On Thu, Aug 27, 2015 at 12:59:31AM +0200, Paul Kocialkowski wrote:
> Le mercredi 26 ao?t 2015 ? 09:23 -0400, Tom Rini a ?crit :
> > On Wed, Aug 26, 2015 at 12:51:07PM +0200, Paul Kocialkowski wrote:
> > 
> > [snip]
> > > On all OMAP platforms, USB peripheral loading is done via bulk USB and I
> > > suspect it is the same on am33xx platforms.
> > 
> > Actually no! :(
> > 
> > On OMAP3/4/5 (so dra7xx/am57xx) it's the same bulk USB.  On
> > AM33xx/AM43xx it's (in ROM) USB RNDIS.  That's why on am33xx/am43xx we
> > continue on with USB RNDIS (which is what SPL_USBETH_SUPPORT implements,
> > not the other USB "ethernet" class).
> 
> Interesting. So with this in mind, would it be acceptable to assume that
> BOOT_DEVICE_USB and BOOT_DEVICE_USBETH are always two distinct boot
> devices and thus not have to care about whether they're equal or not and
> just assume they're not, like any other BOOT_DEVICE?
> 
> This would greatly simplify the logic and make everything more readable.
> 
> I can submit a new patch set in that direction tomorrow.

Yes, lets go with that.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150826/74beef88/attachment.sig>

[U-Boot] [PATCH] omap-common: SYS_BOOT fallback logic correction

[Hannes Schmelzer]

On 2015-08-27 00:59, Paul Kocialkowski wrote:
> Le mercredi 26 ao?t 2015 ? 09:23 -0400, Tom Rini a ?crit :
>> On Wed, Aug 26, 2015 at 12:51:07PM +0200, Paul Kocialkowski wrote:
>>
>> [snip]
>>> On all OMAP platforms, USB peripheral loading is done via bulk USB and I
>>> suspect it is the same on am33xx platforms.
>> Actually no! :(
>>
>> On OMAP3/4/5 (so dra7xx/am57xx) it's the same bulk USB.  On
>> AM33xx/AM43xx it's (in ROM) USB RNDIS.  That's why on am33xx/am43xx we
>> continue on with USB RNDIS (which is what SPL_USBETH_SUPPORT implements,
>> not the other USB "ethernet" class).
> Interesting. So with this in mind, would it be acceptable to assume that
> BOOT_DEVICE_USB and BOOT_DEVICE_USBETH are always two distinct boot
> devices and thus not have to care about whether they're equal or not and
> just assume they're not, like any other BOOT_DEVICE?
>
> This would greatly simplify the logic and make everything more readable.
>
> I can submit a new patch set in that direction tomorrow.
Hi Paul,

agree with that, looking forward your new patch.

best regards,
Hannes

[U-Boot] [PATCH] omap-common: SYS_BOOT fallback logic correction

[Paul Kocialkowski]

By the way, would you like to add am33xx support to the SYS_BOOT-based
fallback interface?

There is already get_sysboot_value on am33xx code (that is unused), I
suggest that you use that function to write a proper
omap_sys_boot_device in boot.c, as is done on omap platforms (and get
rid of get_sysboot_value afterwards).

Most of the work is to fill up a boot_devices array to match up each
first *memory* booting devices based on the SYS_BOOT value, for each
supported platform. Currently, am33xx supports:
* AM33xx
* AM43xx
* TI81xx: TI816x (DM816x/AM389x), TI814x (DM814x/AM387x)

That's 4 platforms and 4 TRMs to get the data from. The list of proper
CONFIG defines to each for each platform can be found in
arch/arm/includes/asm/arch-am33xx/spl.h

Let me know if you're interested, I didn't do it at first because I have
no device to test this with.

-- 
Paul Kocialkowski, Replicant developer

Replicant is a fully free Android distribution running on several
devices, a free software mobile operating system putting the emphasis on
freedom and privacy/security.

Website: http://www.replicant.us/
Blog: http://blog.replicant.us/
Wiki/tracker/forums: http://redmine.replicant.us/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150826/836f528e/attachment.sig>