[U-Boot] [PATCH 0/3] Add Ethernet support for Poplar board
The series adds Ethernet support for Poplar board. It firstly creates a reset driver for HiSilicon platform, then introduces higmacv300 Ethernet driver, and finally enables Ethernet support for Poplar board.
Shawn Guo (3): reset: add reset driver for HiSilicon platform net: add higmacv300 Ethernet driver for HiSilicon platform poplar: enable Ethernet driver support
arch/arm/dts/hi3798cv200-u-boot.dtsi | 6 + configs/poplar_defconfig | 3 + drivers/net/Kconfig | 9 + drivers/net/Makefile | 1 + drivers/net/higmacv300.c | 592 +++++++++++++++++++++++++++ drivers/reset/Kconfig | 6 + drivers/reset/Makefile | 1 + drivers/reset/reset-hisilicon.c | 95 +++++ 8 files changed, 713 insertions(+) create mode 100644 drivers/net/higmacv300.c create mode 100644 drivers/reset/reset-hisilicon.c
It adds a Driver Model compatible reset driver for HiSlicon platform. The driver implements a custom .of_xlate function, and uses .data field as reset register offset and .id field as bit shift.
Signed-off-by: Shawn Guo shawn.guo@linaro.org --- drivers/reset/Kconfig | 6 +++ drivers/reset/Makefile | 1 + drivers/reset/reset-hisilicon.c | 95 +++++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+) create mode 100644 drivers/reset/reset-hisilicon.c
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig index a81e76769604..6ec6f39c85f0 100644 --- a/drivers/reset/Kconfig +++ b/drivers/reset/Kconfig @@ -121,4 +121,10 @@ config RESET_SUNXI This enables support for common reset driver for Allwinner SoCs.
+config RESET_HISILICON + bool "Reset controller driver for HiSilicon SoCs" + depends on DM_RESET + help + Support for reset controller on HiSilicon SoCs. + endmenu diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile index 4fad7d412985..7fec75bb4923 100644 --- a/drivers/reset/Makefile +++ b/drivers/reset/Makefile @@ -19,3 +19,4 @@ obj-$(CONFIG_RESET_MESON) += reset-meson.o obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o obj-$(CONFIG_RESET_MEDIATEK) += reset-mediatek.o obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o +obj-$(CONFIG_RESET_HISILICON) += reset-hisilicon.o diff --git a/drivers/reset/reset-hisilicon.c b/drivers/reset/reset-hisilicon.c new file mode 100644 index 000000000000..3a06110f76f3 --- /dev/null +++ b/drivers/reset/reset-hisilicon.c @@ -0,0 +1,95 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2019, Linaro Limited + */ + +#include <asm/io.h> +#include <common.h> +#include <dm.h> +#include <reset-uclass.h> + +struct hisi_reset_priv { + void __iomem *base; +}; + +static int hisi_reset_deassert(struct reset_ctl *rst) +{ + struct hisi_reset_priv *priv = dev_get_priv(rst->dev); + u32 val; + + val = readl(priv->base + rst->data); + val &= ~BIT(rst->id); + writel(val, priv->base + rst->data); + + return 0; +} + +static int hisi_reset_assert(struct reset_ctl *rst) +{ + struct hisi_reset_priv *priv = dev_get_priv(rst->dev); + u32 val; + + val = readl(priv->base + rst->data); + val |= BIT(rst->id); + writel(val, priv->base + rst->data); + + return 0; +} + +static int hisi_reset_free(struct reset_ctl *rst) +{ + return 0; +} + +static int hisi_reset_request(struct reset_ctl *rst) +{ + return 0; +} + +static int hisi_reset_of_xlate(struct reset_ctl *rst, + struct ofnode_phandle_args *args) +{ + if (args->args_count != 2) { + debug("Invalid args_count: %d\n", args->args_count); + return -EINVAL; + } + + /* We use .data field as register offset and .id field as bit shift. */ + rst->data = args->args[0]; + rst->id = args->args[1]; + + return 0; +} + +static const struct reset_ops hisi_reset_reset_ops = { + .of_xlate = hisi_reset_of_xlate, + .request = hisi_reset_request, + .free = hisi_reset_free, + .rst_assert = hisi_reset_assert, + .rst_deassert = hisi_reset_deassert, +}; + +static const struct udevice_id hisi_reset_ids[] = { + { .compatible = "hisilicon,hi3798cv200-crg" }, + { } +}; + +static int hisi_reset_probe(struct udevice *dev) +{ + struct hisi_reset_priv *priv = dev_get_priv(dev); + + priv->base = dev_remap_addr(dev); + if (!priv->base) + return -ENOMEM; + + return 0; +} + +U_BOOT_DRIVER(hisi_reset) = { + .name = "hisilicon_reset", + .id = UCLASS_RESET, + .of_match = hisi_reset_ids, + .ops = &hisi_reset_reset_ops, + .probe = hisi_reset_probe, + .priv_auto_alloc_size = sizeof(struct hisi_reset_priv), +};
Reviewed-by: Igor Opaniuk igor.opaniuk@linaro.org
On Mon, 28 Jan 2019 at 11:15, Shawn Guo shawn.guo@linaro.org wrote:
It adds a Driver Model compatible reset driver for HiSlicon platform. The driver implements a custom .of_xlate function, and uses .data field as reset register offset and .id field as bit shift.
Signed-off-by: Shawn Guo shawn.guo@linaro.org
drivers/reset/Kconfig | 6 +++ drivers/reset/Makefile | 1 + drivers/reset/reset-hisilicon.c | 95 +++++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+) create mode 100644 drivers/reset/reset-hisilicon.c
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig index a81e76769604..6ec6f39c85f0 100644 --- a/drivers/reset/Kconfig +++ b/drivers/reset/Kconfig @@ -121,4 +121,10 @@ config RESET_SUNXI This enables support for common reset driver for Allwinner SoCs.
+config RESET_HISILICON
bool "Reset controller driver for HiSilicon SoCs"depends on DM_RESEThelpSupport for reset controller on HiSilicon SoCs.endmenu diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile index 4fad7d412985..7fec75bb4923 100644 --- a/drivers/reset/Makefile +++ b/drivers/reset/Makefile @@ -19,3 +19,4 @@ obj-$(CONFIG_RESET_MESON) += reset-meson.o obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o obj-$(CONFIG_RESET_MEDIATEK) += reset-mediatek.o obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o +obj-$(CONFIG_RESET_HISILICON) += reset-hisilicon.o diff --git a/drivers/reset/reset-hisilicon.c b/drivers/reset/reset-hisilicon.c new file mode 100644 index 000000000000..3a06110f76f3 --- /dev/null +++ b/drivers/reset/reset-hisilicon.c @@ -0,0 +1,95 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- Copyright (c) 2019, Linaro Limited
- */
+#include <asm/io.h> +#include <common.h> +#include <dm.h> +#include <reset-uclass.h>
+struct hisi_reset_priv {
void __iomem *base;+};
+static int hisi_reset_deassert(struct reset_ctl *rst) +{
struct hisi_reset_priv *priv = dev_get_priv(rst->dev);u32 val;val = readl(priv->base + rst->data);val &= ~BIT(rst->id);writel(val, priv->base + rst->data);return 0;+}
+static int hisi_reset_assert(struct reset_ctl *rst) +{
struct hisi_reset_priv *priv = dev_get_priv(rst->dev);u32 val;val = readl(priv->base + rst->data);val |= BIT(rst->id);writel(val, priv->base + rst->data);return 0;+}
+static int hisi_reset_free(struct reset_ctl *rst) +{
return 0;+}
+static int hisi_reset_request(struct reset_ctl *rst) +{
return 0;+}
+static int hisi_reset_of_xlate(struct reset_ctl *rst,
struct ofnode_phandle_args *args)+{
if (args->args_count != 2) {debug("Invalid args_count: %d\n", args->args_count);return -EINVAL;}/* We use .data field as register offset and .id field as bit shift. */rst->data = args->args[0];rst->id = args->args[1];return 0;+}
+static const struct reset_ops hisi_reset_reset_ops = {
.of_xlate = hisi_reset_of_xlate,.request = hisi_reset_request,.free = hisi_reset_free,.rst_assert = hisi_reset_assert,.rst_deassert = hisi_reset_deassert,+};
+static const struct udevice_id hisi_reset_ids[] = {
{ .compatible = "hisilicon,hi3798cv200-crg" },{ }+};
+static int hisi_reset_probe(struct udevice *dev) +{
struct hisi_reset_priv *priv = dev_get_priv(dev);priv->base = dev_remap_addr(dev);if (!priv->base)return -ENOMEM;return 0;+}
+U_BOOT_DRIVER(hisi_reset) = {
.name = "hisilicon_reset",.id = UCLASS_RESET,.of_match = hisi_reset_ids,.ops = &hisi_reset_reset_ops,.probe = hisi_reset_probe,.priv_auto_alloc_size = sizeof(struct hisi_reset_priv),+};
2.18.0
It adds the driver for HIGMACV300 Ethernet controller found on HiSilicon SoCs like Hi3798CV200. It's based on a downstream U-Boot driver, but quite a lot of code gets rewritten and cleaned up to adopt driver model and PHY API.
Signed-off-by: Shawn Guo shawn.guo@linaro.org --- drivers/net/Kconfig | 9 + drivers/net/Makefile | 1 + drivers/net/higmacv300.c | 592 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 602 insertions(+) create mode 100644 drivers/net/higmacv300.c
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 39ce4e8a1fc6..e07a382c28c7 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -530,4 +530,13 @@ config MEDIATEK_ETH This Driver support MediaTek Ethernet GMAC Say Y to enable support for the MediaTek Ethernet GMAC.
+config NET_HIGMACV300 + bool "HiSilicon Gigabit Ethernet Controller" + depends on DM_ETH + select DM_RESET + select PHYLIB + help + This driver supports HIGMACV300 Ethernet controller found on + HiSilicon SoCs. + endif # NETDEVICES diff --git a/drivers/net/Makefile b/drivers/net/Makefile index e38c16464478..0951cb17e6ba 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -76,3 +76,4 @@ obj-$(CONFIG_SNI_AVE) += sni_ave.o obj-y += ti/ obj-$(CONFIG_MEDIATEK_ETH) += mtk_eth.o obj-$(CONFIG_MSCC_OCELOT_SWITCH) += ocelot_switch.o +obj-$(CONFIG_NET_HIGMACV300) += higmacv300.o diff --git a/drivers/net/higmacv300.c b/drivers/net/higmacv300.c new file mode 100644 index 000000000000..6cac5cec73df --- /dev/null +++ b/drivers/net/higmacv300.c @@ -0,0 +1,592 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2019, Linaro Limited + */ + +#include <asm/io.h> +#include <common.h> +#include <console.h> +#include <linux/bug.h> +#include <linux/mii.h> +#include <miiphy.h> +#include <net.h> +#include <reset.h> + +#define STATION_ADDR_LOW 0x0000 +#define STATION_ADDR_HIGH 0x0004 +#define MAC_DUPLEX_HALF_CTRL 0x0008 +#define PORT_MODE 0x0040 +#define PORT_EN 0x0044 +#define BIT_TX_EN BIT(2) +#define BIT_RX_EN BIT(1) +#define MODE_CHANGE_EN 0x01b4 +#define BIT_MODE_CHANGE_EN BIT(0) +#define MDIO_SINGLE_CMD 0x03c0 +#define BIT_MDIO_BUSY BIT(20) +#define MDIO_SINGLE_DATA 0x03c4 +#define MDIO_RDATA_STATUS 0x03d0 +#define BIT_MDIO_RDATA_INVALID BIT(0) +#define RX_FQ_START_ADDR 0x0500 +#define RX_FQ_DEPTH 0x0504 +#define RX_FQ_WR_ADDR 0x0508 +#define RX_FQ_RD_ADDR 0x050c +#define RX_FQ_REG_EN 0x0518 +#define RX_BQ_START_ADDR 0x0520 +#define RX_BQ_DEPTH 0x0524 +#define RX_BQ_WR_ADDR 0x0528 +#define RX_BQ_RD_ADDR 0x052c +#define RX_BQ_REG_EN 0x0538 +#define TX_BQ_START_ADDR 0x0580 +#define TX_BQ_DEPTH 0x0584 +#define TX_BQ_WR_ADDR 0x0588 +#define TX_BQ_RD_ADDR 0x058c +#define TX_BQ_REG_EN 0x0598 +#define TX_RQ_START_ADDR 0x05a0 +#define TX_RQ_DEPTH 0x05a4 +#define TX_RQ_WR_ADDR 0x05a8 +#define TX_RQ_RD_ADDR 0x05ac +#define TX_RQ_REG_EN 0x05b8 +#define BIT_START_ADDR_EN BIT(2) +#define BIT_DEPTH_EN BIT(1) +#define DESC_WR_RD_ENA 0x05cc + +/* MACIF_CTRL */ +#define RGMII_SPEED_1000 0x2c +#define RGMII_SPEED_100 0x2f +#define RGMII_SPEED_10 0x2d +#define MII_SPEED_100 0x0f +#define MII_SPEED_10 0x0d +#define GMAC_SPEED_1000 0x05 +#define GMAC_SPEED_100 0x01 +#define GMAC_SPEED_10 0x00 +#define GMAC_FULL_DUPLEX BIT(4) + +#define RX_DESC_NUM 64 +#define TX_DESC_NUM 2 +#define DESC_SIZE 32 +#define DESC_WORD_SHIFT 3 +#define DESC_BYTE_SHIFT 5 +#define DESC_CNT(n) ((n) >> DESC_BYTE_SHIFT) +#define DESC_BYTE(n) ((n) << DESC_BYTE_SHIFT) +#define DESC_VLD_FREE 0 +#define DESC_VLD_BUSY 1 + +#define MAC_MAX_FRAME_SIZE 1600 + +#define MDIO_WRITE 1 +#define MDIO_READ 2 +#define MDIO_COMMAND(rw, addr, reg) (BIT_MDIO_BUSY | \ + (((rw) & 0x3) << 16) | \ + (((addr) & 0x1f) << 8) | \ + ((reg) & 0x1f)) +#define MDIO_WRITE_CMD(addr, reg) MDIO_COMMAND(MDIO_WRITE, addr, reg) +#define MDIO_READ_CMD(addr, reg) MDIO_COMMAND(MDIO_READ, addr, reg) + +enum higmac_queue { + RX_FQ, + RX_BQ, + TX_BQ, + TX_RQ, +}; + +struct higmac_desc { + unsigned int buf_addr; + unsigned int buf_len:11; + unsigned int reserve0:5; + unsigned int data_len:11; + unsigned int reserve1:2; + unsigned int fl:2; + unsigned int descvid:1; + unsigned int reserve2[6]; +}; + +struct higmac_priv { + void __iomem *base; + void __iomem *macif_ctrl; + struct reset_ctl rst_phy; + struct higmac_desc *rxfq; + struct higmac_desc *rxbq; + struct higmac_desc *txbq; + struct higmac_desc *txrq; + struct mii_dev *bus; + struct phy_device *phydev; + int phyintf; + int phyaddr; +}; + +#define flush_desc(d) flush_cache((unsigned long)(d), sizeof(*(d))) +#define invalidate_desc(d) \ + invalidate_dcache_range((unsigned long)(d), \ + (unsigned long)(d) + sizeof(*(d))) + +static int higmac_write_hwaddr(struct udevice *dev) +{ + struct eth_pdata *pdata = dev_get_platdata(dev); + struct higmac_priv *priv = dev_get_priv(dev); + unsigned char *mac = pdata->enetaddr; + u32 val; + + val = mac[1] | (mac[0] << 8); + writel(val, priv->base + STATION_ADDR_HIGH); + + val = mac[5] | (mac[4] << 8) | (mac[3] << 16) | (mac[2] << 24); + writel(val, priv->base + STATION_ADDR_LOW); + + return 0; +} + +static int higmac_free_pkt(struct udevice *dev, uchar *packet, int length) +{ + if (packet) + free(packet); + + return 0; +} + +static int higmac_recv(struct udevice *dev, int flags, uchar **packetp) +{ + struct higmac_priv *priv = dev_get_priv(dev); + struct higmac_desc *fqd = priv->rxfq; + struct higmac_desc *bqd = priv->rxbq; + int fqw_pos, fqr_pos, bqw_pos, bqr_pos; + int timeout = 100000; + unsigned long addr; + int len = 0; + int space; + int i; + + fqw_pos = DESC_CNT(readl(priv->base + RX_FQ_WR_ADDR)); + fqr_pos = DESC_CNT(readl(priv->base + RX_FQ_RD_ADDR)); + + if (fqw_pos >= fqr_pos) + space = RX_DESC_NUM - (fqw_pos - fqr_pos); + else + space = fqr_pos - fqw_pos; + + /* Leave one free to distinguish full filled from empty buffer */ + for (i = 0; i < space - 1; i++) { + void *buf = memalign(ARCH_DMA_MINALIGN, MAC_MAX_FRAME_SIZE); + + if (!buf) + break; + + fqd = priv->rxfq + fqw_pos; + fqd->buf_addr = (unsigned long)buf; + invalidate_dcache_range(fqd->buf_addr, + fqd->buf_addr + MAC_MAX_FRAME_SIZE); + + fqd->descvid = DESC_VLD_FREE; + fqd->buf_len = MAC_MAX_FRAME_SIZE - 1; + flush_desc(fqd); + + if (++fqw_pos >= RX_DESC_NUM) + fqw_pos = 0; + + writel(DESC_BYTE(fqw_pos), priv->base + RX_FQ_WR_ADDR); + } + + bqr_pos = DESC_CNT(readl(priv->base + RX_BQ_RD_ADDR)); + bqd += bqr_pos; + /* BQ is only ever written by GMAC */ + invalidate_desc(bqd); + + do { + bqw_pos = DESC_CNT(readl(priv->base + RX_BQ_WR_ADDR)); + udelay(1); + } while (--timeout && bqw_pos == bqr_pos); + + if (!timeout) + return -ETIMEDOUT; + + if (++bqr_pos >= RX_DESC_NUM) + bqr_pos = 0; + + len = bqd->data_len; + + /* CPU should not have touched this buffer since we added it to FQ */ + addr = bqd->buf_addr; + invalidate_dcache_range(addr, addr + len); + *packetp = (void *)addr; + + writel(DESC_BYTE(bqr_pos), priv->base + RX_BQ_RD_ADDR); + + return len; +} + +static int higmac_send(struct udevice *dev, void *packet, int length) +{ + struct higmac_priv *priv = dev_get_priv(dev); + struct higmac_desc *bqd = priv->txbq; + int bqw_pos, rqw_pos, rqr_pos; + int timeout = 1000; + + flush_cache((unsigned long)packet, length); + + bqw_pos = DESC_CNT(readl(priv->base + TX_BQ_WR_ADDR)); + bqd += bqw_pos; + bqd->buf_addr = (unsigned long)packet; + bqd->descvid = DESC_VLD_BUSY; + bqd->data_len = length; + flush_desc(bqd); + + if (++bqw_pos >= TX_DESC_NUM) + bqw_pos = 0; + + writel(DESC_BYTE(bqw_pos), priv->base + TX_BQ_WR_ADDR); + + rqr_pos = DESC_CNT(readl(priv->base + TX_RQ_RD_ADDR)); + if (++rqr_pos >= TX_DESC_NUM) + rqr_pos = 0; + + do { + rqw_pos = DESC_CNT(readl(priv->base + TX_RQ_WR_ADDR)); + udelay(1); + } while (--timeout && rqr_pos != rqw_pos); + + if (!timeout) + return -ETIMEDOUT; + + writel(DESC_BYTE(rqr_pos), priv->base + TX_RQ_RD_ADDR); + + return 0; +} + +static int higmac_adjust_link(struct higmac_priv *priv) +{ + struct phy_device *phydev = priv->phydev; + int interface = priv->phyintf; + u32 val; + + switch (interface) { + case PHY_INTERFACE_MODE_RGMII: + if (phydev->speed == SPEED_1000) + val = RGMII_SPEED_1000; + else if (phydev->speed == SPEED_100) + val = RGMII_SPEED_100; + else + val = RGMII_SPEED_10; + break; + case PHY_INTERFACE_MODE_MII: + if (phydev->speed == SPEED_100) + val = MII_SPEED_100; + else + val = MII_SPEED_10; + break; + default: + debug("unsupported mode: %d\n", interface); + return -EINVAL; + } + + if (phydev->duplex) + val |= GMAC_FULL_DUPLEX; + + writel(val, priv->macif_ctrl); + + if (phydev->speed == SPEED_1000) + val = GMAC_SPEED_1000; + else if (phydev->speed == SPEED_100) + val = GMAC_SPEED_100; + else + val = GMAC_SPEED_10; + + writel(BIT_MODE_CHANGE_EN, priv->base + MODE_CHANGE_EN); + writel(val, priv->base + PORT_MODE); + writel(0, priv->base + MODE_CHANGE_EN); + writel(phydev->duplex, priv->base + MAC_DUPLEX_HALF_CTRL); + + return 0; +} + +static int higmac_start(struct udevice *dev) +{ + struct higmac_priv *priv = dev_get_priv(dev); + struct phy_device *phydev = priv->phydev; + int ret; + + ret = phy_startup(phydev); + if (ret) + return ret; + + if (!phydev->link) { + debug("%s: link down\n", phydev->dev->name); + return -ENODEV; + } + + ret = higmac_adjust_link(priv); + if (ret) + return ret; + + /* Enable port */ + writel(0xf, priv->base + DESC_WR_RD_ENA); + writel(BIT_TX_EN | BIT_RX_EN, priv->base + PORT_EN); + + return 0; +} + +static void higmac_stop(struct udevice *dev) +{ + struct higmac_priv *priv = dev_get_priv(dev); + + /* Disable port */ + writel(0, priv->base + PORT_EN); + writel(0, priv->base + DESC_WR_RD_ENA); +} + +static const struct eth_ops higmac_ops = { + .start = higmac_start, + .send = higmac_send, + .recv = higmac_recv, + .free_pkt = higmac_free_pkt, + .stop = higmac_stop, + .write_hwaddr = higmac_write_hwaddr, +}; + +static int higmac_wait_mdio_ready(struct higmac_priv *priv) +{ + int timeout = 1000; + + while (--timeout) { + /* Wait until busy bit is cleared */ + if ((readl(priv->base + MDIO_SINGLE_CMD) & BIT_MDIO_BUSY) == 0) + break; + udelay(10); + } + + return timeout ? 0 : -ETIMEDOUT; +} + +static int higmac_mdio_read(struct mii_dev *bus, int addr, int devad, int reg) +{ + struct higmac_priv *priv = bus->priv; + int ret; + + ret = higmac_wait_mdio_ready(priv); + if (ret) + return ret; + + writel(MDIO_READ_CMD(addr, reg), priv->base + MDIO_SINGLE_CMD); + + ret = higmac_wait_mdio_ready(priv); + if (ret) + return ret; + + if (readl(priv->base + MDIO_RDATA_STATUS) & BIT_MDIO_RDATA_INVALID) + return -EINVAL; + + return readl(priv->base + MDIO_SINGLE_DATA) >> 16; +} + +static int higmac_mdio_write(struct mii_dev *bus, int addr, int devad, + int reg, u16 value) +{ + struct higmac_priv *priv = bus->priv; + int ret; + + ret = higmac_wait_mdio_ready(priv); + if (ret) + return ret; + + writel(value, priv->base + MDIO_SINGLE_DATA); + writel(MDIO_WRITE_CMD(addr, reg), priv->base + MDIO_SINGLE_CMD); + + return 0; +} + +static int higmac_init_hw_queue(struct higmac_priv *priv, + enum higmac_queue queue) +{ + struct higmac_desc *desc, **pdesc; + u32 regaddr, regen, regdep; + int depth; + int len; + + switch (queue) { + case RX_FQ: + regaddr = RX_FQ_START_ADDR; + regen = RX_FQ_REG_EN; + regdep = RX_FQ_DEPTH; + depth = RX_DESC_NUM; + pdesc = &priv->rxfq; + break; + case RX_BQ: + regaddr = RX_BQ_START_ADDR; + regen = RX_BQ_REG_EN; + regdep = RX_BQ_DEPTH; + depth = RX_DESC_NUM; + pdesc = &priv->rxbq; + break; + case TX_BQ: + regaddr = TX_BQ_START_ADDR; + regen = TX_BQ_REG_EN; + regdep = TX_BQ_DEPTH; + depth = TX_DESC_NUM; + pdesc = &priv->txbq; + break; + case TX_RQ: + regaddr = TX_RQ_START_ADDR; + regen = TX_RQ_REG_EN; + regdep = TX_RQ_DEPTH; + depth = TX_DESC_NUM; + pdesc = &priv->txrq; + break; + } + + /* Enable depth */ + writel(BIT_DEPTH_EN, priv->base + regen); + writel(depth << DESC_WORD_SHIFT, priv->base + regdep); + writel(0, priv->base + regen); + + len = depth * sizeof(*desc); + desc = memalign(ARCH_DMA_MINALIGN, len); + if (!desc) + return -ENOMEM; + memset(desc, 0, len); + flush_cache((unsigned long)desc, len); + *pdesc = desc; + + /* Enable start address */ + writel(BIT_START_ADDR_EN, priv->base + regen); + writel((unsigned long)desc, priv->base + regaddr); + writel(0, priv->base + regen); + + return 0; +} + +static int higmac_hw_init(struct higmac_priv *priv) +{ + int ret; + + /* Initialize hardware queues */ + ret = higmac_init_hw_queue(priv, RX_FQ); + if (ret) + return ret; + + ret = higmac_init_hw_queue(priv, RX_BQ); + if (ret) + goto free_rx_fq; + + ret = higmac_init_hw_queue(priv, TX_BQ); + if (ret) + goto free_rx_bq; + + ret = higmac_init_hw_queue(priv, TX_RQ); + if (ret) + goto free_tx_bq; + + /* Reset phy */ + reset_assert(&priv->rst_phy); + mdelay(10); + reset_deassert(&priv->rst_phy); + mdelay(30); + reset_assert(&priv->rst_phy); + mdelay(30); + + return 0; + +free_tx_bq: + free(priv->txbq); +free_rx_bq: + free(priv->rxbq); +free_rx_fq: + free(priv->rxfq); + return ret; +} + +static int higmac_probe(struct udevice *dev) +{ + struct higmac_priv *priv = dev_get_priv(dev); + struct phy_device *phydev; + struct mii_dev *bus; + int ret; + + ret = higmac_hw_init(priv); + if (ret) + return ret; + + bus = mdio_alloc(); + if (!bus) + return -ENOMEM; + + bus->read = higmac_mdio_read; + bus->write = higmac_mdio_write; + bus->priv = priv; + priv->bus = bus; + + ret = mdio_register_seq(bus, dev->seq); + if (ret) + return ret; + + phydev = phy_connect(bus, priv->phyaddr, dev, priv->phyintf); + if (!phydev) + return -ENODEV; + + phydev->supported &= PHY_GBIT_FEATURES; + phydev->advertising = phydev->supported; + priv->phydev = phydev; + + ret = phy_config(phydev); + if (ret) + return ret; + + return 0; +} + +static int higmac_remove(struct udevice *dev) +{ + struct higmac_priv *priv = dev_get_priv(dev); + + mdio_unregister(priv->bus); + mdio_free(priv->bus); + + return 0; +} + +static int higmac_ofdata_to_platdata(struct udevice *dev) +{ + struct higmac_priv *priv = dev_get_priv(dev); + ofnode phy_node; + const char *phy_mode; + int phyintf; + int ret; + + priv->base = dev_remap_addr_index(dev, 0); + priv->macif_ctrl = dev_remap_addr_index(dev, 1); + + phyintf = PHY_INTERFACE_MODE_NONE; + phy_mode = dev_read_string(dev, "phy-mode"); + if (phy_mode) + phyintf = phy_get_interface_by_name(phy_mode); + if (phyintf == PHY_INTERFACE_MODE_NONE) + return -ENODEV; + priv->phyintf = phyintf; + + phy_node = dev_read_subnode(dev, "phy"); + if (!ofnode_valid(phy_node)) { + debug("failed to find phy node\n"); + return -ENODEV; + } + priv->phyaddr = ofnode_read_u32_default(phy_node, "reg", 0); + + ret = reset_get_by_name(dev, "phy", &priv->rst_phy); + if (ret) + return ret; + + return 0; +} + +static const struct udevice_id higmac_ids[] = { + { .compatible = "hisilicon,hi3798cv200-gmac" }, + { } +}; + +U_BOOT_DRIVER(eth_higmac) = { + .name = "eth_higmac", + .id = UCLASS_ETH, + .of_match = higmac_ids, + .ofdata_to_platdata = higmac_ofdata_to_platdata, + .probe = higmac_probe, + .remove = higmac_remove, + .ops = &higmac_ops, + .priv_auto_alloc_size = sizeof(struct higmac_priv), + .platdata_auto_alloc_size = sizeof(struct eth_pdata), +};
Hi Shawn,
Please see inline comments (mostly minor stuff). I'll also test the driver a bit later and leave my T-b.
On Mon, 28 Jan 2019 at 11:15, Shawn Guo shawn.guo@linaro.org wrote:
It adds the driver for HIGMACV300 Ethernet controller found on HiSilicon SoCs like Hi3798CV200. It's based on a downstream U-Boot driver, but quite a lot of code gets rewritten and cleaned up to adopt driver model and PHY API.
Signed-off-by: Shawn Guo shawn.guo@linaro.org
drivers/net/Kconfig | 9 + drivers/net/Makefile | 1 + drivers/net/higmacv300.c | 592 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 602 insertions(+) create mode 100644 drivers/net/higmacv300.c
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 39ce4e8a1fc6..e07a382c28c7 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -530,4 +530,13 @@ config MEDIATEK_ETH This Driver support MediaTek Ethernet GMAC Say Y to enable support for the MediaTek Ethernet GMAC.
+config NET_HIGMACV300
bool "HiSilicon Gigabit Ethernet Controller"depends on DM_ETHselect DM_RESETselect PHYLIBhelpThis driver supports HIGMACV300 Ethernet controller found onHiSilicon SoCs.endif # NETDEVICES diff --git a/drivers/net/Makefile b/drivers/net/Makefile index e38c16464478..0951cb17e6ba 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -76,3 +76,4 @@ obj-$(CONFIG_SNI_AVE) += sni_ave.o obj-y += ti/ obj-$(CONFIG_MEDIATEK_ETH) += mtk_eth.o obj-$(CONFIG_MSCC_OCELOT_SWITCH) += ocelot_switch.o +obj-$(CONFIG_NET_HIGMACV300) += higmacv300.o
suggestion: CONFIG_HIGMACV300_NET, similar to CONFIG_MEDIATEK_ETH above?
diff --git a/drivers/net/higmacv300.c b/drivers/net/higmacv300.c new file mode 100644 index 000000000000..6cac5cec73df --- /dev/null +++ b/drivers/net/higmacv300.c @@ -0,0 +1,592 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- Copyright (c) 2019, Linaro Limited
- */
+#include <asm/io.h> +#include <common.h> +#include <console.h> +#include <linux/bug.h> +#include <linux/mii.h> +#include <miiphy.h> +#include <net.h> +#include <reset.h>
+#define STATION_ADDR_LOW 0x0000 +#define STATION_ADDR_HIGH 0x0004 +#define MAC_DUPLEX_HALF_CTRL 0x0008 +#define PORT_MODE 0x0040 +#define PORT_EN 0x0044 +#define BIT_TX_EN BIT(2) +#define BIT_RX_EN BIT(1) +#define MODE_CHANGE_EN 0x01b4 +#define BIT_MODE_CHANGE_EN BIT(0) +#define MDIO_SINGLE_CMD 0x03c0 +#define BIT_MDIO_BUSY BIT(20) +#define MDIO_SINGLE_DATA 0x03c4 +#define MDIO_RDATA_STATUS 0x03d0 +#define BIT_MDIO_RDATA_INVALID BIT(0) +#define RX_FQ_START_ADDR 0x0500 +#define RX_FQ_DEPTH 0x0504 +#define RX_FQ_WR_ADDR 0x0508 +#define RX_FQ_RD_ADDR 0x050c +#define RX_FQ_REG_EN 0x0518 +#define RX_BQ_START_ADDR 0x0520 +#define RX_BQ_DEPTH 0x0524 +#define RX_BQ_WR_ADDR 0x0528 +#define RX_BQ_RD_ADDR 0x052c +#define RX_BQ_REG_EN 0x0538 +#define TX_BQ_START_ADDR 0x0580 +#define TX_BQ_DEPTH 0x0584 +#define TX_BQ_WR_ADDR 0x0588 +#define TX_BQ_RD_ADDR 0x058c +#define TX_BQ_REG_EN 0x0598 +#define TX_RQ_START_ADDR 0x05a0 +#define TX_RQ_DEPTH 0x05a4 +#define TX_RQ_WR_ADDR 0x05a8 +#define TX_RQ_RD_ADDR 0x05ac +#define TX_RQ_REG_EN 0x05b8 +#define BIT_START_ADDR_EN BIT(2) +#define BIT_DEPTH_EN BIT(1) +#define DESC_WR_RD_ENA 0x05cc
Please group above defines into categories. Suggestion: move all of them to a separate header file.
+/* MACIF_CTRL */ +#define RGMII_SPEED_1000 0x2c +#define RGMII_SPEED_100 0x2f +#define RGMII_SPEED_10 0x2d +#define MII_SPEED_100 0x0f +#define MII_SPEED_10 0x0d +#define GMAC_SPEED_1000 0x05 +#define GMAC_SPEED_100 0x01 +#define GMAC_SPEED_10 0x00 +#define GMAC_FULL_DUPLEX BIT(4)
+#define RX_DESC_NUM 64 +#define TX_DESC_NUM 2 +#define DESC_SIZE 32 +#define DESC_WORD_SHIFT 3 +#define DESC_BYTE_SHIFT 5 +#define DESC_CNT(n) ((n) >> DESC_BYTE_SHIFT) +#define DESC_BYTE(n) ((n) << DESC_BYTE_SHIFT) +#define DESC_VLD_FREE 0 +#define DESC_VLD_BUSY 1
+#define MAC_MAX_FRAME_SIZE 1600
+#define MDIO_WRITE 1 +#define MDIO_READ 2 +#define MDIO_COMMAND(rw, addr, reg) (BIT_MDIO_BUSY | \
(((rw) & 0x3) << 16) | \(((addr) & 0x1f) << 8) | \((reg) & 0x1f))+#define MDIO_WRITE_CMD(addr, reg) MDIO_COMMAND(MDIO_WRITE, addr, reg) +#define MDIO_READ_CMD(addr, reg) MDIO_COMMAND(MDIO_READ, addr, reg)
+enum higmac_queue {
RX_FQ,RX_BQ,TX_BQ,TX_RQ,+};
+struct higmac_desc {
unsigned int buf_addr;
uintptr_t?
unsigned int buf_len:11;unsigned int reserve0:5;unsigned int data_len:11;unsigned int reserve1:2;unsigned int fl:2;unsigned int descvid:1;unsigned int reserve2[6];+};
+struct higmac_priv {
void __iomem *base;void __iomem *macif_ctrl;struct reset_ctl rst_phy;struct higmac_desc *rxfq;struct higmac_desc *rxbq;struct higmac_desc *txbq;struct higmac_desc *txrq;struct mii_dev *bus;struct phy_device *phydev;int phyintf;int phyaddr;+};
+#define flush_desc(d) flush_cache((unsigned long)(d), sizeof(*(d))) +#define invalidate_desc(d) \
invalidate_dcache_range((unsigned long)(d), \(unsigned long)(d) + sizeof(*(d)))+static int higmac_write_hwaddr(struct udevice *dev) +{
struct eth_pdata *pdata = dev_get_platdata(dev);struct higmac_priv *priv = dev_get_priv(dev);unsigned char *mac = pdata->enetaddr;u32 val;val = mac[1] | (mac[0] << 8);writel(val, priv->base + STATION_ADDR_HIGH);val = mac[5] | (mac[4] << 8) | (mac[3] << 16) | (mac[2] << 24);writel(val, priv->base + STATION_ADDR_LOW);return 0;+}
+static int higmac_free_pkt(struct udevice *dev, uchar *packet, int length)
__always_unused for length and dev
+{
if (packet)free(packet);return 0;+}
+static int higmac_recv(struct udevice *dev, int flags, uchar **packetp)
__always_unused for flags
+{
struct higmac_priv *priv = dev_get_priv(dev);struct higmac_desc *fqd = priv->rxfq;struct higmac_desc *bqd = priv->rxbq;int fqw_pos, fqr_pos, bqw_pos, bqr_pos;int timeout = 100000;unsigned long addr;int len = 0;int space;int i;fqw_pos = DESC_CNT(readl(priv->base + RX_FQ_WR_ADDR));fqr_pos = DESC_CNT(readl(priv->base + RX_FQ_RD_ADDR));if (fqw_pos >= fqr_pos)space = RX_DESC_NUM - (fqw_pos - fqr_pos);elsespace = fqr_pos - fqw_pos;/* Leave one free to distinguish full filled from empty buffer */for (i = 0; i < space - 1; i++) {void *buf = memalign(ARCH_DMA_MINALIGN, MAC_MAX_FRAME_SIZE);if (!buf)break;fqd = priv->rxfq + fqw_pos;fqd->buf_addr = (unsigned long)buf;
buf_addr is unsigned int, possible value overflow.
invalidate_dcache_range(fqd->buf_addr,fqd->buf_addr + MAC_MAX_FRAME_SIZE);fqd->descvid = DESC_VLD_FREE;fqd->buf_len = MAC_MAX_FRAME_SIZE - 1;flush_desc(fqd);if (++fqw_pos >= RX_DESC_NUM)fqw_pos = 0;writel(DESC_BYTE(fqw_pos), priv->base + RX_FQ_WR_ADDR);}bqr_pos = DESC_CNT(readl(priv->base + RX_BQ_RD_ADDR));bqd += bqr_pos;/* BQ is only ever written by GMAC */invalidate_desc(bqd);do {bqw_pos = DESC_CNT(readl(priv->base + RX_BQ_WR_ADDR));udelay(1);} while (--timeout && bqw_pos == bqr_pos);if (!timeout)return -ETIMEDOUT;if (++bqr_pos >= RX_DESC_NUM)bqr_pos = 0;len = bqd->data_len;/* CPU should not have touched this buffer since we added it to FQ */addr = bqd->buf_addr;
Not sure why we need to use one more var (addr) here, why not to keep using bqd->buf_addr ?
invalidate_dcache_range(addr, addr + len);*packetp = (void *)addr;writel(DESC_BYTE(bqr_pos), priv->base + RX_BQ_RD_ADDR);return len;+}
+static int higmac_send(struct udevice *dev, void *packet, int length) +{
struct higmac_priv *priv = dev_get_priv(dev);struct higmac_desc *bqd = priv->txbq;int bqw_pos, rqw_pos, rqr_pos;int timeout = 1000;flush_cache((unsigned long)packet, length);bqw_pos = DESC_CNT(readl(priv->base + TX_BQ_WR_ADDR));bqd += bqw_pos;bqd->buf_addr = (unsigned long)packet;bqd->descvid = DESC_VLD_BUSY;bqd->data_len = length;flush_desc(bqd);if (++bqw_pos >= TX_DESC_NUM)bqw_pos = 0;writel(DESC_BYTE(bqw_pos), priv->base + TX_BQ_WR_ADDR);rqr_pos = DESC_CNT(readl(priv->base + TX_RQ_RD_ADDR));if (++rqr_pos >= TX_DESC_NUM)rqr_pos = 0;do {rqw_pos = DESC_CNT(readl(priv->base + TX_RQ_WR_ADDR));udelay(1);} while (--timeout && rqr_pos != rqw_pos);if (!timeout)return -ETIMEDOUT;writel(DESC_BYTE(rqr_pos), priv->base + TX_RQ_RD_ADDR);return 0;+}
+static int higmac_adjust_link(struct higmac_priv *priv) +{
struct phy_device *phydev = priv->phydev;int interface = priv->phyintf;u32 val;switch (interface) {case PHY_INTERFACE_MODE_RGMII:if (phydev->speed == SPEED_1000)val = RGMII_SPEED_1000;else if (phydev->speed == SPEED_100)val = RGMII_SPEED_100;elseval = RGMII_SPEED_10;break;case PHY_INTERFACE_MODE_MII:if (phydev->speed == SPEED_100)val = MII_SPEED_100;elseval = MII_SPEED_10;break;default:debug("unsupported mode: %d\n", interface);return -EINVAL;}if (phydev->duplex)val |= GMAC_FULL_DUPLEX;writel(val, priv->macif_ctrl);if (phydev->speed == SPEED_1000)val = GMAC_SPEED_1000;else if (phydev->speed == SPEED_100)val = GMAC_SPEED_100;elseval = GMAC_SPEED_10;writel(BIT_MODE_CHANGE_EN, priv->base + MODE_CHANGE_EN);writel(val, priv->base + PORT_MODE);writel(0, priv->base + MODE_CHANGE_EN);writel(phydev->duplex, priv->base + MAC_DUPLEX_HALF_CTRL);return 0;+}
+static int higmac_start(struct udevice *dev) +{
struct higmac_priv *priv = dev_get_priv(dev);struct phy_device *phydev = priv->phydev;int ret;ret = phy_startup(phydev);if (ret)return ret;if (!phydev->link) {debug("%s: link down\n", phydev->dev->name);return -ENODEV;}ret = higmac_adjust_link(priv);if (ret)return ret;/* Enable port */writel(0xf, priv->base + DESC_WR_RD_ENA);writel(BIT_TX_EN | BIT_RX_EN, priv->base + PORT_EN);return 0;+}
+static void higmac_stop(struct udevice *dev) +{
struct higmac_priv *priv = dev_get_priv(dev);/* Disable port */writel(0, priv->base + PORT_EN);writel(0, priv->base + DESC_WR_RD_ENA);+}
+static const struct eth_ops higmac_ops = {
.start = higmac_start,.send = higmac_send,.recv = higmac_recv,.free_pkt = higmac_free_pkt,.stop = higmac_stop,.write_hwaddr = higmac_write_hwaddr,+};
+static int higmac_wait_mdio_ready(struct higmac_priv *priv) +{
int timeout = 1000;while (--timeout) {/* Wait until busy bit is cleared */if ((readl(priv->base + MDIO_SINGLE_CMD) & BIT_MDIO_BUSY) == 0)break;udelay(10);}return timeout ? 0 : -ETIMEDOUT;+}
+static int higmac_mdio_read(struct mii_dev *bus, int addr, int devad, int reg) +{
struct higmac_priv *priv = bus->priv;int ret;ret = higmac_wait_mdio_ready(priv);if (ret)return ret;writel(MDIO_READ_CMD(addr, reg), priv->base + MDIO_SINGLE_CMD);ret = higmac_wait_mdio_ready(priv);if (ret)return ret;if (readl(priv->base + MDIO_RDATA_STATUS) & BIT_MDIO_RDATA_INVALID)return -EINVAL;return readl(priv->base + MDIO_SINGLE_DATA) >> 16;+}
+static int higmac_mdio_write(struct mii_dev *bus, int addr, int devad,
int reg, u16 value)+{
struct higmac_priv *priv = bus->priv;int ret;ret = higmac_wait_mdio_ready(priv);if (ret)return ret;writel(value, priv->base + MDIO_SINGLE_DATA);writel(MDIO_WRITE_CMD(addr, reg), priv->base + MDIO_SINGLE_CMD);return 0;+}
+static int higmac_init_hw_queue(struct higmac_priv *priv,
enum higmac_queue queue)+{
struct higmac_desc *desc, **pdesc;u32 regaddr, regen, regdep;int depth;int len;switch (queue) {case RX_FQ:regaddr = RX_FQ_START_ADDR;regen = RX_FQ_REG_EN;regdep = RX_FQ_DEPTH;depth = RX_DESC_NUM;pdesc = &priv->rxfq;break;case RX_BQ:regaddr = RX_BQ_START_ADDR;regen = RX_BQ_REG_EN;regdep = RX_BQ_DEPTH;depth = RX_DESC_NUM;pdesc = &priv->rxbq;break;case TX_BQ:regaddr = TX_BQ_START_ADDR;regen = TX_BQ_REG_EN;regdep = TX_BQ_DEPTH;depth = TX_DESC_NUM;pdesc = &priv->txbq;break;case TX_RQ:regaddr = TX_RQ_START_ADDR;regen = TX_RQ_REG_EN;regdep = TX_RQ_DEPTH;depth = TX_DESC_NUM;pdesc = &priv->txrq;break;}/* Enable depth */writel(BIT_DEPTH_EN, priv->base + regen);writel(depth << DESC_WORD_SHIFT, priv->base + regdep);writel(0, priv->base + regen);len = depth * sizeof(*desc);desc = memalign(ARCH_DMA_MINALIGN, len);if (!desc)return -ENOMEM;memset(desc, 0, len);flush_cache((unsigned long)desc, len);*pdesc = desc;/* Enable start address */writel(BIT_START_ADDR_EN, priv->base + regen);writel((unsigned long)desc, priv->base + regaddr);writel(0, priv->base + regen);return 0;+}
+static int higmac_hw_init(struct higmac_priv *priv) +{
int ret;/* Initialize hardware queues */ret = higmac_init_hw_queue(priv, RX_FQ);if (ret)return ret;ret = higmac_init_hw_queue(priv, RX_BQ);if (ret)goto free_rx_fq;ret = higmac_init_hw_queue(priv, TX_BQ);if (ret)goto free_rx_bq;ret = higmac_init_hw_queue(priv, TX_RQ);if (ret)goto free_tx_bq;/* Reset phy */reset_assert(&priv->rst_phy);mdelay(10);reset_deassert(&priv->rst_phy);mdelay(30);reset_assert(&priv->rst_phy);mdelay(30);return 0;+free_tx_bq:
free(priv->txbq);+free_rx_bq:
free(priv->rxbq);+free_rx_fq:
free(priv->rxfq);return ret;+}
+static int higmac_probe(struct udevice *dev) +{
struct higmac_priv *priv = dev_get_priv(dev);struct phy_device *phydev;struct mii_dev *bus;int ret;ret = higmac_hw_init(priv);if (ret)return ret;bus = mdio_alloc();if (!bus)return -ENOMEM;bus->read = higmac_mdio_read;bus->write = higmac_mdio_write;bus->priv = priv;priv->bus = bus;ret = mdio_register_seq(bus, dev->seq);if (ret)return ret;phydev = phy_connect(bus, priv->phyaddr, dev, priv->phyintf);if (!phydev)return -ENODEV;phydev->supported &= PHY_GBIT_FEATURES;phydev->advertising = phydev->supported;priv->phydev = phydev;ret = phy_config(phydev);if (ret)return ret;return 0;
why not simply return ret? and remove the check above?
+}
+static int higmac_remove(struct udevice *dev) +{
struct higmac_priv *priv = dev_get_priv(dev);mdio_unregister(priv->bus);mdio_free(priv->bus);return 0;+}
+static int higmac_ofdata_to_platdata(struct udevice *dev) +{
struct higmac_priv *priv = dev_get_priv(dev);ofnode phy_node;const char *phy_mode;int phyintf;
initialize to PHY_INTERFACE_MODE_NONE here?
int ret;priv->base = dev_remap_addr_index(dev, 0);priv->macif_ctrl = dev_remap_addr_index(dev, 1);phyintf = PHY_INTERFACE_MODE_NONE;phy_mode = dev_read_string(dev, "phy-mode");if (phy_mode)phyintf = phy_get_interface_by_name(phy_mode);if (phyintf == PHY_INTERFACE_MODE_NONE)return -ENODEV;priv->phyintf = phyintf;phy_node = dev_read_subnode(dev, "phy");if (!ofnode_valid(phy_node)) {debug("failed to find phy node\n");return -ENODEV;}priv->phyaddr = ofnode_read_u32_default(phy_node, "reg", 0);ret = reset_get_by_name(dev, "phy", &priv->rst_phy);if (ret)return ret;return 0;
same here with return ret.
+}
+static const struct udevice_id higmac_ids[] = {
{ .compatible = "hisilicon,hi3798cv200-gmac" },{ }+};
+U_BOOT_DRIVER(eth_higmac) = {
.name = "eth_higmac",.id = UCLASS_ETH,.of_match = higmac_ids,.ofdata_to_platdata = higmac_ofdata_to_platdata,.probe = higmac_probe,.remove = higmac_remove,.ops = &higmac_ops,.priv_auto_alloc_size = sizeof(struct higmac_priv),.platdata_auto_alloc_size = sizeof(struct eth_pdata),+};
2.18.0
Hi Igor,
On Tue, Jan 29, 2019 at 11:50:33PM +0200, Igor Opaniuk wrote:
Hi Shawn,
Please see inline comments (mostly minor stuff). I'll also test the driver a bit later and leave my T-b.
Thanks. I appreciate the effort of testing.
On Mon, 28 Jan 2019 at 11:15, Shawn Guo shawn.guo@linaro.org wrote:
It adds the driver for HIGMACV300 Ethernet controller found on HiSilicon SoCs like Hi3798CV200. It's based on a downstream U-Boot driver, but quite a lot of code gets rewritten and cleaned up to adopt driver model and PHY API.
Signed-off-by: Shawn Guo shawn.guo@linaro.org
drivers/net/Kconfig | 9 + drivers/net/Makefile | 1 + drivers/net/higmacv300.c | 592 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 602 insertions(+) create mode 100644 drivers/net/higmacv300.c
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 39ce4e8a1fc6..e07a382c28c7 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -530,4 +530,13 @@ config MEDIATEK_ETH This Driver support MediaTek Ethernet GMAC Say Y to enable support for the MediaTek Ethernet GMAC.
+config NET_HIGMACV300
bool "HiSilicon Gigabit Ethernet Controller"depends on DM_ETHselect DM_RESETselect PHYLIBhelpThis driver supports HIGMACV300 Ethernet controller found onHiSilicon SoCs.endif # NETDEVICES diff --git a/drivers/net/Makefile b/drivers/net/Makefile index e38c16464478..0951cb17e6ba 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -76,3 +76,4 @@ obj-$(CONFIG_SNI_AVE) += sni_ave.o obj-y += ti/ obj-$(CONFIG_MEDIATEK_ETH) += mtk_eth.o obj-$(CONFIG_MSCC_OCELOT_SWITCH) += ocelot_switch.o +obj-$(CONFIG_NET_HIGMACV300) += higmacv300.o
suggestion: CONFIG_HIGMACV300_NET, similar to CONFIG_MEDIATEK_ETH above?
Suggestion taken. I would move one step further to rename it HIGMACV300_ETH to be more consistent with Ethernet driver naming convention.
diff --git a/drivers/net/higmacv300.c b/drivers/net/higmacv300.c new file mode 100644 index 000000000000..6cac5cec73df --- /dev/null +++ b/drivers/net/higmacv300.c @@ -0,0 +1,592 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- Copyright (c) 2019, Linaro Limited
- */
+#include <asm/io.h> +#include <common.h> +#include <console.h> +#include <linux/bug.h> +#include <linux/mii.h> +#include <miiphy.h> +#include <net.h> +#include <reset.h>
+#define STATION_ADDR_LOW 0x0000 +#define STATION_ADDR_HIGH 0x0004 +#define MAC_DUPLEX_HALF_CTRL 0x0008 +#define PORT_MODE 0x0040 +#define PORT_EN 0x0044 +#define BIT_TX_EN BIT(2) +#define BIT_RX_EN BIT(1) +#define MODE_CHANGE_EN 0x01b4 +#define BIT_MODE_CHANGE_EN BIT(0) +#define MDIO_SINGLE_CMD 0x03c0 +#define BIT_MDIO_BUSY BIT(20) +#define MDIO_SINGLE_DATA 0x03c4 +#define MDIO_RDATA_STATUS 0x03d0 +#define BIT_MDIO_RDATA_INVALID BIT(0) +#define RX_FQ_START_ADDR 0x0500 +#define RX_FQ_DEPTH 0x0504 +#define RX_FQ_WR_ADDR 0x0508 +#define RX_FQ_RD_ADDR 0x050c +#define RX_FQ_REG_EN 0x0518 +#define RX_BQ_START_ADDR 0x0520 +#define RX_BQ_DEPTH 0x0524 +#define RX_BQ_WR_ADDR 0x0528 +#define RX_BQ_RD_ADDR 0x052c +#define RX_BQ_REG_EN 0x0538 +#define TX_BQ_START_ADDR 0x0580 +#define TX_BQ_DEPTH 0x0584 +#define TX_BQ_WR_ADDR 0x0588 +#define TX_BQ_RD_ADDR 0x058c +#define TX_BQ_REG_EN 0x0598 +#define TX_RQ_START_ADDR 0x05a0 +#define TX_RQ_DEPTH 0x05a4 +#define TX_RQ_WR_ADDR 0x05a8 +#define TX_RQ_RD_ADDR 0x05ac +#define TX_RQ_REG_EN 0x05b8 +#define BIT_START_ADDR_EN BIT(2) +#define BIT_DEPTH_EN BIT(1) +#define DESC_WR_RD_ENA 0x05cc
Please group above defines into categories.
I would rather organize them in the order of offset, which should be the same to what hardware manual does.
Suggestion: move all of them to a separate header file.
I do not see much point of doing that, which only makes searching of the definitions more difficult.
+/* MACIF_CTRL */ +#define RGMII_SPEED_1000 0x2c +#define RGMII_SPEED_100 0x2f +#define RGMII_SPEED_10 0x2d +#define MII_SPEED_100 0x0f +#define MII_SPEED_10 0x0d +#define GMAC_SPEED_1000 0x05 +#define GMAC_SPEED_100 0x01 +#define GMAC_SPEED_10 0x00 +#define GMAC_FULL_DUPLEX BIT(4)
+#define RX_DESC_NUM 64 +#define TX_DESC_NUM 2 +#define DESC_SIZE 32 +#define DESC_WORD_SHIFT 3 +#define DESC_BYTE_SHIFT 5 +#define DESC_CNT(n) ((n) >> DESC_BYTE_SHIFT) +#define DESC_BYTE(n) ((n) << DESC_BYTE_SHIFT) +#define DESC_VLD_FREE 0 +#define DESC_VLD_BUSY 1
+#define MAC_MAX_FRAME_SIZE 1600
+#define MDIO_WRITE 1 +#define MDIO_READ 2 +#define MDIO_COMMAND(rw, addr, reg) (BIT_MDIO_BUSY | \
(((rw) & 0x3) << 16) | \(((addr) & 0x1f) << 8) | \((reg) & 0x1f))+#define MDIO_WRITE_CMD(addr, reg) MDIO_COMMAND(MDIO_WRITE, addr, reg) +#define MDIO_READ_CMD(addr, reg) MDIO_COMMAND(MDIO_READ, addr, reg)
+enum higmac_queue {
RX_FQ,RX_BQ,TX_BQ,TX_RQ,+};
+struct higmac_desc {
unsigned int buf_addr;uintptr_t?
IMO, the current definition from vendor code is more intuitive to reflect the fact, that the hardware descriptor consists of 8 32-bit words. That said, I would like to use the same type for all fields of this structure. I will change them all to use unsigned long though.
unsigned int buf_len:11;unsigned int reserve0:5;unsigned int data_len:11;unsigned int reserve1:2;unsigned int fl:2;unsigned int descvid:1;unsigned int reserve2[6];+};
+struct higmac_priv {
void __iomem *base;void __iomem *macif_ctrl;struct reset_ctl rst_phy;struct higmac_desc *rxfq;struct higmac_desc *rxbq;struct higmac_desc *txbq;struct higmac_desc *txrq;struct mii_dev *bus;struct phy_device *phydev;int phyintf;int phyaddr;+};
+#define flush_desc(d) flush_cache((unsigned long)(d), sizeof(*(d))) +#define invalidate_desc(d) \
invalidate_dcache_range((unsigned long)(d), \(unsigned long)(d) + sizeof(*(d)))+static int higmac_write_hwaddr(struct udevice *dev) +{
struct eth_pdata *pdata = dev_get_platdata(dev);struct higmac_priv *priv = dev_get_priv(dev);unsigned char *mac = pdata->enetaddr;u32 val;val = mac[1] | (mac[0] << 8);writel(val, priv->base + STATION_ADDR_HIGH);val = mac[5] | (mac[4] << 8) | (mac[3] << 16) | (mac[2] << 24);writel(val, priv->base + STATION_ADDR_LOW);return 0;+}
+static int higmac_free_pkt(struct udevice *dev, uchar *packet, int length)
__always_unused for length and dev
Hmm, do you see any problem without this attribute? None of the Ethernet driver in the tree has this, and I do not want higamc drive to be special.
+{
if (packet)free(packet);return 0;+}
+static int higmac_recv(struct udevice *dev, int flags, uchar **packetp)
__always_unused for flags
+{
struct higmac_priv *priv = dev_get_priv(dev);struct higmac_desc *fqd = priv->rxfq;struct higmac_desc *bqd = priv->rxbq;int fqw_pos, fqr_pos, bqw_pos, bqr_pos;int timeout = 100000;unsigned long addr;int len = 0;int space;int i;fqw_pos = DESC_CNT(readl(priv->base + RX_FQ_WR_ADDR));fqr_pos = DESC_CNT(readl(priv->base + RX_FQ_RD_ADDR));if (fqw_pos >= fqr_pos)space = RX_DESC_NUM - (fqw_pos - fqr_pos);elsespace = fqr_pos - fqw_pos;/* Leave one free to distinguish full filled from empty buffer */for (i = 0; i < space - 1; i++) {void *buf = memalign(ARCH_DMA_MINALIGN, MAC_MAX_FRAME_SIZE);if (!buf)break;fqd = priv->rxfq + fqw_pos;fqd->buf_addr = (unsigned long)buf;buf_addr is unsigned int, possible value overflow.
The implication here is that the buffer has to be in lower 4GB address space, which is true for HiSilicon platform.
invalidate_dcache_range(fqd->buf_addr,fqd->buf_addr + MAC_MAX_FRAME_SIZE);fqd->descvid = DESC_VLD_FREE;fqd->buf_len = MAC_MAX_FRAME_SIZE - 1;flush_desc(fqd);if (++fqw_pos >= RX_DESC_NUM)fqw_pos = 0;writel(DESC_BYTE(fqw_pos), priv->base + RX_FQ_WR_ADDR);}bqr_pos = DESC_CNT(readl(priv->base + RX_BQ_RD_ADDR));bqd += bqr_pos;/* BQ is only ever written by GMAC */invalidate_desc(bqd);do {bqw_pos = DESC_CNT(readl(priv->base + RX_BQ_WR_ADDR));udelay(1);} while (--timeout && bqw_pos == bqr_pos);if (!timeout)return -ETIMEDOUT;if (++bqr_pos >= RX_DESC_NUM)bqr_pos = 0;len = bqd->data_len;/* CPU should not have touched this buffer since we added it to FQ */addr = bqd->buf_addr;Not sure why we need to use one more var (addr) here, why not to keep using bqd->buf_addr ?
You are right.
invalidate_dcache_range(addr, addr + len);*packetp = (void *)addr;writel(DESC_BYTE(bqr_pos), priv->base + RX_BQ_RD_ADDR);return len;+}
+static int higmac_send(struct udevice *dev, void *packet, int length) +{
struct higmac_priv *priv = dev_get_priv(dev);struct higmac_desc *bqd = priv->txbq;int bqw_pos, rqw_pos, rqr_pos;int timeout = 1000;flush_cache((unsigned long)packet, length);bqw_pos = DESC_CNT(readl(priv->base + TX_BQ_WR_ADDR));bqd += bqw_pos;bqd->buf_addr = (unsigned long)packet;bqd->descvid = DESC_VLD_BUSY;bqd->data_len = length;flush_desc(bqd);if (++bqw_pos >= TX_DESC_NUM)bqw_pos = 0;writel(DESC_BYTE(bqw_pos), priv->base + TX_BQ_WR_ADDR);rqr_pos = DESC_CNT(readl(priv->base + TX_RQ_RD_ADDR));if (++rqr_pos >= TX_DESC_NUM)rqr_pos = 0;do {rqw_pos = DESC_CNT(readl(priv->base + TX_RQ_WR_ADDR));udelay(1);} while (--timeout && rqr_pos != rqw_pos);if (!timeout)return -ETIMEDOUT;writel(DESC_BYTE(rqr_pos), priv->base + TX_RQ_RD_ADDR);return 0;+}
+static int higmac_adjust_link(struct higmac_priv *priv) +{
struct phy_device *phydev = priv->phydev;int interface = priv->phyintf;u32 val;switch (interface) {case PHY_INTERFACE_MODE_RGMII:if (phydev->speed == SPEED_1000)val = RGMII_SPEED_1000;else if (phydev->speed == SPEED_100)val = RGMII_SPEED_100;elseval = RGMII_SPEED_10;break;case PHY_INTERFACE_MODE_MII:if (phydev->speed == SPEED_100)val = MII_SPEED_100;elseval = MII_SPEED_10;break;default:debug("unsupported mode: %d\n", interface);return -EINVAL;}if (phydev->duplex)val |= GMAC_FULL_DUPLEX;writel(val, priv->macif_ctrl);if (phydev->speed == SPEED_1000)val = GMAC_SPEED_1000;else if (phydev->speed == SPEED_100)val = GMAC_SPEED_100;elseval = GMAC_SPEED_10;writel(BIT_MODE_CHANGE_EN, priv->base + MODE_CHANGE_EN);writel(val, priv->base + PORT_MODE);writel(0, priv->base + MODE_CHANGE_EN);writel(phydev->duplex, priv->base + MAC_DUPLEX_HALF_CTRL);return 0;+}
+static int higmac_start(struct udevice *dev) +{
struct higmac_priv *priv = dev_get_priv(dev);struct phy_device *phydev = priv->phydev;int ret;ret = phy_startup(phydev);if (ret)return ret;if (!phydev->link) {debug("%s: link down\n", phydev->dev->name);return -ENODEV;}ret = higmac_adjust_link(priv);if (ret)return ret;/* Enable port */writel(0xf, priv->base + DESC_WR_RD_ENA);writel(BIT_TX_EN | BIT_RX_EN, priv->base + PORT_EN);return 0;+}
+static void higmac_stop(struct udevice *dev) +{
struct higmac_priv *priv = dev_get_priv(dev);/* Disable port */writel(0, priv->base + PORT_EN);writel(0, priv->base + DESC_WR_RD_ENA);+}
+static const struct eth_ops higmac_ops = {
.start = higmac_start,.send = higmac_send,.recv = higmac_recv,.free_pkt = higmac_free_pkt,.stop = higmac_stop,.write_hwaddr = higmac_write_hwaddr,+};
+static int higmac_wait_mdio_ready(struct higmac_priv *priv) +{
int timeout = 1000;while (--timeout) {/* Wait until busy bit is cleared */if ((readl(priv->base + MDIO_SINGLE_CMD) & BIT_MDIO_BUSY) == 0)break;udelay(10);}return timeout ? 0 : -ETIMEDOUT;+}
+static int higmac_mdio_read(struct mii_dev *bus, int addr, int devad, int reg) +{
struct higmac_priv *priv = bus->priv;int ret;ret = higmac_wait_mdio_ready(priv);if (ret)return ret;writel(MDIO_READ_CMD(addr, reg), priv->base + MDIO_SINGLE_CMD);ret = higmac_wait_mdio_ready(priv);if (ret)return ret;if (readl(priv->base + MDIO_RDATA_STATUS) & BIT_MDIO_RDATA_INVALID)return -EINVAL;return readl(priv->base + MDIO_SINGLE_DATA) >> 16;+}
+static int higmac_mdio_write(struct mii_dev *bus, int addr, int devad,
int reg, u16 value)+{
struct higmac_priv *priv = bus->priv;int ret;ret = higmac_wait_mdio_ready(priv);if (ret)return ret;writel(value, priv->base + MDIO_SINGLE_DATA);writel(MDIO_WRITE_CMD(addr, reg), priv->base + MDIO_SINGLE_CMD);return 0;+}
+static int higmac_init_hw_queue(struct higmac_priv *priv,
enum higmac_queue queue)+{
struct higmac_desc *desc, **pdesc;u32 regaddr, regen, regdep;int depth;int len;switch (queue) {case RX_FQ:regaddr = RX_FQ_START_ADDR;regen = RX_FQ_REG_EN;regdep = RX_FQ_DEPTH;depth = RX_DESC_NUM;pdesc = &priv->rxfq;break;case RX_BQ:regaddr = RX_BQ_START_ADDR;regen = RX_BQ_REG_EN;regdep = RX_BQ_DEPTH;depth = RX_DESC_NUM;pdesc = &priv->rxbq;break;case TX_BQ:regaddr = TX_BQ_START_ADDR;regen = TX_BQ_REG_EN;regdep = TX_BQ_DEPTH;depth = TX_DESC_NUM;pdesc = &priv->txbq;break;case TX_RQ:regaddr = TX_RQ_START_ADDR;regen = TX_RQ_REG_EN;regdep = TX_RQ_DEPTH;depth = TX_DESC_NUM;pdesc = &priv->txrq;break;}/* Enable depth */writel(BIT_DEPTH_EN, priv->base + regen);writel(depth << DESC_WORD_SHIFT, priv->base + regdep);writel(0, priv->base + regen);len = depth * sizeof(*desc);desc = memalign(ARCH_DMA_MINALIGN, len);if (!desc)return -ENOMEM;memset(desc, 0, len);flush_cache((unsigned long)desc, len);*pdesc = desc;/* Enable start address */writel(BIT_START_ADDR_EN, priv->base + regen);writel((unsigned long)desc, priv->base + regaddr);writel(0, priv->base + regen);return 0;+}
+static int higmac_hw_init(struct higmac_priv *priv) +{
int ret;/* Initialize hardware queues */ret = higmac_init_hw_queue(priv, RX_FQ);if (ret)return ret;ret = higmac_init_hw_queue(priv, RX_BQ);if (ret)goto free_rx_fq;ret = higmac_init_hw_queue(priv, TX_BQ);if (ret)goto free_rx_bq;ret = higmac_init_hw_queue(priv, TX_RQ);if (ret)goto free_tx_bq;/* Reset phy */reset_assert(&priv->rst_phy);mdelay(10);reset_deassert(&priv->rst_phy);mdelay(30);reset_assert(&priv->rst_phy);mdelay(30);return 0;+free_tx_bq:
free(priv->txbq);+free_rx_bq:
free(priv->rxbq);+free_rx_fq:
free(priv->rxfq);return ret;+}
+static int higmac_probe(struct udevice *dev) +{
struct higmac_priv *priv = dev_get_priv(dev);struct phy_device *phydev;struct mii_dev *bus;int ret;ret = higmac_hw_init(priv);if (ret)return ret;bus = mdio_alloc();if (!bus)return -ENOMEM;bus->read = higmac_mdio_read;bus->write = higmac_mdio_write;bus->priv = priv;priv->bus = bus;ret = mdio_register_seq(bus, dev->seq);if (ret)return ret;phydev = phy_connect(bus, priv->phyaddr, dev, priv->phyintf);if (!phydev)return -ENODEV;phydev->supported &= PHY_GBIT_FEATURES;phydev->advertising = phydev->supported;priv->phydev = phydev;ret = phy_config(phydev);if (ret)return ret;return 0;why not simply return ret? and remove the check above?
Will do.
+}
+static int higmac_remove(struct udevice *dev) +{
struct higmac_priv *priv = dev_get_priv(dev);mdio_unregister(priv->bus);mdio_free(priv->bus);return 0;+}
+static int higmac_ofdata_to_platdata(struct udevice *dev) +{
struct higmac_priv *priv = dev_get_priv(dev);ofnode phy_node;const char *phy_mode;int phyintf;initialize to PHY_INTERFACE_MODE_NONE here?
Okay, will do.
int ret;priv->base = dev_remap_addr_index(dev, 0);priv->macif_ctrl = dev_remap_addr_index(dev, 1);phyintf = PHY_INTERFACE_MODE_NONE;phy_mode = dev_read_string(dev, "phy-mode");if (phy_mode)phyintf = phy_get_interface_by_name(phy_mode);if (phyintf == PHY_INTERFACE_MODE_NONE)return -ENODEV;priv->phyintf = phyintf;phy_node = dev_read_subnode(dev, "phy");if (!ofnode_valid(phy_node)) {debug("failed to find phy node\n");return -ENODEV;}priv->phyaddr = ofnode_read_u32_default(phy_node, "reg", 0);ret = reset_get_by_name(dev, "phy", &priv->rst_phy);if (ret)return ret;return 0;same here with return ret.
Okay.
Thanks for the review.
Shawn
+}
+static const struct udevice_id higmac_ids[] = {
{ .compatible = "hisilicon,hi3798cv200-gmac" },{ }+};
+U_BOOT_DRIVER(eth_higmac) = {
.name = "eth_higmac",.id = UCLASS_ETH,.of_match = higmac_ids,.ofdata_to_platdata = higmac_ofdata_to_platdata,.probe = higmac_probe,.remove = higmac_remove,.ops = &higmac_ops,.priv_auto_alloc_size = sizeof(struct higmac_priv),.platdata_auto_alloc_size = sizeof(struct eth_pdata),+};
2.18.0
-- Regards, Igor Opaniuk
On Mon, Jan 28, 2019 at 3:16 AM Shawn Guo shawn.guo@linaro.org wrote:
It adds the driver for HIGMACV300 Ethernet controller found on HiSilicon SoCs like Hi3798CV200. It's based on a downstream U-Boot driver, but quite a lot of code gets rewritten and cleaned up to adopt driver model and PHY API.
Signed-off-by: Shawn Guo shawn.guo@linaro.org
drivers/net/Kconfig | 9 + drivers/net/Makefile | 1 + drivers/net/higmacv300.c | 592 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 602 insertions(+) create mode 100644 drivers/net/higmacv300.c
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 39ce4e8a1fc6..e07a382c28c7 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -530,4 +530,13 @@ config MEDIATEK_ETH This Driver support MediaTek Ethernet GMAC Say Y to enable support for the MediaTek Ethernet GMAC.
+config NET_HIGMACV300
bool "HiSilicon Gigabit Ethernet Controller"depends on DM_ETHselect DM_RESETselect PHYLIBhelpThis driver supports HIGMACV300 Ethernet controller found onHiSilicon SoCs.endif # NETDEVICES diff --git a/drivers/net/Makefile b/drivers/net/Makefile index e38c16464478..0951cb17e6ba 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -76,3 +76,4 @@ obj-$(CONFIG_SNI_AVE) += sni_ave.o obj-y += ti/ obj-$(CONFIG_MEDIATEK_ETH) += mtk_eth.o obj-$(CONFIG_MSCC_OCELOT_SWITCH) += ocelot_switch.o +obj-$(CONFIG_NET_HIGMACV300) += higmacv300.o diff --git a/drivers/net/higmacv300.c b/drivers/net/higmacv300.c new file mode 100644 index 000000000000..6cac5cec73df --- /dev/null +++ b/drivers/net/higmacv300.c @@ -0,0 +1,592 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- Copyright (c) 2019, Linaro Limited
- */
+#include <asm/io.h> +#include <common.h> +#include <console.h> +#include <linux/bug.h> +#include <linux/mii.h> +#include <miiphy.h> +#include <net.h> +#include <reset.h>
+#define STATION_ADDR_LOW 0x0000 +#define STATION_ADDR_HIGH 0x0004 +#define MAC_DUPLEX_HALF_CTRL 0x0008 +#define PORT_MODE 0x0040 +#define PORT_EN 0x0044 +#define BIT_TX_EN BIT(2) +#define BIT_RX_EN BIT(1) +#define MODE_CHANGE_EN 0x01b4 +#define BIT_MODE_CHANGE_EN BIT(0) +#define MDIO_SINGLE_CMD 0x03c0 +#define BIT_MDIO_BUSY BIT(20) +#define MDIO_SINGLE_DATA 0x03c4 +#define MDIO_RDATA_STATUS 0x03d0 +#define BIT_MDIO_RDATA_INVALID BIT(0) +#define RX_FQ_START_ADDR 0x0500 +#define RX_FQ_DEPTH 0x0504 +#define RX_FQ_WR_ADDR 0x0508 +#define RX_FQ_RD_ADDR 0x050c +#define RX_FQ_REG_EN 0x0518 +#define RX_BQ_START_ADDR 0x0520 +#define RX_BQ_DEPTH 0x0524 +#define RX_BQ_WR_ADDR 0x0528 +#define RX_BQ_RD_ADDR 0x052c +#define RX_BQ_REG_EN 0x0538 +#define TX_BQ_START_ADDR 0x0580 +#define TX_BQ_DEPTH 0x0584 +#define TX_BQ_WR_ADDR 0x0588 +#define TX_BQ_RD_ADDR 0x058c +#define TX_BQ_REG_EN 0x0598 +#define TX_RQ_START_ADDR 0x05a0 +#define TX_RQ_DEPTH 0x05a4 +#define TX_RQ_WR_ADDR 0x05a8 +#define TX_RQ_RD_ADDR 0x05ac +#define TX_RQ_REG_EN 0x05b8 +#define BIT_START_ADDR_EN BIT(2) +#define BIT_DEPTH_EN BIT(1) +#define DESC_WR_RD_ENA 0x05cc
+/* MACIF_CTRL */ +#define RGMII_SPEED_1000 0x2c +#define RGMII_SPEED_100 0x2f +#define RGMII_SPEED_10 0x2d +#define MII_SPEED_100 0x0f +#define MII_SPEED_10 0x0d +#define GMAC_SPEED_1000 0x05 +#define GMAC_SPEED_100 0x01 +#define GMAC_SPEED_10 0x00 +#define GMAC_FULL_DUPLEX BIT(4)
+#define RX_DESC_NUM 64 +#define TX_DESC_NUM 2 +#define DESC_SIZE 32 +#define DESC_WORD_SHIFT 3 +#define DESC_BYTE_SHIFT 5 +#define DESC_CNT(n) ((n) >> DESC_BYTE_SHIFT) +#define DESC_BYTE(n) ((n) << DESC_BYTE_SHIFT) +#define DESC_VLD_FREE 0 +#define DESC_VLD_BUSY 1
+#define MAC_MAX_FRAME_SIZE 1600
+#define MDIO_WRITE 1 +#define MDIO_READ 2 +#define MDIO_COMMAND(rw, addr, reg) (BIT_MDIO_BUSY | \
(((rw) & 0x3) << 16) | \(((addr) & 0x1f) << 8) | \((reg) & 0x1f))+#define MDIO_WRITE_CMD(addr, reg) MDIO_COMMAND(MDIO_WRITE, addr, reg) +#define MDIO_READ_CMD(addr, reg) MDIO_COMMAND(MDIO_READ, addr, reg)
This is a strange construct... can you just inline this into the actual functions? I don't see the value in these macros that hide what's happening.
+enum higmac_queue {
RX_FQ,RX_BQ,TX_BQ,TX_RQ,+};
+struct higmac_desc {
unsigned int buf_addr;unsigned int buf_len:11;unsigned int reserve0:5;unsigned int data_len:11;unsigned int reserve1:2;unsigned int fl:2;unsigned int descvid:1;unsigned int reserve2[6];+};
+struct higmac_priv {
void __iomem *base;void __iomem *macif_ctrl;struct reset_ctl rst_phy;struct higmac_desc *rxfq;struct higmac_desc *rxbq;struct higmac_desc *txbq;struct higmac_desc *txrq;struct mii_dev *bus;struct phy_device *phydev;int phyintf;int phyaddr;+};
+#define flush_desc(d) flush_cache((unsigned long)(d), sizeof(*(d))) +#define invalidate_desc(d) \
invalidate_dcache_range((unsigned long)(d), \(unsigned long)(d) + sizeof(*(d)))+static int higmac_write_hwaddr(struct udevice *dev) +{
struct eth_pdata *pdata = dev_get_platdata(dev);struct higmac_priv *priv = dev_get_priv(dev);unsigned char *mac = pdata->enetaddr;u32 val;val = mac[1] | (mac[0] << 8);writel(val, priv->base + STATION_ADDR_HIGH);val = mac[5] | (mac[4] << 8) | (mac[3] << 16) | (mac[2] << 24);writel(val, priv->base + STATION_ADDR_LOW);return 0;+}
+static int higmac_free_pkt(struct udevice *dev, uchar *packet, int length) +{
if (packet)free(packet);
Why free them and reallocate them? Won't that just fragment memory? Also, you should be somehow telling the MAC that the buffer / descriptor is no longer in use here.
return 0;+}
+static int higmac_recv(struct udevice *dev, int flags, uchar **packetp) +{
struct higmac_priv *priv = dev_get_priv(dev);struct higmac_desc *fqd = priv->rxfq;struct higmac_desc *bqd = priv->rxbq;int fqw_pos, fqr_pos, bqw_pos, bqr_pos;int timeout = 100000;unsigned long addr;int len = 0;int space;int i;fqw_pos = DESC_CNT(readl(priv->base + RX_FQ_WR_ADDR));fqr_pos = DESC_CNT(readl(priv->base + RX_FQ_RD_ADDR));if (fqw_pos >= fqr_pos)space = RX_DESC_NUM - (fqw_pos - fqr_pos);elsespace = fqr_pos - fqw_pos;/* Leave one free to distinguish full filled from empty buffer */for (i = 0; i < space - 1; i++) {void *buf = memalign(ARCH_DMA_MINALIGN, MAC_MAX_FRAME_SIZE);if (!buf)break;fqd = priv->rxfq + fqw_pos;fqd->buf_addr = (unsigned long)buf;invalidate_dcache_range(fqd->buf_addr,fqd->buf_addr + MAC_MAX_FRAME_SIZE);fqd->descvid = DESC_VLD_FREE;fqd->buf_len = MAC_MAX_FRAME_SIZE - 1;flush_desc(fqd);if (++fqw_pos >= RX_DESC_NUM)fqw_pos = 0;writel(DESC_BYTE(fqw_pos), priv->base + RX_FQ_WR_ADDR);}bqr_pos = DESC_CNT(readl(priv->base + RX_BQ_RD_ADDR));bqd += bqr_pos;/* BQ is only ever written by GMAC */invalidate_desc(bqd);do {bqw_pos = DESC_CNT(readl(priv->base + RX_BQ_WR_ADDR));udelay(1);} while (--timeout && bqw_pos == bqr_pos);if (!timeout)return -ETIMEDOUT;if (++bqr_pos >= RX_DESC_NUM)bqr_pos = 0;len = bqd->data_len;/* CPU should not have touched this buffer since we added it to FQ */addr = bqd->buf_addr;invalidate_dcache_range(addr, addr + len);*packetp = (void *)addr;writel(DESC_BYTE(bqr_pos), priv->base + RX_BQ_RD_ADDR);
Does this belong in free_pkt()?
return len;+}
+static int higmac_send(struct udevice *dev, void *packet, int length) +{
struct higmac_priv *priv = dev_get_priv(dev);struct higmac_desc *bqd = priv->txbq;int bqw_pos, rqw_pos, rqr_pos;int timeout = 1000;flush_cache((unsigned long)packet, length);bqw_pos = DESC_CNT(readl(priv->base + TX_BQ_WR_ADDR));bqd += bqw_pos;bqd->buf_addr = (unsigned long)packet;bqd->descvid = DESC_VLD_BUSY;bqd->data_len = length;flush_desc(bqd);if (++bqw_pos >= TX_DESC_NUM)bqw_pos = 0;writel(DESC_BYTE(bqw_pos), priv->base + TX_BQ_WR_ADDR);rqr_pos = DESC_CNT(readl(priv->base + TX_RQ_RD_ADDR));if (++rqr_pos >= TX_DESC_NUM)rqr_pos = 0;do {rqw_pos = DESC_CNT(readl(priv->base + TX_RQ_WR_ADDR));udelay(1);} while (--timeout && rqr_pos != rqw_pos);if (!timeout)return -ETIMEDOUT;writel(DESC_BYTE(rqr_pos), priv->base + TX_RQ_RD_ADDR);return 0;+}
+static int higmac_adjust_link(struct higmac_priv *priv) +{
struct phy_device *phydev = priv->phydev;int interface = priv->phyintf;u32 val;switch (interface) {case PHY_INTERFACE_MODE_RGMII:if (phydev->speed == SPEED_1000)val = RGMII_SPEED_1000;else if (phydev->speed == SPEED_100)val = RGMII_SPEED_100;elseval = RGMII_SPEED_10;break;case PHY_INTERFACE_MODE_MII:if (phydev->speed == SPEED_100)val = MII_SPEED_100;elseval = MII_SPEED_10;break;default:debug("unsupported mode: %d\n", interface);return -EINVAL;}if (phydev->duplex)val |= GMAC_FULL_DUPLEX;writel(val, priv->macif_ctrl);if (phydev->speed == SPEED_1000)val = GMAC_SPEED_1000;else if (phydev->speed == SPEED_100)val = GMAC_SPEED_100;elseval = GMAC_SPEED_10;writel(BIT_MODE_CHANGE_EN, priv->base + MODE_CHANGE_EN);writel(val, priv->base + PORT_MODE);writel(0, priv->base + MODE_CHANGE_EN);writel(phydev->duplex, priv->base + MAC_DUPLEX_HALF_CTRL);return 0;+}
+static int higmac_start(struct udevice *dev) +{
struct higmac_priv *priv = dev_get_priv(dev);struct phy_device *phydev = priv->phydev;int ret;ret = phy_startup(phydev);if (ret)return ret;if (!phydev->link) {debug("%s: link down\n", phydev->dev->name);return -ENODEV;}ret = higmac_adjust_link(priv);if (ret)return ret;/* Enable port */writel(0xf, priv->base + DESC_WR_RD_ENA);
What is 0xf? No magic numbers please.
writel(BIT_TX_EN | BIT_RX_EN, priv->base + PORT_EN);return 0;+}
+static void higmac_stop(struct udevice *dev) +{
struct higmac_priv *priv = dev_get_priv(dev);/* Disable port */writel(0, priv->base + PORT_EN);writel(0, priv->base + DESC_WR_RD_ENA);+}
+static const struct eth_ops higmac_ops = {
.start = higmac_start,.send = higmac_send,.recv = higmac_recv,.free_pkt = higmac_free_pkt,.stop = higmac_stop,.write_hwaddr = higmac_write_hwaddr,+};
+static int higmac_wait_mdio_ready(struct higmac_priv *priv) +{
int timeout = 1000;while (--timeout) {/* Wait until busy bit is cleared */if ((readl(priv->base + MDIO_SINGLE_CMD) & BIT_MDIO_BUSY) == 0)break;udelay(10);}
It would be good if you retrofit to use wait_bit or its macros throughout this driver instead of hand-writing it.
return timeout ? 0 : -ETIMEDOUT;+}
+static int higmac_mdio_read(struct mii_dev *bus, int addr, int devad, int reg) +{
struct higmac_priv *priv = bus->priv;int ret;ret = higmac_wait_mdio_ready(priv);if (ret)return ret;writel(MDIO_READ_CMD(addr, reg), priv->base + MDIO_SINGLE_CMD);ret = higmac_wait_mdio_ready(priv);if (ret)return ret;if (readl(priv->base + MDIO_RDATA_STATUS) & BIT_MDIO_RDATA_INVALID)return -EINVAL;return readl(priv->base + MDIO_SINGLE_DATA) >> 16;+}
+static int higmac_mdio_write(struct mii_dev *bus, int addr, int devad,
int reg, u16 value)+{
struct higmac_priv *priv = bus->priv;int ret;ret = higmac_wait_mdio_ready(priv);if (ret)return ret;writel(value, priv->base + MDIO_SINGLE_DATA);writel(MDIO_WRITE_CMD(addr, reg), priv->base + MDIO_SINGLE_CMD);return 0;+}
+static int higmac_init_hw_queue(struct higmac_priv *priv,
enum higmac_queue queue)+{
struct higmac_desc *desc, **pdesc;u32 regaddr, regen, regdep;int depth;int len;switch (queue) {case RX_FQ:regaddr = RX_FQ_START_ADDR;regen = RX_FQ_REG_EN;regdep = RX_FQ_DEPTH;depth = RX_DESC_NUM;pdesc = &priv->rxfq;break;case RX_BQ:regaddr = RX_BQ_START_ADDR;regen = RX_BQ_REG_EN;regdep = RX_BQ_DEPTH;depth = RX_DESC_NUM;pdesc = &priv->rxbq;break;case TX_BQ:regaddr = TX_BQ_START_ADDR;regen = TX_BQ_REG_EN;regdep = TX_BQ_DEPTH;depth = TX_DESC_NUM;pdesc = &priv->txbq;break;case TX_RQ:regaddr = TX_RQ_START_ADDR;regen = TX_RQ_REG_EN;regdep = TX_RQ_DEPTH;depth = TX_DESC_NUM;pdesc = &priv->txrq;break;}/* Enable depth */writel(BIT_DEPTH_EN, priv->base + regen);writel(depth << DESC_WORD_SHIFT, priv->base + regdep);writel(0, priv->base + regen);len = depth * sizeof(*desc);desc = memalign(ARCH_DMA_MINALIGN, len);if (!desc)return -ENOMEM;memset(desc, 0, len);flush_cache((unsigned long)desc, len);*pdesc = desc;/* Enable start address */writel(BIT_START_ADDR_EN, priv->base + regen);writel((unsigned long)desc, priv->base + regaddr);writel(0, priv->base + regen);return 0;+}
+static int higmac_hw_init(struct higmac_priv *priv) +{
int ret;/* Initialize hardware queues */ret = higmac_init_hw_queue(priv, RX_FQ);if (ret)return ret;ret = higmac_init_hw_queue(priv, RX_BQ);if (ret)goto free_rx_fq;ret = higmac_init_hw_queue(priv, TX_BQ);if (ret)goto free_rx_bq;ret = higmac_init_hw_queue(priv, TX_RQ);if (ret)goto free_tx_bq;/* Reset phy */reset_assert(&priv->rst_phy);mdelay(10);reset_deassert(&priv->rst_phy);mdelay(30);reset_assert(&priv->rst_phy);mdelay(30);return 0;+free_tx_bq:
free(priv->txbq);+free_rx_bq:
free(priv->rxbq);+free_rx_fq:
free(priv->rxfq);return ret;+}
+static int higmac_probe(struct udevice *dev) +{
struct higmac_priv *priv = dev_get_priv(dev);struct phy_device *phydev;struct mii_dev *bus;int ret;ret = higmac_hw_init(priv);if (ret)return ret;bus = mdio_alloc();if (!bus)return -ENOMEM;bus->read = higmac_mdio_read;bus->write = higmac_mdio_write;bus->priv = priv;priv->bus = bus;ret = mdio_register_seq(bus, dev->seq);if (ret)return ret;phydev = phy_connect(bus, priv->phyaddr, dev, priv->phyintf);if (!phydev)return -ENODEV;phydev->supported &= PHY_GBIT_FEATURES;
I would expect either phydev->supported &= ~PHY_GBIT_FEATURES; or phydev->supported |= PHY_GBIT_FEATURES;
phydev->advertising = phydev->supported;priv->phydev = phydev;ret = phy_config(phydev);if (ret)return ret;return 0;+}
+static int higmac_remove(struct udevice *dev) +{
struct higmac_priv *priv = dev_get_priv(dev);mdio_unregister(priv->bus);mdio_free(priv->bus);return 0;+}
+static int higmac_ofdata_to_platdata(struct udevice *dev) +{
struct higmac_priv *priv = dev_get_priv(dev);ofnode phy_node;const char *phy_mode;int phyintf;int ret;priv->base = dev_remap_addr_index(dev, 0);priv->macif_ctrl = dev_remap_addr_index(dev, 1);phyintf = PHY_INTERFACE_MODE_NONE;phy_mode = dev_read_string(dev, "phy-mode");if (phy_mode)phyintf = phy_get_interface_by_name(phy_mode);if (phyintf == PHY_INTERFACE_MODE_NONE)return -ENODEV;priv->phyintf = phyintf;phy_node = dev_read_subnode(dev, "phy");if (!ofnode_valid(phy_node)) {debug("failed to find phy node\n");return -ENODEV;}priv->phyaddr = ofnode_read_u32_default(phy_node, "reg", 0);ret = reset_get_by_name(dev, "phy", &priv->rst_phy);if (ret)return ret;return 0;+}
+static const struct udevice_id higmac_ids[] = {
{ .compatible = "hisilicon,hi3798cv200-gmac" },{ }+};
+U_BOOT_DRIVER(eth_higmac) = {
.name = "eth_higmac",.id = UCLASS_ETH,.of_match = higmac_ids,.ofdata_to_platdata = higmac_ofdata_to_platdata,.probe = higmac_probe,.remove = higmac_remove,.ops = &higmac_ops,.priv_auto_alloc_size = sizeof(struct higmac_priv),.platdata_auto_alloc_size = sizeof(struct eth_pdata),+};
2.18.0
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Hi Joe,
Thanks for spending time to review the patch.
On Mon, Feb 04, 2019 at 11:48:40PM +0000, Joe Hershberger wrote:
On Mon, Jan 28, 2019 at 3:16 AM Shawn Guo shawn.guo@linaro.org wrote:
It adds the driver for HIGMACV300 Ethernet controller found on HiSilicon SoCs like Hi3798CV200. It's based on a downstream U-Boot driver, but quite a lot of code gets rewritten and cleaned up to adopt driver model and PHY API.
Signed-off-by: Shawn Guo shawn.guo@linaro.org
drivers/net/Kconfig | 9 + drivers/net/Makefile | 1 + drivers/net/higmacv300.c | 592 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 602 insertions(+) create mode 100644 drivers/net/higmacv300.c
<snip>
+#define MDIO_WRITE 1 +#define MDIO_READ 2 +#define MDIO_COMMAND(rw, addr, reg) (BIT_MDIO_BUSY | \
(((rw) & 0x3) << 16) | \(((addr) & 0x1f) << 8) | \((reg) & 0x1f))+#define MDIO_WRITE_CMD(addr, reg) MDIO_COMMAND(MDIO_WRITE, addr, reg) +#define MDIO_READ_CMD(addr, reg) MDIO_COMMAND(MDIO_READ, addr, reg)
This is a strange construct... can you just inline this into the actual functions? I don't see the value in these macros that hide what's happening.
Yes, I can eliminate the macros with inline code. That's actually what kernel driver does.
+enum higmac_queue {
RX_FQ,RX_BQ,TX_BQ,TX_RQ,+};
<snip>
+static int higmac_free_pkt(struct udevice *dev, uchar *packet, int length) +{
if (packet)free(packet);Why free them and reallocate them? Won't that just fragment memory?
Practically it will not fragment the memory, as all the buffers get a fixed size and allocation/free always happens as a pair for given network transaction. But I still think it's a good suggestion to get all buffers allocated at initialization time, so that we do not need to allocate/free buffers repeatedly.
Also, you should be somehow telling the MAC that the buffer / descriptor is no longer in use here.
Sensible suggestion.
return 0;+}
+static int higmac_recv(struct udevice *dev, int flags, uchar **packetp) +{
struct higmac_priv *priv = dev_get_priv(dev);struct higmac_desc *fqd = priv->rxfq;struct higmac_desc *bqd = priv->rxbq;int fqw_pos, fqr_pos, bqw_pos, bqr_pos;int timeout = 100000;unsigned long addr;int len = 0;int space;int i;fqw_pos = DESC_CNT(readl(priv->base + RX_FQ_WR_ADDR));fqr_pos = DESC_CNT(readl(priv->base + RX_FQ_RD_ADDR));if (fqw_pos >= fqr_pos)space = RX_DESC_NUM - (fqw_pos - fqr_pos);elsespace = fqr_pos - fqw_pos;/* Leave one free to distinguish full filled from empty buffer */for (i = 0; i < space - 1; i++) {void *buf = memalign(ARCH_DMA_MINALIGN, MAC_MAX_FRAME_SIZE);if (!buf)break;fqd = priv->rxfq + fqw_pos;fqd->buf_addr = (unsigned long)buf;invalidate_dcache_range(fqd->buf_addr,fqd->buf_addr + MAC_MAX_FRAME_SIZE);fqd->descvid = DESC_VLD_FREE;fqd->buf_len = MAC_MAX_FRAME_SIZE - 1;flush_desc(fqd);if (++fqw_pos >= RX_DESC_NUM)fqw_pos = 0;writel(DESC_BYTE(fqw_pos), priv->base + RX_FQ_WR_ADDR);}bqr_pos = DESC_CNT(readl(priv->base + RX_BQ_RD_ADDR));bqd += bqr_pos;/* BQ is only ever written by GMAC */invalidate_desc(bqd);do {bqw_pos = DESC_CNT(readl(priv->base + RX_BQ_WR_ADDR));udelay(1);} while (--timeout && bqw_pos == bqr_pos);if (!timeout)return -ETIMEDOUT;if (++bqr_pos >= RX_DESC_NUM)bqr_pos = 0;len = bqd->data_len;/* CPU should not have touched this buffer since we added it to FQ */addr = bqd->buf_addr;invalidate_dcache_range(addr, addr + len);*packetp = (void *)addr;writel(DESC_BYTE(bqr_pos), priv->base + RX_BQ_RD_ADDR);Does this belong in free_pkt()?
Yeah, I guess that's what you mean by telling MAC the descriptors are not longer used above.
return len;+}
<snip>
+static int higmac_start(struct udevice *dev) +{
struct higmac_priv *priv = dev_get_priv(dev);struct phy_device *phydev = priv->phydev;int ret;ret = phy_startup(phydev);if (ret)return ret;if (!phydev->link) {debug("%s: link down\n", phydev->dev->name);return -ENODEV;}ret = higmac_adjust_link(priv);if (ret)return ret;/* Enable port */writel(0xf, priv->base + DESC_WR_RD_ENA);What is 0xf? No magic numbers please.
Okay, will define it.
writel(BIT_TX_EN | BIT_RX_EN, priv->base + PORT_EN);return 0;+}
+static void higmac_stop(struct udevice *dev) +{
struct higmac_priv *priv = dev_get_priv(dev);/* Disable port */writel(0, priv->base + PORT_EN);writel(0, priv->base + DESC_WR_RD_ENA);+}
+static const struct eth_ops higmac_ops = {
.start = higmac_start,.send = higmac_send,.recv = higmac_recv,.free_pkt = higmac_free_pkt,.stop = higmac_stop,.write_hwaddr = higmac_write_hwaddr,+};
+static int higmac_wait_mdio_ready(struct higmac_priv *priv) +{
int timeout = 1000;while (--timeout) {/* Wait until busy bit is cleared */if ((readl(priv->base + MDIO_SINGLE_CMD) & BIT_MDIO_BUSY) == 0)break;udelay(10);}It would be good if you retrofit to use wait_bit or its macros throughout this driver instead of hand-writing it.
I didn't know that before. Thanks for the pointer. Will do.
return timeout ? 0 : -ETIMEDOUT;+}
<snip>
+static int higmac_probe(struct udevice *dev) +{
struct higmac_priv *priv = dev_get_priv(dev);struct phy_device *phydev;struct mii_dev *bus;int ret;ret = higmac_hw_init(priv);if (ret)return ret;bus = mdio_alloc();if (!bus)return -ENOMEM;bus->read = higmac_mdio_read;bus->write = higmac_mdio_write;bus->priv = priv;priv->bus = bus;ret = mdio_register_seq(bus, dev->seq);if (ret)return ret;phydev = phy_connect(bus, priv->phyaddr, dev, priv->phyintf);if (!phydev)return -ENODEV;phydev->supported &= PHY_GBIT_FEATURES;I would expect either phydev->supported &= ~PHY_GBIT_FEATURES; or phydev->supported |= PHY_GBIT_FEATURES;
To be honest, this code was copied from other drivers. It's all over the drivers under driver/net. The phydev->supported gets initialized as phydev->drv->features, and we want to make sure feature bits are set on both sides, no?
Shawn
phydev->advertising = phydev->supported;priv->phydev = phydev;ret = phy_config(phydev);if (ret)return ret;return 0;+}
On Mon, Jan 28, 2019 at 05:13:55PM +0800, Shawn Guo wrote:
It adds the driver for HIGMACV300 Ethernet controller found on HiSilicon SoCs like Hi3798CV200. It's based on a downstream U-Boot driver, but quite a lot of code gets rewritten and cleaned up to adopt driver model and PHY API.
Signed-off-by: Shawn Guo shawn.guo@linaro.org
Please note that you forgot drivers/net/ocelot_switch.c with this patch.
On Fri, Feb 08, 2019 at 07:13:21PM -0500, Tom Rini wrote:
On Mon, Jan 28, 2019 at 05:13:55PM +0800, Shawn Guo wrote:
It adds the driver for HIGMACV300 Ethernet controller found on HiSilicon SoCs like Hi3798CV200. It's based on a downstream U-Boot driver, but quite a lot of code gets rewritten and cleaned up to adopt driver model and PHY API.
Signed-off-by: Shawn Guo shawn.guo@linaro.org
Please note that you forgot drivers/net/ocelot_switch.c with this patch.
Thanks for reminding, Tom. I will rebase to latest master and get the move of ocelot_switch driver considered.
Shawn
The 'phy' reset of gmac device in kernel device tree is not generic enough for u-boot to use, so we need to overwrite the 'resets' property as needed. With this device tree fixup and poplar_defconfig changes, Ethernet starts working on Poplar board.
Signed-off-by: Shawn Guo shawn.guo@linaro.org --- arch/arm/dts/hi3798cv200-u-boot.dtsi | 6 ++++++ configs/poplar_defconfig | 3 +++ 2 files changed, 9 insertions(+)
diff --git a/arch/arm/dts/hi3798cv200-u-boot.dtsi b/arch/arm/dts/hi3798cv200-u-boot.dtsi index 7844c5208c5d..972ea67de3b1 100644 --- a/arch/arm/dts/hi3798cv200-u-boot.dtsi +++ b/arch/arm/dts/hi3798cv200-u-boot.dtsi @@ -16,6 +16,12 @@ }; };
+&gmac1 { + resets = <&crg 0xcc 9>, + <&crg 0xcc 11>, + <&crg 0xcc 13>; +}; + &uart0 { clock = <75000000>; status = "okay"; diff --git a/configs/poplar_defconfig b/configs/poplar_defconfig index 81bd3702e42a..f07fe94a0ad1 100644 --- a/configs/poplar_defconfig +++ b/configs/poplar_defconfig @@ -19,6 +19,9 @@ CONFIG_FASTBOOT_FLASH_MMC_DEV=0 CONFIG_DM_MMC=y CONFIG_MMC_DW=y CONFIG_MMC_DW_K3=y +CONFIG_DM_ETH=y +CONFIG_NET_HIGMACV300=y +CONFIG_RESET_HISILICON=y CONFIG_USB=y CONFIG_USB_EHCI_HCD=y CONFIG_USB_EHCI_GENERIC=y
Reviewed-by: Igor Opaniuk igor.opaniuk@linaro.org
On Mon, 28 Jan 2019 at 11:15, Shawn Guo shawn.guo@linaro.org wrote:
The 'phy' reset of gmac device in kernel device tree is not generic enough for u-boot to use, so we need to overwrite the 'resets' property as needed. With this device tree fixup and poplar_defconfig changes, Ethernet starts working on Poplar board.
Signed-off-by: Shawn Guo shawn.guo@linaro.org
arch/arm/dts/hi3798cv200-u-boot.dtsi | 6 ++++++ configs/poplar_defconfig | 3 +++ 2 files changed, 9 insertions(+)
diff --git a/arch/arm/dts/hi3798cv200-u-boot.dtsi b/arch/arm/dts/hi3798cv200-u-boot.dtsi index 7844c5208c5d..972ea67de3b1 100644 --- a/arch/arm/dts/hi3798cv200-u-boot.dtsi +++ b/arch/arm/dts/hi3798cv200-u-boot.dtsi @@ -16,6 +16,12 @@ }; };
+&gmac1 {
resets = <&crg 0xcc 9>,<&crg 0xcc 11>,<&crg 0xcc 13>;+};
&uart0 { clock = <75000000>; status = "okay"; diff --git a/configs/poplar_defconfig b/configs/poplar_defconfig index 81bd3702e42a..f07fe94a0ad1 100644 --- a/configs/poplar_defconfig +++ b/configs/poplar_defconfig @@ -19,6 +19,9 @@ CONFIG_FASTBOOT_FLASH_MMC_DEV=0 CONFIG_DM_MMC=y CONFIG_MMC_DW=y CONFIG_MMC_DW_K3=y +CONFIG_DM_ETH=y +CONFIG_NET_HIGMACV300=y +CONFIG_RESET_HISILICON=y CONFIG_USB=y CONFIG_USB_EHCI_HCD=y CONFIG_USB_EHCI_GENERIC=y -- 2.18.0
participants (4)
-
Igor Opaniuk -
Joe Hershberger -
Shawn Guo -
Tom Rini