All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] net: ethernet: mediatek: rework GDM setup flow
@ 2019-11-07 10:51 ` MarkLee
  0 siblings, 0 replies; 9+ messages in thread
From: MarkLee @ 2019-11-07 10:51 UTC (permalink / raw)
  To: David S. Miller, Sean Wang, John Crispin, Matthias Brugger, Andrew Lunn
  Cc: Rob Herring, Mark Rutland, Rene van Dorst, devicetree, netdev,
	linux-arm-kernel, linux-mediatek, linux-kernel, Jakub Kicinski,
	MarkLee

net: ethernet: mediatek: rework GDM setup flow

The mt762x GDM block is mainly used to setup the HW 
internal rx path - from GMAC port to RX DMA engine(PDMA/QDMA). 
And the internal packet switching engine(PSE) is responsed 
to do the data forward/drop following the GDM configuration.

This pacth target to simpfy the GDM setup flow by integrating 
them into one single function "mtk_gdm_config".
Besides, accroding to the mt762x HW design, it is recommended to 
enable PSE to forward data after DMA has been started and 
set PSE to drop all data before stopping DMA. 

Note, mt7628 is a different IP from other mt762x, so we exclude it
in mtk_gdm_config function.

Signed-off-by: MarkLee <Mark-MC.Lee@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 44 ++++++++++++++-------
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |  2 +
 2 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 703adb96429e..7220abb3e731 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -2180,6 +2180,31 @@ static int mtk_start_dma(struct mtk_eth *eth)
 	return 0;
 }
 
+static void mtk_gdm_config(struct mtk_eth *eth, u32 config)
+{
+	int i;
+
+	if (MTK_HAS_CAPS(eth->soc->caps, MTK_SOC_MT7628))
+		return;
+
+	for (i = 0; i < 2; i++) {
+		u32 val = mtk_r32(eth, MTK_GDMA_FWD_CFG(i));
+
+		/* Always enable RX checksum */
+		val |= MTK_GDMA_ICS_EN | MTK_GDMA_TCS_EN | MTK_GDMA_UCS_EN;
+
+		/* default setup the forward port to send frame to PDMA */
+		val &= ~0xffff;
+
+		val |= config;
+
+		mtk_w32(eth, val, MTK_GDMA_FWD_CFG(i));
+	}
+	/*Reset and enable PSE*/
+	mtk_w32(eth, RST_GL_PSE, MTK_RST_GL);
+	mtk_w32(eth, 0, MTK_RST_GL);
+}
+
 static int mtk_open(struct net_device *dev)
 {
 	struct mtk_mac *mac = netdev_priv(dev);
@@ -2200,6 +2225,8 @@ static int mtk_open(struct net_device *dev)
 		if (err)
 			return err;
 
+		mtk_gdm_config(eth, MTK_GDMA_TO_PDMA);
+
 		napi_enable(&eth->tx_napi);
 		napi_enable(&eth->rx_napi);
 		mtk_tx_irq_enable(eth, MTK_TX_DONE_INT);
@@ -2252,6 +2279,8 @@ static int mtk_stop(struct net_device *dev)
 	if (!refcount_dec_and_test(&eth->dma_refcnt))
 		return 0;
 
+	mtk_gdm_config(eth, MTK_GDMA_DROP_ALL);
+
 	mtk_tx_irq_disable(eth, MTK_TX_DONE_INT);
 	mtk_rx_irq_disable(eth, MTK_RX_DONE_INT);
 	napi_disable(&eth->tx_napi);
@@ -2375,8 +2404,6 @@ static int mtk_hw_init(struct mtk_eth *eth)
 	mtk_w32(eth, 0, MTK_QDMA_DELAY_INT);
 	mtk_tx_irq_disable(eth, ~0);
 	mtk_rx_irq_disable(eth, ~0);
-	mtk_w32(eth, RST_GL_PSE, MTK_RST_GL);
-	mtk_w32(eth, 0, MTK_RST_GL);
 
 	/* FE int grouping */
 	mtk_w32(eth, MTK_TX_DONE_INT, MTK_PDMA_INT_GRP1);
@@ -2385,19 +2412,6 @@ static int mtk_hw_init(struct mtk_eth *eth)
 	mtk_w32(eth, MTK_RX_DONE_INT, MTK_QDMA_INT_GRP2);
 	mtk_w32(eth, 0x21021000, MTK_FE_INT_GRP);
 
-	for (i = 0; i < MTK_MAC_COUNT; i++) {
-		u32 val = mtk_r32(eth, MTK_GDMA_FWD_CFG(i));
-
-		/* setup the forward port to send frame to PDMA */
-		val &= ~0xffff;
-
-		/* Enable RX checksum */
-		val |= MTK_GDMA_ICS_EN | MTK_GDMA_TCS_EN | MTK_GDMA_UCS_EN;
-
-		/* setup the mac dma */
-		mtk_w32(eth, val, MTK_GDMA_FWD_CFG(i));
-	}
-
 	return 0;
 
 err_disable_pm:
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index 76bd12cb8150..b8bcfdfc995e 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -84,6 +84,8 @@
 #define MTK_GDMA_ICS_EN		BIT(22)
 #define MTK_GDMA_TCS_EN		BIT(21)
 #define MTK_GDMA_UCS_EN		BIT(20)
+#define MTK_GDMA_DROP_ALL	0x7777
+#define MTK_GDMA_TO_PDMA	0x0
 
 /* Unicast Filter MAC Address Register - Low */
 #define MTK_GDMA_MAC_ADRL(x)	(0x508 + (x * 0x1000))
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH net] net: ethernet: mediatek: rework GDM setup flow
@ 2019-11-07 10:51 ` MarkLee
  0 siblings, 0 replies; 9+ messages in thread
From: MarkLee @ 2019-11-07 10:51 UTC (permalink / raw)
  To: David S. Miller, Sean Wang, John Crispin, Matthias Brugger, Andrew Lunn
  Cc: Mark Rutland, devicetree, Jakub Kicinski, netdev, linux-kernel,
	Rene van Dorst, Rob Herring, linux-mediatek, MarkLee,
	linux-arm-kernel

net: ethernet: mediatek: rework GDM setup flow

The mt762x GDM block is mainly used to setup the HW 
internal rx path - from GMAC port to RX DMA engine(PDMA/QDMA). 
And the internal packet switching engine(PSE) is responsed 
to do the data forward/drop following the GDM configuration.

This pacth target to simpfy the GDM setup flow by integrating 
them into one single function "mtk_gdm_config".
Besides, accroding to the mt762x HW design, it is recommended to 
enable PSE to forward data after DMA has been started and 
set PSE to drop all data before stopping DMA. 

Note, mt7628 is a different IP from other mt762x, so we exclude it
in mtk_gdm_config function.

Signed-off-by: MarkLee <Mark-MC.Lee@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 44 ++++++++++++++-------
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |  2 +
 2 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 703adb96429e..7220abb3e731 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -2180,6 +2180,31 @@ static int mtk_start_dma(struct mtk_eth *eth)
 	return 0;
 }
 
+static void mtk_gdm_config(struct mtk_eth *eth, u32 config)
+{
+	int i;
+
+	if (MTK_HAS_CAPS(eth->soc->caps, MTK_SOC_MT7628))
+		return;
+
+	for (i = 0; i < 2; i++) {
+		u32 val = mtk_r32(eth, MTK_GDMA_FWD_CFG(i));
+
+		/* Always enable RX checksum */
+		val |= MTK_GDMA_ICS_EN | MTK_GDMA_TCS_EN | MTK_GDMA_UCS_EN;
+
+		/* default setup the forward port to send frame to PDMA */
+		val &= ~0xffff;
+
+		val |= config;
+
+		mtk_w32(eth, val, MTK_GDMA_FWD_CFG(i));
+	}
+	/*Reset and enable PSE*/
+	mtk_w32(eth, RST_GL_PSE, MTK_RST_GL);
+	mtk_w32(eth, 0, MTK_RST_GL);
+}
+
 static int mtk_open(struct net_device *dev)
 {
 	struct mtk_mac *mac = netdev_priv(dev);
@@ -2200,6 +2225,8 @@ static int mtk_open(struct net_device *dev)
 		if (err)
 			return err;
 
+		mtk_gdm_config(eth, MTK_GDMA_TO_PDMA);
+
 		napi_enable(&eth->tx_napi);
 		napi_enable(&eth->rx_napi);
 		mtk_tx_irq_enable(eth, MTK_TX_DONE_INT);
@@ -2252,6 +2279,8 @@ static int mtk_stop(struct net_device *dev)
 	if (!refcount_dec_and_test(&eth->dma_refcnt))
 		return 0;
 
+	mtk_gdm_config(eth, MTK_GDMA_DROP_ALL);
+
 	mtk_tx_irq_disable(eth, MTK_TX_DONE_INT);
 	mtk_rx_irq_disable(eth, MTK_RX_DONE_INT);
 	napi_disable(&eth->tx_napi);
@@ -2375,8 +2404,6 @@ static int mtk_hw_init(struct mtk_eth *eth)
 	mtk_w32(eth, 0, MTK_QDMA_DELAY_INT);
 	mtk_tx_irq_disable(eth, ~0);
 	mtk_rx_irq_disable(eth, ~0);
-	mtk_w32(eth, RST_GL_PSE, MTK_RST_GL);
-	mtk_w32(eth, 0, MTK_RST_GL);
 
 	/* FE int grouping */
 	mtk_w32(eth, MTK_TX_DONE_INT, MTK_PDMA_INT_GRP1);
@@ -2385,19 +2412,6 @@ static int mtk_hw_init(struct mtk_eth *eth)
 	mtk_w32(eth, MTK_RX_DONE_INT, MTK_QDMA_INT_GRP2);
 	mtk_w32(eth, 0x21021000, MTK_FE_INT_GRP);
 
-	for (i = 0; i < MTK_MAC_COUNT; i++) {
-		u32 val = mtk_r32(eth, MTK_GDMA_FWD_CFG(i));
-
-		/* setup the forward port to send frame to PDMA */
-		val &= ~0xffff;
-
-		/* Enable RX checksum */
-		val |= MTK_GDMA_ICS_EN | MTK_GDMA_TCS_EN | MTK_GDMA_UCS_EN;
-
-		/* setup the mac dma */
-		mtk_w32(eth, val, MTK_GDMA_FWD_CFG(i));
-	}
-
 	return 0;
 
 err_disable_pm:
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index 76bd12cb8150..b8bcfdfc995e 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -84,6 +84,8 @@
 #define MTK_GDMA_ICS_EN		BIT(22)
 #define MTK_GDMA_TCS_EN		BIT(21)
 #define MTK_GDMA_UCS_EN		BIT(20)
+#define MTK_GDMA_DROP_ALL	0x7777
+#define MTK_GDMA_TO_PDMA	0x0
 
 /* Unicast Filter MAC Address Register - Low */
 #define MTK_GDMA_MAC_ADRL(x)	(0x508 + (x * 0x1000))
-- 
2.17.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH net] net: ethernet: mediatek: rework GDM setup flow
@ 2019-11-07 10:51 ` MarkLee
  0 siblings, 0 replies; 9+ messages in thread
From: MarkLee @ 2019-11-07 10:51 UTC (permalink / raw)
  To: David S. Miller, Sean Wang, John Crispin, Matthias Brugger, Andrew Lunn
  Cc: Mark Rutland, devicetree, Jakub Kicinski, netdev, linux-kernel,
	Rene van Dorst, Rob Herring, linux-mediatek, MarkLee,
	linux-arm-kernel

net: ethernet: mediatek: rework GDM setup flow

The mt762x GDM block is mainly used to setup the HW 
internal rx path - from GMAC port to RX DMA engine(PDMA/QDMA). 
And the internal packet switching engine(PSE) is responsed 
to do the data forward/drop following the GDM configuration.

This pacth target to simpfy the GDM setup flow by integrating 
them into one single function "mtk_gdm_config".
Besides, accroding to the mt762x HW design, it is recommended to 
enable PSE to forward data after DMA has been started and 
set PSE to drop all data before stopping DMA. 

Note, mt7628 is a different IP from other mt762x, so we exclude it
in mtk_gdm_config function.

Signed-off-by: MarkLee <Mark-MC.Lee@mediatek.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 44 ++++++++++++++-------
 drivers/net/ethernet/mediatek/mtk_eth_soc.h |  2 +
 2 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 703adb96429e..7220abb3e731 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -2180,6 +2180,31 @@ static int mtk_start_dma(struct mtk_eth *eth)
 	return 0;
 }
 
+static void mtk_gdm_config(struct mtk_eth *eth, u32 config)
+{
+	int i;
+
+	if (MTK_HAS_CAPS(eth->soc->caps, MTK_SOC_MT7628))
+		return;
+
+	for (i = 0; i < 2; i++) {
+		u32 val = mtk_r32(eth, MTK_GDMA_FWD_CFG(i));
+
+		/* Always enable RX checksum */
+		val |= MTK_GDMA_ICS_EN | MTK_GDMA_TCS_EN | MTK_GDMA_UCS_EN;
+
+		/* default setup the forward port to send frame to PDMA */
+		val &= ~0xffff;
+
+		val |= config;
+
+		mtk_w32(eth, val, MTK_GDMA_FWD_CFG(i));
+	}
+	/*Reset and enable PSE*/
+	mtk_w32(eth, RST_GL_PSE, MTK_RST_GL);
+	mtk_w32(eth, 0, MTK_RST_GL);
+}
+
 static int mtk_open(struct net_device *dev)
 {
 	struct mtk_mac *mac = netdev_priv(dev);
@@ -2200,6 +2225,8 @@ static int mtk_open(struct net_device *dev)
 		if (err)
 			return err;
 
+		mtk_gdm_config(eth, MTK_GDMA_TO_PDMA);
+
 		napi_enable(&eth->tx_napi);
 		napi_enable(&eth->rx_napi);
 		mtk_tx_irq_enable(eth, MTK_TX_DONE_INT);
@@ -2252,6 +2279,8 @@ static int mtk_stop(struct net_device *dev)
 	if (!refcount_dec_and_test(&eth->dma_refcnt))
 		return 0;
 
+	mtk_gdm_config(eth, MTK_GDMA_DROP_ALL);
+
 	mtk_tx_irq_disable(eth, MTK_TX_DONE_INT);
 	mtk_rx_irq_disable(eth, MTK_RX_DONE_INT);
 	napi_disable(&eth->tx_napi);
@@ -2375,8 +2404,6 @@ static int mtk_hw_init(struct mtk_eth *eth)
 	mtk_w32(eth, 0, MTK_QDMA_DELAY_INT);
 	mtk_tx_irq_disable(eth, ~0);
 	mtk_rx_irq_disable(eth, ~0);
-	mtk_w32(eth, RST_GL_PSE, MTK_RST_GL);
-	mtk_w32(eth, 0, MTK_RST_GL);
 
 	/* FE int grouping */
 	mtk_w32(eth, MTK_TX_DONE_INT, MTK_PDMA_INT_GRP1);
@@ -2385,19 +2412,6 @@ static int mtk_hw_init(struct mtk_eth *eth)
 	mtk_w32(eth, MTK_RX_DONE_INT, MTK_QDMA_INT_GRP2);
 	mtk_w32(eth, 0x21021000, MTK_FE_INT_GRP);
 
-	for (i = 0; i < MTK_MAC_COUNT; i++) {
-		u32 val = mtk_r32(eth, MTK_GDMA_FWD_CFG(i));
-
-		/* setup the forward port to send frame to PDMA */
-		val &= ~0xffff;
-
-		/* Enable RX checksum */
-		val |= MTK_GDMA_ICS_EN | MTK_GDMA_TCS_EN | MTK_GDMA_UCS_EN;
-
-		/* setup the mac dma */
-		mtk_w32(eth, val, MTK_GDMA_FWD_CFG(i));
-	}
-
 	return 0;
 
 err_disable_pm:
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index 76bd12cb8150..b8bcfdfc995e 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -84,6 +84,8 @@
 #define MTK_GDMA_ICS_EN		BIT(22)
 #define MTK_GDMA_TCS_EN		BIT(21)
 #define MTK_GDMA_UCS_EN		BIT(20)
+#define MTK_GDMA_DROP_ALL	0x7777
+#define MTK_GDMA_TO_PDMA	0x0
 
 /* Unicast Filter MAC Address Register - Low */
 #define MTK_GDMA_MAC_ADRL(x)	(0x508 + (x * 0x1000))
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [PATCH net] net: ethernet: mediatek: rework GDM setup flow
  2019-11-07 10:51 ` MarkLee
  (?)
@ 2019-11-07 23:49   ` David Miller
  -1 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2019-11-07 23:49 UTC (permalink / raw)
  To: Mark-MC.Lee
  Cc: sean.wang, john, matthias.bgg, andrew, robh+dt, mark.rutland,
	opensource, devicetree, netdev, linux-arm-kernel, linux-mediatek,
	linux-kernel, jakub.kicinski

From: MarkLee <Mark-MC.Lee@mediatek.com>
Date: Thu, 7 Nov 2019 18:51:35 +0800

> +	for (i = 0; i < 2; i++) {

This is a regression, because in the existing code...

> -	for (i = 0; i < MTK_MAC_COUNT; i++) {

the proper macro is used instead of a magic constant.

You're doing so many things in one change, it's hard to review
and audit.

If you're going to consolidate code, do that only in one change.

Then make other functional changes such as putting the chip into
GDMA_DROP_ALL mode during the stop operation etc.

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH net] net: ethernet: mediatek: rework GDM setup flow
@ 2019-11-07 23:49   ` David Miller
  0 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2019-11-07 23:49 UTC (permalink / raw)
  To: Mark-MC.Lee
  Cc: mark.rutland, andrew, jakub.kicinski, devicetree, netdev,
	sean.wang, linux-kernel, opensource, robh+dt, linux-mediatek,
	john, matthias.bgg, linux-arm-kernel

From: MarkLee <Mark-MC.Lee@mediatek.com>
Date: Thu, 7 Nov 2019 18:51:35 +0800

> +	for (i = 0; i < 2; i++) {

This is a regression, because in the existing code...

> -	for (i = 0; i < MTK_MAC_COUNT; i++) {

the proper macro is used instead of a magic constant.

You're doing so many things in one change, it's hard to review
and audit.

If you're going to consolidate code, do that only in one change.

Then make other functional changes such as putting the chip into
GDMA_DROP_ALL mode during the stop operation etc.

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH net] net: ethernet: mediatek: rework GDM setup flow
@ 2019-11-07 23:49   ` David Miller
  0 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2019-11-07 23:49 UTC (permalink / raw)
  To: Mark-MC.Lee
  Cc: mark.rutland, andrew, jakub.kicinski, devicetree, netdev,
	sean.wang, linux-kernel, opensource, robh+dt, linux-mediatek,
	john, matthias.bgg, linux-arm-kernel

From: MarkLee <Mark-MC.Lee@mediatek.com>
Date: Thu, 7 Nov 2019 18:51:35 +0800

> +	for (i = 0; i < 2; i++) {

This is a regression, because in the existing code...

> -	for (i = 0; i < MTK_MAC_COUNT; i++) {

the proper macro is used instead of a magic constant.

You're doing so many things in one change, it's hard to review
and audit.

If you're going to consolidate code, do that only in one change.

Then make other functional changes such as putting the chip into
GDMA_DROP_ALL mode during the stop operation etc.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH net] net: ethernet: mediatek: rework GDM setup flow
  2019-11-07 23:49   ` David Miller
  (?)
@ 2019-11-08  8:13     ` mtk15127
  -1 siblings, 0 replies; 9+ messages in thread
From: mtk15127 @ 2019-11-08  8:13 UTC (permalink / raw)
  To: David Miller
  Cc: sean.wang, john, matthias.bgg, andrew, robh+dt, mark.rutland,
	opensource, devicetree, netdev, linux-arm-kernel, linux-mediatek,
	linux-kernel, jakub.kicinski, Mark-MC.Lee

On Thu, 2019-11-07 at 15:49 -0800, David Miller wrote:
> From: MarkLee <Mark-MC.Lee@mediatek.com>
> Date: Thu, 7 Nov 2019 18:51:35 +0800
> 
> > +	for (i = 0; i < 2; i++) {
> 
> This is a regression, because in the existing code...
> 
> > -	for (i = 0; i < MTK_MAC_COUNT; i++) {
> 
> the proper macro is used instead of a magic constant.
 Yes, you are right, I make a mistake here, will correct it in the next
patch
> 
> You're doing so many things in one change, it's hard to review
> and audit.
> 
> If you're going to consolidate code, do that only in one change.
> 
> Then make other functional changes such as putting the chip into
> GDMA_DROP_ALL mode during the stop operation etc.
Thanks for your suggestion, I will separate these changes into
a patch series to make every change to be more clear for its 
purpose.



^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH net] net: ethernet: mediatek: rework GDM setup flow
@ 2019-11-08  8:13     ` mtk15127
  0 siblings, 0 replies; 9+ messages in thread
From: mtk15127 @ 2019-11-08  8:13 UTC (permalink / raw)
  To: David Miller
  Cc: mark.rutland, andrew, jakub.kicinski, devicetree, netdev,
	sean.wang, linux-kernel, opensource, robh+dt, linux-mediatek,
	john, matthias.bgg, Mark-MC.Lee, linux-arm-kernel

On Thu, 2019-11-07 at 15:49 -0800, David Miller wrote:
> From: MarkLee <Mark-MC.Lee@mediatek.com>
> Date: Thu, 7 Nov 2019 18:51:35 +0800
> 
> > +	for (i = 0; i < 2; i++) {
> 
> This is a regression, because in the existing code...
> 
> > -	for (i = 0; i < MTK_MAC_COUNT; i++) {
> 
> the proper macro is used instead of a magic constant.
 Yes, you are right, I make a mistake here, will correct it in the next
patch
> 
> You're doing so many things in one change, it's hard to review
> and audit.
> 
> If you're going to consolidate code, do that only in one change.
> 
> Then make other functional changes such as putting the chip into
> GDMA_DROP_ALL mode during the stop operation etc.
Thanks for your suggestion, I will separate these changes into
a patch series to make every change to be more clear for its 
purpose.


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH net] net: ethernet: mediatek: rework GDM setup flow
@ 2019-11-08  8:13     ` mtk15127
  0 siblings, 0 replies; 9+ messages in thread
From: mtk15127 @ 2019-11-08  8:13 UTC (permalink / raw)
  To: David Miller
  Cc: mark.rutland, andrew, jakub.kicinski, devicetree, netdev,
	sean.wang, linux-kernel, opensource, robh+dt, linux-mediatek,
	john, matthias.bgg, Mark-MC.Lee, linux-arm-kernel

On Thu, 2019-11-07 at 15:49 -0800, David Miller wrote:
> From: MarkLee <Mark-MC.Lee@mediatek.com>
> Date: Thu, 7 Nov 2019 18:51:35 +0800
> 
> > +	for (i = 0; i < 2; i++) {
> 
> This is a regression, because in the existing code...
> 
> > -	for (i = 0; i < MTK_MAC_COUNT; i++) {
> 
> the proper macro is used instead of a magic constant.
 Yes, you are right, I make a mistake here, will correct it in the next
patch
> 
> You're doing so many things in one change, it's hard to review
> and audit.
> 
> If you're going to consolidate code, do that only in one change.
> 
> Then make other functional changes such as putting the chip into
> GDMA_DROP_ALL mode during the stop operation etc.
Thanks for your suggestion, I will separate these changes into
a patch series to make every change to be more clear for its 
purpose.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-11-08  8:14 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-07 10:51 [PATCH net] net: ethernet: mediatek: rework GDM setup flow MarkLee
2019-11-07 10:51 ` MarkLee
2019-11-07 10:51 ` MarkLee
2019-11-07 23:49 ` David Miller
2019-11-07 23:49   ` David Miller
2019-11-07 23:49   ` David Miller
2019-11-08  8:13   ` mtk15127
2019-11-08  8:13     ` mtk15127
2019-11-08  8:13     ` mtk15127

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.