2015-11-10 16:14:46

by Måns Rullgård

[permalink] [raw]
Subject: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

This adds a driver for the Aurora VLSI NB8800 Ethernet controller.
It is an almost complete rewrite of a driver originally found in
a Sigma Designs 2.6.22 tree.

Signed-off-by: Mans Rullgard <[email protected]>
---
Changes:
- Refactored mdio access functions
- Refactored register access helpers
- Improved error handling in rx buffer allocation
- Optimised some fifo parameters
- Overhauled tx dma. Multiple packets are now chained in a single dma
operation if xmit_more is set, improving performance.
- Improved rx irq handling. It's not possible to disable interrupts
entirely for napi poll, but they can be slowed down a little.
- Use readx_poll_timeout in various places
- Improved error detection
- Improved statistics
- Report hardware statistics counters through ethtool
- Improved tangox-specific setup
- Support for flow control using pause frames
- Explanatory comments added
- Various minor stylistic changes
---
drivers/net/ethernet/Kconfig | 1 +
drivers/net/ethernet/Makefile | 1 +
drivers/net/ethernet/aurora/Kconfig | 20 +
drivers/net/ethernet/aurora/Makefile | 1 +
drivers/net/ethernet/aurora/nb8800.c | 1530 ++++++++++++++++++++++++++++++++++
drivers/net/ethernet/aurora/nb8800.h | 314 +++++++
6 files changed, 1867 insertions(+)
create mode 100644 drivers/net/ethernet/aurora/Kconfig
create mode 100644 drivers/net/ethernet/aurora/Makefile
create mode 100644 drivers/net/ethernet/aurora/nb8800.c
create mode 100644 drivers/net/ethernet/aurora/nb8800.h

diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
index 05aa759..8310163 100644
--- a/drivers/net/ethernet/Kconfig
+++ b/drivers/net/ethernet/Kconfig
@@ -29,6 +29,7 @@ source "drivers/net/ethernet/apm/Kconfig"
source "drivers/net/ethernet/apple/Kconfig"
source "drivers/net/ethernet/arc/Kconfig"
source "drivers/net/ethernet/atheros/Kconfig"
+source "drivers/net/ethernet/aurora/Kconfig"
source "drivers/net/ethernet/cadence/Kconfig"
source "drivers/net/ethernet/adi/Kconfig"
source "drivers/net/ethernet/broadcom/Kconfig"
diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile
index ddfc808..b435fb0 100644
--- a/drivers/net/ethernet/Makefile
+++ b/drivers/net/ethernet/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_NET_XGENE) += apm/
obj-$(CONFIG_NET_VENDOR_APPLE) += apple/
obj-$(CONFIG_NET_VENDOR_ARC) += arc/
obj-$(CONFIG_NET_VENDOR_ATHEROS) += atheros/
+obj-$(CONFIG_NET_VENDOR_AURORA) += aurora/
obj-$(CONFIG_NET_CADENCE) += cadence/
obj-$(CONFIG_NET_BFIN) += adi/
obj-$(CONFIG_NET_VENDOR_BROADCOM) += broadcom/
diff --git a/drivers/net/ethernet/aurora/Kconfig b/drivers/net/ethernet/aurora/Kconfig
new file mode 100644
index 0000000..a3c7106
--- /dev/null
+++ b/drivers/net/ethernet/aurora/Kconfig
@@ -0,0 +1,20 @@
+config NET_VENDOR_AURORA
+ bool "Aurora VLSI devices"
+ help
+ If you have a network (Ethernet) device belonging to this class,
+ say Y.
+
+ Note that the answer to this question doesn't directly affect the
+ kernel: saying N will just cause the configurator to skip all
+ questions about Aurora devices. If you say Y, you will be asked
+ for your specific device in the following questions.
+
+if NET_VENDOR_AURORA
+
+config AURORA_NB8800
+ tristate "Aurora AU-NB8800 support"
+ select PHYLIB
+ help
+ Support for the AU-NB8800 gigabit Ethernet controller.
+
+endif
diff --git a/drivers/net/ethernet/aurora/Makefile b/drivers/net/ethernet/aurora/Makefile
new file mode 100644
index 0000000..6cb528a
--- /dev/null
+++ b/drivers/net/ethernet/aurora/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_AURORA_NB8800) += nb8800.o
diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
new file mode 100644
index 0000000..11cd389
--- /dev/null
+++ b/drivers/net/ethernet/aurora/nb8800.c
@@ -0,0 +1,1530 @@
+/*
+ * Copyright (C) 2015 Mans Rullgard <[email protected]>
+ *
+ * Mostly rewritten, based on driver from Sigma Designs. Original
+ * copyright notice below.
+ *
+ *
+ * Driver for tangox SMP864x/SMP865x/SMP867x/SMP868x builtin Ethernet Mac.
+ *
+ * Copyright (C) 2005 Maxime Bizon <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/etherdevice.h>
+#include <linux/delay.h>
+#include <linux/ethtool.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/of_device.h>
+#include <linux/of_mdio.h>
+#include <linux/of_net.h>
+#include <linux/dma-mapping.h>
+#include <linux/phy.h>
+#include <linux/cache.h>
+#include <linux/jiffies.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <asm/barrier.h>
+
+#include "nb8800.h"
+
+static int nb8800_dma_stop(struct net_device *dev);
+
+static inline u8 nb8800_readb(struct nb8800_priv *priv, int reg)
+{
+ return readb(priv->base + reg);
+}
+
+static inline u32 nb8800_readl(struct nb8800_priv *priv, int reg)
+{
+ return readl(priv->base + reg);
+}
+
+static inline void nb8800_writeb(struct nb8800_priv *priv, int reg, u8 val)
+{
+ writeb(val, priv->base + reg);
+}
+
+static inline void nb8800_writew(struct nb8800_priv *priv, int reg, u16 val)
+{
+ writew(val, priv->base + reg);
+}
+
+static inline void nb8800_writel(struct nb8800_priv *priv, int reg, u32 val)
+{
+ writel(val, priv->base + reg);
+}
+
+static inline void nb8800_maskb(struct nb8800_priv *priv, int reg,
+ u32 mask, u32 val)
+{
+ u32 old = nb8800_readb(priv, reg);
+ u32 new = (old & ~mask) | val;
+ if (new != old)
+ nb8800_writeb(priv, reg, new);
+}
+
+static inline void nb8800_maskl(struct nb8800_priv *priv, int reg,
+ u32 mask, u32 val)
+{
+ u32 old = nb8800_readl(priv, reg);
+ u32 new = (old & ~mask) | val;
+ if (new != old)
+ nb8800_writel(priv, reg, new);
+}
+
+static inline void nb8800_modb(struct nb8800_priv *priv, int reg, u8 bits,
+ bool set)
+{
+ nb8800_maskb(priv, reg, bits, set ? bits : 0);
+}
+
+static inline void nb8800_setb(struct nb8800_priv *priv, int reg, u8 bits)
+{
+ nb8800_maskb(priv, reg, bits, bits);
+}
+
+static inline void nb8800_clearb(struct nb8800_priv *priv, int reg, u8 bits)
+{
+ nb8800_maskb(priv, reg, bits, 0);
+}
+
+static inline void nb8800_modl(struct nb8800_priv *priv, int reg, u32 bits,
+ bool set)
+{
+ nb8800_maskl(priv, reg, bits, set ? bits : 0);
+}
+
+static inline void nb8800_setl(struct nb8800_priv *priv, int reg, u32 bits)
+{
+ nb8800_maskl(priv, reg, bits, bits);
+}
+
+static inline void nb8800_clearl(struct nb8800_priv *priv, int reg, u32 bits)
+{
+ nb8800_maskl(priv, reg, bits, 0);
+}
+
+static int nb8800_mdio_wait(struct mii_bus *bus)
+{
+ struct nb8800_priv *priv = bus->priv;
+ u32 val;
+
+ return readl_poll_timeout_atomic(priv->base + NB8800_MDIO_CMD,
+ val, !(val & MDIO_CMD_GO), 1, 1000);
+}
+
+static int nb8800_mdio_cmd(struct mii_bus *bus, u32 cmd)
+{
+ struct nb8800_priv *priv = bus->priv;
+ int err;
+
+ err = nb8800_mdio_wait(bus);
+ if (err)
+ return err;
+
+ nb8800_writel(priv, NB8800_MDIO_CMD, cmd);
+ udelay(10);
+ nb8800_writel(priv, NB8800_MDIO_CMD, cmd | MDIO_CMD_GO);
+
+ return nb8800_mdio_wait(bus);
+}
+
+static int nb8800_mdio_read(struct mii_bus *bus, int phy_id, int reg)
+{
+ struct nb8800_priv *priv = bus->priv;
+ u32 val;
+ int err;
+
+ err = nb8800_mdio_cmd(bus, MIIAR_ADDR(phy_id) | MIIAR_REG(reg));
+ if (err)
+ return err;
+
+ val = nb8800_readl(priv, NB8800_MDIO_STS);
+ if (val & MDIO_STS_ERR)
+ return 0xffff;
+
+ return val & 0xffff;
+}
+
+static int nb8800_mdio_write(struct mii_bus *bus, int phy_id, int reg, u16 val)
+{
+ u32 cmd = MIIAR_DATA(val) | MIIAR_ADDR(phy_id) | MIIAR_REG(reg) |
+ MDIO_CMD_WR;
+
+ return nb8800_mdio_cmd(bus, cmd);
+}
+
+static void nb8800_mac_tx(struct net_device *dev, bool enable)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+
+ while (nb8800_readl(priv, NB8800_TXC_CR) & TCR_EN)
+ cpu_relax();
+
+ nb8800_modb(priv, NB8800_TX_CTL1, TX_EN, enable);
+}
+
+static void nb8800_mac_rx(struct net_device *dev, bool enable)
+{
+ nb8800_modb(netdev_priv(dev), NB8800_RX_CTL, RX_EN, enable);
+}
+
+static void nb8800_mac_af(struct net_device *dev, bool enable)
+{
+ nb8800_modb(netdev_priv(dev), NB8800_RX_CTL, RX_AF_EN, enable);
+}
+
+static void nb8800_start_rx(struct net_device *dev)
+{
+ nb8800_setl(netdev_priv(dev), NB8800_RXC_CR, RCR_EN);
+}
+
+static int nb8800_alloc_rx(struct net_device *dev, int i, bool napi)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+ struct nb8800_rx_desc *rxd = &priv->rx_descs[i];
+ struct nb8800_rx_buf *rxb = &priv->rx_bufs[i];
+ int size = L1_CACHE_ALIGN(RX_BUF_SIZE);
+ dma_addr_t dma_addr;
+ struct page *page;
+ unsigned long offset;
+ void *data;
+
+ data = napi ? napi_alloc_frag(size) : netdev_alloc_frag(size);
+ if (!data)
+ return -ENOMEM;
+
+ page = virt_to_head_page(data);
+ offset = data - page_address(page);
+
+ dma_addr = dma_map_page(&dev->dev, page, offset, RX_BUF_SIZE,
+ DMA_FROM_DEVICE);
+
+ if (dma_mapping_error(&dev->dev, dma_addr)) {
+ skb_free_frag(data);
+ return -ENOMEM;
+ }
+
+ rxb->page = page;
+ rxb->offset = offset;
+ rxd->desc.s_addr = dma_addr;
+
+ return 0;
+}
+
+static void nb8800_receive(struct net_device *dev, int i, int len)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+ struct nb8800_rx_desc *rxd = &priv->rx_descs[i];
+ struct page *page = priv->rx_bufs[i].page;
+ int offset = priv->rx_bufs[i].offset;
+ void *data = page_address(page) + offset;
+ dma_addr_t dma = rxd->desc.s_addr;
+ struct sk_buff *skb;
+ int size;
+ int err;
+
+ size = len <= RX_COPYBREAK ? len : RX_COPYHDR;
+
+ skb = napi_alloc_skb(&priv->napi, size);
+ if (!skb) {
+ netdev_err(dev, "rx skb allocation failed\n");
+ dev->stats.rx_dropped++;
+ return;
+ }
+
+ if (len <= RX_COPYBREAK) {
+ dma_sync_single_for_cpu(&dev->dev, dma, len, DMA_FROM_DEVICE);
+ memcpy(skb_put(skb, len), data, len);
+ dma_sync_single_for_device(&dev->dev, dma, len,
+ DMA_FROM_DEVICE);
+ } else {
+ err = nb8800_alloc_rx(dev, i, true);
+ if (err) {
+ netdev_err(dev, "rx buffer allocation failed\n");
+ dev->stats.rx_dropped++;
+ return;
+ }
+
+ dma_unmap_page(&dev->dev, dma, RX_BUF_SIZE, DMA_FROM_DEVICE);
+ memcpy(skb_put(skb, RX_COPYHDR), data, RX_COPYHDR);
+ skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, page,
+ offset + RX_COPYHDR, len - RX_COPYHDR,
+ RX_BUF_SIZE);
+ }
+
+ skb->protocol = eth_type_trans(skb, dev);
+ netif_receive_skb(skb);
+}
+
+static void nb8800_rx_error(struct net_device *dev, u32 report)
+{
+ if (report & RX_LENGTH_ERR)
+ dev->stats.rx_length_errors++;
+
+ if (report & RX_FCS_ERR)
+ dev->stats.rx_crc_errors++;
+
+ if (report & RX_FIFO_OVERRUN)
+ dev->stats.rx_fifo_errors++;
+
+ if (report & RX_ALIGNMENT_ERROR)
+ dev->stats.rx_frame_errors++;
+
+ dev->stats.rx_errors++;
+}
+
+static int nb8800_poll(struct napi_struct *napi, int budget)
+{
+ struct net_device *dev = napi->dev;
+ struct nb8800_priv *priv = netdev_priv(dev);
+ struct nb8800_rx_desc *rxd;
+ int work = 0;
+ int last = priv->rx_eoc;
+ int next;
+
+again:
+ while (work < budget) {
+ struct nb8800_rx_buf *rxb;
+ int len;
+
+ next = (last + 1) % RX_DESC_COUNT;
+
+ rxb = &priv->rx_bufs[next];
+ rxd = &priv->rx_descs[next];
+
+ if (!rxd->report)
+ break;
+
+ len = RX_BYTES_TRANSFERRED(rxd->report);
+
+ if (IS_RX_ERROR(rxd->report))
+ nb8800_rx_error(dev, rxd->report);
+ else
+ nb8800_receive(dev, next, len);
+
+ dev->stats.rx_packets++;
+ dev->stats.rx_bytes += len;
+
+ if (rxd->report & RX_MULTICAST_PKT)
+ dev->stats.multicast++;
+
+ rxd->report = 0;
+ last = next;
+ work++;
+ }
+
+ if (work) {
+ priv->rx_descs[last].desc.config |= DESC_EOC;
+ wmb(); /* ensure new EOC is written before clearing old */
+ priv->rx_descs[priv->rx_eoc].desc.config &= ~DESC_EOC;
+ priv->rx_eoc = last;
+ nb8800_start_rx(dev);
+ }
+
+ if (work < budget) {
+ nb8800_writel(priv, NB8800_RX_ITR, priv->rx_itr_irq);
+
+ /* If a packet arrived after we last checked but
+ * before writing RX_ITR, the interrupt will be
+ * delayed, so we retrieve it now. */
+ if (priv->rx_descs[next].report)
+ goto again;
+
+ napi_complete_done(napi, work);
+ }
+
+ return work;
+}
+
+static void nb8800_tx_dma_start(struct net_device *dev)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+ struct nb8800_tx_buf *txb;
+ u32 txc_cr;
+
+ txb = &priv->tx_bufs[priv->tx_queue];
+ if (!txb->ready)
+ return;
+
+ txc_cr = nb8800_readl(priv, NB8800_TXC_CR);
+ if (txc_cr & TCR_EN)
+ return;
+
+ nb8800_writel(priv, NB8800_TX_DESC_ADDR, txb->dma_desc);
+ wmb(); /* ensure desc addr is written before starting DMA */
+ nb8800_writel(priv, NB8800_TXC_CR, txc_cr | TCR_EN);
+
+ priv->tx_queue = (priv->tx_queue + txb->chain_len) % TX_DESC_COUNT;
+}
+
+static void nb8800_tx_dma_start_irq(struct net_device *dev)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+
+ spin_lock(&priv->tx_lock);
+ nb8800_tx_dma_start(dev);
+ spin_unlock(&priv->tx_lock);
+}
+
+static int nb8800_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+ struct nb8800_tx_desc *txd;
+ struct nb8800_tx_buf *txb;
+ struct nb8800_dma_desc *dma;
+ dma_addr_t dma_addr;
+ unsigned int dma_len;
+ unsigned long flags;
+ int align;
+ int next;
+
+ if (atomic_read(&priv->tx_free) <= NB8800_DESC_LOW) {
+ netif_stop_queue(dev);
+ return NETDEV_TX_BUSY;
+ }
+
+ align = (8 - (uintptr_t)skb->data) & 7;
+
+ dma_len = skb->len - align;
+ dma_addr = dma_map_single(&dev->dev, skb->data + align,
+ dma_len, DMA_TO_DEVICE);
+
+ if (dma_mapping_error(&dev->dev, dma_addr)) {
+ netdev_err(dev, "tx dma mapping error\n");
+ kfree_skb(skb);
+ dev->stats.tx_dropped++;
+ return NETDEV_TX_OK;
+ }
+
+ if (atomic_dec_return(&priv->tx_free) <= NB8800_DESC_LOW)
+ netif_stop_queue(dev);
+
+ next = priv->tx_next;
+ txb = &priv->tx_bufs[next];
+ txd = &priv->tx_descs[next];
+ dma = &txd->desc[0];
+
+ next = (next + 1) % TX_DESC_COUNT;
+
+ if (align) {
+ memcpy(txd->buf, skb->data, align);
+
+ dma->s_addr =
+ txb->dma_desc + offsetof(struct nb8800_tx_desc, buf);
+ dma->n_addr = txb->dma_desc + sizeof(txd->desc[0]);
+ dma->config = DESC_BTS(2) | DESC_DS | align;
+
+ dma++;
+ }
+
+ dma->s_addr = dma_addr;
+ dma->n_addr = priv->tx_bufs[next].dma_desc;
+ dma->config = DESC_BTS(2) | DESC_DS | DESC_EOF | dma_len;
+
+ if (!skb->xmit_more)
+ dma->config |= DESC_EOC;
+
+ txb->skb = skb;
+ txb->dma_addr = dma_addr;
+ txb->dma_len = dma_len;
+
+ if (!priv->tx_chain) {
+ txb->chain_len = 1;
+ priv->tx_chain = txb;
+ } else {
+ priv->tx_chain->chain_len++;
+ }
+
+ netdev_sent_queue(dev, skb->len);
+
+ smp_mb__before_spinlock();
+ spin_lock_irqsave(&priv->tx_lock, flags);
+
+ if (!skb->xmit_more) {
+ priv->tx_chain->ready = true;
+ priv->tx_chain = NULL;
+ nb8800_tx_dma_start(dev);
+ }
+
+ spin_unlock_irqrestore(&priv->tx_lock, flags);
+
+ priv->tx_next = next;
+
+ return NETDEV_TX_OK;
+}
+
+static void nb8800_tx_error(struct net_device *dev, u32 report)
+{
+ if (report & TX_LATE_COLLISION)
+ dev->stats.collisions++;
+
+ if (report & TX_PACKET_DROPPED)
+ dev->stats.tx_dropped++;
+
+ if (report & TX_FIFO_UNDERRUN)
+ dev->stats.tx_fifo_errors++;
+
+ dev->stats.tx_errors++;
+}
+
+static void nb8800_tx_done(struct net_device *dev)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+ int limit = priv->tx_next;
+ int done = priv->tx_done;
+ unsigned int packets = 0;
+ unsigned int len = 0;
+
+ while (done != limit) {
+ struct nb8800_tx_desc *txd = &priv->tx_descs[done];
+ struct nb8800_tx_buf *txb = &priv->tx_bufs[done];
+ struct sk_buff *skb;
+
+ if (!txd->report)
+ break;
+
+ skb = txb->skb;
+ len += skb->len;
+
+ dma_unmap_single(&dev->dev, txb->dma_addr, txb->dma_len,
+ DMA_TO_DEVICE);
+
+ if (IS_TX_ERROR(txd->report)) {
+ nb8800_tx_error(dev, txd->report);
+ dev_kfree_skb_irq(skb);
+ } else {
+ dev_consume_skb_irq(skb);
+ }
+
+ dev->stats.tx_packets++;
+ dev->stats.tx_bytes += TX_BYTES_TRANSFERRED(txd->report);
+ dev->stats.collisions += TX_EARLY_COLLISIONS(txd->report);
+
+ txb->skb = NULL;
+ txb->ready = false;
+ txd->report = 0;
+
+ done = (done + 1) % TX_DESC_COUNT;
+ packets++;
+ }
+
+ if (packets) {
+ smp_mb__before_atomic();
+ atomic_add(packets, &priv->tx_free);
+ netdev_completed_queue(dev, packets, len);
+ netif_wake_queue(dev);
+ priv->tx_done = done;
+ }
+}
+
+static irqreturn_t nb8800_irq(int irq, void *dev_id)
+{
+ struct net_device *dev = dev_id;
+ struct nb8800_priv *priv = netdev_priv(dev);
+ u32 val;
+
+ /* tx interrupt */
+ val = nb8800_readl(priv, NB8800_TXC_SR);
+ if (val) {
+ nb8800_writel(priv, NB8800_TXC_SR, val);
+
+ if (val & TSR_DI)
+ nb8800_tx_dma_start_irq(dev);
+
+ if (val & TSR_TI)
+ nb8800_tx_done(dev);
+
+ if (unlikely(val & TSR_DE))
+ netdev_err(dev, "TX DMA error\n");
+
+ /* should never happen with automatic status retrieval */
+ if (unlikely(val & TSR_TO))
+ netdev_err(dev, "TX Status FIFO overflow\n");
+ }
+
+ /* rx interrupt */
+ val = nb8800_readl(priv, NB8800_RXC_SR);
+ if (val) {
+ nb8800_writel(priv, NB8800_RXC_SR, val);
+
+ if (likely(val & (RSR_RI | RSR_DI))) {
+ nb8800_writel(priv, NB8800_RX_ITR, priv->rx_itr_poll);
+ napi_schedule_irqoff(&priv->napi);
+ }
+
+ if (unlikely(val & RSR_DE))
+ netdev_err(dev, "RX DMA error\n");
+
+ /* should never happen with automatic status retrieval */
+ if (unlikely(val & RSR_RO))
+ netdev_err(dev, "RX Status FIFO overflow\n");
+ }
+
+ return IRQ_HANDLED;
+}
+
+static void nb8800_mac_config(struct net_device *dev)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+ bool gigabit = priv->speed == SPEED_1000;
+ u32 mac_mode_mask = RGMII_MODE | HALF_DUPLEX | GMAC_MODE;
+ u32 mac_mode = 0;
+ u32 slot_time;
+ u32 phy_clk;
+ u32 ict;
+
+ if (!priv->duplex)
+ mac_mode |= HALF_DUPLEX;
+
+ if (gigabit) {
+ if (priv->phy_mode == PHY_INTERFACE_MODE_RGMII)
+ mac_mode |= RGMII_MODE;
+
+ mac_mode |= GMAC_MODE;
+ phy_clk = 125000000;
+
+ /* Should be 512 but register is only 8 bits */
+ slot_time = 255;
+ } else {
+ phy_clk = 25000000;
+ slot_time = 128;
+ }
+
+ ict = DIV_ROUND_UP(phy_clk, clk_get_rate(priv->clk));
+
+ nb8800_writeb(priv, NB8800_IC_THRESHOLD, ict);
+ nb8800_writeb(priv, NB8800_SLOT_TIME, slot_time);
+ nb8800_maskb(priv, NB8800_MAC_MODE, mac_mode_mask, mac_mode);
+}
+
+static void nb8800_pause_config(struct net_device *dev)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+ struct phy_device *phydev = priv->phydev;
+ u32 rxcr;
+
+ if (priv->pause_aneg) {
+ if (!phydev || !phydev->link)
+ return;
+
+ priv->pause_rx = phydev->pause;
+ priv->pause_tx = phydev->pause ^ phydev->asym_pause;
+ }
+
+ nb8800_modb(priv, NB8800_RX_CTL, RX_PAUSE_EN, priv->pause_rx);
+
+ rxcr = nb8800_readl(priv, NB8800_RXC_CR);
+ if (!!(rxcr & RCR_FL) == priv->pause_tx)
+ return;
+
+ if (netif_running(dev)) {
+ napi_disable(&priv->napi);
+ netif_tx_lock_bh(dev);
+ nb8800_dma_stop(dev);
+ nb8800_modl(priv, NB8800_RXC_CR, RCR_FL, priv->pause_tx);
+ nb8800_start_rx(dev);
+ netif_tx_unlock_bh(dev);
+ napi_enable(&priv->napi);
+ } else {
+ nb8800_modl(priv, NB8800_RXC_CR, RCR_FL, priv->pause_tx);
+ }
+}
+
+static void nb8800_link_reconfigure(struct net_device *dev)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+ struct phy_device *phydev = priv->phydev;
+ int change = 0;
+
+ if (phydev->link) {
+ if (phydev->speed != priv->speed) {
+ priv->speed = phydev->speed;
+ change = 1;
+ }
+
+ if (phydev->duplex != priv->duplex) {
+ priv->duplex = phydev->duplex;
+ change = 1;
+ }
+
+ if (change)
+ nb8800_mac_config(dev);
+
+ nb8800_pause_config(dev);
+ }
+
+ if (phydev->link != priv->link) {
+ priv->link = phydev->link;
+ change = 1;
+ }
+
+ if (change)
+ phy_print_status(priv->phydev);
+}
+
+static void nb8800_update_mac_addr(struct net_device *dev)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+ int i;
+
+ for (i = 0; i < ETH_ALEN; i++)
+ nb8800_writeb(priv, NB8800_SRC_ADDR(i), dev->dev_addr[i]);
+
+ for (i = 0; i < ETH_ALEN; i++)
+ nb8800_writeb(priv, NB8800_UC_ADDR(i), dev->dev_addr[i]);
+}
+
+static int nb8800_set_mac_address(struct net_device *dev, void *addr)
+{
+ struct sockaddr *sock = addr;
+
+ if (netif_running(dev))
+ return -EBUSY;
+
+ ether_addr_copy(dev->dev_addr, sock->sa_data);
+ nb8800_update_mac_addr(dev);
+
+ return 0;
+}
+
+static void nb8800_mc_init(struct net_device *dev, int val)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+
+ nb8800_writeb(priv, NB8800_MC_INIT, val);
+ readb_poll_timeout_atomic(priv->base + NB8800_MC_INIT, val, !val,
+ 1, 1000);
+}
+
+static void nb8800_set_rx_mode(struct net_device *dev)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+ struct netdev_hw_addr *ha;
+ int i;
+
+ if (dev->flags & (IFF_PROMISC | IFF_ALLMULTI)) {
+ nb8800_mac_af(dev, false);
+ return;
+ }
+
+ nb8800_mac_af(dev, true);
+ nb8800_mc_init(dev, 0);
+
+ netdev_for_each_mc_addr(ha, dev) {
+ for (i = 0; i < ETH_ALEN; i++)
+ nb8800_writeb(priv, NB8800_MC_ADDR(i), ha->addr[i]);
+
+ nb8800_mc_init(dev, 0xff);
+ }
+}
+
+#define RX_DESC_SIZE (RX_DESC_COUNT * sizeof(struct nb8800_rx_desc))
+#define TX_DESC_SIZE (TX_DESC_COUNT * sizeof(struct nb8800_tx_desc))
+
+static void nb8800_dma_free(struct net_device *dev)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+ int i;
+
+ if (priv->rx_bufs) {
+ for (i = 0; i < RX_DESC_COUNT; i++)
+ if (priv->rx_bufs[i].page)
+ put_page(priv->rx_bufs[i].page);
+
+ kfree(priv->rx_bufs);
+ priv->rx_bufs = NULL;
+ }
+
+ if (priv->tx_bufs) {
+ for (i = 0; i < TX_DESC_COUNT; i++)
+ kfree_skb(priv->tx_bufs[i].skb);
+
+ kfree(priv->tx_bufs);
+ priv->tx_bufs = NULL;
+ }
+
+ if (priv->rx_descs) {
+ dma_free_coherent(dev->dev.parent, RX_DESC_SIZE, priv->rx_descs,
+ priv->rx_desc_dma);
+ priv->rx_descs = NULL;
+ }
+
+ if (priv->tx_descs) {
+ dma_free_coherent(dev->dev.parent, TX_DESC_SIZE, priv->tx_descs,
+ priv->tx_desc_dma);
+ priv->tx_descs = NULL;
+ }
+}
+
+static void nb8800_dma_reset(struct net_device *dev)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+ struct nb8800_rx_desc *rxd;
+ struct nb8800_tx_desc *txd;
+ int i;
+
+ for (i = 0; i < RX_DESC_COUNT; i++) {
+ dma_addr_t rx_dma = priv->rx_desc_dma + i * sizeof(*rxd);
+
+ rxd = &priv->rx_descs[i];
+ rxd->desc.n_addr = rx_dma + sizeof(*rxd);
+ rxd->desc.r_addr =
+ rx_dma + offsetof(struct nb8800_rx_desc, report);
+ rxd->desc.config = priv->rx_dma_config;
+ rxd->report = 0;
+ }
+
+ rxd->desc.n_addr = priv->rx_desc_dma;
+ rxd->desc.config |= DESC_EOC;
+
+ priv->rx_eoc = RX_DESC_COUNT - 1;
+
+ for (i = 0; i < TX_DESC_COUNT; i++) {
+ struct nb8800_tx_buf *txb = &priv->tx_bufs[i];
+ dma_addr_t r_dma = txb->dma_desc +
+ offsetof(struct nb8800_tx_desc, report);
+
+ txd = &priv->tx_descs[i];
+ txd->desc[0].r_addr = r_dma;
+ txd->desc[1].r_addr = r_dma;
+ txd->report = 0;
+ }
+
+ priv->tx_next = 0;
+ priv->tx_queue = 0;
+ priv->tx_done = 0;
+ atomic_set(&priv->tx_free, TX_DESC_COUNT);
+
+ nb8800_writel(priv, NB8800_RX_DESC_ADDR, priv->rx_desc_dma);
+
+ wmb(); /* ensure all setup is written before starting */
+}
+
+static int nb8800_dma_init(struct net_device *dev)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+ int n_rx = RX_DESC_COUNT;
+ int n_tx = TX_DESC_COUNT;
+ int err;
+ int i;
+
+ priv->rx_descs = dma_alloc_coherent(dev->dev.parent, RX_DESC_SIZE,
+ &priv->rx_desc_dma, GFP_KERNEL);
+ if (!priv->rx_descs)
+ goto err_out;
+
+ priv->rx_bufs = kcalloc(n_rx, sizeof(*priv->rx_bufs), GFP_KERNEL);
+ if (!priv->rx_bufs)
+ goto err_out;
+
+ for (i = 0; i < n_rx; i++) {
+ err = nb8800_alloc_rx(dev, i, false);
+ if (err)
+ goto err_out;
+ }
+
+ priv->tx_descs = dma_alloc_coherent(dev->dev.parent, TX_DESC_SIZE,
+ &priv->tx_desc_dma, GFP_KERNEL);
+ if (!priv->tx_descs)
+ goto err_out;
+
+ priv->tx_bufs = kcalloc(n_tx, sizeof(*priv->tx_bufs), GFP_KERNEL);
+ if (!priv->tx_bufs)
+ goto err_out;
+
+ for (i = 0; i < n_tx; i++)
+ priv->tx_bufs[i].dma_desc =
+ priv->tx_desc_dma + i * sizeof(struct nb8800_tx_desc);
+
+ nb8800_dma_reset(dev);
+
+ return 0;
+
+err_out:
+ nb8800_dma_free(dev);
+
+ return -ENOMEM;
+}
+
+static int nb8800_dma_stop(struct net_device *dev)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+ struct nb8800_tx_buf *txb = &priv->tx_bufs[0];
+ struct nb8800_tx_desc *txd = &priv->tx_descs[0];
+ int retry = 5;
+ u32 txcr;
+ u32 rxcr;
+ int err;
+ int i;
+
+ /* wait for tx to finish */
+ err = readl_poll_timeout_atomic(priv->base + NB8800_TXC_CR, txcr,
+ !(txcr & TCR_EN) &&
+ priv->tx_done == priv->tx_next,
+ 1000, 1000000);
+ if (err)
+ return err;
+
+ /* The rx DMA only stops if it reaches the end of chain.
+ * To make this happen, we set the EOC flag on all rx
+ * descriptors, put the device in loopback mode, and send
+ * a few dummy frames. The interrupt handler will ignore
+ * these since NAPI is disabled and no real frames are in
+ * the tx queue. */
+
+ for (i = 0; i < RX_DESC_COUNT; i++)
+ priv->rx_descs[i].desc.config |= DESC_EOC;
+
+ txd->desc[0].s_addr =
+ txb->dma_desc + offsetof(struct nb8800_tx_desc, buf);
+ txd->desc[0].config = DESC_BTS(2) | DESC_DS | DESC_EOF | DESC_EOC | 8;
+ memset(txd->buf, 0, sizeof(txd->buf));
+
+ nb8800_mac_af(dev, false);
+ nb8800_setb(priv, NB8800_MAC_MODE, LOOPBACK_EN);
+
+ do {
+ nb8800_writel(priv, NB8800_TX_DESC_ADDR, txb->dma_desc);
+ wmb();
+ nb8800_writel(priv, NB8800_TXC_CR, txcr | TCR_EN);
+
+ err = readl_poll_timeout_atomic(priv->base + NB8800_RXC_CR,
+ rxcr, !(rxcr & RCR_EN),
+ 1000, 100000);
+ } while (err && --retry);
+
+ nb8800_mac_af(dev, true);
+ nb8800_clearb(priv, NB8800_MAC_MODE, LOOPBACK_EN);
+ nb8800_dma_reset(dev);
+
+ return retry ? 0 : -ETIMEDOUT;
+}
+
+static void nb8800_pause_adv(struct net_device *dev)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+ u32 adv = 0;
+
+ if (!priv->phydev)
+ return;
+
+ if (priv->pause_rx)
+ adv |= ADVERTISED_Pause | ADVERTISED_Asym_Pause;
+ if (priv->pause_tx)
+ adv ^= ADVERTISED_Asym_Pause;
+
+ priv->phydev->supported |= adv;
+ priv->phydev->advertising |= adv;
+
+}
+
+static int nb8800_open(struct net_device *dev)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+ int err;
+
+ /* clear any pending interrupts */
+ nb8800_writel(priv, NB8800_RXC_SR, 0xf);
+ nb8800_writel(priv, NB8800_TXC_SR, 0xf);
+
+ err = nb8800_dma_init(dev);
+ if (err)
+ return err;
+
+ err = request_irq(dev->irq, nb8800_irq, 0, dev_name(&dev->dev), dev);
+ if (err)
+ goto err_free_dma;
+
+ nb8800_mac_rx(dev, true);
+ nb8800_mac_tx(dev, true);
+
+ priv->phydev = of_phy_connect(dev, priv->phy_node,
+ nb8800_link_reconfigure, 0,
+ priv->phy_mode);
+ if (!priv->phydev)
+ goto err_free_irq;
+
+ nb8800_pause_adv(dev);
+
+ netdev_reset_queue(dev);
+ napi_enable(&priv->napi);
+ netif_start_queue(dev);
+
+ nb8800_start_rx(dev);
+ phy_start(priv->phydev);
+
+ return 0;
+
+err_free_irq:
+ free_irq(dev->irq, dev);
+err_free_dma:
+ nb8800_dma_free(dev);
+
+ return err;
+}
+
+static int nb8800_stop(struct net_device *dev)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+
+ phy_stop(priv->phydev);
+
+ netif_stop_queue(dev);
+ napi_disable(&priv->napi);
+
+ nb8800_dma_stop(dev);
+ nb8800_mac_rx(dev, false);
+ nb8800_mac_tx(dev, false);
+
+ phy_disconnect(priv->phydev);
+ priv->phydev = NULL;
+
+ free_irq(dev->irq, dev);
+
+ nb8800_dma_free(dev);
+
+ return 0;
+}
+
+static int nb8800_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+
+ return phy_mii_ioctl(priv->phydev, rq, cmd);
+}
+
+static const struct net_device_ops nb8800_netdev_ops = {
+ .ndo_open = nb8800_open,
+ .ndo_stop = nb8800_stop,
+ .ndo_start_xmit = nb8800_xmit,
+ .ndo_set_mac_address = nb8800_set_mac_address,
+ .ndo_set_rx_mode = nb8800_set_rx_mode,
+ .ndo_do_ioctl = nb8800_ioctl,
+ .ndo_change_mtu = eth_change_mtu,
+ .ndo_validate_addr = eth_validate_addr,
+};
+
+static int nb8800_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+
+ if (!priv->phydev)
+ return -ENODEV;
+
+ return phy_ethtool_gset(priv->phydev, cmd);
+}
+
+static int nb8800_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+
+ if (!priv->phydev)
+ return -ENODEV;
+
+ return phy_ethtool_sset(priv->phydev, cmd);
+}
+
+static int nb8800_nway_reset(struct net_device *dev)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+
+ if (!priv->phydev)
+ return -ENODEV;
+
+ return genphy_restart_aneg(priv->phydev);
+}
+
+static void nb8800_get_pauseparam(struct net_device *dev,
+ struct ethtool_pauseparam *pp)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+
+ pp->autoneg = priv->pause_aneg;
+ pp->rx_pause = priv->pause_rx;
+ pp->tx_pause = priv->pause_tx;
+}
+
+static int nb8800_set_pauseparam(struct net_device *dev,
+ struct ethtool_pauseparam *pp)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+
+ priv->pause_aneg = pp->autoneg;
+ priv->pause_rx = pp->rx_pause;
+ priv->pause_tx = pp->tx_pause;
+
+ nb8800_pause_adv(dev);
+
+ if (!priv->pause_aneg)
+ nb8800_pause_config(dev);
+ else if (priv->phydev)
+ phy_start_aneg(priv->phydev);
+
+ return 0;
+}
+
+static const char nb8800_stats_names[][ETH_GSTRING_LEN] = {
+ "rx_bytes_ok",
+ "rx_frames_ok",
+ "rx_undersize_frames",
+ "rx_fragment_frames",
+ "rx_64_byte_frames",
+ "rx_127_byte_frames",
+ "rx_255_byte_frames",
+ "rx_511_byte_frames",
+ "rx_1023_byte_frames",
+ "rx_max_size_frames",
+ "rx_oversize_frames",
+ "rx_bad_fcs_frames",
+ "rx_broadcast_frames",
+ "rx_multicast_frames",
+ "rx_control_frames",
+ "rx_pause_frames",
+ "rx_unsup_control_frames",
+ "rx_align_error_frames",
+ "rx_overrun_frames",
+ "rx_jabber_frames",
+ "rx_bytes",
+ "rx_frames",
+
+ "tx_bytes_ok",
+ "tx_frames_ok",
+ "tx_64_byte_frames",
+ "tx_127_byte_frames",
+ "tx_255_byte_frames",
+ "tx_511_byte_frames",
+ "tx_1023_byte_frames",
+ "tx_max_size_frames",
+ "tx_oversize_frames",
+ "tx_broadcast_frames",
+ "tx_multicast_frames",
+ "tx_control_frames",
+ "tx_pause_frames",
+ "tx_underrun_frames",
+ "tx_single_collision_frames",
+ "tx_multi_collision_frames",
+ "tx_deferred_collision_frames",
+ "tx_late_collision_frames",
+ "tx_excessive_collision_frames",
+ "tx_bytes",
+ "tx_frames",
+ "tx_collisions",
+};
+
+#define NB8800_NUM_STATS ARRAY_SIZE(nb8800_stats_names)
+
+static int nb8800_get_sset_count(struct net_device *dev, int sset)
+{
+ if (sset == ETH_SS_STATS)
+ return NB8800_NUM_STATS;
+
+ return -EOPNOTSUPP;
+}
+
+static void nb8800_get_strings(struct net_device *dev, u32 sset, u8 *buf)
+{
+ if (sset == ETH_SS_STATS)
+ memcpy(buf, &nb8800_stats_names, sizeof(nb8800_stats_names));
+}
+
+static u32 nb8800_read_stat(struct net_device *dev, int index)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+
+ nb8800_writeb(priv, NB8800_STAT_INDEX, index);
+
+ return nb8800_readl(priv, NB8800_STAT_DATA);
+}
+
+static void nb8800_get_ethtool_stats(struct net_device *dev,
+ struct ethtool_stats *estats, u64 *st)
+{
+ u32 rx, tx;
+ int i;
+
+ for (i = 0; i < NB8800_NUM_STATS / 2; i++) {
+ rx = nb8800_read_stat(dev, i);
+ tx = nb8800_read_stat(dev, i | 0x80);
+ st[i] = rx;
+ st[i + NB8800_NUM_STATS / 2] = tx;
+ }
+}
+
+static const struct ethtool_ops nb8800_ethtool_ops = {
+ .get_settings = nb8800_get_settings,
+ .set_settings = nb8800_set_settings,
+ .nway_reset = nb8800_nway_reset,
+ .get_link = ethtool_op_get_link,
+ .get_pauseparam = nb8800_get_pauseparam,
+ .set_pauseparam = nb8800_set_pauseparam,
+ .get_sset_count = nb8800_get_sset_count,
+ .get_strings = nb8800_get_strings,
+ .get_ethtool_stats = nb8800_get_ethtool_stats,
+};
+
+static int nb8800_hw_init(struct net_device *dev)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+ u32 val;
+
+ val = TX_RETRY_EN | TX_PAD_EN | TX_APPEND_FCS;
+ nb8800_writeb(priv, NB8800_TX_CTL1, val);
+
+ /* Collision retry count */
+ nb8800_writeb(priv, NB8800_TX_CTL2, 5);
+
+ val = RX_PAD_STRIP | RX_AF_EN;
+ nb8800_writeb(priv, NB8800_RX_CTL, val);
+
+ /* Chosen by fair dice roll */
+ nb8800_writeb(priv, NB8800_RANDOM_SEED, 4);
+
+ /* TX cycles per deferral period */
+ nb8800_writeb(priv, NB8800_TX_SDP, 12);
+
+ /* The following three threshold values have been
+ * experimentally determined for good results. */
+
+ /* RX/TX FIFO threshold for partial empty (64-bit entries) */
+ nb8800_writeb(priv, NB8800_PE_THRESHOLD, 0);
+
+ /* RX/TX FIFO threshold for partial full (64-bit entries) */
+ nb8800_writeb(priv, NB8800_PF_THRESHOLD, 255);
+
+ /* Buffer size for transmit (64-bit entries) */
+ nb8800_writeb(priv, NB8800_TX_BUFSIZE, 64);
+
+ /* Configure tx DMA */
+
+ val = nb8800_readl(priv, NB8800_TXC_CR);
+ val &= TCR_LE; /* keep endian setting */
+ val |= TCR_DM; /* DMA descriptor mode */
+ val |= TCR_RS; /* automatically store tx status */
+ val |= TCR_DIE; /* interrupt on DMA chain completion */
+ val |= TCR_TFI(7); /* interrupt after 7 frames transmitted */
+ val |= TCR_BTS(2); /* 32-byte bus transaction size */
+ nb8800_writel(priv, NB8800_TXC_CR, val);
+
+ /* TX complete interrupt after 10 ms or 7 frames (see above) */
+ val = clk_get_rate(priv->clk) / 100;
+ nb8800_writel(priv, NB8800_TX_ITR, val);
+
+ /* Configure rx DMA */
+
+ val = nb8800_readl(priv, NB8800_RXC_CR);
+ val &= RCR_LE; /* keep endian setting */
+ val |= RCR_DM; /* DMA descriptor mode */
+ val |= RCR_RS; /* automatically store rx status */
+ val |= RCR_DIE; /* interrupt at end of DMA chain */
+ val |= RCR_RFI(7); /* interrupt after 7 frames received */
+ val |= RCR_BTS(2); /* 32-byte bus transaction size */
+ nb8800_writel(priv, NB8800_RXC_CR, val);
+
+ /* The rx interrupt can fire before the DMA has completed
+ * unless a small delay is added. 50 us is hopefully enough. */
+ priv->rx_itr_irq = clk_get_rate(priv->clk) / 20000;
+
+ /* In NAPI poll mode we want to disable interrupts, but the
+ * hardware does not permit this. Delay 10 ms instead. */
+ priv->rx_itr_poll = clk_get_rate(priv->clk) / 100;
+
+ nb8800_writel(priv, NB8800_RX_ITR, priv->rx_itr_irq);
+
+ priv->rx_dma_config = RX_BUF_SIZE | DESC_BTS(2) | DESC_DS | DESC_EOF;
+
+ /* Flow control settings */
+
+ /* Pause time of 0.1 ms */
+ val = 100000 / 512;
+ nb8800_writeb(priv, NB8800_PQ1, val >> 8);
+ nb8800_writeb(priv, NB8800_PQ2, val & 0xff);
+
+ /* Auto-negotiate by default */
+ priv->pause_aneg = true;
+ priv->pause_rx = true;
+ priv->pause_tx = true;
+
+ nb8800_mc_init(dev, 0);
+
+ return 0;
+}
+
+static int nb8800_tangox_init(struct net_device *dev)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+ u32 pad_mode = PAD_MODE_MII;
+
+ switch (priv->phy_mode) {
+ case PHY_INTERFACE_MODE_MII:
+ case PHY_INTERFACE_MODE_GMII:
+ pad_mode = PAD_MODE_MII;
+ break;
+
+ case PHY_INTERFACE_MODE_RGMII:
+ pad_mode = PAD_MODE_RGMII;
+ break;
+
+ case PHY_INTERFACE_MODE_RGMII_TXID:
+ pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY;
+ break;
+
+ default:
+ dev_err(dev->dev.parent, "unsupported phy mode %s\n",
+ phy_modes(priv->phy_mode));
+ return -EINVAL;
+ }
+
+ nb8800_writeb(priv, NB8800_TANGOX_PAD_MODE, pad_mode);
+
+ return 0;
+}
+
+static int nb8800_tangox_reset(struct net_device *dev)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+ int clk_div;
+
+ nb8800_writeb(priv, NB8800_TANGOX_RESET, 0);
+ usleep_range(1000, 10000);
+ nb8800_writeb(priv, NB8800_TANGOX_RESET, 1);
+
+ wmb(); /* ensure reset is cleared before proceeding */
+
+ clk_div = DIV_ROUND_UP(clk_get_rate(priv->clk), 2 * MAX_MDC_CLOCK);
+ nb8800_writew(priv, NB8800_TANGOX_MDIO_CLKDIV, clk_div);
+
+ return 0;
+}
+
+static const struct nb8800_ops nb8800_tangox_ops = {
+ .init = nb8800_tangox_init,
+ .reset = nb8800_tangox_reset,
+};
+
+static int nb8800_tango4_init(struct net_device *dev)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+ int err;
+
+ err = nb8800_tangox_init(dev);
+ if (err)
+ return err;
+
+ /* On tango4 interrupt on DMA completion per frame works and gives
+ * better performance despite generating more rx interrupts. */
+
+ /* Disable unnecessary interrupt on rx completion */
+ nb8800_clearl(priv, NB8800_RXC_CR, RCR_RFI(7));
+
+ /* Request interrupt on descriptor DMA completion */
+ priv->rx_dma_config |= DESC_ID;
+
+ return 0;
+}
+
+static const struct nb8800_ops nb8800_tango4_ops = {
+ .init = nb8800_tango4_init,
+ .reset = nb8800_tangox_reset,
+};
+
+static const struct of_device_id nb8800_dt_ids[] = {
+ {
+ .compatible = "aurora,nb8800",
+ },
+ {
+ .compatible = "sigma,smp8642-ethernet",
+ .data = &nb8800_tangox_ops,
+ },
+ {
+ .compatible = "sigma,smp8734-ethernet",
+ .data = &nb8800_tango4_ops,
+ },
+ { }
+};
+
+static int nb8800_probe(struct platform_device *pdev)
+{
+ const struct of_device_id *match;
+ const struct nb8800_ops *ops = NULL;
+ struct nb8800_priv *priv;
+ struct resource *res;
+ struct net_device *dev;
+ struct mii_bus *bus;
+ const unsigned char *mac;
+ void __iomem *base;
+ int irq;
+ int ret;
+
+ match = of_match_device(nb8800_dt_ids, &pdev->dev);
+ if (match)
+ ops = match->data;
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq <= 0) {
+ dev_err(&pdev->dev, "No IRQ\n");
+ return -EINVAL;
+ }
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ dev_dbg(&pdev->dev, "AU-NB8800 Ethernet at %pa\n", &res->start);
+
+ dev = alloc_etherdev(sizeof(*priv));
+ if (!dev)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, dev);
+ SET_NETDEV_DEV(dev, &pdev->dev);
+
+ priv = netdev_priv(dev);
+ priv->base = base;
+
+ priv->phy_mode = of_get_phy_mode(pdev->dev.of_node);
+ if (priv->phy_mode < 0)
+ priv->phy_mode = PHY_INTERFACE_MODE_RGMII;
+
+ priv->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(priv->clk)) {
+ dev_err(&pdev->dev, "failed to get clock\n");
+ ret = PTR_ERR(priv->clk);
+ goto err_free_dev;
+ }
+
+ ret = clk_prepare_enable(priv->clk);
+ if (ret)
+ goto err_free_dev;
+
+ spin_lock_init(&priv->tx_lock);
+
+ if (ops && ops->reset) {
+ ret = ops->reset(dev);
+ if (ret)
+ goto err_free_dev;
+ }
+
+ bus = devm_mdiobus_alloc(&pdev->dev);
+ if (!bus) {
+ ret = -ENOMEM;
+ goto err_disable_clk;
+ }
+
+ bus->name = "nb8800-mii";
+ bus->read = nb8800_mdio_read;
+ bus->write = nb8800_mdio_write;
+ bus->parent = &pdev->dev;
+ snprintf(bus->id, MII_BUS_ID_SIZE, "%lx.nb8800-mii",
+ (unsigned long)res->start);
+ bus->priv = priv;
+
+ ret = of_mdiobus_register(bus, pdev->dev.of_node);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to register MII bus\n");
+ goto err_disable_clk;
+ }
+
+ priv->phy_node = of_parse_phandle(pdev->dev.of_node, "phy-handle", 0);
+ if (!priv->phy_node) {
+ dev_err(&pdev->dev, "no PHY specified\n");
+ ret = -ENODEV;
+ goto err_free_bus;
+ }
+
+ priv->mii_bus = bus;
+
+ ret = nb8800_hw_init(dev);
+ if (ret)
+ goto err_free_bus;
+
+ if (ops && ops->init) {
+ ret = ops->init(dev);
+ if (ret)
+ goto err_free_bus;
+ }
+
+ dev->netdev_ops = &nb8800_netdev_ops;
+ dev->ethtool_ops = &nb8800_ethtool_ops;
+ dev->flags |= IFF_MULTICAST;
+ dev->irq = irq;
+
+ mac = of_get_mac_address(pdev->dev.of_node);
+ if (mac)
+ ether_addr_copy(dev->dev_addr, mac);
+
+ if (!is_valid_ether_addr(dev->dev_addr))
+ eth_hw_addr_random(dev);
+
+ nb8800_update_mac_addr(dev);
+
+ netif_carrier_off(dev);
+
+ ret = register_netdev(dev);
+ if (ret) {
+ netdev_err(dev, "failed to register netdev\n");
+ goto err_free_dma;
+ }
+
+ netif_napi_add(dev, &priv->napi, nb8800_poll, NAPI_POLL_WEIGHT);
+
+ netdev_info(dev, "MAC address %pM\n", dev->dev_addr);
+
+ return 0;
+
+err_free_dma:
+ nb8800_dma_free(dev);
+err_free_bus:
+ mdiobus_unregister(bus);
+err_disable_clk:
+ clk_disable_unprepare(priv->clk);
+err_free_dev:
+ free_netdev(dev);
+
+ return ret;
+}
+
+static int nb8800_remove(struct platform_device *pdev)
+{
+ struct net_device *ndev = platform_get_drvdata(pdev);
+ struct nb8800_priv *priv = netdev_priv(ndev);
+
+ unregister_netdev(ndev);
+
+ mdiobus_unregister(priv->mii_bus);
+
+ clk_disable_unprepare(priv->clk);
+
+ nb8800_dma_free(ndev);
+ free_netdev(ndev);
+
+ return 0;
+}
+
+static struct platform_driver nb8800_driver = {
+ .driver = {
+ .name = "nb8800",
+ .of_match_table = nb8800_dt_ids,
+ },
+ .probe = nb8800_probe,
+ .remove = nb8800_remove,
+};
+
+module_platform_driver(nb8800_driver);
+
+MODULE_DESCRIPTION("Aurora AU-NB8800 Ethernet driver");
+MODULE_AUTHOR("Mans Rullgard <[email protected]>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/ethernet/aurora/nb8800.h b/drivers/net/ethernet/aurora/nb8800.h
new file mode 100644
index 0000000..e108d01
--- /dev/null
+++ b/drivers/net/ethernet/aurora/nb8800.h
@@ -0,0 +1,314 @@
+#ifndef _NB8800_H_
+#define _NB8800_H_
+
+#include <linux/types.h>
+#include <linux/skbuff.h>
+#include <linux/phy.h>
+#include <linux/clk.h>
+#include <linux/bitops.h>
+
+#define RX_DESC_COUNT 256
+#define TX_DESC_COUNT 256
+
+#define NB8800_DESC_LOW 4
+
+#define RX_BUF_SIZE 1552
+
+#define RX_COPYBREAK 256
+#define RX_COPYHDR 128
+
+#define MAX_MDC_CLOCK 2500000
+
+/* Stargate Solutions SSN8800 core registers */
+#define NB8800_TX_CTL1 0x000
+#define TX_TPD BIT(5)
+#define TX_APPEND_FCS BIT(4)
+#define TX_PAD_EN BIT(3)
+#define TX_RETRY_EN BIT(2)
+#define TX_EN BIT(0)
+
+#define NB8800_TX_CTL2 0x001
+
+#define NB8800_RX_CTL 0x004
+#define RX_BC_DISABLE BIT(7)
+#define RX_RUNT BIT(6)
+#define RX_AF_EN BIT(5)
+#define RX_PAUSE_EN BIT(3)
+#define RX_SEND_CRC BIT(2)
+#define RX_PAD_STRIP BIT(1)
+#define RX_EN BIT(0)
+
+#define NB8800_RANDOM_SEED 0x008
+#define NB8800_TX_SDP 0x14
+#define NB8800_TX_TPDP1 0x18
+#define NB8800_TX_TPDP2 0x19
+#define NB8800_SLOT_TIME 0x1c
+
+#define NB8800_MDIO_CMD 0x020
+#define MIIAR_ADDR(x) ((x) << 21)
+#define MIIAR_REG(x) ((x) << 16)
+#define MIIAR_DATA(x) ((x) << 0)
+#define MDIO_CMD_GO BIT(31)
+#define MDIO_CMD_WR BIT(26)
+
+#define NB8800_MDIO_STS 0x024
+#define MDIO_STS_ERR BIT(31)
+
+#define NB8800_MC_ADDR(i) (0x028 + (i))
+#define NB8800_MC_INIT 0x02e
+#define NB8800_UC_ADDR(i) (0x03c + (i))
+
+#define NB8800_MAC_MODE 0x044
+#define RGMII_MODE BIT(7)
+#define HALF_DUPLEX BIT(4)
+#define BURST_EN BIT(3)
+#define LOOPBACK_EN BIT(2)
+#define GMAC_MODE BIT(0)
+
+#define NB8800_IC_THRESHOLD 0x050
+#define NB8800_PE_THRESHOLD 0x051
+#define NB8800_PF_THRESHOLD 0x052
+#define NB8800_TX_BUFSIZE 0x054
+#define NB8800_FIFO_CTL 0x056
+#define NB8800_PQ1 0x060
+#define NB8800_PQ2 0x061
+#define NB8800_SRC_ADDR(i) (0x06a + (i))
+#define NB8800_STAT_DATA 0x078
+#define NB8800_STAT_INDEX 0x07c
+#define NB8800_STAT_CLEAR 0x07d
+
+#define NB8800_SLEEP_MODE 0x07e
+#define SLEEP_MODE BIT(0)
+
+#define NB8800_WAKEUP 0x07f
+#define WAKEUP BIT(0)
+
+/* Aurora NB8800 host interface registers */
+#define NB8800_TXC_CR 0x100
+#define TCR_LK BIT(12)
+#define TCR_DS BIT(11)
+#define TCR_BTS(x) (((x) & 0x7) << 8)
+#define TCR_DIE BIT(7)
+#define TCR_TFI(x) (((x) & 0x7) << 4)
+#define TCR_LE BIT(3)
+#define TCR_RS BIT(2)
+#define TCR_DM BIT(1)
+#define TCR_EN BIT(0)
+
+#define NB8800_TXC_SR 0x104
+#define TSR_DE BIT(3)
+#define TSR_DI BIT(2)
+#define TSR_TO BIT(1)
+#define TSR_TI BIT(0)
+
+#define NB8800_TX_SAR 0x108
+#define NB8800_TX_DESC_ADDR 0x10c
+
+#define NB8800_TX_REPORT_ADDR 0x110
+#define TX_BYTES_TRANSFERRED(x) (((x) >> 16) & 0xffff)
+#define TX_FIRST_DEFERRAL BIT(7)
+#define TX_EARLY_COLLISIONS(x) (((x) >> 3) & 0xf)
+#define TX_LATE_COLLISION BIT(2)
+#define TX_PACKET_DROPPED BIT(1)
+#define TX_FIFO_UNDERRUN BIT(0)
+#define IS_TX_ERROR(r) ((r) & 0x07)
+
+#define NB8800_TX_FIFO_SR 0x114
+#define NB8800_TX_ITR 0x118
+
+#define NB8800_RXC_CR 0x200
+#define RCR_FL BIT(13)
+#define RCR_LK BIT(12)
+#define RCR_DS BIT(11)
+#define RCR_BTS(x) (((x) & 7) << 8)
+#define RCR_DIE BIT(7)
+#define RCR_RFI(x) (((x) & 7) << 4)
+#define RCR_LE BIT(3)
+#define RCR_RS BIT(2)
+#define RCR_DM BIT(1)
+#define RCR_EN BIT(0)
+
+#define NB8800_RXC_SR 0x204
+#define RSR_DE BIT(3)
+#define RSR_DI BIT(2)
+#define RSR_RO BIT(1)
+#define RSR_RI BIT(0)
+
+#define NB8800_RX_SAR 0x208
+#define NB8800_RX_DESC_ADDR 0x20c
+
+#define NB8800_RX_REPORT_ADDR 0x210
+#define RX_BYTES_TRANSFERRED(x) (((x) >> 16) & 0xFFFF)
+#define RX_MULTICAST_PKT BIT(9)
+#define RX_BROADCAST_PKT BIT(8)
+#define RX_LENGTH_ERR BIT(7)
+#define RX_FCS_ERR BIT(6)
+#define RX_RUNT_PKT BIT(5)
+#define RX_FIFO_OVERRUN BIT(4)
+#define RX_LATE_COLLISION BIT(3)
+#define RX_ALIGNMENT_ERROR BIT(2)
+#define RX_ERROR_MASK 0xfc
+#define IS_RX_ERROR(r) ((r) & RX_ERROR_MASK)
+
+#define NB8800_RX_FIFO_SR 0x214
+#define NB8800_RX_ITR 0x218
+
+/* Sigma Designs SMP86xx additional registers */
+#define NB8800_TANGOX_PAD_MODE 0x400
+#define PAD_MODE_MASK 0x7
+#define PAD_MODE_MII 0x0
+#define PAD_MODE_RGMII 0x1
+#define PAD_MODE_GTX_CLK_INV BIT(3)
+#define PAD_MODE_GTX_CLK_DELAY BIT(4)
+
+#define NB8800_TANGOX_MDIO_CLKDIV 0x420
+#define NB8800_TANGOX_RESET 0x424
+
+/* Hardware DMA descriptor */
+struct nb8800_dma_desc {
+ u32 s_addr; /* start address */
+ u32 n_addr; /* next descriptor address */
+ u32 r_addr; /* report address */
+ u32 config;
+} __aligned(8);
+
+#define DESC_ID BIT(23)
+#define DESC_EOC BIT(22)
+#define DESC_EOF BIT(21)
+#define DESC_LK BIT(20)
+#define DESC_DS BIT(19)
+#define DESC_BTS(x) (((x) & 0x7) << 16)
+
+/* DMA descriptor and associated data for rx.
+ * Allocated from coherent memory.
+ */
+struct nb8800_rx_desc {
+ /* DMA descriptor */
+ struct nb8800_dma_desc desc;
+
+ /* Status report filled in by hardware */
+ u32 report;
+};
+
+/* Address of buffer on rx ring */
+struct nb8800_rx_buf {
+ struct page *page;
+ unsigned long offset;
+};
+
+/* DMA descriptors and associated data for tx.
+ * Allocated from coherent memory.
+ */
+struct nb8800_tx_desc {
+ /* DMA descriptor. The second descriptor is used if packet
+ * data is unaligned. */
+ struct nb8800_dma_desc desc[2];
+
+ /* Status report filled in by hardware */
+ u32 report;
+
+ /* Bounce buffer for initial unaligned part of packet */
+ u8 buf[8] __aligned(8);
+};
+
+/* Packet in tx queue */
+struct nb8800_tx_buf {
+ /* Currently queued skb */
+ struct sk_buff *skb;
+
+ /* DMA address of the first descriptor */
+ dma_addr_t dma_desc;
+
+ /* DMA address of packet data */
+ dma_addr_t dma_addr;
+
+ /* Length of DMA mapping, less than skb->len if alignment
+ * buffer is used. */
+ unsigned int dma_len;
+
+ /* Number of packets in chain starting here */
+ unsigned int chain_len;
+
+ /* Packet chain ready to be submitted to hardware */
+ bool ready;
+};
+
+struct nb8800_priv {
+ struct napi_struct napi;
+
+ void __iomem *base;
+
+ /* RX DMA descriptors */
+ struct nb8800_rx_desc *rx_descs;
+
+ /* RX buffers referenced by DMA descriptors */
+ struct nb8800_rx_buf *rx_bufs;
+
+ /* Current end of chain */
+ u32 rx_eoc;
+
+ /* Value for rx interrupt time register in NAPI interrupt mode */
+ u32 rx_itr_irq;
+
+ /* Value for rx interrupt time register in NAPI poll mode */
+ u32 rx_itr_poll;
+
+ /* Value for config field of rx DMA descriptors */
+ u32 rx_dma_config;
+
+ /* TX DMA descriptors */
+ struct nb8800_tx_desc *tx_descs;
+
+ /* TX packet queue */
+ struct nb8800_tx_buf *tx_bufs;
+
+ /* Number of free tx queue entries */
+ atomic_t tx_free;
+
+ /* First free tx queue entry */
+ u32 tx_next;
+
+ /* Next buffer to transmit */
+ u32 tx_queue;
+
+ /* Start of current packet chain */
+ struct nb8800_tx_buf *tx_chain;
+
+ /* Next buffer to reclaim */
+ u32 tx_done;
+
+ /* Lock for DMA activation */
+ spinlock_t tx_lock;
+
+ struct mii_bus *mii_bus;
+ struct device_node *phy_node;
+ struct phy_device *phydev;
+
+ /* PHY connection type from DT */
+ int phy_mode;
+
+ /* Current link status */
+ int speed;
+ int duplex;
+ int link;
+
+ /* Pause settings */
+ bool pause_aneg;
+ bool pause_rx;
+ bool pause_tx;
+
+ /* DMA base address of rx descriptors, see rx_descs above */
+ dma_addr_t rx_desc_dma;
+
+ /* DMA base address of tx descriptors, see tx_descs above */
+ dma_addr_t tx_desc_dma;
+
+ struct clk *clk;
+};
+
+struct nb8800_ops {
+ int (*init)(struct net_device *dev);
+ int (*reset)(struct net_device *dev);
+};
+
+#endif /* _NB8800_H_ */
--
2.6.3


2015-11-10 17:55:50

by Eric Dumazet

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

On Tue, 2015-11-10 at 16:14 +0000, Mans Rullgard wrote:
> This adds a driver for the Aurora VLSI NB8800 Ethernet controller.
> It is an almost complete rewrite of a driver originally found in
> a Sigma Designs 2.6.22 tree.

...

> +
> +static void nb8800_receive(struct net_device *dev, int i, int len)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> + struct nb8800_rx_desc *rxd = &priv->rx_descs[i];
> + struct page *page = priv->rx_bufs[i].page;
> + int offset = priv->rx_bufs[i].offset;
> + void *data = page_address(page) + offset;
> + dma_addr_t dma = rxd->desc.s_addr;
> + struct sk_buff *skb;
> + int size;
> + int err;
> +
> + size = len <= RX_COPYBREAK ? len : RX_COPYHDR;
> +
> + skb = napi_alloc_skb(&priv->napi, size);
> + if (!skb) {
> + netdev_err(dev, "rx skb allocation failed\n");
> + dev->stats.rx_dropped++;
> + return;
> + }
> +
> + if (len <= RX_COPYBREAK) {
> + dma_sync_single_for_cpu(&dev->dev, dma, len, DMA_FROM_DEVICE);
> + memcpy(skb_put(skb, len), data, len);
> + dma_sync_single_for_device(&dev->dev, dma, len,
> + DMA_FROM_DEVICE);
> + } else {
> + err = nb8800_alloc_rx(dev, i, true);
> + if (err) {
> + netdev_err(dev, "rx buffer allocation failed\n");
> + dev->stats.rx_dropped++;
> + return;
> + }
> +
> + dma_unmap_page(&dev->dev, dma, RX_BUF_SIZE, DMA_FROM_DEVICE);
> + memcpy(skb_put(skb, RX_COPYHDR), data, RX_COPYHDR);
> + skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, page,
> + offset + RX_COPYHDR, len - RX_COPYHDR,
> + RX_BUF_SIZE);
> + }
> +
> + skb->protocol = eth_type_trans(skb, dev);
> + netif_receive_skb(skb);
> +}
> +


Any reason you do not use napi_gro_receive(&priv->napi, skb) instead of
netif_receive_skb() ?

I see you correctly use late napi_complete_done(napi, work), but this
matters only if napi_gro_receive() was used.


2015-11-10 17:58:17

by Eric Dumazet

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

On Tue, 2015-11-10 at 16:14 +0000, Mans Rullgard wrote:
> This adds a driver for the Aurora VLSI NB8800 Ethernet controller.
> It is an almost complete rewrite of a driver originally found in
> a Sigma Designs 2.6.22 tree.

...

> +
> +static int nb8800_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> + struct nb8800_tx_desc *txd;
> + struct nb8800_tx_buf *txb;
> + struct nb8800_dma_desc *dma;
> + dma_addr_t dma_addr;
> + unsigned int dma_len;
> + unsigned long flags;
> + int align;
> + int next;
> +
> + if (atomic_read(&priv->tx_free) <= NB8800_DESC_LOW) {
> + netif_stop_queue(dev);
> + return NETDEV_TX_BUSY;
> + }
> +
> + align = (8 - (uintptr_t)skb->data) & 7;
> +
> + dma_len = skb->len - align;
> + dma_addr = dma_map_single(&dev->dev, skb->data + align,
> + dma_len, DMA_TO_DEVICE);
> +
> + if (dma_mapping_error(&dev->dev, dma_addr)) {
> + netdev_err(dev, "tx dma mapping error\n");
> + kfree_skb(skb);
> + dev->stats.tx_dropped++;
> + return NETDEV_TX_OK;
> + }
> +
> + if (atomic_dec_return(&priv->tx_free) <= NB8800_DESC_LOW)
> + netif_stop_queue(dev);


You probably also want to clear skb->xmit_more or risk a stall

xmit_more is tricky ;)

> +
> + next = priv->tx_next;
> + txb = &priv->tx_bufs[next];
> + txd = &priv->tx_descs[next];
> + dma = &txd->desc[0];
> +
> + next = (next + 1) % TX_DESC_COUNT;
> +
> + if (align) {
> + memcpy(txd->buf, skb->data, align);
> +
> + dma->s_addr =
> + txb->dma_desc + offsetof(struct nb8800_tx_desc, buf);
> + dma->n_addr = txb->dma_desc + sizeof(txd->desc[0]);
> + dma->config = DESC_BTS(2) | DESC_DS | align;
> +
> + dma++;
> + }
> +
> + dma->s_addr = dma_addr;
> + dma->n_addr = priv->tx_bufs[next].dma_desc;
> + dma->config = DESC_BTS(2) | DESC_DS | DESC_EOF | dma_len;
> +
> + if (!skb->xmit_more)
> + dma->config |= DESC_EOC;
> +
> + txb->skb = skb;
> + txb->dma_addr = dma_addr;
> + txb->dma_len = dma_len;
> +
> + if (!priv->tx_chain) {
> + txb->chain_len = 1;
> + priv->tx_chain = txb;
> + } else {
> + priv->tx_chain->chain_len++;
> + }
> +
> + netdev_sent_queue(dev, skb->len);
> +
> + smp_mb__before_spinlock();
> + spin_lock_irqsave(&priv->tx_lock, flags);
> +
> + if (!skb->xmit_more) {
> + priv->tx_chain->ready = true;
> + priv->tx_chain = NULL;
> + nb8800_tx_dma_start(dev);
> + }
> +
> + spin_unlock_irqrestore(&priv->tx_lock, flags);
> +
> + priv->tx_next = next;
> +
> + return NETDEV_TX_OK;
> +}
> +

2015-11-10 18:05:28

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

Eric Dumazet <[email protected]> writes:

> On Tue, 2015-11-10 at 16:14 +0000, Mans Rullgard wrote:
>> This adds a driver for the Aurora VLSI NB8800 Ethernet controller.
>> It is an almost complete rewrite of a driver originally found in
>> a Sigma Designs 2.6.22 tree.
>
> ...
>
>> +
>> +static void nb8800_receive(struct net_device *dev, int i, int len)
>> +{
>> + struct nb8800_priv *priv = netdev_priv(dev);
>> + struct nb8800_rx_desc *rxd = &priv->rx_descs[i];
>> + struct page *page = priv->rx_bufs[i].page;
>> + int offset = priv->rx_bufs[i].offset;
>> + void *data = page_address(page) + offset;
>> + dma_addr_t dma = rxd->desc.s_addr;
>> + struct sk_buff *skb;
>> + int size;
>> + int err;
>> +
>> + size = len <= RX_COPYBREAK ? len : RX_COPYHDR;
>> +
>> + skb = napi_alloc_skb(&priv->napi, size);
>> + if (!skb) {
>> + netdev_err(dev, "rx skb allocation failed\n");
>> + dev->stats.rx_dropped++;
>> + return;
>> + }
>> +
>> + if (len <= RX_COPYBREAK) {
>> + dma_sync_single_for_cpu(&dev->dev, dma, len, DMA_FROM_DEVICE);
>> + memcpy(skb_put(skb, len), data, len);
>> + dma_sync_single_for_device(&dev->dev, dma, len,
>> + DMA_FROM_DEVICE);
>> + } else {
>> + err = nb8800_alloc_rx(dev, i, true);
>> + if (err) {
>> + netdev_err(dev, "rx buffer allocation failed\n");
>> + dev->stats.rx_dropped++;
>> + return;
>> + }
>> +
>> + dma_unmap_page(&dev->dev, dma, RX_BUF_SIZE, DMA_FROM_DEVICE);
>> + memcpy(skb_put(skb, RX_COPYHDR), data, RX_COPYHDR);
>> + skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, page,
>> + offset + RX_COPYHDR, len - RX_COPYHDR,
>> + RX_BUF_SIZE);
>> + }
>> +
>> + skb->protocol = eth_type_trans(skb, dev);
>> + netif_receive_skb(skb);
>> +}
>> +
>
> Any reason you do not use napi_gro_receive(&priv->napi, skb) instead of
> netif_receive_skb() ?

Because I haven't been following the netdev list closely for the last
five years, and no documentation I read mentioned this function. I can
certainly change it.

--
M?ns Rullg?rd
[email protected]

2015-11-10 18:08:03

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

Eric Dumazet <[email protected]> writes:

> On Tue, 2015-11-10 at 16:14 +0000, Mans Rullgard wrote:
>> This adds a driver for the Aurora VLSI NB8800 Ethernet controller.
>> It is an almost complete rewrite of a driver originally found in
>> a Sigma Designs 2.6.22 tree.
>
> ...
>
>> +
>> +static int nb8800_xmit(struct sk_buff *skb, struct net_device *dev)
>> +{
>> + struct nb8800_priv *priv = netdev_priv(dev);
>> + struct nb8800_tx_desc *txd;
>> + struct nb8800_tx_buf *txb;
>> + struct nb8800_dma_desc *dma;
>> + dma_addr_t dma_addr;
>> + unsigned int dma_len;
>> + unsigned long flags;
>> + int align;
>> + int next;
>> +
>> + if (atomic_read(&priv->tx_free) <= NB8800_DESC_LOW) {
>> + netif_stop_queue(dev);
>> + return NETDEV_TX_BUSY;
>> + }
>> +
>> + align = (8 - (uintptr_t)skb->data) & 7;
>> +
>> + dma_len = skb->len - align;
>> + dma_addr = dma_map_single(&dev->dev, skb->data + align,
>> + dma_len, DMA_TO_DEVICE);
>> +
>> + if (dma_mapping_error(&dev->dev, dma_addr)) {
>> + netdev_err(dev, "tx dma mapping error\n");
>> + kfree_skb(skb);
>> + dev->stats.tx_dropped++;
>> + return NETDEV_TX_OK;
>> + }
>> +
>> + if (atomic_dec_return(&priv->tx_free) <= NB8800_DESC_LOW)
>> + netif_stop_queue(dev);
>
> You probably also want to clear skb->xmit_more or risk a stall

Very true, will fix.

--
M?ns Rullg?rd
[email protected]

2015-11-10 19:13:11

by Mason

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

On 10/11/2015 17:14, Mans Rullgard wrote:

> This adds a driver for the Aurora VLSI NB8800 Ethernet controller.
> It is an almost complete rewrite of a driver originally found in
> a Sigma Designs 2.6.22 tree.
>
> Signed-off-by: Mans Rullgard <[email protected]>
> ---
> Changes:
> - Refactored mdio access functions
> - Refactored register access helpers
> - Improved error handling in rx buffer allocation
> - Optimised some fifo parameters
> - Overhauled tx dma. Multiple packets are now chained in a single dma
> operation if xmit_more is set, improving performance.
> - Improved rx irq handling. It's not possible to disable interrupts
> entirely for napi poll, but they can be slowed down a little.
> - Use readx_poll_timeout in various places
> - Improved error detection
> - Improved statistics
> - Report hardware statistics counters through ethtool
> - Improved tangox-specific setup
> - Support for flow control using pause frames
> - Explanatory comments added
> - Various minor stylistic changes
> ---
> drivers/net/ethernet/Kconfig | 1 +
> drivers/net/ethernet/Makefile | 1 +
> drivers/net/ethernet/aurora/Kconfig | 20 +
> drivers/net/ethernet/aurora/Makefile | 1 +
> drivers/net/ethernet/aurora/nb8800.c | 1530 ++++++++++++++++++++++++++++++++++
> drivers/net/ethernet/aurora/nb8800.h | 314 +++++++
> 6 files changed, 1867 insertions(+)

The code has grown much since the previous patch, despite some
refactoring. Is this mostly due to ethtool_ops support?

drivers/net/ethernet/aurora/nb8800.c | 1146 ++++++++++++++++++++++++++++++++++
drivers/net/ethernet/aurora/nb8800.h | 230 +++++++

Regards.

2015-11-10 19:25:51

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

Mason <[email protected]> writes:

> On 10/11/2015 17:14, Mans Rullgard wrote:
>
>> This adds a driver for the Aurora VLSI NB8800 Ethernet controller.
>> It is an almost complete rewrite of a driver originally found in
>> a Sigma Designs 2.6.22 tree.
>>
>> Signed-off-by: Mans Rullgard <[email protected]>
>> ---
>> Changes:
>> - Refactored mdio access functions
>> - Refactored register access helpers
>> - Improved error handling in rx buffer allocation
>> - Optimised some fifo parameters
>> - Overhauled tx dma. Multiple packets are now chained in a single dma
>> operation if xmit_more is set, improving performance.
>> - Improved rx irq handling. It's not possible to disable interrupts
>> entirely for napi poll, but they can be slowed down a little.
>> - Use readx_poll_timeout in various places
>> - Improved error detection
>> - Improved statistics
>> - Report hardware statistics counters through ethtool
>> - Improved tangox-specific setup
>> - Support for flow control using pause frames
>> - Explanatory comments added
>> - Various minor stylistic changes
>> ---
>> drivers/net/ethernet/Kconfig | 1 +
>> drivers/net/ethernet/Makefile | 1 +
>> drivers/net/ethernet/aurora/Kconfig | 20 +
>> drivers/net/ethernet/aurora/Makefile | 1 +
>> drivers/net/ethernet/aurora/nb8800.c | 1530 ++++++++++++++++++++++++++++++++++
>> drivers/net/ethernet/aurora/nb8800.h | 314 +++++++
>> 6 files changed, 1867 insertions(+)
>
> The code has grown much since the previous patch, despite some
> refactoring. Is this mostly due to ethtool_ops support?
>
> drivers/net/ethernet/aurora/nb8800.c | 1146 ++++++++++++++++++++++++++++++++++
> drivers/net/ethernet/aurora/nb8800.h | 230 +++++++

Some of the increase is from new features, some from improvements, and
then there are a bunch of new comments.

--
M?ns Rullg?rd
[email protected]

2015-11-10 20:04:09

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

From: M?ns Rullg?rd <[email protected]>
Date: Tue, 10 Nov 2015 18:05:15 +0000

> Because I haven't been following the netdev list closely for the last
> five years, and no documentation I read mentioned this function. I can
> certainly change it.

It is always advisable to mimick what other drivers do and use them as
a reference, rather than depend upon documentation which by definition
is always going to be out of sync with the source tree.

2015-11-10 20:53:30

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

David Miller <[email protected]> writes:

> From: M?ns Rullg?rd <[email protected]>
> Date: Tue, 10 Nov 2015 18:05:15 +0000
>
>> Because I haven't been following the netdev list closely for the last
>> five years, and no documentation I read mentioned this function. I can
>> certainly change it.
>
> It is always advisable to mimick what other drivers do and use them as
> a reference, rather than depend upon documentation which by definition
> is always going to be out of sync with the source tree.

Sure. The trick is to pick the right driver(s) to use as reference.
Quite a few of them don't use that function.

--
M?ns Rullg?rd
[email protected]

2015-11-10 21:06:20

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

From: M?ns Rullg?rd <[email protected]>
Date: Tue, 10 Nov 2015 20:53:19 +0000

> David Miller <[email protected]> writes:
>
>> From: M?ns Rullg?rd <[email protected]>
>> Date: Tue, 10 Nov 2015 18:05:15 +0000
>>
>>> Because I haven't been following the netdev list closely for the last
>>> five years, and no documentation I read mentioned this function. I can
>>> certainly change it.
>>
>> It is always advisable to mimick what other drivers do and use them as
>> a reference, rather than depend upon documentation which by definition
>> is always going to be out of sync with the source tree.
>
> Sure. The trick is to pick the right driver(s) to use as reference.
> Quite a few of them don't use that function.

If you really are stumped on this matter, start at least with the
ixgbe driver. In fact pretty much every Intel ethernet driver is
a reasonable reference. Others to check out are bnx2x and mlx5.

2015-11-10 21:21:06

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

David Miller <[email protected]> writes:

> From: M?ns Rullg?rd <[email protected]>
> Date: Tue, 10 Nov 2015 20:53:19 +0000
>
>> David Miller <[email protected]> writes:
>>
>>> From: M?ns Rullg?rd <[email protected]>
>>> Date: Tue, 10 Nov 2015 18:05:15 +0000
>>>
>>>> Because I haven't been following the netdev list closely for the last
>>>> five years, and no documentation I read mentioned this function. I can
>>>> certainly change it.
>>>
>>> It is always advisable to mimick what other drivers do and use them as
>>> a reference, rather than depend upon documentation which by definition
>>> is always going to be out of sync with the source tree.
>>
>> Sure. The trick is to pick the right driver(s) to use as reference.
>> Quite a few of them don't use that function.
>
> If you really are stumped on this matter, start at least with the
> ixgbe driver. In fact pretty much every Intel ethernet driver is
> a reasonable reference. Others to check out are bnx2x and mlx5.

Even ixgbe uses napi_complete() while netdevice.h says one should
"consider using napi_complete_done() instead." Did the author consider
it and decide not to, or has the driver simply not been updated?

As for the napi_gro_receive() function, calling that instead of
netif_receive_skb() is easy enough, or are there other things I should
be doing in addition?

--
M?ns Rullg?rd
[email protected]

2015-11-10 21:51:24

by Eric Dumazet

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

On Tue, 2015-11-10 at 21:21 +0000, Måns Rullgård wrote:

> Even ixgbe uses napi_complete() while netdevice.h says one should
> "consider using napi_complete_done() instead." Did the author consider
> it and decide not to, or has the driver simply not been updated?

napi_complete_done() is quite new, very few drivers use it.

It still requires a tuning (/sys/class/net/ethX/gro_flush_timeout)

>
> As for the napi_gro_receive() function, calling that instead of
> netif_receive_skb() is easy enough, or are there other things I should
> be doing in addition?

Nothing comes to mind, if you already have a NAPI context,
napi_gro_receive() is the way to go...


2015-11-10 22:09:28

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

On Tue, Nov 10, 2015 at 6:14 PM, Mans Rullgard <[email protected]> wrote:
> This adds a driver for the Aurora VLSI NB8800 Ethernet controller.
> It is an almost complete rewrite of a driver originally found in
> a Sigma Designs 2.6.22 tree.

Few nitpicks below.

>
> Signed-off-by: Mans Rullgard <[email protected]>
> ---
> Changes:
> - Refactored mdio access functions
> - Refactored register access helpers
> - Improved error handling in rx buffer allocation
> - Optimised some fifo parameters
> - Overhauled tx dma. Multiple packets are now chained in a single dma
> operation if xmit_more is set, improving performance.
> - Improved rx irq handling. It's not possible to disable interrupts
> entirely for napi poll, but they can be slowed down a little.
> - Use readx_poll_timeout in various places
> - Improved error detection
> - Improved statistics
> - Report hardware statistics counters through ethtool
> - Improved tangox-specific setup
> - Support for flow control using pause frames
> - Explanatory comments added
> - Various minor stylistic changes
> ---
> drivers/net/ethernet/Kconfig | 1 +
> drivers/net/ethernet/Makefile | 1 +
> drivers/net/ethernet/aurora/Kconfig | 20 +
> drivers/net/ethernet/aurora/Makefile | 1 +
> drivers/net/ethernet/aurora/nb8800.c | 1530 ++++++++++++++++++++++++++++++++++
> drivers/net/ethernet/aurora/nb8800.h | 314 +++++++
> 6 files changed, 1867 insertions(+)
> create mode 100644 drivers/net/ethernet/aurora/Kconfig
> create mode 100644 drivers/net/ethernet/aurora/Makefile
> create mode 100644 drivers/net/ethernet/aurora/nb8800.c
> create mode 100644 drivers/net/ethernet/aurora/nb8800.h
>
> diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
> index 05aa759..8310163 100644
> --- a/drivers/net/ethernet/Kconfig
> +++ b/drivers/net/ethernet/Kconfig
> @@ -29,6 +29,7 @@ source "drivers/net/ethernet/apm/Kconfig"
> source "drivers/net/ethernet/apple/Kconfig"
> source "drivers/net/ethernet/arc/Kconfig"
> source "drivers/net/ethernet/atheros/Kconfig"
> +source "drivers/net/ethernet/aurora/Kconfig"
> source "drivers/net/ethernet/cadence/Kconfig"
> source "drivers/net/ethernet/adi/Kconfig"
> source "drivers/net/ethernet/broadcom/Kconfig"
> diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile
> index ddfc808..b435fb0 100644
> --- a/drivers/net/ethernet/Makefile
> +++ b/drivers/net/ethernet/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_NET_XGENE) += apm/
> obj-$(CONFIG_NET_VENDOR_APPLE) += apple/
> obj-$(CONFIG_NET_VENDOR_ARC) += arc/
> obj-$(CONFIG_NET_VENDOR_ATHEROS) += atheros/
> +obj-$(CONFIG_NET_VENDOR_AURORA) += aurora/
> obj-$(CONFIG_NET_CADENCE) += cadence/
> obj-$(CONFIG_NET_BFIN) += adi/
> obj-$(CONFIG_NET_VENDOR_BROADCOM) += broadcom/
> diff --git a/drivers/net/ethernet/aurora/Kconfig b/drivers/net/ethernet/aurora/Kconfig
> new file mode 100644
> index 0000000..a3c7106
> --- /dev/null
> +++ b/drivers/net/ethernet/aurora/Kconfig
> @@ -0,0 +1,20 @@
> +config NET_VENDOR_AURORA
> + bool "Aurora VLSI devices"
> + help
> + If you have a network (Ethernet) device belonging to this class,
> + say Y.
> +
> + Note that the answer to this question doesn't directly affect the
> + kernel: saying N will just cause the configurator to skip all
> + questions about Aurora devices. If you say Y, you will be asked
> + for your specific device in the following questions.
> +
> +if NET_VENDOR_AURORA
> +
> +config AURORA_NB8800
> + tristate "Aurora AU-NB8800 support"
> + select PHYLIB
> + help
> + Support for the AU-NB8800 gigabit Ethernet controller.
> +
> +endif
> diff --git a/drivers/net/ethernet/aurora/Makefile b/drivers/net/ethernet/aurora/Makefile
> new file mode 100644
> index 0000000..6cb528a
> --- /dev/null
> +++ b/drivers/net/ethernet/aurora/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_AURORA_NB8800) += nb8800.o
> diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
> new file mode 100644
> index 0000000..11cd389
> --- /dev/null
> +++ b/drivers/net/ethernet/aurora/nb8800.c
> @@ -0,0 +1,1530 @@
> +/*
> + * Copyright (C) 2015 Mans Rullgard <[email protected]>
> + *
> + * Mostly rewritten, based on driver from Sigma Designs. Original
> + * copyright notice below.
> + *
> + *
> + * Driver for tangox SMP864x/SMP865x/SMP867x/SMP868x builtin Ethernet Mac.
> + *
> + * Copyright (C) 2005 Maxime Bizon <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/etherdevice.h>
> +#include <linux/delay.h>
> +#include <linux/ethtool.h>
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <linux/of_device.h>
> +#include <linux/of_mdio.h>
> +#include <linux/of_net.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/phy.h>
> +#include <linux/cache.h>
> +#include <linux/jiffies.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <asm/barrier.h>
> +
> +#include "nb8800.h"
> +
> +static int nb8800_dma_stop(struct net_device *dev);
> +
> +static inline u8 nb8800_readb(struct nb8800_priv *priv, int reg)
> +{
> + return readb(priv->base + reg);
> +}
> +
> +static inline u32 nb8800_readl(struct nb8800_priv *priv, int reg)
> +{
> + return readl(priv->base + reg);
> +}
> +
> +static inline void nb8800_writeb(struct nb8800_priv *priv, int reg, u8 val)
> +{
> + writeb(val, priv->base + reg);
> +}
> +
> +static inline void nb8800_writew(struct nb8800_priv *priv, int reg, u16 val)
> +{
> + writew(val, priv->base + reg);
> +}
> +
> +static inline void nb8800_writel(struct nb8800_priv *priv, int reg, u32 val)
> +{
> + writel(val, priv->base + reg);
> +}
> +
> +static inline void nb8800_maskb(struct nb8800_priv *priv, int reg,
> + u32 mask, u32 val)
> +{
> + u32 old = nb8800_readb(priv, reg);
> + u32 new = (old & ~mask) | val;

Shoudn't be "… | (val & mask);" ?

+ empty line?

> + if (new != old)
> + nb8800_writeb(priv, reg, new);
> +}
> +
> +static inline void nb8800_maskl(struct nb8800_priv *priv, int reg,
> + u32 mask, u32 val)
> +{
> + u32 old = nb8800_readl(priv, reg);
> + u32 new = (old & ~mask) | val;

Ditto.

> + if (new != old)
> + nb8800_writel(priv, reg, new);
> +}
> +
> +static inline void nb8800_modb(struct nb8800_priv *priv, int reg, u8 bits,
> + bool set)
> +{
> + nb8800_maskb(priv, reg, bits, set ? bits : 0);
> +}
> +
> +static inline void nb8800_setb(struct nb8800_priv *priv, int reg, u8 bits)
> +{
> + nb8800_maskb(priv, reg, bits, bits);
> +}
> +
> +static inline void nb8800_clearb(struct nb8800_priv *priv, int reg, u8 bits)
> +{
> + nb8800_maskb(priv, reg, bits, 0);
> +}
> +
> +static inline void nb8800_modl(struct nb8800_priv *priv, int reg, u32 bits,
> + bool set)
> +{
> + nb8800_maskl(priv, reg, bits, set ? bits : 0);
> +}
> +
> +static inline void nb8800_setl(struct nb8800_priv *priv, int reg, u32 bits)
> +{
> + nb8800_maskl(priv, reg, bits, bits);
> +}
> +
> +static inline void nb8800_clearl(struct nb8800_priv *priv, int reg, u32 bits)
> +{
> + nb8800_maskl(priv, reg, bits, 0);
> +}
> +
> +static int nb8800_mdio_wait(struct mii_bus *bus)
> +{
> + struct nb8800_priv *priv = bus->priv;
> + u32 val;
> +
> + return readl_poll_timeout_atomic(priv->base + NB8800_MDIO_CMD,
> + val, !(val & MDIO_CMD_GO), 1, 1000);
> +}
> +
> +static int nb8800_mdio_cmd(struct mii_bus *bus, u32 cmd)
> +{
> + struct nb8800_priv *priv = bus->priv;
> + int err;
> +
> + err = nb8800_mdio_wait(bus);
> + if (err)
> + return err;
> +
> + nb8800_writel(priv, NB8800_MDIO_CMD, cmd);
> + udelay(10);
> + nb8800_writel(priv, NB8800_MDIO_CMD, cmd | MDIO_CMD_GO);
> +
> + return nb8800_mdio_wait(bus);
> +}
> +
> +static int nb8800_mdio_read(struct mii_bus *bus, int phy_id, int reg)
> +{
> + struct nb8800_priv *priv = bus->priv;
> + u32 val;
> + int err;
> +
> + err = nb8800_mdio_cmd(bus, MIIAR_ADDR(phy_id) | MIIAR_REG(reg));
> + if (err)
> + return err;
> +
> + val = nb8800_readl(priv, NB8800_MDIO_STS);
> + if (val & MDIO_STS_ERR)
> + return 0xffff;
> +
> + return val & 0xffff;
> +}
> +
> +static int nb8800_mdio_write(struct mii_bus *bus, int phy_id, int reg, u16 val)
> +{
> + u32 cmd = MIIAR_DATA(val) | MIIAR_ADDR(phy_id) | MIIAR_REG(reg) |
> + MDIO_CMD_WR;
> +
> + return nb8800_mdio_cmd(bus, cmd);
> +}
> +
> +static void nb8800_mac_tx(struct net_device *dev, bool enable)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> +
> + while (nb8800_readl(priv, NB8800_TXC_CR) & TCR_EN)
> + cpu_relax();
> +
> + nb8800_modb(priv, NB8800_TX_CTL1, TX_EN, enable);
> +}
> +
> +static void nb8800_mac_rx(struct net_device *dev, bool enable)
> +{
> + nb8800_modb(netdev_priv(dev), NB8800_RX_CTL, RX_EN, enable);
> +}
> +
> +static void nb8800_mac_af(struct net_device *dev, bool enable)
> +{
> + nb8800_modb(netdev_priv(dev), NB8800_RX_CTL, RX_AF_EN, enable);
> +}
> +
> +static void nb8800_start_rx(struct net_device *dev)
> +{
> + nb8800_setl(netdev_priv(dev), NB8800_RXC_CR, RCR_EN);
> +}
> +
> +static int nb8800_alloc_rx(struct net_device *dev, int i, bool napi)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> + struct nb8800_rx_desc *rxd = &priv->rx_descs[i];
> + struct nb8800_rx_buf *rxb = &priv->rx_bufs[i];
> + int size = L1_CACHE_ALIGN(RX_BUF_SIZE);
> + dma_addr_t dma_addr;
> + struct page *page;
> + unsigned long offset;
> + void *data;
> +
> + data = napi ? napi_alloc_frag(size) : netdev_alloc_frag(size);
> + if (!data)
> + return -ENOMEM;
> +
> + page = virt_to_head_page(data);
> + offset = data - page_address(page);
> +
> + dma_addr = dma_map_page(&dev->dev, page, offset, RX_BUF_SIZE,
> + DMA_FROM_DEVICE);
> +
> + if (dma_mapping_error(&dev->dev, dma_addr)) {
> + skb_free_frag(data);
> + return -ENOMEM;
> + }
> +
> + rxb->page = page;
> + rxb->offset = offset;
> + rxd->desc.s_addr = dma_addr;
> +
> + return 0;
> +}
> +
> +static void nb8800_receive(struct net_device *dev, int i, int len)

unsigned int i ?
len as well?

> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> + struct nb8800_rx_desc *rxd = &priv->rx_descs[i];
> + struct page *page = priv->rx_bufs[i].page;
> + int offset = priv->rx_bufs[i].offset;
> + void *data = page_address(page) + offset;
> + dma_addr_t dma = rxd->desc.s_addr;

dma -> addr ?

> + struct sk_buff *skb;
> + int size;
> + int err;
> +
> + size = len <= RX_COPYBREAK ? len : RX_COPYHDR;
> +
> + skb = napi_alloc_skb(&priv->napi, size);
> + if (!skb) {
> + netdev_err(dev, "rx skb allocation failed\n");
> + dev->stats.rx_dropped++;
> + return;
> + }
> +
> + if (len <= RX_COPYBREAK) {
> + dma_sync_single_for_cpu(&dev->dev, dma, len, DMA_FROM_DEVICE);
> + memcpy(skb_put(skb, len), data, len);
> + dma_sync_single_for_device(&dev->dev, dma, len,
> + DMA_FROM_DEVICE);
> + } else {
> + err = nb8800_alloc_rx(dev, i, true);
> + if (err) {
> + netdev_err(dev, "rx buffer allocation failed\n");
> + dev->stats.rx_dropped++;
> + return;
> + }
> +
> + dma_unmap_page(&dev->dev, dma, RX_BUF_SIZE, DMA_FROM_DEVICE);
> + memcpy(skb_put(skb, RX_COPYHDR), data, RX_COPYHDR);
> + skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, page,
> + offset + RX_COPYHDR, len - RX_COPYHDR,
> + RX_BUF_SIZE);
> + }
> +
> + skb->protocol = eth_type_trans(skb, dev);
> + netif_receive_skb(skb);
> +}
> +
> +static void nb8800_rx_error(struct net_device *dev, u32 report)
> +{
> + if (report & RX_LENGTH_ERR)
> + dev->stats.rx_length_errors++;
> +
> + if (report & RX_FCS_ERR)
> + dev->stats.rx_crc_errors++;
> +
> + if (report & RX_FIFO_OVERRUN)
> + dev->stats.rx_fifo_errors++;
> +
> + if (report & RX_ALIGNMENT_ERROR)
> + dev->stats.rx_frame_errors++;
> +
> + dev->stats.rx_errors++;
> +}
> +
> +static int nb8800_poll(struct napi_struct *napi, int budget)
> +{
> + struct net_device *dev = napi->dev;
> + struct nb8800_priv *priv = netdev_priv(dev);
> + struct nb8800_rx_desc *rxd;
> + int work = 0;
> + int last = priv->rx_eoc;
> + int next;
> +
> +again:
> + while (work < budget) {
> + struct nb8800_rx_buf *rxb;
> + int len;
> +
> + next = (last + 1) % RX_DESC_COUNT;
> +
> + rxb = &priv->rx_bufs[next];
> + rxd = &priv->rx_descs[next];
> +
> + if (!rxd->report)
> + break;
> +
> + len = RX_BYTES_TRANSFERRED(rxd->report);
> +
> + if (IS_RX_ERROR(rxd->report))
> + nb8800_rx_error(dev, rxd->report);
> + else
> + nb8800_receive(dev, next, len);
> +
> + dev->stats.rx_packets++;
> + dev->stats.rx_bytes += len;
> +
> + if (rxd->report & RX_MULTICAST_PKT)
> + dev->stats.multicast++;
> +
> + rxd->report = 0;
> + last = next;
> + work++;
> + }
> +
> + if (work) {
> + priv->rx_descs[last].desc.config |= DESC_EOC;
> + wmb(); /* ensure new EOC is written before clearing old */
> + priv->rx_descs[priv->rx_eoc].desc.config &= ~DESC_EOC;
> + priv->rx_eoc = last;
> + nb8800_start_rx(dev);
> + }
> +
> + if (work < budget) {
> + nb8800_writel(priv, NB8800_RX_ITR, priv->rx_itr_irq);
> +
> + /* If a packet arrived after we last checked but
> + * before writing RX_ITR, the interrupt will be
> + * delayed, so we retrieve it now. */

Block comments usually
/*
* text
*/

Can be longer lines?

> + if (priv->rx_descs[next].report)
> + goto again;
> +
> + napi_complete_done(napi, work);
> + }
> +
> + return work;
> +}
> +
> +static void nb8800_tx_dma_start(struct net_device *dev)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> + struct nb8800_tx_buf *txb;
> + u32 txc_cr;
> +
> + txb = &priv->tx_bufs[priv->tx_queue];
> + if (!txb->ready)
> + return;
> +
> + txc_cr = nb8800_readl(priv, NB8800_TXC_CR);
> + if (txc_cr & TCR_EN)
> + return;
> +
> + nb8800_writel(priv, NB8800_TX_DESC_ADDR, txb->dma_desc);
> + wmb(); /* ensure desc addr is written before starting DMA */

Hm… Have I missed corresponding rmb() ? If it's about MMIO, perhaps mmiowb() ?

> + nb8800_writel(priv, NB8800_TXC_CR, txc_cr | TCR_EN);
> +
> + priv->tx_queue = (priv->tx_queue + txb->chain_len) % TX_DESC_COUNT;
> +}
> +
> +static void nb8800_tx_dma_start_irq(struct net_device *dev)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> +
> + spin_lock(&priv->tx_lock);
> + nb8800_tx_dma_start(dev);
> + spin_unlock(&priv->tx_lock);
> +}
> +
> +static int nb8800_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> + struct nb8800_tx_desc *txd;
> + struct nb8800_tx_buf *txb;
> + struct nb8800_dma_desc *dma;

dma -> desc ?

> + dma_addr_t dma_addr;
> + unsigned int dma_len;
> + unsigned long flags;
> + int align;
> + int next;
> +
> + if (atomic_read(&priv->tx_free) <= NB8800_DESC_LOW) {
> + netif_stop_queue(dev);
> + return NETDEV_TX_BUSY;
> + }
> +
> + align = (8 - (uintptr_t)skb->data) & 7;
> +
> + dma_len = skb->len - align;
> + dma_addr = dma_map_single(&dev->dev, skb->data + align,
> + dma_len, DMA_TO_DEVICE);
> +
> + if (dma_mapping_error(&dev->dev, dma_addr)) {
> + netdev_err(dev, "tx dma mapping error\n");
> + kfree_skb(skb);
> + dev->stats.tx_dropped++;
> + return NETDEV_TX_OK;
> + }
> +
> + if (atomic_dec_return(&priv->tx_free) <= NB8800_DESC_LOW)
> + netif_stop_queue(dev);
> +
> + next = priv->tx_next;
> + txb = &priv->tx_bufs[next];
> + txd = &priv->tx_descs[next];
> + dma = &txd->desc[0];
> +
> + next = (next + 1) % TX_DESC_COUNT;
> +
> + if (align) {
> + memcpy(txd->buf, skb->data, align);
> +
> + dma->s_addr =
> + txb->dma_desc + offsetof(struct nb8800_tx_desc, buf);
> + dma->n_addr = txb->dma_desc + sizeof(txd->desc[0]);
> + dma->config = DESC_BTS(2) | DESC_DS | align;
> +
> + dma++;
> + }
> +
> + dma->s_addr = dma_addr;
> + dma->n_addr = priv->tx_bufs[next].dma_desc;
> + dma->config = DESC_BTS(2) | DESC_DS | DESC_EOF | dma_len;
> +
> + if (!skb->xmit_more)
> + dma->config |= DESC_EOC;
> +
> + txb->skb = skb;
> + txb->dma_addr = dma_addr;
> + txb->dma_len = dma_len;
> +
> + if (!priv->tx_chain) {
> + txb->chain_len = 1;
> + priv->tx_chain = txb;
> + } else {
> + priv->tx_chain->chain_len++;
> + }
> +
> + netdev_sent_queue(dev, skb->len);
> +
> + smp_mb__before_spinlock();
> + spin_lock_irqsave(&priv->tx_lock, flags);
> +
> + if (!skb->xmit_more) {
> + priv->tx_chain->ready = true;
> + priv->tx_chain = NULL;
> + nb8800_tx_dma_start(dev);
> + }
> +
> + spin_unlock_irqrestore(&priv->tx_lock, flags);
> +
> + priv->tx_next = next;
> +
> + return NETDEV_TX_OK;
> +}
> +
> +static void nb8800_tx_error(struct net_device *dev, u32 report)
> +{
> + if (report & TX_LATE_COLLISION)
> + dev->stats.collisions++;
> +
> + if (report & TX_PACKET_DROPPED)
> + dev->stats.tx_dropped++;
> +
> + if (report & TX_FIFO_UNDERRUN)
> + dev->stats.tx_fifo_errors++;
> +
> + dev->stats.tx_errors++;
> +}
> +
> +static void nb8800_tx_done(struct net_device *dev)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> + int limit = priv->tx_next;
> + int done = priv->tx_done;
> + unsigned int packets = 0;
> + unsigned int len = 0;
> +
> + while (done != limit) {
> + struct nb8800_tx_desc *txd = &priv->tx_descs[done];
> + struct nb8800_tx_buf *txb = &priv->tx_bufs[done];
> + struct sk_buff *skb;
> +
> + if (!txd->report)
> + break;
> +
> + skb = txb->skb;
> + len += skb->len;
> +
> + dma_unmap_single(&dev->dev, txb->dma_addr, txb->dma_len,
> + DMA_TO_DEVICE);
> +
> + if (IS_TX_ERROR(txd->report)) {
> + nb8800_tx_error(dev, txd->report);
> + dev_kfree_skb_irq(skb);
> + } else {
> + dev_consume_skb_irq(skb);
> + }
> +
> + dev->stats.tx_packets++;
> + dev->stats.tx_bytes += TX_BYTES_TRANSFERRED(txd->report);
> + dev->stats.collisions += TX_EARLY_COLLISIONS(txd->report);
> +
> + txb->skb = NULL;
> + txb->ready = false;
> + txd->report = 0;
> +
> + done = (done + 1) % TX_DESC_COUNT;
> + packets++;
> + }
> +
> + if (packets) {
> + smp_mb__before_atomic();
> + atomic_add(packets, &priv->tx_free);
> + netdev_completed_queue(dev, packets, len);
> + netif_wake_queue(dev);
> + priv->tx_done = done;
> + }
> +}
> +
> +static irqreturn_t nb8800_irq(int irq, void *dev_id)
> +{
> + struct net_device *dev = dev_id;
> + struct nb8800_priv *priv = netdev_priv(dev);
> + u32 val;
> +
> + /* tx interrupt */
> + val = nb8800_readl(priv, NB8800_TXC_SR);
> + if (val) {
> + nb8800_writel(priv, NB8800_TXC_SR, val);
> +
> + if (val & TSR_DI)
> + nb8800_tx_dma_start_irq(dev);
> +
> + if (val & TSR_TI)
> + nb8800_tx_done(dev);
> +
> + if (unlikely(val & TSR_DE))
> + netdev_err(dev, "TX DMA error\n");
> +
> + /* should never happen with automatic status retrieval */
> + if (unlikely(val & TSR_TO))
> + netdev_err(dev, "TX Status FIFO overflow\n");
> + }
> +
> + /* rx interrupt */
> + val = nb8800_readl(priv, NB8800_RXC_SR);
> + if (val) {
> + nb8800_writel(priv, NB8800_RXC_SR, val);
> +
> + if (likely(val & (RSR_RI | RSR_DI))) {
> + nb8800_writel(priv, NB8800_RX_ITR, priv->rx_itr_poll);
> + napi_schedule_irqoff(&priv->napi);
> + }
> +
> + if (unlikely(val & RSR_DE))
> + netdev_err(dev, "RX DMA error\n");
> +
> + /* should never happen with automatic status retrieval */
> + if (unlikely(val & RSR_RO))
> + netdev_err(dev, "RX Status FIFO overflow\n");
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void nb8800_mac_config(struct net_device *dev)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> + bool gigabit = priv->speed == SPEED_1000;
> + u32 mac_mode_mask = RGMII_MODE | HALF_DUPLEX | GMAC_MODE;
> + u32 mac_mode = 0;
> + u32 slot_time;
> + u32 phy_clk;
> + u32 ict;
> +
> + if (!priv->duplex)
> + mac_mode |= HALF_DUPLEX;
> +
> + if (gigabit) {
> + if (priv->phy_mode == PHY_INTERFACE_MODE_RGMII)
> + mac_mode |= RGMII_MODE;
> +
> + mac_mode |= GMAC_MODE;
> + phy_clk = 125000000;
> +
> + /* Should be 512 but register is only 8 bits */
> + slot_time = 255;
> + } else {
> + phy_clk = 25000000;
> + slot_time = 128;
> + }
> +
> + ict = DIV_ROUND_UP(phy_clk, clk_get_rate(priv->clk));
> +
> + nb8800_writeb(priv, NB8800_IC_THRESHOLD, ict);
> + nb8800_writeb(priv, NB8800_SLOT_TIME, slot_time);
> + nb8800_maskb(priv, NB8800_MAC_MODE, mac_mode_mask, mac_mode);
> +}
> +
> +static void nb8800_pause_config(struct net_device *dev)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> + struct phy_device *phydev = priv->phydev;
> + u32 rxcr;
> +
> + if (priv->pause_aneg) {
> + if (!phydev || !phydev->link)
> + return;
> +
> + priv->pause_rx = phydev->pause;
> + priv->pause_tx = phydev->pause ^ phydev->asym_pause;
> + }
> +
> + nb8800_modb(priv, NB8800_RX_CTL, RX_PAUSE_EN, priv->pause_rx);
> +
> + rxcr = nb8800_readl(priv, NB8800_RXC_CR);
> + if (!!(rxcr & RCR_FL) == priv->pause_tx)
> + return;
> +
> + if (netif_running(dev)) {
> + napi_disable(&priv->napi);
> + netif_tx_lock_bh(dev);
> + nb8800_dma_stop(dev);
> + nb8800_modl(priv, NB8800_RXC_CR, RCR_FL, priv->pause_tx);
> + nb8800_start_rx(dev);
> + netif_tx_unlock_bh(dev);
> + napi_enable(&priv->napi);
> + } else {
> + nb8800_modl(priv, NB8800_RXC_CR, RCR_FL, priv->pause_tx);
> + }
> +}
> +
> +static void nb8800_link_reconfigure(struct net_device *dev)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> + struct phy_device *phydev = priv->phydev;
> + int change = 0;
> +
> + if (phydev->link) {
> + if (phydev->speed != priv->speed) {
> + priv->speed = phydev->speed;
> + change = 1;
> + }
> +
> + if (phydev->duplex != priv->duplex) {
> + priv->duplex = phydev->duplex;
> + change = 1;
> + }
> +
> + if (change)
> + nb8800_mac_config(dev);
> +
> + nb8800_pause_config(dev);
> + }
> +
> + if (phydev->link != priv->link) {
> + priv->link = phydev->link;
> + change = 1;
> + }
> +
> + if (change)
> + phy_print_status(priv->phydev);
> +}
> +
> +static void nb8800_update_mac_addr(struct net_device *dev)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> + int i;
> +
> + for (i = 0; i < ETH_ALEN; i++)
> + nb8800_writeb(priv, NB8800_SRC_ADDR(i), dev->dev_addr[i]);
> +
> + for (i = 0; i < ETH_ALEN; i++)
> + nb8800_writeb(priv, NB8800_UC_ADDR(i), dev->dev_addr[i]);
> +}
> +
> +static int nb8800_set_mac_address(struct net_device *dev, void *addr)
> +{
> + struct sockaddr *sock = addr;
> +
> + if (netif_running(dev))
> + return -EBUSY;
> +
> + ether_addr_copy(dev->dev_addr, sock->sa_data);
> + nb8800_update_mac_addr(dev);
> +
> + return 0;
> +}
> +
> +static void nb8800_mc_init(struct net_device *dev, int val)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> +
> + nb8800_writeb(priv, NB8800_MC_INIT, val);
> + readb_poll_timeout_atomic(priv->base + NB8800_MC_INIT, val, !val,
> + 1, 1000);
> +}
> +
> +static void nb8800_set_rx_mode(struct net_device *dev)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> + struct netdev_hw_addr *ha;
> + int i;
> +
> + if (dev->flags & (IFF_PROMISC | IFF_ALLMULTI)) {
> + nb8800_mac_af(dev, false);
> + return;
> + }
> +
> + nb8800_mac_af(dev, true);
> + nb8800_mc_init(dev, 0);
> +
> + netdev_for_each_mc_addr(ha, dev) {
> + for (i = 0; i < ETH_ALEN; i++)
> + nb8800_writeb(priv, NB8800_MC_ADDR(i), ha->addr[i]);
> +
> + nb8800_mc_init(dev, 0xff);
> + }
> +}
> +
> +#define RX_DESC_SIZE (RX_DESC_COUNT * sizeof(struct nb8800_rx_desc))
> +#define TX_DESC_SIZE (TX_DESC_COUNT * sizeof(struct nb8800_tx_desc))
> +
> +static void nb8800_dma_free(struct net_device *dev)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> + int i;

looks like unsigned int i;

> +
> + if (priv->rx_bufs) {
> + for (i = 0; i < RX_DESC_COUNT; i++)
> + if (priv->rx_bufs[i].page)
> + put_page(priv->rx_bufs[i].page);
> +
> + kfree(priv->rx_bufs);
> + priv->rx_bufs = NULL;
> + }
> +
> + if (priv->tx_bufs) {
> + for (i = 0; i < TX_DESC_COUNT; i++)
> + kfree_skb(priv->tx_bufs[i].skb);
> +
> + kfree(priv->tx_bufs);
> + priv->tx_bufs = NULL;
> + }
> +
> + if (priv->rx_descs) {
> + dma_free_coherent(dev->dev.parent, RX_DESC_SIZE, priv->rx_descs,
> + priv->rx_desc_dma);
> + priv->rx_descs = NULL;
> + }
> +
> + if (priv->tx_descs) {
> + dma_free_coherent(dev->dev.parent, TX_DESC_SIZE, priv->tx_descs,
> + priv->tx_desc_dma);
> + priv->tx_descs = NULL;
> + }
> +}
> +
> +static void nb8800_dma_reset(struct net_device *dev)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> + struct nb8800_rx_desc *rxd;
> + struct nb8800_tx_desc *txd;
> + int i;

Ditto.

> +
> + for (i = 0; i < RX_DESC_COUNT; i++) {
> + dma_addr_t rx_dma = priv->rx_desc_dma + i * sizeof(*rxd);
> +
> + rxd = &priv->rx_descs[i];
> + rxd->desc.n_addr = rx_dma + sizeof(*rxd);
> + rxd->desc.r_addr =
> + rx_dma + offsetof(struct nb8800_rx_desc, report);
> + rxd->desc.config = priv->rx_dma_config;
> + rxd->report = 0;
> + }
> +
> + rxd->desc.n_addr = priv->rx_desc_dma;
> + rxd->desc.config |= DESC_EOC;
> +
> + priv->rx_eoc = RX_DESC_COUNT - 1;
> +
> + for (i = 0; i < TX_DESC_COUNT; i++) {
> + struct nb8800_tx_buf *txb = &priv->tx_bufs[i];
> + dma_addr_t r_dma = txb->dma_desc +
> + offsetof(struct nb8800_tx_desc, report);
> +
> + txd = &priv->tx_descs[i];
> + txd->desc[0].r_addr = r_dma;
> + txd->desc[1].r_addr = r_dma;
> + txd->report = 0;
> + }
> +
> + priv->tx_next = 0;
> + priv->tx_queue = 0;
> + priv->tx_done = 0;
> + atomic_set(&priv->tx_free, TX_DESC_COUNT);
> +
> + nb8800_writel(priv, NB8800_RX_DESC_ADDR, priv->rx_desc_dma);
> +
> + wmb(); /* ensure all setup is written before starting */
> +}
> +
> +static int nb8800_dma_init(struct net_device *dev)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> + int n_rx = RX_DESC_COUNT;
> + int n_tx = TX_DESC_COUNT;
> + int err;
> + int i;

Ditto.

> +
> + priv->rx_descs = dma_alloc_coherent(dev->dev.parent, RX_DESC_SIZE,
> + &priv->rx_desc_dma, GFP_KERNEL);
> + if (!priv->rx_descs)
> + goto err_out;
> +
> + priv->rx_bufs = kcalloc(n_rx, sizeof(*priv->rx_bufs), GFP_KERNEL);
> + if (!priv->rx_bufs)
> + goto err_out;
> +
> + for (i = 0; i < n_rx; i++) {
> + err = nb8800_alloc_rx(dev, i, false);
> + if (err)
> + goto err_out;
> + }
> +
> + priv->tx_descs = dma_alloc_coherent(dev->dev.parent, TX_DESC_SIZE,
> + &priv->tx_desc_dma, GFP_KERNEL);
> + if (!priv->tx_descs)
> + goto err_out;
> +
> + priv->tx_bufs = kcalloc(n_tx, sizeof(*priv->tx_bufs), GFP_KERNEL);
> + if (!priv->tx_bufs)
> + goto err_out;
> +
> + for (i = 0; i < n_tx; i++)
> + priv->tx_bufs[i].dma_desc =
> + priv->tx_desc_dma + i * sizeof(struct nb8800_tx_desc);
> +
> + nb8800_dma_reset(dev);
> +
> + return 0;
> +
> +err_out:
> + nb8800_dma_free(dev);
> +
> + return -ENOMEM;
> +}
> +
> +static int nb8800_dma_stop(struct net_device *dev)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> + struct nb8800_tx_buf *txb = &priv->tx_bufs[0];
> + struct nb8800_tx_desc *txd = &priv->tx_descs[0];
> + int retry = 5;
> + u32 txcr;
> + u32 rxcr;
> + int err;
> + int i;

Ditto.

> +
> + /* wait for tx to finish */
> + err = readl_poll_timeout_atomic(priv->base + NB8800_TXC_CR, txcr,
> + !(txcr & TCR_EN) &&
> + priv->tx_done == priv->tx_next,
> + 1000, 1000000);
> + if (err)
> + return err;
> +
> + /* The rx DMA only stops if it reaches the end of chain.
> + * To make this happen, we set the EOC flag on all rx
> + * descriptors, put the device in loopback mode, and send
> + * a few dummy frames. The interrupt handler will ignore
> + * these since NAPI is disabled and no real frames are in
> + * the tx queue. */

Block comment
/*
* text
*/

> +
> + for (i = 0; i < RX_DESC_COUNT; i++)
> + priv->rx_descs[i].desc.config |= DESC_EOC;
> +
> + txd->desc[0].s_addr =
> + txb->dma_desc + offsetof(struct nb8800_tx_desc, buf);
> + txd->desc[0].config = DESC_BTS(2) | DESC_DS | DESC_EOF | DESC_EOC | 8;
> + memset(txd->buf, 0, sizeof(txd->buf));
> +
> + nb8800_mac_af(dev, false);
> + nb8800_setb(priv, NB8800_MAC_MODE, LOOPBACK_EN);
> +
> + do {
> + nb8800_writel(priv, NB8800_TX_DESC_ADDR, txb->dma_desc);
> + wmb();
> + nb8800_writel(priv, NB8800_TXC_CR, txcr | TCR_EN);
> +
> + err = readl_poll_timeout_atomic(priv->base + NB8800_RXC_CR,
> + rxcr, !(rxcr & RCR_EN),
> + 1000, 100000);
> + } while (err && --retry);
> +
> + nb8800_mac_af(dev, true);
> + nb8800_clearb(priv, NB8800_MAC_MODE, LOOPBACK_EN);
> + nb8800_dma_reset(dev);
> +
> + return retry ? 0 : -ETIMEDOUT;
> +}
> +
> +static void nb8800_pause_adv(struct net_device *dev)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> + u32 adv = 0;
> +
> + if (!priv->phydev)
> + return;
> +
> + if (priv->pause_rx)
> + adv |= ADVERTISED_Pause | ADVERTISED_Asym_Pause;
> + if (priv->pause_tx)
> + adv ^= ADVERTISED_Asym_Pause;
> +
> + priv->phydev->supported |= adv;
> + priv->phydev->advertising |= adv;
> +
> +}
> +
> +static int nb8800_open(struct net_device *dev)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> + int err;
> +
> + /* clear any pending interrupts */
> + nb8800_writel(priv, NB8800_RXC_SR, 0xf);
> + nb8800_writel(priv, NB8800_TXC_SR, 0xf);
> +
> + err = nb8800_dma_init(dev);
> + if (err)
> + return err;
> +
> + err = request_irq(dev->irq, nb8800_irq, 0, dev_name(&dev->dev), dev);
> + if (err)
> + goto err_free_dma;
> +
> + nb8800_mac_rx(dev, true);
> + nb8800_mac_tx(dev, true);
> +
> + priv->phydev = of_phy_connect(dev, priv->phy_node,
> + nb8800_link_reconfigure, 0,
> + priv->phy_mode);
> + if (!priv->phydev)
> + goto err_free_irq;
> +
> + nb8800_pause_adv(dev);
> +
> + netdev_reset_queue(dev);
> + napi_enable(&priv->napi);
> + netif_start_queue(dev);
> +
> + nb8800_start_rx(dev);
> + phy_start(priv->phydev);
> +
> + return 0;
> +
> +err_free_irq:
> + free_irq(dev->irq, dev);
> +err_free_dma:
> + nb8800_dma_free(dev);
> +
> + return err;
> +}
> +
> +static int nb8800_stop(struct net_device *dev)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> +
> + phy_stop(priv->phydev);
> +
> + netif_stop_queue(dev);
> + napi_disable(&priv->napi);
> +
> + nb8800_dma_stop(dev);
> + nb8800_mac_rx(dev, false);
> + nb8800_mac_tx(dev, false);
> +
> + phy_disconnect(priv->phydev);
> + priv->phydev = NULL;
> +
> + free_irq(dev->irq, dev);
> +
> + nb8800_dma_free(dev);
> +
> + return 0;
> +}
> +
> +static int nb8800_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> +
> + return phy_mii_ioctl(priv->phydev, rq, cmd);
> +}
> +
> +static const struct net_device_ops nb8800_netdev_ops = {
> + .ndo_open = nb8800_open,
> + .ndo_stop = nb8800_stop,
> + .ndo_start_xmit = nb8800_xmit,
> + .ndo_set_mac_address = nb8800_set_mac_address,
> + .ndo_set_rx_mode = nb8800_set_rx_mode,
> + .ndo_do_ioctl = nb8800_ioctl,
> + .ndo_change_mtu = eth_change_mtu,
> + .ndo_validate_addr = eth_validate_addr,
> +};
> +
> +static int nb8800_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> +
> + if (!priv->phydev)
> + return -ENODEV;
> +
> + return phy_ethtool_gset(priv->phydev, cmd);
> +}
> +
> +static int nb8800_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> +
> + if (!priv->phydev)
> + return -ENODEV;
> +
> + return phy_ethtool_sset(priv->phydev, cmd);
> +}
> +
> +static int nb8800_nway_reset(struct net_device *dev)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> +
> + if (!priv->phydev)
> + return -ENODEV;
> +
> + return genphy_restart_aneg(priv->phydev);
> +}
> +
> +static void nb8800_get_pauseparam(struct net_device *dev,
> + struct ethtool_pauseparam *pp)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> +
> + pp->autoneg = priv->pause_aneg;
> + pp->rx_pause = priv->pause_rx;
> + pp->tx_pause = priv->pause_tx;
> +}
> +
> +static int nb8800_set_pauseparam(struct net_device *dev,
> + struct ethtool_pauseparam *pp)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> +
> + priv->pause_aneg = pp->autoneg;
> + priv->pause_rx = pp->rx_pause;
> + priv->pause_tx = pp->tx_pause;
> +
> + nb8800_pause_adv(dev);
> +
> + if (!priv->pause_aneg)
> + nb8800_pause_config(dev);
> + else if (priv->phydev)
> + phy_start_aneg(priv->phydev);
> +
> + return 0;
> +}
> +
> +static const char nb8800_stats_names[][ETH_GSTRING_LEN] = {
> + "rx_bytes_ok",
> + "rx_frames_ok",
> + "rx_undersize_frames",
> + "rx_fragment_frames",
> + "rx_64_byte_frames",
> + "rx_127_byte_frames",
> + "rx_255_byte_frames",
> + "rx_511_byte_frames",
> + "rx_1023_byte_frames",
> + "rx_max_size_frames",
> + "rx_oversize_frames",
> + "rx_bad_fcs_frames",
> + "rx_broadcast_frames",
> + "rx_multicast_frames",
> + "rx_control_frames",
> + "rx_pause_frames",
> + "rx_unsup_control_frames",
> + "rx_align_error_frames",
> + "rx_overrun_frames",
> + "rx_jabber_frames",
> + "rx_bytes",
> + "rx_frames",
> +
> + "tx_bytes_ok",
> + "tx_frames_ok",
> + "tx_64_byte_frames",
> + "tx_127_byte_frames",
> + "tx_255_byte_frames",
> + "tx_511_byte_frames",
> + "tx_1023_byte_frames",
> + "tx_max_size_frames",
> + "tx_oversize_frames",
> + "tx_broadcast_frames",
> + "tx_multicast_frames",
> + "tx_control_frames",
> + "tx_pause_frames",
> + "tx_underrun_frames",
> + "tx_single_collision_frames",
> + "tx_multi_collision_frames",
> + "tx_deferred_collision_frames",
> + "tx_late_collision_frames",
> + "tx_excessive_collision_frames",
> + "tx_bytes",
> + "tx_frames",
> + "tx_collisions",
> +};
> +
> +#define NB8800_NUM_STATS ARRAY_SIZE(nb8800_stats_names)
> +
> +static int nb8800_get_sset_count(struct net_device *dev, int sset)
> +{
> + if (sset == ETH_SS_STATS)
> + return NB8800_NUM_STATS;
> +
> + return -EOPNOTSUPP;
> +}
> +
> +static void nb8800_get_strings(struct net_device *dev, u32 sset, u8 *buf)
> +{
> + if (sset == ETH_SS_STATS)
> + memcpy(buf, &nb8800_stats_names, sizeof(nb8800_stats_names));
> +}
> +
> +static u32 nb8800_read_stat(struct net_device *dev, int index)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> +
> + nb8800_writeb(priv, NB8800_STAT_INDEX, index);
> +
> + return nb8800_readl(priv, NB8800_STAT_DATA);
> +}
> +
> +static void nb8800_get_ethtool_stats(struct net_device *dev,
> + struct ethtool_stats *estats, u64 *st)
> +{
> + u32 rx, tx;
> + int i;

unsigned int i;

> +
> + for (i = 0; i < NB8800_NUM_STATS / 2; i++) {
> + rx = nb8800_read_stat(dev, i);
> + tx = nb8800_read_stat(dev, i | 0x80);
> + st[i] = rx;
> + st[i + NB8800_NUM_STATS / 2] = tx;
> + }
> +}
> +
> +static const struct ethtool_ops nb8800_ethtool_ops = {
> + .get_settings = nb8800_get_settings,
> + .set_settings = nb8800_set_settings,
> + .nway_reset = nb8800_nway_reset,
> + .get_link = ethtool_op_get_link,
> + .get_pauseparam = nb8800_get_pauseparam,
> + .set_pauseparam = nb8800_set_pauseparam,
> + .get_sset_count = nb8800_get_sset_count,
> + .get_strings = nb8800_get_strings,
> + .get_ethtool_stats = nb8800_get_ethtool_stats,
> +};
> +
> +static int nb8800_hw_init(struct net_device *dev)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> + u32 val;
> +
> + val = TX_RETRY_EN | TX_PAD_EN | TX_APPEND_FCS;
> + nb8800_writeb(priv, NB8800_TX_CTL1, val);
> +
> + /* Collision retry count */
> + nb8800_writeb(priv, NB8800_TX_CTL2, 5);
> +
> + val = RX_PAD_STRIP | RX_AF_EN;
> + nb8800_writeb(priv, NB8800_RX_CTL, val);
> +
> + /* Chosen by fair dice roll */
> + nb8800_writeb(priv, NB8800_RANDOM_SEED, 4);
> +
> + /* TX cycles per deferral period */
> + nb8800_writeb(priv, NB8800_TX_SDP, 12);
> +
> + /* The following three threshold values have been
> + * experimentally determined for good results. */
> +
> + /* RX/TX FIFO threshold for partial empty (64-bit entries) */
> + nb8800_writeb(priv, NB8800_PE_THRESHOLD, 0);
> +
> + /* RX/TX FIFO threshold for partial full (64-bit entries) */
> + nb8800_writeb(priv, NB8800_PF_THRESHOLD, 255);
> +
> + /* Buffer size for transmit (64-bit entries) */
> + nb8800_writeb(priv, NB8800_TX_BUFSIZE, 64);
> +
> + /* Configure tx DMA */
> +
> + val = nb8800_readl(priv, NB8800_TXC_CR);
> + val &= TCR_LE; /* keep endian setting */
> + val |= TCR_DM; /* DMA descriptor mode */
> + val |= TCR_RS; /* automatically store tx status */
> + val |= TCR_DIE; /* interrupt on DMA chain completion */
> + val |= TCR_TFI(7); /* interrupt after 7 frames transmitted */
> + val |= TCR_BTS(2); /* 32-byte bus transaction size */
> + nb8800_writel(priv, NB8800_TXC_CR, val);
> +
> + /* TX complete interrupt after 10 ms or 7 frames (see above) */
> + val = clk_get_rate(priv->clk) / 100;
> + nb8800_writel(priv, NB8800_TX_ITR, val);
> +
> + /* Configure rx DMA */
> +
> + val = nb8800_readl(priv, NB8800_RXC_CR);
> + val &= RCR_LE; /* keep endian setting */
> + val |= RCR_DM; /* DMA descriptor mode */
> + val |= RCR_RS; /* automatically store rx status */
> + val |= RCR_DIE; /* interrupt at end of DMA chain */
> + val |= RCR_RFI(7); /* interrupt after 7 frames received */
> + val |= RCR_BTS(2); /* 32-byte bus transaction size */
> + nb8800_writel(priv, NB8800_RXC_CR, val);
> +
> + /* The rx interrupt can fire before the DMA has completed
> + * unless a small delay is added. 50 us is hopefully enough. */
> + priv->rx_itr_irq = clk_get_rate(priv->clk) / 20000;
> +
> + /* In NAPI poll mode we want to disable interrupts, but the
> + * hardware does not permit this. Delay 10 ms instead. */
> + priv->rx_itr_poll = clk_get_rate(priv->clk) / 100;
> +
> + nb8800_writel(priv, NB8800_RX_ITR, priv->rx_itr_irq);
> +
> + priv->rx_dma_config = RX_BUF_SIZE | DESC_BTS(2) | DESC_DS | DESC_EOF;
> +
> + /* Flow control settings */
> +
> + /* Pause time of 0.1 ms */
> + val = 100000 / 512;
> + nb8800_writeb(priv, NB8800_PQ1, val >> 8);
> + nb8800_writeb(priv, NB8800_PQ2, val & 0xff);
> +
> + /* Auto-negotiate by default */
> + priv->pause_aneg = true;
> + priv->pause_rx = true;
> + priv->pause_tx = true;
> +
> + nb8800_mc_init(dev, 0);
> +
> + return 0;
> +}
> +
> +static int nb8800_tangox_init(struct net_device *dev)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> + u32 pad_mode = PAD_MODE_MII;
> +
> + switch (priv->phy_mode) {
> + case PHY_INTERFACE_MODE_MII:
> + case PHY_INTERFACE_MODE_GMII:
> + pad_mode = PAD_MODE_MII;
> + break;
> +
> + case PHY_INTERFACE_MODE_RGMII:
> + pad_mode = PAD_MODE_RGMII;
> + break;
> +
> + case PHY_INTERFACE_MODE_RGMII_TXID:
> + pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY;
> + break;
> +
> + default:
> + dev_err(dev->dev.parent, "unsupported phy mode %s\n",
> + phy_modes(priv->phy_mode));
> + return -EINVAL;
> + }
> +
> + nb8800_writeb(priv, NB8800_TANGOX_PAD_MODE, pad_mode);
> +
> + return 0;
> +}
> +
> +static int nb8800_tangox_reset(struct net_device *dev)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> + int clk_div;
> +
> + nb8800_writeb(priv, NB8800_TANGOX_RESET, 0);
> + usleep_range(1000, 10000);
> + nb8800_writeb(priv, NB8800_TANGOX_RESET, 1);
> +
> + wmb(); /* ensure reset is cleared before proceeding */
> +
> + clk_div = DIV_ROUND_UP(clk_get_rate(priv->clk), 2 * MAX_MDC_CLOCK);
> + nb8800_writew(priv, NB8800_TANGOX_MDIO_CLKDIV, clk_div);
> +
> + return 0;
> +}
> +
> +static const struct nb8800_ops nb8800_tangox_ops = {
> + .init = nb8800_tangox_init,
> + .reset = nb8800_tangox_reset,
> +};
> +
> +static int nb8800_tango4_init(struct net_device *dev)
> +{
> + struct nb8800_priv *priv = netdev_priv(dev);
> + int err;
> +
> + err = nb8800_tangox_init(dev);
> + if (err)
> + return err;
> +
> + /* On tango4 interrupt on DMA completion per frame works and gives
> + * better performance despite generating more rx interrupts. */
> +
> + /* Disable unnecessary interrupt on rx completion */
> + nb8800_clearl(priv, NB8800_RXC_CR, RCR_RFI(7));
> +
> + /* Request interrupt on descriptor DMA completion */
> + priv->rx_dma_config |= DESC_ID;
> +
> + return 0;
> +}
> +
> +static const struct nb8800_ops nb8800_tango4_ops = {
> + .init = nb8800_tango4_init,
> + .reset = nb8800_tangox_reset,
> +};
> +
> +static const struct of_device_id nb8800_dt_ids[] = {
> + {
> + .compatible = "aurora,nb8800",
> + },
> + {
> + .compatible = "sigma,smp8642-ethernet",
> + .data = &nb8800_tangox_ops,
> + },
> + {
> + .compatible = "sigma,smp8734-ethernet",
> + .data = &nb8800_tango4_ops,
> + },
> + { }
> +};
> +
> +static int nb8800_probe(struct platform_device *pdev)
> +{
> + const struct of_device_id *match;
> + const struct nb8800_ops *ops = NULL;
> + struct nb8800_priv *priv;
> + struct resource *res;
> + struct net_device *dev;
> + struct mii_bus *bus;
> + const unsigned char *mac;
> + void __iomem *base;
> + int irq;
> + int ret;
> +
> + match = of_match_device(nb8800_dt_ids, &pdev->dev);
> + if (match)
> + ops = match->data;
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq <= 0) {
> + dev_err(&pdev->dev, "No IRQ\n");
> + return -EINVAL;
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + dev_dbg(&pdev->dev, "AU-NB8800 Ethernet at %pa\n", &res->start);
> +
> + dev = alloc_etherdev(sizeof(*priv));
> + if (!dev)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, dev);
> + SET_NETDEV_DEV(dev, &pdev->dev);
> +
> + priv = netdev_priv(dev);
> + priv->base = base;
> +
> + priv->phy_mode = of_get_phy_mode(pdev->dev.of_node);
> + if (priv->phy_mode < 0)
> + priv->phy_mode = PHY_INTERFACE_MODE_RGMII;
> +
> + priv->clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(priv->clk)) {
> + dev_err(&pdev->dev, "failed to get clock\n");
> + ret = PTR_ERR(priv->clk);
> + goto err_free_dev;
> + }
> +
> + ret = clk_prepare_enable(priv->clk);
> + if (ret)
> + goto err_free_dev;
> +
> + spin_lock_init(&priv->tx_lock);
> +
> + if (ops && ops->reset) {
> + ret = ops->reset(dev);
> + if (ret)
> + goto err_free_dev;
> + }
> +
> + bus = devm_mdiobus_alloc(&pdev->dev);
> + if (!bus) {
> + ret = -ENOMEM;
> + goto err_disable_clk;
> + }
> +
> + bus->name = "nb8800-mii";
> + bus->read = nb8800_mdio_read;
> + bus->write = nb8800_mdio_write;
> + bus->parent = &pdev->dev;
> + snprintf(bus->id, MII_BUS_ID_SIZE, "%lx.nb8800-mii",
> + (unsigned long)res->start);
> + bus->priv = priv;
> +
> + ret = of_mdiobus_register(bus, pdev->dev.of_node);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to register MII bus\n");
> + goto err_disable_clk;
> + }
> +
> + priv->phy_node = of_parse_phandle(pdev->dev.of_node, "phy-handle", 0);
> + if (!priv->phy_node) {
> + dev_err(&pdev->dev, "no PHY specified\n");
> + ret = -ENODEV;
> + goto err_free_bus;
> + }
> +
> + priv->mii_bus = bus;
> +
> + ret = nb8800_hw_init(dev);
> + if (ret)
> + goto err_free_bus;
> +
> + if (ops && ops->init) {
> + ret = ops->init(dev);
> + if (ret)
> + goto err_free_bus;
> + }
> +
> + dev->netdev_ops = &nb8800_netdev_ops;
> + dev->ethtool_ops = &nb8800_ethtool_ops;
> + dev->flags |= IFF_MULTICAST;
> + dev->irq = irq;
> +
> + mac = of_get_mac_address(pdev->dev.of_node);
> + if (mac)
> + ether_addr_copy(dev->dev_addr, mac);
> +
> + if (!is_valid_ether_addr(dev->dev_addr))
> + eth_hw_addr_random(dev);
> +
> + nb8800_update_mac_addr(dev);
> +
> + netif_carrier_off(dev);
> +
> + ret = register_netdev(dev);
> + if (ret) {
> + netdev_err(dev, "failed to register netdev\n");
> + goto err_free_dma;
> + }
> +
> + netif_napi_add(dev, &priv->napi, nb8800_poll, NAPI_POLL_WEIGHT);
> +
> + netdev_info(dev, "MAC address %pM\n", dev->dev_addr);
> +
> + return 0;
> +
> +err_free_dma:
> + nb8800_dma_free(dev);
> +err_free_bus:
> + mdiobus_unregister(bus);
> +err_disable_clk:
> + clk_disable_unprepare(priv->clk);
> +err_free_dev:
> + free_netdev(dev);
> +
> + return ret;
> +}
> +
> +static int nb8800_remove(struct platform_device *pdev)
> +{
> + struct net_device *ndev = platform_get_drvdata(pdev);
> + struct nb8800_priv *priv = netdev_priv(ndev);
> +
> + unregister_netdev(ndev);
> +
> + mdiobus_unregister(priv->mii_bus);
> +
> + clk_disable_unprepare(priv->clk);
> +
> + nb8800_dma_free(ndev);
> + free_netdev(ndev);
> +
> + return 0;
> +}
> +
> +static struct platform_driver nb8800_driver = {
> + .driver = {
> + .name = "nb8800",
> + .of_match_table = nb8800_dt_ids,
> + },
> + .probe = nb8800_probe,
> + .remove = nb8800_remove,
> +};
> +
> +module_platform_driver(nb8800_driver);
> +
> +MODULE_DESCRIPTION("Aurora AU-NB8800 Ethernet driver");
> +MODULE_AUTHOR("Mans Rullgard <[email protected]>");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/net/ethernet/aurora/nb8800.h b/drivers/net/ethernet/aurora/nb8800.h
> new file mode 100644
> index 0000000..e108d01
> --- /dev/null
> +++ b/drivers/net/ethernet/aurora/nb8800.h
> @@ -0,0 +1,314 @@
> +#ifndef _NB8800_H_
> +#define _NB8800_H_
> +
> +#include <linux/types.h>
> +#include <linux/skbuff.h>
> +#include <linux/phy.h>
> +#include <linux/clk.h>
> +#include <linux/bitops.h>
> +
> +#define RX_DESC_COUNT 256
> +#define TX_DESC_COUNT 256
> +
> +#define NB8800_DESC_LOW 4
> +
> +#define RX_BUF_SIZE 1552
> +
> +#define RX_COPYBREAK 256
> +#define RX_COPYHDR 128
> +
> +#define MAX_MDC_CLOCK 2500000
> +
> +/* Stargate Solutions SSN8800 core registers */
> +#define NB8800_TX_CTL1 0x000
> +#define TX_TPD BIT(5)
> +#define TX_APPEND_FCS BIT(4)
> +#define TX_PAD_EN BIT(3)
> +#define TX_RETRY_EN BIT(2)
> +#define TX_EN BIT(0)
> +
> +#define NB8800_TX_CTL2 0x001
> +
> +#define NB8800_RX_CTL 0x004
> +#define RX_BC_DISABLE BIT(7)
> +#define RX_RUNT BIT(6)
> +#define RX_AF_EN BIT(5)
> +#define RX_PAUSE_EN BIT(3)
> +#define RX_SEND_CRC BIT(2)
> +#define RX_PAD_STRIP BIT(1)
> +#define RX_EN BIT(0)
> +
> +#define NB8800_RANDOM_SEED 0x008
> +#define NB8800_TX_SDP 0x14
> +#define NB8800_TX_TPDP1 0x18
> +#define NB8800_TX_TPDP2 0x19
> +#define NB8800_SLOT_TIME 0x1c
> +
> +#define NB8800_MDIO_CMD 0x020

> +#define MIIAR_ADDR(x) ((x) << 21)
> +#define MIIAR_REG(x) ((x) << 16)
> +#define MIIAR_DATA(x) ((x) << 0)
> +#define MDIO_CMD_GO BIT(31)
> +#define MDIO_CMD_WR BIT(26)

Perhaps sort them by bits used?

> +
> +#define NB8800_MDIO_STS 0x024
> +#define MDIO_STS_ERR BIT(31)
> +
> +#define NB8800_MC_ADDR(i) (0x028 + (i))
> +#define NB8800_MC_INIT 0x02e
> +#define NB8800_UC_ADDR(i) (0x03c + (i))
> +
> +#define NB8800_MAC_MODE 0x044
> +#define RGMII_MODE BIT(7)
> +#define HALF_DUPLEX BIT(4)
> +#define BURST_EN BIT(3)
> +#define LOOPBACK_EN BIT(2)
> +#define GMAC_MODE BIT(0)
> +
> +#define NB8800_IC_THRESHOLD 0x050
> +#define NB8800_PE_THRESHOLD 0x051
> +#define NB8800_PF_THRESHOLD 0x052
> +#define NB8800_TX_BUFSIZE 0x054
> +#define NB8800_FIFO_CTL 0x056
> +#define NB8800_PQ1 0x060
> +#define NB8800_PQ2 0x061
> +#define NB8800_SRC_ADDR(i) (0x06a + (i))
> +#define NB8800_STAT_DATA 0x078
> +#define NB8800_STAT_INDEX 0x07c
> +#define NB8800_STAT_CLEAR 0x07d
> +
> +#define NB8800_SLEEP_MODE 0x07e
> +#define SLEEP_MODE BIT(0)
> +
> +#define NB8800_WAKEUP 0x07f
> +#define WAKEUP BIT(0)
> +
> +/* Aurora NB8800 host interface registers */
> +#define NB8800_TXC_CR 0x100
> +#define TCR_LK BIT(12)
> +#define TCR_DS BIT(11)
> +#define TCR_BTS(x) (((x) & 0x7) << 8)
> +#define TCR_DIE BIT(7)
> +#define TCR_TFI(x) (((x) & 0x7) << 4)
> +#define TCR_LE BIT(3)
> +#define TCR_RS BIT(2)
> +#define TCR_DM BIT(1)
> +#define TCR_EN BIT(0)
> +
> +#define NB8800_TXC_SR 0x104
> +#define TSR_DE BIT(3)
> +#define TSR_DI BIT(2)
> +#define TSR_TO BIT(1)
> +#define TSR_TI BIT(0)
> +
> +#define NB8800_TX_SAR 0x108
> +#define NB8800_TX_DESC_ADDR 0x10c
> +
> +#define NB8800_TX_REPORT_ADDR 0x110
> +#define TX_BYTES_TRANSFERRED(x) (((x) >> 16) & 0xffff)
> +#define TX_FIRST_DEFERRAL BIT(7)
> +#define TX_EARLY_COLLISIONS(x) (((x) >> 3) & 0xf)
> +#define TX_LATE_COLLISION BIT(2)
> +#define TX_PACKET_DROPPED BIT(1)
> +#define TX_FIFO_UNDERRUN BIT(0)
> +#define IS_TX_ERROR(r) ((r) & 0x07)
> +
> +#define NB8800_TX_FIFO_SR 0x114
> +#define NB8800_TX_ITR 0x118
> +
> +#define NB8800_RXC_CR 0x200
> +#define RCR_FL BIT(13)
> +#define RCR_LK BIT(12)
> +#define RCR_DS BIT(11)
> +#define RCR_BTS(x) (((x) & 7) << 8)
> +#define RCR_DIE BIT(7)
> +#define RCR_RFI(x) (((x) & 7) << 4)
> +#define RCR_LE BIT(3)
> +#define RCR_RS BIT(2)
> +#define RCR_DM BIT(1)
> +#define RCR_EN BIT(0)
> +
> +#define NB8800_RXC_SR 0x204
> +#define RSR_DE BIT(3)
> +#define RSR_DI BIT(2)
> +#define RSR_RO BIT(1)
> +#define RSR_RI BIT(0)
> +
> +#define NB8800_RX_SAR 0x208
> +#define NB8800_RX_DESC_ADDR 0x20c
> +
> +#define NB8800_RX_REPORT_ADDR 0x210
> +#define RX_BYTES_TRANSFERRED(x) (((x) >> 16) & 0xFFFF)
> +#define RX_MULTICAST_PKT BIT(9)
> +#define RX_BROADCAST_PKT BIT(8)
> +#define RX_LENGTH_ERR BIT(7)
> +#define RX_FCS_ERR BIT(6)
> +#define RX_RUNT_PKT BIT(5)
> +#define RX_FIFO_OVERRUN BIT(4)
> +#define RX_LATE_COLLISION BIT(3)
> +#define RX_ALIGNMENT_ERROR BIT(2)
> +#define RX_ERROR_MASK 0xfc
> +#define IS_RX_ERROR(r) ((r) & RX_ERROR_MASK)
> +
> +#define NB8800_RX_FIFO_SR 0x214
> +#define NB8800_RX_ITR 0x218
> +
> +/* Sigma Designs SMP86xx additional registers */
> +#define NB8800_TANGOX_PAD_MODE 0x400
> +#define PAD_MODE_MASK 0x7
> +#define PAD_MODE_MII 0x0
> +#define PAD_MODE_RGMII 0x1
> +#define PAD_MODE_GTX_CLK_INV BIT(3)
> +#define PAD_MODE_GTX_CLK_DELAY BIT(4)
> +
> +#define NB8800_TANGOX_MDIO_CLKDIV 0x420
> +#define NB8800_TANGOX_RESET 0x424
> +
> +/* Hardware DMA descriptor */
> +struct nb8800_dma_desc {
> + u32 s_addr; /* start address */
> + u32 n_addr; /* next descriptor address */
> + u32 r_addr; /* report address */
> + u32 config;
> +} __aligned(8);
> +
> +#define DESC_ID BIT(23)
> +#define DESC_EOC BIT(22)
> +#define DESC_EOF BIT(21)
> +#define DESC_LK BIT(20)
> +#define DESC_DS BIT(19)
> +#define DESC_BTS(x) (((x) & 0x7) << 16)
> +
> +/* DMA descriptor and associated data for rx.
> + * Allocated from coherent memory.
> + */
> +struct nb8800_rx_desc {
> + /* DMA descriptor */
> + struct nb8800_dma_desc desc;
> +
> + /* Status report filled in by hardware */
> + u32 report;
> +};
> +
> +/* Address of buffer on rx ring */
> +struct nb8800_rx_buf {
> + struct page *page;
> + unsigned long offset;
> +};
> +
> +/* DMA descriptors and associated data for tx.
> + * Allocated from coherent memory.
> + */

Convert to kernel doc?

> +struct nb8800_tx_desc {
> + /* DMA descriptor. The second descriptor is used if packet
> + * data is unaligned. *


> + struct nb8800_dma_desc desc[2];
> +
> + /* Status report filled in by hardware */
> + u32 report;
> +
> + /* Bounce buffer for initial unaligned part of packet */

> + u8 buf[8] __aligned(8);
> +};
> +
> +/* Packet in tx queue */
> +struct nb8800_tx_buf {


> + /* Currently queued skb */
> + struct sk_buff *skb;
> +
> + /* DMA address of the first descriptor */
> + dma_addr_t dma_desc;
> +
> + /* DMA address of packet data */
> + dma_addr_t dma_addr;
> +
> + /* Length of DMA mapping, less than skb->len if alignment
> + * buffer is used. */
> + unsigned int dma_len;
> +
> + /* Number of packets in chain starting here */
> + unsigned int chain_len;
> +
> + /* Packet chain ready to be submitted to hardware */
> + bool ready;
> +};
> +
> +struct nb8800_priv {

> + struct napi_struct napi;
> +
> + void __iomem *base;
> +
> + /* RX DMA descriptors */
> + struct nb8800_rx_desc *rx_descs;
> +
> + /* RX buffers referenced by DMA descriptors */
> + struct nb8800_rx_buf *rx_bufs;
> +
> + /* Current end of chain */
> + u32 rx_eoc;
> +
> + /* Value for rx interrupt time register in NAPI interrupt mode */
> + u32 rx_itr_irq;
> +
> + /* Value for rx interrupt time register in NAPI poll mode */
> + u32 rx_itr_poll;
> +
> + /* Value for config field of rx DMA descriptors */
> + u32 rx_dma_config;
> +
> + /* TX DMA descriptors */
> + struct nb8800_tx_desc *tx_descs;
> +
> + /* TX packet queue */
> + struct nb8800_tx_buf *tx_bufs;
> +
> + /* Number of free tx queue entries */
> + atomic_t tx_free;
> +
> + /* First free tx queue entry */
> + u32 tx_next;
> +
> + /* Next buffer to transmit */
> + u32 tx_queue;
> +
> + /* Start of current packet chain */
> + struct nb8800_tx_buf *tx_chain;
> +
> + /* Next buffer to reclaim */
> + u32 tx_done;
> +
> + /* Lock for DMA activation */
> + spinlock_t tx_lock;
> +
> + struct mii_bus *mii_bus;
> + struct device_node *phy_node;
> + struct phy_device *phydev;
> +
> + /* PHY connection type from DT */
> + int phy_mode;
> +
> + /* Current link status */
> + int speed;
> + int duplex;
> + int link;
> +
> + /* Pause settings */
> + bool pause_aneg;
> + bool pause_rx;
> + bool pause_tx;
> +
> + /* DMA base address of rx descriptors, see rx_descs above */
> + dma_addr_t rx_desc_dma;
> +
> + /* DMA base address of tx descriptors, see tx_descs above */
> + dma_addr_t tx_desc_dma;
> +
> + struct clk *clk;
> +};
> +
> +struct nb8800_ops {
> + int (*init)(struct net_device *dev);
> + int (*reset)(struct net_device *dev);
> +};
> +
> +#endif /* _NB8800_H_ */
> --
> 2.6.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/



--
With Best Regards,
Andy Shevchenko

2015-11-10 22:35:04

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

Andy Shevchenko <[email protected]> writes:

>> +static inline void nb8800_maskb(struct nb8800_priv *priv, int reg,
>> + u32 mask, u32 val)
>> +{
>> + u32 old = nb8800_readb(priv, reg);
>> + u32 new = (old & ~mask) | val;
>
> Shoudn't be "… | (val & mask);" ?

No, it's meant to replace the bits in mask with the corresponding bits
from val.

> + empty line?
>
>> + if (new != old)
>> + nb8800_writeb(priv, reg, new);
>> +}
>> +

[...]

>> +static void nb8800_receive(struct net_device *dev, int i, int len)
>
> unsigned int i ?
> len as well?

Does it matter? The values are nowhere near overflowing signed int.


[...]

>> + /* If a packet arrived after we last checked but
>> + * before writing RX_ITR, the interrupt will be
>> + * delayed, so we retrieve it now. */
>
> Block comments usually
> /*
> * text
> */

Documentation/CodingStyle says net/ and drivers/net/ are special, though
currently a mix of styles can be found. Personally, I don't
particularly care.

> Can be longer lines?

Still won't fit on two lines.

>> + if (priv->rx_descs[next].report)
>> + goto again;
>> +
>> + napi_complete_done(napi, work);
>> + }
>> +
>> + return work;
>> +}
>> +
>> +static void nb8800_tx_dma_start(struct net_device *dev)
>> +{
>> + struct nb8800_priv *priv = netdev_priv(dev);
>> + struct nb8800_tx_buf *txb;
>> + u32 txc_cr;
>> +
>> + txb = &priv->tx_bufs[priv->tx_queue];
>> + if (!txb->ready)
>> + return;
>> +
>> + txc_cr = nb8800_readl(priv, NB8800_TXC_CR);
>> + if (txc_cr & TCR_EN)
>> + return;
>> +
>> + nb8800_writel(priv, NB8800_TX_DESC_ADDR, txb->dma_desc);
>> + wmb(); /* ensure desc addr is written before starting DMA */
>
> Hm… Have I missed corresponding rmb() ? If it's about MMIO, perhaps mmiowb() ?

Possibly.

>> + nb8800_writel(priv, NB8800_TXC_CR, txc_cr | TCR_EN);
>> +
>> + priv->tx_queue = (priv->tx_queue + txb->chain_len) % TX_DESC_COUNT;
>> +}

--
Måns Rullgård
[email protected]

2015-11-10 22:40:56

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

On Wed, Nov 11, 2015 at 12:34 AM, Måns Rullgård <[email protected]> wrote:
> Andy Shevchenko <[email protected]> writes:
>
>>> +static inline void nb8800_maskb(struct nb8800_priv *priv, int reg,
>>> + u32 mask, u32 val)
>>> +{
>>> + u32 old = nb8800_readb(priv, reg);
>>> + u32 new = (old & ~mask) | val;
>>
>> Shoudn't be "… | (val & mask);" ?
>
> No, it's meant to replace the bits in mask with the corresponding bits
> from val.

But you unconditionally use entire val value which might bring bits
outside of mask.

>> Block comments usually
>> /*
>> * text
>> */
>
> Documentation/CodingStyle says net/ and drivers/net/ are special, though
> currently a mix of styles can be found. Personally, I don't
> particularly care.

OK.

>>> + nb8800_writel(priv, NB8800_TX_DESC_ADDR, txb->dma_desc);
>>> + wmb(); /* ensure desc addr is written before starting DMA */
>>
>> Hm… Have I missed corresponding rmb() ? If it's about MMIO, perhaps mmiowb() ?
>
> Possibly.

Standalone wmb() doesn't make sense.

--
With Best Regards,
Andy Shevchenko

2015-11-10 23:07:29

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

Andy Shevchenko <[email protected]> writes:

> On Wed, Nov 11, 2015 at 12:34 AM, Måns Rullgård <[email protected]> wrote:
>> Andy Shevchenko <[email protected]> writes:
>>
>>>> +static inline void nb8800_maskb(struct nb8800_priv *priv, int reg,
>>>> + u32 mask, u32 val)
>>>> +{
>>>> + u32 old = nb8800_readb(priv, reg);
>>>> + u32 new = (old & ~mask) | val;
>>>
>>> Shoudn't be "… | (val & mask);" ?
>>
>> No, it's meant to replace the bits in mask with the corresponding bits
>> from val.
>
> But you unconditionally use entire val value which might bring bits
> outside of mask.

Very well, I'll apply the mask to both then.

>>>> + nb8800_writel(priv, NB8800_TX_DESC_ADDR, txb->dma_desc);
>>>> + wmb(); /* ensure desc addr is written before starting DMA */
>>>
>>> Hm… Have I missed corresponding rmb() ? If it's about MMIO, perhaps mmiowb() ?
>>
>> Possibly.
>
> Standalone wmb() doesn't make sense.

It does if you need to enforce ordering between normal and I/O memory.
In fact, since the descriptor is filled in using normal memory accesses,
my understanding is that mmiowb() would be insufficient here. The
comment could be improved, however.

--
Måns Rullgård
[email protected]

2015-11-10 23:34:55

by Francois Romieu

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

Mans Rullgard <[email protected]> :
> diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
> new file mode 100644
> index 0000000..11cd389
> --- /dev/null
> +++ b/drivers/net/ethernet/aurora/nb8800.c
[...]
> +static int nb8800_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
[...]
> +
> + netdev_sent_queue(dev, skb->len);
> +
> + smp_mb__before_spinlock();
> + spin_lock_irqsave(&priv->tx_lock, flags);

At some point you may consider performing both Tx and Rx from napi context
and thus replacing priv->tx_lock with netif_tx_lock.

> +
> + if (!skb->xmit_more) {
> + priv->tx_chain->ready = true;
> + priv->tx_chain = NULL;
> + nb8800_tx_dma_start(dev);
> + }
> +
> + spin_unlock_irqrestore(&priv->tx_lock, flags);
> +
> + priv->tx_next = next;

Are there strong reasons why nb8800_tx_done could not kick between
spin_unlock_irqrestore and the non-local update of priv->tx_next ?

[...]
> +static irqreturn_t nb8800_irq(int irq, void *dev_id)
> +{
> + struct net_device *dev = dev_id;
> + struct nb8800_priv *priv = netdev_priv(dev);
> + u32 val;
> +
> + /* tx interrupt */
> + val = nb8800_readl(priv, NB8800_TXC_SR);
> + if (val) {
[...]
> + }
> +
> + /* rx interrupt */
> + val = nb8800_readl(priv, NB8800_RXC_SR);
> + if (val) {
[...]
> + }
> +
> + return IRQ_HANDLED;

Returning IRQ_HANDLED is fine if one of those hold:
1. you're sure that at least one of the "if" branch is used
2. you'll be able to quickly figure out what's happening whenever 1. stops
being true.

--
Ueimor

2015-11-11 00:36:45

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

On Wed, Nov 11, 2015 at 1:07 AM, Måns Rullgård <[email protected]> wrote:
> Andy Shevchenko <[email protected]> writes:

>>>>> + nb8800_writel(priv, NB8800_TX_DESC_ADDR, txb->dma_desc);
>>>>> + wmb(); /* ensure desc addr is written before starting DMA */
>>>>
>>>> Hm… Have I missed corresponding rmb() ? If it's about MMIO, perhaps mmiowb() ?
>>>
>>> Possibly.
>>
>> Standalone wmb() doesn't make sense.
>
> It does if you need to enforce ordering between normal and I/O memory.
> In fact, since the descriptor is filled in using normal memory accesses,
> my understanding is that mmiowb() would be insufficient here. The
> comment could be improved, however.

Can you then explain what exactly you are assured against in all cases
where you are using wmb()s? It seems I don't recognize this part in
some excerpts.


--
With Best Regards,
Andy Shevchenko

2015-11-11 00:40:16

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

Francois Romieu <[email protected]> writes:

> Mans Rullgard <[email protected]> :
>> diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
>> new file mode 100644
>> index 0000000..11cd389
>> --- /dev/null
>> +++ b/drivers/net/ethernet/aurora/nb8800.c
> [...]
>> +static int nb8800_xmit(struct sk_buff *skb, struct net_device *dev)
>> +{
> [...]
>> +
>> + netdev_sent_queue(dev, skb->len);
>> +
>> + smp_mb__before_spinlock();
>> + spin_lock_irqsave(&priv->tx_lock, flags);
>
> At some point you may consider performing both Tx and Rx from napi context
> and thus replacing priv->tx_lock with netif_tx_lock.

That lock is to synchronise the DMA start between nb8800_xmit() and the
interrupt handler. When the DMA complete interrupt arrives, the next
chain should be kicked off as quickly as possible, and I don't see why
that would benefit from being done in napi context.

>> +
>> + if (!skb->xmit_more) {
>> + priv->tx_chain->ready = true;
>> + priv->tx_chain = NULL;
>> + nb8800_tx_dma_start(dev);
>> + }
>> +
>> + spin_unlock_irqrestore(&priv->tx_lock, flags);
>> +
>> + priv->tx_next = next;
>
> Are there strong reasons why nb8800_tx_done could not kick between
> spin_unlock_irqrestore and the non-local update of priv->tx_next ?

Good catch. priv->tx_next wasn't accessed elsewhere in an earlier
version, and I forgot to fix that. nb8800_tx_done() makes sure the DMA
has actually finished, so priv->tx_next should be updated before
starting the DMA rather than after. The check against tx_next in
nb8800_tx_done() is only to put some limit on the loop and to avoid
confusion when nb8800_dma_stop() does it's dance. There should be no
need for more synchronisation here than what the already present memory
barriers provide.

> [...]
>> +static irqreturn_t nb8800_irq(int irq, void *dev_id)
>> +{
>> + struct net_device *dev = dev_id;
>> + struct nb8800_priv *priv = netdev_priv(dev);
>> + u32 val;
>> +
>> + /* tx interrupt */
>> + val = nb8800_readl(priv, NB8800_TXC_SR);
>> + if (val) {
> [...]
>> + }
>> +
>> + /* rx interrupt */
>> + val = nb8800_readl(priv, NB8800_RXC_SR);
>> + if (val) {
> [...]
>> + }
>> +
>> + return IRQ_HANDLED;
>
> Returning IRQ_HANDLED is fine if one of those hold:
> 1. you're sure that at least one of the "if" branch is used
> 2. you'll be able to quickly figure out what's happening whenever 1. stops
> being true.

You're right, better to check that the device really did have something
to say.

--
M?ns Rullg?rd
[email protected]

2015-11-11 00:44:28

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

Andy Shevchenko <[email protected]> writes:

> On Wed, Nov 11, 2015 at 1:07 AM, Måns Rullgård <[email protected]> wrote:
>> Andy Shevchenko <[email protected]> writes:
>
>>>>>> + nb8800_writel(priv, NB8800_TX_DESC_ADDR, txb->dma_desc);
>>>>>> + wmb(); /* ensure desc addr is written before starting DMA */
>>>>>
>>>>> Hm… Have I missed corresponding rmb() ? If it's about MMIO, perhaps mmiowb() ?
>>>>
>>>> Possibly.
>>>
>>> Standalone wmb() doesn't make sense.
>>
>> It does if you need to enforce ordering between normal and I/O memory.
>> In fact, since the descriptor is filled in using normal memory accesses,
>> my understanding is that mmiowb() would be insufficient here. The
>> comment could be improved, however.
>
> Can you then explain what exactly you are assured against in all cases
> where you are using wmb()s? It seems I don't recognize this part in
> some excerpts.

Certainly. I'll re-read memory-barriers.txt to make sure I'm doing the
right thing, then write a better comment.

--
Måns Rullgård
[email protected]

2015-11-11 02:11:26

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

From: M?ns Rullg?rd <[email protected]>
Date: Wed, 11 Nov 2015 00:40:09 +0000

> When the DMA complete interrupt arrives, the next chain should be
> kicked off as quickly as possible, and I don't see why that would
> benefit from being done in napi context.

NAPI isn't about low latency, it's about fairness and interrupt
mitigation.

You probably don't even realize that all of the TX SKB freeing you do
in the hardware interrupt handler end up being actually processed by a
scheduled software interrupt anyways.

So you are gaining almost nothing by not doing TX completion in NAPI
context, whereas by doing so you would be gaining a lot including
more simplified locking or even the ability to do no locking at all.

2015-11-11 12:22:38

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

David Miller <[email protected]> writes:

> From: M?ns Rullg?rd <[email protected]>
> Date: Wed, 11 Nov 2015 00:40:09 +0000
>
>> When the DMA complete interrupt arrives, the next chain should be
>> kicked off as quickly as possible, and I don't see why that would
>> benefit from being done in napi context.
>
> NAPI isn't about low latency, it's about fairness and interrupt
> mitigation.
>
> You probably don't even realize that all of the TX SKB freeing you do
> in the hardware interrupt handler end up being actually processed by a
> scheduled software interrupt anyways.
>
> So you are gaining almost nothing by not doing TX completion in NAPI
> context, whereas by doing so you would be gaining a lot including
> more simplified locking or even the ability to do no locking at all.

TX completion is separate from restarting the DMA, and moving that to
NAPI may well be a good idea. Should I simply napi_schedule() if the
hardware indicates TX is complete and do the cleanup in the NAPI poll
function?

--
M?ns Rullg?rd
[email protected]

2015-11-11 13:04:27

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

M?ns Rullg?rd <[email protected]> writes:

> David Miller <[email protected]> writes:
>
>> From: M?ns Rullg?rd <[email protected]>
>> Date: Wed, 11 Nov 2015 00:40:09 +0000
>>
>>> When the DMA complete interrupt arrives, the next chain should be
>>> kicked off as quickly as possible, and I don't see why that would
>>> benefit from being done in napi context.
>>
>> NAPI isn't about low latency, it's about fairness and interrupt
>> mitigation.
>>
>> You probably don't even realize that all of the TX SKB freeing you do
>> in the hardware interrupt handler end up being actually processed by a
>> scheduled software interrupt anyways.
>>
>> So you are gaining almost nothing by not doing TX completion in NAPI
>> context, whereas by doing so you would be gaining a lot including
>> more simplified locking or even the ability to do no locking at all.
>
> TX completion is separate from restarting the DMA, and moving that to
> NAPI may well be a good idea. Should I simply napi_schedule() if the
> hardware indicates TX is complete and do the cleanup in the NAPI poll
> function?

I tried that, and throughput (as measured by iperf3) dropped by 2%.
Maybe I did something wrong.

--
M?ns Rullg?rd
[email protected]

2015-11-11 13:29:59

by Eric Dumazet

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

On Wed, 2015-11-11 at 13:04 +0000, Måns Rullgård wrote:

> I tried that, and throughput (as measured by iperf3) dropped by 2%.
> Maybe I did something wrong.

What link speed have you used, what was the throughput you got,
and is the receiver using the same NIC ?



2015-11-11 13:41:36

by Mason

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

On 10/11/2015 22:51, Eric Dumazet wrote:

> On Tue, 2015-11-10 at 21:21 +0000, M?ns Rullg?rd wrote:
>
>> Even ixgbe uses napi_complete() while netdevice.h says one should
>> "consider using napi_complete_done() instead." Did the author consider
>> it and decide not to, or has the driver simply not been updated?
>
> napi_complete_done() is quite new, very few drivers use it.

I was hoping to back-port this driver for my platform. Are the recent
features used in the driver available in 4.1? What about 3.14?

Regards.

2015-11-11 13:49:06

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

Eric Dumazet <[email protected]> writes:

> On Wed, 2015-11-11 at 13:04 +0000, M?ns Rullg?rd wrote:
>
>> I tried that, and throughput (as measured by iperf3) dropped by 2%.
>> Maybe I did something wrong.
>
> What link speed have you used, what was the throughput you got,
> and is the receiver using the same NIC ?

1Gbps link, 640 Mbps TCP transmit throughput to a PC with Intel NIC.
Why does it matter what NIC the receiver has?

--
M?ns Rullg?rd
[email protected]

2015-11-11 13:54:54

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

Mason <[email protected]> writes:

> On 10/11/2015 22:51, Eric Dumazet wrote:
>
>> On Tue, 2015-11-10 at 21:21 +0000, M?ns Rullg?rd wrote:
>>
>>> Even ixgbe uses napi_complete() while netdevice.h says one should
>>> "consider using napi_complete_done() instead." Did the author consider
>>> it and decide not to, or has the driver simply not been updated?
>>
>> napi_complete_done() is quite new, very few drivers use it.
>
> I was hoping to back-port this driver for my platform. Are the recent
> features used in the driver available in 4.1? What about 3.14?

xmit_more was added in 3.18, napi_complete_done() in 3.19.

--
M?ns Rullg?rd
[email protected]

2015-11-11 14:06:17

by Eric Dumazet

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

On Wed, 2015-11-11 at 14:41 +0100, Mason wrote:
> On 10/11/2015 22:51, Eric Dumazet wrote:
>
> > On Tue, 2015-11-10 at 21:21 +0000, Måns Rullgård wrote:
> >
> >> Even ixgbe uses napi_complete() while netdevice.h says one should
> >> "consider using napi_complete_done() instead." Did the author consider
> >> it and decide not to, or has the driver simply not been updated?
> >
> > napi_complete_done() is quite new, very few drivers use it.
>
> I was hoping to back-port this driver for my platform. Are the recent
> features used in the driver available in 4.1? What about 3.14?

Sure, this is all available

napi_complete_done() can be backported on old kernels as a
napi_complete()

2015-11-11 14:09:26

by Eric Dumazet

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

On Wed, 2015-11-11 at 13:48 +0000, Måns Rullgård wrote:
> Eric Dumazet <[email protected]> writes:
>
> > On Wed, 2015-11-11 at 13:04 +0000, Måns Rullgård wrote:
> >
> >> I tried that, and throughput (as measured by iperf3) dropped by 2%.
> >> Maybe I did something wrong.
> >
> > What link speed have you used, what was the throughput you got,
> > and is the receiver using the same NIC ?
>
> 1Gbps link, 640 Mbps TCP transmit throughput to a PC with Intel NIC.
> Why does it matter what NIC the receiver has?

Because at 1Gb line rate, you better get GRO properly implemented in the
receiver, so that TCP stack does not send one ACK every 2 MSS.

Send speed is also dependent on the number of ACK packets the sender has
to process.

This is why I suggested you use napi_gro_receive() in your driver.


2015-11-11 14:10:23

by Eric Dumazet

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

On Wed, 2015-11-11 at 13:54 +0000, Måns Rullgård wrote:
> Mason <[email protected]> writes:
>
> > On 10/11/2015 22:51, Eric Dumazet wrote:
> >
> >> On Tue, 2015-11-10 at 21:21 +0000, Måns Rullgård wrote:
> >>
> >>> Even ixgbe uses napi_complete() while netdevice.h says one should
> >>> "consider using napi_complete_done() instead." Did the author consider
> >>> it and decide not to, or has the driver simply not been updated?
> >>
> >> napi_complete_done() is quite new, very few drivers use it.
> >
> > I was hoping to back-port this driver for my platform. Are the recent
> > features used in the driver available in 4.1? What about 3.14?
>
> xmit_more was added in 3.18, napi_complete_done() in 3.19.

xmit_more is a hint, you can simply consider skb->xmit_more being
replaced by 0 on old kernels if you want to backport.


2015-11-11 14:15:35

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

Eric Dumazet <[email protected]> writes:

> On Wed, 2015-11-11 at 13:48 +0000, M?ns Rullg?rd wrote:
>> Eric Dumazet <[email protected]> writes:
>>
>> > On Wed, 2015-11-11 at 13:04 +0000, M?ns Rullg?rd wrote:
>> >
>> >> I tried that, and throughput (as measured by iperf3) dropped by 2%.
>> >> Maybe I did something wrong.
>> >
>> > What link speed have you used, what was the throughput you got,
>> > and is the receiver using the same NIC ?
>>
>> 1Gbps link, 640 Mbps TCP transmit throughput to a PC with Intel NIC.
>> Why does it matter what NIC the receiver has?
>
> Because at 1Gb line rate, you better get GRO properly implemented in the
> receiver, so that TCP stack does not send one ACK every 2 MSS.
>
> Send speed is also dependent on the number of ACK packets the sender has
> to process.
>
> This is why I suggested you use napi_gro_receive() in your driver.

FWIW, with UDP I get 650 Mbps.

--
M?ns Rullg?rd
[email protected]

2015-11-11 14:35:26

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

M?ns Rullg?rd <[email protected]> writes:

> Eric Dumazet <[email protected]> writes:
>
>> On Wed, 2015-11-11 at 13:48 +0000, M?ns Rullg?rd wrote:
>>> Eric Dumazet <[email protected]> writes:
>>>
>>> > On Wed, 2015-11-11 at 13:04 +0000, M?ns Rullg?rd wrote:
>>> >
>>> >> I tried that, and throughput (as measured by iperf3) dropped by 2%.
>>> >> Maybe I did something wrong.
>>> >
>>> > What link speed have you used, what was the throughput you got,
>>> > and is the receiver using the same NIC ?
>>>
>>> 1Gbps link, 640 Mbps TCP transmit throughput to a PC with Intel NIC.
>>> Why does it matter what NIC the receiver has?
>>
>> Because at 1Gb line rate, you better get GRO properly implemented in the
>> receiver, so that TCP stack does not send one ACK every 2 MSS.
>>
>> Send speed is also dependent on the number of ACK packets the sender has
>> to process.
>>
>> This is why I suggested you use napi_gro_receive() in your driver.
>
> FWIW, with UDP I get 650 Mbps.

It seems the on-chip interconnect is limiting memory bandwidth available
to the Ethernet DMA. If I increase the limits, I get 800 Mbps over TCP
and 850 Mbps over UDP with the driver posted here, the TCP case being
CPU bound in csum_partial_copy_from_user.

--
M?ns Rullg?rd
[email protected]

2015-11-11 14:42:14

by Eric Dumazet

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

On Wed, 2015-11-11 at 14:15 +0000, Måns Rullgård wrote:

> FWIW, with UDP I get 650 Mbps.

Have you tried pktgen ? It should give you raw numbers, without the
stack overhead, especially when using the clone operator.

(It also has xmit_more support)

If the NIC can not get more than 650 Mbps, not worth trying implementing
tso in your driver (using net/core/tso.c helpers)


2015-11-11 14:44:37

by Eric Dumazet

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

On Wed, 2015-11-11 at 14:35 +0000, Måns Rullgård wrote:

> It seems the on-chip interconnect is limiting memory bandwidth available
> to the Ethernet DMA. If I increase the limits, I get 800 Mbps over TCP
> and 850 Mbps over UDP with the driver posted here, the TCP case being
> CPU bound in csum_partial_copy_from_user.

Ah right, NIC has no TX checksum offload :(


2015-11-11 16:21:06

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

From: M?ns Rullg?rd <[email protected]>
Date: Wed, 11 Nov 2015 12:22:28 +0000

> David Miller <[email protected]> writes:
>
>> From: M?ns Rullg?rd <[email protected]>
>> Date: Wed, 11 Nov 2015 00:40:09 +0000
>>
>>> When the DMA complete interrupt arrives, the next chain should be
>>> kicked off as quickly as possible, and I don't see why that would
>>> benefit from being done in napi context.
>>
>> NAPI isn't about low latency, it's about fairness and interrupt
>> mitigation.
>>
>> You probably don't even realize that all of the TX SKB freeing you do
>> in the hardware interrupt handler end up being actually processed by a
>> scheduled software interrupt anyways.
>>
>> So you are gaining almost nothing by not doing TX completion in NAPI
>> context, whereas by doing so you would be gaining a lot including
>> more simplified locking or even the ability to do no locking at all.
>
> TX completion is separate from restarting the DMA, and moving that to
> NAPI may well be a good idea. Should I simply napi_schedule() if the
> hardware indicates TX is complete and do the cleanup in the NAPI poll
> function?

... just like every other high end driver... Yes.

2015-11-11 16:24:30

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

From: M?ns Rullg?rd <[email protected]>
Date: Wed, 11 Nov 2015 13:04:07 +0000

> M?ns Rullg?rd <[email protected]> writes:
>
>> David Miller <[email protected]> writes:
>>
>>> From: M?ns Rullg?rd <[email protected]>
>>> Date: Wed, 11 Nov 2015 00:40:09 +0000
>>>
>>>> When the DMA complete interrupt arrives, the next chain should be
>>>> kicked off as quickly as possible, and I don't see why that would
>>>> benefit from being done in napi context.
>>>
>>> NAPI isn't about low latency, it's about fairness and interrupt
>>> mitigation.
>>>
>>> You probably don't even realize that all of the TX SKB freeing you do
>>> in the hardware interrupt handler end up being actually processed by a
>>> scheduled software interrupt anyways.
>>>
>>> So you are gaining almost nothing by not doing TX completion in NAPI
>>> context, whereas by doing so you would be gaining a lot including
>>> more simplified locking or even the ability to do no locking at all.
>>
>> TX completion is separate from restarting the DMA, and moving that to
>> NAPI may well be a good idea. Should I simply napi_schedule() if the
>> hardware indicates TX is complete and do the cleanup in the NAPI poll
>> function?
>
> I tried that, and throughput (as measured by iperf3) dropped by 2%.
> Maybe I did something wrong.

Did you fix all the locking in that change?

Since all of your TX handling runs in software interrupt context, you
can stop using IRQ locking and use BH locking driver-wide instead.

And actually, no locking is really needed for TX processing. With
proper memory barriers and properly crafter queue state tests, you
can run completely lockless.

Again, look at example drivers. I know, for example, that
drivers/net/ethernet/broadcom/tg3.c runs TX lockless. You'll
see that tg3_tx() takes no locks at all.

2015-11-11 18:25:16

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

David Miller <[email protected]> writes:

> From: M?ns Rullg?rd <[email protected]>
> Date: Wed, 11 Nov 2015 13:04:07 +0000
>
>> M?ns Rullg?rd <[email protected]> writes:
>>
>>> David Miller <[email protected]> writes:
>>>
>>>> From: M?ns Rullg?rd <[email protected]>
>>>> Date: Wed, 11 Nov 2015 00:40:09 +0000
>>>>
>>>>> When the DMA complete interrupt arrives, the next chain should be
>>>>> kicked off as quickly as possible, and I don't see why that would
>>>>> benefit from being done in napi context.
>>>>
>>>> NAPI isn't about low latency, it's about fairness and interrupt
>>>> mitigation.
>>>>
>>>> You probably don't even realize that all of the TX SKB freeing you do
>>>> in the hardware interrupt handler end up being actually processed by a
>>>> scheduled software interrupt anyways.
>>>>
>>>> So you are gaining almost nothing by not doing TX completion in NAPI
>>>> context, whereas by doing so you would be gaining a lot including
>>>> more simplified locking or even the ability to do no locking at all.
>>>
>>> TX completion is separate from restarting the DMA, and moving that to
>>> NAPI may well be a good idea. Should I simply napi_schedule() if the
>>> hardware indicates TX is complete and do the cleanup in the NAPI poll
>>> function?
>>
>> I tried that, and throughput (as measured by iperf3) dropped by 2%.
>> Maybe I did something wrong.
>
> Did you fix all the locking in that change?
>
> Since all of your TX handling runs in software interrupt context, you
> can stop using IRQ locking and use BH locking driver-wide instead.
>
> And actually, no locking is really needed for TX processing. With
> proper memory barriers and properly crafter queue state tests, you
> can run completely lockless.
>
> Again, look at example drivers. I know, for example, that
> drivers/net/ethernet/broadcom/tg3.c runs TX lockless. You'll
> see that tg3_tx() takes no locks at all.

The way the hardware works, once a DMA operation has been started,
adding more packets to the active chain can't be done reliably. For
that reason, if start_xmit is called (with xmit_more zero) while a DMA
operation is in progress, the new packet(s) must be queued until the
hardware raises the DMA complete interrupt. At that time, the next
pending DMA chain, if any, can be kicked off. If the TX DMA channel is
idle when start_xmit is called, it can be started immediately. Checking
the DMA status and starting it if idle has to be done atomically
somehow.

There is a separate indication for actual TX completion, and the
interrupt for that can be set to only fire every 7 frames or when a
timeout expires. When this happens, the TX cleanup needs to run, and
that can obviously be done from NAPI without using any locks.

Bear in mind that this hardware is quite primitive compared to modern
high-performance Ethernet controllers from the likes of Intel and
Broadcom. The documentation I have is dated 2003.

--
M?ns Rullg?rd
[email protected]

2015-11-11 19:02:55

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

From: M?ns Rullg?rd <[email protected]>
Date: Wed, 11 Nov 2015 18:25:05 +0000

> If the TX DMA channel is idle when start_xmit is called, it can be
> started immediately. Checking the DMA status and starting it if
> idle has to be done atomically somehow.

->ndo_start_xmit() is guaranteed to be invoked atomically, protected
by the TX queue spinlock.

2015-11-11 19:09:23

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

David Miller <[email protected]> writes:

> From: M?ns Rullg?rd <[email protected]>
> Date: Wed, 11 Nov 2015 18:25:05 +0000
>
>> If the TX DMA channel is idle when start_xmit is called, it can be
>> started immediately. Checking the DMA status and starting it if
>> idle has to be done atomically somehow.
>
> ->ndo_start_xmit() is guaranteed to be invoked atomically, protected
> by the TX queue spinlock.

Yes, but the DMA needs to be restarted from some other context if it was
busy when start_xmit checked.

--
M?ns Rullg?rd
[email protected]

2015-11-11 19:13:13

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

From: M?ns Rullg?rd <[email protected]>
Date: Wed, 11 Nov 2015 19:09:19 +0000

> David Miller <[email protected]> writes:
>
>> From: M?ns Rullg?rd <[email protected]>
>> Date: Wed, 11 Nov 2015 18:25:05 +0000
>>
>>> If the TX DMA channel is idle when start_xmit is called, it can be
>>> started immediately. Checking the DMA status and starting it if
>>> idle has to be done atomically somehow.
>>
>> ->ndo_start_xmit() is guaranteed to be invoked atomically, protected
>> by the TX queue spinlock.
>
> Yes, but the DMA needs to be restarted from some other context if it was
> busy when start_xmit checked.

Then you can probably use the TXQ lock in the interrupt handler just for
that.

2015-11-11 19:17:11

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

David Miller <[email protected]> writes:

> From: M?ns Rullg?rd <[email protected]>
> Date: Wed, 11 Nov 2015 19:09:19 +0000
>
>> David Miller <[email protected]> writes:
>>
>>> From: M?ns Rullg?rd <[email protected]>
>>> Date: Wed, 11 Nov 2015 18:25:05 +0000
>>>
>>>> If the TX DMA channel is idle when start_xmit is called, it can be
>>>> started immediately. Checking the DMA status and starting it if
>>>> idle has to be done atomically somehow.
>>>
>>> ->ndo_start_xmit() is guaranteed to be invoked atomically, protected
>>> by the TX queue spinlock.
>>
>> Yes, but the DMA needs to be restarted from some other context if it was
>> busy when start_xmit checked.
>
> Then you can probably use the TXQ lock in the interrupt handler just for
> that.

That seems a bit heavy-handed when the critical section for this is only
a tiny part of the start_xmit function.

--
M?ns Rullg?rd
[email protected]

2015-11-11 19:20:04

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

From: M?ns Rullg?rd <[email protected]>
Date: Wed, 11 Nov 2015 19:17:07 +0000

> David Miller <[email protected]> writes:
>
>> From: M?ns Rullg?rd <[email protected]>
>> Date: Wed, 11 Nov 2015 19:09:19 +0000
>>
>>> David Miller <[email protected]> writes:
>>>
>>>> From: M?ns Rullg?rd <[email protected]>
>>>> Date: Wed, 11 Nov 2015 18:25:05 +0000
>>>>
>>>>> If the TX DMA channel is idle when start_xmit is called, it can be
>>>>> started immediately. Checking the DMA status and starting it if
>>>>> idle has to be done atomically somehow.
>>>>
>>>> ->ndo_start_xmit() is guaranteed to be invoked atomically, protected
>>>> by the TX queue spinlock.
>>>
>>> Yes, but the DMA needs to be restarted from some other context if it was
>>> busy when start_xmit checked.
>>
>> Then you can probably use the TXQ lock in the interrupt handler just for
>> that.
>
> That seems a bit heavy-handed when the critical section for this is only
> a tiny part of the start_xmit function.

Then what synchornization primitive other than spin locks are you going
to use for this?

My point is that there is a spinlock the core code is _already_ taking,
unconditionally, when ->ndo_start_xmit() executes. And you can therefore
take advantage of that rather than using another lock of your own.

2015-11-11 19:25:54

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

David Miller <[email protected]> writes:

> From: M?ns Rullg?rd <[email protected]>
> Date: Wed, 11 Nov 2015 19:17:07 +0000
>
>> David Miller <[email protected]> writes:
>>
>>> From: M?ns Rullg?rd <[email protected]>
>>> Date: Wed, 11 Nov 2015 19:09:19 +0000
>>>
>>>> David Miller <[email protected]> writes:
>>>>
>>>>> From: M?ns Rullg?rd <[email protected]>
>>>>> Date: Wed, 11 Nov 2015 18:25:05 +0000
>>>>>
>>>>>> If the TX DMA channel is idle when start_xmit is called, it can be
>>>>>> started immediately. Checking the DMA status and starting it if
>>>>>> idle has to be done atomically somehow.
>>>>>
>>>>> ->ndo_start_xmit() is guaranteed to be invoked atomically, protected
>>>>> by the TX queue spinlock.
>>>>
>>>> Yes, but the DMA needs to be restarted from some other context if it was
>>>> busy when start_xmit checked.
>>>
>>> Then you can probably use the TXQ lock in the interrupt handler just for
>>> that.
>>
>> That seems a bit heavy-handed when the critical section for this is only
>> a tiny part of the start_xmit function.
>
> Then what synchornization primitive other than spin locks are you going
> to use for this?
>
> My point is that there is a spinlock the core code is _already_ taking,
> unconditionally, when ->ndo_start_xmit() executes. And you can therefore
> take advantage of that rather than using another lock of your own.

I get that. But that remains locked for the duration of ndo_start_xmit()
whereas the part that needs to be synchronised with the DMA completion
IRQ handler is tiny. Having the IRQ handler spin for the duration of
ndo_start_xmit() seemed silly to me.

--
M?ns Rullg?rd
[email protected]

2015-11-11 19:26:53

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

From: M?ns Rullg?rd <[email protected]>
Date: Wed, 11 Nov 2015 19:25:46 +0000

> David Miller <[email protected]> writes:
>
>> From: M?ns Rullg?rd <[email protected]>
>> Date: Wed, 11 Nov 2015 19:17:07 +0000
>>
>>> David Miller <[email protected]> writes:
>>>
>>>> From: M?ns Rullg?rd <[email protected]>
>>>> Date: Wed, 11 Nov 2015 19:09:19 +0000
>>>>
>>>>> David Miller <[email protected]> writes:
>>>>>
>>>>>> From: M?ns Rullg?rd <[email protected]>
>>>>>> Date: Wed, 11 Nov 2015 18:25:05 +0000
>>>>>>
>>>>>>> If the TX DMA channel is idle when start_xmit is called, it can be
>>>>>>> started immediately. Checking the DMA status and starting it if
>>>>>>> idle has to be done atomically somehow.
>>>>>>
>>>>>> ->ndo_start_xmit() is guaranteed to be invoked atomically, protected
>>>>>> by the TX queue spinlock.
>>>>>
>>>>> Yes, but the DMA needs to be restarted from some other context if it was
>>>>> busy when start_xmit checked.
>>>>
>>>> Then you can probably use the TXQ lock in the interrupt handler just for
>>>> that.
>>>
>>> That seems a bit heavy-handed when the critical section for this is only
>>> a tiny part of the start_xmit function.
>>
>> Then what synchornization primitive other than spin locks are you going
>> to use for this?
>>
>> My point is that there is a spinlock the core code is _already_ taking,
>> unconditionally, when ->ndo_start_xmit() executes. And you can therefore
>> take advantage of that rather than using another lock of your own.
>
> I get that. But that remains locked for the duration of ndo_start_xmit()
> whereas the part that needs to be synchronised with the DMA completion
> IRQ handler is tiny. Having the IRQ handler spin for the duration of
> ndo_start_xmit() seemed silly to me.

I don't think it's silly at all.

And unless you can measure it making a difference, don't knock the idea.

2015-11-11 19:35:10

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

David Miller <[email protected]> writes:

> From: M?ns Rullg?rd <[email protected]>
> Date: Wed, 11 Nov 2015 19:25:46 +0000
>
>> David Miller <[email protected]> writes:
>>
>>> From: M?ns Rullg?rd <[email protected]>
>>> Date: Wed, 11 Nov 2015 19:17:07 +0000
>>>
>>>> David Miller <[email protected]> writes:
>>>>
>>>>> From: M?ns Rullg?rd <[email protected]>
>>>>> Date: Wed, 11 Nov 2015 19:09:19 +0000
>>>>>
>>>>>> David Miller <[email protected]> writes:
>>>>>>
>>>>>>> From: M?ns Rullg?rd <[email protected]>
>>>>>>> Date: Wed, 11 Nov 2015 18:25:05 +0000
>>>>>>>
>>>>>>>> If the TX DMA channel is idle when start_xmit is called, it can be
>>>>>>>> started immediately. Checking the DMA status and starting it if
>>>>>>>> idle has to be done atomically somehow.
>>>>>>>
>>>>>>> ->ndo_start_xmit() is guaranteed to be invoked atomically, protected
>>>>>>> by the TX queue spinlock.
>>>>>>
>>>>>> Yes, but the DMA needs to be restarted from some other context if it was
>>>>>> busy when start_xmit checked.
>>>>>
>>>>> Then you can probably use the TXQ lock in the interrupt handler just for
>>>>> that.
>>>>
>>>> That seems a bit heavy-handed when the critical section for this is only
>>>> a tiny part of the start_xmit function.
>>>
>>> Then what synchornization primitive other than spin locks are you going
>>> to use for this?
>>>
>>> My point is that there is a spinlock the core code is _already_ taking,
>>> unconditionally, when ->ndo_start_xmit() executes. And you can therefore
>>> take advantage of that rather than using another lock of your own.
>>
>> I get that. But that remains locked for the duration of ndo_start_xmit()
>> whereas the part that needs to be synchronised with the DMA completion
>> IRQ handler is tiny. Having the IRQ handler spin for the duration of
>> ndo_start_xmit() seemed silly to me.
>
> I don't think it's silly at all.

I'm sure I read somewhere that the time spent spinning on a lock should
be kept as small as possible.

> And unless you can measure it making a difference, don't knock the idea.

I tried using netif_tx_lock() in the IRQ handler instead, and it locked
up solid. Clearly that was the wrong thing to do.

--
M?ns Rullg?rd
[email protected]

2015-11-11 19:48:43

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

From: M?ns Rullg?rd <[email protected]>
Date: Wed, 11 Nov 2015 19:35:05 +0000

>> I don't think it's silly at all.
>
> I'm sure I read somewhere that the time spent spinning on a lock should
> be kept as small as possible.
>
>> And unless you can measure it making a difference, don't knock the idea.
>
> I tried using netif_tx_lock() in the IRQ handler instead, and it locked
> up solid. Clearly that was the wrong thing to do.

Oh that's right, it's a BH lock not an IRQ one.

Yet another argument for doing everything in ->poll(), thus making all
operations outside of NAPI scheduling run in software interrupt
context, and therefore being able to make use of the TXQ lock for
this.

2015-11-11 20:48:06

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

David Miller <[email protected]> writes:

> From: M?ns Rullg?rd <[email protected]>
> Date: Wed, 11 Nov 2015 19:35:05 +0000
>
>>> I don't think it's silly at all.
>>
>> I'm sure I read somewhere that the time spent spinning on a lock should
>> be kept as small as possible.
>>
>>> And unless you can measure it making a difference, don't knock the idea.
>>
>> I tried using netif_tx_lock() in the IRQ handler instead, and it locked
>> up solid. Clearly that was the wrong thing to do.
>
> Oh that's right, it's a BH lock not an IRQ one.
>
> Yet another argument for doing everything in ->poll(), thus making all
> operations outside of NAPI scheduling run in software interrupt
> context, and therefore being able to make use of the TXQ lock for
> this.

Well, I tried calling the DMA restart function from NAPI poll under
netif_tx_lock(). Now it works only as long as there is incoming
traffic.

--
M?ns Rullg?rd
[email protected]

2015-11-12 13:33:58

by Mason

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

On 10/11/2015 20:25, M?ns Rullg?rd wrote:

> Mason writes:
>
>> On 10/11/2015 17:14, Mans Rullgard wrote:
>>
>>> This adds a driver for the Aurora VLSI NB8800 Ethernet controller.
>>> It is an almost complete rewrite of a driver originally found in
>>> a Sigma Designs 2.6.22 tree.
>>>
>>> Signed-off-by: Mans Rullgard <[email protected]>
>>> ---
>>> Changes:
>>> - Refactored mdio access functions
>>> - Refactored register access helpers
>>> - Improved error handling in rx buffer allocation
>>> - Optimised some fifo parameters
>>> - Overhauled tx dma. Multiple packets are now chained in a single dma
>>> operation if xmit_more is set, improving performance.
>>> - Improved rx irq handling. It's not possible to disable interrupts
>>> entirely for napi poll, but they can be slowed down a little.
>>> - Use readx_poll_timeout in various places
>>> - Improved error detection
>>> - Improved statistics
>>> - Report hardware statistics counters through ethtool
>>> - Improved tangox-specific setup
>>> - Support for flow control using pause frames
>>> - Explanatory comments added
>>> - Various minor stylistic changes
>>> ---
>>> drivers/net/ethernet/Kconfig | 1 +
>>> drivers/net/ethernet/Makefile | 1 +
>>> drivers/net/ethernet/aurora/Kconfig | 20 +
>>> drivers/net/ethernet/aurora/Makefile | 1 +
>>> drivers/net/ethernet/aurora/nb8800.c | 1530 ++++++++++++++++++++++++++++++++++
>>> drivers/net/ethernet/aurora/nb8800.h | 314 +++++++
>>> 6 files changed, 1867 insertions(+)
>>
>> The code has grown much since the previous patch, despite some
>> refactoring. Is this mostly due to ethtool_ops support?
>>
>> drivers/net/ethernet/aurora/nb8800.c | 1146 ++++++++++++++++++++++++++++++++++
>> drivers/net/ethernet/aurora/nb8800.h | 230 +++++++
>
> Some of the increase is from new features, some from improvements, and
> then there are a bunch of new comments.

Sweet.

With this version, my kernel boots faster than before
(I had been using a 5 month-old version.)

Before:

[ 0.613623] tangox-enet 26000.ethernet: SMP86xx internal Ethernet at 0x26000
[ 0.623638] libphy: tangox-mii: probed
[ 0.686527] tangox-enet 26000.ethernet: PHY: found Atheros 8035 ethernet at 0x4
[ 0.697169] tangox-enet 26000.ethernet eth0: MAC address 00:16:e8:02:08:42
...
[ 1.306360] Sending DHCP requests ..
[ 4.699969] tangox-enet 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
[ 8.899671] ., OK
[ 8.926343] IP-Config: Got DHCP answer from 172.27.200.1, my address is 172.27.64.49
...
[ 8.987327] Freeing unused kernel memory: 168K (c039e000 - c03c8000)

After:

[ 0.623526] libphy: nb8800-mii: probed
[ 0.628092] nb8800 26000.ethernet eth0: MAC address 00:16:e8:02:08:42
...
[ 4.732948] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
[ 4.752655] Sending DHCP requests ., OK
[ 4.782644] IP-Config: Got DHCP answer from 172.27.200.1, my address is 172.27.64.49
...
[ 4.849298] Freeing unused kernel memory: 164K (c039f000 - c03c8000)


The DHCP request is sent later, but the kernel doesn't twiddle its thumbs
for 4 seconds after the link comes up. Does this come from not probing the
PHY anymore?

BTW, you're not using the PHY IRQ, right? I think I remember you saying
it didn't work reliably?

Regards.

2015-11-12 14:04:54

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

Mason <[email protected]> writes:

> On 10/11/2015 20:25, M?ns Rullg?rd wrote:
>
>> Mason writes:
>>
>>> On 10/11/2015 17:14, Mans Rullgard wrote:
>>>
>>>> This adds a driver for the Aurora VLSI NB8800 Ethernet controller.
>>>> It is an almost complete rewrite of a driver originally found in
>>>> a Sigma Designs 2.6.22 tree.
>>>>
>>>> Signed-off-by: Mans Rullgard <[email protected]>
>>>> ---
>>>> Changes:
>>>> - Refactored mdio access functions
>>>> - Refactored register access helpers
>>>> - Improved error handling in rx buffer allocation
>>>> - Optimised some fifo parameters
>>>> - Overhauled tx dma. Multiple packets are now chained in a single dma
>>>> operation if xmit_more is set, improving performance.
>>>> - Improved rx irq handling. It's not possible to disable interrupts
>>>> entirely for napi poll, but they can be slowed down a little.
>>>> - Use readx_poll_timeout in various places
>>>> - Improved error detection
>>>> - Improved statistics
>>>> - Report hardware statistics counters through ethtool
>>>> - Improved tangox-specific setup
>>>> - Support for flow control using pause frames
>>>> - Explanatory comments added
>>>> - Various minor stylistic changes
>>>> ---
>>>> drivers/net/ethernet/Kconfig | 1 +
>>>> drivers/net/ethernet/Makefile | 1 +
>>>> drivers/net/ethernet/aurora/Kconfig | 20 +
>>>> drivers/net/ethernet/aurora/Makefile | 1 +
>>>> drivers/net/ethernet/aurora/nb8800.c | 1530 ++++++++++++++++++++++++++++++++++
>>>> drivers/net/ethernet/aurora/nb8800.h | 314 +++++++
>>>> 6 files changed, 1867 insertions(+)
>>>
>>> The code has grown much since the previous patch, despite some
>>> refactoring. Is this mostly due to ethtool_ops support?
>>>
>>> drivers/net/ethernet/aurora/nb8800.c | 1146 ++++++++++++++++++++++++++++++++++
>>> drivers/net/ethernet/aurora/nb8800.h | 230 +++++++
>>
>> Some of the increase is from new features, some from improvements, and
>> then there are a bunch of new comments.
>
> Sweet.
>
> With this version, my kernel boots faster than before
> (I had been using a 5 month-old version.)
>
> Before:
>
> [ 0.613623] tangox-enet 26000.ethernet: SMP86xx internal Ethernet at 0x26000
> [ 0.623638] libphy: tangox-mii: probed
> [ 0.686527] tangox-enet 26000.ethernet: PHY: found Atheros 8035 ethernet at 0x4
> [ 0.697169] tangox-enet 26000.ethernet eth0: MAC address 00:16:e8:02:08:42
> ...
> [ 1.306360] Sending DHCP requests ..
> [ 4.699969] tangox-enet 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
> [ 8.899671] ., OK
> [ 8.926343] IP-Config: Got DHCP answer from 172.27.200.1, my address is 172.27.64.49
> ...
> [ 8.987327] Freeing unused kernel memory: 168K (c039e000 - c03c8000)
>
> After:
>
> [ 0.623526] libphy: nb8800-mii: probed
> [ 0.628092] nb8800 26000.ethernet eth0: MAC address 00:16:e8:02:08:42
> ...
> [ 4.732948] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
> [ 4.752655] Sending DHCP requests ., OK
> [ 4.782644] IP-Config: Got DHCP answer from 172.27.200.1, my address is 172.27.64.49
> ...
> [ 4.849298] Freeing unused kernel memory: 164K (c039f000 - c03c8000)
>
> The DHCP request is sent later, but the kernel doesn't twiddle its thumbs
> for 4 seconds after the link comes up. Does this come from not probing the
> PHY anymore?

No, that's from properly setting the link state initially down.

> BTW, you're not using the PHY IRQ, right? I think I remember you saying
> it didn't work reliably?

It doesn't seem to be wired up on any of my boards, or there's some
magic required to activate it that I'm unaware of.

--
M?ns Rullg?rd
[email protected]

2015-11-12 16:19:50

by Mason

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

[ CCing a few knowledgeable people ]

Despite the subject, this is about an Atheros 8035 PHY :-)

On 12/11/2015 15:04, M?ns Rullg?rd wrote:

> Mason wrote:
>
>> BTW, you're not using the PHY IRQ, right? I think I remember you saying
>> it didn't work reliably?
>
> It doesn't seem to be wired up on any of my boards, or there's some
> magic required to activate it that I'm unaware of.

Weird. The board schematics for the 1172 show Tango ETH0_MDINT# pin
properly connected to AR8035 INT pin (pin 20).

<Thinking out loud>

http://www.redeszone.net/app/uploads/2014/04/AR8035.pdf

INT pin 20
I/O, D, PD
Interrupt Signal to System; default OD-gate, needs an external
10Kohm pull-up, active low; can be configured to I/O by register,
active high.


4.1.17 Interrupt Enable
Offset: 0x12
Mode: Read/Write
Hardware Reset: 0


Strange... it looks like AT803X_INER and AT803X_INTR_ENABLE refer to
the same "Interrupt Enable" register?

In fact, AT803X_INER_INIT == 0xec00 makes sense for register 0x12:
link success/fail, speed/duplex changed, autoneg error

Looks like at803x_config_intr() is used for 8031, but not for 8035...

Relevant commit:
77a9939426f7a "phy/at8031: enable at8031 to work on interrupt mode"

If I add .config_intr and .ack_interrupt to the 8035 struct, then I get
(also added some traces)

[ 0.883517] *** at803x_config_intr: ENABLE
[ 1.576108] *** at803x_config_intr: DISABLE
[ 1.580467] *** at803x_config_intr: ENABLE
[ 1.584959] *** at803x_config_intr: DISABLE
[ 1.589297] *** at803x_config_intr: ENABLE
[ 4.321722] *** at803x_config_intr: DISABLE
[ 4.326054] *** at803x_config_intr: ENABLE
[ 4.330489] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
[ 4.338335] *** at803x_config_intr: ENABLE

(Are all the ENABLE/DISABLE events expected?)

And if I unplug/replug the Ethernet cable,

[ 71.903051] *** at803x_config_intr: DISABLE
[ 71.907410] *** at803x_config_intr: ENABLE
[ 71.912232] nb8800 26000.ethernet eth0: Link is Down
[ 71.917309] *** at803x_config_intr: ENABLE
[ 78.008972] *** at803x_config_intr: DISABLE
[ 78.013375] *** at803x_config_intr: ENABLE
[ 78.017797] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
[ 78.025702] *** at803x_config_intr: ENABLE

(Are all the ENABLE/DISABLE events expected there too?)

# cat /proc/interrupts
CPU0 CPU1
18: 107 0 irq0 1 Level serial
54: 5 0 irq0 37 Edge phy_interrupt
55: 4953 0 irq0 38 Level eth0
211: 1147 254 GIC 29 Edge twd


Questions:

Can't at803x_ack_interrupt() just return phy_read(phydev, AT803X_INSR);
Can at803x_config_intr() be used with the 8035
What about AT803X_INER/AT803X_INTR_ENABLE and AT803X_INSR/AT803X_INTR_STATUS

Regards.

2015-11-12 16:57:57

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

Mason <[email protected]> writes:

> [ CCing a few knowledgeable people ]
>
> Despite the subject, this is about an Atheros 8035 PHY :-)
>
> On 12/11/2015 15:04, M?ns Rullg?rd wrote:
>
>> Mason wrote:
>>
>>> BTW, you're not using the PHY IRQ, right? I think I remember you saying
>>> it didn't work reliably?
>>
>> It doesn't seem to be wired up on any of my boards, or there's some
>> magic required to activate it that I'm unaware of.
>
> Weird. The board schematics for the 1172 show Tango ETH0_MDINT# pin
> properly connected to AR8035 INT pin (pin 20).

I have a different board.

> <Thinking out loud>
>
> http://www.redeszone.net/app/uploads/2014/04/AR8035.pdf
>
> INT pin 20
> I/O, D, PD
> Interrupt Signal to System; default OD-gate, needs an external
> 10Kohm pull-up, active low; can be configured to I/O by register,
> active high.
>
> 4.1.17 Interrupt Enable
> Offset: 0x12
> Mode: Read/Write
> Hardware Reset: 0
>
> Strange... it looks like AT803X_INER and AT803X_INTR_ENABLE refer to
> the same "Interrupt Enable" register?

Seems like someone missed that it was already defined.

> In fact, AT803X_INER_INIT == 0xec00 makes sense for register 0x12:
> link success/fail, speed/duplex changed, autoneg error
>
> Looks like at803x_config_intr() is used for 8031, but not for 8035...
>
> Relevant commit:
> 77a9939426f7a "phy/at8031: enable at8031 to work on interrupt mode"
>
> If I add .config_intr and .ack_interrupt to the 8035 struct, then I get
> (also added some traces)

I tried that just now, and I get nothing. What interrupt did you
specify in your device tree?

> Questions:
>
> Can't at803x_ack_interrupt() just return phy_read(phydev, AT803X_INSR);

No, that would return the actual value of the register. The caller
doesn't care about the value, but should be notified if there was an
error.

> Can at803x_config_intr() be used with the 8035

Probably. The person who sent the patch for 8031 probably happened to
have that model.

> What about AT803X_INER/AT803X_INTR_ENABLE and AT803X_INSR/AT803X_INTR_STATUS

Accidental duplicates.

--
M?ns Rullg?rd
[email protected]

2015-11-12 17:20:34

by Måns Rullgård

[permalink] [raw]
Subject: Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

M?ns Rullg?rd <[email protected]> writes:

> Mason <[email protected]> writes:
>
>> [ CCing a few knowledgeable people ]
>>
>> Despite the subject, this is about an Atheros 8035 PHY :-)
>>
>> On 12/11/2015 15:04, M?ns Rullg?rd wrote:
>>
>>> Mason wrote:
>>>
>>>> BTW, you're not using the PHY IRQ, right? I think I remember you saying
>>>> it didn't work reliably?
>>>
>>> It doesn't seem to be wired up on any of my boards, or there's some
>>> magic required to activate it that I'm unaware of.
>>
>> Weird. The board schematics for the 1172 show Tango ETH0_MDINT# pin
>> properly connected to AR8035 INT pin (pin 20).
>
> I have a different board.
>
>> <Thinking out loud>
>>
>> http://www.redeszone.net/app/uploads/2014/04/AR8035.pdf
>>
>> INT pin 20
>> I/O, D, PD
>> Interrupt Signal to System; default OD-gate, needs an external
>> 10Kohm pull-up, active low; can be configured to I/O by register,
>> active high.
>>
>> 4.1.17 Interrupt Enable
>> Offset: 0x12
>> Mode: Read/Write
>> Hardware Reset: 0
>>
>> Strange... it looks like AT803X_INER and AT803X_INTR_ENABLE refer to
>> the same "Interrupt Enable" register?
>
> Seems like someone missed that it was already defined.
>
>> In fact, AT803X_INER_INIT == 0xec00 makes sense for register 0x12:
>> link success/fail, speed/duplex changed, autoneg error
>>
>> Looks like at803x_config_intr() is used for 8031, but not for 8035...
>>
>> Relevant commit:
>> 77a9939426f7a "phy/at8031: enable at8031 to work on interrupt mode"
>>
>> If I add .config_intr and .ack_interrupt to the 8035 struct, then I get
>> (also added some traces)
>
> I tried that just now, and I get nothing. What interrupt did you
> specify in your device tree?

It works with the interrupt set to trigger on rising edge.

--
M?ns Rullg?rd
[email protected]