linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [v1 0/6] *** nps_enet fixups ***
@ 2015-08-17  5:58 Noam Camus
  2015-08-17  5:58 ` [v1 1/6] NET: nps_enet: replace use of cause register Noam Camus
  2015-08-17 17:36 ` [v1 0/6] *** nps_enet fixups *** David Miller
  0 siblings, 2 replies; 17+ messages in thread
From: Noam Camus @ 2015-08-17  5:58 UTC (permalink / raw)
  To: linux-kernel, netdev; +Cc: davem, Alexey.Brodkin, vgupta, talz, Noam Camus

From: Noam Camus <noamc@ezchip.com>

This patch set is a bunch of fixes to make nps_enet work correctly with
all platforms, i.e. real device, emulation system, and simulation system.
The main trigger for this patch set was that in our emulation system
the TX end interrupt is "edge-sensitive" and therefore we cannot use the
cause register since it is not sticky.
Also:
TX is handled during HW interrupt context and not NAPI job.
race with TX done was fixed.
added acknowledge for TX when device is "level sensitive".
enable drop of control frames which is not needed for regular usage.

So most of this patch set is about TX handling, which is now more complete.

Noam Camus (6):
  NET: nps_enet: replace use of cause register
  NET: nps_enet: reduce processing latency.
  NET: nps_enet: TX done race condition
  NET: nps_enet: drop control frames
  NET: nps_enet: TX done acknowledge.
  NET: nps_enet: minor namespace cleanup

 drivers/net/ethernet/ezchip/nps_enet.c |   44 +++++++++++++++++--------------
 drivers/net/ethernet/ezchip/nps_enet.h |   20 --------------
 2 files changed, 24 insertions(+), 40 deletions(-)


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [v1 1/6] NET: nps_enet: replace use of cause register
  2015-08-17  5:58 [v1 0/6] *** nps_enet fixups *** Noam Camus
@ 2015-08-17  5:58 ` Noam Camus
  2015-08-17  5:58   ` [v1 2/6] NET: nps_enet: reduce processing latency Noam Camus
  2015-08-17 17:36 ` [v1 0/6] *** nps_enet fixups *** David Miller
  1 sibling, 1 reply; 17+ messages in thread
From: Noam Camus @ 2015-08-17  5:58 UTC (permalink / raw)
  To: linux-kernel, netdev; +Cc: davem, Alexey.Brodkin, vgupta, talz, Noam Camus

From: Noam Camus <noamc@ezchip.com>

When interrupt is received we read directly from control
register for RX/TX instead of reading cause register
since this register fails to indicate TX done when
TX interrupt is "edge mode".

Signed-off-by: Noam Camus <noamc@ezchip.com>
---
 drivers/net/ethernet/ezchip/nps_enet.c |    9 +++++----
 drivers/net/ethernet/ezchip/nps_enet.h |   20 --------------------
 2 files changed, 5 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ethernet/ezchip/nps_enet.c b/drivers/net/ethernet/ezchip/nps_enet.c
index 24a85b2..0e652b4 100644
--- a/drivers/net/ethernet/ezchip/nps_enet.c
+++ b/drivers/net/ethernet/ezchip/nps_enet.c
@@ -211,12 +211,13 @@ static irqreturn_t nps_enet_irq_handler(s32 irq, void *dev_instance)
 {
 	struct net_device *ndev = dev_instance;
 	struct nps_enet_priv *priv = netdev_priv(ndev);
-	struct nps_enet_buf_int_cause buf_int_cause;
+	struct nps_enet_rx_ctl rx_ctrl;
+	struct nps_enet_tx_ctl tx_ctrl;
 
-	buf_int_cause.value =
-			nps_enet_reg_get(priv, NPS_ENET_REG_BUF_INT_CAUSE);
+	rx_ctrl.value = nps_enet_reg_get(priv, NPS_ENET_REG_RX_CTL);
+	tx_ctrl.value = nps_enet_reg_get(priv, NPS_ENET_REG_TX_CTL);
 
-	if (buf_int_cause.tx_done || buf_int_cause.rx_rdy)
+	if ((!tx_ctrl.ct && priv->tx_packet_sent) || rx_ctrl.cr)
 		if (likely(napi_schedule_prep(&priv->napi))) {
 			nps_enet_reg_set(priv, NPS_ENET_REG_BUF_INT_ENABLE, 0);
 			__napi_schedule(&priv->napi);
diff --git a/drivers/net/ethernet/ezchip/nps_enet.h b/drivers/net/ethernet/ezchip/nps_enet.h
index fc45c9d..6703674 100644
--- a/drivers/net/ethernet/ezchip/nps_enet.h
+++ b/drivers/net/ethernet/ezchip/nps_enet.h
@@ -36,7 +36,6 @@
 #define NPS_ENET_REG_RX_CTL		0x810
 #define NPS_ENET_REG_RX_BUF		0x818
 #define NPS_ENET_REG_BUF_INT_ENABLE	0x8C0
-#define NPS_ENET_REG_BUF_INT_CAUSE	0x8C4
 #define NPS_ENET_REG_GE_MAC_CFG_0	0x1000
 #define NPS_ENET_REG_GE_MAC_CFG_1	0x1004
 #define NPS_ENET_REG_GE_MAC_CFG_2	0x1008
@@ -108,25 +107,6 @@ struct nps_enet_buf_int_enable {
 	};
 };
 
-/* Interrupt cause for data buffer events register */
-struct nps_enet_buf_int_cause {
-	union {
-		/* tx_done: Interrupt in the case when current frame was
-		 *          read from TX buffer.
-		 * rx_rdy:  Interrupt in the case when new frame is ready
-		 *          in RX buffer.
-		 */
-		struct {
-			u32
-			__reserved:30,
-			tx_done:1,
-			rx_rdy:1;
-		};
-
-		u32 value;
-	};
-};
-
 /* Gbps Eth MAC Configuration 0 register */
 struct nps_enet_ge_mac_cfg_0 {
 	union {
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [v1 2/6] NET: nps_enet: reduce processing latency.
  2015-08-17  5:58 ` [v1 1/6] NET: nps_enet: replace use of cause register Noam Camus
@ 2015-08-17  5:58   ` Noam Camus
  2015-08-17  5:58     ` [v1 3/6] NET: nps_enet: TX done race condition Noam Camus
  0 siblings, 1 reply; 17+ messages in thread
From: Noam Camus @ 2015-08-17  5:58 UTC (permalink / raw)
  To: linux-kernel, netdev; +Cc: davem, Alexey.Brodkin, vgupta, talz, Noam Camus

From: Noam Camus <noamc@ezchip.com>

TX handler is minimalistic and there is no need to schedule
a NAPI job.
Tx done will be processed during hardware interrupt context.

Signed-off-by: Noam Camus <noamc@ezchip.com>
---
 drivers/net/ethernet/ezchip/nps_enet.c |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/ezchip/nps_enet.c b/drivers/net/ethernet/ezchip/nps_enet.c
index 0e652b4..af72181 100644
--- a/drivers/net/ethernet/ezchip/nps_enet.c
+++ b/drivers/net/ethernet/ezchip/nps_enet.c
@@ -159,7 +159,7 @@ static void nps_enet_tx_handler(struct net_device *ndev)
 	}
 
 	if (priv->tx_skb) {
-		dev_kfree_skb(priv->tx_skb);
+		dev_kfree_skb_irq(priv->tx_skb);
 		priv->tx_skb = NULL;
 	}
 
@@ -185,7 +185,6 @@ static int nps_enet_poll(struct napi_struct *napi, int budget)
 
 	buf_int_enable.rx_rdy = NPS_ENET_ENABLE;
 	buf_int_enable.tx_done = NPS_ENET_ENABLE;
-	nps_enet_tx_handler(ndev);
 	work_done = nps_enet_rx_handler(ndev);
 	if (work_done < budget) {
 		napi_complete(napi);
@@ -212,14 +211,18 @@ static irqreturn_t nps_enet_irq_handler(s32 irq, void *dev_instance)
 	struct net_device *ndev = dev_instance;
 	struct nps_enet_priv *priv = netdev_priv(ndev);
 	struct nps_enet_rx_ctl rx_ctrl;
-	struct nps_enet_tx_ctl tx_ctrl;
 
-	rx_ctrl.value = nps_enet_reg_get(priv, NPS_ENET_REG_RX_CTL);
-	tx_ctrl.value = nps_enet_reg_get(priv, NPS_ENET_REG_TX_CTL);
+	nps_enet_tx_handler(ndev);
 
-	if ((!tx_ctrl.ct && priv->tx_packet_sent) || rx_ctrl.cr)
+	rx_ctrl.value = nps_enet_reg_get(priv, NPS_ENET_REG_RX_CTL);
+	if (rx_ctrl.cr)
 		if (likely(napi_schedule_prep(&priv->napi))) {
-			nps_enet_reg_set(priv, NPS_ENET_REG_BUF_INT_ENABLE, 0);
+			struct nps_enet_buf_int_enable buf_int_enable;
+
+			buf_int_enable.rx_rdy = NPS_ENET_DISABLE;
+			buf_int_enable.tx_done = NPS_ENET_ENABLE;
+			nps_enet_reg_set(priv, NPS_ENET_REG_BUF_INT_ENABLE,
+					 buf_int_enable.value);
 			__napi_schedule(&priv->napi);
 		}
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [v1 3/6] NET: nps_enet: TX done race condition
  2015-08-17  5:58   ` [v1 2/6] NET: nps_enet: reduce processing latency Noam Camus
@ 2015-08-17  5:58     ` Noam Camus
  2015-08-17  5:58       ` [v1 4/6] NET: nps_enet: drop control frames Noam Camus
  0 siblings, 1 reply; 17+ messages in thread
From: Noam Camus @ 2015-08-17  5:58 UTC (permalink / raw)
  To: linux-kernel, netdev; +Cc: davem, Alexey.Brodkin, vgupta, talz, Noam Camus

From: Noam Camus <noamc@ezchip.com>

We need to set tx_skb pointer before send frame.
If we receive interrupt before we set pointer we will try
to free SKB with wrong pointer.
Now we are sure that SKB pointer will never be NULL during
handling TX done and check is removed.

Signed-off-by: Noam Camus <noamc@ezchip.com>
---
 drivers/net/ethernet/ezchip/nps_enet.c |   10 +++-------
 1 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/ezchip/nps_enet.c b/drivers/net/ethernet/ezchip/nps_enet.c
index af72181..f78ad3d 100644
--- a/drivers/net/ethernet/ezchip/nps_enet.c
+++ b/drivers/net/ethernet/ezchip/nps_enet.c
@@ -158,11 +158,7 @@ static void nps_enet_tx_handler(struct net_device *ndev)
 		ndev->stats.tx_bytes += tx_ctrl.nt;
 	}
 
-	if (priv->tx_skb) {
-		dev_kfree_skb_irq(priv->tx_skb);
-		priv->tx_skb = NULL;
-	}
-
+	dev_kfree_skb_irq(priv->tx_skb);
 	priv->tx_packet_sent = false;
 
 	if (netif_queue_stopped(ndev))
@@ -531,10 +527,10 @@ static netdev_tx_t nps_enet_start_xmit(struct sk_buff *skb,
 	/* This driver handles one frame at a time  */
 	netif_stop_queue(ndev);
 
-	nps_enet_send_frame(ndev, skb);
-
 	priv->tx_skb = skb;
 
+	nps_enet_send_frame(ndev, skb);
+
 	return NETDEV_TX_OK;
 }
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [v1 4/6] NET: nps_enet: drop control frames
  2015-08-17  5:58     ` [v1 3/6] NET: nps_enet: TX done race condition Noam Camus
@ 2015-08-17  5:58       ` Noam Camus
  2015-08-17  5:58         ` [v1 5/6] NET: nps_enet: TX done acknowledge Noam Camus
  0 siblings, 1 reply; 17+ messages in thread
From: Noam Camus @ 2015-08-17  5:58 UTC (permalink / raw)
  To: linux-kernel, netdev; +Cc: davem, Alexey.Brodkin, vgupta, talz, Noam Camus

From: Noam Camus <noamc@ezchip.com>

We set controller to drop control frames and not trying
to pass them on. This is only needed for debug reasons.

Signed-off-by: Noam Camus <noamc@ezchip.com>
---
 drivers/net/ethernet/ezchip/nps_enet.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ezchip/nps_enet.c b/drivers/net/ethernet/ezchip/nps_enet.c
index f78ad3d..0c13015 100644
--- a/drivers/net/ethernet/ezchip/nps_enet.c
+++ b/drivers/net/ethernet/ezchip/nps_enet.c
@@ -307,11 +307,8 @@ static void nps_enet_hw_enable_control(struct net_device *ndev)
 
 	/* Discard Packets bigger than max frame length */
 	max_frame_length = ETH_HLEN + ndev->mtu + ETH_FCS_LEN;
-	if (max_frame_length <= NPS_ENET_MAX_FRAME_LENGTH) {
+	if (max_frame_length <= NPS_ENET_MAX_FRAME_LENGTH)
 		ge_mac_cfg_3->max_len = max_frame_length;
-		nps_enet_reg_set(priv, NPS_ENET_REG_GE_MAC_CFG_3,
-				 ge_mac_cfg_3->value);
-	}
 
 	/* Enable interrupts */
 	buf_int_enable.rx_rdy = NPS_ENET_ENABLE;
@@ -339,11 +336,14 @@ static void nps_enet_hw_enable_control(struct net_device *ndev)
 	ge_mac_cfg_0.tx_fc_en = NPS_ENET_ENABLE;
 	ge_mac_cfg_0.rx_fc_en = NPS_ENET_ENABLE;
 	ge_mac_cfg_0.tx_fc_retr = NPS_ENET_GE_MAC_CFG_0_TX_FC_RETR;
+	ge_mac_cfg_3->cf_drop = NPS_ENET_ENABLE;
 
 	/* Enable Rx and Tx */
 	ge_mac_cfg_0.rx_en = NPS_ENET_ENABLE;
 	ge_mac_cfg_0.tx_en = NPS_ENET_ENABLE;
 
+	nps_enet_reg_set(priv, NPS_ENET_REG_GE_MAC_CFG_3,
+			 ge_mac_cfg_3->value);
 	nps_enet_reg_set(priv, NPS_ENET_REG_GE_MAC_CFG_0,
 			 ge_mac_cfg_0.value);
 }
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [v1 5/6] NET: nps_enet: TX done acknowledge.
  2015-08-17  5:58       ` [v1 4/6] NET: nps_enet: drop control frames Noam Camus
@ 2015-08-17  5:58         ` Noam Camus
  2015-08-17  5:58           ` [v1 6/6] NET: nps_enet: minor namespace cleanup Noam Camus
  0 siblings, 1 reply; 17+ messages in thread
From: Noam Camus @ 2015-08-17  5:58 UTC (permalink / raw)
  To: linux-kernel, netdev; +Cc: davem, Alexey.Brodkin, vgupta, talz, Noam Camus

From: Noam Camus <noamc@ezchip.com>

This is needed for when TX done interrupt is in
"level mode".
For example it is true for some simulators of this device.

Signed-off-by: Noam Camus <noamc@ezchip.com>
---
 drivers/net/ethernet/ezchip/nps_enet.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/ezchip/nps_enet.c b/drivers/net/ethernet/ezchip/nps_enet.c
index 0c13015..de4aafd 100644
--- a/drivers/net/ethernet/ezchip/nps_enet.c
+++ b/drivers/net/ethernet/ezchip/nps_enet.c
@@ -150,6 +150,9 @@ static void nps_enet_tx_handler(struct net_device *ndev)
 	if (!priv->tx_packet_sent || tx_ctrl.ct)
 		return;
 
+	/* Ack Tx ctrl register */
+	nps_enet_reg_set(priv, NPS_ENET_REG_TX_CTL, 0);
+
 	/* Check Tx transmit error */
 	if (unlikely(tx_ctrl.et)) {
 		ndev->stats.tx_errors++;
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [v1 6/6] NET: nps_enet: minor namespace cleanup
  2015-08-17  5:58         ` [v1 5/6] NET: nps_enet: TX done acknowledge Noam Camus
@ 2015-08-17  5:58           ` Noam Camus
  0 siblings, 0 replies; 17+ messages in thread
From: Noam Camus @ 2015-08-17  5:58 UTC (permalink / raw)
  To: linux-kernel, netdev; +Cc: davem, Alexey.Brodkin, vgupta, talz, Noam Camus

From: Noam Camus <noamc@ezchip.com>

We define buf_int_enable in the minimal namespace it is used.
Signed-off-by: Noam Camus <noamc@ezchip.com>
---
 drivers/net/ethernet/ezchip/nps_enet.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ezchip/nps_enet.c b/drivers/net/ethernet/ezchip/nps_enet.c
index de4aafd..3ba9be4 100644
--- a/drivers/net/ethernet/ezchip/nps_enet.c
+++ b/drivers/net/ethernet/ezchip/nps_enet.c
@@ -179,14 +179,15 @@ static int nps_enet_poll(struct napi_struct *napi, int budget)
 {
 	struct net_device *ndev = napi->dev;
 	struct nps_enet_priv *priv = netdev_priv(ndev);
-	struct nps_enet_buf_int_enable buf_int_enable;
 	u32 work_done;
 
-	buf_int_enable.rx_rdy = NPS_ENET_ENABLE;
-	buf_int_enable.tx_done = NPS_ENET_ENABLE;
 	work_done = nps_enet_rx_handler(ndev);
 	if (work_done < budget) {
+		struct nps_enet_buf_int_enable buf_int_enable;
+
 		napi_complete(napi);
+		buf_int_enable.rx_rdy = NPS_ENET_ENABLE;
+		buf_int_enable.tx_done = NPS_ENET_ENABLE;
 		nps_enet_reg_set(priv, NPS_ENET_REG_BUF_INT_ENABLE,
 				 buf_int_enable.value);
 	}
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [v1 0/6] *** nps_enet fixups ***
  2015-08-17  5:58 [v1 0/6] *** nps_enet fixups *** Noam Camus
  2015-08-17  5:58 ` [v1 1/6] NET: nps_enet: replace use of cause register Noam Camus
@ 2015-08-17 17:36 ` David Miller
  2015-08-18  5:04   ` Noam Camus
  2015-08-20  5:00   ` [v2 0/5] " Noam Camus
  1 sibling, 2 replies; 17+ messages in thread
From: David Miller @ 2015-08-17 17:36 UTC (permalink / raw)
  To: noamc; +Cc: linux-kernel, netdev, Alexey.Brodkin, vgupta, talz

From: Noam Camus <noamc@ezchip.com>
Date: Mon, 17 Aug 2015 08:58:33 +0300

> This patch set is a bunch of fixes to make nps_enet work correctly with
> all platforms, i.e. real device, emulation system, and simulation system.
> The main trigger for this patch set was that in our emulation system
> the TX end interrupt is "edge-sensitive" and therefore we cannot use the
> cause register since it is not sticky.
> Also:
> TX is handled during HW interrupt context and not NAPI job.
> race with TX done was fixed.
> added acknowledge for TX when device is "level sensitive".
> enable drop of control frames which is not needed for regular usage.
> 
> So most of this patch set is about TX handling, which is now more complete.

You should not move TX completion out of NAPI handling, NAPI poll is
exactly where it belongs.

If you handle it in hardware interrupt context you have to use
dev_kfree_skb_irq() which defers the operation to software interrupt
context anyways and is thus expensive.

Whereas if you keep TX completion in your NAPI handler the kfree is
handled synchronously and efficiently, as well as making SKB's
potentially available for RX reclaim.

I'm not applying this series, you are doing with TX handling exactly
what we tell people not to do.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [v1 0/6] *** nps_enet fixups ***
  2015-08-17 17:36 ` [v1 0/6] *** nps_enet fixups *** David Miller
@ 2015-08-18  5:04   ` Noam Camus
  2015-08-18  5:26     ` David Miller
  2015-08-20  5:00   ` [v2 0/5] " Noam Camus
  1 sibling, 1 reply; 17+ messages in thread
From: Noam Camus @ 2015-08-18  5:04 UTC (permalink / raw)
  To: David Miller; +Cc: linux-kernel, netdev, Alexey.Brodkin, vgupta, Tal Zilcer

From: David Miller [mailto:davem@davemloft.net] 
Sent: Monday, August 17, 2015 8:36 PM


> You should not move TX completion out of NAPI handling, NAPI poll is exactly where it belongs.
>
> If you handle it in hardware interrupt context you have to use
> dev_kfree_skb_irq() which defers the operation to software interrupt context anyways and is thus expensive.

> Whereas if you keep TX completion in your NAPI handler the kfree is handled synchronously and efficiently, as well as making SKB's potentially available for RX reclaim.

I followed "Hardware Architecture" section from:
http://www.linuxfoundation.org/collaborate/workgroups/networking/napi
and came up with "reduce processing latency" idea.

Anyway, I will restore TX completion back to NAPI poll.

> I'm not applying this series, you are doing with TX handling exactly what we tell people not to do.

I will come up with revised series in v2.

Noam

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [v1 0/6] *** nps_enet fixups ***
  2015-08-18  5:04   ` Noam Camus
@ 2015-08-18  5:26     ` David Miller
  0 siblings, 0 replies; 17+ messages in thread
From: David Miller @ 2015-08-18  5:26 UTC (permalink / raw)
  To: noamc; +Cc: linux-kernel, netdev, Alexey.Brodkin, vgupta, talz

From: Noam Camus <noamc@ezchip.com>
Date: Tue, 18 Aug 2015 05:04:20 +0000

> I followed "Hardware Architecture" section from:
> http://www.linuxfoundation.org/collaborate/workgroups/networking/napi
> and came up with "reduce processing latency" idea.

That document has lots of incorrect advice, that's for sure.

For one thing, it also says to count TX work against the budget, you
absolutely should not do this.  TX work is "free" compared to RX work
(which is several orders of magnitude more expensive) and therefore
should not count against the NAPI budget value.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [v2 0/5] *** nps_enet fixups ***
  2015-08-17 17:36 ` [v1 0/6] *** nps_enet fixups *** David Miller
  2015-08-18  5:04   ` Noam Camus
@ 2015-08-20  5:00   ` Noam Camus
  2015-08-20  5:00     ` [v2 1/5] NET: nps_enet: replace use of cause register Noam Camus
  2015-08-23 23:09     ` [v2 0/5] *** nps_enet fixups *** David Miller
  1 sibling, 2 replies; 17+ messages in thread
From: Noam Camus @ 2015-08-20  5:00 UTC (permalink / raw)
  To: linux-kernel, netdev; +Cc: davem, Alexey.Brodkin, vgupta, talz, Noam Camus

From: Noam Camus <noamc@ezchip.com>

Change v2
TX done is handled back with NAPI poll.

Change v1
This patch set is a bunch of fixes to make nps_enet work correctly with
all platforms, i.e. real device, emulation system, and simulation system.
The main trigger for this patch set was that in our emulation system
the TX end interrupt is "edge-sensitive" and therefore we cannot use the
cause register since it is not sticky.
Also:
TX is handled during HW interrupt context and not NAPI job.
race with TX done was fixed.
added acknowledge for TX when device is "level sensitive".
enable drop of control frames which is not needed for regular usage.

So most of this patch set is about TX handling, which is now more complete.

Noam Camus (5):
  NET: nps_enet: replace use of cause register
  NET: nps_enet: TX done race condition
  NET: nps_enet: drop control frames
  NET: nps_enet: TX done acknowledge.
  NET: nps_enet: minor namespace cleanup

 drivers/net/ethernet/ezchip/nps_enet.c |   37 ++++++++++++++++---------------
 drivers/net/ethernet/ezchip/nps_enet.h |   20 -----------------
 2 files changed, 19 insertions(+), 38 deletions(-)


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [v2 1/5] NET: nps_enet: replace use of cause register
  2015-08-20  5:00   ` [v2 0/5] " Noam Camus
@ 2015-08-20  5:00     ` Noam Camus
  2015-08-20  5:00       ` [v2 2/5] NET: nps_enet: TX done race condition Noam Camus
  2015-08-23 23:09     ` [v2 0/5] *** nps_enet fixups *** David Miller
  1 sibling, 1 reply; 17+ messages in thread
From: Noam Camus @ 2015-08-20  5:00 UTC (permalink / raw)
  To: linux-kernel, netdev; +Cc: davem, Alexey.Brodkin, vgupta, talz, Noam Camus

From: Noam Camus <noamc@ezchip.com>

When interrupt is received we read directly from control
register for RX/TX instead of reading cause register
since this register fails to indicate TX done when
TX interrupt is "edge mode".

Signed-off-by: Noam Camus <noamc@ezchip.com>
---
 drivers/net/ethernet/ezchip/nps_enet.c |    9 +++++----
 drivers/net/ethernet/ezchip/nps_enet.h |   20 --------------------
 2 files changed, 5 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ethernet/ezchip/nps_enet.c b/drivers/net/ethernet/ezchip/nps_enet.c
index 24a85b2..0e652b4 100644
--- a/drivers/net/ethernet/ezchip/nps_enet.c
+++ b/drivers/net/ethernet/ezchip/nps_enet.c
@@ -211,12 +211,13 @@ static irqreturn_t nps_enet_irq_handler(s32 irq, void *dev_instance)
 {
 	struct net_device *ndev = dev_instance;
 	struct nps_enet_priv *priv = netdev_priv(ndev);
-	struct nps_enet_buf_int_cause buf_int_cause;
+	struct nps_enet_rx_ctl rx_ctrl;
+	struct nps_enet_tx_ctl tx_ctrl;
 
-	buf_int_cause.value =
-			nps_enet_reg_get(priv, NPS_ENET_REG_BUF_INT_CAUSE);
+	rx_ctrl.value = nps_enet_reg_get(priv, NPS_ENET_REG_RX_CTL);
+	tx_ctrl.value = nps_enet_reg_get(priv, NPS_ENET_REG_TX_CTL);
 
-	if (buf_int_cause.tx_done || buf_int_cause.rx_rdy)
+	if ((!tx_ctrl.ct && priv->tx_packet_sent) || rx_ctrl.cr)
 		if (likely(napi_schedule_prep(&priv->napi))) {
 			nps_enet_reg_set(priv, NPS_ENET_REG_BUF_INT_ENABLE, 0);
 			__napi_schedule(&priv->napi);
diff --git a/drivers/net/ethernet/ezchip/nps_enet.h b/drivers/net/ethernet/ezchip/nps_enet.h
index fc45c9d..6703674 100644
--- a/drivers/net/ethernet/ezchip/nps_enet.h
+++ b/drivers/net/ethernet/ezchip/nps_enet.h
@@ -36,7 +36,6 @@
 #define NPS_ENET_REG_RX_CTL		0x810
 #define NPS_ENET_REG_RX_BUF		0x818
 #define NPS_ENET_REG_BUF_INT_ENABLE	0x8C0
-#define NPS_ENET_REG_BUF_INT_CAUSE	0x8C4
 #define NPS_ENET_REG_GE_MAC_CFG_0	0x1000
 #define NPS_ENET_REG_GE_MAC_CFG_1	0x1004
 #define NPS_ENET_REG_GE_MAC_CFG_2	0x1008
@@ -108,25 +107,6 @@ struct nps_enet_buf_int_enable {
 	};
 };
 
-/* Interrupt cause for data buffer events register */
-struct nps_enet_buf_int_cause {
-	union {
-		/* tx_done: Interrupt in the case when current frame was
-		 *          read from TX buffer.
-		 * rx_rdy:  Interrupt in the case when new frame is ready
-		 *          in RX buffer.
-		 */
-		struct {
-			u32
-			__reserved:30,
-			tx_done:1,
-			rx_rdy:1;
-		};
-
-		u32 value;
-	};
-};
-
 /* Gbps Eth MAC Configuration 0 register */
 struct nps_enet_ge_mac_cfg_0 {
 	union {
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [v2 2/5] NET: nps_enet: TX done race condition
  2015-08-20  5:00     ` [v2 1/5] NET: nps_enet: replace use of cause register Noam Camus
@ 2015-08-20  5:00       ` Noam Camus
  2015-08-20  5:00         ` [v2 3/5] NET: nps_enet: drop control frames Noam Camus
  0 siblings, 1 reply; 17+ messages in thread
From: Noam Camus @ 2015-08-20  5:00 UTC (permalink / raw)
  To: linux-kernel, netdev; +Cc: davem, Alexey.Brodkin, vgupta, talz, Noam Camus

From: Noam Camus <noamc@ezchip.com>

We need to set tx_skb pointer before send frame.
If we receive interrupt before we set pointer we will try
to free SKB with wrong pointer.
Now we are sure that SKB pointer will never be NULL during
handling TX done and check is removed.

Signed-off-by: Noam Camus <noamc@ezchip.com>
---
 drivers/net/ethernet/ezchip/nps_enet.c |   10 +++-------
 1 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/ezchip/nps_enet.c b/drivers/net/ethernet/ezchip/nps_enet.c
index 0e652b4..8b25f24 100644
--- a/drivers/net/ethernet/ezchip/nps_enet.c
+++ b/drivers/net/ethernet/ezchip/nps_enet.c
@@ -158,11 +158,7 @@ static void nps_enet_tx_handler(struct net_device *ndev)
 		ndev->stats.tx_bytes += tx_ctrl.nt;
 	}
 
-	if (priv->tx_skb) {
-		dev_kfree_skb(priv->tx_skb);
-		priv->tx_skb = NULL;
-	}
-
+	dev_kfree_skb(priv->tx_skb);
 	priv->tx_packet_sent = false;
 
 	if (netif_queue_stopped(ndev))
@@ -528,10 +524,10 @@ static netdev_tx_t nps_enet_start_xmit(struct sk_buff *skb,
 	/* This driver handles one frame at a time  */
 	netif_stop_queue(ndev);
 
-	nps_enet_send_frame(ndev, skb);
-
 	priv->tx_skb = skb;
 
+	nps_enet_send_frame(ndev, skb);
+
 	return NETDEV_TX_OK;
 }
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [v2 3/5] NET: nps_enet: drop control frames
  2015-08-20  5:00       ` [v2 2/5] NET: nps_enet: TX done race condition Noam Camus
@ 2015-08-20  5:00         ` Noam Camus
  2015-08-20  5:00           ` [v2 4/5] NET: nps_enet: TX done acknowledge Noam Camus
  0 siblings, 1 reply; 17+ messages in thread
From: Noam Camus @ 2015-08-20  5:00 UTC (permalink / raw)
  To: linux-kernel, netdev; +Cc: davem, Alexey.Brodkin, vgupta, talz, Noam Camus

From: Noam Camus <noamc@ezchip.com>

We set controller to drop control frames and not trying
to pass them on. This is only needed for debug reasons.

Signed-off-by: Noam Camus <noamc@ezchip.com>
---
 drivers/net/ethernet/ezchip/nps_enet.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ezchip/nps_enet.c b/drivers/net/ethernet/ezchip/nps_enet.c
index 8b25f24..e553e6a 100644
--- a/drivers/net/ethernet/ezchip/nps_enet.c
+++ b/drivers/net/ethernet/ezchip/nps_enet.c
@@ -304,11 +304,8 @@ static void nps_enet_hw_enable_control(struct net_device *ndev)
 
 	/* Discard Packets bigger than max frame length */
 	max_frame_length = ETH_HLEN + ndev->mtu + ETH_FCS_LEN;
-	if (max_frame_length <= NPS_ENET_MAX_FRAME_LENGTH) {
+	if (max_frame_length <= NPS_ENET_MAX_FRAME_LENGTH)
 		ge_mac_cfg_3->max_len = max_frame_length;
-		nps_enet_reg_set(priv, NPS_ENET_REG_GE_MAC_CFG_3,
-				 ge_mac_cfg_3->value);
-	}
 
 	/* Enable interrupts */
 	buf_int_enable.rx_rdy = NPS_ENET_ENABLE;
@@ -336,11 +333,14 @@ static void nps_enet_hw_enable_control(struct net_device *ndev)
 	ge_mac_cfg_0.tx_fc_en = NPS_ENET_ENABLE;
 	ge_mac_cfg_0.rx_fc_en = NPS_ENET_ENABLE;
 	ge_mac_cfg_0.tx_fc_retr = NPS_ENET_GE_MAC_CFG_0_TX_FC_RETR;
+	ge_mac_cfg_3->cf_drop = NPS_ENET_ENABLE;
 
 	/* Enable Rx and Tx */
 	ge_mac_cfg_0.rx_en = NPS_ENET_ENABLE;
 	ge_mac_cfg_0.tx_en = NPS_ENET_ENABLE;
 
+	nps_enet_reg_set(priv, NPS_ENET_REG_GE_MAC_CFG_3,
+			 ge_mac_cfg_3->value);
 	nps_enet_reg_set(priv, NPS_ENET_REG_GE_MAC_CFG_0,
 			 ge_mac_cfg_0.value);
 }
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [v2 4/5] NET: nps_enet: TX done acknowledge.
  2015-08-20  5:00         ` [v2 3/5] NET: nps_enet: drop control frames Noam Camus
@ 2015-08-20  5:00           ` Noam Camus
  2015-08-20  5:00             ` [v2 5/5] NET: nps_enet: minor namespace cleanup Noam Camus
  0 siblings, 1 reply; 17+ messages in thread
From: Noam Camus @ 2015-08-20  5:00 UTC (permalink / raw)
  To: linux-kernel, netdev; +Cc: davem, Alexey.Brodkin, vgupta, talz, Noam Camus

From: Noam Camus <noamc@ezchip.com>

This is needed for when TX done interrupt is in
"level mode".
For example it is true for some simulators of this device.

Signed-off-by: Noam Camus <noamc@ezchip.com>
---
 drivers/net/ethernet/ezchip/nps_enet.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/ezchip/nps_enet.c b/drivers/net/ethernet/ezchip/nps_enet.c
index e553e6a..69b9129 100644
--- a/drivers/net/ethernet/ezchip/nps_enet.c
+++ b/drivers/net/ethernet/ezchip/nps_enet.c
@@ -150,6 +150,9 @@ static void nps_enet_tx_handler(struct net_device *ndev)
 	if (!priv->tx_packet_sent || tx_ctrl.ct)
 		return;
 
+	/* Ack Tx ctrl register */
+	nps_enet_reg_set(priv, NPS_ENET_REG_TX_CTL, 0);
+
 	/* Check Tx transmit error */
 	if (unlikely(tx_ctrl.et)) {
 		ndev->stats.tx_errors++;
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [v2 5/5] NET: nps_enet: minor namespace cleanup
  2015-08-20  5:00           ` [v2 4/5] NET: nps_enet: TX done acknowledge Noam Camus
@ 2015-08-20  5:00             ` Noam Camus
  0 siblings, 0 replies; 17+ messages in thread
From: Noam Camus @ 2015-08-20  5:00 UTC (permalink / raw)
  To: linux-kernel, netdev; +Cc: davem, Alexey.Brodkin, vgupta, talz, Noam Camus

From: Noam Camus <noamc@ezchip.com>

We define buf_int_enable in the minimal namespace it is used.
Signed-off-by: Noam Camus <noamc@ezchip.com>
---
 drivers/net/ethernet/ezchip/nps_enet.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ezchip/nps_enet.c b/drivers/net/ethernet/ezchip/nps_enet.c
index 69b9129..63c2bcf 100644
--- a/drivers/net/ethernet/ezchip/nps_enet.c
+++ b/drivers/net/ethernet/ezchip/nps_enet.c
@@ -179,15 +179,16 @@ static int nps_enet_poll(struct napi_struct *napi, int budget)
 {
 	struct net_device *ndev = napi->dev;
 	struct nps_enet_priv *priv = netdev_priv(ndev);
-	struct nps_enet_buf_int_enable buf_int_enable;
 	u32 work_done;
 
-	buf_int_enable.rx_rdy = NPS_ENET_ENABLE;
-	buf_int_enable.tx_done = NPS_ENET_ENABLE;
 	nps_enet_tx_handler(ndev);
 	work_done = nps_enet_rx_handler(ndev);
 	if (work_done < budget) {
+		struct nps_enet_buf_int_enable buf_int_enable;
+
 		napi_complete(napi);
+		buf_int_enable.rx_rdy = NPS_ENET_ENABLE;
+		buf_int_enable.tx_done = NPS_ENET_ENABLE;
 		nps_enet_reg_set(priv, NPS_ENET_REG_BUF_INT_ENABLE,
 				 buf_int_enable.value);
 	}
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [v2 0/5] *** nps_enet fixups ***
  2015-08-20  5:00   ` [v2 0/5] " Noam Camus
  2015-08-20  5:00     ` [v2 1/5] NET: nps_enet: replace use of cause register Noam Camus
@ 2015-08-23 23:09     ` David Miller
  1 sibling, 0 replies; 17+ messages in thread
From: David Miller @ 2015-08-23 23:09 UTC (permalink / raw)
  To: noamc; +Cc: linux-kernel, netdev, Alexey.Brodkin, vgupta, talz

From: Noam Camus <noamc@ezchip.com>
Date: Thu, 20 Aug 2015 08:00:00 +0300

> Change v2
> TX done is handled back with NAPI poll.
> 
> Change v1
> This patch set is a bunch of fixes to make nps_enet work correctly with
> all platforms, i.e. real device, emulation system, and simulation system.
> The main trigger for this patch set was that in our emulation system
> the TX end interrupt is "edge-sensitive" and therefore we cannot use the
> cause register since it is not sticky.
> Also:
> TX is handled during HW interrupt context and not NAPI job.
> race with TX done was fixed.
> added acknowledge for TX when device is "level sensitive".
> enable drop of control frames which is not needed for regular usage.
> 
> So most of this patch set is about TX handling, which is now more complete.

Series applied, thank you.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2015-08-23 23:09 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-17  5:58 [v1 0/6] *** nps_enet fixups *** Noam Camus
2015-08-17  5:58 ` [v1 1/6] NET: nps_enet: replace use of cause register Noam Camus
2015-08-17  5:58   ` [v1 2/6] NET: nps_enet: reduce processing latency Noam Camus
2015-08-17  5:58     ` [v1 3/6] NET: nps_enet: TX done race condition Noam Camus
2015-08-17  5:58       ` [v1 4/6] NET: nps_enet: drop control frames Noam Camus
2015-08-17  5:58         ` [v1 5/6] NET: nps_enet: TX done acknowledge Noam Camus
2015-08-17  5:58           ` [v1 6/6] NET: nps_enet: minor namespace cleanup Noam Camus
2015-08-17 17:36 ` [v1 0/6] *** nps_enet fixups *** David Miller
2015-08-18  5:04   ` Noam Camus
2015-08-18  5:26     ` David Miller
2015-08-20  5:00   ` [v2 0/5] " Noam Camus
2015-08-20  5:00     ` [v2 1/5] NET: nps_enet: replace use of cause register Noam Camus
2015-08-20  5:00       ` [v2 2/5] NET: nps_enet: TX done race condition Noam Camus
2015-08-20  5:00         ` [v2 3/5] NET: nps_enet: drop control frames Noam Camus
2015-08-20  5:00           ` [v2 4/5] NET: nps_enet: TX done acknowledge Noam Camus
2015-08-20  5:00             ` [v2 5/5] NET: nps_enet: minor namespace cleanup Noam Camus
2015-08-23 23:09     ` [v2 0/5] *** nps_enet fixups *** David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).