On Thu, 3 Aug 2023 at 03:32, Simon Glass sjg@google.com wrote:
Hi Maxim,
On Wed, 2 Aug 2023 at 08:11, Maxim Uvarov maxim.uvarov@linaro.org wrote:
subject: U-Boot
commit message please (throughout series)
Signed-off-by: Maxim Uvarov maxim.uvarov@linaro.org
lib/lwip/port/if.c | 256 ++++++++++++++++++++++++++ lib/lwip/port/include/arch/cc.h | 46 +++++ lib/lwip/port/include/arch/sys_arch.h | 59 ++++++ lib/lwip/port/include/limits.h | 0 lib/lwip/port/sys-arch.c | 20 ++ 5 files changed, 381 insertions(+) create mode 100644 lib/lwip/port/if.c create mode 100644 lib/lwip/port/include/arch/cc.h create mode 100644 lib/lwip/port/include/arch/sys_arch.h create mode 100644 lib/lwip/port/include/limits.h create mode 100644 lib/lwip/port/sys-arch.c
I wonder if this port/ directory should go away and this code should be in net/ ? It feels a bit odd, as lib/ code suggests it is for libraries, not the integration with them.
diff --git a/lib/lwip/port/if.c b/lib/lwip/port/if.c new file mode 100644 index 0000000000..2ed59a0c4e --- /dev/null +++ b/lib/lwip/port/if.c @@ -0,0 +1,256 @@ +// SPDX-License-Identifier: GPL-2.0
+/*
- (C) Copyright 2023 Linaro Ltd. maxim.uvarov@linaro.org
- */
+#include <common.h> +#include <command.h> +extern int eth_init(void); /* net.h */ +extern void string_to_enetaddr(const char *addr, uint8_t *enetaddr); /*
net.h */
+extern struct in_addr net_ip; +extern u8 net_ethaddr[6];
Can that go in a header file?
I tried to do as minimal changes as I could. In general these are definitions from include/net.h. And the problem that net.h has not only ethernet things, but also protocol defines which overlaps with lwip protocol defines and data structures. More clean fix will be to clean up net.h and move ip to net/ip.h udp to net/udp.h and so on. So here we can include <net.h> to get eth_init() and friends accessed.
+#include "lwip/debug.h" +#include "lwip/arch.h" +#include "netif/etharp.h" +#include "lwip/stats.h" +#include "lwip/def.h" +#include "lwip/mem.h" +#include "lwip/pbuf.h" +#include "lwip/sys.h" +#include "lwip/netif.h"
+#include "lwip/ip.h"
+#define IFNAME0 'e' +#define IFNAME1 '0'
+static struct pbuf *low_level_input(struct netif *netif); +static int uboot_net_use_lwip;
Can we put this stuff in the UCLASS_ETH private data instead of using statics? They require BSS which is typically not available in SPL builds.
yes, that will work. I expect this flag to be used for compatibility mode. I.e. if it's set before cmd then lwip controls net_loop(), if not set then an original code controls net_loop(). I can even add an argument to eth_init(). But because there are too many eth_init() calls I think I will add an entry to uclass.
+int ulwip_enabled(void) +{
return uboot_net_use_lwip;+}
+/* 1 - in loop
- 0 - no loop
- */
+static int loop_lwip;
+/* ret 1 - in loop
0 - no loop??
This all needs some more detail in the comments
Yes, maybe with UCLASS_ETH some of that will go away.
- */
+int ulwip_in_loop(void) +{
return loop_lwip;+}
+void ulwip_loop_set(int loop) +{
loop_lwip = loop;+}
+static int ulwip_app_err;
+void ulwip_exit(int err) +{
ulwip_app_err = err;ulwip_loop_set(0);+}
+int ulwip_app_get_err(void) +{
return ulwip_app_err;+}
+struct uboot_lwip_if { +};
+static struct netif uboot_netif;
+#define LWIP_PORT_INIT_NETMASK(addr) IP4_ADDR((addr), 255, 255, 255, 0)
+extern uchar *net_rx_packet; +extern int net_rx_packet_len;
header file please
ok, the same thing here, it's from net.h
+int uboot_lwip_poll(void) +{
struct pbuf *p;int err;p = low_level_input(&uboot_netif);if (!p) {printf("error p = low_level_input = NULL\n");return 0;}err = ethernet_input(p, &uboot_netif);if (err)printf("ip4_input err %d\n", err);log_err() ?
But what is going on here? Just return the error code, rather than suppressing it, then the caller can handle it.
Looked more detail - current version ethernet_input() (lwip master) always returns ERR_OK (0). When I wrote I added a return code check for non function. So I expect that it can be considered as a bug if some time later we receive something non 0.
But in general the poll() function has to be void. I will correct it.
return 0;+}
+static struct pbuf *low_level_input(struct netif *netif) +{
struct pbuf *p, *q;u16_t len = net_rx_packet_len;uchar *data = net_rx_packet;+#if ETH_PAD_SIZE
len += ETH_PAD_SIZE; /* allow room for Ethernet padding */Please find a way to drop any use of #if
This can be defined to 0 if not needed, for example. Or you could have a static inline eth_pad_size() in the header file (where #if is permitted)
+#endif
/* We allocate a pbuf chain of pbufs from the pool. */p = pbuf_alloc(PBUF_RAW, len, PBUF_POOL);if (p) {+#if ETH_PAD_SIZE
pbuf_remove_header(p, ETH_PAD_SIZE); /* drop the paddingword */
+#endif
/* We iterate over the pbuf chain until we have read theentire
* packet into the pbuf.*/for (q = p; q != NULL; q = q->next) {/* Read enough bytes to fill this pbuf in thechain. The
Comment style
/*
- Read
* available data in the pbuf is given by theq->len
* variable.* This does not necessarily have to be amemcpy, you can also preallocate
* pbufs for a DMA-enabled MAC and afterreceiving truncate it to the
* actually received size. In this case, ensurethe tot_len member of the
* pbuf is the sum of the chained pbuf lenmembers.
*/MEMCPY(q->payload, data, q->len);data += q->len;}//acknowledge that packet has been read();Space after // (please fix throughout)
+#if ETH_PAD_SIZE
pbuf_add_header(p, ETH_PAD_SIZE); /* reclaim the paddingword */
+#endif
LINK_STATS_INC(link.recv);} else {//drop packet();LINK_STATS_INC(link.memerr);LINK_STATS_INC(link.drop);}return p;+}
+static int ethernetif_input(struct pbuf *p, struct netif *netif) +{
struct ethernetif *ethernetif;ethernetif = netif->state;/* move received packet into a new pbuf */p = low_level_input(netif);/* if no packet could be read, silently ignore this */if (p) {/* pass all packets to ethernet_input, which decideswhat packets it supports */
if (netif->input(p, netif) != ERR_OK) {LWIP_DEBUGF(NETIF_DEBUG, ("%s: IP inputerror\n", __func__));
pbuf_free(p);p = NULL;}}blank line before final return
return 0;+}
+static err_t low_level_output(struct netif *netif, struct pbuf *p) +{
int err;err = eth_send(p->payload, p->len);if (err != 0) {if (err)
printf("eth_send error %d\n", err);return ERR_ABRT;}return ERR_OK;+}
+err_t uboot_lwip_if_init(struct netif *netif) +{
struct uboot_lwip_if *uif = (struct uboot_lwip_if*)malloc(sizeof(struct uboot_lwip_if));
if (!uif) {printf("uboot_lwip_if: out of memory\n");return ERR_MEM;}netif->state = uif;netif->name[0] = IFNAME0;netif->name[1] = IFNAME1;netif->hwaddr_len = ETHARP_HWADDR_LEN;string_to_enetaddr(env_get("ethaddr"), netif->hwaddr);+#if defined(CONFIG_LWIP_LIB_DEBUG)
printf(" MAC: %02X:%02X:%02X:%02X:%02X:%02X\n",netif->hwaddr[0], netif->hwaddr[1], netif->hwaddr[2],netif->hwaddr[3], netif->hwaddr[4], netif->hwaddr[5]);+#endif
+#if LWIP_IPV4
netif->output = etharp_output;+#endif /* LWIP_IPV4 */ +#if LWIP_IPV6
netif->output_ip6 = ethip6_output;+#endif /* LWIP_IPV6 */
You may need to add accessors in the header file (as global_data.h) so you don't need the #if
netif->linkoutput = low_level_output;netif->mtu = 1500;netif->flags = NETIF_FLAG_BROADCAST | NETIF_FLAG_ETHARP |NETIF_FLAG_LINK_UP;
eth_init(); /* activate u-boot eth dev */if (IS_ENABLED(CONFIG_LWIP_LIB_DEBUG)) {printf("Initialized LWIP stack\n");}return ERR_OK;+}
+int uboot_lwip_init(void) +{
ip4_addr_t ipaddr, netmask, gw;if (uboot_net_use_lwip)return CMD_RET_SUCCESS;ip4_addr_set_zero(&gw);ip4_addr_set_zero(&ipaddr);ip4_addr_set_zero(&netmask);ipaddr_aton(env_get("ipaddr"), &ipaddr);ipaddr_aton(env_get("ipaddr"), &netmask);LWIP_PORT_INIT_NETMASK(&netmask);if (IS_ENABLED(CONFIG_LWIP_LIB_DEBUG)) {printf("Starting lwIP, IP: %s\n", ip4addr_ntoa(&ipaddr));printf(" GW: %s\n", ip4addr_ntoa(&gw));printf(" mask: %s\n",ip4addr_ntoa(&netmask));
}if (!netif_add(&uboot_netif, &ipaddr, &netmask, &gw,&uboot_netif, uboot_lwip_if_init,ethernetif_input))
printf("err: netif_add failed!\n");netif_set_up(&uboot_netif);netif_set_link_up(&uboot_netif);+#if LWIP_IPV6
if ()
netif_create_ip6_linklocal_address(&uboot_netif, 1);printf(" IPv6: %s\n",ip6addr_ntoa(netif_ip6_addr(uboot_netif, 0)));
+#endif /* LWIP_IPV6 */
uboot_net_use_lwip = 1;return CMD_RET_SUCCESS;+}
+/* placeholder, not used now */ +void uboot_lwip_destroy(void) +{
uboot_net_use_lwip = 0;+} diff --git a/lib/lwip/port/include/arch/cc.h
b/lib/lwip/port/include/arch/cc.h
new file mode 100644 index 0000000000..db30d7614e --- /dev/null +++ b/lib/lwip/port/include/arch/cc.h @@ -0,0 +1,46 @@ +/* SPDX-License-Identifier: GPL-2.0 */
+/*
It would help to have a little one-line comment as to what these files are for.
- (C) Copyright 2023 Linaro Ltd. maxim.uvarov@linaro.org
- */
+#ifndef LWIP_ARCH_CC_H +#define LWIP_ARCH_CC_H
+#include <linux/types.h> +#include <linux/kernel.h>
+#define LWIP_ERRNO_INCLUDE <errno.h>
+#define LWIP_ERRNO_STDINCLUDE 1 +#define LWIP_NO_UNISTD_H 1 +#define LWIP_TIMEVAL_PRIVATE 1
+extern unsigned int lwip_port_rand(void); +#define LWIP_RAND() (lwip_port_rand())
+/* different handling for unit test, normally not needed */ +#ifdef LWIP_NOASSERT_ON_ERROR +#define LWIP_ERROR(message, expression, handler) do { if
(!(expression)) { \
handler; }} while (0)+#endif
+#define LWIP_DONT_PROVIDE_BYTEORDER_FUNCTIONS
+#define LWIP_PLATFORM_ASSERT(x) do {printf("Assertion "%s" failed at
line %d in %s\n", \
x, __LINE__, __FILE__); } while (0)+static inline int atoi(const char *str)
Can we use U-Boot's version?
+{
int r = 0;int i;for (i = 0; str[i] != '\0'; ++i)r = r * 10 + str[i] - '0';return r;+}
+#define LWIP_ERR_T int
+#endif /* LWIP_ARCH_CC_H */ diff --git a/lib/lwip/port/include/arch/sys_arch.h
b/lib/lwip/port/include/arch/sys_arch.h
new file mode 100644 index 0000000000..8d95146275 --- /dev/null +++ b/lib/lwip/port/include/arch/sys_arch.h @@ -0,0 +1,59 @@ +/* SPDX-License-Identifier: GPL-2.0 */
+/*
- (C) Copyright 2023 Linaro Ltd. maxim.uvarov@linaro.org
- */
+#ifndef LWIP_ARCH_SYS_ARCH_H +#define LWIP_ARCH_SYS_ARCH_H
+#include "lwip/opt.h" +#include "lwip/arch.h" +#include "lwip/err.h"
+#define ERR_NEED_SCHED 123
+void sys_arch_msleep(u32_t delay_ms); +#define sys_msleep(ms) sys_arch_msleep(ms)
+#if SYS_LIGHTWEIGHT_PROT +typedef u32_t sys_prot_t; +#endif /* SYS_LIGHTWEIGHT_PROT */
+#include <errno.h>
+#define SYS_MBOX_NULL NULL +#define SYS_SEM_NULL NULL
+typedef u32_t sys_prot_t;
+struct sys_sem; +typedef struct sys_sem *sys_sem_t; +#define sys_sem_valid(sem) (((sem) != NULL) && (*(sem) != NULL)) +#define sys_sem_set_invalid(sem) do { if ((sem) != NULL) { *(sem) =
NULL; }} while (0)
+/* let sys.h use binary semaphores for mutexes */ +#define LWIP_COMPAT_MUTEX 1 +#define LWIP_COMPAT_MUTEX_ALLOWED 1
+struct sys_mbox; +typedef struct sys_mbox *sys_mbox_t; +#define sys_mbox_valid(mbox) (((mbox) != NULL) && (*(mbox) != NULL)) +#define sys_mbox_set_invalid(mbox) do { if ((mbox) != NULL) { *(mbox) =
NULL; }} while (0)
+struct sys_thread; +typedef struct sys_thread *sys_thread_t;
+static inline u32_t sys_arch_sem_wait(sys_sem_t *sem, u32_t timeout) +{
return 0;+};
+static inline err_t sys_mbox_trypost(sys_mbox_t *mbox, void *msg) +{
return 0;+};
+#define sys_sem_signal(s)
+#endif /* LWIP_ARCH_SYS_ARCH_H */ diff --git a/lib/lwip/port/include/limits.h
b/lib/lwip/port/include/limits.h
new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lib/lwip/port/sys-arch.c b/lib/lwip/port/sys-arch.c new file mode 100644 index 0000000000..609eeccf8c --- /dev/null +++ b/lib/lwip/port/sys-arch.c @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: GPL-2.0
+/*
- (C) Copyright 2023 Linaro Ltd. maxim.uvarov@linaro.org
- */
+#include <common.h> +#include <rand.h> +#include "lwip/opt.h"
+u32_t sys_now(void) +{
return get_timer(0);+}
+u32_t lwip_port_rand(void) +{
return (u32_t)rand();+}
-- 2.30.2
Regards, Simon