[PATCH net 0/3] net: stmmac: Misc fixes

[PATCH net 0/3] net: stmmac: Misc fixes

[Jose Abreu]

Some misc fixes for stmmac targeting -net.

Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>

Jose Abreu (3):
  net: stmmac: Fallback to Platform Data clock in Watchdog conversion
  net: stmmac: Send TSO packets always from Queue 0
  net: stmmac: Disable EEE mode earlier in XMIT callback

 drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 14 ++++++++++----
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c    | 17 +++++++++++++----
 include/linux/stmmac.h                               |  1 +
 3 files changed, 24 insertions(+), 8 deletions(-)

-- 
2.7.4

[PATCH net 1/3] net: stmmac: Fallback to Platform Data clock in Watchdog conversion

[Jose Abreu]

If we don't have DT then stmmac_clk will not be available. Let's add a
new Platform Data field so that we can specify the refclk by this mean.

This way we can still use the coalesce command in PCI based setups.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 14 ++++++++++----
 include/linux/stmmac.h                               |  1 +
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
index d1f61c25d82b..5d85742a2be0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
@@ -721,8 +721,11 @@ static u32 stmmac_usec2riwt(u32 usec, struct stmmac_priv *priv)
 {
 	unsigned long clk = clk_get_rate(priv->plat->stmmac_clk);
 
-	if (!clk)
-		return 0;
+	if (!clk) {
+		clk = priv->plat->clk_ref_rate;
+		if (!clk)
+			return 0;
+	}
 
 	return (usec * (clk / 1000000)) / 256;
 }
@@ -731,8 +734,11 @@ static u32 stmmac_riwt2usec(u32 riwt, struct stmmac_priv *priv)
 {
 	unsigned long clk = clk_get_rate(priv->plat->stmmac_clk);
 
-	if (!clk)
-		return 0;
+	if (!clk) {
+		clk = priv->plat->clk_ref_rate;
+		if (!clk)
+			return 0;
+	}
 
 	return (riwt * 256) / (clk / 1000000);
 }
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 7ddfc65586b0..4335bd771ce5 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -184,6 +184,7 @@ struct plat_stmmacenet_data {
 	struct clk *pclk;
 	struct clk *clk_ptp_ref;
 	unsigned int clk_ptp_rate;
+	unsigned int clk_ref_rate;
 	struct reset_control *stmmac_rst;
 	struct stmmac_axi *axi;
 	int has_gmac4;
-- 
2.7.4

[PATCH net 2/3] net: stmmac: Send TSO packets always from Queue 0

[Jose Abreu]

The number of TSO enabled channels in HW can be different than the
number of total channels. There is no way to determined, at runtime, the
number of TSO capable channels and its safe to assume that if TSO is
enabled then at least channel 0 will be TSO capable.

Lets always send TSO packets from Queue 0.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 5afba69981cf..6656008068de 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3025,8 +3025,17 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	/* Manage oversized TCP frames for GMAC4 device */
 	if (skb_is_gso(skb) && priv->tso) {
-		if (skb_shinfo(skb)->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))
+		if (skb_shinfo(skb)->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6)) {
+			/*
+			 * There is no way to determine the number of TSO
+			 * capable Queues. Let's use always the Queue 0
+			 * because if TSO is supported then at least this
+			 * one will be capable.
+			 */
+			skb_set_queue_mapping(skb, 0);
+
 			return stmmac_tso_xmit(skb, dev);
+		}
 	}
 
 	if (unlikely(stmmac_tx_avail(priv, queue) < nfrags + 1)) {
-- 
2.7.4

[PATCH net 3/3] net: stmmac: Disable EEE mode earlier in XMIT callback

[Jose Abreu]

In stmmac xmit callback we use a different flow for TSO packets but TSO
xmit callback is not disabling the EEE mode.

Fix this by disabling earlier the EEE mode, i.e. before calling the TSO
xmit callback.

Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 6656008068de..685d20472358 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3023,6 +3023,9 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	tx_q = &priv->tx_queue[queue];
 
+	if (priv->tx_path_in_lpi_mode)
+		stmmac_disable_eee_mode(priv);
+
 	/* Manage oversized TCP frames for GMAC4 device */
 	if (skb_is_gso(skb) && priv->tso) {
 		if (skb_shinfo(skb)->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6)) {
@@ -3050,9 +3053,6 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 		return NETDEV_TX_BUSY;
 	}
 
-	if (priv->tx_path_in_lpi_mode)
-		stmmac_disable_eee_mode(priv);
-
 	entry = tx_q->cur_tx;
 	first_entry = entry;
 	WARN_ON(tx_q->tx_skbuff[first_entry]);
-- 
2.7.4

Re: [PATCH net 0/3] net: stmmac: Misc fixes

[Corentin Labbe]

On Wed, Jan 30, 2019 at 03:54:18PM +0100, Jose Abreu wrote:
> Some misc fixes for stmmac targeting -net.
> 
> Cc: Joao Pinto <jpinto@synopsys.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> 
> Jose Abreu (3):
>   net: stmmac: Fallback to Platform Data clock in Watchdog conversion
>   net: stmmac: Send TSO packets always from Queue 0
>   net: stmmac: Disable EEE mode earlier in XMIT callback
> 
>  drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 14 ++++++++++----
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c    | 17 +++++++++++++----
>  include/linux/stmmac.h                               |  1 +
>  3 files changed, 24 insertions(+), 8 deletions(-)
> 
> -- 
> 2.7.4
> 

Hello

Could you CC linux-kernel@vger.kernel.org when you send patch. (as asked by get_maintainer.pl)
By avoiding it, your patchset is not stored on lore nor is handled by all checkbots reading lkml.

Thanks
Regards

Re: [PATCH net 0/3] net: stmmac: Misc fixes

[David Miller]

From: Jose Abreu <jose.abreu@synopsys.com>
Date: Wed, 30 Jan 2019 15:54:18 +0100

> Some misc fixes for stmmac targeting -net.

Series applied.

Re: [PATCH net 0/3] net: stmmac: Misc fixes

[Niklas Cassel]

On Wed, Jan 30, 2019 at 03:54:18PM +0100, Jose Abreu wrote:
> 
> Some misc fixes for stmmac targeting -net.
> 
> Cc: Joao Pinto <jpinto@synopsys.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> 
> Jose Abreu (3):
>   net: stmmac: Fallback to Platform Data clock in Watchdog conversion
>   net: stmmac: Send TSO packets always from Queue 0
>   net: stmmac: Disable EEE mode earlier in XMIT callback

Hello Jose,

Thanks for your great work of improving the stmmac driver.

Unfortunately, I'm seeing that a lot of important stmmac fixes
aren't getting backported to stable kernels.


I think that the problem is that a lot of stmmac commit messages do
not clearly indicate what happens if these patches are not included.

See netdev FAQ:

Q: I have created a network patch and I think it should be backported to
   stable.  Should I add a "Cc: stable@vger.kernel.org" like the references
   in the kernel's Documentation/ directory say?

A: No.  See above answer.  In short, if you think it really belongs in
   stable, then ensure you write a decent commit log that describes who
   gets impacted by the bugfix and how it manifests itself, and when the
   bug was introduced.  If you do that properly, then the commit will
   get handled appropriately and most likely get put in the patchworks
   stable queue if it really warrants it.



Could you please tell me what happens if I don't include
"net: stmmac: Disable EEE mode earlier in XMIT callback" ?


likewise if I don't include "net: stmmac: Send TSO packets always from
Queue 0" ?

I assume that I will get a TX timeout if I try to send a TSO packet
from a queue that does not support it?

Also, we already define the TX queues in the snps,mtl-tx-config:
Documentation/devicetree/bindings/net/stmmac.txt

Wouldn't it be possible to add a snps,tso-capable property for each tx
queue node that supports tso?


Kind regards,
Niklas

> 
>  drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 14 ++++++++++----
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c    | 17 +++++++++++++----
>  include/linux/stmmac.h                               |  1 +
>  3 files changed, 24 insertions(+), 8 deletions(-)
> 
> -- 
> 2.7.4
> 

Re: [PATCH net 0/3] net: stmmac: Misc fixes

[Jose Abreu]

Hi Niklas,

On 2/5/2019 10:05 PM, Niklas Cassel wrote:
> On Wed, Jan 30, 2019 at 03:54:18PM +0100, Jose Abreu wrote:
>>
>> Some misc fixes for stmmac targeting -net.
>>
>> Cc: Joao Pinto <jpinto@synopsys.com>
>> Cc: David S. Miller <davem@davemloft.net>
>> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>> Cc: Alexandre Torgue <alexandre.torgue@st.com>
>>
>> Jose Abreu (3):
>>   net: stmmac: Fallback to Platform Data clock in Watchdog conversion
>>   net: stmmac: Send TSO packets always from Queue 0
>>   net: stmmac: Disable EEE mode earlier in XMIT callback
> 
> Hello Jose,
> 
> Thanks for your great work of improving the stmmac driver.
> 
> Unfortunately, I'm seeing that a lot of important stmmac fixes
> aren't getting backported to stable kernels.
> 
> 
> I think that the problem is that a lot of stmmac commit messages do
> not clearly indicate what happens if these patches are not included.
> 
> See netdev FAQ:
> 
> Q: I have created a network patch and I think it should be backported to
>    stable.  Should I add a "Cc: stable@vger.kernel.org" like the references
>    in the kernel's Documentation/ directory say?
> 
> A: No.  See above answer.  In short, if you think it really belongs in
>    stable, then ensure you write a decent commit log that describes who
>    gets impacted by the bugfix and how it manifests itself, and when the
>    bug was introduced.  If you do that properly, then the commit will
>    get handled appropriately and most likely get put in the patchworks
>    stable queue if it really warrants it.

I will try to add a Fixes tag and explain more carefully the next
time. Sorry.

> 
> 
> 
> Could you please tell me what happens if I don't include
> "net: stmmac: Disable EEE mode earlier in XMIT callback" ?

Probably a drop in performance. Not critical though.

> 
> 
> likewise if I don't include "net: stmmac: Send TSO packets always from
> Queue 0" ?

In my tests I get a drop in performance also.

> 
> I assume that I will get a TX timeout if I try to send a TSO packet
> from a queue that does not support it?

No. Packets will still be sent but they will not be TSO packets.

> 
> Also, we already define the TX queues in the snps,mtl-tx-config:
> Documentation/devicetree/bindings/net/stmmac.txt
> 
> Wouldn't it be possible to add a snps,tso-capable property for each tx
> queue node that supports tso?

Yes but that would be more like a -next improvement.

Thanks,
Jose Miguel Abreu

> 
> 
> Kind regards,
> Niklas
> 
>>
>>  drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 14 ++++++++++----
>>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c    | 17 +++++++++++++----
>>  include/linux/stmmac.h                               |  1 +
>>  3 files changed, 24 insertions(+), 8 deletions(-)
>>
>> -- 
>> 2.7.4
>>