[PATCH net-next 0/6] drivers: net: xgene-v2: Add RGMII based 1G driver

[PATCH net-next 0/6] drivers: net: xgene-v2: Add RGMII based 1G driver

[Iyappan Subramanian]

This patch set adds support for RGMII based 1GbE hardware which uses a linked
list of DMA descriptor architecture (v2) for APM X-Gene SoCs.

Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
---

Iyappan Subramanian (6):
  drivers: net: xgene-v2: Add DMA descriptor
  drivers: net: xgene-v2: Add mac configuration
  drivers: net: xgene-v2: Add ethernet hardware configuration
  drivers: net: xgene-v2: Add base driver
  drivers: net: xgene-v2: Add transmit and receive
  MAINTAINERS: Add entry for APM X-Gene SoC Ethernet (v2) driver

 MAINTAINERS                                |   6 +
 drivers/net/ethernet/apm/Kconfig           |   1 +
 drivers/net/ethernet/apm/Makefile          |   1 +
 drivers/net/ethernet/apm/xgene-v2/Kconfig  |  11 +
 drivers/net/ethernet/apm/xgene-v2/Makefile |   6 +
 drivers/net/ethernet/apm/xgene-v2/enet.c   |  71 ++++
 drivers/net/ethernet/apm/xgene-v2/enet.h   |  43 ++
 drivers/net/ethernet/apm/xgene-v2/mac.c    | 116 +++++
 drivers/net/ethernet/apm/xgene-v2/mac.h    | 132 ++++++
 drivers/net/ethernet/apm/xgene-v2/main.c   | 657 +++++++++++++++++++++++++++++
 drivers/net/ethernet/apm/xgene-v2/main.h   |  88 ++++
 drivers/net/ethernet/apm/xgene-v2/ring.c   |  73 ++++
 drivers/net/ethernet/apm/xgene-v2/ring.h   |  77 ++++
 13 files changed, 1282 insertions(+)
 create mode 100644 drivers/net/ethernet/apm/xgene-v2/Kconfig
 create mode 100644 drivers/net/ethernet/apm/xgene-v2/Makefile
 create mode 100644 drivers/net/ethernet/apm/xgene-v2/enet.c
 create mode 100644 drivers/net/ethernet/apm/xgene-v2/enet.h
 create mode 100644 drivers/net/ethernet/apm/xgene-v2/mac.c
 create mode 100644 drivers/net/ethernet/apm/xgene-v2/mac.h
 create mode 100644 drivers/net/ethernet/apm/xgene-v2/main.c
 create mode 100644 drivers/net/ethernet/apm/xgene-v2/main.h
 create mode 100644 drivers/net/ethernet/apm/xgene-v2/ring.c
 create mode 100644 drivers/net/ethernet/apm/xgene-v2/ring.h

-- 
1.9.1

[PATCH net-next 1/6] drivers: net: xgene-v2: Add DMA descriptor

[Iyappan Subramanian]

This patch adds DMA descriptor setup and interrupt enable/disable
functions.

Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
---
 drivers/net/ethernet/apm/xgene-v2/main.h | 88 ++++++++++++++++++++++++++++++++
 drivers/net/ethernet/apm/xgene-v2/ring.c | 73 ++++++++++++++++++++++++++
 drivers/net/ethernet/apm/xgene-v2/ring.h | 77 ++++++++++++++++++++++++++++
 3 files changed, 238 insertions(+)
 create mode 100644 drivers/net/ethernet/apm/xgene-v2/main.h
 create mode 100644 drivers/net/ethernet/apm/xgene-v2/ring.c
 create mode 100644 drivers/net/ethernet/apm/xgene-v2/ring.h

diff --git a/drivers/net/ethernet/apm/xgene-v2/main.h b/drivers/net/ethernet/apm/xgene-v2/main.h
new file mode 100644
index 0000000..c8a40af
--- /dev/null
+++ b/drivers/net/ethernet/apm/xgene-v2/main.h
@@ -0,0 +1,88 @@
+/*
+ * Applied Micro X-Gene SoC Ethernet v2 Driver
+ *
+ * Copyright (c) 2017, Applied Micro Circuits Corporation
+ * Author(s): Iyappan Subramanian <isubramanian@apm.com>
+ *	      Keyur Chudgar <kchudgar@apm.com>
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __XGENE_ENET_V2_MAIN_H__
+#define __XGENE_ENET_V2_MAIN_H__
+
+#include <linux/acpi.h>
+#include <linux/clk.h>
+#include <linux/efi.h>
+#include <linux/if_vlan.h>
+#include <linux/irq.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/of_net.h>
+#include <linux/of_mdio.h>
+#include <linux/prefetch.h>
+#include <linux/phy.h>
+#include <net/ip.h>
+#include "mac.h"
+#include "enet.h"
+#include "ring.h"
+
+#define XGENE_ENET_V2_VERSION	"v1.0"
+#define XGENE_ENET_STD_MTU	1536
+#define XGENE_ENET_MIN_FRAME	60
+#define IRQ_ID_SIZE             16
+
+struct xge_resource {
+	void __iomem *base_addr;
+	int phy_mode;
+	u32 irq;
+};
+
+struct xge_stats {
+	u64 tx_packets;
+	u64 tx_bytes;
+	u64 rx_packets;
+	u64 rx_bytes;
+};
+
+/* software context of a descriptor ring */
+struct xge_desc_ring {
+	struct net_device *ndev;
+	dma_addr_t dma_addr;
+	u8 head;
+	u8 tail;
+	union {
+		void *desc_addr;
+		struct xge_raw_desc *raw_desc;
+	};
+	struct sk_buff *(*skbs);
+	void *(*pkt_bufs);
+};
+
+/* ethernet private data */
+struct xge_pdata {
+	struct xge_resource resources;
+	struct xge_desc_ring *tx_ring;
+	struct xge_desc_ring *rx_ring;
+	struct platform_device *pdev;
+	char irq_name[IRQ_ID_SIZE];
+	struct net_device *ndev;
+	struct napi_struct napi;
+	struct xge_stats stats;
+	int phy_speed;
+	u8 nbufs;
+};
+
+#endif /* __XGENE_ENET_V2_MAIN_H__ */
diff --git a/drivers/net/ethernet/apm/xgene-v2/ring.c b/drivers/net/ethernet/apm/xgene-v2/ring.c
new file mode 100644
index 0000000..3289bf3
--- /dev/null
+++ b/drivers/net/ethernet/apm/xgene-v2/ring.c
@@ -0,0 +1,73 @@
+/*
+ * Applied Micro X-Gene SoC Ethernet v2 Driver
+ *
+ * Copyright (c) 2017, Applied Micro Circuits Corporation
+ * Author(s): Iyappan Subramanian <isubramanian@apm.com>
+ *	      Keyur Chudgar <kchudgar@apm.com>
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "main.h"
+
+/* create circular linked list of descriptors */
+void xge_setup_desc(struct xge_desc_ring *ring)
+{
+	struct xge_raw_desc *raw_desc;
+	dma_addr_t dma_h, next_dma;
+	u16 offset;
+	int i;
+
+	for (i = 0; i < XGENE_ENET_NUM_DESC; i++) {
+		raw_desc = &ring->raw_desc[i];
+
+		offset = (i + 1) & (XGENE_ENET_NUM_DESC - 1);
+		next_dma = ring->dma_addr + (offset * XGENE_ENET_DESC_SIZE);
+
+		raw_desc->m0 = cpu_to_le64(SET_BITS(E, 1) |
+					   SET_BITS(PKT_SIZE, 0));
+		dma_h = next_dma >> NEXT_DESC_ADDRL_LEN;
+		raw_desc->m1 = cpu_to_le64(SET_BITS(NEXT_DESC_ADDRL, next_dma) |
+					   SET_BITS(NEXT_DESC_ADDRL, dma_h));
+	}
+}
+
+void xge_update_tx_desc_addr(struct xge_pdata *pdata)
+{
+	dma_addr_t dma_addr = pdata->tx_ring->dma_addr;
+
+	xge_wr_csr(pdata, DMATXDESCL, dma_addr);
+	xge_wr_csr(pdata, DMATXDESCH, (dma_addr >> NEXT_DESC_ADDRL_LEN));
+}
+
+void xge_update_rx_desc_addr(struct xge_pdata *pdata)
+{
+	dma_addr_t dma_addr = pdata->rx_ring->dma_addr;
+
+	xge_wr_csr(pdata, DMARXDESCL, dma_addr);
+	xge_wr_csr(pdata, DMARXDESCH, (dma_addr >> NEXT_DESC_ADDRL_LEN));
+}
+
+void xge_intr_enable(struct xge_pdata *pdata)
+{
+	u32 data;
+
+	data = RX_PKT_RCVD | TX_PKT_SENT;
+	xge_wr_csr(pdata, DMAINTRMASK, data);
+}
+
+void xge_intr_disable(struct xge_pdata *pdata)
+{
+	xge_wr_csr(pdata, DMAINTRMASK, 0);
+}
diff --git a/drivers/net/ethernet/apm/xgene-v2/ring.h b/drivers/net/ethernet/apm/xgene-v2/ring.h
new file mode 100644
index 0000000..f2711f4
--- /dev/null
+++ b/drivers/net/ethernet/apm/xgene-v2/ring.h
@@ -0,0 +1,77 @@
+/*
+ * Applied Micro X-Gene SoC Ethernet v2 Driver
+ *
+ * Copyright (c) 2017, Applied Micro Circuits Corporation
+ * Author(s): Iyappan Subramanian <isubramanian@apm.com>
+ *	      Keyur Chudgar <kchudgar@apm.com>
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __XGENE_ENET_V2_RING_H__
+#define __XGENE_ENET_V2_RING_H__
+
+struct xge_desc_ring;
+
+#define XGENE_ENET_DESC_SIZE	64
+#define XGENE_ENET_NUM_DESC	256
+#define NUM_BUFS		8
+
+#define E_POS			63
+#define E_LEN			1
+#define PKT_ADDRL_POS		0
+#define PKT_ADDRL_LEN		32
+#define PKT_ADDRH_POS		32
+#define PKT_ADDRH_LEN		10
+#define PKT_SIZE_POS		32
+#define PKT_SIZE_LEN		12
+#define NEXT_DESC_ADDRL_POS	0
+#define NEXT_DESC_ADDRL_LEN	32
+#define NEXT_DESC_ADDRH_POS	48
+#define NEXT_DESC_ADDRH_LEN	10
+
+struct xge_raw_desc {
+	__le64 m0;
+	__le64 m1;
+	__le64 m2;
+	__le64 m3;
+	__le64 m4;
+	__le64 m5;
+	__le64 m6;
+	__le64 m7;
+};
+
+static inline u64 xge_set_desc_bits(int pos, int len, u64 val)
+{
+	return (val & ((1ULL << len) - 1)) << pos;
+}
+
+static inline u64 xge_get_desc_bits(int pos, int len, u64 src)
+{
+	return (src >> pos) & ((1ULL << len) - 1);
+}
+
+#define SET_BITS(field, val) \
+		xge_set_desc_bits(field ## _POS, field ## _LEN, val)
+
+#define GET_BITS(field, src) \
+		xge_get_desc_bits(field ## _POS, field ## _LEN, src)
+
+void xge_setup_desc(struct xge_desc_ring *ring);
+void xge_update_tx_desc_addr(struct xge_pdata *pdata);
+void xge_update_rx_desc_addr(struct xge_pdata *pdata);
+void xge_intr_enable(struct xge_pdata *pdata);
+void xge_intr_disable(struct xge_pdata *pdata);
+
+#endif  /* __XGENE_ENET_V2_RING_H__ */
-- 
1.9.1

[PATCH net-next 2/6] drivers: net: xgene-v2: Add mac configuration

[Iyappan Subramanian]

This patch adds functions to configure and control mac.  This
patch also adds helper functions to get/set registers.

Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
---
 drivers/net/ethernet/apm/xgene-v2/mac.c | 116 ++++++++++++++++++++++++++++
 drivers/net/ethernet/apm/xgene-v2/mac.h | 132 ++++++++++++++++++++++++++++++++
 2 files changed, 248 insertions(+)
 create mode 100644 drivers/net/ethernet/apm/xgene-v2/mac.c
 create mode 100644 drivers/net/ethernet/apm/xgene-v2/mac.h

diff --git a/drivers/net/ethernet/apm/xgene-v2/mac.c b/drivers/net/ethernet/apm/xgene-v2/mac.c
new file mode 100644
index 0000000..9c3d32d
--- /dev/null
+++ b/drivers/net/ethernet/apm/xgene-v2/mac.c
@@ -0,0 +1,116 @@
+/*
+ * Applied Micro X-Gene SoC Ethernet v2 Driver
+ *
+ * Copyright (c) 2017, Applied Micro Circuits Corporation
+ * Author(s): Iyappan Subramanian <isubramanian@apm.com>
+ *	      Keyur Chudgar <kchudgar@apm.com>
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "main.h"
+
+void xge_mac_reset(struct xge_pdata *pdata)
+{
+	xge_wr_csr(pdata, MAC_CONFIG_1, SOFT_RESET);
+	xge_wr_csr(pdata, MAC_CONFIG_1, 0);
+}
+
+static void xge_mac_set_speed(struct xge_pdata *pdata)
+{
+	u32 icm0, icm2, ecm0, mc2;
+	u32 intf_ctrl, rgmii;
+
+	icm0 = xge_rd_csr(pdata, ICM_CONFIG0_REG_0);
+	icm2 = xge_rd_csr(pdata, ICM_CONFIG2_REG_0);
+	ecm0 = xge_rd_csr(pdata, ECM_CONFIG0_REG_0);
+	rgmii = xge_rd_csr(pdata, RGMII_REG_0);
+	mc2 = xge_rd_csr(pdata, MAC_CONFIG_2);
+	intf_ctrl = xge_rd_csr(pdata, INTERFACE_CONTROL);
+	icm2 |= CFG_WAITASYNCRD_EN;
+
+	switch (pdata->phy_speed) {
+	case SPEED_10:
+		SET_REG_BITS(&mc2, INTF_MODE, 1);
+		SET_REG_BITS(&intf_ctrl, HD_MODE, 0);
+		SET_REG_BITS(&icm0, CFG_MACMODE, 0);
+		SET_REG_BITS(&icm2, CFG_WAITASYNCRD, 500);
+		SET_REG_BIT(&rgmii, CFG_SPEED_125, 0);
+		break;
+	case SPEED_100:
+		SET_REG_BITS(&mc2, INTF_MODE, 1);
+		SET_REG_BITS(&intf_ctrl, HD_MODE, 1);
+		SET_REG_BITS(&icm0, CFG_MACMODE, 1);
+		SET_REG_BITS(&icm2, CFG_WAITASYNCRD, 80);
+		SET_REG_BIT(&rgmii, CFG_SPEED_125, 0);
+		break;
+	default:
+		SET_REG_BITS(&mc2, INTF_MODE, 2);
+		SET_REG_BITS(&intf_ctrl, HD_MODE, 2);
+		SET_REG_BITS(&icm0, CFG_MACMODE, 2);
+		SET_REG_BITS(&icm2, CFG_WAITASYNCRD, 16);
+		SET_REG_BIT(&rgmii, CFG_SPEED_125, 1);
+		break;
+	}
+
+	mc2 |= FULL_DUPLEX | CRC_EN | PAD_CRC;
+	SET_REG_BITS(&ecm0, CFG_WFIFOFULLTHR, 0x32);
+
+	xge_wr_csr(pdata, MAC_CONFIG_2, mc2);
+	xge_wr_csr(pdata, INTERFACE_CONTROL, intf_ctrl);
+	xge_wr_csr(pdata, RGMII_REG_0, rgmii);
+	xge_wr_csr(pdata, ICM_CONFIG0_REG_0, icm0);
+	xge_wr_csr(pdata, ICM_CONFIG2_REG_0, icm2);
+	xge_wr_csr(pdata, ECM_CONFIG0_REG_0, ecm0);
+}
+
+void xge_mac_set_station_addr(struct xge_pdata *pdata)
+{
+	u32 addr0, addr1;
+	u8 *dev_addr = pdata->ndev->dev_addr;
+
+	addr0 = (dev_addr[3] << 24) | (dev_addr[2] << 16) |
+		(dev_addr[1] << 8) | dev_addr[0];
+	addr1 = (dev_addr[5] << 24) | (dev_addr[4] << 16);
+
+	xge_wr_csr(pdata, STATION_ADDR0, addr0);
+	xge_wr_csr(pdata, STATION_ADDR1, addr1);
+}
+
+void xge_mac_init(struct xge_pdata *pdata)
+{
+	xge_mac_reset(pdata);
+	xge_mac_set_speed(pdata);
+	xge_mac_set_station_addr(pdata);
+}
+
+void xge_mac_enable(struct xge_pdata *pdata)
+{
+	u32 data;
+
+	data = xge_rd_csr(pdata, MAC_CONFIG_1);
+	data |= TX_EN | RX_EN;
+	xge_wr_csr(pdata, MAC_CONFIG_1, data);
+
+	data = xge_rd_csr(pdata, MAC_CONFIG_1);
+}
+
+void xge_mac_disable(struct xge_pdata *pdata)
+{
+	u32 data;
+
+	data = xge_rd_csr(pdata, MAC_CONFIG_1);
+	data &= ~(TX_EN | RX_EN);
+	xge_wr_csr(pdata, MAC_CONFIG_1, data);
+}
diff --git a/drivers/net/ethernet/apm/xgene-v2/mac.h b/drivers/net/ethernet/apm/xgene-v2/mac.h
new file mode 100644
index 0000000..6237eb2
--- /dev/null
+++ b/drivers/net/ethernet/apm/xgene-v2/mac.h
@@ -0,0 +1,132 @@
+/*
+ * Applied Micro X-Gene SoC Ethernet v2 Driver
+ *
+ * Copyright (c) 2017, Applied Micro Circuits Corporation
+ * Author(s): Iyappan Subramanian <isubramanian@apm.com>
+ *	      Keyur Chudgar <kchudgar@apm.com>
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __XGENE_ENET_V2_MAC_H__
+#define __XGENE_ENET_V2_MAC_H__
+
+/* Register offsets */
+#define MAC_CONFIG_1		0xa000
+#define MAC_CONFIG_2		0xa004
+#define MII_MGMT_CONFIG		0xa020
+#define MII_MGMT_COMMAND	0xa024
+#define MII_MGMT_ADDRESS	0xa028
+#define MII_MGMT_CONTROL	0xa02c
+#define MII_MGMT_STATUS		0xa030
+#define MII_MGMT_INDICATORS	0xa034
+#define INTERFACE_CONTROL	0xa038
+#define STATION_ADDR0		0xa040
+#define STATION_ADDR1		0xa044
+#define RBYT			0xa09c
+#define RPKT			0xa0a0
+#define RFCS			0xa0a4
+
+#define RGMII_REG_0		0x27e0
+#define ICM_CONFIG0_REG_0	0x2c00
+#define ICM_CONFIG2_REG_0	0x2c08
+#define ECM_CONFIG0_REG_0	0x2d00
+
+#define DMATXCTRL		0xa180
+#define DMATXDESCL		0xa184
+#define DMATXDESCH		0xa1a0
+#define DMATXSTATUS		0xa188
+#define DMARXCTRL		0xa18c
+#define DMARXDESCL		0xa190
+#define DMARXDESCH		0xa1a4
+#define DMARXSTATUS		0xa194
+#define DMAINTRMASK		0xa198
+#define DMAINTERRUPT		0xa19c
+
+/* Register fields */
+#define SOFT_RESET		BIT(31)
+#define TX_EN			BIT(0)
+#define RX_EN			BIT(2)
+#define PAD_CRC			BIT(2)
+#define CRC_EN			BIT(1)
+#define FULL_DUPLEX		BIT(0)
+
+#define INTF_MODE_POS		8
+#define INTF_MODE_LEN		2
+#define HD_MODE_POS		25
+#define HD_MODE_LEN		2
+#define CFG_MACMODE_POS		18
+#define CFG_MACMODE_LEN		2
+#define CFG_WAITASYNCRD_POS	0
+#define CFG_WAITASYNCRD_LEN	16
+#define CFG_SPEED_125_POS	24
+#define CFG_WFIFOFULLTHR_POS	0
+#define CFG_WFIFOFULLTHR_LEN	7
+#define MGMT_CLOCK_SEL_POS	0
+#define MGMT_CLOCK_SEL_LEN	3
+#define PHY_ADDR_POS		8
+#define PHY_ADDR_LEN		5
+#define REG_ADDR_POS		0
+#define REG_ADDR_LEN		5
+#define MII_MGMT_BUSY		BIT(0)
+#define MII_READ_CYCLE		BIT(0)
+#define CFG_WAITASYNCRD_EN	BIT(16)
+
+#define TXPKTCOUNT_POS		16
+#define TXPKTCOUNT_LEN		8
+#define RXPKTCOUNT_POS		16
+#define RXPKTCOUNT_LEN		8
+
+#define TX_PKT_SENT		BIT(0)
+#define TX_BUS_ERROR		BIT(3)
+#define RX_PKT_RCVD		BIT(4)
+#define RX_BUS_ERROR		BIT(7)
+#define RXSTATUS_RXPKTRCVD	BIT(0)
+
+static inline void xgene_set_reg_bits(u32 *var, int pos, int len, u32 val)
+{
+	u32 mask = GENMASK(pos + len, pos);
+
+	*var &= ~mask;
+	*var |= ((val << pos) & mask);
+}
+
+static inline u32 xgene_get_reg_bits(u32 var, int pos, int len)
+{
+	u32 mask = GENMASK(pos + len, pos);
+
+	return (var & mask) >> pos;
+}
+
+#define SET_REG_BITS(var, field, val)					\
+	xgene_set_reg_bits(var, field ## _POS, field ## _LEN, val)
+
+#define SET_REG_BIT(var, field, val)					\
+	xgene_set_reg_bits(var, field ## _POS, 1, val)
+
+#define GET_REG_BITS(var, field)					\
+	xgene_get_reg_bits(var, field ## _POS, field ## _LEN)
+
+#define GET_REG_BIT(var, field)		((var) & (field))
+
+struct xge_pdata;
+
+void xge_mac_reset(struct xge_pdata *pdata);
+void xge_mac_enable(struct xge_pdata *pdata);
+void xge_mac_disable(struct xge_pdata *pdata);
+void xge_mac_init(struct xge_pdata *pdata);
+int xge_port_init(struct net_device *ndev);
+void xge_mac_set_station_addr(struct xge_pdata *pdata);
+
+#endif /* __XGENE_ENET_V2_MAC_H__ */
-- 
1.9.1

[PATCH net-next 3/6] drivers: net: xgene-v2: Add ethernet hardware configuration

[Iyappan Subramanian]

This patch adds functions to configure ethernet hardware.

Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
---
 drivers/net/ethernet/apm/xgene-v2/enet.c | 71 ++++++++++++++++++++++++++++++++
 drivers/net/ethernet/apm/xgene-v2/enet.h | 43 +++++++++++++++++++
 2 files changed, 114 insertions(+)
 create mode 100644 drivers/net/ethernet/apm/xgene-v2/enet.c
 create mode 100644 drivers/net/ethernet/apm/xgene-v2/enet.h

diff --git a/drivers/net/ethernet/apm/xgene-v2/enet.c b/drivers/net/ethernet/apm/xgene-v2/enet.c
new file mode 100644
index 0000000..b49edee
--- /dev/null
+++ b/drivers/net/ethernet/apm/xgene-v2/enet.c
@@ -0,0 +1,71 @@
+/*
+ * Applied Micro X-Gene SoC Ethernet v2 Driver
+ *
+ * Copyright (c) 2017, Applied Micro Circuits Corporation
+ * Author(s): Iyappan Subramanian <isubramanian@apm.com>
+ *	      Keyur Chudgar <kchudgar@apm.com>
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "main.h"
+
+void xge_wr_csr(struct xge_pdata *pdata, u32 offset, u32 val)
+{
+	void __iomem *addr = pdata->resources.base_addr + offset;
+
+	iowrite32(val, addr);
+}
+
+u32 xge_rd_csr(struct xge_pdata *pdata, u32 offset)
+{
+	void __iomem *addr = pdata->resources.base_addr + offset;
+
+	return ioread32(addr);
+}
+
+int xge_port_reset(struct net_device *ndev)
+{
+	struct xge_pdata *pdata = netdev_priv(ndev);
+
+	xge_wr_csr(pdata, ENET_SRST, 0x3);
+	xge_wr_csr(pdata, ENET_SRST, 0x2);
+	xge_wr_csr(pdata, ENET_SRST, 0x0);
+
+	xge_wr_csr(pdata, ENET_SHIM, DEVM_ARAUX_COH | DEVM_AWAUX_COH);
+
+	return 0;
+}
+
+static void xge_traffic_resume(struct net_device *ndev)
+{
+	struct xge_pdata *pdata = netdev_priv(ndev);
+
+	xge_wr_csr(pdata, CFG_FORCE_LINK_STATUS_EN, 1);
+	xge_wr_csr(pdata, FORCE_LINK_STATUS, 1);
+
+	xge_wr_csr(pdata, CFG_LINK_AGGR_RESUME, 1);
+	xge_wr_csr(pdata, RX_DV_GATE_REG, 1);
+}
+
+int xge_port_init(struct net_device *ndev)
+{
+	struct xge_pdata *pdata = netdev_priv(ndev);
+
+	pdata->phy_speed = SPEED_1000;
+	xge_mac_init(pdata);
+	xge_traffic_resume(ndev);
+
+	return 0;
+}
diff --git a/drivers/net/ethernet/apm/xgene-v2/enet.h b/drivers/net/ethernet/apm/xgene-v2/enet.h
new file mode 100644
index 0000000..40371cf
--- /dev/null
+++ b/drivers/net/ethernet/apm/xgene-v2/enet.h
@@ -0,0 +1,43 @@
+/*
+ * Applied Micro X-Gene SoC Ethernet v2 Driver
+ *
+ * Copyright (c) 2017, Applied Micro Circuits Corporation
+ * Author(s): Iyappan Subramanian <isubramanian@apm.com>
+ *	      Keyur Chudgar <kchudgar@apm.com>
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __XGENE_ENET_V2_ENET_H__
+#define __XGENE_ENET_V2_ENET_H__
+
+#define ENET_CLKEN		0xc008
+#define ENET_SRST		0xc000
+#define ENET_SHIM		0xc010
+#define CFG_MEM_RAM_SHUTDOWN	0xd070
+#define BLOCK_MEM_RDY		0xd074
+
+#define DEVM_ARAUX_COH		BIT(19)
+#define DEVM_AWAUX_COH		BIT(3)
+
+#define CFG_FORCE_LINK_STATUS_EN	0x229c
+#define FORCE_LINK_STATUS		0x22a0
+#define CFG_LINK_AGGR_RESUME		0x27c8
+#define RX_DV_GATE_REG			0x2dfc
+
+void xge_wr_csr(struct xge_pdata *pdata, u32 offset, u32 val);
+u32 xge_rd_csr(struct xge_pdata *pdata, u32 offset);
+int xge_port_reset(struct net_device *ndev);
+
+#endif  /* __XGENE_ENET_V2_ENET__H__ */
-- 
1.9.1

[PATCH net-next 4/6] drivers: net: xgene-v2: Add base driver

[Iyappan Subramanian]

This patch adds,

     - probe, remove, shutdown
     - open, close and stats
     - create and delete ring
     - request and delete irq

Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
---
 drivers/net/ethernet/apm/xgene-v2/main.c | 459 +++++++++++++++++++++++++++++++
 1 file changed, 459 insertions(+)
 create mode 100644 drivers/net/ethernet/apm/xgene-v2/main.c

diff --git a/drivers/net/ethernet/apm/xgene-v2/main.c b/drivers/net/ethernet/apm/xgene-v2/main.c
new file mode 100644
index 0000000..3881f27
--- /dev/null
+++ b/drivers/net/ethernet/apm/xgene-v2/main.c
@@ -0,0 +1,459 @@
+/*
+ * Applied Micro X-Gene SoC Ethernet v2 Driver
+ *
+ * Copyright (c) 2017, Applied Micro Circuits Corporation
+ * Author(s): Iyappan Subramanian <isubramanian@apm.com>
+ *	      Keyur Chudgar <kchudgar@apm.com>
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "main.h"
+
+static const struct acpi_device_id xge_acpi_match[];
+
+static int xge_get_resources(struct xge_pdata *pdata)
+{
+	struct platform_device *pdev;
+	struct net_device *ndev;
+	struct device *dev;
+	struct resource *res;
+	int phy_mode, ret = 0;
+
+	pdev = pdata->pdev;
+	dev = &pdev->dev;
+	ndev = pdata->ndev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(dev, "Resource enet_csr not defined\n");
+		return -ENODEV;
+	}
+
+	pdata->resources.base_addr = devm_ioremap(dev, res->start,
+						  resource_size(res));
+	if (!pdata->resources.base_addr) {
+		dev_err(dev, "Unable to retrieve ENET Port CSR region\n");
+		return -ENOMEM;
+	}
+
+	if (!device_get_mac_address(dev, ndev->dev_addr, ETH_ALEN))
+		eth_hw_addr_random(ndev);
+
+	memcpy(ndev->perm_addr, ndev->dev_addr, ndev->addr_len);
+
+	phy_mode = device_get_phy_mode(dev);
+	if (phy_mode < 0) {
+		dev_err(dev, "Unable to get phy-connection-type\n");
+		return phy_mode;
+	}
+	pdata->resources.phy_mode = phy_mode;
+
+	if (pdata->resources.phy_mode != PHY_INTERFACE_MODE_RGMII) {
+		dev_err(dev, "Incorrect phy-connection-type specified\n");
+		return -ENODEV;
+	}
+
+	ret = platform_get_irq(pdev, 0);
+	if (ret <= 0) {
+		dev_err(dev, "Unable to get ENET IRQ\n");
+		ret = ret ? : -ENXIO;
+		return ret;
+	}
+	pdata->resources.irq = ret;
+
+	return 0;
+}
+
+static void xge_delete_desc_rings(struct net_device *ndev)
+{
+	struct xge_pdata *pdata = netdev_priv(ndev);
+	struct device *dev = &pdata->pdev->dev;
+	struct xge_desc_ring *ring;
+
+	ring = pdata->tx_ring;
+	if (ring) {
+		if (ring->skbs)
+			devm_kfree(dev, ring->skbs);
+		if (ring->pkt_bufs)
+			devm_kfree(dev, ring->pkt_bufs);
+		devm_kfree(dev, ring);
+	}
+
+	ring = pdata->rx_ring;
+	if (ring) {
+		if (ring->skbs)
+			devm_kfree(dev, ring->skbs);
+		devm_kfree(dev, ring);
+	}
+}
+
+static struct xge_desc_ring *xge_create_desc_ring(struct net_device *ndev)
+{
+	struct xge_pdata *pdata = netdev_priv(ndev);
+	struct device *dev = &pdata->pdev->dev;
+	struct xge_desc_ring *ring;
+	u16 size;
+
+	ring = devm_kzalloc(dev, sizeof(struct xge_desc_ring), GFP_KERNEL);
+	if (!ring)
+		return NULL;
+
+	ring->ndev = ndev;
+
+	size = XGENE_ENET_DESC_SIZE * XGENE_ENET_NUM_DESC;
+	ring->desc_addr = dmam_alloc_coherent(dev, size, &ring->dma_addr,
+					      GFP_KERNEL | __GFP_ZERO);
+	if (!ring->desc_addr) {
+		devm_kfree(dev, ring);
+		return NULL;
+	}
+
+	xge_setup_desc(ring);
+
+	return ring;
+}
+
+static int xge_refill_buffers(struct net_device *ndev, u32 nbuf)
+{
+	struct xge_pdata *pdata = netdev_priv(ndev);
+	struct xge_desc_ring *ring = pdata->rx_ring;
+	const u8 slots = XGENE_ENET_NUM_DESC - 1;
+	struct device *dev = &pdata->pdev->dev;
+	struct xge_raw_desc *raw_desc;
+	u64 addr_lo, addr_hi;
+	u8 tail = ring->tail;
+	struct sk_buff *skb;
+	dma_addr_t dma_addr;
+	u16 len;
+	int i;
+
+	for (i = 0; i < nbuf; i++) {
+		raw_desc = &ring->raw_desc[tail];
+
+		len = XGENE_ENET_STD_MTU;
+		skb = netdev_alloc_skb(ndev, len);
+		if (unlikely(!skb))
+			return -ENOMEM;
+
+		dma_addr = dma_map_single(dev, skb->data, len, DMA_FROM_DEVICE);
+		if (dma_mapping_error(dev, dma_addr)) {
+			netdev_err(ndev, "DMA mapping error\n");
+			dev_kfree_skb_any(skb);
+			return -EINVAL;
+		}
+
+		addr_hi = GET_BITS(NEXT_DESC_ADDRH, le64_to_cpu(raw_desc->m1));
+		addr_lo = GET_BITS(NEXT_DESC_ADDRL, le64_to_cpu(raw_desc->m1));
+		raw_desc->m1 = cpu_to_le64(SET_BITS(NEXT_DESC_ADDRL, addr_lo) |
+					   SET_BITS(NEXT_DESC_ADDRH, addr_hi) |
+					   SET_BITS(PKT_ADDRH,
+						    dma_addr >> PKT_ADDRL_LEN));
+
+		dma_wmb();
+		raw_desc->m0 = cpu_to_le64(SET_BITS(PKT_ADDRL, dma_addr) |
+					   SET_BITS(E, 1));
+		ring->skbs[tail] = skb;
+		tail = (tail + 1) & slots;
+	}
+
+	ring->tail = tail;
+
+	return 0;
+}
+
+static int xge_create_desc_rings(struct net_device *ndev)
+{
+	struct xge_pdata *pdata = netdev_priv(ndev);
+	struct device *dev = &pdata->pdev->dev;
+	struct xge_desc_ring *ring;
+	int ret;
+
+	/* create tx ring */
+	ring = xge_create_desc_ring(ndev);
+	if (!ring)
+		return -ENOMEM;
+
+	ring->skbs = devm_kcalloc(dev, XGENE_ENET_NUM_DESC,
+				  sizeof(struct sk_buff *), GFP_KERNEL);
+	if (!ring->skbs)
+		goto err;
+
+	ring->pkt_bufs = devm_kcalloc(dev, XGENE_ENET_NUM_DESC,
+				  sizeof(void *), GFP_KERNEL);
+	if (!ring->pkt_bufs)
+		goto err;
+
+	pdata->tx_ring = ring;
+	xge_update_tx_desc_addr(pdata);
+
+	/* create rx ring */
+	ring = xge_create_desc_ring(ndev);
+	if (!ring)
+		goto err;
+
+	ring->skbs = devm_kcalloc(dev, XGENE_ENET_NUM_DESC,
+				  sizeof(struct sk_buff *), GFP_KERNEL);
+	if (!ring->skbs)
+		goto err;
+
+	pdata->rx_ring = ring;
+	xge_update_rx_desc_addr(pdata);
+
+	ret = xge_refill_buffers(ndev, XGENE_ENET_NUM_DESC);
+	if (!ret)
+		return 0;
+
+err:
+	xge_delete_desc_rings(ndev);
+
+	return -ENOMEM;
+}
+
+static int xge_init_hw(struct net_device *ndev)
+{
+	struct xge_pdata *pdata = netdev_priv(ndev);
+	int ret;
+
+	ret = xge_port_reset(ndev);
+	if (ret)
+		return ret;
+
+	xge_create_desc_rings(ndev);
+	xge_port_init(ndev);
+	pdata->nbufs = NUM_BUFS;
+
+	return 0;
+}
+
+static irqreturn_t xge_irq(const int irq, void *data)
+{
+	struct xge_pdata *pdata = data;
+
+	if (napi_schedule_prep(&pdata->napi)) {
+		xge_intr_disable(pdata);
+		__napi_schedule(&pdata->napi);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int xge_request_irq(struct net_device *ndev)
+{
+	struct xge_pdata *pdata = netdev_priv(ndev);
+	struct device *dev = &pdata->pdev->dev;
+	int ret;
+
+	snprintf(pdata->irq_name, IRQ_ID_SIZE, "%s", ndev->name);
+
+	ret = devm_request_irq(dev, pdata->resources.irq, xge_irq,
+			       0, pdata->irq_name, pdata);
+	if (ret)
+		netdev_err(ndev, "Failed to request irq %s\n", pdata->irq_name);
+
+	return ret;
+}
+
+static void xge_free_irq(struct net_device *ndev)
+{
+	struct xge_pdata *pdata = netdev_priv(ndev);
+	struct device *dev = &pdata->pdev->dev;
+
+	devm_free_irq(dev, pdata->resources.irq, pdata);
+}
+
+static int xge_open(struct net_device *ndev)
+{
+	struct xge_pdata *pdata = netdev_priv(ndev);
+	int ret;
+
+	ret = xge_request_irq(ndev);
+	if (ret)
+		return ret;
+
+	xge_intr_enable(pdata);
+
+	xge_wr_csr(pdata, DMARXCTRL, 1);
+	xge_mac_enable(pdata);
+	netif_start_queue(ndev);
+
+	return 0;
+}
+
+static int xge_close(struct net_device *ndev)
+{
+	struct xge_pdata *pdata = netdev_priv(ndev);
+
+	netif_stop_queue(ndev);
+	xge_mac_disable(pdata);
+
+	xge_free_irq(ndev);
+
+	return 0;
+}
+
+static int xge_set_mac_addr(struct net_device *ndev, void *addr)
+{
+	struct xge_pdata *pdata = netdev_priv(ndev);
+	int ret;
+
+	ret = eth_mac_addr(ndev, addr);
+	if (ret)
+		return ret;
+
+	xge_mac_set_station_addr(pdata);
+
+	return 0;
+}
+
+static void xge_timeout(struct net_device *ndev)
+{
+	struct xge_pdata *pdata = netdev_priv(ndev);
+	struct netdev_queue *txq;
+
+	xge_mac_reset(pdata);
+
+	txq = netdev_get_tx_queue(ndev, 0);
+	txq->trans_start = jiffies;
+	netif_tx_start_queue(txq);
+}
+
+static void xge_get_stats64(struct net_device *ndev,
+			    struct rtnl_link_stats64 *storage)
+{
+	struct xge_pdata *pdata = netdev_priv(ndev);
+	struct xge_stats *stats = &pdata->stats;
+
+	storage->tx_packets += stats->tx_packets;
+	storage->tx_bytes += stats->tx_bytes;
+
+	storage->rx_packets += stats->rx_packets;
+	storage->rx_bytes += stats->rx_bytes;
+}
+
+static const struct net_device_ops xgene_ndev_ops = {
+	.ndo_open = xge_open,
+	.ndo_stop = xge_close,
+	.ndo_set_mac_address = xge_set_mac_addr,
+	.ndo_tx_timeout = xge_timeout,
+	.ndo_get_stats64 = xge_get_stats64,
+};
+
+static int xge_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct net_device *ndev;
+	struct xge_pdata *pdata;
+	int ret;
+
+	ndev = alloc_etherdev(sizeof(struct xge_pdata));
+	if (!ndev)
+		return -ENOMEM;
+
+	pdata = netdev_priv(ndev);
+
+	pdata->pdev = pdev;
+	pdata->ndev = ndev;
+	SET_NETDEV_DEV(ndev, dev);
+	platform_set_drvdata(pdev, pdata);
+	ndev->netdev_ops = &xgene_ndev_ops;
+
+	ndev->features |= NETIF_F_GSO |
+			  NETIF_F_GRO;
+
+	ret = xge_get_resources(pdata);
+	if (ret)
+		goto err;
+
+	ndev->hw_features = ndev->features;
+
+	ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64));
+	if (ret) {
+		netdev_err(ndev, "No usable DMA configuration\n");
+		goto err;
+	}
+
+	ret = xge_init_hw(ndev);
+	if (ret)
+		goto err;
+
+	ret = register_netdev(ndev);
+	if (ret) {
+		netdev_err(ndev, "Failed to register netdev\n");
+		goto err;
+	}
+
+	return 0;
+
+err:
+	free_netdev(ndev);
+
+	return ret;
+}
+
+static int xge_remove(struct platform_device *pdev)
+{
+	struct xge_pdata *pdata;
+	struct net_device *ndev;
+
+	pdata = platform_get_drvdata(pdev);
+	ndev = pdata->ndev;
+
+	rtnl_lock();
+	if (netif_running(ndev))
+		dev_close(ndev);
+	rtnl_unlock();
+
+	unregister_netdev(ndev);
+	xge_delete_desc_rings(ndev);
+	free_netdev(ndev);
+
+	return 0;
+}
+
+static void xge_shutdown(struct platform_device *pdev)
+{
+	struct xge_pdata *pdata;
+
+	pdata = platform_get_drvdata(pdev);
+	if (!pdata)
+		return;
+
+	if (!pdata->ndev)
+		return;
+
+	xge_remove(pdev);
+}
+
+static const struct acpi_device_id xge_acpi_match[] = {
+	{ "APMC0D80" },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, xge_acpi_match);
+
+static struct platform_driver xge_driver = {
+	.driver = {
+		   .name = "xgene-enet-v2",
+		   .acpi_match_table = ACPI_PTR(xge_acpi_match),
+	},
+	.probe = xge_probe,
+	.remove = xge_remove,
+	.shutdown = xge_shutdown,
+};
+module_platform_driver(xge_driver);
+
+MODULE_DESCRIPTION("APM X-Gene SoC Ethernet v2 driver");
+MODULE_AUTHOR("Iyappan Subramanian <isubramanian@apm.com>");
+MODULE_VERSION(XGENE_ENET_V2_VERSION);
+MODULE_LICENSE("GPL");
-- 
1.9.1

[PATCH net-next 5/6] drivers: net: xgene-v2: Add transmit and receive

[Iyappan Subramanian]

This patch adds,
    - Transmit
    - Transmit completion poll
    - Receive poll
    - NAPI handler

and enables the driver.

Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
---
 drivers/net/ethernet/apm/Kconfig           |   1 +
 drivers/net/ethernet/apm/Makefile          |   1 +
 drivers/net/ethernet/apm/xgene-v2/Kconfig  |  11 ++
 drivers/net/ethernet/apm/xgene-v2/Makefile |   6 +
 drivers/net/ethernet/apm/xgene-v2/main.c   | 200 ++++++++++++++++++++++++++++-
 5 files changed, 218 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/ethernet/apm/xgene-v2/Kconfig
 create mode 100644 drivers/net/ethernet/apm/xgene-v2/Makefile

diff --git a/drivers/net/ethernet/apm/Kconfig b/drivers/net/ethernet/apm/Kconfig
index ec63d70..59efe5b 100644
--- a/drivers/net/ethernet/apm/Kconfig
+++ b/drivers/net/ethernet/apm/Kconfig
@@ -1 +1,2 @@
 source "drivers/net/ethernet/apm/xgene/Kconfig"
+source "drivers/net/ethernet/apm/xgene-v2/Kconfig"
diff --git a/drivers/net/ethernet/apm/Makefile b/drivers/net/ethernet/apm/Makefile
index 65ce32a..946b2a4 100644
--- a/drivers/net/ethernet/apm/Makefile
+++ b/drivers/net/ethernet/apm/Makefile
@@ -3,3 +3,4 @@
 #
 
 obj-$(CONFIG_NET_XGENE) += xgene/
+obj-$(CONFIG_NET_XGENE_V2) += xgene-v2/
diff --git a/drivers/net/ethernet/apm/xgene-v2/Kconfig b/drivers/net/ethernet/apm/xgene-v2/Kconfig
new file mode 100644
index 0000000..1205861
--- /dev/null
+++ b/drivers/net/ethernet/apm/xgene-v2/Kconfig
@@ -0,0 +1,11 @@
+config NET_XGENE_V2
+	tristate "APM X-Gene SoC Ethernet-v2 Driver"
+	depends on HAS_DMA
+	depends on ARCH_XGENE || COMPILE_TEST
+	help
+	  This is the Ethernet driver for the on-chip ethernet interface
+	  which uses a linked list of DMA descriptor architecture (v2) for
+	  APM X-Gene SoCs.
+
+	  To compile this driver as a module, choose M here. This module will
+	  be called xgene-enet-v2.
diff --git a/drivers/net/ethernet/apm/xgene-v2/Makefile b/drivers/net/ethernet/apm/xgene-v2/Makefile
new file mode 100644
index 0000000..735309c
--- /dev/null
+++ b/drivers/net/ethernet/apm/xgene-v2/Makefile
@@ -0,0 +1,6 @@
+#
+# Makefile for APM X-Gene Ethernet v2 driver
+#
+
+xgene-enet-v2-objs := main.o mac.o enet.o ring.o
+obj-$(CONFIG_NET_XGENE_V2) += xgene-enet-v2.o
diff --git a/drivers/net/ethernet/apm/xgene-v2/main.c b/drivers/net/ethernet/apm/xgene-v2/main.c
index 3881f27..fc90298 100644
--- a/drivers/net/ethernet/apm/xgene-v2/main.c
+++ b/drivers/net/ethernet/apm/xgene-v2/main.c
@@ -164,9 +164,11 @@ static int xge_refill_buffers(struct net_device *ndev, u32 nbuf)
 		dma_wmb();
 		raw_desc->m0 = cpu_to_le64(SET_BITS(PKT_ADDRL, dma_addr) |
 					   SET_BITS(E, 1));
+
 		ring->skbs[tail] = skb;
 		tail = (tail + 1) & slots;
 	}
+	xge_wr_csr(pdata, DMARXCTRL, 1);
 
 	ring->tail = tail;
 
@@ -278,13 +280,14 @@ static int xge_open(struct net_device *ndev)
 	struct xge_pdata *pdata = netdev_priv(ndev);
 	int ret;
 
+	napi_enable(&pdata->napi);
+
 	ret = xge_request_irq(ndev);
 	if (ret)
 		return ret;
 
 	xge_intr_enable(pdata);
 
-	xge_wr_csr(pdata, DMARXCTRL, 1);
 	xge_mac_enable(pdata);
 	netif_start_queue(ndev);
 
@@ -298,11 +301,204 @@ static int xge_close(struct net_device *ndev)
 	netif_stop_queue(ndev);
 	xge_mac_disable(pdata);
 
+	xge_intr_disable(pdata);
 	xge_free_irq(ndev);
+	napi_disable(&pdata->napi);
 
 	return 0;
 }
 
+static netdev_tx_t xge_start_xmit(struct sk_buff *skb, struct net_device *ndev)
+{
+	struct xge_pdata *pdata = netdev_priv(ndev);
+	struct device *dev = &pdata->pdev->dev;
+	static dma_addr_t dma_addr;
+	struct xge_desc_ring *tx_ring;
+	struct xge_raw_desc *raw_desc;
+	u64 addr_lo, addr_hi;
+	void *pkt_buf;
+	u8 tail;
+	u16 len;
+
+	tx_ring = pdata->tx_ring;
+	tail = tx_ring->tail;
+	len = skb_headlen(skb);
+	raw_desc = &tx_ring->raw_desc[tail];
+
+	/* Tx descriptor not available */
+	if (!GET_BITS(E, le64_to_cpu(raw_desc->m0)) ||
+	    GET_BITS(PKT_SIZE, le64_to_cpu(raw_desc->m0)))
+		return NETDEV_TX_BUSY;
+
+	/* Packet buffers should be 64B aligned */
+	pkt_buf = dma_alloc_coherent(dev, XGENE_ENET_STD_MTU, &dma_addr,
+				     GFP_ATOMIC);
+	if (unlikely(!pkt_buf))
+		goto out;
+
+	memcpy(pkt_buf, skb->data, len);
+
+	addr_hi = GET_BITS(NEXT_DESC_ADDRH, le64_to_cpu(raw_desc->m1));
+	addr_lo = GET_BITS(NEXT_DESC_ADDRL, le64_to_cpu(raw_desc->m1));
+	raw_desc->m1 = cpu_to_le64(SET_BITS(NEXT_DESC_ADDRL, addr_lo) |
+				   SET_BITS(NEXT_DESC_ADDRH, addr_hi) |
+				   SET_BITS(PKT_ADDRH,
+					    dma_addr >> PKT_ADDRL_LEN));
+
+	dma_wmb();
+
+	raw_desc->m0 = cpu_to_le64(SET_BITS(PKT_ADDRL, dma_addr) |
+				   SET_BITS(PKT_SIZE, len) |
+				   SET_BITS(E, 0));
+
+	skb_tx_timestamp(skb);
+	xge_wr_csr(pdata, DMATXCTRL, 1);
+
+	pdata->stats.tx_packets++;
+	pdata->stats.tx_bytes += skb->len;
+
+	tx_ring->skbs[tail] = skb;
+	tx_ring->pkt_bufs[tail] = pkt_buf;
+	tx_ring->tail = (tail + 1) & (XGENE_ENET_NUM_DESC - 1);
+
+out:
+	dev_kfree_skb_any(skb);
+
+	return NETDEV_TX_OK;
+}
+
+static void xge_txc_poll(struct net_device *ndev, unsigned int budget)
+{
+	struct xge_pdata *pdata = netdev_priv(ndev);
+	struct device *dev = &pdata->pdev->dev;
+	struct xge_desc_ring *tx_ring;
+	struct xge_raw_desc *raw_desc;
+	u64 addr_lo, addr_hi;
+	dma_addr_t dma_addr;
+	void *pkt_buf;
+	bool pktsent;
+	u32 data;
+	u8 head;
+	int i;
+
+	tx_ring = pdata->tx_ring;
+	head = tx_ring->head;
+
+	data = xge_rd_csr(pdata, DMATXSTATUS);
+	pktsent = data & TX_PKT_SENT;
+	if (unlikely(!pktsent))
+		return;
+
+	for (i = 0; i < budget; i++) {
+		raw_desc = &tx_ring->raw_desc[head];
+
+		if (!GET_BITS(E, le64_to_cpu(raw_desc->m0)))
+			break;
+
+		dma_rmb();
+
+		addr_hi = GET_BITS(PKT_ADDRH, le64_to_cpu(raw_desc->m1));
+		addr_lo = GET_BITS(PKT_ADDRL, le64_to_cpu(raw_desc->m0));
+		dma_addr = (addr_hi << PKT_ADDRL_LEN) | addr_lo;
+
+		pkt_buf = tx_ring->pkt_bufs[head];
+
+		/* clear pktstart address and pktsize */
+		raw_desc->m0 = cpu_to_le64(SET_BITS(E, 1) |
+					   SET_BITS(PKT_SIZE, 0));
+		xge_wr_csr(pdata, DMATXSTATUS, 1);
+
+		dma_free_coherent(dev, XGENE_ENET_STD_MTU, pkt_buf, dma_addr);
+
+		head = (head + 1) & (XGENE_ENET_NUM_DESC - 1);
+	}
+
+	tx_ring->head = head;
+}
+
+static int xge_rx_poll(struct net_device *ndev, unsigned int budget)
+{
+	struct xge_pdata *pdata = netdev_priv(ndev);
+	struct device *dev = &pdata->pdev->dev;
+	dma_addr_t addr_hi, addr_lo, dma_addr;
+	struct xge_desc_ring *rx_ring;
+	struct xge_raw_desc *raw_desc;
+	struct sk_buff *skb;
+	int i, npkts, ret = 0;
+	bool pktrcvd;
+	u32 data;
+	u8 head;
+	u16 len;
+
+	rx_ring = pdata->rx_ring;
+	head = rx_ring->head;
+
+	data = xge_rd_csr(pdata, DMARXSTATUS);
+	pktrcvd = data & RXSTATUS_RXPKTRCVD;
+
+	if (unlikely(!pktrcvd))
+		return 0;
+
+	npkts = 0;
+	for (i = 0; i < budget; i++) {
+		raw_desc = &rx_ring->raw_desc[head];
+
+		if (GET_BITS(E, le64_to_cpu(raw_desc->m0)))
+			break;
+
+		dma_rmb();
+
+		addr_hi = GET_BITS(PKT_ADDRH, le64_to_cpu(raw_desc->m1));
+		addr_lo = GET_BITS(PKT_ADDRL, le64_to_cpu(raw_desc->m0));
+		dma_addr = (addr_hi << PKT_ADDRL_LEN) | addr_lo;
+		len = GET_BITS(PKT_SIZE, le64_to_cpu(raw_desc->m0));
+
+		dma_unmap_single(dev, dma_addr, XGENE_ENET_STD_MTU,
+				 DMA_FROM_DEVICE);
+
+		skb = rx_ring->skbs[head];
+		skb_put(skb, len);
+
+		skb->protocol = eth_type_trans(skb, ndev);
+
+		pdata->stats.rx_packets++;
+		pdata->stats.rx_bytes += len;
+		napi_gro_receive(&pdata->napi, skb);
+		npkts++;
+
+		ret = xge_refill_buffers(ndev, 1);
+		xge_wr_csr(pdata, DMARXSTATUS, 1);
+
+		if (ret)
+			break;
+
+		head = (head + 1) & (XGENE_ENET_NUM_DESC - 1);
+	}
+
+	rx_ring->head = head;
+
+	return npkts;
+}
+
+static int xge_napi(struct napi_struct *napi, const int budget)
+{
+	struct net_device *ndev = napi->dev;
+	struct xge_pdata *pdata = netdev_priv(ndev);
+	int processed;
+
+	pdata = netdev_priv(ndev);
+
+	xge_txc_poll(ndev, budget);
+	processed = xge_rx_poll(ndev, budget);
+
+	if (processed < budget) {
+		napi_complete(napi);
+		xge_intr_enable(pdata);
+	}
+
+	return processed;
+}
+
 static int xge_set_mac_addr(struct net_device *ndev, void *addr)
 {
 	struct xge_pdata *pdata = netdev_priv(ndev);
@@ -345,6 +541,7 @@ static void xge_get_stats64(struct net_device *ndev,
 static const struct net_device_ops xgene_ndev_ops = {
 	.ndo_open = xge_open,
 	.ndo_stop = xge_close,
+	.ndo_start_xmit = xge_start_xmit,
 	.ndo_set_mac_address = xge_set_mac_addr,
 	.ndo_tx_timeout = xge_timeout,
 	.ndo_get_stats64 = xge_get_stats64,
@@ -388,6 +585,7 @@ static int xge_probe(struct platform_device *pdev)
 	if (ret)
 		goto err;
 
+	netif_napi_add(ndev, &pdata->napi, xge_napi, NAPI_POLL_WEIGHT);
 	ret = register_netdev(ndev);
 	if (ret) {
 		netdev_err(ndev, "Failed to register netdev\n");
-- 
1.9.1

[PATCH net-next 6/6] MAINTAINERS: Add entry for APM X-Gene SoC Ethernet (v2) driver

[Iyappan Subramanian]

This patch adds a MAINTAINERS entry for the ethernet driver for
the on-chip ethernet interface which uses a linked list of DMA
descriptor architecture (v2) for APM X-Gene SoCs.

Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index cc106f7..35f4eeb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -896,6 +896,12 @@ F:	drivers/net/phy/mdio-xgene.c
 F:	Documentation/devicetree/bindings/net/apm-xgene-enet.txt
 F:	Documentation/devicetree/bindings/net/apm-xgene-mdio.txt
 
+APPLIED MICRO (APM) X-GENE SOC ETHERNET (V2) DRIVER
+M:	Iyappan Subramanian <isubramanian@apm.com>
+M:	Keyur Chudgar <kchudgar@apm.com>
+S:	Supported
+F:	drivers/net/ethernet/apm/xgene-v2/
+
 APPLIED MICRO (APM) X-GENE SOC PMU
 M:	Tai Nguyen <ttnguyen@apm.com>
 S:	Supported
-- 
1.9.1

[PATCH net-next 4/6] drivers: net: xgene-v2: Add base driver

[Andrew Lunn]

> +	phy_mode = device_get_phy_mode(dev);
> +	if (phy_mode < 0) {
> +		dev_err(dev, "Unable to get phy-connection-type\n");
> +		return phy_mode;
> +	}
> +	pdata->resources.phy_mode = phy_mode;
> +
> +	if (pdata->resources.phy_mode != PHY_INTERFACE_MODE_RGMII) {
> +		dev_err(dev, "Incorrect phy-connection-type specified\n");
> +		return -ENODEV;
> +	}

This seems a bit limiting. What if you need to use:

PHY_INTERFACE_MODE_RGMII_ID,
PHY_INTERFACE_MODE_RGMII_RXID,
PHY_INTERFACE_MODE_RGMII_TXID,

in order to set the RGMII delays.

   Andrew

[PATCH net-next 0/6] drivers: net: xgene-v2: Add RGMII based 1G driver

[Andrew Lunn]

On Tue, Jan 31, 2017 at 11:03:15AM -0800, Iyappan Subramanian wrote:
> This patch set adds support for RGMII based 1GbE hardware which uses a linked
> list of DMA descriptor architecture (v2) for APM X-Gene SoCs.

Hi Iyappan

Should we assume there are more patches to follow adding MDIO bus
support and phylib integration?

	Andrew

[PATCH net-next 4/6] drivers: net: xgene-v2: Add base driver

[Florian Fainelli]

On 01/31/2017 11:03 AM, Iyappan Subramanian wrote:
> This patch adds,
> 
>      - probe, remove, shutdown
>      - open, close and stats
>      - create and delete ring
>      - request and delete irq
> 
> Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
> Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
> ---

> +static void xge_delete_desc_rings(struct net_device *ndev)
> +{
> +	struct xge_pdata *pdata = netdev_priv(ndev);
> +	struct device *dev = &pdata->pdev->dev;
> +	struct xge_desc_ring *ring;
> +
> +	ring = pdata->tx_ring;
> +	if (ring) {
> +		if (ring->skbs)
> +			devm_kfree(dev, ring->skbs);
> +		if (ring->pkt_bufs)
> +			devm_kfree(dev, ring->pkt_bufs);
> +		devm_kfree(dev, ring);
> +	}

The very fact that you have to do the devm_kfree suggests that the way
you manage the lifetime of the ring is not appropriate, and in fact, if
we look at how xge_create_desc_ring() is called, in the driver's probe
function indicates that if the network interface is never openeded, we
are just wasting memory sitting there and doing nothing. You should
consider moving this to the ndo_open(), resp. ndo_close() functions to
optimize memory consumption wrt. the network interface state.

> +
> +	ring = pdata->rx_ring;
> +	if (ring) {
> +		if (ring->skbs)
> +			devm_kfree(dev, ring->skbs);
> +		devm_kfree(dev, ring);
> +	}
> +}
> +
> +static struct xge_desc_ring *xge_create_desc_ring(struct net_device *ndev)
> +{
> +	struct xge_pdata *pdata = netdev_priv(ndev);
> +	struct device *dev = &pdata->pdev->dev;
> +	struct xge_desc_ring *ring;
> +	u16 size;
> +
> +	ring = devm_kzalloc(dev, sizeof(struct xge_desc_ring), GFP_KERNEL);
> +	if (!ring)
> +		return NULL;
> +
> +	ring->ndev = ndev;
> +
> +	size = XGENE_ENET_DESC_SIZE * XGENE_ENET_NUM_DESC;
> +	ring->desc_addr = dmam_alloc_coherent(dev, size, &ring->dma_addr,
> +					      GFP_KERNEL | __GFP_ZERO);

There is no dmam_zalloc_coherent()? Then again, that seems to be a
candidate for dma_zalloc_coherent() and moving this to the ndo_open()
function.

> +	if (!ring->desc_addr) {
> +		devm_kfree(dev, ring);
> +		return NULL;
> +	}
> +
> +	xge_setup_desc(ring);
> +
> +	return ring;
> +}
> +
> +static int xge_refill_buffers(struct net_device *ndev, u32 nbuf)
> +{
> +	struct xge_pdata *pdata = netdev_priv(ndev);
> +	struct xge_desc_ring *ring = pdata->rx_ring;
> +	const u8 slots = XGENE_ENET_NUM_DESC - 1;
> +	struct device *dev = &pdata->pdev->dev;
> +	struct xge_raw_desc *raw_desc;
> +	u64 addr_lo, addr_hi;
> +	u8 tail = ring->tail;
> +	struct sk_buff *skb;
> +	dma_addr_t dma_addr;
> +	u16 len;
> +	int i;
> +
> +	for (i = 0; i < nbuf; i++) {
> +		raw_desc = &ring->raw_desc[tail];
> +
> +		len = XGENE_ENET_STD_MTU;
> +		skb = netdev_alloc_skb(ndev, len);
> +		if (unlikely(!skb))
> +			return -ENOMEM;

Are not you leaving holes in your RX ring if you do that?

> +
> +		dma_addr = dma_map_single(dev, skb->data, len, DMA_FROM_DEVICE);
> +		if (dma_mapping_error(dev, dma_addr)) {
> +			netdev_err(ndev, "DMA mapping error\n");
> +			dev_kfree_skb_any(skb);
> +			return -EINVAL;
> +		}


> +static void xge_timeout(struct net_device *ndev)
> +{
> +	struct xge_pdata *pdata = netdev_priv(ndev);
> +	struct netdev_queue *txq;
> +
> +	xge_mac_reset(pdata);
> +
> +	txq = netdev_get_tx_queue(ndev, 0);
> +	txq->trans_start = jiffies;
> +	netif_tx_start_queue(txq);

It most likely is not that simple, don't you want to walk the list of
pending transmissed SKBs and free them all?

> +}
> +
> +static void xge_get_stats64(struct net_device *ndev,
> +			    struct rtnl_link_stats64 *storage)
> +{
> +	struct xge_pdata *pdata = netdev_priv(ndev);
> +	struct xge_stats *stats = &pdata->stats;
> +
> +	storage->tx_packets += stats->tx_packets;
> +	storage->tx_bytes += stats->tx_bytes;
> +
> +	storage->rx_packets += stats->rx_packets;
> +	storage->rx_bytes += stats->rx_bytes;

Pretty sure you need some synchronization primitives here for non 64-bit
architectures (maybe this driver is not used outside of 64-bit, but still).


> +
> +	ndev->hw_features = ndev->features;
> +
> +	ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64));
> +	if (ret) {
> +		netdev_err(ndev, "No usable DMA configuration\n");
> +		goto err;
> +	}
> +
> +	ret = xge_init_hw(ndev);
> +	if (ret)
> +		goto err;

Missing netif_carrier_off() right before the register_netdev().

> +
> +	ret = register_netdev(ndev);
> +	if (ret) {
> +		netdev_err(ndev, "Failed to register netdev\n");
> +		goto err;
> +	}



-- 
Florian

[PATCH net-next 5/6] drivers: net: xgene-v2: Add transmit and receive

[Florian Fainelli]

On 01/31/2017 11:03 AM, Iyappan Subramanian wrote:
> This patch adds,
>     - Transmit
>     - Transmit completion poll
>     - Receive poll
>     - NAPI handler
> 
> and enables the driver.
> 
> Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
> Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
> ---

> +
> +	tx_ring = pdata->tx_ring;
> +	tail = tx_ring->tail;
> +	len = skb_headlen(skb);
> +	raw_desc = &tx_ring->raw_desc[tail];
> +
> +	/* Tx descriptor not available */
> +	if (!GET_BITS(E, le64_to_cpu(raw_desc->m0)) ||
> +	    GET_BITS(PKT_SIZE, le64_to_cpu(raw_desc->m0)))
> +		return NETDEV_TX_BUSY;
> +
> +	/* Packet buffers should be 64B aligned */
> +	pkt_buf = dma_alloc_coherent(dev, XGENE_ENET_STD_MTU, &dma_addr,
> +				     GFP_ATOMIC);
> +	if (unlikely(!pkt_buf))
> +		goto out;

Can't you obtain a DMA-API mapping for skb->data and pass it down to the
hardware? This copy here is inefficient.

> +
> +	memcpy(pkt_buf, skb->data, len);
> +
> +	addr_hi = GET_BITS(NEXT_DESC_ADDRH, le64_to_cpu(raw_desc->m1));
> +	addr_lo = GET_BITS(NEXT_DESC_ADDRL, le64_to_cpu(raw_desc->m1));
> +	raw_desc->m1 = cpu_to_le64(SET_BITS(NEXT_DESC_ADDRL, addr_lo) |
> +				   SET_BITS(NEXT_DESC_ADDRH, addr_hi) |
> +				   SET_BITS(PKT_ADDRH,
> +					    dma_addr >> PKT_ADDRL_LEN));
> +
> +	dma_wmb();
> +
> +	raw_desc->m0 = cpu_to_le64(SET_BITS(PKT_ADDRL, dma_addr) |
> +				   SET_BITS(PKT_SIZE, len) |
> +				   SET_BITS(E, 0));
> +
> +	skb_tx_timestamp(skb);
> +	xge_wr_csr(pdata, DMATXCTRL, 1);
> +
> +	pdata->stats.tx_packets++;
> +	pdata->stats.tx_bytes += skb->len;

This is both racy and incorrect. Racy because after you wrote DMATXCTRL,
your TX completion can run, and it can do that while interrupting your
CPU presumably, and free the SKB, therefore making you access a freed
SKB (or it should, if it does not), it's also incorrect, because before
you get signaled a TX completion, there is no guarantee that the packets
did actually make it through, you must update your stats in the TX
completion handler.

> +
> +	tx_ring->skbs[tail] = skb;
> +	tx_ring->pkt_bufs[tail] = pkt_buf;
> +	tx_ring->tail = (tail + 1) & (XGENE_ENET_NUM_DESC - 1);
> +
> +out:
> +	dev_kfree_skb_any(skb);

Don't do this, remember a pointer to the SKB, free the SKB in TX
completion handler, preferably in NAPI context.

> +
> +	return NETDEV_TX_OK;
> +}
> +
> +static void xge_txc_poll(struct net_device *ndev, unsigned int budget)
> +{
> +	struct xge_pdata *pdata = netdev_priv(ndev);
> +	struct device *dev = &pdata->pdev->dev;
> +	struct xge_desc_ring *tx_ring;
> +	struct xge_raw_desc *raw_desc;
> +	u64 addr_lo, addr_hi;
> +	dma_addr_t dma_addr;
> +	void *pkt_buf;
> +	bool pktsent;
> +	u32 data;
> +	u8 head;
> +	int i;
> +
> +	tx_ring = pdata->tx_ring;
> +	head = tx_ring->head;
> +
> +	data = xge_rd_csr(pdata, DMATXSTATUS);
> +	pktsent = data & TX_PKT_SENT;
> +	if (unlikely(!pktsent))
> +		return;
> +
> +	for (i = 0; i < budget; i++) {

TX completion handlers should run unbound and free the entire TX ring,
don't make it obey to an upper bound.

> +		raw_desc = &tx_ring->raw_desc[head];
> +
> +		if (!GET_BITS(E, le64_to_cpu(raw_desc->m0)))
> +			break;
> +
> +		dma_rmb();
> +
> +		addr_hi = GET_BITS(PKT_ADDRH, le64_to_cpu(raw_desc->m1));
> +		addr_lo = GET_BITS(PKT_ADDRL, le64_to_cpu(raw_desc->m0));
> +		dma_addr = (addr_hi << PKT_ADDRL_LEN) | addr_lo;
> +
> +		pkt_buf = tx_ring->pkt_bufs[head];
> +
> +		/* clear pktstart address and pktsize */
> +		raw_desc->m0 = cpu_to_le64(SET_BITS(E, 1) |
> +					   SET_BITS(PKT_SIZE, 0));
> +		xge_wr_csr(pdata, DMATXSTATUS, 1);
> +
> +		dma_free_coherent(dev, XGENE_ENET_STD_MTU, pkt_buf, dma_addr);
> +
> +		head = (head + 1) & (XGENE_ENET_NUM_DESC - 1);
> +	}
> +
> +	tx_ring->head = head;
> +}
> +
> +static int xge_rx_poll(struct net_device *ndev, unsigned int budget)
> +{
> +	struct xge_pdata *pdata = netdev_priv(ndev);
> +	struct device *dev = &pdata->pdev->dev;
> +	dma_addr_t addr_hi, addr_lo, dma_addr;
> +	struct xge_desc_ring *rx_ring;
> +	struct xge_raw_desc *raw_desc;
> +	struct sk_buff *skb;
> +	int i, npkts, ret = 0;
> +	bool pktrcvd;
> +	u32 data;
> +	u8 head;
> +	u16 len;
> +
> +	rx_ring = pdata->rx_ring;
> +	head = rx_ring->head;
> +
> +	data = xge_rd_csr(pdata, DMARXSTATUS);
> +	pktrcvd = data & RXSTATUS_RXPKTRCVD;
> +
> +	if (unlikely(!pktrcvd))
> +		return 0;
> +
> +	npkts = 0;
> +	for (i = 0; i < budget; i++) {

So pktrcvd is not an indication of the produced number of packets, just
that there are packets, that's not very convenient, and it's redundant
with the very fact of being interrupted.

> +		raw_desc = &rx_ring->raw_desc[head];
> +
> +		if (GET_BITS(E, le64_to_cpu(raw_desc->m0)))
> +			break;
> +
> +		dma_rmb();
> +
> +		addr_hi = GET_BITS(PKT_ADDRH, le64_to_cpu(raw_desc->m1));
> +		addr_lo = GET_BITS(PKT_ADDRL, le64_to_cpu(raw_desc->m0));
> +		dma_addr = (addr_hi << PKT_ADDRL_LEN) | addr_lo;
> +		len = GET_BITS(PKT_SIZE, le64_to_cpu(raw_desc->m0));

Is not there some kind of additional status that would indicate if the
packet is possibly invalid (oversize, undersize, etc.)?

> +
> +		dma_unmap_single(dev, dma_addr, XGENE_ENET_STD_MTU,
> +				 DMA_FROM_DEVICE);
> +
> +		skb = rx_ring->skbs[head];
> +		skb_put(skb, len);
> +
> +		skb->protocol = eth_type_trans(skb, ndev);
> +
> +		pdata->stats.rx_packets++;
> +		pdata->stats.rx_bytes += len;
> +		napi_gro_receive(&pdata->napi, skb);
> +		npkts++;

-- 
Florian

[PATCH net-next 5/6] drivers: net: xgene-v2: Add transmit and receive

[kbuild test robot]

Hi Iyappan,

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Iyappan-Subramanian/drivers-net-xgene-v2-Add-RGMII-based-1G-driver/20170201-034317
config: parisc-allyesconfig (attached as .config)
compiler: hppa-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=parisc 

All warnings (new ones prefixed by >>):

   In file included from include/linux/swab.h:4:0,
                    from include/uapi/linux/byteorder/big_endian.h:12,
                    from include/linux/byteorder/big_endian.h:4,
                    from arch/parisc/include/uapi/asm/byteorder.h:4,
                    from arch/parisc/include/asm/bitops.h:10,
                    from include/linux/bitops.h:36,
                    from include/linux/kernel.h:10,
                    from include/linux/list.h:8,
                    from include/linux/resource_ext.h:17,
                    from include/linux/acpi.h:26,
                    from drivers/net/ethernet/apm/xgene-v2/main.h:25,
                    from drivers/net/ethernet/apm/xgene-v2/main.c:22:
   drivers/net/ethernet/apm/xgene-v2/main.c: In function 'xge_refill_buffers':
   drivers/net/ethernet/apm/xgene-v2/main.c:162:20: warning: right shift count >= width of type [-Wshift-count-overflow]
              dma_addr >> PKT_ADDRL_LEN));
                       ^
   include/uapi/linux/swab.h:129:32: note: in definition of macro '__swab64'
     (__builtin_constant_p((__u64)(x)) ? \
                                   ^
>> include/linux/byteorder/generic.h:85:21: note: in expansion of macro '__cpu_to_le64'
    #define cpu_to_le64 __cpu_to_le64
                        ^~~~~~~~~~~~~
   drivers/net/ethernet/apm/xgene-v2/main.c:161:9: note: in expansion of macro 'SET_BITS'
            SET_BITS(PKT_ADDRH,
            ^~~~~~~~
   drivers/net/ethernet/apm/xgene-v2/main.c:162:20: warning: right shift count >= width of type [-Wshift-count-overflow]
              dma_addr >> PKT_ADDRL_LEN));
                       ^
   include/uapi/linux/swab.h:131:12: note: in definition of macro '__swab64'
     __fswab64(x))
               ^
>> include/linux/byteorder/generic.h:85:21: note: in expansion of macro '__cpu_to_le64'
    #define cpu_to_le64 __cpu_to_le64
                        ^~~~~~~~~~~~~
   drivers/net/ethernet/apm/xgene-v2/main.c:161:9: note: in expansion of macro 'SET_BITS'
            SET_BITS(PKT_ADDRH,
            ^~~~~~~~
   drivers/net/ethernet/apm/xgene-v2/main.c: In function 'xge_start_xmit':
   drivers/net/ethernet/apm/xgene-v2/main.c:346:19: warning: right shift count >= width of type [-Wshift-count-overflow]
             dma_addr >> PKT_ADDRL_LEN));
                      ^
   include/uapi/linux/swab.h:129:32: note: in definition of macro '__swab64'
     (__builtin_constant_p((__u64)(x)) ? \
                                   ^
>> include/linux/byteorder/generic.h:85:21: note: in expansion of macro '__cpu_to_le64'
    #define cpu_to_le64 __cpu_to_le64
                        ^~~~~~~~~~~~~
   drivers/net/ethernet/apm/xgene-v2/main.c:345:8: note: in expansion of macro 'SET_BITS'
           SET_BITS(PKT_ADDRH,
           ^~~~~~~~
   drivers/net/ethernet/apm/xgene-v2/main.c:346:19: warning: right shift count >= width of type [-Wshift-count-overflow]
             dma_addr >> PKT_ADDRL_LEN));
                      ^
   include/uapi/linux/swab.h:131:12: note: in definition of macro '__swab64'
     __fswab64(x))
               ^
>> include/linux/byteorder/generic.h:85:21: note: in expansion of macro '__cpu_to_le64'
    #define cpu_to_le64 __cpu_to_le64
                        ^~~~~~~~~~~~~
   drivers/net/ethernet/apm/xgene-v2/main.c:345:8: note: in expansion of macro 'SET_BITS'
           SET_BITS(PKT_ADDRH,
           ^~~~~~~~
   drivers/net/ethernet/apm/xgene-v2/main.c: In function 'xge_rx_poll':
   drivers/net/ethernet/apm/xgene-v2/main.c:453:23: warning: left shift count >= width of type [-Wshift-count-overflow]
      dma_addr = (addr_hi << PKT_ADDRL_LEN) | addr_lo;
                          ^~

vim +/__cpu_to_le64 +85 include/linux/byteorder/generic.h

^1da177e Linus Torvalds 2005-04-16  69   *	cpu_to_[bl]eXX(__uXX x)
^1da177e Linus Torvalds 2005-04-16  70   *	[bl]eXX_to_cpu(__uXX x)
^1da177e Linus Torvalds 2005-04-16  71   *
^1da177e Linus Torvalds 2005-04-16  72   * The same, but takes a pointer to the value to convert
^1da177e Linus Torvalds 2005-04-16  73   *	cpu_to_[bl]eXXp(__uXX x)
^1da177e Linus Torvalds 2005-04-16  74   *	[bl]eXX_to_cpup(__uXX x)
^1da177e Linus Torvalds 2005-04-16  75   *
^1da177e Linus Torvalds 2005-04-16  76   * The same, but change in situ
^1da177e Linus Torvalds 2005-04-16  77   *	cpu_to_[bl]eXXs(__uXX x)
^1da177e Linus Torvalds 2005-04-16  78   *	[bl]eXX_to_cpus(__uXX x)
^1da177e Linus Torvalds 2005-04-16  79   *
^1da177e Linus Torvalds 2005-04-16  80   * See asm-foo/byteorder.h for examples of how to provide
^1da177e Linus Torvalds 2005-04-16  81   * architecture-optimized versions
^1da177e Linus Torvalds 2005-04-16  82   *
^1da177e Linus Torvalds 2005-04-16  83   */
^1da177e Linus Torvalds 2005-04-16  84  
^1da177e Linus Torvalds 2005-04-16 @85  #define cpu_to_le64 __cpu_to_le64
^1da177e Linus Torvalds 2005-04-16  86  #define le64_to_cpu __le64_to_cpu
^1da177e Linus Torvalds 2005-04-16  87  #define cpu_to_le32 __cpu_to_le32
^1da177e Linus Torvalds 2005-04-16  88  #define le32_to_cpu __le32_to_cpu
^1da177e Linus Torvalds 2005-04-16  89  #define cpu_to_le16 __cpu_to_le16
^1da177e Linus Torvalds 2005-04-16  90  #define le16_to_cpu __le16_to_cpu
^1da177e Linus Torvalds 2005-04-16  91  #define cpu_to_be64 __cpu_to_be64
^1da177e Linus Torvalds 2005-04-16  92  #define be64_to_cpu __be64_to_cpu
^1da177e Linus Torvalds 2005-04-16  93  #define cpu_to_be32 __cpu_to_be32

:::::: The code at line 85 was first introduced by commit
:::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2

:::::: TO: Linus Torvalds <torvalds@ppc970.osdl.org>
:::::: CC: Linus Torvalds <torvalds@ppc970.osdl.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 48442 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170201/7498852d/attachment-0001.gz>

[PATCH net-next 5/6] drivers: net: xgene-v2: Add transmit and receive

[David Laight]

>From Florian Fainelli
> Sent: 31 January 2017 20:33
> On 01/31/2017 11:03 AM, Iyappan Subramanian wrote:
> > This patch adds,
> >     - Transmit
> >     - Transmit completion poll
> >     - Receive poll
> >     - NAPI handler
> >
> > and enables the driver.
> >
> > Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
> > Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
> > ---
> 
> > +
> > +	tx_ring = pdata->tx_ring;
> > +	tail = tx_ring->tail;
> > +	len = skb_headlen(skb);
> > +	raw_desc = &tx_ring->raw_desc[tail];
> > +
> > +	/* Tx descriptor not available */
> > +	if (!GET_BITS(E, le64_to_cpu(raw_desc->m0)) ||
> > +	    GET_BITS(PKT_SIZE, le64_to_cpu(raw_desc->m0)))
> > +		return NETDEV_TX_BUSY;

Aren't you supposed to detect 'ring full' and stop the code
giving you packets to transmit.

> > +
> > +	/* Packet buffers should be 64B aligned */

Is that really a requirement of the hardware?
Almost all ethernet frames are 4n+2 aligned.

> > +	pkt_buf = dma_alloc_coherent(dev, XGENE_ENET_STD_MTU, &dma_addr,
> > +				     GFP_ATOMIC);
> > +	if (unlikely(!pkt_buf))
> > +		goto out;
> 
> Can't you obtain a DMA-API mapping for skb->data and pass it down to the
> hardware? This copy here is inefficient.
> 
> > +
> > +	memcpy(pkt_buf, skb->data, len);

You really need to verify that the len <= XGENE_ENET_STD_MTU.

Isn't this code only transmitting the 'head' of the packet?
What about the fragments??
...
	David

[PATCH net-next 4/6] drivers: net: xgene-v2: Add base driver

[Iyappan Subramanian]

Hi Andrew,

On Tue, Jan 31, 2017 at 12:01 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> +     phy_mode = device_get_phy_mode(dev);
>> +     if (phy_mode < 0) {
>> +             dev_err(dev, "Unable to get phy-connection-type\n");
>> +             return phy_mode;
>> +     }
>> +     pdata->resources.phy_mode = phy_mode;
>> +
>> +     if (pdata->resources.phy_mode != PHY_INTERFACE_MODE_RGMII) {
>> +             dev_err(dev, "Incorrect phy-connection-type specified\n");
>> +             return -ENODEV;
>> +     }
>
> This seems a bit limiting. What if you need to use:
>
> PHY_INTERFACE_MODE_RGMII_ID,
> PHY_INTERFACE_MODE_RGMII_RXID,
> PHY_INTERFACE_MODE_RGMII_TXID,
>
> in order to set the RGMII delays.

This version of the driver doesn't support setting delays.  The delay
support will be added in the future.

>
>    Andrew
>

[PATCH net-next 4/6] drivers: net: xgene-v2: Add base driver

[Iyappan Subramanian]

Hi Florian,

On Tue, Jan 31, 2017 at 12:31 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 01/31/2017 11:03 AM, Iyappan Subramanian wrote:
>> This patch adds,
>>
>>      - probe, remove, shutdown
>>      - open, close and stats
>>      - create and delete ring
>>      - request and delete irq
>>
>> Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
>> Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
>> ---
>
>> +static void xge_delete_desc_rings(struct net_device *ndev)
>> +{
>> +     struct xge_pdata *pdata = netdev_priv(ndev);
>> +     struct device *dev = &pdata->pdev->dev;
>> +     struct xge_desc_ring *ring;
>> +
>> +     ring = pdata->tx_ring;
>> +     if (ring) {
>> +             if (ring->skbs)
>> +                     devm_kfree(dev, ring->skbs);
>> +             if (ring->pkt_bufs)
>> +                     devm_kfree(dev, ring->pkt_bufs);
>> +             devm_kfree(dev, ring);
>> +     }
>
> The very fact that you have to do the devm_kfree suggests that the way
> you manage the lifetime of the ring is not appropriate, and in fact, if
> we look at how xge_create_desc_ring() is called, in the driver's probe
> function indicates that if the network interface is never openeded, we
> are just wasting memory sitting there and doing nothing. You should
> consider moving this to the ndo_open(), resp. ndo_close() functions to
> optimize memory consumption wrt. the network interface state.

I will move these to open and close functions and will use dma_zalloc() APIs.

>
>> +
>> +     ring = pdata->rx_ring;
>> +     if (ring) {
>> +             if (ring->skbs)
>> +                     devm_kfree(dev, ring->skbs);
>> +             devm_kfree(dev, ring);
>> +     }
>> +}
>> +
>> +static struct xge_desc_ring *xge_create_desc_ring(struct net_device *ndev)
>> +{
>> +     struct xge_pdata *pdata = netdev_priv(ndev);
>> +     struct device *dev = &pdata->pdev->dev;
>> +     struct xge_desc_ring *ring;
>> +     u16 size;
>> +
>> +     ring = devm_kzalloc(dev, sizeof(struct xge_desc_ring), GFP_KERNEL);
>> +     if (!ring)
>> +             return NULL;
>> +
>> +     ring->ndev = ndev;
>> +
>> +     size = XGENE_ENET_DESC_SIZE * XGENE_ENET_NUM_DESC;
>> +     ring->desc_addr = dmam_alloc_coherent(dev, size, &ring->dma_addr,
>> +                                           GFP_KERNEL | __GFP_ZERO);
>
> There is no dmam_zalloc_coherent()? Then again, that seems to be a
> candidate for dma_zalloc_coherent() and moving this to the ndo_open()
> function.
>
>> +     if (!ring->desc_addr) {
>> +             devm_kfree(dev, ring);
>> +             return NULL;
>> +     }
>> +
>> +     xge_setup_desc(ring);
>> +
>> +     return ring;
>> +}
>> +
>> +static int xge_refill_buffers(struct net_device *ndev, u32 nbuf)
>> +{
>> +     struct xge_pdata *pdata = netdev_priv(ndev);
>> +     struct xge_desc_ring *ring = pdata->rx_ring;
>> +     const u8 slots = XGENE_ENET_NUM_DESC - 1;
>> +     struct device *dev = &pdata->pdev->dev;
>> +     struct xge_raw_desc *raw_desc;
>> +     u64 addr_lo, addr_hi;
>> +     u8 tail = ring->tail;
>> +     struct sk_buff *skb;
>> +     dma_addr_t dma_addr;
>> +     u16 len;
>> +     int i;
>> +
>> +     for (i = 0; i < nbuf; i++) {
>> +             raw_desc = &ring->raw_desc[tail];
>> +
>> +             len = XGENE_ENET_STD_MTU;
>> +             skb = netdev_alloc_skb(ndev, len);
>> +             if (unlikely(!skb))
>> +                     return -ENOMEM;
>
> Are not you leaving holes in your RX ring if you do that?

No.  The probe will fail and clean up the unused buffers.

>
>> +
>> +             dma_addr = dma_map_single(dev, skb->data, len, DMA_FROM_DEVICE);
>> +             if (dma_mapping_error(dev, dma_addr)) {
>> +                     netdev_err(ndev, "DMA mapping error\n");
>> +                     dev_kfree_skb_any(skb);
>> +                     return -EINVAL;
>> +             }
>
>
>> +static void xge_timeout(struct net_device *ndev)
>> +{
>> +     struct xge_pdata *pdata = netdev_priv(ndev);
>> +     struct netdev_queue *txq;
>> +
>> +     xge_mac_reset(pdata);
>> +
>> +     txq = netdev_get_tx_queue(ndev, 0);
>> +     txq->trans_start = jiffies;
>> +     netif_tx_start_queue(txq);
>
> It most likely is not that simple, don't you want to walk the list of
> pending transmissed SKBs and free them all?

I'll add more exhaustive clean up and restart of Tx hardware.

>
>> +}
>> +
>> +static void xge_get_stats64(struct net_device *ndev,
>> +                         struct rtnl_link_stats64 *storage)
>> +{
>> +     struct xge_pdata *pdata = netdev_priv(ndev);
>> +     struct xge_stats *stats = &pdata->stats;
>> +
>> +     storage->tx_packets += stats->tx_packets;
>> +     storage->tx_bytes += stats->tx_bytes;
>> +
>> +     storage->rx_packets += stats->rx_packets;
>> +     storage->rx_bytes += stats->rx_bytes;
>
> Pretty sure you need some synchronization primitives here for non 64-bit
> architectures (maybe this driver is not used outside of 64-bit, but still).

Synchronization primitives are not required for this driver, yet!

>
>
>> +
>> +     ndev->hw_features = ndev->features;
>> +
>> +     ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64));
>> +     if (ret) {
>> +             netdev_err(ndev, "No usable DMA configuration\n");
>> +             goto err;
>> +     }
>> +
>> +     ret = xge_init_hw(ndev);
>> +     if (ret)
>> +             goto err;
>
> Missing netif_carrier_off() right before the register_netdev().

I'll add them.

>
>> +
>> +     ret = register_netdev(ndev);
>> +     if (ret) {
>> +             netdev_err(ndev, "Failed to register netdev\n");
>> +             goto err;
>> +     }
>
>
>
> --
> Florian

[PATCH net-next 5/6] drivers: net: xgene-v2: Add transmit and receive

[Iyappan Subramanian]

Hi Florian,

On Tue, Jan 31, 2017 at 12:33 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 01/31/2017 11:03 AM, Iyappan Subramanian wrote:
>> This patch adds,
>>     - Transmit
>>     - Transmit completion poll
>>     - Receive poll
>>     - NAPI handler
>>
>> and enables the driver.
>>
>> Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
>> Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
>> ---
>
>> +
>> +     tx_ring = pdata->tx_ring;
>> +     tail = tx_ring->tail;
>> +     len = skb_headlen(skb);
>> +     raw_desc = &tx_ring->raw_desc[tail];
>> +
>> +     /* Tx descriptor not available */
>> +     if (!GET_BITS(E, le64_to_cpu(raw_desc->m0)) ||
>> +         GET_BITS(PKT_SIZE, le64_to_cpu(raw_desc->m0)))
>> +             return NETDEV_TX_BUSY;
>> +
>> +     /* Packet buffers should be 64B aligned */
>> +     pkt_buf = dma_alloc_coherent(dev, XGENE_ENET_STD_MTU, &dma_addr,
>> +                                  GFP_ATOMIC);
>> +     if (unlikely(!pkt_buf))
>> +             goto out;
>
> Can't you obtain a DMA-API mapping for skb->data and pass it down to the
> hardware? This copy here is inefficient.

This hardware requires 64-byte alignment.

>
>> +
>> +     memcpy(pkt_buf, skb->data, len);
>> +
>> +     addr_hi = GET_BITS(NEXT_DESC_ADDRH, le64_to_cpu(raw_desc->m1));
>> +     addr_lo = GET_BITS(NEXT_DESC_ADDRL, le64_to_cpu(raw_desc->m1));
>> +     raw_desc->m1 = cpu_to_le64(SET_BITS(NEXT_DESC_ADDRL, addr_lo) |
>> +                                SET_BITS(NEXT_DESC_ADDRH, addr_hi) |
>> +                                SET_BITS(PKT_ADDRH,
>> +                                         dma_addr >> PKT_ADDRL_LEN));
>> +
>> +     dma_wmb();
>> +
>> +     raw_desc->m0 = cpu_to_le64(SET_BITS(PKT_ADDRL, dma_addr) |
>> +                                SET_BITS(PKT_SIZE, len) |
>> +                                SET_BITS(E, 0));
>> +
>> +     skb_tx_timestamp(skb);
>> +     xge_wr_csr(pdata, DMATXCTRL, 1);
>> +
>> +     pdata->stats.tx_packets++;
>> +     pdata->stats.tx_bytes += skb->len;
>
> This is both racy and incorrect. Racy because after you wrote DMATXCTRL,
> your TX completion can run, and it can do that while interrupting your
> CPU presumably, and free the SKB, therefore making you access a freed
> SKB (or it should, if it does not), it's also incorrect, because before
> you get signaled a TX completion, there is no guarantee that the packets
> did actually make it through, you must update your stats in the TX
> completion handler.

Thanks.  I'll move the tx stats part to Tx completion.

>
>> +
>> +     tx_ring->skbs[tail] = skb;
>> +     tx_ring->pkt_bufs[tail] = pkt_buf;
>> +     tx_ring->tail = (tail + 1) & (XGENE_ENET_NUM_DESC - 1);
>> +
>> +out:
>> +     dev_kfree_skb_any(skb);
>
> Don't do this, remember a pointer to the SKB, free the SKB in TX
> completion handler, preferably in NAPI context.

I'll implement this.

>
>> +
>> +     return NETDEV_TX_OK;
>> +}
>> +
>> +static void xge_txc_poll(struct net_device *ndev, unsigned int budget)
>> +{
>> +     struct xge_pdata *pdata = netdev_priv(ndev);
>> +     struct device *dev = &pdata->pdev->dev;
>> +     struct xge_desc_ring *tx_ring;
>> +     struct xge_raw_desc *raw_desc;
>> +     u64 addr_lo, addr_hi;
>> +     dma_addr_t dma_addr;
>> +     void *pkt_buf;
>> +     bool pktsent;
>> +     u32 data;
>> +     u8 head;
>> +     int i;
>> +
>> +     tx_ring = pdata->tx_ring;
>> +     head = tx_ring->head;
>> +
>> +     data = xge_rd_csr(pdata, DMATXSTATUS);
>> +     pktsent = data & TX_PKT_SENT;
>> +     if (unlikely(!pktsent))
>> +             return;
>> +
>> +     for (i = 0; i < budget; i++) {
>
> TX completion handlers should run unbound and free the entire TX ring,
> don't make it obey to an upper bound.

I'll do as suggested.

>
>> +             raw_desc = &tx_ring->raw_desc[head];
>> +
>> +             if (!GET_BITS(E, le64_to_cpu(raw_desc->m0)))
>> +                     break;
>> +
>> +             dma_rmb();
>> +
>> +             addr_hi = GET_BITS(PKT_ADDRH, le64_to_cpu(raw_desc->m1));
>> +             addr_lo = GET_BITS(PKT_ADDRL, le64_to_cpu(raw_desc->m0));
>> +             dma_addr = (addr_hi << PKT_ADDRL_LEN) | addr_lo;
>> +
>> +             pkt_buf = tx_ring->pkt_bufs[head];
>> +
>> +             /* clear pktstart address and pktsize */
>> +             raw_desc->m0 = cpu_to_le64(SET_BITS(E, 1) |
>> +                                        SET_BITS(PKT_SIZE, 0));
>> +             xge_wr_csr(pdata, DMATXSTATUS, 1);
>> +
>> +             dma_free_coherent(dev, XGENE_ENET_STD_MTU, pkt_buf, dma_addr);
>> +
>> +             head = (head + 1) & (XGENE_ENET_NUM_DESC - 1);
>> +     }
>> +
>> +     tx_ring->head = head;
>> +}
>> +
>> +static int xge_rx_poll(struct net_device *ndev, unsigned int budget)
>> +{
>> +     struct xge_pdata *pdata = netdev_priv(ndev);
>> +     struct device *dev = &pdata->pdev->dev;
>> +     dma_addr_t addr_hi, addr_lo, dma_addr;
>> +     struct xge_desc_ring *rx_ring;
>> +     struct xge_raw_desc *raw_desc;
>> +     struct sk_buff *skb;
>> +     int i, npkts, ret = 0;
>> +     bool pktrcvd;
>> +     u32 data;
>> +     u8 head;
>> +     u16 len;
>> +
>> +     rx_ring = pdata->rx_ring;
>> +     head = rx_ring->head;
>> +
>> +     data = xge_rd_csr(pdata, DMARXSTATUS);
>> +     pktrcvd = data & RXSTATUS_RXPKTRCVD;
>> +
>> +     if (unlikely(!pktrcvd))
>> +             return 0;
>> +
>> +     npkts = 0;
>> +     for (i = 0; i < budget; i++) {
>
> So pktrcvd is not an indication of the produced number of packets, just
> that there are packets, that's not very convenient, and it's redundant
> with the very fact of being interrupted.

Agree, but the interrupt is common for Tx completion and Rx, this
check is still required.

>
>> +             raw_desc = &rx_ring->raw_desc[head];
>> +
>> +             if (GET_BITS(E, le64_to_cpu(raw_desc->m0)))
>> +                     break;
>> +
>> +             dma_rmb();
>> +
>> +             addr_hi = GET_BITS(PKT_ADDRH, le64_to_cpu(raw_desc->m1));
>> +             addr_lo = GET_BITS(PKT_ADDRL, le64_to_cpu(raw_desc->m0));
>> +             dma_addr = (addr_hi << PKT_ADDRL_LEN) | addr_lo;
>> +             len = GET_BITS(PKT_SIZE, le64_to_cpu(raw_desc->m0));
>
> Is not there some kind of additional status that would indicate if the
> packet is possibly invalid (oversize, undersize, etc.)?

I'll add error checking.

>
>> +
>> +             dma_unmap_single(dev, dma_addr, XGENE_ENET_STD_MTU,
>> +                              DMA_FROM_DEVICE);
>> +
>> +             skb = rx_ring->skbs[head];
>> +             skb_put(skb, len);
>> +
>> +             skb->protocol = eth_type_trans(skb, ndev);
>> +
>> +             pdata->stats.rx_packets++;
>> +             pdata->stats.rx_bytes += len;
>> +             napi_gro_receive(&pdata->napi, skb);
>> +             npkts++;
>
> --
> Florian

[PATCH net-next 5/6] drivers: net: xgene-v2: Add transmit and receive

[Iyappan Subramanian]

On Wed, Feb 1, 2017 at 3:09 AM, David Laight <David.Laight@aculab.com> wrote:
> From Florian Fainelli
>> Sent: 31 January 2017 20:33
>> On 01/31/2017 11:03 AM, Iyappan Subramanian wrote:
>> > This patch adds,
>> >     - Transmit
>> >     - Transmit completion poll
>> >     - Receive poll
>> >     - NAPI handler
>> >
>> > and enables the driver.
>> >
>> > Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
>> > Signed-off-by: Keyur Chudgar <kchudgar@apm.com>
>> > ---
>>
>> > +
>> > +   tx_ring = pdata->tx_ring;
>> > +   tail = tx_ring->tail;
>> > +   len = skb_headlen(skb);
>> > +   raw_desc = &tx_ring->raw_desc[tail];
>> > +
>> > +   /* Tx descriptor not available */
>> > +   if (!GET_BITS(E, le64_to_cpu(raw_desc->m0)) ||
>> > +       GET_BITS(PKT_SIZE, le64_to_cpu(raw_desc->m0)))
>> > +           return NETDEV_TX_BUSY;
>
> Aren't you supposed to detect 'ring full' and stop the code
> giving you packets to transmit.

I'll add stop queue and wake queue.

>
>> > +
>> > +   /* Packet buffers should be 64B aligned */
>
> Is that really a requirement of the hardware?
> Almost all ethernet frames are 4n+2 aligned.

Yes, it's a hardware requirement.

>
>> > +   pkt_buf = dma_alloc_coherent(dev, XGENE_ENET_STD_MTU, &dma_addr,
>> > +                                GFP_ATOMIC);
>> > +   if (unlikely(!pkt_buf))
>> > +           goto out;
>>
>> Can't you obtain a DMA-API mapping for skb->data and pass it down to the
>> hardware? This copy here is inefficient.
>>
>> > +
>> > +   memcpy(pkt_buf, skb->data, len);
>
> You really need to verify that the len <= XGENE_ENET_STD_MTU.

This version of the driver, doesn't support jumbo frame.  So, the
check is not required.

>
> Isn't this code only transmitting the 'head' of the packet?
> What about the fragments??

This driver doesn't enable SG yet.

> ...
>         David
>