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

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 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  | 288 ++++++++++++++++---
 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, 369 insertions(+), 41 deletions(-)

-- 
2.39.0


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

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

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

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] 14+ messages in thread

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

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.

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 | 59 +++++++++++++++++++--
 1 file changed, 55 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..ffd4dbee0488 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,11 @@ static int mtk_hw_init(struct mtk_eth *eth)
 		return 0;
 	}
 
-	mtk_hw_reset(eth);
+	mdelay(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 +3794,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 +4423,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] 14+ messages in thread

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

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

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  | 86 ++++++++++++++------
 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, 108 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index ffd4dbee0488..ba924ceb6c94 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 (!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");
+
+	if (test_bit(MTK_RESETTING, &eth->state))
+		return;
+
 	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,
@@ -3686,8 +3703,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;
 }
@@ -3769,46 +3788,65 @@ static int mtk_do_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 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);
 
+	/* 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));
+	}
+
 	/* 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);
+	mdelay(15);
 
 	if (eth->dev->pins)
 		pinctrl_select_state(eth->dev->pins->p,
 				     eth->dev->pins->default_state);
+
 	mtk_hw_init(eth, true);
 
 	/* restart DMA and enable IRQs */
 	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..994408d8f416 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);
+	mdelay(10);
+
+	/* 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);
+	mdelay(10);
+
+	/* disable scan mode */
+	ppe_clear(ppe, MTK_PPE_TB_CFG, MTK_PPE_TB_CFG_SCAN_MODE);
+	mdelay(10);
+
+	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] 14+ messages in thread

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

Introduce mtk_hw_check_dma_hang routine to monitor possible dma hangs.

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 | 103 ++++++++++++++++++++
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |  26 +++++
 2 files changed, 129 insertions(+)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index ba924ceb6c94..bafae4f0312e 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,99 @@ 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);
+
+	/* DMA stuck checks */
+	if (!test_bit(MTK_RESETTING, &eth->state) &&
+	    mtk_hw_check_dma_hang(eth))
+		schedule_work(&eth->pending_work);
+
+	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 |
@@ -3894,6 +3993,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;
 }
@@ -4348,6 +4448,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);
@@ -4550,6 +4651,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] 14+ messages in thread

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

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 |  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 ++
 4 files changed, 56 insertions(+)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index bafae4f0312e..2d74e26f45c9 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -3913,6 +3913,10 @@ static void mtk_pending_work(struct work_struct *work)
 		mtk_w32(eth, val, MTK_MAC_MCR(i));
 	}
 
+	rtnl_unlock();
+	mtk_wed_fe_reset();
+	rtnl_lock();
+
 	/* stop all devices to make sure that dma is properly shut down */
 	for (i = 0; i < MTK_MAC_COUNT; i++) {
 		if (!eth->netdev[i] || !netif_running(eth->netdev[i]))
@@ -3949,6 +3953,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] 14+ messages in thread

* Re: [PATCH v2 net-next 5/5] net: ethernet: mtk_wed: add reset/reset_complete callbacks
  2023-01-04 14:03 ` [PATCH v2 net-next 5/5] net: ethernet: mtk_wed: add reset/reset_complete callbacks Lorenzo Bianconi
@ 2023-01-04 14:17   ` Leon Romanovsky
  2023-01-04 14:59     ` Lorenzo Bianconi
  0 siblings, 1 reply; 14+ messages in thread
From: Leon Romanovsky @ 2023-01-04 14:17 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: netdev, davem, edumazet, kuba, pabeni, lorenzo.bianconi, nbd,
	john, sean.wang, Mark-MC.Lee, sujuan.chen, daniel

On Wed, Jan 04, 2023 at 03:03:14PM +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 |  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 ++
>  4 files changed, 56 insertions(+)
> 
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index bafae4f0312e..2d74e26f45c9 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -3913,6 +3913,10 @@ static void mtk_pending_work(struct work_struct *work)
>  		mtk_w32(eth, val, MTK_MAC_MCR(i));
>  	}
>  
> +	rtnl_unlock();
> +	mtk_wed_fe_reset();
> +	rtnl_lock();

Is it safe to call rtnl_unlock(), perform some work and lock again?

> +
>  	/* stop all devices to make sure that dma is properly shut down */
>  	for (i = 0; i < MTK_MAC_COUNT; i++) {
>  		if (!eth->netdev[i] || !netif_running(eth->netdev[i]))
> @@ -3949,6 +3953,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);

I don't see any driver implementation of these callbacks in this series.
Did I miss it?

Thanks

>  	} wlan;
>  #endif
>  };
> -- 
> 2.39.0
> 

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

* Re: [PATCH v2 net-next 5/5] net: ethernet: mtk_wed: add reset/reset_complete callbacks
  2023-01-04 14:17   ` Leon Romanovsky
@ 2023-01-04 14:59     ` Lorenzo Bianconi
  2023-01-05  9:22       ` Leon Romanovsky
  0 siblings, 1 reply; 14+ messages in thread
From: Lorenzo Bianconi @ 2023-01-04 14:59 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: netdev, davem, edumazet, kuba, pabeni, lorenzo.bianconi, nbd,
	john, sean.wang, Mark-MC.Lee, sujuan.chen, daniel

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

> On Wed, Jan 04, 2023 at 03:03:14PM +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 |  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 ++
> >  4 files changed, 56 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > index bafae4f0312e..2d74e26f45c9 100644
> > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > @@ -3913,6 +3913,10 @@ static void mtk_pending_work(struct work_struct *work)
> >  		mtk_w32(eth, val, MTK_MAC_MCR(i));
> >  	}
> >  
> > +	rtnl_unlock();
> > +	mtk_wed_fe_reset();
> > +	rtnl_lock();
> 
> Is it safe to call rtnl_unlock(), perform some work and lock again?

Yes, mtk_pending_work sets MTK_RESETTING bit and a new reset work is not
scheduled until this bit is cleared

> 
> > +
> >  	/* stop all devices to make sure that dma is properly shut down */
> >  	for (i = 0; i < MTK_MAC_COUNT; i++) {
> >  		if (!eth->netdev[i] || !netif_running(eth->netdev[i]))
> > @@ -3949,6 +3953,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);
> 
> I don't see any driver implementation of these callbacks in this series.
> Did I miss it?

These callbacks are implemented in the mt76 driver. I have not added these
patches to the series since mt76 patches usually go through Felix/Kalle's
trees (anyway I am fine to add them to the series if they can go into net-next
directly).

Regards,
Lorenzo

> 
> Thanks
> 
> >  	} wlan;
> >  #endif
> >  };
> > -- 
> > 2.39.0
> > 

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

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

* Re: [PATCH v2 net-next 5/5] net: ethernet: mtk_wed: add reset/reset_complete callbacks
  2023-01-04 14:59     ` Lorenzo Bianconi
@ 2023-01-05  9:22       ` Leon Romanovsky
  2023-01-05 11:49         ` Lorenzo Bianconi
  2023-01-07 14:39         ` Lorenzo Bianconi
  0 siblings, 2 replies; 14+ messages in thread
From: Leon Romanovsky @ 2023-01-05  9:22 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: netdev, davem, edumazet, kuba, pabeni, lorenzo.bianconi, nbd,
	john, sean.wang, Mark-MC.Lee, sujuan.chen, daniel

On Wed, Jan 04, 2023 at 03:59:17PM +0100, Lorenzo Bianconi wrote:
> > On Wed, Jan 04, 2023 at 03:03:14PM +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 |  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 ++
> > >  4 files changed, 56 insertions(+)
> > > 
> > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > index bafae4f0312e..2d74e26f45c9 100644
> > > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > @@ -3913,6 +3913,10 @@ static void mtk_pending_work(struct work_struct *work)
> > >  		mtk_w32(eth, val, MTK_MAC_MCR(i));
> > >  	}
> > >  
> > > +	rtnl_unlock();
> > > +	mtk_wed_fe_reset();
> > > +	rtnl_lock();
> > 
> > Is it safe to call rtnl_unlock(), perform some work and lock again?
> 
> Yes, mtk_pending_work sets MTK_RESETTING bit and a new reset work is not
> scheduled until this bit is cleared

I'm more worried about opening a window for user-space access while you
are performing FW reset.

<...>

> > > 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);
> > 
> > I don't see any driver implementation of these callbacks in this series.
> > Did I miss it?
> 
> These callbacks are implemented in the mt76 driver. I have not added these
> patches to the series since mt76 patches usually go through Felix/Kalle's
> trees (anyway I am fine to add them to the series if they can go into net-next
> directly).

Usually patches that use specific functionality are submitted together
with API changes.

> 
> Regards,
> Lorenzo
> 
> > 
> > Thanks
> > 
> > >  	} wlan;
> > >  #endif
> > >  };
> > > -- 
> > > 2.39.0
> > > 



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

* Re: [PATCH v2 net-next 5/5] net: ethernet: mtk_wed: add reset/reset_complete callbacks
  2023-01-05  9:22       ` Leon Romanovsky
@ 2023-01-05 11:49         ` Lorenzo Bianconi
  2023-01-06  5:48           ` Jakub Kicinski
  2023-01-07 14:39         ` Lorenzo Bianconi
  1 sibling, 1 reply; 14+ messages in thread
From: Lorenzo Bianconi @ 2023-01-05 11:49 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: netdev, davem, edumazet, kuba, pabeni, lorenzo.bianconi, nbd,
	john, sean.wang, Mark-MC.Lee, sujuan.chen, daniel, kvalo

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

> On Wed, Jan 04, 2023 at 03:59:17PM +0100, Lorenzo Bianconi wrote:
> > > On Wed, Jan 04, 2023 at 03:03:14PM +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 |  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 ++
> > > >  4 files changed, 56 insertions(+)
> > > > 
> > > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > > index bafae4f0312e..2d74e26f45c9 100644
> > > > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > > @@ -3913,6 +3913,10 @@ static void mtk_pending_work(struct work_struct *work)
> > > >  		mtk_w32(eth, val, MTK_MAC_MCR(i));
> > > >  	}
> > > >  
> > > > +	rtnl_unlock();
> > > > +	mtk_wed_fe_reset();
> > > > +	rtnl_lock();
> > > 
> > > Is it safe to call rtnl_unlock(), perform some work and lock again?
> > 
> > Yes, mtk_pending_work sets MTK_RESETTING bit and a new reset work is not
> > scheduled until this bit is cleared
> 
> I'm more worried about opening a window for user-space access while you
> are performing FW reset.

ack, right. I will work on it.

> 
> <...>
> 
> > > > 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);
> > > 
> > > I don't see any driver implementation of these callbacks in this series.
> > > Did I miss it?
> > 
> > These callbacks are implemented in the mt76 driver. I have not added these
> > patches to the series since mt76 patches usually go through Felix/Kalle's
> > trees (anyway I am fine to add them to the series if they can go into net-next
> > directly).
> 
> Usually patches that use specific functionality are submitted together
> with API changes.

I would say it is better mt76 patches go through Felix/Kalle's tree in order to avoid
conflicts.

@Felix, Kalle: any opinions?

Regards,
Lorenzo

> 
> > 
> > Regards,
> > Lorenzo
> > 
> > > 
> > > Thanks
> > > 
> > > >  	} wlan;
> > > >  #endif
> > > >  };
> > > > -- 
> > > > 2.39.0
> > > > 
> 
> 

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

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

* Re: [PATCH v2 net-next 5/5] net: ethernet: mtk_wed: add reset/reset_complete callbacks
  2023-01-05 11:49         ` Lorenzo Bianconi
@ 2023-01-06  5:48           ` Jakub Kicinski
  2023-01-06  9:11             ` Lorenzo Bianconi
  2023-01-09 14:46             ` Kalle Valo
  0 siblings, 2 replies; 14+ messages in thread
From: Jakub Kicinski @ 2023-01-06  5:48 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: Leon Romanovsky, netdev, davem, edumazet, pabeni,
	lorenzo.bianconi, nbd, john, sean.wang, Mark-MC.Lee, sujuan.chen,
	daniel, kvalo

On Thu, 5 Jan 2023 12:49:49 +0100 Lorenzo Bianconi wrote:
> > > These callbacks are implemented in the mt76 driver. I have not added these
> > > patches to the series since mt76 patches usually go through Felix/Kalle's
> > > trees (anyway I am fine to add them to the series if they can go into net-next
> > > directly).  
> > 
> > Usually patches that use specific functionality are submitted together
> > with API changes.  
> 
> I would say it is better mt76 patches go through Felix/Kalle's tree in order to avoid
> conflicts.
> 
> @Felix, Kalle: any opinions?

FWIW as long as the implementation is in net-next before the merge
window I'm fine either way. But it would be good to see the
implementation, a co-posted RFC maybe?

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

* Re: [PATCH v2 net-next 5/5] net: ethernet: mtk_wed: add reset/reset_complete callbacks
  2023-01-06  5:48           ` Jakub Kicinski
@ 2023-01-06  9:11             ` Lorenzo Bianconi
  2023-01-09 14:46             ` Kalle Valo
  1 sibling, 0 replies; 14+ messages in thread
From: Lorenzo Bianconi @ 2023-01-06  9:11 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Leon Romanovsky, netdev, davem, edumazet, pabeni,
	lorenzo.bianconi, nbd, john, sean.wang, Mark-MC.Lee, sujuan.chen,
	daniel, kvalo

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

On Jan 05, Jakub Kicinski wrote:
> On Thu, 5 Jan 2023 12:49:49 +0100 Lorenzo Bianconi wrote:
> > > > These callbacks are implemented in the mt76 driver. I have not added these
> > > > patches to the series since mt76 patches usually go through Felix/Kalle's
> > > > trees (anyway I am fine to add them to the series if they can go into net-next
> > > > directly).  
> > > 
> > > Usually patches that use specific functionality are submitted together
> > > with API changes.  
> > 
> > I would say it is better mt76 patches go through Felix/Kalle's tree in order to avoid
> > conflicts.
> > 
> > @Felix, Kalle: any opinions?
> 
> FWIW as long as the implementation is in net-next before the merge
> window I'm fine either way. But it would be good to see the
> implementation, a co-posted RFC maybe?

ack, I will post mt76 series to wireless mailing list just after the new version
of this one.

Regards,
Lorenzo

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

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

* Re: [PATCH v2 net-next 5/5] net: ethernet: mtk_wed: add reset/reset_complete callbacks
  2023-01-05  9:22       ` Leon Romanovsky
  2023-01-05 11:49         ` Lorenzo Bianconi
@ 2023-01-07 14:39         ` Lorenzo Bianconi
  1 sibling, 0 replies; 14+ messages in thread
From: Lorenzo Bianconi @ 2023-01-07 14:39 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: netdev, davem, edumazet, kuba, pabeni, lorenzo.bianconi, nbd,
	john, sean.wang, Mark-MC.Lee, sujuan.chen, daniel

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

> On Wed, Jan 04, 2023 at 03:59:17PM +0100, Lorenzo Bianconi wrote:
> > > On Wed, Jan 04, 2023 at 03:03:14PM +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 |  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 ++
> > > >  4 files changed, 56 insertions(+)
> > > > 
> > > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > > index bafae4f0312e..2d74e26f45c9 100644
> > > > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > > @@ -3913,6 +3913,10 @@ static void mtk_pending_work(struct work_struct *work)
> > > >  		mtk_w32(eth, val, MTK_MAC_MCR(i));
> > > >  	}
> > > >  
> > > > +	rtnl_unlock();
> > > > +	mtk_wed_fe_reset();
> > > > +	rtnl_lock();
> > > 
> > > Is it safe to call rtnl_unlock(), perform some work and lock again?
> > 
> > Yes, mtk_pending_work sets MTK_RESETTING bit and a new reset work is not
> > scheduled until this bit is cleared
> 
> I'm more worried about opening a window for user-space access while you
> are performing FW reset.

looking at mtk_pending_work() I guess running mtk_wed_fe_reset() releasing RTNL
lock is not harmful since we just perform few actions (ppe reset, ...)  before
running mtk_wed_fe_reset(). Moreover, the core reset part in mtk_pending_work()
(mtk_stop(), mtk_open(), ..) is done after mtk_wed_fe_reset() where we reacquired
RTNL lock.
In order to avoid any possible race, I guess we can just re-do the preliminary
reset configuration done before mtk_wed_fe_reset() just after re-acquiring RTNL
lock.

Regards,
Lorenzo

> 
> <...>
> 
> > > > 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);
> > > 
> > > I don't see any driver implementation of these callbacks in this series.
> > > Did I miss it?
> > 
> > These callbacks are implemented in the mt76 driver. I have not added these
> > patches to the series since mt76 patches usually go through Felix/Kalle's
> > trees (anyway I am fine to add them to the series if they can go into net-next
> > directly).
> 
> Usually patches that use specific functionality are submitted together
> with API changes.
> 
> > 
> > Regards,
> > Lorenzo
> > 
> > > 
> > > Thanks
> > > 
> > > >  	} wlan;
> > > >  #endif
> > > >  };
> > > > -- 
> > > > 2.39.0
> > > > 
> 
> 

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

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

* Re: [PATCH v2 net-next 5/5] net: ethernet: mtk_wed: add reset/reset_complete callbacks
  2023-01-06  5:48           ` Jakub Kicinski
  2023-01-06  9:11             ` Lorenzo Bianconi
@ 2023-01-09 14:46             ` Kalle Valo
  1 sibling, 0 replies; 14+ messages in thread
From: Kalle Valo @ 2023-01-09 14:46 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Lorenzo Bianconi, Leon Romanovsky, netdev, davem, edumazet,
	pabeni, lorenzo.bianconi, nbd, john, sean.wang, Mark-MC.Lee,
	sujuan.chen, daniel

(now back from vacation)

Jakub Kicinski <kuba@kernel.org> writes:

> On Thu, 5 Jan 2023 12:49:49 +0100 Lorenzo Bianconi wrote:
>> > > These callbacks are implemented in the mt76 driver. I have not added these
>> > > patches to the series since mt76 patches usually go through Felix/Kalle's
>> > > trees (anyway I am fine to add them to the series if they can go into net-next
>> > > directly).  
>> > 
>> > Usually patches that use specific functionality are submitted together
>> > with API changes.  
>> 
>> I would say it is better mt76 patches go through Felix/Kalle's tree in order to avoid
>> conflicts.
>> 
>> @Felix, Kalle: any opinions?
>
> FWIW as long as the implementation is in net-next before the merge
> window I'm fine either way. But it would be good to see the
> implementation, a co-posted RFC maybe?

FWIW I agree with Lorenzo, it would be good to get mt76 patches via
Felix's tree. Otherwise the conflicts might be annoying to fix.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

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

end of thread, other threads:[~2023-01-09 14:50 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-04 14:03 [PATCH v2 net-next 0/5] net: ethernet: mtk_wed: introduce reset support Lorenzo Bianconi
2023-01-04 14:03 ` [PATCH v2 net-next 1/5] net: ethernet: mtk_eth_soc: introduce mtk_hw_reset utility routine Lorenzo Bianconi
2023-01-04 14:03 ` [PATCH v2 net-next 2/5] net: ethernet: mtk_eth_soc: introduce mtk_hw_warm_reset support Lorenzo Bianconi
2023-01-04 14:03 ` [PATCH v2 net-next 3/5] net: ethernet: mtk_eth_soc: align reset procedure to vendor sdk Lorenzo Bianconi
2023-01-04 14:03 ` [PATCH v2 net-next 4/5] net: ethernet: mtk_eth_soc: add dma checks to mtk_hw_reset_check Lorenzo Bianconi
2023-01-04 14:03 ` [PATCH v2 net-next 5/5] net: ethernet: mtk_wed: add reset/reset_complete callbacks Lorenzo Bianconi
2023-01-04 14:17   ` Leon Romanovsky
2023-01-04 14:59     ` Lorenzo Bianconi
2023-01-05  9:22       ` Leon Romanovsky
2023-01-05 11:49         ` Lorenzo Bianconi
2023-01-06  5:48           ` Jakub Kicinski
2023-01-06  9:11             ` Lorenzo Bianconi
2023-01-09 14:46             ` Kalle Valo
2023-01-07 14:39         ` Lorenzo Bianconi

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.