| Message ID | 1439477798-21955-1-git-send-email-lokeshvutla@ti.com |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Tom Rini |
| Headers | show |
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!
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>
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.
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..
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.
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.
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!
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" \
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(+)