diff mbox

[05/13] ftgmac100: Cleanup speed/duplex tracking and fix duplex config

Message ID 20170402033523.9482-6-benh@kernel.crashing.org
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Benjamin Herrenschmidt April 2, 2017, 3:35 a.m. UTC 
Keep track of both the current speed and duplex settings
instead of only speed and properly apply the duplex setting
to the HW.

This reworks the adjust_link() function to also avoid trying
to reconfigure the HW when there is no link and to display
the link state to the user.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 54 +++++++++++++++++++++++++++-----
 1 file changed, 46 insertions(+), 8 deletions(-)

Comments

Andrew Lunn April 2, 2017, 6:28 p.m. UTC | #1 
> +	if (new_speed) {
> +		netdev_info(netdev, "Link up at %d Mbit/s %s duplex\n",
> +			    new_speed,
> +			    phydev->duplex == DUPLEX_FULL ? "full" : "half");
> +	} else if (priv->cur_speed) {
> +		/* No link, just return. Leave the HW alone so it can
> +		 * continue draining the tx ring.
> +		 */
> +		netdev_info(netdev, "Link down\n");
>  		return;

Hi Ben

Please consider using phy_print_status().

       Andrew
Benjamin Herrenschmidt April 2, 2017, 9:03 p.m. UTC | #2 
On Sun, 2017-04-02 at 20:28 +0200, Andrew Lunn wrote:
> > +	if (new_speed) {
> > +		netdev_info(netdev, "Link up at %d Mbit/s %s
> > duplex\n",
> > +			    new_speed,
> > +			    phydev->duplex == DUPLEX_FULL ? "full"
> > : "half");
> > +	} else if (priv->cur_speed) {
> > +		/* No link, just return. Leave the HW alone so it
> > can
> > +		 * continue draining the tx ring.
> > +		 */
> > +		netdev_info(netdev, "Link down\n");
> >  		return;
> 
> Hi Ben
> 
> Please consider using phy_print_status().

Thanks. I didn't know about that one. I'll update this.

Cheers,
Ben.
Benjamin Herrenschmidt April 2, 2017, 9:27 p.m. UTC | #3 
On Sun, 2017-04-02 at 13:35 +1000, Benjamin Herrenschmidt wrote:
> +       } else if (priv->cur_speed) {
> +               /* No link, just return. Leave the HW alone so it can
> +                * continue draining the tx ring.
> +                */
> +               netdev_info(netdev, "Link down\n");
>                 return;
> +       }
>  
> -       priv->old_speed = phydev->speed;
> +       priv->cur_speed = new_speed;
> +       priv->cur_duplex = phydev->duplex;

Finding my own bugs too ... the return above makes us fail
to update priv->cur_speed, thus the driver still thinks we
have a link. Not a huge deal but I'll fix too. (Caused by a
recent re-org in that code).

Cheers,
Ben.
Benjamin Herrenschmidt April 2, 2017, 9:35 p.m. UTC | #4 
On Sun, 2017-04-02 at 20:28 +0200, Andrew Lunn wrote:

> Please consider using phy_print_status().

So in a subsequent (not yet posted) patch, I add Pause support. I
noticed that phy_print_status() doesn't distinguish between full
and partial (asym) pause support.

Is that intentional ?

Otherwise, I'll submit a latch later to update it along with the
patch that adds pause support.

Cheers,
Ben.
diff mbox

Patch

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index cc2271b..219131c 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -77,7 +77,8 @@  struct ftgmac100 {
 	struct mii_bus *mii_bus;
 
 	/* Link management */
-	int old_speed;
+	int cur_speed;
+	int cur_duplex;
 	bool use_ncsi;
 
 	/* Misc */
@@ -210,16 +211,15 @@  static void ftgmac100_init_hw(struct ftgmac100 *priv)
 				 FTGMAC100_MACCR_RXDMA_EN	| \
 				 FTGMAC100_MACCR_TXMAC_EN	| \
 				 FTGMAC100_MACCR_RXMAC_EN	| \
-				 FTGMAC100_MACCR_FULLDUP	| \
 				 FTGMAC100_MACCR_CRC_APD	| \
 				 FTGMAC100_MACCR_RX_RUNT	| \
 				 FTGMAC100_MACCR_RX_BROADPKT)
 
-static void ftgmac100_start_hw(struct ftgmac100 *priv, int speed)
+static void ftgmac100_start_hw(struct ftgmac100 *priv)
 {
 	int maccr = MACCR_ENABLE_ALL;
 
-	switch (speed) {
+	switch (priv->cur_speed) {
 	default:
 	case 10:
 		break;
@@ -233,6 +233,9 @@  static void ftgmac100_start_hw(struct ftgmac100 *priv, int speed)
 		break;
 	}
 
+	if (priv->cur_duplex == DUPLEX_FULL)
+		maccr |= FTGMAC100_MACCR_FULLDUP;
+
 	iowrite32(maccr, priv->base + FTGMAC100_OFFSET_MACCR);
 }
 
@@ -852,12 +855,33 @@  static void ftgmac100_adjust_link(struct net_device *netdev)
 {
 	struct ftgmac100 *priv = netdev_priv(netdev);
 	struct phy_device *phydev = netdev->phydev;
+	int new_speed;
 	int ier;
 
-	if (phydev->speed == priv->old_speed)
+	/* We store "no link" as speed 0 */
+	if (!phydev->link)
+		new_speed = 0;
+	else
+		new_speed = phydev->speed;
+
+	if (phydev->speed == priv->cur_speed &&
+	    phydev->duplex == priv->cur_duplex)
+		return;
+
+	if (new_speed) {
+		netdev_info(netdev, "Link up at %d Mbit/s %s duplex\n",
+			    new_speed,
+			    phydev->duplex == DUPLEX_FULL ? "full" : "half");
+	} else if (priv->cur_speed) {
+		/* No link, just return. Leave the HW alone so it can
+		 * continue draining the tx ring.
+		 */
+		netdev_info(netdev, "Link down\n");
 		return;
+	}
 
-	priv->old_speed = phydev->speed;
+	priv->cur_speed = new_speed;
+	priv->cur_duplex = phydev->duplex;
 
 	ier = ioread32(priv->base + FTGMAC100_OFFSET_IER);
 
@@ -869,7 +893,7 @@  static void ftgmac100_adjust_link(struct net_device *netdev)
 
 	netif_start_queue(netdev);
 	ftgmac100_init_hw(priv);
-	ftgmac100_start_hw(priv, phydev->speed);
+	ftgmac100_start_hw(priv);
 
 	/* re-enable interrupts */
 	iowrite32(ier, priv->base + FTGMAC100_OFFSET_IER);
@@ -1089,6 +1113,20 @@  static int ftgmac100_open(struct net_device *netdev)
 		goto err_irq;
 	}
 
+	/* When using NC-SI we force the speed to 100Mbit/s full duplex,
+	 *
+	 * Otherwise we leave it set to 0 (no link), the link
+	 * message from the PHY layer will handle setting it up to
+	 * something else if needed.
+	 */
+	if (priv->use_ncsi) {
+		priv->cur_duplex = DUPLEX_FULL;
+		priv->cur_speed = SPEED_100;
+	} else {
+		priv->cur_duplex = 0;
+		priv->cur_speed = 0;
+	}
+
 	priv->rx_pointer = 0;
 	priv->tx_clean_pointer = 0;
 	priv->tx_pointer = 0;
@@ -1099,7 +1137,7 @@  static int ftgmac100_open(struct net_device *netdev)
 		goto err_hw;
 
 	ftgmac100_init_hw(priv);
-	ftgmac100_start_hw(priv, priv->use_ncsi ? 100 : 10);
+	ftgmac100_start_hw(priv);
 
 	/* Clear stale interrupts */
 	status = ioread32(priv->base + FTGMAC100_OFFSET_ISR);