From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lukasz Majewski Date: Mon, 10 Aug 2015 19:01:45 +0200 Subject: [U-Boot] [PATCH v2 6/9] update: tftp: dfu: Extend update_tftp() function to support DFU In-Reply-To: References: <1437811877-13764-1-git-send-email-l.majewski@majess.pl> <1437811877-13764-7-git-send-email-l.majewski@majess.pl> Message-ID: <20150810190145.20420a5f@jawa> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Simon, > Hi Lukasz, > > On 25 July 2015 at 02:11, Lukasz Majewski > wrote: > > This code allows using DFU defined mediums for storing data > > received via TFTP protocol. > > > > It reuses and preserves functionality of legacy code at > > common/update.c. > > > > The update_tftp() function now accepts parameters - namely medium > > device name and its number (e.g. mmc 1). > > > > Without this information passed old behavior is preserved. > > > > Signed-off-by: Lukasz Majewski > > --- > > Changes for v2: > > - Remove env variables from update_tftp() function > > - Add parameters to update_tftp() function - without them old > > behavior is preserved > > - Stop compilation when legacy flags (CONFIG_UPDATE_TFTP and > > CONFIG_SYS_NO_FLASH) are wrongly defined > > - In the u-boot code legacy calls to update_tftp(0UL) have been > > changed to update_tftp(0UL, NULL, NULL) > > --- > > common/Makefile | 1 + > > common/cmd_fitupd.c | 2 +- > > common/main.c | 2 +- > > common/update.c | 40 ++++++++++++++++++++++++++++++---------- > > include/net.h | 2 +- > > 5 files changed, 34 insertions(+), 13 deletions(-) > > > > diff --git a/common/Makefile b/common/Makefile > > index d6c1d48..76626f1 100644 > > --- a/common/Makefile > > +++ b/common/Makefile > > @@ -208,6 +208,7 @@ obj-$(CONFIG_LYNXKDI) += lynxkdi.o > > obj-$(CONFIG_MENU) += menu.o > > obj-$(CONFIG_MODEM_SUPPORT) += modem.o > > obj-$(CONFIG_UPDATE_TFTP) += update.o > > +obj-$(CONFIG_DFU_TFTP) += update.o > > obj-$(CONFIG_USB_KEYBOARD) += usb_kbd.o > > obj-$(CONFIG_CMD_DFU) += cmd_dfu.o > > obj-$(CONFIG_CMD_GPT) += cmd_gpt.o > > diff --git a/common/cmd_fitupd.c b/common/cmd_fitupd.c > > index b045974..78b8747 100644 > > --- a/common/cmd_fitupd.c > > +++ b/common/cmd_fitupd.c > > @@ -23,7 +23,7 @@ static int do_fitupd(cmd_tbl_t *cmdtp, int flag, > > int argc, char * const argv[]) if (argc == 2) > > addr = simple_strtoul(argv[1], NULL, 16); > > > > - return update_tftp(addr); > > + return update_tftp(addr, NULL, NULL); > > } > > > > U_BOOT_CMD(fitupd, 2, 0, do_fitupd, > > diff --git a/common/main.c b/common/main.c > > index 2979fbe..ead0cd1 100644 > > --- a/common/main.c > > +++ b/common/main.c > > @@ -75,7 +75,7 @@ void main_loop(void) > > run_preboot_environment_command(); > > > > #if defined(CONFIG_UPDATE_TFTP) > > - update_tftp(0UL); > > + update_tftp(0UL, NULL, NULL); > > #endif /* CONFIG_UPDATE_TFTP */ > > > > s = bootdelay_process(); > > diff --git a/common/update.c b/common/update.c > > index 542915c..1d082b0 100644 > > --- a/common/update.c > > +++ b/common/update.c > > @@ -13,11 +13,16 @@ > > #error "CONFIG_FIT and CONFIG_OF_LIBFDT are required for > > auto-update feature" #endif > > > > +#if defined(CONFIG_UPDATE_TFTP) && defined(CONFIG_SYS_NO_FLASH) > > +#error "CONFIG_UPDATE_TFTP and CONFIG_SYS_NO_FLASH needed for > > legacy behaviour" +#endif > > + > > #include > > #include > > #include > > #include > > #include > > +#include > > > > /* env variable holding the location of the update file */ > > #define UPDATE_FILE_ENV "updatefile" > > @@ -222,13 +227,17 @@ static int update_fit_getparams(const void > > *fit, int noffset, ulong *addr, return 0; > > } > > > > -int update_tftp(ulong addr) > > +int update_tftp(ulong addr, char *interface, char *devstring) > > Should these be const char *? > > > { > > - char *filename, *env_addr; > > - int images_noffset, ndepth, noffset; > > + char *filename, *env_addr, *fit_image_name; > > And these? I'm quite puzzled here. In other places DFU code is operating on the char * strings. Even in the dfu.h file all other functions use char *. To do it right I would need to change char * to const char * in many places at the DFU subsystem. Hence I think that such major change deserves a separate patch series - not related to this one. For the reasons presented above, I would opt for leaving the code as it is and afterwards change char * to const char * globally for DFU subsystem. > > > ulong update_addr, update_fladdr, update_size; > > - void *fit; > > + int images_noffset, ndepth, noffset; > > + bool update_tftp_dfu = false; > > int ret = 0; > > + void *fit; > > + > > + if (interface && devstring) > > + update_tftp_dfu = true; > > > > /* use already present image */ > > if (addr) > > @@ -277,8 +286,8 @@ got_update_file: > > if (ndepth != 1) > > goto next_node; > > > > - printf("Processing update '%s' :", > > - fit_get_name(fit, noffset, NULL)); > > + fit_image_name = (char *)fit_get_name(fit, noffset, > > NULL); > > Can you drop that cast? > > > + printf("Processing update '%s' :", fit_image_name); > > > > if (!fit_image_verify(fit, noffset)) { > > printf("Error: invalid update hash, > > aborting\n"); @@ -294,10 +303,21 @@ got_update_file: > > ret = 1; > > goto next_node; > > } > > - if (update_flash(update_addr, update_fladdr, > > update_size)) { > > - printf("Error: can't flash update, > > aborting\n"); > > - ret = 1; > > - goto next_node; > > + > > + if (!update_tftp_dfu) { > > + if (update_flash(update_addr, update_fladdr, > > + update_size)) { > > + printf("Error: can't flash update, > > aborting\n"); > > + ret = 1; > > + goto next_node; > > + } > > + } else if (fit_image_check_type(fit, noffset, > > + IH_TYPE_FIRMWARE)) { > > + if (dfu_tftp_write(fit_image_name, > > update_addr, > > + update_size, interface, > > devstring)) { > > + ret = 1; > > + goto next_node; > > + } > > } > > next_node: > > noffset = fdt_next_node(fit, noffset, &ndepth); > > diff --git a/include/net.h b/include/net.h > > index d17173d..9af3190 100644 > > --- a/include/net.h > > +++ b/include/net.h > > @@ -804,7 +804,7 @@ void copy_filename(char *dst, const char *src, > > int size); unsigned int random_port(void); > > > > /* Update U-Boot over TFTP */ > > -int update_tftp(ulong addr); > > +int update_tftp(ulong addr, char *interface, char *devstring); > > Function comment - what are the parameters? No problem, I will fix it. > > > > > /**********************************************************************/ > > > > -- > > 2.1.4 > > > > Regards, > Simon Best regards, Lukasz Majewski -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 181 bytes Desc: OpenPGP digital signature URL: