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