Hi Neil, Jean-Marie,
On 5/12/25 10:10 AM, neil.armstr...@linaro.org wrote:
Hi,
Thanks for fixing it !
On 11/05/2025 20:02, ver...@hpe.com wrote:
From: Jean-Marie Verdun <ver...@hpe.com>
Add support for the Wiznet W5500 spi to ethernet controller
Signed-off-by: Jean-Marie Verdun <ver...@hpe.com>
---
drivers/net/Kconfig | 9 +
drivers/net/Makefile | 1 +
drivers/net/w5500.c | 584 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 594 insertions(+)
create mode 100644 drivers/net/w5500.c
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 950ed0f25a9..74668f477ae 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -19,6 +19,15 @@ config SPL_DM_ETH
depends on SPL_NET
def_bool y
+config W5500
+ bool "W5500 device driver"
+ depends on SPI
+ help
+ Enable w5500 driver
+
+ Adds support for Wiznet W5500 device. The device must be attached
+ to a SPI bus.
+
config DM_MDIO
bool "Enable Driver Model for MDIO devices"
depends on PHYLIB
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 67bba3a8536..6d85c38d869 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -103,6 +103,7 @@ obj-$(CONFIG_SUN8I_EMAC) += sun8i_emac.o
obj-$(CONFIG_TSEC_ENET) += tsec.o fsl_mdio.o
obj-$(CONFIG_TULIP) += dc2114x.o
obj-$(CONFIG_VSC7385_ENET) += vsc7385.o
+obj-$(CONFIG_W5500) += w5500.o
obj-$(CONFIG_XILINX_AXIEMAC) += xilinx_axi_emac.o
obj-$(CONFIG_XILINX_AXIMRMAC) += xilinx_axi_mrmac.o
obj-$(CONFIG_XILINX_EMACLITE) += xilinx_emaclite.o
diff --git a/drivers/net/w5500.c b/drivers/net/w5500.c
new file mode 100644
index 00000000000..a15255bb457
--- /dev/null
+++ b/drivers/net/w5500.c
@@ -0,0 +1,584 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright Hewlett Packard Enterprise Development LP.
+ *
+ * Jean-Marie Verdun <ver...@hpe.com>
+ *
+ * Inspired from the linux kernel driver from:
+ * - Copyright (C) 2006-2008 WIZnet Co.,Ltd.
+ * - Copyright (C) 2012 Mike Sinkovsky <ms...@permonline.ru>
+ *
+ * available at
+ * - https://eur02.safelinks.protection.outlook.com/?
url=https%3A%2F%2Fgithub.com%2Ftorvalds%2Flinux%2Fblob%2Fmaster%2Fdrivers%2Fnet%2Fethernet%2Fwiznet%2Fw5100.c&data=05%7C02%7Cquentin.schulz%40cherry.de%7Cc46cc28f1cef4e123f8008dd912c7e86%7C5e0e1b5221b54e7b83bb514ec460677e%7C0%7C0%7C638826343382626512%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=nnWuRiS1B1wya4helTsVcJ92ZEjG81Ir%2BlSt0Kh9woI%3D&reserved=0
+ *
+ * Datasheet:
+ * - https://eur02.safelinks.protection.outlook.com/?
url=http%3A%2F%2Fwww.wiznet.co.kr%2Fwp-
content%2Fuploads%2Fwiznethome%2FChip%2FW5100%2FDocument%2FW5100_Datasheet_v1.2.6.pdf&data=05%7C02%7Cquentin.schulz%40cherry.de%7Cc46cc28f1cef4e123f8008dd912c7e86%7C5e0e1b5221b54e7b83bb514ec460677e%7C0%7C0%7C638826343382649703%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=Z57oXSn9Nmwbd5RkkxVq9BfG56JB0LgWuYavpYOJgA8%3D&reserved=0
+ * - https://eur02.safelinks.protection.outlook.com/?
url=http%3A%2F%2Fwiznethome.cafe24.com%2Fwp-
content%2Fuploads%2Fwiznethome%2FChip%2FW5200%2FDocuments%2FW5200_DS_V140E.pdf&data=05%7C02%7Cquentin.schulz%40cherry.de%7Cc46cc28f1cef4e123f8008dd912c7e86%7C5e0e1b5221b54e7b83bb514ec460677e%7C0%7C0%7C638826343382668572%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=uDhG%2BbKQyck51qSdZCMvFbXJCDcj4I40Q18Ogl2oBnw%3D&reserved=0
+ * - https://eur02.safelinks.protection.outlook.com/?
url=http%3A%2F%2Fwizwiki.net%2Fwiki%2Flib%2Fexe%2Ffetch.php%3Fmedia%3Dproducts%3Aw5500%3Aw5500_ds_v106e_141230.pdf&data=05%7C02%7Cquentin.schulz%40cherry.de%7Cc46cc28f1cef4e123f8008dd912c7e86%7C5e0e1b5221b54e7b83bb514ec460677e%7C0%7C0%7C638826343382685514%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=NkSjVHnMa4kAWiB21NDTTF%2F2ODvFbUS33zwz5%2FK69Jw%3D&reserved=0
+ *
+ */
+
+#include <dm.h>
+#include <log.h>
+#include <malloc.h>
+#include <spi.h>
+#include <net.h>
+#include <asm/global_data.h>
+#include <dm/device_compat.h>
+#include <linux/delay.h>
+#include <linux/iopoll.h>
+#include <net.h>
+#include <malloc.h>
+
+#define BUFFER_SZ 16384
+#define MAX_PACKET_SZ 9000
+#define W5100_SPI_WRITE_OPCODE 0xf0
+#define W5100_SPI_READ_OPCODE 0x0f
+#define W5100_SHAR 0x0009 /* Source MAC address */
+#define W5500_S0_REGS 0x10000
+#define S0_REGS(priv) ((priv)->s0_regs)
+#define W5100_MR 0x0000 /* Mode Register */
+#define MR_RST 0x80 /* S/W reset */
+#define MR_PB 0x10 /* Ping block */
+
+#define W5100_Sn_MR 0x0000 /* Sn Mode Register */
+#define W5100_Sn_CR 0x0001 /* Sn Command Register */
+#define W5100_Sn_IR 0x0002 /* Sn Interrupt Register */
+#define W5100_Sn_SR 0x0003 /* Sn Status Register */
+#define W5100_Sn_TX_FSR 0x0020 /* Sn Transmit free memory
size */
+#define W5100_Sn_TX_RD 0x0022 /* Sn Transmit memory read
pointer */
+#define W5100_Sn_TX_WR 0x0024 /* Sn Transmit memory write
pointer */
+#define W5100_Sn_RX_RSR 0x0026 /* Sn Receive free memory
size */
+#define W5100_Sn_RX_RD 0x0028 /* Sn Receive memory read
pointer */
+
+#define W5100_S0_MR(priv) (S0_REGS(priv) + W5100_Sn_MR)
+
+#define S0_MR_MACRAW 0x04 /* MAC RAW mode */
+#define S0_MR_MF 0x40 /* MAC Filter for W5100 and
W5200 */
+#define W5500_S0_MR_MF 0x80 /* MAC Filter for W5500 */
+#define W5100_S0_MR(priv) (S0_REGS(priv) + W5100_Sn_MR)
+
+#define S0_MR_MACRAW 0x04 /* MAC RAW mode */
+#define S0_MR_MF 0x40 /* MAC Filter for W5100 and
W5200 */
+#define W5500_S0_MR_MF 0x80 /* MAC Filter for W5500 */
+
+/*
+ * W5100 and W5200 common registers about the same with the W5500
+ */
+#define W5100_IMR 0x0016 /* Interrupt Mask Register */
+#define IR_S0 0x01 /* S0 interrupt */
+#define W5100_RTR 0x0017 /* Retry Time-value
Register */
+#define RTR_DEFAULT 2000 /* =0x07d0 (2000) */
+#define W5500_SIMR 0x0018 /* Socket Interrupt Mask
Register */
+#define W5500_RTR 0x0019 /* Retry Time-value
Register */
+
+#define W5100_S0_CR(priv) (S0_REGS(priv) + W5100_Sn_CR)
+#define S0_CR_OPEN 0x01 /* OPEN command */
+#define S0_CR_CLOSE 0x10 /* CLOSE command */
+#define S0_CR_SEND 0x20 /* SEND command */
+#define S0_CR_RECV 0x40 /* RECV command */
+#define W5100_S0_IR(priv) (S0_REGS(priv) + W5100_Sn_IR)
+#define S0_IR_SENDOK 0x10 /* complete sending */
+#define S0_IR_RECV 0x04 /* receiving data */
+#define W5100_S0_SR(priv) (S0_REGS(priv) + W5100_Sn_SR)
+#define S0_SR_MACRAW 0x42 /* mac raw mode */
+#define W5100_S0_TX_FSR(priv) (S0_REGS(priv) + W5100_Sn_TX_FSR)
+#define W5100_S0_TX_RD(priv) (S0_REGS(priv) + W5100_Sn_TX_RD)
+#define W5100_S0_TX_WR(priv) (S0_REGS(priv) + W5100_Sn_TX_WR)
+#define W5100_S0_RX_RSR(priv) (S0_REGS(priv) + W5100_Sn_RX_RSR)
+#define W5100_S0_RX_RD(priv) (S0_REGS(priv) + W5100_Sn_RX_RD)
+
+#define W5500_TX_MEM_START 0x20000
+#define W5500_TX_MEM_SIZE 0x04000
+#define W5500_RX_MEM_START 0x30000
+#define W5500_RX_MEM_SIZE 0x04000
+
+#define W5500_Sn_RXMEM_SIZE(n) \
+ (0x1001e + (n) * 0x40000) /* Sn RX Memory Size */
+#define W5500_Sn_TXMEM_SIZE(n) \
+ (0x1001f + (n) * 0x40000) /* Sn TX Memory Size */
+
+/**
+ * struct eth_w5500_priv - memory for w5500 driver
+ */
+struct eth_w5500_priv {
+ uchar host_hwaddr[ARP_HLEN];
+ bool disabled;
+ uchar * recv_packet_buffer[PKTBUFSRX];
+ int recv_packet_length[PKTBUFSRX];
+ int recv_packets;
+ const struct dm_spi_ops *spi_ops;
+ struct udevice **spi_dev;
+ u32 s0_regs;
+ u32 s0_tx_buf;
+ u16 s0_tx_buf_size;
+ u32 s0_rx_buf;
+ u16 s0_rx_buf_size;
+ bool promisc;
+ u32 msg_enable;
+ u16 offset;
+ void *priv;
+};
+
+static int xfer(struct udevice *dev, void *dout, unsigned int bout,
void *din,
+ unsigned int bin)
+{
+ /* Ok the xfer function in uboot is symmetrical
+ * (write and read operation happens at the same time)
+ * w5500 requires bytes send followed by receive operation on the
MISO line
+ * So we need to create a buffer which contain bout+bin bytes and
pass the various
+ * pointer to the xfer function
+ */
+
+ struct udevice *bus = dev_get_parent(dev);
+ u8 buffin[BUFFER_SZ];
+ u8 buffout[BUFFER_SZ];
It's dangerous to allocate 2 16k buffers on the stack, move them global
or allocate them at probe and store the pointers in eth_w5500_priv
+
+ if ((bout + bin) < BUFFER_SZ) {
+ for (int i = 0; i < bout; i++) {
+ buffout[i] = ((u8 *)(dout))[i];
+ buffin[i] = 0;
+ }
+ for (int i = bout; i < (bin + bout); i++) {
+ buffin[i] = 0;
+ buffout[i] = 0;
+ }
+ if (!bus)
+ return -1;
+ dm_spi_xfer(dev, 8 * (bout + bin), buffout, buffin,
+ SPI_XFER_BEGIN | SPI_XFER_END);
+ for (int i = bout; i < (bin + bout); i++)
+ ((u8 *)(din))[bin + bout - i - 1] = buffin[i];
+ }
+
+ return 0;
+}
+
+static int w5500_spi_read(struct udevice *dev, u32 addr)
+{
+ u8 cmd[3];
+ int ret;
+ u8 bank;
+ u8 data;
+
+ bank = (addr >> 16);
+
+ if (bank > 0)
+ bank = bank << 3;
+
+ cmd[0] = addr >> 8;
+ cmd[1] = addr & 0xff;
+ cmd[2] = bank;
+
+ ret = xfer(dev, cmd, sizeof(cmd), &data, 1);
+ if (ret != 0)
if (ret)
+ return ret;
+
+ return data;
This is dangerous too. How do I know if the returned data is an error
code or actual data?
Right now, xfer returns 0 if successful, -1 otherwise. I would suggest
return ret only if it is < 0.
Or even better, propagate the error code and return data through a
pointer as argument of the function?
e.g.
static int w5500_spi_read(struct udevice *dev, u32 addr, u8 *data)
{
[...]
return xfer(dev, cmd, sizeof(cmd), &data, 1);
}
+}
+
+static int w5500_spi_write(struct udevice *dev, u32 addr, u8 data)
+{
+ u8 cmd[4];
+ u8 bank;
+ u8 din;
+
+ bank = (addr >> 16);
+ din = 0;
+
+ if (bank > 0)
+ bank = bank << 3;
+
+ cmd[0] = (addr >> 8) & 0xff;
+ cmd[1] = addr & 0xff;
+ cmd[2] = bank | 0x4;
+ cmd[3] = data;
+
+ return xfer(dev, cmd, sizeof(cmd), &din, 0);
+}
+
+static int w5500_spi_read16(struct udevice *dev, u32 addr)
+{
+ u8 cmd[3];
+ u16 data;
+ int ret;
+ u8 bank;
+
+ bank = (addr >> 16);
+
+ if (bank > 0)
+ bank = bank << 3;
+
+ cmd[0] = addr >> 8;
+ cmd[1] = addr & 0xff;
+ cmd[2] = bank;
+
+ ret = xfer(dev, cmd, sizeof(cmd), &data, 2);
+ if (ret != 0)
if (ret)
+ return ret;
+
+ return data;
+}
+
Ditto.
Considering the only difference between w5500_spi_read16 and
w5500_spi_read is the 2 or 1 as last argument of xfer() call, you
probably want to factor them out into a common function to limit
duplicated code.
If you have access to the info, I would suggest to NOT use a u8[3] array
but create a small struct which specifies what each u8 is for so it's
easier to read and debug later on. This applies to all other functions.
+static int w5500_writebulk(struct udevice *dev, u32 addr, const u8 *buf,
+ int len)
+{
+ int i;
+ u8 bank;
+ u8 cmd[9000];
You really need to justify that big of an array, why do you need that?
Can't we malloc it based on len?
+ u8 din = 0;
+
+ bank = (addr >> 16);
+ if (bank > 0)
+ bank = bank << 3;
+
+ cmd[0] = (addr >> 8) & 0xff;
+ cmd[1] = addr & 0xff;
+ cmd[2] = bank | 0x4;
+
+ for (i = 0; i < len; i++)
+ cmd[i + 3] = buf[i];
+
+ return xfer(dev, cmd, len + 3, &din, 0);
+}
+
+static int w5500_spi_write16(struct udevice *dev, u32 addr, u16 data)
+{
+ u8 buf[2];
+
+ buf[0] = data >> 8;
+ buf[1] = data & 0xff;
+
Looks like endianness swap to me? Is this supposed to happen if you have
another endianness for the SoC than the one you're currently using to
test this?
+ return w5500_writebulk(dev, addr, &buf[0], 2);
+}
+
+static int w5500_readbulk(struct udevice *dev, u32 addr, u8 *buf, int
len)
+{
+ u8 cmd[3];
+ u8 *data;
+ int i;
+ int ret;
+ u8 bank;
+
+ data = (u8 *)malloc(MAX_PACKET_SZ);
Why not malloc(len) ?
+
+ if (!data)
+ return -1;
-ENOMEM?
+
+ bank = (addr >> 16);
+
+ if (bank > 0)
+ bank = bank << 3;
+
+ cmd[0] = addr >> 8;
+ cmd[1] = addr & 0xff;
+ cmd[2] = bank;
+
+ ret = xfer(dev, cmd, sizeof(cmd), &data, len);
+
+ if (ret != 0) {
if (ret) {
+ free(data);
+ return ret;
+ }
+
+ for (i = 0; i < len; i++)
+ buf[(len - 1) - i] = data[i];
+ free(data);
+ return 0;
or add:
out:
free(data)
return ret;
and replace with:
if (ret)
goto out;
Agree with Neil here, go for the typical goto solution for unwinding
code in the error path(s).
+}
+
+static int w5500_readbuf(struct udevice *dev, u16 offset, u8 *buf,
int len)
+{
+ struct eth_w5500_priv *priv = dev_get_priv(dev);
+ u32 addr;
+ const u32 mem_start = priv->s0_rx_buf;
+ int remain = 0;
+ int ret;
+ const u16 mem_size = priv->s0_rx_buf_size;
Order by line length
+
+ offset %= mem_size;
+ addr = mem_start + offset;
+
+ if (offset + len > mem_size) {
+ remain = (offset + len) % mem_size;
+ len = mem_size - offset;
+ }
+
+ ret = w5500_readbulk(dev, addr, buf, len);
+ if (ret || !remain)
+ return ret;
+
+ return w5500_readbulk(dev, mem_start, buf + len, remain);
+}
+
+static int w5500_writebuf(struct udevice *dev, u16 offset, const u8
*buf,
+ int len)
+{
+ struct eth_w5500_priv *priv = dev_get_priv(dev);
+ u32 addr;
+ const u32 mem_start = priv->s0_tx_buf;
+ int ret;
+ int remain = 0;
+ const u16 mem_size = priv->s0_tx_buf_size;
Ditto
+
+ offset %= mem_size;
+ addr = mem_start + offset;
+
+ if (offset + len > mem_size) {
+ remain = (offset + len) % mem_size;
+ len = mem_size - offset;
+ }
+
+ ret = w5500_writebulk(dev, addr, buf, len);
+ if (ret || !remain)
+ return ret;
+
+ return w5500_writebulk(dev, mem_start, buf + len, remain);
+}
+
+static int w5500_command(struct udevice *dev, u8 cmd)
+{
+ struct eth_w5500_priv *priv = dev_get_priv(dev);
+ u8 val;
+ u16 check;
+
+ w5500_spi_write(dev, W5100_S0_CR(priv), cmd);
+ check = read_poll_timeout(w5500_spi_read, val, val != 0,
+ 10, 5000, dev, W5100_S0_CR(priv));
+ if (check != 0)
if (check)
+ return -EIO;
return ret
and use ret instead of check for the variable name.
+
+ return 0;
+}
+
+static int w5500_eth_start(struct udevice *dev)
+{
+ struct eth_w5500_priv *priv = dev_get_priv(dev);
+
+ debug("eth_w5500: Start\n");
Please use log_debug()/dev_dbg instead of debug().
Cheers,
Quentin
