From f5fb734672f3fc78f63ed6a14cbdca2251ba3415 Mon Sep 17 00:00:00 2001 From: "Albert ARIBAUD \\(3ADEV\\)" Date: Mon, 12 Oct 2015 00:02:57 +0200 Subject: net: TFTP: variables cleanup and addition TFTP source and destination port variable names are 'tftpsrcp' and 'tftpdstp' in the code, but 'tftpsrcport' and 'tftpdstport' in the README file. Fix the README. Add environment variable 'tftptimeoutcountmax'. As per the comments about the global variable tftp_timeout_count_max, make sure tftptimeoutcountmax is nonnegative. Introduce configuration option CONFIG_NET_TFTP_VARS, which controls whether environment variables tftpblocksize, tftptimeout, and tftptimoueoutcountmax are read by the TFTP client code. CONFIG_NET_TFTP_VARS defaults to y but can be set to n by targets with to tight size contraints. Make bf527-ezkit set CONFIG_NET_TFTP_VARS to n to keep the target size below limit. --- README | 12 ++++++++++-- configs/bf527-ezkit_defconfig | 1 + net/Kconfig | 10 ++++++++++ net/tftp.c | 17 +++++++++++++++-- 4 files changed, 36 insertions(+), 4 deletions(-) diff --git a/README b/README index d18df54ec9..c0a9ea0ed8 100644 --- a/README +++ b/README @@ -5450,10 +5450,10 @@ List of environment variables (most likely not complete): unset, then it will be made silent if the U-Boot console is silent. - tftpsrcport - If this is set, the value is used for TFTP's + tftpsrcp - If this is set, the value is used for TFTP's UDP source port. - tftpdstport - If this is set, the value is used for TFTP's UDP + tftpdstp - If this is set, the value is used for TFTP's UDP destination port instead of the Well Know Port 69. tftpblocksize - Block size to use for TFTP transfers; if not set, @@ -5467,6 +5467,14 @@ List of environment variables (most likely not complete): faster in networks with high packet loss rates or with unreliable TFTP servers. + tftptimeoutcountmax - maximum count of TFTP timeouts (no + unit, minimum value = 0). Defines how many timeouts + can happen during a single file transfer before that + transfer is aborted. The default is 10, and 0 means + 'no timeouts allowed'. Increasing this value may help + downloads succeed with high packet loss rates, or with + unreliable TFTP servers or client hardware. + vlan - When set to a value < 4095 the traffic over Ethernet is encapsulated/received over 802.1q VLAN tagged frames. diff --git a/configs/bf527-ezkit_defconfig b/configs/bf527-ezkit_defconfig index 2e75225c10..0cc81cc44c 100644 --- a/configs/bf527-ezkit_defconfig +++ b/configs/bf527-ezkit_defconfig @@ -4,3 +4,4 @@ CONFIG_TARGET_BF527_EZKIT=y CONFIG_NET_RANDOM_ETHADDR=y CONFIG_SPI_FLASH=y CONFIG_CC_OPTIMIZE_LIBS_FOR_SPEED=y +CONFIG_NET_TFTP_VARS=n diff --git a/net/Kconfig b/net/Kconfig index 77a2f7e07e..a44a783cae 100644 --- a/net/Kconfig +++ b/net/Kconfig @@ -22,4 +22,14 @@ config NETCONSOLE Support the 'nc' input/output device for networked console. See README.NetConsole for details. +config NET_TFTP_VARS + bool "Control TFTP timeout and count through environment" + default y + help + If set, allows controlling the TFTP timeout through the + environment variable tftptimeout, and the TFTP maximum + timeout count through the variable tftptimeoutcountmax. + If unset, timeout and maximum are hard-defined as 1 second + and 10 timouts per TFTP transfer. + endif # if NET diff --git a/net/tftp.c b/net/tftp.c index 1a5113179a..f2889fe4c9 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -602,7 +602,7 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip, } tftp_prev_block = tftp_cur_block; - timeout_count_max = TIMEOUT_COUNT; + timeout_count_max = tftp_timeout_count_max; net_set_timeout_handler(timeout_ms, tftp_timeout_handler); store_block(tftp_cur_block - 1, pkt + 2, len); @@ -697,12 +697,14 @@ static void tftp_timeout_handler(void) void tftp_start(enum proto_t protocol) { +#if CONFIG_NET_TFTP_VARS char *ep; /* Environment pointer */ /* * Allow the user to choose TFTP blocksize and timeout. * TFTP protocol has a minimal timeout of 1 second. */ + ep = getenv("tftpblocksize"); if (ep != NULL) tftp_block_size_option = simple_strtol(ep, NULL, 10); @@ -717,6 +719,17 @@ void tftp_start(enum proto_t protocol) timeout_ms = 1000; } + ep = getenv("tftptimeoutcountmax"); + if (ep != NULL) + tftp_timeout_count_max = simple_strtol(ep, NULL, 10); + + if (tftp_timeout_count_max < 0) { + printf("TFTP timeout count max (%d ms) negative, set to 0\n", + tftp_timeout_count_max); + tftp_timeout_count_max = 0; + } +#endif + debug("TFTP blocksize = %i, timeout = %ld ms\n", tftp_block_size_option, timeout_ms); @@ -842,7 +855,7 @@ void tftp_start_server(void) puts("Loading: *\b"); - timeout_count_max = TIMEOUT_COUNT; + timeout_count_max = tftp_timeout_count_max; timeout_count = 0; timeout_ms = TIMEOUT; net_set_timeout_handler(timeout_ms, tftp_timeout_handler); -- cgit v1.2.1 From 214dc1da4aa90c40e653c4daa097290fd61b3f10 Mon Sep 17 00:00:00 2001 From: Hannes Petermaier Date: Tue, 25 Aug 2015 12:17:59 +0200 Subject: net: bootp fix vci string on SPL-Boot If CONFIG_CMD_DHCP is enabled, the vci (vendor-class-identifier) string isn't inserted into the bootp-packet during SPL stage because the CONFIG_BOOTP_VCI_STRING instead CONFIG_SPL_NET_VCI_STRING We fix this with testing for CONFIG_SPL_BUILD and testing for existing CONFIG_SPL_NET_VCI_STRING. Signed-off-by: Hannes Schmelzer Acked-by: Joe Hershberger Reviewed-by: Tom Rini --- net/bootp.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/net/bootp.c b/net/bootp.c index b2f8ad4ded..defad73d1a 100644 --- a/net/bootp.c +++ b/net/bootp.c @@ -498,7 +498,9 @@ static int dhcp_extended(u8 *e, int message_type, struct in_addr server_ip, } #endif -#ifdef CONFIG_BOOTP_VCI_STRING +#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_NET_VCI_STRING) + put_vci(e, CONFIG_SPL_NET_VCI_STRING); +#elif defined(CONFIG_BOOTP_VCI_STRING) put_vci(e, CONFIG_BOOTP_VCI_STRING); #endif -- cgit v1.2.1 From 829533287afbc2aff2b8c65eb0cbf8f8ee887ae2 Mon Sep 17 00:00:00 2001 From: Thomas Chou Date: Tue, 25 Aug 2015 20:54:24 +0800 Subject: net: protect status led access in bootp This fixes the error when STATUS_LED_BOOT is not defined. Signed-off-by: Thomas Chou Acked-by: Joe Hershberger --- net/bootp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/bootp.c b/net/bootp.c index defad73d1a..fbdc7cb3a3 100644 --- a/net/bootp.c +++ b/net/bootp.c @@ -349,7 +349,7 @@ static void bootp_handler(uchar *pkt, unsigned dest, struct in_addr sip, /* * Got a good BOOTP reply. Copy the data into our variables. */ -#ifdef CONFIG_STATUS_LED +#if defined(CONFIG_STATUS_LED) && defined(STATUS_LED_BOOT) status_led_set(STATUS_LED_BOOT, STATUS_LED_OFF); #endif -- cgit v1.2.1 From 867d6ae2c9cec6a0f6f3a7472cf8397693f6cc35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Br=C3=BCns?= Date: Thu, 27 Aug 2015 23:53:26 +0200 Subject: net: reject Bootp/DHCP packets with bad OP value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename check_packet to check_reply_packet to make its function more obvious. The check for DHCP_* values is completely off, as it should compare against DHCP option 53 (Message Type). Only valid value for any Bootp/DHCP reply is BOOTREPLY. Signed-off-by: Stefan Brüns Acked-by: Joe Hershberger --- net/bootp.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/net/bootp.c b/net/bootp.c index fbdc7cb3a3..3fac7f3b3a 100644 --- a/net/bootp.c +++ b/net/bootp.c @@ -109,7 +109,8 @@ static bool bootp_match_id(ulong id) return false; } -static int check_packet(uchar *pkt, unsigned dest, unsigned src, unsigned len) +static int check_reply_packet(uchar *pkt, unsigned dest, unsigned src, + unsigned len) { struct bootp_hdr *bp = (struct bootp_hdr *)pkt; int retval = 0; @@ -118,11 +119,7 @@ static int check_packet(uchar *pkt, unsigned dest, unsigned src, unsigned len) retval = -1; else if (len < sizeof(struct bootp_hdr) - OPT_FIELD_SIZE) retval = -2; - else if (bp->bp_op != OP_BOOTREQUEST && - bp->bp_op != OP_BOOTREPLY && - bp->bp_op != DHCP_OFFER && - bp->bp_op != DHCP_ACK && - bp->bp_op != DHCP_NAK) + else if (bp->bp_op != OP_BOOTREPLY) retval = -3; else if (bp->bp_htype != HWT_ETHER) retval = -4; @@ -343,7 +340,7 @@ static void bootp_handler(uchar *pkt, unsigned dest, struct in_addr sip, bp = (struct bootp_hdr *)pkt; /* Filter out pkts we don't want */ - if (check_packet(pkt, dest, src, len)) + if (check_reply_packet(pkt, dest, src, len)) return; /* @@ -960,7 +957,7 @@ static void dhcp_handler(uchar *pkt, unsigned dest, struct in_addr sip, src, dest, len, dhcp_state); /* Filter out pkts we don't want */ - if (check_packet(pkt, dest, src, len)) + if (check_reply_packet(pkt, dest, src, len)) return; debug("DHCPHandler: got DHCP packet: (src=%d, dst=%d, len=%d) state: " -- cgit v1.2.1 From 454d9d3ec81c5b77adaecb9a6254336d4c5775a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Br=C3=BCns?= Date: Thu, 27 Aug 2015 23:57:18 +0200 Subject: net: send RFC1542 compliant value for bootp requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RFC1542, 3.2: "The 'secs' field of a BOOTREQUEST message SHOULD represent the elapsed time, in seconds, since the client sent its first BOOTREQUEST message. Note that this implies that the 'secs' field of the first BOOTREQUEST message SHOULD be set to zero." Signed-off-by: Stefan Brüns Acked-by: Joe Hershberger --- net/bootp.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/net/bootp.c b/net/bootp.c index 3fac7f3b3a..93eff87246 100644 --- a/net/bootp.c +++ b/net/bootp.c @@ -711,7 +711,11 @@ void bootp_request(void) bp->bp_htype = HWT_ETHER; bp->bp_hlen = HWL_ETHER; bp->bp_hops = 0; - bp->bp_secs = htons(get_timer(0) / 1000); + /* + * according to RFC1542, should be 0 on first request, secs since + * first request otherwise + */ + bp->bp_secs = htons(get_timer(bootp_start) / 1000); zero_ip.s_addr = 0; net_write_ip(&bp->bp_ciaddr, zero_ip); net_write_ip(&bp->bp_yiaddr, zero_ip); @@ -905,7 +909,7 @@ static void dhcp_send_request_packet(struct bootp_hdr *bp_offer) bp->bp_htype = HWT_ETHER; bp->bp_hlen = HWL_ETHER; bp->bp_hops = 0; - bp->bp_secs = htons(get_timer(0) / 1000); + bp->bp_secs = htons(get_timer(bootp_start) / 1000); /* Do not set the client IP, your IP, or server IP yet, since it * hasn't been ACK'ed by the server yet */ -- cgit v1.2.1 From c56eb57316ac0094aa2b5b805762d239a18f0c63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Br=C3=BCns?= Date: Fri, 28 Aug 2015 10:15:54 +0200 Subject: net: Fix parsing of Bootp/DHCP option 0 (Pad) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pad has no len byte, so the normal parsing code fails. Signed-off-by: Stefan Brüns Acked-by: Joe Hershberger --- net/bootp.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/net/bootp.c b/net/bootp.c index 93eff87246..1316f00dd8 100644 --- a/net/bootp.c +++ b/net/bootp.c @@ -776,6 +776,9 @@ static void dhcp_process_options(uchar *popt, struct bootp_hdr *bp) while (popt < end && *popt != 0xff) { oplen = *(popt + 1); switch (*popt) { + case 0: + oplen = -1; /* Pad omits len byte */ + break; case 1: net_copy_ip(&net_netmask, (popt + 2)); break; @@ -879,7 +882,13 @@ static int dhcp_message_type(unsigned char *popt) while (*popt != 0xff) { if (*popt == 53) /* DHCP Message Type */ return *(popt + 2); - popt += *(popt + 1) + 2; /* Scan through all options */ + if (*popt == 0) { + /* Pad */ + popt += 1; + } else { + /* Scan through all options */ + popt += *(popt + 1) + 2; + } } return -1; } -- cgit v1.2.1 From 943231119f3caaaca7db4c588e4f3e9a6cec426a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Br=C3=BCns?= Date: Sun, 30 Aug 2015 17:46:43 +0200 Subject: net/arp: Do not run net_start_again() on timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit net_start_again() will be called from net_loop() if state is NETLOOP_FAIL. Signed-off-by: Stefan Brüns Acked-by: Joe Hershberger --- net/arp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/arp.c b/net/arp.c index b8655700a8..b1f12bf10d 100644 --- a/net/arp.c +++ b/net/arp.c @@ -112,7 +112,7 @@ void arp_timeout_check(void) if (arp_wait_try >= ARP_TIMEOUT_COUNT) { puts("\nARP Retry count exceeded; starting again\n"); arp_wait_try = 0; - net_start_again(); + net_set_state(NETLOOP_FAIL); } else { arp_wait_timer_start = t; arp_request(); -- cgit v1.2.1 From 45b47734a0788721c76e3bb621a5133554e0a640 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Br=C3=BCns?= Date: Sun, 30 Aug 2015 17:46:54 +0200 Subject: net/arp: account for ARP delay, avoid duplicate packets on timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit eth_rx() in the main reception loop may trigger sending a packet which is already timed out (or will immediately) upon reception of an ARP reply. As long as the ARP reply is pending, the timeout handler of a packet should be postponed. Happens on TFTP with bad network (e.g. WLAN). Signed-off-by: Stefan Brüns Acked-by: Joe Hershberger --- net/arp.c | 5 +++-- net/arp.h | 2 +- net/net.c | 4 +++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/net/arp.c b/net/arp.c index b1f12bf10d..824d2e9393 100644 --- a/net/arp.c +++ b/net/arp.c @@ -96,12 +96,12 @@ void arp_request(void) arp_raw_request(net_ip, net_null_ethaddr, net_arp_wait_reply_ip); } -void arp_timeout_check(void) +int arp_timeout_check(void) { ulong t; if (!net_arp_wait_packet_ip.s_addr) - return; + return 0; t = get_timer(0); @@ -118,6 +118,7 @@ void arp_timeout_check(void) arp_request(); } } + return 1; } void arp_receive(struct ethernet_hdr *et, struct ip_udp_hdr *ip, int len) diff --git a/net/arp.h b/net/arp.h index 43c6296f7e..a288d618b6 100644 --- a/net/arp.h +++ b/net/arp.h @@ -25,7 +25,7 @@ void arp_init(void); void arp_request(void); void arp_raw_request(struct in_addr source_ip, const uchar *targetEther, struct in_addr target_ip); -void arp_timeout_check(void); +int arp_timeout_check(void); void arp_receive(struct ethernet_hdr *et, struct ip_udp_hdr *ip, int len); #endif /* __ARP_H__ */ diff --git a/net/net.c b/net/net.c index a115ce2892..6f75e3ce06 100644 --- a/net/net.c +++ b/net/net.c @@ -569,7 +569,9 @@ restart: goto done; } - arp_timeout_check(); + if (arp_timeout_check() > 0) { + time_start = get_timer(0); + } /* * Check for a timeout, and run the timeout handler -- cgit v1.2.1 From 4f28c9b169eb3dfa0981f58f9c649a03be39fda0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Br=C3=BCns?= Date: Sun, 30 Aug 2015 17:47:17 +0200 Subject: net: cancel timeout handler after DHCPACK MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Timeout handler should be stopped after reception of DHCPACK. If "autoload" is not set, the handler is immediately replaced by the TFTP handler, otherwise it may trigger before the next boot stage begins. Signed-off-by: Stefan Brüns Acked-by: Joe Hershberger --- net/bootp.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/bootp.c b/net/bootp.c index 1316f00dd8..5d1edf1dcb 100644 --- a/net/bootp.c +++ b/net/bootp.c @@ -1018,6 +1018,7 @@ static void dhcp_handler(uchar *pkt, unsigned dest, struct in_addr sip, dhcp_state = BOUND; printf("DHCP client bound to address %pI4 (%lu ms)\n", &net_ip, get_timer(bootp_start)); + net_set_timeout_handler(0, (thand_f *)0); bootstage_mark_name(BOOTSTAGE_ID_BOOTP_STOP, "bootp_stop"); -- cgit v1.2.1 From 0d2837cc8f2575d0ebad7abb80860c6bd8fa3dae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Br=C3=BCns?= Date: Sun, 30 Aug 2015 17:59:45 +0200 Subject: smsc95xx: Fetch whole burst with 1 URB, avoid framing errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit smsc95xx_recv() does not reassemble bursts spread over multiple URBs. If there is a lot of broadcast traffic, the fifo will fill up to the burst cap limit. Lowering the burst cap to the URB size ensures no packet spans multiple urbs. Caveat, lower limit for working burst cap is 5/33 HS/FS packets. Signed-off-by: Stefan Brüns Acked-by: Joe Hershberger --- drivers/usb/eth/smsc95xx.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/usb/eth/smsc95xx.c b/drivers/usb/eth/smsc95xx.c index dc8fa8891b..0f47ccb0b2 100644 --- a/drivers/usb/eth/smsc95xx.c +++ b/drivers/usb/eth/smsc95xx.c @@ -120,8 +120,9 @@ /* Some extra defines */ #define HS_USB_PKT_SIZE 512 #define FS_USB_PKT_SIZE 64 -#define DEFAULT_HS_BURST_CAP_SIZE (16 * 1024 + 5 * HS_USB_PKT_SIZE) -#define DEFAULT_FS_BURST_CAP_SIZE (6 * 1024 + 33 * FS_USB_PKT_SIZE) +/* 5/33 is lower limit for BURST_CAP to work */ +#define DEFAULT_HS_BURST_CAP_SIZE (5 * HS_USB_PKT_SIZE) +#define DEFAULT_FS_BURST_CAP_SIZE (33 * FS_USB_PKT_SIZE) #define DEFAULT_BULK_IN_DELAY 0x00002000 #define MAX_SINGLE_PACKET_SIZE 2048 #define EEPROM_MAC_OFFSET 0x01 @@ -135,7 +136,7 @@ #define USB_BULK_SEND_TIMEOUT 5000 #define USB_BULK_RECV_TIMEOUT 5000 -#define RX_URB_SIZE 2048 +#define RX_URB_SIZE DEFAULT_HS_BURST_CAP_SIZE #define PHY_CONNECT_TIMEOUT 5000 #define TURBO_MODE -- cgit v1.2.1 From 7aba0f2c2ce80406661ea2dc79ea1892cc93a26f Mon Sep 17 00:00:00 2001 From: Gong Qianyu Date: Mon, 31 Aug 2015 11:34:43 +0800 Subject: net/eth: fix a bug in on_ethaddr() The loop should check all ethenet devices, not only the first device, to set each specified ethaddr, or it'll cause failure when we use other devices. Signed-off-by: Gong Qianyu Acked-by: Joe Hershberger --- net/eth.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/eth.c b/net/eth.c index 2e24b55726..e9b22d823f 100644 --- a/net/eth.c +++ b/net/eth.c @@ -691,6 +691,7 @@ static int on_ethaddr(const char *name, const char *value, enum env_op op, memset(dev->enetaddr, 0, 6); } } + dev = dev->next; } while (dev != eth_devices); return 0; -- cgit v1.2.1 From 219cc94a3f3440ea9c5a29d6b114b371a6aec430 Mon Sep 17 00:00:00 2001 From: Josh Wu Date: Tue, 1 Sep 2015 18:22:55 +0800 Subject: net: change the env name to use const As we don't modify the 'name' parameter, so change it to const. Signed-off-by: Josh Wu Reviewed-by: Simon Glass Acked-by: Joe Hershberger --- include/net.h | 4 ++-- net/eth.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/net.h b/include/net.h index 3a787cc4e9..1aabd7438b 100644 --- a/include/net.h +++ b/include/net.h @@ -233,8 +233,8 @@ void eth_set_current(void); /* set nterface to ethcur var */ int eth_get_dev_index(void); /* get the device index */ void eth_parse_enetaddr(const char *addr, uchar *enetaddr); -int eth_getenv_enetaddr(char *name, uchar *enetaddr); -int eth_setenv_enetaddr(char *name, const uchar *enetaddr); +int eth_getenv_enetaddr(const char *name, uchar *enetaddr); +int eth_setenv_enetaddr(const char *name, const uchar *enetaddr); /* * Get the hardware address for an ethernet interface . diff --git a/net/eth.c b/net/eth.c index e9b22d823f..b978aae101 100644 --- a/net/eth.c +++ b/net/eth.c @@ -31,13 +31,13 @@ void eth_parse_enetaddr(const char *addr, uchar *enetaddr) } } -int eth_getenv_enetaddr(char *name, uchar *enetaddr) +int eth_getenv_enetaddr(const char *name, uchar *enetaddr) { eth_parse_enetaddr(getenv(name), enetaddr); return is_valid_ethaddr(enetaddr); } -int eth_setenv_enetaddr(char *name, const uchar *enetaddr) +int eth_setenv_enetaddr(const char *name, const uchar *enetaddr) { char buf[20]; -- cgit v1.2.1 From ec87b1b39b786cb61c5acc01542119663fb157c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Br=C3=BCns?= Date: Fri, 4 Sep 2015 00:31:48 +0200 Subject: net: Do not overwrite options found in overloaded 'file' field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If 'file' is overloaded, it is wrong to get or put the bootfile name from it/to it. Signed-off-by: Stefan Brüns Acked-by: Joe Hershberger --- net/bootp.c | 44 ++++++++++++++++++-------------------------- 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/net/bootp.c b/net/bootp.c index 5d1edf1dcb..9cf0d64025 100644 --- a/net/bootp.c +++ b/net/bootp.c @@ -64,6 +64,9 @@ char net_root_path[64] = {0,}; /* Our bootpath */ static dhcp_state_t dhcp_state = INIT; static u32 dhcp_leasetime; static struct in_addr dhcp_server_ip; +static u8 dhcp_option_overload; +#define OVERLOAD_FILE 1 +#define OVERLOAD_SNAME 2 static void dhcp_handler(uchar *pkt, unsigned dest, struct in_addr sip, unsigned src, unsigned len); @@ -146,9 +149,14 @@ static void store_net_params(struct bootp_hdr *bp) net_copy_ip(&net_server_ip, &bp->bp_siaddr); memcpy(net_server_ethaddr, ((struct ethernet_hdr *)net_rx_packet)->et_src, 6); - if (strlen(bp->bp_file) > 0) + if ( +#if defined(CONFIG_CMD_DHCP) + !(dhcp_option_overload & OVERLOAD_FILE) && +#endif + (strlen(bp->bp_file) > 0)) { copy_filename(net_boot_file_name, bp->bp_file, sizeof(net_boot_file_name)); + } debug("net_boot_file_name: %s\n", net_boot_file_name); @@ -772,6 +780,7 @@ static void dhcp_process_options(uchar *popt, struct bootp_hdr *bp) #if defined(CONFIG_CMD_SNTP) && defined(CONFIG_BOOTP_TIMEOFFSET) int *to_ptr; #endif + dhcp_option_overload = 0; while (popt < end && *popt != 0xff) { oplen = *(popt + 1); @@ -823,6 +832,9 @@ static void dhcp_process_options(uchar *popt, struct bootp_hdr *bp) case 51: net_copy_u32(&dhcp_leasetime, (u32 *)(popt + 2)); break; + case 52: + dhcp_option_overload = popt[2]; + break; case 53: /* Ignore Message Type Option */ break; case 54: @@ -834,31 +846,11 @@ static void dhcp_process_options(uchar *popt, struct bootp_hdr *bp) break; case 66: /* Ignore TFTP server name */ break; - case 67: /* vendor opt bootfile */ - /* - * I can't use dhcp_vendorex_proc here because I need - * to write into the bootp packet - even then I had to - * pass the bootp packet pointer into here as the - * second arg - */ - size = truncate_sz("Opt Boot File", - sizeof(bp->bp_file), - oplen); - if (bp->bp_file[0] == '\0' && size > 0) { - /* - * only use vendor boot file if we didn't - * receive a boot file in the main non-vendor - * part of the packet - god only knows why - * some vendors chose not to use this perfectly - * good spot to store the boot file (join on - * Tru64 Unix) it seems mind bogglingly crazy - * to me - */ - printf("*** WARNING: using vendor " - "optional boot file\n"); - memcpy(bp->bp_file, popt + 2, size); - bp->bp_file[size] = '\0'; - } + case 67: /* Bootfile option */ + size = truncate_sz("Bootfile", + sizeof(net_boot_file_name), oplen); + memcpy(&net_boot_file_name, popt + 2, size); + net_boot_file_name[size] = 0; break; default: #if defined(CONFIG_BOOTP_VENDOREX) -- cgit v1.2.1 From 774c3e05ec0a9c298e14681fb662c392842c2ced Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Br=C3=BCns?= Date: Fri, 4 Sep 2015 00:31:49 +0200 Subject: net: parse DHCP options from overloaded file/sname fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If Option 52 in the vendor option field signals overloading of the file and/or sname fields, these field may contain additional options. Formatting of file/sname contained options is the same as in the vendor options field, but without the leading magic. Signed-off-by: Stefan Brüns Acked-by: Joe Hershberger --- net/bootp.c | 42 ++++++++++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/net/bootp.c b/net/bootp.c index 9cf0d64025..8aeddb08ea 100644 --- a/net/bootp.c +++ b/net/bootp.c @@ -773,14 +773,12 @@ void bootp_request(void) } #if defined(CONFIG_CMD_DHCP) -static void dhcp_process_options(uchar *popt, struct bootp_hdr *bp) +static void dhcp_process_options(uchar *popt, uchar *end) { - uchar *end = popt + BOOTP_HDR_SIZE; int oplen, size; #if defined(CONFIG_CMD_SNTP) && defined(CONFIG_BOOTP_TIMEOFFSET) int *to_ptr; #endif - dhcp_option_overload = 0; while (popt < end && *popt != 0xff) { oplen = *(popt + 1); @@ -865,6 +863,35 @@ static void dhcp_process_options(uchar *popt, struct bootp_hdr *bp) } } +static void dhcp_packet_process_options(struct bootp_hdr *bp) +{ + uchar *popt = (uchar *)&bp->bp_vend[4]; + uchar *end = popt + BOOTP_HDR_SIZE; + + if (net_read_u32((u32 *)&bp->bp_vend[0]) != htonl(BOOTP_VENDOR_MAGIC)) + return; + + dhcp_option_overload = 0; + + /* + * The 'options' field MUST be interpreted first, 'file' next, + * 'sname' last. + */ + dhcp_process_options(popt, end); + + if (dhcp_option_overload & OVERLOAD_FILE) { + popt = (uchar *)bp->bp_file; + end = popt + sizeof(bp->bp_file); + dhcp_process_options(popt, end); + } + + if (dhcp_option_overload & OVERLOAD_SNAME) { + popt = (uchar *)bp->bp_sname; + end = popt + sizeof(bp->bp_sname); + dhcp_process_options(popt, end); + } +} + static int dhcp_message_type(unsigned char *popt) { if (net_read_u32((u32 *)popt) != htonl(BOOTP_VENDOR_MAGIC)) @@ -982,14 +1009,11 @@ static void dhcp_handler(uchar *pkt, unsigned dest, struct in_addr sip, CONFIG_SYS_BOOTFILE_PREFIX, strlen(CONFIG_SYS_BOOTFILE_PREFIX)) == 0) { #endif /* CONFIG_SYS_BOOTFILE_PREFIX */ + dhcp_packet_process_options(bp); debug("TRANSITIONING TO REQUESTING STATE\n"); dhcp_state = REQUESTING; - if (net_read_u32((u32 *)&bp->bp_vend[0]) == - htonl(BOOTP_VENDOR_MAGIC)) - dhcp_process_options((u8 *)&bp->bp_vend[4], bp); - net_set_timeout_handler(5000, bootp_timeout_handler); dhcp_send_request_packet(bp); #ifdef CONFIG_SYS_BOOTFILE_PREFIX @@ -1002,9 +1026,7 @@ static void dhcp_handler(uchar *pkt, unsigned dest, struct in_addr sip, debug("DHCP State: REQUESTING\n"); if (dhcp_message_type((u8 *)bp->bp_vend) == DHCP_ACK) { - if (net_read_u32((u32 *)&bp->bp_vend[0]) == - htonl(BOOTP_VENDOR_MAGIC)) - dhcp_process_options((u8 *)&bp->bp_vend[4], bp); + dhcp_packet_process_options(bp); /* Store net params from reply */ store_net_params(bp); dhcp_state = BOUND; -- cgit v1.2.1 From 4a4ced0b6fde31fe28395e710bcee36f82f29730 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Br=C3=BCns?= Date: Tue, 8 Sep 2015 05:12:00 +0200 Subject: smsc95xx: Use zero length packets when RX fifo is empty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using NAKs on empty RX fifo for bulk in transfers is the right choice for a interrupt driven model, but U-Boot uses polling and expects an immediate answer if there is no incoming packet. Using ZLP Bulk In Response (BIR) mode avoids unexpected timeouts in the host controller driver. As ZLP mode is reset default, there is no need to set it. Signed-off-by: Stefan Brüns Acked-by: Joe Hershberger --- drivers/usb/eth/smsc95xx.c | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/drivers/usb/eth/smsc95xx.c b/drivers/usb/eth/smsc95xx.c index 0f47ccb0b2..e694364122 100644 --- a/drivers/usb/eth/smsc95xx.c +++ b/drivers/usb/eth/smsc95xx.c @@ -530,22 +530,6 @@ static int smsc95xx_init_common(struct usb_device *udev, struct ueth_data *dev, if (ret < 0) return ret; - ret = smsc95xx_read_reg(udev, HW_CFG, &read_buf); - if (ret < 0) - return ret; - debug("Read Value from HW_CFG : 0x%08x\n", read_buf); - - read_buf |= HW_CFG_BIR_; - ret = smsc95xx_write_reg(udev, HW_CFG, read_buf); - if (ret < 0) - return ret; - - ret = smsc95xx_read_reg(udev, HW_CFG, &read_buf); - if (ret < 0) - return ret; - debug("Read Value from HW_CFG after writing " - "HW_CFG_BIR_: 0x%08x\n", read_buf); - #ifdef TURBO_MODE if (dev->pusb_dev->speed == USB_SPEED_HIGH) { burst_cap = DEFAULT_HS_BURST_CAP_SIZE / HS_USB_PKT_SIZE; -- cgit v1.2.1 From 6617f87668dec898470224fe9fb22c0807649a1c Mon Sep 17 00:00:00 2001 From: Sylvain Lemieux Date: Wed, 9 Sep 2015 16:29:51 -0400 Subject: net: phy: micrel: add support for KSZ8021RNL & KSZ8031RNL This patch adds support for Micrel KSZ8021RNL & KSZ8031RNL. Signed-off-by: Sylvain Lemieux Acked-by: Joe Hershberger --- drivers/net/phy/micrel.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c index 49f444ac4c..cbec928ce4 100644 --- a/drivers/net/phy/micrel.c +++ b/drivers/net/phy/micrel.c @@ -22,6 +22,16 @@ static struct phy_driver KSZ804_driver = { .shutdown = &genphy_shutdown, }; +static struct phy_driver KSZ8031_driver = { + .name = "Micrel KSZ8021/KSZ8031", + .uid = 0x221550, + .mask = 0xfffff0, + .features = PHY_BASIC_FEATURES, + .config = &genphy_config, + .startup = &genphy_startup, + .shutdown = &genphy_shutdown, +}; + static struct phy_driver KSZ8081_driver = { .name = "Micrel KSZ8081", .uid = 0x221560, @@ -282,6 +292,7 @@ static struct phy_driver ksz9031_driver = { int phy_micrel_init(void) { phy_register(&KSZ804_driver); + phy_register(&KSZ8031_driver); phy_register(&KSZ8081_driver); #ifdef CONFIG_PHY_MICREL_KSZ9021 phy_register(&ksz9021_driver); -- cgit v1.2.1 From 11a69ff85b0a459f4ac096ae054d0b527d44b095 Mon Sep 17 00:00:00 2001 From: Jacob Stiffler Date: Wed, 30 Sep 2015 10:12:05 -0400 Subject: net: Increase the size of the net_boot_file_name buffer The net_boot_file_name buffer is used as storage for the bootfilename command line argument to network boot commands such as tftp and nfs. Increase the size of this buffer to 1024 bytes as the current size of 128 bytes is restrictive for arbitrary paths on the server. Signed-off-by: Jacob Stiffler Acked-by: Joe Hershberger --- include/net.h | 2 +- net/net.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/net.h b/include/net.h index 1aabd7438b..ebed29ad57 100644 --- a/include/net.h +++ b/include/net.h @@ -516,7 +516,7 @@ enum proto_t { TFTPSRV, TFTPPUT, LINKLOCAL }; -extern char net_boot_file_name[128];/* Boot File name */ +extern char net_boot_file_name[1024];/* Boot File name */ /* The actual transferred size of the bootfile (in bytes) */ extern u32 net_boot_file_size; /* Boot file size in blocks as reported by the DHCP server */ diff --git a/net/net.c b/net/net.c index 6f75e3ce06..2926bceacb 100644 --- a/net/net.c +++ b/net/net.c @@ -164,7 +164,7 @@ ushort net_our_vlan = 0xFFFF; ushort net_native_vlan = 0xFFFF; /* Boot File name */ -char net_boot_file_name[128]; +char net_boot_file_name[1024]; /* The actual transferred size of the bootfile (in bytes) */ u32 net_boot_file_size; /* Boot file size in blocks as reported by the DHCP server */ -- cgit v1.2.1 From f3ba55235dddb11bbb615669b43688f4c1b73f69 Mon Sep 17 00:00:00 2001 From: Stephen Warren Date: Fri, 2 Oct 2015 17:44:34 -0600 Subject: net: rtl8169: Build warning fixes for 64-bit Casting from dev->priv to pci_dev_t changes the value's size on a 64-bit system. This causes the compiler to complain about casting a pointer to an integer of a different (smaller) size. To avoid this, cast to an integer of matching size first, then perform an int->int cast to perform the size change. This signals explicitly that we do want to change the size, and avoids the compiler warning. This is legitimate since we know the pointer actually stores a small integer, not a pointer value. Signed-off-by: Stephen Warren Acked-by: Joe Hershberger --- drivers/net/rtl8169.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/net/rtl8169.c b/drivers/net/rtl8169.c index ebd46b27e5..19422c4a2a 100644 --- a/drivers/net/rtl8169.c +++ b/drivers/net/rtl8169.c @@ -581,7 +581,8 @@ int rtl8169_eth_recv(struct udevice *dev, int flags, uchar **packetp) #else static int rtl_recv(struct eth_device *dev) { - return rtl_recv_common((pci_dev_t)dev->priv, dev->iobase, NULL); + return rtl_recv_common((pci_dev_t)(unsigned long)dev->priv, + dev->iobase, NULL); } #endif /* nCONFIG_DM_ETH */ @@ -666,8 +667,8 @@ int rtl8169_eth_send(struct udevice *dev, void *packet, int length) #else static int rtl_send(struct eth_device *dev, void *packet, int length) { - return rtl_send_common((pci_dev_t)dev->priv, dev->iobase, packet, - length); + return rtl_send_common((pci_dev_t)(unsigned long)dev->priv, + dev->iobase, packet, length); } #endif @@ -846,7 +847,8 @@ RESET - Finish setting up the ethernet interface ***************************************************************************/ static int rtl_reset(struct eth_device *dev, bd_t *bis) { - rtl8169_common_start((pci_dev_t)dev->priv, dev->enetaddr); + rtl8169_common_start((pci_dev_t)(unsigned long)dev->priv, + dev->enetaddr); return 0; } -- cgit v1.2.1 From 4f485150cfdc1cbbf841e46236dd5052fbe5c542 Mon Sep 17 00:00:00 2001 From: Sylvain Rochet Date: Wed, 7 Oct 2015 22:54:22 +0200 Subject: net: phy: micrel: disable NAND-tree for KSZ8051 NAND-tree is used to check wiring between MAC and PHY using NAND gates on the PHY side, hence the name. NAND-tree initial status is latched at reset by probing the IRQ pin. However some devices are sharing the PHY IRQ pin with other peripherals such as Atmel SAMA5D[34]x-EK boards when using the optional TM7000 display module, therefore they are switching the PHY in NAND-tree test mode depending on the current IRQ line status at reset. This patch ensure PHY is not in NAND-tree test mode only for the Micrel KSZ8051 PHY used by Atmel. There are other Micrel PHY affected but I doubt they are used on such weird hardware design. Signed-off-by: Sylvain Rochet Acked-by: Joe Hershberger --- drivers/net/phy/micrel.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c index cbec928ce4..5e49666bbb 100644 --- a/drivers/net/phy/micrel.c +++ b/drivers/net/phy/micrel.c @@ -32,6 +32,34 @@ static struct phy_driver KSZ8031_driver = { .shutdown = &genphy_shutdown, }; +/** + * KSZ8051 + */ +#define MII_KSZ8051_PHY_OMSO 0x16 +#define MII_KSZ8051_PHY_OMSO_NAND_TREE_ON (1 << 5) + +static int ksz8051_config(struct phy_device *phydev) +{ + unsigned val; + + /* Disable NAND-tree */ + val = phy_read(phydev, MDIO_DEVAD_NONE, MII_KSZ8051_PHY_OMSO); + val &= ~MII_KSZ8051_PHY_OMSO_NAND_TREE_ON; + phy_write(phydev, MDIO_DEVAD_NONE, MII_KSZ8051_PHY_OMSO, val); + + return genphy_config(phydev); +} + +static struct phy_driver KSZ8051_driver = { + .name = "Micrel KSZ8051", + .uid = 0x221550, + .mask = 0xfffff0, + .features = PHY_BASIC_FEATURES, + .config = &ksz8051_config, + .startup = &genphy_startup, + .shutdown = &genphy_shutdown, +}; + static struct phy_driver KSZ8081_driver = { .name = "Micrel KSZ8081", .uid = 0x221560, @@ -293,6 +321,7 @@ int phy_micrel_init(void) { phy_register(&KSZ804_driver); phy_register(&KSZ8031_driver); + phy_register(&KSZ8051_driver); phy_register(&KSZ8081_driver); #ifdef CONFIG_PHY_MICREL_KSZ9021 phy_register(&ksz9021_driver); -- cgit v1.2.1 From 0132b9ab6e6593d1fd259cdd26261f184c436fdd Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Wed, 7 Oct 2015 21:19:29 -0700 Subject: net: phy: Don't create phy device when there is no phy In get_phy_device_by_mask(), when no phy is found, we should not create any phy device. Signed-off-by: Bin Meng Acked-by: Joe Hershberger --- drivers/net/phy/phy.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index a6023f1033..4063894bf4 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -672,7 +672,8 @@ static struct phy_device *get_phy_device_by_mask(struct mii_dev *bus, return phydev; } printf("Phy %d not found\n", ffs(phy_mask) - 1); - return phy_device_create(bus, ffs(phy_mask) - 1, 0xffffffff, interface); + + return NULL; } /** -- cgit v1.2.1 From 3e1949d77463b062a4f8d380128abb7854f4907b Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Wed, 7 Oct 2015 21:19:30 -0700 Subject: net: phy: Change to print all phys that are not found In get_phy_device_by_mask(), when no phy is found, currently we only print a message to show the first phy address that is not found. But this is not always the case as multiple phys can be specified by phy_mask. Change to print all phys that are not found, and to reduce the console boot log, change to use 'debug' instead of 'printf'. Signed-off-by: Bin Meng Acked-by: Joe Hershberger --- drivers/net/phy/phy.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index 4063894bf4..d0b3e8523f 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -671,7 +671,14 @@ static struct phy_device *get_phy_device_by_mask(struct mii_dev *bus, if (phydev) return phydev; } - printf("Phy %d not found\n", ffs(phy_mask) - 1); + + debug("\n%s PHY: ", bus->name); + while (phy_mask) { + int addr = ffs(phy_mask) - 1; + debug("%d ", addr); + phy_mask &= ~(1 << addr); + } + debug("not found\n"); return NULL; } -- cgit v1.2.1 From 17ecfa9b45db7964f6a20cd710a87decd2f2e1f5 Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Wed, 7 Oct 2015 21:19:31 -0700 Subject: net: phy: Test previous phydev->dev against new mac dev In phy_connect_dev(), if the phy device has an accociated mac device before, a warning message will be printed. But we should test the old device against the new one, if they are actually the same one, don't print the warning message. Signed-off-by: Bin Meng Acked-by: Joe Hershberger --- drivers/net/phy/phy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index d0b3e8523f..d7364ffc34 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -789,7 +789,7 @@ void phy_connect_dev(struct phy_device *phydev, struct eth_device *dev) { /* Soft Reset the PHY */ phy_reset(phydev); - if (phydev->dev) { + if (phydev->dev && phydev->dev != dev) { printf("%s:%d is connected to %s. Reconnecting to %s\n", phydev->bus->name, phydev->addr, phydev->dev->name, dev->name); -- cgit v1.2.1 From cb6baca77bca0ef999203a7ed73bd123e7da062e Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Wed, 7 Oct 2015 21:32:37 -0700 Subject: net: mdio: Add mdio_free() and mdio_unregister() API Currently there is no API to uninitialize mdio. Add two APIs for this. Signed-off-by: Bin Meng Acked-by: Joe Hershberger --- common/miiphyutil.c | 19 +++++++++++++++++++ include/miiphy.h | 2 ++ 2 files changed, 21 insertions(+) diff --git a/common/miiphyutil.c b/common/miiphyutil.c index c88c28adbf..e499b58836 100644 --- a/common/miiphyutil.c +++ b/common/miiphyutil.c @@ -152,6 +152,11 @@ struct mii_dev *mdio_alloc(void) return bus; } +void mdio_free(struct mii_dev *bus) +{ + free(bus); +} + int mdio_register(struct mii_dev *bus) { if (!bus || !bus->name || !bus->read || !bus->write) @@ -173,6 +178,20 @@ int mdio_register(struct mii_dev *bus) return 0; } +int mdio_unregister(struct mii_dev *bus) +{ + if (!bus) + return 0; + + /* delete it from the list */ + list_del(&bus->link); + + if (current_mii == bus) + current_mii = NULL; + + return 0; +} + void mdio_list_devices(void) { struct list_head *entry; diff --git a/include/miiphy.h b/include/miiphy.h index 088797e4c6..af12274c81 100644 --- a/include/miiphy.h +++ b/include/miiphy.h @@ -59,7 +59,9 @@ struct phy_device *mdio_phydev_for_ethname(const char *devname); void miiphy_listdev(void); struct mii_dev *mdio_alloc(void); +void mdio_free(struct mii_dev *bus); int mdio_register(struct mii_dev *bus); +int mdio_unregister(struct mii_dev *bus); void mdio_list_devices(void); #ifdef CONFIG_BITBANGMII -- cgit v1.2.1 From 5d2459fd466ebf86c717ccb28a8f56707a29724a Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Wed, 7 Oct 2015 21:32:38 -0700 Subject: net: designware: Add driver remove support In designware_eth_probe(), some additional resources are allocated (eg: mdio, phy). We should free these in the driver remove phase. Add designware_eth_remove() to clean it up. Signed-off-by: Bin Meng Acked-by: Simon Glass --- drivers/net/designware.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/net/designware.c b/drivers/net/designware.c index 6433896eec..a6c39c39ff 100644 --- a/drivers/net/designware.c +++ b/drivers/net/designware.c @@ -613,6 +613,17 @@ static int designware_eth_probe(struct udevice *dev) return ret; } +static int designware_eth_remove(struct udevice *dev) +{ + struct dw_eth_dev *priv = dev_get_priv(dev); + + free(priv->phydev); + mdio_unregister(priv->bus); + mdio_free(priv->bus); + + return 0; +} + static const struct eth_ops designware_eth_ops = { .start = designware_eth_start, .send = designware_eth_send, @@ -653,6 +664,7 @@ U_BOOT_DRIVER(eth_designware) = { .ofdata_to_platdata = designware_eth_ofdata_to_platdata, .bind = designware_eth_bind, .probe = designware_eth_probe, + .remove = designware_eth_remove, .ops = &designware_eth_ops, .priv_auto_alloc_size = sizeof(struct dw_eth_dev), .platdata_auto_alloc_size = sizeof(struct eth_pdata), -- cgit v1.2.1 From 3f616b6053517927f564fc8ed6dc4a87bd39e857 Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Wed, 7 Oct 2015 21:32:39 -0700 Subject: net: pch_gbe: Add driver remove support In pch_gbe_probe(), some additional resources are allocated (eg: mdio, phy). We should free these in the driver remove phase. Add pch_gbe_remove() to clean it up. Signed-off-by: Bin Meng Reviewed-by: Simon Glass Acked-by: Joe Hershberger --- drivers/net/pch_gbe.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/net/pch_gbe.c b/drivers/net/pch_gbe.c index 004fcf88c2..dfc01000fc 100644 --- a/drivers/net/pch_gbe.c +++ b/drivers/net/pch_gbe.c @@ -452,6 +452,17 @@ int pch_gbe_probe(struct udevice *dev) return pch_gbe_phy_init(dev); } +int pch_gbe_remove(struct udevice *dev) +{ + struct pch_gbe_priv *priv = dev_get_priv(dev); + + free(priv->phydev); + mdio_unregister(priv->bus); + mdio_free(priv->bus); + + return 0; +} + static const struct eth_ops pch_gbe_ops = { .start = pch_gbe_start, .send = pch_gbe_send, @@ -470,6 +481,7 @@ U_BOOT_DRIVER(eth_pch_gbe) = { .id = UCLASS_ETH, .of_match = pch_gbe_ids, .probe = pch_gbe_probe, + .remove = pch_gbe_remove, .ops = &pch_gbe_ops, .priv_auto_alloc_size = sizeof(struct pch_gbe_priv), .platdata_auto_alloc_size = sizeof(struct eth_pdata), -- cgit v1.2.1 From 50dae85c713e11c52abf25dfd283db336257dbc5 Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Wed, 7 Oct 2015 21:32:40 -0700 Subject: dm: core: Remove unnecessary codes in uclass_pre_remove_device() dev->uclass->uc_drv->per_device_auto_alloc_size is to be freed in device_free(), so is dev->seq. Remove these unnecessary codes. Signed-off-by: Bin Meng Acked-by: Simon Glass --- drivers/core/uclass.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c index e800c28653..1af09472a2 100644 --- a/drivers/core/uclass.c +++ b/drivers/core/uclass.c @@ -523,22 +523,15 @@ int uclass_post_probe_device(struct udevice *dev) #if CONFIG_IS_ENABLED(DM_DEVICE_REMOVE) int uclass_pre_remove_device(struct udevice *dev) { - struct uclass_driver *uc_drv; struct uclass *uc; int ret; uc = dev->uclass; - uc_drv = uc->uc_drv; if (uc->uc_drv->pre_remove) { ret = uc->uc_drv->pre_remove(dev); if (ret) return ret; } - if (uc_drv->per_device_auto_alloc_size) { - free(dev->uclass_priv); - dev->uclass_priv = NULL; - } - dev->seq = -1; return 0; } -- cgit v1.2.1 From a16edabe7f744b2d7a97bba4d4bc95b6894fd592 Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Wed, 7 Oct 2015 21:45:42 -0700 Subject: net: eth: Clear MAC address in eth_pre_remove() platdata->enetaddr was assigned to a value in dev_probe() last time. If we don't clear it, for dev_probe() at the second time, dm eth will end up treating it as a MAC address from ROM no matter where it came from originally (maybe env, ROM, or even random). Fix this by clearing platdata->enetaddr when removing an Ethernet device. Signed-off-by: Bin Meng Acked-by: Joe Hershberger --- net/eth.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/net/eth.c b/net/eth.c index b978aae101..c661775d1c 100644 --- a/net/eth.c +++ b/net/eth.c @@ -575,8 +575,13 @@ static int eth_post_probe(struct udevice *dev) static int eth_pre_remove(struct udevice *dev) { + struct eth_pdata *pdata = dev->platdata; + eth_get_ops(dev)->stop(dev); + /* clear the MAC address */ + memset(pdata->enetaddr, 0, 6); + return 0; } -- cgit v1.2.1 From 6d9764c2a87cc5a1ce4f3919add6360ec36f81c7 Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Wed, 7 Oct 2015 21:45:43 -0700 Subject: dm: test: Add a new test case against dm eth codes for NULL pointer access U-Boot crashes when doing a 'ping' with the following test scenario: - All ethernet devices are not probed - "ethaddr" for all ethernet devices are not set - "ethact" is set to a valid ethernet device name Add a new test case 'dm_test_eth_act' to hit such scenario. Signed-off-by: Bin Meng Acked-by: Joe Hershberger --- test/dm/eth.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/test/dm/eth.c b/test/dm/eth.c index fcfb3e1ece..6288ae24ab 100644 --- a/test/dm/eth.c +++ b/test/dm/eth.c @@ -13,11 +13,15 @@ #include #include #include +#include +#include #include #include DECLARE_GLOBAL_DATA_PTR; +#define DM_TEST_ETH_NUM 4 + static int dm_test_eth(struct unit_test_state *uts) { net_ping_ip = string_to_ip("1.1.2.2"); @@ -82,6 +86,66 @@ static int dm_test_eth_prime(struct unit_test_state *uts) } DM_TEST(dm_test_eth_prime, DM_TESTF_SCAN_FDT); +/** + * This test case is trying to test the following scenario: + * - All ethernet devices are not probed + * - "ethaddr" for all ethernet devices are not set + * - "ethact" is set to a valid ethernet device name + * + * With Sandbox default test configuration, all ethernet devices are + * probed after power-up, so we have to manually create such scenario: + * - Remove all ethernet devices + * - Remove all "ethaddr" environment variables + * - Set "ethact" to the first ethernet device + * + * Do a ping test to see if anything goes wrong. + */ +static int dm_test_eth_act(struct unit_test_state *uts) +{ + struct udevice *dev[DM_TEST_ETH_NUM]; + const char *ethname[DM_TEST_ETH_NUM] = {"eth@10002000", "eth@10003000", + "sbe5", "eth@10004000"}; + const char *addrname[DM_TEST_ETH_NUM] = {"ethaddr", "eth5addr", + "eth3addr", "eth1addr"}; + char ethaddr[DM_TEST_ETH_NUM][18]; + int i; + + net_ping_ip = string_to_ip("1.1.2.2"); + + /* Prepare the test scenario */ + for (i = 0; i < DM_TEST_ETH_NUM; i++) { + ut_assertok(uclass_find_device_by_name(UCLASS_ETH, + ethname[i], &dev[i])); + ut_assertok(device_remove(dev[i])); + + /* Invalidate MAC address */ + strcpy(ethaddr[i], getenv(addrname[i])); + /* Must disable access protection for ethaddr before clearing */ + setenv(".flags", addrname[i]); + setenv(addrname[i], NULL); + } + + /* Set ethact to "eth@10002000" */ + setenv("ethact", ethname[0]); + + /* Segment fault might happen if something is wrong */ + ut_asserteq(-ENODEV, net_loop(PING)); + + for (i = 0; i < DM_TEST_ETH_NUM; i++) { + /* Restore the env */ + setenv(".flags", addrname[i]); + setenv(addrname[i], ethaddr[i]); + + /* Probe the device again */ + ut_assertok(device_probe(dev[i])); + } + setenv(".flags", NULL); + setenv("ethact", NULL); + + return 0; +} +DM_TEST(dm_test_eth_act, DM_TESTF_SCAN_FDT); + /* The asserts include a return on fail; cleanup in the caller */ static int _dm_test_eth_rotate1(struct unit_test_state *uts) { -- cgit v1.2.1 From ac1d31380618f3f68bf7f05b73b6ab0cdeab0e9f Mon Sep 17 00:00:00 2001 From: Bin Meng Date: Wed, 7 Oct 2015 21:45:44 -0700 Subject: net: eth: Check return value in various places eth_get_dev() can return NULL which means device_probe() fails for that ethernet device. Add return value check in various places or U-Boot will crash due to NULL pointer access. With this commit, 'dm_test_eth_act' test case passes. Signed-off-by: Bin Meng Acked-by: Joe Hershberger --- net/eth.c | 43 +++++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/net/eth.c b/net/eth.c index c661775d1c..c542f4aa3b 100644 --- a/net/eth.c +++ b/net/eth.c @@ -179,8 +179,12 @@ struct udevice *eth_get_dev(void) */ static void eth_set_dev(struct udevice *dev) { - if (dev && !device_active(dev)) + if (dev && !device_active(dev)) { eth_errno = device_probe(dev); + if (eth_errno) + dev = NULL; + } + eth_get_uclass_priv()->current = dev; } @@ -213,10 +217,9 @@ struct udevice *eth_get_dev_by_name(const char *devname) * match an alias or it will match a literal name and we'll pick * up the error when we try to probe again in eth_set_dev(). */ - device_probe(it); - /* - * Check for the name or the sequence number to match - */ + if (device_probe(it)) + continue; + /* Check for the name or the sequence number to match */ if (strcmp(it->name, devname) == 0 || (endp > startp && it->seq == seq)) return it; @@ -346,23 +349,27 @@ int eth_init(void) old_current = current; do { - debug("Trying %s\n", current->name); - - if (device_active(current)) { - ret = eth_get_ops(current)->start(current); - if (ret >= 0) { - struct eth_device_priv *priv = - current->uclass_priv; - - priv->state = ETH_STATE_ACTIVE; - return 0; + if (current) { + debug("Trying %s\n", current->name); + + if (device_active(current)) { + ret = eth_get_ops(current)->start(current); + if (ret >= 0) { + struct eth_device_priv *priv = + current->uclass_priv; + + priv->state = ETH_STATE_ACTIVE; + return 0; + } + } else { + ret = eth_errno; } + + debug("FAIL\n"); } else { - ret = eth_errno; + debug("PROBE FAIL\n"); } - debug("FAIL\n"); - /* * If ethrotate is enabled, this will change "current", * otherwise we will drop out of this while loop immediately -- cgit v1.2.1