| Message ID | 1437811877-13764-8-git-send-email-l.majewski@majess.pl |
|---|---|
| State | Superseded |
| Delegated to: | Łukasz Majewski |
| Headers | show |
Hi Lukasz, On 25 July 2015 at 02:11, Lukasz Majewski <l.majewski@majess.pl> wrote: > The "dfu" command has been extended to support transfers via TFTP protocol. > > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl> > --- > Changes for v2: > - Remove "dfutftp" command > - Modify "dfu" command to support optional [tftp] parameter > - Only one flag (CONFIG_DFU_TFTP) needs to be enabled > --- > common/cmd_dfu.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) Reviewed-by: Simon Glass <sjg@chromium.org> See below. > > diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c > index 857148f..aea466b 100644 > --- a/common/cmd_dfu.c > +++ b/common/cmd_dfu.c > @@ -1,6 +1,9 @@ > /* > * cmd_dfu.c -- dfu command > * > + * Copyright (C) 2015 > + * Lukasz Majewski <l.majewski@majess.pl> > + * > * Copyright (C) 2012 Samsung Electronics > * authors: Andrzej Pietrasiewicz <andrzej.p@samsung.com> > * Lukasz Majewski <l.majewski@samsung.com> > @@ -13,6 +16,9 @@ > #include <dfu.h> > #include <g_dnl.h> > #include <usb.h> > +#ifdef CONFIG_DFU_TFTP > +#include <net.h> > +#endif > > static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > { > @@ -26,7 +32,18 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > char *devstring = argv[3]; > > int ret, i = 0; > +#ifdef CONFIG_DFU_TFTP > + unsigned long addr = 0; > + if (!strcmp(usb_controller, "tftp")) { > + usb_controller = argv[2]; > + interface = argv[3]; > + devstring = argv[4]; > + if (argc == 6) > + addr = simple_strtoul(argv[5], NULL, 0); > > + return update_tftp(addr, interface, devstring); > + } > +#endif > ret = dfu_init_env_entities(interface, devstring); > if (ret) > goto done; > @@ -84,9 +101,17 @@ done: > > U_BOOT_CMD(dfu, CONFIG_SYS_MAXARGS, 1, do_dfu, > "Device Firmware Upgrade", > +#ifdef CONFIG_DFU_TFTP > + "[tftp] <USB_controller> <interface> <dev> [list] [addr]\n" It's a bit confusing since tftp looks like a parameter but I think you mean we should type it. Maybe it would be better to have: + "[tftp] <USB_controller> <interface> <dev> [<list>] [<addr>]\n" > +#else > "<USB_controller> <interface> <dev> [list]\n" > +#endif > " - device firmware upgrade via <USB_controller>\n" > " on device <dev>, attached to interface\n" > " <interface>\n" > " [list] - list available alt settings\n" > +#ifdef CONFIG_DFU_TFTP > + " [tftp] - use TFTP protocol to download data\n" > + " [addr] - address where FIT image has been stored\n" > +#endif > ); > -- > 2.1.4 > Regards, Simon
Hi Lukasz, On Sat, Jul 25, 2015 at 3:11 AM, Lukasz Majewski <l.majewski@majess.pl> wrote: > The "dfu" command has been extended to support transfers via TFTP protocol. > > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl> > --- > Changes for v2: > - Remove "dfutftp" command > - Modify "dfu" command to support optional [tftp] parameter > - Only one flag (CONFIG_DFU_TFTP) needs to be enabled > --- > common/cmd_dfu.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c > index 857148f..aea466b 100644 > --- a/common/cmd_dfu.c > +++ b/common/cmd_dfu.c > @@ -1,6 +1,9 @@ > /* > * cmd_dfu.c -- dfu command > * > + * Copyright (C) 2015 > + * Lukasz Majewski <l.majewski@majess.pl> > + * > * Copyright (C) 2012 Samsung Electronics > * authors: Andrzej Pietrasiewicz <andrzej.p@samsung.com> > * Lukasz Majewski <l.majewski@samsung.com> > @@ -13,6 +16,9 @@ > #include <dfu.h> > #include <g_dnl.h> > #include <usb.h> > +#ifdef CONFIG_DFU_TFTP > +#include <net.h> > +#endif Generally you shouldn't need to guard an include. Just include net.h all the time. > static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > { > @@ -26,7 +32,18 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > char *devstring = argv[3]; > > int ret, i = 0; > +#ifdef CONFIG_DFU_TFTP > + unsigned long addr = 0; Maybe you should reinitialize devstring to NULL here? > + if (!strcmp(usb_controller, "tftp")) { This would look a lot clearer if you used argv[1] instead of usb_controller. You are not using it as the usb_controller parameter, it just happens to be the second word. > + usb_controller = argv[2]; You shouldn't be keeping the usb_controller parameter around. It makes no sense for the tftp case. Why not just drop it? > + interface = argv[3]; > + devstring = argv[4]; Is it safe to read argv[4] on your optional parameter without checking for argc >= 5? > + if (argc == 6) > + addr = simple_strtoul(argv[5], NULL, 0); > > + return update_tftp(addr, interface, devstring); > + } > +#endif > ret = dfu_init_env_entities(interface, devstring); > if (ret) > goto done; > @@ -84,9 +101,17 @@ done: > > U_BOOT_CMD(dfu, CONFIG_SYS_MAXARGS, 1, do_dfu, > "Device Firmware Upgrade", > +#ifdef CONFIG_DFU_TFTP > + "[tftp] <USB_controller> <interface> <dev> [list] [addr]\n" Also drop <USB_controller> from the help here. > +#else > "<USB_controller> <interface> <dev> [list]\n" > +#endif > " - device firmware upgrade via <USB_controller>\n" > " on device <dev>, attached to interface\n" > " <interface>\n" > " [list] - list available alt settings\n" > +#ifdef CONFIG_DFU_TFTP > + " [tftp] - use TFTP protocol to download data\n" > + " [addr] - address where FIT image has been stored\n" Probably not helpful to include the '[' and ']' here. They are supposed to indicate optional parameters. We already know they are optional from above. Good to fix up the '[list]' as well. > +#endif > ); > -- > 2.1.4 >
diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c index 857148f..aea466b 100644 --- a/common/cmd_dfu.c +++ b/common/cmd_dfu.c @@ -1,6 +1,9 @@ /* * cmd_dfu.c -- dfu command * + * Copyright (C) 2015 + * Lukasz Majewski <l.majewski@majess.pl> + * * Copyright (C) 2012 Samsung Electronics * authors: Andrzej Pietrasiewicz <andrzej.p@samsung.com> * Lukasz Majewski <l.majewski@samsung.com> @@ -13,6 +16,9 @@ #include <dfu.h> #include <g_dnl.h> #include <usb.h> +#ifdef CONFIG_DFU_TFTP +#include <net.h> +#endif static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { @@ -26,7 +32,18 @@ static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) char *devstring = argv[3]; int ret, i = 0; +#ifdef CONFIG_DFU_TFTP + unsigned long addr = 0; + if (!strcmp(usb_controller, "tftp")) { + usb_controller = argv[2]; + interface = argv[3]; + devstring = argv[4]; + if (argc == 6) + addr = simple_strtoul(argv[5], NULL, 0); + return update_tftp(addr, interface, devstring); + } +#endif ret = dfu_init_env_entities(interface, devstring); if (ret) goto done; @@ -84,9 +101,17 @@ done: U_BOOT_CMD(dfu, CONFIG_SYS_MAXARGS, 1, do_dfu, "Device Firmware Upgrade", +#ifdef CONFIG_DFU_TFTP + "[tftp] <USB_controller> <interface> <dev> [list] [addr]\n" +#else "<USB_controller> <interface> <dev> [list]\n" +#endif " - device firmware upgrade via <USB_controller>\n" " on device <dev>, attached to interface\n" " <interface>\n" " [list] - list available alt settings\n" +#ifdef CONFIG_DFU_TFTP + " [tftp] - use TFTP protocol to download data\n" + " [addr] - address where FIT image has been stored\n" +#endif );
The "dfu" command has been extended to support transfers via TFTP protocol. Signed-off-by: Lukasz Majewski <l.majewski@majess.pl> --- Changes for v2: - Remove "dfutftp" command - Modify "dfu" command to support optional [tftp] parameter - Only one flag (CONFIG_DFU_TFTP) needs to be enabled --- common/cmd_dfu.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)