All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 net] net: stmmac: removed enabling eee in EEE set callback
@ 2020-09-23  8:56 Voon Weifeng
  2020-09-23 23:37 ` mark gross
  2020-09-24  1:09 ` David Miller
  0 siblings, 2 replies; 3+ messages in thread
From: Voon Weifeng @ 2020-09-23  8:56 UTC (permalink / raw)
  To: David S . Miller, Maxime Coquelin
  Cc: netdev, linux-kernel, Jose Abreu, Giuseppe Cavallaro,
	Andrew Lunn, Alexandre Torgue, Ong Boon Leong, Voon Weifeng,
	Seow Chen Yong, Mark Gross

EEE should be only be enabled during stmmac_mac_link_up() when the
link are up and being set up properly. set_eee should only do settings
configuration and disabling the eee.

Without this fix, turning on EEE using ethtool will return
"Operation not supported". This is due to the driver is in a dead loop
waiting for eee to be advertised in the for eee to be activated but the
driver will only configure the EEE advertisement after the eee is
activated.

Ethtool should only return "Operation not supported" if there is no EEE
capbility in the MAC controller.

Fixes: 8a7493e58ad6 ("net: stmmac: Fix a race in EEE enable callback")

Signed-off-by: Voon Weifeng <weifeng.voon@intel.com>
---
 .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c  | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
index ac5e8cc5fb9f..430a4b32ec1e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
@@ -675,23 +675,16 @@ static int stmmac_ethtool_op_set_eee(struct net_device *dev,
 	struct stmmac_priv *priv = netdev_priv(dev);
 	int ret;
 
-	if (!edata->eee_enabled) {
+	if (!priv->dma_cap.eee)
+		return -EOPNOTSUPP;
+
+	if (!edata->eee_enabled)
 		stmmac_disable_eee_mode(priv);
-	} else {
-		/* We are asking for enabling the EEE but it is safe
-		 * to verify all by invoking the eee_init function.
-		 * In case of failure it will return an error.
-		 */
-		edata->eee_enabled = stmmac_eee_init(priv);
-		if (!edata->eee_enabled)
-			return -EOPNOTSUPP;
-	}
 
 	ret = phylink_ethtool_set_eee(priv->phylink, edata);
 	if (ret)
 		return ret;
 
-	priv->eee_enabled = edata->eee_enabled;
 	priv->tx_lpi_timer = edata->tx_lpi_timer;
 	return 0;
 }
-- 
2.17.1


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

* Re: [PATCH v1 net] net: stmmac: removed enabling eee in EEE set callback
  2020-09-23  8:56 [PATCH v1 net] net: stmmac: removed enabling eee in EEE set callback Voon Weifeng
@ 2020-09-23 23:37 ` mark gross
  2020-09-24  1:09 ` David Miller
  1 sibling, 0 replies; 3+ messages in thread
From: mark gross @ 2020-09-23 23:37 UTC (permalink / raw)
  To: Voon Weifeng
  Cc: David S . Miller, Maxime Coquelin, netdev, linux-kernel,
	Jose Abreu, Giuseppe Cavallaro, Andrew Lunn, Alexandre Torgue,
	Ong Boon Leong, Seow Chen Yong

Acked-by: Mark Gross <mgross@linux.intel.com>

On Wed, Sep 23, 2020 at 04:56:14PM +0800, Voon Weifeng wrote:
> EEE should be only be enabled during stmmac_mac_link_up() when the
> link are up and being set up properly. set_eee should only do settings
> configuration and disabling the eee.
> 
> Without this fix, turning on EEE using ethtool will return
> "Operation not supported". This is due to the driver is in a dead loop
> waiting for eee to be advertised in the for eee to be activated but the
> driver will only configure the EEE advertisement after the eee is
> activated.
> 
> Ethtool should only return "Operation not supported" if there is no EEE
> capbility in the MAC controller.
> 
> Fixes: 8a7493e58ad6 ("net: stmmac: Fix a race in EEE enable callback")
> 
> Signed-off-by: Voon Weifeng <weifeng.voon@intel.com>
> ---
>  .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c  | 15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> index ac5e8cc5fb9f..430a4b32ec1e 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> @@ -675,23 +675,16 @@ static int stmmac_ethtool_op_set_eee(struct net_device *dev,
>  	struct stmmac_priv *priv = netdev_priv(dev);
>  	int ret;
>  
> -	if (!edata->eee_enabled) {
> +	if (!priv->dma_cap.eee)
> +		return -EOPNOTSUPP;
> +
> +	if (!edata->eee_enabled)
>  		stmmac_disable_eee_mode(priv);
> -	} else {
> -		/* We are asking for enabling the EEE but it is safe
> -		 * to verify all by invoking the eee_init function.
> -		 * In case of failure it will return an error.
> -		 */
> -		edata->eee_enabled = stmmac_eee_init(priv);
> -		if (!edata->eee_enabled)
> -			return -EOPNOTSUPP;
> -	}
>  
>  	ret = phylink_ethtool_set_eee(priv->phylink, edata);
>  	if (ret)
>  		return ret;
>  
> -	priv->eee_enabled = edata->eee_enabled;
>  	priv->tx_lpi_timer = edata->tx_lpi_timer;
>  	return 0;
>  }
> -- 
> 2.17.1
> 

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

* Re: [PATCH v1 net] net: stmmac: removed enabling eee in EEE set callback
  2020-09-23  8:56 [PATCH v1 net] net: stmmac: removed enabling eee in EEE set callback Voon Weifeng
  2020-09-23 23:37 ` mark gross
@ 2020-09-24  1:09 ` David Miller
  1 sibling, 0 replies; 3+ messages in thread
From: David Miller @ 2020-09-24  1:09 UTC (permalink / raw)
  To: weifeng.voon
  Cc: mcoquelin.stm32, netdev, linux-kernel, joabreu, peppe.cavallaro,
	andrew, alexandre.torgue, boon.leong.ong, chen.yong.seow, mgross

From: Voon Weifeng <weifeng.voon@intel.com>
Date: Wed, 23 Sep 2020 16:56:14 +0800

> EEE should be only be enabled during stmmac_mac_link_up() when the
> link are up and being set up properly. set_eee should only do settings
> configuration and disabling the eee.
> 
> Without this fix, turning on EEE using ethtool will return
> "Operation not supported". This is due to the driver is in a dead loop
> waiting for eee to be advertised in the for eee to be activated but the
> driver will only configure the EEE advertisement after the eee is
> activated.
> 
> Ethtool should only return "Operation not supported" if there is no EEE
> capbility in the MAC controller.
> 
> Fixes: 8a7493e58ad6 ("net: stmmac: Fix a race in EEE enable callback")
> 
> Signed-off-by: Voon Weifeng <weifeng.voon@intel.com>

Please do not insert empty lines between Fixes: and other tags.  All tags
are equal and belong together in a group.

I fixed that up, applied the patch, and queued it up for -stable.

Thank you.

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

end of thread, other threads:[~2020-09-24  1:09 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-23  8:56 [PATCH v1 net] net: stmmac: removed enabling eee in EEE set callback Voon Weifeng
2020-09-23 23:37 ` mark gross
2020-09-24  1:09 ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.