Skip to contentSkip to conversation

dpaa_eth: add support for DPAA Ethernet

RFC PATCH Superseded Madalin Bucur 4 participants 21 messages Apr 1, 2015 – Jul 20, 2015

Subsystems (4)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

RFC PATCH 2/10 dpaa_eth: add support for DPAA Ethernet

This introduces the Freescale Data Path Acceleration Architecture (DPAA) Ethernet driver (dpaa_eth) that builds upon the DPAA QMan, BMan, PAMU and FMan drivers to deliver Ethernet connectivity on the Freescale DPAA QorIQ platforms.

---
 drivers/net/ethernet/freescale/Kconfig             |    2 +
 drivers/net/ethernet/freescale/Makefile            |    1 +
 drivers/net/ethernet/freescale/dpaa/Kconfig        |   49 +
 drivers/net/ethernet/freescale/dpaa/Makefile       |   13 +
 drivers/net/ethernet/freescale/dpaa/dpaa_eth.c     |  835 +++++++++++++
 drivers/net/ethernet/freescale/dpaa/dpaa_eth.h     |  446 +++++++
 .../net/ethernet/freescale/dpaa/dpaa_eth_common.c  | 1288 ++++++++++++++++++++
 .../net/ethernet/freescale/dpaa/dpaa_eth_common.h  |  119 ++
 drivers/net/ethernet/freescale/dpaa/dpaa_eth_sg.c  |  428 +++++++
 9 files changed, 3181 insertions(+)
 create mode 100644 drivers/net/ethernet/freescale/dpaa/Kconfig
 create mode 100644 drivers/net/ethernet/freescale/dpaa/Makefile
 create mode 100644 drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
 create mode 100644 drivers/net/ethernet/freescale/dpaa/dpaa_eth.h
 create mode 100644 drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.c
 create mode 100644 drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.h
 create mode 100644 drivers/net/ethernet/freescale/dpaa/dpaa_eth_sg.c
drivers/net/ethernet/freescale/Kconfig +2 -0
index ba84c4a..dd485f1 100644
@@ -95,4 +95,6 @@ config GIANFAR
 	  This driver supports the Gigabit TSEC on the MPC83xx, MPC85xx,
 	  and MPC86xx family of chips, and the FEC on the 8540.
 
+source "drivers/net/ethernet/freescale/dpaa/Kconfig"
+
 endif # NET_VENDOR_FREESCALE
drivers/net/ethernet/freescale/Makefile +1 -0
index 71debd1..2768178 100644
@@ -12,6 +12,7 @@ obj-$(CONFIG_FS_ENET) += fs_enet/
 obj-$(CONFIG_FSL_PQ_MDIO) += fsl_pq_mdio.o
 obj-$(CONFIG_FSL_XGMAC_MDIO) += xgmac_mdio.o
 obj-$(CONFIG_GIANFAR) += gianfar_driver.o
+obj-$(CONFIG_FSL_DPAA_ETH) += dpaa/
 obj-$(CONFIG_PTP_1588_CLOCK_GIANFAR) += gianfar_ptp.o
 gianfar_driver-objs := gianfar.o \
 		gianfar_ethtool.o
drivers/net/ethernet/freescale/dpaa/Kconfig new file +49 -0

49 lines not on this page · raw message

drivers/net/ethernet/freescale/dpaa/Makefile new file +13 -0
new file mode 100644
index 0000000..60f29db
@@ -0,0 +1,13 @@
+#
+# Makefile for the Freescale DPAA Ethernet controllers
+#
+
+# Include FMan headers
+FMAN        = $(srctree)/drivers/soc/fsl/fman
+ccflags-y += -I$(FMAN)
+ccflags-y += -I$(FMAN)/inc
+ccflags-y += -I$(FMAN)/flib
+
+obj-$(CONFIG_FSL_DPAA_ETH) += fsl_dpa.o
+
+fsl_dpa-objs += dpaa_eth.o dpaa_eth_sg.o dpaa_eth_common.o
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c new file +835 -0

835 lines not on this page · raw message

drivers/net/ethernet/freescale/dpaa/dpaa_eth.h new file +446 -0

446 lines not on this page · raw message

drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.c new file +1288 -0

1,288 lines not on this page · raw message

drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.h new file +119 -0

119 lines not on this page · raw message

drivers/net/ethernet/freescale/dpaa/dpaa_eth_sg.c new file +428 -0

237 lines not on this page · raw message

The rest of this diff is not shown. View the raw message.

Signature · 1 line
1.7.11.7

Part of the diff is not shown here. View the raw message.

#1 lore raw

RFC PATCH 3/10 dpaa_eth: add configurable bpool thresholds

Allow the user to tweak the refill threshold and the total number of buffers in the buffer pool. The provided values are for one CPU.

---
 drivers/net/ethernet/freescale/dpaa/Kconfig       | 20 ++++++++++++++++++++
 drivers/net/ethernet/freescale/dpaa/dpaa_eth.c    |  2 +-
 drivers/net/ethernet/freescale/dpaa/dpaa_eth.h    |  3 ---
 drivers/net/ethernet/freescale/dpaa/dpaa_eth_sg.c |  6 +++---
 4 files changed, 24 insertions(+), 7 deletions(-)
drivers/net/ethernet/freescale/dpaa/Kconfig +20 -0
index 7ef703c..9db7d25 100644
@@ -11,6 +11,26 @@ menuconfig FSL_DPAA_ETH
 
 if FSL_DPAA_ETH
 
+config FSL_DPAA_ETH_MAX_BUF_COUNT
+	int "Maximum nuber of buffers in private bpool"
+	depends on FSL_DPAA_ETH
+	range 64 2048
+	default "128"
+	---help---
+	  The maximum number of buffers to be by default allocated in the DPAA-Ethernet private port's
+	  buffer pool. One needn't normally modify this, as it has probably been tuned for performance
+	  already. This cannot be lower than DPAA_ETH_REFILL_THRESHOLD.
+
+config FSL_DPAA_ETH_REFILL_THRESHOLD
+	int "Private bpool refill threshold"
+	depends on FSL_DPAA_ETH
+	range 32 FSL_DPAA_ETH_MAX_BUF_COUNT
+	default "80"
+	---help---
+	  The DPAA-Ethernet driver will start replenishing buffer pools whose count
+	  falls below this threshold. This must be related to DPAA_ETH_MAX_BUF_COUNT. One needn't normally
+	  modify this value unless one has very specific performance reasons.
+
 config FSL_DPAA_CS_THRESHOLD_1G
 	hex "Egress congestion threshold on 1G ports"
 	depends on FSL_DPAA_ETH
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +1 -1
index ee65ca2..9e78371 100644
@@ -481,7 +481,7 @@ dpa_priv_bp_probe(struct device *dev)
 	}
 
 	dpa_bp->percpu_count = devm_alloc_percpu(dev, *dpa_bp->percpu_count);
-	dpa_bp->target_count = FSL_DPAA_ETH_MAX_BUF_COUNT;
+	dpa_bp->target_count = CONFIG_FSL_DPAA_ETH_MAX_BUF_COUNT;
 
 	dpa_bp->seed_cb = dpa_bp_priv_seed;
 	dpa_bp->free_buf_cb = _dpa_bp_free_pf;
drivers/net/ethernet/freescale/dpaa/dpaa_eth.h +0 -3
index b643c52..5f3157d 100644
@@ -126,9 +126,6 @@ struct dpa_buffer_layout_s {
 
 #define DPAA_ETH_RX_QUEUES	128
 
-#define FSL_DPAA_ETH_MAX_BUF_COUNT	128
-#define FSL_DPAA_ETH_REFILL_THRESHOLD	80
-
 /* More detailed FQ types - used for fine-grained WQ assignments */
 enum dpa_fq_type {
 	FQ_TYPE_RX_DEFAULT = 1, /* Rx Default FQs */
drivers/net/ethernet/freescale/dpaa/dpaa_eth_sg.c +3 -3
index ef3117e..1b3dc28 100644
@@ -153,7 +153,7 @@ int dpaa_eth_refill_bpools(struct dpa_bp *dpa_bp, int *countptr)
 	int count = *countptr;
 	int new_bufs;
 
-	if (unlikely(count < FSL_DPAA_ETH_REFILL_THRESHOLD)) {
+	if (unlikely(count < CONFIG_FSL_DPAA_ETH_REFILL_THRESHOLD)) {
 		do {
 			new_bufs = _dpa_bp_add_8_bufs(dpa_bp);
 			if (unlikely(!new_bufs)) {
@@ -164,10 +164,10 @@ int dpaa_eth_refill_bpools(struct dpa_bp *dpa_bp, int *countptr)
 				break;
 			}
 			count += new_bufs;
-		} while (count < FSL_DPAA_ETH_MAX_BUF_COUNT);
+		} while (count < CONFIG_FSL_DPAA_ETH_MAX_BUF_COUNT);
 
 		*countptr = count;
-		if (unlikely(count < FSL_DPAA_ETH_MAX_BUF_COUNT))
+		if (unlikely(count < CONFIG_FSL_DPAA_ETH_MAX_BUF_COUNT))
 			return -ENOMEM;
 	}
 
Signature · 1 line
1.7.11.7
#2 lore raw

RFC PATCH 4/10 dpaa_eth: add support for S/G frames

Add support for Scater/Gather (S/G) frames. The FMan can place the frame content into multiple buffers and provide a S/G Table (SGT) into one first buffer with references to the others.

---
 drivers/net/ethernet/freescale/dpaa/dpaa_eth.c     |   6 +
 .../net/ethernet/freescale/dpaa/dpaa_eth_common.c  |  34 +-
 .../net/ethernet/freescale/dpaa/dpaa_eth_common.h  |   2 +
 drivers/net/ethernet/freescale/dpaa/dpaa_eth_sg.c  | 356 +++++++++++++++++++--
 4 files changed, 367 insertions(+), 31 deletions(-)
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +6 -0
index 9e78371..036e272 100644
@@ -464,6 +464,12 @@ static int dpa_private_netdev_init(struct net_device *net_dev)
 	net_dev->hw_features |= (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
 		NETIF_F_LLTX);
 
+	/* Advertise S/G and HIGHDMA support for private interfaces */
+	net_dev->hw_features |= NETIF_F_SG | NETIF_F_HIGHDMA;
+	/* Recent kernels enable GSO automatically, if
+	 * we declare NETIF_F_SG. For conformity, we'll
+	 * still declare GSO explicitly.
+	 */
 	net_dev->features |= NETIF_F_GSO;
 
 	return dpa_netdev_init(net_dev, mac_addr, tx_timeout);
drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.c +33 -1
index 7094a45..1fdb35d 100644
@@ -1162,9 +1162,38 @@ void dpaa_eth_init_ports(struct mac_device *mac_dev,
 }
 EXPORT_SYMBOL(dpaa_eth_init_ports);
 
+void dpa_release_sgt(struct qm_sg_entry *sgt)
+{
+	struct dpa_bp *dpa_bp;
+	struct bm_buffer bmb[DPA_BUFF_RELEASE_MAX];
+	u8 i = 0, j;
+
+	do {
+		dpa_bp = dpa_bpid2pool(sgt[i].bpid);
+		DPA_ERR_ON(!dpa_bp);
+
+		j = 0;
+		do {
+			DPA_ERR_ON(sgt[i].extension);
+
+			bmb[j].hi = sgt[i].addr_hi;
+			bmb[j].lo = sgt[i].addr_lo;
+
+			j++; i++;
+		} while (j < ARRAY_SIZE(bmb) &&
+				!sgt[i - 1].final &&
+				sgt[i - 1].bpid == sgt[i].bpid);
+
+		while (bman_release(dpa_bp->pool, bmb, j, 0))
+			cpu_relax();
+	} while (!sgt[i - 1].final);
+}
+EXPORT_SYMBOL(dpa_release_sgt);
+
 void __attribute__((nonnull))
 dpa_fd_release(const struct net_device *net_dev, const struct qm_fd *fd)
 {
+	struct qm_sg_entry		*sgt;
 	struct dpa_bp			*_dpa_bp;
 	struct bm_buffer		 _bmb;
 
@@ -1174,7 +1203,10 @@ dpa_fd_release(const struct net_device *net_dev, const struct qm_fd *fd)
 	_dpa_bp = dpa_bpid2pool(fd->bpid);
 	DPA_ERR_ON(!_dpa_bp);
 
-	DPA_ERR_ON(fd->format == qm_fd_sg);
+	if (fd->format == qm_fd_sg) {
+		sgt = (phys_to_virt(bm_buf_addr(&_bmb)) + dpa_fd_offset(fd));
+		dpa_release_sgt(sgt);
+	}
 
 	while (bman_release(_dpa_bp->pool, &_bmb, 1, 0))
 		cpu_relax();
drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.h +2 -0
index 96bc63e..9b1774e 100644
@@ -52,6 +52,7 @@
 	fm_set_##type##_port_params(port, &param); \
 }
 
+#define DPA_SGT_MAX_ENTRIES 16 /* maximum number of entries in SG Table */
 #define DPA_BUFF_RELEASE_MAX 8 /* maximum number of buffers released at once */
 
 /* used in napi related functions */
@@ -110,6 +111,7 @@ void dpaa_eth_init_ports(struct mac_device *mac_dev,
 			 struct fm_port_fqs *port_fqs,
 			 struct dpa_buffer_layout_s *buf_layout,
 			 struct device *dev);
+void dpa_release_sgt(struct qm_sg_entry *sgt);
 void __attribute__((nonnull))
 dpa_fd_release(const struct net_device *net_dev, const struct qm_fd *fd);
 int dpa_enable_tx_csum(struct dpa_priv_s *priv,
drivers/net/ethernet/freescale/dpaa/dpaa_eth_sg.c +326 -30

424 lines not on this page · raw message

Signature · 1 line
1.7.11.7
#3 lore raw

RFC PATCH 5/10 dpaa_eth: add driver's Tx queue selection mechanism

Allow the selection of the transmission queue based on the CPU id.

---
 drivers/net/ethernet/freescale/dpaa/Kconfig           | 11 +++++++++++
 drivers/net/ethernet/freescale/dpaa/dpaa_eth.c        |  3 +++
 drivers/net/ethernet/freescale/dpaa/dpaa_eth.h        |  6 ++++++
 drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.c |  9 +++++++++
 drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.h |  4 ++++
 5 files changed, 33 insertions(+)
drivers/net/ethernet/freescale/dpaa/Kconfig +11 -0
index 9db7d25..a124639 100644
@@ -11,6 +11,17 @@ menuconfig FSL_DPAA_ETH
 
 if FSL_DPAA_ETH
 
+config FSL_DPAA_ETH_USE_NDO_SELECT_QUEUE
+	bool "Use driver's Tx queue selection mechanism"
+	default y
+	depends on FSL_DPAA_ETH
+	---help---
+	  The DPAA-Ethernet driver defines a ndo_select_queue() callback for optimal selection
+	  of the egress FQ. That will override the XPS support for this netdevice.
+	  If for whatever reason you want to be in control of the egress FQ-to-CPU selection and mapping,
+	  or simply don't want to use the driver's ndo_select_queue() callback, then unselect this
+	  and use the standard XPS support instead.
+
 config FSL_DPAA_ETH_MAX_BUF_COUNT
 	int "Maximum nuber of buffers in private bpool"
 	depends on FSL_DPAA_ETH
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +3 -0
index 036e272..6ca637d 100644
@@ -388,6 +388,9 @@ static const struct net_device_ops dpa_private_ops = {
 	.ndo_get_stats64 = dpa_get_stats64,
 	.ndo_set_mac_address = dpa_set_mac_address,
 	.ndo_validate_addr = eth_validate_addr,
+#ifdef CONFIG_FSL_DPAA_ETH_USE_NDO_SELECT_QUEUE
+	.ndo_select_queue = dpa_select_queue,
+#endif
 	.ndo_change_mtu = dpa_change_mtu,
 	.ndo_set_rx_mode = dpa_set_rx_mode,
 	.ndo_init = dpa_ndo_init,
drivers/net/ethernet/freescale/dpaa/dpaa_eth.h +6 -0
index 5f3157d..da2fd6b 100644
@@ -431,9 +431,15 @@ static inline void _dpa_assign_wq(struct dpa_fq *fq)
 	}
 }
 
+#ifdef CONFIG_FSL_DPAA_ETH_USE_NDO_SELECT_QUEUE
+/* Use in lieu of skb_get_queue_mapping() */
+#define dpa_get_queue_mapping(skb) \
+	smp_processor_id()
+#else
 /* Use the queue selected by XPS */
 #define dpa_get_queue_mapping(skb) \
 	skb_get_queue_mapping(skb)
+#endif
 
 static inline void _dpa_bp_free_pf(void *addr)
 {
drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.c +9 -0
index 1fdb35d..eb7a4a0 100644
@@ -615,6 +615,15 @@ bool dpa_bpid2pool_use(int bpid)
 	return false;
 }
 
+#ifdef CONFIG_FSL_DPAA_ETH_USE_NDO_SELECT_QUEUE
+u16 dpa_select_queue(struct net_device *net_dev, struct sk_buff *skb,
+		     void *accel_priv, select_queue_fallback_t fallback)
+{
+	return dpa_get_queue_mapping(skb);
+}
+EXPORT_SYMBOL(dpa_select_queue);
+#endif
+
 struct dpa_fq *dpa_fq_alloc(struct device *dev,
 			    const struct fqid_cell *fqids,
 			    struct list_head *list,
drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.h +4 -0
index 9b1774e..de5b68e 100644
@@ -89,6 +89,10 @@ struct dpa_bp *dpa_bpid2pool(int bpid);
 void dpa_bpid2pool_map(int bpid, struct dpa_bp *dpa_bp);
 bool dpa_bpid2pool_use(int bpid);
 void dpa_bp_drain(struct dpa_bp *bp);
+#ifdef CONFIG_FSL_DPAA_ETH_USE_NDO_SELECT_QUEUE
+u16 dpa_select_queue(struct net_device *net_dev, struct sk_buff *skb,
+		     void *accel_priv, select_queue_fallback_t fallback);
+#endif
 struct dpa_fq *dpa_fq_alloc(struct device *dev,
 			    const struct fqid_cell *fqids,
 			    struct list_head *list,
Signature · 1 line
1.7.11.7
#4 lore raw

RFC PATCH 6/10 dpaa_eth: add ethtool functionality

Add support for basic ethtool operations.

---
 drivers/net/ethernet/freescale/dpaa/Makefile       |   2 +-
 .../net/ethernet/freescale/dpaa/dpaa_eth_common.c  |   2 +
 .../net/ethernet/freescale/dpaa/dpaa_eth_common.h  |   3 +
 drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c | 254 +++++++++++++++++++++
 4 files changed, 260 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
drivers/net/ethernet/freescale/dpaa/Makefile +1 -1
index 60f29db..d17f67e 100644
@@ -10,4 +10,4 @@ ccflags-y += -I$(FMAN)/flib
 
 obj-$(CONFIG_FSL_DPAA_ETH) += fsl_dpa.o
 
-fsl_dpa-objs += dpaa_eth.o dpaa_eth_sg.o dpaa_eth_common.o
+fsl_dpa-objs += dpaa_eth.o dpaa_eth_sg.o dpaa_eth_common.o dpaa_ethtool.o
drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.c +2 -0
index eb7a4a0..acefc2b 100644
@@ -81,6 +81,8 @@ int dpa_netdev_init(struct net_device *net_dev,
 	memcpy(net_dev->perm_addr, mac_addr, net_dev->addr_len);
 	memcpy(net_dev->dev_addr, mac_addr, net_dev->addr_len);
 
+	net_dev->ethtool_ops = &dpa_ethtool_ops;
+
 	net_dev->needed_headroom = priv->tx_headroom;
 	net_dev->watchdog_timeo = msecs_to_jiffies(tx_timeout);
 
drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.h +3 -0
index de5b68e..53da367 100644
@@ -58,6 +58,9 @@
 /* used in napi related functions */
 extern u16 qman_portal_max;
 
+/* from dpa_ethtool.c */
+extern const struct ethtool_ops dpa_ethtool_ops;
+
 int dpa_netdev_init(struct net_device *net_dev,
 		    const u8 *mac_addr,
 		    u16 tx_timeout);
drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c new file +254 -0

254 lines not on this page · raw message

Signature · 1 line
1.7.11.7
#5 lore raw

RFC PATCH 7/10 dpaa_eth: add sysfs exports

Export Frame Queue and Buffer Pool IDs through sysfs.

---
 drivers/net/ethernet/freescale/dpaa/Makefile       |   2 +-
 drivers/net/ethernet/freescale/dpaa/dpaa_eth.c     |   2 +
 drivers/net/ethernet/freescale/dpaa/dpaa_eth.h     |   3 +
 .../net/ethernet/freescale/dpaa/dpaa_eth_common.c  |   2 +
 .../net/ethernet/freescale/dpaa/dpaa_eth_sysfs.c   | 168 +++++++++++++++++++++
 5 files changed, 176 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/ethernet/freescale/dpaa/dpaa_eth_sysfs.c
drivers/net/ethernet/freescale/dpaa/Makefile +1 -1
index d17f67e..8a72163 100644
@@ -10,4 +10,4 @@ ccflags-y += -I$(FMAN)/flib
 
 obj-$(CONFIG_FSL_DPAA_ETH) += fsl_dpa.o
 
-fsl_dpa-objs += dpaa_eth.o dpaa_eth_sg.o dpaa_eth_common.o dpaa_ethtool.o
+fsl_dpa-objs += dpaa_eth.o dpaa_eth_sg.o dpaa_eth_common.o dpaa_ethtool.o dpaa_eth_sysfs.o
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +2 -0
index 6ca637d..f50638e7 100644
@@ -747,6 +747,8 @@ dpaa_eth_priv_probe(struct platform_device *pdev)
 	if (err < 0)
 		goto netdev_init_failed;
 
+	dpaa_eth_sysfs_init(&net_dev->dev);
+
 	pr_info("Probed interface %s\n", net_dev->name);
 
 	return 0;
drivers/net/ethernet/freescale/dpaa/dpaa_eth.h +3 -0
index da2fd6b..117627b 100644
@@ -345,6 +345,9 @@ static inline u16 dpa_get_headroom(struct dpa_buffer_layout_s *bl)
 	return bl->data_align ? ALIGN(headroom, bl->data_align) : headroom;
 }
 
+void dpaa_eth_sysfs_remove(struct device *dev);
+void dpaa_eth_sysfs_init(struct device *dev);
+
 void dpa_private_napi_del(struct net_device *net_dev);
 
 static inline void clear_fd(struct qm_fd *fd)
drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.c +2 -0
index acefc2b..7921b64 100644
@@ -294,6 +294,8 @@ int __cold dpa_remove(struct platform_device *pdev)
 
 	priv = netdev_priv(net_dev);
 
+	dpaa_eth_sysfs_remove(dev);
+
 	dev_set_drvdata(dev, NULL);
 	unregister_netdev(net_dev);
 
drivers/net/ethernet/freescale/dpaa/dpaa_eth_sysfs.c new file +168 -0

168 lines not on this page · raw message

Signature · 1 line
1.7.11.7
#6 lore raw

RFC PATCH 8/10 dpaa_eth: add debugfs counters

Add a series of counters to be exported through debugfs:
- add detailed counters for reception errors;
- add detailed counters for QMan enqueue reject events;
- count the number of fragmented skbs received from the stack;
- count all frames received on the Tx confirmation path;
- add congestion group statistics;
- count the number of interrupts for each CPU.

---
 drivers/net/ethernet/freescale/dpaa/dpaa_eth.c     | 12 +++++++
 drivers/net/ethernet/freescale/dpaa/dpaa_eth.h     | 35 ++++++++++++++++++
 .../net/ethernet/freescale/dpaa/dpaa_eth_common.c  | 41 ++++++++++++++++++++--
 .../net/ethernet/freescale/dpaa/dpaa_eth_common.h  |  2 ++
 drivers/net/ethernet/freescale/dpaa/dpaa_eth_sg.c  |  1 +
 5 files changed, 89 insertions(+), 2 deletions(-)
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +12 -0
index f50638e7..eba0809 100644
@@ -99,6 +99,15 @@ static void _dpa_rx_error(struct net_device *net_dev,
 
 	percpu_priv->stats.rx_errors++;
 
+	if (fd->status & FM_PORT_FRM_ERR_DMA)
+		percpu_priv->rx_errors.dme++;
+	if (fd->status & FM_PORT_FRM_ERR_PHYSICAL)
+		percpu_priv->rx_errors.fpe++;
+	if (fd->status & FM_PORT_FRM_ERR_SIZE)
+		percpu_priv->rx_errors.fse++;
+	if (fd->status & FM_PORT_FRM_ERR_PRS_HDR_ERR)
+		percpu_priv->rx_errors.phe++;
+
 	dpa_fd_release(net_dev, fd);
 }
 
@@ -162,6 +171,8 @@ static void __hot _dpa_tx_conf(struct net_device	*net_dev,
 		percpu_priv->stats.tx_errors++;
 	}
 
+	percpu_priv->tx_confirm++;
+
 	skb = _dpa_cleanup_tx_fd(priv, fd);
 
 	dev_kfree_skb(skb);
@@ -297,6 +308,7 @@ static void priv_ern(struct qman_portal		*portal,
 
 	percpu_priv->stats.tx_dropped++;
 	percpu_priv->stats.tx_fifo_errors++;
+	count_ern(percpu_priv, msg);
 
 	/* If we intended this buffer to go into the pool
 	 * when the FM was done, we need to put it in
drivers/net/ethernet/freescale/dpaa/dpaa_eth.h +35 -0
index 117627b..761063d 100644
@@ -199,6 +199,25 @@ struct dpa_bp {
 	void (*free_buf_cb)(void *addr);
 };
 
+struct dpa_rx_errors {
+	u64 dme;		/* DMA Error */
+	u64 fpe;		/* Frame Physical Error */
+	u64 fse;		/* Frame Size Error */
+	u64 phe;		/* Header Error */
+};
+
+/* Counters for QMan ERN frames - one counter per rejection code */
+struct dpa_ern_cnt {
+	u64 cg_tdrop;		/* Congestion group taildrop */
+	u64 wred;		/* WRED congestion */
+	u64 err_cond;		/* Error condition */
+	u64 early_window;	/* Order restoration, frame too early */
+	u64 late_window;	/* Order restoration, frame too late */
+	u64 fq_tdrop;		/* FQ taildrop */
+	u64 fq_retired;		/* FQ is retired */
+	u64 orp_zero;		/* ORP disabled */
+};
+
 struct dpa_napi_portal {
 	struct napi_struct napi;
 	struct qman_portal *p;
@@ -207,7 +226,13 @@ struct dpa_napi_portal {
 struct dpa_percpu_priv_s {
 	struct net_device *net_dev;
 	struct dpa_napi_portal *np;
+	u64 in_interrupt;
+	u64 tx_confirm;
+	/* fragmented (non-linear) skbuffs received from the stack */
+	u64 tx_frag_skbuffs;
 	struct rtnl_link_stats64 stats;
+	struct dpa_rx_errors rx_errors;
+	struct dpa_ern_cnt ern_cnt;
 };
 
 struct dpa_priv_s {
@@ -235,6 +260,15 @@ struct dpa_priv_s {
 		 * (and the same) congestion group.
 		 */
 		struct qman_cgr cgr;
+		/* If congested, when it began. Used for performance stats. */
+		u32 congestion_start_jiffies;
+		/* Number of jiffies the Tx port was congested. */
+		u32 congested_jiffies;
+		/**
+		 * Counter for the number of times the CGR
+		 * entered congestion state
+		 */
+		u32 cgr_congested_count;
 	} cgr_data;
 	/* Use a per-port CGR for ingress traffic. */
 	bool use_ingress_cgr;
@@ -296,6 +330,7 @@ static inline int dpaa_eth_napi_schedule(struct dpa_percpu_priv_s *percpu_priv,
 
 			np->p = portal;
 			napi_schedule(&np->napi);
+			percpu_priv->in_interrupt++;
 			return 1;
 		}
 	}
drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.c +39 -2

55 lines not on this page · raw message

drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.h +2 -0
index 53da367..33acacc 100644
@@ -121,6 +121,8 @@ void dpaa_eth_init_ports(struct mac_device *mac_dev,
 void dpa_release_sgt(struct qm_sg_entry *sgt);
 void __attribute__((nonnull))
 dpa_fd_release(const struct net_device *net_dev, const struct qm_fd *fd);
+void count_ern(struct dpa_percpu_priv_s *percpu_priv,
+	       const struct qm_mr_entry *msg);
 int dpa_enable_tx_csum(struct dpa_priv_s *priv,
 		       struct sk_buff *skb,
 		       struct qm_fd *fd,
drivers/net/ethernet/freescale/dpaa/dpaa_eth_sg.c +1 -0
index 410effd..74f22eb 100644
@@ -656,6 +656,7 @@ int __hot dpa_tx(struct sk_buff *skb, struct net_device *net_dev)
 	    likely(skb_shinfo(skb)->nr_frags < DPA_SGT_MAX_ENTRIES)) {
 		/* Just create a S/G fd based on the skb */
 		err = skb_to_sg_fd(priv, skb, &fd);
+		percpu_priv->tx_frag_skbuffs++;
 	} else {
 		/* Make sure we have enough headroom to accommodate private
 		 * data, parse results, etc. Normally this shouldn't happen if
Signature · 1 line
1.7.11.7
#7 lore raw

RFC PATCH 9/10 dpaa_eth: add debugfs entries

Export per CPU counters through debugfs.

---
 drivers/net/ethernet/freescale/dpaa/Kconfig        |   7 +
 drivers/net/ethernet/freescale/dpaa/Makefile       |   3 +
 drivers/net/ethernet/freescale/dpaa/dpaa_debugfs.c | 273 +++++++++++++++++++++
 drivers/net/ethernet/freescale/dpaa/dpaa_debugfs.h |  43 ++++
 drivers/net/ethernet/freescale/dpaa/dpaa_eth.c     |  11 +
 drivers/net/ethernet/freescale/dpaa/dpaa_eth.h     |   4 +
 .../net/ethernet/freescale/dpaa/dpaa_eth_common.c  |  17 ++
 7 files changed, 358 insertions(+)
 create mode 100644 drivers/net/ethernet/freescale/dpaa/dpaa_debugfs.c
 create mode 100644 drivers/net/ethernet/freescale/dpaa/dpaa_debugfs.h
drivers/net/ethernet/freescale/dpaa/Kconfig +7 -0
index a124639..fc65d71 100644
@@ -77,4 +77,11 @@ config FSL_DPAA_INGRESS_CS_THRESHOLD
 	  The size in bytes of the ingress tail-drop threshold on FMan ports.
 	  Traffic piling up above this value will be rejected by QMan and discarded by FMan.
 
+config FSL_DPAA_ETH_DEBUGFS
+	bool "DPAA Ethernet debugfs interface"
+	depends on DEBUG_FS && FSL_DPAA_ETH
+	default y
+	---help---
+	  This option compiles debugfs code for the DPAA Ethernet driver.
+
 endif # FSL_DPAA_ETH
drivers/net/ethernet/freescale/dpaa/Makefile +3 -0
index 8a72163..867ec81 100644
@@ -11,3 +11,6 @@ ccflags-y += -I$(FMAN)/flib
 obj-$(CONFIG_FSL_DPAA_ETH) += fsl_dpa.o
 
 fsl_dpa-objs += dpaa_eth.o dpaa_eth_sg.o dpaa_eth_common.o dpaa_ethtool.o dpaa_eth_sysfs.o
+ifeq ($(CONFIG_FSL_DPAA_ETH_DEBUGFS),y)
+fsl_dpa-objs += dpaa_debugfs.o
+endif
drivers/net/ethernet/freescale/dpaa/dpaa_debugfs.c new file +273 -0

273 lines not on this page · raw message

drivers/net/ethernet/freescale/dpaa/dpaa_debugfs.h new file +43 -0

43 lines not on this page · raw message

drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +11 -0
index eba0809..a7875fb 100644
@@ -58,6 +58,9 @@
 #include "mac.h"
 #include "dpaa_eth.h"
 #include "dpaa_eth_common.h"
+#ifdef CONFIG_FSL_DPAA_ETH_DEBUGFS
+#include "dpaa_debugfs.h"
+#endif /* CONFIG_FSL_DPAA_ETH_DEBUGFS */
 
 #define DPA_NAPI_WEIGHT		64
 
@@ -818,6 +821,10 @@ static int __init __cold dpa_load(void)
 
 	pr_info(DPA_DESCRIPTION "\n");
 
+#ifdef CONFIG_FSL_DPAA_ETH_DEBUGFS
+	dpa_debugfs_module_init();
+#endif /* CONFIG_FSL_DPAA_ETH_DEBUGFS */
+
 	/* initialise dpaa_eth mirror values */
 	dpa_rx_extra_headroom = fm_get_rx_extra_headroom();
 	dpa_max_frm = fm_get_max_frm();
@@ -843,6 +850,10 @@ static void __exit __cold dpa_unload(void)
 
 	platform_driver_unregister(&dpa_driver);
 
+#ifdef CONFIG_FSL_DPAA_ETH_DEBUGFS
+	dpa_debugfs_module_exit();
+#endif /* CONFIG_FSL_DPAA_ETH_DEBUGFS */
+
 	/* Only one channel is used and needs to be relased after all
 	 * interfaces are removed
 	 */
drivers/net/ethernet/freescale/dpaa/dpaa_eth.h +4 -0
index 761063d..83f89f4 100644
@@ -252,6 +252,10 @@ struct dpa_priv_s {
 	u16 channel;	/* "fsl,qman-channel-id" */
 	struct list_head	 dpa_fq_list;
 
+#ifdef CONFIG_FSL_DPAA_ETH_DEBUGFS
+	struct dentry		*debugfs_file;
+#endif /* CONFIG_FSL_DPAA_ETH_DEBUGFS */
+
 	u32 msg_enable;	/* net_device message level */
 
 	struct {
drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.c +17 -0
index e92183a..71c108f 100644
@@ -43,6 +43,9 @@
 #include <linux/if_vlan.h>	/* vlan_eth_hdr */
 #include "dpaa_eth.h"
 #include "dpaa_eth_common.h"
+#ifdef CONFIG_FSL_DPAA_ETH_DEBUGFS
+#include "dpaa_debugfs.h"
+#endif /* CONFIG_FSL_DPAA_ETH_DEBUGFS */
 #include "mac.h"
 
 /* Size in bytes of the FQ taildrop threshold */
@@ -92,6 +95,15 @@ int dpa_netdev_init(struct net_device *net_dev,
 		return err;
 	}
 
+#ifdef CONFIG_FSL_DPAA_ETH_DEBUGFS
+	/* create debugfs entry for this net_device */
+	err = dpa_netdev_debugfs_create(net_dev);
+	if (err) {
+		unregister_netdev(net_dev);
+		return err;
+	}
+#endif /* CONFIG_FSL_DPAA_ETH_DEBUGFS */
+
 	return 0;
 }
 EXPORT_SYMBOL(dpa_netdev_init);
@@ -313,6 +325,11 @@ int __cold dpa_remove(struct platform_device *pdev)
 	if (priv->buf_layout)
 		devm_kfree(dev, priv->buf_layout);
 
+#ifdef CONFIG_FSL_DPAA_ETH_DEBUGFS
+	/* remove debugfs entry for this net_device */
+	dpa_netdev_debugfs_remove(net_dev);
+#endif /* CONFIG_FSL_DPAA_ETH_DEBUGFS */
+
 	free_netdev(net_dev);
 
 	return err;
Signature · 1 line
1.7.11.7
#8 lore raw

RFC PATCH 10/10 dpaa_eth: add trace points

Add trace points on the hot processing path.

---
 drivers/net/ethernet/freescale/dpaa/Makefile       |   1 +
 drivers/net/ethernet/freescale/dpaa/dpaa_eth.c     |  12 ++
 drivers/net/ethernet/freescale/dpaa/dpaa_eth.h     |   4 +
 .../net/ethernet/freescale/dpaa/dpaa_eth_trace.h   | 143 +++++++++++++++++++++
 4 files changed, 160 insertions(+)
 create mode 100644 drivers/net/ethernet/freescale/dpaa/dpaa_eth_trace.h
drivers/net/ethernet/freescale/dpaa/Makefile +1 -0
index 867ec81..4f9a619 100644
@@ -11,6 +11,7 @@ ccflags-y += -I$(FMAN)/flib
 obj-$(CONFIG_FSL_DPAA_ETH) += fsl_dpa.o
 
 fsl_dpa-objs += dpaa_eth.o dpaa_eth_sg.o dpaa_eth_common.o dpaa_ethtool.o dpaa_eth_sysfs.o
+CFLAGS_dpaa_eth.o := -I$(src)
 ifeq ($(CONFIG_FSL_DPAA_ETH_DEBUGFS),y)
 fsl_dpa-objs += dpaa_debugfs.o
 endif
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +12 -0
index a7875fb..8190c3d 100644
@@ -62,6 +62,12 @@
 #include "dpaa_debugfs.h"
 #endif /* CONFIG_FSL_DPAA_ETH_DEBUGFS */
 
+/* CREATE_TRACE_POINTS only needs to be defined once. Other dpa files
+ * using trace events only need to #include <trace/events/sched.h>
+ */
+#define CREATE_TRACE_POINTS
+#include "dpaa_eth_trace.h"
+
 #define DPA_NAPI_WEIGHT		64
 
 /* Valid checksum indication */
@@ -227,6 +233,9 @@ priv_rx_default_dqrr(struct qman_portal		*portal,
 	priv = netdev_priv(net_dev);
 	dpa_bp = priv->dpa_bp;
 
+	/* Trace the Rx fd */
+	trace_dpa_rx_fd(net_dev, fq, &dq->fd);
+
 	/* IRQ handler, non-migratable; safe to use raw_cpu_ptr here */
 	percpu_priv = raw_cpu_ptr(priv->percpu_priv);
 	count_ptr = raw_cpu_ptr(dpa_bp->percpu_count);
@@ -283,6 +292,9 @@ priv_tx_conf_default_dqrr(struct qman_portal		*portal,
 	net_dev = ((struct dpa_fq *)fq)->net_dev;
 	priv = netdev_priv(net_dev);
 
+	/* Trace the fd */
+	trace_dpa_tx_conf_fd(net_dev, fq, &dq->fd);
+
 	/* Non-migratable context, safe to use raw_cpu_ptr */
 	percpu_priv = raw_cpu_ptr(priv->percpu_priv);
 
drivers/net/ethernet/freescale/dpaa/dpaa_eth.h +4 -0
index 83f89f4..133a3f2 100644
@@ -36,6 +36,7 @@
 
 #include "fm_ext.h"
 #include "mac.h"
+#include "dpaa_eth_trace.h"
 
 extern int dpa_rx_extra_headroom;
 extern int dpa_max_frm;
@@ -423,6 +424,9 @@ static inline int __hot dpa_xmit(struct dpa_priv_s *priv,
 				_dpa_get_tx_conf_queue(priv, egress_fq)
 				);
 
+	/* Trace this Tx fd */
+	trace_dpa_tx_fd(priv->net_dev, egress_fq, fd);
+
 	for (i = 0; i < 100000; i++) {
 		err = qman_enqueue(egress_fq, fd, 0);
 		if (err != -EBUSY)
drivers/net/ethernet/freescale/dpaa/dpaa_eth_trace.h new file +143 -0

143 lines not on this page · raw message

Signature · 1 line
1.7.11.7
maintainer
#9 lore raw

Re: [PATCH RFC 10/10] dpaa_eth: add trace points

Quoted 1 line · Madalin Bucur

Add trace points on the hot processing path.

more trivia:

drivers/net/ethernet/freescale/dpaa/dpaa_eth_trace.h

Quoted 1 line
drivers/net/ethernet/freescale/dpaa/dpaa_eth_trace.h +0 -0

[]

drivers/net/ethernet/freescale/dpaa/dpaa_eth_trace.h line 46

Quoted 4 lines
drivers/net/ethernet/freescale/dpaa/dpaa_eth_trace.h +4 -0
+#define fd_format_name(format)	{ qm_fd_##format, #format }
+#define fd_format_list	\
+	fd_format_name(contig),	\
+	fd_format_name(sg)

Are these used anywhere?

drivers/net/ethernet/freescale/dpaa/dpaa_eth_trace.h line 48

Quoted 2 lines
drivers/net/ethernet/freescale/dpaa/dpaa_eth_trace.h +2 -0
+#define TR_FMT "[%s] fqid=%d, fd: addr=0x%llx, format=%s, off=%u, len=%u," \
+	" status=0x%08x"

It's nicer to coalesce string fragments.

Unless this is intended to be used more than once, perhaps it's better to remove it and use the string directly instead.

Quoted 5 lines
drivers/net/ethernet/freescale/dpaa/dpaa_eth_trace.h +5 -0
+	/* This is what gets printed when the trace event is triggered */
+	TP_printk(TR_FMT,
+		  __get_str(name), __entry->fqid, __entry->fd_addr,
+		  __print_symbolic(__entry->fd_format, fd_format_list),
+		  __entry->fd_offset, __entry->fd_length, __entry->fd_status)
#10 lore raw

RE: [PATCH RFC 10/10] dpaa_eth: add trace points

Quoted 11 lines

-----Original Message-----
From: Joe Perches [mailto:[email protected]]

Quoted 1 line · Madalin Bucur

Add trace points on the hot processing path.

more trivia:

Review

Quoted 1 line
(quoted diff) +0 -0

b/drivers/net/ethernet/freescale/dpaa/dpaa_eth_trace.h []

Quoted 6 lines

drivers/net/ethernet/freescale/dpaa/dpaa_eth_trace.h line 46

Quoted 4 lines
drivers/net/ethernet/freescale/dpaa/dpaa_eth_trace.h new file +4 -0
+#define fd_format_name(format)	{ qm_fd_##format, #format }
+#define fd_format_list	\
+	fd_format_name(contig),	\
+	fd_format_name(sg)

Are these used anywhere?

Yes, by the Frame Descriptor print:

        /* This is what gets printed when the trace event is triggered */
        TP_printk(TR_FMT,
                  __get_str(name), __entry->fqid, __entry->fd_addr,
                  __print_symbolic(__entry->fd_format, fd_format_list), 		// <-- here
                  __entry->fd_offset, __entry->fd_length, __entry->fd_status)
Quoted 10 lines
Quoted 1 line

+#define TR_FMT "[%s] fqid=%d, fd: addr=0x%llx, format=%s, off=%u,

len=%u," \

drivers/net/ethernet/freescale/dpaa/dpaa_eth_trace.h line 48

Quoted 1 line
drivers/net/ethernet/freescale/dpaa/dpaa_eth_trace.h new file +1 -0
+	" status=0x%08x"

It's nicer to coalesce string fragments.

Unless this is intended to be used more than once, perhaps it's better to remove it and use the string directly instead.

Quoted 7 lines
Quoted 6 lines
drivers/net/ethernet/freescale/dpaa/dpaa_eth_trace.h new file +5 -0
+	/* This is what gets printed when the trace event is triggered */
+	TP_printk(TR_FMT,
+		  __get_str(name), __entry->fqid, __entry->fd_addr,
+		  __print_symbolic(__entry->fd_format, fd_format_list),
+		  __entry->fd_offset, __entry->fd_length, __entry-

fd_status)

Checkpatch seems to be less forgiving if the long string is not in a printk:

WARNING: line over 80 characters
#22: FILE: drivers/net/ethernet/freescale/dpaa/dpaa_eth_trace.h:47:
+#define TR_FMT "[%s] fqid=%d, fd: addr=0x%llx, format=%s, off=%u, len=%u, status=0x%08x"

Madalin

maintainer
#11 lore raw

Re: [PATCH RFC 10/10] dpaa_eth: add trace points

Quoted 17 lines · Madalin-Cristian Bucur
Quoted 11 lines

-----Original Message-----
From: Joe Perches [mailto:[email protected]]

Quoted 1 line · Madalin Bucur

Add trace points on the hot processing path.

more trivia:

Review

Quoted 1 line
(quoted diff) +0 -0

b/drivers/net/ethernet/freescale/dpaa/dpaa_eth_trace.h []

Quoted 6 lines

drivers/net/ethernet/freescale/dpaa/dpaa_eth_trace.h line 46

Quoted 4 lines
drivers/net/ethernet/freescale/dpaa/dpaa_eth_trace.h new file +4 -0
+#define fd_format_name(format)	{ qm_fd_##format, #format }
+#define fd_format_list	\
+	fd_format_name(contig),	\
+	fd_format_name(sg)

Are these used anywhere?

Quoted 7 lines

Yes, by the Frame Descriptor print:

        /* This is what gets printed when the trace event is triggered */
        TP_printk(TR_FMT,
                  __get_str(name), __entry->fqid, __entry->fd_addr,
                  __print_symbolic(__entry->fd_format, fd_format_list), 		// <-- here
                  __entry->fd_offset, __entry->fd_length, __entry->fd_status)

Thanks.

I think it's nicer when format and argument match
and aren't indirected by macros.

Perhaps this would be better as:

	TP_printk("[%s] fqid=%d, fd: addr=0x%llx, format=%s, off=%u, len=%u, status=0x%08x"
		  __get_str(name), __entry->fqid, __entry->fd_addr,
		  __print_symbolic(__entry->fd_format, fd_format_list),
		  __entry->fd_offset, __entry->fd_length, __entry->fd_status)
Quoted 1 line

Checkpatch seems to be less forgiving if the long string is not in a printk:

checkpatch is brainless, people aren't.

Quoted 3 lines

WARNING: line over 80 characters
#22: FILE: drivers/net/ethernet/freescale/dpaa/dpaa_eth_trace.h:47:
+#define TR_FMT "[%s] fqid=%d, fd: addr=0x%llx, format=%s, off=%u, len=%u, status=0x%08x"

That's a checkpatch defect. Thanks. I'll see about fixing it.

maintainer
#12 lore raw

PATCH 1/1 checkpatch: Add #define foo "string" long line exception

There are #defines with long string constants like:

	#define foo "some really long string > 80 columns"

Add a long line exception for them.

Miscellanea:

Use the $String variable for slightly better readability

---

Quoted 5 lines
Quoted 3 lines

WARNING: line over 80 characters
#22: FILE: drivers/net/ethernet/freescale/dpaa/dpaa_eth_trace.h:47:
+#define TR_FMT "[%s] fqid=%d, fd: addr=0x%llx, format=%s, off=%u, len=%u, status=0x%08x"

That's a checkpatch defect. Thanks. I'll see about fixing it.

 scripts/checkpatch.pl | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
scripts/checkpatch.pl +3 -2
index 9a8b2bd..36d7e70 100755
@@ -2513,8 +2513,9 @@ sub process {
 #line length limit
 		if ($line =~ /^\+/ && $prevrawline !~ /\/\*\*/ &&
 		    $rawline !~ /^.\s*\*\s*\@$Ident\s/ &&
-		    !($line =~ /^\+\s*$logFunctions\s*\(\s*(?:(KERN_\S+\s*|[^"]*))?"[X\t]*"\s*(?:|,|\)\s*;)\s*$/ ||
-		    $line =~ /^\+\s*"[^"]*"\s*(?:\s*|,|\)\s*;)\s*$/) &&
+		    !($line =~ /^\+\s*$logFunctions\s*\(\s*(?:(KERN_\S+\s*|[^"]*))?$String\s*(?:|,|\)\s*;)\s*$/ ||
+		      $line =~ /^\+\s*$String\s*(?:\s*|,|\)\s*;)\s*$/ ||
+		      $line =~ /^\+\s*#\s*define\s+\w+\s+$String$/) &&
 		    $length > $max_line_length)
 		{
 			WARN("LONG_LINE",
maintainer
#13 lore raw

Re: [PATCH RFC 06/10] dpaa_eth: add ethtool functionality

In reply to Madalin Bucur

drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c

Quoted 3 lines · Madalin Bucur

Add support for basic ethtool operations.

drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c +0 -0

[]

drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c line 44

Quoted 4 lines
drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c +4 -0
+static int __cold dpa_get_settings(struct net_device *net_dev,
+				   struct ethtool_cmd *et_cmd)
+{
+	int			 _errno;

Using a variable name of _errno is misleading at best, (btw: the only return value for phy_ethtool_gset is 0)

drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c line 87

Quoted 7 lines
drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c +7 -0
+static void __cold dpa_get_drvinfo(struct net_device *net_dev,
+				   struct ethtool_drvinfo *drvinfo)
+{
+	int		 _errno;
+
+	strncpy(drvinfo->driver, KBUILD_MODNAME,
+		sizeof(drvinfo->driver) - 1)[sizeof(drvinfo->driver) - 1] = 0;

That's a really odd and unusual construct
more commonly written as strlcpy

drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c line 88

Quoted 1 line
drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c +1 -0
+	_errno = snprintf(drvinfo->version, sizeof(drvinfo->version),

Using errno here is especially misleading as that's not the return value of an snprintf

maintainer
#14 lore raw
In reply to Madalin Bucur
Quoted 4 lines · Madalin Bucur

This introduces the Freescale Data Path Acceleration Architecture (DPAA) Ethernet driver (dpaa_eth) that builds upon the DPAA QMan, BMan, PAMU and FMan drivers to deliver Ethernet connectivity on the Freescale DPAA QorIQ platforms.

trivial notes:

drivers/net/ethernet/freescale/dpaa/dpaa_eth.c

Quoted 1 line
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +0 -0

[]

Quoted 8 lines
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +7 -0
@@ -0,0 +1,835 @@
+/* Copyright 2008 - 2015 Freescale Semiconductor Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *     * Redistributions of source code must retain the above copyright
+ *	 notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright

drivers/net/ethernet/freescale/dpaa/dpaa_eth.c line 17

Quoted 10 lines
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +10 -0
+ *	 notice, this list of conditions and the following disclaimer in the
+ *	 documentation and/or other materials provided with the distribution.
+ *     * Neither the name of Freescale Semiconductor nor the
+ *	 names of its contributors may be used to endorse or promote products
+ *	 derived from this software without specific prior written permission.
+ *
+ * ALTERNATIVELY, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") as published by the Free Software
+ * Foundation, either version 2 of that License or (at your option) any
+ * later version.

Given this is GPLed here, does the first block need to exist?

drivers/net/ethernet/freescale/dpaa/dpaa_eth.c line 19

Quoted 2 lines
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +2 -0
+#define pr_fmt(fmt) \
+	KBUILD_MODNAME ": " fmt

single line please

drivers/net/ethernet/freescale/dpaa/dpaa_eth.c line 48

Quoted 9 lines
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +9 -0
+#include <linux/if_arp.h>	/* arp_hdr_len() */
+#include <linux/if_vlan.h>	/* VLAN_HLEN */
+#include <linux/icmp.h>		/* struct icmphdr */
+#include <linux/ip.h>		/* struct iphdr */
+#include <linux/ipv6.h>		/* struct ipv6hdr */
+#include <linux/udp.h>		/* struct udphdr */
+#include <linux/tcp.h>		/* struct tcphdr */
+#include <linux/net.h>		/* net_ratelimit() */
+#include <linux/if_ether.h>	/* ETH_P_IP and ETH_P_IPV6 */

These comments are pretty unusual and likely incomplete so they're probably not useful.

drivers/net/ethernet/freescale/dpaa/dpaa_eth.c line 71

Quoted 3 lines
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +3 -0
+static u8 debug = -1;
+module_param(debug, byte, S_IRUGO);
+MODULE_PARM_DESC(debug, "Module/Driver verbosity level");

default on? Likely default off would be better.

Quoted 5 lines
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +5 -0
+static void _dpa_rx_error(struct net_device *net_dev,
+			  const struct dpa_priv_s	*priv,
+			  struct dpa_percpu_priv_s *percpu_priv,
+			  const struct qm_fd *fd,
+			  u32 fqid)

drivers/net/ethernet/freescale/dpaa/dpaa_eth.c line 98

Quoted 10 lines
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +10 -0
+{
+	/* limit common, possibly innocuous Rx FIFO Overflow errors'
+	 * interference with zero-loss convergence benchmark results.
+	 */
+	if (likely(fd->status & FM_FD_STAT_ERR_PHYSICAL))
+		pr_warn_once("non-zero error counters in fman statistics (sysfs)\n");
+	else
+		if (netif_msg_hw(priv) && net_ratelimit())
+			netdev_err(net_dev, "Err FD status = 0x%08x\n",
+				   fd->status & FM_FD_STAT_RX_ERRORS);
	if (netif_msg_<foo>(priv))
		netdev_<level>(netdev, ...)

uses can be written like:

	netif_<level>(priv, <foo>, netdev, ...);

So this is perhaps better as

	if (likely(fd->status & FM_FD_STAT_ERR_PHYSICAL))
		pr_warn_once("non-zero error counters in fman statistics (sysfs)\n");
	else if (net_ratelimit())
		netif_err(priv, hw, net_dev, "Err FD status = 0x%08x\n",
			  fd->status & FM_FD_STAT_RX_ERRORS);
Quoted 7 lines
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +7 -0
+
+	percpu_priv->stats.rx_errors++;
+
+	dpa_fd_release(net_dev, fd);,
+}
+
+static void _dpa_tx_error(struct net_device		*net_dev,

drivers/net/ethernet/freescale/dpaa/dpaa_eth.c line 115

Quoted 10 lines
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +10 -0
+			  const struct dpa_priv_s	*priv,
+			  struct dpa_percpu_priv_s	*percpu_priv,
+			  const struct qm_fd		*fd,
+			  u32				 fqid)
+{
+	struct sk_buff *skb;
+
+	if (netif_msg_hw(priv) && net_ratelimit())
+		netdev_warn(net_dev, "FD status = 0x%08x\n",
+			    fd->status & FM_FD_STAT_TX_ERRORS);
	netif_warn(priv, hw, net_dev, etc...);

drivers/net/ethernet/freescale/dpaa/dpaa_eth.c line 364

Quoted 1 line
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +1 -0
+static int __cold dpa_eth_priv_stop(struct net_device *net_dev)

Use of __cold is pretty unusual in drivers

drivers/net/ethernet/freescale/dpaa/dpaa_eth.c line 479

Quoted 8 lines
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +8 -0
+static struct dpa_bp * __cold
+dpa_priv_bp_probe(struct device *dev)
+{
+	struct dpa_bp *dpa_bp;
+
+	dpa_bp = devm_kzalloc(dev, sizeof(*dpa_bp), GFP_KERNEL);
+	if (unlikely(!dpa_bp)) {
+		dev_err(dev, "devm_kzalloc() failed\n");

No need for this alloc failure message

drivers/net/ethernet/freescale/dpaa/dpaa_eth.c line 551

Quoted 9 lines
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +9 -0
+static int dpa_priv_bp_create(struct net_device *net_dev, struct dpa_bp *dpa_bp,
+			      size_t count)
+{
+	struct dpa_priv_s *priv = netdev_priv(net_dev);
+	int i;
+
+	if (netif_msg_probe(priv))
+		dev_dbg(net_dev->dev.parent,
+			"Using private BM buffer pools\n");

Why emit using dev.parent?

drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.h line 65

Quoted 5 lines
drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.h +3 -0
 
+int __cold dpa_start(struct net_device *net_dev);
+int __cold dpa_stop(struct net_device *net_dev);
+void __cold dpa_timeout(struct net_device *net_dev);

Marking a function prototype with an attribute means the implementation doesn't need the same attribute.

Quoted 4 lines
drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.h +4 -0
+/* Convenience macros for storing/retrieving the skb back-pointers.
+ *
+ * NB: @off is an offset from a (struct sk_buff **) pointer!
+ */

drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.h

Quoted 10 lines
drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.h +10 -0
+#define DPA_WRITE_SKB_PTR(skb, skbh, addr, off) \
+	{ \
+		skbh = (struct sk_buff **)addr; \
+		*(skbh + (off)) = skb; \
+	}
+#define DPA_READ_SKB_PTR(skb, skbh, addr, off) \
+	{ \
+		skbh = (struct sk_buff **)addr; \
+		skb = *(skbh + (off)); \
+	}

Maybe these are better as static inlines?

#15 lore raw
In reply to Madalin Bucur

Just a few nits.

This series is posted as an RFC, so this might not be what you're expecting right now. But as these messages got tangled up in my mail filter anyhow, I thought I might as well bother you with these nits now.

Quoted 9 lines · Madalin Bucur
drivers/net/ethernet/freescale/dpaa/Kconfig new file +6 -0
@@ -0,0 +1,49 @@
+menuconfig FSL_DPAA_ETH
+	tristate "DPAA Ethernet"
+	depends on FSL_SOC && FSL_BMAN && FSL_QMAN && FSL_FMAN
+	select PHYLIB
+	select FSL_FMAN_MAC
+	---help---

drivers/net/ethernet/freescale/dpaa/Kconfig line 16

Quoted 10 lines
drivers/net/ethernet/freescale/dpaa/Kconfig new file +10 -0
+	  Data Path Acceleration Architecture Ethernet driver,
+	  supporting the Freescale QorIQ chips.
+	  Depends on Freescale Buffer Manager and Queue Manager
+	  driver and Frame Manager Driver.
+
+if FSL_DPAA_ETH
+
+config FSL_DPAA_CS_THRESHOLD_1G
+	hex "Egress congestion threshold on 1G ports"
+	depends on FSL_DPAA_ETH

This entry is inside the "if FSL_DPAA_ETH" block. So this line should be superfluous.

Quoted 9 lines
drivers/net/ethernet/freescale/dpaa/Kconfig new file +9 -0
+	range 0x1000 0x10000000
+	default "0x06000000"
+	---help---
+	  The size in bytes of the egress Congestion State notification threshold on 1G ports.
+	  The 1G dTSECs can quite easily be flooded by cores doing Tx in a tight loop
+	  (e.g. by sending UDP datagrams at "while(1) speed"),
+	  and the larger the frame size, the more acute the problem.
+	  So we have to find a balance between these factors:
+	       - avoiding the device staying congested for a prolonged time (risking

drivers/net/ethernet/freescale/dpaa/Kconfig line 35

Quoted 10 lines
drivers/net/ethernet/freescale/dpaa/Kconfig new file +10 -0
+                 the netdev watchdog to fire - see also the tx_timeout module param);
+               - affecting performance of protocols such as TCP, which otherwise
+	         behave well under the congestion notification mechanism;
+	       - preventing the Tx cores from tightly-looping (as if the congestion
+	         threshold was too low to be effective);
+	       - running out of memory if the CS threshold is set too high.
+
+config FSL_DPAA_CS_THRESHOLD_10G
+	hex "Egress congestion threshold on 10G ports"
+	depends on FSL_DPAA_ETH

Ditto.

drivers/net/ethernet/freescale/dpaa/Kconfig line 43

Quoted 8 lines
drivers/net/ethernet/freescale/dpaa/Kconfig new file +8 -0
+	range 0x1000 0x20000000
+	default "0x10000000"
+	---help ---
+	  The size in bytes of the egress Congestion State notification threshold on 10G ports.
+
+config FSL_DPAA_INGRESS_CS_THRESHOLD
+	hex "Ingress congestion threshold on FMan ports"
+	depends on FSL_DPAA_ETH

Ditto.

drivers/net/ethernet/freescale/dpaa/Kconfig line 49

Quoted 6 lines
drivers/net/ethernet/freescale/dpaa/Kconfig new file +6 -0
+	default "0x10000000"
+	---help---
+	  The size in bytes of the ingress tail-drop threshold on FMan ports.
+	  Traffic piling up above this value will be rejected by QMan and discarded by FMan.
+
+endif # FSL_DPAA_ETH

A similar comment can be made for the entries added in 03/10, 05/10, and 09/10.

Thanks,

Paul Bolle

#16 lore raw
Quoted 24 lines

-----Original Message-----
From: Paul Bolle [mailto:[email protected]]

Just a few nits.

This series is posted as an RFC, so this might not be what you're expecting right now. But as these messages got tangled up in my mail filter anyhow, I thought I might as well bother you with these nits now.

Quoted 14 lines · Madalin Bucur
drivers/net/ethernet/freescale/dpaa/Kconfig new file +11 -0
@@ -0,0 +1,49 @@
+menuconfig FSL_DPAA_ETH
+	tristate "DPAA Ethernet"
+	depends on FSL_SOC && FSL_BMAN && FSL_QMAN && FSL_FMAN
+	select PHYLIB
+	select FSL_FMAN_MAC
+	---help---
+	  Data Path Acceleration Architecture Ethernet driver,
+	  supporting the Freescale QorIQ chips.
+	  Depends on Freescale Buffer Manager and Queue Manager
+	  driver and Frame Manager Driver.
+
Quoted 8 lines

drivers/net/ethernet/freescale/dpaa/Kconfig line 16

Quoted 5 lines
drivers/net/ethernet/freescale/dpaa/Kconfig new file +5 -0
+if FSL_DPAA_ETH
+
+config FSL_DPAA_CS_THRESHOLD_1G
+	hex "Egress congestion threshold on 1G ports"
+	depends on FSL_DPAA_ETH

This entry is inside the "if FSL_DPAA_ETH" block. So this line should be superfluous.

Will address this and the other occurrences.

Quoted 50 lines

drivers/net/ethernet/freescale/dpaa/Kconfig

Quoted 4 lines
drivers/net/ethernet/freescale/dpaa/Kconfig new file +4 -0
+	range 0x1000 0x10000000
+	default "0x06000000"
+	---help---
+	  The size in bytes of the egress Congestion State notification

threshold on 1G ports.

drivers/net/ethernet/freescale/dpaa/Kconfig line 21

Quoted 1 line
drivers/net/ethernet/freescale/dpaa/Kconfig new file +1 -0
+	  The 1G dTSECs can quite easily be flooded by cores doing Tx in a

tight loop

drivers/net/ethernet/freescale/dpaa/Kconfig

Quoted 4 lines
drivers/net/ethernet/freescale/dpaa/Kconfig new file +4 -0
+	  (e.g. by sending UDP datagrams at "while(1) speed"),
+	  and the larger the frame size, the more acute the problem.
+	  So we have to find a balance between these factors:
+	       - avoiding the device staying congested for a prolonged time

(risking

drivers/net/ethernet/freescale/dpaa/Kconfig line 26

Quoted 1 line
drivers/net/ethernet/freescale/dpaa/Kconfig new file +1 -0
+                 the netdev watchdog to fire - see also the tx_timeout module

param);

drivers/net/ethernet/freescale/dpaa/Kconfig

Quoted 3 lines
drivers/net/ethernet/freescale/dpaa/Kconfig new file +3 -0
+               - affecting performance of protocols such as TCP, which otherwise
+	         behave well under the congestion notification mechanism;
+	       - preventing the Tx cores from tightly-looping (as if the

congestion

drivers/net/ethernet/freescale/dpaa/Kconfig line 35

Quoted 6 lines
drivers/net/ethernet/freescale/dpaa/Kconfig new file +6 -0
+	         threshold was too low to be effective);
+	       - running out of memory if the CS threshold is set too high.
+
+config FSL_DPAA_CS_THRESHOLD_10G
+	hex "Egress congestion threshold on 10G ports"
+	depends on FSL_DPAA_ETH

Ditto.

drivers/net/ethernet/freescale/dpaa/Kconfig

Quoted 4 lines
drivers/net/ethernet/freescale/dpaa/Kconfig new file +4 -0
+	range 0x1000 0x20000000
+	default "0x10000000"
+	---help ---
+	  The size in bytes of the egress Congestion State notification

threshold on 10G ports.

drivers/net/ethernet/freescale/dpaa/Kconfig line 43

Quoted 4 lines
drivers/net/ethernet/freescale/dpaa/Kconfig new file +4 -0
+
+config FSL_DPAA_INGRESS_CS_THRESHOLD
+	hex "Ingress congestion threshold on FMan ports"
+	depends on FSL_DPAA_ETH

Ditto.

drivers/net/ethernet/freescale/dpaa/Kconfig

Quoted 4 lines
drivers/net/ethernet/freescale/dpaa/Kconfig new file +4 -0
+	default "0x10000000"
+	---help---
+	  The size in bytes of the ingress tail-drop threshold on FMan ports.
+	  Traffic piling up above this value will be rejected by QMan and

discarded by FMan.

drivers/net/ethernet/freescale/dpaa/Kconfig line 49

Quoted 2 lines
drivers/net/ethernet/freescale/dpaa/Kconfig new file +2 -0
+
+endif # FSL_DPAA_ETH

A similar comment can be made for the entries added in 03/10, 05/10, and 09/10.

Quoted 4 lines

Thanks,

Paul Bolle

Thank you,
Madalin

#17 lore raw
In reply to Madalin Bucur
Quoted 7 lines · Madalin Bucur

This introduces the Freescale Data Path Acceleration Architecture (DPAA) Ethernet driver (dpaa_eth) that builds upon the DPAA QMan, BMan, PAMU and FMan drivers to deliver Ethernet connectivity on the Freescale DPAA QorIQ platforms.

---

....

drivers/net/ethernet/freescale/dpaa/dpaa_eth.c line 611

Quoted 9 lines
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c new file +9 -0
+	net_dev = alloc_etherdev_mq(sizeof(*priv), DPAA_ETH_TX_QUEUES);
+	if (!net_dev) {
+		dev_err(dev, "alloc_etherdev_mq() failed\n");
+		goto alloc_etherdev_mq_failed;
+	}
+
+	snprintf(net_dev->name, IFNAMSIZ, "fm%d-mac%d",
+		 dpa_mac_fman_index_get(pdev),
+		 dpa_mac_hw_index_get(pdev));

Should ethernet drivers dictate interface name in user space nowadays? I would prefer if you didn't.

I am trying these patches on a custom T1042 board using Linux 4.1 but I cannot get Fixed PHY to work:
libphy: PHY fixed-0:00 not found
fsl_dpa dpaa-ethernet.2 eth2: Could not connect to PHY fixed-0:00 fsl_dpa dpaa-ethernet.2 eth2: init_phy() = -19

Not sure what I have missed here, any ideas?

Jocke

PS.
I would really appreciate if you could add new revisions of the FMAN/DPAA driver(s) to http://git.freescale.com/git/cgit.cgi/ppc/upstream/linux.git/

#18 lore raw
Quoted 22 lines · Joakim Tjernlund
Quoted 7 lines · Madalin Bucur

This introduces the Freescale Data Path Acceleration Architecture (DPAA) Ethernet driver (dpaa_eth) that builds upon the DPAA QMan, BMan, PAMU and FMan drivers to deliver Ethernet connectivity on the Freescale DPAA QorIQ platforms.

---

....

drivers/net/ethernet/freescale/dpaa/dpaa_eth.c line 611

Quoted 9 lines
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c new file +9 -0
+	net_dev = alloc_etherdev_mq(sizeof(*priv), DPAA_ETH_TX_QUEUES);
+	if (!net_dev) {
+		dev_err(dev, "alloc_etherdev_mq() failed\n");
+		goto alloc_etherdev_mq_failed;
+	}
+
+	snprintf(net_dev->name, IFNAMSIZ, "fm%d-mac%d",
+		 dpa_mac_fman_index_get(pdev),
+		 dpa_mac_hw_index_get(pdev));

Should ethernet drivers dictate interface name in user space nowadays? I would prefer if you didn't.

Quoted 7 lines

I am trying these patches on a custom T1042 board using Linux 4.1 but I cannot get Fixed PHY to work:
libphy: PHY fixed-0:00 not found
fsl_dpa dpaa-ethernet.2 eth2: Could not connect to PHY fixed-0:00 fsl_dpa dpaa-ethernet.2 eth2: init_phy() = -19

Not sure what I have missed here, any ideas?

I meant I am using http://git.freescale.com/git/cgit.cgi/ppc/upstream/linux.git/
on top of 4.1

Jocke

#19 lore raw

Hi Joakim

Quoted 8 lines

-----Original Message-----
From: Joakim Tjernlund [mailto:[email protected]]
Sent: Monday, July 20, 2015 10:57 AM
To: [email protected]; [email protected]; Bucur Madalin- Cristian-B32716
Cc: [email protected]
Subject: Re: [PATCH RFC 02/10] dpaa_eth: add support for DPAA Ethernet

Quoted 16 lines
Quoted 15 lines · Joakim Tjernlund
Quoted 7 lines · Madalin Bucur

This introduces the Freescale Data Path Acceleration Architecture (DPAA) Ethernet driver (dpaa_eth) that builds upon the DPAA QMan, BMan, PAMU and FMan drivers to deliver Ethernet connectivity on the Freescale DPAA QorIQ platforms.

---

....

drivers/net/ethernet/freescale/dpaa/dpaa_eth.c line 611

Quoted 3 lines
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c new file +3 -0
+	snprintf(net_dev->name, IFNAMSIZ, "fm%d-mac%d",
+		 dpa_mac_fman_index_get(pdev),
+		 dpa_mac_hw_index_get(pdev));

Should ethernet drivers dictate interface name in user space nowadays? I would prefer if you didn't.

The preformatted interface name was thought as a helper for quick interface identification. It also ensures constant naming of the interfaces, i.e. if you add/remove PCI network cards. One can make use of udev rules to override default interface names ("eth%d") in userspace.

Another reason for using this is that the interface name was also used for the debugfs file name and when compiling dpaa_eth as a module there was a problem with udev concurrently renaming interfaces from eth0 to something like fmx-macy, making the next probed DPAA interface temporarily get the eth0 name (before being renamed fmx-macw). Subsequently,
the debugfs_create_file(net_dev->name,...) call failed because of duplicated names.

If this is considered more of a bug than a feature, I can remove it and only change the naming of the debugfs entries to avoid the udev issue.

Quoted 6 lines
Quoted 6 lines

I am trying these patches on a custom T1042 board using Linux 4.1 but I cannot get Fixed PHY to work:
libphy: PHY fixed-0:00 not found
fsl_dpa dpaa-ethernet.2 eth2: Could not connect to PHY fixed-0:00 fsl_dpa dpaa-ethernet.2 eth2: init_phy() = -19

Quoted 7 lines
Quoted 1 line

Not sure what I have missed here, any ideas?

I meant I am using
http://git.freescale.com/git/cgit.cgi/ppc/upstream/linux.git/
on top of 4.1

Jocke

Please make sure you have CONFIG_FIXED_PHY=y in your .config.
Can you please share the device tree part where you've added the fixed-link entry?

Madalin

#20 lore raw
Quoted 27 lines · Madalin-Cristian Bucur

Hi Joakim

Quoted 8 lines

-----Original Message-----
From: Joakim Tjernlund [mailto:[email protected]]
Sent: Monday, July 20, 2015 10:57 AM
To: [email protected]; [email protected]; Bucur Madalin- Cristian-B32716
Cc: [email protected]
Subject: Re: [PATCH RFC 02/10] dpaa_eth: add support for DPAA Ethernet

Quoted 16 lines
Quoted 15 lines · Joakim Tjernlund
Quoted 7 lines · Madalin Bucur

This introduces the Freescale Data Path Acceleration Architecture (DPAA) Ethernet driver (dpaa_eth) that builds upon the DPAA QMan, BMan, PAMU and FMan drivers to deliver Ethernet connectivity on the Freescale DPAA QorIQ platforms.

---

....

drivers/net/ethernet/freescale/dpaa/dpaa_eth.c line 611

Quoted 3 lines
drivers/net/ethernet/freescale/dpaa/dpaa_eth.c new file +3 -0
+	snprintf(net_dev->name, IFNAMSIZ, "fm%d-mac%d",
+		 dpa_mac_fman_index_get(pdev),
+		 dpa_mac_hw_index_get(pdev));

Should ethernet drivers dictate interface name in user space nowadays? I would prefer if you didn't.

Quoted 4 lines

The preformatted interface name was thought as a helper for quick interface identification. It also ensures constant naming of the interfaces, i.e. if you add/remove PCI network cards. One can make use of udev rules to override default interface names ("eth%d") in userspace.

I am not well versed in ethernet interface naming but I think you are just creating another name space here. To me one either have the old eth%d names or let udev do the naming so there is some order w.r.t interface naming.

I also find it handy to be able to control which I/F is eth0 etc. just by changing the order they are listed in the device tree. This makes our application happy :)

Quoted 11 lines

Another reason for using this is that the interface name was also used for the debugfs file name and when compiling dpaa_eth as a module there was a problem with udev concurrently renaming interfaces from eth0 to something like fmx-macy, making the next probed DPAA interface temporarily get the eth0 name (before being renamed fmx-macw). Subsequently,
the debugfs_create_file(net_dev->name,...) call failed because of duplicated names.

If this is considered more of a bug than a feature, I can remove it and only change the naming of the debugfs entries to avoid the udev issue.

Perhaps netdev list can advise?

Jocke