On Mon, 14 Aug 2023 at 20:25, Ilias Apalodimas <ilias.apalodi...@linaro.org> wrote:
> > +// SPDX-License-Identifier: GPL-2.0 > > + > > +/* > > + * (C) Copyright 2023 Linaro Ltd. <maxim.uva...@linaro.org> > > + */ > > + > > +#include <common.h> > > +#include <command.h> > > +#include <console.h> > > +#include <bootstage.h> > > + > > +#include "lwip/apps/tftp_client.h" > > +#include "lwip/apps/tftp_server.h" > > +#include <tftp_example.h> > > + > > +#include <string.h> > > + > > +#include <net/ulwip.h> > > + > > +#if LWIP_UDP > > Why do we have this? I dont think it makes sense to start reasoning about > LWIP config options from within U-Boot code. Instead U-Boot makefiles > should enable all the LWIP features we need when a command is included > > That is how example used it. Agree do drop this if. > > + > > +static ulong daddr; > > +static ulong size; > > + > > +static void *tftp_open(const char *fname, const char *mode, u8_t > is_write) > > +{ > > + LWIP_UNUSED_ARG(mode); > > + return NULL; > > +} > > + > > +static void tftp_close(void *handle) > > +{ > > + log_info("\ndone\n"); > > + log_info("Bytes transferred = %ld (0x%lx hex)\n", size, size); > > + > > + bootstage_mark_name(BOOTSTAGE_KERNELREAD_STOP, "tftp_done"); > > + env_set_ulong("filesize", size); > > + ulwip_exit(0); > > +} > > + > > +static int tftp_read(void *handle, void *buf, int bytes) > > +{ > > + return 0; > > +} > > + > > +static int tftp_write(void *handle, struct pbuf *p) > > +{ > > + struct pbuf *q; > > + > > + for (q = p; q != NULL; q = q->next) { > > + memcpy((void *)daddr, q->payload, q->len); > > + daddr += q->len; > > + size += q->len; > > + log_info("#"); > > + } > > + > > + return 0; > > +} > > + > > +static void tftp_error(void *handle, int err, const char *msg, int size) > > +{ > > + char message[100]; > > + > > + LWIP_UNUSED_ARG(handle); > > + > > + memset(message, 0, sizeof(message)); > > + MEMCPY(message, msg, LWIP_MIN(sizeof(message)-1, (size_t)size)); > > + > > + log_info("TFTP error: %d (%s)", err, message); > > +} > > + > > +static const struct tftp_context tftp = { > > + tftp_open, > > + tftp_close, > > + tftp_read, > > + tftp_write, > > + tftp_error > > +}; > > + > > +int ulwip_tftp(ulong addr, char *fname) > > +{ > > + void *f = (void *)0x1; /* unused fake file handle*/ > > + err_t err; > > + ip_addr_t srv; > > + int ret; > > + char *server_ip; > > + > > + if (!fname || addr == 0) > > + return CMD_RET_FAILURE; > > + > > + size = 0; > > + daddr = addr; > > + server_ip = env_get("serverip"); > > + if (!server_ip) { > > + log_err("error: serverip variable has to be set\n"); > > + return CMD_RET_FAILURE; > > + } > > + > > + ret = ipaddr_aton(server_ip, &srv); > > + if (!ret) { > > + log_err("error: ipaddr_aton\n"); > > + return CMD_RET_FAILURE; > > + } > > + > > + log_info("TFTP from server %s; our IP address is %s\n", > > + server_ip, env_get("ipaddr")); > > + log_info("Filename '%s'.\n", fname); > > + log_info("Load address: 0x%lx\n", daddr); > > + log_info("Loading:"); > > + > > + bootstage_mark_name(BOOTSTAGE_KERNELREAD_START, "tftp_start"); > > + > > + err = tftp_init_client(&tftp); > > + if (!(err == ERR_OK || err == ERR_USE)) > > + log_err("tftp_init_client err: %d\n", err); > > + > > + err = tftp_get(f, &srv, TFTP_PORT, fname, TFTP_MODE_OCTET); > > Shouldn't this be part of tftp_read()? > > tft_get() actually does some initialization. Then when the polling loop works tftp_write() or tftp_read() callbacks will be called. So here it should stay as is. > > + /* might return different errors, like routing problems */ > > + if (err != ERR_OK) { > > + log_err("tftp_get err=%d\n", err); > > + return CMD_RET_FAILURE; > > + } > > + > > + env_set_hex("fileaddr", addr); > > + return err; > > +} > > +#else > > +#error "UDP has to be supported" > > +#endif /* LWIP_UDP */ > > -- > > 2.30.2 > > >