Re: [PATCH net-next 1/7] net: stmmac: set MSS for each tx DMA channel

[Giuseppe CAVALLARO]

Hi Niklas

I had a look at this series that looks ok for me.

Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>

On 2/19/2018 6:11 PM, Niklas Cassel wrote:

The DMA engine in dwmac4 can segment a large TSO packet to several
smaller packets of (max) size Maximum Segment Size (MSS).

The DMA engine fetches and saves the MSS via a context descriptor.

This context decriptor has to be provided to each tx DMA channel.
To ensure that this is done, move struct member mss from stmmac_priv
to stmmac_tx_queue.

stmmac_reset_queues_param() now also resets mss, together with other
queue parameters, so reset of mss value can be removed from
stmmac_resume().

init_dma_tx_desc_rings() now also resets mss, together with other
queue parameters, so reset of mss value can be removed from
stmmac_open().

This fixes tx queue timeouts for dwmac4, with DT property
snps,tx-queues-to-use > 1, when running iperf3 with multiple threads.

Fixes: ce736788e8a9 ("net: stmmac: adding multiple buffers for TX")
Signed-off-by: Niklas Cassel <niklas.cas...@axis.com>
---
  drivers/net/ethernet/stmicro/stmmac/stmmac.h  |  2 +-
  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 13 +
  2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h 
b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index a916e13624eb..75161e1b7e55 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -58,6 +58,7 @@ struct stmmac_tx_queue {
unsigned int dirty_tx;
dma_addr_t dma_tx_phy;
u32 tx_tail_addr;
+   u32 mss;
  };
  
  struct stmmac_rx_queue {

@@ -138,7 +139,6 @@ struct stmmac_priv {
spinlock_t ptp_lock;
void __iomem *mmcaddr;
void __iomem *ptpaddr;
-   u32 mss;
  
  #ifdef CONFIG_DEBUG_FS

struct dentry *dbgfs_dir;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 7ad841434ec8..d38bf38f12f5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1355,6 +1355,7 @@ static int init_dma_tx_desc_rings(struct net_device *dev)
  
  		tx_q->dirty_tx = 0;

tx_q->cur_tx = 0;
+   tx_q->mss = 0;
  
  		netdev_tx_reset_queue(netdev_get_tx_queue(priv->dev, queue));

}
@@ -1946,6 +1947,7 @@ static void stmmac_tx_err(struct stmmac_priv *priv, u32 
chan)
 (i == DMA_TX_SIZE - 1));
tx_q->dirty_tx = 0;
tx_q->cur_tx = 0;
+   tx_q->mss = 0;
netdev_tx_reset_queue(netdev_get_tx_queue(priv->dev, chan));
stmmac_start_tx_dma(priv, chan);
  
@@ -2632,7 +2634,6 @@ static int stmmac_open(struct net_device *dev)
  
  	priv->dma_buf_sz = STMMAC_ALIGN(buf_sz);

priv->rx_copybreak = STMMAC_RX_COPYBREAK;
-   priv->mss = 0;
  
  	ret = alloc_dma_desc_resources(priv);

if (ret < 0) {
@@ -2872,10 +2873,10 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, 
struct net_device *dev)
mss = skb_shinfo(skb)->gso_size;
  
  	/* set new MSS value if needed */

-   if (mss != priv->mss) {
+   if (mss != tx_q->mss) {
mss_desc = tx_q->dma_tx + tx_q->cur_tx;
priv->hw->desc->set_mss(mss_desc, mss);
-   priv->mss = mss;
+   tx_q->mss = mss;
tx_q->cur_tx = STMMAC_GET_ENTRY(tx_q->cur_tx, DMA_TX_SIZE);
}
  
@@ -4436,6 +4437,7 @@ static void stmmac_reset_queues_param(struct stmmac_priv *priv)
  
  		tx_q->cur_tx = 0;

tx_q->dirty_tx = 0;
+   tx_q->mss = 0;
}
  }
  
@@ -4481,11 +4483,6 @@ int stmmac_resume(struct device *dev)
  
  	stmmac_reset_queues_param(priv);
  
-	/* reset private mss value to force mss context settings at

-* next tso xmit (only used for gmac4).
-*/
-   priv->mss = 0;
-
stmmac_clear_descriptors(priv);
  
  	stmmac_hw_setup(ndev, false);

Re: [PATCH net-next v2] net: stmmac: Fix reception of Broadcom switches tags

[Giuseppe CAVALLARO]

On 1/19/2018 12:12 AM, Florian Fainelli wrote:

Broadcom tags inserted by Broadcom switches put a 4 byte header after
the MAC SA and before the EtherType, which may look like some sort of 0
length LLC/SNAP packet (tcpdump and wireshark do think that way). With
ACS enabled in stmmac the packets were truncated to 8 bytes on
reception, whereas clearing this bit allowed normal reception to occur.

In order to make that possible, we need to pass a net_device argument to
the different core_init() functions and we are dependent on the Broadcom
tagger padding packets correctly (which it now does). To be as little
invasive as possible, this is only done for gmac1000 when the network
device is DSA-enabled (netdev_uses_dsa() returns true).

Signed-off-by: Florian Fainelli <f.faine...@gmail.com>


Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


---
Changes in v2:

- fixed build failure in dwmac4_core.c
- updated dwmac100_core.c to also clear the ASTP bit

  drivers/net/ethernet/stmicro/stmmac/common.h |  2 +-
  drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c|  3 ++-
  drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 12 +++-
  drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c  | 15 +--
  drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c| 12 +++-
  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c|  2 +-
  6 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h 
b/drivers/net/ethernet/stmicro/stmmac/common.h
index ce2ea2d491ac..2ffe76c0ff74 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -474,7 +474,7 @@ struct mac_device_info;
  /* Helpers to program the MAC core */
  struct stmmac_ops {
/* MAC core initialization */
-   void (*core_init)(struct mac_device_info *hw, int mtu);
+   void (*core_init)(struct mac_device_info *hw, struct net_device *dev);
/* Enable the MAC RX/TX */
void (*set_mac)(void __iomem *ioaddr, bool enable);
/* Enable and verify that the IPC module is supported */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 9eb7f65d8000..a3fa65b1ca8e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -483,7 +483,8 @@ static int sun8i_dwmac_init(struct platform_device *pdev, 
void *priv)
return 0;
  }
  
-static void sun8i_dwmac_core_init(struct mac_device_info *hw, int mtu)

+static void sun8i_dwmac_core_init(struct mac_device_info *hw,
+ struct net_device *dev)
  {
void __iomem *ioaddr = hw->pcsr;
u32 v;
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
index 8a86340ff2d3..540d21786a43 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
@@ -25,18 +25,28 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include "stmmac_pcs.h"
  #include "dwmac1000.h"
  
-static void dwmac1000_core_init(struct mac_device_info *hw, int mtu)

+static void dwmac1000_core_init(struct mac_device_info *hw,
+   struct net_device *dev)
  {
void __iomem *ioaddr = hw->pcsr;
u32 value = readl(ioaddr + GMAC_CONTROL);
+   int mtu = dev->mtu;
  
  	/* Configure GMAC core */

value |= GMAC_CORE_INIT;
  
+	/* Clear ACS bit because Ethernet switch tagging formats such as

+* Broadcom tags can look like invalid LLC/SNAP packets and cause the
+* hardware to truncate packets on reception.
+*/
+   if (netdev_uses_dsa(dev))
+   value &= ~GMAC_CONTROL_ACS;
+
if (mtu > 1500)
value |= GMAC_CONTROL_2K;
if (mtu > 2000)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
index 8ef517356313..91b23f9db31a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
@@ -25,15 +25,26 @@
  
***/
  
  #include 

+#include 
  #include 
  #include "dwmac100.h"
  
-static void dwmac100_core_init(struct mac_device_info *hw, int mtu)

+static void dwmac100_core_init(struct mac_device_info *hw,
+  struct net_device *dev)
  {
void __iomem *ioaddr = hw->pcsr;
u32 value = readl(ioaddr + MAC_CONTROL);
  
-	writel((value | MAC_CORE_INIT), ioaddr + MAC_CONTROL);

+   value |= MAC_CORE_INIT;
+
+   /* Clear ASTP bit because Ethernet switch tagging formats such as
+* Broadcom tags can look like invalid LLC/SNAP packets an

Re: [PATCH net-next] net: stmmac: Fix reception of Broadcom switches tags

[Giuseppe CAVALLARO]

Hi Florian

for gmac4.x and gmac3.x series the ACS bit is the Automatic Pad or CRC 
Stripping, so the
core strips the Pad or FCS on frames if the value of the length field is 
< 1536 bytes.
For MAC10-100 there is the Bit 8 (ASTP) of the reg0 that does the same 
if len is < 46bytes.
In your patch I can just suggest to add a new field to strip the PAD/FCS 
w/o passing the whole
netdev struct to the core_init. In the main driver, we could manage the 
pad-strip feature (also
by using dt) or disable it in case of netdev_uses_dsa; then propagating 
this setting to the core_init

or calling a new callback. What do you think?

Regards
Peppe

On 1/17/2018 12:25 AM, Florian Fainelli wrote:

Broadcom tags inserted by Broadcom switches put a 4 byte header after
the MAC SA and before the EtherType, which may look like some sort of 0
length LLC/SNAP packet (tcpdump and wireshark do think that way). With
ACS enabled in stmmac the packets were truncated to 8 bytes on
reception, whereas clearing this bit allowed normal reception to occur.

In order to make that possible, we need to pass a net_device argument to
the different core_init() functions and we are dependent on the Broadcom
tagger padding packets correctly (which it now does). To be as little
invasive as possible, this is only done for gmac1000 when the network
device is DSA-enabled (netdev_uses_dsa() returns true).

Signed-off-by: Florian Fainelli 
---
  drivers/net/ethernet/stmicro/stmmac/common.h |  2 +-
  drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c|  3 ++-
  drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 12 +++-
  drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c  |  3 ++-
  drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c| 11 ++-
  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c|  2 +-
  6 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h 
b/drivers/net/ethernet/stmicro/stmmac/common.h
index ce2ea2d491ac..2ffe76c0ff74 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -474,7 +474,7 @@ struct mac_device_info;
  /* Helpers to program the MAC core */
  struct stmmac_ops {
/* MAC core initialization */
-   void (*core_init)(struct mac_device_info *hw, int mtu);
+   void (*core_init)(struct mac_device_info *hw, struct net_device *dev);
/* Enable the MAC RX/TX */
void (*set_mac)(void __iomem *ioaddr, bool enable);
/* Enable and verify that the IPC module is supported */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 9eb7f65d8000..a3fa65b1ca8e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -483,7 +483,8 @@ static int sun8i_dwmac_init(struct platform_device *pdev, 
void *priv)
return 0;
  }
  
-static void sun8i_dwmac_core_init(struct mac_device_info *hw, int mtu)

+static void sun8i_dwmac_core_init(struct mac_device_info *hw,
+ struct net_device *dev)
  {
void __iomem *ioaddr = hw->pcsr;
u32 v;
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
index 8a86340ff2d3..540d21786a43 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
@@ -25,18 +25,28 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include "stmmac_pcs.h"
  #include "dwmac1000.h"
  
-static void dwmac1000_core_init(struct mac_device_info *hw, int mtu)

+static void dwmac1000_core_init(struct mac_device_info *hw,
+   struct net_device *dev)
  {
void __iomem *ioaddr = hw->pcsr;
u32 value = readl(ioaddr + GMAC_CONTROL);
+   int mtu = dev->mtu;
  
  	/* Configure GMAC core */

value |= GMAC_CORE_INIT;
  
+	/* Clear ACS bit because Ethernet switch tagging formats such as

+* Broadcom tags can look like invalid LLC/SNAP packets and cause the
+* hardware to truncate packets on reception.
+*/
+   if (netdev_uses_dsa(dev))
+   value &= ~GMAC_CONTROL_ACS;
+
if (mtu > 1500)
value |= GMAC_CONTROL_2K;
if (mtu > 2000)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
index 8ef517356313..c1ee427c42cb 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
@@ -28,7 +28,8 @@
  #include 
  #include "dwmac100.h"
  
-static void dwmac100_core_init(struct mac_device_info *hw, int mtu)

+static void dwmac100_core_init(struct mac_device_info *hw,
+  struct net_device *dev)
  {
void __iomem *ioaddr = hw->pcsr;
u32 value = readl(ioaddr 

Re: NETDEV WATCHDOG: eth0 (dwc-eth-dwmac): transmit queue 1 timed out

[Giuseppe CAVALLARO]

Ok Bhadram

thx for this check, I was afraid that the HW FIFO had some issues.

Best Regards
Peppe

On 12/1/2017 4:39 PM, Bhadram Varka wrote:

Hi Giuseppe,

I don't see any issue with if we execute "ping -s 1400" case. I believe in this 
case TSO not triggered.

Thanks,
Bhadram.

-Original Message-----
From: Giuseppe CAVALLARO [mailto:peppe.cavall...@st.com]
Sent: Thursday, November 23, 2017 11:58 AM
To: Bhadram Varka <vbhad...@nvidia.com>; joao.pi...@synopsys.com
Cc: linux-netdev <netdev@vger.kernel.org>
Subject: Re: NETDEV WATCHDOG: eth0 (dwc-eth-dwmac): transmit queue 1 timed out

Hi Bhadram

you said that  In normal ping scenario this is not observed, I wonder if you 
could try for example, ping with -s 1400. In that case, if still fail I think 
the issue could be the FIFO tuning and I expect overflow on RX MMC counters.

Let me know
Regards,
Peppe

On 11/20/2017 3:22 PM, Bhadram Varka wrote:

Hi Giuseppe,

Thanks for responding.

Actually I am using net-next tree for making the changes. Below patches already 
present in code base.

a0daae1 net: stmmac: Disable flow ctrl for RX AVB queues and really
enable TX AVB queues
52a7623 net: stmmac: Use correct values in TQS/RQS fields

Thanks,
Bhadram.

-Original Message-
From: Giuseppe CAVALLARO [mailto:peppe.cavall...@st.com]
Sent: Monday, November 20, 2017 6:37 PM
To: Bhadram Varka <vbhad...@nvidia.com>; joao.pi...@synopsys.com
Cc: linux-netdev <netdev@vger.kernel.org>
Subject: Re: NETDEV WATCHDOG: eth0 (dwc-eth-dwmac): transmit queue 1
timed out

Hello Bhadram

there are some new patches actually in net/net-next repo that you should have; 
for example:

  [PATCH net-next v2 0/2] net: stmmac: Improvements for
multi-queuing and for AVB

Let me know if these help you.

Regards
Peppe

On 11/20/2017 7:38 AM, Bhadram Varka wrote:

Hi Joao/Peppe,

Observed this issue more frequently with multi-channel case. Am I missing 
something in DT ?
Please help here to understand the issue.

Thanks,
Bhadram

-Original Message-
From: Bhadram Varka
Sent: Thursday, November 16, 2017 9:41 AM
To: linux-netdev <netdev@vger.kernel.org>
Subject: NETDEV WATCHDOG: eth0 (dwc-eth-dwmac): transmit queue 1
timed out

Hi,

I am trying to enable multi-queue in Tegra186 EQOS (which has support for 4 
channels). Observed below netdev watchdog warning. Its easily reproable with 
iperf test.
In normal ping scenario this is not observed. I did not observe any issue if we 
disable TSO. Looks like issue in stmmac_tso_xmit() in multi-channel scenario.

[   88.801672] NETDEV WATCHDOG: eth0 (dwc-eth-dwmac): transmit queue 0 timed out
[   88.808818] [ cut here ]
[   88.813435] WARNING: CPU: 5 PID: 0 at net/sched/sch_generic.c:320 
dev_watchdog+0x2cc/0x2d8
[   88.821681] Modules linked in: dwmac_dwc_qos_eth stmmac_platform crc32_ce 
crct10dif_ce stmmac ip_tables x_tables ipv6
[   88.832290] CPU: 5 PID: 0 Comm: swapper/5 Tainted: G S  
4.14.0-rc7-01956-g9395db5-dirty #21
[   88.841663] Hardware name: NVIDIA Tegra186 P2771- Development Board (DT)
[   88.848697] task: 8001ec8fd400 task.stack: 09e38000
[   88.854606] PC is at dev_watchdog+0x2cc/0x2d8
[   88.858952] LR is at dev_watchdog+0x2cc/0x2d8
[   88.863300] pc : [] lr : [] pstate: 
2145
[   88.870678] sp : 0802bd80
[   88.873983] x29: 0802bd80 x28: 00a0
[   88.879287] x27:  x26: 8001eae2c3b0
[   88.884589] x25: 0005 x24: 8001ecb6be80
[   88.889891] x23: 8001eae2c39c x22: 8001eae2bfb0
[   88.895192] x21: 8001eae2c000 x20: 08fe7000
[   88.900493] x19: 0001 x18: 0010
[   88.905795] x17:  x16: 
[   88.911098] x15:  x14: 756f2064656d6974
[   88.916399] x13: 2031206575657571 x12: 08fe9df0
[   88.921699] x11: 08586180 x10: 642d6874652d6377
[   88.927000] x9 : 0016 x8 : 3a474f4448435441
[   88.932301] x7 : 572056454454454e x6 : 014f
[   88.937602] x5 : 0020 x4 : 
[   88.942902] x3 :  x2 : 08fec4c0
[   88.948203] x1 : 8001ec8fd400 x0 : 0041
[   88.953504] Call trace:
[   88.955944] Exception stack(0x0802bc40 to 0x0802bd80)
[   88.962371] bc40: 0041 8001ec8fd400 08fec4c0 

[   88.970184] bc60:  0020 014f 
572056454454454e
[   88.977998] bc80: 3a474f4448435441 0016 642d6874652d6377 
08586180
[   88.985811] bca0: 08fe9df0 2031206575657571 756f2064656d6974 

[   88.993624] bcc0:   0010 
0001
[   89.001439] bce0: 08fe7000 8001eae2c000 8001eae2bfb0 
8001eae2c39c
[   89.009252] bd00: 8001ecb6be80 0005 8001eae2c3b0 

[   89.017065]

Re: NETDEV WATCHDOG: eth0 (dwc-eth-dwmac): transmit queue 1 timed out

[Giuseppe CAVALLARO]

Hi Bhadram

you said that  In normal ping scenario this is not observed, I wonder if 
you could try
for example, ping with -s 1400. In that case, if still fail I think the 
issue could be the

FIFO tuning and I expect overflow on RX MMC counters.

Let me know
Regards,
Peppe

On 11/20/2017 3:22 PM, Bhadram Varka wrote:

Hi Giuseppe,

Thanks for responding.

Actually I am using net-next tree for making the changes. Below patches already 
present in code base.

a0daae1 net: stmmac: Disable flow ctrl for RX AVB queues and really enable TX 
AVB queues
52a7623 net: stmmac: Use correct values in TQS/RQS fields

Thanks,
Bhadram.

-Original Message-
From: Giuseppe CAVALLARO [mailto:peppe.cavall...@st.com]
Sent: Monday, November 20, 2017 6:37 PM
To: Bhadram Varka <vbhad...@nvidia.com>; joao.pi...@synopsys.com
Cc: linux-netdev <netdev@vger.kernel.org>
Subject: Re: NETDEV WATCHDOG: eth0 (dwc-eth-dwmac): transmit queue 1 timed out

Hello Bhadram

there are some new patches actually in net/net-next repo that you should have; 
for example:

 [PATCH net-next v2 0/2] net: stmmac: Improvements for multi-queuing and 
for AVB

Let me know if these help you.

Regards
Peppe

On 11/20/2017 7:38 AM, Bhadram Varka wrote:

Hi Joao/Peppe,

Observed this issue more frequently with multi-channel case. Am I missing 
something in DT ?
Please help here to understand the issue.

Thanks,
Bhadram

-Original Message-
From: Bhadram Varka
Sent: Thursday, November 16, 2017 9:41 AM
To: linux-netdev <netdev@vger.kernel.org>
Subject: NETDEV WATCHDOG: eth0 (dwc-eth-dwmac): transmit queue 1 timed
out

Hi,

I am trying to enable multi-queue in Tegra186 EQOS (which has support for 4 
channels). Observed below netdev watchdog warning. Its easily reproable with 
iperf test.
In normal ping scenario this is not observed. I did not observe any issue if we 
disable TSO. Looks like issue in stmmac_tso_xmit() in multi-channel scenario.

[   88.801672] NETDEV WATCHDOG: eth0 (dwc-eth-dwmac): transmit queue 0 timed out
[   88.808818] [ cut here ]
[   88.813435] WARNING: CPU: 5 PID: 0 at net/sched/sch_generic.c:320 
dev_watchdog+0x2cc/0x2d8
[   88.821681] Modules linked in: dwmac_dwc_qos_eth stmmac_platform crc32_ce 
crct10dif_ce stmmac ip_tables x_tables ipv6
[   88.832290] CPU: 5 PID: 0 Comm: swapper/5 Tainted: G S  
4.14.0-rc7-01956-g9395db5-dirty #21
[   88.841663] Hardware name: NVIDIA Tegra186 P2771- Development Board (DT)
[   88.848697] task: 8001ec8fd400 task.stack: 09e38000
[   88.854606] PC is at dev_watchdog+0x2cc/0x2d8
[   88.858952] LR is at dev_watchdog+0x2cc/0x2d8
[   88.863300] pc : [] lr : [] pstate: 
2145
[   88.870678] sp : 0802bd80
[   88.873983] x29: 0802bd80 x28: 00a0
[   88.879287] x27:  x26: 8001eae2c3b0
[   88.884589] x25: 0005 x24: 8001ecb6be80
[   88.889891] x23: 8001eae2c39c x22: 8001eae2bfb0
[   88.895192] x21: 8001eae2c000 x20: 08fe7000
[   88.900493] x19: 0001 x18: 0010
[   88.905795] x17:  x16: 
[   88.911098] x15:  x14: 756f2064656d6974
[   88.916399] x13: 2031206575657571 x12: 08fe9df0
[   88.921699] x11: 08586180 x10: 642d6874652d6377
[   88.927000] x9 : 0016 x8 : 3a474f4448435441
[   88.932301] x7 : 572056454454454e x6 : 014f
[   88.937602] x5 : 0020 x4 : 
[   88.942902] x3 :  x2 : 08fec4c0
[   88.948203] x1 : 8001ec8fd400 x0 : 0041
[   88.953504] Call trace:
[   88.955944] Exception stack(0x0802bc40 to 0x0802bd80)
[   88.962371] bc40: 0041 8001ec8fd400 08fec4c0 

[   88.970184] bc60:  0020 014f 
572056454454454e
[   88.977998] bc80: 3a474f4448435441 0016 642d6874652d6377 
08586180
[   88.985811] bca0: 08fe9df0 2031206575657571 756f2064656d6974 

[   88.993624] bcc0:   0010 
0001
[   89.001439] bce0: 08fe7000 8001eae2c000 8001eae2bfb0 
8001eae2c39c
[   89.009252] bd00: 8001ecb6be80 0005 8001eae2c3b0 

[   89.017065] bd20: 00a0 0802bd80 0894a76c 
0802bd80
[   89.024879] bd40: 0894a76c 2145 00b67570 
0001
[   89.032693] bd60: 0001 8001ecb6b200 0802bd80 
0894a76c
[   89.040508] [] dev_watchdog+0x2cc/0x2d8
[   89.045900] [] call_timer_fn.isra.5+0x24/0x80
[   89.051809] [] expire_timers+0xa4/0xb0
[   89.057111] [] run_timer_softirq+0x140/0x170
[   89.062933] [] __do_softirq+0x12c/0x228
[   89.068323] [] irq_exit+0xd0/0x108
[   89.073278] [] __handle_domain_irq+0x60/0xb8
[   89.079098] [] gic_handle_irq+0x58/0xa8
[   89.08448

Re: NETDEV WATCHDOG: eth0 (dwc-eth-dwmac): transmit queue 1 timed out

[Giuseppe CAVALLARO]

Hello Bhadram

there are some new patches actually in net/net-next repo that you should 
have; for example:


   [PATCH net-next v2 0/2] net: stmmac: Improvements for multi-queuing 
and for AVB


Let me know if these help you.

Regards
Peppe

On 11/20/2017 7:38 AM, Bhadram Varka wrote:

Hi Joao/Peppe,

Observed this issue more frequently with multi-channel case. Am I missing 
something in DT ?
Please help here to understand the issue.

Thanks,
Bhadram

-Original Message-
From: Bhadram Varka
Sent: Thursday, November 16, 2017 9:41 AM
To: linux-netdev 
Subject: NETDEV WATCHDOG: eth0 (dwc-eth-dwmac): transmit queue 1 timed out

Hi,

I am trying to enable multi-queue in Tegra186 EQOS (which has support for 4 
channels). Observed below netdev watchdog warning. Its easily reproable with 
iperf test.
In normal ping scenario this is not observed. I did not observe any issue if we 
disable TSO. Looks like issue in stmmac_tso_xmit() in multi-channel scenario.

[   88.801672] NETDEV WATCHDOG: eth0 (dwc-eth-dwmac): transmit queue 0 timed out
[   88.808818] [ cut here ]
[   88.813435] WARNING: CPU: 5 PID: 0 at net/sched/sch_generic.c:320 
dev_watchdog+0x2cc/0x2d8
[   88.821681] Modules linked in: dwmac_dwc_qos_eth stmmac_platform crc32_ce 
crct10dif_ce stmmac ip_tables x_tables ipv6
[   88.832290] CPU: 5 PID: 0 Comm: swapper/5 Tainted: G S  
4.14.0-rc7-01956-g9395db5-dirty #21
[   88.841663] Hardware name: NVIDIA Tegra186 P2771- Development Board (DT)
[   88.848697] task: 8001ec8fd400 task.stack: 09e38000
[   88.854606] PC is at dev_watchdog+0x2cc/0x2d8
[   88.858952] LR is at dev_watchdog+0x2cc/0x2d8
[   88.863300] pc : [] lr : [] pstate: 
2145
[   88.870678] sp : 0802bd80
[   88.873983] x29: 0802bd80 x28: 00a0
[   88.879287] x27:  x26: 8001eae2c3b0
[   88.884589] x25: 0005 x24: 8001ecb6be80
[   88.889891] x23: 8001eae2c39c x22: 8001eae2bfb0
[   88.895192] x21: 8001eae2c000 x20: 08fe7000
[   88.900493] x19: 0001 x18: 0010
[   88.905795] x17:  x16: 
[   88.911098] x15:  x14: 756f2064656d6974
[   88.916399] x13: 2031206575657571 x12: 08fe9df0
[   88.921699] x11: 08586180 x10: 642d6874652d6377
[   88.927000] x9 : 0016 x8 : 3a474f4448435441
[   88.932301] x7 : 572056454454454e x6 : 014f
[   88.937602] x5 : 0020 x4 : 
[   88.942902] x3 :  x2 : 08fec4c0
[   88.948203] x1 : 8001ec8fd400 x0 : 0041
[   88.953504] Call trace:
[   88.955944] Exception stack(0x0802bc40 to 0x0802bd80)
[   88.962371] bc40: 0041 8001ec8fd400 08fec4c0 

[   88.970184] bc60:  0020 014f 
572056454454454e
[   88.977998] bc80: 3a474f4448435441 0016 642d6874652d6377 
08586180
[   88.985811] bca0: 08fe9df0 2031206575657571 756f2064656d6974 

[   88.993624] bcc0:   0010 
0001
[   89.001439] bce0: 08fe7000 8001eae2c000 8001eae2bfb0 
8001eae2c39c
[   89.009252] bd00: 8001ecb6be80 0005 8001eae2c3b0 

[   89.017065] bd20: 00a0 0802bd80 0894a76c 
0802bd80
[   89.024879] bd40: 0894a76c 2145 00b67570 
0001
[   89.032693] bd60: 0001 8001ecb6b200 0802bd80 
0894a76c
[   89.040508] [] dev_watchdog+0x2cc/0x2d8
[   89.045900] [] call_timer_fn.isra.5+0x24/0x80
[   89.051809] [] expire_timers+0xa4/0xb0
[   89.057111] [] run_timer_softirq+0x140/0x170
[   89.062933] [] __do_softirq+0x12c/0x228
[   89.068323] [] irq_exit+0xd0/0x108
[   89.073278] [] __handle_domain_irq+0x60/0xb8
[   89.079098] [] gic_handle_irq+0x58/0xa8
[   89.084484] Exception stack(0x09e3be20 to 0x09e3bf60)
[   89.090910] be20:   0001 

[   89.098724] be40:  09e3bf60 8001ecffd000 
0001
[   89.106537] be60: 0002 09e3bee0 0a00 

[   89.114351] be80: 0001  001c3dfbd9959589 
1daf5b7a4860
[   89.122164] bea0: 0825b000  c0311284 
08fc5000
[   89.129978] bec0: 08fe9000 08fe9000 08fd04a0 
08fe9e90
[   89.137792] bee0:   8001ec8fd400 

[   89.145605] bf00:  09e3bf60 0808548c 
09e3bf60
[   89.153418] bf20: 08085490 0145  

[   89.161231] bf40:  081409c4 09e3bf60 
08085490

Re: [PATCH net-next 2/2] bindings: net: stmmac: Add documentation for TSN parameters

[Giuseppe CAVALLARO]

Hello

On 10/27/2017 11:05 AM, Jose Abreu wrote:

I think we should take advantage of the fact that this is working
and ready to be merged. Its just HW configuration but maybe it
can serve as momentum for other drivers to also integrate this?


Let me propose to have it now in the next-next.

This support is for dwmac 5.x so no disturb for the common
driver APIs and there is no risk to have regressions, IMO.

IIUC, you already tested it on a real environment that
sounds to be very good progress.

Regards
Peppe


Best Regards,
Jose Miguel Abreu

Re: [PATCH net-next v2 0/2] net: stmmac: Improvements for multi-queuing and for AVB

[Giuseppe CAVALLARO]

Yes you need these settings, thx for these patches.

Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>

On 10/13/2017 11:58 AM, Jose Abreu wrote:

Hi,

Two improvements for stmmac: First one corrects the available fifo size per 
queue, second one corrects enabling of AVB queues. More info in commit log.

Best regards,
Jose Miguel Abreu

Cc: David S. Miller <da...@davemloft.net>
Cc: Joao Pinto <jpi...@synopsys.com>
Cc: Giuseppe Cavallaro <peppe.cavall...@st.com>
Cc: Alexandre Torgue <alexandre.tor...@st.com>

Changes from v1:
- Fix typo in second patch

Jose Abreu (2):
   net: stmmac: Use correct values in TQS/RQS fields
   net: stmmac: Disable flow ctrl for RX AVB queues and really enable TX
 AVB queues

  drivers/net/ethernet/stmicro/stmmac/common.h  |  5 +--
  drivers/net/ethernet/stmicro/stmmac/dwmac4.h  |  2 ++
  drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c  | 27 ++--
  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 39 +++
  4 files changed, 56 insertions(+), 17 deletions(-)

Re: [PATCH] net: ethernet: stmmac: Convert timers to use

[Giuseppe CAVALLARO]

On 10/5/2017 2:50 AM, Kees Cook wrote:

In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.

Cc: Giuseppe Cavallaro <peppe.cavall...@st.com>
Cc: Alexandre Torgue <alexandre.tor...@st.com>
Cc: netdev@vger.kernel.org
Cc: Thomas Gleixner <t...@linutronix.de>
Signed-off-by: Kees Cook <keesc...@chromium.org>

Acked-by:  Giuseppe Cavallaro <peppe.cavall...@st.com>

---
This requires commit 686fef928bba ("timer: Prepare to change timer
callback argument type") in v4.14-rc3, but should be otherwise
stand-alone.
---
  drivers/net/ethernet/stmicro/stmmac/altr_tse_pcs.c | 22 ++
  1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/altr_tse_pcs.c 
b/drivers/net/ethernet/stmicro/stmmac/altr_tse_pcs.c
index 6a9c954492f2..8b50afcdb52d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/altr_tse_pcs.c
+++ b/drivers/net/ethernet/stmicro/stmmac/altr_tse_pcs.c
@@ -118,10 +118,9 @@ int tse_pcs_init(void __iomem *base, struct tse_pcs *pcs)
return ret;
  }
  
-static void pcs_link_timer_callback(unsigned long data)

+static void pcs_link_timer_callback(struct tse_pcs *pcs)
  {
u16 val = 0;
-   struct tse_pcs *pcs = (struct tse_pcs *)data;
void __iomem *tse_pcs_base = pcs->tse_pcs_base;
void __iomem *sgmii_adapter_base = pcs->sgmii_adapter_base;
  
@@ -138,12 +137,11 @@ static void pcs_link_timer_callback(unsigned long data)

}
  }
  
-static void auto_nego_timer_callback(unsigned long data)

+static void auto_nego_timer_callback(struct tse_pcs *pcs)
  {
u16 val = 0;
u16 speed = 0;
u16 duplex = 0;
-   struct tse_pcs *pcs = (struct tse_pcs *)data;
void __iomem *tse_pcs_base = pcs->tse_pcs_base;
void __iomem *sgmii_adapter_base = pcs->sgmii_adapter_base;
  
@@ -201,14 +199,14 @@ static void auto_nego_timer_callback(unsigned long data)

}
  }
  
-static void aneg_link_timer_callback(unsigned long data)

+static void aneg_link_timer_callback(struct timer_list *t)
  {
-   struct tse_pcs *pcs = (struct tse_pcs *)data;
+   struct tse_pcs *pcs = from_timer(pcs, t, aneg_link_timer);
  
  	if (pcs->autoneg == AUTONEG_ENABLE)

-   auto_nego_timer_callback(data);
+   auto_nego_timer_callback(pcs);
else if (pcs->autoneg == AUTONEG_DISABLE)
-   pcs_link_timer_callback(data);
+   pcs_link_timer_callback(pcs);
  }
  
  void tse_pcs_fix_mac_speed(struct tse_pcs *pcs, struct phy_device *phy_dev,

@@ -237,8 +235,8 @@ void tse_pcs_fix_mac_speed(struct tse_pcs *pcs, struct 
phy_device *phy_dev,
  
  		tse_pcs_reset(tse_pcs_base, pcs);
  
-		setup_timer(>aneg_link_timer,

-   aneg_link_timer_callback, (unsigned long)pcs);
+   timer_setup(>aneg_link_timer, aneg_link_timer_callback,
+   0);
mod_timer(>aneg_link_timer, jiffies +
  msecs_to_jiffies(AUTONEGO_LINK_TIMER));
} else if (phy_dev->autoneg == AUTONEG_DISABLE) {
@@ -270,8 +268,8 @@ void tse_pcs_fix_mac_speed(struct tse_pcs *pcs, struct 
phy_device *phy_dev,
  
  		tse_pcs_reset(tse_pcs_base, pcs);
  
-		setup_timer(>aneg_link_timer,

-   aneg_link_timer_callback, (unsigned long)pcs);
+   timer_setup(>aneg_link_timer, aneg_link_timer_callback,
+   0);
mod_timer(>aneg_link_timer, jiffies +
  msecs_to_jiffies(AUTONEGO_LINK_TIMER));
}

Re: [PATCH] net: ethernet: stmmac: properly set PS bit in MII configurations during reset

[Giuseppe CAVALLARO]

Hi Thomas

On 7/29/2017 9:54 PM, Thomas Petazzoni wrote:

Hello Giuseppe,

On Wed, 28 Jun 2017 16:40:51 +0200, Giuseppe CAVALLARO wrote:


I do not want to change a critical reset function shared among
different platforms where
this problem has never met but you are right that we have to find a
way to proceed in order
to finalize your work. Let me elaborate your initial patch and I try
to give you a proposal asap.
In my mind, we should have a dedicated spear_dma_reset for your case
that should be used on
SPEAr platform driver (or by using st,spear600-gmac compatibility).
Also your patch did not consider the RMII and (R)GMII cases.

Have you had the chance to cook a different proposal? Alternatively, do
you have some specific hints to give me to make a new proposal that
would be acceptable for you ?


yes you are right and I had no chance to enter in this topic. :-(
We could follow one of the following approaches:

- add a new small platform driver where you can add ad-hoc code for 
SPEAr.


 Today there is a compatibility for st,spear600-gmac inside the 
dwmac-generic.c


- introduce a new DT parameter to set the PS bit when resetting the HW.

The latter should be quite easy to implement starting from your original 
patch,
this approach is not intrusive and can help others in case of the same 
behavior is found.


What do you think?

Regards
Peppe



Thanks a lot,

Thomas Petazzoni

Re: [PATCH 3/3] net: stmmac: Make 'alloc_dma_[rt]x_desc_resources()' look even closer

[Giuseppe CAVALLARO]

On 7/8/2017 9:46 AM, Christophe JAILLET wrote:

'alloc_dma_[rt]x_desc_resources()' functions look very close.
Remove a useless initialization and use the same label name for error
handling path in order to get them even closer.

Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>


Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


---
  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 07d486a70118..1853f7ff6657 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1449,7 +1449,7 @@ static void free_dma_rx_desc_resources(struct stmmac_priv 
*priv)
  static void free_dma_tx_desc_resources(struct stmmac_priv *priv)
  {
u32 tx_count = priv->plat->tx_queues_to_use;
-   u32 queue = 0;
+   u32 queue;
  
  	/* Free TX queue resources */

for (queue = 0; queue < tx_count; queue++) {
@@ -1561,13 +1561,13 @@ static int alloc_dma_tx_desc_resources(struct 
stmmac_priv *priv)

sizeof(*tx_q->tx_skbuff_dma),
GFP_KERNEL);
if (!tx_q->tx_skbuff_dma)
-   goto err_dma_buffers;
+   goto err_dma;
  
  		tx_q->tx_skbuff = kmalloc_array(DMA_TX_SIZE,

sizeof(struct sk_buff *),
GFP_KERNEL);
if (!tx_q->tx_skbuff)
-   goto err_dma_buffers;
+   goto err_dma;
  
  		if (priv->extend_desc) {

tx_q->dma_etx = dma_zalloc_coherent(priv->device,
@@ -1577,7 +1577,7 @@ static int alloc_dma_tx_desc_resources(struct stmmac_priv 
*priv)
_q->dma_tx_phy,
GFP_KERNEL);
if (!tx_q->dma_etx)
-   goto err_dma_buffers;
+   goto err_dma;
} else {
tx_q->dma_tx = dma_zalloc_coherent(priv->device,
   DMA_TX_SIZE *
@@ -1586,13 +1586,13 @@ static int alloc_dma_tx_desc_resources(struct 
stmmac_priv *priv)
   _q->dma_tx_phy,
   GFP_KERNEL);
if (!tx_q->dma_tx)
-   goto err_dma_buffers;
+   goto err_dma;
}
}
  
  	return 0;
  
-err_dma_buffers:

+err_dma:
free_dma_tx_desc_resources(priv);
  
  	return ret;

Re: [PATCH 2/3] net: stmmac: Fix error handling path in 'alloc_dma_tx_desc_resources()'

[Giuseppe CAVALLARO]

On 7/8/2017 9:46 AM, Christophe JAILLET wrote:

If the first 'kmalloc_array' within the loop fails, we should free what
as already been allocated, as done in all other error handling path.

Fixes: ce736788e8a9 ("net: stmmac: adding multiple buffers for TX")
Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>

Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>

---
  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 4322fa4a13e8..07d486a70118 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1561,7 +1561,7 @@ static int alloc_dma_tx_desc_resources(struct stmmac_priv 
*priv)

sizeof(*tx_q->tx_skbuff_dma),
GFP_KERNEL);
if (!tx_q->tx_skbuff_dma)
-   return -ENOMEM;
+   goto err_dma_buffers;
  
  		tx_q->tx_skbuff = kmalloc_array(DMA_TX_SIZE,

sizeof(struct sk_buff *),

Re: [PATCH 1/3] net: stmmac: Fix error handling path in 'alloc_dma_rx_desc_resources()'

[Giuseppe CAVALLARO]

On 7/8/2017 9:46 AM, Christophe JAILLET wrote:

If the first 'kmalloc_array' within the loop fails, we should free what
as already been allocated, as done in all other error handling path.

Fixes: 54139cf3bb33 ("net: stmmac: adding multiple buffers for rx")
Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>


Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


---
  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 19bba6281dab..4322fa4a13e8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1498,7 +1498,7 @@ static int alloc_dma_rx_desc_resources(struct stmmac_priv 
*priv)
sizeof(dma_addr_t),
GFP_KERNEL);
if (!rx_q->rx_skbuff_dma)
-   return -ENOMEM;
+   goto err_dma;
  
  		rx_q->rx_skbuff = kmalloc_array(DMA_RX_SIZE,

sizeof(struct sk_buff *),

Re: [PATCHv2 1/3] ethtool: stmmac: Fix Designware ethtool register dump

[Giuseppe CAVALLARO]

On 7/6/2017 10:22 PM, Thor Thayer wrote:

On 06/28/2017 10:13 AM, thor.tha...@linux.intel.com wrote:

From: Thor Thayer <thor.tha...@linux.intel.com>

The commit fbf68229ffe7 ("net: stmmac: unify registers dumps methods")

in the Linux kernel modified the register dump to store the DMA 
registers

at the DMA register offset (0x1000) but ethtool (stmmac.c) looks for the
DMA registers after the MAC registers which is offset 12.
This patch adds the DMA register offset so that indexing is correct.

Signed-off-by: Thor Thayer <thor.tha...@linux.intel.com>
Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>
---
v2  Modify the commit message to specify commit from Linux kernel.
 Add Acked-by.
---


Please disregard this patch.

After further reflection, it would be better to leave this alone and 
change the kernel driver. This change would require using different 
ethtool for different versions.


The other 2 patches with macro changes are still valid.


I think it is better to resend a V3 with the two patches

Regards
Peppe



Thanks,

Thor


  stmmac.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/stmmac.c b/stmmac.c
index fb69bfe..e1bb291 100644
--- a/stmmac.c
+++ b/stmmac.c
@@ -14,6 +14,9 @@
  #include 
  #include "internal.h"
  +/* The DMA Registers start at offset 0x1000 in the DW IP */
+#define DMA_REG_OFFSET(0x1000 / 4)
+
  int st_mac100_dump_regs(struct ethtool_drvinfo *info,
  struct ethtool_regs *regs)
  {
@@ -36,6 +39,7 @@ int st_mac100_dump_regs(struct ethtool_drvinfo *info,
fprintf(stdout, "\n");
  fprintf(stdout, "DMA Registers\n");
+stmmac_reg = (unsigned int *)regs->data + DMA_REG_OFFSET;
  for (i = 0; i < 9; i++)
  fprintf(stdout, "CSR%d  0x%08X\n", i, *stmmac_reg++);
  @@ -59,6 +63,7 @@ int st_gmac_dump_regs(struct ethtool_drvinfo 
*info, struct ethtool_regs *regs)

fprintf(stdout, "\n");
  fprintf(stdout, "DMA Registers\n");
+stmmac_reg = (unsigned int *)regs->data + DMA_REG_OFFSET;
  for (i = 0; i < 22; i++)
  fprintf(stdout, "Reg%d  0x%08X\n", i, *stmmac_reg++);

Re: [PATCH] net: ethernet: stmmac: properly set PS bit in MII configurations during reset

[Giuseppe CAVALLARO]

Hello Thomas

I do not want to change a critical reset function shared among different 
platforms where
this problem has never met but you are right that we have to find a way 
to proceed in order
to finalize your work. Let me elaborate your initial patch and I try to 
give you a proposal asap.
In my mind, we should have a dedicated spear_dma_reset for your case 
that should be used on

SPEAr platform driver (or by using st,spear600-gmac compatibility).
Also your patch did not consider the RMII and (R)GMII cases.

Regards
Peppe

On 6/25/2017 2:32 PM, Thomas Petazzoni wrote:

Hello Giuseppe,

On Mon, 15 May 2017 16:27:34 +0200, Thomas Petazzoni wrote:


On Wed, 10 May 2017 09:18:17 +0200, Thomas Petazzoni wrote:


On Wed, 10 May 2017 09:03:12 +0200, Giuseppe CAVALLARO wrote:
   

Please, read again my patch and the description of the problem that I
have sent. But basically, any solution that does not allow to set the
PS bit between asserting the DMA reset bit and polling for it to clear
will not work for MII PHYs.

yes your point was clear to me, I was just wondering if we could find an
easier way
to solve it w/o changing the API, adding  the set_ps and propagating the
"interface"
inside the DMA reset.

Maybe this could be fixed in the glue-logic in some way. Let me know
what do you think.

Well, it's more up to you to tell me how you would like this be solved.
We figured out what the problem was, but I don't know well enough the
architecture of the driver to decide how the solution to this problem
should be designed. I made an initial simple proposal to show what is
needed, but I'm definitely open to suggestions.

Do you have any suggestion on how to move forward with this?

Another kind ping on this topic. I really would like to have the
SPEAr600 network support work out of the box in mainline, which
currently isn't the case with an MII PHY.

I posted a patch that fixes the problem, see
https://patchwork.ozlabs.org/patch/755926/, but the feedback I got so
far does not give any direction on how to rework the patch to make it
acceptable. Would it be possible to get some more feedback?

Re: [PATCH 2/2] ethtool: stmmac: Add DMA HW Feature Register

[Giuseppe CAVALLARO]

On 6/27/2017 11:51 PM, thor.tha...@linux.intel.com wrote:

From: Thor Thayer <thor.tha...@linux.intel.com>

This patch adds the DMA HW Feature Register which is at the end
of the DMA registers and is documented in Version 3.70a.

Signed-off-by: Thor Thayer <thor.tha...@linux.intel.com>
---
  stmmac.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/stmmac.c b/stmmac.c
index e1bb291..7d7bebd 100644
--- a/stmmac.c
+++ b/stmmac.c
@@ -64,7 +64,7 @@ int st_gmac_dump_regs(struct ethtool_drvinfo *info, struct 
ethtool_regs *regs)
fprintf(stdout, "\n");
fprintf(stdout, "DMA Registers\n");
stmmac_reg = (unsigned int *)regs->data + DMA_REG_OFFSET;
-   for (i = 0; i < 22; i++)
+   for (i = 0; i < 23; i++)


thx Thor for these changes, I wonder if you could add a macro instead 23 
while doing this kind of changes


Sorry if I didn't it in the past.

the, you can send the series with my Acked-by: Giuseppe Cavallaro 
<peppe.cavall...@st.com>


Regards

peppe



fprintf(stdout, "Reg%d  0x%08X\n", i, *stmmac_reg++);
  
  	return 0;

Re: [PATCH] net: stmmac: Add additional registers for dwmac1000_dma ethtool

[Giuseppe CAVALLARO]

On 6/28/2017 12:16 AM, thor.tha...@linux.intel.com wrote:

From: Thor Thayer <thor.tha...@linux.intel.com>

Version 3.70a of the Designware has additional DMA registers so
add those to the ethtool DMA Register dump.
Offset 9  - Receive Interrupt Watchdog Timer Register
Offset 10 - AXI Bus Mode Register
Offset 11 - AHB or AXI Status Register
Offset 22 - HW Feature Register

Signed-off-by: Thor Thayer <thor.tha...@linux.intel.com>


Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


---
  drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c  | 4 ++--
  drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 2 +-
  2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
index 471a9aa..22cf635 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
@@ -205,8 +205,8 @@ static void dwmac1000_dump_dma_regs(void __iomem *ioaddr, 
u32 *reg_space)
  {
int i;
  
-	for (i = 0; i < 22; i++)

-   if ((i < 9) || (i > 17))
+   for (i = 0; i < 23; i++)
+   if ((i < 12) || (i > 17))
reg_space[DMA_BUS_MODE / 4 + i] =
readl(ioaddr + DMA_BUS_MODE + i * 4);
  }
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
index 743170d..babb39c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
@@ -29,7 +29,7 @@
  #include "stmmac.h"
  #include "dwmac_dma.h"
  
-#define REG_SPACE_SIZE	0x1054

+#define REG_SPACE_SIZE 0x1060
  #define MAC100_ETHTOOL_NAME   "st_mac100"
  #define GMAC_ETHTOOL_NAME "st_gmac"
  

Re: [PATCH 2/2 net-next] net: stmmac: Improve documentation on AVB parameters

[Giuseppe CAVALLARO]

Hi Joao

On 6/8/2017 8:02 PM, Joao Pinto wrote:

This patch fixes the description of the DT AVB parameters and gives
an accurate example. It was also included the base values that were
used to get the example' CBS paremeter values.

Signed-off-by: Joao Pinto 
---
  Documentation/devicetree/bindings/net/stmmac.txt | 24 
  1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/stmmac.txt 
b/Documentation/devicetree/bindings/net/stmmac.txt
index c3a7be6..707426d 100644
--- a/Documentation/devicetree/bindings/net/stmmac.txt
+++ b/Documentation/devicetree/bindings/net/stmmac.txt
@@ -109,10 +109,10 @@ Optional properties:
  [Attention] Queue 0 is reserved for legacy traffic
  and so no AVB is available in this queue.
- Configure Credit Base Shaper (if AVB Mode selected):
-   - snps,send_slope: enable Low Power Interface
-   - snps,idle_slope: unlock on WoL
-   - snps,high_credit: max write outstanding req. limit
-   - snps,low_credit: max read outstanding req. limit
+   - snps,send_slope: Send Slope Credit value
+   - snps,idle_slope:  Idle Slope Credit value
+   - snps,high_credit: High Credit value
+   - snps,low_credit: Low Credit value
- snps,priority: TX queue priority (Range: 0x0 to 0xF)
  Examples:
  
@@ -143,10 +143,18 @@ Examples:
  
  		queue1 {

snps,avb-algorithm;
-   snps,send_slope = <0x1000>;
-   snps,idle_slope = <0x1000>;
-   snps,high_credit = <0x3E800>;
-   snps,low_credit = <0xFFC18000>;
+   /*
+*  Example AVB parameters based on:
+*   Allocated Bandwidth: 40%
+*   Maximum Frame size: 1000 bytes
+*   Maximum Interference size: 1500 bytes
+*   Port Transmit Rate: 8
+*   Scaling Factor: 1024
+*/
+   snps,idle_slope = <0xCCC>;
+   snps,send_slope = <0x1333>;
+   snps,high_credit = <0x4B>;


Thanks for having taken care about this changes, please, as required, 
add a cover-letter
and give more information about these values that can be tuned by user 
and, for example,
the snps,high_credit could be as default = 0xbe4000 that is a reasonable 
value because

comes from 1522 * 8 * 1024 and LOW credit is the two complement.
  ^
frame size ---> maximum is 16

Regards
Peppe


+   snps,low_credit = <0xFFB5>;
snps,priority = <0x1>;
};
};

Re: [PATCH net] net: stmmac: fix completely hung TX when using TSO

[Giuseppe CAVALLARO]

Hi Niklas

I get the point and I acked the patch but Alex, please, can you confirm
that this issue has never seen on your boxes where the TSO has been
fully tested? The initial development (commit f748be531) introduces
the following:
(last_segment) && (buff_size < TSO_MAX_BUFF_SIZE),
...

On 6/6/2017 9:25 AM, Niklas Cassel wrote:

stmmac_tso_allocator can fail to set the Last Descriptor bit
on a descriptor that actually was the last descriptor.

This happens when the buffer of the last descriptor ends
up having a size of exactly TSO_MAX_BUFF_SIZE.

When the IP eventually reaches the next last descriptor,
which actually has the bit set, the DMA will hang.

When the DMA hangs, we get a tx timeout, however,
since stmmac does not do a complete reset of the IP
in stmmac_tx_timeout, we end up in a state with
completely hung TX.

Signed-off-by: Niklas Cassel <niklas.cas...@axis.com>


Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


---
  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 68a188e74c54..440bea049a7f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2723,7 +2723,7 @@ static void stmmac_tso_allocator(struct stmmac_priv 
*priv, unsigned int des,
  
  		priv->hw->desc->prepare_tso_tx_desc(desc, 0, buff_size,

0, 1,
-   (last_segment) && (buff_size < TSO_MAX_BUFF_SIZE),
+   (last_segment) && (tmp_len <= TSO_MAX_BUFF_SIZE),
0, 0);
  
  		tmp_len -= TSO_MAX_BUFF_SIZE;


Regards
Peppe

Re: Stmmac: fix for hw timestamp of GMAC 3 unit

[Giuseppe CAVALLARO]

Hi Mario

thanks for your tests, and, at first glance, your patches seem to be 
sensible so,

please, send the changes as patches for net.git kernel.

Regards
Peppe


On 6/6/2017 12:11 AM, Mario Molitor wrote:

Dear stmmac maintainer group,

I have found an problem in stmmac driver of linux kernel and I hope for a fix 
in the mainline kernel.
At the moment I have two patch files which fix this problem for me.
The problem seems created with the commit 
d2042052a0aa6a54f01a0c9e14243ec040b100e2 and 
ba1ffd74df74a9efa5290f87632a0ed55f1aa387 has breakage the functionality of 
GMAC3 unit.
I assume that these commits were only tested with a GMAC4 unit.
I have got seen this problem with execution of ptp4l daemon on system with 
linux 4.11 Kernel.

===
root@QuantumXsoc:~ ptp4l -f /etc/ptp.cfg -i eth0 -m
ptp4l[47.928]: selected /dev/ptp0 as PTP clock
ptp4l[47.937]: port 1: INITIALIZING to LISTENING on INITIALIZE
ptp4l[47.938]: port 0: INITIALIZING to LISTENING on INITIALIZE
ptp4l[47.938]: port 1: link up
[   48.282709] socfpga-dwmac ff702000.ethernet eth0: get valid RX hw timestamp 0
[   48.316316] socfpga-dwmac ff702000.ethernet eth0: get valid RX hw timestamp 0
[   48.340260] socfpga-dwmac ff702000.ethernet eth0: get valid RX hw timestamp 0
[   48.456738] socfpga-dwmac ff702000.ethernet eth0: get valid RX hw timestamp 0
ptp4l[48.457]: port 1: received DELAY_REQ without timestamp
[   48.488442] socfpga-dwmac ff702000.ethernet eth0: get valid RX hw timestamp 0
[   48.495599] socfpga-dwmac ff702000.ethernet eth0: get valid RX hw timestamp 0
ptp4l[48.489]: port 1: received SYNC without timestamp



I have found two kind of problems and for this two patches (based on the 4.11 
kernel) which fix this problem.

1.) PTP_TCR_SNAPTYPSEL_1

The first problem was for my point of view the change of definition 
PTP_TCR_SNAPTYPSEL_1.  I think according the
CYCLON V documention only the bit 16 of snaptypesel should be set. (more 
information see Table 17-20 (cv_5v4.pdf) : Timestamp Snapshot Dependency on 
Register Bits)
I believe that the GMAC4 have another necessary definition.

( patch 0001-stmmac-fix-ptp-header-for-GMAC3-hw-timestamp.patch )

>From 2d54d58dc8548d98572eb5fbfe488ec59b4c0ef5 Mon Sep 17 00:00:00 2001
From: Mario Molitor 
Date: Mon, 5 Jun 2017 18:58:49 +0200
Subject: [PATCH 1/2] stmmac: fix ptp header for GMAC3 hw timestamp

According the CYCLON V documention only the bit 16 of snaptypesel should set.
(more information see Table 17-20 (cv_5v4.pdf) : Timestamp Snapshot Dependency 
on Register Bits)

fixed: d2042052a0aa6a54f01a0c9e14243ec040b100e2
---
  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 15 ---
  drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h  |  3 ++-
  2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 4498a38..13a1ac9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -471,7 +471,10 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, 
struct ifreq *ifr)
/* PTP v1, UDP, any kind of event packet */
config.rx_filter = HWTSTAMP_FILTER_PTP_V1_L4_EVENT;
/* take time stamp for all event messages */
-   snap_type_sel = PTP_TCR_SNAPTYPSEL_1;
+   if (priv->plat->has_gmac4)
+   snap_type_sel = PTP_GMAC4_TCR_SNAPTYPSEL_1;
+   else
+   snap_type_sel = PTP_TCR_SNAPTYPSEL_1;
  
  			ptp_over_ipv4_udp = PTP_TCR_TSIPV4ENA;

ptp_over_ipv6_udp = PTP_TCR_TSIPV6ENA;
@@ -503,7 +506,10 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, 
struct ifreq *ifr)
config.rx_filter = HWTSTAMP_FILTER_PTP_V2_L4_EVENT;
ptp_v2 = PTP_TCR_TSVER2ENA;
/* take time stamp for all event messages */
-   snap_type_sel = PTP_TCR_SNAPTYPSEL_1;
+   if (priv->plat->has_gmac4)
+   snap_type_sel = PTP_GMAC4_TCR_SNAPTYPSEL_1;
+   else
+   snap_type_sel = PTP_TCR_SNAPTYPSEL_1;
  
  			ptp_over_ipv4_udp = PTP_TCR_TSIPV4ENA;

ptp_over_ipv6_udp = PTP_TCR_TSIPV6ENA;
@@ -537,7 +543,10 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, 
struct ifreq *ifr)
config.rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
ptp_v2 = PTP_TCR_TSVER2ENA;
/* take time stamp for all event 

Re: [PATCH] net: ethernet: stmmac: Fix altr_tse_pcs SGMII Initialization

[Giuseppe CAVALLARO]

On 5/31/2017 9:28 PM, thor.tha...@linux.intel.com wrote:

From: Thor Thayer <thor.tha...@linux.intel.com>

Fix NETDEV WATCHDOG timeout on startup by adding missing register
writes that properly setup SGMII.

Signed-off-by: Thor Thayer <thor.tha...@linux.intel.com>

Thanks a lot for this fix.

Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


---
  drivers/net/ethernet/stmicro/stmmac/altr_tse_pcs.c | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/altr_tse_pcs.c 
b/drivers/net/ethernet/stmicro/stmmac/altr_tse_pcs.c
index 489ef14..6a9c954 100644
--- a/drivers/net/ethernet/stmicro/stmmac/altr_tse_pcs.c
+++ b/drivers/net/ethernet/stmicro/stmmac/altr_tse_pcs.c
@@ -37,6 +37,7 @@
  #define TSE_PCS_CONTROL_AN_EN_MASKBIT(12)
  #define TSE_PCS_CONTROL_REG   0x00
  #define TSE_PCS_CONTROL_RESTART_AN_MASK   BIT(9)
+#define TSE_PCS_CTRL_AUTONEG_SGMII 0x1140
  #define TSE_PCS_IF_MODE_REG   0x28
  #define TSE_PCS_LINK_TIMER_0_REG  0x24
  #define TSE_PCS_LINK_TIMER_1_REG  0x26
@@ -65,6 +66,7 @@
  #define TSE_PCS_SW_RESET_TIMEOUT  100
  #define TSE_PCS_USE_SGMII_AN_MASK BIT(1)
  #define TSE_PCS_USE_SGMII_ENA BIT(0)
+#define TSE_PCS_IF_USE_SGMII   0x03
  
  #define SGMII_ADAPTER_CTRL_REG0x00

  #define SGMII_ADAPTER_DISABLE 0x0001
@@ -101,7 +103,9 @@ int tse_pcs_init(void __iomem *base, struct tse_pcs *pcs)
  {
int ret = 0;
  
-	writew(TSE_PCS_USE_SGMII_ENA, base + TSE_PCS_IF_MODE_REG);

+   writew(TSE_PCS_IF_USE_SGMII, base + TSE_PCS_IF_MODE_REG);
+
+   writew(TSE_PCS_CTRL_AUTONEG_SGMII, base + TSE_PCS_CONTROL_REG);
  
  	writew(TSE_PCS_SGMII_LINK_TIMER_0, base + TSE_PCS_LINK_TIMER_0_REG);

writew(TSE_PCS_SGMII_LINK_TIMER_1, base + TSE_PCS_LINK_TIMER_1_REG);

Re: [PATCH] net: ethernet: stmmac: properly set PS bit in MII configurations during reset

[Giuseppe CAVALLARO]

Hi Thomas

On 5/8/2017 9:12 PM, Thomas Petazzoni wrote:

Hello,

On Mon, 8 May 2017 16:28:21 +0200, Giuseppe CAVALLARO wrote:


I just see that GMAC_CONTROL and MAC_CTRL_REG are the same, so why not create a 
custom adjust_link for each dwmac type ?
This will permit to call it instead of set_ps() and remove lots of if 
(has_gmac) and co in stmmac_adjust_link()
Basicly replace all between "ctrl = readl()... and writel(ctrl)" by a sot of 
priv->hw->mac->adjust_link()

It will also help a lot for my dwmac-sun8i inclusion (since I add some if 
has_sun8i:))

Corentin, I think this is a good idea and maybe necessary now that the
driver is supporting a lot of chips.
In the past it was sufficient to have a adjust link function and a
stmmac_hw_fix_mac_speed
to invoke dedicated hook shared between MAC10/100 and GMAC inside STM
platforms.

Thomas, I wonder if you could take a look at the
priv->plat->fix_mac_speed. This can be used
for setting internal registers too.

Once again, this is not called at the right time to fix the issue I'm
seeing with a MII PHY. I need to adjust the PS bit between asserting the
reset and polling for the reset bit to clear.

->fix_mac_speed() is called in the adjust_link() call-back, which is
called way too late.

Please, read again my patch and the description of the problem that I
have sent. But basically, any solution that does not allow to set the
PS bit between asserting the DMA reset bit and polling for it to clear
will not work for MII PHYs.


yes your point was clear to me, I was just wondering if we could find an 
easier way
to solve it w/o changing the API, adding  the set_ps and propagating the 
"interface"

inside the DMA reset.

Maybe this could be fixed in the glue-logic in some way. Let me know 
what do you think.


peppe



Best regards,

Thomas Petazzoni

Re: [PATCH 0/4] net: stmmac: Fine-tuning for four function implementations

[Giuseppe CAVALLARO]

Hello Markus

Thanks a lot for your effort on stmmac

On 5/9/2017 1:51 PM, SF Markus Elfring wrote:

From: Markus Elfring <elfr...@users.sourceforge.net>
Date: Tue, 9 May 2017 13:48:03 +0200

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (4):
   Combine three seq_printf() calls into a seq_puts() in 
stmmac_sysfs_dma_cap_read()
   Replace five seq_printf() calls by seq_puts()
   Use seq_putc() in sysfs_display_ring()
   Delete an unnecessary return statement in stmmac_get_tx_hwtstamp()

  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 22 ++
  1 file changed, 10 insertions(+), 12 deletions(-)


Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>

Re: [PATCH] net: ethernet: stmmac: properly set PS bit in MII configurations during reset

[Giuseppe CAVALLARO]

Hello

On 5/3/2017 4:30 PM, Corentin Labbe wrote:

On Wed, May 03, 2017 at 10:13:56AM +0200, Giuseppe CAVALLARO wrote:

Hello Thomas

this was initially set by using the hw->link.port; both the core_init
and adjust callback
should invoke the hook and tuning the PS bit according to the speed and
mode.
So maybe the ->set_ps is superfluous and you could reuse the existent hook

let me know

Regards
peppe


I just see that GMAC_CONTROL and MAC_CTRL_REG are the same, so why not create a 
custom adjust_link for each dwmac type ?
This will permit to call it instead of set_ps() and remove lots of if 
(has_gmac) and co in stmmac_adjust_link()
Basicly replace all between "ctrl = readl()... and writel(ctrl)" by a sot of 
priv->hw->mac->adjust_link()

It will also help a lot for my dwmac-sun8i inclusion (since I add some if 
has_sun8i:))


Corentin, I think this is a good idea and maybe necessary now that the 
driver is supporting a lot of chips.
In the past it was sufficient to have a adjust link function and a 
stmmac_hw_fix_mac_speed
to invoke dedicated hook shared between MAC10/100 and GMAC inside STM 
platforms.


Thomas, I wonder if you could take a look at the 
priv->plat->fix_mac_speed. This can be used

for setting internal registers too.

Regards
Peppe



Regards


On 4/27/2017 11:45 AM, Thomas Petazzoni wrote:

On the SPEAr600 SoC, which has the dwmac1000 variant of the IP block,
the DMA reset never succeeds when a MII PHY is used (no problem with a
GMII PHY). The dwmac_dma_reset() function sets the
DMA_BUS_MODE_SFT_RESET bit in the DMA_BUS_MODE register, and then
polls until this bit clears. When a MII PHY is used, with the current
driver, this bit never clears and the driver therefore doesn't work.

The reason is that the PS bit of the GMAC_CONTROL register should be
correctly configured for the DMA reset to work. When the PS bit is 0,
it tells the MAC we have a GMII PHY, when the PS bit is 1, it tells
the MAC we have a MII PHY.

Doing a DMA reset clears all registers, so the PS bit is cleared as
well. This makes the DMA reset work fine with a GMII PHY. However,
with MII PHY, the PS bit should be set.

We have identified this issue thanks to two SPEAr600 platform:

   - One equipped with a GMII PHY, with which the existing driver was
 working fine.

   - One equipped with a MII PHY, where the current driver fails because
 the DMA reset times out.

This patch fixes the problem for the MII PHY configuration, and has
been tested with a GMII PHY configuration as well.

In terms of implement, since the ->reset() hook is implemented in the
DMA related code, we do not want to touch directly from this function
the MAC registers. Therefore, a ->set_ps() hook has been added to
stmmac_ops, which gets called between the moment the reset is asserted
and the polling loop waiting for the reset bit to clear.

In order for this ->set_ps() hook to decide what to do, we pass it the
"struct mac_device_info" so it can access the MAC registers, and the
PHY interface type so it knows if we're using a MII PHY or not.

The ->set_ps() hook is only implemented for the dwmac1000 case.

Signed-off-by: Thomas Petazzoni <thomas.petazz...@free-electrons.com>
Cc: <sta...@vger.kernel.org>
---
Do not hesitate to suggest ideas for alternative implementations, I'm
not sure if the current proposal is the one that fits best with the
current design of the driver.
---
   drivers/net/ethernet/stmicro/stmmac/common.h | 12 +---
   drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 16 
   drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h |  3 ++-
   drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c |  7 ++-
   drivers/net/ethernet/stmicro/stmmac/dwmac_dma.h  |  3 ++-
   drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c  |  6 +-
   drivers/net/ethernet/stmicro/stmmac/stmmac_main.c|  3 ++-
   7 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h 
b/drivers/net/ethernet/stmicro/stmmac/common.h
index 04d9245..d576f95 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -407,10 +407,13 @@ struct stmmac_desc_ops {
   extern const struct stmmac_desc_ops enh_desc_ops;
   extern const struct stmmac_desc_ops ndesc_ops;
   
+struct mac_device_info;

+
   /* Specific DMA helpers */
   struct stmmac_dma_ops {
/* DMA core initialization */
-   int (*reset)(void __iomem *ioaddr);
+   int (*reset)(void __iomem *ioaddr, struct mac_device_info *hw,
+phy_interface_t interface);
void (*init)(void __iomem *ioaddr, struct stmmac_dma_cfg *dma_cfg,
 u32 dma_tx, u32 dma_rx, int atds);
/* Configure the AXI Bus Mode Register */
@@ -445,12 +448,15 @@ struct stmmac_dma_ops {
void (*enable_tso)(void __iomem *ioaddr, bool en, u32 chan);
   };
   

Re: [PATCH v1 1/4] stmmac: pci: set default number of rx and tx queues

[Giuseppe CAVALLARO]

On 5/8/2017 4:14 PM, Andy Shevchenko wrote:

The commit 26d6851fd24e

("net: stmmac: set default number of rx and tx queues in stmmac_pci")

missed Intel Quark configuration. Append it here.

Fixes: 26d6851fd24e ("net: stmmac: set default number of rx and tx queues in 
stmmac_pci")
Cc: Joao Pinto <joao.pi...@synopsys.com>
Signed-off-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
---
  drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 39be96779145..ae3e836f9bb6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -145,6 +145,10 @@ static int quark_default_data(struct plat_stmmacenet_data 
*plat,
/* Set the maxmtu to a default of JUMBO_LEN */
plat->maxmtu = JUMBO_LEN;
  
+	/* Set default number of RX and TX queues to use */

+   plat->tx_queues_to_use = 1;
+   plat->rx_queues_to_use = 1;
+
return 0;
  }
  


For the Series, please consider my

Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


thx a lot

Re: [PATCH] net: ethernet: stmmac: properly set PS bit in MII configurations during reset

[Giuseppe CAVALLARO]

Hello Thomas

this was initially set by using the hw->link.port; both the core_init 
and adjust callback
should invoke the hook and tuning the PS bit according to the speed and 
mode.

So maybe the ->set_ps is superfluous and you could reuse the existent hook

let me know

Regards
peppe

On 4/27/2017 11:45 AM, Thomas Petazzoni wrote:

On the SPEAr600 SoC, which has the dwmac1000 variant of the IP block,
the DMA reset never succeeds when a MII PHY is used (no problem with a
GMII PHY). The dwmac_dma_reset() function sets the
DMA_BUS_MODE_SFT_RESET bit in the DMA_BUS_MODE register, and then
polls until this bit clears. When a MII PHY is used, with the current
driver, this bit never clears and the driver therefore doesn't work.

The reason is that the PS bit of the GMAC_CONTROL register should be
correctly configured for the DMA reset to work. When the PS bit is 0,
it tells the MAC we have a GMII PHY, when the PS bit is 1, it tells
the MAC we have a MII PHY.

Doing a DMA reset clears all registers, so the PS bit is cleared as
well. This makes the DMA reset work fine with a GMII PHY. However,
with MII PHY, the PS bit should be set.

We have identified this issue thanks to two SPEAr600 platform:

  - One equipped with a GMII PHY, with which the existing driver was
working fine.

  - One equipped with a MII PHY, where the current driver fails because
the DMA reset times out.

This patch fixes the problem for the MII PHY configuration, and has
been tested with a GMII PHY configuration as well.

In terms of implement, since the ->reset() hook is implemented in the
DMA related code, we do not want to touch directly from this function
the MAC registers. Therefore, a ->set_ps() hook has been added to
stmmac_ops, which gets called between the moment the reset is asserted
and the polling loop waiting for the reset bit to clear.

In order for this ->set_ps() hook to decide what to do, we pass it the
"struct mac_device_info" so it can access the MAC registers, and the
PHY interface type so it knows if we're using a MII PHY or not.

The ->set_ps() hook is only implemented for the dwmac1000 case.

Signed-off-by: Thomas Petazzoni 
Cc: 
---
Do not hesitate to suggest ideas for alternative implementations, I'm
not sure if the current proposal is the one that fits best with the
current design of the driver.
---
  drivers/net/ethernet/stmicro/stmmac/common.h | 12 +---
  drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 16 
  drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h |  3 ++-
  drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c |  7 ++-
  drivers/net/ethernet/stmicro/stmmac/dwmac_dma.h  |  3 ++-
  drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c  |  6 +-
  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c|  3 ++-
  7 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h 
b/drivers/net/ethernet/stmicro/stmmac/common.h
index 04d9245..d576f95 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -407,10 +407,13 @@ struct stmmac_desc_ops {
  extern const struct stmmac_desc_ops enh_desc_ops;
  extern const struct stmmac_desc_ops ndesc_ops;
  
+struct mac_device_info;

+
  /* Specific DMA helpers */
  struct stmmac_dma_ops {
/* DMA core initialization */
-   int (*reset)(void __iomem *ioaddr);
+   int (*reset)(void __iomem *ioaddr, struct mac_device_info *hw,
+phy_interface_t interface);
void (*init)(void __iomem *ioaddr, struct stmmac_dma_cfg *dma_cfg,
 u32 dma_tx, u32 dma_rx, int atds);
/* Configure the AXI Bus Mode Register */
@@ -445,12 +448,15 @@ struct stmmac_dma_ops {
void (*enable_tso)(void __iomem *ioaddr, bool en, u32 chan);
  };
  
-struct mac_device_info;

-
  /* Helpers to program the MAC core */
  struct stmmac_ops {
/* MAC core initialization */
void (*core_init)(struct mac_device_info *hw, int mtu);
+   /* Set port select. Called between asserting DMA reset and
+* waiting for the reset bit to clear.
+*/
+   void (*set_ps)(struct mac_device_info *hw,
+  phy_interface_t interface);
/* Enable and verify that the IPC module is supported */
int (*rx_ipc)(struct mac_device_info *hw);
/* Enable RX Queues */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
index 19b9b308..dfcbb5b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
@@ -75,6 +75,21 @@ static void dwmac1000_core_init(struct mac_device_info *hw, 
int mtu)
  #endif
  }
  
+static void dwmac1000_set_ps(struct mac_device_info *hw,

+phy_interface_t interface)
+{
+   void __iomem *ioaddr 

Re: [PATCH net-next v3] bindings: net: stmmac: add missing note about LPI interrupt

[Giuseppe CAVALLARO]

On 4/18/2017 2:39 PM, Niklas Cassel wrote:

From: Niklas Cassel <niklas.cas...@axis.com>

The hardware has a LPI interrupt.
There is already code in the stmmac driver to parse and handle the
interrupt. However, this information was missing from the DT binding.

At the same time, improve the description of the existing interrupts.

Signed-off-by: Niklas Cassel <niklas.cas...@axis.com>
---
  Documentation/devicetree/bindings/net/stmmac.txt | 13 -
  1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/stmmac.txt 
b/Documentation/devicetree/bindings/net/stmmac.txt
index f652b0c384ce..c3a7be6615c5 100644
--- a/Documentation/devicetree/bindings/net/stmmac.txt
+++ b/Documentation/devicetree/bindings/net/stmmac.txt
@@ -7,9 +7,12 @@ Required properties:
  - interrupt-parent: Should be the phandle for the interrupt controller
that services interrupts for this device
  - interrupts: Should contain the STMMAC interrupts
-- interrupt-names: Should contain the interrupt names "macirq"
-  "eth_wake_irq" if this interrupt is supported in the "interrupts"
-  property
+- interrupt-names: Should contain a list of interrupt names corresponding to
+   the interrupts in the interrupts property, if available.
+   Valid interrupt names are:
+  - "macirq" (combined signal for various interrupt events)
+  - "eth_wake_irq" (the interrupt to manage the remote wake-up packet 
detection)
+  - "eth_lpi" (the interrupt that occurs when Tx or Rx enters/exits LPI state)
  - phy-mode: See ethernet.txt file in the same directory.
  - snps,reset-gpio gpio number for phy reset.
  - snps,reset-active-low boolean flag to indicate if phy reset is active low.
@@ -152,8 +155,8 @@ Examples:
compatible = "st,spear600-gmac";
reg = <0xe080 0x8000>;
interrupt-parent = <>;
-   interrupts = <24 23>;
-   interrupt-names = "macirq", "eth_wake_irq";
+   interrupts = <24 23 22>;
+   interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
    mac-address = []; /* Filled in by U-Boot */
max-frame-size = <3800>;
phy-mode = "gmii";


Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>

Re: [PATCH net-next] net: stmmac: set total length of the packet to be transmitted in TDES3

[Giuseppe CAVALLARO]

On 4/10/2017 8:33 PM, Niklas Cassel wrote:

From: Niklas Cassel <niklas.cas...@axis.com>

Field FL/TPL in register TDES3 is not correctly set on GMAC4.
TX appears to be functional on GMAC 4.10a even if this field is not set,
however, to avoid relying on undefined behavior, set the length in TDES3.

The field has a different meaning depending on if the TSE bit in TDES3
is set or not (TSO). However, regardless of the TSE bit, the field is
not optional. The field is already set correctly when the TSE bit is set.

Since there is no limit for the number of descriptors that can be
used for a single packet, the field should be set to the sum of
the buffers contained in:
[ ...  ...
], which should be equal to skb->len.

Signed-off-by: Niklas Cassel <niklas.cas...@axis.com>


Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


---
  drivers/net/ethernet/stmicro/stmmac/chain_mode.c   | 6 +++---
  drivers/net/ethernet/stmicro/stmmac/common.h   | 2 +-
  drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c | 3 ++-
  drivers/net/ethernet/stmicro/stmmac/enh_desc.c | 2 +-
  drivers/net/ethernet/stmicro/stmmac/norm_desc.c| 2 +-
  drivers/net/ethernet/stmicro/stmmac/ring_mode.c| 9 ++---
  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 5 +++--
  7 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/chain_mode.c 
b/drivers/net/ethernet/stmicro/stmmac/chain_mode.c
index 37881f81319e..e93c40b4631e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/chain_mode.c
+++ b/drivers/net/ethernet/stmicro/stmmac/chain_mode.c
@@ -52,7 +52,7 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int 
csum)
tx_q->tx_skbuff_dma[entry].len = bmax;
/* do not close the descriptor and do not set own bit */
priv->hw->desc->prepare_tx_desc(desc, 1, bmax, csum, STMMAC_CHAIN_MODE,
-   0, false);
+   0, false, skb->len);
  
  	while (len != 0) {

tx_q->tx_skbuff[entry] = NULL;
@@ -70,7 +70,7 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int 
csum)
tx_q->tx_skbuff_dma[entry].len = bmax;
priv->hw->desc->prepare_tx_desc(desc, 0, bmax, csum,
STMMAC_CHAIN_MODE, 1,
-   false);
+   false, skb->len);
len -= bmax;
i++;
} else {
@@ -85,7 +85,7 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int 
csum)
/* last descriptor can be set now */
priv->hw->desc->prepare_tx_desc(desc, 0, len, csum,
STMMAC_CHAIN_MODE, 1,
-   true);
+   true, skb->len);
len = 0;
}
}
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h 
b/drivers/net/ethernet/stmicro/stmmac/common.h
index 90d28bcad880..b7ce3fbb5375 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -373,7 +373,7 @@ struct stmmac_desc_ops {
/* Invoked by the xmit function to prepare the tx descriptor */
void (*prepare_tx_desc) (struct dma_desc *p, int is_fs, int len,
 bool csum_flag, int mode, bool tx_own,
-bool ls);
+bool ls, unsigned int tot_pkt_len);
void (*prepare_tso_tx_desc)(struct dma_desc *p, int is_fs, int len1,
int len2, bool tx_own, bool ls,
unsigned int tcphdrlen,
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
index 843ec69222ea..aa6476439aee 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
@@ -304,12 +304,13 @@ static void dwmac4_rd_init_tx_desc(struct dma_desc *p, 
int mode, int end)
  
  static void dwmac4_rd_prepare_tx_desc(struct dma_desc *p, int is_fs, int len,

  bool csum_flag, int mode, bool tx_own,
- bool ls)
+ bool ls, unsigned int tot_pkt_len)
  {
unsigned int tdes3 = le32_to_cpu(p->des3);
  
  	p->des2 |= cpu_to_le32(len & TDES2_BUFFER1_SIZE_MASK);
  
+	tdes3 |= tot_pkt_len & TDES3_PACKET_SIZE_MASK;

if (is_fs)
tdes3 |= TDES3_FIRST_DESCRIPTOR;
else
diff --git a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c 
b/drivers/net/ethernet/stmicro/stmm

Re: [PATCH net-next v2] bindings: net: stmmac: add missing note about LPI interrupt

[Giuseppe CAVALLARO]

Hi Niklas

On 4/10/2017 9:43 AM, Niklas Cassel wrote:

From: Niklas Cassel 

The hardware has a LPI interrupt.
There is already code in the stmmac driver to parse and handle the
interrupt. However, this information was missing from the DT binding.


i wonder if we could improve the comments in this patch too

Signed-off-by: Niklas Cassel 
---
  Documentation/devicetree/bindings/net/stmmac.txt | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/stmmac.txt 
b/Documentation/devicetree/bindings/net/stmmac.txt
index f652b0c384ce..84e4cbfd3b0f 100644
--- a/Documentation/devicetree/bindings/net/stmmac.txt
+++ b/Documentation/devicetree/bindings/net/stmmac.txt
@@ -8,8 +8,8 @@ Required properties:
that services interrupts for this device
  - interrupts: Should contain the STMMAC interrupts
  - interrupt-names: Should contain the interrupt names "macirq"
-  "eth_wake_irq" if this interrupt is supported in the "interrupts"
-  property
+  "eth_wake_irq" if this interrupt is supported in the "interrupts" property

this is the PMT interrupt to manage the remote wake-up packet detection

+  "eth_lpi" if this interrupt is supported in the "interrupts" property

This is the interrupt that occurs when Tx or Rx enter/exit from LPI state

Regards
Peppe

  - phy-mode: See ethernet.txt file in the same directory.
  - snps,reset-gpio gpio number for phy reset.
  - snps,reset-active-low boolean flag to indicate if phy reset is active low.
@@ -152,8 +152,8 @@ Examples:
compatible = "st,spear600-gmac";
reg = <0xe080 0x8000>;
interrupt-parent = <>;
-   interrupts = <24 23>;
-   interrupt-names = "macirq", "eth_wake_irq";
+   interrupts = <24 23 22>;
+   interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
mac-address = []; /* Filled in by U-Boot */
max-frame-size = <3800>;
phy-mode = "gmii";

Re: [PATCH net-next] net: stmmac: set total length of the packet to be transmitted in TDES3

[Giuseppe CAVALLARO]

Hi Niklas

patch looks ok for me, Alex any feedback?

peppe

On 4/10/2017 8:33 PM, Niklas Cassel wrote:

From: Niklas Cassel 

Field FL/TPL in register TDES3 is not correctly set on GMAC4.
TX appears to be functional on GMAC 4.10a even if this field is not set,
however, to avoid relying on undefined behavior, set the length in TDES3.

The field has a different meaning depending on if the TSE bit in TDES3
is set or not (TSO). However, regardless of the TSE bit, the field is
not optional. The field is already set correctly when the TSE bit is set.

Since there is no limit for the number of descriptors that can be
used for a single packet, the field should be set to the sum of
the buffers contained in:
[ ...  ...
], which should be equal to skb->len.

Signed-off-by: Niklas Cassel 
---
  drivers/net/ethernet/stmicro/stmmac/chain_mode.c   | 6 +++---
  drivers/net/ethernet/stmicro/stmmac/common.h   | 2 +-
  drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c | 3 ++-
  drivers/net/ethernet/stmicro/stmmac/enh_desc.c | 2 +-
  drivers/net/ethernet/stmicro/stmmac/norm_desc.c| 2 +-
  drivers/net/ethernet/stmicro/stmmac/ring_mode.c| 9 ++---
  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 5 +++--
  7 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/chain_mode.c 
b/drivers/net/ethernet/stmicro/stmmac/chain_mode.c
index 37881f81319e..e93c40b4631e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/chain_mode.c
+++ b/drivers/net/ethernet/stmicro/stmmac/chain_mode.c
@@ -52,7 +52,7 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int 
csum)
tx_q->tx_skbuff_dma[entry].len = bmax;
/* do not close the descriptor and do not set own bit */
priv->hw->desc->prepare_tx_desc(desc, 1, bmax, csum, STMMAC_CHAIN_MODE,
-   0, false);
+   0, false, skb->len);
  
  	while (len != 0) {

tx_q->tx_skbuff[entry] = NULL;
@@ -70,7 +70,7 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int 
csum)
tx_q->tx_skbuff_dma[entry].len = bmax;
priv->hw->desc->prepare_tx_desc(desc, 0, bmax, csum,
STMMAC_CHAIN_MODE, 1,
-   false);
+   false, skb->len);
len -= bmax;
i++;
} else {
@@ -85,7 +85,7 @@ static int stmmac_jumbo_frm(void *p, struct sk_buff *skb, int 
csum)
/* last descriptor can be set now */
priv->hw->desc->prepare_tx_desc(desc, 0, len, csum,
STMMAC_CHAIN_MODE, 1,
-   true);
+   true, skb->len);
len = 0;
}
}
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h 
b/drivers/net/ethernet/stmicro/stmmac/common.h
index 90d28bcad880..b7ce3fbb5375 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -373,7 +373,7 @@ struct stmmac_desc_ops {
/* Invoked by the xmit function to prepare the tx descriptor */
void (*prepare_tx_desc) (struct dma_desc *p, int is_fs, int len,
 bool csum_flag, int mode, bool tx_own,
-bool ls);
+bool ls, unsigned int tot_pkt_len);
void (*prepare_tso_tx_desc)(struct dma_desc *p, int is_fs, int len1,
int len2, bool tx_own, bool ls,
unsigned int tcphdrlen,
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
index 843ec69222ea..aa6476439aee 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
@@ -304,12 +304,13 @@ static void dwmac4_rd_init_tx_desc(struct dma_desc *p, 
int mode, int end)
  
  static void dwmac4_rd_prepare_tx_desc(struct dma_desc *p, int is_fs, int len,

  bool csum_flag, int mode, bool tx_own,
- bool ls)
+ bool ls, unsigned int tot_pkt_len)
  {
unsigned int tdes3 = le32_to_cpu(p->des3);
  
  	p->des2 |= cpu_to_le32(len & TDES2_BUFFER1_SIZE_MASK);
  
+	tdes3 |= tot_pkt_len & TDES3_PACKET_SIZE_MASK;

if (is_fs)
tdes3 |= TDES3_FIRST_DESCRIPTOR;
else
diff --git a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c 
b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
index 323b59ec74a3..7546b3664113 100644
--- 

Re: [PATCH] [net-next] stmmac: use netif_set_real_num_{rx,tx}_queues

[Giuseppe CAVALLARO]

Hello Joao

On 4/3/2017 3:12 PM, Joao Pinto wrote:

Yes older cores do not support multiple queues and I tried to isolate the
features not to affect older versions.


ok so we are inline ;-)



Do you think that functions as "ndev = alloc_etherdev_mqs" has some sort of
influence?


I do not think so is we are passing the right txqs and rxqs values.

Regards
Peppe



Thanks.

Re: [PATCH] [net-next] stmmac: use netif_set_real_num_{rx,tx}_queues

[Giuseppe CAVALLARO]

Hello Joao

On 3/30/2017 6:42 PM, Joao Pinto wrote:

Às 5:35 PM de 3/30/2017, Niklas Cassel escreveu:

On 03/30/2017 04:34 PM, Thierry Reding wrote:

On Thu, Mar 30, 2017 at 09:45:36AM +0200, Corentin Labbe wrote:

On Tue, Mar 28, 2017 at 06:01:05PM -0700, David Miller wrote:

From: Arnd Bergmann 
Date: Tue, 28 Mar 2017 11:48:21 +0200


A driver must not access the two fields directly but should instead use
the helper functions to set the values and keep a consistent internal
state:

ethernet/stmicro/stmmac/stmmac_main.c: In function 'stmmac_dvr_probe':
ethernet/stmicro/stmmac/stmmac_main.c:4083:8: error: 'struct net_device' has no 
member named 'real_num_rx_queues'; did you mean 'real_num_tx_queues'?

Fixes: a8f5102af2a7 ("net: stmmac: TX and RX queue priority configuration")
Signed-off-by: Arnd Bergmann 

Applied.

This break my revert patch. (since it patch ("net: stmmac: enable multiple 
buffers").
Since dwmac-sunxi is still broken, what can I do ? send two revert patch ? or 
adapt the reverting patch.

Have you tried if the kcalloc() patch I sent on Tuesday fixes things the
issues introduced by the multiple buffers patch? Niklas reported that it
restores functionality on his setup.

If it makes things work for you as well, we could maybe avoid the revert
altogether.

Thierry, I know that you are using DWMAC CORE 4.XX
How many RX queues and how many TX queues have you got?

I'm also using DWMAC CORE 4.XX
We have 2 TX queues and 1 RX queue.

I think that Corentin is using DWMAC CORE 3.XX

I know that Joao is using an IP Prototyping Kit that uses
DWMAC CORE 4.XX (connected via PCIe).
It would be nice if Joao could get an IP Prototyping Kit
based on DWMAC CORE 3.XX.

Doesn't Synopsys have an IP Prototyping Kit based on
DWMAC CORE 3.XX laying around somewhere? :)


I requested a prototyping platform with MAC 100 or a MAC 1000 in order to make
more tests, but I don't have an ETA for it yet.

The implication of the multiple buffers patch in 3.xx is some flow change in the
configuration of dma op mode or similar. I would recomend Corentin to dump the
dma & mac registers in the end of the _open function in order to see if the DMA
is really being well configured and is really started.


Old devices managed as stmmac: mac10/100 platform driver have no 
multi-queue support.


It was introduced in the new versions managed by mac1000 but in my opinion
the stmmac driver has to only work with a single queue for tx and rx on ALL
the 3.x versions.

In fact, although the latest series have the multi-queue and channels 
there is

just one IRQ and this is a very poor implementation.
The 3.xx is very different from what we have in the 4.XX (QoS) on this 
part and AV/B.


Nobody has ever asked for having this support in all these years for the 
previous

chips.

Adding this kind of support we'll spend time for having a no useful 
optimization
and risking to break the compatibility with a lot platforms that use 
these chips.


I image, multi-queue stable on 4.XX and no support on all older versions.
Let me know if you share that.

Regards
Peppe



Thanks.

Joao

Re: [PATCH v3 01/20] net: stmmac: export stmmac_set_mac_addr/stmmac_get_mac_addr

[Giuseppe CAVALLARO]

Hello Alex

do you can check if this has to be done for ST platforms?
I do not remember that it was necessary when build as module so
I cannot expect this should be only for dwmac-sun8i.

Regards
peppe



On 4/3/2017 11:14 AM, Corentin Labbe wrote:

Thoses symbol will be needed for the dwmac-sun8i ethernet driver.
For letting it to be build as module, they need to be exported.

Signed-off-by: Corentin Labbe 
---
  drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
index 38f9430..67af0bd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
@@ -248,6 +248,7 @@ void stmmac_set_mac_addr(void __iomem *ioaddr, u8 addr[6],
data = (addr[3] << 24) | (addr[2] << 16) | (addr[1] << 8) | addr[0];
writel(data, ioaddr + low);
  }
+EXPORT_SYMBOL_GPL(stmmac_set_mac_addr);
  
  /* Enable disable MAC RX/TX */

  void stmmac_set_mac(void __iomem *ioaddr, bool enable)
@@ -279,4 +280,4 @@ void stmmac_get_mac_addr(void __iomem *ioaddr, unsigned 
char *addr,
addr[4] = hi_addr & 0xff;
addr[5] = (hi_addr >> 8) & 0xff;
  }
-
+EXPORT_SYMBOL_GPL(stmmac_get_mac_addr);

Re: [PATCH v3 02/20] net: stmmac: add optional setup function

[Giuseppe CAVALLARO]

Hello Corentin

On 4/3/2017 11:14 AM, Corentin Labbe wrote:

Instead of adding more ifthen logic for adding a new mac_device_info
setup function, it is easier to add a function pointer to the function
needed.

Signed-off-by: Corentin Labbe 
---
  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 +++-
  include/linux/stmmac.h| 3 +++
  2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 43361f3..3beca41 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3508,7 +3508,9 @@ static int stmmac_hw_init(struct stmmac_priv *priv)
struct mac_device_info *mac;
  
  	/* Identify the MAC HW device */

-   if (priv->plat->has_gmac) {
+   if (priv->plat->setup) {
+   mac = priv->plat->setup(priv);
+   } else if (priv->plat->has_gmac) {
priv->dev->priv_flags |= IFF_UNICAST_FLT;
mac = dwmac1000_setup(priv->ioaddr,
  priv->plat->multicast_filter_bins,
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 3921cb9..dadd74b 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -144,6 +144,8 @@ struct stmmac_txq_cfg {
u32 prio;
  };
  
+struct stmmac_priv;

+
  struct plat_stmmacenet_data {
int bus_id;
int phy_addr;
@@ -177,6 +179,7 @@ struct plat_stmmacenet_data {
void (*fix_mac_speed)(void *priv, unsigned int speed);
int (*init)(struct platform_device *pdev, void *priv);
void (*exit)(struct platform_device *pdev, void *priv);
+   struct mac_device_info *(*setup)(struct stmmac_priv *priv);


In that case  I prefer to have void *priv as done for the other callbacks.
I mean, keep the priv struct inside the driver part.

peppe


void *bsp_priv;
struct clk *stmmac_clk;
struct clk *pclk;

Re: stmmac still supporting spear600 ?

[Giuseppe CAVALLARO]

Hi Thomas

I tested the SMSC on other platform (+ stmmac), not on SPEAr.

ok for reset, keep the radar on clock. Hmm, can you attach a piece of 
log file to see the failure?


Regards
Peppe

On 4/2/2017 11:30 PM, Thomas Petazzoni wrote:

Hello,

On Thu, 23 Mar 2017 11:33:23 +0100, Giuseppe CAVALLARO wrote:


Further research has revealed that everything is working fine on a
platform with a Gigabit PHY connected via GMII.

However, on a different platform (which I'm using) with a 10/100 PHY
connected via MII, DMA_RESET never clears, and networking doesn't work.
The SMSC PHY LAN8700 is also supposed to be providing the clock through
its TX_CLK pin. I double checked, and both the MAC and PHY are in MII
mode, but still no luck so far.

Of course, if you have any suggestion or hint, I'm all ears :)

I can just you to keep the focus on clock configuration. I tested the
SMSC PHY LAN8700
w/o any issues on several platform.  In MII both rx/tx_clk are provided
by PHY and if you
have an external oscillator this should be safe enough, indeed.
Another check you can do is about the reset time! Maybe you need to
change something
when reset the SMSC transceiver, try to increase the delay (if you use
GPIO to reset it).

On which platform did you test with the LAN8700 PHY ? Was it on a
SPEAr600 based platform ?

If you tested on SPEAr600, what is the GMAC clock configuration (i.e
the value of the GMAC_CFG_CTR and GMAC_CFG_SYNT registers) ?

Regarding the PHY reset time, our PHY reset pin is not connected to a
GPIO, but to the system reset logic, so Linux cannot reset the PHY with
a GPIO.

Thanks!

Thomas

Re: stmmac: Performance regression after commit aff3d9eff843 "net: stmmac: enable multiple buffers"

[Giuseppe CAVALLARO]

On 3/23/2017 11:48 AM, Giuseppe CAVALLARO wrote:

Hello

On 3/23/2017 11:20 AM, Corentin Labbe wrote:

I have a 4.21 QoS Core with 4 RX + 4 TX and detected no regression.
>Could you please share the iperf cmds you are using in order for me 
to reproduce

>in my side?


Joao, you have a really powerful HW integration with multiple channels 
for both RX and TX.
Often this is not the same for other setup where, usually just a DMA0 
is present or, sometime, there

is just one RX extra channel.

My question is, what happens on this kind of configurations? Are we 
still guarantying the best performances?


Also we have to guarantee, that the TSO and SG are always working. 
Another point is the buffer sizes that

can be different among platforms.

The problem  below reported by Corentin push me to think that there is 
a bug, so we should
understand when this has been introduced and if likely fixed by some 
configuration we are

not take care right now.

ndesc_get_rx_status: Oversized frame spanned multiple buffers"


I wonder if this could be easily triggered by getting a big file via 
FTP. So not properly related on performance benchs


peppe




Best Regards
Peppe

Re: stmmac: Performance regression after commit aff3d9eff843 "net: stmmac: enable multiple buffers"

[Giuseppe CAVALLARO]

Hello

On 3/23/2017 11:20 AM, Corentin Labbe wrote:

I have a 4.21 QoS Core with 4 RX + 4 TX and detected no regression.
>Could you please share the iperf cmds you are using in order for me to 
reproduce
>in my side?


Joao, you have a really powerful HW integration with multiple channels 
for both RX and TX.
Often this is not the same for other setup where, usually just a DMA0 is 
present or, sometime, there

is just one RX extra channel.

My question is, what happens on this kind of configurations? Are we 
still guarantying the best performances?


Also we have to guarantee, that the TSO and SG are always working. 
Another point is the buffer sizes that

can be different among platforms.

The problem  below reported by Corentin push me to think that there is a 
bug, so we should
understand when this has been introduced and if likely fixed by some 
configuration we are

not take care right now.

ndesc_get_rx_status: Oversized frame spanned multiple buffers"


Best Regards
Peppe

Re: stmmac still supporting spear600 ?

[Giuseppe CAVALLARO]

Hello Thomas

On 3/21/2017 3:50 PM, Thomas Petazzoni wrote:

Hello,

On Thu, 9 Mar 2017 15:56:31 +0100, Giuseppe CAVALLARO wrote:


On 3/9/2017 10:32 AM, Thomas Petazzoni wrote:


OK, I'll have a look. However, I'm still confused by this DMA_RESET bit
that never clears, contrary to what the datasheet says. Are there some
erratas?

I suggest you to take a look at the tx/rx clocks from PHY.
You have to provide these otherwise you cannot reset the engine.

Thanks for the hint.


you are welcome


Further research has revealed that everything is working fine on a
platform with a Gigabit PHY connected via GMII.

However, on a different platform (which I'm using) with a 10/100 PHY
connected via MII, DMA_RESET never clears, and networking doesn't work.
The SMSC PHY LAN8700 is also supposed to be providing the clock through
its TX_CLK pin. I double checked, and both the MAC and PHY are in MII
mode, but still no luck so far.

Of course, if you have any suggestion or hint, I'm all ears :)


I can just you to keep the focus on clock configuration. I tested the 
SMSC PHY LAN8700
w/o any issues on several platform.  In MII both rx/tx_clk are provided 
by PHY and if you

have an external oscillator this should be safe enough, indeed.
Another check you can do is about the reset time! Maybe you need to 
change something
when reset the SMSC transceiver, try to increase the delay (if you use 
GPIO to reset it).


Regards
Peppe



Thanks,

Thomas

Re: linux-next-20170320 break stmmac on dwmac-sunxi

[Giuseppe CAVALLARO]

Hello Corentin

yes, bisect process is really good approach to me. Pls give us more details.
Recently the multi DMA channel logic has been added so it could be that
something is needed to allow your platform to manage the new code.
Or we introduced some regression. If I have some other idea, I ping you.

Peppe

On 3/20/2017 8:54 PM, Corentin Labbe wrote:

Hello

Just pushed next-20170320 to my boards and stmmac stop working on both intree 
dwmac-sunxi and my dev dwmac-sun8i.
It seems that interrupts never fire, and transmit queue timeout.
I will try to bisect this problem but perhaps other people could try to 
reproduce it.

Regards
Corentin Labbe

Re: linux-next-20170320 break stmmac on dwmac-sunxi

[Giuseppe CAVALLARO]

+ Joao

please Joao, could you do a check if new changes have any impact on that?

peppe

On 3/20/2017 8:54 PM, Corentin Labbe wrote:

Hello

Just pushed next-20170320 to my boards and stmmac stop working on both intree 
dwmac-sunxi and my dev dwmac-sun8i.
It seems that interrupts never fire, and transmit queue timeout.
I will try to bisect this problem but perhaps other people could try to 
reproduce it.

Regards
Corentin Labbe

Re: stmmac still supporting spear600 ?

[Giuseppe CAVALLARO]

Hello Thomas

On 3/9/2017 10:32 AM, Thomas Petazzoni wrote:


OK, I'll have a look. However, I'm still confused by this DMA_RESET bit
that never clears, contrary to what the datasheet says. Are there some
erratas?


I suggest you to take a look at the tx/rx clocks from PHY.
You have to provide these otherwise you cannot reset the engine.

Best Regards
Peppe



Best regards,

Thomas

Re: stmmac still supporting spear600 ?

[Giuseppe CAVALLARO]

Hello

We do not test stmmac on this spear board since many years
and I guess you have to provide parameters from the platform.
In fact, stmmac is recently tested with DT.
IIRC, the mac on spear600 could not have the HW cap register
so it is mandatory to provide all the right parameters and
configurations for this HW.

Regards
Peppe

On 3/9/2017 9:34 AM, Thomas Petazzoni wrote:

Hello,

I'm porting Linux to an old spear600 based platform, and therefore
trying to use the stmmac driver on this SoC. Unfortunately, it doesn't
work quite well and I'm wondering if the stmmac driver still properly
supports the old version of the IP that is used in this SoC. I'm
testing with v4.11-rc1.

First, the logs:

Linux version 4.11.0-rc1 (thomas@skate) (gcc version 5.4.0 20160609 
(Ubuntu/Linaro 5.4.0-6ubuntu1~16.04.4) ) #18 Thu Mar 9 09:18:01 CET 2017
[...]
libphy: Fixed MDIO Bus: probed
stmmaceth e080.ethernet: no reset control found
stmmac - user ID: 0x10, Synopsys ID: 0x32
stmmaceth e080.ethernet: Ring mode enabled
stmmaceth e080.ethernet: DMA HW capability register supported
stmmaceth e080.ethernet: Normal descriptors
libphy: stmmac: probed
stmmaceth e080.ethernet (unnamed net_device) (uninitialized): PHY ID 
0007c0c4 at 31 IRQ POLL (stmmac-0:1f) active
[...]
# ifconfig eth0 192.168.1.89
stmmaceth e080.ethernet eth0: device MAC address 00:30:d3:21:22:60
Generic PHY stmmac-0:1f: attached PHY driver [Generic PHY] 
(mii_bus:phy_addr=stmmac-0:1f, irq=-1)
stmmaceth e080.ethernet: Failed to reset the dma
stmmaceth e080.ethernet eth0: stmmac_hw_setup: DMA engine initialization 
failed
stmmaceth e080.ethernet eth0: stmmac_open: Hw setup failed
ifconfig: SIOCSIFFLAGS: Device or resource busy

So the reason why it fails to reset the DMA is because
dwmac_dma_reset() sets bit DMA_BUS_MODE_SFT_RESET in register
DMA_BUS_MODE and waits for this bit to clear, but that never happens.

The reason why I'm not sure if this IP is still supported is because
dwmac1000_get_hw_feature() reads the DMA_HW_FEATURE register (offset
0x1058), and my spear600 datasheet doesn't mention this register at
all.

It is worth mentioning that:

 - Ethernet is working fine under U-Boot (it's an old 2010.03 U-Boot
   version, with a good number of patches), so I'm sure the HW is
   working.

 - Setting bit 1 in the DMA_BUS_MODE register from U-Boot also leaves
   this bit set forever, it apparently never clears (unless my tests
   were wrong, which is very possible).

In terms of Device Tree, I'm simply using spear600.dtsi, and enabling
the gmac node, nothing else.

Has the stmmac driver been recently used/tested on spear600 ?

Thanks,

Thomas Petazzoni

Re: [PATCH RFC] net: stmmac: unify registers dumps methods

[Giuseppe CAVALLARO]

On 2/20/2017 2:44 PM, Corentin Labbe wrote:

The stmmac driver have two methods for registers dumps: via ethtool and
at init (if NETIF_MSG_HW is enabled).

It is better to keep only one method, ethtool, since the other was ugly.

This patch convert all dump_regs() function from "printing regs" to
"fill the reg_space used by ethtool".

Signed-off-by: Corentin Labbe <clabbe.montj...@gmail.com>


Thx I find it really useful,

Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


---
 drivers/net/ethernet/stmicro/stmmac/common.h   |  4 +-
 .../net/ethernet/stmicro/stmmac/dwmac1000_core.c   | 10 +--
 .../net/ethernet/stmicro/stmmac/dwmac1000_dma.c| 16 ++---
 .../net/ethernet/stmicro/stmmac/dwmac100_core.c| 30 +++--
 drivers/net/ethernet/stmicro/stmmac/dwmac100_dma.c | 15 ++---
 drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c  | 12 +---
 drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c   | 78 +++---
 .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c   | 21 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  |  5 --
 9 files changed, 71 insertions(+), 120 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h 
b/drivers/net/ethernet/stmicro/stmmac/common.h
index daaafa9..7552775 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -416,7 +416,7 @@ struct stmmac_dma_ops {
/* Configure the AXI Bus Mode Register */
void (*axi)(void __iomem *ioaddr, struct stmmac_axi *axi);
/* Dump DMA registers */
-   void (*dump_regs) (void __iomem *ioaddr);
+   void (*dump_regs)(void __iomem *ioaddr, u32 *reg_space);
/* Set tx/rx threshold in the csr6 register
 * An invalid value enables the store-and-forward mode */
void (*dma_mode)(void __iomem *ioaddr, int txmode, int rxmode,
@@ -456,7 +456,7 @@ struct stmmac_ops {
/* Enable RX Queues */
void (*rx_queue_enable)(struct mac_device_info *hw, u32 queue);
/* Dump MAC registers */
-   void (*dump_regs)(struct mac_device_info *hw);
+   void (*dump_regs)(struct mac_device_info *hw, u32 *reg_space);
/* Handle extra events on specific interrupts hw dependent */
int (*host_irq_status)(struct mac_device_info *hw,
   struct stmmac_extra_stats *x);
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
index 91c8926..19b9b30 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
@@ -92,17 +92,13 @@ static int dwmac1000_rx_ipc_enable(struct mac_device_info 
*hw)
return !!(value & GMAC_CONTROL_IPC);
 }

-static void dwmac1000_dump_regs(struct mac_device_info *hw)
+static void dwmac1000_dump_regs(struct mac_device_info *hw, u32 *reg_space)
 {
void __iomem *ioaddr = hw->pcsr;
int i;
-   pr_info("\tDWMAC1000 regs (base addr = 0x%p)\n", ioaddr);

-   for (i = 0; i < 55; i++) {
-   int offset = i * 4;
-   pr_info("\tReg No. %d (offset 0x%x): 0x%08x\n", i,
-   offset, readl(ioaddr + offset));
-   }
+   for (i = 0; i < 55; i++)
+   reg_space[i] = readl(ioaddr + i * 4);
 }

 static void dwmac1000_set_umac_addr(struct mac_device_info *hw,
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
index fbaec0f..d3654a4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c
@@ -201,18 +201,14 @@ static void dwmac1000_dma_operation_mode(void __iomem 
*ioaddr, int txmode,
writel(csr6, ioaddr + DMA_CONTROL);
 }

-static void dwmac1000_dump_dma_regs(void __iomem *ioaddr)
+static void dwmac1000_dump_dma_regs(void __iomem *ioaddr, u32 *reg_space)
 {
int i;
-   pr_info(" DMA registers\n");
-   for (i = 0; i < 22; i++) {
-   if ((i < 9) || (i > 17)) {
-   int offset = i * 4;
-   pr_err("\t Reg No. %d (offset 0x%x): 0x%08x\n", i,
-  (DMA_BUS_MODE + offset),
-  readl(ioaddr + DMA_BUS_MODE + offset));
-   }
-   }
+
+   for (i = 0; i < 22; i++)
+   if ((i < 9) || (i > 17))
+   reg_space[DMA_BUS_MODE / 4 + i] =
+   readl(ioaddr + DMA_BUS_MODE + i * 4);
 }

 static void dwmac1000_get_hw_feature(void __iomem *ioaddr,
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
index 8ab5189..e370cce 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c
@@ -40,28 

Re: [PATCH 0/8] net: stmmac: misc patchs

[Giuseppe CAVALLARO]

On 2/15/2017 8:39 AM, Corentin Labbe wrote:

Since all patch in v2 already hit linux-next, I just want to be sure, I can add 
"Acked-by and Reviewed-by" to thoses 8 new patchs ?


Right I have just seen the V2 patches are in net-next.

pls consider, for this set, my

Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


Regards
Peppe



Regards



Corentin Labbe (8):
  net: stmmac: remove useless parenthesis
  net: stmmac: likely is useless in occasional function
  net: stmmac: use SPEED_UNKNOWN/DUPLEX_UNKNOWN
  net: stmmac: set speed at SPEED_UNKNOWN in case of broken speed
  net: stmmac: run stmmac_hw_fix_mac_speed when speed is valid
  net: stmmac: split the stmmac_adjust_link 10/100 case
  net: stmmac: reduce indentation by adding a continue
  net: stmmac: invert the logic for dumping regs

 .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c   | 18 ++---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 40 ++-
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c  | 82 +++---
 3 files changed, 71 insertions(+), 69 deletions(-)

Re: [PATCH 0/8] net: stmmac: misc patchs

[Giuseppe CAVALLARO]

Hello Corentin

On 2/14/2017 8:54 PM, Corentin Labbe wrote:

Hello

This is a follow up of my previous stmmac serie which address some comment
done in v2.


I wonder if you can resend all the patches as V3
please add the Acked-by and Reviewed-by. This will help
on final review and integration.

Thx for your support and effort

Peppe



Corentin Labbe (8):
  net: stmmac: remove useless parenthesis
  net: stmmac: likely is useless in occasional function
  net: stmmac: use SPEED_UNKNOWN/DUPLEX_UNKNOWN
  net: stmmac: set speed at SPEED_UNKNOWN in case of broken speed
  net: stmmac: run stmmac_hw_fix_mac_speed when speed is valid
  net: stmmac: split the stmmac_adjust_link 10/100 case
  net: stmmac: reduce indentation by adding a continue
  net: stmmac: invert the logic for dumping regs

 .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c   | 18 ++---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 40 ++-
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c  | 82 +++---
 3 files changed, 71 insertions(+), 69 deletions(-)

Re: Performance in stmmac

[Giuseppe CAVALLARO]

Hi Joao

On 2/7/2017 8:33 PM, Joao Pinto wrote:

I am seeing tx being performed in all queues, but rx is just being routed to
queue 0. The reason why only queue 0 is processing the packets is because I am
using priority tagged routing for now and iperf is not capable of producing
priority tagged traffic.


I wonder if you tried the multi-queue channels & AV/B stack.

Regards
Peppe

Re: [PATCH v2 04/16] net: stmmac: remove freesoftware address

[Giuseppe CAVALLARO]

On 2/8/2017 9:31 AM, Corentin Labbe wrote:

This patch fix the checkpatch warning about free software address.

Signed-off-by: Corentin Labbe <clabbe.montj...@gmail.com>


just wonder if this should be sent separately for net-next instead of

Anyway...
Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


---
 drivers/net/ethernet/stmicro/stmmac/chain_mode.c  | 4 
 drivers/net/ethernet/stmicro/stmmac/common.h  | 4 
 drivers/net/ethernet/stmicro/stmmac/descs.h   | 4 
 drivers/net/ethernet/stmicro/stmmac/descs_com.h   | 4 
 drivers/net/ethernet/stmicro/stmmac/dwmac100.h| 4 
 drivers/net/ethernet/stmicro/stmmac/dwmac1000.h   | 4 
 drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c  | 4 
 drivers/net/ethernet/stmicro/stmmac/dwmac1000_dma.c   | 4 
 drivers/net/ethernet/stmicro/stmmac/dwmac100_core.c   | 4 
 drivers/net/ethernet/stmicro/stmmac/dwmac100_dma.c| 4 
 drivers/net/ethernet/stmicro/stmmac/dwmac_dma.h   | 4 
 drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c   | 4 
 drivers/net/ethernet/stmicro/stmmac/enh_desc.c| 4 
 drivers/net/ethernet/stmicro/stmmac/mmc.h | 4 
 drivers/net/ethernet/stmicro/stmmac/mmc_core.c| 4 
 drivers/net/ethernet/stmicro/stmmac/norm_desc.c   | 4 
 drivers/net/ethernet/stmicro/stmmac/ring_mode.c   | 4 
 drivers/net/ethernet/stmicro/stmmac/stmmac.h  | 4 
 drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c  | 4 
 drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c | 4 
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 4 
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c  | 4 
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 4 
 drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c  | 4 
 drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h  | 4 
 26 files changed, 104 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/chain_mode.c 
b/drivers/net/ethernet/stmicro/stmmac/chain_mode.c
index 026e8e9..01a8c02 100644
--- a/drivers/net/ethernet/stmicro/stmmac/chain_mode.c
+++ b/drivers/net/ethernet/stmicro/stmmac/chain_mode.c
@@ -16,10 +16,6 @@
   FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
   more details.

-  You should have received a copy of the GNU General Public License along with
-  this program; if not, write to the Free Software Foundation, Inc.,
-  51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
-
   The full GNU General Public License is included in this distribution in
   the file called "COPYING".

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h 
b/drivers/net/ethernet/stmicro/stmmac/common.h
index 262a1c4..24929bf 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -12,10 +12,6 @@
   FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
   more details.

-  You should have received a copy of the GNU General Public License along with
-  this program; if not, write to the Free Software Foundation, Inc.,
-  51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
-
   The full GNU General Public License is included in this distribution in
   the file called "COPYING".

diff --git a/drivers/net/ethernet/stmicro/stmmac/descs.h 
b/drivers/net/ethernet/stmicro/stmmac/descs.h
index faeeef7..0c2432b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/descs.h
+++ b/drivers/net/ethernet/stmicro/stmmac/descs.h
@@ -11,10 +11,6 @@
   FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
   more details.

-  You should have received a copy of the GNU General Public License along with
-  this program; if not, write to the Free Software Foundation, Inc.,
-  51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
-
   The full GNU General Public License is included in this distribution in
   the file called "COPYING".

diff --git a/drivers/net/ethernet/stmicro/stmmac/descs_com.h 
b/drivers/net/ethernet/stmicro/stmmac/descs_com.h
index 1d181e2..ca9d7e4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/descs_com.h
+++ b/drivers/net/ethernet/stmicro/stmmac/descs_com.h
@@ -17,10 +17,6 @@
   FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
   more details.

-  You should have received a copy of the GNU General Public License along with
-  this program; if not, write to the Free Software Foundation, Inc.,
-  51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
-
   The full GNU General Public License is included in this distribution in
   the file called "COPYING".

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac100.h 
b/drivers/net/ethernet/stmicro/stmmac/dwmac100.h
index 1657acf..e149848 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac100.h
+++ b/drivers/net/ethernet/stm

Re: [PATCH v2 08/16] net: stmmac: Use readl_poll_timeout

[Giuseppe CAVALLARO]

On 2/8/2017 9:31 AM, Corentin Labbe wrote:

The dwmac_dma_reset function use an open coded of readl_poll_timeout().
Replace the open coded handling with the proper function.

Signed-off-by: Corentin Labbe <clabbe.montj...@gmail.com>

Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


---
 drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
index e4cda39..e60bfca 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
@@ -17,6 +17,7 @@
 
***/

 #include 
+#include 
 #include "common.h"
 #include "dwmac_dma.h"

@@ -25,19 +26,16 @@
 int dwmac_dma_reset(void __iomem *ioaddr)
 {
u32 value = readl(ioaddr + DMA_BUS_MODE);
-   int limit;
+   int err;

/* DMA SW reset */
value |= DMA_BUS_MODE_SFT_RESET;
writel(value, ioaddr + DMA_BUS_MODE);
-   limit = 10;
-   while (limit--) {
-   if (!(readl(ioaddr + DMA_BUS_MODE) & DMA_BUS_MODE_SFT_RESET))
-   break;
-   mdelay(10);
-   }

-   if (limit < 0)
+   err = readl_poll_timeout(ioaddr + DMA_BUS_MODE, value,
+!(value & DMA_BUS_MODE_SFT_RESET),
+10, 1);
+   if (err)
return -EBUSY;

return 0;

Re: [PATCH v2 13/16] net: stmmac: print phy information

[Giuseppe CAVALLARO]

On 2/8/2017 9:31 AM, Corentin Labbe wrote:

When a PHY is found, printing which one was found (and which type/model) is
a good information to know.

Signed-off-by: Corentin Labbe <clabbe.montj...@gmail.com>


Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index cc88bdb..9805aa8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -870,9 +870,7 @@ static int stmmac_init_phy(struct net_device *dev)
if (phydev->is_pseudo_fixed_link)
phydev->irq = PHY_POLL;

-   netdev_dbg(priv->dev, "%s: attached to PHY (UID 0x%x) Link = %d\n",
-  __func__, phydev->phy_id, phydev->link);
-
+   phy_attached_info(phydev);
return 0;
 }

Re: [PATCH v2 10/16] net: stmmac: Correct the error message about invalid speed

[Giuseppe CAVALLARO]

On 2/8/2017 9:31 AM, Corentin Labbe wrote:

The message about invalid speed does not state 1000 as a valid speed.
It is much simpler to said that the speed is invalid.

Signed-off-by: Corentin Labbe <clabbe.montj...@gmail.com>


Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index a13fcc4..ed81375 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -738,8 +738,7 @@ static void stmmac_adjust_link(struct net_device *dev)
break;
default:
netif_warn(priv, link, priv->dev,
-  "Speed (%d) not 10/100\n",
-  phydev->speed);
+  "broken speed: %d\n", phydev->speed);
break;
}

Re: [PATCH v2 02/16] net: stmmac: Remove the bus_setup function pointer

[Giuseppe CAVALLARO]

On 2/8/2017 9:31 AM, Corentin Labbe wrote:

The bus_setup function pointer is not used at all, this patch remove it.


no longer used...



Signed-off-by: Corentin Labbe <clabbe.montj...@gmail.com>


Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 
 include/linux/stmmac.h| 1 -
 2 files changed, 5 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index bd83bf9..1ef60282 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1682,10 +1682,6 @@ static int stmmac_hw_setup(struct net_device *dev, bool 
init_ptp)
/* Copy the MAC addr into the HW  */
priv->hw->mac->set_umac_addr(priv->hw, dev->dev_addr, 0);

-   /* If required, perform hw setup of the bus. */
-   if (priv->plat->bus_setup)
-   priv->plat->bus_setup(priv->ioaddr);
-
/* PS and related bits will be programmed according to the speed */
if (priv->hw->pcs) {
int speed = priv->plat->mac_port_sel_speed;
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index d76033d6..fc273e9 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -134,7 +134,6 @@ struct plat_stmmacenet_data {
int tx_fifo_size;
int rx_fifo_size;
void (*fix_mac_speed)(void *priv, unsigned int speed);
-   void (*bus_setup)(void __iomem *ioaddr);
int (*init)(struct platform_device *pdev, void *priv);
void (*exit)(struct platform_device *pdev, void *priv);
void *bsp_priv;

Re: stmmac: GMAC_RGSMIIIS reports bogus values

[Giuseppe CAVALLARO]

Hello Alexey

On 1/31/2017 2:24 PM, Alexey Brodkin wrote:

Hi Giuseppe,

On Tue, 2017-01-31 at 10:55 +0100, Giuseppe CAVALLARO wrote:

On 1/27/2017 11:23 AM, Alexey Brodkin wrote:


That's why my initial proposal was to ignore whatever we read from this register
if we have MDIO bus instantiated already.


sorry for my late reply, I agree with this approach, according to the
HW and platform configuration the driver has to understand and then
work to use either SMA or PCS module.


I already submitted another solution which IMHO is much cleaner and appropriate,
see David has it already in master tree here:
http://git.kernel.org/cgit/linux/kernel/git/davem/net.git/commit/?id=0a764db103376cf69d04449b10688f3516cc0b88



it's ok, thx for that

Peppe


-Alexey

Re: [PATCH 07/17] net: stmmac: replace stmmac_mdio_busy_wait by readl_poll_timeout

[Giuseppe CAVALLARO]

On 1/31/2017 10:11 AM, Corentin Labbe wrote:

The stmmac_mdio_busy_wait() function do the same job than
readl_poll_timeout().
So is is better to replace it.

Signed-off-by: Corentin Labbe <clabbe.montj...@gmail.com>


I just wonder if you also tested it, this impacts all the platforms
where SMA block is used

if yes, pls consider my:

Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


---
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 33 ---
 1 file changed, 11 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index c24bef2..d9893cf 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -21,6 +21,7 @@
 
***/

 #include 
+#include 
 #include 
 #include 
 #include 
@@ -38,22 +39,6 @@
 #define MII_GMAC4_WRITE(1 << MII_GMAC4_GOC_SHIFT)
 #define MII_GMAC4_READ (3 << MII_GMAC4_GOC_SHIFT)

-static int stmmac_mdio_busy_wait(void __iomem *ioaddr, unsigned int mii_addr)
-{
-   unsigned long curr;
-   unsigned long finish = jiffies + 3 * HZ;
-
-   do {
-   curr = jiffies;
-   if (readl(ioaddr + mii_addr) & MII_BUSY)
-   cpu_relax();
-   else
-   return 0;
-   } while (!time_after_eq(curr, finish));
-
-   return -EBUSY;
-}
-
 /**
  * stmmac_mdio_read
  * @bus: points to the mii_bus structure
@@ -70,7 +55,7 @@ static int stmmac_mdio_read(struct mii_bus *bus, int phyaddr, 
int phyreg)
struct stmmac_priv *priv = netdev_priv(ndev);
unsigned int mii_address = priv->hw->mii.addr;
unsigned int mii_data = priv->hw->mii.data;
-
+   u32 v;
int data;
u32 value = MII_BUSY;

@@ -82,12 +67,14 @@ static int stmmac_mdio_read(struct mii_bus *bus, int 
phyaddr, int phyreg)
if (priv->plat->has_gmac4)
value |= MII_GMAC4_READ;

-   if (stmmac_mdio_busy_wait(priv->ioaddr, mii_address))
+   if (readl_poll_timeout(priv->ioaddr + mii_address, v, !(v & MII_BUSY),
+  100, 1))
return -EBUSY;

writel(value, priv->ioaddr + mii_address);

-   if (stmmac_mdio_busy_wait(priv->ioaddr, mii_address))
+   if (readl_poll_timeout(priv->ioaddr + mii_address, v, !(v & MII_BUSY),
+  100, 1))
return -EBUSY;

/* Read the data from the MII data register */
@@ -111,7 +98,7 @@ static int stmmac_mdio_write(struct mii_bus *bus, int 
phyaddr, int phyreg,
struct stmmac_priv *priv = netdev_priv(ndev);
unsigned int mii_address = priv->hw->mii.addr;
unsigned int mii_data = priv->hw->mii.data;
-
+   u32 v;
u32 value = MII_BUSY;

value |= (phyaddr << priv->hw->mii.addr_shift)
@@ -126,7 +113,8 @@ static int stmmac_mdio_write(struct mii_bus *bus, int 
phyaddr, int phyreg,
value |= MII_WRITE;

/* Wait until any existing MII operation is complete */
-   if (stmmac_mdio_busy_wait(priv->ioaddr, mii_address))
+   if (readl_poll_timeout(priv->ioaddr + mii_address, v, !(v & MII_BUSY),
+  100, 1))
return -EBUSY;

/* Set the MII address register to write */
@@ -134,7 +122,8 @@ static int stmmac_mdio_write(struct mii_bus *bus, int 
phyaddr, int phyreg,
writel(value, priv->ioaddr + mii_address);

/* Wait until any existing MII operation is complete */
-   return stmmac_mdio_busy_wait(priv->ioaddr, mii_address);
+   return readl_poll_timeout(priv->ioaddr + mii_address, v, !(v & 
MII_BUSY),
+ 100, 1);
 }

 /**

Re: [PATCH 03/17] net: stmmac: fix some typos in comments

[Giuseppe CAVALLARO]

On 1/31/2017 10:11 AM, Corentin Labbe wrote:

This patch fix some typos in comments.

Signed-off-by: Corentin Labbe <clabbe.montj...@gmail.com>


Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


---
 drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c   |  6 +++---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 16 
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c |  2 +-
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
index a414bde..3b1570d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
@@ -102,7 +102,7 @@ static void show_tx_process_state(unsigned int status)
pr_debug("- TX (Stopped): Reset or Stop command\n");
break;
case 1:
-   pr_debug("- TX (Running):Fetching the Tx desc\n");
+   pr_debug("- TX (Running): Fetching the Tx desc\n");
break;
case 2:
pr_debug("- TX (Running): Waiting for end of tx\n");
@@ -136,7 +136,7 @@ static void show_rx_process_state(unsigned int status)
pr_debug("- RX (Running): Fetching the Rx desc\n");
break;
case 2:
-   pr_debug("- RX (Running):Checking for end of pkt\n");
+   pr_debug("- RX (Running): Checking for end of pkt\n");
break;
case 3:
pr_debug("- RX (Running): Waiting for Rx pkt\n");
@@ -246,7 +246,7 @@ void stmmac_set_mac_addr(void __iomem *ioaddr, u8 addr[6],
unsigned long data;

data = (addr[5] << 8) | addr[4];
-   /* For MAC Addr registers se have to set the Address Enable (AE)
+   /* For MAC Addr registers we have to set the Address Enable (AE)
 * bit that has no effect on the High Reg 0 where the bit 31 (MO)
 * is RO.
 */
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index bf2d8e6..ee71c07 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -216,7 +216,7 @@ static inline u32 stmmac_rx_dirty(struct stmmac_priv *priv)
 /**
  * stmmac_hw_fix_mac_speed - callback for speed selection
  * @priv: driver private structure
- * Description: on some platforms (e.g. ST), some HW system configuraton
+ * Description: on some platforms (e.g. ST), some HW system configuration
  * registers have to be set according to the link speed negotiated.
  */
 static inline void stmmac_hw_fix_mac_speed(struct stmmac_priv *priv)
@@ -415,7 +415,7 @@ static void stmmac_get_rx_hwtstamp(struct stmmac_priv 
*priv, struct dma_desc *p,
 /**
  *  stmmac_hwtstamp_ioctl - control hardware timestamping.
  *  @dev: device pointer.
- *  @ifr: An IOCTL specefic structure, that can contain a pointer to
+ *  @ifr: An IOCTL specific structure, that can contain a pointer to
  *  a proprietary structure used to pass information to the driver.
  *  Description:
  *  This function configures the MAC to enable/disable both outgoing(TX)
@@ -1014,7 +1014,7 @@ static void stmmac_free_rx_buffers(struct stmmac_priv 
*priv, int i)
  * @dev: net device structure
  * @flags: gfp flag.
  * Description: this function initializes the DMA RX/TX descriptors
- * and allocates the socket buffers. It suppors the chained and ring
+ * and allocates the socket buffers. It supports the chained and ring
  * modes.
  */
 static int init_dma_desc_rings(struct net_device *dev, gfp_t flags)
@@ -2515,7 +2515,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit)
if (unlikely(status == discard_frame)) {
priv->dev->stats.rx_errors++;
if (priv->hwts_rx_en && !priv->extend_desc) {
-   /* DESC2 & DESC3 will be overwitten by device
+   /* DESC2 & DESC3 will be overwritten by device
 * with timestamp value, hence reinitialize
 * them in stmmac_rx_refill() function so that
 * device can reuse it.
@@ -2538,7 +2538,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit)

frame_len = priv->hw->desc->get_rx_frame_len(p, coe);

-   /*  If frame length is greather than skb buffer size
+   /*  If frame length is greater than skb buffer size
 *  (preallocated during init) then the packet is
 *  ignored
 */
@@ -2744,7 +2744,7 @@ static netdev_features_t stmmac_fix_features(struct 
net_device *dev,
/* Some GMAC devices have a bugged Jumbo frame support t

Re: [PATCH 07/17] net: stmmac: replace stmmac_mdio_busy_wait by readl_poll_timeout

[Giuseppe CAVALLARO]

On 1/31/2017 11:39 AM, Corentin Labbe wrote:

On Tue, Jan 31, 2017 at 11:13:49AM +0100, Giuseppe CAVALLARO wrote:

On 1/31/2017 10:11 AM, Corentin Labbe wrote:

The stmmac_mdio_busy_wait() function do the same job than
readl_poll_timeout().
So is is better to replace it.

Signed-off-by: Corentin Labbe <clabbe.montj...@gmail.com>


I just wonder if you also tested it, this impacts all the platforms
where SMA block is used


I have tested all patch in this series on my cubieboard2 (dwmac-sunxi) and my 
opipc/pine64/bpim2+ (dwmac-sun8i)
(Yes I could have said that in cover letter).
So this code was tested on two different stmmac glue driver.


perfect :-) thx for the clarification and consider my Acked-by.

peppe



if yes, pls consider my:

Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


---
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 33 ---
 1 file changed, 11 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index c24bef2..d9893cf 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -21,6 +21,7 @@
 
***/

 #include 
+#include 
 #include 
 #include 
 #include 
@@ -38,22 +39,6 @@
 #define MII_GMAC4_WRITE(1 << MII_GMAC4_GOC_SHIFT)
 #define MII_GMAC4_READ (3 << MII_GMAC4_GOC_SHIFT)

-static int stmmac_mdio_busy_wait(void __iomem *ioaddr, unsigned int mii_addr)
-{
-   unsigned long curr;
-   unsigned long finish = jiffies + 3 * HZ;
-
-   do {
-   curr = jiffies;
-   if (readl(ioaddr + mii_addr) & MII_BUSY)
-   cpu_relax();
-   else
-   return 0;
-   } while (!time_after_eq(curr, finish));
-
-   return -EBUSY;
-}
-
 /**
  * stmmac_mdio_read
  * @bus: points to the mii_bus structure
@@ -70,7 +55,7 @@ static int stmmac_mdio_read(struct mii_bus *bus, int phyaddr, 
int phyreg)
struct stmmac_priv *priv = netdev_priv(ndev);
unsigned int mii_address = priv->hw->mii.addr;
unsigned int mii_data = priv->hw->mii.data;
-
+   u32 v;
int data;
u32 value = MII_BUSY;

@@ -82,12 +67,14 @@ static int stmmac_mdio_read(struct mii_bus *bus, int 
phyaddr, int phyreg)
if (priv->plat->has_gmac4)
value |= MII_GMAC4_READ;

-   if (stmmac_mdio_busy_wait(priv->ioaddr, mii_address))
+   if (readl_poll_timeout(priv->ioaddr + mii_address, v, !(v & MII_BUSY),
+  100, 1))
return -EBUSY;

writel(value, priv->ioaddr + mii_address);

-   if (stmmac_mdio_busy_wait(priv->ioaddr, mii_address))
+   if (readl_poll_timeout(priv->ioaddr + mii_address, v, !(v & MII_BUSY),
+  100, 1))
return -EBUSY;

/* Read the data from the MII data register */
@@ -111,7 +98,7 @@ static int stmmac_mdio_write(struct mii_bus *bus, int 
phyaddr, int phyreg,
struct stmmac_priv *priv = netdev_priv(ndev);
unsigned int mii_address = priv->hw->mii.addr;
unsigned int mii_data = priv->hw->mii.data;
-
+   u32 v;
u32 value = MII_BUSY;

value |= (phyaddr << priv->hw->mii.addr_shift)
@@ -126,7 +113,8 @@ static int stmmac_mdio_write(struct mii_bus *bus, int 
phyaddr, int phyreg,
value |= MII_WRITE;

/* Wait until any existing MII operation is complete */
-   if (stmmac_mdio_busy_wait(priv->ioaddr, mii_address))
+   if (readl_poll_timeout(priv->ioaddr + mii_address, v, !(v & MII_BUSY),
+  100, 1))
return -EBUSY;

/* Set the MII address register to write */
@@ -134,7 +122,8 @@ static int stmmac_mdio_write(struct mii_bus *bus, int 
phyaddr, int phyreg,
writel(value, priv->ioaddr + mii_address);

/* Wait until any existing MII operation is complete */
-   return stmmac_mdio_busy_wait(priv->ioaddr, mii_address);
+   return readl_poll_timeout(priv->ioaddr + mii_address, v, !(v & 
MII_BUSY),
+ 100, 1);
 }

 /**

Re: [PATCH 15/17] net: stmmac: remove dead code in stmmac_tx_clean

[Giuseppe CAVALLARO]

On 1/31/2017 10:11 AM, Corentin Labbe wrote:

Since commit cf32deec16e4 ("stmmac: add tx_skbuff_dma to save descriptors used by 
PTP"),
the struct dma_desc *p in stmmac_tx_clean was not used at all.

This patch remove this dead code.

Signed-off-by: Corentin Labbe <clabbe.montj...@gmail.com>


Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 3d52b8c..b494bc2 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1124,13 +1124,6 @@ static void dma_free_tx_skbufs(struct stmmac_priv *priv)
int i;

for (i = 0; i < DMA_TX_SIZE; i++) {
-   struct dma_desc *p;
-
-   if (priv->extend_desc)
-   p = &((priv->dma_etx + i)->basic);
-   else
-   p = priv->dma_tx + i;
-
if (priv->tx_skbuff_dma[i].buf) {
if (priv->tx_skbuff_dma[i].map_as_page)
dma_unmap_page(priv->device,

Re: [PATCH 00/17] net: stmmac: misc fix

[Giuseppe CAVALLARO]

On 1/31/2017 11:23 AM, Joao Pinto wrote:


Hi Peppe,

Às 10:00 AM de 1/31/2017, Giuseppe CAVALLARO escreveu:

Hello Corentin

On 1/31/2017 10:11 AM, Corentin Labbe wrote:

Hello

I am currently working on dwmac-sun8i glue driver for Allwinner H3/A83T/A64.
This serie is the result of all minor problem found in the stmmac driver.


thank for this effort, many changes are to tidy up some part of the code
so  I wonder if these should stay on top of net-next.
There is on-going a thread to change something in the stmmac (where I
missed more details and I tried to refresh all asap); so added on copy
Joao.


I submitted 2 suggestions that would rename folder stmicro to dwc and dwmac4*
files to eqos. The second one is important, since there is no dwmc4/gmac4 core,
in fact it is called eqos that will have 5.00 version very soon. Both
suggestions were rejected and so I turned to developing features.
I am available to discuss with you any improvement on these aspects if you think
it makes sense.

Currently I am developing the support for multi-queues / multi-channels which
will enable in the future stmmac to be used in QAv and TSN applications.


this is a really good point, so I am happy that you are looking for this
supports that can stay on high priority and do not generate conflicts
with this patch series.

We will reopen the renaming story later... I hope to be more reactive
on that and sorry for my absence.

Regards
Peppe



Thanks,
Joao



I will give you my feedback on some patches too.

peppe


Regards

Corentin Labbe (17):
  net: stmmac: fix the typo on MAC_RNABLE_RX
  net: stmmac: Remove the bus_setup function pointer
  net: stmmac: fix some typos in comments
  net: stmmac: remove freesoftware address
  net: stmmac: remplace asm/io.h by linux/io.h
  net: stmmac: fix some code style problem
  net: stmmac: replace stmmac_mdio_busy_wait by readl_poll_timeout
  net: stmmac: Use readl_poll_timeout
  net: stmmac: replace ENOSYS by EINVAL
  net: stmmac: Correct the error message about invalid speed
  net: stmmac: Rewrite two test against NULL value
  net: stmmac: rename rx_crc to rx_crc_errors
  net: stmmac: Implement NAPI for TX
  net: stmmac: print phy information
  net: stmmac: remove dead code in stmmac_tx_clean
  net: stmmac: remove unused variable in sysfs_display_ring
  net: stmmac: replace unsigned by u32

 drivers/net/ethernet/stmicro/stmmac/chain_mode.c   |  4 --
 drivers/net/ethernet/stmicro/stmmac/common.h   |  8 +--
 drivers/net/ethernet/stmicro/stmmac/descs.h|  4 --
 drivers/net/ethernet/stmicro/stmmac/descs_com.h|  4 --
 drivers/net/ethernet/stmicro/stmmac/dwmac100.h |  4 --
 drivers/net/ethernet/stmicro/stmmac/dwmac1000.h|  4 --
 .../net/ethernet/stmicro/stmmac/dwmac1000_core.c   |  4 --
 .../net/ethernet/stmicro/stmmac/dwmac1000_dma.c|  4 --
 .../net/ethernet/stmicro/stmmac/dwmac100_core.c|  4 --
 drivers/net/ethernet/stmicro/stmmac/dwmac100_dma.c |  4 --
 drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c |  2 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac_dma.h|  4 --
 drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c| 28 --
 drivers/net/ethernet/stmicro/stmmac/enh_desc.c |  6 +--
 drivers/net/ethernet/stmicro/stmmac/mmc.h  |  4 --
 drivers/net/ethernet/stmicro/stmmac/mmc_core.c |  4 --
 drivers/net/ethernet/stmicro/stmmac/norm_desc.c|  6 +--
 drivers/net/ethernet/stmicro/stmmac/ring_mode.c|  4 --
 drivers/net/ethernet/stmicro/stmmac/stmmac.h   |  4 --
 .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c   |  6 +--
 .../net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c  |  4 --
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 63 ++
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c  | 52 +++---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c   |  4 --
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |  6 +--
 drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c   |  4 --
 drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h   |  4 --
 include/linux/stmmac.h |  1 -
 28 files changed, 65 insertions(+), 185 deletions(-)

Re: [PATCH 13/17] net: stmmac: Implement NAPI for TX

[Giuseppe CAVALLARO]

On 1/31/2017 10:11 AM, Corentin Labbe wrote:

The stmmac driver run TX completion under NAPI but without checking the
work done by the TX completion function.

This patch add work/budget to the TX completion function.

The visible effect is that it keep the driver longer under NAPI and
boost performance.
Under dwmac-sun8i the iperf goes from 140Mbit/s to 500Mbit/s.
Under dwmac-sunxi an iperf run use half less interrupts.


I think that this patch should be sent separately with more details
about the implementation you are adopting and results.

For example, in the timer callback you force 256 (it seems
DMA_TX_SIZE/2); do you think this should be tunable or fixed to
NAPI budget?

I'd like to understand if performance you get are for TCP traffic;
can you tell me what happens on unidirectional traffic?

Thx a lot for your effort, pls let me know

Regards
peppe



Signed-off-by: Corentin Labbe 
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 2df36bd..e53b727 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1299,10 +1299,11 @@ static void stmmac_dma_operation_mode(struct 
stmmac_priv *priv)
  * @priv: driver private structure
  * Description: it reclaims the transmit resources after transmission 
completes.
  */
-static void stmmac_tx_clean(struct stmmac_priv *priv)
+static int stmmac_tx_clean(struct stmmac_priv *priv, int budget)
 {
unsigned int bytes_compl = 0, pkts_compl = 0;
unsigned int entry = priv->dirty_tx;
+   int work = 0;

netif_tx_lock(priv->dev);

@@ -1369,6 +1370,9 @@ static void stmmac_tx_clean(struct stmmac_priv *priv)
priv->hw->desc->release_tx_desc(p, priv->mode);

entry = STMMAC_GET_ENTRY(entry, DMA_TX_SIZE);
+   work++;
+   if (work >= budget)
+   break;
}
priv->dirty_tx = entry;

@@ -1386,6 +1390,11 @@ static void stmmac_tx_clean(struct stmmac_priv *priv)
mod_timer(>eee_ctrl_timer, STMMAC_LPI_T(eee_timer));
}
netif_tx_unlock(priv->dev);
+
+   if (work < budget)
+   work = 0;
+
+   return work;
 }

 static inline void stmmac_enable_dma_irq(struct stmmac_priv *priv)
@@ -1617,7 +1626,7 @@ static void stmmac_tx_timer(unsigned long data)
 {
struct stmmac_priv *priv = (struct stmmac_priv *)data;

-   stmmac_tx_clean(priv);
+   stmmac_tx_clean(priv, 256);
 }

 /**
@@ -2657,9 +2666,10 @@ static int stmmac_poll(struct napi_struct *napi, int 
budget)
int work_done = 0;

priv->xstats.napi_poll++;
-   stmmac_tx_clean(priv);
+   work_done += stmmac_tx_clean(priv, budget);

-   work_done = stmmac_rx(priv, budget);
+   if (work_done < budget)
+   work_done += stmmac_rx(priv, budget - work_done);
if (work_done < budget) {
napi_complete(napi);
stmmac_enable_dma_irq(priv);

Re: [PATCH 17/17] net: stmmac: replace unsigned by u32

[Giuseppe CAVALLARO]

On 1/31/2017 10:11 AM, Corentin Labbe wrote:

checkpatch complains about two unsigned without type after.
Since the value return is u32, it is simpler to replace it by u32 instead
of "unsigned int"

Signed-off-by: Corentin Labbe <clabbe.montj...@gmail.com>



Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index f0ce780..6260b6f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -187,7 +187,7 @@ static void print_pkt(unsigned char *buf, int len)

 static inline u32 stmmac_tx_avail(struct stmmac_priv *priv)
 {
-   unsigned avail;
+   u32 avail;

if (priv->dirty_tx > priv->cur_tx)
avail = priv->dirty_tx - priv->cur_tx - 1;
@@ -199,7 +199,7 @@ static inline u32 stmmac_tx_avail(struct stmmac_priv *priv)

 static inline u32 stmmac_rx_dirty(struct stmmac_priv *priv)
 {
-   unsigned dirty;
+   u32 dirty;

if (priv->dirty_rx <= priv->cur_rx)
dirty = priv->cur_rx - priv->dirty_rx;

Re: [PATCH 16/17] net: stmmac: remove unused variable in sysfs_display_ring

[Giuseppe CAVALLARO]

On 1/31/2017 10:11 AM, Corentin Labbe wrote:

The u64 x variable in sysfs_display_ring is unused.
This patch remove it.

Signed-off-by: Corentin Labbe <clabbe.montj...@gmail.com>



well spot

Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>



---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index b494bc2..f0ce780 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2890,9 +2890,7 @@ static void sysfs_display_ring(void *head, int size, int 
extend_desc,
struct dma_desc *p = (struct dma_desc *)head;

for (i = 0; i < size; i++) {
-   u64 x;
if (extend_desc) {
-   x = *(u64 *) ep;
seq_printf(seq, "%d [0x%x]: 0x%x 0x%x 0x%x 0x%x\n",
   i, (unsigned int)virt_to_phys(ep),
   le32_to_cpu(ep->basic.des0),
@@ -2901,7 +2899,6 @@ static void sysfs_display_ring(void *head, int size, int 
extend_desc,
   le32_to_cpu(ep->basic.des3));
ep++;
} else {
-   x = *(u64 *) p;
seq_printf(seq, "%d [0x%x]: 0x%x 0x%x 0x%x 0x%x\n",
   i, (unsigned int)virt_to_phys(ep),
   le32_to_cpu(p->des0), le32_to_cpu(p->des1),

Re: [PATCH 14/17] net: stmmac: print phy information

[Giuseppe CAVALLARO]

On 1/31/2017 10:11 AM, Corentin Labbe wrote:

When a PHY is found, printing which one was found (and which type/model) is
a good information to know.

Signed-off-by: Corentin Labbe 
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index e53b727..3d52b8c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -885,6 +885,7 @@ static int stmmac_init_phy(struct net_device *dev)
netdev_dbg(priv->dev, "%s: attached to PHY (UID 0x%x) Link = %d\n",
   __func__, phydev->phy_id, phydev->link);

+   phy_attached_info(phydev);


maybe we could remove the netdev_dbg above and just keep
phy_attached_info(phydev);

peppe


return 0;
 }

Re: [PATCH 12/17] net: stmmac: rename rx_crc to rx_crc_errors

[Giuseppe CAVALLARO]

On 1/31/2017 10:11 AM, Corentin Labbe wrote:

The ethtool stat counter rx_crc from stmmac is mis-named, the name
seems to speak about the number of RX CRC done, but in fact it is about
errors.

This patch rename it to rx_crc_errors, just like the same ifconfig
counter.

Signed-off-by: Corentin Labbe <clabbe.montj...@gmail.com>


Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


---
 drivers/net/ethernet/stmicro/stmmac/common.h | 2 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c   | 2 +-
 drivers/net/ethernet/stmicro/stmmac/enh_desc.c   | 2 +-
 drivers/net/ethernet/stmicro/stmmac/norm_desc.c  | 2 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h 
b/drivers/net/ethernet/stmicro/stmmac/common.h
index 9da4877..b7ee15a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -67,7 +67,7 @@ struct stmmac_extra_stats {
unsigned long overflow_error;
unsigned long ipc_csum_error;
unsigned long rx_collision;
-   unsigned long rx_crc;
+   unsigned long rx_crc_errors;
unsigned long dribbling_bit;
unsigned long rx_length;
unsigned long rx_mii;
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
index 8816515..843ec69 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
@@ -103,7 +103,7 @@ static int dwmac4_wrback_get_rx_status(void *data, struct 
stmmac_extra_stats *x,
x->rx_mii++;

if (unlikely(rdes3 & RDES3_CRC_ERROR)) {
-   x->rx_crc++;
+   x->rx_crc_errors++;
stats->rx_crc_errors++;
}

diff --git a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c 
b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
index 8427643..323b59e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
@@ -221,7 +221,7 @@ static int enh_desc_get_rx_status(void *data, struct 
stmmac_extra_stats *x,
x->rx_mii++;

if (unlikely(rdes0 & RDES0_CRC_ERROR)) {
-   x->rx_crc++;
+   x->rx_crc_errors++;
stats->rx_crc_errors++;
}
ret = discard_frame;
diff --git a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c 
b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
index 5a0d4b0..efb818e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
@@ -111,7 +111,7 @@ static int ndesc_get_rx_status(void *data, struct 
stmmac_extra_stats *x,
stats->collisions++;
}
if (unlikely(rdes0 & RDES0_CRC_ERROR)) {
-   x->rx_crc++;
+   x->rx_crc_errors++;
stats->rx_crc_errors++;
}
ret = discard_frame;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
index 69db8cb..6ca0a10 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
@@ -61,7 +61,7 @@ static const struct stmmac_stats stmmac_gstrings_stats[] = {
STMMAC_STAT(overflow_error),
STMMAC_STAT(ipc_csum_error),
STMMAC_STAT(rx_collision),
-   STMMAC_STAT(rx_crc),
+   STMMAC_STAT(rx_crc_errors),
STMMAC_STAT(dribbling_bit),
STMMAC_STAT(rx_length),
STMMAC_STAT(rx_mii),

Re: [PATCH 10/17] net: stmmac: Correct the error message about invalid speed

[Giuseppe CAVALLARO]

On 1/31/2017 10:11 AM, Corentin Labbe wrote:

Add 1000 as a valid speed in the error message about invalid speed
in stmmac_adjust_link()

Signed-off-by: Corentin Labbe 
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 98f544e..b0154d5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -749,7 +749,7 @@ static void stmmac_adjust_link(struct net_device *dev)
break;
default:
netif_warn(priv, link, priv->dev,
-  "Speed (%d) not 10/100\n",
+  "Speed (%d) not 10/100/1000\n",


maybe, it could be "broken speed: %d"

peppe


   phydev->speed);
break;
}

Re: [PATCH 11/17] net: stmmac: Rewrite two test against NULL value

[Giuseppe CAVALLARO]

On 1/31/2017 10:11 AM, Corentin Labbe wrote:

This patch rewrite two test against NULL value with correct style.

Signed-off-by: Corentin Labbe <clabbe.montj...@gmail.com>


Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index b0154d5..2df36bd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -700,7 +700,7 @@ static void stmmac_adjust_link(struct net_device *dev)
int new_state = 0;
unsigned int fc = priv->flow_ctrl, pause_time = priv->pause;

-   if (phydev == NULL)
+   if (!phydev)
return;

spin_lock_irqsave(>lock, flags);
@@ -1143,7 +1143,7 @@ static void dma_free_tx_skbufs(struct stmmac_priv *priv)
 DMA_TO_DEVICE);
}

-   if (priv->tx_skbuff[i] != NULL) {
+   if (priv->tx_skbuff[i]) {
dev_kfree_skb_any(priv->tx_skbuff[i]);
priv->tx_skbuff[i] = NULL;
priv->tx_skbuff_dma[i].buf = 0;

Re: [PATCH 06/17] net: stmmac: fix some code style problem

[Giuseppe CAVALLARO]

On 1/31/2017 10:11 AM, Corentin Labbe wrote:

Checkpatch complains about some code style problem on stmmac_mdio.c.
This patch fix them.

Signed-off-by: Corentin Labbe <clabbe.montj...@gmail.com>

Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>

---
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index 3fdc6ec..c24bef2 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -152,9 +152,9 @@ int stmmac_mdio_reset(struct mii_bus *bus)

 #ifdef CONFIG_OF
if (priv->device->of_node) {
-
if (data->reset_gpio < 0) {
struct device_node *np = priv->device->of_node;
+
if (!np)
return 0;

@@ -221,7 +221,7 @@ int stmmac_mdio_register(struct net_device *ndev)
return 0;

new_bus = mdiobus_alloc();
-   if (new_bus == NULL)
+   if (!new_bus)
return -ENOMEM;

if (mdio_bus_data->irqs)
@@ -258,6 +258,7 @@ int stmmac_mdio_register(struct net_device *ndev)
found = 0;
for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
struct phy_device *phydev = mdiobus_get_phy(new_bus, addr);
+
if (phydev) {
int act = 0;
char irq_num[4];
@@ -267,7 +268,7 @@ int stmmac_mdio_register(struct net_device *ndev)
 * If an IRQ was provided to be assigned after
 * the bus probe, do it here.
 */
-   if ((mdio_bus_data->irqs == NULL) &&
+   if ((!mdio_bus_data->irqs) &&
(mdio_bus_data->probed_phy_irq > 0)) {
new_bus->irq[addr] =
mdio_bus_data->probed_phy_irq;

Re: [PATCH 09/17] net: stmmac: replace ENOSYS by EINVAL

[Giuseppe CAVALLARO]

On 1/31/2017 10:11 AM, Corentin Labbe wrote:

As said by checkpatch ENOSYS means 'invalid syscall nr' and nothing
else.
This patch replace ENOSYS by the more appropriate value EINVAL.

Signed-off-by: Corentin Labbe <clabbe.montj...@gmail.com>


Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


---
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index eff6282..485b1cd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -353,7 +353,7 @@ void stmmac_remove_config_dt(struct platform_device *pdev,
 struct plat_stmmacenet_data *
 stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
 {
-   return ERR_PTR(-ENOSYS);
+   return ERR_PTR(-EINVAL);
 }

 void stmmac_remove_config_dt(struct platform_device *pdev,

Re: [PATCH 01/17] net: stmmac: fix the typo on MAC_RNABLE_RX

[Giuseppe CAVALLARO]

On 1/31/2017 10:11 AM, Corentin Labbe wrote:

the define MAC_RNABLE_RX have a typo, rename it to MAC_ENABLE_RX

Signed-off-by: Corentin Labbe <clabbe.montj...@gmail.com>


Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


---
 drivers/net/ethernet/stmicro/stmmac/common.h| 2 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h 
b/drivers/net/ethernet/stmicro/stmmac/common.h
index b13a144..0f90f91 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -340,7 +340,7 @@ struct dma_features {
 /* Common MAC defines */
 #define MAC_CTRL_REG   0x  /* MAC Control */
 #define MAC_ENABLE_TX  0x0008  /* Transmitter Enable */
-#define MAC_RNABLE_RX  0x0004  /* Receiver Enable */
+#define MAC_ENABLE_RX  0x0004  /* Receiver Enable */

 /* Default LPI timers */
 #define STMMAC_DEFAULT_LIT_LS  0x3E8
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
index 84e3e84..a414bde 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c
@@ -261,9 +261,9 @@ void stmmac_set_mac(void __iomem *ioaddr, bool enable)
u32 value = readl(ioaddr + MAC_CTRL_REG);

if (enable)
-   value |= MAC_RNABLE_RX | MAC_ENABLE_TX;
+   value |= MAC_ENABLE_RX | MAC_ENABLE_TX;
else
-   value &= ~(MAC_ENABLE_TX | MAC_RNABLE_RX);
+   value &= ~(MAC_ENABLE_TX | MAC_ENABLE_RX);

writel(value, ioaddr + MAC_CTRL_REG);
 }

Re: [PATCH 02/17] net: stmmac: Remove the bus_setup function pointer

[Giuseppe CAVALLARO]

On 1/31/2017 10:11 AM, Corentin Labbe wrote:

The bus_setup function pointer is not used at all, this patch remove it.


indeed this was used and documented on some previous kernels where some
ST40/SH4 platforms (w/o DT).

It's ok to remove it in the new MAC generation; I do not think that
ST will re-use it on new development.

Peppe



Signed-off-by: Corentin Labbe 
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 
 include/linux/stmmac.h| 1 -
 2 files changed, 5 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index e3f6389..bf2d8e6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1671,10 +1671,6 @@ static int stmmac_hw_setup(struct net_device *dev, bool 
init_ptp)
/* Copy the MAC addr into the HW  */
priv->hw->mac->set_umac_addr(priv->hw, dev->dev_addr, 0);

-   /* If required, perform hw setup of the bus. */
-   if (priv->plat->bus_setup)
-   priv->plat->bus_setup(priv->ioaddr);
-
/* PS and related bits will be programmed according to the speed */
if (priv->hw->pcs) {
int speed = priv->plat->mac_port_sel_speed;
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 266dab9..2d82df9 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -135,7 +135,6 @@ struct plat_stmmacenet_data {
int tx_fifo_size;
int rx_fifo_size;
void (*fix_mac_speed)(void *priv, unsigned int speed);
-   void (*bus_setup)(void __iomem *ioaddr);
int (*init)(struct platform_device *pdev, void *priv);
void (*exit)(struct platform_device *pdev, void *priv);
void *bsp_priv;

Re: [PATCH 00/17] net: stmmac: misc fix

[Giuseppe CAVALLARO]

Hello Corentin

On 1/31/2017 10:11 AM, Corentin Labbe wrote:

Hello

I am currently working on dwmac-sun8i glue driver for Allwinner H3/A83T/A64.
This serie is the result of all minor problem found in the stmmac driver.


thank for this effort, many changes are to tidy up some part of the code
so  I wonder if these should stay on top of net-next.
There is on-going a thread to change something in the stmmac (where I
missed more details and I tried to refresh all asap); so added on copy
Joao.

I will give you my feedback on some patches too.

peppe


Regards

Corentin Labbe (17):
  net: stmmac: fix the typo on MAC_RNABLE_RX
  net: stmmac: Remove the bus_setup function pointer
  net: stmmac: fix some typos in comments
  net: stmmac: remove freesoftware address
  net: stmmac: remplace asm/io.h by linux/io.h
  net: stmmac: fix some code style problem
  net: stmmac: replace stmmac_mdio_busy_wait by readl_poll_timeout
  net: stmmac: Use readl_poll_timeout
  net: stmmac: replace ENOSYS by EINVAL
  net: stmmac: Correct the error message about invalid speed
  net: stmmac: Rewrite two test against NULL value
  net: stmmac: rename rx_crc to rx_crc_errors
  net: stmmac: Implement NAPI for TX
  net: stmmac: print phy information
  net: stmmac: remove dead code in stmmac_tx_clean
  net: stmmac: remove unused variable in sysfs_display_ring
  net: stmmac: replace unsigned by u32

 drivers/net/ethernet/stmicro/stmmac/chain_mode.c   |  4 --
 drivers/net/ethernet/stmicro/stmmac/common.h   |  8 +--
 drivers/net/ethernet/stmicro/stmmac/descs.h|  4 --
 drivers/net/ethernet/stmicro/stmmac/descs_com.h|  4 --
 drivers/net/ethernet/stmicro/stmmac/dwmac100.h |  4 --
 drivers/net/ethernet/stmicro/stmmac/dwmac1000.h|  4 --
 .../net/ethernet/stmicro/stmmac/dwmac1000_core.c   |  4 --
 .../net/ethernet/stmicro/stmmac/dwmac1000_dma.c|  4 --
 .../net/ethernet/stmicro/stmmac/dwmac100_core.c|  4 --
 drivers/net/ethernet/stmicro/stmmac/dwmac100_dma.c |  4 --
 drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c |  2 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac_dma.h|  4 --
 drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c| 28 --
 drivers/net/ethernet/stmicro/stmmac/enh_desc.c |  6 +--
 drivers/net/ethernet/stmicro/stmmac/mmc.h  |  4 --
 drivers/net/ethernet/stmicro/stmmac/mmc_core.c |  4 --
 drivers/net/ethernet/stmicro/stmmac/norm_desc.c|  6 +--
 drivers/net/ethernet/stmicro/stmmac/ring_mode.c|  4 --
 drivers/net/ethernet/stmicro/stmmac/stmmac.h   |  4 --
 .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c   |  6 +--
 .../net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c  |  4 --
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 63 ++
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c  | 52 +++---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c   |  4 --
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |  6 +--
 drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c   |  4 --
 drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h   |  4 --
 include/linux/stmmac.h |  1 -
 28 files changed, 65 insertions(+), 185 deletions(-)

Re: stmmac: GMAC_RGSMIIIS reports bogus values

[Giuseppe CAVALLARO]

On 1/27/2017 11:23 AM, Alexey Brodkin wrote:

That's why my initial proposal was to ignore whatever we read from this register
if we have MDIO bus instantiated already.


sorry for my late reply, I agree with this approach, according to the
HW and platform configuration the driver has to understand and then
work to use either SMA or PCS module.

Regards
Peppe

Re: [PATCH] stmmac: fix memory barriers

[Giuseppe CAVALLARO]

On 12/18/2016 9:38 PM, Pavel Machek wrote:


Fix up memory barriers in stmmac driver. They are meant to protect
against DMA engine, so smp_ variants are certainly wrong, and dma_
variants are preferable.

Signed-off-by: Pavel Machek <pa...@denx.de>


Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>



diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
index a340fc8..8816515 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
@@ -334,7 +334,7 @@ static void dwmac4_rd_prepare_tx_desc(struct dma_desc *p, 
int is_fs, int len,
 * descriptors for the same frame has to be set before, to
 * avoid race condition.
 */
-   wmb();
+   dma_wmb();

p->des3 = cpu_to_le32(tdes3);
 }
@@ -377,7 +377,7 @@ static void dwmac4_rd_prepare_tso_tx_desc(struct dma_desc 
*p, int is_fs,
 * descriptors for the same frame has to be set before, to
 * avoid race condition.
 */
-   wmb();
+   dma_wmb();

p->des3 = cpu_to_le32(tdes3);
 }
diff --git a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c 
b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
index ce97e52..f0d8632 100644
--- a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
@@ -350,7 +350,7 @@ static void enh_desc_prepare_tx_desc(struct dma_desc *p, 
int is_fs, int len,
 * descriptors for the same frame has to be set before, to
 * avoid race condition.
 */
-   wmb();
+   dma_wmb();

p->des0 = cpu_to_le32(tdes0);
 }
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 3e40578..bb40382 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2125,7 +2125,7 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, 
struct net_device *dev)
 * descriptor and then barrier is needed to make sure that
 * all is coherent before granting the DMA engine.
 */
-   smp_wmb();
+   dma_wmb();

if (netif_msg_pktdata(priv)) {
pr_info("%s: curr=%d dirty=%d f=%d, e=%d, f_p=%p, nfrags %d\n",
@@ -2338,7 +2338,7 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, 
struct net_device *dev)
 * descriptor and then barrier is needed to make sure that
 * all is coherent before granting the DMA engine.
 */
-   smp_wmb();
+   dma_wmb();
}

netdev_sent_queue(dev, skb->len);
@@ -2443,14 +2443,14 @@ static inline void stmmac_rx_refill(struct stmmac_priv 
*priv)
netif_dbg(priv, rx_status, priv->dev,
  "refill entry #%d\n", entry);
}
-   wmb();
+   dma_wmb();

if (unlikely(priv->synopsys_id >= DWMAC_CORE_4_00))
priv->hw->desc->init_rx_desc(p, priv->use_riwt, 0, 0);
else
priv->hw->desc->set_rx_owner(p);

-   wmb();
+   dma_wmb();

entry = STMMAC_GET_ENTRY(entry, DMA_RX_SIZE);
}

Re: [PATCH v2 2/2] net: ethernet: stmmac: remove private tx queue lock

[Giuseppe CAVALLARO]

On 12/15/2016 10:45 AM, Pavel Machek wrote:

Giuseppe, is there documentation available for the chip? Driver says

  Documentation available at:
  http://www.stlinux.com

but that page does not work for me...


Hi Pavel, yes the page has been removed but all the relevant and
updated driver doc is inside the kernel sources.

Regards
Peppe

Re: stmmac driver...

[Giuseppe CAVALLARO]

Hello Jie

On 12/14/2016 5:05 AM, Jie Deng wrote:

Hi Peppe,

On 2016/12/12 22:17, Giuseppe CAVALLARO wrote:

Hi David

On 12/7/2016 7:06 PM, David Miller wrote:


Giuseppe and Alexandre,

There are a lot of patches and discussions happening around the stammc
driver lately and both of you are listed as the maintainers.

I really need prompt and conclusive reviews of these patch submissions
from you, and participation in all discussions about the driver.


yes we are trying to do the best.


Otherwise I have only three things I can do: 1) let the patches rot in
patchwork for days 2) trust that the patches are sane and fit your
desires and goals and just apply them or 3) reject them since they
aren't being reviewed properly.


at this stage, I think the best is: (3).

I think the patches David mentioned also included XLGMAC. He sent this email
before I explained QoS and XLGMAC were different IPs. Do you mind we do XLGMAC
development under drivers/net/ethernet/synopsys/ ? I think we don't have
conflict since we will keep QoS development in stmmac.


Great. Many thanks for the clarification :-)

Regards
Peppe





Thanks in advance.


you are welcome


Peppe

Re: Synopsys Ethernet QoS

[Giuseppe CAVALLARO]

Hello Niklas

On 12/13/2016 10:38 AM, Niklas Cassel wrote:

On 12/13/2016 08:22 AM, Giuseppe CAVALLARO wrote:

On 12/12/2016 5:25 PM, Niklas Cassel wrote:



On 12/12/2016 11:19 AM, Joao Pinto wrote:

Hi,

Às 1:44 AM de 12/10/2016, Florian Fainelli escreveu:

Le 12/09/16 à 16:16, Andy Shevchenko a écrit :

On Sat, Dec 10, 2016 at 12:52 AM, Florian Fainelli <f.faine...@gmail.com> wrote:


It's kind of sad that customers of that IP (stmmac, amd-xgbe, sxgbe)
did
actually pioneer the upstreaming effort, but it is good to see people
from Synopsys willing to fix that in the future.

Wait, you would like to tell that we have more than 2 drivers for the
same (okay, same vendor) IP?!
It's better to unify them earlier, than have n+ copies.

Unfortunately that is the case, see this email:

https://www.mail-archive.com/netdev@vger.kernel.org/msg142796.html

dwc_eth_qos and stmmac have some overlap. There seems to be work
underway to unify these two to begin with.


P.S. Though, I don't see how sxgbe got in the list. First glance on
the code doesn't show similarities.

Well samsung/sxgbe looks potentially similar to amd/xgbe, but that's
just my cursory look at the code, it may very well be something entirely
different. The descriptor formats just look suspiciously similar.


Thank you for your inputs! Renaming seems to be a hotspot. I agree that maybe
instead of renaming (breaking retro-compatibility as David and Florian
mentioned), the best is to move stmmac to synopsys/ after merging *qos* and
removing it. As Florian mentioned, git is capable of detecting folder 
restructured.

@Rabin Vincent: Hi Rabin. Since Axis is more familiar with the synopsys/*qos*
driver would it be possible for you to make an initial analysis of what has to
be merged into Stmmac? This way the development would speed-up.


I can answer that question.

I've sent out 12 patches to the stmmac driver
(all patches are included in the current net-next tree),


ok I have seen these patches applied, I had just a minor concern about
the  failure when DMA configuration is missing.
In these years, I have noticed that, for this kind of HW, default DMA
configuration is usually good to have a driver working. AHB, AXI
parameters can be provided to have a best tuning or to fix know issues
on some platforms. So IMO, we should relax the check with a warning.
Please, consider that, the stmmac also supports very old MAC10/100
versions where the DMA configuration was often never passed.



This might be a bit off-topic, but:


yes indeed ;-)


The patch should not affect any existing code.
All glue drivers uses stmmac_probe_config_dt,
which sets a default value if the property is missing or zero.
The PCI glue driver sets the property explicitly.
Hence, all current code should not be affected.
The error check was added as a sanity check, just in case some
new code is added, which bypasses stmmac_probe_config_dt,
lets say support for GMAC4 in the PCI glue driver.



ok





There are some performance problems with the stmmac driver though:

When running iperf3 with 3 streams:
iperf3 -c 192.168.0.90 -P 3 -t 30
iperf3 -c 192.168.0.90 -P 3 -t 30 -R

I get really bad fairness between the streams.


Can you confirm you are using the 4.xxa version?


Yes, 4.10a.
Reading the MAC_Version register gives:
0x4041
Where SNPSVER is bit 7:0, so 0x41.


ok






This doesn't match with Alex's experiments on ARM platforms.



We are also using an ARM platform.
There is no problem with throughput.


ok


The problem is fairness between the streams,
when using for example 3 streams in iperf3.


hmm I let Alex to reply.

AFAIK, other people played with stream tests and
no issue but, if you get problems so we have to
investigate.


Did Alex test this? I could not find Alex mail in the netdev archives,
could you link to it?
The problem goes away when disabling TX IRQ coalescing,
which I guess makes sense, since like David Miller said:

"the driver is using non-highres timers
to implement the TX coalescing.  [...]
1 HZ, which is the lowest granularity of non-highres timers in the
kernel, is variable as well as already too large of a delay for
effective TX coalescing."

We are using CONFIG_HZ=100, not CONFIG_HZ=1000.


1000 was a default on our platforms



So if there is a long time before handling interrupts,
I guess that it makes sense that one stream could
get an advantage in the net scheduler.


ok



If I find the time, and if no one beats me to it, I will try to replace
the normal timers with HR timers + a smaller default timeout.


that's great.




We have a local patch that implements TX IRQ coalescing in the dwceqos driver,
and we don't see the same problem.


please, if you have new patch add me on CC and we will review all
together.


I think you misunderstood me, we have a local patch for the 
synopsys/dwc_eth_qos.c
version, not for stmmac.


oops, sorry



I have been using get_maintainer.pl, so you should have been added to all my 
patches,
but if I 

Re: stmmac DT property snps,axi_all

[Giuseppe CAVALLARO]

On 12/13/2016 9:26 AM, Niklas Cassel wrote:

On 12/13/2016 07:47 AM, Giuseppe CAVALLARO wrote:

Hello Niklas, Alex,

my fault and a step behind... Current code is OK
when manage the AAL that, although it is passed from
the axi structure, it is always used to program,
for all the chip versions, the writable bit inside the
DMA_BUS_MODE register.
So I guess no extra patch is needed.


Hello Giuseppe

I'm not sure what you mean.

Yes, when setting "snps,aal",
the code is correct for all versions of the IP.

However, "snps,axi_all" seems to just be dead code.
Try git grep in the whole tree.
So I still think that my patch is valid.


ok there is the snps,aal, proceed with your patch and
add my Acked-by. Sorry for this traffic but I am trying
to keep myself aligned with all these new patches.

So, IIUC, you have some new patch for TX Timer; pls
do not forget to add me on copy.

Regards
Peppe






Regards
Peppe

On 12/12/2016 3:18 PM, Giuseppe CAVALLARO wrote:

Please Niklas

when you send the patch, add my

Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


On 12/9/2016 10:53 AM, Niklas Cassel wrote:

On 12/09/2016 10:20 AM, Niklas Cassel wrote:

On 12/08/2016 02:36 PM, Alexandre Torgue wrote:

Hi Niklas,

On 12/05/2016 05:18 PM, Niklas Cassel wrote:

Hello Giuseppe


I'm trying to figure out what snps,axi_all is supposed to represent.

It appears that the value is saved, but never used in the code.

Looking at the register specification, I'm guessing that it represents
Address-Aligned Beats, but there is already the property snps,aal
for that.

IMO, it is not useful. Indeed AXI_AAL is a read only bit (in AXI bus
mode register) and reflects the aal bit in DMA bus register.
As you know we use "snps,aal" to set aal bit in DMA bus register.
So "snps,axi_all" entry seems useless. Let's see with Peppe.

Ok, I see. GMAC and GMAC4 is different here.

For GMAC4 AAL only exists in DMA_SYS_BUS_MODE.
It's not reflected anywhere else.

The code is correct in the driver.

If snps,axi_all is just created for a read-only register,
and it is currently never used in the code,
while we have snps,aal, which is correct and works,
I guess it should be ok to remove snps,axi_all.

I can cook up a patch.



Here we go :)

I will send it as a real patch once net-next reopens.



From defc01cb7c22611b89d9cf1fcae72544092bd62c Mon Sep 17 00:00:00 2001

From: Niklas Cassel <niklas.cas...@axis.com>
Date: Fri, 9 Dec 2016 10:27:00 +0100
Subject: [PATCH net-next] net: stmmac: remove unused duplicate property
 snps,axi_all

For core revision 3.x Address-Aligned Beats is available in two
registers.
The DT property snps,aal was created for AAL in the DMA bus register,
which is a read/write bit.
The DT property snps,axi_all was created for AXI_AAL in the AXI bus mode
register, which is a read only bit that reflects the value of AAL in the
DMA bus register.

Since the value of snps,axi_all is never used in the driver,
and since the property was created for a bit that is read only,
it should be safe to remove the property.

Signed-off-by: Niklas Cassel <niklas.cas...@axis.com>
---
 Documentation/devicetree/bindings/net/stmmac.txt  | 1 -
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 1 -
 include/linux/stmmac.h| 1 -
 3 files changed, 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/stmmac.txt
b/Documentation/devicetree/bindings/net/stmmac.txt
index 128da752fec9..c3d2fd480a1b 100644
--- a/Documentation/devicetree/bindings/net/stmmac.txt
+++ b/Documentation/devicetree/bindings/net/stmmac.txt
@@ -65,7 +65,6 @@ Optional properties:
 - snps,wr_osr_lmt: max write outstanding req. limit
 - snps,rd_osr_lmt: max read outstanding req. limit
 - snps,kbbe: do not cross 1KiB boundary.
-- snps,axi_all: align address
 - snps,blen: this is a vector of supported burst length.
 - snps,fb: fixed-burst
 - snps,mb: mixed-burst
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 082cd48db6a7..60ba8993c650 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -121,7 +121,6 @@ static struct stmmac_axi *stmmac_axi_setup(struct
platform_device *pdev)
 axi->axi_lpi_en = of_property_read_bool(np, "snps,lpi_en");
 axi->axi_xit_frm = of_property_read_bool(np, "snps,xit_frm");
 axi->axi_kbbe = of_property_read_bool(np, "snps,axi_kbbe");
-axi->axi_axi_all = of_property_read_bool(np, "snps,axi_all");
 axi->axi_fb = of_property_read_bool(np, "snps,axi_fb");
 axi->axi_mb = of_property_read_bool(np, "snps,axi_mb");
 axi->axi_rb =  of_property_read_bool(np, "snps,axi_rb");
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 266dab9ad782..88

Re: Synopsys Ethernet QoS

[Giuseppe CAVALLARO]

On 12/12/2016 5:25 PM, Niklas Cassel wrote:



On 12/12/2016 11:19 AM, Joao Pinto wrote:

Hi,

Às 1:44 AM de 12/10/2016, Florian Fainelli escreveu:

Le 12/09/16 à 16:16, Andy Shevchenko a écrit :

On Sat, Dec 10, 2016 at 12:52 AM, Florian Fainelli  wrote:


It's kind of sad that customers of that IP (stmmac, amd-xgbe, sxgbe)
did
actually pioneer the upstreaming effort, but it is good to see people
from Synopsys willing to fix that in the future.

Wait, you would like to tell that we have more than 2 drivers for the
same (okay, same vendor) IP?!
It's better to unify them earlier, than have n+ copies.

Unfortunately that is the case, see this email:

https://www.mail-archive.com/netdev@vger.kernel.org/msg142796.html

dwc_eth_qos and stmmac have some overlap. There seems to be work
underway to unify these two to begin with.


P.S. Though, I don't see how sxgbe got in the list. First glance on
the code doesn't show similarities.

Well samsung/sxgbe looks potentially similar to amd/xgbe, but that's
just my cursory look at the code, it may very well be something entirely
different. The descriptor formats just look suspiciously similar.


Thank you for your inputs! Renaming seems to be a hotspot. I agree that maybe
instead of renaming (breaking retro-compatibility as David and Florian
mentioned), the best is to move stmmac to synopsys/ after merging *qos* and
removing it. As Florian mentioned, git is capable of detecting folder 
restructured.

@Rabin Vincent: Hi Rabin. Since Axis is more familiar with the synopsys/*qos*
driver would it be possible for you to make an initial analysis of what has to
be merged into Stmmac? This way the development would speed-up.


I can answer that question.

I've sent out 12 patches to the stmmac driver
(all patches are included in the current net-next tree),


ok I have seen these patches applied, I had just a minor concern about
the  failure when DMA configuration is missing.
In these years, I have noticed that, for this kind of HW, default DMA
configuration is usually good to have a driver working. AHB, AXI
parameters can be provided to have a best tuning or to fix know issues
on some platforms. So IMO, we should relax the check with a warning.
Please, consider that, the stmmac also supports very old MAC10/100
versions where the DMA configuration was often never passed.


with these patches the stmmac driver works properly on Axis hardware
(we use Synopsys GMAC 4.10a synthesized with multiple TX queues).


perfect and thanks a lot for this effort.


stmmac's DT binding has also been extended with properties that
existed in DWC EQoS's DT binding, such as no-pbl-x8, txpbl, rxpbl.

Since we have no problem updating the DTB together with the kernel,
we will simply move to using the start using the stmmac driver,
with stmmac's DT binding.

However, I've noticed that NVIDIA has extended the DWC EQoS DT binding,
I don't how easy it would be for them to switch to stmmac's DT binding.
(Adding Stephen Warren to CC.)


ok



The reset sequence that Lars Persson was worried about is not an issue
with the stmmac driver.



thx for this check.



There are some performance problems with the stmmac driver though:

When running iperf3 with 3 streams:
iperf3 -c 192.168.0.90 -P 3 -t 30
iperf3 -c 192.168.0.90 -P 3 -t 30 -R

I get really bad fairness between the streams.


Can you confirm you are using the 4.xxa version?

This doesn't match with Alex's experiments on ARM platforms.


This appears to be an issue with how TX IRQ coalescing is implemented in stmmac.
Disabling TX IRQ coalescing in the stmmac driver makes the problem go away.


this doesn't match with what we had seen but I am happy and open to
review and accept new strategy.


We have a local patch that implements TX IRQ coalescing in the dwceqos driver,
and we don't see the same problem.


please, if you have new patch add me on CC and we will review all
together.


Also netperf TCP_RR and UDP_RR gives really bad results compared to the
dwceqos driver (without IRQ coalescing).
2000 transactions/sec vs 9000 transactions/sec.
Turning TX IRQ coalescing off and RX interrupt watchdog off in stmmac
gives the same performance. I guess it's a trade off, low CPU usage
vs low latency, so I don't know how important TCP_RR/UDP_RR really is.

The best thing would be to get a good working TX IRQ coalesce
implementation with HR timers in stmmac.


as said, welcome patches.

Basically, the default tuning of coalescence parameters comes from
ST platform experiences. I mean, we tuned to driver to have good
performance and saving CPU on SH4 (UP) and ARM (SMP) systems.
In these years, these default was accepted but, if today we need
to change something welcome effort. On my side, I can try to
perform some bench to see if I have regressions on not.


Perhaps it should also be investigated if the RX interrupt watchdog
timeout should have a lower default value.


Do not expect to get many improvements to play with the HW watchdog

Re: stmmac DT property snps,axi_all

[Giuseppe CAVALLARO]

Hello Niklas, Alex,

my fault and a step behind... Current code is OK
when manage the AAL that, although it is passed from
the axi structure, it is always used to program,
for all the chip versions, the writable bit inside the
DMA_BUS_MODE register.
So I guess no extra patch is needed.

Regards
Peppe

On 12/12/2016 3:18 PM, Giuseppe CAVALLARO wrote:

Please Niklas

when you send the patch, add my

Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


On 12/9/2016 10:53 AM, Niklas Cassel wrote:

On 12/09/2016 10:20 AM, Niklas Cassel wrote:

On 12/08/2016 02:36 PM, Alexandre Torgue wrote:

Hi Niklas,

On 12/05/2016 05:18 PM, Niklas Cassel wrote:

Hello Giuseppe


I'm trying to figure out what snps,axi_all is supposed to represent.

It appears that the value is saved, but never used in the code.

Looking at the register specification, I'm guessing that it represents
Address-Aligned Beats, but there is already the property snps,aal
for that.

IMO, it is not useful. Indeed AXI_AAL is a read only bit (in AXI bus
mode register) and reflects the aal bit in DMA bus register.
As you know we use "snps,aal" to set aal bit in DMA bus register.
So "snps,axi_all" entry seems useless. Let's see with Peppe.

Ok, I see. GMAC and GMAC4 is different here.

For GMAC4 AAL only exists in DMA_SYS_BUS_MODE.
It's not reflected anywhere else.

The code is correct in the driver.

If snps,axi_all is just created for a read-only register,
and it is currently never used in the code,
while we have snps,aal, which is correct and works,
I guess it should be ok to remove snps,axi_all.

I can cook up a patch.



Here we go :)

I will send it as a real patch once net-next reopens.



From defc01cb7c22611b89d9cf1fcae72544092bd62c Mon Sep 17 00:00:00 2001

From: Niklas Cassel <niklas.cas...@axis.com>
Date: Fri, 9 Dec 2016 10:27:00 +0100
Subject: [PATCH net-next] net: stmmac: remove unused duplicate property
 snps,axi_all

For core revision 3.x Address-Aligned Beats is available in two
registers.
The DT property snps,aal was created for AAL in the DMA bus register,
which is a read/write bit.
The DT property snps,axi_all was created for AXI_AAL in the AXI bus mode
register, which is a read only bit that reflects the value of AAL in the
DMA bus register.

Since the value of snps,axi_all is never used in the driver,
and since the property was created for a bit that is read only,
it should be safe to remove the property.

Signed-off-by: Niklas Cassel <niklas.cas...@axis.com>
---
 Documentation/devicetree/bindings/net/stmmac.txt  | 1 -
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 1 -
 include/linux/stmmac.h| 1 -
 3 files changed, 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/stmmac.txt
b/Documentation/devicetree/bindings/net/stmmac.txt
index 128da752fec9..c3d2fd480a1b 100644
--- a/Documentation/devicetree/bindings/net/stmmac.txt
+++ b/Documentation/devicetree/bindings/net/stmmac.txt
@@ -65,7 +65,6 @@ Optional properties:
 - snps,wr_osr_lmt: max write outstanding req. limit
 - snps,rd_osr_lmt: max read outstanding req. limit
 - snps,kbbe: do not cross 1KiB boundary.
-- snps,axi_all: align address
 - snps,blen: this is a vector of supported burst length.
 - snps,fb: fixed-burst
 - snps,mb: mixed-burst
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 082cd48db6a7..60ba8993c650 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -121,7 +121,6 @@ static struct stmmac_axi *stmmac_axi_setup(struct
platform_device *pdev)
 axi->axi_lpi_en = of_property_read_bool(np, "snps,lpi_en");
 axi->axi_xit_frm = of_property_read_bool(np, "snps,xit_frm");
 axi->axi_kbbe = of_property_read_bool(np, "snps,axi_kbbe");
-axi->axi_axi_all = of_property_read_bool(np, "snps,axi_all");
 axi->axi_fb = of_property_read_bool(np, "snps,axi_fb");
 axi->axi_mb = of_property_read_bool(np, "snps,axi_mb");
 axi->axi_rb =  of_property_read_bool(np, "snps,axi_rb");
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 266dab9ad782..889e0e9a3f1c 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -103,7 +103,6 @@ struct stmmac_axi {
 u32 axi_wr_osr_lmt;
 u32 axi_rd_osr_lmt;
 bool axi_kbbe;
-bool axi_axi_all;
 u32 axi_blen[AXI_BLEN];
 bool axi_fb;
 bool axi_mb;

Re: stmmac DT property snps,axi_all

[Giuseppe CAVALLARO]

Please Niklas

when you send the patch, add my

Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


On 12/9/2016 10:53 AM, Niklas Cassel wrote:

On 12/09/2016 10:20 AM, Niklas Cassel wrote:

On 12/08/2016 02:36 PM, Alexandre Torgue wrote:

Hi Niklas,

On 12/05/2016 05:18 PM, Niklas Cassel wrote:

Hello Giuseppe


I'm trying to figure out what snps,axi_all is supposed to represent.

It appears that the value is saved, but never used in the code.

Looking at the register specification, I'm guessing that it represents
Address-Aligned Beats, but there is already the property snps,aal
for that.

IMO, it is not useful. Indeed AXI_AAL is a read only bit (in AXI bus mode 
register) and reflects the aal bit in DMA bus register.
As you know we use "snps,aal" to set aal bit in DMA bus register.
So "snps,axi_all" entry seems useless. Let's see with Peppe.

Ok, I see. GMAC and GMAC4 is different here.

For GMAC4 AAL only exists in DMA_SYS_BUS_MODE.
It's not reflected anywhere else.

The code is correct in the driver.

If snps,axi_all is just created for a read-only register,
and it is currently never used in the code,
while we have snps,aal, which is correct and works,
I guess it should be ok to remove snps,axi_all.

I can cook up a patch.



Here we go :)

I will send it as a real patch once net-next reopens.



From defc01cb7c22611b89d9cf1fcae72544092bd62c Mon Sep 17 00:00:00 2001

From: Niklas Cassel <niklas.cas...@axis.com>
Date: Fri, 9 Dec 2016 10:27:00 +0100
Subject: [PATCH net-next] net: stmmac: remove unused duplicate property
 snps,axi_all

For core revision 3.x Address-Aligned Beats is available in two registers.
The DT property snps,aal was created for AAL in the DMA bus register,
which is a read/write bit.
The DT property snps,axi_all was created for AXI_AAL in the AXI bus mode
register, which is a read only bit that reflects the value of AAL in the
DMA bus register.

Since the value of snps,axi_all is never used in the driver,
and since the property was created for a bit that is read only,
it should be safe to remove the property.

Signed-off-by: Niklas Cassel <niklas.cas...@axis.com>
---
 Documentation/devicetree/bindings/net/stmmac.txt  | 1 -
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 1 -
 include/linux/stmmac.h| 1 -
 3 files changed, 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/stmmac.txt 
b/Documentation/devicetree/bindings/net/stmmac.txt
index 128da752fec9..c3d2fd480a1b 100644
--- a/Documentation/devicetree/bindings/net/stmmac.txt
+++ b/Documentation/devicetree/bindings/net/stmmac.txt
@@ -65,7 +65,6 @@ Optional properties:
 - snps,wr_osr_lmt: max write outstanding req. limit
 - snps,rd_osr_lmt: max read outstanding req. limit
 - snps,kbbe: do not cross 1KiB boundary.
-- snps,axi_all: align address
 - snps,blen: this is a vector of supported burst length.
 - snps,fb: fixed-burst
 - snps,mb: mixed-burst
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 082cd48db6a7..60ba8993c650 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -121,7 +121,6 @@ static struct stmmac_axi *stmmac_axi_setup(struct 
platform_device *pdev)
 axi->axi_lpi_en = of_property_read_bool(np, "snps,lpi_en");
 axi->axi_xit_frm = of_property_read_bool(np, "snps,xit_frm");
 axi->axi_kbbe = of_property_read_bool(np, "snps,axi_kbbe");
-axi->axi_axi_all = of_property_read_bool(np, "snps,axi_all");
 axi->axi_fb = of_property_read_bool(np, "snps,axi_fb");
 axi->axi_mb = of_property_read_bool(np, "snps,axi_mb");
 axi->axi_rb =  of_property_read_bool(np, "snps,axi_rb");
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 266dab9ad782..889e0e9a3f1c 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -103,7 +103,6 @@ struct stmmac_axi {
 u32 axi_wr_osr_lmt;
 u32 axi_rd_osr_lmt;
 bool axi_kbbe;
-bool axi_axi_all;
 u32 axi_blen[AXI_BLEN];
 bool axi_fb;
 bool axi_mb;

Re: stmmac driver...

[Giuseppe CAVALLARO]

Hi David

On 12/7/2016 7:06 PM, David Miller wrote:


Giuseppe and Alexandre,

There are a lot of patches and discussions happening around the stammc
driver lately and both of you are listed as the maintainers.

I really need prompt and conclusive reviews of these patch submissions
from you, and participation in all discussions about the driver.


yes we are trying to do the best.


Otherwise I have only three things I can do: 1) let the patches rot in
patchwork for days 2) trust that the patches are sane and fit your
desires and goals and just apply them or 3) reject them since they
aren't being reviewed properly.


at this stage, I think the best is: (3).



Thanks in advance.


you are welcome


Peppe

Re: Synopsys Ethernet QoS

[Giuseppe CAVALLARO]

Hello All.

On 12/12/2016 11:19 AM, Joao Pinto wrote:

Hi,

Às 1:44 AM de 12/10/2016, Florian Fainelli escreveu:

Le 12/09/16 à 16:16, Andy Shevchenko a écrit :

On Sat, Dec 10, 2016 at 12:52 AM, Florian Fainelli  wrote:


It's kind of sad that customers of that IP (stmmac, amd-xgbe, sxgbe)



did
actually pioneer the upstreaming effort, but it is good to see people
from Synopsys willing to fix that in the future.


Wait, you would like to tell that we have more than 2 drivers for the
same (okay, same vendor) IP?!
It's better to unify them earlier, than have n+ copies.


Unfortunately that is the case, see this email:

https://www.mail-archive.com/netdev@vger.kernel.org/msg142796.html

dwc_eth_qos and stmmac have some overlap. There seems to be work
underway to unify these two to begin with.



P.S. Though, I don't see how sxgbe got in the list. First glance on
the code doesn't show similarities.


Well samsung/sxgbe looks potentially similar to amd/xgbe, but that's
just my cursory look at the code, it may very well be something entirely
different. The descriptor formats just look suspiciously similar.



Thank you for your inputs! Renaming seems to be a hotspot. I agree that maybe
instead of renaming (breaking retro-compatibility as David and Florian
mentioned), the best is to move stmmac to synopsys/ after merging *qos* and
removing it. As Florian mentioned, git is capable of detecting folder 
restructured.

@Rabin Vincent: Hi Rabin. Since Axis is more familiar with the synopsys/*qos*
driver would it be possible for you to make an initial analysis of what has to
be merged into Stmmac? This way the development would speed-up.


my priority on this topic was to help Joao to easily port his
platform on stmmac and IIUC we already achieved some results.
For several reasons, we had discussed to support GMAC IP with stmmac
and, as first stage, to evaluate and port the relevant changes
from  dwc_eth_qos.c. If renaming of moving the stmmac should be
discussed later and for sure all together could find the best solution
in the right moment where some critical patches and supports are in
place.

As highlighted by Jie, the XLGMAC is another IP (that I do not know
at all so I cannot say if there is some way to support it
by using stmmac).

Thanks a lot.

peppe



Thanks to all.

Joao

Re: stmmac DT property snps,axi_all

[Giuseppe CAVALLARO]

Hello

On 12/9/2016 5:06 PM, Alexandre Torgue wrote:

Hi Niklas

On 12/09/2016 10:53 AM, Niklas Cassel wrote:

On 12/09/2016 10:20 AM, Niklas Cassel wrote:

On 12/08/2016 02:36 PM, Alexandre Torgue wrote:

Hi Niklas,

On 12/05/2016 05:18 PM, Niklas Cassel wrote:

Hello Giuseppe


I'm trying to figure out what snps,axi_all is supposed to represent.

It appears that the value is saved, but never used in the code.

Looking at the register specification, I'm guessing that it represents
Address-Aligned Beats, but there is already the property snps,aal
for that.

IMO, it is not useful. Indeed AXI_AAL is a read only bit (in AXI bus
mode register) and reflects the aal bit in DMA bus register.
As you know we use "snps,aal" to set aal bit in DMA bus register.
So "snps,axi_all" entry seems useless. Let's see with Peppe.

Ok, I see. GMAC and GMAC4 is different here.

For GMAC4 AAL only exists in DMA_SYS_BUS_MODE.
It's not reflected anywhere else.

The code is correct in the driver.

If snps,axi_all is just created for a read-only register,
and it is currently never used in the code,
while we have snps,aal, which is correct and works,
I guess it should be ok to remove snps,axi_all.

I can cook up a patch.



Here we go :)

I will send it as a real patch once net-next reopens.


Thanks ;). Just check with Peppe next week (as he added in the past this
property).


Yes, we can conclude that the axi_all can be safely removed from DT, in
fact on all GMACs the AaL will be set via DMA_BUS_MODE register.

peppe

Re: stmmac ethernet in kernel 4.9-rc6: coalescing related pauses.

[Giuseppe CAVALLARO]

Hi Alex

On 12/2/2016 3:26 PM, Alexandre Torgue wrote:

4.4 has no GMAC4 support.
Alex, do you remember any patches to fix that?


No sorry Peppe.

Pavel,

Sorry but I'm a little bit confused. I'm dropped in some mails without
historic. I see cleanup, coalescence issue and TSO question.
What is your main issue? Are you working on gmac4 or 3.x ?
Can you refresh a little bit the story please ?


let me try to do a sum, please Pavel feel free to correct me.

There are some open points about the tx mitigation schema
that we are trying to detail and eventually tune or change
(but keeping the same performance on other user-case).

In particular, the test case that is raising problem is
an unicast tx bench.
I suggested Pavel to tune coalesce (IC bit settings) via
ethtool and monitor stats but he is getting problems (maybe
due to lock).

IIUC problems are mainly on new kernel and not on 4.4 where
the gmac4 is missing. Please Pavel, could you confirm?

Then, there are some open points about lock protections
for xstat and Pavel is getting some problem on SMP.
I do think that we need to review that. This also could
improve the code in critical parts.

Also there are some other discussion about the lock
protection on NAPI still under discussion. I have not
clear if in this case Pavel is getting strange behavior.

Regards
Peppe

Re: stmmac: turn coalescing / NAPI off in stmmac

[Giuseppe CAVALLARO]

Hi Pavel

On 12/2/2016 11:42 AM, Pavel Machek wrote:

Hi!


Anyway... since you asked. I belive I have way to disable NAPI / tx
coalescing in the driver. Unfortunately, locking is missing on the rx
path, and needs to be extended to _irqsave variant on tx path.


I have just replied to a previous thread about that...


Yeah, please reply to David's mail where he describes why it can't
work.


let me to re-check the mails :-) I can try to provide you
more details about what I experimented




So patch currently looks like this (hand edited, can't be
applied, got it working few hours ago). Does it look acceptable?

I'd prefer this to go after the patch that pulls common code to single
place, so that single place needs to be patched. Plus I guess I should
add ifdefs, so that more advanced NAPI / tx coalescing code can be
reactivated when it is fixed. Trivial fixes can go on top. Does that
sound like a plan?


Hmm, what I find strange is that, just this code is running since a
long time on several platforms and Chip versions. No raise condition
have been found or lock protection problems (also proving look
mechanisms).


Well, it works better for me when I disable CONFIG_SMP. It is normal
that locking problems are hard to reproduce :-(.


can you share me the test, maybe I can try to reproduce on ARM box.
Are you using 3.x or 4.x GMAC?


Pavel, I ask you sorry if I missed some problems so, if you can
(as D. Miller asked) to send us a cover letter + all patches
I will try to reply soon. I can do also some tests if you ask
me that! I could run on 3.x and 4.x but I cannot promise you
benchmarks.


Actually... I have questions here. David normally pulls from you (can
I have a address of your git tree?).


No I send the patches to the mailing list.



Could you apply these to your git?

[PATCH] stmmac ethernet: unify locking
[PATCH] stmmac: simplify flag assignment
[PATCH] stmmac: cleanup documenation, make it match reality

They are rather trivial and independend, I'm not sure what cover
letter would say, besides "simple fixes".

Then I can re-do the reset on top of that...


Which tree do you want patches against?

https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/ ?


I think that bug fixing should be on top of net.git but I let Miller
to decide.


Hmm. It is "only" a performance problem (40msec delays).. I guess
-next is better target.


ok, maybe if you resend these with a cover-letter I can try to
contribute on reviewing (in case of I have missed some detail).

Best Regards
Peppe



Best regards,
Pavel

Re: stmmac ethernet in kernel 4.9-rc6: coalescing related pauses.

[Giuseppe CAVALLARO]

On 12/2/2016 1:32 PM, Pavel Machek wrote:

Hi!


Well, if you have a workload that sends and receive packets, it tends
to work ok, as you do tx_clean() in stmmac_poll(). My workload is not
like that -- it is "sending packets at 3MB/sec, receiving none". So
the stmmac_tx_timer() is rescheduled and rescheduled and rescheduled,
and then we run out of transmit descriptors, and then 40msec passes,
and then we clean them. Bad.

And that's why low-res timers do not cut it.


in that case, I expect that the tuning of the driver could help you.
I mean, by using ethtool, it could be enough to set the IC bit on all
the descriptors. You should touch the tx_coal_frames.

Then you can use ethtool -S to monitor the status.


Yes, I did something similar. Unfortnunately that meant crash within
minutes, at least with 4.4 kernel. (If you know what was fixed between
4.4 and 4.9, that would be helpful).


4.4 has no GMAC4 support.
Alex, do you remember any patches to fix that?


We had experimented this tuning on STB IP where just datagrams
had to send externally. To be honest, although we had seen
better results w/o any timer, we kept this approach enabled
because the timer was fast enough to cover our tests on SH4 boxes.


Please reply to David, and explain how it is supposed to
work... because right now it does not. 40 msec delays are not
acceptable in default configuration.


I mean, that on UP and SMP system this schema helped
to improve the performance saving CPU on my side and this has been
tested since a long time (~4 years).
I tested something similar to yours where unidirectional traffic
with limited throughput was needed and I can confirm you that
tuning/removing coalesce parameters this helped. The tuning I decided
to keep in the driver was suitable in several user cases and if now
you have problems or you want to review it I can just confirm that
there are no problems on my side. If you want to simply the logic
around the tx process and remove timer on official driver I can accept
that. I will just ask you uto double check if the throughput and
CPU usage when request max throughput (better if on GiGa setup) has
no regressions.
Otherwise we could start thinking about adaptive schema if feasible.


In the ring, some descriptors can raise the irq (according to a
threshold) and set the IC bit. In this path, the NAPI  poll will be
scheduled.


Not NAPI poll but stmmac_tx_timer(), right?


in the xmit according the the threshold the timer is started or the
interrupt is set inside the descriptor.
Then stmmac_tx_clean will be always called and, if you see the flow,
no irqlock protection is needed!


Agreed that no irqlock protection is needed if we rely on napi and timers.


ok




Concerning the lock protection, we had reviewed long time ago and
IIRC, no raise condition should be present. Open to review it,
again!

...

There's nothing that protect stmmac_poll() from running concurently
with stmmac_dma_interrupt(), right?


This is not necessary.


dma_interrupt accesses shared priv->xstats; variables are of type
unsigned long (not atomic_t), yet they are accesssed from interrupt
context and from stmmac_ethtool without any locking. That can result
in broken statistics AFAICT.


ok we can check this and welcome patches and I'd prefer to
remove xstats from critical part of the code like ISR (that
comes from old story of the driver).



Please take another look. As far as I can tell, you can have two cpus
at #1 and #2 in the code, at the same time. It looks like napi_... has
some atomic opertions inside so that looks safe at the first look. But
I'm not sure if they also include enough memory barriers to make it
safe...?


Although I have never reproduced related issues on SMP platforms
due to reordering of memory operations but, as said above, welcome
review on this especially if you are seeing problems when manage NAPI.

FYI, the only memory barrier you will see in the driver are about the
OWN_BIT setting till now.


static void stmmac_dma_interrupt(struct stmmac_priv *priv)
{
...
status = priv->hw->dma->dma_interrupt(priv->ioaddr, >xstats);
if (likely((status & handle_rx)) || (status & handle_tx)) {
if (likely(napi_schedule_prep(>napi))) {
#1
stmmac_disable_dma_irq(priv);
__napi_schedule(>napi);
}
}


static int stmmac_poll(struct napi_struct *napi, int budget)
{
struct stmmac_priv *priv = container_of(napi, struct stmmac_priv, napi);
int work_done = 0;

priv->xstats.napi_poll++;
stmmac_tx_clean(priv);

work_done = stmmac_rx(priv, budget);
if (work_done < budget) {
napi_complete(napi);
#2
stmmac_enable_dma_irq(priv);
}


hmm, I have to check (and refresh my memory) but the driver
uses the napi_schedule_prep.

Regards

Peppe


return work_done;
}


Best regards,
   

Re: stmmac ethernet in kernel 4.9-rc6: coalescing related pauses.

[Giuseppe CAVALLARO]

Hi Pavel

On 12/2/2016 9:45 AM, Pavel Machek wrote:

Hi!


1 HZ, which is the lowest granularity of non-highres timers in the
kernel, is variable as well as already too large of a delay for
effective TX coalescing.

I seriously think that the TX coalescing support should be ripped out
or disabled entirely until it is implemented properly in this
driver.


Ok, I'd disable coalescing, but could not figure it out till. What is
generic way to do that?

It seems only thing stmmac_tx_timer() does is calling
stmmac_tx_clean(), which reclaims tx_skbuff[] entries. It should be
possible to do that explicitely, without delay, but it stops working
completely if I attempt to do that.

On a side note, stmmac_poll() does stmmac_enable_dma_irq() while
stmmac_dma_interrupt() disables interrupts. But I don't see any
protection between the two, so IMO it could race and we'd end up
without polling or interrupts...



the idea behind the TX mitigation is to mix the interrupt and
timer and this approach gave us real benefit in terms
of performances and CPU usage (especially on SH4-200/SH4-300 platforms
based).


Well, if you have a workload that sends and receive packets, it tends
to work ok, as you do tx_clean() in stmmac_poll(). My workload is not
like that -- it is "sending packets at 3MB/sec, receiving none". So
the stmmac_tx_timer() is rescheduled and rescheduled and rescheduled,
and then we run out of transmit descriptors, and then 40msec passes,
and then we clean them. Bad.

And that's why low-res timers do not cut it.


in that case, I expect that the tuning of the driver could help you.
I mean, by using ethtool, it could be enough to set the IC bit on all
the descriptors. You should touch the tx_coal_frames.

Then you can use ethtool -S to monitor the status.

We had experimented this tuning on STB IP where just datagrams
had to send externally. To be honest, although we had seen
better results w/o any timer, we kept this approach enabled
because the timer was fast enough to cover our tests on SH4 boxes.

FYI, stmmac doesn't implement adaptive algo.




In the ring, some descriptors can raise the irq (according to a
threshold) and set the IC bit. In this path, the NAPI  poll will be
scheduled.


Not NAPI poll but stmmac_tx_timer(), right?


in the xmit according the the threshold the timer is started or the
interrupt is set inside the descriptor.
Then stmmac_tx_clean will be always called and, if you see the flow,
no irqlock protection is needed!




But there is a timer that can run (and we experimented that no high
resolution is needed) to clear the tx resources.
Concerning the lock protection, we had reviewed long time ago and
IIRC, no raise condition should be present. Open to review it,
again!


Well, I certainly like the fact that we are talking :-).

And yes, I have some questions.

There's nothing that protect stmmac_poll() from running concurently
with stmmac_dma_interrupt(), right?


This is not necessary.

Best Regards
peppe



Best regards,
Pavel

Re: [PATCH 1/2] net: stmmac: avoid Camelcase naming

[Giuseppe CAVALLARO]

On 12/2/2016 9:58 AM, Corentin Labbe wrote:

On Fri, Dec 02, 2016 at 09:44:48AM +0100, Giuseppe CAVALLARO wrote:

Hello Corentin

patches look ok, I just wonder if you tested it in case of
the stmmac is connected to a transceiver. Let me consider it
a critical part of the driver to properly work.

Regards
Peppe



I tested it on a Cubieboard 2 (dwmac-sunxi).
What do you mean by "connected to a transceiver" ? an external PHY ?


yes an external PHY. AFAIK, users have, sometime, a switch
with fixed link enabled.

Thx for your support and patches

Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>



Regards

Re: [PATCH v2] cpsw: ethtool: add support for getting/setting EEE registers

[Giuseppe CAVALLARO]

Hi Florian
sorry for my delay.

On 11/24/2016 7:23 PM, Florian Fainelli wrote:

+Peppe,

Le 24/11/2016 à 07:38, Andrew Lunn a écrit :

As for enabling advertising and correct working of cpsw do you mean it
would be better to disable EEE in any PHY on cpsw initialization as
long as cpsw doesn't provide support for EEE?

We observe some strange behavior with our gigabit PHYs and a link
partner in a EEE-capable unmanaged NetGear switch. Disabling
advertising seems to help. Though we're still investigating the issue.


Hi Florian

Am i right in saying, a PHY should not advertise EEE until the MAC
driver calls phy_init_eee(), indicating the MAC supports EEE?


You would think so, but I don't see how this could possibly work if that
was not the case already, see below.



If so, it looks like we need to change a few of the PHY drivers, in
particular, the bcm-*.c.


The first part that bcm-phy-lib.c does is make sure that EEE is enabled
such that this gets reflected in MDIO_PCS_EEE_ABLE, without this, we
won't be able to pass the first test in phy_init_eee(). The second part
is to advertise EEE such that this gets reflected in MDIO_AN_EEE_ADV,
also to make sure that we can pass the second check in phy_init_eee().

Now, looking at phy_init_eee(), and what stmmac does (and bcmgenet,
copied after stmmac), we need to somehow, have EEE advertised for
phy_init_eee() to succeed, prepare the MAC to support EEE, and finally
conclude with a call to phy_ethtool_set_eee(), which writes to the
MDIO_AN_EEE_ADV register, and concludes the EEE auto-negotiated process.
Since we already have EEE advertised, we are essentially just checking
that the EEE advertised settings and the LP advertised settings actually
do match, so it sounds like the final call to phy_ethtool_set_eee() is
potentially useless if the resolved advertised and link partner
advertised settings already match...

So it sounds like at least, the first time you try to initialize EEE, we
should start with EEE not advertised, and then, if we have EEE enabled
at some point, and we re-negotiate the link parameters, somehow
phy_init_eee() does a right job for that.

Peppe, any thoughts on this?


I share what you say.

In sum, the EEE management inside the stmmac is:

- the driver looks at own HW cap register if EEE is supported

(indeed the user could keep disable EEE if bugged on some HW
 + Alex, Fabrice: we had some patches for this to propose where we
 called the phy_ethtool_set_eee to disable feature at phy
 level

- then the stmmac asks PHY layer to understand if transceiver and
  partners are EEE capable.

- If all matches the EEE is actually initialized.

the logic above should be respected when use ethtool, hmm, I will
check the stmmac_ethtool_op_set_eee asap.

Hoping this is useful

Regards
Peppe

Re: [PATCH 1/4] bindings: net: stmmac: correct note about TSO

[Giuseppe CAVALLARO]

On 11/23/2016 3:24 PM, Niklas Cassel wrote:

From: Niklas Cassel <niklas.cas...@axis.com>

snps,tso was previously placed under AXI BUS Mode parameters,
suggesting that the property should be in the stmmac-axi-config node.

TSO (TCP Segmentation Offloading) has nothing to do with AXI BUS Mode
parameters, and the parser actually expects it to be in the root node,
not in the stmmac-axi-config.

Also added a note about snps,tso only being available on GMAC4 and newer.

Signed-off-by: Niklas Cassel <niklas.cas...@axis.com>


Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>



---
 Documentation/devicetree/bindings/net/stmmac.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/stmmac.txt 
b/Documentation/devicetree/bindings/net/stmmac.txt
index 41b49e6075f5..b95ff998ba73 100644
--- a/Documentation/devicetree/bindings/net/stmmac.txt
+++ b/Documentation/devicetree/bindings/net/stmmac.txt
@@ -1,7 +1,7 @@
 * STMicroelectronics 10/100/1000 Ethernet driver (GMAC)

 Required properties:
-- compatible: Should be "snps,dwmac-" "snps,dwmac"
+- compatible: Should be "snps,dwmac-", "snps,dwmac"
For backwards compatibility: "st,spear600-gmac" is also supported.
 - reg: Address and length of the register set for the device
 - interrupt-parent: Should be the phandle for the interrupt controller
@@ -50,6 +50,8 @@ Optional properties:
 - snps,ps-speed: port selection speed that can be passed to the core when
 PCS is supported. For example, this is used in case of SGMII
 and MAC2MAC connection.
+- snps,tso: this enables the TSO feature otherwise it will be managed by
+MAC HW capability register. Only for GMAC4 and newer.
 - AXI BUS Mode parameters: below the list of all the parameters to program the
   AXI register inside the DMA module:
- snps,lpi_en: enable Low Power Interface
@@ -62,8 +64,6 @@ Optional properties:
- snps,fb: fixed-burst
- snps,mb: mixed-burst
- snps,rb: rebuild INCRx Burst
-   - snps,tso: this enables the TSO feature otherwise it will be managed by
-   MAC HW capability register.
 - mdio: with compatible = "snps,dwmac-mdio", create and register mdio bus.

 Examples:

Re: [PATCH net 0/7] net: stmmac: fix probe error handling and phydev leaks

[Giuseppe CAVALLARO]

On 11/30/2016 3:29 PM, Johan Hovold wrote:

This series fixes a number of issues with the stmmac-driver probe error
handling, which for example left clocks enabled after probe failures.

The final patch fixes a failure to deregister and free any fixed-link
PHYs that were registered during probe on probe errors and on driver
unbind. It also fixes a related of-node leak on late probe errors.

This series depends on the of_phy_deregister_fixed_link() helper that
was just merged to net.

As mentioned earlier, one staging driver also suffers from a similar
leak and can be fixed up once the above mentioned helper hits mainline.

Note that these patches have only been compile tested.


For common and STi part:

Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>

thx
peppe



Johan


Johan Hovold (7):
  net: ethernet: stmmac: dwmac-socfpga: fix use-after-free on probe
errors
  net: ethernet: stmmac: dwmac-sti: fix probe error path
  net: ethernet: stmmac: dwmac-rk: fix probe error path
  net: ethernet: stmmac: dwmac-generic: fix probe error path
  net: ethernet: stmmac: dwmac-meson8b: fix probe error path
  net: ethernet: stmmac: platform: fix outdated function header
  net: ethernet: stmmac: fix of-node and fixed-link-phydev leaks

 .../net/ethernet/stmicro/stmmac/dwmac-generic.c| 17 --
 .../net/ethernet/stmicro/stmmac/dwmac-ipq806x.c| 25 ++
 .../net/ethernet/stmicro/stmmac/dwmac-lpc18xx.c| 17 --
 drivers/net/ethernet/stmicro/stmmac/dwmac-meson.c  | 23 ++---
 .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c| 32 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 21 +---
 .../net/ethernet/stmicro/stmmac/dwmac-socfpga.c| 39 ++
 drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c| 23 ++---
 drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c  | 19 ---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sunxi.c  | 26 +++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  |  1 -
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  | 33 +++---
 .../net/ethernet/stmicro/stmmac/stmmac_platform.h  |  2 ++
 13 files changed, 215 insertions(+), 63 deletions(-)

Re: [PATCH 1/2] net: stmmac: avoid Camelcase naming

[Giuseppe CAVALLARO]

Hello Corentin

patches look ok, I just wonder if you tested it in case of
the stmmac is connected to a transceiver. Let me consider it
a critical part of the driver to properly work.

Regards
Peppe

On 12/1/2016 4:19 PM, Corentin Labbe wrote:

This patch simply rename regValue to value, like it was named in other
mdio functions.

Signed-off-by: Corentin Labbe 
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index e3216e5..6796c28 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -83,14 +83,14 @@ static int stmmac_mdio_read(struct mii_bus *bus, int 
phyaddr, int phyreg)
unsigned int mii_data = priv->hw->mii.data;

int data;
-   u16 regValue = (((phyaddr << 11) & (0xF800)) |
+   u16 value = (((phyaddr << 11) & (0xF800)) |
((phyreg << 6) & (0x07C0)));
-   regValue |= MII_BUSY | ((priv->clk_csr & 0xF) << 2);
+   value |= MII_BUSY | ((priv->clk_csr & 0xF) << 2);

if (stmmac_mdio_busy_wait(priv->ioaddr, mii_address))
return -EBUSY;

-   writel(regValue, priv->ioaddr + mii_address);
+   writel(value, priv->ioaddr + mii_address);

if (stmmac_mdio_busy_wait(priv->ioaddr, mii_address))
return -EBUSY;

Re: stmmac ethernet in kernel 4.9-rc6: coalescing related pauses.

[Giuseppe CAVALLARO]

+ Lino

On 12/2/2016 9:24 AM, Giuseppe CAVALLARO wrote:


Hello


On 11/24/2016 10:25 PM, Pavel Machek wrote:

Hi!


I'm debugging strange delays during transmit in stmmac driver. They
seem to be present in 4.4 kernel (and older kernels, too). Workload is
burst of udp packets being sent, pause, burst of udp packets, ...

...

4.9-rc6 still has the delays. With the

#define STMMAC_COAL_TX_TIMER 1000
#define STMMAC_TX_MAX_FRAMES 2

settings, delays go away, and driver still works. (It fails fairly
fast in 4.4). Good news. But the question still is: what is going on
there?


256 packets looks way too large for being a trigger for aborting the
TX coalescing timer.

Looking more deeply into this, the driver is using non-highres timers
to implement the TX coalescing.  This simply cannot work.

1 HZ, which is the lowest granularity of non-highres timers in the
kernel, is variable as well as already too large of a delay for
effective TX coalescing.

I seriously think that the TX coalescing support should be ripped out
or disabled entirely until it is implemented properly in this
driver.


Ok, I'd disable coalescing, but could not figure it out till. What is
generic way to do that?

It seems only thing stmmac_tx_timer() does is calling
stmmac_tx_clean(), which reclaims tx_skbuff[] entries. It should be
possible to do that explicitely, without delay, but it stops working
completely if I attempt to do that.

On a side note, stmmac_poll() does stmmac_enable_dma_irq() while
stmmac_dma_interrupt() disables interrupts. But I don't see any
protection between the two, so IMO it could race and we'd end up
without polling or interrupts...



the idea behind the TX mitigation is to mix the interrupt and
timer and this approach gave us real benefit in terms
of performances and CPU usage (especially on SH4-200/SH4-300 platforms
based).
In the ring, some descriptors can raise the irq (according to a
threshold) and set the IC bit. In this path, the NAPI  poll will be
scheduled.
But there is a timer that can run (and we experimented that no high
resolution is needed) to clear the tx resources.
Concerning the lock protection, we had reviewed long time ago and
IIRC, no raise condition should be present. Open to review it, again!

So, welcome any other schema and testing on platforms supported.


Hoping this summary can help.

Peppe




Thanks and best regards,
Pavel

Re: stmmac: turn coalescing / NAPI off in stmmac

[Giuseppe CAVALLARO]

On 12/1/2016 11:48 PM, Pavel Machek wrote:



@@ -2771,12 +2771,8 @@ static netdev_features_t stmmac_fix_features(struct 
net_device *dev,
features &= ~NETIF_F_CSUM_MASK;

/* Disable tso if asked by ethtool */
-   if ((priv->plat->tso_en) && (priv->dma_cap.tsoen)) {
-   if (features & NETIF_F_TSO)
-   priv->tso = true;
-   else
-   priv->tso = false;
-   }
+   if ((priv->plat->tso_en) && (priv->dma_cap.tsoen))
+   priv->tso = !!(features & NETIF_F_TSO);



Pavel, this really seems arbitrary.

Whilst I really appreciate you're looking into this driver a bit because
of some issues you are trying to resolve, I'd like to ask that you not
start bombarding me with nit-pick cleanups here and there and instead
concentrate on the real bug or issue.


Well, fixing clean code is easier than fixing strange code... Plus I
was hoping to make the mainainers to talk. The driver is listed as
supported after all.


Absolutely, I am available to support you, better I can.
So no problem to clarify strange or complex parts of the driver
and find/try new solutions to enhance it.


Anyway... since you asked. I belive I have way to disable NAPI / tx
coalescing in the driver. Unfortunately, locking is missing on the rx
path, and needs to be extended to _irqsave variant on tx path.


I have just replied to a previous thread about that...

To be honest, I have in the box just a patch to fix lock on lpi
as I had discussed in this mailing list some week ago.
I will provide it asap.



So patch currently looks like this (hand edited, can't be
applied, got it working few hours ago). Does it look acceptable?

I'd prefer this to go after the patch that pulls common code to single
place, so that single place needs to be patched. Plus I guess I should
add ifdefs, so that more advanced NAPI / tx coalescing code can be
reactivated when it is fixed. Trivial fixes can go on top. Does that
sound like a plan?


Hmm, what I find strange is that, just this code is running since a
long time on several platforms and Chip versions. No raise condition
have been found or lock protection problems (also proving look
mechanisms).

I'd like to avoid to break old compatibilities and having the same
performances but if there are some bugs I can support to review
and test. Indeed, this year we have added the 4.x but some parts
of the code (for TSO) should be self-contained. So I cannot image
regressions on common part of the code... I let Alex to do a double
check.

Pavel, I ask you sorry if I missed some problems so, if you can
(as D. Miller asked) to send us a cover letter + all patches
I will try to reply soon. I can do also some tests if you ask
me that! I could run on 3.x and 4.x but I cannot promise you
benchmarks.


Which tree do you want patches against?

https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/ ?


I think that bug fixing should be on top of net.git but I let Miller
to decide.

Best Regards
Peppe



Best regards,
Pavel


diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 0b706a7..c0016c8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1395,9 +1397,10 @@ static void __stmmac_tx_clean(struct stmmac_priv *priv)

 static void stmmac_tx_clean(struct stmmac_priv *priv)
 {
-   spin_lock(>tx_lock);
+   unsigned long flags;
+   spin_lock_irqsave(>tx_lock, flags);
__stmmac_tx_clean(priv);
-   spin_unlock(>tx_lock); 
+   spin_unlock_irqrestore(>tx_lock, flags);   
 }

 static inline void stmmac_enable_dma_irq(struct stmmac_priv *priv)
@@ -1441,6 +1444,8 @@ static void stmmac_tx_err(struct stmmac_priv *priv)
netif_wake_queue(priv->dev);
 }

+static int stmmac_rx(struct stmmac_priv *priv, int limit);
+
 /**
  * stmmac_dma_interrupt - DMA ISR
  * @priv: driver private structure
@@ -1452,10 +1457,17 @@ static void stmmac_dma_interrupt(struct stmmac_priv 
*priv)
 {
int status;
int rxfifosz = priv->plat->rx_fifo_size;
+   unsigned long flags;

status = priv->hw->dma->dma_interrupt(priv->ioaddr, >xstats);
if (likely((status & handle_rx)) || (status & handle_tx)) {
+   int r;
+   spin_lock_irqsave(>tx_lock, flags);
+   r = stmmac_rx(priv, 999);
+   spin_unlock_irqrestore(>tx_lock, flags);   
+#if 0
if (likely(napi_schedule_prep(>napi))) {
//pr_err("napi: schedule\n");
stmmac_disable_dma_irq(priv);
@@ -1463,7 +1475,8 @@ static void stmmac_dma_interrupt(struct stmmac_priv *priv)
} else
pr_err("napi: schedule failed\n");
 #endif
+   stmmac_tx_clean(priv);
}
if 

Re: [PATCH] stmmac: simplify flag assignment

[Giuseppe CAVALLARO]

+ Alex

On 11/30/2016 12:44 PM, Pavel Machek wrote:


Simplify flag assignment.

Signed-off-by: Pavel Machek 

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index ed20668..0b706a7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2771,12 +2771,8 @@ static netdev_features_t stmmac_fix_features(struct 
net_device *dev,
features &= ~NETIF_F_CSUM_MASK;

/* Disable tso if asked by ethtool */
-   if ((priv->plat->tso_en) && (priv->dma_cap.tsoen)) {
-   if (features & NETIF_F_TSO)
-   priv->tso = true;
-   else
-   priv->tso = false;
-   }
+   if ((priv->plat->tso_en) && (priv->dma_cap.tsoen))
+   priv->tso = !!(features & NETIF_F_TSO);

return features;
 }

Re: stmmac ethernet in kernel 4.9-rc6: coalescing related pauses.

[Giuseppe CAVALLARO]

Hello


On 11/24/2016 10:25 PM, Pavel Machek wrote:

Hi!


I'm debugging strange delays during transmit in stmmac driver. They
seem to be present in 4.4 kernel (and older kernels, too). Workload is
burst of udp packets being sent, pause, burst of udp packets, ...

...

4.9-rc6 still has the delays. With the

#define STMMAC_COAL_TX_TIMER 1000
#define STMMAC_TX_MAX_FRAMES 2

settings, delays go away, and driver still works. (It fails fairly
fast in 4.4). Good news. But the question still is: what is going on
there?


256 packets looks way too large for being a trigger for aborting the
TX coalescing timer.

Looking more deeply into this, the driver is using non-highres timers
to implement the TX coalescing.  This simply cannot work.

1 HZ, which is the lowest granularity of non-highres timers in the
kernel, is variable as well as already too large of a delay for
effective TX coalescing.

I seriously think that the TX coalescing support should be ripped out
or disabled entirely until it is implemented properly in this
driver.


Ok, I'd disable coalescing, but could not figure it out till. What is
generic way to do that?

It seems only thing stmmac_tx_timer() does is calling
stmmac_tx_clean(), which reclaims tx_skbuff[] entries. It should be
possible to do that explicitely, without delay, but it stops working
completely if I attempt to do that.

On a side note, stmmac_poll() does stmmac_enable_dma_irq() while
stmmac_dma_interrupt() disables interrupts. But I don't see any
protection between the two, so IMO it could race and we'd end up
without polling or interrupts...



the idea behind the TX mitigation is to mix the interrupt and
timer and this approach gave us real benefit in terms
of performances and CPU usage (especially on SH4-200/SH4-300 platforms
based).
In the ring, some descriptors can raise the irq (according to a
threshold) and set the IC bit. In this path, the NAPI  poll will be
scheduled.
But there is a timer that can run (and we experimented that no high
resolution is needed) to clear the tx resources.
Concerning the lock protection, we had reviewed long time ago and
IIRC, no raise condition should be present. Open to review it, again!

So, welcome any other schema and testing on platforms supported.


Hoping this summary can help.

Peppe




Thanks and best regards,
Pavel

Re: Synopsys Ethernet QoS Driver

[Giuseppe CAVALLARO]

On 11/23/2016 12:43 PM, Joao Pinto wrote:

> Rabin Vincent can review and test that the port works properly on our 
Artpec-chips that use dwc_eth_qos.c today.
>
> The main porting step is to implement the device tree binding in 
bindings/net/snps,dwc-qos-ethernet.txt. Also our chip has a strict requirement 
that the phy is enabled when the SWR reset bit is set (it needs a tx clock to 
complete the reset).
>
> - Lars

Ok, I will do the task.

@Peppe: Agree with the plan?


Agree

peppe

Re: [net-next PATCH v1 0/2] stmmac: dwmac-meson8b: configurable RGMII TX delay

[Giuseppe CAVALLARO]

On 11/24/2016 5:08 PM, Jerome Brunet wrote:

On Thu, 2016-11-24 at 15:34 +0100, Martin Blumenstingl wrote:

Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
cycle TX clock delay. This seems to work fine for many boards (for
example Odroid-C2 or Amlogic's reference boards) but there are some
others where TX traffic is simply broken.
There are probably multiple reasons why it's working on some boards
while it's broken on others:
- some of Amlogic's reference boards are using a Micrel PHY
- hardware circuit design
- maybe more...

This raises a question though:
Which device is supposed to enable the TX delay when both MAC and PHY
support it? And should we implement it for each PHY / MAC separately
or should we think about a more generic solution (currently it's not
possible to disable the TX delay generated by the RTL8211F PHY via
devicetree when using phy-mode "rgmii")?

iperf3 results on my Mecool BB2 board (Meson GXM, RTL8211F PHY) with
TX clock delay disabled on the MAC (as it's enabled in the PHY
driver).
TX throughput was virtually zero before:
$ iperf3 -c 192.168.1.100 -R
Connecting to host 192.168.1.100, port 5201
Reverse mode, remote host 192.168.1.100 is sending
[  4] local 192.168.1.206 port 52828 connected to 192.168.1.100 port
5201
[ ID] Interval   Transfer Bandwidth
[  4]   0.00-1.00   sec   108 MBytes   901
Mbits/sec
[  4]   1.00-2.00   sec  94.2 MBytes   791
Mbits/sec
[  4]   2.00-3.00   sec  96.5 MBytes   810
Mbits/sec
[  4]   3.00-4.00   sec  96.2 MBytes   808
Mbits/sec
[  4]   4.00-5.00   sec  96.6 MBytes   810
Mbits/sec
[  4]   5.00-6.00   sec  96.5 MBytes   810
Mbits/sec
[  4]   6.00-7.00   sec  96.6 MBytes   810
Mbits/sec
[  4]   7.00-8.00   sec  96.5 MBytes   809
Mbits/sec
[  4]   8.00-9.00   sec   105 MBytes   884
Mbits/sec
[  4]   9.00-10.00  sec   111 MBytes   934
Mbits/sec
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval   Transfer Bandwidth   Retr
[  4]   0.00-10.00  sec  1000 MBytes   839
Mbits/sec0 sender
[  4]   0.00-10.00  sec   998 MBytes   837
Mbits/sec  receiver

iperf Done.
$ iperf3 -c 192.168.1.100
Connecting to host 192.168.1.100, port 5201
[  4] local 192.168.1.206 port 52832 connected to 192.168.1.100 port
5201
[ ID] Interval   Transfer Bandwidth   Retr  Cwnd
[  4]   0.00-1.01   sec  99.5 MBytes   829 Mbits/sec  117139
KBytes
[  4]   1.01-2.00   sec   105 MBytes   884 Mbits/sec  129   70.7
KBytes
[  4]   2.00-3.01   sec   107 MBytes   889 Mbits/sec  106187
KBytes
[  4]   3.01-4.01   sec   105 MBytes   878 Mbits/sec   92143
KBytes
[  4]   4.01-5.00   sec   105 MBytes   882 Mbits/sec  140129
KBytes
[  4]   5.00-6.01   sec   106 MBytes   883 Mbits/sec  115195
KBytes
[  4]   6.01-7.00   sec   102 MBytes   863 Mbits/sec  133   70.7
KBytes
[  4]   7.00-8.01   sec   106 MBytes   884 Mbits/sec  143   97.6
KBytes
[  4]   8.01-9.01   sec   104 MBytes   875 Mbits/sec  124107
KBytes
[  4]   9.01-10.01  sec   105 MBytes   876 Mbits/sec   90139
KBytes
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval   Transfer Bandwidth   Retr
[  4]   0.00-10.01  sec  1.02 GBytes   874
Mbits/sec  1189 sender
[  4]   0.00-10.01  sec  1.02 GBytes   873
Mbits/sec  receiver



Cool, one more board working ;)
I tried your patch on another board (MXQ_V2.1, with sheep printed on
the PCB). It 's not improving the situation for this one unfortunately.
Actually I already tried playing with the TX delay on the MAC and PHY
but I could get any good results with the boards I have.

It is strange that we can adjust the delay by 2ns steps, when delay
seens by the phy should be 2ns ...



Ok, as said, I could expect that extra delay was needed on GiGa.
FYI, on ST platforms, this extra delay was added in the pintctrl
dtsi. I mean, in some way, ad-hoc setup was solved at device-tree
level. Usually, extra delay could depend on the PCB.

peppe


iperf Done.


Martin Blumenstingl (2):
  net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac
  net: stmmac: dwmac-meson8b: make the RGMII TX delay configurable

 Documentation/devicetree/bindings/net/meson-dwmac.txt | 11
+++
 drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c   | 16
+++-
 include/dt-bindings/net/dwmac-meson8b.h   | 18
++
 3 files changed, 40 insertions(+), 5 deletions(-)
 create mode 100644 include/dt-bindings/net/dwmac-meson8b.h

Re: [PATCH 3/4] net: stmmac: dwmac-generic: add missing compatible strings

[Giuseppe CAVALLARO]

Hello Niklas

On 11/23/2016 3:25 PM, Niklas Cassel wrote:

From: Niklas Cassel <niklas.cas...@axis.com>

devicetree binding for stmmac states:
- compatible: Should be "snps,dwmac-", "snps,dwmac"
For backwards compatibility: "st,spear600-gmac" is also supported.

Since dwmac-generic.c calls stmmac_probe_config_dt explicitly,
another alternative would have been to remove all compatible strings
other than "snps,dwmac" and "st,spear600-gmac" from dwmac-generic.c.

However, that would probably do more good than harm, since when trying
to figure out what hardware a certain driver supports, you usually look
at the compatible strings in the struct of_device_id, and not in some
function defined in a completely different file.

No functional change intended.

Signed-off-by: Niklas Cassel <niklas.cas...@axis.com>


Acked-by: Giuseppe Cavallaro <peppe.cavall...@st.com>


---
 drivers/net/ethernet/stmicro/stmmac/dwmac-generic.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-generic.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-generic.c
index b1e5f24708c9..52cd365b8e5e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-generic.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-generic.c
@@ -58,9 +58,12 @@ static int dwmac_generic_probe(struct platform_device *pdev)

 static const struct of_device_id dwmac_generic_match[] = {
{ .compatible = "st,spear600-gmac"},
+   { .compatible = "snps,dwmac-3.50a"},
{ .compatible = "snps,dwmac-3.610"},
{ .compatible = "snps,dwmac-3.70a"},
{ .compatible = "snps,dwmac-3.710"},
+   { .compatible = "snps,dwmac-4.00"},
+   { .compatible = "snps,dwmac-4.10a"},
{ .compatible = "snps,dwmac"},
{ }
 };

Re: Synopsys Ethernet QoS Driver

[Giuseppe CAVALLARO]

Hello Joao, Lars.

On 11/22/2016 3:16 PM, Joao Pinto wrote:

Ok, it makes sense.
> Just for curiosity the target setup is the following:
> https://www.youtube.com/watch?v=8V-LB5y2Cos
> but instead of using internal drivers, we desire to use mainline drivers only.
>
> Thanks!

Regarding this subject, I am thinking of making the following adaption:

a) delete ethernet/synopsys
b) rename ethernet/stmicro/stmmac to ethernet/synopsys

and send you a patch for you to evaluate. Both agree with the approach?
To have a new work base would be important, because I will add to the "new"
structure some missing QoS features like Multichannel support, CBS and later 
TSN.


IMO, we have to agree on a common strategy making the change for
net-next; I imaged the following steps:

- to port missing feature or fixes from ethernet/synopsys
  inside the stmmac taking care about the documentation too.
- remove ethernet/synopsys
- rename ethernet/stmicro/stmmac to ethernet/synopsys

  These latest two have some relevant impacts.

  This change should be propagated to all the platforms that are using:
  CONFIG_SYNOPSYS_DWC_ETH_QOS and CONFIG_STMMAC_ETH
  plus device-tree compatibility.

- enhance the stmmac with new features and new glue (part of these
  can be anticipated for sure).

what do you think? does it make sense? If yes, we can also
understand how/who starts.

Regards,
Peppe


Thanks.

Re: Synopsys Ethernet QoS Driver

[Giuseppe CAVALLARO]

Hello Ozgur

On 11/22/2016 9:38 AM, Ozgur Karatas wrote:

Hello all,

I think, ethtool and mdio don't work because the tool's not support to "QoS", 
right?

Maybe, need a new API. I'm looking for dwceqos code but "tc" tools is very idea.

I hope to be me always helpful.


tools work but indeed should be extended to support more for QoS.
This is another task we have to keep in mind, well spot.

Peppe



Regards,

Ozgur

21.11.2016, 16:38, "Giuseppe CAVALLARO" <peppe.cavall...@st.com>:

Hello Joao

On 11/21/2016 2:48 PM, Joao Pinto wrote:

 Synopsys QoS IP is a separated hardware component, so it should be reusable by
 all implementations using it and so have its own "core driver" and platform +
 pci glue drivers. This is necessary for example in hardware validation, where
 you prototype an IP and instantiate its drivers and test it.

 Was there a strong reason to integrate QoS features directly in stmmac and not
 in synopsys/dwc_eth_qos.*?


We decided to enhance the stmmac on supporting the QoS for several
reasons; for example the common APIs that the driver already exposed and
actually suitable for other SYNP chips. Then, PTP, EEE,
S/RGMII, MMC could be shared among different chips with a minimal
effort. This meant a lot of code already ready.

For sure, the net-core, Ethtool, mdio parts were reused. Same for the
glue logic files.
For the latter, this helped to easily bring-up new platforms also
because the stmmac uses the HW cap register to auto-configure many
parts of the MAC core, DMA and modules. This helped many users, AFAIK.

For validation purpose, this is my experience, the stmmac helped
a lot because people used the same code to validate different HW
and it was easy to switch to a platform to another one in order to
verify / check if the support was ok or if a regression was introduced.
This is important for complex supports like PTP or EEE.

Hoping this can help.

Do not hesitate to contact me for further details

peppe

Re: Synopsys Ethernet QoS Driver

[Giuseppe CAVALLARO]

On 11/21/2016 4:00 PM, Joao Pinto wrote:

On 21-11-2016 14:36, Giuseppe CAVALLARO wrote:

Hello Joao

On 11/21/2016 2:48 PM, Joao Pinto wrote:

Synopsys QoS IP is a separated hardware component, so it should be reusable by
all implementations using it and so have its own "core driver" and platform +
pci glue drivers. This is necessary for example in hardware validation, where
you prototype an IP and instantiate its drivers and test it.

Was there a strong reason to integrate QoS features directly in stmmac and not
in synopsys/dwc_eth_qos.*?


We decided to enhance the stmmac on supporting the QoS for several
reasons; for example the common APIs that the driver already exposed and
actually suitable for other SYNP chips. Then, PTP, EEE,
S/RGMII, MMC could be shared among different chips with a minimal
effort.  This meant a lot of code already ready.

For sure, the net-core, Ethtool, mdio parts were reused. Same for the
glue logic files.
For the latter, this helped to easily bring-up new platforms also
because the stmmac uses the HW cap register to auto-configure many
parts of the MAC core, DMA and modules. This helped many users, AFAIK.

For validation purpose, this is my experience, the stmmac helped
a lot because people used the same code to validate different HW
and it was easy to switch to a platform to another one in order to
verify / check if the support was ok or if a regression was introduced.
This is important for complex supports like PTP or EEE.

Hoping this can help.

Do not hesitate to contact me for further details


Thanks for the highly detailed info.
My target application is to prototype the Ethernet QoS IP in a FPGA, with a PHY
attached and make hardware validation.

In your opinion a refactored stmmac with the missing QoS features would be
suitable for it?


I think so; somebody also added code for FPGA.

In any case, step-by-step we can explore and understand
how to proceed. I wonder if you could start looking at the internal
of the stmmac. Then welcome doubts and open question...



Thanks.


welcome

peppe





peppe