[U-Boot] [PATCH v2 01/10] net: Revert "tftp: adjust settings to be suitable for 100Mbit ethernet"
Commit 620776d "tftp: adjust settings to be suitable for 100Mbit ethernet" causes the following error message when trying to load a file using 'tftp' command via a tftp server.
TFTP error: 'Unsupported option(s) requested' (8)
This is due to with commit 620776d changes, the tftp option 'timeout' value is now set to zero which is an invalid value as per RFC2349 [1]. Valid values range between "1" and "255" seconds, inclusive. With some tftp servers that strictly implement the RFC requirement, it reports such an error message.
Revert commit 620776d for RFC compliance.
[1] https://www.ietf.org/rfc/rfc2349.txt
Signed-off-by: Bin Meng bmeng.cn@gmail.com
---
Changes in v2: - Rewrite the commit message to mention RFC2349
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
---
Changes in v2: - Use lower_32_bits() and upper_32_bits()
drivers/net/e1000.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/e1000.c b/drivers/net/e1000.c index 6f74d30..7b830ff 100644 --- a/drivers/net/e1000.c +++ b/drivers/net/e1000.c @@ -4978,8 +4978,8 @@ e1000_configure_tx(struct e1000_hw *hw) unsigned long tipg, tarc; uint32_t ipgr1, ipgr2;
- 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, lower_32_bits((unsigned long)tx_base)); + E1000_WRITE_REG(hw, TDBAH, upper_32_bits((unsigned long)tx_base));
E1000_WRITE_REG(hw, TDLEN, 128);
@@ -5103,6 +5103,7 @@ e1000_configure_rx(struct e1000_hw *hw) { unsigned long rctl, ctrl_ext; rx_tail = 0; + /* 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 +5123,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, lower_32_bits((unsigned long)rx_base)); + E1000_WRITE_REG(hw, RDBAH, upper_32_bits((unsigned long)rx_base));
E1000_WRITE_REG(hw, RDLEN, 128);
Hi,
On 26 August 2015 at 06:17, Bin Meng bmeng.cn@gmail.com 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
Changes in v2:
- Use lower_32_bits() and upper_32_bits()
drivers/net/e1000.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
Acked-by: Simon Glass sjg@chromium.org
I'd like to pick this up for u-boot-x86 as it fixes warnings in most x86 boards.
Regards, Simon
Hi Bin,
On Wed, Aug 26, 2015 at 8:17 AM, Bin Meng bmeng.cn@gmail.com 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
Acked-by: Joe Hershberger joe.hershberger@ni.com
On 26 August 2015 at 07:42, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Wed, Aug 26, 2015 at 8:17 AM, Bin Meng bmeng.cn@gmail.com 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
Acked-by: Joe Hershberger joe.hershberger@ni.com
Applied to u-boot-x86, thanks!
With driver model, board_eth_init() or cpu_eth_init() is not a must. Thus we don't need print a misleading "Net Initialization Skipped".
Signed-off-by: Bin Meng bmeng.cn@gmail.com
---
Changes in v2: - Change to only comment out "Net Initialization Skipped" printf
net/eth.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/net/eth.c b/net/eth.c index d3ec8d6..c46a8c3 100644 --- a/net/eth.c +++ b/net/eth.c @@ -107,7 +107,9 @@ static void eth_common_init(void) if (cpu_eth_init(gd->bd) < 0) printf("CPU Net Initialization Failed\n"); } else { +#ifndef CONFIG_DM_ETH printf("Net Initialization Skipped\n"); +#endif } }
On Wed, Aug 26, 2015 at 8:17 AM, Bin Meng bmeng.cn@gmail.com wrote:
With driver model, board_eth_init() or cpu_eth_init() is not a must. Thus we don't need print a misleading "Net Initialization Skipped".
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Acked-by: Joe Hershberger joe.hershberger@ni.com
Add one more ethernet device node in the sandbox test device tree, with name 'sbe5'. This is to support a new test case for testing network device rotation.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
---
Changes in v2: - New patch to add a new test case for dm_test_eth_rotate
arch/sandbox/dts/test.dts | 7 +++++++ include/configs/sandbox.h | 3 ++- test/dm/eth.c | 5 +++++ 3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index c948df8..f5217fb 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -9,6 +9,7 @@ aliases { console = &uart0; eth0 = "/eth@10002000"; + eth3 = ð_3; eth5 = ð_5; i2c0 = "/i2c@0"; pci0 = &pci; @@ -121,6 +122,12 @@ fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x11>; };
+ eth_3: sbe5 { + compatible = "sandbox,eth"; + reg = <0x10005000 0x1000>; + fake-host-hwaddr = <0x00 0x00 0x66 0x44 0x22 0x33>; + }; + eth@10004000 { compatible = "sandbox,eth"; reg = <0x10004000 0x1000>; diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h index 6965d92..32e3a9b 100644 --- a/include/configs/sandbox.h +++ b/include/configs/sandbox.h @@ -187,7 +187,8 @@
#define SANDBOX_ETH_SETTINGS "ethaddr=00:00:11:22:33:44\0" \ "eth1addr=00:00:11:22:33:45\0" \ - "eth5addr=00:00:11:22:33:46\0" \ + "eth3addr=00:00:11:22:33:46\0" \ + "eth5addr=00:00:11:22:33:47\0" \ "ipaddr=1.2.3.4\0"
#define MEM_LAYOUT_ENV_SETTINGS \ diff --git a/test/dm/eth.c b/test/dm/eth.c index 700abdd..fcfb3e1 100644 --- a/test/dm/eth.c +++ b/test/dm/eth.c @@ -106,6 +106,11 @@ static int _dm_test_eth_rotate2(struct unit_test_state *uts) ut_assertok(net_loop(PING)); ut_asserteq_str("eth@10004000", getenv("ethact"));
+ /* Make sure we can handle device name which is not eth# */ + setenv("ethact", "sbe5"); + ut_assertok(net_loop(PING)); + ut_asserteq_str("sbe5", getenv("ethact")); + return 0; }
Hi Bin,
On Wed, Aug 26, 2015 at 8:17 AM, Bin Meng bmeng.cn@gmail.com wrote:
Add one more ethernet device node in the sandbox test device tree, with name 'sbe5'. This is to support a new test case for testing network device rotation.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Acked-by: Joe Hershberger joe.hershberger@ni.com
When given a device name string, we should test to see if it is really an alias like "eth#".
Signed-off-by: Bin Meng bmeng.cn@gmail.com
---
Changes in v2: - Change to use strncmp()
net/eth.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/eth.c b/net/eth.c index c46a8c3..26520d3 100644 --- a/net/eth.c +++ b/net/eth.c @@ -195,10 +195,11 @@ struct udevice *eth_get_dev_by_name(const char *devname) const char *startp = NULL; struct udevice *it; struct uclass *uc; + int len = strlen("eth");
/* Must be longer than 3 to be an alias */ - if (strlen(devname) > strlen("eth")) { - startp = devname + strlen("eth"); + if (!strncmp(devname, "eth", len) && strlen(devname) > len) { + startp = devname + len; seq = simple_strtoul(startp, &endp, 10); }
On Wed, Aug 26, 2015 at 8:17 AM, Bin Meng bmeng.cn@gmail.com wrote:
When given a device name string, we should test to see if it is really an alias like "eth#".
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Acked-by: Joe Hershberger joe.hershberger@ni.com
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 Acked-by: Simon Glass sjg@chromium.org ---
Changes in v2: None
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
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 ---
Changes in v2: None
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 Wed, Aug 26, 2015 at 8:17 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.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Acked-by: Joe Hershberger joe.hershberger@ni.com
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 Acked-by: Joe Hershberger joe.hershberger@ni.com Acked-by: Simon Glass sjg@chromium.org ---
Changes in v2: None
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
Add Kconfig option in preparation for moving board to use Kconfig.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
---
Changes in v2: - Add PHYLIB Kconfig option and let PCH_GBE select PHYLIB
drivers/net/Kconfig | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 7367d9e..5e1687b 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -8,6 +8,12 @@ config DM_ETH This is currently implemented in net/eth.c Look in include/net.h for details.
+config PHYLIB + bool + default n + help + Enable Ethernet PHY (physical media interface) support. + menuconfig NETDEVICES bool "Network device support" depends on NET @@ -79,4 +85,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_ETH && DM_PCI + select PHYLIB + default n + help + This MAC is present in Intel Platform Controller Hub EG20T. It + supports 10/100/1000 Mbps operation. + endif # NETDEVICES
Hi Bin,
On Wed, Aug 26, 2015 at 8:17 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
Changes in v2:
- Add PHYLIB Kconfig option and let PCH_GBE select PHYLIB
drivers/net/Kconfig | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 7367d9e..5e1687b 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -8,6 +8,12 @@ config DM_ETH This is currently implemented in net/eth.c Look in include/net.h for details.
+config PHYLIB
booldefault nhelpEnable Ethernet PHY (physical media interface) support.menuconfig NETDEVICES bool "Network device support" depends on NET @@ -79,4 +85,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_ETH && DM_PCIselect PHYLIBdefault nhelpThis MAC is present in Intel Platform Controller Hub EG20T. Itsupports 10/100/1000 Mbps operation.endif # NETDEVICES
1.8.2.1
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi Bin,
On Wed, Aug 26, 2015 at 8:17 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
Changes in v2:
- Add PHYLIB Kconfig option and let PCH_GBE select PHYLIB
drivers/net/Kconfig | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 7367d9e..5e1687b 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -8,6 +8,12 @@ config DM_ETH This is currently implemented in net/eth.c Look in include/net.h for details.
+config PHYLIB
bool
I don't think we want this as an unchoose-able option. You should include some bool text.
default n
This is not needed, remove it.
helpEnable Ethernet PHY (physical media interface) support.menuconfig NETDEVICES bool "Network device support" depends on NET @@ -79,4 +85,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_ETH && DM_PCIselect PHYLIBdefault n
This is not needed, remove it.
helpThis MAC is present in Intel Platform Controller Hub EG20T. Itsupports 10/100/1000 Mbps operation.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 11:23 PM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Wed, Aug 26, 2015 at 8:17 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
Changes in v2:
- Add PHYLIB Kconfig option and let PCH_GBE select PHYLIB
drivers/net/Kconfig | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 7367d9e..5e1687b 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -8,6 +8,12 @@ config DM_ETH This is currently implemented in net/eth.c Look in include/net.h for details.
+config PHYLIB
boolI don't think we want this as an unchoose-able option. You should include some bool text.
I hide it from menuconfig, as I thought this was actually a driver requirement, not something user can choose. Even if someone selects this option in the menuconfig, without an ethernet driver to make use of it, it is useless.
default nThis is not needed, remove it.
OK
helpEnable Ethernet PHY (physical media interface) support.menuconfig NETDEVICES bool "Network device support" depends on NET @@ -79,4 +85,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_ETH && DM_PCIselect PHYLIBdefault nThis is not needed, remove it.
OK
helpThis MAC is present in Intel Platform Controller Hub EG20T. Itsupports 10/100/1000 Mbps operation.endif # NETDEVICES
Regards, Bin
Hi Bin,
On Wed, Aug 26, 2015 at 10:34 AM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Joe,
On Wed, Aug 26, 2015 at 11:23 PM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Wed, Aug 26, 2015 at 8:17 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
Changes in v2:
- Add PHYLIB Kconfig option and let PCH_GBE select PHYLIB
drivers/net/Kconfig | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 7367d9e..5e1687b 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -8,6 +8,12 @@ config DM_ETH This is currently implemented in net/eth.c Look in include/net.h for details.
+config PHYLIB
boolI don't think we want this as an unchoose-able option. You should include some bool text.
I hide it from menuconfig, as I thought this was actually a driver requirement, not something user can choose. Even if someone selects this option in the menuconfig, without an ethernet driver to make use of it, it is useless.
Based on its selection, the capability of CMD_MII is altered. It certainly is tied to the driver though. I guess I could go either way.
default nThis is not needed, remove it.
OK
helpEnable Ethernet PHY (physical media interface) support.menuconfig NETDEVICES bool "Network device support" depends on NET @@ -79,4 +85,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_ETH && DM_PCIselect PHYLIBdefault nThis is not needed, remove it.
OK
helpThis MAC is present in Intel Platform Controller Hub EG20T. Itsupports 10/100/1000 Mbps operation.endif # NETDEVICES
Regards, Bin
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 Acked-by: Simon Glass sjg@chromium.org
---
Changes in v2: - Remove CONFIG_PHYLIB from crownbay.h
configs/crownbay_defconfig | 1 + include/configs/crownbay.h | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-)
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 diff --git a/include/configs/crownbay.h b/include/configs/crownbay.h index a344c85..3153a74 100644 --- a/include/configs/crownbay.h +++ b/include/configs/crownbay.h @@ -49,9 +49,6 @@ #define CONFIG_MMC_SDMA #define CONFIG_CMD_MMC
-/* Topcliff Gigabit Ethernet */ -#define CONFIG_PHYLIB - /* Environment configuration */ #define CONFIG_ENV_SECT_SIZE 0x1000 #define CONFIG_ENV_OFFSET 0
Hi Bin,
On Wed, Aug 26, 2015 at 8:17 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 Acked-by: Simon Glass sjg@chromium.org
Changes in v2:
- Remove CONFIG_PHYLIB from crownbay.h
configs/crownbay_defconfig | 1 + include/configs/crownbay.h | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-)
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 diff --git a/include/configs/crownbay.h b/include/configs/crownbay.h index a344c85..3153a74 100644 --- a/include/configs/crownbay.h +++ b/include/configs/crownbay.h @@ -49,9 +49,6 @@ #define CONFIG_MMC_SDMA #define CONFIG_CMD_MMC
-/* Topcliff Gigabit Ethernet */ -#define CONFIG_PHYLIB
Since you added the PHYLIB config to the Kconfig, you need to run the moveconfig.py tool to convert all boards so there is not a conflict between the 2 config approaches.
/* Environment configuration */ #define CONFIG_ENV_SECT_SIZE 0x1000
#define CONFIG_ENV_OFFSET 0
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 11:25 PM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Wed, Aug 26, 2015 at 8:17 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 Acked-by: Simon Glass sjg@chromium.org
Changes in v2:
- Remove CONFIG_PHYLIB from crownbay.h
configs/crownbay_defconfig | 1 + include/configs/crownbay.h | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-)
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 diff --git a/include/configs/crownbay.h b/include/configs/crownbay.h index a344c85..3153a74 100644 --- a/include/configs/crownbay.h +++ b/include/configs/crownbay.h @@ -49,9 +49,6 @@ #define CONFIG_MMC_SDMA #define CONFIG_CMD_MMC
-/* Topcliff Gigabit Ethernet */ -#define CONFIG_PHYLIB
Since you added the PHYLIB config to the Kconfig, you need to run the moveconfig.py tool to convert all boards so there is not a conflict between the 2 config approaches.
Since it is default n, so I believe there should be no conflicts. I thought moveconfig causes lots of changes, which might be risky at this time for this release? Do you think we can leave the PHYLIB moveconfig to next release? But I may be wrong, e.g. I should trust the moveconfig tool ;-)
Regards, Bin
Hi Joe,
On Wed, Aug 26, 2015 at 11:31 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Joe,
On Wed, Aug 26, 2015 at 11:25 PM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Wed, Aug 26, 2015 at 8:17 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 Acked-by: Simon Glass sjg@chromium.org
Changes in v2:
- Remove CONFIG_PHYLIB from crownbay.h
configs/crownbay_defconfig | 1 + include/configs/crownbay.h | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-)
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 diff --git a/include/configs/crownbay.h b/include/configs/crownbay.h index a344c85..3153a74 100644 --- a/include/configs/crownbay.h +++ b/include/configs/crownbay.h @@ -49,9 +49,6 @@ #define CONFIG_MMC_SDMA #define CONFIG_CMD_MMC
-/* Topcliff Gigabit Ethernet */ -#define CONFIG_PHYLIB
Since you added the PHYLIB config to the Kconfig, you need to run the moveconfig.py tool to convert all boards so there is not a conflict between the 2 config approaches.
Since it is default n, so I believe there should be no conflicts. I thought moveconfig causes lots of changes, which might be risky at this time for this release? Do you think we can leave the PHYLIB moveconfig to next release? But I may be wrong, e.g. I should trust the moveconfig tool ;-)
Please let me know if you want me to update all boards' header and defconfig files with moveconfig for PHYLIB for this release.
Regards, Bin
Hi Bin,
On Thu, Aug 27, 2015 at 10:43 AM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Joe,
On Wed, Aug 26, 2015 at 11:31 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Joe,
On Wed, Aug 26, 2015 at 11:25 PM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Wed, Aug 26, 2015 at 8:17 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 Acked-by: Simon Glass sjg@chromium.org
Changes in v2:
- Remove CONFIG_PHYLIB from crownbay.h
configs/crownbay_defconfig | 1 + include/configs/crownbay.h | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-)
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 diff --git a/include/configs/crownbay.h b/include/configs/crownbay.h index a344c85..3153a74 100644 --- a/include/configs/crownbay.h +++ b/include/configs/crownbay.h @@ -49,9 +49,6 @@ #define CONFIG_MMC_SDMA #define CONFIG_CMD_MMC
-/* Topcliff Gigabit Ethernet */ -#define CONFIG_PHYLIB
Since you added the PHYLIB config to the Kconfig, you need to run the moveconfig.py tool to convert all boards so there is not a conflict between the 2 config approaches.
Since it is default n, so I believe there should be no conflicts. I thought moveconfig causes lots of changes, which might be risky at this time for this release? Do you think we can leave the PHYLIB moveconfig to next release? But I may be wrong, e.g. I should trust the moveconfig tool ;-)
Please let me know if you want me to update all boards' header and defconfig files with moveconfig for PHYLIB for this release.
I guess it's unnecessary for this release. At least make sure that this doesn't cause problems on any boards with DM_ETH enabled.
-Joe
On Wed, Aug 26, 2015 at 8:17 AM, Bin Meng bmeng.cn@gmail.com wrote:
Commit 620776d "tftp: adjust settings to be suitable for 100Mbit ethernet" causes the following error message when trying to load a file using 'tftp' command via a tftp server.
TFTP error: 'Unsupported option(s) requested' (8)This is due to with commit 620776d changes, the tftp option 'timeout' value is now set to zero which is an invalid value as per RFC2349 [1]. Valid values range between "1" and "255" seconds, inclusive. With some tftp servers that strictly implement the RFC requirement, it reports such an error message.
Revert commit 620776d for RFC compliance.
[1] https://www.ietf.org/rfc/rfc2349.txt
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Acked-by: Joe Hershberger joe.hershberger@ni.com
You should always copy the author of the patch you are reverting.
Hi Joe,
On Wed, Aug 26, 2015 at 10:50 PM, Joe Hershberger joe.hershberger@gmail.com wrote:
On Wed, Aug 26, 2015 at 8:17 AM, Bin Meng bmeng.cn@gmail.com wrote:
Commit 620776d "tftp: adjust settings to be suitable for 100Mbit ethernet" causes the following error message when trying to load a file using 'tftp' command via a tftp server.
TFTP error: 'Unsupported option(s) requested' (8)This is due to with commit 620776d changes, the tftp option 'timeout' value is now set to zero which is an invalid value as per RFC2349 [1]. Valid values range between "1" and "255" seconds, inclusive. With some tftp servers that strictly implement the RFC requirement, it reports such an error message.
Revert commit 620776d for RFC compliance.
[1] https://www.ietf.org/rfc/rfc2349.txt
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Acked-by: Joe Hershberger joe.hershberger@ni.com
You should always copy the author of the patch you are reverting.
Sorry, I should have. I believe this can be handled automatically by patman's --cc-cmd, like "--cc-cmd "./tools/patman/patman --cc-cmd /tmp/patman.17080""? But unfortunately I never get this work.
Regards, Bin
participants (3)
-
Bin Meng -
Joe Hershberger -
Simon Glass