Netdev Archive on lore.kernel.org
 help / color / Atom feed
* RE: [PATCH net 1/7] net: stmmac: fix error in updating rx tail pointer to last free entry
  2020-01-14  2:01 ` [PATCH net 1/7] net: stmmac: fix error in updating rx tail pointer to last free entry Ong Boon Leong
@ 2020-01-13 10:29   ` Jose Abreu
  2020-01-14  6:54     ` Ong, Boon Leong
  0 siblings, 1 reply; 14+ messages in thread
From: Jose Abreu @ 2020-01-13 10:29 UTC (permalink / raw)
  To: Ong Boon Leong, netdev
  Cc: Giuseppe Cavallaro, Alexandre Torgue, David S . Miller,
	Maxime Coquelin, Tan Tee Min, Voon Weifeng, linux-stm32,
	linux-arm-kernel, linux-kernel

From: Ong Boon Leong <boon.leong.ong@intel.com>
Date: Jan/14/2020, 02:01:10 (UTC+00:00)

> DMA_CH(#i)_RxDesc_Tail_Pointer points to an offset from the base and
> indicates the location of the last valid descriptor.
> 
> The change introduced by "net: stmmac: Update RX Tail Pointer to last
> free entry" incorrectly updates the RxDesc_Tail_Pointer and causess
> Rx operation to freeze in corner case. The issue is explained as
> follow:-
> 
> Say, cur_rx=1 and dirty_rx=0, then we have dirty=1 and entry=0 before
> the while (dirty-- > 0) loop of stmmac_rx_refill() is entered. When
> the while loop is 1st entered, Rx buffer[entry=0] is refilled and after
> entry++, then, entry=1. Now, the while loop condition check "dirty-- > 0"
> and the while loop bails out because dirty=0. Up to this point, the
> driver code works correctly.
> 
> However, the current implementation sets the Rx Tail Pointer to the
> location pointed by dirty_rx, just updated to the value of entry(=1).
> This is incorrect because the last Rx buffer that is refileld with empty
> buffer is with entry=0. In another words, the current logics always sets
> Rx Tail Pointer to the next Rx buffer to be refilled (too early).
> 
> So, we fix this by tracking the index of the most recently refilled Rx
> buffer by using "last_refill" and use "last_refill" to update the Rx Tail
> Pointer instead of using "entry" which points to the next dirty_rx to be
> refilled in future.

I'm not sure about this ...

RX Tail points to last valid descriptor but it doesn't point to the base 
address of that one, it points to the end address.

Let's say we have a ring buffer with just 1 descriptor. With your new 
logic then: RX base == RX tail (== RX base), so the IP will not see any 
descriptor. But with old logic: RX base == (RX base + 1), which causes 
the IP to correctly see the descriptor.

Can you provide more information on the Rx operation freeze you 
mentioned ? Can it be another issue ?

---
Thanks,
Jose Miguel Abreu

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

* RE: [PATCH net 4/7] net: stmmac: Fix priority steering for tx/rx queue >3
  2020-01-14  2:01 ` [PATCH net 4/7] net: stmmac: Fix priority steering for tx/rx queue >3 Ong Boon Leong
@ 2020-01-13 10:33   ` Jose Abreu
  2020-01-14 15:24     ` Voon, Weifeng
  0 siblings, 1 reply; 14+ messages in thread
From: Jose Abreu @ 2020-01-13 10:33 UTC (permalink / raw)
  To: Ong Boon Leong, netdev
  Cc: Giuseppe Cavallaro, Alexandre Torgue, David S . Miller,
	Maxime Coquelin, Tan Tee Min, Voon Weifeng, linux-stm32,
	linux-arm-kernel, linux-kernel

From: Ong Boon Leong <boon.leong.ong@intel.com>
Date: Jan/14/2020, 02:01:13 (UTC+00:00)

> Fix MACRO function define for TX and RX user priority queue steering for
> register masking and shifting.

I think this was already fixed as seen on:
- https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=e8df7e8c233a18d2704e37ecff47583b494789d3

Did I forget something ?

---
Thanks,
Jose Miguel Abreu

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

* Re: [PATCH net 3/7] net: stmmac: fix missing netdev->features in stmmac_set_features
  2020-01-14  2:01 ` [PATCH net 3/7] net: stmmac: fix missing netdev->features in stmmac_set_features Ong Boon Leong
@ 2020-01-13 13:17   ` Jakub Kicinski
  2020-01-14  6:56     ` Ong, Boon Leong
  0 siblings, 1 reply; 14+ messages in thread
From: Jakub Kicinski @ 2020-01-13 13:17 UTC (permalink / raw)
  To: Ong Boon Leong
  Cc: netdev, Jose Abreu, Giuseppe Cavallaro, Alexandre Torgue,
	David S . Miller, Maxime Coquelin, Tan Tee Min, Voon Weifeng,
	linux-stm32, linux-arm-kernel, linux-kernel

On Tue, 14 Jan 2020 10:01:12 +0800, Ong Boon Leong wrote:

Please fix the date on your system.

Please always provide a patch description. For bug fixes description of
how the bug manifest to the users is important to have.

> Fixes: d2afb5bdffde ("stmmac: fix the rx csum feature")
> 

Please remove the empty lines between the Fixes tag and the other tags
on all patches.

> Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index cd55d16..dc739cd 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -3911,6 +3911,8 @@ static int stmmac_set_features(struct net_device *netdev,
>  	for (chan = 0; chan < priv->plat->rx_queues_to_use; chan++)
>  		stmmac_enable_sph(priv, priv->ioaddr, sph_en, chan);
>  
> +	netdev->features = features;
> +
>  	return 0;
>  }
>  


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

* [PATCH net 0/7] net: stmmac: general fixes for Ethernet functionality
@ 2020-01-14  2:01 Ong Boon Leong
  2020-01-14  2:01 ` [PATCH net 1/7] net: stmmac: fix error in updating rx tail pointer to last free entry Ong Boon Leong
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Ong Boon Leong @ 2020-01-14  2:01 UTC (permalink / raw)
  To: netdev
  Cc: Jose Abreu, Giuseppe Cavallaro, Alexandre Torgue,
	David S . Miller, Maxime Coquelin, Ong Boon Leong, Tan Tee Min,
	Voon Weifeng, linux-stm32, linux-arm-kernel, linux-kernel

Patch-series that fixes couple of issues in stmmac:-

1/7: It fixes the incorrect setting of Rx Tail Pointer. Rx Tail Pointer
     should points to the last valid descriptor that was replenished by
     stmmac_rx_refill().

2/7: It ensures that the real_num_rx|tx_queues are set in both driver
     probe() and resume(). So, move the netif_set_real_num_rx|tx_queues()
     into stmmac_hw_setup().

3/7: It fixes missing netdev->features = features update in
     stmmac_set_features().

4/7: It fixes incorrect MACRO function defininition for TX and RX user
     priority queue steering for queue > 3.

5/7: It ensures that the previous value of GMAC_VLAN_TAG register is
     read first before for updating the register.

6/7: It ensures the GMAC IP v4.xx and above behaves correctly to:-
      ip link set <devname> multicast off|on

7/7: It ensures PCI platform data is using plat->phy_interface.

Thanks,
Boon Leong

Aashish Verma (1):
  net: stmmac: Fix incorrect location to set real_num_rx|tx_queues

Ong Boon Leong (2):
  net: stmmac: fix error in updating rx tail pointer to last free entry
  net: stmmac: fix missing netdev->features in stmmac_set_features

Tan, Tee Min (1):
  net: stmmac: fix incorrect GMAC_VLAN_TAG register writting
    implementation

Verma, Aashish (1):
  net: stmmac: fix missing IFF_MULTICAST check in dwmac4_set_filter

Voon Weifeng (2):
  net: stmmac: Fix priority steering for tx/rx queue >3
  net: stmmac: update pci platform data to use phy_interface

 drivers/net/ethernet/stmicro/stmmac/dwmac4.h      | 10 ++++++----
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c |  9 +++++----
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 16 ++++++++++------
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c  | 14 ++++++++------
 4 files changed, 29 insertions(+), 20 deletions(-)

-- 
2.7.4


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

* [PATCH net 1/7] net: stmmac: fix error in updating rx tail pointer to last free entry
  2020-01-14  2:01 [PATCH net 0/7] net: stmmac: general fixes for Ethernet functionality Ong Boon Leong
@ 2020-01-14  2:01 ` Ong Boon Leong
  2020-01-13 10:29   ` Jose Abreu
  2020-01-14  2:01 ` [PATCH net 2/7] net: stmmac: Fix incorrect location to set real_num_rx|tx_queues Ong Boon Leong
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Ong Boon Leong @ 2020-01-14  2:01 UTC (permalink / raw)
  To: netdev
  Cc: Jose Abreu, Giuseppe Cavallaro, Alexandre Torgue,
	David S . Miller, Maxime Coquelin, Ong Boon Leong, Tan Tee Min,
	Voon Weifeng, linux-stm32, linux-arm-kernel, linux-kernel

DMA_CH(#i)_RxDesc_Tail_Pointer points to an offset from the base and
indicates the location of the last valid descriptor.

The change introduced by "net: stmmac: Update RX Tail Pointer to last
free entry" incorrectly updates the RxDesc_Tail_Pointer and causess
Rx operation to freeze in corner case. The issue is explained as
follow:-

Say, cur_rx=1 and dirty_rx=0, then we have dirty=1 and entry=0 before
the while (dirty-- > 0) loop of stmmac_rx_refill() is entered. When
the while loop is 1st entered, Rx buffer[entry=0] is refilled and after
entry++, then, entry=1. Now, the while loop condition check "dirty-- > 0"
and the while loop bails out because dirty=0. Up to this point, the
driver code works correctly.

However, the current implementation sets the Rx Tail Pointer to the
location pointed by dirty_rx, just updated to the value of entry(=1).
This is incorrect because the last Rx buffer that is refileld with empty
buffer is with entry=0. In another words, the current logics always sets
Rx Tail Pointer to the next Rx buffer to be refilled (too early).

So, we fix this by tracking the index of the most recently refilled Rx
buffer by using "last_refill" and use "last_refill" to update the Rx Tail
Pointer instead of using "entry" which points to the next dirty_rx to be
refilled in future.

Fixes: 858a31ffc3d9 ("net: stmmac: Update RX Tail Pointer to last free entry")

Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 80d59b7..a317f67 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3417,6 +3417,7 @@ static inline void stmmac_rx_refill(struct stmmac_priv *priv, u32 queue)
 	struct stmmac_rx_queue *rx_q = &priv->rx_queue[queue];
 	int len, dirty = stmmac_rx_dirty(priv, queue);
 	unsigned int entry = rx_q->dirty_rx;
+	unsigned int last_refill = entry;
 
 	len = DIV_ROUND_UP(priv->dma_buf_sz, PAGE_SIZE) * PAGE_SIZE;
 
@@ -3471,12 +3472,13 @@ static inline void stmmac_rx_refill(struct stmmac_priv *priv, u32 queue)
 
 		dma_wmb();
 		stmmac_set_rx_owner(priv, p, use_rx_wd);
-
+		last_refill = entry;
 		entry = STMMAC_GET_ENTRY(entry, DMA_RX_SIZE);
 	}
+
 	rx_q->dirty_rx = entry;
 	rx_q->rx_tail_addr = rx_q->dma_rx_phy +
-			    (rx_q->dirty_rx * sizeof(struct dma_desc));
+			     (last_refill * sizeof(struct dma_desc));
 	stmmac_set_rx_tail_ptr(priv, priv->ioaddr, rx_q->rx_tail_addr, queue);
 }
 
-- 
2.7.4


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

* [PATCH net 2/7] net: stmmac: Fix incorrect location to set real_num_rx|tx_queues
  2020-01-14  2:01 [PATCH net 0/7] net: stmmac: general fixes for Ethernet functionality Ong Boon Leong
  2020-01-14  2:01 ` [PATCH net 1/7] net: stmmac: fix error in updating rx tail pointer to last free entry Ong Boon Leong
@ 2020-01-14  2:01 ` Ong Boon Leong
  2020-01-14  2:01 ` [PATCH net 3/7] net: stmmac: fix missing netdev->features in stmmac_set_features Ong Boon Leong
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Ong Boon Leong @ 2020-01-14  2:01 UTC (permalink / raw)
  To: netdev
  Cc: Jose Abreu, Giuseppe Cavallaro, Alexandre Torgue,
	David S . Miller, Maxime Coquelin, Ong Boon Leong, Tan Tee Min,
	Voon Weifeng, linux-stm32, linux-arm-kernel, linux-kernel

From: Aashish Verma <aashishx.verma@intel.com>

netif_set_real_num_tx_queues() & netif_set_real_num_rx_queues() should be
used to inform network stack about the real Tx & Rx queue (active) number
in both stmmac_open() and stmmac_resume(), therefore, we move the code
from stmmac_dvr_probe() to stmmac_hw_setup().

Fixes: c02b7a914551 ("net: stmmac: use netif_set_real_num_{rx,tx}_queues")

Signed-off-by: Aashish Verma <aashishx.verma@intel.com>
Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index a317f67..cd55d16 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2624,6 +2624,10 @@ static int stmmac_hw_setup(struct net_device *dev, bool init_ptp)
 	if (priv->dma_cap.vlins)
 		stmmac_enable_vlan(priv, priv->hw, STMMAC_VLAN_INSERT);
 
+	/* Configure real RX and TX queues */
+	netif_set_real_num_rx_queues(dev, priv->plat->rx_queues_to_use);
+	netif_set_real_num_tx_queues(dev, priv->plat->tx_queues_to_use);
+
 	/* Start the ball rolling... */
 	stmmac_start_all_dma(priv);
 
@@ -4624,10 +4628,6 @@ int stmmac_dvr_probe(struct device *device,
 
 	stmmac_check_ether_addr(priv);
 
-	/* Configure real RX and TX queues */
-	netif_set_real_num_rx_queues(ndev, priv->plat->rx_queues_to_use);
-	netif_set_real_num_tx_queues(ndev, priv->plat->tx_queues_to_use);
-
 	ndev->netdev_ops = &stmmac_netdev_ops;
 
 	ndev->hw_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
-- 
2.7.4


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

* [PATCH net 3/7] net: stmmac: fix missing netdev->features in stmmac_set_features
  2020-01-14  2:01 [PATCH net 0/7] net: stmmac: general fixes for Ethernet functionality Ong Boon Leong
  2020-01-14  2:01 ` [PATCH net 1/7] net: stmmac: fix error in updating rx tail pointer to last free entry Ong Boon Leong
  2020-01-14  2:01 ` [PATCH net 2/7] net: stmmac: Fix incorrect location to set real_num_rx|tx_queues Ong Boon Leong
@ 2020-01-14  2:01 ` Ong Boon Leong
  2020-01-13 13:17   ` Jakub Kicinski
  2020-01-14  2:01 ` [PATCH net 4/7] net: stmmac: Fix priority steering for tx/rx queue >3 Ong Boon Leong
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Ong Boon Leong @ 2020-01-14  2:01 UTC (permalink / raw)
  To: netdev
  Cc: Jose Abreu, Giuseppe Cavallaro, Alexandre Torgue,
	David S . Miller, Maxime Coquelin, Ong Boon Leong, Tan Tee Min,
	Voon Weifeng, linux-stm32, linux-arm-kernel, linux-kernel

Fixes: d2afb5bdffde ("stmmac: fix the rx csum feature")

Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index cd55d16..dc739cd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3911,6 +3911,8 @@ static int stmmac_set_features(struct net_device *netdev,
 	for (chan = 0; chan < priv->plat->rx_queues_to_use; chan++)
 		stmmac_enable_sph(priv, priv->ioaddr, sph_en, chan);
 
+	netdev->features = features;
+
 	return 0;
 }
 
-- 
2.7.4


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

* [PATCH net 4/7] net: stmmac: Fix priority steering for tx/rx queue >3
  2020-01-14  2:01 [PATCH net 0/7] net: stmmac: general fixes for Ethernet functionality Ong Boon Leong
                   ` (2 preceding siblings ...)
  2020-01-14  2:01 ` [PATCH net 3/7] net: stmmac: fix missing netdev->features in stmmac_set_features Ong Boon Leong
@ 2020-01-14  2:01 ` Ong Boon Leong
  2020-01-13 10:33   ` Jose Abreu
  2020-01-14  2:01 ` [PATCH net 5/7] net: stmmac: fix incorrect GMAC_VLAN_TAG register writting implementation Ong Boon Leong
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Ong Boon Leong @ 2020-01-14  2:01 UTC (permalink / raw)
  To: netdev
  Cc: Jose Abreu, Giuseppe Cavallaro, Alexandre Torgue,
	David S . Miller, Maxime Coquelin, Ong Boon Leong, Tan Tee Min,
	Voon Weifeng, linux-stm32, linux-arm-kernel, linux-kernel

From: Voon Weifeng <weifeng.voon@intel.com>

Fix MACRO function define for TX and RX user priority queue steering for
register masking and shifting.

Fixes: a8f5102af2a7 ("net: stmmac: TX and RX queue priority configuration")

Signed-off-by: Voon Weifeng <weifeng.voon@intel.com>
Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac4.h | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
index 2dc70d1..798a53a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
@@ -97,12 +97,14 @@
 #define GMAC_RX_FLOW_CTRL_RFE		BIT(0)
 
 /* RX Queues Priorities */
-#define GMAC_RXQCTRL_PSRQX_MASK(x)	GENMASK(7 + ((x) * 8), 0 + ((x) * 8))
-#define GMAC_RXQCTRL_PSRQX_SHIFT(x)	((x) * 8)
+#define GMAC_RXQCTRL_PSRQX_MASK(x)	GENMASK(7 + (((x) % 4) * 8), \
+						0 + (((x) % 4) * 8))
+#define GMAC_RXQCTRL_PSRQX_SHIFT(x)	(((x) % 4) * 8)
 
 /* TX Queues Priorities */
-#define GMAC_TXQCTRL_PSTQX_MASK(x)	GENMASK(7 + ((x) * 8), 0 + ((x) * 8))
-#define GMAC_TXQCTRL_PSTQX_SHIFT(x)	((x) * 8)
+#define GMAC_TXQCTRL_PSTQX_MASK(x)	GENMASK(7 + (((x) % 4) * 8), \
+						0 + (((x) % 4) * 8))
+#define GMAC_TXQCTRL_PSTQX_SHIFT(x)	(((x) % 4) * 8)
 
 /* MAC Flow Control TX */
 #define GMAC_TX_FLOW_CTRL_TFE		BIT(1)
-- 
2.7.4


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

* [PATCH net 5/7] net: stmmac: fix incorrect GMAC_VLAN_TAG register writting implementation
  2020-01-14  2:01 [PATCH net 0/7] net: stmmac: general fixes for Ethernet functionality Ong Boon Leong
                   ` (3 preceding siblings ...)
  2020-01-14  2:01 ` [PATCH net 4/7] net: stmmac: Fix priority steering for tx/rx queue >3 Ong Boon Leong
@ 2020-01-14  2:01 ` Ong Boon Leong
  2020-01-14  2:01 ` [PATCH net 6/7] net: stmmac: fix missing IFF_MULTICAST check in dwmac4_set_filter Ong Boon Leong
  2020-01-14  2:01 ` [PATCH net 7/7] net: stmmac: update pci platform data to use phy_interface Ong Boon Leong
  6 siblings, 0 replies; 14+ messages in thread
From: Ong Boon Leong @ 2020-01-14  2:01 UTC (permalink / raw)
  To: netdev
  Cc: Jose Abreu, Giuseppe Cavallaro, Alexandre Torgue,
	David S . Miller, Maxime Coquelin, Ong Boon Leong, Tan Tee Min,
	Voon Weifeng, linux-stm32, linux-arm-kernel, linux-kernel

From: "Tan, Tee Min" <tee.min.tan@intel.com>

It should always do a read of current value of GMAC_VLAN_TAG instead of
directly overwriting the register value.

Fixes: c1be0022df0d ("net: stmmac: Add VLAN HASH filtering support in GMAC4+")

Signed-off-by: Tan, Tee Min <tee.min.tan@intel.com>
Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index 40ca00e..6e3d0ab 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -736,11 +736,14 @@ static void dwmac4_update_vlan_hash(struct mac_device_info *hw, u32 hash,
 				    __le16 perfect_match, bool is_double)
 {
 	void __iomem *ioaddr = hw->pcsr;
+	u32 value;
 
 	writel(hash, ioaddr + GMAC_VLAN_HASH_TABLE);
 
+	value = readl(ioaddr + GMAC_VLAN_TAG);
+
 	if (hash) {
-		u32 value = GMAC_VLAN_VTHM | GMAC_VLAN_ETV;
+		value |= GMAC_VLAN_VTHM | GMAC_VLAN_ETV;
 		if (is_double) {
 			value |= GMAC_VLAN_EDVLP;
 			value |= GMAC_VLAN_ESVL;
@@ -759,8 +762,6 @@ static void dwmac4_update_vlan_hash(struct mac_device_info *hw, u32 hash,
 
 		writel(value | perfect_match, ioaddr + GMAC_VLAN_TAG);
 	} else {
-		u32 value = readl(ioaddr + GMAC_VLAN_TAG);
-
 		value &= ~(GMAC_VLAN_VTHM | GMAC_VLAN_ETV);
 		value &= ~(GMAC_VLAN_EDVLP | GMAC_VLAN_ESVL);
 		value &= ~GMAC_VLAN_DOVLTC;
-- 
2.7.4


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

* [PATCH net 6/7] net: stmmac: fix missing IFF_MULTICAST check in dwmac4_set_filter
  2020-01-14  2:01 [PATCH net 0/7] net: stmmac: general fixes for Ethernet functionality Ong Boon Leong
                   ` (4 preceding siblings ...)
  2020-01-14  2:01 ` [PATCH net 5/7] net: stmmac: fix incorrect GMAC_VLAN_TAG register writting implementation Ong Boon Leong
@ 2020-01-14  2:01 ` Ong Boon Leong
  2020-01-14  2:01 ` [PATCH net 7/7] net: stmmac: update pci platform data to use phy_interface Ong Boon Leong
  6 siblings, 0 replies; 14+ messages in thread
From: Ong Boon Leong @ 2020-01-14  2:01 UTC (permalink / raw)
  To: netdev
  Cc: Jose Abreu, Giuseppe Cavallaro, Alexandre Torgue,
	David S . Miller, Maxime Coquelin, Ong Boon Leong, Tan Tee Min,
	Voon Weifeng, linux-stm32, linux-arm-kernel, linux-kernel

From: "Verma, Aashish" <aashishx.verma@intel.com>

Without checking for IFF_MULTICAST flag, it is wrong to assume multicast
filtering is always enabled. By checking against IFF_MULTICAST, now
the driver behaves correctly when the multicast support is toggled by below
command:-

  ip link set <devname> multicast off|on

Fixes: 477286b53f55 ("stmmac: add GMAC4 core support")

Signed-off-by: Verma, Aashish <aashishx.verma@intel.com>
Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index 6e3d0ab..53be936 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -420,7 +420,7 @@ static void dwmac4_set_filter(struct mac_device_info *hw,
 		value |= GMAC_PACKET_FILTER_PM;
 		/* Set all the bits of the HASH tab */
 		memset(mc_filter, 0xff, sizeof(mc_filter));
-	} else if (!netdev_mc_empty(dev)) {
+	} else if (!netdev_mc_empty(dev) && (dev->flags & IFF_MULTICAST)) {
 		struct netdev_hw_addr *ha;
 
 		/* Hash filter for multicast */
-- 
2.7.4


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

* [PATCH net 7/7] net: stmmac: update pci platform data to use phy_interface
  2020-01-14  2:01 [PATCH net 0/7] net: stmmac: general fixes for Ethernet functionality Ong Boon Leong
                   ` (5 preceding siblings ...)
  2020-01-14  2:01 ` [PATCH net 6/7] net: stmmac: fix missing IFF_MULTICAST check in dwmac4_set_filter Ong Boon Leong
@ 2020-01-14  2:01 ` Ong Boon Leong
  6 siblings, 0 replies; 14+ messages in thread
From: Ong Boon Leong @ 2020-01-14  2:01 UTC (permalink / raw)
  To: netdev
  Cc: Jose Abreu, Giuseppe Cavallaro, Alexandre Torgue,
	David S . Miller, Maxime Coquelin, Ong Boon Leong, Tan Tee Min,
	Voon Weifeng, linux-stm32, linux-arm-kernel, linux-kernel

From: Voon Weifeng <weifeng.voon@intel.com>

The recent patch to support passive mode converter did not take care the
phy interface configuration in PCI platform data. Hence, converting all
the PCI platform data from plat->interface to plat->phy_interface as the
default mode is meant for PHY.

Fixes: 0060c8783330 ("net: stmmac: implement support for passive mode converters via dt")

Signed-off-by: Voon Weifeng <weifeng.voon@intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 8237dbc..d2bc04d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -96,7 +96,7 @@ static int stmmac_default_data(struct pci_dev *pdev,
 
 	plat->bus_id = 1;
 	plat->phy_addr = 0;
-	plat->interface = PHY_INTERFACE_MODE_GMII;
+	plat->phy_interface = PHY_INTERFACE_MODE_GMII;
 
 	plat->dma_cfg->pbl = 32;
 	plat->dma_cfg->pblx8 = true;
@@ -220,7 +220,8 @@ static int ehl_sgmii_data(struct pci_dev *pdev,
 {
 	plat->bus_id = 1;
 	plat->phy_addr = 0;
-	plat->interface = PHY_INTERFACE_MODE_SGMII;
+	plat->phy_interface = PHY_INTERFACE_MODE_SGMII;
+
 	return ehl_common_data(pdev, plat);
 }
 
@@ -233,7 +234,8 @@ static int ehl_rgmii_data(struct pci_dev *pdev,
 {
 	plat->bus_id = 1;
 	plat->phy_addr = 0;
-	plat->interface = PHY_INTERFACE_MODE_RGMII;
+	plat->phy_interface = PHY_INTERFACE_MODE_RGMII;
+
 	return ehl_common_data(pdev, plat);
 }
 
@@ -261,7 +263,7 @@ static int tgl_sgmii_data(struct pci_dev *pdev,
 {
 	plat->bus_id = 1;
 	plat->phy_addr = 0;
-	plat->interface = PHY_INTERFACE_MODE_SGMII;
+	plat->phy_interface = PHY_INTERFACE_MODE_SGMII;
 	return tgl_common_data(pdev, plat);
 }
 
@@ -361,7 +363,7 @@ static int quark_default_data(struct pci_dev *pdev,
 
 	plat->bus_id = pci_dev_id(pdev);
 	plat->phy_addr = ret;
-	plat->interface = PHY_INTERFACE_MODE_RMII;
+	plat->phy_interface = PHY_INTERFACE_MODE_RMII;
 
 	plat->dma_cfg->pbl = 16;
 	plat->dma_cfg->pblx8 = true;
@@ -418,7 +420,7 @@ static int snps_gmac5_default_data(struct pci_dev *pdev,
 
 	plat->bus_id = 1;
 	plat->phy_addr = -1;
-	plat->interface = PHY_INTERFACE_MODE_GMII;
+	plat->phy_interface = PHY_INTERFACE_MODE_GMII;
 
 	plat->dma_cfg->pbl = 32;
 	plat->dma_cfg->pblx8 = true;
-- 
2.7.4


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

* RE: [PATCH net 1/7] net: stmmac: fix error in updating rx tail pointer to last free entry
  2020-01-13 10:29   ` Jose Abreu
@ 2020-01-14  6:54     ` Ong, Boon Leong
  0 siblings, 0 replies; 14+ messages in thread
From: Ong, Boon Leong @ 2020-01-14  6:54 UTC (permalink / raw)
  To: Jose Abreu, netdev
  Cc: Giuseppe Cavallaro, Alexandre Torgue, David S . Miller,
	Maxime Coquelin, Tan, Tee Min, Voon, Weifeng, linux-stm32,
	linux-arm-kernel, linux-kernel

>From: Ong Boon Leong <boon.leong.ong@intel.com>
>Date: Jan/14/2020, 02:01:10 (UTC+00:00)
>
>> DMA_CH(#i)_RxDesc_Tail_Pointer points to an offset from the base and
>> indicates the location of the last valid descriptor.
>>
>> The change introduced by "net: stmmac: Update RX Tail Pointer to last
>> free entry" incorrectly updates the RxDesc_Tail_Pointer and causess
>> Rx operation to freeze in corner case. The issue is explained as
>> follow:-
>>
>> Say, cur_rx=1 and dirty_rx=0, then we have dirty=1 and entry=0 before
>> the while (dirty-- > 0) loop of stmmac_rx_refill() is entered. When
>> the while loop is 1st entered, Rx buffer[entry=0] is refilled and after
>> entry++, then, entry=1. Now, the while loop condition check "dirty-- > 0"
>> and the while loop bails out because dirty=0. Up to this point, the
>> driver code works correctly.
>>
>> However, the current implementation sets the Rx Tail Pointer to the
>> location pointed by dirty_rx, just updated to the value of entry(=1).
>> This is incorrect because the last Rx buffer that is refileld with empty
>> buffer is with entry=0. In another words, the current logics always sets
>> Rx Tail Pointer to the next Rx buffer to be refilled (too early).
>>
>> So, we fix this by tracking the index of the most recently refilled Rx
>> buffer by using "last_refill" and use "last_refill" to update the Rx Tail
>> Pointer instead of using "entry" which points to the next dirty_rx to be
>> refilled in future.
>
>I'm not sure about this ...
>
>RX Tail points to last valid descriptor but it doesn't point to the base
>address of that one, it points to the end address.
>
>Let's say we have a ring buffer with just 1 descriptor. With your new
>logic then: RX base == RX tail (== RX base), so the IP will not see any
>descriptor. But with old logic: RX base == (RX base + 1), which causes
>the IP to correctly see the descriptor.
>
>Can you provide more information on the Rx operation freeze you
>mentioned ? Can it be another issue ?

I recheck on my side, it seems like the fix needed for simics model at my
end and not needed for actual SoC. This is strange but I can check internal
team. I also read through the databook which says that for 40-bit or 48-bit
addressing mode, the tail pointer must be advanced to the location
immediately after the descriptors are set, for the DMA to know that
additional descriptors are available.

Now, relooking at the current logic which sets the rx tail pointer according
to the value of "dirty_rx" which can be "zero" as it takes value from entry
that is incremented through STMMAC_GET_ENTRY(entry, DMA_RX_SIZE).
This too can give a situation that the base and tail registers is pointing to
the same location.

According to SNPS databook, the DMA engine goes into SUSPEND state if the
Rx descriptors are not OWN=1. The operation can be resumed by ensuring that
the descriptors are owned by the DMA and then update the tail pointer.

What is your opinion here if we always update the Rx tail pointer to pointer
the boundary of the DMA size as follow without depending on dirty_rx.

rx_q->rx_tail_addr = rx_q->dma_rx_phy + (DMA_RX_SIZE *
		     sizeof(struct dma_desc))

Thanks
Boon Leong

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

* RE: [PATCH net 3/7] net: stmmac: fix missing netdev->features in stmmac_set_features
  2020-01-13 13:17   ` Jakub Kicinski
@ 2020-01-14  6:56     ` Ong, Boon Leong
  0 siblings, 0 replies; 14+ messages in thread
From: Ong, Boon Leong @ 2020-01-14  6:56 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: netdev, Jose Abreu, Giuseppe Cavallaro, Alexandre Torgue,
	David S . Miller, Maxime Coquelin, Tan, Tee Min, Voon, Weifeng,
	linux-stm32, linux-arm-kernel, linux-kernel

>On Tue, 14 Jan 2020 10:01:12 +0800, Ong Boon Leong wrote:
>
>Please fix the date on your system.
>
>Please always provide a patch description. For bug fixes description of
>how the bug manifest to the users is important to have.
>
>> Fixes: d2afb5bdffde ("stmmac: fix the rx csum feature")
>>
>
>Please remove the empty lines between the Fixes tag and the other tags
>on all patches.

Thanks for input. Will fix. 


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

* RE: [PATCH net 4/7] net: stmmac: Fix priority steering for tx/rx queue >3
  2020-01-13 10:33   ` Jose Abreu
@ 2020-01-14 15:24     ` Voon, Weifeng
  0 siblings, 0 replies; 14+ messages in thread
From: Voon, Weifeng @ 2020-01-14 15:24 UTC (permalink / raw)
  To: Jose Abreu, Ong, Boon Leong, netdev
  Cc: Giuseppe Cavallaro, Alexandre Torgue, David S . Miller,
	Maxime Coquelin, Tan, Tee Min, linux-stm32, linux-arm-kernel,
	linux-kernel

> > Fix MACRO function define for TX and RX user priority queue steering
> > for register masking and shifting.
> 
> I think this was already fixed as seen on:
> - https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-
> next.git/commit/?id=e8df7e8c233a18d2704e37ecff47583b494789d3
> 
> Did I forget something ?
This issue is indeed already fixed by the patch that you have pointed out.
Weifeng 

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

end of thread, back to index

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-14  2:01 [PATCH net 0/7] net: stmmac: general fixes for Ethernet functionality Ong Boon Leong
2020-01-14  2:01 ` [PATCH net 1/7] net: stmmac: fix error in updating rx tail pointer to last free entry Ong Boon Leong
2020-01-13 10:29   ` Jose Abreu
2020-01-14  6:54     ` Ong, Boon Leong
2020-01-14  2:01 ` [PATCH net 2/7] net: stmmac: Fix incorrect location to set real_num_rx|tx_queues Ong Boon Leong
2020-01-14  2:01 ` [PATCH net 3/7] net: stmmac: fix missing netdev->features in stmmac_set_features Ong Boon Leong
2020-01-13 13:17   ` Jakub Kicinski
2020-01-14  6:56     ` Ong, Boon Leong
2020-01-14  2:01 ` [PATCH net 4/7] net: stmmac: Fix priority steering for tx/rx queue >3 Ong Boon Leong
2020-01-13 10:33   ` Jose Abreu
2020-01-14 15:24     ` Voon, Weifeng
2020-01-14  2:01 ` [PATCH net 5/7] net: stmmac: fix incorrect GMAC_VLAN_TAG register writting implementation Ong Boon Leong
2020-01-14  2:01 ` [PATCH net 6/7] net: stmmac: fix missing IFF_MULTICAST check in dwmac4_set_filter Ong Boon Leong
2020-01-14  2:01 ` [PATCH net 7/7] net: stmmac: update pci platform data to use phy_interface Ong Boon Leong

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org
	public-inbox-index netdev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git