linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [Patch net-next 0/4] net: fec: add interrupt coalescence
@ 2014-09-15 17:12 Frank.Li at freescale.com
  2014-09-15 17:12 ` [Patch net-next 1/4] net: fec: refine error handle of parser queue number from DT Frank.Li at freescale.com
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Frank.Li at freescale.com @ 2014-09-15 17:12 UTC (permalink / raw)
  To: linux-arm-kernel

From: Frank Li <Frank.Li@freescale.com>

improve error handle when parse queue number.
add interrupt coalescence feature.

Frank Li (1):
  net: fec: refine error handle of parser queue number from DT

Fugang Duan (3):
  net: fec: add interrupt coalescence feature support
  net:fec: increase DMA queue number
  net: fec: Workaround for imx6sx enet tx hang when enable three queues

 drivers/net/ethernet/freescale/fec.h      |  16 +++-
 drivers/net/ethernet/freescale/fec_main.c | 144 ++++++++++++++++++++++++++++--
 2 files changed, 150 insertions(+), 10 deletions(-)

-- 
1.9.1

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

* [Patch net-next 1/4] net: fec: refine error handle of parser queue number from DT
  2014-09-15 17:12 [Patch net-next 0/4] net: fec: add interrupt coalescence Frank.Li at freescale.com
@ 2014-09-15 17:12 ` Frank.Li at freescale.com
  2014-09-15 21:32   ` David Miller
  2014-09-15 17:12 ` [Patch net-next 2/4] net: fec: add interrupt coalescence feature support Frank.Li at freescale.com
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Frank.Li at freescale.com @ 2014-09-15 17:12 UTC (permalink / raw)
  To: linux-arm-kernel

From: Frank Li <Frank.Li@freescale.com>

check tx and rx queue seperately.
fix typo, "Invalidate" and "fail".
change pr_err to pr_warn.

Signed-off-by: Frank Li <Frank.Li@freescale.com>
---
 drivers/net/ethernet/freescale/fec_main.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 8f8e55e..f958e91 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2890,22 +2890,22 @@ fec_enet_get_queue_num(struct platform_device *pdev, int *num_tx, int *num_rx)
 
 	/* parse the num of tx and rx queues */
 	err = of_property_read_u32(np, "fsl,num-tx-queues", num_tx);
-	err |= of_property_read_u32(np, "fsl,num-rx-queues", num_rx);
-	if (err) {
+	if (err)
 		*num_tx = 1;
+
+	err = of_property_read_u32(np, "fsl,num-rx-queues", num_rx);
+	if (err)
 		*num_rx = 1;
-		return;
-	}
 
 	if (*num_tx < 1 || *num_tx > FEC_ENET_MAX_TX_QS) {
-		dev_err(&pdev->dev, "Invalidate num_tx(=%d), fail back to 1\n",
+		dev_warn(&pdev->dev, "Invalid num_tx(=%d), fall back to 1\n",
 			*num_tx);
 		*num_tx = 1;
 		return;
 	}
 
 	if (*num_rx < 1 || *num_rx > FEC_ENET_MAX_RX_QS) {
-		dev_err(&pdev->dev, "Invalidate num_rx(=%d), fail back to 1\n",
+		dev_warn(&pdev->dev, "Invalid num_rx(=%d), fall back to 1\n",
 			*num_rx);
 		*num_rx = 1;
 		return;
@@ -2924,8 +2924,8 @@ fec_probe(struct platform_device *pdev)
 	const struct of_device_id *of_id;
 	static int dev_id;
 	struct device_node *np = pdev->dev.of_node, *phy_node;
-	int num_tx_qs = 1;
-	int num_rx_qs = 1;
+	int num_tx_qs;
+	int num_rx_qs;
 
 	of_id = of_match_device(fec_dt_ids, &pdev->dev);
 	if (of_id)
-- 
1.9.1

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

* [Patch net-next 2/4] net: fec: add interrupt coalescence feature support
  2014-09-15 17:12 [Patch net-next 0/4] net: fec: add interrupt coalescence Frank.Li at freescale.com
  2014-09-15 17:12 ` [Patch net-next 1/4] net: fec: refine error handle of parser queue number from DT Frank.Li at freescale.com
@ 2014-09-15 17:12 ` Frank.Li at freescale.com
  2014-09-15 21:34   ` David Miller
  2014-09-15 17:12 ` [Patch net-next 3/4] net:fec: increase DMA queue number Frank.Li at freescale.com
  2014-09-15 17:12 ` [Patch net-next 4/4] net: fec: Workaround for imx6sx enet tx hang when enable three queues Frank.Li at freescale.com
  3 siblings, 1 reply; 10+ messages in thread
From: Frank.Li at freescale.com @ 2014-09-15 17:12 UTC (permalink / raw)
  To: linux-arm-kernel

From: Fugang Duan <B38611@freescale.com>

i.MX6 SX support interrupt coalescence feature
By default, init the interrupt coalescing frame count threshold and
timer threshold.

Supply the ethtool interfaces as below for user tuning to improve
enet performance:
	rx_max_coalesced_frames
	rx_coalesce_usecs
	tx_max_coalesced_frames
	tx_coalesce_usecs

Signed-off-by: Fugang Duan <B38611@freescale.com>
Signed-off-by: Frank Li <Frank.Li@freescale.com>
---
 drivers/net/ethernet/freescale/fec.h      |  14 ++++
 drivers/net/ethernet/freescale/fec_main.c | 111 ++++++++++++++++++++++++++++++
 2 files changed, 125 insertions(+)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index b7c7722..f13e319 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -335,6 +335,14 @@ struct bufdesc_ex {
 #define FEC_DEFAULT_IMASK (FEC_ENET_TXF | FEC_ENET_RXF | FEC_ENET_MII | FEC_ENET_TS_TIMER)
 #define FEC_RX_DISABLED_IMASK (FEC_DEFAULT_IMASK & (~FEC_ENET_RXF))
 
+/* ENET interrupt coalescing macro define */
+#define FEC_ITR_CLK_SEL		(0x1 << 30)
+#define FEC_ITR_EN		(0x1 << 31)
+#define FEC_ITR_ICFT(X)		((X & 0xFF) << 20)
+#define FEC_ITR_ICTT(X)		((X) & 0xFFFF)
+#define FEC_ITR_ICFT_DEFAULT	200  /* Set 200 frame count threshold */
+#define FEC_ITR_ICTT_DEFAULT	1000 /* Set 1000us timer threshold */
+
 #define FEC_VLAN_TAG_LEN       0x04
 #define FEC_ETHTYPE_LEN                0x02
 
@@ -446,6 +454,12 @@ struct fec_enet_private {
 
 	unsigned int tx_align;
 	unsigned int rx_align;
+
+	/* hw interrupt coalesce */
+	uint rx_pkts_itr;
+	uint rx_time_itr;
+	uint tx_pkts_itr;
+	uint tx_time_itr;
 };
 
 void fec_ptp_init(struct platform_device *pdev);
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index f958e91..524ddbe 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -63,6 +63,7 @@
 #include "fec.h"
 
 static void set_multicast_list(struct net_device *ndev);
+static void fec_enet_itr_coal_init(struct net_device *ndev);
 
 #define DRIVER_NAME	"fec"
 
@@ -1095,6 +1096,10 @@ fec_restart(struct net_device *ndev)
 
 	/* Enable interrupts we wish to service */
 	writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
+
+	/* Init the interrupt coalescing */
+	fec_enet_itr_coal_init(ndev);
+
 }
 
 static void
@@ -2234,12 +2239,118 @@ static int fec_enet_nway_reset(struct net_device *dev)
 	return genphy_restart_aneg(phydev);
 }
 
+/*
+ * ITR clock source is enet system clock (clk_ahb).
+ * TCTT unit is cycle_ns * 64 cycle
+ * So, the ICTT value = X us / (cycle_ns * 64)
+ */
+static int fec_enet_us_to_itr_clock(struct net_device *ndev, int us)
+{
+	struct fec_enet_private *fep = netdev_priv(ndev);
+
+	return us * (clk_get_rate(fep->clk_ahb) / 64000) / 1000;
+}
+
+/* Set threshold for interrupt coalescing */
+static void fec_enet_itr_coal_set(struct net_device *ndev)
+{
+	struct fec_enet_private *fep = netdev_priv(ndev);
+	const struct platform_device_id *id_entry =
+				platform_get_device_id(fep->pdev);
+	int rx_itr, tx_itr;
+
+	if (!(id_entry->driver_data & FEC_QUIRK_HAS_AVB))
+		return;
+
+	/* Must be greater than zero to avoid unpredictable behavior */
+	if (!fep->rx_time_itr || !fep->rx_pkts_itr ||
+		!fep->tx_time_itr || !fep->tx_pkts_itr)
+		return;
+	/*
+	 * Select enet system clock as Interrupt Coalescing
+	 * timer Clock Source
+	 */
+	rx_itr = FEC_ITR_CLK_SEL;
+	tx_itr = FEC_ITR_CLK_SEL;
+
+	/* set ICFT and ICTT */
+	rx_itr |= FEC_ITR_ICFT(fep->rx_pkts_itr);
+	rx_itr |= FEC_ITR_ICTT(fec_enet_us_to_itr_clock(ndev, fep->rx_time_itr));
+	tx_itr |= FEC_ITR_ICFT(fep->tx_pkts_itr);
+	tx_itr |= FEC_ITR_ICTT(fec_enet_us_to_itr_clock(ndev, fep->tx_time_itr));
+
+	rx_itr |= FEC_ITR_EN;
+	tx_itr |= FEC_ITR_EN;
+
+	writel(tx_itr, fep->hwp + FEC_TXIC0);
+	writel(rx_itr, fep->hwp + FEC_RXIC0);
+	writel(tx_itr, fep->hwp + FEC_TXIC1);
+	writel(rx_itr, fep->hwp + FEC_RXIC1);
+	writel(tx_itr, fep->hwp + FEC_TXIC2);
+	writel(rx_itr, fep->hwp + FEC_RXIC2);
+}
+
+static int fec_enet_get_coalesce(struct net_device *ndev,
+				struct ethtool_coalesce *ec)
+{
+	struct fec_enet_private *fep = netdev_priv(ndev);
+	const struct platform_device_id *id_entry =
+				platform_get_device_id(fep->pdev);
+
+	if (!(id_entry->driver_data & FEC_QUIRK_HAS_AVB))
+		return -EOPNOTSUPP;
+
+	ec->rx_coalesce_usecs = fep->rx_time_itr;
+	ec->rx_max_coalesced_frames = fep->rx_pkts_itr;
+
+	ec->tx_coalesce_usecs = fep->tx_time_itr;
+	ec->tx_max_coalesced_frames = fep->tx_pkts_itr;
+
+	return 0;
+}
+
+static int fec_enet_set_coalesce(struct net_device *ndev,
+				struct ethtool_coalesce *ec)
+{
+	struct fec_enet_private *fep = netdev_priv(ndev);
+	const struct platform_device_id *id_entry =
+				platform_get_device_id(fep->pdev);
+
+	if (!(id_entry->driver_data & FEC_QUIRK_HAS_AVB))
+		return -EOPNOTSUPP;
+
+	fep->rx_time_itr = ec->rx_coalesce_usecs;
+	fep->rx_pkts_itr = ec->rx_max_coalesced_frames;
+
+	fep->tx_time_itr = ec->tx_coalesce_usecs;
+	fep->tx_pkts_itr = ec->tx_max_coalesced_frames;
+
+	fec_enet_itr_coal_set(ndev);
+
+	return 0;
+}
+
+static void fec_enet_itr_coal_init(struct net_device *ndev)
+{
+	struct ethtool_coalesce ec;
+
+	ec.rx_coalesce_usecs = FEC_ITR_ICTT_DEFAULT;
+	ec.rx_max_coalesced_frames = FEC_ITR_ICFT_DEFAULT;
+
+	ec.tx_coalesce_usecs = FEC_ITR_ICTT_DEFAULT;
+	ec.tx_max_coalesced_frames = FEC_ITR_ICFT_DEFAULT;
+
+	fec_enet_set_coalesce(ndev, &ec);
+}
+
 static const struct ethtool_ops fec_enet_ethtool_ops = {
 	.get_settings		= fec_enet_get_settings,
 	.set_settings		= fec_enet_set_settings,
 	.get_drvinfo		= fec_enet_get_drvinfo,
 	.nway_reset		= fec_enet_nway_reset,
 	.get_link		= ethtool_op_get_link,
+	.get_coalesce		= fec_enet_get_coalesce,
+	.set_coalesce		= fec_enet_set_coalesce,
 #ifndef CONFIG_M5272
 	.get_pauseparam		= fec_enet_get_pauseparam,
 	.set_pauseparam		= fec_enet_set_pauseparam,
-- 
1.9.1

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

* [Patch net-next 3/4] net:fec: increase DMA queue number
  2014-09-15 17:12 [Patch net-next 0/4] net: fec: add interrupt coalescence Frank.Li at freescale.com
  2014-09-15 17:12 ` [Patch net-next 1/4] net: fec: refine error handle of parser queue number from DT Frank.Li at freescale.com
  2014-09-15 17:12 ` [Patch net-next 2/4] net: fec: add interrupt coalescence feature support Frank.Li at freescale.com
@ 2014-09-15 17:12 ` Frank.Li at freescale.com
  2014-09-15 17:12 ` [Patch net-next 4/4] net: fec: Workaround for imx6sx enet tx hang when enable three queues Frank.Li at freescale.com
  3 siblings, 0 replies; 10+ messages in thread
From: Frank.Li at freescale.com @ 2014-09-15 17:12 UTC (permalink / raw)
  To: linux-arm-kernel

From: Fugang Duan <B38611@freescale.com>

when enable interrupt coalesce, 8 BD is not enough.

Signed-off-by: Frank Li <Frank.Li@freescale.com>
---
 drivers/net/ethernet/freescale/fec.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index f13e319..c61eff6 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -296,7 +296,7 @@ struct bufdesc_ex {
  * the skbuffer directly.
  */
 
-#define FEC_ENET_RX_PAGES	8
+#define FEC_ENET_RX_PAGES	256
 #define FEC_ENET_RX_FRSIZE	2048
 #define FEC_ENET_RX_FRPPG	(PAGE_SIZE / FEC_ENET_RX_FRSIZE)
 #define RX_RING_SIZE		(FEC_ENET_RX_FRPPG * FEC_ENET_RX_PAGES)
-- 
1.9.1

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

* [Patch net-next 4/4] net: fec: Workaround for imx6sx enet tx hang when enable three queues
  2014-09-15 17:12 [Patch net-next 0/4] net: fec: add interrupt coalescence Frank.Li at freescale.com
                   ` (2 preceding siblings ...)
  2014-09-15 17:12 ` [Patch net-next 3/4] net:fec: increase DMA queue number Frank.Li at freescale.com
@ 2014-09-15 17:12 ` Frank.Li at freescale.com
  2014-09-15 21:35   ` David Miller
  2014-09-16  0:52   ` Shawn Guo
  3 siblings, 2 replies; 10+ messages in thread
From: Frank.Li at freescale.com @ 2014-09-15 17:12 UTC (permalink / raw)
  To: linux-arm-kernel

From: Fugang Duan <B38611@freescale.com>

When enable three queues on imx6sx enet, and then do tx performance
test with iperf tool, after some time running, tx hang.

Found that:
	If uDMA is running, software set TDAR may cause tx hang.
	If uDMA is in idle, software set TDAR don't cause tx hang.

There is a TDAR race condition for mutliQ when the software sets TDAR
and the UDMA clears TDAR simultaneously or in a small window (2-4 cycles).
This will cause the udma_tx and udma_tx_arbiter state machines to hang.
The issue exist at i.MX6SX enet IP.

So, the Workaround is checking TDAR status four time, if TDAR cleared by
hardware and then write TDAR, otherwise don't set TDAR.

The patch is only one Workaround for the issue TKT210582.

Signed-off-by: Fugang Duan <B38611@freescale.com>
Signed-off-by: Frank Li <Frank.Li@freescale.com>
---
 drivers/net/ethernet/freescale/fec_main.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 524ddbe..452e576 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -111,6 +111,13 @@ static void fec_enet_itr_coal_init(struct net_device *ndev);
  *   independent rings
  */
 #define FEC_QUIRK_HAS_AVB		(1 << 8)
+/*
+ * There is a TDAR race condition for mutliQ when the software sets TDAR
+ * and the UDMA clears TDAR simultaneously or in a small window (2-4 cycles).
+ * This will cause the udma_tx and udma_tx_arbiter state machines to hang.
+ * The issue exist@i.MX6SX enet IP.
+ */
+#define FEC_QUIRK_TKT210582		(1 << 9)
 
 static struct platform_device_id fec_devtype[] = {
 	{
@@ -139,7 +146,7 @@ static struct platform_device_id fec_devtype[] = {
 		.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_HAS_GBIT |
 				FEC_QUIRK_HAS_BUFDESC_EX | FEC_QUIRK_HAS_CSUM |
 				FEC_QUIRK_HAS_VLAN | FEC_QUIRK_ERR006358 |
-				FEC_QUIRK_HAS_AVB,
+				FEC_QUIRK_HAS_AVB | FEC_QUIRK_TKT210582,
 	}, {
 		/* sentinel */
 	}
@@ -709,6 +716,8 @@ static int fec_enet_txq_submit_tso(struct fec_enet_priv_tx_q *txq,
 	struct tso_t tso;
 	unsigned int index = 0;
 	int ret;
+	const struct platform_device_id *id_entry =
+				platform_get_device_id(fep->pdev);
 
 	if (tso_count_descs(skb) >= fec_enet_get_free_txdesc_num(fep, txq)) {
 		dev_kfree_skb_any(skb);
@@ -770,7 +779,12 @@ static int fec_enet_txq_submit_tso(struct fec_enet_priv_tx_q *txq,
 	txq->cur_tx = bdp;
 
 	/* Trigger transmission start */
-	writel(0, fep->hwp + FEC_X_DES_ACTIVE(queue));
+	if (!(id_entry->driver_data & FEC_QUIRK_TKT210582) ||
+		!readl(fep->hwp + FEC_X_DES_ACTIVE(queue)) ||
+		!readl(fep->hwp + FEC_X_DES_ACTIVE(queue)) ||
+		!readl(fep->hwp + FEC_X_DES_ACTIVE(queue)) ||
+		!readl(fep->hwp + FEC_X_DES_ACTIVE(queue)))
+		writel(0, fep->hwp + FEC_X_DES_ACTIVE(queue));
 
 	return 0;
 
-- 
1.9.1

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

* [Patch net-next 1/4] net: fec: refine error handle of parser queue number from DT
  2014-09-15 17:12 ` [Patch net-next 1/4] net: fec: refine error handle of parser queue number from DT Frank.Li at freescale.com
@ 2014-09-15 21:32   ` David Miller
  0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2014-09-15 21:32 UTC (permalink / raw)
  To: linux-arm-kernel

From: <Frank.Li@freescale.com>
Date: Tue, 16 Sep 2014 01:12:54 +0800

> -		dev_err(&pdev->dev, "Invalidate num_tx(=%d), fail back to 1\n",
> +		dev_warn(&pdev->dev, "Invalid num_tx(=%d), fall back to 1\n",
>  			*num_tx);

Any time you change the function name of a multi-line function call, you must
be mindful to adjust the indention of the subsequent argument lines, if
necessary.

>  	if (*num_rx < 1 || *num_rx > FEC_ENET_MAX_RX_QS) {
> -		dev_err(&pdev->dev, "Invalidate num_rx(=%d), fail back to 1\n",
> +		dev_warn(&pdev->dev, "Invalid num_rx(=%d), fall back to 1\n",
>  			*num_rx);

Likewise.

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

* [Patch net-next 2/4] net: fec: add interrupt coalescence feature support
  2014-09-15 17:12 ` [Patch net-next 2/4] net: fec: add interrupt coalescence feature support Frank.Li at freescale.com
@ 2014-09-15 21:34   ` David Miller
  0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2014-09-15 21:34 UTC (permalink / raw)
  To: linux-arm-kernel

From: <Frank.Li@freescale.com>
Date: Tue, 16 Sep 2014 01:12:55 +0800

>  	unsigned int tx_align;
>  	unsigned int rx_align;
> +
> +	/* hw interrupt coalesce */
> +	uint rx_pkts_itr;
> +	uint rx_time_itr;
> +	uint tx_pkts_itr;
> +	uint tx_time_itr;

Please use explicit "unsigned int" just like the lines right above the ones
you are adding.

> +	/* Must be greater than zero to avoid unpredictable behavior */
> +	if (!fep->rx_time_itr || !fep->rx_pkts_itr ||
> +		!fep->tx_time_itr || !fep->tx_pkts_itr)

This is not indented properly.

On the second and subsequent lines of a multi-line conditional, things
must start exactly at the first column after the openning parenthesis
of the first line.

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

* [Patch net-next 4/4] net: fec: Workaround for imx6sx enet tx hang when enable three queues
  2014-09-15 17:12 ` [Patch net-next 4/4] net: fec: Workaround for imx6sx enet tx hang when enable three queues Frank.Li at freescale.com
@ 2014-09-15 21:35   ` David Miller
  2014-09-16  0:52   ` Shawn Guo
  1 sibling, 0 replies; 10+ messages in thread
From: David Miller @ 2014-09-15 21:35 UTC (permalink / raw)
  To: linux-arm-kernel

From: <Frank.Li@freescale.com>
Date: Tue, 16 Sep 2014 01:12:57 +0800

> @@ -111,6 +111,13 @@ static void fec_enet_itr_coal_init(struct net_device *ndev);
>   *   independent rings
>   */
>  #define FEC_QUIRK_HAS_AVB		(1 << 8)
> +/*
> + * There is a TDAR race condition for mutliQ when the software sets TDAR
> + * and the UDMA clears TDAR simultaneously or in a small window (2-4 cycles).
> + * This will cause the udma_tx and udma_tx_arbiter state machines to hang.
> + * The issue exist at i.MX6SX enet IP.
> + */
> +#define FEC_QUIRK_TKT210582		(1 << 9)

Networking comments should be of the form:

	/* Like
	 * this.
	 */

>  	/* Trigger transmission start */
> -	writel(0, fep->hwp + FEC_X_DES_ACTIVE(queue));
> +	if (!(id_entry->driver_data & FEC_QUIRK_TKT210582) ||
> +		!readl(fep->hwp + FEC_X_DES_ACTIVE(queue)) ||
> +		!readl(fep->hwp + FEC_X_DES_ACTIVE(queue)) ||
> +		!readl(fep->hwp + FEC_X_DES_ACTIVE(queue)) ||
> +		!readl(fep->hwp + FEC_X_DES_ACTIVE(queue)))
> +		writel(0, fep->hwp + FEC_X_DES_ACTIVE(queue));

This conditional is not indented properly, see my feedback for patch #2.

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

* [Patch net-next 4/4] net: fec: Workaround for imx6sx enet tx hang when enable three queues
  2014-09-15 17:12 ` [Patch net-next 4/4] net: fec: Workaround for imx6sx enet tx hang when enable three queues Frank.Li at freescale.com
  2014-09-15 21:35   ` David Miller
@ 2014-09-16  0:52   ` Shawn Guo
  2014-09-16  3:09     ` fugang.duan at freescale.com
  1 sibling, 1 reply; 10+ messages in thread
From: Shawn Guo @ 2014-09-16  0:52 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Sep 16, 2014 at 01:12:57AM +0800, Frank.Li at freescale.com wrote:
> From: Fugang Duan <B38611@freescale.com>
> 
> When enable three queues on imx6sx enet, and then do tx performance
> test with iperf tool, after some time running, tx hang.
> 
> Found that:
> 	If uDMA is running, software set TDAR may cause tx hang.
> 	If uDMA is in idle, software set TDAR don't cause tx hang.
> 
> There is a TDAR race condition for mutliQ when the software sets TDAR
> and the UDMA clears TDAR simultaneously or in a small window (2-4 cycles).
> This will cause the udma_tx and udma_tx_arbiter state machines to hang.
> The issue exist at i.MX6SX enet IP.
> 
> So, the Workaround is checking TDAR status four time, if TDAR cleared by
> hardware and then write TDAR, otherwise don't set TDAR.
> 
> The patch is only one Workaround for the issue TKT210582.
> 
> Signed-off-by: Fugang Duan <B38611@freescale.com>
> Signed-off-by: Frank Li <Frank.Li@freescale.com>
> ---
>  drivers/net/ethernet/freescale/fec_main.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index 524ddbe..452e576 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -111,6 +111,13 @@ static void fec_enet_itr_coal_init(struct net_device *ndev);
>   *   independent rings
>   */
>  #define FEC_QUIRK_HAS_AVB		(1 << 8)
> +/*
> + * There is a TDAR race condition for mutliQ when the software sets TDAR
> + * and the UDMA clears TDAR simultaneously or in a small window (2-4 cycles).
> + * This will cause the udma_tx and udma_tx_arbiter state machines to hang.
> + * The issue exist at i.MX6SX enet IP.
> + */
> +#define FEC_QUIRK_TKT210582		(1 << 9)

I think TKTxxx number is used by Freescale design team to track issues
internally, and there should be a corresponding errata number like
ERRxxx which should be accessible by external people in errata document?

Shawn

>  
>  static struct platform_device_id fec_devtype[] = {
>  	{
> @@ -139,7 +146,7 @@ static struct platform_device_id fec_devtype[] = {
>  		.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_HAS_GBIT |
>  				FEC_QUIRK_HAS_BUFDESC_EX | FEC_QUIRK_HAS_CSUM |
>  				FEC_QUIRK_HAS_VLAN | FEC_QUIRK_ERR006358 |
> -				FEC_QUIRK_HAS_AVB,
> +				FEC_QUIRK_HAS_AVB | FEC_QUIRK_TKT210582,
>  	}, {
>  		/* sentinel */
>  	}
> @@ -709,6 +716,8 @@ static int fec_enet_txq_submit_tso(struct fec_enet_priv_tx_q *txq,
>  	struct tso_t tso;
>  	unsigned int index = 0;
>  	int ret;
> +	const struct platform_device_id *id_entry =
> +				platform_get_device_id(fep->pdev);
>  
>  	if (tso_count_descs(skb) >= fec_enet_get_free_txdesc_num(fep, txq)) {
>  		dev_kfree_skb_any(skb);
> @@ -770,7 +779,12 @@ static int fec_enet_txq_submit_tso(struct fec_enet_priv_tx_q *txq,
>  	txq->cur_tx = bdp;
>  
>  	/* Trigger transmission start */
> -	writel(0, fep->hwp + FEC_X_DES_ACTIVE(queue));
> +	if (!(id_entry->driver_data & FEC_QUIRK_TKT210582) ||
> +		!readl(fep->hwp + FEC_X_DES_ACTIVE(queue)) ||
> +		!readl(fep->hwp + FEC_X_DES_ACTIVE(queue)) ||
> +		!readl(fep->hwp + FEC_X_DES_ACTIVE(queue)) ||
> +		!readl(fep->hwp + FEC_X_DES_ACTIVE(queue)))
> +		writel(0, fep->hwp + FEC_X_DES_ACTIVE(queue));
>  
>  	return 0;
>  
> -- 
> 1.9.1
> 

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

* [Patch net-next 4/4] net: fec: Workaround for imx6sx enet tx hang when enable three queues
  2014-09-16  0:52   ` Shawn Guo
@ 2014-09-16  3:09     ` fugang.duan at freescale.com
  0 siblings, 0 replies; 10+ messages in thread
From: fugang.duan at freescale.com @ 2014-09-16  3:09 UTC (permalink / raw)
  To: linux-arm-kernel

From: Shawn Guo <shawn.guo@linaro.org> Sent: Tuesday, September 16, 2014 8:52 AM
>To: Li Frank-B20596
>Cc: Duan Fugang-B38611; davem at davemloft.net; netdev at vger.kernel.org;
>lznuaa at gmail.com; linux-arm-kernel at lists.infradead.org
>Subject: Re: [Patch net-next 4/4] net: fec: Workaround for imx6sx enet tx
>hang when enable three queues
>
>On Tue, Sep 16, 2014 at 01:12:57AM +0800, Frank.Li at freescale.com wrote:
>> From: Fugang Duan <B38611@freescale.com>
>>
>> When enable three queues on imx6sx enet, and then do tx performance
>> test with iperf tool, after some time running, tx hang.
>>
>> Found that:
>> 	If uDMA is running, software set TDAR may cause tx hang.
>> 	If uDMA is in idle, software set TDAR don't cause tx hang.
>>
>> There is a TDAR race condition for mutliQ when the software sets TDAR
>> and the UDMA clears TDAR simultaneously or in a small window (2-4
>cycles).
>> This will cause the udma_tx and udma_tx_arbiter state machines to hang.
>> The issue exist at i.MX6SX enet IP.
>>
>> So, the Workaround is checking TDAR status four time, if TDAR cleared
>> by hardware and then write TDAR, otherwise don't set TDAR.
>>
>> The patch is only one Workaround for the issue TKT210582.
>>
>> Signed-off-by: Fugang Duan <B38611@freescale.com>
>> Signed-off-by: Frank Li <Frank.Li@freescale.com>
>> ---
>>  drivers/net/ethernet/freescale/fec_main.c | 18 ++++++++++++++++--
>>  1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/freescale/fec_main.c
>> b/drivers/net/ethernet/freescale/fec_main.c
>> index 524ddbe..452e576 100644
>> --- a/drivers/net/ethernet/freescale/fec_main.c
>> +++ b/drivers/net/ethernet/freescale/fec_main.c
>> @@ -111,6 +111,13 @@ static void fec_enet_itr_coal_init(struct
>net_device *ndev);
>>   *   independent rings
>>   */
>>  #define FEC_QUIRK_HAS_AVB		(1 << 8)
>> +/*
>> + * There is a TDAR race condition for mutliQ when the software sets
>> +TDAR
>> + * and the UDMA clears TDAR simultaneously or in a small window (2-4
>cycles).
>> + * This will cause the udma_tx and udma_tx_arbiter state machines to
>hang.
>> + * The issue exist at i.MX6SX enet IP.
>> + */
>> +#define FEC_QUIRK_TKT210582		(1 << 9)
>
>I think TKTxxx number is used by Freescale design team to track issues
>internally, and there should be a corresponding errata number like ERRxxx
>which should be accessible by external people in errata document?
>
>Shawn
The issue ticket is: TDAR race condition for mutliQ - Errata: ERR007885

>
>>
>>  static struct platform_device_id fec_devtype[] = {
>>  	{
>> @@ -139,7 +146,7 @@ static struct platform_device_id fec_devtype[] = {
>>  		.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_HAS_GBIT |
>>  				FEC_QUIRK_HAS_BUFDESC_EX | FEC_QUIRK_HAS_CSUM |
>>  				FEC_QUIRK_HAS_VLAN | FEC_QUIRK_ERR006358 |
>> -				FEC_QUIRK_HAS_AVB,
>> +				FEC_QUIRK_HAS_AVB | FEC_QUIRK_TKT210582,
>>  	}, {
>>  		/* sentinel */
>>  	}
>> @@ -709,6 +716,8 @@ static int fec_enet_txq_submit_tso(struct
>fec_enet_priv_tx_q *txq,
>>  	struct tso_t tso;
>>  	unsigned int index = 0;
>>  	int ret;
>> +	const struct platform_device_id *id_entry =
>> +				platform_get_device_id(fep->pdev);
>>
>>  	if (tso_count_descs(skb) >= fec_enet_get_free_txdesc_num(fep, txq))
>{
>>  		dev_kfree_skb_any(skb);
>> @@ -770,7 +779,12 @@ static int fec_enet_txq_submit_tso(struct
>fec_enet_priv_tx_q *txq,
>>  	txq->cur_tx = bdp;
>>
>>  	/* Trigger transmission start */
>> -	writel(0, fep->hwp + FEC_X_DES_ACTIVE(queue));
>> +	if (!(id_entry->driver_data & FEC_QUIRK_TKT210582) ||
>> +		!readl(fep->hwp + FEC_X_DES_ACTIVE(queue)) ||
>> +		!readl(fep->hwp + FEC_X_DES_ACTIVE(queue)) ||
>> +		!readl(fep->hwp + FEC_X_DES_ACTIVE(queue)) ||
>> +		!readl(fep->hwp + FEC_X_DES_ACTIVE(queue)))
>> +		writel(0, fep->hwp + FEC_X_DES_ACTIVE(queue));
>>
>>  	return 0;
>>
>> --
>> 1.9.1
>>

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

end of thread, other threads:[~2014-09-16  3:09 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-15 17:12 [Patch net-next 0/4] net: fec: add interrupt coalescence Frank.Li at freescale.com
2014-09-15 17:12 ` [Patch net-next 1/4] net: fec: refine error handle of parser queue number from DT Frank.Li at freescale.com
2014-09-15 21:32   ` David Miller
2014-09-15 17:12 ` [Patch net-next 2/4] net: fec: add interrupt coalescence feature support Frank.Li at freescale.com
2014-09-15 21:34   ` David Miller
2014-09-15 17:12 ` [Patch net-next 3/4] net:fec: increase DMA queue number Frank.Li at freescale.com
2014-09-15 17:12 ` [Patch net-next 4/4] net: fec: Workaround for imx6sx enet tx hang when enable three queues Frank.Li at freescale.com
2014-09-15 21:35   ` David Miller
2014-09-16  0:52   ` Shawn Guo
2014-09-16  3:09     ` fugang.duan at freescale.com

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