[U-Boot] [PATCH] ARM: OMAP5+: configs: Fix default boot command

[U-Boot] [PATCH] ARM: OMAP5+: configs: Fix default boot command

[Lokesh Vutla]

The default boot command searches for dofastboot varaiable
and does a fastboot if it is set to 1.
But the condition "if test ${dofastboot} -eq 1" always
returns true if dofastboot is not defined and breaking mmc boot.
So make dofastboot as 0 by default and let the runtime
environment set it if fastboot is required.

Reported-by: Yan Liu <yan-liu@ti.com>
Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
 include/configs/ti_omap5_common.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/configs/ti_omap5_common.h b/include/configs/ti_omap5_common.h
index fe04692..1c1f8c0 100644
--- a/include/configs/ti_omap5_common.h
+++ b/include/configs/ti_omap5_common.h
@@ -79,6 +79,7 @@
 	"vram=16M\0" \
 	"partitions=" PARTS_DEFAULT "\0" \
 	"optargs=\0" \
+	"dofastboot=0\0" \
 	"mmcdev=0\0" \
 	"mmcroot=/dev/mmcblk0p2 rw\0" \
 	"mmcrootfstype=ext4 rootwait\0" \
-- 
2.1.4

[U-Boot] [PATCH] ARM: OMAP5+: configs: Fix default boot command

[Nishanth Menon]

On 08/13/2015 09:56 AM, Lokesh Vutla wrote:
> The default boot command searches for dofastboot varaiable
> and does a fastboot if it is set to 1.
> But the condition "if test ${dofastboot} -eq 1" always
> returns true if dofastboot is not defined and breaking mmc boot.
> So make dofastboot as 0 by default and let the runtime
> environment set it if fastboot is required.
> 
> Reported-by: Yan Liu <yan-liu@ti.com>
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
>  include/configs/ti_omap5_common.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/configs/ti_omap5_common.h b/include/configs/ti_omap5_common.h
> index fe04692..1c1f8c0 100644
> --- a/include/configs/ti_omap5_common.h
> +++ b/include/configs/ti_omap5_common.h
> @@ -79,6 +79,7 @@
>  	"vram=16M\0" \
>  	"partitions=" PARTS_DEFAULT "\0" \
>  	"optargs=\0" \
> +	"dofastboot=0\0" \
>  	"mmcdev=0\0" \
>  	"mmcroot=/dev/mmcblk0p2 rw\0" \
>  	"mmcrootfstype=ext4 rootwait\0" \
> 
arch/arm/cpu/armv7/omap-common/boot-common.c sets it to one. so what is
the point of this? dra7_evm defines  CONFIG_USB_FUNCTION_FASTBOOT -> so
it is setting up dofastboot blindly.

Is'nt fixing the source of the issue a better thing to do than depending
on env default -a hoping to save us (which btw will only help opentest
farm).

Looks like the code blindly assumes fastboot mode - which is weird!

-- 
Regards,
Nishanth Menon

[U-Boot] [PATCH] ARM: OMAP5+: configs: Fix default boot command

[Tom Rini]

On Thu, Aug 13, 2015 at 08:26:38PM +0530, Lokesh Vutla wrote:

> The default boot command searches for dofastboot varaiable
> and does a fastboot if it is set to 1.
> But the condition "if test ${dofastboot} -eq 1" always
> returns true if dofastboot is not defined and breaking mmc boot.
> So make dofastboot as 0 by default and let the runtime
> environment set it if fastboot is required.
> 
> Reported-by: Yan Liu <yan-liu@ti.com>
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>

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

-- 
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/20150813/ee6f5923/attachment.sig>

[U-Boot] [PATCH] ARM: OMAP5+: configs: Fix default boot command

[Tom Rini]

On Thu, Aug 13, 2015 at 10:06:08AM -0500, Nishanth Menon wrote:
> On 08/13/2015 09:56 AM, Lokesh Vutla wrote:
> > The default boot command searches for dofastboot varaiable
> > and does a fastboot if it is set to 1.
> > But the condition "if test ${dofastboot} -eq 1" always
> > returns true if dofastboot is not defined and breaking mmc boot.
> > So make dofastboot as 0 by default and let the runtime
> > environment set it if fastboot is required.
> > 
> > Reported-by: Yan Liu <yan-liu@ti.com>
> > Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> > ---
> >  include/configs/ti_omap5_common.h | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/include/configs/ti_omap5_common.h b/include/configs/ti_omap5_common.h
> > index fe04692..1c1f8c0 100644
> > --- a/include/configs/ti_omap5_common.h
> > +++ b/include/configs/ti_omap5_common.h
> > @@ -79,6 +79,7 @@
> >  	"vram=16M\0" \
> >  	"partitions=" PARTS_DEFAULT "\0" \
> >  	"optargs=\0" \
> > +	"dofastboot=0\0" \
> >  	"mmcdev=0\0" \
> >  	"mmcroot=/dev/mmcblk0p2 rw\0" \
> >  	"mmcrootfstype=ext4 rootwait\0" \
> > 
> arch/arm/cpu/armv7/omap-common/boot-common.c sets it to one. so what is
> the point of this? dra7_evm defines  CONFIG_USB_FUNCTION_FASTBOOT -> so
> it is setting up dofastboot blindly.
> 
> Is'nt fixing the source of the issue a better thing to do than depending
> on env default -a hoping to save us (which btw will only help opentest
> farm).
> 
> Looks like the code blindly assumes fastboot mode - which is weird!

As I read things the problem is the env code which checks for dofastboot
but due to HUSH annoyances evalues to true rather than false when we
don't have dofastboot set.

-- 
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/20150813/f8dd5ca7/attachment.sig>

[U-Boot] [PATCH] ARM: OMAP5+: configs: Fix default boot command

[Nishanth Menon]

On Thu, Aug 13, 2015 at 10:25 AM, Tom Rini <trini@konsulko.com> wrote:
> On Thu, Aug 13, 2015 at 10:06:08AM -0500, Nishanth Menon wrote:
>> On 08/13/2015 09:56 AM, Lokesh Vutla wrote:
>> > The default boot command searches for dofastboot varaiable
>> > and does a fastboot if it is set to 1.
>> > But the condition "if test ${dofastboot} -eq 1" always
>> > returns true if dofastboot is not defined and breaking mmc boot.
>> > So make dofastboot as 0 by default and let the runtime
>> > environment set it if fastboot is required.
>> >
>> > Reported-by: Yan Liu <yan-liu@ti.com>
>> > Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> > ---
>> >  include/configs/ti_omap5_common.h | 1 +
>> >  1 file changed, 1 insertion(+)
>> >
>> > diff --git a/include/configs/ti_omap5_common.h b/include/configs/ti_omap5_common.h
>> > index fe04692..1c1f8c0 100644
>> > --- a/include/configs/ti_omap5_common.h
>> > +++ b/include/configs/ti_omap5_common.h
>> > @@ -79,6 +79,7 @@
>> >     "vram=16M\0" \
>> >     "partitions=" PARTS_DEFAULT "\0" \
>> >     "optargs=\0" \
>> > +   "dofastboot=0\0" \
>> >     "mmcdev=0\0" \
>> >     "mmcroot=/dev/mmcblk0p2 rw\0" \
>> >     "mmcrootfstype=ext4 rootwait\0" \
>> >
>> arch/arm/cpu/armv7/omap-common/boot-common.c sets it to one. so what is
>> the point of this? dra7_evm defines  CONFIG_USB_FUNCTION_FASTBOOT -> so
>> it is setting up dofastboot blindly.
>>
>> Is'nt fixing the source of the issue a better thing to do than depending
>> on env default -a hoping to save us (which btw will only help opentest
>> farm).
>>
>> Looks like the code blindly assumes fastboot mode - which is weird!
>
> As I read things the problem is the env code which checks for dofastboot
> but due to HUSH annoyances evalues to true rather than false when we
> don't have dofastboot set.

Then, this belongs to armv7_common? fastboot is not custom to just
dra7/omap5, right? omap3_beagle has the same problem etc.. even better
might have been a hush fix... but then.. anyways..

-- 
---
Regards,
Nishanth Menon

[U-Boot] [PATCH] ARM: OMAP5+: configs: Fix default boot command

[Tom Rini]

On Thu, Aug 13, 2015 at 10:28:55AM -0500, Nishanth Menon wrote:
> On Thu, Aug 13, 2015 at 10:25 AM, Tom Rini <trini@konsulko.com> wrote:
> > On Thu, Aug 13, 2015 at 10:06:08AM -0500, Nishanth Menon wrote:
> >> On 08/13/2015 09:56 AM, Lokesh Vutla wrote:
> >> > The default boot command searches for dofastboot varaiable
> >> > and does a fastboot if it is set to 1.
> >> > But the condition "if test ${dofastboot} -eq 1" always
> >> > returns true if dofastboot is not defined and breaking mmc boot.
> >> > So make dofastboot as 0 by default and let the runtime
> >> > environment set it if fastboot is required.
> >> >
> >> > Reported-by: Yan Liu <yan-liu@ti.com>
> >> > Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> >> > ---
> >> >  include/configs/ti_omap5_common.h | 1 +
> >> >  1 file changed, 1 insertion(+)
> >> >
> >> > diff --git a/include/configs/ti_omap5_common.h b/include/configs/ti_omap5_common.h
> >> > index fe04692..1c1f8c0 100644
> >> > --- a/include/configs/ti_omap5_common.h
> >> > +++ b/include/configs/ti_omap5_common.h
> >> > @@ -79,6 +79,7 @@
> >> >     "vram=16M\0" \
> >> >     "partitions=" PARTS_DEFAULT "\0" \
> >> >     "optargs=\0" \
> >> > +   "dofastboot=0\0" \
> >> >     "mmcdev=0\0" \
> >> >     "mmcroot=/dev/mmcblk0p2 rw\0" \
> >> >     "mmcrootfstype=ext4 rootwait\0" \
> >> >
> >> arch/arm/cpu/armv7/omap-common/boot-common.c sets it to one. so what is
> >> the point of this? dra7_evm defines  CONFIG_USB_FUNCTION_FASTBOOT -> so
> >> it is setting up dofastboot blindly.
> >>
> >> Is'nt fixing the source of the issue a better thing to do than depending
> >> on env default -a hoping to save us (which btw will only help opentest
> >> farm).
> >>
> >> Looks like the code blindly assumes fastboot mode - which is weird!
> >
> > As I read things the problem is the env code which checks for dofastboot
> > but due to HUSH annoyances evalues to true rather than false when we
> > don't have dofastboot set.
> 
> Then, this belongs to armv7_common? fastboot is not custom to just
> dra7/omap5, right? omap3_beagle has the same problem etc.. even better
> might have been a hush fix... but then.. anyways..

I'm sorry, I think you're misreading the code a bit.  "dofastboot" is
only fiddled around with in ti_omap5_common.h _and_
arch/arm/cpu/armv7/omap-common/boot-common.c::fb_set_reboot_flag() which
in turn is only called in fastboot gadget code.  This isn't a generic
fastboot feature, this is a special case opt-in thing.

-- 
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/20150813/42c7742b/attachment.sig>

[U-Boot] [PATCH] ARM: OMAP5+: configs: Fix default boot command

[Nishanth Menon]

On Thu, Aug 13, 2015 at 10:39 AM, Tom Rini <trini@konsulko.com> wrote:
> On Thu, Aug 13, 2015 at 10:28:55AM -0500, Nishanth Menon wrote:
>> On Thu, Aug 13, 2015 at 10:25 AM, Tom Rini <trini@konsulko.com> wrote:
>> > On Thu, Aug 13, 2015 at 10:06:08AM -0500, Nishanth Menon wrote:
>> >> On 08/13/2015 09:56 AM, Lokesh Vutla wrote:
>> >> > The default boot command searches for dofastboot varaiable
>> >> > and does a fastboot if it is set to 1.
>> >> > But the condition "if test ${dofastboot} -eq 1" always
>> >> > returns true if dofastboot is not defined and breaking mmc boot.
>> >> > So make dofastboot as 0 by default and let the runtime
>> >> > environment set it if fastboot is required.
>> >> >
>> >> > Reported-by: Yan Liu <yan-liu@ti.com>
>> >> > Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> >> > ---
>> >> >  include/configs/ti_omap5_common.h | 1 +
>> >> >  1 file changed, 1 insertion(+)
>> >> >
>> >> > diff --git a/include/configs/ti_omap5_common.h b/include/configs/ti_omap5_common.h
>> >> > index fe04692..1c1f8c0 100644
>> >> > --- a/include/configs/ti_omap5_common.h
>> >> > +++ b/include/configs/ti_omap5_common.h
>> >> > @@ -79,6 +79,7 @@
>> >> >     "vram=16M\0" \
>> >> >     "partitions=" PARTS_DEFAULT "\0" \
>> >> >     "optargs=\0" \
>> >> > +   "dofastboot=0\0" \
>> >> >     "mmcdev=0\0" \
>> >> >     "mmcroot=/dev/mmcblk0p2 rw\0" \
>> >> >     "mmcrootfstype=ext4 rootwait\0" \
>> >> >
>> >> arch/arm/cpu/armv7/omap-common/boot-common.c sets it to one. so what is
>> >> the point of this? dra7_evm defines  CONFIG_USB_FUNCTION_FASTBOOT -> so
>> >> it is setting up dofastboot blindly.
>> >>
>> >> Is'nt fixing the source of the issue a better thing to do than depending
>> >> on env default -a hoping to save us (which btw will only help opentest
>> >> farm).
>> >>
>> >> Looks like the code blindly assumes fastboot mode - which is weird!
>> >
>> > As I read things the problem is the env code which checks for dofastboot
>> > but due to HUSH annoyances evalues to true rather than false when we
>> > don't have dofastboot set.
>>
>> Then, this belongs to armv7_common? fastboot is not custom to just
>> dra7/omap5, right? omap3_beagle has the same problem etc.. even better
>> might have been a hush fix... but then.. anyways..
>
> I'm sorry, I think you're misreading the code a bit.  "dofastboot" is
> only fiddled around with in ti_omap5_common.h _and_
> arch/arm/cpu/armv7/omap-common/boot-common.c::fb_set_reboot_flag() which
> in turn is only called in fastboot gadget code.  This isn't a generic
> fastboot feature, this is a special case opt-in thing.

Aaah. ok. thanks for clarifying.


-- 
---
Regards,
Nishanth Menon

[U-Boot] ARM: OMAP5+: configs: Fix default boot command

[Tom Rini]

On Thu, Aug 13, 2015 at 08:26:38PM +0530, Lokesh Vutla wrote:

> The default boot command searches for dofastboot varaiable
> and does a fastboot if it is set to 1.
> But the condition "if test ${dofastboot} -eq 1" always
> returns true if dofastboot is not defined and breaking mmc boot.
> So make dofastboot as 0 by default and let the runtime
> environment set it if fastboot is required.
> 
> Reported-by: Yan Liu <yan-liu@ti.com>
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, 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/20150814/3fa97007/attachment.sig>