Re: [U-Boot] [U-BOOT] [RFC PATCH] EMAC driver for H3/A64

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@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