* [U-Boot] [PATCH v2 0/3] Add Ethernet support for Poplar board
@ 2019-02-18 3:37 Shawn Guo
2019-02-18 3:37 ` [U-Boot] [PATCH v2 1/3] reset: add reset driver for HiSilicon platform Shawn Guo
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Shawn Guo @ 2019-02-18 3:37 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.
Changes for v2:
- Rename driver symbol to HIGMACV300_ETH.
- Remove the use of temp variable 'addr' in higmac_recv().
- Simplify the return of function higmac_ofdata_to_platdata() and
higmac_probe().
- Combine delaration and initialization for phyintf in function
higmac_ofdata_to_platdata().
- Eliminate the MDIO read/write macros.
- Use wait_for_bit_le32() for MDIO command completion polling.
- Set up RX packet buffers in RX_FQ descriptor at initialization time,
so that we do not need to allocate/free packet buffers repeatedly.
- Inform GMAC that the RX descriptor is no longer in use in function
higmac_free_pkt().
- Define BITS_DESC_ENA instead of using magic number 0xf.
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 | 597 +++++++++++++++++++++++++++
drivers/reset/Kconfig | 6 +
drivers/reset/Makefile | 1 +
drivers/reset/reset-hisilicon.c | 95 +++++
8 files changed, 718 insertions(+)
create mode 100644 drivers/net/higmacv300.c
create mode 100644 drivers/reset/reset-hisilicon.c
--
2.18.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH v2 1/3] reset: add reset driver for HiSilicon platform
2019-02-18 3:37 [U-Boot] [PATCH v2 0/3] Add Ethernet support for Poplar board Shawn Guo
@ 2019-02-18 3:37 ` Shawn Guo
2019-02-18 3:37 ` [U-Boot] [PATCH v2 2/3] net: add higmacv300 Ethernet " Shawn Guo
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Shawn Guo @ 2019-02-18 3:37 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>
Reviewed-by: Igor Opaniuk <igor.opaniuk@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] 9+ messages in thread
* [U-Boot] [PATCH v2 2/3] net: add higmacv300 Ethernet driver for HiSilicon platform
2019-02-18 3:37 [U-Boot] [PATCH v2 0/3] Add Ethernet support for Poplar board Shawn Guo
2019-02-18 3:37 ` [U-Boot] [PATCH v2 1/3] reset: add reset driver for HiSilicon platform Shawn Guo
@ 2019-02-18 3:37 ` Shawn Guo
2019-03-05 23:58 ` Joe Hershberger
2019-02-18 3:37 ` [U-Boot] [PATCH v2 3/3] poplar: enable Ethernet driver support Shawn Guo
2019-03-04 6:28 ` [U-Boot] [PATCH v2 0/3] Add Ethernet support for Poplar board Shawn Guo
3 siblings, 1 reply; 9+ messages in thread
From: Shawn Guo @ 2019-02-18 3:37 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 | 597 +++++++++++++++++++++++++++++++++++++++
3 files changed, 607 insertions(+)
create mode 100644 drivers/net/higmacv300.c
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 6a570285aac5..ad1e50c0e8ca 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -525,4 +525,13 @@ config MEDIATEK_ETH
This Driver support MediaTek Ethernet GMAC
Say Y to enable support for the MediaTek Ethernet GMAC.
+config HIGMACV300_ETH
+ 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 51be72b0aa86..8d02a378964b 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-y += mscc_eswitch/
+obj-$(CONFIG_HIGMACV300_ETH) += higmacv300.o
diff --git a/drivers/net/higmacv300.c b/drivers/net/higmacv300.c
new file mode 100644
index 000000000000..549c26b19b99
--- /dev/null
+++ b/drivers/net/higmacv300.c
@@ -0,0 +1,597 @@
+// 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>
+#include <wait_bit.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_READ (BIT(17) | BIT_MDIO_BUSY)
+#define MDIO_WRITE (BIT(16) | BIT_MDIO_BUSY)
+#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
+#define BIT_RX_OUTCFF_WR BIT(3)
+#define BIT_RX_CFF_RD BIT(2)
+#define BIT_TX_OUTCFF_WR BIT(1)
+#define BIT_TX_CFF_RD BIT(0)
+#define BITS_DESC_ENA (BIT_RX_OUTCFF_WR | BIT_RX_CFF_RD | \
+ BIT_TX_OUTCFF_WR | BIT_TX_CFF_RD)
+
+/* 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
+
+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;
+ int rxdesc_in_use;
+ 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)
+{
+ struct higmac_priv *priv = dev_get_priv(dev);
+
+ /* Inform GMAC that the RX descriptor is no longer in use */
+ writel(DESC_BYTE(priv->rxdesc_in_use), priv->base + RX_BQ_RD_ADDR);
+
+ 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;
+ 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++) {
+ fqd = priv->rxfq + fqw_pos;
+ invalidate_dcache_range(fqd->buf_addr,
+ fqd->buf_addr + MAC_MAX_FRAME_SIZE);
+
+ 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 */
+ invalidate_dcache_range(bqd->buf_addr, bqd->buf_addr + len);
+ *packetp = (void *)(unsigned long)bqd->buf_addr;
+
+ /* Record the RX_BQ descriptor that is holding RX data */
+ priv->rxdesc_in_use = bqr_pos;
+
+ 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(BITS_DESC_ENA, 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_mdio_read(struct mii_dev *bus, int addr, int devad, int reg)
+{
+ struct higmac_priv *priv = bus->priv;
+ int ret;
+
+ ret = wait_for_bit_le32(priv->base + MDIO_SINGLE_CMD, BIT_MDIO_BUSY,
+ false, 1000, false);
+ if (ret)
+ return ret;
+
+ writel(MDIO_READ | addr << 8 | reg, priv->base + MDIO_SINGLE_CMD);
+
+ ret = wait_for_bit_le32(priv->base + MDIO_SINGLE_CMD, BIT_MDIO_BUSY,
+ false, 1000, false);
+ 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 = wait_for_bit_le32(priv->base + MDIO_SINGLE_CMD, BIT_MDIO_BUSY,
+ false, 1000, false);
+ if (ret)
+ return ret;
+
+ writel(value, priv->base + MDIO_SINGLE_DATA);
+ writel(MDIO_WRITE | addr << 8 | reg, priv->base + MDIO_SINGLE_CMD);
+
+ return 0;
+}
+
+static int higmac_init_rx_descs(struct higmac_desc *descs, int num)
+{
+ int i;
+
+ for (i = 0; i < num; i++) {
+ struct higmac_desc *desc = &descs[i];
+
+ desc->buf_addr = (unsigned long)memalign(ARCH_DMA_MINALIGN,
+ MAC_MAX_FRAME_SIZE);
+ if (!desc->buf_addr)
+ goto free_bufs;
+
+ desc->descvid = DESC_VLD_FREE;
+ desc->buf_len = MAC_MAX_FRAME_SIZE - 1;
+ flush_desc(desc);
+ }
+
+ return 0;
+
+free_bufs:
+ while (--i > 0)
+ free((void *)(unsigned long)descs[i].buf_addr);
+ return -ENOMEM;
+}
+
+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;
+
+ /* Set up RX_FQ descriptors */
+ if (queue == RX_FQ)
+ higmac_init_rx_descs(desc, depth);
+
+ /* 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;
+
+ return phy_config(phydev);
+}
+
+static int higmac_remove(struct udevice *dev)
+{
+ struct higmac_priv *priv = dev_get_priv(dev);
+ int i;
+
+ mdio_unregister(priv->bus);
+ mdio_free(priv->bus);
+
+ /* Free RX packet buffers */
+ for (i = 0; i < RX_DESC_NUM; i++)
+ free((void *)(unsigned long)priv->rxfq[i].buf_addr);
+
+ return 0;
+}
+
+static int higmac_ofdata_to_platdata(struct udevice *dev)
+{
+ struct higmac_priv *priv = dev_get_priv(dev);
+ int phyintf = PHY_INTERFACE_MODE_NONE;
+ const char *phy_mode;
+ ofnode phy_node;
+
+ priv->base = dev_remap_addr_index(dev, 0);
+ priv->macif_ctrl = dev_remap_addr_index(dev, 1);
+
+ 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);
+
+ return reset_get_by_name(dev, "phy", &priv->rst_phy);
+}
+
+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] 9+ messages in thread
* [U-Boot] [PATCH v2 3/3] poplar: enable Ethernet driver support
2019-02-18 3:37 [U-Boot] [PATCH v2 0/3] Add Ethernet support for Poplar board Shawn Guo
2019-02-18 3:37 ` [U-Boot] [PATCH v2 1/3] reset: add reset driver for HiSilicon platform Shawn Guo
2019-02-18 3:37 ` [U-Boot] [PATCH v2 2/3] net: add higmacv300 Ethernet " Shawn Guo
@ 2019-02-18 3:37 ` Shawn Guo
2019-03-04 6:28 ` [U-Boot] [PATCH v2 0/3] Add Ethernet support for Poplar board Shawn Guo
3 siblings, 0 replies; 9+ messages in thread
From: Shawn Guo @ 2019-02-18 3:37 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>
Reviewed-by: Igor Opaniuk <igor.opaniuk@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..76ab5eb70e7e 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_HIGMACV300_ETH=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] 9+ messages in thread
* [U-Boot] [PATCH v2 0/3] Add Ethernet support for Poplar board
2019-02-18 3:37 [U-Boot] [PATCH v2 0/3] Add Ethernet support for Poplar board Shawn Guo
` (2 preceding siblings ...)
2019-02-18 3:37 ` [U-Boot] [PATCH v2 3/3] poplar: enable Ethernet driver support Shawn Guo
@ 2019-03-04 6:28 ` Shawn Guo
3 siblings, 0 replies; 9+ messages in thread
From: Shawn Guo @ 2019-03-04 6:28 UTC (permalink / raw)
To: u-boot
On Mon, Feb 18, 2019 at 11:37:39AM +0800, Shawn Guo wrote:
> 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.
>
> Changes for v2:
> - Rename driver symbol to HIGMACV300_ETH.
> - Remove the use of temp variable 'addr' in higmac_recv().
> - Simplify the return of function higmac_ofdata_to_platdata() and
> higmac_probe().
> - Combine delaration and initialization for phyintf in function
> higmac_ofdata_to_platdata().
> - Eliminate the MDIO read/write macros.
> - Use wait_for_bit_le32() for MDIO command completion polling.
> - Set up RX packet buffers in RX_FQ descriptor at initialization time,
> so that we do not need to allocate/free packet buffers repeatedly.
> - Inform GMAC that the RX descriptor is no longer in use in function
> higmac_free_pkt().
> - Define BITS_DESC_ENA instead of using magic number 0xf.
Hi Joe,
Does this version look good to you?
Shawn
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH v2 2/3] net: add higmacv300 Ethernet driver for HiSilicon platform
2019-02-18 3:37 ` [U-Boot] [PATCH v2 2/3] net: add higmacv300 Ethernet " Shawn Guo
@ 2019-03-05 23:58 ` Joe Hershberger
2019-03-06 1:41 ` Shawn Guo
0 siblings, 1 reply; 9+ messages in thread
From: Joe Hershberger @ 2019-03-05 23:58 UTC (permalink / raw)
To: u-boot
On Sun, Feb 17, 2019 at 9:39 PM 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>
Looks pretty good. I have a few questions / comments below.
> ---
> drivers/net/Kconfig | 9 +
> drivers/net/Makefile | 1 +
> drivers/net/higmacv300.c | 597 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 607 insertions(+)
> create mode 100644 drivers/net/higmacv300.c
>
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index 6a570285aac5..ad1e50c0e8ca 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -525,4 +525,13 @@ config MEDIATEK_ETH
> This Driver support MediaTek Ethernet GMAC
> Say Y to enable support for the MediaTek Ethernet GMAC.
>
> +config HIGMACV300_ETH
> + 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 51be72b0aa86..8d02a378964b 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-y += mscc_eswitch/
> +obj-$(CONFIG_HIGMACV300_ETH) += higmacv300.o
> diff --git a/drivers/net/higmacv300.c b/drivers/net/higmacv300.c
> new file mode 100644
> index 000000000000..549c26b19b99
> --- /dev/null
> +++ b/drivers/net/higmacv300.c
> @@ -0,0 +1,597 @@
> +// 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>
> +#include <wait_bit.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_READ (BIT(17) | BIT_MDIO_BUSY)
> +#define MDIO_WRITE (BIT(16) | BIT_MDIO_BUSY)
> +#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
> +#define BIT_RX_OUTCFF_WR BIT(3)
> +#define BIT_RX_CFF_RD BIT(2)
> +#define BIT_TX_OUTCFF_WR BIT(1)
> +#define BIT_TX_CFF_RD BIT(0)
> +#define BITS_DESC_ENA (BIT_RX_OUTCFF_WR | BIT_RX_CFF_RD | \
> + BIT_TX_OUTCFF_WR | BIT_TX_CFF_RD)
> +
> +/* 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
> +
> +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;
> + int rxdesc_in_use;
> + 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)
> +{
> + struct higmac_priv *priv = dev_get_priv(dev);
> +
> + /* Inform GMAC that the RX descriptor is no longer in use */
> + writel(DESC_BYTE(priv->rxdesc_in_use), priv->base + RX_BQ_RD_ADDR);
> +
> + 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;
> + 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++) {
> + fqd = priv->rxfq + fqw_pos;
> + invalidate_dcache_range(fqd->buf_addr,
> + fqd->buf_addr + MAC_MAX_FRAME_SIZE);
> +
> + 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);
Did you look into using wait bit macros?
> +
> + 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 */
> + invalidate_dcache_range(bqd->buf_addr, bqd->buf_addr + len);
> + *packetp = (void *)(unsigned long)bqd->buf_addr;
> +
> + /* Record the RX_BQ descriptor that is holding RX data */
> + priv->rxdesc_in_use = bqr_pos;
> +
> + 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);
Same here on wait bit.
> +
> + 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(BITS_DESC_ENA, 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_mdio_read(struct mii_dev *bus, int addr, int devad, int reg)
> +{
> + struct higmac_priv *priv = bus->priv;
> + int ret;
> +
> + ret = wait_for_bit_le32(priv->base + MDIO_SINGLE_CMD, BIT_MDIO_BUSY,
> + false, 1000, false);
> + if (ret)
> + return ret;
> +
> + writel(MDIO_READ | addr << 8 | reg, priv->base + MDIO_SINGLE_CMD);
> +
> + ret = wait_for_bit_le32(priv->base + MDIO_SINGLE_CMD, BIT_MDIO_BUSY,
> + false, 1000, false);
> + 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 = wait_for_bit_le32(priv->base + MDIO_SINGLE_CMD, BIT_MDIO_BUSY,
> + false, 1000, false);
> + if (ret)
> + return ret;
> +
> + writel(value, priv->base + MDIO_SINGLE_DATA);
> + writel(MDIO_WRITE | addr << 8 | reg, priv->base + MDIO_SINGLE_CMD);
> +
> + return 0;
> +}
> +
> +static int higmac_init_rx_descs(struct higmac_desc *descs, int num)
> +{
> + int i;
> +
> + for (i = 0; i < num; i++) {
> + struct higmac_desc *desc = &descs[i];
> +
> + desc->buf_addr = (unsigned long)memalign(ARCH_DMA_MINALIGN,
> + MAC_MAX_FRAME_SIZE);
> + if (!desc->buf_addr)
> + goto free_bufs;
> +
> + desc->descvid = DESC_VLD_FREE;
> + desc->buf_len = MAC_MAX_FRAME_SIZE - 1;
> + flush_desc(desc);
> + }
> +
> + return 0;
> +
> +free_bufs:
> + while (--i > 0)
> + free((void *)(unsigned long)descs[i].buf_addr);
> + return -ENOMEM;
> +}
> +
> +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;
> +
> + /* Set up RX_FQ descriptors */
> + if (queue == RX_FQ)
> + higmac_init_rx_descs(desc, depth);
> +
> + /* 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);
I'm surprised the delay here is not a DT parameter.
> + reset_deassert(&priv->rst_phy);
> + mdelay(30);
I'm surprised the delay here is not a DT parameter.
> + reset_assert(&priv->rst_phy);
> + mdelay(30);
Why is this reasserted?
> +
> + 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;
> +
> + return phy_config(phydev);
> +}
> +
> +static int higmac_remove(struct udevice *dev)
> +{
> + struct higmac_priv *priv = dev_get_priv(dev);
> + int i;
> +
> + mdio_unregister(priv->bus);
> + mdio_free(priv->bus);
> +
> + /* Free RX packet buffers */
> + for (i = 0; i < RX_DESC_NUM; i++)
> + free((void *)(unsigned long)priv->rxfq[i].buf_addr);
> +
> + return 0;
> +}
> +
> +static int higmac_ofdata_to_platdata(struct udevice *dev)
> +{
> + struct higmac_priv *priv = dev_get_priv(dev);
> + int phyintf = PHY_INTERFACE_MODE_NONE;
> + const char *phy_mode;
> + ofnode phy_node;
> +
> + priv->base = dev_remap_addr_index(dev, 0);
> + priv->macif_ctrl = dev_remap_addr_index(dev, 1);
> +
> + phy_mode = dev_read_string(dev, "phy-mode");
Should there not be a default? Is it supposed to be required to be
specified in the DT?
> + 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);
> +
> + return reset_get_by_name(dev, "phy", &priv->rst_phy);
> +}
> +
> +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] 9+ messages in thread
* [U-Boot] [PATCH v2 2/3] net: add higmacv300 Ethernet driver for HiSilicon platform
2019-03-05 23:58 ` Joe Hershberger
@ 2019-03-06 1:41 ` Shawn Guo
2019-03-06 20:48 ` Joe Hershberger
0 siblings, 1 reply; 9+ messages in thread
From: Shawn Guo @ 2019-03-06 1:41 UTC (permalink / raw)
To: u-boot
On Tue, Mar 05, 2019 at 11:58:26PM +0000, Joe Hershberger wrote:
> > +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;
> > + 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++) {
> > + fqd = priv->rxfq + fqw_pos;
> > + invalidate_dcache_range(fqd->buf_addr,
> > + fqd->buf_addr + MAC_MAX_FRAME_SIZE);
> > +
> > + 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);
>
> Did you look into using wait bit macros?
I may miss your point, but this is not a loop waiting for some bits set
or clear. It's waiting for a given number.
>
> > +
> > + 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 */
> > + invalidate_dcache_range(bqd->buf_addr, bqd->buf_addr + len);
> > + *packetp = (void *)(unsigned long)bqd->buf_addr;
> > +
> > + /* Record the RX_BQ descriptor that is holding RX data */
> > + priv->rxdesc_in_use = bqr_pos;
> > +
> > + return len;
> > +}
<snip>
> > +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);
>
> I'm surprised the delay here is not a DT parameter.
We do not see the necessity for now. We can make it a DT parameter when
we see the real need in the future.
>
> > + reset_deassert(&priv->rst_phy);
> > + mdelay(30);
>
> I'm surprised the delay here is not a DT parameter.
>
> > + reset_assert(&priv->rst_phy);
> > + mdelay(30);
>
> Why is this reasserted?
I have to admit this is a bit hackish. Ideally, the reset sequence
should be: deassert -> assert -> deassert. But this reset signal gets
an opposite polarity than others that reset driver handles. I can add a
comment to explain it if you can tolerate this little hack, or I will
add polarity support to reset driver to handle this phy reset quirk.
Please let me know your preference.
>
> > +
> > + 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;
> > +
> > + return phy_config(phydev);
> > +}
> > +
> > +static int higmac_remove(struct udevice *dev)
> > +{
> > + struct higmac_priv *priv = dev_get_priv(dev);
> > + int i;
> > +
> > + mdio_unregister(priv->bus);
> > + mdio_free(priv->bus);
> > +
> > + /* Free RX packet buffers */
> > + for (i = 0; i < RX_DESC_NUM; i++)
> > + free((void *)(unsigned long)priv->rxfq[i].buf_addr);
> > +
> > + return 0;
> > +}
> > +
> > +static int higmac_ofdata_to_platdata(struct udevice *dev)
> > +{
> > + struct higmac_priv *priv = dev_get_priv(dev);
> > + int phyintf = PHY_INTERFACE_MODE_NONE;
> > + const char *phy_mode;
> > + ofnode phy_node;
> > +
> > + priv->base = dev_remap_addr_index(dev, 0);
> > + priv->macif_ctrl = dev_remap_addr_index(dev, 1);
> > +
> > + phy_mode = dev_read_string(dev, "phy-mode");
>
> Should there not be a default? Is it supposed to be required to be
> specified in the DT?
Yes, we choose to implement it as a required property. If the property
is missing, the device probe would fail.
Shawn
>
> > + 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);
> > +
> > + return reset_get_by_name(dev, "phy", &priv->rst_phy);
> > +}
> > +
> > +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] 9+ messages in thread
* [U-Boot] [PATCH v2 2/3] net: add higmacv300 Ethernet driver for HiSilicon platform
2019-03-06 1:41 ` Shawn Guo
@ 2019-03-06 20:48 ` Joe Hershberger
2019-03-07 8:29 ` Shawn Guo
0 siblings, 1 reply; 9+ messages in thread
From: Joe Hershberger @ 2019-03-06 20:48 UTC (permalink / raw)
To: u-boot
On Tue, Mar 5, 2019 at 7:42 PM Shawn Guo <shawn.guo@linaro.org> wrote:
>
> On Tue, Mar 05, 2019 at 11:58:26PM +0000, Joe Hershberger wrote:
> > > +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;
> > > + 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++) {
> > > + fqd = priv->rxfq + fqw_pos;
> > > + invalidate_dcache_range(fqd->buf_addr,
> > > + fqd->buf_addr + MAC_MAX_FRAME_SIZE);
> > > +
> > > + 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);
> >
> > Did you look into using wait bit macros?
>
> I may miss your point, but this is not a loop waiting for some bits set
> or clear. It's waiting for a given number.
OK, I see that, thanks. Should you make these "breakable" in the same
way that wait_for_bit_* does? The timeout seems quite long.
>
> >
> > > +
> > > + 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 */
> > > + invalidate_dcache_range(bqd->buf_addr, bqd->buf_addr + len);
> > > + *packetp = (void *)(unsigned long)bqd->buf_addr;
> > > +
> > > + /* Record the RX_BQ descriptor that is holding RX data */
> > > + priv->rxdesc_in_use = bqr_pos;
> > > +
> > > + return len;
> > > +}
>
> <snip>
>
> > > +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);
> >
> > I'm surprised the delay here is not a DT parameter.
>
> We do not see the necessity for now. We can make it a DT parameter when
> we see the real need in the future.
OK
>
> >
> > > + reset_deassert(&priv->rst_phy);
> > > + mdelay(30);
> >
> > I'm surprised the delay here is not a DT parameter.
> >
> > > + reset_assert(&priv->rst_phy);
> > > + mdelay(30);
> >
> > Why is this reasserted?
>
> I have to admit this is a bit hackish. Ideally, the reset sequence
> should be: deassert -> assert -> deassert. But this reset signal gets
> an opposite polarity than others that reset driver handles. I can add a
> comment to explain it if you can tolerate this little hack, or I will
> add polarity support to reset driver to handle this phy reset quirk.
> Please let me know your preference.
I would prefer a polarity to be defined in the DT for this reset.
>
> >
> > > +
> > > + 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;
> > > +
> > > + return phy_config(phydev);
> > > +}
> > > +
> > > +static int higmac_remove(struct udevice *dev)
> > > +{
> > > + struct higmac_priv *priv = dev_get_priv(dev);
> > > + int i;
> > > +
> > > + mdio_unregister(priv->bus);
> > > + mdio_free(priv->bus);
> > > +
> > > + /* Free RX packet buffers */
> > > + for (i = 0; i < RX_DESC_NUM; i++)
> > > + free((void *)(unsigned long)priv->rxfq[i].buf_addr);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int higmac_ofdata_to_platdata(struct udevice *dev)
> > > +{
> > > + struct higmac_priv *priv = dev_get_priv(dev);
> > > + int phyintf = PHY_INTERFACE_MODE_NONE;
> > > + const char *phy_mode;
> > > + ofnode phy_node;
> > > +
> > > + priv->base = dev_remap_addr_index(dev, 0);
> > > + priv->macif_ctrl = dev_remap_addr_index(dev, 1);
> > > +
> > > + phy_mode = dev_read_string(dev, "phy-mode");
> >
> > Should there not be a default? Is it supposed to be required to be
> > specified in the DT?
>
> Yes, we choose to implement it as a required property. If the property
> is missing, the device probe would fail.
OK.
Thanks,
-Joe
>
> Shawn
>
> >
> > > + 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);
> > > +
> > > + return reset_get_by_name(dev, "phy", &priv->rst_phy);
> > > +}
> > > +
> > > +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
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH v2 2/3] net: add higmacv300 Ethernet driver for HiSilicon platform
2019-03-06 20:48 ` Joe Hershberger
@ 2019-03-07 8:29 ` Shawn Guo
0 siblings, 0 replies; 9+ messages in thread
From: Shawn Guo @ 2019-03-07 8:29 UTC (permalink / raw)
To: u-boot
On Wed, Mar 06, 2019 at 08:48:40PM +0000, Joe Hershberger wrote:
> On Tue, Mar 5, 2019 at 7:42 PM Shawn Guo <shawn.guo@linaro.org> wrote:
> >
> > On Tue, Mar 05, 2019 at 11:58:26PM +0000, Joe Hershberger wrote:
> > > > +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;
> > > > + 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++) {
> > > > + fqd = priv->rxfq + fqw_pos;
> > > > + invalidate_dcache_range(fqd->buf_addr,
> > > > + fqd->buf_addr + MAC_MAX_FRAME_SIZE);
> > > > +
> > > > + 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);
> > >
> > > Did you look into using wait bit macros?
> >
> > I may miss your point, but this is not a loop waiting for some bits set
> > or clear. It's waiting for a given number.
>
> OK, I see that, thanks. Should you make these "breakable" in the same
> way that wait_for_bit_* does? The timeout seems quite long.
I assume that you mean the "breakable" as user interaction (CTRL-C)?
I'm not sure 100000 us (0.1 s) is so long for user to break.
<snip>
> > > > + reset_assert(&priv->rst_phy);
> > > > + mdelay(30);
> > >
> > > Why is this reasserted?
> >
> > I have to admit this is a bit hackish. Ideally, the reset sequence
> > should be: deassert -> assert -> deassert. But this reset signal gets
> > an opposite polarity than others that reset driver handles. I can add a
> > comment to explain it if you can tolerate this little hack, or I will
> > add polarity support to reset driver to handle this phy reset quirk.
> > Please let me know your preference.
>
> I would prefer a polarity to be defined in the DT for this reset.
OK, I will implement it in v3. Thanks for the comment.
Shawn
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-03-07 8:29 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-18 3:37 [U-Boot] [PATCH v2 0/3] Add Ethernet support for Poplar board Shawn Guo
2019-02-18 3:37 ` [U-Boot] [PATCH v2 1/3] reset: add reset driver for HiSilicon platform Shawn Guo
2019-02-18 3:37 ` [U-Boot] [PATCH v2 2/3] net: add higmacv300 Ethernet " Shawn Guo
2019-03-05 23:58 ` Joe Hershberger
2019-03-06 1:41 ` Shawn Guo
2019-03-06 20:48 ` Joe Hershberger
2019-03-07 8:29 ` Shawn Guo
2019-02-18 3:37 ` [U-Boot] [PATCH v2 3/3] poplar: enable Ethernet driver support Shawn Guo
2019-03-04 6:28 ` [U-Boot] [PATCH v2 0/3] Add Ethernet support for Poplar board Shawn Guo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox