| Message ID | 1436715044-18706-8-git-send-email-l.majewski@majess.pl |
|---|---|
| State | Superseded |
| Headers | show |
Hi Lukasz, On Sun, Jul 12, 2015 at 10:30 AM, Lukasz Majewski <l.majewski@majess.pl> wrote: > The new 'dfutftp' command has syntax similar to 'dfu' command. > This new command however, requires some extra env variables to allow > update_tftp() code to work properly. For more explanation, please consult > ./doc/README.dfutftp It would be great if it didn't require them. It would also be great if there were just a dfu command and "tftp" were a subcommand. It's a more common pattern now instead of adding new, top-level commands. > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl> > --- > common/Makefile | 1 + > common/cmd_dfutftp.c | 43 +++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 44 insertions(+) > create mode 100644 common/cmd_dfutftp.c > > diff --git a/common/Makefile b/common/Makefile > index d6c1d48..483905c 100644 > --- a/common/Makefile > +++ b/common/Makefile > @@ -211,6 +211,7 @@ obj-$(CONFIG_UPDATE_TFTP) += update.o > obj-$(CONFIG_USB_KEYBOARD) += usb_kbd.o > obj-$(CONFIG_CMD_DFU) += cmd_dfu.o > obj-$(CONFIG_CMD_GPT) += cmd_gpt.o > +obj-$(CONFIG_CMD_DFUTFTP) += cmd_dfutftp.o > > # Power > obj-$(CONFIG_CMD_PMIC) += cmd_pmic.o > diff --git a/common/cmd_dfutftp.c b/common/cmd_dfutftp.c > new file mode 100644 > index 0000000..2b75a09 > --- /dev/null > +++ b/common/cmd_dfutftp.c > @@ -0,0 +1,43 @@ > +/* > + * cmd_dfutftp.c -- dfutftp command > + * > + * Copyright (C) 2015 > + * Lukasz Majewski <l.majewski@majess.pl> > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#include <common.h> > +#include <net.h> > + > +static > +int do_dfutftp(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > +{ > + unsigned long addr = 0; > + > + if (argc < 4 || argc > 5) > + return CMD_RET_USAGE; > + > + char *interface = argv[2]; > + char *devstring = argv[3]; > + > + if (argc == 5) > + addr = simple_strtoul(argv[4], NULL, 0); > + > + /* Below env variables are descibed in detail at ./doc/README.dfutftp */ > + setenv("update_tftp_exec_at_boot", "true"); > + setenv("update_tftp_dfu", "true"); > + setenv("update_tftp_dfu_interface", interface); > + setenv("update_tftp_dfu_devstring", devstring); > + > + return update_tftp(addr); > +} > + > +U_BOOT_CMD(dfutftp, CONFIG_SYS_MAXARGS, 1, do_dfutftp, > + "Device Firmware Upgrade via TFTP", > + "<ETH_controller> <interface> <dev>\n" > + " - device firmware upgrade via <ETH_controller>\n" > + " using TFTP protocol on device <dev>, attached\n" > + " to interface <interface>\n" > + " [addr] - address where FIT image has been stored\n" > +); > -- > 2.1.4 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot
Hi Joe, > Hi Lukasz, > > On Sun, Jul 12, 2015 at 10:30 AM, Lukasz Majewski > <l.majewski@majess.pl> wrote: > > The new 'dfutftp' command has syntax similar to 'dfu' command. > > This new command however, requires some extra env variables to allow > > update_tftp() code to work properly. For more explanation, please > > consult ./doc/README.dfutftp > > It would be great if it didn't require them. I've described reasoning for them in the previous reply. > It would also be great if > there were just a dfu command and "tftp" were a subcommand. It's a > more common pattern now instead of adding new, top-level commands. Good point. However, I've tried to avoid #ifdefs in the cmd_dfu.c code. Some boards only use USB and they would not need support for tftp in the cmd_dfu.c command. Separate command allows adding the code only when it is needed. > > > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl> > > --- > > common/Makefile | 1 + > > common/cmd_dfutftp.c | 43 > > +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 > > insertions(+) create mode 100644 common/cmd_dfutftp.c > > > > diff --git a/common/Makefile b/common/Makefile > > index d6c1d48..483905c 100644 > > --- a/common/Makefile > > +++ b/common/Makefile > > @@ -211,6 +211,7 @@ obj-$(CONFIG_UPDATE_TFTP) += update.o > > obj-$(CONFIG_USB_KEYBOARD) += usb_kbd.o > > obj-$(CONFIG_CMD_DFU) += cmd_dfu.o > > obj-$(CONFIG_CMD_GPT) += cmd_gpt.o > > +obj-$(CONFIG_CMD_DFUTFTP) += cmd_dfutftp.o > > > > # Power > > obj-$(CONFIG_CMD_PMIC) += cmd_pmic.o > > diff --git a/common/cmd_dfutftp.c b/common/cmd_dfutftp.c > > new file mode 100644 > > index 0000000..2b75a09 > > --- /dev/null > > +++ b/common/cmd_dfutftp.c > > @@ -0,0 +1,43 @@ > > +/* > > + * cmd_dfutftp.c -- dfutftp command > > + * > > + * Copyright (C) 2015 > > + * Lukasz Majewski <l.majewski@majess.pl> > > + * > > + * SPDX-License-Identifier: GPL-2.0+ > > + */ > > + > > +#include <common.h> > > +#include <net.h> > > + > > +static > > +int do_dfutftp(cmd_tbl_t *cmdtp, int flag, int argc, char * const > > argv[]) +{ > > + unsigned long addr = 0; > > + > > + if (argc < 4 || argc > 5) > > + return CMD_RET_USAGE; > > + > > + char *interface = argv[2]; > > + char *devstring = argv[3]; > > + > > + if (argc == 5) > > + addr = simple_strtoul(argv[4], NULL, 0); > > + > > + /* Below env variables are descibed in detail > > at ./doc/README.dfutftp */ > > + setenv("update_tftp_exec_at_boot", "true"); > > + setenv("update_tftp_dfu", "true"); > > + setenv("update_tftp_dfu_interface", interface); > > + setenv("update_tftp_dfu_devstring", devstring); > > + > > + return update_tftp(addr); > > +} > > + > > +U_BOOT_CMD(dfutftp, CONFIG_SYS_MAXARGS, 1, do_dfutftp, > > + "Device Firmware Upgrade via TFTP", > > + "<ETH_controller> <interface> <dev>\n" > > + " - device firmware upgrade via <ETH_controller>\n" > > + " using TFTP protocol on device <dev>, attached\n" > > + " to interface <interface>\n" > > + " [addr] - address where FIT image has been stored\n" > > +); > > -- > > 2.1.4 > > > > _______________________________________________ > > U-Boot mailing list > > U-Boot@lists.denx.de > > http://lists.denx.de/mailman/listinfo/u-boot Best regards, Lukasz Majewski
Hi Lukasz, On Thu, Jul 16, 2015 at 3:26 PM, Lukasz Majewski <l.majewski@majess.pl> wrote: > Hi Joe, > > >> Hi Lukasz, >> >> On Sun, Jul 12, 2015 at 10:30 AM, Lukasz Majewski >> <l.majewski@majess.pl> wrote: >> > The new 'dfutftp' command has syntax similar to 'dfu' command. >> > This new command however, requires some extra env variables to allow >> > update_tftp() code to work properly. For more explanation, please >> > consult ./doc/README.dfutftp >> >> It would be great if it didn't require them. > > I've described reasoning for them in the previous reply. > >> It would also be great if >> there were just a dfu command and "tftp" were a subcommand. It's a >> more common pattern now instead of adding new, top-level commands. > > Good point. However, I've tried to avoid #ifdefs in the cmd_dfu.c code. > Some boards only use USB and they would not need support for tftp in > the cmd_dfu.c command. > > Separate command allows adding the code only when it is needed. That's true, but it seems a simple thing to have an ifdef in the command list. You could even keep the sub-command implementation in a separate file. >> > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl> >> > --- >> > common/Makefile | 1 + >> > common/cmd_dfutftp.c | 43 >> > +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 >> > insertions(+) create mode 100644 common/cmd_dfutftp.c >> > >> > diff --git a/common/Makefile b/common/Makefile >> > index d6c1d48..483905c 100644 >> > --- a/common/Makefile >> > +++ b/common/Makefile >> > @@ -211,6 +211,7 @@ obj-$(CONFIG_UPDATE_TFTP) += update.o >> > obj-$(CONFIG_USB_KEYBOARD) += usb_kbd.o >> > obj-$(CONFIG_CMD_DFU) += cmd_dfu.o >> > obj-$(CONFIG_CMD_GPT) += cmd_gpt.o >> > +obj-$(CONFIG_CMD_DFUTFTP) += cmd_dfutftp.o >> > >> > # Power >> > obj-$(CONFIG_CMD_PMIC) += cmd_pmic.o >> > diff --git a/common/cmd_dfutftp.c b/common/cmd_dfutftp.c >> > new file mode 100644 >> > index 0000000..2b75a09 >> > --- /dev/null >> > +++ b/common/cmd_dfutftp.c >> > @@ -0,0 +1,43 @@ >> > +/* >> > + * cmd_dfutftp.c -- dfutftp command >> > + * >> > + * Copyright (C) 2015 >> > + * Lukasz Majewski <l.majewski@majess.pl> >> > + * >> > + * SPDX-License-Identifier: GPL-2.0+ >> > + */ >> > + >> > +#include <common.h> >> > +#include <net.h> >> > + >> > +static >> > +int do_dfutftp(cmd_tbl_t *cmdtp, int flag, int argc, char * const >> > argv[]) +{ >> > + unsigned long addr = 0; >> > + >> > + if (argc < 4 || argc > 5) >> > + return CMD_RET_USAGE; >> > + >> > + char *interface = argv[2]; >> > + char *devstring = argv[3]; >> > + >> > + if (argc == 5) >> > + addr = simple_strtoul(argv[4], NULL, 0); >> > + >> > + /* Below env variables are descibed in detail >> > at ./doc/README.dfutftp */ >> > + setenv("update_tftp_exec_at_boot", "true"); >> > + setenv("update_tftp_dfu", "true"); >> > + setenv("update_tftp_dfu_interface", interface); >> > + setenv("update_tftp_dfu_devstring", devstring); >> > + >> > + return update_tftp(addr); >> > +} >> > + >> > +U_BOOT_CMD(dfutftp, CONFIG_SYS_MAXARGS, 1, do_dfutftp, >> > + "Device Firmware Upgrade via TFTP", >> > + "<ETH_controller> <interface> <dev>\n" >> > + " - device firmware upgrade via <ETH_controller>\n" >> > + " using TFTP protocol on device <dev>, attached\n" >> > + " to interface <interface>\n" >> > + " [addr] - address where FIT image has been stored\n" >> > +); >> > -- >> > 2.1.4 >> > >> > _______________________________________________ >> > U-Boot mailing list >> > U-Boot@lists.denx.de >> > http://lists.denx.de/mailman/listinfo/u-boot > > Best regards, > Lukasz Majewski
Hi Joe, > Hi Lukasz, > > On Thu, Jul 16, 2015 at 3:26 PM, Lukasz Majewski > <l.majewski@majess.pl> wrote: > > Hi Joe, > > > > > >> Hi Lukasz, > >> > >> On Sun, Jul 12, 2015 at 10:30 AM, Lukasz Majewski > >> <l.majewski@majess.pl> wrote: > >> > The new 'dfutftp' command has syntax similar to 'dfu' command. > >> > This new command however, requires some extra env variables to > >> > allow update_tftp() code to work properly. For more explanation, > >> > please consult ./doc/README.dfutftp > >> > >> It would be great if it didn't require them. > > > > I've described reasoning for them in the previous reply. > > > >> It would also be great if > >> there were just a dfu command and "tftp" were a subcommand. It's a > >> more common pattern now instead of adding new, top-level commands. > > > > Good point. However, I've tried to avoid #ifdefs in the cmd_dfu.c > > code. Some boards only use USB and they would not need support for > > tftp in the cmd_dfu.c command. > > > > Separate command allows adding the code only when it is needed. > > That's true, but it seems a simple thing to have an ifdef in the > command list. > > You could even keep the sub-command implementation in a separate file. If I've understood you correctly - you propose extending cmd_dfu.c to support tftp. In this way we would have following dfu command syntax: dfu [tftp] <usb/eth controller> <medium> <medium number> e.g. dfu [tftp] 0 mmc 1 And implementation of [tftp] part should go to another file? > > >> > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl> > >> > --- > >> > common/Makefile | 1 + > >> > common/cmd_dfutftp.c | 43 > >> > +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 > >> > insertions(+) create mode 100644 common/cmd_dfutftp.c > >> > > >> > diff --git a/common/Makefile b/common/Makefile > >> > index d6c1d48..483905c 100644 > >> > --- a/common/Makefile > >> > +++ b/common/Makefile > >> > @@ -211,6 +211,7 @@ obj-$(CONFIG_UPDATE_TFTP) += update.o > >> > obj-$(CONFIG_USB_KEYBOARD) += usb_kbd.o > >> > obj-$(CONFIG_CMD_DFU) += cmd_dfu.o > >> > obj-$(CONFIG_CMD_GPT) += cmd_gpt.o > >> > +obj-$(CONFIG_CMD_DFUTFTP) += cmd_dfutftp.o > >> > > >> > # Power > >> > obj-$(CONFIG_CMD_PMIC) += cmd_pmic.o > >> > diff --git a/common/cmd_dfutftp.c b/common/cmd_dfutftp.c > >> > new file mode 100644 > >> > index 0000000..2b75a09 > >> > --- /dev/null > >> > +++ b/common/cmd_dfutftp.c > >> > @@ -0,0 +1,43 @@ > >> > +/* > >> > + * cmd_dfutftp.c -- dfutftp command > >> > + * > >> > + * Copyright (C) 2015 > >> > + * Lukasz Majewski <l.majewski@majess.pl> > >> > + * > >> > + * SPDX-License-Identifier: GPL-2.0+ > >> > + */ > >> > + > >> > +#include <common.h> > >> > +#include <net.h> > >> > + > >> > +static > >> > +int do_dfutftp(cmd_tbl_t *cmdtp, int flag, int argc, char * > >> > const argv[]) +{ > >> > + unsigned long addr = 0; > >> > + > >> > + if (argc < 4 || argc > 5) > >> > + return CMD_RET_USAGE; > >> > + > >> > + char *interface = argv[2]; > >> > + char *devstring = argv[3]; > >> > + > >> > + if (argc == 5) > >> > + addr = simple_strtoul(argv[4], NULL, 0); > >> > + > >> > + /* Below env variables are descibed in detail > >> > at ./doc/README.dfutftp */ > >> > + setenv("update_tftp_exec_at_boot", "true"); > >> > + setenv("update_tftp_dfu", "true"); > >> > + setenv("update_tftp_dfu_interface", interface); > >> > + setenv("update_tftp_dfu_devstring", devstring); > >> > + > >> > + return update_tftp(addr); > >> > +} > >> > + > >> > +U_BOOT_CMD(dfutftp, CONFIG_SYS_MAXARGS, 1, do_dfutftp, > >> > + "Device Firmware Upgrade via TFTP", > >> > + "<ETH_controller> <interface> <dev>\n" > >> > + " - device firmware upgrade via <ETH_controller>\n" > >> > + " using TFTP protocol on device <dev>, attached\n" > >> > + " to interface <interface>\n" > >> > + " [addr] - address where FIT image has been > >> > stored\n" +); > >> > -- > >> > 2.1.4 > >> > > >> > _______________________________________________ > >> > U-Boot mailing list > >> > U-Boot@lists.denx.de > >> > http://lists.denx.de/mailman/listinfo/u-boot > > > > Best regards, > > Lukasz Majewski Best regards, Lukasz Majewski
Hi Lukasz, On Mon, Jul 20, 2015 at 12:59 PM, Lukasz Majewski <l.majewski@majess.pl> wrote: > Hi Joe, > >> Hi Lukasz, >> >> On Thu, Jul 16, 2015 at 3:26 PM, Lukasz Majewski >> <l.majewski@majess.pl> wrote: >> > Hi Joe, >> > >> > >> >> Hi Lukasz, >> >> >> >> On Sun, Jul 12, 2015 at 10:30 AM, Lukasz Majewski >> >> <l.majewski@majess.pl> wrote: >> >> > The new 'dfutftp' command has syntax similar to 'dfu' command. >> >> > This new command however, requires some extra env variables to >> >> > allow update_tftp() code to work properly. For more explanation, >> >> > please consult ./doc/README.dfutftp >> >> >> >> It would be great if it didn't require them. >> > >> > I've described reasoning for them in the previous reply. >> > >> >> It would also be great if >> >> there were just a dfu command and "tftp" were a subcommand. It's a >> >> more common pattern now instead of adding new, top-level commands. >> > >> > Good point. However, I've tried to avoid #ifdefs in the cmd_dfu.c >> > code. Some boards only use USB and they would not need support for >> > tftp in the cmd_dfu.c command. >> > >> > Separate command allows adding the code only when it is needed. >> >> That's true, but it seems a simple thing to have an ifdef in the >> command list. >> >> You could even keep the sub-command implementation in a separate file. > > If I've understood you correctly - you propose extending cmd_dfu.c to > support tftp. > > In this way we would have following dfu command syntax: > > dfu [tftp] <usb/eth controller> <medium> <medium number> > > e.g. dfu [tftp] 0 mmc 1 Correct. > And implementation of [tftp] part should go to another file? This is up to you, but that would probably be nicer. >> >> > Signed-off-by: Lukasz Majewski <l.majewski@majess.pl> >> >> > --- >> >> > common/Makefile | 1 + >> >> > common/cmd_dfutftp.c | 43 >> >> > +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 >> >> > insertions(+) create mode 100644 common/cmd_dfutftp.c >> >> > >> >> > diff --git a/common/Makefile b/common/Makefile >> >> > index d6c1d48..483905c 100644 >> >> > --- a/common/Makefile >> >> > +++ b/common/Makefile >> >> > @@ -211,6 +211,7 @@ obj-$(CONFIG_UPDATE_TFTP) += update.o >> >> > obj-$(CONFIG_USB_KEYBOARD) += usb_kbd.o >> >> > obj-$(CONFIG_CMD_DFU) += cmd_dfu.o >> >> > obj-$(CONFIG_CMD_GPT) += cmd_gpt.o >> >> > +obj-$(CONFIG_CMD_DFUTFTP) += cmd_dfutftp.o >> >> > >> >> > # Power >> >> > obj-$(CONFIG_CMD_PMIC) += cmd_pmic.o >> >> > diff --git a/common/cmd_dfutftp.c b/common/cmd_dfutftp.c >> >> > new file mode 100644 >> >> > index 0000000..2b75a09 >> >> > --- /dev/null >> >> > +++ b/common/cmd_dfutftp.c >> >> > @@ -0,0 +1,43 @@ >> >> > +/* >> >> > + * cmd_dfutftp.c -- dfutftp command >> >> > + * >> >> > + * Copyright (C) 2015 >> >> > + * Lukasz Majewski <l.majewski@majess.pl> >> >> > + * >> >> > + * SPDX-License-Identifier: GPL-2.0+ >> >> > + */ >> >> > + >> >> > +#include <common.h> >> >> > +#include <net.h> >> >> > + >> >> > +static >> >> > +int do_dfutftp(cmd_tbl_t *cmdtp, int flag, int argc, char * >> >> > const argv[]) +{ >> >> > + unsigned long addr = 0; >> >> > + >> >> > + if (argc < 4 || argc > 5) >> >> > + return CMD_RET_USAGE; >> >> > + >> >> > + char *interface = argv[2]; >> >> > + char *devstring = argv[3]; >> >> > + >> >> > + if (argc == 5) >> >> > + addr = simple_strtoul(argv[4], NULL, 0); >> >> > + >> >> > + /* Below env variables are descibed in detail >> >> > at ./doc/README.dfutftp */ >> >> > + setenv("update_tftp_exec_at_boot", "true"); >> >> > + setenv("update_tftp_dfu", "true"); >> >> > + setenv("update_tftp_dfu_interface", interface); >> >> > + setenv("update_tftp_dfu_devstring", devstring); >> >> > + >> >> > + return update_tftp(addr); >> >> > +} >> >> > + >> >> > +U_BOOT_CMD(dfutftp, CONFIG_SYS_MAXARGS, 1, do_dfutftp, >> >> > + "Device Firmware Upgrade via TFTP", >> >> > + "<ETH_controller> <interface> <dev>\n" >> >> > + " - device firmware upgrade via <ETH_controller>\n" >> >> > + " using TFTP protocol on device <dev>, attached\n" >> >> > + " to interface <interface>\n" >> >> > + " [addr] - address where FIT image has been >> >> > stored\n" +); >> >> > -- >> >> > 2.1.4 >> >> > >> >> > _______________________________________________ >> >> > U-Boot mailing list >> >> > U-Boot@lists.denx.de >> >> > http://lists.denx.de/mailman/listinfo/u-boot >> > >> > Best regards, >> > Lukasz Majewski > > Best regards, > Lukasz Majewski
diff --git a/common/Makefile b/common/Makefile index d6c1d48..483905c 100644 --- a/common/Makefile +++ b/common/Makefile @@ -211,6 +211,7 @@ obj-$(CONFIG_UPDATE_TFTP) += update.o obj-$(CONFIG_USB_KEYBOARD) += usb_kbd.o obj-$(CONFIG_CMD_DFU) += cmd_dfu.o obj-$(CONFIG_CMD_GPT) += cmd_gpt.o +obj-$(CONFIG_CMD_DFUTFTP) += cmd_dfutftp.o # Power obj-$(CONFIG_CMD_PMIC) += cmd_pmic.o diff --git a/common/cmd_dfutftp.c b/common/cmd_dfutftp.c new file mode 100644 index 0000000..2b75a09 --- /dev/null +++ b/common/cmd_dfutftp.c @@ -0,0 +1,43 @@ +/* + * cmd_dfutftp.c -- dfutftp command + * + * Copyright (C) 2015 + * Lukasz Majewski <l.majewski@majess.pl> + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <net.h> + +static +int do_dfutftp(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{ + unsigned long addr = 0; + + if (argc < 4 || argc > 5) + return CMD_RET_USAGE; + + char *interface = argv[2]; + char *devstring = argv[3]; + + if (argc == 5) + addr = simple_strtoul(argv[4], NULL, 0); + + /* Below env variables are descibed in detail at ./doc/README.dfutftp */ + setenv("update_tftp_exec_at_boot", "true"); + setenv("update_tftp_dfu", "true"); + setenv("update_tftp_dfu_interface", interface); + setenv("update_tftp_dfu_devstring", devstring); + + return update_tftp(addr); +} + +U_BOOT_CMD(dfutftp, CONFIG_SYS_MAXARGS, 1, do_dfutftp, + "Device Firmware Upgrade via TFTP", + "<ETH_controller> <interface> <dev>\n" + " - device firmware upgrade via <ETH_controller>\n" + " using TFTP protocol on device <dev>, attached\n" + " to interface <interface>\n" + " [addr] - address where FIT image has been stored\n" +);
The new 'dfutftp' command has syntax similar to 'dfu' command. This new command however, requires some extra env variables to allow update_tftp() code to work properly. For more explanation, please consult ./doc/README.dfutftp Signed-off-by: Lukasz Majewski <l.majewski@majess.pl> --- common/Makefile | 1 + common/cmd_dfutftp.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 common/cmd_dfutftp.c