[U-Boot] [PATCH 7/8] dfu: command: Provide support for 'dfutftp' command to handle receiving data via TFTP

[Joe Hershberger]

Hi Lukasz,

On Mon, Jul 20, 2015 at 12:59 PM, Lukasz Majewski <l.majewski at majess.pl> wrote:
> Hi Joe,
>
>> Hi Lukasz,
>>
>> On Thu, Jul 16, 2015 at 3:26 PM, Lukasz Majewski
>> <l.majewski at majess.pl> wrote:
>> > Hi Joe,
>> >
>> >
>> >> Hi Lukasz,
>> >>
>> >> On Sun, Jul 12, 2015 at 10:30 AM, Lukasz Majewski
>> >> <l.majewski at 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 at 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 at 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 at lists.denx.de
>> >> > http://lists.denx.de/mailman/listinfo/u-boot
>> >
>> > Best regards,
>> > Lukasz Majewski
>
> Best regards,
> Lukasz Majewski