* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu
[not found] ` <20170427095527.2a3992fe@jawa>
@ 2017-04-27 8:26 ` B, Ravi
2017-04-27 10:33 ` Lukasz Majewski
0 siblings, 1 reply; 15+ messages in thread
From: B, Ravi @ 2017-04-27 8:26 UTC (permalink / raw)
To: u-boot
Lukasz
>> diff --git a/common/dfu.c b/common/dfu.c index 0e9f5f5..fa77526 100644
>> --- a/common/dfu.c
>> +++ b/common/dfu.c
>> @@ -87,6 +87,9 @@ exit:
>> g_dnl_unregister();
>> board_usb_cleanup(usbctrl_index, USB_INIT_DEVICE);
>>
>> +#ifdef CONFIG_SPL_BUILD
>> + dfu_reset = 0;
>> +#endif
>Why do you only ifdef this part? What problem does this solve?
Common/dfu.c is common code for SPL and U-boot, for SPL-DFU dfu_reset should not be given. This is must fix.
Also this avoid use of run_command for SPL-DFU altogether, SPL size also will reduce by removing cli.c/cli_hush.c
Regards
Ravi
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu
2017-04-27 8:26 ` [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu B, Ravi
@ 2017-04-27 10:33 ` Lukasz Majewski
2017-04-27 10:34 ` Lukasz Majewski
0 siblings, 1 reply; 15+ messages in thread
From: Lukasz Majewski @ 2017-04-27 10:33 UTC (permalink / raw)
To: u-boot
On Thu, 27 Apr 2017 08:26:57 +0000
"B, Ravi" <ravibabu@ti.com> wrote:
> Lukasz
>
> >> diff --git a/common/dfu.c b/common/dfu.c index 0e9f5f5..fa77526
> >> 100644 --- a/common/dfu.c
> >> +++ b/common/dfu.c
> >> @@ -87,6 +87,9 @@ exit:
> >> g_dnl_unregister();
> >> board_usb_cleanup(usbctrl_index, USB_INIT_DEVICE);
> >>
> >> +#ifdef CONFIG_SPL_BUILD
> >> + dfu_reset = 0;
> >> +#endif
>
> >Why do you only ifdef this part? What problem does this solve?
>
> Common/dfu.c is common code for SPL and U-boot, for SPL-DFU dfu_reset
> should not be given. This is must fix. Also this avoid use of
> run_command for SPL-DFU altogether, SPL size also will reduce by
> removing cli.c/cli_hush.c
As I've metioned in the other mail. Kconfig option would be OK.
Please look into the dfu_usb_get_reset() __weak function definition.
It is by default set to true.
You can extend this function to take into account a Kconfig option to
return false during SPL builds.
>
> Regards
> Ravi
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu
2017-04-27 10:33 ` Lukasz Majewski
@ 2017-04-27 10:34 ` Lukasz Majewski
2017-04-27 11:19 ` B, Ravi
0 siblings, 1 reply; 15+ messages in thread
From: Lukasz Majewski @ 2017-04-27 10:34 UTC (permalink / raw)
To: u-boot
Hi,
> On Thu, 27 Apr 2017 08:26:57 +0000
> "B, Ravi" <ravibabu@ti.com> wrote:
>
> > Lukasz
> >
> > >> diff --git a/common/dfu.c b/common/dfu.c index 0e9f5f5..fa77526
> > >> 100644 --- a/common/dfu.c
> > >> +++ b/common/dfu.c
> > >> @@ -87,6 +87,9 @@ exit:
> > >> g_dnl_unregister();
> > >> board_usb_cleanup(usbctrl_index, USB_INIT_DEVICE);
> > >>
> > >> +#ifdef CONFIG_SPL_BUILD
> > >> + dfu_reset = 0;
> > >> +#endif
> >
> > >Why do you only ifdef this part? What problem does this solve?
> >
> > Common/dfu.c is common code for SPL and U-boot, for SPL-DFU
> > dfu_reset should not be given. This is must fix. Also this avoid
> > use of run_command for SPL-DFU altogether, SPL size also will
> > reduce by removing cli.c/cli_hush.c
>
> As I've metioned in the other mail. Kconfig option would be OK.
And this Kconfig should be _only_ enabled for your SPL-DFU support
enabled (also in Kconfig).
>
> Please look into the dfu_usb_get_reset() __weak function definition.
>
> It is by default set to true.
>
> You can extend this function to take into account a Kconfig option to
> return false during SPL builds.
>
> >
> > Regards
> > Ravi
>
>
>
>
> Best regards,
>
> Lukasz Majewski
>
> --
>
> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu
2017-04-27 10:34 ` Lukasz Majewski
@ 2017-04-27 11:19 ` B, Ravi
2017-04-27 13:09 ` Lukasz Majewski
0 siblings, 1 reply; 15+ messages in thread
From: B, Ravi @ 2017-04-27 11:19 UTC (permalink / raw)
To: u-boot
Hi Lukasz
>> > >> +#ifdef CONFIG_SPL_BUILD
>> > >> + dfu_reset = 0;
>> > >> +#endif
>> >
>> > >Why do you only ifdef this part? What problem does this solve?
>> >
>> > Common/dfu.c is common code for SPL and U-boot, for SPL-DFU
>> > dfu_reset should not be given. This is must fix. Also this avoid use
>> > of run_command for SPL-DFU altogether, SPL size also will reduce by
>> > removing cli.c/cli_hush.c
>>
>> As I've metioned in the other mail. Kconfig option would be OK.
> And this Kconfig should be _only_ enabled for your SPL-DFU support enabled (also in Kconfig).
I have already added CONFIG_SPL_DFU_NO_RESET option for SPL-DFU in Kconfig.
>>
>> Please look into the dfu_usb_get_reset() __weak function definition.
>>
>> It is by default set to true.
>>
>> You can extend this function to take into account a Kconfig option to
>> return false during SPL builds.
>>
As suggested by you, I feel this is best option.
I have test verified this option, will post in next patch version.
__weak bool dfu_usb_get_reset(void)
{
+#ifdef CONFIG_SPL_DFU_NO_RESET
+ return false
+#else
return true;
+#endif
}
Also changing run_command() to do_reset().
If (dfu_reset)
do_reset(NULL, 0, 0, NULL);
Regards
Ravi
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 3/3] spl: dfu: reduce spl-dfu MLO size
[not found] ` <20170427095808.4a87f63c@jawa>
@ 2017-04-27 11:21 ` B, Ravi
0 siblings, 0 replies; 15+ messages in thread
From: B, Ravi @ 2017-04-27 11:21 UTC (permalink / raw)
To: u-boot
{Corrected typo u-boot mailing list.}
Hi Lukasz
>> Since spl-dfu does not dfu-reset, there is no need of
>> run_command_cli, hence compiling out cli.c and cli_hush.c to reduce
>> the spl-dfu memory foot print.
>>
[..]
>> */
>> -#ifdef CONFIG_DFU_TFTP
>> +#if defined(CONFIG_DFU_TFTP) && !defined(CONFIG_SPL_BUILD)
>And then somebody would like to use DFU TFTP in the SPL (like you use
>DFU) and we would have new set of problems.
>Sorry, but this is a wrong approach.
I agree with you for TFTP use-case.
But as aligned, in order to reduce SPL size, use SPL-DFU to load and execute u-boot from RAM.
And then use complete DFU facility in u-boot for flashing MMC/SF/NAND etc.
I don't see any harm to allow for DFU_TFTP in SPL.
And SPL shall not support CONFIG_DFU_{MMC/NAND/SF} or any memory device except DFU_RAM.
Regards
Ravi
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu
2017-04-27 11:19 ` B, Ravi
@ 2017-04-27 13:09 ` Lukasz Majewski
2017-04-27 17:30 ` B, Ravi
0 siblings, 1 reply; 15+ messages in thread
From: Lukasz Majewski @ 2017-04-27 13:09 UTC (permalink / raw)
To: u-boot
Hi Ravi,
> Hi Lukasz
>
> >> > >> +#ifdef CONFIG_SPL_BUILD
> >> > >> + dfu_reset = 0;
> >> > >> +#endif
> >> >
> >> > >Why do you only ifdef this part? What problem does this solve?
> >> >
> >> > Common/dfu.c is common code for SPL and U-boot, for SPL-DFU
> >> > dfu_reset should not be given. This is must fix. Also this avoid
> >> > use of run_command for SPL-DFU altogether, SPL size also will
> >> > reduce by removing cli.c/cli_hush.c
> >>
> >> As I've metioned in the other mail. Kconfig option would be OK.
>
> > And this Kconfig should be _only_ enabled for your SPL-DFU support
> > enabled (also in Kconfig).
>
> I have already added CONFIG_SPL_DFU_NO_RESET option for SPL-DFU in
> Kconfig.
>
> >>
> >> Please look into the dfu_usb_get_reset() __weak function
> >> definition.
> >>
> >> It is by default set to true.
> >>
> >> You can extend this function to take into account a Kconfig option
> >> to return false during SPL builds.
> >>
>
> As suggested by you, I feel this is best option.
>
> I have test verified this option, will post in next patch version.
>
> __weak bool dfu_usb_get_reset(void)
> {
> +#ifdef CONFIG_SPL_DFU_NO_RESET
> + return false
> +#else
> return true;
> +#endif
> }
>
> Also changing run_command() to do_reset().
>
> If (dfu_reset)
> do_reset(NULL, 0, 0, NULL);
+1
One question - could you write some numbers before SPL dfu tinification
and afterwards?
I'm just curious how much we can save up.
>
> Regards
> Ravi
>
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu
2017-04-27 13:09 ` Lukasz Majewski
@ 2017-04-27 17:30 ` B, Ravi
0 siblings, 0 replies; 15+ messages in thread
From: B, Ravi @ 2017-04-27 17:30 UTC (permalink / raw)
To: u-boot
Hi Lukasz,
>>
>> Also changing run_command() to do_reset().
>>
>> If (dfu_reset)
>> do_reset(NULL, 0, 0, NULL);
>+1
>One question - could you write some numbers before SPL dfu tinification and afterwards?
>I'm just curious how much we can save up.
I am OOO till May 2nd. Will provide more details once I am back.
Regards
Ravi
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu
2017-04-27 8:06 ` Lukasz Majewski
2017-04-27 8:37 ` B, Ravi
@ 2017-04-27 8:37 ` B, Ravi
1 sibling, 0 replies; 15+ messages in thread
From: B, Ravi @ 2017-04-27 8:37 UTC (permalink / raw)
To: u-boot
Lukasz
>> >
>> > The SPL-DFU will load and execute u-boot.img from RAM. If we issue
>> > dfu-reset (-R switch), this leads to cpu-reset and we lost the
>> > purpose of SPL-DFU itself. Hence dfu-reset issue shall not be
>> > issued for SPL-DFU.
>It seems like a valid use case - maybe it would be beneficial to add Kconfig option (CONFIG_DFU_SPL_NO_RESET) to give the user possibility to decide (and in this way document it?).
Yes, make sense, to differentiate dfu-reset for SPL-DFU.
Ok, I will include CONFIG_SPL_DFU_NO_RESET in next version of patch.
Thanks.
>> >
>> > I agree, the dfu-reset is needed in u-boot, after flashing images
>> > to QSPI/eMMC/SD using the DFU to execute newly loaded image. So,
>> > dfu-reset is needed for u-boot, but not required for SPL-DFU.
>> >
>> > For u-boot, we can continue to use run_command() for dfu-reset.
>>
>> OK. I guess if someone else wants to try and use SPL for DFU
>> flashing that requires more work and they can address the above then, thanks!
>>
>> Reviewed-by: Tom Rini <trini@konsulko.com>
Regards
Ravi
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu
2017-04-27 8:06 ` Lukasz Majewski
@ 2017-04-27 8:37 ` B, Ravi
2017-04-27 8:37 ` B, Ravi
1 sibling, 0 replies; 15+ messages in thread
From: B, Ravi @ 2017-04-27 8:37 UTC (permalink / raw)
To: u-boot
Lukasz
>> >
>> > The SPL-DFU will load and execute u-boot.img from RAM. If we issue
>> > dfu-reset (-R switch), this leads to cpu-reset and we lost the
>> > purpose of SPL-DFU itself. Hence dfu-reset issue shall not be
>> > issued for SPL-DFU.
>It seems like a valid use case - maybe it would be beneficial to add Kconfig option (CONFIG_DFU_SPL_NO_RESET) to give the user possibility to decide (and in this way document it?).
Yes, make sense, to differentiate dfu-reset for SPL-DFU.
Ok, I will include CONFIG_SPL_DFU_NO_RESET in next version of patch.
Thanks.
>> >
>> > I agree, the dfu-reset is needed in u-boot, after flashing images to
>> > QSPI/eMMC/SD using the DFU to execute newly loaded image. So,
>> > dfu-reset is needed for u-boot, but not required for SPL-DFU.
>> >
>> > For u-boot, we can continue to use run_command() for dfu-reset.
>>
>> OK. I guess if someone else wants to try and use SPL for DFU flashing
>> that requires more work and they can address the above then, thanks!
>>
>> Reviewed-by: Tom Rini <trini@konsulko.com>
Regards
Ravi
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu
2017-04-26 16:24 ` Tom Rini
2017-04-26 16:25 ` B, Ravi
@ 2017-04-27 8:06 ` Lukasz Majewski
2017-04-27 8:37 ` B, Ravi
2017-04-27 8:37 ` B, Ravi
1 sibling, 2 replies; 15+ messages in thread
From: Lukasz Majewski @ 2017-04-27 8:06 UTC (permalink / raw)
To: u-boot
On Wed, 26 Apr 2017 12:24:06 -0400
Tom Rini <trini@konsulko.com> wrote:
> On Wed, Apr 26, 2017 at 03:58:27PM +0000, B, Ravi wrote:
> > Hi Tom
> >
> > >> The SPL-DFU feature enable to load and execute u-boot over usb
> > >> from PC using dfu-util.
> > >> Hence dfu-reset should not be issued
> > >> when dfu-util -R switch is issued.
> > >>
> > >> Signed-off-by: Ravi Babu <ravibabu@ti.com>
> > >> ---
> > >> common/dfu.c | 3 +++
> > >> 1 file changed, 3 insertions(+)
> > >>
> > >> diff --git a/common/dfu.c b/common/dfu.c index 0e9f5f5..fa77526
> > >> 100644 --- a/common/dfu.c
> > >> +++ b/common/dfu.c
> > >> @@ -87,6 +87,9 @@ exit:
> > >> g_dnl_unregister();
> > >> board_usb_cleanup(usbctrl_index, USB_INIT_DEVICE);
> > >>
> > >> +#ifdef CONFIG_SPL_BUILD
> > >> + dfu_reset = 0;
> > >> +#endif
> > >> if (dfu_reset)
> > >> run_command("reset", 0);
> >
> > >So we "fix" some of the problems we see by saying that you can't
> > >reset the board in SPL via DFU. I think maybe we should instead
> > >drop run_command here and make reset-via-DFU call do_reset()
> > >directly like some other small-size-required cases do. This will
> > >let us drop the command >requirement here but still allow for "use
> > >DFU to flash and reset the board with just SPL" as a use-case.
> > >Thanks!
> >
> > The SPL-DFU will load and execute u-boot.img from RAM. If we issue
> > dfu-reset (-R switch), this leads to cpu-reset and we lost the
> > purpose of SPL-DFU itself. Hence dfu-reset issue shall not be
> > issued for SPL-DFU.
It seems like a valid use case - maybe it would be beneficial to add
Kconfig option (CONFIG_DFU_SPL_NO_RESET) to give the user possibility
to decide (and in this way document it?).
> >
> > I agree, the dfu-reset is needed in u-boot, after flashing images to
> > QSPI/eMMC/SD using the DFU to execute newly loaded image. So,
> > dfu-reset is needed for u-boot, but not required for SPL-DFU.
> >
> > For u-boot, we can continue to use run_command() for dfu-reset.
>
> OK. I guess if someone else wants to try and use SPL for DFU flashing
> that requires more work and they can address the above then, thanks!
>
> Reviewed-by: Tom Rini <trini@konsulko.com>
>
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170427/f2c24af4/attachment.sig>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu
2017-04-26 16:24 ` Tom Rini
@ 2017-04-26 16:25 ` B, Ravi
2017-04-27 8:06 ` Lukasz Majewski
1 sibling, 0 replies; 15+ messages in thread
From: B, Ravi @ 2017-04-26 16:25 UTC (permalink / raw)
To: u-boot
Hi Tom
>>
>> The SPL-DFU will load and execute u-boot.img from RAM. If we issue
>> dfu-reset (-R switch), this leads to cpu-reset and we lost the purpose
>> of SPL-DFU itself. Hence dfu-reset issue shall not be issued for
>> SPL-DFU.
>>
>> I agree, the dfu-reset is needed in u-boot, after flashing images to
>> QSPI/eMMC/SD using the DFU to execute newly loaded image. So,
>> dfu-reset is needed for u-boot, but not required for SPL-DFU.
>>
>> For u-boot, we can continue to use run_command() for dfu-reset.
>OK. I guess if someone else wants to try and use SPL for DFU flashing that requires more work and they can address the above then, thanks!
>Reviewed-by: Tom Rini <trini@konsulko.com>
Thanks.
Any comments on [PATCH 3/3]?
Regards
Ravi
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu
2017-04-26 15:58 ` B, Ravi
@ 2017-04-26 16:24 ` Tom Rini
2017-04-26 16:25 ` B, Ravi
2017-04-27 8:06 ` Lukasz Majewski
0 siblings, 2 replies; 15+ messages in thread
From: Tom Rini @ 2017-04-26 16:24 UTC (permalink / raw)
To: u-boot
On Wed, Apr 26, 2017 at 03:58:27PM +0000, B, Ravi wrote:
> Hi Tom
>
> >> The SPL-DFU feature enable to load and execute u-boot over usb from PC
> >> using dfu-util.
> >> Hence dfu-reset should not be issued
> >> when dfu-util -R switch is issued.
> >>
> >> Signed-off-by: Ravi Babu <ravibabu@ti.com>
> >> ---
> >> common/dfu.c | 3 +++
> >> 1 file changed, 3 insertions(+)
> >>
> >> diff --git a/common/dfu.c b/common/dfu.c index 0e9f5f5..fa77526 100644
> >> --- a/common/dfu.c
> >> +++ b/common/dfu.c
> >> @@ -87,6 +87,9 @@ exit:
> >> g_dnl_unregister();
> >> board_usb_cleanup(usbctrl_index, USB_INIT_DEVICE);
> >>
> >> +#ifdef CONFIG_SPL_BUILD
> >> + dfu_reset = 0;
> >> +#endif
> >> if (dfu_reset)
> >> run_command("reset", 0);
>
> >So we "fix" some of the problems we see by saying that you can't
> >reset the board in SPL via DFU. I think maybe we should instead drop
> >run_command here and make reset-via-DFU call do_reset() directly like
> >some other small-size-required cases do. This will let us drop the
> >command >requirement here but still allow for "use DFU to flash and
> >reset the board with just SPL" as a use-case. Thanks!
>
> The SPL-DFU will load and execute u-boot.img from RAM. If we issue
> dfu-reset (-R switch), this leads to cpu-reset and we lost the purpose
> of SPL-DFU itself. Hence dfu-reset issue shall not be issued for
> SPL-DFU.
>
> I agree, the dfu-reset is needed in u-boot, after flashing images to
> QSPI/eMMC/SD using the DFU to execute newly loaded image. So,
> dfu-reset is needed for u-boot, but not required for SPL-DFU.
>
> For u-boot, we can continue to use run_command() for dfu-reset.
OK. I guess if someone else wants to try and use SPL for DFU flashing
that requires more work and they can address the above then, thanks!
Reviewed-by: Tom Rini <trini@konsulko.com>
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170426/f8088a03/attachment.sig>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu
2017-04-26 13:40 ` Tom Rini
@ 2017-04-26 15:58 ` B, Ravi
2017-04-26 16:24 ` Tom Rini
0 siblings, 1 reply; 15+ messages in thread
From: B, Ravi @ 2017-04-26 15:58 UTC (permalink / raw)
To: u-boot
Hi Tom
>> The SPL-DFU feature enable to load and execute u-boot over usb from PC
>> using dfu-util.
>> Hence dfu-reset should not be issued
>> when dfu-util -R switch is issued.
>>
>> Signed-off-by: Ravi Babu <ravibabu@ti.com>
>> ---
>> common/dfu.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/common/dfu.c b/common/dfu.c index 0e9f5f5..fa77526 100644
>> --- a/common/dfu.c
>> +++ b/common/dfu.c
>> @@ -87,6 +87,9 @@ exit:
>> g_dnl_unregister();
>> board_usb_cleanup(usbctrl_index, USB_INIT_DEVICE);
>>
>> +#ifdef CONFIG_SPL_BUILD
>> + dfu_reset = 0;
>> +#endif
>> if (dfu_reset)
>> run_command("reset", 0);
>So we "fix" some of the problems we see by saying that you can't reset the board in SPL via DFU.
> I think maybe we should instead drop run_command here and make reset-via-DFU call do_reset() directly like some other small-size-required cases do. This will let us drop the command >requirement here but still allow for "use DFU to flash and reset the board with just SPL" as a use-case. Thanks!
The SPL-DFU will load and execute u-boot.img from RAM. If we issue dfu-reset (-R switch), this leads to cpu-reset and we lost the purpose of SPL-DFU itself.
Hence dfu-reset issue shall not be issued for SPL-DFU.
I agree, the dfu-reset is needed in u-boot, after flashing images to QSPI/eMMC/SD using the DFU to execute newly loaded image.
So, dfu-reset is needed for u-boot, but not required for SPL-DFU.
For u-boot, we can continue to use run_command() for dfu-reset.
Regards
Ravi
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu
2017-04-26 13:14 ` [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu Ravi Babu
@ 2017-04-26 13:40 ` Tom Rini
2017-04-26 15:58 ` B, Ravi
0 siblings, 1 reply; 15+ messages in thread
From: Tom Rini @ 2017-04-26 13:40 UTC (permalink / raw)
To: u-boot
On Wed, Apr 26, 2017 at 06:44:08PM +0530, Ravi Babu wrote:
> The SPL-DFU feature enable to load and
> execute u-boot over usb from PC using
> dfu-util.
> Hence dfu-reset should not be issued
> when dfu-util -R switch is issued.
>
> Signed-off-by: Ravi Babu <ravibabu@ti.com>
> ---
> common/dfu.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/common/dfu.c b/common/dfu.c
> index 0e9f5f5..fa77526 100644
> --- a/common/dfu.c
> +++ b/common/dfu.c
> @@ -87,6 +87,9 @@ exit:
> g_dnl_unregister();
> board_usb_cleanup(usbctrl_index, USB_INIT_DEVICE);
>
> +#ifdef CONFIG_SPL_BUILD
> + dfu_reset = 0;
> +#endif
> if (dfu_reset)
> run_command("reset", 0);
So we "fix" some of the problems we see by saying that you can't reset
the board in SPL via DFU. I think maybe we should instead drop
run_command here and make reset-via-DFU call do_reset() directly like
some other small-size-required cases do. This will let us drop the
command requirement here but still allow for "use DFU to flash and reset
the board with just SPL" as a use-case. Thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170426/b0bf380c/attachment.sig>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu
2017-04-26 13:14 [U-Boot] [PATCH 0/3] spl: dfu: misc fixes and reduce MLO foot print Ravi Babu
@ 2017-04-26 13:14 ` Ravi Babu
2017-04-26 13:40 ` Tom Rini
0 siblings, 1 reply; 15+ messages in thread
From: Ravi Babu @ 2017-04-26 13:14 UTC (permalink / raw)
To: u-boot
The SPL-DFU feature enable to load and
execute u-boot over usb from PC using
dfu-util.
Hence dfu-reset should not be issued
when dfu-util -R switch is issued.
Signed-off-by: Ravi Babu <ravibabu@ti.com>
---
common/dfu.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/common/dfu.c b/common/dfu.c
index 0e9f5f5..fa77526 100644
--- a/common/dfu.c
+++ b/common/dfu.c
@@ -87,6 +87,9 @@ exit:
g_dnl_unregister();
board_usb_cleanup(usbctrl_index, USB_INIT_DEVICE);
+#ifdef CONFIG_SPL_BUILD
+ dfu_reset = 0;
+#endif
if (dfu_reset)
run_command("reset", 0);
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2017-04-27 17:30 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1493212325-31879-1-git-send-email-ravibabu@ti.com>
[not found] ` <1493212325-31879-3-git-send-email-ravibabu@ti.com>
[not found] ` <20170427095527.2a3992fe@jawa>
2017-04-27 8:26 ` [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu B, Ravi
2017-04-27 10:33 ` Lukasz Majewski
2017-04-27 10:34 ` Lukasz Majewski
2017-04-27 11:19 ` B, Ravi
2017-04-27 13:09 ` Lukasz Majewski
2017-04-27 17:30 ` B, Ravi
[not found] ` <1493212325-31879-4-git-send-email-ravibabu@ti.com>
[not found] ` <20170427095808.4a87f63c@jawa>
2017-04-27 11:21 ` [U-Boot] [PATCH 3/3] spl: dfu: reduce spl-dfu MLO size B, Ravi
2017-04-26 13:14 [U-Boot] [PATCH 0/3] spl: dfu: misc fixes and reduce MLO foot print Ravi Babu
2017-04-26 13:14 ` [U-Boot] [PATCH 2/3] common: dfu: ignore reset for spl-dfu Ravi Babu
2017-04-26 13:40 ` Tom Rini
2017-04-26 15:58 ` B, Ravi
2017-04-26 16:24 ` Tom Rini
2017-04-26 16:25 ` B, Ravi
2017-04-27 8:06 ` Lukasz Majewski
2017-04-27 8:37 ` B, Ravi
2017-04-27 8:37 ` B, Ravi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.