net: Revert "tftp: adjust settings to be suitable for 100Mbit ethernet"
Subsystems (2)
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.
---
net/tftp.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
net/tftp.c +5 -5
@@ -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",
Signature · 1 line
1.8.2.1
PATCH 1/9 net: Revert "tftp: adjust settings to be suitable for 100Mbit ethernet"
Hi Joe,
Quoted 6 lines · Bin Meng
Testing either
pch_gbeor 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)
Quoted 29 lines
Signed-off-by Bin Meng <[email protected]>---
net/tftp.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)net/tftp.c +2
-3@@ -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 ) {
net/tftp.c void tftp_start(enum proto_t protocol)
Quoted 10 lines
net/tftp.c +3
-3-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
PATCH 1/9 net: Revert "tftp: adjust settings to be suitable for 100Mbit ethernet"
Hi Bin,
Quoted 8 lines · Bin Meng
Hi Joe,
Quoted 5 lines · Bin Meng
Testing either
pch_gbeor e1000 driver via tftp command on Intel Crown Bay board, shows the following failure.TFTP error: 'Unsupported option(s) requested' (8)
Quoted 8 lines
Quoted 1 line
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
PATCH 2/9 net: e1000: Fix build warnings for 32-bit
In reply to Bin Mengcommit 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.
---
drivers/net/e1000.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
drivers/net/e1000.c +7 -4
@@ -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 );
Signature · 1 line
1.8.2.1
PATCH 2/9 net: e1000: Fix build warnings for 32-bit
Quoted 23 lines · Bin Meng
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 <[email protected]>---
drivers/net/e1000.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)drivers/net/e1000.c +0
-0
drivers/net/e1000.c e1000_configure_tx(struct e1000_hw *hw) · line 4983
Quoted 9 lines
drivers/net/e1000.c +3
-2@@ -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
PATCH 3/9 dm: eth: Do not call board_eth_init() or cpu_eth_init()
In reply to Bin MengWith driver model, board_eth_init() or cpu_eth_init() is not needed. Remove the call to these in eth_common_init().
---
net/eth.c | 2 ++ 1 file changed, 2 insertions(+)
net/eth.c +2 -0
@@ -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
Signature · 1 line
1.8.2.1
PATCH 3/9 dm: eth: Do not call board_eth_init() or cpu_eth_init()
Hi Bin,
Quoted 2 lines · Bin Meng
With driver model,
board_eth_init()orcpu_eth_init()is not needed. Remove the call to these ineth_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
PATCH 3/9 dm: eth: Do not call board_eth_init() or cpu_eth_init()
Hi Joe,
Quoted 12 lines · Joe Hershberger
Hi Bin,
Quoted 2 lines · Bin Meng
With driver model,
board_eth_init()orcpu_eth_init()is not needed. Remove the call to these ineth_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
PATCH 3/9 dm: eth: Do not call board_eth_init() or cpu_eth_init()
Hi Simon,
Quoted 17 lines · Bin Meng
Hi Joe,
Quoted 12 lines · Joe Hershberger
Hi Bin,
Quoted 2 lines · Bin Meng
With driver model,
board_eth_init()orcpu_eth_init()is not needed. Remove the call to these ineth_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)
Quoted 6 lines
I think my patch does not break Simon's. My patch only comments out the call to
board_eth_init()orcpu_eth_init()which is not needed for driver model. Other stuff ineth_common_init()is still there. In fact, my patch series also needsphy_init()call (required bypch_gbedriver).
Would you confirm this does not break your boards?
Regards,
Bin
PATCH 3/9 dm: eth: Do not call board_eth_init() or cpu_eth_init()
In reply to Bin MengHi Bin,
Quoted 17 lines · Bin Meng
Hi Joe,
Quoted 12 lines · Joe Hershberger
Hi Bin,
Quoted 2 lines · Bin Meng
With driver model,
board_eth_init()orcpu_eth_init()is not needed. Remove the call to these ineth_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)
Quoted 5 lines
I think my patch does not break Simon's. My patch only comments out the call to
board_eth_init()orcpu_eth_init()which is not needed for driver model. Other stuff ineth_common_init()is still there. In fact, my patch series also needsphy_init()call (required bypch_gbedriver).
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
PATCH 3/9 dm: eth: Do not call board_eth_init() or cpu_eth_init()
Hi Joe,
Quoted 26 lines · Joe Hershberger
Hi Bin,
Quoted 17 lines · Bin Meng
Hi Joe,
Quoted 12 lines · Joe Hershberger
Hi Bin,
Quoted 2 lines · Bin Meng
With driver model,
board_eth_init()orcpu_eth_init()is not needed. Remove the call to these ineth_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)Quoted 5 lines
I think my patch does not break Simon's. My patch only comments out the call to
board_eth_init()orcpu_eth_init()which is not needed for driver model. Other stuff ineth_common_init()is still there. In fact, my patch series also needsphy_init()call (required bypch_gbedriver).
Quoted 5 lines
Even if it doesn't break Simon's board, why remove the ability for a board to add
eth_initcode. 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?
Quoted 4 lines
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.
Quoted 2 lines
I guess it can just be put back when such a board is converted.
Regards,
Bin
PATCH 3/9 dm: eth: Do not call board_eth_init() or cpu_eth_init()
Hi Bin,
Quoted 36 lines · Bin Meng
Hi Joe,
Quoted 26 lines · Joe Hershberger
Hi Bin,
Quoted 17 lines · Bin Meng
Hi Joe,
Quoted 12 lines · Joe Hershberger
Hi Bin,
Quoted 2 lines · Bin Meng
With driver model,
board_eth_init()orcpu_eth_init()is not needed. Remove the call to these ineth_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)Quoted 5 lines
I think my patch does not break Simon's. My patch only comments out the call to
board_eth_init()orcpu_eth_init()which is not needed for driver model. Other stuff ineth_common_init()is still there. In fact, my patch series also needsphy_init()call (required bypch_gbedriver).Quoted 5 lines
Even if it doesn't break Simon's board, why remove the ability for a board to add
eth_initcode. 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.
Quoted 5 lines
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, andpci_eth_init()can be called fromboard_eth_init()orcpu_eth_init().
I agree we shouldn't use them if it makes sense not to.
Quoted 4 lines
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.
Quoted 5 lines
Quoted 4 lines
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?
Quoted 8 lines
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.
Quoted 5 lines
Quoted 2 lines
I guess it can just be put back when such a board is converted.
Regards,
Bin
PATCH 3/9 dm: eth: Do not call board_eth_init() or cpu_eth_init()
In reply to Bin MengHi,
Quoted 36 lines · Bin Meng
Hi Joe,
Quoted 26 lines · Joe Hershberger
Hi Bin,
Quoted 17 lines · Bin Meng
Hi Joe,
Quoted 12 lines · Joe Hershberger
Hi Bin,
Quoted 2 lines · Bin Meng
With driver model,
board_eth_init()orcpu_eth_init()is not needed. Remove the call to these ineth_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)Quoted 5 lines
I think my patch does not break Simon's. My patch only comments out the call to
board_eth_init()orcpu_eth_init()which is not needed for driver model. Other stuff ineth_common_init()is still there. In fact, my patch series also needsphy_init()call (required bypch_gbedriver).Quoted 5 lines
Even if it doesn't break Simon's board, why remove the ability for a board to add
eth_initcode. 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.
Quoted 8 lines
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, andpci_eth_init()can be called fromboard_eth_init()orcpu_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.
Quoted 13 lines
Quoted 5 lines
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"); }
Quoted 7 lines
This is misleading message.
Quoted 2 lines
I guess it can just be put back when such a board is converted.
Regards,
Bin
Regards,
Simon
PATCH 4/9 dm: eth: Correctly detect alias in eth_get_dev_by_name()
In reply to Bin MengWhen 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).
---
net/eth.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
net/eth.c +1 -1
@@ -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 ); }
Signature · 1 line
1.8.2.1
PATCH 4/9 dm: eth: Correctly detect alias in eth_get_dev_by_name()
Hi Bin,
Quoted 5 lines · Bin Meng
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.
Quoted 27 lines
Signed-off-by Bin Meng <[email protected]>---
net/eth.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)net/eth.c +1
-2@@ -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 at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
PATCH 5/9 x86: crownbay: Convert to use CONFIG_DM_USB
In reply to Bin MengMove to driver model for USB on Intel Crown Bay.
---
configs/crownbay_defconfig | 2 ++ 1 file changed, 2 insertions(+)
configs/crownbay_defconfig +2 -0
@@ -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
Signature · 1 line
1.8.2.1
PATCH 5/9 x86: crownbay: Convert to use CONFIG_DM_USB
Hi Bin,
Quoted 3 lines · Bin Meng
Move to driver model for USB on Intel Crown Bay.
Signed-off-by Bin Meng <[email protected]>
PATCH 5/9 x86: crownbay: Convert to use CONFIG_DM_USB
In reply to Bin MengQuoted 7 lines · Bin Meng
Move to driver model for USB on Intel Crown Bay.
Signed-off-by Bin Meng <[email protected]>---
configs/crownbay_defconfig | 2 ++ 1 file changed, 2 insertions(+)
PATCH 6/9 x86: crownbay: Convert to use CONFIG_DM_ETH for E1000
In reply to Bin MengSince 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.
---
board/intel/crownbay/crownbay.c | 6 ------ configs/crownbay_defconfig | 2 +- include/configs/crownbay.h | 1 - 3 files changed, 1 insertion(+), 8 deletions(-)
board/intel/crownbay/crownbay.c +0 -6
@@ -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); -}
configs/crownbay_defconfig +1 -1
@@ -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
include/configs/crownbay.h +0 -1
@@ -50,7 +50,6 @@#define CONFIG_CMD_MMC /* Topcliff Gigabit Ethernet */ -#define CONFIG_PCH_GBE #define CONFIG_PHYLIB /* Environment configuration */
Signature · 1 line
1.8.2.1
PATCH 6/9 x86: crownbay: Convert to use CONFIG_DM_ETH for E1000
Hi Bin,
Quoted 3 lines · Bin Meng
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.
Quoted 1 line
Signed-off-by Bin Meng <[email protected]>
PATCH 6/9 x86: crownbay: Convert to use CONFIG_DM_ETH for E1000
Hi Joe,
Quoted 10 lines · Joe Hershberger
Hi Bin,
Quoted 3 lines · Bin Meng
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.
Quoted 2 lines
Quoted 2 lines
Signed-off-by Bin Meng <[email protected]>
Regards,
Bin
PATCH 6/9 x86: crownbay: Convert to use CONFIG_DM_ETH for E1000
Hi,
Quoted 22 lines · Bin Meng
Hi Joe,
Quoted 10 lines · Joe Hershberger
Hi Bin,
Quoted 3 lines · Bin Meng
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 disabledCONFIG_PCH_GBE(non-dm driver). Then after patch 7 converts thepch_gbedriver to dm, I added that driver back in patch
9. This way it passes buildman testing without breaking bisectability.
Quoted 2 lines
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.
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
Signed-off-by Bin Meng <[email protected]>Regards,
Bin
Regards,
Simon
PATCH 6/9 x86: crownbay: Convert to use CONFIG_DM_ETH for E1000
Quoted 25 lines · Simon Glass
Hi,
Quoted 22 lines · Bin Meng
Hi Joe,
Quoted 10 lines · Joe Hershberger
Hi Bin,
Quoted 3 lines · Bin Meng
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 disabledCONFIG_PCH_GBE(non-dm driver). Then after patch 7 converts thepch_gbedriver to dm, I added that driver back in patch
9. This way it passes buildman testing without breaking bisectability.
Quoted 5 lines
Quoted 2 lines
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
PATCH 7/9 net: pch_gbe: Convert to driver model
In reply to Bin MengThis 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.
---
drivers/net/pch_gbe.c | 133 +++++++++++++++++++++++++++----------------------- drivers/net/pch_gbe.h | 2 - include/netdev.h | 4 -- 3 files changed, 73 insertions(+), 66 deletions(-)
drivers/net/pch_gbe.c +73 -60
@@ -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);
drivers/net/pch_gbe.h +0 -2
@@ -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; };
include/netdev.h +0 -4
@@ -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
Signature · 1 line
1.8.2.1
PATCH 7/9 net: pch_gbe: Convert to driver model
Hi Bin,
Quoted 6 lines · Bin Meng
This commit converts
pch_gbeethernet 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 <[email protected]>
This looks great.
PATCH 7/9 net: pch_gbe: Convert to driver model
In reply to Bin MengQuoted 5 lines · Bin Meng
This commit converts
pch_gbeethernet driver to driver model.Since this driver is only used by Intel Crown Bay board, the conversion does not keep the non-dm version.
Quoted 7 lines
Signed-off-by Bin Meng <[email protected]>---
drivers/net/pch_gbe.c | 133 +++++++++++++++++++++++++++----------------------- drivers/net/pch_gbe.h | 2 - include/netdev.h | 4 -- 3 files changed, 73 insertions(+), 66 deletions(-)
Nice!
PATCH 8/9 net: pch_gbe: Add Kconfig option
In reply to Bin MengAdd Kconfig option in preparation for moving board to use Kconfig.
---
drivers/net/Kconfig | 9 +++++++++ 1 file changed, 9 insertions(+)
drivers/net/Kconfig +9 -0
@@ -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
Signature · 1 line
1.8.2.1
PATCH 8/9 net: pch_gbe: Add Kconfig option
Hi Bin,
(with list this time)
Quoted 14 lines · Bin Meng
Add
Kconfigoption in preparation for moving board to useKconfig.Signed-off-by Bin Meng <[email protected]>---
drivers/net/Kconfig | 9 +++++++++ 1 file changed, 9 insertions(+)drivers/net/Kconfig +0
-0@@ -79,4 +79,13 @@ config ETH_DESIGNWARE 100Mbit and 1 Gbit operation. You must enable CONFIG_PHYLIB to
drivers/net/Kconfig config ETH_DESIGNWARE · line 89
Quoted 10 lines
drivers/net/Kconfig +8
-0provide 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).
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.
Quoted 9 lines
drivers/net/Kconfig +1
-1+ endif # NETDEVICES --1.8.2.1
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
PATCH 8/9 net: pch_gbe: Add Kconfig option
Hi Joe,
Quoted 21 lines · Joe Hershberger
Hi Bin,
(with list this time)
Quoted 16 lines · Bin Meng
Add
Kconfigoption in preparation for moving board to useKconfig.Signed-off-by Bin Meng <[email protected]>---
drivers/net/Kconfig | 9 +++++++++ 1 file changed, 9 insertions(+)drivers/net/Kconfig +0
-0@@ -79,4 +79,13 @@ config ETH_DESIGNWARE 100Mbit and 1 Gbit operation. You must enable CONFIG_PHYLIB to provide the PHY (physical media interface).
Quoted 13 lines
drivers/net/Kconfig config ETH_DESIGNWARE · line 89
Quoted 8 lines
drivers/net/Kconfig +8
-0+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).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_ETHas 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
PATCH 8/9 net: pch_gbe: Add Kconfig option
Hi Bin,
Quoted 39 lines · Bin Meng
Hi Joe,
Quoted 21 lines · Joe Hershberger
Hi Bin,
(with list this time)
Quoted 16 lines · Bin Meng
Add
Kconfigoption in preparation for moving board to useKconfig.Signed-off-by Bin Meng <[email protected]>---
drivers/net/Kconfig | 9 +++++++++ 1 file changed, 9 insertions(+)drivers/net/Kconfig +0
-0@@ -79,4 +79,13 @@ config ETH_DESIGNWARE 100Mbit and 1 Gbit operation. You must enable CONFIG_PHYLIB to provide the PHY (physical media interface).Quoted 13 lines
drivers/net/Kconfig config ETH_DESIGNWARE · line 89
Quoted 8 lines
drivers/net/Kconfig +8
-0+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).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_ETHas well.
Quoted 3 lines
Yep, looks that I need prepare a new patch to convert PHYLIB to a
Kconfigoption first. As for dependency onDM_ETH, I think this is already indicated by NETDEVICES which is default y ifDM_ETH.
It's not the same thing since NETDEVICES is only default y if DM_ETH... NETDEVICES does not select DM_ETH.
-Joe
PATCH 9/9 x86: crownbay: Enable CONFIG_PCH_GBE
In reply to Bin MengNow that we have converted the pch_gbe driver to driver moel, enable it on Intel Crown Bay board.
---
configs/crownbay_defconfig | 1 + 1 file changed, 1 insertion(+)
configs/crownbay_defconfig +1 -0
@@ -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
Signature · 1 line
1.8.2.1
PATCH 9/9 x86: crownbay: Enable CONFIG_PCH_GBE
Hi Bin,
Quoted 4 lines · Bin Meng
Now that we have converted the
pch_gbedriver to driver moel, enable it on Intel Crown Bay board.Signed-off-by Bin Meng <[email protected]>
Looks good if squashed with patch 6.
-Joe
PATCH 9/9 x86: crownbay: Enable CONFIG_PCH_GBE
In reply to Bin MengQuoted 8 lines · Bin Meng
Now that we have converted the
pch_gbedriver to driver moel, enable it on Intel Crown Bay board.Signed-off-by Bin Meng <[email protected]>---
configs/crownbay_defconfig | 1 + 1 file changed, 1 insertion(+)