* [U-Boot] [PATCH 0/3] Add Ethernet support for Poplar board
@ 2019-01-28 9:13 Shawn Guo
2019-01-28 9:13 ` [U-Boot] [PATCH 1/3] reset: add reset driver for HiSilicon platform Shawn Guo
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Shawn Guo @ 2019-01-28 9:13 UTC (permalink / raw)
To: u-boot
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
--
2.18.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH 1/3] reset: add reset driver for HiSilicon platform
2019-01-28 9:13 [U-Boot] [PATCH 0/3] Add Ethernet support for Poplar board Shawn Guo
@ 2019-01-28 9:13 ` Shawn Guo
2019-01-29 21:57 ` Igor Opaniuk
2019-01-28 9:13 ` [U-Boot] [PATCH 2/3] net: add higmacv300 Ethernet " Shawn Guo
2019-01-28 9:13 ` [U-Boot] [PATCH 3/3] poplar: enable Ethernet driver support Shawn Guo
2 siblings, 1 reply; 12+ messages in thread
From: Shawn Guo @ 2019-01-28 9:13 UTC (permalink / raw)
To: u-boot
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),
+};
--
2.18.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH 2/3] net: add higmacv300 Ethernet driver for HiSilicon platform
2019-01-28 9:13 [U-Boot] [PATCH 0/3] Add Ethernet support for Poplar board Shawn Guo
2019-01-28 9:13 ` [U-Boot] [PATCH 1/3] reset: add reset driver for HiSilicon platform Shawn Guo
@ 2019-01-28 9:13 ` Shawn Guo
2019-01-29 21:50 ` Igor Opaniuk
` (2 more replies)
2019-01-28 9:13 ` [U-Boot] [PATCH 3/3] poplar: enable Ethernet driver support Shawn Guo
2 siblings, 3 replies; 12+ messages in thread
From: Shawn Guo @ 2019-01-28 9:13 UTC (permalink / raw)
To: u-boot
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),
+};
--
2.18.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH 3/3] poplar: enable Ethernet driver support
2019-01-28 9:13 [U-Boot] [PATCH 0/3] Add Ethernet support for Poplar board Shawn Guo
2019-01-28 9:13 ` [U-Boot] [PATCH 1/3] reset: add reset driver for HiSilicon platform Shawn Guo
2019-01-28 9:13 ` [U-Boot] [PATCH 2/3] net: add higmacv300 Ethernet " Shawn Guo
@ 2019-01-28 9:13 ` Shawn Guo
2019-01-29 21:52 ` Igor Opaniuk
2 siblings, 1 reply; 12+ messages in thread
From: Shawn Guo @ 2019-01-28 9:13 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH 2/3] net: add higmacv300 Ethernet driver for HiSilicon platform
2019-01-28 9:13 ` [U-Boot] [PATCH 2/3] net: add higmacv300 Ethernet " Shawn Guo
@ 2019-01-29 21:50 ` Igor Opaniuk
2019-01-31 10:17 ` Shawn Guo
2019-02-04 23:53 ` Joe Hershberger
2019-02-09 0:13 ` [U-Boot] [U-Boot, " Tom Rini
2 siblings, 1 reply; 12+ messages in thread
From: Igor Opaniuk @ 2019-01-29 21:50 UTC (permalink / raw)
To: u-boot
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_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
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);
> + 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;
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;
> + 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;
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
>
--
Regards,
Igor Opaniuk
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH 3/3] poplar: enable Ethernet driver support
2019-01-28 9:13 ` [U-Boot] [PATCH 3/3] poplar: enable Ethernet driver support Shawn Guo
@ 2019-01-29 21:52 ` Igor Opaniuk
0 siblings, 0 replies; 12+ messages in thread
From: Igor Opaniuk @ 2019-01-29 21:52 UTC (permalink / raw)
To: u-boot
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
>
--
Regards,
Igor Opaniuk
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH 1/3] reset: add reset driver for HiSilicon platform
2019-01-28 9:13 ` [U-Boot] [PATCH 1/3] reset: add reset driver for HiSilicon platform Shawn Guo
@ 2019-01-29 21:57 ` Igor Opaniuk
0 siblings, 0 replies; 12+ messages in thread
From: Igor Opaniuk @ 2019-01-29 21:57 UTC (permalink / raw)
To: u-boot
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_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),
> +};
> --
> 2.18.0
>
--
Regards,
Igor Opaniuk
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH 2/3] net: add higmacv300 Ethernet driver for HiSilicon platform
2019-01-29 21:50 ` Igor Opaniuk
@ 2019-01-31 10:17 ` Shawn Guo
0 siblings, 0 replies; 12+ messages in thread
From: Shawn Guo @ 2019-01-31 10:17 UTC (permalink / raw)
To: u-boot
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_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
> 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);
> > + 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;
>
> 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;
> > + 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;
>
> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH 2/3] net: add higmacv300 Ethernet driver for HiSilicon platform
2019-01-28 9:13 ` [U-Boot] [PATCH 2/3] net: add higmacv300 Ethernet " Shawn Guo
2019-01-29 21:50 ` Igor Opaniuk
@ 2019-02-04 23:53 ` Joe Hershberger
2019-02-13 11:46 ` Shawn Guo
2019-02-09 0:13 ` [U-Boot] [U-Boot, " Tom Rini
2 siblings, 1 reply; 12+ messages in thread
From: Joe Hershberger @ 2019-02-04 23:53 UTC (permalink / raw)
To: u-boot
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_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)
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);
> + 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);
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;
> + 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);
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 at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [U-Boot, 2/3] net: add higmacv300 Ethernet driver for HiSilicon platform
2019-01-28 9:13 ` [U-Boot] [PATCH 2/3] net: add higmacv300 Ethernet " Shawn Guo
2019-01-29 21:50 ` Igor Opaniuk
2019-02-04 23:53 ` Joe Hershberger
@ 2019-02-09 0:13 ` Tom Rini
2019-02-13 7:21 ` Shawn Guo
2 siblings, 1 reply; 12+ messages in thread
From: Tom Rini @ 2019-02-09 0:13 UTC (permalink / raw)
To: u-boot
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.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190208/9586a341/attachment.sig>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [U-Boot, 2/3] net: add higmacv300 Ethernet driver for HiSilicon platform
2019-02-09 0:13 ` [U-Boot] [U-Boot, " Tom Rini
@ 2019-02-13 7:21 ` Shawn Guo
0 siblings, 0 replies; 12+ messages in thread
From: Shawn Guo @ 2019-02-13 7:21 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH 2/3] net: add higmacv300 Ethernet driver for HiSilicon platform
2019-02-04 23:53 ` Joe Hershberger
@ 2019-02-13 11:46 ` Shawn Guo
0 siblings, 0 replies; 12+ messages in thread
From: Shawn Guo @ 2019-02-13 11:46 UTC (permalink / raw)
To: 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);
> > + 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);
>
> 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;
> > +}
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-02-13 11:46 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-28 9:13 [U-Boot] [PATCH 0/3] Add Ethernet support for Poplar board Shawn Guo
2019-01-28 9:13 ` [U-Boot] [PATCH 1/3] reset: add reset driver for HiSilicon platform Shawn Guo
2019-01-29 21:57 ` Igor Opaniuk
2019-01-28 9:13 ` [U-Boot] [PATCH 2/3] net: add higmacv300 Ethernet " Shawn Guo
2019-01-29 21:50 ` Igor Opaniuk
2019-01-31 10:17 ` Shawn Guo
2019-02-04 23:53 ` Joe Hershberger
2019-02-13 11:46 ` Shawn Guo
2019-02-09 0:13 ` [U-Boot] [U-Boot, " Tom Rini
2019-02-13 7:21 ` Shawn Guo
2019-01-28 9:13 ` [U-Boot] [PATCH 3/3] poplar: enable Ethernet driver support Shawn Guo
2019-01-29 21:52 ` Igor Opaniuk
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.