[U-Boot] [U-BOOT] [RFC PATCH] EMAC driver for H3/A64
LABBE Corentin
clabbe.montjoie at gmail.com
Mon Apr 25 10:44:29 CEST 2016
On Mon, Apr 25, 2016 at 12:34:06AM +0530, Amit Singh Tomar wrote:
> Hello
>
> I wanted to contribute and in the process trying to add support for the driver for EMAC controller found on H3/A64 based SoC.
>
> Sorry, I don't want to post it at this moment but I am kind of stuck from last few days.
>
> What is working:
> - Able to establish the link between host and Organepi pc(H3).
> - Tx on host side getting incremented.
> - When ping from Orangepi PC to host machine , I see ONE
> packet is trasnmitted over wire but its bad packet with following output
> after running tcpdump on host
>
> 20:41:49.148111 00:00:00:00:00:00 (oui Ethernet) > 00:00:00:00:00:00 (oui Ethernet) Null Information, send seq 0, rcv seq 0, Flags [Command], length 46
> 0x0000: 0000 0000 0000 0000 0000 0000 0000 0000 ................
> 0x0010: 0000 0000 0000 0000 0000 0000 0000 0000 ................
> 0x0020: 0000 0000 0000 0000 0000 0000 0000 0000 ................
> 0x0030: 0000 0000 0000 0000 0000 0000 ............
>
> What needs to be done:
> - Why bad packet is transmitted over the wire from Orangepi pc to Host ?
> - Make it compatible with external PHY.
>
> This Patch is based on LABBE Corentin's and Chen-Yu Tsai's work done for Linux Kernel(Thanks to both of them).
>
> Signed-off-by: Amit Singh Tomar <amittomer25 at gmail.com>
> ---
> arch/arm/dts/sun8i-h3-orangepi-pc.dts | 10 +
> arch/arm/dts/sun8i-h3.dtsi | 7 +
> configs/orangepi_pc_defconfig | 2 +
> drivers/net/Makefile | 1 +
> drivers/net/sun8i_emac.c | 619 +++++++++++++++++++++++++++++++++
> 5 files changed, 639 insertions(+)
> create mode 100644 drivers/net/sun8i_emac.c
>
Hello
I have some comments below
> +
> +#define CONFIG_TX_DESCR_NUM 128
> +#define CONFIG_RX_DESCR_NUM 64
> +#define CONFIG_ETH_BUFSIZE 2048
I do not think you will doing Jumbo frame, so 2048 is too much
Anyway, I have got strange reaction from hw with this value (Even BSP use 2047)
> +#define TX_TOTAL_BUFSIZE (CONFIG_ETH_BUFSIZE * CONFIG_TX_DESCR_NUM)
> +#define RX_TOTAL_BUFSIZE (CONFIG_ETH_BUFSIZE * CONFIG_RX_DESCR_NUM)
> +
> +#define REG_DEFAULT_VALUE 0x58000
> +#define REG_DEFAULT_MASK GENMASK(31, 15)
> +
> +#define REG_PHY_ADDR_SHIFT 20
> +#define REG_PHY_ADDR_MASK GENMASK(4, 0)
> +#define REG_LED_POL BIT(17) /* 1: active low, 0: active high */
> +#define REG_SHUTDOWN BIT(16) /* 1: shutdown, 0: power up */
> +#define REG_PHY_SELECT BIT(15) /* 1: internal PHY, 0: external PHY */
> +
> +
> +#define RX_BUF_SZ (2048*64)
Unused, and if used, use the proper define (CONFIG_RX_DESCR_NUM/CONFIG_ETH_BUFSIZE)
> +
> +#define CONFIG_MDIO_TIMEOUT (3 * CONFIG_SYS_HZ)
> +
> +#define AHB_GATE_OFFSET_GMAC 17
> +#define AHB_GATE_OFFSET_EPHY 0
> +
> +#define SUN8I_GPD8_GMAC 4
> +
> +/* H3/A64 EMAC Register Base*/
> +#define SUN8I_EMAC_BASE 0x01c30000
> +
> +/* H3/A64 EMAC Register's offset */
> +#define CTL0 0x00
> +#define CTL1 0x04
> +#define INT_STA 0x08
> +#define INT_EN 0x0c
> +#define TX_CTL0 0x10
> +#define TX_CTL1 0x14
> +#define TX_FLOW_CTL 0x1c
> +#define TX_DMA_DESC 0x20
> +#define RX_CTL0 0x24
> +#define RX_CTL1 0x28
> +#define RX_DMA_DESC 0x34
> +#define MII_CMD 0x48
> +#define MII_DATA 0x4c
> +#define ADDR0_HIGH 0x50
> +#define ADDR0_LOW 0x54
> +#define TX_DMA_STA 0xb0
> +#define TX_CUR_DESC 0xb4
> +#define TX_CUR_BUF 0xb8
> +#define RX_DMA_STA 0xc0
> +#define RX_CUR_DESC 0xc4
> +#define TX_CUR_BUF 0xc8
> +
> +/* CCM Register base */
> +#define SUN8I_CCM_BASE 0x01c20000
> +
> +/* CCM Regsiter's offset */
typo
your patch have some typos, run aspell on it
> +#define CLK_GATING_REG0 0x60
> +#define CLK_GATING_REG1 0x64
> +#define CLK_GATING_REG2 0x68
> +#define CLK_GATING_REG3 0x6c
> +#define CLK_GATING_REG4 0x70
> +
> +/* System Control Register base */
> +#define SUN8I_SCTL_BASE 0x01c00000
> +
> +/* System Control Register's offset */
> +#define VER_REG 0x24
> +#define EMAC_CLK_REG 0x30
> +
> +struct emac_dma_desc {
> + u32 status;
> + u32 st;
> + void *buf_addr;
> + struct emac_dma_desc *next;
> +} __aligned(4);
> +
> +struct emac_eth_dev {
> + struct emac_dma_desc rx_chain[CONFIG_TX_DESCR_NUM];
> + struct emac_dma_desc tx_chain[CONFIG_RX_DESCR_NUM];
> + char rxbuffer[RX_TOTAL_BUFSIZE] __aligned(ARCH_DMA_MINALIGN);
> + char txbuffer[TX_TOTAL_BUFSIZE] __aligned(ARCH_DMA_MINALIGN);
> +
> + u32 interface;
> + u32 link;
> + u32 speed;
> + u32 duplex;
> + u32 phy_configured;
> + u32 tx_currdescnum;
> + u32 rx_currdescnum;
> + u32 addr;
> + u32 tx_slot;
> + void *mac_reg;
> + void *sysctl_reg;
> + void *ccu_reg;
> + struct phy_device *phydev;
> + struct mii_dev *bus;
> +};
> +
> +static int sun8i_mdio_read(struct mii_dev *bus, int addr, int devad, int reg)
> +{
> + struct emac_eth_dev *priv = bus->priv;
> + ulong start;
> + u32 miiaddr;
> + int timeout = CONFIG_MDIO_TIMEOUT;
> +
> + /*miiaddr = ((addr << MDIO_CMD_MII_PHY_ADDR_SHIFT) & MDIO_CMD_MII_PHY_ADDR_MASK ) |
> + ((reg << MDIO_CMD_MII_PHY_REG_ADDR_SHIFT) & MDIO_CMD_MII_PHY_REG_ADDR_MASK);
> +
> + writel(miiaddr | MDIO_CMD_MII_BUSY, &mac_p->miicmd);*/
> + miiaddr &= ~MDIO_CMD_MII_WRITE;
> + miiaddr &= ~MDIO_CMD_MII_PHY_REG_ADDR_MASK;
> + miiaddr |= (reg << MDIO_CMD_MII_PHY_REG_ADDR_SHIFT) &
> + MDIO_CMD_MII_PHY_REG_ADDR_MASK;
> +
> + miiaddr &= ~MDIO_CMD_MII_PHY_ADDR_MASK;
> +
> + miiaddr |= (addr << MDIO_CMD_MII_PHY_ADDR_SHIFT) &
> + MDIO_CMD_MII_PHY_ADDR_MASK;
> +
> + miiaddr |= MDIO_CMD_MII_BUSY;
> +
> + writel(miiaddr, priv->mac_reg + MII_CMD);
> +
> + start = get_timer(0);
> + while (get_timer(start) < timeout) {
> + if (!(readl(priv->mac_reg + MII_CMD) & MDIO_CMD_MII_BUSY))
> + return readl(priv->mac_reg + MII_DATA);
> + udelay(10);
> + };
> +
> + return -1;
> +}
> +
> +static int sun8i_mdio_write(struct mii_dev *bus, int addr, int devad, int reg,
> + u16 val)
> +{
> + struct emac_eth_dev *priv = bus->priv;
> + ulong start;
> + u32 miiaddr;
> + int ret = -1, timeout = CONFIG_MDIO_TIMEOUT;
> +
> + /*writel(val, &mac_p->miidata);
> + miiaddr = ((addr << MIIADDRSHIFT) & MII_ADDRMSK) |
> + ((reg << MIIREGSHIFT) & MII_REGMSK) | MII_WRITE;
> +
> + writel(miiaddr | MII_BUSY, &mac_p->miiaddr);*/
> +
> + miiaddr &= ~MDIO_CMD_MII_PHY_REG_ADDR_MASK;
> + miiaddr |= (reg << MDIO_CMD_MII_PHY_REG_ADDR_SHIFT) &
> + MDIO_CMD_MII_PHY_REG_ADDR_MASK;
> +
> + miiaddr &= ~MDIO_CMD_MII_PHY_ADDR_MASK;
> + miiaddr |= (addr << MDIO_CMD_MII_PHY_ADDR_SHIFT) &
> + MDIO_CMD_MII_PHY_ADDR_MASK;
> +
> + miiaddr |= MDIO_CMD_MII_WRITE;
> + miiaddr |= MDIO_CMD_MII_BUSY;
> +
> + writel(miiaddr, priv->mac_reg + MII_CMD);
> + writel(val, priv->mac_reg + MII_DATA);
> +
> + start = get_timer(0);
> + while (get_timer(start) < timeout) {
> + if (!(readl(priv->mac_reg + MII_CMD) & MDIO_CMD_MII_BUSY)) {
> + ret = 0;
> + break;
> + }
> + udelay(10);
> + };
> +
> + return ret;
> +}
> +
> +static int sun8i_phy_init(struct emac_eth_dev *priv, void *dev)
> +{
> + u32 val;
> + int mask = 0xffffffff;
> +
> +#ifdef CONFIG_PHY_ADDR
> + mask = 1 << CONFIG_PHY_ADDR;
> +#endif
> + struct phy_device *phydev;
> +
> + /* H3 based SoC has an Internal PHY that needs
> + to be configured and powered up befoe use
> + */
> +
> + val = readl(priv->sysctl_reg + EMAC_CLK_REG);
> + val = (val & ~REG_DEFAULT_MASK) | REG_DEFAULT_VALUE;
> +
> + val |= CONFIG_PHY_ADDR << REG_PHY_ADDR_SHIFT;
> + val &= ~REG_SHUTDOWN;
> + val |= REG_PHY_SELECT;
> +
> + setbits_le32((priv->sysctl_reg + EMAC_CLK_REG), (1 << 13));
> + writel(val, (priv->sysctl_reg + EMAC_CLK_REG));
> +
> + phydev = phy_find_by_mask(priv->bus, mask,
> + PHY_INTERFACE_MODE_MII);
> + if (!phydev)
> + return -ENODEV;
> +
> + phy_connect_dev(phydev, dev);
> +
> + priv->phydev = phydev;
> + phy_config(phydev);
> +
> + return 0;
> +}
> +
> +static void sun8i_adjust_link(struct emac_eth_dev *priv,
> + struct phy_device *phydev)
> +{
> + u32 v;
> + v = readl(priv->mac_reg + CTL0);
> +
> + if (phydev->duplex)
> + v |= BIT(0);
> + else
> + v &= ~BIT(0);
> +
> + v &= ~0x0C;
> +
> + switch (phydev->speed) {
> + case 1000:
> + break;
> + case 100:
> + v |= BIT(2);
> + v |= BIT(3);
> + break;
> + case 10:
> + v |= BIT(3);
> + break;
> + }
> + writel(v, priv->mac_reg + CTL0);
> +}
> +
> +static void rx_descs_init(struct emac_eth_dev *priv)
> +{
> + struct emac_dma_desc *desc_table_p = &priv->rx_chain[0];
> + char *rxbuffs = &priv->rxbuffer[0];
> + struct emac_dma_desc *desc_p;
> + u32 idx;
> +
> + flush_dcache_range((unsigned int)rxbuffs, (unsigned int)rxbuffs +
> + RX_TOTAL_BUFSIZE);
> +
> + for (idx = 0; idx < CONFIG_RX_DESCR_NUM; idx++) {
> + desc_p = &desc_table_p[idx];
> + desc_p->buf_addr = &rxbuffs[idx * CONFIG_ETH_BUFSIZE];
> + desc_p->next = &desc_table_p[idx + 1];
> + }
> +
> + /* Correcting the last pointer of the chain */
> + desc_p->next = &desc_table_p[0];
> + flush_dcache_range((unsigned int)priv->rx_chain,
> + (unsigned int)priv->rx_chain +
> + sizeof(priv->rx_chain));
> +
> + writel((ulong)&desc_table_p[0], (priv->mac_reg + RX_DMA_DESC));
> + priv->rx_currdescnum = 0;
> +}
> +
> +static void tx_descs_init(struct emac_eth_dev *priv)
> +{
> + struct emac_dma_desc *desc_table_p = &priv->tx_chain[0];
> + char *txbuffs = &priv->txbuffer[0];
> + struct emac_dma_desc *desc_p;
> + u32 idx;
> +
> + for (idx = 0; idx < CONFIG_TX_DESCR_NUM; idx++) {
> + desc_p = &desc_table_p[idx];
> + desc_p->buf_addr = &txbuffs[idx * CONFIG_ETH_BUFSIZE];
> + desc_p->next = &desc_table_p[idx + 1];
> + desc_p->status = 0;
> + desc_p->st = 0;
> + }
> +
> + /* Correcting the last pointer of the chain */
> + desc_p->next = &desc_table_p[0];
> + flush_dcache_range((unsigned int)priv->tx_chain,
> + (unsigned int)priv->tx_chain +
> + sizeof(priv->tx_chain));
> +
> + writel((ulong)&desc_table_p[0], priv->mac_reg + TX_DMA_DESC);
> + priv->tx_currdescnum = 0;
> +}
> +
> +static int _sun8i_emac_eth_init(struct emac_eth_dev *priv, u8 *enetaddr)
> +{
> + u32 reg, v;
> + priv->tx_slot = 0;
> +
> + reg = readl((priv->mac_reg + CTL1));
> +
> + if (!(reg & 0x1)) {
> + /* Soft reset MAC */
> + setbits_le32((priv->mac_reg + CTL1), 0x1 << 0);
> + do {
> + reg = readl(priv->mac_reg + CTL1);
> + } while ((reg & 0x01) != 0);
> + }
You need to setup a timeout
> +
> + /* Rewrite mac addres after reset */
> + writel((enetaddr[3] << 24 | enetaddr[2] << 16 | enetaddr[1] << 8
> + | enetaddr[0]), priv->mac_reg + ADDR0_LOW);
> + writel((enetaddr[5] << 8 | enetaddr[4]), priv->mac_reg + ADDR0_HIGH);
Why not using sun8i_write_hwaddr ?
> +
> + v = readl(priv->mac_reg + TX_CTL1);
> + /* TX_MD Transmission starts after a full frame located in TX DMA FIFO*/
> + v |= BIT(1);
> + writel(v, priv->mac_reg + TX_CTL1);
> +
> + v = readl(priv->mac_reg + RX_CTL1);
> + /* RX_MD RX DMA reads data from RX DMA FIFO to host memory after a
> + * complete frame has been written to RX DMA FIFO
> + */
> + v |= BIT(1);
> + writel(v, priv->mac_reg + RX_CTL1);
> +
> + /* DMA */
> + writel(8 << 24 , priv->mac_reg + CTL1);
> +
> + /* Enable recieve and trasnmit interupt */
Double typo and I do not see any interrupt handling in this file
> + writel(RX_INT | TX_INT | TX_UNF_INT, (priv->mac_reg + INT_EN));
> +
> + /* Intialize rx/tx descriptors */
> + rx_descs_init(priv);
> + tx_descs_init(priv);
> +
> + /*Start up the PHY */
> + if (phy_startup(priv->phydev)) {
> + printf("Could not initialize PHY %s\n",
> + priv->phydev->dev->name);
> + return -1;
> + }
> +
> + sun8i_adjust_link(priv, priv->phydev);
> +
> + /* start RX DMA */
> + v = readl(priv->mac_reg + RX_CTL1);
> + v |= BIT(30);
> + writel(v, priv->mac_reg + RX_CTL1);
> +
> + v = readl(priv->mac_reg + TX_CTL1);
> + v |= BIT(30);
> + writel(v, priv->mac_reg + TX_CTL1);
> +
> + /* Enable RX/TX */
> + setbits_le32(priv->mac_reg + RX_CTL0, 0x1 << 31);
> + setbits_le32(priv->mac_reg + TX_CTL0, 0x1 << 31);
> +
> + writel(0x3FFF, priv->mac_reg + INT_STA);
> +
> + return 0;
> +}
> +
> +static void sun8i_emac_board_setup(struct emac_eth_dev *priv)
> +{
> + priv->ccu_reg = (void *)SUN8I_CCU_BASE;
> + priv->sysctl_reg = (void *)SUN8I_SCTL_BASE;
> + int pin;
> +
> + /* Set clock gating for emac */
> + setbits_le32((priv->ccu_reg + CLK_GATING_REG0),
> + 0x1 << AHB_GATE_OFFSET_GMAC);
> +
> + /* Set clock gating for ephy */
> + setbits_le32((priv->ccu_reg + CLK_GATING_REG4),
> + 0x1 << AHB_GATE_OFFSET_EPHY);
> +
> + /* Set Tx clock source as MII with rate 25MZ */
> + clrbits_le32((priv->sysctl_reg + EMAC_CLK_REG),
> + SCTL_EMAC_TX_CLK_SRC_MII |
> + SCTL_EMAC_EPIT_MII | SCTL_EMAC_CLK_SEL);
> +
> + /* Set EMAC clock */
> + setbits_le32((priv->ccu_reg + AHB2_CFG_REG), (0x1 << 1) | (0x1 << 0));
> +
> + /* Deassert EPHY */
> + setbits_le32((priv->ccu_reg + BUS_SOFT_RST_REG2), EPHY_RST);
> +
> + /* Deassert EMAC */
> + setbits_le32((priv->ccu_reg + BUS_SOFT_RST_REG0), EMAC_RST);
> +
> + /* Pin mux setting for emac */
> + for (pin = SUNXI_GPD(8); pin <= SUNXI_GPD(22); pin++)
> + sunxi_gpio_set_cfgpin(pin, SUN8I_GPD8_GMAC);
> +}
> +
> +static int _sun8i_eth_recv(struct emac_eth_dev *priv, uchar **packetp)
> +{
> + u32 status, desc_num = priv->rx_currdescnum;
> + struct emac_dma_desc *desc_p = &priv->rx_chain[desc_num];
> + int length = -EAGAIN;
> + uint32_t desc_start = (uint32_t)desc_p;
> + uint32_t desc_end = desc_start +
> + roundup(sizeof(*desc_p), ARCH_DMA_MINALIGN);
> +
> + uint32_t data_start = (uint32_t)desc_p->buf_addr;
> + uint32_t data_end;
> +
> + /* Invalidate entire buffer descriptor */
> + invalidate_dcache_range(desc_start, desc_end);
> +
> + status = desc_p->status;
> +
> + length = (desc_p->status >> 16) & 0x3FFF;
> +
> + data_end = data_start + roundup(length, ARCH_DMA_MINALIGN);
> + invalidate_dcache_range(data_start, data_end);
> + *packetp = desc_p->buf_addr;
> +
> + return length;
> +}
I do not understand why you do not check for BIT(31)(DMA own)
> +
> +static int _sun8i_emac_eth_send(struct emac_eth_dev *priv, void *packet,
> + int len)
> +{
> + u32 v, status, desc_num = priv->tx_currdescnum;
> + struct emac_dma_desc *desc_p = &priv->tx_chain[desc_num];
> +
> + int length = -EAGAIN;
not used
> + uint32_t desc_start = (uint32_t)desc_p;
> + uint32_t desc_end = desc_start +
> + roundup(sizeof(*desc_p), ARCH_DMA_MINALIGN);
> +
> + uint32_t data_start = (uint32_t)desc_p->buf_addr;
> + uint32_t data_end;
> +
> + /* Invalidate entire buffer descriptor */
> + invalidate_dcache_range(desc_start, desc_end);
> +
> + desc_p->st = len;
> + desc_p->st |= 1<<24;
> + desc_p->st |= 1<<29;
Use BIT()
> +
> + memcpy((unsigned *)desc_p->buf_addr, packet, len);
> +
> + flush_dcache_range(data_start, data_end);
> +
> + desc_p->st |= BIT(31);
> + desc_p->st |= BIT(30);
> +
> + desc_p->st |= 1<<29;
Why setting this twice ?
> + desc_p->status = 1<<31;
Use BIT() and put a comment of meaning of thoses bits.
For your send error, try using the same order than my driver (bit 29)
Problem with sending I got was about write order and cache.
Regards
More information about the U-Boot
mailing list