netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 net-next 0/5] net: ethernet: mtk_wed: introduce reset support
@ 2023-01-11 17:22 Lorenzo Bianconi
  2023-01-11 17:22 ` [PATCH v5 net-next 1/5] net: ethernet: mtk_eth_soc: introduce mtk_hw_reset utility routine Lorenzo Bianconi
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Lorenzo Bianconi @ 2023-01-11 17:22 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, kuba, pabeni, lorenzo.bianconi, nbd, john,
	sean.wang, Mark-MC.Lee, sujuan.chen, daniel, leon

Introduce proper reset integration between ethernet and wlan drivers in order
to schedule wlan driver reset when ethernet/wed driver is resetting.
Introduce mtk_hw_reset_monitor work in order to detect possible DMA hangs.

Changes since v4:
- add missing usleep_range

Changes since v3:
- rely on msleep() utility instead of mdelay() in mtk_hw_init() and
  usleep_range() in mtk_ppe_prepare_reset() since the code runs in
  non-atomic context

Changes since v2:
- rebase on top of net-next
- move rtnl_lock/rtnl_unlock in reset callback
- re-run mtk_prepare_for_reset() after mtk_wed_fe_reset() acquiring RTNL lock

Changes since v1:
- rebase on top of net-next

Lorenzo Bianconi (5):
  net: ethernet: mtk_eth_soc: introduce mtk_hw_reset utility routine
  net: ethernet: mtk_eth_soc: introduce mtk_hw_warm_reset support
  net: ethernet: mtk_eth_soc: align reset procedure to vendor sdk
  net: ethernet: mtk_eth_soc: add dma checks to mtk_hw_reset_check
  net: ethernet: mtk_wed: add reset/reset_complete callbacks

 drivers/net/ethernet/mediatek/mtk_eth_soc.c  | 297 ++++++++++++++++---
 drivers/net/ethernet/mediatek/mtk_eth_soc.h  |  38 +++
 drivers/net/ethernet/mediatek/mtk_ppe.c      |  27 ++
 drivers/net/ethernet/mediatek/mtk_ppe.h      |   1 +
 drivers/net/ethernet/mediatek/mtk_ppe_regs.h |   6 +
 drivers/net/ethernet/mediatek/mtk_wed.c      |  40 +++
 drivers/net/ethernet/mediatek/mtk_wed.h      |   8 +
 include/linux/soc/mediatek/mtk_wed.h         |   2 +
 8 files changed, 380 insertions(+), 39 deletions(-)

-- 
2.39.0


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

* [PATCH v5 net-next 1/5] net: ethernet: mtk_eth_soc: introduce mtk_hw_reset utility routine
  2023-01-11 17:22 [PATCH v5 net-next 0/5] net: ethernet: mtk_wed: introduce reset support Lorenzo Bianconi
@ 2023-01-11 17:22 ` Lorenzo Bianconi
  2023-01-11 17:22 ` [PATCH v5 net-next 2/5] net: ethernet: mtk_eth_soc: introduce mtk_hw_warm_reset support Lorenzo Bianconi
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Lorenzo Bianconi @ 2023-01-11 17:22 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, kuba, pabeni, lorenzo.bianconi, nbd, john,
	sean.wang, Mark-MC.Lee, sujuan.chen, daniel, leon

This is a preliminary patch to add Wireless Ethernet Dispatcher reset
support.

Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Tested-by: Daniel Golle <daniel@makrotopia.org>
Co-developed-by: Sujuan Chen <sujuan.chen@mediatek.com>
Signed-off-by: Sujuan Chen <sujuan.chen@mediatek.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 36 +++++++++++++--------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index e3de9a53b2d9..ce429deea389 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -3471,6 +3471,27 @@ static void mtk_set_mcr_max_rx(struct mtk_mac *mac, u32 val)
 		mtk_w32(mac->hw, mcr_new, MTK_MAC_MCR(mac->id));
 }
 
+static void mtk_hw_reset(struct mtk_eth *eth)
+{
+	u32 val;
+
+	if (MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_V2)) {
+		regmap_write(eth->ethsys, ETHSYS_FE_RST_CHK_IDLE_EN, 0);
+		val = RSTCTRL_PPE0_V2;
+	} else {
+		val = RSTCTRL_PPE0;
+	}
+
+	if (MTK_HAS_CAPS(eth->soc->caps, MTK_RSTCTRL_PPE1))
+		val |= RSTCTRL_PPE1;
+
+	ethsys_reset(eth, RSTCTRL_ETH | RSTCTRL_FE | val);
+
+	if (MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_V2))
+		regmap_write(eth->ethsys, ETHSYS_FE_RST_CHK_IDLE_EN,
+			     0x3ffffff);
+}
+
 static int mtk_hw_init(struct mtk_eth *eth)
 {
 	u32 dma_mask = ETHSYS_DMA_AG_MAP_PDMA | ETHSYS_DMA_AG_MAP_QDMA |
@@ -3510,22 +3531,9 @@ static int mtk_hw_init(struct mtk_eth *eth)
 		return 0;
 	}
 
-	if (MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_V2)) {
-		regmap_write(eth->ethsys, ETHSYS_FE_RST_CHK_IDLE_EN, 0);
-		val = RSTCTRL_PPE0_V2;
-	} else {
-		val = RSTCTRL_PPE0;
-	}
-
-	if (MTK_HAS_CAPS(eth->soc->caps, MTK_RSTCTRL_PPE1))
-		val |= RSTCTRL_PPE1;
-
-	ethsys_reset(eth, RSTCTRL_ETH | RSTCTRL_FE | val);
+	mtk_hw_reset(eth);
 
 	if (MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_V2)) {
-		regmap_write(eth->ethsys, ETHSYS_FE_RST_CHK_IDLE_EN,
-			     0x3ffffff);
-
 		/* Set FE to PDMAv2 if necessary */
 		val = mtk_r32(eth, MTK_FE_GLO_MISC);
 		mtk_w32(eth,  val | BIT(4), MTK_FE_GLO_MISC);
-- 
2.39.0


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

* [PATCH v5 net-next 2/5] net: ethernet: mtk_eth_soc: introduce mtk_hw_warm_reset support
  2023-01-11 17:22 [PATCH v5 net-next 0/5] net: ethernet: mtk_wed: introduce reset support Lorenzo Bianconi
  2023-01-11 17:22 ` [PATCH v5 net-next 1/5] net: ethernet: mtk_eth_soc: introduce mtk_hw_reset utility routine Lorenzo Bianconi
@ 2023-01-11 17:22 ` Lorenzo Bianconi
  2023-01-11 17:22 ` [PATCH v5 net-next 3/5] net: ethernet: mtk_eth_soc: align reset procedure to vendor sdk Lorenzo Bianconi
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Lorenzo Bianconi @ 2023-01-11 17:22 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, kuba, pabeni, lorenzo.bianconi, nbd, john,
	sean.wang, Mark-MC.Lee, sujuan.chen, daniel, leon

Introduce mtk_hw_warm_reset utility routine. This is a preliminary patch
to align reset procedure to vendor sdk and avoid to power down the chip
during hw reset.

Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Tested-by: Daniel Golle <daniel@makrotopia.org>
Co-developed-by: Sujuan Chen <sujuan.chen@mediatek.com>
Signed-off-by: Sujuan Chen <sujuan.chen@mediatek.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 60 +++++++++++++++++++--
 1 file changed, 56 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index ce429deea389..051c81eff403 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -3492,7 +3492,54 @@ static void mtk_hw_reset(struct mtk_eth *eth)
 			     0x3ffffff);
 }
 
-static int mtk_hw_init(struct mtk_eth *eth)
+static u32 mtk_hw_reset_read(struct mtk_eth *eth)
+{
+	u32 val;
+
+	regmap_read(eth->ethsys, ETHSYS_RSTCTRL, &val);
+	return val;
+}
+
+static void mtk_hw_warm_reset(struct mtk_eth *eth)
+{
+	u32 rst_mask, val;
+
+	regmap_update_bits(eth->ethsys, ETHSYS_RSTCTRL, RSTCTRL_FE,
+			   RSTCTRL_FE);
+	if (readx_poll_timeout_atomic(mtk_hw_reset_read, eth, val,
+				      val & RSTCTRL_FE, 1, 1000)) {
+		dev_err(eth->dev, "warm reset failed\n");
+		mtk_hw_reset(eth);
+		return;
+	}
+
+	if (MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_V2))
+		rst_mask = RSTCTRL_ETH | RSTCTRL_PPE0_V2;
+	else
+		rst_mask = RSTCTRL_ETH | RSTCTRL_PPE0;
+
+	if (MTK_HAS_CAPS(eth->soc->caps, MTK_RSTCTRL_PPE1))
+		rst_mask |= RSTCTRL_PPE1;
+
+	regmap_update_bits(eth->ethsys, ETHSYS_RSTCTRL, rst_mask, rst_mask);
+
+	udelay(1);
+	val = mtk_hw_reset_read(eth);
+	if (!(val & rst_mask))
+		dev_err(eth->dev, "warm reset stage0 failed %08x (%08x)\n",
+			val, rst_mask);
+
+	rst_mask |= RSTCTRL_FE;
+	regmap_update_bits(eth->ethsys, ETHSYS_RSTCTRL, rst_mask, ~rst_mask);
+
+	udelay(1);
+	val = mtk_hw_reset_read(eth);
+	if (val & rst_mask)
+		dev_err(eth->dev, "warm reset stage1 failed %08x (%08x)\n",
+			val, rst_mask);
+}
+
+static int mtk_hw_init(struct mtk_eth *eth, bool reset)
 {
 	u32 dma_mask = ETHSYS_DMA_AG_MAP_PDMA | ETHSYS_DMA_AG_MAP_QDMA |
 		       ETHSYS_DMA_AG_MAP_PPE;
@@ -3531,7 +3578,12 @@ static int mtk_hw_init(struct mtk_eth *eth)
 		return 0;
 	}
 
-	mtk_hw_reset(eth);
+	msleep(100);
+
+	if (reset)
+		mtk_hw_warm_reset(eth);
+	else
+		mtk_hw_reset(eth);
 
 	if (MTK_HAS_CAPS(eth->soc->caps, MTK_NETSYS_V2)) {
 		/* Set FE to PDMAv2 if necessary */
@@ -3743,7 +3795,7 @@ static void mtk_pending_work(struct work_struct *work)
 	if (eth->dev->pins)
 		pinctrl_select_state(eth->dev->pins->p,
 				     eth->dev->pins->default_state);
-	mtk_hw_init(eth);
+	mtk_hw_init(eth, true);
 
 	/* restart DMA and enable IRQs */
 	for (i = 0; i < MTK_MAC_COUNT; i++) {
@@ -4372,7 +4424,7 @@ static int mtk_probe(struct platform_device *pdev)
 	eth->msg_enable = netif_msg_init(mtk_msg_level, MTK_DEFAULT_MSG_ENABLE);
 	INIT_WORK(&eth->pending_work, mtk_pending_work);
 
-	err = mtk_hw_init(eth);
+	err = mtk_hw_init(eth, false);
 	if (err)
 		goto err_wed_exit;
 
-- 
2.39.0


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

* [PATCH v5 net-next 3/5] net: ethernet: mtk_eth_soc: align reset procedure to vendor sdk
  2023-01-11 17:22 [PATCH v5 net-next 0/5] net: ethernet: mtk_wed: introduce reset support Lorenzo Bianconi
  2023-01-11 17:22 ` [PATCH v5 net-next 1/5] net: ethernet: mtk_eth_soc: introduce mtk_hw_reset utility routine Lorenzo Bianconi
  2023-01-11 17:22 ` [PATCH v5 net-next 2/5] net: ethernet: mtk_eth_soc: introduce mtk_hw_warm_reset support Lorenzo Bianconi
@ 2023-01-11 17:22 ` Lorenzo Bianconi
  2023-01-11 17:22 ` [PATCH v5 net-next 4/5] net: ethernet: mtk_eth_soc: add dma checks to mtk_hw_reset_check Lorenzo Bianconi
  2023-01-11 17:22 ` [PATCH v5 net-next 5/5] net: ethernet: mtk_wed: add reset/reset_complete callbacks Lorenzo Bianconi
  4 siblings, 0 replies; 10+ messages in thread
From: Lorenzo Bianconi @ 2023-01-11 17:22 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, kuba, pabeni, lorenzo.bianconi, nbd, john,
	sean.wang, Mark-MC.Lee, sujuan.chen, daniel, leon

Avoid to power-down the ethernet chip during hw reset and align reset
procedure to vendor sdk.

Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Tested-by: Daniel Golle <daniel@makrotopia.org>
Co-developed-by: Sujuan Chen <sujuan.chen@mediatek.com>
Signed-off-by: Sujuan Chen <sujuan.chen@mediatek.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c  | 92 +++++++++++++++-----
 drivers/net/ethernet/mediatek/mtk_eth_soc.h  | 12 +++
 drivers/net/ethernet/mediatek/mtk_ppe.c      | 27 ++++++
 drivers/net/ethernet/mediatek/mtk_ppe.h      |  1 +
 drivers/net/ethernet/mediatek/mtk_ppe_regs.h |  6 ++
 5 files changed, 115 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 051c81eff403..68ef94a100cd 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -2984,14 +2984,29 @@ static void mtk_dma_free(struct mtk_eth *eth)
 	kfree(eth->scratch_head);
 }
 
+static bool mtk_hw_reset_check(struct mtk_eth *eth)
+{
+	u32 val = mtk_r32(eth, MTK_INT_STATUS2);
+
+	return (val & MTK_FE_INT_FQ_EMPTY) || (val & MTK_FE_INT_RFIFO_UF) ||
+	       (val & MTK_FE_INT_RFIFO_OV) || (val & MTK_FE_INT_TSO_FAIL) ||
+	       (val & MTK_FE_INT_TSO_ALIGN) || (val & MTK_FE_INT_TSO_ILLEGAL);
+}
+
 static void mtk_tx_timeout(struct net_device *dev, unsigned int txqueue)
 {
 	struct mtk_mac *mac = netdev_priv(dev);
 	struct mtk_eth *eth = mac->hw;
 
+	if (test_bit(MTK_RESETTING, &eth->state))
+		return;
+
+	if (!mtk_hw_reset_check(eth))
+		return;
+
 	eth->netdev[mac->id]->stats.tx_errors++;
-	netif_err(eth, tx_err, dev,
-		  "transmit timed out\n");
+	netif_err(eth, tx_err, dev, "transmit timed out\n");
+
 	schedule_work(&eth->pending_work);
 }
 
@@ -3546,15 +3561,17 @@ static int mtk_hw_init(struct mtk_eth *eth, bool reset)
 	const struct mtk_reg_map *reg_map = eth->soc->reg_map;
 	int i, val, ret;
 
-	if (test_and_set_bit(MTK_HW_INIT, &eth->state))
+	if (!reset && test_and_set_bit(MTK_HW_INIT, &eth->state))
 		return 0;
 
-	pm_runtime_enable(eth->dev);
-	pm_runtime_get_sync(eth->dev);
+	if (!reset) {
+		pm_runtime_enable(eth->dev);
+		pm_runtime_get_sync(eth->dev);
 
-	ret = mtk_clk_enable(eth);
-	if (ret)
-		goto err_disable_pm;
+		ret = mtk_clk_enable(eth);
+		if (ret)
+			goto err_disable_pm;
+	}
 
 	if (eth->ethsys)
 		regmap_update_bits(eth->ethsys, ETHSYS_DMA_AG_MAP, dma_mask,
@@ -3687,8 +3704,10 @@ static int mtk_hw_init(struct mtk_eth *eth, bool reset)
 	return 0;
 
 err_disable_pm:
-	pm_runtime_put_sync(eth->dev);
-	pm_runtime_disable(eth->dev);
+	if (!reset) {
+		pm_runtime_put_sync(eth->dev);
+		pm_runtime_disable(eth->dev);
+	}
 
 	return ret;
 }
@@ -3767,30 +3786,53 @@ static int mtk_do_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 	return -EOPNOTSUPP;
 }
 
+static void mtk_prepare_for_reset(struct mtk_eth *eth)
+{
+	u32 val;
+	int i;
+
+	/* disabe FE P3 and P4 */
+	val = mtk_r32(eth, MTK_FE_GLO_CFG) | MTK_FE_LINK_DOWN_P3;
+	if (MTK_HAS_CAPS(eth->soc->caps, MTK_RSTCTRL_PPE1))
+		val |= MTK_FE_LINK_DOWN_P4;
+	mtk_w32(eth, val, MTK_FE_GLO_CFG);
+
+	/* adjust PPE configurations to prepare for reset */
+	for (i = 0; i < ARRAY_SIZE(eth->ppe); i++)
+		mtk_ppe_prepare_reset(eth->ppe[i]);
+
+	/* disable NETSYS interrupts */
+	mtk_w32(eth, 0, MTK_FE_INT_ENABLE);
+
+	/* force link down GMAC */
+	for (i = 0; i < 2; i++) {
+		val = mtk_r32(eth, MTK_MAC_MCR(i)) & ~MAC_MCR_FORCE_LINK;
+		mtk_w32(eth, val, MTK_MAC_MCR(i));
+	}
+}
+
 static void mtk_pending_work(struct work_struct *work)
 {
 	struct mtk_eth *eth = container_of(work, struct mtk_eth, pending_work);
-	int err, i;
 	unsigned long restart = 0;
+	u32 val;
+	int i;
 
 	rtnl_lock();
-
-	dev_dbg(eth->dev, "[%s][%d] reset\n", __func__, __LINE__);
 	set_bit(MTK_RESETTING, &eth->state);
 
+	mtk_prepare_for_reset(eth);
+
 	/* stop all devices to make sure that dma is properly shut down */
 	for (i = 0; i < MTK_MAC_COUNT; i++) {
-		if (!eth->netdev[i])
+		if (!eth->netdev[i] || !netif_running(eth->netdev[i]))
 			continue;
+
 		mtk_stop(eth->netdev[i]);
 		__set_bit(i, &restart);
 	}
-	dev_dbg(eth->dev, "[%s][%d] mtk_stop ends\n", __func__, __LINE__);
 
-	/* restart underlying hardware such as power, clock, pin mux
-	 * and the connected phy
-	 */
-	mtk_hw_deinit(eth);
+	usleep_range(15000, 16000);
 
 	if (eth->dev->pins)
 		pinctrl_select_state(eth->dev->pins->p,
@@ -3801,15 +3843,19 @@ static void mtk_pending_work(struct work_struct *work)
 	for (i = 0; i < MTK_MAC_COUNT; i++) {
 		if (!test_bit(i, &restart))
 			continue;
-		err = mtk_open(eth->netdev[i]);
-		if (err) {
+
+		if (mtk_open(eth->netdev[i])) {
 			netif_alert(eth, ifup, eth->netdev[i],
-			      "Driver up/down cycle failed, closing device.\n");
+				    "Driver up/down cycle failed\n");
 			dev_close(eth->netdev[i]);
 		}
 	}
 
-	dev_dbg(eth->dev, "[%s][%d] reset done\n", __func__, __LINE__);
+	/* enabe FE P3 and P4 */
+	val = mtk_r32(eth, MTK_FE_GLO_CFG) & ~MTK_FE_LINK_DOWN_P3;
+	if (MTK_HAS_CAPS(eth->soc->caps, MTK_RSTCTRL_PPE1))
+		val &= ~MTK_FE_LINK_DOWN_P4;
+	mtk_w32(eth, val, MTK_FE_GLO_CFG);
 
 	clear_bit(MTK_RESETTING, &eth->state);
 
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index 18a50529ce7b..a8066b3ee3ed 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -77,12 +77,24 @@
 #define	MTK_HW_LRO_REPLACE_DELTA	1000
 #define	MTK_HW_LRO_SDL_REMAIN_ROOM	1522
 
+/* Frame Engine Global Configuration */
+#define MTK_FE_GLO_CFG		0x00
+#define MTK_FE_LINK_DOWN_P3	BIT(11)
+#define MTK_FE_LINK_DOWN_P4	BIT(12)
+
 /* Frame Engine Global Reset Register */
 #define MTK_RST_GL		0x04
 #define RST_GL_PSE		BIT(0)
 
 /* Frame Engine Interrupt Status Register */
 #define MTK_INT_STATUS2		0x08
+#define MTK_FE_INT_ENABLE	0x0c
+#define MTK_FE_INT_FQ_EMPTY	BIT(8)
+#define MTK_FE_INT_TSO_FAIL	BIT(12)
+#define MTK_FE_INT_TSO_ILLEGAL	BIT(13)
+#define MTK_FE_INT_TSO_ALIGN	BIT(14)
+#define MTK_FE_INT_RFIFO_OV	BIT(18)
+#define MTK_FE_INT_RFIFO_UF	BIT(19)
 #define MTK_GDM1_AF		BIT(28)
 #define MTK_GDM2_AF		BIT(29)
 
diff --git a/drivers/net/ethernet/mediatek/mtk_ppe.c b/drivers/net/ethernet/mediatek/mtk_ppe.c
index 269208a841c7..451a87b1bc20 100644
--- a/drivers/net/ethernet/mediatek/mtk_ppe.c
+++ b/drivers/net/ethernet/mediatek/mtk_ppe.c
@@ -730,6 +730,33 @@ int mtk_foe_entry_idle_time(struct mtk_ppe *ppe, struct mtk_flow_entry *entry)
 	return __mtk_foe_entry_idle_time(ppe, entry->data.ib1);
 }
 
+int mtk_ppe_prepare_reset(struct mtk_ppe *ppe)
+{
+	if (!ppe)
+		return -EINVAL;
+
+	/* disable KA */
+	ppe_clear(ppe, MTK_PPE_TB_CFG, MTK_PPE_TB_CFG_KEEPALIVE);
+	ppe_clear(ppe, MTK_PPE_BIND_LMT1, MTK_PPE_NTU_KEEPALIVE);
+	ppe_w32(ppe, MTK_PPE_KEEPALIVE, 0);
+	usleep_range(10000, 11000);
+
+	/* set KA timer to maximum */
+	ppe_set(ppe, MTK_PPE_BIND_LMT1, MTK_PPE_NTU_KEEPALIVE);
+	ppe_w32(ppe, MTK_PPE_KEEPALIVE, 0xffffffff);
+
+	/* set KA tick select */
+	ppe_set(ppe, MTK_PPE_TB_CFG, MTK_PPE_TB_TICK_SEL);
+	ppe_set(ppe, MTK_PPE_TB_CFG, MTK_PPE_TB_CFG_KEEPALIVE);
+	usleep_range(10000, 11000);
+
+	/* disable scan mode */
+	ppe_clear(ppe, MTK_PPE_TB_CFG, MTK_PPE_TB_CFG_SCAN_MODE);
+	usleep_range(10000, 11000);
+
+	return mtk_ppe_wait_busy(ppe);
+}
+
 struct mtk_ppe *mtk_ppe_init(struct mtk_eth *eth, void __iomem *base,
 			     int version, int index)
 {
diff --git a/drivers/net/ethernet/mediatek/mtk_ppe.h b/drivers/net/ethernet/mediatek/mtk_ppe.h
index ea64fac1d425..16b02e1d4649 100644
--- a/drivers/net/ethernet/mediatek/mtk_ppe.h
+++ b/drivers/net/ethernet/mediatek/mtk_ppe.h
@@ -309,6 +309,7 @@ struct mtk_ppe *mtk_ppe_init(struct mtk_eth *eth, void __iomem *base,
 void mtk_ppe_deinit(struct mtk_eth *eth);
 void mtk_ppe_start(struct mtk_ppe *ppe);
 int mtk_ppe_stop(struct mtk_ppe *ppe);
+int mtk_ppe_prepare_reset(struct mtk_ppe *ppe);
 
 void __mtk_ppe_check_skb(struct mtk_ppe *ppe, struct sk_buff *skb, u16 hash);
 
diff --git a/drivers/net/ethernet/mediatek/mtk_ppe_regs.h b/drivers/net/ethernet/mediatek/mtk_ppe_regs.h
index 59596d823d8b..0fdb983b0a88 100644
--- a/drivers/net/ethernet/mediatek/mtk_ppe_regs.h
+++ b/drivers/net/ethernet/mediatek/mtk_ppe_regs.h
@@ -58,6 +58,12 @@
 #define MTK_PPE_TB_CFG_SCAN_MODE		GENMASK(17, 16)
 #define MTK_PPE_TB_CFG_HASH_DEBUG		GENMASK(19, 18)
 #define MTK_PPE_TB_CFG_INFO_SEL			BIT(20)
+#define MTK_PPE_TB_TICK_SEL			BIT(24)
+
+#define MTK_PPE_BIND_LMT1			0x230
+#define MTK_PPE_NTU_KEEPALIVE			GENMASK(23, 16)
+
+#define MTK_PPE_KEEPALIVE			0x234
 
 enum {
 	MTK_PPE_SCAN_MODE_DISABLED,
-- 
2.39.0


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

* [PATCH v5 net-next 4/5] net: ethernet: mtk_eth_soc: add dma checks to mtk_hw_reset_check
  2023-01-11 17:22 [PATCH v5 net-next 0/5] net: ethernet: mtk_wed: introduce reset support Lorenzo Bianconi
                   ` (2 preceding siblings ...)
  2023-01-11 17:22 ` [PATCH v5 net-next 3/5] net: ethernet: mtk_eth_soc: align reset procedure to vendor sdk Lorenzo Bianconi
@ 2023-01-11 17:22 ` Lorenzo Bianconi
  2023-01-11 17:22 ` [PATCH v5 net-next 5/5] net: ethernet: mtk_wed: add reset/reset_complete callbacks Lorenzo Bianconi
  4 siblings, 0 replies; 10+ messages in thread
From: Lorenzo Bianconi @ 2023-01-11 17:22 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, kuba, pabeni, lorenzo.bianconi, nbd, john,
	sean.wang, Mark-MC.Lee, sujuan.chen, daniel, leon

Introduce mtk_hw_check_dma_hang routine to monitor possible dma hangs.

Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Tested-by: Daniel Golle <daniel@makrotopia.org>
Co-developed-by: Sujuan Chen <sujuan.chen@mediatek.com>
Signed-off-by: Sujuan Chen <sujuan.chen@mediatek.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 106 ++++++++++++++++++++
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |  26 +++++
 2 files changed, 132 insertions(+)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 68ef94a100cd..1af74e9a6cd3 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -51,6 +51,7 @@ static const struct mtk_reg_map mtk_reg_map = {
 		.delay_irq	= 0x0a0c,
 		.irq_status	= 0x0a20,
 		.irq_mask	= 0x0a28,
+		.adma_rx_dbg0	= 0x0a38,
 		.int_grp	= 0x0a50,
 	},
 	.qdma = {
@@ -82,6 +83,8 @@ static const struct mtk_reg_map mtk_reg_map = {
 		[0]		= 0x2800,
 		[1]		= 0x2c00,
 	},
+	.pse_iq_sta		= 0x0110,
+	.pse_oq_sta		= 0x0118,
 };
 
 static const struct mtk_reg_map mt7628_reg_map = {
@@ -112,6 +115,7 @@ static const struct mtk_reg_map mt7986_reg_map = {
 		.delay_irq	= 0x620c,
 		.irq_status	= 0x6220,
 		.irq_mask	= 0x6228,
+		.adma_rx_dbg0	= 0x6238,
 		.int_grp	= 0x6250,
 	},
 	.qdma = {
@@ -143,6 +147,8 @@ static const struct mtk_reg_map mt7986_reg_map = {
 		[0]		= 0x4800,
 		[1]		= 0x4c00,
 	},
+	.pse_iq_sta		= 0x0180,
+	.pse_oq_sta		= 0x01a0,
 };
 
 /* strings used by ethtool */
@@ -3554,6 +3560,102 @@ static void mtk_hw_warm_reset(struct mtk_eth *eth)
 			val, rst_mask);
 }
 
+static bool mtk_hw_check_dma_hang(struct mtk_eth *eth)
+{
+	const struct mtk_reg_map *reg_map = eth->soc->reg_map;
+	bool gmac1_tx, gmac2_tx, gdm1_tx, gdm2_tx;
+	bool oq_hang, cdm1_busy, adma_busy;
+	bool wtx_busy, cdm_full, oq_free;
+	u32 wdidx, val, gdm1_fc, gdm2_fc;
+	bool qfsm_hang, qfwd_hang;
+	bool ret = false;
+
+	if (MTK_HAS_CAPS(eth->soc->caps, MTK_SOC_MT7628))
+		return false;
+
+	/* WDMA sanity checks */
+	wdidx = mtk_r32(eth, reg_map->wdma_base[0] + 0xc);
+
+	val = mtk_r32(eth, reg_map->wdma_base[0] + 0x204);
+	wtx_busy = FIELD_GET(MTK_TX_DMA_BUSY, val);
+
+	val = mtk_r32(eth, reg_map->wdma_base[0] + 0x230);
+	cdm_full = !FIELD_GET(MTK_CDM_TXFIFO_RDY, val);
+
+	oq_free  = (!(mtk_r32(eth, reg_map->pse_oq_sta) & GENMASK(24, 16)) &&
+		    !(mtk_r32(eth, reg_map->pse_oq_sta + 0x4) & GENMASK(8, 0)) &&
+		    !(mtk_r32(eth, reg_map->pse_oq_sta + 0x10) & GENMASK(24, 16)));
+
+	if (wdidx == eth->reset.wdidx && wtx_busy && cdm_full && oq_free) {
+		if (++eth->reset.wdma_hang_count > 2) {
+			eth->reset.wdma_hang_count = 0;
+			ret = true;
+		}
+		goto out;
+	}
+
+	/* QDMA sanity checks */
+	qfsm_hang = !!mtk_r32(eth, reg_map->qdma.qtx_cfg + 0x234);
+	qfwd_hang = !mtk_r32(eth, reg_map->qdma.qtx_cfg + 0x308);
+
+	gdm1_tx = FIELD_GET(GENMASK(31, 16), mtk_r32(eth, MTK_FE_GDM1_FSM)) > 0;
+	gdm2_tx = FIELD_GET(GENMASK(31, 16), mtk_r32(eth, MTK_FE_GDM2_FSM)) > 0;
+	gmac1_tx = FIELD_GET(GENMASK(31, 24), mtk_r32(eth, MTK_MAC_FSM(0))) != 1;
+	gmac2_tx = FIELD_GET(GENMASK(31, 24), mtk_r32(eth, MTK_MAC_FSM(1))) != 1;
+	gdm1_fc = mtk_r32(eth, reg_map->gdm1_cnt + 0x24);
+	gdm2_fc = mtk_r32(eth, reg_map->gdm1_cnt + 0x64);
+
+	if (qfsm_hang && qfwd_hang &&
+	    ((gdm1_tx && gmac1_tx && gdm1_fc < 1) ||
+	     (gdm2_tx && gmac2_tx && gdm2_fc < 1))) {
+		if (++eth->reset.qdma_hang_count > 2) {
+			eth->reset.qdma_hang_count = 0;
+			ret = true;
+		}
+		goto out;
+	}
+
+	/* ADMA sanity checks */
+	oq_hang = !!(mtk_r32(eth, reg_map->pse_oq_sta) & GENMASK(8, 0));
+	cdm1_busy = !!(mtk_r32(eth, MTK_FE_CDM1_FSM) & GENMASK(31, 16));
+	adma_busy = !(mtk_r32(eth, reg_map->pdma.adma_rx_dbg0) & GENMASK(4, 0)) &&
+		    !(mtk_r32(eth, reg_map->pdma.adma_rx_dbg0) & BIT(6));
+
+	if (oq_hang && cdm1_busy && adma_busy) {
+		if (++eth->reset.adma_hang_count > 2) {
+			eth->reset.adma_hang_count = 0;
+			ret = true;
+		}
+		goto out;
+	}
+
+	eth->reset.wdma_hang_count = 0;
+	eth->reset.qdma_hang_count = 0;
+	eth->reset.adma_hang_count = 0;
+out:
+	eth->reset.wdidx = wdidx;
+
+	return ret;
+}
+
+static void mtk_hw_reset_monitor_work(struct work_struct *work)
+{
+	struct delayed_work *del_work = to_delayed_work(work);
+	struct mtk_eth *eth = container_of(del_work, struct mtk_eth,
+					   reset.monitor_work);
+
+	if (test_bit(MTK_RESETTING, &eth->state))
+		goto out;
+
+	/* DMA stuck checks */
+	if (mtk_hw_check_dma_hang(eth))
+		schedule_work(&eth->pending_work);
+
+out:
+	schedule_delayed_work(&eth->reset.monitor_work,
+			      MTK_DMA_MONITOR_TIMEOUT);
+}
+
 static int mtk_hw_init(struct mtk_eth *eth, bool reset)
 {
 	u32 dma_mask = ETHSYS_DMA_AG_MAP_PDMA | ETHSYS_DMA_AG_MAP_QDMA |
@@ -3903,6 +4005,7 @@ static int mtk_cleanup(struct mtk_eth *eth)
 	mtk_unreg_dev(eth);
 	mtk_free_dev(eth);
 	cancel_work_sync(&eth->pending_work);
+	cancel_delayed_work_sync(&eth->reset.monitor_work);
 
 	return 0;
 }
@@ -4357,6 +4460,7 @@ static int mtk_probe(struct platform_device *pdev)
 
 	eth->rx_dim.mode = DIM_CQ_PERIOD_MODE_START_FROM_EQE;
 	INIT_WORK(&eth->rx_dim.work, mtk_dim_rx);
+	INIT_DELAYED_WORK(&eth->reset.monitor_work, mtk_hw_reset_monitor_work);
 
 	eth->tx_dim.mode = DIM_CQ_PERIOD_MODE_START_FROM_EQE;
 	INIT_WORK(&eth->tx_dim.work, mtk_dim_tx);
@@ -4559,6 +4663,8 @@ static int mtk_probe(struct platform_device *pdev)
 	netif_napi_add(&eth->dummy_dev, &eth->rx_napi, mtk_napi_rx);
 
 	platform_set_drvdata(pdev, eth);
+	schedule_delayed_work(&eth->reset.monitor_work,
+			      MTK_DMA_MONITOR_TIMEOUT);
 
 	return 0;
 
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index a8066b3ee3ed..dff0e3ad2de6 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -284,6 +284,8 @@
 
 #define MTK_RX_DONE_INT_V2	BIT(14)
 
+#define MTK_CDM_TXFIFO_RDY	BIT(7)
+
 /* QDMA Interrupt grouping registers */
 #define MTK_RLS_DONE_INT	BIT(0)
 
@@ -574,6 +576,17 @@
 #define MT7628_SDM_RBCNT	(MT7628_SDM_OFFSET + 0x10c)
 #define MT7628_SDM_CS_ERR	(MT7628_SDM_OFFSET + 0x110)
 
+#define MTK_FE_CDM1_FSM		0x220
+#define MTK_FE_CDM2_FSM		0x224
+#define MTK_FE_CDM3_FSM		0x238
+#define MTK_FE_CDM4_FSM		0x298
+#define MTK_FE_CDM5_FSM		0x318
+#define MTK_FE_CDM6_FSM		0x328
+#define MTK_FE_GDM1_FSM		0x228
+#define MTK_FE_GDM2_FSM		0x22C
+
+#define MTK_MAC_FSM(x)		(0x1010C + ((x) * 0x100))
+
 struct mtk_rx_dma {
 	unsigned int rxd1;
 	unsigned int rxd2;
@@ -970,6 +983,7 @@ struct mtk_reg_map {
 		u32	delay_irq;	/* delay interrupt */
 		u32	irq_status;	/* interrupt status */
 		u32	irq_mask;	/* interrupt mask */
+		u32	adma_rx_dbg0;
 		u32	int_grp;
 	} pdma;
 	struct {
@@ -998,6 +1012,8 @@ struct mtk_reg_map {
 	u32	gdma_to_ppe;
 	u32	ppe_base;
 	u32	wdma_base[2];
+	u32	pse_iq_sta;
+	u32	pse_oq_sta;
 };
 
 /* struct mtk_eth_data -	This is the structure holding all differences
@@ -1040,6 +1056,8 @@ struct mtk_soc_data {
 	} txrx;
 };
 
+#define MTK_DMA_MONITOR_TIMEOUT		msecs_to_jiffies(1000)
+
 /* currently no SoC has more than 2 macs */
 #define MTK_MAX_DEVS			2
 
@@ -1164,6 +1182,14 @@ struct mtk_eth {
 	struct rhashtable		flow_table;
 
 	struct bpf_prog			__rcu *prog;
+
+	struct {
+		struct delayed_work monitor_work;
+		u32 wdidx;
+		u8 wdma_hang_count;
+		u8 qdma_hang_count;
+		u8 adma_hang_count;
+	} reset;
 };
 
 /* struct mtk_mac -	the structure that holds the info about the MACs of the
-- 
2.39.0


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

* [PATCH v5 net-next 5/5] net: ethernet: mtk_wed: add reset/reset_complete callbacks
  2023-01-11 17:22 [PATCH v5 net-next 0/5] net: ethernet: mtk_wed: introduce reset support Lorenzo Bianconi
                   ` (3 preceding siblings ...)
  2023-01-11 17:22 ` [PATCH v5 net-next 4/5] net: ethernet: mtk_eth_soc: add dma checks to mtk_hw_reset_check Lorenzo Bianconi
@ 2023-01-11 17:22 ` Lorenzo Bianconi
  2023-01-12 16:15   ` Alexander H Duyck
  4 siblings, 1 reply; 10+ messages in thread
From: Lorenzo Bianconi @ 2023-01-11 17:22 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, kuba, pabeni, lorenzo.bianconi, nbd, john,
	sean.wang, Mark-MC.Lee, sujuan.chen, daniel, leon

Introduce reset and reset_complete wlan callback to schedule WLAN driver
reset when ethernet/wed driver is resetting.

Tested-by: Daniel Golle <daniel@makrotopia.org>
Co-developed-by: Sujuan Chen <sujuan.chen@mediatek.com>
Signed-off-by: Sujuan Chen <sujuan.chen@mediatek.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c |  7 ++++
 drivers/net/ethernet/mediatek/mtk_wed.c     | 40 +++++++++++++++++++++
 drivers/net/ethernet/mediatek/mtk_wed.h     |  8 +++++
 include/linux/soc/mediatek/mtk_wed.h        |  2 ++
 4 files changed, 57 insertions(+)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 1af74e9a6cd3..0147e98009c2 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -3924,6 +3924,11 @@ static void mtk_pending_work(struct work_struct *work)
 	set_bit(MTK_RESETTING, &eth->state);
 
 	mtk_prepare_for_reset(eth);
+	mtk_wed_fe_reset();
+	/* Run again reset preliminary configuration in order to avoid any
+	 * possible race during FE reset since it can run releasing RTNL lock.
+	 */
+	mtk_prepare_for_reset(eth);
 
 	/* stop all devices to make sure that dma is properly shut down */
 	for (i = 0; i < MTK_MAC_COUNT; i++) {
@@ -3961,6 +3966,8 @@ static void mtk_pending_work(struct work_struct *work)
 
 	clear_bit(MTK_RESETTING, &eth->state);
 
+	mtk_wed_fe_reset_complete();
+
 	rtnl_unlock();
 }
 
diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
index a6271449617f..4854993f2941 100644
--- a/drivers/net/ethernet/mediatek/mtk_wed.c
+++ b/drivers/net/ethernet/mediatek/mtk_wed.c
@@ -206,6 +206,46 @@ mtk_wed_wo_reset(struct mtk_wed_device *dev)
 	iounmap(reg);
 }
 
+void mtk_wed_fe_reset(void)
+{
+	int i;
+
+	mutex_lock(&hw_lock);
+
+	for (i = 0; i < ARRAY_SIZE(hw_list); i++) {
+		struct mtk_wed_hw *hw = hw_list[i];
+		struct mtk_wed_device *dev = hw->wed_dev;
+
+		if (!dev || !dev->wlan.reset)
+			continue;
+
+		/* reset callback blocks until WLAN reset is completed */
+		if (dev->wlan.reset(dev))
+			dev_err(dev->dev, "wlan reset failed\n");
+	}
+
+	mutex_unlock(&hw_lock);
+}
+
+void mtk_wed_fe_reset_complete(void)
+{
+	int i;
+
+	mutex_lock(&hw_lock);
+
+	for (i = 0; i < ARRAY_SIZE(hw_list); i++) {
+		struct mtk_wed_hw *hw = hw_list[i];
+		struct mtk_wed_device *dev = hw->wed_dev;
+
+		if (!dev || !dev->wlan.reset_complete)
+			continue;
+
+		dev->wlan.reset_complete(dev);
+	}
+
+	mutex_unlock(&hw_lock);
+}
+
 static struct mtk_wed_hw *
 mtk_wed_assign(struct mtk_wed_device *dev)
 {
diff --git a/drivers/net/ethernet/mediatek/mtk_wed.h b/drivers/net/ethernet/mediatek/mtk_wed.h
index e012b8a82133..6108a7e69a80 100644
--- a/drivers/net/ethernet/mediatek/mtk_wed.h
+++ b/drivers/net/ethernet/mediatek/mtk_wed.h
@@ -128,6 +128,8 @@ void mtk_wed_add_hw(struct device_node *np, struct mtk_eth *eth,
 void mtk_wed_exit(void);
 int mtk_wed_flow_add(int index);
 void mtk_wed_flow_remove(int index);
+void mtk_wed_fe_reset(void);
+void mtk_wed_fe_reset_complete(void);
 #else
 static inline void
 mtk_wed_add_hw(struct device_node *np, struct mtk_eth *eth,
@@ -146,6 +148,12 @@ static inline int mtk_wed_flow_add(int index)
 static inline void mtk_wed_flow_remove(int index)
 {
 }
+static inline void mtk_wed_fe_reset(void)
+{
+}
+static inline void mtk_wed_fe_reset_complete(void)
+{
+}
 
 #endif
 
diff --git a/include/linux/soc/mediatek/mtk_wed.h b/include/linux/soc/mediatek/mtk_wed.h
index db637a13888d..ddff54fc9717 100644
--- a/include/linux/soc/mediatek/mtk_wed.h
+++ b/include/linux/soc/mediatek/mtk_wed.h
@@ -150,6 +150,8 @@ struct mtk_wed_device {
 		void (*release_rx_buf)(struct mtk_wed_device *wed);
 		void (*update_wo_rx_stats)(struct mtk_wed_device *wed,
 					   struct mtk_wed_wo_rx_stats *stats);
+		int (*reset)(struct mtk_wed_device *wed);
+		int (*reset_complete)(struct mtk_wed_device *wed);
 	} wlan;
 #endif
 };
-- 
2.39.0


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

* Re: [PATCH v5 net-next 5/5] net: ethernet: mtk_wed: add reset/reset_complete callbacks
  2023-01-11 17:22 ` [PATCH v5 net-next 5/5] net: ethernet: mtk_wed: add reset/reset_complete callbacks Lorenzo Bianconi
@ 2023-01-12 16:15   ` Alexander H Duyck
  2023-01-12 16:26     ` Lorenzo Bianconi
  0 siblings, 1 reply; 10+ messages in thread
From: Alexander H Duyck @ 2023-01-12 16:15 UTC (permalink / raw)
  To: Lorenzo Bianconi, netdev
  Cc: davem, edumazet, kuba, pabeni, lorenzo.bianconi, nbd, john,
	sean.wang, Mark-MC.Lee, sujuan.chen, daniel, leon

On Wed, 2023-01-11 at 18:22 +0100, Lorenzo Bianconi wrote:
> Introduce reset and reset_complete wlan callback to schedule WLAN driver
> reset when ethernet/wed driver is resetting.
> 
> Tested-by: Daniel Golle <daniel@makrotopia.org>
> Co-developed-by: Sujuan Chen <sujuan.chen@mediatek.com>
> Signed-off-by: Sujuan Chen <sujuan.chen@mediatek.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>  drivers/net/ethernet/mediatek/mtk_eth_soc.c |  7 ++++
>  drivers/net/ethernet/mediatek/mtk_wed.c     | 40 +++++++++++++++++++++
>  drivers/net/ethernet/mediatek/mtk_wed.h     |  8 +++++
>  include/linux/soc/mediatek/mtk_wed.h        |  2 ++
>  4 files changed, 57 insertions(+)
> 

Do we have any updates on the implementation that would be making use
of this? It looks like there was a discussion for the v2 of this set to
include a link to an RFC posting that would make use of this set.

> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index 1af74e9a6cd3..0147e98009c2 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -3924,6 +3924,11 @@ static void mtk_pending_work(struct work_struct *work)
>  	set_bit(MTK_RESETTING, &eth->state);
>  
>  	mtk_prepare_for_reset(eth);
> +	mtk_wed_fe_reset();
> +	/* Run again reset preliminary configuration in order to avoid any
> +	 * possible race during FE reset since it can run releasing RTNL lock.
> +	 */
> +	mtk_prepare_for_reset(eth);
>  
>  	/* stop all devices to make sure that dma is properly shut down */
>  	for (i = 0; i < MTK_MAC_COUNT; i++) {
> @@ -3961,6 +3966,8 @@ static void mtk_pending_work(struct work_struct *work)
>  
>  	clear_bit(MTK_RESETTING, &eth->state);
>  
> +	mtk_wed_fe_reset_complete();
> +
>  	rtnl_unlock();
>  }
>  
> diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
> index a6271449617f..4854993f2941 100644
> --- a/drivers/net/ethernet/mediatek/mtk_wed.c
> +++ b/drivers/net/ethernet/mediatek/mtk_wed.c
> @@ -206,6 +206,46 @@ mtk_wed_wo_reset(struct mtk_wed_device *dev)
>  	iounmap(reg);
>  }
>  
> +void mtk_wed_fe_reset(void)
> +{
> +	int i;
> +
> +	mutex_lock(&hw_lock);
> +
> +	for (i = 0; i < ARRAY_SIZE(hw_list); i++) {
> +		struct mtk_wed_hw *hw = hw_list[i];
> +		struct mtk_wed_device *dev = hw->wed_dev;
> +
> +		if (!dev || !dev->wlan.reset)
> +			continue;
> +
> +		/* reset callback blocks until WLAN reset is completed */
> +		if (dev->wlan.reset(dev))
> +			dev_err(dev->dev, "wlan reset failed\n");

The reason why having the consumer would be useful are cases like this.
My main concern is if the error value might be useful to actually
expose rather than just treating it as a boolean. Usually for things
like this I prefer to see the result captured and if it indicates error
we return the error value since this could be one of several possible
causes for the error assuming this returns an int and not a bool.

> +	}
> +
> +	mutex_unlock(&hw_lock);
> +}
> +
> +void mtk_wed_fe_reset_complete(void)
> +{
> +	int i;
> +
> +	mutex_lock(&hw_lock);
> +
> +	for (i = 0; i < ARRAY_SIZE(hw_list); i++) {
> +		struct mtk_wed_hw *hw = hw_list[i];
> +		struct mtk_wed_device *dev = hw->wed_dev;
> +
> +		if (!dev || !dev->wlan.reset_complete)
> +			continue;
> +
> +		dev->wlan.reset_complete(dev);
> +	}
> +
> +	mutex_unlock(&hw_lock);
> +}
> +
>  static struct mtk_wed_hw *
>  mtk_wed_assign(struct mtk_wed_device *dev)
>  {


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

* Re: [PATCH v5 net-next 5/5] net: ethernet: mtk_wed: add reset/reset_complete callbacks
  2023-01-12 16:15   ` Alexander H Duyck
@ 2023-01-12 16:26     ` Lorenzo Bianconi
  2023-01-12 18:13       ` Alexander Duyck
  0 siblings, 1 reply; 10+ messages in thread
From: Lorenzo Bianconi @ 2023-01-12 16:26 UTC (permalink / raw)
  To: Alexander H Duyck
  Cc: netdev, davem, edumazet, kuba, pabeni, lorenzo.bianconi, nbd,
	john, sean.wang, Mark-MC.Lee, sujuan.chen, daniel, leon

[-- Attachment #1: Type: text/plain, Size: 4174 bytes --]

> On Wed, 2023-01-11 at 18:22 +0100, Lorenzo Bianconi wrote:
> > Introduce reset and reset_complete wlan callback to schedule WLAN driver
> > reset when ethernet/wed driver is resetting.
> > 
> > Tested-by: Daniel Golle <daniel@makrotopia.org>
> > Co-developed-by: Sujuan Chen <sujuan.chen@mediatek.com>
> > Signed-off-by: Sujuan Chen <sujuan.chen@mediatek.com>
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> >  drivers/net/ethernet/mediatek/mtk_eth_soc.c |  7 ++++
> >  drivers/net/ethernet/mediatek/mtk_wed.c     | 40 +++++++++++++++++++++
> >  drivers/net/ethernet/mediatek/mtk_wed.h     |  8 +++++
> >  include/linux/soc/mediatek/mtk_wed.h        |  2 ++
> >  4 files changed, 57 insertions(+)
> > 
> 
> Do we have any updates on the implementation that would be making use
> of this? It looks like there was a discussion for the v2 of this set to
> include a link to an RFC posting that would make use of this set.

I posted the series to linux-wireless mailing list adding netdev one in cc:
https://lore.kernel.org/linux-wireless/cover.1673103214.git.lorenzo@kernel.org/T/#md34b4ffcb07056794378fa4e8079458ecca69109

> 
> > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > index 1af74e9a6cd3..0147e98009c2 100644
> > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > @@ -3924,6 +3924,11 @@ static void mtk_pending_work(struct work_struct *work)
> >  	set_bit(MTK_RESETTING, &eth->state);
> >  
> >  	mtk_prepare_for_reset(eth);
> > +	mtk_wed_fe_reset();
> > +	/* Run again reset preliminary configuration in order to avoid any
> > +	 * possible race during FE reset since it can run releasing RTNL lock.
> > +	 */
> > +	mtk_prepare_for_reset(eth);
> >  
> >  	/* stop all devices to make sure that dma is properly shut down */
> >  	for (i = 0; i < MTK_MAC_COUNT; i++) {
> > @@ -3961,6 +3966,8 @@ static void mtk_pending_work(struct work_struct *work)
> >  
> >  	clear_bit(MTK_RESETTING, &eth->state);
> >  
> > +	mtk_wed_fe_reset_complete();
> > +
> >  	rtnl_unlock();
> >  }
> >  
> > diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
> > index a6271449617f..4854993f2941 100644
> > --- a/drivers/net/ethernet/mediatek/mtk_wed.c
> > +++ b/drivers/net/ethernet/mediatek/mtk_wed.c
> > @@ -206,6 +206,46 @@ mtk_wed_wo_reset(struct mtk_wed_device *dev)
> >  	iounmap(reg);
> >  }
> >  
> > +void mtk_wed_fe_reset(void)
> > +{
> > +	int i;
> > +
> > +	mutex_lock(&hw_lock);
> > +
> > +	for (i = 0; i < ARRAY_SIZE(hw_list); i++) {
> > +		struct mtk_wed_hw *hw = hw_list[i];
> > +		struct mtk_wed_device *dev = hw->wed_dev;
> > +
> > +		if (!dev || !dev->wlan.reset)
> > +			continue;
> > +
> > +		/* reset callback blocks until WLAN reset is completed */
> > +		if (dev->wlan.reset(dev))
> > +			dev_err(dev->dev, "wlan reset failed\n");
> 
> The reason why having the consumer would be useful are cases like this.
> My main concern is if the error value might be useful to actually
> expose rather than just treating it as a boolean. Usually for things
> like this I prefer to see the result captured and if it indicates error
> we return the error value since this could be one of several possible
> causes for the error assuming this returns an int and not a bool.

we can have 2 independent wireless chips connected here so, if the first one
fails, should we exit or just log the error?

Regards,
Lorenzo

> 
> > +	}
> > +
> > +	mutex_unlock(&hw_lock);
> > +}
> > +
> > +void mtk_wed_fe_reset_complete(void)
> > +{
> > +	int i;
> > +
> > +	mutex_lock(&hw_lock);
> > +
> > +	for (i = 0; i < ARRAY_SIZE(hw_list); i++) {
> > +		struct mtk_wed_hw *hw = hw_list[i];
> > +		struct mtk_wed_device *dev = hw->wed_dev;
> > +
> > +		if (!dev || !dev->wlan.reset_complete)
> > +			continue;
> > +
> > +		dev->wlan.reset_complete(dev);
> > +	}
> > +
> > +	mutex_unlock(&hw_lock);
> > +}
> > +
> >  static struct mtk_wed_hw *
> >  mtk_wed_assign(struct mtk_wed_device *dev)
> >  {
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH v5 net-next 5/5] net: ethernet: mtk_wed: add reset/reset_complete callbacks
  2023-01-12 16:26     ` Lorenzo Bianconi
@ 2023-01-12 18:13       ` Alexander Duyck
  2023-01-12 19:25         ` Lorenzo Bianconi
  0 siblings, 1 reply; 10+ messages in thread
From: Alexander Duyck @ 2023-01-12 18:13 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: netdev, davem, edumazet, kuba, pabeni, lorenzo.bianconi, nbd,
	john, sean.wang, Mark-MC.Lee, sujuan.chen, daniel, leon

On Thu, Jan 12, 2023 at 8:26 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
>
> > On Wed, 2023-01-11 at 18:22 +0100, Lorenzo Bianconi wrote:
> > > Introduce reset and reset_complete wlan callback to schedule WLAN driver
> > > reset when ethernet/wed driver is resetting.
> > >
> > > Tested-by: Daniel Golle <daniel@makrotopia.org>
> > > Co-developed-by: Sujuan Chen <sujuan.chen@mediatek.com>
> > > Signed-off-by: Sujuan Chen <sujuan.chen@mediatek.com>
> > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > ---
> > >  drivers/net/ethernet/mediatek/mtk_eth_soc.c |  7 ++++
> > >  drivers/net/ethernet/mediatek/mtk_wed.c     | 40 +++++++++++++++++++++
> > >  drivers/net/ethernet/mediatek/mtk_wed.h     |  8 +++++
> > >  include/linux/soc/mediatek/mtk_wed.h        |  2 ++
> > >  4 files changed, 57 insertions(+)
> > >
> >
> > Do we have any updates on the implementation that would be making use
> > of this? It looks like there was a discussion for the v2 of this set to
> > include a link to an RFC posting that would make use of this set.
>
> I posted the series to linux-wireless mailing list adding netdev one in cc:
> https://lore.kernel.org/linux-wireless/cover.1673103214.git.lorenzo@kernel.org/T/#md34b4ffcb07056794378fa4e8079458ecca69109

Thanks. It would be useful to include this link in the next revision
to make it easier to review.

> >
> > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > index 1af74e9a6cd3..0147e98009c2 100644
> > > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > @@ -3924,6 +3924,11 @@ static void mtk_pending_work(struct work_struct *work)
> > >     set_bit(MTK_RESETTING, &eth->state);
> > >
> > >     mtk_prepare_for_reset(eth);
> > > +   mtk_wed_fe_reset();
> > > +   /* Run again reset preliminary configuration in order to avoid any
> > > +    * possible race during FE reset since it can run releasing RTNL lock.
> > > +    */
> > > +   mtk_prepare_for_reset(eth);
> > >
> > >     /* stop all devices to make sure that dma is properly shut down */
> > >     for (i = 0; i < MTK_MAC_COUNT; i++) {
> > > @@ -3961,6 +3966,8 @@ static void mtk_pending_work(struct work_struct *work)
> > >
> > >     clear_bit(MTK_RESETTING, &eth->state);
> > >
> > > +   mtk_wed_fe_reset_complete();
> > > +
> > >     rtnl_unlock();
> > >  }
> > >
> > > diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
> > > index a6271449617f..4854993f2941 100644
> > > --- a/drivers/net/ethernet/mediatek/mtk_wed.c
> > > +++ b/drivers/net/ethernet/mediatek/mtk_wed.c
> > > @@ -206,6 +206,46 @@ mtk_wed_wo_reset(struct mtk_wed_device *dev)
> > >     iounmap(reg);
> > >  }
> > >
> > > +void mtk_wed_fe_reset(void)
> > > +{
> > > +   int i;
> > > +
> > > +   mutex_lock(&hw_lock);
> > > +
> > > +   for (i = 0; i < ARRAY_SIZE(hw_list); i++) {
> > > +           struct mtk_wed_hw *hw = hw_list[i];
> > > +           struct mtk_wed_device *dev = hw->wed_dev;
> > > +
> > > +           if (!dev || !dev->wlan.reset)
> > > +                   continue;
> > > +
> > > +           /* reset callback blocks until WLAN reset is completed */
> > > +           if (dev->wlan.reset(dev))
> > > +                   dev_err(dev->dev, "wlan reset failed\n");
> >
> > The reason why having the consumer would be useful are cases like this.
> > My main concern is if the error value might be useful to actually
> > expose rather than just treating it as a boolean. Usually for things
> > like this I prefer to see the result captured and if it indicates error
> > we return the error value since this could be one of several possible
> > causes for the error assuming this returns an int and not a bool.
>
> we can have 2 independent wireless chips connected here so, if the first one
> fails, should we exit or just log the error?

I would think you should log the error. I notice in your wireless
implementation you can return BUSY or TIMEOUT. Rather than doing the
dev_err in your reset function to distinguish between the two you
could just return the error and leave the printing of the error to
this dev_err message.

Also a follow-on question I had. It looks like reset_complete returns
an int but it is being ignored and in your implementation it is just
returning 0. Should that be a void instead of an int?

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

* Re: [PATCH v5 net-next 5/5] net: ethernet: mtk_wed: add reset/reset_complete callbacks
  2023-01-12 18:13       ` Alexander Duyck
@ 2023-01-12 19:25         ` Lorenzo Bianconi
  0 siblings, 0 replies; 10+ messages in thread
From: Lorenzo Bianconi @ 2023-01-12 19:25 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Lorenzo Bianconi, netdev, davem, edumazet, kuba, pabeni, nbd,
	john, sean.wang, Mark-MC.Lee, sujuan.chen, daniel, leon

[-- Attachment #1: Type: text/plain, Size: 4765 bytes --]

> On Thu, Jan 12, 2023 at 8:26 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> >
> > > On Wed, 2023-01-11 at 18:22 +0100, Lorenzo Bianconi wrote:
> > > > Introduce reset and reset_complete wlan callback to schedule WLAN driver
> > > > reset when ethernet/wed driver is resetting.
> > > >
> > > > Tested-by: Daniel Golle <daniel@makrotopia.org>
> > > > Co-developed-by: Sujuan Chen <sujuan.chen@mediatek.com>
> > > > Signed-off-by: Sujuan Chen <sujuan.chen@mediatek.com>
> > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > > ---
> > > >  drivers/net/ethernet/mediatek/mtk_eth_soc.c |  7 ++++
> > > >  drivers/net/ethernet/mediatek/mtk_wed.c     | 40 +++++++++++++++++++++
> > > >  drivers/net/ethernet/mediatek/mtk_wed.h     |  8 +++++
> > > >  include/linux/soc/mediatek/mtk_wed.h        |  2 ++
> > > >  4 files changed, 57 insertions(+)
> > > >
> > >
> > > Do we have any updates on the implementation that would be making use
> > > of this? It looks like there was a discussion for the v2 of this set to
> > > include a link to an RFC posting that would make use of this set.
> >
> > I posted the series to linux-wireless mailing list adding netdev one in cc:
> > https://lore.kernel.org/linux-wireless/cover.1673103214.git.lorenzo@kernel.org/T/#md34b4ffcb07056794378fa4e8079458ecca69109
> 
> Thanks. It would be useful to include this link in the next revision
> to make it easier to review.

ack, will do.

> 
> > >
> > > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > > index 1af74e9a6cd3..0147e98009c2 100644
> > > > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > > @@ -3924,6 +3924,11 @@ static void mtk_pending_work(struct work_struct *work)
> > > >     set_bit(MTK_RESETTING, &eth->state);
> > > >
> > > >     mtk_prepare_for_reset(eth);
> > > > +   mtk_wed_fe_reset();
> > > > +   /* Run again reset preliminary configuration in order to avoid any
> > > > +    * possible race during FE reset since it can run releasing RTNL lock.
> > > > +    */
> > > > +   mtk_prepare_for_reset(eth);
> > > >
> > > >     /* stop all devices to make sure that dma is properly shut down */
> > > >     for (i = 0; i < MTK_MAC_COUNT; i++) {
> > > > @@ -3961,6 +3966,8 @@ static void mtk_pending_work(struct work_struct *work)
> > > >
> > > >     clear_bit(MTK_RESETTING, &eth->state);
> > > >
> > > > +   mtk_wed_fe_reset_complete();
> > > > +
> > > >     rtnl_unlock();
> > > >  }
> > > >
> > > > diff --git a/drivers/net/ethernet/mediatek/mtk_wed.c b/drivers/net/ethernet/mediatek/mtk_wed.c
> > > > index a6271449617f..4854993f2941 100644
> > > > --- a/drivers/net/ethernet/mediatek/mtk_wed.c
> > > > +++ b/drivers/net/ethernet/mediatek/mtk_wed.c
> > > > @@ -206,6 +206,46 @@ mtk_wed_wo_reset(struct mtk_wed_device *dev)
> > > >     iounmap(reg);
> > > >  }
> > > >
> > > > +void mtk_wed_fe_reset(void)
> > > > +{
> > > > +   int i;
> > > > +
> > > > +   mutex_lock(&hw_lock);
> > > > +
> > > > +   for (i = 0; i < ARRAY_SIZE(hw_list); i++) {
> > > > +           struct mtk_wed_hw *hw = hw_list[i];
> > > > +           struct mtk_wed_device *dev = hw->wed_dev;
> > > > +
> > > > +           if (!dev || !dev->wlan.reset)
> > > > +                   continue;
> > > > +
> > > > +           /* reset callback blocks until WLAN reset is completed */
> > > > +           if (dev->wlan.reset(dev))
> > > > +                   dev_err(dev->dev, "wlan reset failed\n");
> > >
> > > The reason why having the consumer would be useful are cases like this.
> > > My main concern is if the error value might be useful to actually
> > > expose rather than just treating it as a boolean. Usually for things
> > > like this I prefer to see the result captured and if it indicates error
> > > we return the error value since this could be one of several possible
> > > causes for the error assuming this returns an int and not a bool.
> >
> > we can have 2 independent wireless chips connected here so, if the first one
> > fails, should we exit or just log the error?
> 
> I would think you should log the error. I notice in your wireless
> implementation you can return BUSY or TIMEOUT. Rather than doing the
> dev_err in your reset function to distinguish between the two you
> could just return the error and leave the printing of the error to
> this dev_err message.

ack, will do.

> 
> Also a follow-on question I had. It looks like reset_complete returns
> an int but it is being ignored and in your implementation it is just
> returning 0. Should that be a void instead of an int?
> 

ack, will do.

Regards,
Lorenzo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

end of thread, other threads:[~2023-01-12 19:31 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-11 17:22 [PATCH v5 net-next 0/5] net: ethernet: mtk_wed: introduce reset support Lorenzo Bianconi
2023-01-11 17:22 ` [PATCH v5 net-next 1/5] net: ethernet: mtk_eth_soc: introduce mtk_hw_reset utility routine Lorenzo Bianconi
2023-01-11 17:22 ` [PATCH v5 net-next 2/5] net: ethernet: mtk_eth_soc: introduce mtk_hw_warm_reset support Lorenzo Bianconi
2023-01-11 17:22 ` [PATCH v5 net-next 3/5] net: ethernet: mtk_eth_soc: align reset procedure to vendor sdk Lorenzo Bianconi
2023-01-11 17:22 ` [PATCH v5 net-next 4/5] net: ethernet: mtk_eth_soc: add dma checks to mtk_hw_reset_check Lorenzo Bianconi
2023-01-11 17:22 ` [PATCH v5 net-next 5/5] net: ethernet: mtk_wed: add reset/reset_complete callbacks Lorenzo Bianconi
2023-01-12 16:15   ` Alexander H Duyck
2023-01-12 16:26     ` Lorenzo Bianconi
2023-01-12 18:13       ` Alexander Duyck
2023-01-12 19:25         ` Lorenzo Bianconi

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