* [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 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.