All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dylan Hung <dylan_hung@aspeedtech.com>
To: <davem@davemloft.net>, <kuba@kernel.org>,
	<netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<ratbert@faraday-tech.com>, <linux-aspeed@lists.ozlabs.org>,
	<openbmc@lists.ozlabs.org>
Cc: <BMC-SW@aspeedtech.com>, Joel Stanley <joel@jms.id.au>
Subject: [PATCH 3/4] ftgmac100: Add a dummy read to ensure running sequence
Date: Mon, 19 Oct 2020 16:57:16 +0800	[thread overview]
Message-ID: <20201019085717.32413-4-dylan_hung@aspeedtech.com> (raw)
In-Reply-To: <20201019085717.32413-1-dylan_hung@aspeedtech.com>

On the AST2600 care must be taken to ensure writes appear correctly when
modifying the interrupt reglated registers.

Add a function to perform a read after all writes to the IER and ISR registers.

Fixes: 39bfab8844a0 ("net: ftgmac100: Add support for DT phy-handle property")
Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 38 ++++++++++++------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 810bda80f138..0c67fc3e27df 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -111,6 +111,14 @@ struct ftgmac100 {
 	bool is_aspeed;
 };
 
+/* Helper to ensure writes are observed with the correct ordering. Use only
+ * for IER and ISR accesses. */
+static void ftgmac100_write(u32 val, void __iomem *addr)
+{
+	iowrite32(val, addr);
+	ioread32(addr);
+}
+
 static int ftgmac100_reset_mac(struct ftgmac100 *priv, u32 maccr)
 {
 	struct net_device *netdev = priv->netdev;
@@ -1048,7 +1056,7 @@ static void ftgmac100_adjust_link(struct net_device *netdev)
 		return;
 
 	/* Disable all interrupts */
-	iowrite32(0, priv->base + FTGMAC100_OFFSET_IER);
+	ftgmac100_write(0, priv->base + FTGMAC100_OFFSET_IER);
 
 	/* Reset the adapter asynchronously */
 	schedule_work(&priv->reset_task);
@@ -1246,7 +1254,7 @@ static irqreturn_t ftgmac100_interrupt(int irq, void *dev_id)
 
 	/* Fetch and clear interrupt bits, process abnormal ones */
 	status = ioread32(priv->base + FTGMAC100_OFFSET_ISR);
-	iowrite32(status, priv->base + FTGMAC100_OFFSET_ISR);
+	ftgmac100_write(status, priv->base + FTGMAC100_OFFSET_ISR);
 	if (unlikely(status & FTGMAC100_INT_BAD)) {
 
 		/* RX buffer unavailable */
@@ -1266,7 +1274,7 @@ static irqreturn_t ftgmac100_interrupt(int irq, void *dev_id)
 			if (net_ratelimit())
 				netdev_warn(netdev,
 					   "AHB bus error ! Resetting chip.\n");
-			iowrite32(0, priv->base + FTGMAC100_OFFSET_IER);
+			ftgmac100_write(0, priv->base + FTGMAC100_OFFSET_IER);
 			schedule_work(&priv->reset_task);
 			return IRQ_HANDLED;
 		}
@@ -1281,7 +1289,7 @@ static irqreturn_t ftgmac100_interrupt(int irq, void *dev_id)
 	}
 
 	/* Only enable "bad" interrupts while NAPI is on */
-	iowrite32(new_mask, priv->base + FTGMAC100_OFFSET_IER);
+	ftgmac100_write(new_mask, priv->base + FTGMAC100_OFFSET_IER);
 
 	/* Schedule NAPI bh */
 	napi_schedule_irqoff(&priv->napi);
@@ -1320,8 +1328,7 @@ static int ftgmac100_poll(struct napi_struct *napi, int budget)
 		ftgmac100_start_hw(priv);
 
 		/* Re-enable "bad" interrupts */
-		iowrite32(FTGMAC100_INT_BAD,
-			  priv->base + FTGMAC100_OFFSET_IER);
+		ftgmac100_write(FTGMAC100_INT_BAD, priv->base + FTGMAC100_OFFSET_IER);
 	}
 
 	/* As long as we are waiting for transmit packets to be
@@ -1336,13 +1343,7 @@ static int ftgmac100_poll(struct napi_struct *napi, int budget)
 		 * they were masked. So we clear them first, then we need
 		 * to re-check if there's something to process
 		 */
-		iowrite32(FTGMAC100_INT_RXTX,
-			  priv->base + FTGMAC100_OFFSET_ISR);
-
-		/* Push the above (and provides a barrier vs. subsequent
-		 * reads of the descriptor).
-		 */
-		ioread32(priv->base + FTGMAC100_OFFSET_ISR);
+		ftgmac100_write(FTGMAC100_INT_RXTX, priv->base + FTGMAC100_OFFSET_ISR);
 
 		/* Check RX and TX descriptors for more work to do */
 		if (ftgmac100_check_rx(priv) ||
@@ -1353,8 +1354,7 @@ static int ftgmac100_poll(struct napi_struct *napi, int budget)
 		napi_complete(napi);
 
 		/* enable all interrupts */
-		iowrite32(FTGMAC100_INT_ALL,
-			  priv->base + FTGMAC100_OFFSET_IER);
+		ftgmac100_write(FTGMAC100_INT_ALL, priv->base + FTGMAC100_OFFSET_IER);
 	}
 
 	return work_done;
@@ -1382,7 +1382,7 @@ static int ftgmac100_init_all(struct ftgmac100 *priv, bool ignore_alloc_err)
 	netif_start_queue(priv->netdev);
 
 	/* Enable all interrupts */
-	iowrite32(FTGMAC100_INT_ALL, priv->base + FTGMAC100_OFFSET_IER);
+	ftgmac100_write(FTGMAC100_INT_ALL, priv->base + FTGMAC100_OFFSET_IER);
 
 	return err;
 }
@@ -1508,7 +1508,7 @@ static int ftgmac100_open(struct net_device *netdev)
  err_irq:
 	netif_napi_del(&priv->napi);
  err_hw:
-	iowrite32(0, priv->base + FTGMAC100_OFFSET_IER);
+	ftgmac100_write(0, priv->base + FTGMAC100_OFFSET_IER);
 	ftgmac100_free_rings(priv);
 	return err;
 }
@@ -1526,7 +1526,7 @@ static int ftgmac100_stop(struct net_device *netdev)
 	 */
 
 	/* disable all interrupts */
-	iowrite32(0, priv->base + FTGMAC100_OFFSET_IER);
+	ftgmac100_write(0, priv->base + FTGMAC100_OFFSET_IER);
 
 	netif_stop_queue(netdev);
 	napi_disable(&priv->napi);
@@ -1549,7 +1549,7 @@ static void ftgmac100_tx_timeout(struct net_device *netdev, unsigned int txqueue
 	struct ftgmac100 *priv = netdev_priv(netdev);
 
 	/* Disable all interrupts */
-	iowrite32(0, priv->base + FTGMAC100_OFFSET_IER);
+	ftgmac100_write(0, priv->base + FTGMAC100_OFFSET_IER);
 
 	/* Do the reset outside of interrupt context */
 	schedule_work(&priv->reset_task);
-- 
2.17.1


WARNING: multiple messages have this Message-ID (diff)
From: Dylan Hung <dylan_hung@aspeedtech.com>
To: <davem@davemloft.net>, <kuba@kernel.org>,
	<netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<ratbert@faraday-tech.com>, <linux-aspeed@lists.ozlabs.org>,
	<openbmc@lists.ozlabs.org>
Cc: BMC-SW@aspeedtech.com
Subject: [PATCH 3/4] ftgmac100: Add a dummy read to ensure running sequence
Date: Mon, 19 Oct 2020 16:57:16 +0800	[thread overview]
Message-ID: <20201019085717.32413-4-dylan_hung@aspeedtech.com> (raw)
In-Reply-To: <20201019085717.32413-1-dylan_hung@aspeedtech.com>

On the AST2600 care must be taken to ensure writes appear correctly when
modifying the interrupt reglated registers.

Add a function to perform a read after all writes to the IER and ISR registers.

Fixes: 39bfab8844a0 ("net: ftgmac100: Add support for DT phy-handle property")
Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 38 ++++++++++++------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 810bda80f138..0c67fc3e27df 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -111,6 +111,14 @@ struct ftgmac100 {
 	bool is_aspeed;
 };
 
+/* Helper to ensure writes are observed with the correct ordering. Use only
+ * for IER and ISR accesses. */
+static void ftgmac100_write(u32 val, void __iomem *addr)
+{
+	iowrite32(val, addr);
+	ioread32(addr);
+}
+
 static int ftgmac100_reset_mac(struct ftgmac100 *priv, u32 maccr)
 {
 	struct net_device *netdev = priv->netdev;
@@ -1048,7 +1056,7 @@ static void ftgmac100_adjust_link(struct net_device *netdev)
 		return;
 
 	/* Disable all interrupts */
-	iowrite32(0, priv->base + FTGMAC100_OFFSET_IER);
+	ftgmac100_write(0, priv->base + FTGMAC100_OFFSET_IER);
 
 	/* Reset the adapter asynchronously */
 	schedule_work(&priv->reset_task);
@@ -1246,7 +1254,7 @@ static irqreturn_t ftgmac100_interrupt(int irq, void *dev_id)
 
 	/* Fetch and clear interrupt bits, process abnormal ones */
 	status = ioread32(priv->base + FTGMAC100_OFFSET_ISR);
-	iowrite32(status, priv->base + FTGMAC100_OFFSET_ISR);
+	ftgmac100_write(status, priv->base + FTGMAC100_OFFSET_ISR);
 	if (unlikely(status & FTGMAC100_INT_BAD)) {
 
 		/* RX buffer unavailable */
@@ -1266,7 +1274,7 @@ static irqreturn_t ftgmac100_interrupt(int irq, void *dev_id)
 			if (net_ratelimit())
 				netdev_warn(netdev,
 					   "AHB bus error ! Resetting chip.\n");
-			iowrite32(0, priv->base + FTGMAC100_OFFSET_IER);
+			ftgmac100_write(0, priv->base + FTGMAC100_OFFSET_IER);
 			schedule_work(&priv->reset_task);
 			return IRQ_HANDLED;
 		}
@@ -1281,7 +1289,7 @@ static irqreturn_t ftgmac100_interrupt(int irq, void *dev_id)
 	}
 
 	/* Only enable "bad" interrupts while NAPI is on */
-	iowrite32(new_mask, priv->base + FTGMAC100_OFFSET_IER);
+	ftgmac100_write(new_mask, priv->base + FTGMAC100_OFFSET_IER);
 
 	/* Schedule NAPI bh */
 	napi_schedule_irqoff(&priv->napi);
@@ -1320,8 +1328,7 @@ static int ftgmac100_poll(struct napi_struct *napi, int budget)
 		ftgmac100_start_hw(priv);
 
 		/* Re-enable "bad" interrupts */
-		iowrite32(FTGMAC100_INT_BAD,
-			  priv->base + FTGMAC100_OFFSET_IER);
+		ftgmac100_write(FTGMAC100_INT_BAD, priv->base + FTGMAC100_OFFSET_IER);
 	}
 
 	/* As long as we are waiting for transmit packets to be
@@ -1336,13 +1343,7 @@ static int ftgmac100_poll(struct napi_struct *napi, int budget)
 		 * they were masked. So we clear them first, then we need
 		 * to re-check if there's something to process
 		 */
-		iowrite32(FTGMAC100_INT_RXTX,
-			  priv->base + FTGMAC100_OFFSET_ISR);
-
-		/* Push the above (and provides a barrier vs. subsequent
-		 * reads of the descriptor).
-		 */
-		ioread32(priv->base + FTGMAC100_OFFSET_ISR);
+		ftgmac100_write(FTGMAC100_INT_RXTX, priv->base + FTGMAC100_OFFSET_ISR);
 
 		/* Check RX and TX descriptors for more work to do */
 		if (ftgmac100_check_rx(priv) ||
@@ -1353,8 +1354,7 @@ static int ftgmac100_poll(struct napi_struct *napi, int budget)
 		napi_complete(napi);
 
 		/* enable all interrupts */
-		iowrite32(FTGMAC100_INT_ALL,
-			  priv->base + FTGMAC100_OFFSET_IER);
+		ftgmac100_write(FTGMAC100_INT_ALL, priv->base + FTGMAC100_OFFSET_IER);
 	}
 
 	return work_done;
@@ -1382,7 +1382,7 @@ static int ftgmac100_init_all(struct ftgmac100 *priv, bool ignore_alloc_err)
 	netif_start_queue(priv->netdev);
 
 	/* Enable all interrupts */
-	iowrite32(FTGMAC100_INT_ALL, priv->base + FTGMAC100_OFFSET_IER);
+	ftgmac100_write(FTGMAC100_INT_ALL, priv->base + FTGMAC100_OFFSET_IER);
 
 	return err;
 }
@@ -1508,7 +1508,7 @@ static int ftgmac100_open(struct net_device *netdev)
  err_irq:
 	netif_napi_del(&priv->napi);
  err_hw:
-	iowrite32(0, priv->base + FTGMAC100_OFFSET_IER);
+	ftgmac100_write(0, priv->base + FTGMAC100_OFFSET_IER);
 	ftgmac100_free_rings(priv);
 	return err;
 }
@@ -1526,7 +1526,7 @@ static int ftgmac100_stop(struct net_device *netdev)
 	 */
 
 	/* disable all interrupts */
-	iowrite32(0, priv->base + FTGMAC100_OFFSET_IER);
+	ftgmac100_write(0, priv->base + FTGMAC100_OFFSET_IER);
 
 	netif_stop_queue(netdev);
 	napi_disable(&priv->napi);
@@ -1549,7 +1549,7 @@ static void ftgmac100_tx_timeout(struct net_device *netdev, unsigned int txqueue
 	struct ftgmac100 *priv = netdev_priv(netdev);
 
 	/* Disable all interrupts */
-	iowrite32(0, priv->base + FTGMAC100_OFFSET_IER);
+	ftgmac100_write(0, priv->base + FTGMAC100_OFFSET_IER);
 
 	/* Do the reset outside of interrupt context */
 	schedule_work(&priv->reset_task);
-- 
2.17.1


  parent reply	other threads:[~2020-10-19  8:58 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-19  8:57 [PATCH 0/4] fix ftgmac100 issues on aspeed soc Dylan Hung
2020-10-19  8:57 ` Dylan Hung
2020-10-19  8:57 ` [PATCH 1/4] ftgmac100: Fix race issue on TX descriptor[0] Dylan Hung
2020-10-19  8:57   ` Dylan Hung
2020-10-19 23:19   ` Benjamin Herrenschmidt
2020-10-19 23:19     ` Benjamin Herrenschmidt
2020-10-20  4:13     ` Joel Stanley
2020-10-20  4:13       ` Joel Stanley
2020-10-20  6:23       ` Benjamin Herrenschmidt
2020-10-20  6:23         ` Benjamin Herrenschmidt
2020-10-20  7:13         ` Joel Stanley
2020-10-20  7:13           ` Joel Stanley
2020-10-19  8:57 ` [PATCH 2/4] ftgmac100: Fix missing-poll issue Dylan Hung
2020-10-19  8:57   ` Dylan Hung
2020-10-19  8:57 ` Dylan Hung [this message]
2020-10-19  8:57   ` [PATCH 3/4] ftgmac100: Add a dummy read to ensure running sequence Dylan Hung
2020-10-19 23:25   ` Benjamin Herrenschmidt
2020-10-19 23:25     ` Benjamin Herrenschmidt
2020-10-19  8:57 ` [PATCH 4/4] ftgmac100: Restart MAC HW once Dylan Hung
2020-10-19  8:57   ` Dylan Hung
2020-10-19 23:26   ` Benjamin Herrenschmidt
2020-10-19 23:26     ` Benjamin Herrenschmidt
2020-10-20  4:14   ` Joel Stanley
2020-10-20  4:14     ` Joel Stanley
2021-03-12  0:26     ` Joel Stanley
2021-03-12  0:26       ` Joel Stanley
2021-03-12  0:28       ` David Miller
2021-03-12  0:28         ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201019085717.32413-4-dylan_hung@aspeedtech.com \
    --to=dylan_hung@aspeedtech.com \
    --cc=BMC-SW@aspeedtech.com \
    --cc=davem@davemloft.net \
    --cc=joel@jms.id.au \
    --cc=kuba@kernel.org \
    --cc=linux-aspeed@lists.ozlabs.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=openbmc@lists.ozlabs.org \
    --cc=ratbert@faraday-tech.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.