diff mbox

[v3,2/4] net: nb8800: Simplify nb8800_pause_config()

Message ID dd0c2014-8ef9-749c-16d3-6a56f4161658@sigmadesigns.com (mailing list archive)
State New, archived
Headers show

Commit Message

Marc Gonzalez Nov. 14, 2017, 10:55 a.m. UTC 
The "flow control enable" bit can be tweaked, even if DMA is enabled.

Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
---
 drivers/net/ethernet/aurora/nb8800.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

Comments

Måns Rullgård Nov. 14, 2017, 12:38 p.m. UTC | #1 
Marc Gonzalez <marc_gonzalez@sigmadesigns.com> writes:

> The "flow control enable" bit can be tweaked, even if DMA is enabled.

No, it can't.  Maybe on some of your newer chips it can, but not on the
older ones.

> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> ---
>  drivers/net/ethernet/aurora/nb8800.c | 18 +-----------------
>  1 file changed, 1 insertion(+), 17 deletions(-)
>
> diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
> index 26f719e2d6ca..09b8001e1ecc 100644
> --- a/drivers/net/ethernet/aurora/nb8800.c
> +++ b/drivers/net/ethernet/aurora/nb8800.c
> @@ -633,7 +633,6 @@ static void nb8800_pause_config(struct net_device *dev)
>  {
>  	struct nb8800_priv *priv = netdev_priv(dev);
>  	struct phy_device *phydev = dev->phydev;
> -	u32 rxcr;
>
>  	if (priv->pause_aneg) {
>  		if (!phydev || !phydev->link)
> @@ -644,22 +643,7 @@ static void nb8800_pause_config(struct net_device *dev)
>  	}
>
>  	nb8800_modb(priv, NB8800_RX_CTL, RX_PAUSE_EN, priv->pause_rx);
> -
> -	rxcr = nb8800_readl(priv, NB8800_RXC_CR);
> -	if (!!(rxcr & RCR_FL) == priv->pause_tx)
> -		return;
> -
> -	if (netif_running(dev)) {
> -		napi_disable(&priv->napi);
> -		netif_tx_lock_bh(dev);
> -		nb8800_dma_stop(dev);
> -		nb8800_modl(priv, NB8800_RXC_CR, RCR_FL, priv->pause_tx);
> -		nb8800_start_rx(dev);
> -		netif_tx_unlock_bh(dev);
> -		napi_enable(&priv->napi);
> -	} else {
> -		nb8800_modl(priv, NB8800_RXC_CR, RCR_FL, priv->pause_tx);
> -	}
> +	nb8800_modl(priv, NB8800_RXC_CR, RCR_FL, priv->pause_tx);
>  }
>
>  static void nb8800_link_reconfigure(struct net_device *dev)
> --
Marc Gonzalez Nov. 14, 2017, 12:56 p.m. UTC | #2 
On 14/11/2017 13:38, Måns Rullgård wrote:

> Marc Gonzalez writes:
> 
>> The "flow control enable" bit can be tweaked, even if DMA is enabled.
> 
> No, it can't.  Maybe on some of your newer chips it can, but not on the
> older ones.

Again, I suppose you are referring to your SMP8642-based board.

Are you saying you tested this patch, and it doesn't work?
What are the symptoms?
Måns Rullgård Nov. 14, 2017, 1:22 p.m. UTC | #3 
Marc Gonzalez <marc_gonzalez@sigmadesigns.com> writes:

> On 14/11/2017 13:38, Måns Rullgård wrote:
>
>> Marc Gonzalez writes:
>> 
>>> The "flow control enable" bit can be tweaked, even if DMA is enabled.
>> 
>> No, it can't.  Maybe on some of your newer chips it can, but not on the
>> older ones.
>
> Again, I suppose you are referring to your SMP8642-based board.
>
> Are you saying you tested this patch, and it doesn't work?
> What are the symptoms?

I didn't test that patch per se.  I did initially try simply changing
that bit, and this didn't work.  Either it had no effect, or it locked
up the controller, I forget which.  The manual explicitly states that
writes are forbidden while the DMA enabled bit is set.

If shutting down the DMA really isn't possible, I would rather just
allow changing the flow control setting only when the interface is
stopped.  The normal case of getting the initial setting from the
auto-negotiation will still work properly.  It just won't be possible to
change it while the link is active.
diff mbox

Patch

diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
index 26f719e2d6ca..09b8001e1ecc 100644
--- a/drivers/net/ethernet/aurora/nb8800.c
+++ b/drivers/net/ethernet/aurora/nb8800.c
@@ -633,7 +633,6 @@  static void nb8800_pause_config(struct net_device *dev)
 {
 	struct nb8800_priv *priv = netdev_priv(dev);
 	struct phy_device *phydev = dev->phydev;
-	u32 rxcr;
 
 	if (priv->pause_aneg) {
 		if (!phydev || !phydev->link)
@@ -644,22 +643,7 @@  static void nb8800_pause_config(struct net_device *dev)
 	}
 
 	nb8800_modb(priv, NB8800_RX_CTL, RX_PAUSE_EN, priv->pause_rx);
-
-	rxcr = nb8800_readl(priv, NB8800_RXC_CR);
-	if (!!(rxcr & RCR_FL) == priv->pause_tx)
-		return;
-
-	if (netif_running(dev)) {
-		napi_disable(&priv->napi);
-		netif_tx_lock_bh(dev);
-		nb8800_dma_stop(dev);
-		nb8800_modl(priv, NB8800_RXC_CR, RCR_FL, priv->pause_tx);
-		nb8800_start_rx(dev);
-		netif_tx_unlock_bh(dev);
-		napi_enable(&priv->napi);
-	} else {
-		nb8800_modl(priv, NB8800_RXC_CR, RCR_FL, priv->pause_tx);
-	}
+	nb8800_modl(priv, NB8800_RXC_CR, RCR_FL, priv->pause_tx);
 }
 
 static void nb8800_link_reconfigure(struct net_device *dev)