[U-Boot] [PATCH 1/9] net: Revert "tftp: adjust settings to be suitable for 100Mbit ethernet"
Testing either pch_gbe or e1000 driver via tftp command on Intel Crown Bay board, shows the following failure.
TFTP error: 'Unsupported option(s) requested' (8)
It turns out commit 620776d causes this. Revert this commit for now.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
net/tftp.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/tftp.c b/net/tftp.c index 18ce84c..89be32a 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -19,10 +19,10 @@ /* Well known TFTP port # */ #define WELL_KNOWN_PORT 69 /* Millisecs to timeout for lost pkt */ -#define TIMEOUT 100UL +#define TIMEOUT 5000UL #ifndef CONFIG_NET_RETRY_COUNT /* # of timeouts before giving up */ -# define TIMEOUT_COUNT 1000 +# define TIMEOUT_COUNT 10 #else # define TIMEOUT_COUNT (CONFIG_NET_RETRY_COUNT * 2) #endif @@ -711,10 +711,10 @@ void tftp_start(enum proto_t protocol) if (ep != NULL) timeout_ms = simple_strtol(ep, NULL, 10);
- if (timeout_ms < 10) { - printf("TFTP timeout (%ld ms) too low, set min = 10 ms\n", + if (timeout_ms < 1000) { + printf("TFTP timeout (%ld ms) too low, set min = 1000 ms\n", timeout_ms); - timeout_ms = 10; + timeout_ms = 1000; }
debug("TFTP blocksize = %i, timeout = %ld ms\n",
commit 6497e37 "net: e1000: Support 64-bit physical address" causes compiler warnings on 32-bit U-Boot build below.
drivers/net/e1000.c: In function 'e1000_configure_tx': drivers/net/e1000.c:4982:2: warning: right shift count >= width of type [enabled by default] drivers/net/e1000.c: In function 'e1000_configure_rx': drivers/net/e1000.c:5126:2: warning: right shift count >= width of type [enabled by default]
This commit fixes the build warnings.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
drivers/net/e1000.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/e1000.c b/drivers/net/e1000.c index 6f74d30..a467280 100644 --- a/drivers/net/e1000.c +++ b/drivers/net/e1000.c @@ -4977,9 +4977,10 @@ e1000_configure_tx(struct e1000_hw *hw) unsigned long tctl; unsigned long tipg, tarc; uint32_t ipgr1, ipgr2; + uint64_t tdba = (unsigned long)tx_base;
- E1000_WRITE_REG(hw, TDBAL, (unsigned long)tx_base & 0xffffffff); - E1000_WRITE_REG(hw, TDBAH, (unsigned long)tx_base >> 32); + E1000_WRITE_REG(hw, TDBAL, (uint32_t)(tdba & 0xffffffff)); + E1000_WRITE_REG(hw, TDBAH, (uint32_t)(tdba >> 32));
E1000_WRITE_REG(hw, TDLEN, 128);
@@ -5103,6 +5104,8 @@ e1000_configure_rx(struct e1000_hw *hw) { unsigned long rctl, ctrl_ext; rx_tail = 0; + uint64_t rdba = (unsigned long)rx_base; + /* make sure receives are disabled while setting up the descriptors */ rctl = E1000_READ_REG(hw, RCTL); E1000_WRITE_REG(hw, RCTL, rctl & ~E1000_RCTL_EN); @@ -5122,8 +5125,8 @@ e1000_configure_rx(struct e1000_hw *hw) E1000_WRITE_FLUSH(hw); } /* Setup the Base and Length of the Rx Descriptor Ring */ - E1000_WRITE_REG(hw, RDBAL, (unsigned long)rx_base & 0xffffffff); - E1000_WRITE_REG(hw, RDBAH, (unsigned long)rx_base >> 32); + E1000_WRITE_REG(hw, RDBAL, (uint32_t)(rdba & 0xffffffff)); + E1000_WRITE_REG(hw, RDBAH, (uint32_t)(rdba >> 32));
E1000_WRITE_REG(hw, RDLEN, 128);
On Tue, 2015-08-25 at 00:22 -0700, Bin Meng wrote:
commit 6497e37 "net: e1000: Support 64-bit physical address" causes compiler warnings on 32-bit U-Boot build below.
drivers/net/e1000.c: In function 'e1000_configure_tx': drivers/net/e1000.c:4982:2: warning: right shift count >= width of type [enabled by default] drivers/net/e1000.c: In function 'e1000_configure_rx': drivers/net/e1000.c:5126:2: warning: right shift count >= width of type [enabled by default]
This commit fixes the build warnings.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/net/e1000.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/e1000.c b/drivers/net/e1000.c index 6f74d30..a467280 100644 --- a/drivers/net/e1000.c +++ b/drivers/net/e1000.c @@ -4977,9 +4977,10 @@ e1000_configure_tx(struct e1000_hw *hw) unsigned long tctl; unsigned long tipg, tarc; uint32_t ipgr1, ipgr2;
uint64_t tdba = (unsigned long)tx_base;
E1000_WRITE_REG(hw, TDBAL, (unsigned long)tx_base & 0xffffffff);E1000_WRITE_REG(hw, TDBAH, (unsigned long)tx_base >> 32);
E1000_WRITE_REG(hw, TDBAL, (uint32_t)(tdba & 0xffffffff));E1000_WRITE_REG(hw, TDBAH, (uint32_t)(tdba >> 32));
You could use upper_32_bits()/lower_32_bits().
-Scott
With driver model, board_eth_init() or cpu_eth_init() is not needed. Remove the call to these in eth_common_init().
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
net/eth.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/net/eth.c b/net/eth.c index d3ec8d6..0b4b08a 100644 --- a/net/eth.c +++ b/net/eth.c @@ -96,6 +96,7 @@ static void eth_common_init(void) phy_init(); #endif
+#ifndef CONFIG_DM_ETH /* * If board-specific initialization exists, call it. * If not, call a CPU-specific one @@ -109,6 +110,7 @@ static void eth_common_init(void) } else { printf("Net Initialization Skipped\n"); } +#endif }
#ifdef CONFIG_DM_ETH
Hi Bin,
On Tue, Aug 25, 2015 at 2:22 AM, Bin Meng bmeng.cn@gmail.com wrote:
With driver model, board_eth_init() or cpu_eth_init() is not needed. Remove the call to these in eth_common_init().
I'm pretty sure Simon needed this when he ported some allwinner board originally.
3bc427006ac8d0661169ed771b3cac7e86f960e8 (dm: net: Use existing Ethernet init for driver model)
Cheers, -Joe
Hi Joe,
On Wed, Aug 26, 2015 at 2:43 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Tue, Aug 25, 2015 at 2:22 AM, Bin Meng bmeng.cn@gmail.com wrote:
With driver model, board_eth_init() or cpu_eth_init() is not needed. Remove the call to these in eth_common_init().
I'm pretty sure Simon needed this when he ported some allwinner board originally.
3bc427006ac8d0661169ed771b3cac7e86f960e8 (dm: net: Use existing Ethernet init for driver model)
I think my patch does not break Simon's. My patch only comments out the call to board_eth_init() or cpu_eth_init() which is not needed for driver model. Other stuff in eth_common_init() is still there. In fact, my patch series also needs phy_init() call (required by pch_gbe driver).
Regards, Bin
Hi Simon,
On Wed, Aug 26, 2015 at 9:29 AM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Joe,
On Wed, Aug 26, 2015 at 2:43 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Tue, Aug 25, 2015 at 2:22 AM, Bin Meng bmeng.cn@gmail.com wrote:
With driver model, board_eth_init() or cpu_eth_init() is not needed. Remove the call to these in eth_common_init().
I'm pretty sure Simon needed this when he ported some allwinner board originally.
3bc427006ac8d0661169ed771b3cac7e86f960e8 (dm: net: Use existing Ethernet init for driver model)
I think my patch does not break Simon's. My patch only comments out the call to board_eth_init() or cpu_eth_init() which is not needed for driver model. Other stuff in eth_common_init() is still there. In fact, my patch series also needs phy_init() call (required by pch_gbe driver).
Would you confirm this does not break your boards?
Regards, Bin
Hi Bin,
On Tue, Aug 25, 2015 at 8:29 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Joe,
On Wed, Aug 26, 2015 at 2:43 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Tue, Aug 25, 2015 at 2:22 AM, Bin Meng bmeng.cn@gmail.com wrote:
With driver model, board_eth_init() or cpu_eth_init() is not needed. Remove the call to these in eth_common_init().
I'm pretty sure Simon needed this when he ported some allwinner board originally.
3bc427006ac8d0661169ed771b3cac7e86f960e8 (dm: net: Use existing Ethernet init for driver model)
I think my patch does not break Simon's. My patch only comments out the call to board_eth_init() or cpu_eth_init() which is not needed for driver model. Other stuff in eth_common_init() is still there. In fact, my patch series also needs phy_init() call (required by pch_gbe driver).
Even if it doesn't break Simon's board, why remove the ability for a board to add eth_init code. You're trying to say that there is no case where a DM board would need to init anything related to eth. That seems unlikely.
Also, why is it hurting your board to have an optional call to such a function. Presumably you just don't define those functions and you're fine, right?
I guess it can just be put back when such a board is converted.
-Joe
Hi Joe,
On Wed, Aug 26, 2015 at 10:24 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Tue, Aug 25, 2015 at 8:29 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Joe,
On Wed, Aug 26, 2015 at 2:43 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Tue, Aug 25, 2015 at 2:22 AM, Bin Meng bmeng.cn@gmail.com wrote:
With driver model, board_eth_init() or cpu_eth_init() is not needed. Remove the call to these in eth_common_init().
I'm pretty sure Simon needed this when he ported some allwinner board originally.
3bc427006ac8d0661169ed771b3cac7e86f960e8 (dm: net: Use existing Ethernet init for driver model)
I think my patch does not break Simon's. My patch only comments out the call to board_eth_init() or cpu_eth_init() which is not needed for driver model. Other stuff in eth_common_init() is still there. In fact, my patch series also needs phy_init() call (required by pch_gbe driver).
Even if it doesn't break Simon's board, why remove the ability for a board to add eth_init code. You're trying to say that there is no case where a DM board would need to init anything related to eth. That seems unlikely.
I believe with driver model, we should avoid these calls. All the drive initialization needs to be done in the bind or probe phase, and called by driver model automatically. Like pci_eth_init() which just calls non-dm ethernet drivers, and pci_eth_init() can be called from board_eth_init() or cpu_eth_init(). But I think you are right, there might be some board-specific thing to be put there, even right now it does not break any board. But that's maybe we don't have proper driver model uclass to handle these misc things?
Also, why is it hurting your board to have an optional call to such a function. Presumably you just don't define those functions and you're fine, right?
It does not hurt but I think at least we should comment out the following printf for DM.
} else { printf("Net Initialization Skipped\n"); }
This is misleading message.
I guess it can just be put back when such a board is converted.
Regards, Bin
Hi Bin,
On Tue, Aug 25, 2015 at 9:36 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Joe,
On Wed, Aug 26, 2015 at 10:24 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Tue, Aug 25, 2015 at 8:29 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Joe,
On Wed, Aug 26, 2015 at 2:43 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Tue, Aug 25, 2015 at 2:22 AM, Bin Meng bmeng.cn@gmail.com wrote:
With driver model, board_eth_init() or cpu_eth_init() is not needed. Remove the call to these in eth_common_init().
I'm pretty sure Simon needed this when he ported some allwinner board originally.
3bc427006ac8d0661169ed771b3cac7e86f960e8 (dm: net: Use existing Ethernet init for driver model)
I think my patch does not break Simon's. My patch only comments out the call to board_eth_init() or cpu_eth_init() which is not needed for driver model. Other stuff in eth_common_init() is still there. In fact, my patch series also needs phy_init() call (required by pch_gbe driver).
Even if it doesn't break Simon's board, why remove the ability for a board to add eth_init code. You're trying to say that there is no case where a DM board would need to init anything related to eth. That seems unlikely.
I believe with driver model, we should avoid these calls. All the drive initialization needs to be done in the bind or probe phase, and called by driver model automatically. Like pci_eth_init() which just calls non-dm ethernet drivers, and pci_eth_init() can be called from board_eth_init() or cpu_eth_init().
I agree we shouldn't use them if it makes sense not to.
But I think you are right, there might be some board-specific thing to be put there, even right now it does not break any board. But that's maybe we don't have proper driver model uclass to handle these misc things?
I think we can evaluate that for each case. We don't necessarily want the uclass to be a union of all crazy board features.
Also, why is it hurting your board to have an optional call to such a function. Presumably you just don't define those functions and you're fine, right?
It does not hurt but I think at least we should comment out the following printf for DM.
} else { printf("Net Initialization Skipped\n"); }This is misleading message.
I agree.
I guess it can just be put back when such a board is converted.
Regards, Bin
Hi,
On 25 August 2015 at 19:36, Bin Meng bmeng.cn@gmail.com wrote:
Hi Joe,
On Wed, Aug 26, 2015 at 10:24 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Tue, Aug 25, 2015 at 8:29 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Joe,
On Wed, Aug 26, 2015 at 2:43 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Tue, Aug 25, 2015 at 2:22 AM, Bin Meng bmeng.cn@gmail.com wrote:
With driver model, board_eth_init() or cpu_eth_init() is not needed. Remove the call to these in eth_common_init().
I'm pretty sure Simon needed this when he ported some allwinner board originally.
3bc427006ac8d0661169ed771b3cac7e86f960e8 (dm: net: Use existing Ethernet init for driver model)
I think my patch does not break Simon's. My patch only comments out the call to board_eth_init() or cpu_eth_init() which is not needed for driver model. Other stuff in eth_common_init() is still there. In fact, my patch series also needs phy_init() call (required by pch_gbe driver).
Even if it doesn't break Simon's board, why remove the ability for a board to add eth_init code. You're trying to say that there is no case where a DM board would need to init anything related to eth. That seems unlikely.
I believe with driver model, we should avoid these calls. All the drive initialization needs to be done in the bind or probe phase, and called by driver model automatically. Like pci_eth_init() which just calls non-dm ethernet drivers, and pci_eth_init() can be called from board_eth_init() or cpu_eth_init(). But I think you are right, there might be some board-specific thing to be put there, even right now it does not break any board. But that's maybe we don't have proper driver model uclass to handle these misc things?
Normally the board init should happen in board_init(). We don't currently have a good way to handle 'associated init' for drivers.
For example, if an Ethernet port needs some pinmux settings, or some clock settings, with driver model we currently have to init this in board_init() so it is not 'lazy init', it always happens at start of day.
I don't actually think this is a problem in practice, but it is not in the spirit of driver model. Drivers should 'pull in' the init that they need and we should make that work automatically.
But it is very important the we do not put this init into the drivers themselves. The drivers need to be generic. E.g. an Ethernet driver should be able to operate on any board that has that Ethernet controller. The setup to make the controller available on pins, enable it clock it correctly should not be in driver code.
Soon we will have pinctrl which will deal with the pinmux problem. We already have a clock framework would could handle clocking. So progress is being made.
I wonder whether we will end up wanting a way to request that the resources for a driver be activated, in a general sense. But until we have a use case I'm not keen to spend much time thinking about it.
Also, why is it hurting your board to have an optional call to such a function. Presumably you just don't define those functions and you're fine, right?
It does not hurt but I think at least we should comment out the following printf for DM.
} else { printf("Net Initialization Skipped\n"); }This is misleading message.
I guess it can just be put back when such a board is converted.
Regards, Bin
Regards, Simon
When given a device name string, we should test if it contains "eth" before we treat it as an alias.
With this commit, now we are really able to rotate between network interfaces with driver model (previously it was broken).
Signed-off-by: Bin Meng bmeng.cn@gmail.com
---
net/eth.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/eth.c b/net/eth.c index 0b4b08a..fbf30b0 100644 --- a/net/eth.c +++ b/net/eth.c @@ -197,7 +197,7 @@ struct udevice *eth_get_dev_by_name(const char *devname) struct uclass *uc;
/* Must be longer than 3 to be an alias */ - if (strlen(devname) > strlen("eth")) { + if (strstr(devname, "eth") && strlen(devname) > strlen("eth")) { startp = devname + strlen("eth"); seq = simple_strtoul(startp, &endp, 10); }
Hi Bin,
On Tue, Aug 25, 2015 at 2:22 AM, Bin Meng bmeng.cn@gmail.com wrote:
When given a device name string, we should test if it contains "eth" before we treat it as an alias.
With this commit, now we are really able to rotate between network interfaces with driver model (previously it was broken).
I believe there is a test for this (dm_test_eth_rotate). Apparently it is not a sufficient test. Please describe the way in which this behavior is broken and add a test case to test/dm/eth.c that would fail without your change.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
net/eth.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/eth.c b/net/eth.c index 0b4b08a..fbf30b0 100644 --- a/net/eth.c +++ b/net/eth.c @@ -197,7 +197,7 @@ struct udevice *eth_get_dev_by_name(const char *devname) struct uclass *uc;
/* Must be longer than 3 to be an alias */
if (strlen(devname) > strlen("eth")) {
if (strstr(devname, "eth") && strlen(devname) > strlen("eth")) { startp = devname + strlen("eth"); seq = simple_strtoul(startp, &endp, 10); }-- 1.8.2.1
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Move to driver model for USB on Intel Crown Bay.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
configs/crownbay_defconfig | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/configs/crownbay_defconfig b/configs/crownbay_defconfig index 4fc1827..6edd710 100644 --- a/configs/crownbay_defconfig +++ b/configs/crownbay_defconfig @@ -23,6 +23,8 @@ CONFIG_NETDEVICES=y CONFIG_E1000=y CONFIG_VIDEO_VESA=y CONFIG_FRAMEBUFFER_SET_VESA_MODE=y +CONFIG_USB=y +CONFIG_DM_USB=y CONFIG_DM_RTC=y CONFIG_USE_PRIVATE_LIBGCC=y CONFIG_SYS_VSNPRINTF=y
Hi Bin,
On Tue, Aug 25, 2015 at 2:22 AM, Bin Meng bmeng.cn@gmail.com wrote:
Move to driver model for USB on Intel Crown Bay.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Reviewed-by: Joe Hershberger joe.hershberger@ni.com
On 25 August 2015 at 01:22, Bin Meng bmeng.cn@gmail.com wrote:
Move to driver model for USB on Intel Crown Bay.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
configs/crownbay_defconfig | 2 ++ 1 file changed, 2 insertions(+)
Acked-by: Simon Glass sjg@chromium.org
Since E1000 driver has been converted to driver model, enable it on Intel Crown Bay. But the Intel Topcliff GbE driver has not been converted to driver model yet, disable it for now.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
board/intel/crownbay/crownbay.c | 6 ------ configs/crownbay_defconfig | 2 +- include/configs/crownbay.h | 1 - 3 files changed, 1 insertion(+), 8 deletions(-)
diff --git a/board/intel/crownbay/crownbay.c b/board/intel/crownbay/crownbay.c index d6de9fa..3a79e69 100644 --- a/board/intel/crownbay/crownbay.c +++ b/board/intel/crownbay/crownbay.c @@ -7,7 +7,6 @@ #include <common.h> #include <asm/ibmpc.h> #include <asm/pnp_def.h> -#include <netdev.h> #include <smsc_lpc47m.h>
int board_early_init_f(void) @@ -24,8 +23,3 @@ void setup_pch_gpios(u16 gpiobase, const struct pch_gpio_map *gpio) { return; } - -int board_eth_init(bd_t *bis) -{ - return pci_eth_init(bis); -} diff --git a/configs/crownbay_defconfig b/configs/crownbay_defconfig index 6edd710..f027faf 100644 --- a/configs/crownbay_defconfig +++ b/configs/crownbay_defconfig @@ -19,7 +19,7 @@ CONFIG_OF_CONTROL=y CONFIG_CPU=y CONFIG_DM_PCI=y CONFIG_SPI_FLASH=y -CONFIG_NETDEVICES=y +CONFIG_DM_ETH=y CONFIG_E1000=y CONFIG_VIDEO_VESA=y CONFIG_FRAMEBUFFER_SET_VESA_MODE=y diff --git a/include/configs/crownbay.h b/include/configs/crownbay.h index 998da78..a344c85 100644 --- a/include/configs/crownbay.h +++ b/include/configs/crownbay.h @@ -50,7 +50,6 @@ #define CONFIG_CMD_MMC
/* Topcliff Gigabit Ethernet */ -#define CONFIG_PCH_GBE #define CONFIG_PHYLIB
/* Environment configuration */
Hi Bin,
On Tue, Aug 25, 2015 at 2:22 AM, Bin Meng bmeng.cn@gmail.com wrote:
Since E1000 driver has been converted to driver model, enable it on Intel Crown Bay. But the Intel Topcliff GbE driver has not been converted to driver model yet, disable it for now.
If you reorder your series a bit you can squash this into the last patch, right? That would be more clear. You can simply move patch 7 & 8 to before this one and you can squash 9 into 6.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Hi Joe,
On Wed, Aug 26, 2015 at 2:59 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Tue, Aug 25, 2015 at 2:22 AM, Bin Meng bmeng.cn@gmail.com wrote:
Since E1000 driver has been converted to driver model, enable it on Intel Crown Bay. But the Intel Topcliff GbE driver has not been converted to driver model yet, disable it for now.
If you reorder your series a bit you can squash this into the last patch, right? That would be more clear. You can simply move patch 7 & 8 to before this one and you can squash 9 into 6.
I cannot squash this into the last patch because when patch 7 comes, it breaks Intel Crown Bay board build as non-dm version is gone. That's why you see in this patch when I turned on CONFIG_DM_ETH, I also disabled CONFIG_PCH_GBE (non-dm driver). Then after patch 7 converts the pch_gbe driver to dm, I added that driver back in patch 9. This way it passes buildman testing without breaking bisectability.
But if you think we can break such kind of bisectability, I can reorder these patches.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Regards, Bin
Hi,
On 25 August 2015 at 19:20, Bin Meng bmeng.cn@gmail.com wrote:
Hi Joe,
On Wed, Aug 26, 2015 at 2:59 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Tue, Aug 25, 2015 at 2:22 AM, Bin Meng bmeng.cn@gmail.com wrote:
Since E1000 driver has been converted to driver model, enable it on Intel Crown Bay. But the Intel Topcliff GbE driver has not been converted to driver model yet, disable it for now.
If you reorder your series a bit you can squash this into the last patch, right? That would be more clear. You can simply move patch 7 & 8 to before this one and you can squash 9 into 6.
I cannot squash this into the last patch because when patch 7 comes, it breaks Intel Crown Bay board build as non-dm version is gone. That's why you see in this patch when I turned on CONFIG_DM_ETH, I also disabled CONFIG_PCH_GBE (non-dm driver). Then after patch 7 converts the pch_gbe driver to dm, I added that driver back in patch 9. This way it passes buildman testing without breaking bisectability.
But if you think we can break such kind of bisectability, I can reorder these patches.
No, we should keep bisectability. It's just such a pain chasing down regressions otherwise.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Regards, Bin
Regards, Simon
On Tue, Aug 25, 2015 at 9:28 PM, Simon Glass sjg@chromium.org wrote:
Hi,
On 25 August 2015 at 19:20, Bin Meng bmeng.cn@gmail.com wrote:
Hi Joe,
On Wed, Aug 26, 2015 at 2:59 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Tue, Aug 25, 2015 at 2:22 AM, Bin Meng bmeng.cn@gmail.com wrote:
Since E1000 driver has been converted to driver model, enable it on Intel Crown Bay. But the Intel Topcliff GbE driver has not been converted to driver model yet, disable it for now.
If you reorder your series a bit you can squash this into the last patch, right? That would be more clear. You can simply move patch 7 & 8 to before this one and you can squash 9 into 6.
I cannot squash this into the last patch because when patch 7 comes, it breaks Intel Crown Bay board build as non-dm version is gone. That's why you see in this patch when I turned on CONFIG_DM_ETH, I also disabled CONFIG_PCH_GBE (non-dm driver). Then after patch 7 converts the pch_gbe driver to dm, I added that driver back in patch 9. This way it passes buildman testing without breaking bisectability.
But if you think we can break such kind of bisectability, I can reorder these patches.
No, we should keep bisectability. It's just such a pain chasing down regressions otherwise.
Yep
This commit converts pch_gbe ethernet driver to driver model.
Since this driver is only used by Intel Crown Bay board, the conversion does not keep the non-dm version.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
drivers/net/pch_gbe.c | 133 +++++++++++++++++++++++++++----------------------- drivers/net/pch_gbe.h | 2 - include/netdev.h | 4 -- 3 files changed, 73 insertions(+), 66 deletions(-)
diff --git a/drivers/net/pch_gbe.c b/drivers/net/pch_gbe.c index a03bdc0..004fcf8 100644 --- a/drivers/net/pch_gbe.c +++ b/drivers/net/pch_gbe.c @@ -7,10 +7,10 @@ */
#include <common.h> +#include <dm.h> #include <errno.h> #include <asm/io.h> #include <pci.h> -#include <malloc.h> #include <miiphy.h> #include "pch_gbe.h"
@@ -19,7 +19,7 @@ #endif
static struct pci_device_id supported[] = { - { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_TCF_GBE }, + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_TCF_GBE) }, { } };
@@ -62,9 +62,10 @@ static int pch_gbe_mac_write(struct pch_gbe_regs *mac_regs, u8 *addr) return -ETIME; }
-static int pch_gbe_reset(struct eth_device *dev) +static int pch_gbe_reset(struct udevice *dev) { - struct pch_gbe_priv *priv = dev->priv; + struct pch_gbe_priv *priv = dev_get_priv(dev); + struct eth_pdata *plat = dev_get_platdata(dev); struct pch_gbe_regs *mac_regs = priv->mac_regs; ulong start;
@@ -97,7 +98,7 @@ static int pch_gbe_reset(struct eth_device *dev) * so we have to reload MAC address here in order to * make linux pch_gbe driver happy. */ - return pch_gbe_mac_write(mac_regs, dev->enetaddr); + return pch_gbe_mac_write(mac_regs, plat->enetaddr); }
udelay(10); @@ -107,9 +108,9 @@ static int pch_gbe_reset(struct eth_device *dev) return -ETIME; }
-static void pch_gbe_rx_descs_init(struct eth_device *dev) +static void pch_gbe_rx_descs_init(struct udevice *dev) { - struct pch_gbe_priv *priv = dev->priv; + struct pch_gbe_priv *priv = dev_get_priv(dev); struct pch_gbe_regs *mac_regs = priv->mac_regs; struct pch_gbe_rx_desc *rx_desc = &priv->rx_desc[0]; int i; @@ -128,9 +129,9 @@ static void pch_gbe_rx_descs_init(struct eth_device *dev) &mac_regs->rx_dsc_sw_p); }
-static void pch_gbe_tx_descs_init(struct eth_device *dev) +static void pch_gbe_tx_descs_init(struct udevice *dev) { - struct pch_gbe_priv *priv = dev->priv; + struct pch_gbe_priv *priv = dev_get_priv(dev); struct pch_gbe_regs *mac_regs = priv->mac_regs; struct pch_gbe_tx_desc *tx_desc = &priv->tx_desc[0];
@@ -183,9 +184,9 @@ static void pch_gbe_adjust_link(struct pch_gbe_regs *mac_regs, return; }
-static int pch_gbe_init(struct eth_device *dev, bd_t *bis) +static int pch_gbe_start(struct udevice *dev) { - struct pch_gbe_priv *priv = dev->priv; + struct pch_gbe_priv *priv = dev_get_priv(dev); struct pch_gbe_regs *mac_regs = priv->mac_regs;
if (pch_gbe_reset(dev)) @@ -226,18 +227,18 @@ static int pch_gbe_init(struct eth_device *dev, bd_t *bis) return 0; }
-static void pch_gbe_halt(struct eth_device *dev) +static void pch_gbe_stop(struct udevice *dev) { - struct pch_gbe_priv *priv = dev->priv; + struct pch_gbe_priv *priv = dev_get_priv(dev);
pch_gbe_reset(dev);
phy_shutdown(priv->phydev); }
-static int pch_gbe_send(struct eth_device *dev, void *packet, int length) +static int pch_gbe_send(struct udevice *dev, void *packet, int length) { - struct pch_gbe_priv *priv = dev->priv; + struct pch_gbe_priv *priv = dev_get_priv(dev); struct pch_gbe_regs *mac_regs = priv->mac_regs; struct pch_gbe_tx_desc *tx_head, *tx_desc; u16 frame_ctrl = 0; @@ -277,15 +278,13 @@ static int pch_gbe_send(struct eth_device *dev, void *packet, int length) return -ETIME; }
-static int pch_gbe_recv(struct eth_device *dev) +static int pch_gbe_recv(struct udevice *dev, int flags, uchar **packetp) { - struct pch_gbe_priv *priv = dev->priv; + struct pch_gbe_priv *priv = dev_get_priv(dev); struct pch_gbe_regs *mac_regs = priv->mac_regs; - struct pch_gbe_rx_desc *rx_head, *rx_desc; + struct pch_gbe_rx_desc *rx_desc; u32 hw_desc, buffer_addr, length; - int rx_swp;
- rx_head = &priv->rx_desc[0]; rx_desc = &priv->rx_desc[priv->rx_idx];
readl(&mac_regs->int_st); @@ -293,11 +292,21 @@ static int pch_gbe_recv(struct eth_device *dev)
/* Just return if not receiving any packet */ if ((u32)rx_desc == hw_desc) - return 0; + return -EAGAIN;
buffer_addr = pci_mem_to_phys(priv->bdf, rx_desc->buffer_addr); + *packetp = (uchar *)buffer_addr; length = rx_desc->rx_words_eob - 3 - ETH_FCS_LEN; - net_process_received_packet((uchar *)buffer_addr, length); + + return length; +} + +static int pch_gbe_free_pkt(struct udevice *dev, uchar *packet, int length) +{ + struct pch_gbe_priv *priv = dev_get_priv(dev); + struct pch_gbe_regs *mac_regs = priv->mac_regs; + struct pch_gbe_rx_desc *rx_head = &priv->rx_desc[0]; + int rx_swp;
/* Test the wrap-around condition */ if (++priv->rx_idx >= PCH_GBE_DESC_NUM) @@ -309,7 +318,7 @@ static int pch_gbe_recv(struct eth_device *dev) writel(pci_phys_to_mem(priv->bdf, (u32)(rx_head + rx_swp)), &mac_regs->rx_dsc_sw_p);
- return length; + return 0; }
static int pch_gbe_mdio_ready(struct pch_gbe_regs *mac_regs) @@ -365,7 +374,7 @@ static int pch_gbe_mdio_write(struct mii_dev *bus, int addr, int devad, return 0; }
-static int pch_gbe_mdio_init(char *name, struct pch_gbe_regs *mac_regs) +static int pch_gbe_mdio_init(const char *name, struct pch_gbe_regs *mac_regs) { struct mii_dev *bus;
@@ -384,13 +393,14 @@ static int pch_gbe_mdio_init(char *name, struct pch_gbe_regs *mac_regs) return mdio_register(bus); }
-static int pch_gbe_phy_init(struct eth_device *dev) +static int pch_gbe_phy_init(struct udevice *dev) { - struct pch_gbe_priv *priv = dev->priv; + struct pch_gbe_priv *priv = dev_get_priv(dev); + struct eth_pdata *plat = dev_get_platdata(dev); struct phy_device *phydev; int mask = 0xffffffff;
- phydev = phy_find_by_mask(priv->bus, mask, priv->interface); + phydev = phy_find_by_mask(priv->bus, mask, plat->phy_interface); if (!phydev) { printf("pch_gbe: cannot find the phy\n"); return -1; @@ -404,63 +414,66 @@ static int pch_gbe_phy_init(struct eth_device *dev) priv->phydev = phydev; phy_config(phydev);
- return 1; + return 0; }
-int pch_gbe_register(bd_t *bis) +int pch_gbe_probe(struct udevice *dev) { - struct eth_device *dev; struct pch_gbe_priv *priv; + struct eth_pdata *plat = dev_get_platdata(dev); pci_dev_t devno; u32 iobase;
- devno = pci_find_devices(supported, 0); - if (devno == -1) - return -ENODEV; - - dev = (struct eth_device *)malloc(sizeof(*dev)); - if (!dev) - return -ENOMEM; - memset(dev, 0, sizeof(*dev)); + devno = pci_get_bdf(dev);
/* * The priv structure contains the descriptors and frame buffers which - * need a strict buswidth alignment (64 bytes) + * need a strict buswidth alignment (64 bytes). This is guaranteed by + * DM_FLAG_ALLOC_PRIV_DMA flag in the U_BOOT_DRIVER. */ - priv = (struct pch_gbe_priv *)memalign(PCH_GBE_ALIGN_SIZE, - sizeof(*priv)); - if (!priv) { - free(dev); - return -ENOMEM; - } - memset(priv, 0, sizeof(*priv)); + priv = dev_get_priv(dev);
- dev->priv = priv; - priv->dev = dev; priv->bdf = devno;
pci_read_config_dword(devno, PCI_BASE_ADDRESS_1, &iobase); iobase &= PCI_BASE_ADDRESS_MEM_MASK; iobase = pci_mem_to_phys(devno, iobase);
- dev->iobase = iobase; + plat->iobase = iobase; priv->mac_regs = (struct pch_gbe_regs *)iobase;
- sprintf(dev->name, "pch_gbe"); - /* Read MAC address from SROM and initialize dev->enetaddr with it */ - pch_gbe_mac_read(priv->mac_regs, dev->enetaddr); - - dev->init = pch_gbe_init; - dev->halt = pch_gbe_halt; - dev->send = pch_gbe_send; - dev->recv = pch_gbe_recv; + pch_gbe_mac_read(priv->mac_regs, plat->enetaddr);
- eth_register(dev); - - priv->interface = PHY_INTERFACE_MODE_RGMII; + plat->phy_interface = PHY_INTERFACE_MODE_RGMII; pch_gbe_mdio_init(dev->name, priv->mac_regs); priv->bus = miiphy_get_dev_by_name(dev->name);
return pch_gbe_phy_init(dev); } + +static const struct eth_ops pch_gbe_ops = { + .start = pch_gbe_start, + .send = pch_gbe_send, + .recv = pch_gbe_recv, + .free_pkt = pch_gbe_free_pkt, + .stop = pch_gbe_stop, +}; + +static const struct udevice_id pch_gbe_ids[] = { + { .compatible = "intel,pch-gbe" }, + { } +}; + +U_BOOT_DRIVER(eth_pch_gbe) = { + .name = "pch_gbe", + .id = UCLASS_ETH, + .of_match = pch_gbe_ids, + .probe = pch_gbe_probe, + .ops = &pch_gbe_ops, + .priv_auto_alloc_size = sizeof(struct pch_gbe_priv), + .platdata_auto_alloc_size = sizeof(struct eth_pdata), + .flags = DM_FLAG_ALLOC_PRIV_DMA, +}; + +U_BOOT_PCI_DEVICE(eth_pch_gbe, supported); diff --git a/drivers/net/pch_gbe.h b/drivers/net/pch_gbe.h index 11329d4..afcb03d 100644 --- a/drivers/net/pch_gbe.h +++ b/drivers/net/pch_gbe.h @@ -287,12 +287,10 @@ struct pch_gbe_priv { struct pch_gbe_rx_desc rx_desc[PCH_GBE_DESC_NUM]; struct pch_gbe_tx_desc tx_desc[PCH_GBE_DESC_NUM]; char rx_buff[PCH_GBE_DESC_NUM][PCH_GBE_RX_FRAME_LEN]; - struct eth_device *dev; struct phy_device *phydev; struct mii_dev *bus; struct pch_gbe_regs *mac_regs; pci_dev_t bdf; - u32 interface; int rx_idx; int tx_idx; }; diff --git a/include/netdev.h b/include/netdev.h index 662d173..3d5a54f 100644 --- a/include/netdev.h +++ b/include/netdev.h @@ -70,7 +70,6 @@ int natsemi_initialize(bd_t *bis); int ne2k_register(void); int npe_initialize(bd_t *bis); int ns8382x_initialize(bd_t *bis); -int pch_gbe_register(bd_t *bis); int pcnet_initialize(bd_t *bis); int ppc_4xx_eth_initialize (bd_t *bis); int rtl8139_initialize(bd_t *bis); @@ -123,9 +122,6 @@ static inline int pci_eth_init(bd_t *bis) #ifdef CONFIG_E1000 num += e1000_initialize(bis); #endif -#ifdef CONFIG_PCH_GBE - num += pch_gbe_register(bis); -#endif #ifdef CONFIG_PCNET num += pcnet_initialize(bis); #endif
Hi Bin,
On Tue, Aug 25, 2015 at 2:22 AM, Bin Meng bmeng.cn@gmail.com wrote:
This commit converts pch_gbe ethernet driver to driver model.
Since this driver is only used by Intel Crown Bay board, the conversion does not keep the non-dm version.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
This looks great.
Acked-by: Joe Hershberger joe.hershberger@ni.com
On 25 August 2015 at 01:22, Bin Meng bmeng.cn@gmail.com wrote:
This commit converts pch_gbe ethernet driver to driver model.
Since this driver is only used by Intel Crown Bay board, the conversion does not keep the non-dm version.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/net/pch_gbe.c | 133 +++++++++++++++++++++++++++----------------------- drivers/net/pch_gbe.h | 2 - include/netdev.h | 4 -- 3 files changed, 73 insertions(+), 66 deletions(-)
Nice!
Acked-by: Simon Glass sjg@chromium.org
Add Kconfig option in preparation for moving board to use Kconfig.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
drivers/net/Kconfig | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 7367d9e..ccaf675 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -79,4 +79,13 @@ config ETH_DESIGNWARE 100Mbit and 1 Gbit operation. You must enable CONFIG_PHYLIB to provide the PHY (physical media interface).
+config PCH_GBE + bool "Intel Platform Controller Hub EG20T GMAC driver" + depends on DM_PCI + default n + help + This MAC is present in Intel Platform Controller Hub EG20T. It + supports 10/100/1000 Mbps operation. You must enable CONFIG_PHYLIB + to provide the PHY (physical media interface). + endif # NETDEVICES
Hi Bin,
(with list this time)
On Tue, Aug 25, 2015 at 2:22 AM, Bin Meng bmeng.cn@gmail.com wrote:
Add Kconfig option in preparation for moving board to use Kconfig.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/net/Kconfig | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 7367d9e..ccaf675 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -79,4 +79,13 @@ config ETH_DESIGNWARE 100Mbit and 1 Gbit operation. You must enable CONFIG_PHYLIB to provide the PHY (physical media interface).
+config PCH_GBE
bool "Intel Platform Controller Hub EG20T GMAC driver"depends on DM_PCIdefault nhelpThis MAC is present in Intel Platform Controller Hub EG20T. Itsupports 10/100/1000 Mbps operation. You must enable CONFIG_PHYLIBto provide the PHY (physical media interface).
You should not have statements like this in the help. You should either "select" or "depends on" PHYLIB in this config (I would use select). Also this depends DM_ETH as well.
endif # NETDEVICES
1.8.2.1
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi Joe,
On Wed, Aug 26, 2015 at 3:10 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
(with list this time)
On Tue, Aug 25, 2015 at 2:22 AM, Bin Meng bmeng.cn@gmail.com wrote:
Add Kconfig option in preparation for moving board to use Kconfig.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/net/Kconfig | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 7367d9e..ccaf675 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -79,4 +79,13 @@ config ETH_DESIGNWARE 100Mbit and 1 Gbit operation. You must enable CONFIG_PHYLIB to provide the PHY (physical media interface).
+config PCH_GBE
bool "Intel Platform Controller Hub EG20T GMAC driver"depends on DM_PCIdefault nhelpThis MAC is present in Intel Platform Controller Hub EG20T. Itsupports 10/100/1000 Mbps operation. You must enable CONFIG_PHYLIBto provide the PHY (physical media interface).You should not have statements like this in the help. You should either "select" or "depends on" PHYLIB in this config (I would use select). Also this depends DM_ETH as well.
Yep, looks that I need prepare a new patch to convert PHYLIB to a Kconfig option first. As for dependency on DM_ETH, I think this is already indicated by NETDEVICES which is default y if DM_ETH.
Regards, Bin
Hi Bin,
On Tue, Aug 25, 2015 at 8:25 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Joe,
On Wed, Aug 26, 2015 at 3:10 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
(with list this time)
On Tue, Aug 25, 2015 at 2:22 AM, Bin Meng bmeng.cn@gmail.com wrote:
Add Kconfig option in preparation for moving board to use Kconfig.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
drivers/net/Kconfig | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 7367d9e..ccaf675 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -79,4 +79,13 @@ config ETH_DESIGNWARE 100Mbit and 1 Gbit operation. You must enable CONFIG_PHYLIB to provide the PHY (physical media interface).
+config PCH_GBE
bool "Intel Platform Controller Hub EG20T GMAC driver"depends on DM_PCIdefault nhelpThis MAC is present in Intel Platform Controller Hub EG20T. Itsupports 10/100/1000 Mbps operation. You must enable CONFIG_PHYLIBto provide the PHY (physical media interface).You should not have statements like this in the help. You should either "select" or "depends on" PHYLIB in this config (I would use select). Also this depends DM_ETH as well.
Yep, looks that I need prepare a new patch to convert PHYLIB to a Kconfig option first. As for dependency on DM_ETH, I think this is already indicated by NETDEVICES which is default y if DM_ETH.
It's not the same thing since NETDEVICES is only default y if DM_ETH... NETDEVICES does not select DM_ETH.
-Joe
Now that we have converted the pch_gbe driver to driver moel, enable it on Intel Crown Bay board.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
configs/crownbay_defconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/configs/crownbay_defconfig b/configs/crownbay_defconfig index f027faf..f328159 100644 --- a/configs/crownbay_defconfig +++ b/configs/crownbay_defconfig @@ -21,6 +21,7 @@ CONFIG_DM_PCI=y CONFIG_SPI_FLASH=y CONFIG_DM_ETH=y CONFIG_E1000=y +CONFIG_PCH_GBE=y CONFIG_VIDEO_VESA=y CONFIG_FRAMEBUFFER_SET_VESA_MODE=y CONFIG_USB=y
Hi Bin,
On Tue, Aug 25, 2015 at 2:22 AM, Bin Meng bmeng.cn@gmail.com wrote:
Now that we have converted the pch_gbe driver to driver moel, enable it on Intel Crown Bay board.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Looks good if squashed with patch 6.
-Joe
On 25 August 2015 at 01:22, Bin Meng bmeng.cn@gmail.com wrote:
Now that we have converted the pch_gbe driver to driver moel, enable it on Intel Crown Bay board.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
configs/crownbay_defconfig | 1 + 1 file changed, 1 insertion(+)
Acked-by: Simon Glass sjg@chromium.org
Hi Joe,
On Tue, Aug 25, 2015 at 3:22 PM, Bin Meng bmeng.cn@gmail.com wrote:
Testing either pch_gbe or e1000 driver via tftp command on Intel Crown Bay board, shows the following failure.
TFTP error: 'Unsupported option(s) requested' (8)It turns out commit 620776d causes this. Revert this commit for now.
Please check http://lists.denx.de/pipermail/u-boot/2015-August/225187.html on why this commit should be reverted.
Let me know if you have different thoughts (eg: I can respin a v2 to explicitly mention in the commit message that commit 620776d is a spec violation to RTC 2349 thus we need revert it)
Signed-off-by: Bin Meng bmeng.cn@gmail.com
net/tftp.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/tftp.c b/net/tftp.c index 18ce84c..89be32a 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -19,10 +19,10 @@ /* Well known TFTP port # */ #define WELL_KNOWN_PORT 69 /* Millisecs to timeout for lost pkt */ -#define TIMEOUT 100UL +#define TIMEOUT 5000UL #ifndef CONFIG_NET_RETRY_COUNT /* # of timeouts before giving up */ -# define TIMEOUT_COUNT 1000 +# define TIMEOUT_COUNT 10 #else # define TIMEOUT_COUNT (CONFIG_NET_RETRY_COUNT * 2) #endif @@ -711,10 +711,10 @@ void tftp_start(enum proto_t protocol) if (ep != NULL) timeout_ms = simple_strtol(ep, NULL, 10);
if (timeout_ms < 10) {printf("TFTP timeout (%ld ms) too low, set min = 10 ms\n",
if (timeout_ms < 1000) {printf("TFTP timeout (%ld ms) too low, set min = 1000 ms\n", timeout_ms);
timeout_ms = 10;
timeout_ms = 1000; } debug("TFTP blocksize = %i, timeout = %ld ms\n",--
Regards, Bin
Hi Bin,
On Tue, Aug 25, 2015 at 4:26 AM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Joe,
On Tue, Aug 25, 2015 at 3:22 PM, Bin Meng bmeng.cn@gmail.com wrote:
Testing either pch_gbe or e1000 driver via tftp command on Intel Crown Bay board, shows the following failure.
TFTP error: 'Unsupported option(s) requested' (8)It turns out commit 620776d causes this. Revert this commit for now.
Please check http://lists.denx.de/pipermail/u-boot/2015-August/225187.html on why this commit should be reverted.
Let me know if you have different thoughts (eg: I can respin a v2 to explicitly mention in the commit message that commit 620776d is a spec violation to RTC 2349 thus we need revert it)
I agree we should revert it for this release. Please send a v2 that describes as much detail from the RFC as needed and references the RFC as well. We will revert it and figure out if there is a compliant way we can improve performance in a high-load situation.
By pushing this to next release we will get much more testing and not risk losing compatibility in this release.
-Joe
participants (4)
-
Bin Meng -
Joe Hershberger -
Scott Wood -
Simon Glass