[net-next PATCH 0/6] enic: updates to version 1.1.0.241a

[net-next PATCH 0/6] enic: updates to version 1.1.0.241a

[Scott Feldman]

Resending with updates to address issues raised by Ben Hutchings and
Simon Horman.

Please apply to net-next.

1/6 - Bug fix: use safe queue shutdown in dev->stop
2/6 - Bug fix: try harder to fill Rx ring on skb allocation failures
3/6 - minimize pkt filter updates to firmware
4/6 - Bug fix: align desc ring sizes to 32 descs
5/6 - feature add: add ethtool -c/C support
6/6 - whitespace cleanup; #define cleanup; more verbose err msg

Signed-off-by: Vasanthy Kolluri <vkolluri@cisco.com>
Signed-off-by: Scott Feldman <scofeldm@cisco.com>

[net-next PATCH 1/6] enic: Bug fix: use safe queue shutdown in dev->stop

[Scott Feldman]

From: Scott Feldman <scofeldm@cisco.com>

Fix dev->stop shutdown bug where driver was stopping xmit queue and then
disabling intrs.  Fix is to disable intrs first and then stop the xmit
queue, otherwise an interrupt could cause the queue to be rewoken.  Also,
no need to explicitly do queue servicing because queues are cleaned and
reset back to initial state at end of dev->stop.  Servicing queues also
had the side-effect of also rewakening the xmit queue, which is not what
we want.

Signed-off-by: Vasanthy Kolluri <vkolluri@cisco.com>
Signed-off-by: Scott Feldman <scofeldm@cisco.com>
---
 drivers/net/enic/enic.h      |    2 +
 drivers/net/enic/enic_main.c |   62 +++++++++++++++++-------------------------
 drivers/net/enic/vnic_intr.h |    1 +
 3 files changed, 27 insertions(+), 38 deletions(-)

diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
index e1c2076..8dd0105 100644
--- a/drivers/net/enic/enic.h
+++ b/drivers/net/enic/enic.h
@@ -34,7 +34,7 @@
 
 #define DRV_NAME		"enic"
 #define DRV_DESCRIPTION		"Cisco 10G Ethernet Driver"
-#define DRV_VERSION		"1.1.0.100"
+#define DRV_VERSION		"1.1.0.241a"
 #define DRV_COPYRIGHT		"Copyright 2008-2009 Cisco Systems, Inc"
 #define PFX			DRV_NAME ": "
 
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index f875751..b4a11be 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -1084,34 +1084,6 @@ static int enic_rq_service(struct vnic_dev *vdev, struct cq_desc *cq_desc,
 	return 0;
 }
 
-static void enic_rq_drop_buf(struct vnic_rq *rq,
-	struct cq_desc *cq_desc, struct vnic_rq_buf *buf,
-	int skipped, void *opaque)
-{
-	struct enic *enic = vnic_dev_priv(rq->vdev);
-	struct sk_buff *skb = buf->os_buf;
-
-	if (skipped)
-		return;
-
-	pci_unmap_single(enic->pdev, buf->dma_addr,
-		buf->len, PCI_DMA_FROMDEVICE);
-
-	dev_kfree_skb_any(skb);
-}
-
-static int enic_rq_service_drop(struct vnic_dev *vdev, struct cq_desc *cq_desc,
-	u8 type, u16 q_number, u16 completed_index, void *opaque)
-{
-	struct enic *enic = vnic_dev_priv(vdev);
-
-	vnic_rq_service(&enic->rq[q_number], cq_desc,
-		completed_index, VNIC_RQ_RETURN_DESC,
-		enic_rq_drop_buf, opaque);
-
-	return 0;
-}
-
 static int enic_poll(struct napi_struct *napi, int budget)
 {
 	struct enic *enic = container_of(napi, struct enic, napi);
@@ -1304,6 +1276,24 @@ static int enic_request_intr(struct enic *enic)
 	return err;
 }
 
+static void enic_synchronize_irqs(struct enic *enic)
+{
+	unsigned int i;
+
+	switch (vnic_dev_get_intr_mode(enic->vdev)) {
+	case VNIC_DEV_INTR_MODE_INTX:
+	case VNIC_DEV_INTR_MODE_MSI:
+		synchronize_irq(enic->pdev->irq);
+		break;
+	case VNIC_DEV_INTR_MODE_MSIX:
+		for (i = 0; i < enic->intr_count; i++)
+			synchronize_irq(enic->msix_entry[i].vector);
+		break;
+	default:
+		break;
+	}
+}
+
 static int enic_notify_set(struct enic *enic)
 {
 	int err;
@@ -1409,16 +1399,19 @@ static int enic_stop(struct net_device *netdev)
 	unsigned int i;
 	int err;
 
+	for (i = 0; i < enic->intr_count; i++)
+		vnic_intr_mask(&enic->intr[i]);
+
+	enic_synchronize_irqs(enic);
+
 	del_timer_sync(&enic->notify_timer);
 
 	spin_lock(&enic->devcmd_lock);
 	vnic_dev_disable(enic->vdev);
 	spin_unlock(&enic->devcmd_lock);
 	napi_disable(&enic->napi);
-	netif_stop_queue(netdev);
-
-	for (i = 0; i < enic->intr_count; i++)
-		vnic_intr_mask(&enic->intr[i]);
+	netif_carrier_off(netdev);
+	netif_tx_disable(netdev);
 
 	for (i = 0; i < enic->wq_count; i++) {
 		err = vnic_wq_disable(&enic->wq[i]);
@@ -1436,11 +1429,6 @@ static int enic_stop(struct net_device *netdev)
 	spin_unlock(&enic->devcmd_lock);
 	enic_free_intr(enic);
 
-	(void)vnic_cq_service(&enic->cq[ENIC_CQ_RQ],
-		-1, enic_rq_service_drop, NULL);
-	(void)vnic_cq_service(&enic->cq[ENIC_CQ_WQ],
-		-1, enic_wq_service, NULL);
-
 	for (i = 0; i < enic->wq_count; i++)
 		vnic_wq_clean(&enic->wq[i], enic_free_wq_buf);
 	for (i = 0; i < enic->rq_count; i++)
diff --git a/drivers/net/enic/vnic_intr.h b/drivers/net/enic/vnic_intr.h
index 9a53604..f79a722 100644
--- a/drivers/net/enic/vnic_intr.h
+++ b/drivers/net/enic/vnic_intr.h
@@ -61,6 +61,7 @@ static inline void vnic_intr_unmask(struct vnic_intr *intr)
 static inline void vnic_intr_mask(struct vnic_intr *intr)
 {
 	iowrite32(1, &intr->ctrl->mask);
+	(void)ioread32(&intr->ctrl->mask);
 }
 
 static inline void vnic_intr_return_credits(struct vnic_intr *intr,

[net-next PATCH 2/6] enic: Bug fix: try harder to fill Rx ring on skb allocation failures

[Scott Feldman]

From: Scott Feldman <scofeldm@cisco.com>

During dev->open(), make sure we get at least one skb on the Rx ring. 
Otherwise abort the interface load.  Also, if we get skb allocation
failures in NAPI poll while trying to replenish the ring, try again
later so we don't end up starving out the Rx ring completely.

Signed-off-by: Vasanthy Kolluri <vkolluri@cisco.com>
Signed-off-by: Scott Feldman <scofeldm@cisco.com>
---
 drivers/net/enic/enic_main.c |   54 ++++++++++++++++++++++++++----------------
 1 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index b4a11be..452a6b7 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -1091,6 +1091,7 @@ static int enic_poll(struct napi_struct *napi, int budget)
 	unsigned int rq_work_to_do = budget;
 	unsigned int wq_work_to_do = -1; /* no limit */
 	unsigned int  work_done, rq_work_done, wq_work_done;
+	int err;
 
 	/* Service RQ (first) and WQ
 	 */
@@ -1114,16 +1115,19 @@ static int enic_poll(struct napi_struct *napi, int budget)
 			0 /* don't unmask intr */,
 			0 /* don't reset intr timer */);
 
-	if (rq_work_done > 0) {
+	err = vnic_rq_fill(&enic->rq[0], enic->rq_alloc_buf);
 
-		/* Replenish RQ
-		 */
+	/* Buffer allocation failed. Stay in polling
+	 * mode so we can try to fill the ring again.
+	 */
 
-		vnic_rq_fill(&enic->rq[0], enic->rq_alloc_buf);
+	if (err)
+		rq_work_done = rq_work_to_do;
 
-	} else {
+	if (rq_work_done < rq_work_to_do) {
 
-		/* If no work done, flush all LROs and exit polling
+		/* Some work done, but not enough to stay in polling,
+		 * flush all LROs and exit polling
 		 */
 
 		if (netdev->features & NETIF_F_LRO)
@@ -1142,6 +1146,7 @@ static int enic_poll_msix(struct napi_struct *napi, int budget)
 	struct net_device *netdev = enic->netdev;
 	unsigned int work_to_do = budget;
 	unsigned int work_done;
+	int err;
 
 	/* Service RQ
 	 */
@@ -1149,25 +1154,30 @@ static int enic_poll_msix(struct napi_struct *napi, int budget)
 	work_done = vnic_cq_service(&enic->cq[ENIC_CQ_RQ],
 		work_to_do, enic_rq_service, NULL);
 
-	if (work_done > 0) {
-
-		/* Replenish RQ
-		 */
-
-		vnic_rq_fill(&enic->rq[0], enic->rq_alloc_buf);
-
-		/* Return intr event credits for this polling
-		 * cycle.  An intr event is the completion of a
-		 * RQ packet.
-		 */
+	/* Return intr event credits for this polling
+	 * cycle.  An intr event is the completion of a
+	 * RQ packet.
+	 */
 
+	if (work_done > 0)
 		vnic_intr_return_credits(&enic->intr[ENIC_MSIX_RQ],
 			work_done,
 			0 /* don't unmask intr */,
 			0 /* don't reset intr timer */);
-	} else {
 
-		/* If no work done, flush all LROs and exit polling
+	err = vnic_rq_fill(&enic->rq[0], enic->rq_alloc_buf);
+
+	/* Buffer allocation failed. Stay in polling mode
+	 * so we can try to fill the ring again.
+	 */
+
+	if (err)
+		work_done = work_to_do;
+
+	if (work_done < work_to_do) {
+
+		/* Some work done, but not enough to stay in polling,
+		 * flush all LROs and exit polling
 		 */
 
 		if (netdev->features & NETIF_F_LRO)
@@ -1350,11 +1360,13 @@ static int enic_open(struct net_device *netdev)
 	}
 
 	for (i = 0; i < enic->rq_count; i++) {
-		err = vnic_rq_fill(&enic->rq[i], enic->rq_alloc_buf);
-		if (err) {
+		vnic_rq_fill(&enic->rq[i], enic->rq_alloc_buf);
+		/* Need at least one buffer on ring to get going */
+		if (vnic_rq_desc_used(&enic->rq[i]) == 0) {
 			printk(KERN_ERR PFX
 				"%s: Unable to alloc receive buffers.\n",
 				netdev->name);
+			err = -ENOMEM;
 			goto err_out_notify_unset;
 		}
 	}

[net-next PATCH 3/6] enic: minimize pkt filter updates to firmware

[Scott Feldman]

From: Scott Feldman <scofeldm@cisco.com>

In set_multicast(), only push pkt filter changes down to firmware if
pkt filter actually changes.

Signed-off-by: Vasanthy Kolluri <vkolluri@cisco.com>
Signed-off-by: Scott Feldman <scofeldm@cisco.com>
---
 drivers/net/enic/enic.h      |    1 +
 drivers/net/enic/enic_main.c |    8 ++++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
index 8dd0105..b090d65 100644
--- a/drivers/net/enic/enic.h
+++ b/drivers/net/enic/enic.h
@@ -89,6 +89,7 @@ struct enic {
 	spinlock_t devcmd_lock;
 	u8 mac_addr[ETH_ALEN];
 	u8 mc_addr[ENIC_MULTICAST_PERFECT_FILTERS][ETH_ALEN];
+	unsigned int flags;
 	unsigned int mc_count;
 	int csum_rx_enabled;
 	u32 port_mtu;
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 452a6b7..019b148 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -771,6 +771,7 @@ static void enic_set_multicast_list(struct net_device *netdev)
 	int promisc = (netdev->flags & IFF_PROMISC) ? 1 : 0;
 	int allmulti = (netdev->flags & IFF_ALLMULTI) ||
 	    (netdev->mc_count > ENIC_MULTICAST_PERFECT_FILTERS);
+	unsigned int flags = netdev->flags | (allmulti ? IFF_ALLMULTI : 0);
 	u8 mc_addr[ENIC_MULTICAST_PERFECT_FILTERS][ETH_ALEN];
 	unsigned int mc_count = netdev->mc_count;
 	unsigned int i, j;
@@ -780,8 +781,11 @@ static void enic_set_multicast_list(struct net_device *netdev)
 
 	spin_lock(&enic->devcmd_lock);
 
-	vnic_dev_packet_filter(enic->vdev, directed,
-		multicast, broadcast, promisc, allmulti);
+	if (enic->flags != flags) {
+		enic->flags = flags;
+		vnic_dev_packet_filter(enic->vdev, directed,
+			multicast, broadcast, promisc, allmulti);
+	}
 
 	/* Is there an easier way?  Trying to minimize to
 	 * calls to add/del multicast addrs.  We keep the

[net-next PATCH 4/6] enic: Bug fix: align desc ring sizes to 32 descs

[Scott Feldman]

From: Scott Feldman <scofeldm@cisco.com>

Previous driver was aligning ring sizes to 16 descs, but hardware actually
wants desc ring sizes to be aligned to 32 descs.

Signed-off-by: Vasanthy Kolluri <vkolluri@cisco.com>
Signed-off-by: Scott Feldman <scofeldm@cisco.com>
---
 drivers/net/enic/enic_res.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/enic/enic_res.c b/drivers/net/enic/enic_res.c
index 3211114..a605da1 100644
--- a/drivers/net/enic/enic_res.c
+++ b/drivers/net/enic/enic_res.c
@@ -74,13 +74,13 @@ int enic_get_vnic_config(struct enic *enic)
 		min_t(u32, ENIC_MAX_WQ_DESCS,
 		max_t(u32, ENIC_MIN_WQ_DESCS,
 		c->wq_desc_count));
-	c->wq_desc_count &= 0xfffffff0; /* must be aligned to groups of 16 */
+	c->wq_desc_count &= 0xffffffe0; /* must be aligned to groups of 32 */
 
 	c->rq_desc_count =
 		min_t(u32, ENIC_MAX_RQ_DESCS,
 		max_t(u32, ENIC_MIN_RQ_DESCS,
 		c->rq_desc_count));
-	c->rq_desc_count &= 0xfffffff0; /* must be aligned to groups of 16 */
+	c->rq_desc_count &= 0xffffffe0; /* must be aligned to groups of 32 */
 
 	if (c->mtu == 0)
 		c->mtu = 1500;

[net-next PATCH 5/6] enic: feature add: add ethtool -c/C support

[Scott Feldman]

From: Scott Feldman <scofeldm@cisco.com>

Only rx_usec and tx_usec options for ethtool -C are settable as those
are the only settings that make sense to HW.  Adds driver reporting of 
intr coalescing timer value in usec units rather than HW units.

Signed-off-by: Vasanthy Kolluri <vkolluri@cisco.com>
Signed-off-by: Scott Feldman <scofeldm@cisco.com>
---
 drivers/net/enic/enic.h      |    2 +
 drivers/net/enic/enic_main.c |   65 +++++++++++++++++++++++++++++++++++++++++-
 drivers/net/enic/enic_res.c  |   12 +++++---
 drivers/net/enic/vnic_enet.h |    5 +++
 drivers/net/enic/vnic_intr.c |    8 +++++
 drivers/net/enic/vnic_intr.h |    2 +
 6 files changed, 86 insertions(+), 8 deletions(-)

diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
index b090d65..ee01f5a 100644
--- a/drivers/net/enic/enic.h
+++ b/drivers/net/enic/enic.h
@@ -93,6 +93,8 @@ struct enic {
 	unsigned int mc_count;
 	int csum_rx_enabled;
 	u32 port_mtu;
+	u32 rx_coalesce_usecs;
+	u32 tx_coalesce_usecs;
 
 	/* work queue cache line section */
 	____cacheline_aligned struct vnic_wq wq[ENIC_WQ_MAX];
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 019b148..81cc68d 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -261,7 +261,63 @@ static void enic_set_msglevel(struct net_device *netdev, u32 value)
 	enic->msg_enable = value;
 }
 
-static const struct ethtool_ops enic_ethtool_ops = {
+static int enic_get_coalesce(struct net_device *netdev,
+	struct ethtool_coalesce *ecmd)
+{
+	struct enic *enic = netdev_priv(netdev);
+
+	ecmd->tx_coalesce_usecs = enic->tx_coalesce_usecs;
+	ecmd->rx_coalesce_usecs = enic->rx_coalesce_usecs;
+
+	return 0;
+}
+
+static int enic_set_coalesce(struct net_device *netdev,
+	struct ethtool_coalesce *ecmd)
+{
+	struct enic *enic = netdev_priv(netdev);
+	u32 tx_coalesce_usecs;
+	u32 rx_coalesce_usecs;
+
+	tx_coalesce_usecs = min_t(u32,
+		INTR_COALESCE_HW_TO_USEC(VNIC_INTR_TIMER_MAX),
+		ecmd->tx_coalesce_usecs);
+	rx_coalesce_usecs = min_t(u32,
+		INTR_COALESCE_HW_TO_USEC(VNIC_INTR_TIMER_MAX),
+		ecmd->rx_coalesce_usecs);
+
+	switch (vnic_dev_get_intr_mode(enic->vdev)) {
+	case VNIC_DEV_INTR_MODE_INTX:
+		if (tx_coalesce_usecs != rx_coalesce_usecs)
+			return -EINVAL;
+
+		vnic_intr_coalescing_timer_set(&enic->intr[ENIC_INTX_WQ_RQ],
+			INTR_COALESCE_USEC_TO_HW(tx_coalesce_usecs));
+		break;
+	case VNIC_DEV_INTR_MODE_MSI:
+		if (tx_coalesce_usecs != rx_coalesce_usecs)
+			return -EINVAL;
+
+		vnic_intr_coalescing_timer_set(&enic->intr[0],
+			INTR_COALESCE_USEC_TO_HW(tx_coalesce_usecs));
+		break;
+	case VNIC_DEV_INTR_MODE_MSIX:
+		vnic_intr_coalescing_timer_set(&enic->intr[ENIC_MSIX_WQ],
+			INTR_COALESCE_USEC_TO_HW(tx_coalesce_usecs));
+		vnic_intr_coalescing_timer_set(&enic->intr[ENIC_MSIX_RQ],
+			INTR_COALESCE_USEC_TO_HW(rx_coalesce_usecs));
+		break;
+	default:
+		break;
+	}
+
+	enic->tx_coalesce_usecs = tx_coalesce_usecs;
+	enic->rx_coalesce_usecs = rx_coalesce_usecs;
+
+	return 0;
+}
+
+static struct ethtool_ops enic_ethtool_ops = {
 	.get_settings = enic_get_settings,
 	.get_drvinfo = enic_get_drvinfo,
 	.get_msglevel = enic_get_msglevel,
@@ -278,6 +334,8 @@ static const struct ethtool_ops enic_ethtool_ops = {
 	.set_sg = ethtool_op_set_sg,
 	.get_tso = ethtool_op_get_tso,
 	.set_tso = enic_set_tso,
+	.get_coalesce = enic_get_coalesce,
+	.set_coalesce = enic_set_coalesce,
 	.get_flags = ethtool_op_get_flags,
 	.set_flags = ethtool_op_set_flags,
 };
@@ -363,12 +421,12 @@ static void enic_mtu_check(struct enic *enic)
 	u32 mtu = vnic_dev_mtu(enic->vdev);
 
 	if (mtu && mtu != enic->port_mtu) {
+		enic->port_mtu = mtu;
 		if (mtu < enic->netdev->mtu)
 			printk(KERN_WARNING PFX
 				"%s: interface MTU (%d) set higher "
 				"than switch port MTU (%d)\n",
 				enic->netdev->name, enic->netdev->mtu, mtu);
-		enic->port_mtu = mtu;
 	}
 }
 
@@ -1990,6 +2048,9 @@ static int __devinit enic_probe(struct pci_dev *pdev,
 		goto err_out_dev_deinit;
 	}
 
+	enic->tx_coalesce_usecs = enic->config.intr_timer_usec;
+	enic->rx_coalesce_usecs = enic->tx_coalesce_usecs;
+
 	netdev->netdev_ops = &enic_netdev_ops;
 	netdev->watchdog_timeo = 2 * HZ;
 	netdev->ethtool_ops = &enic_ethtool_ops;
diff --git a/drivers/net/enic/enic_res.c b/drivers/net/enic/enic_res.c
index a605da1..02839bf 100644
--- a/drivers/net/enic/enic_res.c
+++ b/drivers/net/enic/enic_res.c
@@ -66,9 +66,9 @@ int enic_get_vnic_config(struct enic *enic)
 	GET_CONFIG(wq_desc_count);
 	GET_CONFIG(rq_desc_count);
 	GET_CONFIG(mtu);
-	GET_CONFIG(intr_timer);
 	GET_CONFIG(intr_timer_type);
 	GET_CONFIG(intr_mode);
+	GET_CONFIG(intr_timer_usec);
 
 	c->wq_desc_count =
 		min_t(u32, ENIC_MAX_WQ_DESCS,
@@ -88,15 +88,17 @@ int enic_get_vnic_config(struct enic *enic)
 		max_t(u16, ENIC_MIN_MTU,
 		c->mtu));
 
-	c->intr_timer = min_t(u16, VNIC_INTR_TIMER_MAX, c->intr_timer);
+	c->intr_timer_usec = min_t(u32,
+		INTR_COALESCE_HW_TO_USEC(VNIC_INTR_TIMER_MAX),
+		c->intr_timer_usec);
 
 	printk(KERN_INFO PFX "vNIC MAC addr %pM wq/rq %d/%d\n",
 		enic->mac_addr, c->wq_desc_count, c->rq_desc_count);
 	printk(KERN_INFO PFX "vNIC mtu %d csum tx/rx %d/%d tso/lro %d/%d "
-		"intr timer %d\n",
+		"intr timer %d usec\n",
 		c->mtu, ENIC_SETTING(enic, TXCSUM),
 		ENIC_SETTING(enic, RXCSUM), ENIC_SETTING(enic, TSO),
-		ENIC_SETTING(enic, LRO), c->intr_timer);
+		ENIC_SETTING(enic, LRO), c->intr_timer_usec);
 
 	return 0;
 }
@@ -303,7 +305,7 @@ void enic_init_vnic_resources(struct enic *enic)
 
 	for (i = 0; i < enic->intr_count; i++) {
 		vnic_intr_init(&enic->intr[i],
-			enic->config.intr_timer,
+			INTR_COALESCE_USEC_TO_HW(enic->config.intr_timer_usec),
 			enic->config.intr_timer_type,
 			mask_on_assertion);
 	}
diff --git a/drivers/net/enic/vnic_enet.h b/drivers/net/enic/vnic_enet.h
index 6332ac9..8eeb675 100644
--- a/drivers/net/enic/vnic_enet.h
+++ b/drivers/net/enic/vnic_enet.h
@@ -20,6 +20,10 @@
 #ifndef _VNIC_ENIC_H_
 #define _VNIC_ENIC_H_
 
+/* Hardware intr coalesce timer is in units of 1.5us */
+#define INTR_COALESCE_USEC_TO_HW(usec) ((usec) * 2/3)
+#define INTR_COALESCE_HW_TO_USEC(usec) ((usec) * 3/2)
+
 /* Device-specific region: enet configuration */
 struct vnic_enet_config {
 	u32 flags;
@@ -30,6 +34,7 @@ struct vnic_enet_config {
 	u8 intr_timer_type;
 	u8 intr_mode;
 	char devname[16];
+	u32 intr_timer_usec;
 };
 
 #define VENETF_TSO		0x1	/* TSO enabled */
diff --git a/drivers/net/enic/vnic_intr.c b/drivers/net/enic/vnic_intr.c
index 1f8786d..3934309 100644
--- a/drivers/net/enic/vnic_intr.c
+++ b/drivers/net/enic/vnic_intr.c
@@ -50,12 +50,18 @@ int vnic_intr_alloc(struct vnic_dev *vdev, struct vnic_intr *intr,
 void vnic_intr_init(struct vnic_intr *intr, unsigned int coalescing_timer,
 	unsigned int coalescing_type, unsigned int mask_on_assertion)
 {
-	iowrite32(coalescing_timer, &intr->ctrl->coalescing_timer);
+	vnic_intr_coalescing_timer_set(intr, coalescing_timer);
 	iowrite32(coalescing_type, &intr->ctrl->coalescing_type);
 	iowrite32(mask_on_assertion, &intr->ctrl->mask_on_assertion);
 	iowrite32(0, &intr->ctrl->int_credits);
 }
 
+void vnic_intr_coalescing_timer_set(struct vnic_intr *intr,
+	unsigned int coalescing_timer)
+{
+	iowrite32(coalescing_timer, &intr->ctrl->coalescing_timer);
+}
+
 void vnic_intr_clean(struct vnic_intr *intr)
 {
 	iowrite32(0, &intr->ctrl->int_credits);
diff --git a/drivers/net/enic/vnic_intr.h b/drivers/net/enic/vnic_intr.h
index f79a722..2fe6c63 100644
--- a/drivers/net/enic/vnic_intr.h
+++ b/drivers/net/enic/vnic_intr.h
@@ -102,6 +102,8 @@ int vnic_intr_alloc(struct vnic_dev *vdev, struct vnic_intr *intr,
 	unsigned int index);
 void vnic_intr_init(struct vnic_intr *intr, unsigned int coalescing_timer,
 	unsigned int coalescing_type, unsigned int mask_on_assertion);
+void vnic_intr_coalescing_timer_set(struct vnic_intr *intr,
+	unsigned int coalescing_timer);
 void vnic_intr_clean(struct vnic_intr *intr);
 
 #endif /* _VNIC_INTR_H_ */

Re: [net-next PATCH 5/6] enic: feature add: add ethtool -c/C support

[Simon Horman]

On Mon, Dec 21, 2009 at 06:21:47PM -0800, Scott Feldman wrote:
> From: Scott Feldman <scofeldm@cisco.com>
> 
> Only rx_usec and tx_usec options for ethtool -C are settable as those
> are the only settings that make sense to HW.  Adds driver reporting of 
> intr coalescing timer value in usec units rather than HW units.
> 
> Signed-off-by: Vasanthy Kolluri <vkolluri@cisco.com>
> Signed-off-by: Scott Feldman <scofeldm@cisco.com>

Reviewed-by: Simon Horman <horms@verge.net.au>

> ---
>  drivers/net/enic/enic.h      |    2 +
>  drivers/net/enic/enic_main.c |   65 +++++++++++++++++++++++++++++++++++++++++-
>  drivers/net/enic/enic_res.c  |   12 +++++---
>  drivers/net/enic/vnic_enet.h |    5 +++
>  drivers/net/enic/vnic_intr.c |    8 +++++
>  drivers/net/enic/vnic_intr.h |    2 +
>  6 files changed, 86 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
> index b090d65..ee01f5a 100644
> --- a/drivers/net/enic/enic.h
> +++ b/drivers/net/enic/enic.h
> @@ -93,6 +93,8 @@ struct enic {
>  	unsigned int mc_count;
>  	int csum_rx_enabled;
>  	u32 port_mtu;
> +	u32 rx_coalesce_usecs;
> +	u32 tx_coalesce_usecs;
>  
>  	/* work queue cache line section */
>  	____cacheline_aligned struct vnic_wq wq[ENIC_WQ_MAX];
> diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
> index 019b148..81cc68d 100644
> --- a/drivers/net/enic/enic_main.c
> +++ b/drivers/net/enic/enic_main.c
> @@ -261,7 +261,63 @@ static void enic_set_msglevel(struct net_device *netdev, u32 value)
>  	enic->msg_enable = value;
>  }
>  
> -static const struct ethtool_ops enic_ethtool_ops = {
> +static int enic_get_coalesce(struct net_device *netdev,
> +	struct ethtool_coalesce *ecmd)
> +{
> +	struct enic *enic = netdev_priv(netdev);
> +
> +	ecmd->tx_coalesce_usecs = enic->tx_coalesce_usecs;
> +	ecmd->rx_coalesce_usecs = enic->rx_coalesce_usecs;
> +
> +	return 0;
> +}
> +
> +static int enic_set_coalesce(struct net_device *netdev,
> +	struct ethtool_coalesce *ecmd)
> +{
> +	struct enic *enic = netdev_priv(netdev);
> +	u32 tx_coalesce_usecs;
> +	u32 rx_coalesce_usecs;
> +
> +	tx_coalesce_usecs = min_t(u32,
> +		INTR_COALESCE_HW_TO_USEC(VNIC_INTR_TIMER_MAX),
> +		ecmd->tx_coalesce_usecs);
> +	rx_coalesce_usecs = min_t(u32,
> +		INTR_COALESCE_HW_TO_USEC(VNIC_INTR_TIMER_MAX),
> +		ecmd->rx_coalesce_usecs);
> +
> +	switch (vnic_dev_get_intr_mode(enic->vdev)) {
> +	case VNIC_DEV_INTR_MODE_INTX:
> +		if (tx_coalesce_usecs != rx_coalesce_usecs)
> +			return -EINVAL;
> +
> +		vnic_intr_coalescing_timer_set(&enic->intr[ENIC_INTX_WQ_RQ],
> +			INTR_COALESCE_USEC_TO_HW(tx_coalesce_usecs));
> +		break;
> +	case VNIC_DEV_INTR_MODE_MSI:
> +		if (tx_coalesce_usecs != rx_coalesce_usecs)
> +			return -EINVAL;
> +
> +		vnic_intr_coalescing_timer_set(&enic->intr[0],
> +			INTR_COALESCE_USEC_TO_HW(tx_coalesce_usecs));
> +		break;
> +	case VNIC_DEV_INTR_MODE_MSIX:
> +		vnic_intr_coalescing_timer_set(&enic->intr[ENIC_MSIX_WQ],
> +			INTR_COALESCE_USEC_TO_HW(tx_coalesce_usecs));
> +		vnic_intr_coalescing_timer_set(&enic->intr[ENIC_MSIX_RQ],
> +			INTR_COALESCE_USEC_TO_HW(rx_coalesce_usecs));
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	enic->tx_coalesce_usecs = tx_coalesce_usecs;
> +	enic->rx_coalesce_usecs = rx_coalesce_usecs;
> +
> +	return 0;
> +}
> +
> +static struct ethtool_ops enic_ethtool_ops = {
>  	.get_settings = enic_get_settings,
>  	.get_drvinfo = enic_get_drvinfo,
>  	.get_msglevel = enic_get_msglevel,
> @@ -278,6 +334,8 @@ static const struct ethtool_ops enic_ethtool_ops = {
>  	.set_sg = ethtool_op_set_sg,
>  	.get_tso = ethtool_op_get_tso,
>  	.set_tso = enic_set_tso,
> +	.get_coalesce = enic_get_coalesce,
> +	.set_coalesce = enic_set_coalesce,
>  	.get_flags = ethtool_op_get_flags,
>  	.set_flags = ethtool_op_set_flags,
>  };
> @@ -363,12 +421,12 @@ static void enic_mtu_check(struct enic *enic)
>  	u32 mtu = vnic_dev_mtu(enic->vdev);
>  
>  	if (mtu && mtu != enic->port_mtu) {
> +		enic->port_mtu = mtu;
>  		if (mtu < enic->netdev->mtu)
>  			printk(KERN_WARNING PFX
>  				"%s: interface MTU (%d) set higher "
>  				"than switch port MTU (%d)\n",
>  				enic->netdev->name, enic->netdev->mtu, mtu);
> -		enic->port_mtu = mtu;
>  	}
>  }
>  
> @@ -1990,6 +2048,9 @@ static int __devinit enic_probe(struct pci_dev *pdev,
>  		goto err_out_dev_deinit;
>  	}
>  
> +	enic->tx_coalesce_usecs = enic->config.intr_timer_usec;
> +	enic->rx_coalesce_usecs = enic->tx_coalesce_usecs;
> +
>  	netdev->netdev_ops = &enic_netdev_ops;
>  	netdev->watchdog_timeo = 2 * HZ;
>  	netdev->ethtool_ops = &enic_ethtool_ops;
> diff --git a/drivers/net/enic/enic_res.c b/drivers/net/enic/enic_res.c
> index a605da1..02839bf 100644
> --- a/drivers/net/enic/enic_res.c
> +++ b/drivers/net/enic/enic_res.c
> @@ -66,9 +66,9 @@ int enic_get_vnic_config(struct enic *enic)
>  	GET_CONFIG(wq_desc_count);
>  	GET_CONFIG(rq_desc_count);
>  	GET_CONFIG(mtu);
> -	GET_CONFIG(intr_timer);
>  	GET_CONFIG(intr_timer_type);
>  	GET_CONFIG(intr_mode);
> +	GET_CONFIG(intr_timer_usec);
>  
>  	c->wq_desc_count =
>  		min_t(u32, ENIC_MAX_WQ_DESCS,
> @@ -88,15 +88,17 @@ int enic_get_vnic_config(struct enic *enic)
>  		max_t(u16, ENIC_MIN_MTU,
>  		c->mtu));
>  
> -	c->intr_timer = min_t(u16, VNIC_INTR_TIMER_MAX, c->intr_timer);
> +	c->intr_timer_usec = min_t(u32,
> +		INTR_COALESCE_HW_TO_USEC(VNIC_INTR_TIMER_MAX),
> +		c->intr_timer_usec);
>  
>  	printk(KERN_INFO PFX "vNIC MAC addr %pM wq/rq %d/%d\n",
>  		enic->mac_addr, c->wq_desc_count, c->rq_desc_count);
>  	printk(KERN_INFO PFX "vNIC mtu %d csum tx/rx %d/%d tso/lro %d/%d "
> -		"intr timer %d\n",
> +		"intr timer %d usec\n",
>  		c->mtu, ENIC_SETTING(enic, TXCSUM),
>  		ENIC_SETTING(enic, RXCSUM), ENIC_SETTING(enic, TSO),
> -		ENIC_SETTING(enic, LRO), c->intr_timer);
> +		ENIC_SETTING(enic, LRO), c->intr_timer_usec);
>  
>  	return 0;
>  }
> @@ -303,7 +305,7 @@ void enic_init_vnic_resources(struct enic *enic)
>  
>  	for (i = 0; i < enic->intr_count; i++) {
>  		vnic_intr_init(&enic->intr[i],
> -			enic->config.intr_timer,
> +			INTR_COALESCE_USEC_TO_HW(enic->config.intr_timer_usec),
>  			enic->config.intr_timer_type,
>  			mask_on_assertion);
>  	}
> diff --git a/drivers/net/enic/vnic_enet.h b/drivers/net/enic/vnic_enet.h
> index 6332ac9..8eeb675 100644
> --- a/drivers/net/enic/vnic_enet.h
> +++ b/drivers/net/enic/vnic_enet.h
> @@ -20,6 +20,10 @@
>  #ifndef _VNIC_ENIC_H_
>  #define _VNIC_ENIC_H_
>  
> +/* Hardware intr coalesce timer is in units of 1.5us */
> +#define INTR_COALESCE_USEC_TO_HW(usec) ((usec) * 2/3)
> +#define INTR_COALESCE_HW_TO_USEC(usec) ((usec) * 3/2)
> +
>  /* Device-specific region: enet configuration */
>  struct vnic_enet_config {
>  	u32 flags;
> @@ -30,6 +34,7 @@ struct vnic_enet_config {
>  	u8 intr_timer_type;
>  	u8 intr_mode;
>  	char devname[16];
> +	u32 intr_timer_usec;
>  };
>  
>  #define VENETF_TSO		0x1	/* TSO enabled */
> diff --git a/drivers/net/enic/vnic_intr.c b/drivers/net/enic/vnic_intr.c
> index 1f8786d..3934309 100644
> --- a/drivers/net/enic/vnic_intr.c
> +++ b/drivers/net/enic/vnic_intr.c
> @@ -50,12 +50,18 @@ int vnic_intr_alloc(struct vnic_dev *vdev, struct vnic_intr *intr,
>  void vnic_intr_init(struct vnic_intr *intr, unsigned int coalescing_timer,
>  	unsigned int coalescing_type, unsigned int mask_on_assertion)
>  {
> -	iowrite32(coalescing_timer, &intr->ctrl->coalescing_timer);
> +	vnic_intr_coalescing_timer_set(intr, coalescing_timer);
>  	iowrite32(coalescing_type, &intr->ctrl->coalescing_type);
>  	iowrite32(mask_on_assertion, &intr->ctrl->mask_on_assertion);
>  	iowrite32(0, &intr->ctrl->int_credits);
>  }
>  
> +void vnic_intr_coalescing_timer_set(struct vnic_intr *intr,
> +	unsigned int coalescing_timer)
> +{
> +	iowrite32(coalescing_timer, &intr->ctrl->coalescing_timer);
> +}
> +
>  void vnic_intr_clean(struct vnic_intr *intr)
>  {
>  	iowrite32(0, &intr->ctrl->int_credits);
> diff --git a/drivers/net/enic/vnic_intr.h b/drivers/net/enic/vnic_intr.h
> index f79a722..2fe6c63 100644
> --- a/drivers/net/enic/vnic_intr.h
> +++ b/drivers/net/enic/vnic_intr.h
> @@ -102,6 +102,8 @@ int vnic_intr_alloc(struct vnic_dev *vdev, struct vnic_intr *intr,
>  	unsigned int index);
>  void vnic_intr_init(struct vnic_intr *intr, unsigned int coalescing_timer,
>  	unsigned int coalescing_type, unsigned int mask_on_assertion);
> +void vnic_intr_coalescing_timer_set(struct vnic_intr *intr,
> +	unsigned int coalescing_timer);
>  void vnic_intr_clean(struct vnic_intr *intr);
>  
>  #endif /* _VNIC_INTR_H_ */
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [net-next PATCH 5/6] enic: feature add: add ethtool -c/C support

[Ben Hutchings]

On Mon, 2009-12-21 at 18:21 -0800, Scott Feldman wrote:
> From: Scott Feldman <scofeldm@cisco.com>
> 
> Only rx_usec and tx_usec options for ethtool -C are settable as those
> are the only settings that make sense to HW.  Adds driver reporting of 
> intr coalescing timer value in usec units rather than HW units.
[...]
> diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
> index 019b148..81cc68d 100644
> --- a/drivers/net/enic/enic_main.c
> +++ b/drivers/net/enic/enic_main.c
> @@ -261,7 +261,63 @@ static void enic_set_msglevel(struct net_device *netdev, u32 value)
>  	enic->msg_enable = value;
>  }
>  
> -static const struct ethtool_ops enic_ethtool_ops = {
[...]
> +static struct ethtool_ops enic_ethtool_ops = {
[...]

I don't see any reason to remove the const here.  Is that an accidental
change?

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [net-next PATCH 5/6] enic: feature add: add ethtool -c/C support

[Scott Feldman]

On 12/23/09 10:13 AM, "Ben Hutchings" <bhutchings@solarflare.com> wrote:

> On Mon, 2009-12-21 at 18:21 -0800, Scott Feldman wrote:
>> From: Scott Feldman <scofeldm@cisco.com>
>> 
>> Only rx_usec and tx_usec options for ethtool -C are settable as those
>> are the only settings that make sense to HW.  Adds driver reporting of
>> intr coalescing timer value in usec units rather than HW units.
> [...]
>> diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
>> index 019b148..81cc68d 100644
>> --- a/drivers/net/enic/enic_main.c
>> +++ b/drivers/net/enic/enic_main.c
>> @@ -261,7 +261,63 @@ static void enic_set_msglevel(struct net_device *netdev,
>> u32 value)
>> enic->msg_enable = value;
>>  }
>>  
>> -static const struct ethtool_ops enic_ethtool_ops = {
> [...]
>> +static struct ethtool_ops enic_ethtool_ops = {
> [...]
> 
> I don't see any reason to remove the const here.  Is that an accidental
> change?

That was a mistake.  Sigh.  Let me resend patch set without that change...

[I'll wait a while in case there are any more review comments]

-scott

[net-next PATCH 6/6] enic: whitespace cleanup; #define cleanup; more verbose err msg

[Scott Feldman]

From: Scott Feldman <scofeldm@cisco.com>

Some misc changes to cleanup whitespace issues and fix/remove some #define
HW defintions.

1) fix some whitespace issues
2) more verbose err msg when resources aren't available to configure vnic
3) remove unused #define
4) fix RSS #define rss hash types

Signed-off-by: Vasanthy Kolluri <vkolluri@cisco.com>
Signed-off-by: Scott Feldman <scofeldm@cisco.com>
---
 drivers/net/enic/enic_main.c |    5 +++--
 drivers/net/enic/vnic_dev.c  |    1 -
 drivers/net/enic/vnic_nic.h  |   12 ++++++------
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 81cc68d..978af6c 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -731,7 +731,7 @@ static inline void enic_queue_wq_skb(struct enic *enic,
 
 /* netif_tx_lock held, process context with BHs disabled, or BH */
 static netdev_tx_t enic_hard_start_xmit(struct sk_buff *skb,
-					      struct net_device *netdev)
+	struct net_device *netdev)
 {
 	struct enic *enic = netdev_priv(netdev);
 	struct vnic_wq *wq = &enic->wq[0];
@@ -1824,7 +1824,8 @@ int enic_dev_init(struct enic *enic)
 	err = enic_set_intr_mode(enic);
 	if (err) {
 		printk(KERN_ERR PFX
-			"Failed to set intr mode, aborting.\n");
+			"Failed to set intr mode based on resource "
+			"counts and system capabilities, aborting.\n");
 		return err;
 	}
 
diff --git a/drivers/net/enic/vnic_dev.c b/drivers/net/enic/vnic_dev.c
index 29a48e8..69b9b70 100644
--- a/drivers/net/enic/vnic_dev.c
+++ b/drivers/net/enic/vnic_dev.c
@@ -36,7 +36,6 @@ struct vnic_res {
 };
 
 #define VNIC_DEV_CAP_INIT	0x0001
-#define VNIC_DEV_CAP_PERBI	0x0002
 
 struct vnic_dev {
 	void *priv;
diff --git a/drivers/net/enic/vnic_nic.h b/drivers/net/enic/vnic_nic.h
index eeaf329..cf80ab4 100644
--- a/drivers/net/enic/vnic_nic.h
+++ b/drivers/net/enic/vnic_nic.h
@@ -41,12 +41,12 @@
 #define NIC_CFG_IG_VLAN_STRIP_EN_MASK_FIELD	1UL
 #define NIC_CFG_IG_VLAN_STRIP_EN_SHIFT		24
 
-#define NIC_CFG_RSS_HASH_TYPE_IPV4		(1 << 0)
-#define NIC_CFG_RSS_HASH_TYPE_TCP_IPV4		(1 << 1)
-#define NIC_CFG_RSS_HASH_TYPE_IPV6		(1 << 2)
-#define NIC_CFG_RSS_HASH_TYPE_TCP_IPV6		(1 << 3)
-#define NIC_CFG_RSS_HASH_TYPE_IPV6_EX		(1 << 4)
-#define NIC_CFG_RSS_HASH_TYPE_TCP_IPV6_EX	(1 << 5)
+#define NIC_CFG_RSS_HASH_TYPE_IPV4		(1 << 1)
+#define NIC_CFG_RSS_HASH_TYPE_TCP_IPV4		(1 << 2)
+#define NIC_CFG_RSS_HASH_TYPE_IPV6		(1 << 3)
+#define NIC_CFG_RSS_HASH_TYPE_TCP_IPV6		(1 << 4)
+#define NIC_CFG_RSS_HASH_TYPE_IPV6_EX		(1 << 5)
+#define NIC_CFG_RSS_HASH_TYPE_TCP_IPV6_EX	(1 << 6)
 
 static inline void vnic_set_nic_cfg(u32 *nic_cfg,
 	u8 rss_default_cpu, u8 rss_hash_type,

[net-next PATCH 0/6] enic: updates to version 1.1.0.241a

[Scott Feldman]

Resending with updates to address issues raised by Ben Hutchings and
Simon Horman and then Ben Hutchings again.

Please apply to net-next.

1/6 - Bug fix: use safe queue shutdown in dev->stop
2/6 - Bug fix: try harder to fill Rx ring on skb allocation failures
3/6 - minimize pkt filter updates to firmware
4/6 - Bug fix: align desc ring sizes to 32 descs
5/6 - feature add: add ethtool -c/C support
6/6 - whitespace cleanup; #define cleanup; more verbose err msg

Signed-off-by: Vasanthy Kolluri <vkolluri@cisco.com>
Signed-off-by: Scott Feldman <scofeldm@cisco.com>

[net-next PATCH 5/6] enic: feature add: add ethtool -c/C support

[Scott Feldman]

From: Scott Feldman <scofeldm@cisco.com>

Only rx_usec and tx_usec options for ethtool -C are settable as those
are the only settings that make sense to HW.  Adds driver reporting of 
intr coalescing timer value in usec units rather than HW units.

Signed-off-by: Vasanthy Kolluri <vkolluri@cisco.com>
Signed-off-by: Scott Feldman <scofeldm@cisco.com>
---
 drivers/net/enic/enic.h      |    2 +
 drivers/net/enic/enic_main.c |   63 +++++++++++++++++++++++++++++++++++++++++-
 drivers/net/enic/enic_res.c  |   12 +++++---
 drivers/net/enic/vnic_enet.h |    5 +++
 drivers/net/enic/vnic_intr.c |    8 +++++
 drivers/net/enic/vnic_intr.h |    2 +
 6 files changed, 85 insertions(+), 7 deletions(-)

diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
index b090d65..ee01f5a 100644
--- a/drivers/net/enic/enic.h
+++ b/drivers/net/enic/enic.h
@@ -93,6 +93,8 @@ struct enic {
 	unsigned int mc_count;
 	int csum_rx_enabled;
 	u32 port_mtu;
+	u32 rx_coalesce_usecs;
+	u32 tx_coalesce_usecs;
 
 	/* work queue cache line section */
 	____cacheline_aligned struct vnic_wq wq[ENIC_WQ_MAX];
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 019b148..e56f416 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -261,6 +261,62 @@ static void enic_set_msglevel(struct net_device *netdev, u32 value)
 	enic->msg_enable = value;
 }
 
+static int enic_get_coalesce(struct net_device *netdev,
+	struct ethtool_coalesce *ecmd)
+{
+	struct enic *enic = netdev_priv(netdev);
+
+	ecmd->tx_coalesce_usecs = enic->tx_coalesce_usecs;
+	ecmd->rx_coalesce_usecs = enic->rx_coalesce_usecs;
+
+	return 0;
+}
+
+static int enic_set_coalesce(struct net_device *netdev,
+	struct ethtool_coalesce *ecmd)
+{
+	struct enic *enic = netdev_priv(netdev);
+	u32 tx_coalesce_usecs;
+	u32 rx_coalesce_usecs;
+
+	tx_coalesce_usecs = min_t(u32,
+		INTR_COALESCE_HW_TO_USEC(VNIC_INTR_TIMER_MAX),
+		ecmd->tx_coalesce_usecs);
+	rx_coalesce_usecs = min_t(u32,
+		INTR_COALESCE_HW_TO_USEC(VNIC_INTR_TIMER_MAX),
+		ecmd->rx_coalesce_usecs);
+
+	switch (vnic_dev_get_intr_mode(enic->vdev)) {
+	case VNIC_DEV_INTR_MODE_INTX:
+		if (tx_coalesce_usecs != rx_coalesce_usecs)
+			return -EINVAL;
+
+		vnic_intr_coalescing_timer_set(&enic->intr[ENIC_INTX_WQ_RQ],
+			INTR_COALESCE_USEC_TO_HW(tx_coalesce_usecs));
+		break;
+	case VNIC_DEV_INTR_MODE_MSI:
+		if (tx_coalesce_usecs != rx_coalesce_usecs)
+			return -EINVAL;
+
+		vnic_intr_coalescing_timer_set(&enic->intr[0],
+			INTR_COALESCE_USEC_TO_HW(tx_coalesce_usecs));
+		break;
+	case VNIC_DEV_INTR_MODE_MSIX:
+		vnic_intr_coalescing_timer_set(&enic->intr[ENIC_MSIX_WQ],
+			INTR_COALESCE_USEC_TO_HW(tx_coalesce_usecs));
+		vnic_intr_coalescing_timer_set(&enic->intr[ENIC_MSIX_RQ],
+			INTR_COALESCE_USEC_TO_HW(rx_coalesce_usecs));
+		break;
+	default:
+		break;
+	}
+
+	enic->tx_coalesce_usecs = tx_coalesce_usecs;
+	enic->rx_coalesce_usecs = rx_coalesce_usecs;
+
+	return 0;
+}
+
 static const struct ethtool_ops enic_ethtool_ops = {
 	.get_settings = enic_get_settings,
 	.get_drvinfo = enic_get_drvinfo,
@@ -278,6 +334,8 @@ static const struct ethtool_ops enic_ethtool_ops = {
 	.set_sg = ethtool_op_set_sg,
 	.get_tso = ethtool_op_get_tso,
 	.set_tso = enic_set_tso,
+	.get_coalesce = enic_get_coalesce,
+	.set_coalesce = enic_set_coalesce,
 	.get_flags = ethtool_op_get_flags,
 	.set_flags = ethtool_op_set_flags,
 };
@@ -363,12 +421,12 @@ static void enic_mtu_check(struct enic *enic)
 	u32 mtu = vnic_dev_mtu(enic->vdev);
 
 	if (mtu && mtu != enic->port_mtu) {
+		enic->port_mtu = mtu;
 		if (mtu < enic->netdev->mtu)
 			printk(KERN_WARNING PFX
 				"%s: interface MTU (%d) set higher "
 				"than switch port MTU (%d)\n",
 				enic->netdev->name, enic->netdev->mtu, mtu);
-		enic->port_mtu = mtu;
 	}
 }
 
@@ -1990,6 +2048,9 @@ static int __devinit enic_probe(struct pci_dev *pdev,
 		goto err_out_dev_deinit;
 	}
 
+	enic->tx_coalesce_usecs = enic->config.intr_timer_usec;
+	enic->rx_coalesce_usecs = enic->tx_coalesce_usecs;
+
 	netdev->netdev_ops = &enic_netdev_ops;
 	netdev->watchdog_timeo = 2 * HZ;
 	netdev->ethtool_ops = &enic_ethtool_ops;
diff --git a/drivers/net/enic/enic_res.c b/drivers/net/enic/enic_res.c
index a605da1..02839bf 100644
--- a/drivers/net/enic/enic_res.c
+++ b/drivers/net/enic/enic_res.c
@@ -66,9 +66,9 @@ int enic_get_vnic_config(struct enic *enic)
 	GET_CONFIG(wq_desc_count);
 	GET_CONFIG(rq_desc_count);
 	GET_CONFIG(mtu);
-	GET_CONFIG(intr_timer);
 	GET_CONFIG(intr_timer_type);
 	GET_CONFIG(intr_mode);
+	GET_CONFIG(intr_timer_usec);
 
 	c->wq_desc_count =
 		min_t(u32, ENIC_MAX_WQ_DESCS,
@@ -88,15 +88,17 @@ int enic_get_vnic_config(struct enic *enic)
 		max_t(u16, ENIC_MIN_MTU,
 		c->mtu));
 
-	c->intr_timer = min_t(u16, VNIC_INTR_TIMER_MAX, c->intr_timer);
+	c->intr_timer_usec = min_t(u32,
+		INTR_COALESCE_HW_TO_USEC(VNIC_INTR_TIMER_MAX),
+		c->intr_timer_usec);
 
 	printk(KERN_INFO PFX "vNIC MAC addr %pM wq/rq %d/%d\n",
 		enic->mac_addr, c->wq_desc_count, c->rq_desc_count);
 	printk(KERN_INFO PFX "vNIC mtu %d csum tx/rx %d/%d tso/lro %d/%d "
-		"intr timer %d\n",
+		"intr timer %d usec\n",
 		c->mtu, ENIC_SETTING(enic, TXCSUM),
 		ENIC_SETTING(enic, RXCSUM), ENIC_SETTING(enic, TSO),
-		ENIC_SETTING(enic, LRO), c->intr_timer);
+		ENIC_SETTING(enic, LRO), c->intr_timer_usec);
 
 	return 0;
 }
@@ -303,7 +305,7 @@ void enic_init_vnic_resources(struct enic *enic)
 
 	for (i = 0; i < enic->intr_count; i++) {
 		vnic_intr_init(&enic->intr[i],
-			enic->config.intr_timer,
+			INTR_COALESCE_USEC_TO_HW(enic->config.intr_timer_usec),
 			enic->config.intr_timer_type,
 			mask_on_assertion);
 	}
diff --git a/drivers/net/enic/vnic_enet.h b/drivers/net/enic/vnic_enet.h
index 6332ac9..8eeb675 100644
--- a/drivers/net/enic/vnic_enet.h
+++ b/drivers/net/enic/vnic_enet.h
@@ -20,6 +20,10 @@
 #ifndef _VNIC_ENIC_H_
 #define _VNIC_ENIC_H_
 
+/* Hardware intr coalesce timer is in units of 1.5us */
+#define INTR_COALESCE_USEC_TO_HW(usec) ((usec) * 2/3)
+#define INTR_COALESCE_HW_TO_USEC(usec) ((usec) * 3/2)
+
 /* Device-specific region: enet configuration */
 struct vnic_enet_config {
 	u32 flags;
@@ -30,6 +34,7 @@ struct vnic_enet_config {
 	u8 intr_timer_type;
 	u8 intr_mode;
 	char devname[16];
+	u32 intr_timer_usec;
 };
 
 #define VENETF_TSO		0x1	/* TSO enabled */
diff --git a/drivers/net/enic/vnic_intr.c b/drivers/net/enic/vnic_intr.c
index 1f8786d..3934309 100644
--- a/drivers/net/enic/vnic_intr.c
+++ b/drivers/net/enic/vnic_intr.c
@@ -50,12 +50,18 @@ int vnic_intr_alloc(struct vnic_dev *vdev, struct vnic_intr *intr,
 void vnic_intr_init(struct vnic_intr *intr, unsigned int coalescing_timer,
 	unsigned int coalescing_type, unsigned int mask_on_assertion)
 {
-	iowrite32(coalescing_timer, &intr->ctrl->coalescing_timer);
+	vnic_intr_coalescing_timer_set(intr, coalescing_timer);
 	iowrite32(coalescing_type, &intr->ctrl->coalescing_type);
 	iowrite32(mask_on_assertion, &intr->ctrl->mask_on_assertion);
 	iowrite32(0, &intr->ctrl->int_credits);
 }
 
+void vnic_intr_coalescing_timer_set(struct vnic_intr *intr,
+	unsigned int coalescing_timer)
+{
+	iowrite32(coalescing_timer, &intr->ctrl->coalescing_timer);
+}
+
 void vnic_intr_clean(struct vnic_intr *intr)
 {
 	iowrite32(0, &intr->ctrl->int_credits);
diff --git a/drivers/net/enic/vnic_intr.h b/drivers/net/enic/vnic_intr.h
index f79a722..2fe6c63 100644
--- a/drivers/net/enic/vnic_intr.h
+++ b/drivers/net/enic/vnic_intr.h
@@ -102,6 +102,8 @@ int vnic_intr_alloc(struct vnic_dev *vdev, struct vnic_intr *intr,
 	unsigned int index);
 void vnic_intr_init(struct vnic_intr *intr, unsigned int coalescing_timer,
 	unsigned int coalescing_type, unsigned int mask_on_assertion);
+void vnic_intr_coalescing_timer_set(struct vnic_intr *intr,
+	unsigned int coalescing_timer);
 void vnic_intr_clean(struct vnic_intr *intr);
 
 #endif /* _VNIC_INTR_H_ */

[net-next PATCH 0/6] enic: updates to version 1.1.0.241

[Scott Feldman]

The following series implements enic driver updates:

1/6 - Bug fix: use safe queue shutdown in dev->stop
2/6 - Bug fix: try harder to fill Rx ring on skb allocation failures
3/6 - minimize pkt filter updates to firmware
4/6 - Bug fix: align desc ring sizes to 32 descs
5/6 - feature add: add ethtool -c/C support
6/6 - whitespace cleanup; #define cleanup; more verbose err msg

Signed-off-by: Scott Feldman <scofeldm@cisco.com>
Signed-off-by: Vasanthy Kolluri <vkolluri@cisco.com>

[net-next PATCH 5/6] enic: feature add: add ethtool -c/C support

[Scott Feldman]

enic: feature add: add ethtool -c/C support



Signed-off-by: Scott Feldman <scofeldm@cisco.com>
Signed-off-by: Vasanthy Kolluri <vkolluri@cisco.com>
---
 drivers/net/enic/enic.h      |    2 +
 drivers/net/enic/enic_main.c |   83 +++++++++++++++++++++++++++++++++++++++++-
 drivers/net/enic/enic_res.c  |   12 ++++--
 drivers/net/enic/vnic_enet.h |    5 +++
 drivers/net/enic/vnic_intr.c |    8 ++++
 drivers/net/enic/vnic_intr.h |    2 +
 6 files changed, 104 insertions(+), 8 deletions(-)

diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
index b09b091..68d7a66 100644
--- a/drivers/net/enic/enic.h
+++ b/drivers/net/enic/enic.h
@@ -93,6 +93,8 @@ struct enic {
 	unsigned int mc_count;
 	int csum_rx_enabled;
 	u32 port_mtu;
+	u32 rx_coalesce_usecs;
+	u32 tx_coalesce_usecs;
 
 	/* work queue cache line section */
 	____cacheline_aligned struct vnic_wq wq[ENIC_WQ_MAX];
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 2708ecf..664d132 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -262,7 +262,81 @@ static void enic_set_msglevel(struct net_device *netdev, u32 value)
 	enic->msg_enable = value;
 }
 
-static const struct ethtool_ops enic_ethtool_ops = {
+static int enic_get_coalesce(struct net_device *netdev,
+	struct ethtool_coalesce *ecmd)
+{
+	struct enic *enic = netdev_priv(netdev);
+
+	ecmd->tx_coalesce_usecs = enic->tx_coalesce_usecs;
+	ecmd->rx_coalesce_usecs = enic->rx_coalesce_usecs;
+
+	return 0;
+}
+
+static int enic_set_coalesce(struct net_device *netdev,
+	struct ethtool_coalesce *ecmd)
+{
+	struct enic *enic = netdev_priv(netdev);
+	u32 tx_coalesce_usecs;
+	u32 rx_coalesce_usecs;
+
+	/* Only rx-usecs and tx-usecs can be set */
+	if (ecmd->rx_max_coalesced_frames || ecmd->rx_coalesce_usecs_irq ||
+	    ecmd->rx_max_coalesced_frames_irq ||
+	    ecmd->tx_max_coalesced_frames || ecmd->tx_coalesce_usecs_irq ||
+	    ecmd->tx_max_coalesced_frames_irq ||
+	    ecmd->stats_block_coalesce_usecs ||
+	    ecmd->use_adaptive_rx_coalesce || ecmd->use_adaptive_tx_coalesce ||
+	    ecmd->pkt_rate_low ||
+	    ecmd->rx_coalesce_usecs_low || ecmd->rx_max_coalesced_frames_low ||
+	    ecmd->tx_coalesce_usecs_low || ecmd->tx_max_coalesced_frames_low ||
+	    ecmd->pkt_rate_high ||
+	    ecmd->rx_coalesce_usecs_high ||
+	    ecmd->rx_max_coalesced_frames_high ||
+	    ecmd->tx_coalesce_usecs_high ||
+	    ecmd->tx_max_coalesced_frames_high ||
+	    ecmd->rate_sample_interval)
+		return -EINVAL;
+
+	tx_coalesce_usecs = min_t(u32,
+		INTR_COALESCE_HW_TO_USEC(VNIC_INTR_TIMER_MAX),
+		ecmd->tx_coalesce_usecs);
+	rx_coalesce_usecs = min_t(u32,
+		INTR_COALESCE_HW_TO_USEC(VNIC_INTR_TIMER_MAX),
+		ecmd->rx_coalesce_usecs);
+
+	switch (vnic_dev_get_intr_mode(enic->vdev)) {
+	case VNIC_DEV_INTR_MODE_INTX:
+		if (tx_coalesce_usecs != rx_coalesce_usecs)
+			return -EINVAL;
+
+		vnic_intr_coalescing_timer_set(&enic->intr[ENIC_INTX_WQ_RQ],
+			INTR_COALESCE_USEC_TO_HW(tx_coalesce_usecs));
+		break;
+	case VNIC_DEV_INTR_MODE_MSI:
+		if (tx_coalesce_usecs != rx_coalesce_usecs)
+			return -EINVAL;
+
+		vnic_intr_coalescing_timer_set(&enic->intr[0],
+			INTR_COALESCE_USEC_TO_HW(tx_coalesce_usecs));
+		break;
+	case VNIC_DEV_INTR_MODE_MSIX:
+		vnic_intr_coalescing_timer_set(&enic->intr[ENIC_MSIX_WQ],
+			INTR_COALESCE_USEC_TO_HW(tx_coalesce_usecs));
+		vnic_intr_coalescing_timer_set(&enic->intr[ENIC_MSIX_RQ],
+			INTR_COALESCE_USEC_TO_HW(rx_coalesce_usecs));
+		break;
+	default:
+		break;
+	}
+
+	enic->tx_coalesce_usecs = tx_coalesce_usecs;
+	enic->rx_coalesce_usecs = rx_coalesce_usecs;
+
+	return 0;
+}
+
+static struct ethtool_ops enic_ethtool_ops = {
 	.get_settings = enic_get_settings,
 	.get_drvinfo = enic_get_drvinfo,
 	.get_msglevel = enic_get_msglevel,
@@ -279,6 +353,8 @@ static const struct ethtool_ops enic_ethtool_ops = {
 	.set_sg = ethtool_op_set_sg,
 	.get_tso = ethtool_op_get_tso,
 	.set_tso = enic_set_tso,
+	.get_coalesce = enic_get_coalesce,
+	.set_coalesce = enic_set_coalesce,
 	.get_flags = ethtool_op_get_flags,
 	.set_flags = ethtool_op_set_flags,
 };
@@ -364,12 +440,12 @@ static void enic_mtu_check(struct enic *enic)
 	u32 mtu = vnic_dev_mtu(enic->vdev);
 
 	if (mtu && mtu != enic->port_mtu) {
+		enic->port_mtu = mtu;
 		if (mtu < enic->netdev->mtu)
 			printk(KERN_WARNING PFX
 				"%s: interface MTU (%d) set higher "
 				"than switch port MTU (%d)\n",
 				enic->netdev->name, enic->netdev->mtu, mtu);
-		enic->port_mtu = mtu;
 	}
 }
 
@@ -1972,6 +2048,9 @@ static int __devinit enic_probe(struct pci_dev *pdev,
 		goto err_out_dev_deinit;
 	}
 
+	enic->tx_coalesce_usecs = enic->config.intr_timer_usec;
+	enic->rx_coalesce_usecs =  enic->tx_coalesce_usecs;
+
 	netdev->netdev_ops = &enic_netdev_ops;
 	netdev->watchdog_timeo = 2 * HZ;
 	netdev->ethtool_ops = &enic_ethtool_ops;
diff --git a/drivers/net/enic/enic_res.c b/drivers/net/enic/enic_res.c
index a605da1..02839bf 100644
--- a/drivers/net/enic/enic_res.c
+++ b/drivers/net/enic/enic_res.c
@@ -66,9 +66,9 @@ int enic_get_vnic_config(struct enic *enic)
 	GET_CONFIG(wq_desc_count);
 	GET_CONFIG(rq_desc_count);
 	GET_CONFIG(mtu);
-	GET_CONFIG(intr_timer);
 	GET_CONFIG(intr_timer_type);
 	GET_CONFIG(intr_mode);
+	GET_CONFIG(intr_timer_usec);
 
 	c->wq_desc_count =
 		min_t(u32, ENIC_MAX_WQ_DESCS,
@@ -88,15 +88,17 @@ int enic_get_vnic_config(struct enic *enic)
 		max_t(u16, ENIC_MIN_MTU,
 		c->mtu));
 
-	c->intr_timer = min_t(u16, VNIC_INTR_TIMER_MAX, c->intr_timer);
+	c->intr_timer_usec = min_t(u32,
+		INTR_COALESCE_HW_TO_USEC(VNIC_INTR_TIMER_MAX),
+		c->intr_timer_usec);
 
 	printk(KERN_INFO PFX "vNIC MAC addr %pM wq/rq %d/%d\n",
 		enic->mac_addr, c->wq_desc_count, c->rq_desc_count);
 	printk(KERN_INFO PFX "vNIC mtu %d csum tx/rx %d/%d tso/lro %d/%d "
-		"intr timer %d\n",
+		"intr timer %d usec\n",
 		c->mtu, ENIC_SETTING(enic, TXCSUM),
 		ENIC_SETTING(enic, RXCSUM), ENIC_SETTING(enic, TSO),
-		ENIC_SETTING(enic, LRO), c->intr_timer);
+		ENIC_SETTING(enic, LRO), c->intr_timer_usec);
 
 	return 0;
 }
@@ -303,7 +305,7 @@ void enic_init_vnic_resources(struct enic *enic)
 
 	for (i = 0; i < enic->intr_count; i++) {
 		vnic_intr_init(&enic->intr[i],
-			enic->config.intr_timer,
+			INTR_COALESCE_USEC_TO_HW(enic->config.intr_timer_usec),
 			enic->config.intr_timer_type,
 			mask_on_assertion);
 	}
diff --git a/drivers/net/enic/vnic_enet.h b/drivers/net/enic/vnic_enet.h
index 6332ac9..8eeb675 100644
--- a/drivers/net/enic/vnic_enet.h
+++ b/drivers/net/enic/vnic_enet.h
@@ -20,6 +20,10 @@
 #ifndef _VNIC_ENIC_H_
 #define _VNIC_ENIC_H_
 
+/* Hardware intr coalesce timer is in units of 1.5us */
+#define INTR_COALESCE_USEC_TO_HW(usec) ((usec) * 2/3)
+#define INTR_COALESCE_HW_TO_USEC(usec) ((usec) * 3/2)
+
 /* Device-specific region: enet configuration */
 struct vnic_enet_config {
 	u32 flags;
@@ -30,6 +34,7 @@ struct vnic_enet_config {
 	u8 intr_timer_type;
 	u8 intr_mode;
 	char devname[16];
+	u32 intr_timer_usec;
 };
 
 #define VENETF_TSO		0x1	/* TSO enabled */
diff --git a/drivers/net/enic/vnic_intr.c b/drivers/net/enic/vnic_intr.c
index 1f8786d..3934309 100644
--- a/drivers/net/enic/vnic_intr.c
+++ b/drivers/net/enic/vnic_intr.c
@@ -50,12 +50,18 @@ int vnic_intr_alloc(struct vnic_dev *vdev, struct vnic_intr *intr,
 void vnic_intr_init(struct vnic_intr *intr, unsigned int coalescing_timer,
 	unsigned int coalescing_type, unsigned int mask_on_assertion)
 {
-	iowrite32(coalescing_timer, &intr->ctrl->coalescing_timer);
+	vnic_intr_coalescing_timer_set(intr, coalescing_timer);
 	iowrite32(coalescing_type, &intr->ctrl->coalescing_type);
 	iowrite32(mask_on_assertion, &intr->ctrl->mask_on_assertion);
 	iowrite32(0, &intr->ctrl->int_credits);
 }
 
+void vnic_intr_coalescing_timer_set(struct vnic_intr *intr,
+	unsigned int coalescing_timer)
+{
+	iowrite32(coalescing_timer, &intr->ctrl->coalescing_timer);
+}
+
 void vnic_intr_clean(struct vnic_intr *intr)
 {
 	iowrite32(0, &intr->ctrl->int_credits);
diff --git a/drivers/net/enic/vnic_intr.h b/drivers/net/enic/vnic_intr.h
index 9a53604..0c1e094 100644
--- a/drivers/net/enic/vnic_intr.h
+++ b/drivers/net/enic/vnic_intr.h
@@ -101,6 +101,8 @@ int vnic_intr_alloc(struct vnic_dev *vdev, struct vnic_intr *intr,
 	unsigned int index);
 void vnic_intr_init(struct vnic_intr *intr, unsigned int coalescing_timer,
 	unsigned int coalescing_type, unsigned int mask_on_assertion);
+void vnic_intr_coalescing_timer_set(struct vnic_intr *intr,
+	unsigned int coalescing_timer);
 void vnic_intr_clean(struct vnic_intr *intr);
 
 #endif /* _VNIC_INTR_H_ */

Re: [net-next PATCH 5/6] enic: feature add: add ethtool -c/C support

[Simon Horman]

On Fri, Dec 18, 2009 at 06:10:02PM -0800, Scott Feldman wrote:
> enic: feature add: add ethtool -c/C support
> 
> 
> 
> Signed-off-by: Scott Feldman <scofeldm@cisco.com>
> Signed-off-by: Vasanthy Kolluri <vkolluri@cisco.com>
> ---
>  drivers/net/enic/enic.h      |    2 +
>  drivers/net/enic/enic_main.c |   83 +++++++++++++++++++++++++++++++++++++++++-
>  drivers/net/enic/enic_res.c  |   12 ++++--
>  drivers/net/enic/vnic_enet.h |    5 +++
>  drivers/net/enic/vnic_intr.c |    8 ++++
>  drivers/net/enic/vnic_intr.h |    2 +
>  6 files changed, 104 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
> index b09b091..68d7a66 100644
> --- a/drivers/net/enic/enic.h
> +++ b/drivers/net/enic/enic.h
> @@ -93,6 +93,8 @@ struct enic {
>  	unsigned int mc_count;
>  	int csum_rx_enabled;
>  	u32 port_mtu;
> +	u32 rx_coalesce_usecs;
> +	u32 tx_coalesce_usecs;
>  
>  	/* work queue cache line section */
>  	____cacheline_aligned struct vnic_wq wq[ENIC_WQ_MAX];
> diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
> index 2708ecf..664d132 100644
> --- a/drivers/net/enic/enic_main.c
> +++ b/drivers/net/enic/enic_main.c
> @@ -262,7 +262,81 @@ static void enic_set_msglevel(struct net_device *netdev, u32 value)
>  	enic->msg_enable = value;
>  }
>  
> -static const struct ethtool_ops enic_ethtool_ops = {
> +static int enic_get_coalesce(struct net_device *netdev,
> +	struct ethtool_coalesce *ecmd)
> +{
> +	struct enic *enic = netdev_priv(netdev);
> +
> +	ecmd->tx_coalesce_usecs = enic->tx_coalesce_usecs;
> +	ecmd->rx_coalesce_usecs = enic->rx_coalesce_usecs;
> +
> +	return 0;
> +}
> +
> +static int enic_set_coalesce(struct net_device *netdev,
> +	struct ethtool_coalesce *ecmd)
> +{
> +	struct enic *enic = netdev_priv(netdev);
> +	u32 tx_coalesce_usecs;
> +	u32 rx_coalesce_usecs;
> +
> +	/* Only rx-usecs and tx-usecs can be set */
> +	if (ecmd->rx_max_coalesced_frames || ecmd->rx_coalesce_usecs_irq ||
> +	    ecmd->rx_max_coalesced_frames_irq ||
> +	    ecmd->tx_max_coalesced_frames || ecmd->tx_coalesce_usecs_irq ||
> +	    ecmd->tx_max_coalesced_frames_irq ||
> +	    ecmd->stats_block_coalesce_usecs ||
> +	    ecmd->use_adaptive_rx_coalesce || ecmd->use_adaptive_tx_coalesce ||
> +	    ecmd->pkt_rate_low ||
> +	    ecmd->rx_coalesce_usecs_low || ecmd->rx_max_coalesced_frames_low ||
> +	    ecmd->tx_coalesce_usecs_low || ecmd->tx_max_coalesced_frames_low ||
> +	    ecmd->pkt_rate_high ||
> +	    ecmd->rx_coalesce_usecs_high ||
> +	    ecmd->rx_max_coalesced_frames_high ||
> +	    ecmd->tx_coalesce_usecs_high ||
> +	    ecmd->tx_max_coalesced_frames_high ||
> +	    ecmd->rate_sample_interval)
> +		return -EINVAL;

This seems rather verbose.  Does it really matter if there is junk in other
ecmd fields?  They seem to be otherwise ignored. Removing the check above
would seem to be consistent with other enic_set_*() functions.

> +
> +	tx_coalesce_usecs = min_t(u32,
> +		INTR_COALESCE_HW_TO_USEC(VNIC_INTR_TIMER_MAX),
> +		ecmd->tx_coalesce_usecs);
> +	rx_coalesce_usecs = min_t(u32,
> +		INTR_COALESCE_HW_TO_USEC(VNIC_INTR_TIMER_MAX),
> +		ecmd->rx_coalesce_usecs);
> +
> +	switch (vnic_dev_get_intr_mode(enic->vdev)) {
> +	case VNIC_DEV_INTR_MODE_INTX:
> +		if (tx_coalesce_usecs != rx_coalesce_usecs)
> +			return -EINVAL;
> +
> +		vnic_intr_coalescing_timer_set(&enic->intr[ENIC_INTX_WQ_RQ],
> +			INTR_COALESCE_USEC_TO_HW(tx_coalesce_usecs));
> +		break;
> +	case VNIC_DEV_INTR_MODE_MSI:
> +		if (tx_coalesce_usecs != rx_coalesce_usecs)
> +			return -EINVAL;
> +
> +		vnic_intr_coalescing_timer_set(&enic->intr[0],
> +			INTR_COALESCE_USEC_TO_HW(tx_coalesce_usecs));
> +		break;
> +	case VNIC_DEV_INTR_MODE_MSIX:
> +		vnic_intr_coalescing_timer_set(&enic->intr[ENIC_MSIX_WQ],
> +			INTR_COALESCE_USEC_TO_HW(tx_coalesce_usecs));
> +		vnic_intr_coalescing_timer_set(&enic->intr[ENIC_MSIX_RQ],
> +			INTR_COALESCE_USEC_TO_HW(rx_coalesce_usecs));
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	enic->tx_coalesce_usecs = tx_coalesce_usecs;
> +	enic->rx_coalesce_usecs = rx_coalesce_usecs;
> +
> +	return 0;
> +}
> +
> +static struct ethtool_ops enic_ethtool_ops = {
>  	.get_settings = enic_get_settings,
>  	.get_drvinfo = enic_get_drvinfo,
>  	.get_msglevel = enic_get_msglevel,
> @@ -279,6 +353,8 @@ static const struct ethtool_ops enic_ethtool_ops = {
>  	.set_sg = ethtool_op_set_sg,
>  	.get_tso = ethtool_op_get_tso,
>  	.set_tso = enic_set_tso,
> +	.get_coalesce = enic_get_coalesce,
> +	.set_coalesce = enic_set_coalesce,
>  	.get_flags = ethtool_op_get_flags,
>  	.set_flags = ethtool_op_set_flags,
>  };
> @@ -364,12 +440,12 @@ static void enic_mtu_check(struct enic *enic)
>  	u32 mtu = vnic_dev_mtu(enic->vdev);
>  
>  	if (mtu && mtu != enic->port_mtu) {
> +		enic->port_mtu = mtu;
>  		if (mtu < enic->netdev->mtu)
>  			printk(KERN_WARNING PFX
>  				"%s: interface MTU (%d) set higher "
>  				"than switch port MTU (%d)\n",
>  				enic->netdev->name, enic->netdev->mtu, mtu);
> -		enic->port_mtu = mtu;
>  	}
>  }

This hunk seems unrelated to the rest of the patch.
Am I missing something?

> @@ -1972,6 +2048,9 @@ static int __devinit enic_probe(struct pci_dev *pdev,
>  		goto err_out_dev_deinit;
>  	}
>  
> +	enic->tx_coalesce_usecs = enic->config.intr_timer_usec;
> +	enic->rx_coalesce_usecs =  enic->tx_coalesce_usecs;
> +

It looks like there is an space after =

>  	netdev->netdev_ops = &enic_netdev_ops;
>  	netdev->watchdog_timeo = 2 * HZ;
>  	netdev->ethtool_ops = &enic_ethtool_ops;
> diff --git a/drivers/net/enic/enic_res.c b/drivers/net/enic/enic_res.c
> index a605da1..02839bf 100644
> --- a/drivers/net/enic/enic_res.c
> +++ b/drivers/net/enic/enic_res.c
> @@ -66,9 +66,9 @@ int enic_get_vnic_config(struct enic *enic)
>  	GET_CONFIG(wq_desc_count);
>  	GET_CONFIG(rq_desc_count);
>  	GET_CONFIG(mtu);
> -	GET_CONFIG(intr_timer);
>  	GET_CONFIG(intr_timer_type);
>  	GET_CONFIG(intr_mode);
> +	GET_CONFIG(intr_timer_usec);
>  
>  	c->wq_desc_count =
>  		min_t(u32, ENIC_MAX_WQ_DESCS,
> @@ -88,15 +88,17 @@ int enic_get_vnic_config(struct enic *enic)
>  		max_t(u16, ENIC_MIN_MTU,
>  		c->mtu));
>  
> -	c->intr_timer = min_t(u16, VNIC_INTR_TIMER_MAX, c->intr_timer);
> +	c->intr_timer_usec = min_t(u32,
> +		INTR_COALESCE_HW_TO_USEC(VNIC_INTR_TIMER_MAX),
> +		c->intr_timer_usec);
>  
>  	printk(KERN_INFO PFX "vNIC MAC addr %pM wq/rq %d/%d\n",
>  		enic->mac_addr, c->wq_desc_count, c->rq_desc_count);
>  	printk(KERN_INFO PFX "vNIC mtu %d csum tx/rx %d/%d tso/lro %d/%d "
> -		"intr timer %d\n",
> +		"intr timer %d usec\n",
>  		c->mtu, ENIC_SETTING(enic, TXCSUM),
>  		ENIC_SETTING(enic, RXCSUM), ENIC_SETTING(enic, TSO),
> -		ENIC_SETTING(enic, LRO), c->intr_timer);
> +		ENIC_SETTING(enic, LRO), c->intr_timer_usec);
>  
>  	return 0;
>  }
> @@ -303,7 +305,7 @@ void enic_init_vnic_resources(struct enic *enic)
>  
>  	for (i = 0; i < enic->intr_count; i++) {
>  		vnic_intr_init(&enic->intr[i],
> -			enic->config.intr_timer,
> +			INTR_COALESCE_USEC_TO_HW(enic->config.intr_timer_usec),
>  			enic->config.intr_timer_type,
>  			mask_on_assertion);
>  	}
> diff --git a/drivers/net/enic/vnic_enet.h b/drivers/net/enic/vnic_enet.h
> index 6332ac9..8eeb675 100644
> --- a/drivers/net/enic/vnic_enet.h
> +++ b/drivers/net/enic/vnic_enet.h
> @@ -20,6 +20,10 @@
>  #ifndef _VNIC_ENIC_H_
>  #define _VNIC_ENIC_H_
>  
> +/* Hardware intr coalesce timer is in units of 1.5us */
> +#define INTR_COALESCE_USEC_TO_HW(usec) ((usec) * 2/3)
> +#define INTR_COALESCE_HW_TO_USEC(usec) ((usec) * 3/2)
> +
>  /* Device-specific region: enet configuration */
>  struct vnic_enet_config {
>  	u32 flags;
> @@ -30,6 +34,7 @@ struct vnic_enet_config {
>  	u8 intr_timer_type;
>  	u8 intr_mode;
>  	char devname[16];
> +	u32 intr_timer_usec;
>  };
>  
>  #define VENETF_TSO		0x1	/* TSO enabled */
> diff --git a/drivers/net/enic/vnic_intr.c b/drivers/net/enic/vnic_intr.c
> index 1f8786d..3934309 100644
> --- a/drivers/net/enic/vnic_intr.c
> +++ b/drivers/net/enic/vnic_intr.c
> @@ -50,12 +50,18 @@ int vnic_intr_alloc(struct vnic_dev *vdev, struct vnic_intr *intr,
>  void vnic_intr_init(struct vnic_intr *intr, unsigned int coalescing_timer,
>  	unsigned int coalescing_type, unsigned int mask_on_assertion)
>  {
> -	iowrite32(coalescing_timer, &intr->ctrl->coalescing_timer);
> +	vnic_intr_coalescing_timer_set(intr, coalescing_timer);
>  	iowrite32(coalescing_type, &intr->ctrl->coalescing_type);
>  	iowrite32(mask_on_assertion, &intr->ctrl->mask_on_assertion);
>  	iowrite32(0, &intr->ctrl->int_credits);
>  }
>  
> +void vnic_intr_coalescing_timer_set(struct vnic_intr *intr,
> +	unsigned int coalescing_timer)
> +{
> +	iowrite32(coalescing_timer, &intr->ctrl->coalescing_timer);
> +}
> +
>  void vnic_intr_clean(struct vnic_intr *intr)
>  {
>  	iowrite32(0, &intr->ctrl->int_credits);
> diff --git a/drivers/net/enic/vnic_intr.h b/drivers/net/enic/vnic_intr.h
> index 9a53604..0c1e094 100644
> --- a/drivers/net/enic/vnic_intr.h
> +++ b/drivers/net/enic/vnic_intr.h
> @@ -101,6 +101,8 @@ int vnic_intr_alloc(struct vnic_dev *vdev, struct vnic_intr *intr,
>  	unsigned int index);
>  void vnic_intr_init(struct vnic_intr *intr, unsigned int coalescing_timer,
>  	unsigned int coalescing_type, unsigned int mask_on_assertion);
> +void vnic_intr_coalescing_timer_set(struct vnic_intr *intr,
> +	unsigned int coalescing_timer);
>  void vnic_intr_clean(struct vnic_intr *intr);
>  
>  #endif /* _VNIC_INTR_H_ */
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [net-next PATCH 5/6] enic: feature add: add ethtool -c/C support

[Scott Feldman]

Thanks for the review comments Simon.

On 12/19/09 2:10 AM, "Simon Horman" <horms@verge.net.au> wrote:

> On Fri, Dec 18, 2009 at 06:10:02PM -0800, Scott Feldman wrote:
>> + /* Only rx-usecs and tx-usecs can be set */
>> + if (ecmd->rx_max_coalesced_frames || ecmd->rx_coalesce_usecs_irq ||
>> +     ecmd->rx_max_coalesced_frames_irq ||
>> +     ecmd->tx_max_coalesced_frames || ecmd->tx_coalesce_usecs_irq ||
>> +     ecmd->tx_max_coalesced_frames_irq ||
>> +     ecmd->stats_block_coalesce_usecs ||
>> +     ecmd->use_adaptive_rx_coalesce || ecmd->use_adaptive_tx_coalesce ||
>> +     ecmd->pkt_rate_low ||
>> +     ecmd->rx_coalesce_usecs_low || ecmd->rx_max_coalesced_frames_low ||
>> +     ecmd->tx_coalesce_usecs_low || ecmd->tx_max_coalesced_frames_low ||
>> +     ecmd->pkt_rate_high ||
>> +     ecmd->rx_coalesce_usecs_high ||
>> +     ecmd->rx_max_coalesced_frames_high ||
>> +     ecmd->tx_coalesce_usecs_high ||
>> +     ecmd->tx_max_coalesced_frames_high ||
>> +     ecmd->rate_sample_interval)
>> +  return -EINVAL;
> 
> This seems rather verbose.  Does it really matter if there is junk in other
> ecmd fields?  They seem to be otherwise ignored. Removing the check above
> would seem to be consistent with other enic_set_*() functions.

Ya, we can do that.
 
>> if (mtu && mtu != enic->port_mtu) {
>> +  enic->port_mtu = mtu;
>> if (mtu < enic->netdev->mtu)
>> printk(KERN_WARNING PFX
>> "%s: interface MTU (%d) set higher "
>> "than switch port MTU (%d)\n",
>> enic->netdev->name, enic->netdev->mtu, mtu);
>> -  enic->port_mtu = mtu;
>> }
>>  }
> 
> This hunk seems unrelated to the rest of the patch.
> Am I missing something?

Oops, that change snuck in there.  That line got moved due to some backward
compat changes, which have been stripped away, so it looks like that line is
moving for no reason.  It's a benign change so let's make the change to be
consistent with our internal code base.
 
>> + enic->tx_coalesce_usecs = enic->config.intr_timer_usec;
>> + enic->rx_coalesce_usecs =  enic->tx_coalesce_usecs;
>> +
> 
> It looks like there is an space after =

Got it.

-scott

Re: [net-next PATCH 5/6] enic: feature add: add ethtool -c/C support

[Simon Horman]

On Sat, Dec 19, 2009 at 11:33:54AM -0800, Scott Feldman wrote:
> Thanks for the review comments Simon.
> 
> On 12/19/09 2:10 AM, "Simon Horman" <horms@verge.net.au> wrote:
> 
> > On Fri, Dec 18, 2009 at 06:10:02PM -0800, Scott Feldman wrote:
> >> + /* Only rx-usecs and tx-usecs can be set */
> >> + if (ecmd->rx_max_coalesced_frames || ecmd->rx_coalesce_usecs_irq ||
> >> +     ecmd->rx_max_coalesced_frames_irq ||
> >> +     ecmd->tx_max_coalesced_frames || ecmd->tx_coalesce_usecs_irq ||
> >> +     ecmd->tx_max_coalesced_frames_irq ||
> >> +     ecmd->stats_block_coalesce_usecs ||
> >> +     ecmd->use_adaptive_rx_coalesce || ecmd->use_adaptive_tx_coalesce ||
> >> +     ecmd->pkt_rate_low ||
> >> +     ecmd->rx_coalesce_usecs_low || ecmd->rx_max_coalesced_frames_low ||
> >> +     ecmd->tx_coalesce_usecs_low || ecmd->tx_max_coalesced_frames_low ||
> >> +     ecmd->pkt_rate_high ||
> >> +     ecmd->rx_coalesce_usecs_high ||
> >> +     ecmd->rx_max_coalesced_frames_high ||
> >> +     ecmd->tx_coalesce_usecs_high ||
> >> +     ecmd->tx_max_coalesced_frames_high ||
> >> +     ecmd->rate_sample_interval)
> >> +  return -EINVAL;
> > 
> > This seems rather verbose.  Does it really matter if there is junk in other
> > ecmd fields?  They seem to be otherwise ignored. Removing the check above
> > would seem to be consistent with other enic_set_*() functions.
> 
> Ya, we can do that.
>  
> >> if (mtu && mtu != enic->port_mtu) {
> >> +  enic->port_mtu = mtu;
> >> if (mtu < enic->netdev->mtu)
> >> printk(KERN_WARNING PFX
> >> "%s: interface MTU (%d) set higher "
> >> "than switch port MTU (%d)\n",
> >> enic->netdev->name, enic->netdev->mtu, mtu);
> >> -  enic->port_mtu = mtu;
> >> }
> >>  }
> > 
> > This hunk seems unrelated to the rest of the patch.
> > Am I missing something?
> 
> Oops, that change snuck in there.  That line got moved due to some backward
> compat changes, which have been stripped away, so it looks like that line is
> moving for no reason.  It's a benign change so let's make the change to be
> consistent with our internal code base.

Sure, it doesn't seem to be doing any harm.

> >> + enic->tx_coalesce_usecs = enic->config.intr_timer_usec;
> >> + enic->rx_coalesce_usecs =  enic->tx_coalesce_usecs;
> >> +
> > 
> > It looks like there is an space after =
> 
> Got it.
> 
> -scott
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html