2015-08-17 05:59:55

by Noam Camus

[permalink] [raw]
Subject: [v1 0/6] *** nps_enet fixups ***

From: Noam Camus <[email protected]>

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(-)


2015-08-17 06:00:20

by Noam Camus

[permalink] [raw]
Subject: [v1 1/6] NET: nps_enet: replace use of cause register

From: Noam Camus <[email protected]>

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 <[email protected]>
---
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

2015-08-17 06:00:44

by Noam Camus

[permalink] [raw]
Subject: [v1 2/6] NET: nps_enet: reduce processing latency.

From: Noam Camus <[email protected]>

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 <[email protected]>
---
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

2015-08-17 06:01:09

by Noam Camus

[permalink] [raw]
Subject: [v1 3/6] NET: nps_enet: TX done race condition

From: Noam Camus <[email protected]>

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 <[email protected]>
---
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

2015-08-17 06:01:32

by Noam Camus

[permalink] [raw]
Subject: [v1 4/6] NET: nps_enet: drop control frames

From: Noam Camus <[email protected]>

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 <[email protected]>
---
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

2015-08-17 06:01:57

by Noam Camus

[permalink] [raw]
Subject: [v1 5/6] NET: nps_enet: TX done acknowledge.

From: Noam Camus <[email protected]>

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 <[email protected]>
---
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

2015-08-17 06:02:23

by Noam Camus

[permalink] [raw]
Subject: [v1 6/6] NET: nps_enet: minor namespace cleanup

From: Noam Camus <[email protected]>

We define buf_int_enable in the minimal namespace it is used.
Signed-off-by: Noam Camus <[email protected]>
---
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

2015-08-17 17:36:10

by David Miller

[permalink] [raw]
Subject: Re: [v1 0/6] *** nps_enet fixups ***

From: Noam Camus <[email protected]>
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.

2015-08-18 05:04:26

by Noam Camus

[permalink] [raw]
Subject: RE: [v1 0/6] *** nps_enet fixups ***

From: David Miller [mailto:[email protected]]
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

2015-08-18 05:26:25

by David Miller

[permalink] [raw]
Subject: Re: [v1 0/6] *** nps_enet fixups ***

From: Noam Camus <[email protected]>
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.

2015-08-20 05:05:25

by Noam Camus

[permalink] [raw]
Subject: [v2 0/5] *** nps_enet fixups ***

From: Noam Camus <[email protected]>

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(-)

2015-08-20 05:05:50

by Noam Camus

[permalink] [raw]
Subject: [v2 1/5] NET: nps_enet: replace use of cause register

From: Noam Camus <[email protected]>

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 <[email protected]>
---
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

2015-08-20 05:06:08

by Noam Camus

[permalink] [raw]
Subject: [v2 2/5] NET: nps_enet: TX done race condition

From: Noam Camus <[email protected]>

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 <[email protected]>
---
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

2015-08-20 05:06:32

by Noam Camus

[permalink] [raw]
Subject: [v2 3/5] NET: nps_enet: drop control frames

From: Noam Camus <[email protected]>

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 <[email protected]>
---
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

2015-08-20 05:06:55

by Noam Camus

[permalink] [raw]
Subject: [v2 4/5] NET: nps_enet: TX done acknowledge.

From: Noam Camus <[email protected]>

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 <[email protected]>
---
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

2015-08-20 05:07:12

by Noam Camus

[permalink] [raw]
Subject: [v2 5/5] NET: nps_enet: minor namespace cleanup

From: Noam Camus <[email protected]>

We define buf_int_enable in the minimal namespace it is used.
Signed-off-by: Noam Camus <[email protected]>
---
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

2015-08-23 23:09:12

by David Miller

[permalink] [raw]
Subject: Re: [v2 0/5] *** nps_enet fixups ***

From: Noam Camus <[email protected]>
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.