Patchwork net: fec: restart the FEC when PHY speed changes

login
register
mail settings
Submitter Lucas Stach
Date March 14, 2013, 3:12 p.m.
Message ID <1363273921-5035-1-git-send-email-l.stach@pengutronix.de>
Download mbox | patch
Permalink /patch/227677/
State Accepted
Delegated to: David Miller
Headers show

Comments

Lucas Stach - March 14, 2013, 3:12 p.m.
Proviously we would only restart the FEC when PHY link or duplex state
changed. PHY does not always bring down the link for speed changes, in
which case we would not detect any change and keep FEC running.

Switching link speed without restarting the FEC results in the FEC being
stuck in an indefinite state, generating error conditions for every
packet.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/net/ethernet/freescale/fec.c |   26 +++++++++++++++-----------
 drivers/net/ethernet/freescale/fec.h |    1 +
 2 files changed, 16 insertions(+), 11 deletions(-)
Frank Li - March 15, 2013, 4:37 a.m.
Acked

2013/3/14 Lucas Stach <l.stach@pengutronix.de>:
> Proviously we would only restart the FEC when PHY link or duplex state
> changed. PHY does not always bring down the link for speed changes, in
> which case we would not detect any change and keep FEC running.
>
> Switching link speed without restarting the FEC results in the FEC being
> stuck in an indefinite state, generating error conditions for every
> packet.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  drivers/net/ethernet/freescale/fec.c |   26 +++++++++++++++-----------
>  drivers/net/ethernet/freescale/fec.h |    1 +
>  2 files changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
> index 069a155..61d2e62 100644
> --- a/drivers/net/ethernet/freescale/fec.c
> +++ b/drivers/net/ethernet/freescale/fec.c
> @@ -934,24 +934,28 @@ static void fec_enet_adjust_link(struct net_device *ndev)
>                 goto spin_unlock;
>         }
>
> -       /* Duplex link change */
>         if (phy_dev->link) {
> -               if (fep->full_duplex != phy_dev->duplex) {
> -                       fec_restart(ndev, phy_dev->duplex);
> -                       /* prevent unnecessary second fec_restart() below */
> +               if (!fep->link) {
>                         fep->link = phy_dev->link;
>                         status_change = 1;
>                 }
> -       }
>
> -       /* Link on or off change */
> -       if (phy_dev->link != fep->link) {
> -               fep->link = phy_dev->link;
> -               if (phy_dev->link)
> +               if (fep->full_duplex != phy_dev->duplex)
> +                       status_change = 1;
> +
> +               if (phy_dev->speed != fep->speed) {
> +                       fep->speed = phy_dev->speed;
> +                       status_change = 1;
> +               }
> +
> +               /* if any of the above changed restart the FEC */
> +               if (status_change)
>                         fec_restart(ndev, phy_dev->duplex);
> -               else
> +       } else {
> +               if (fep->link) {
>                         fec_stop(ndev);
> -               status_change = 1;
> +                       status_change = 1;
> +               }
>         }
>
>  spin_unlock:
> diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
> index f539007..eb43729 100644
> --- a/drivers/net/ethernet/freescale/fec.h
> +++ b/drivers/net/ethernet/freescale/fec.h
> @@ -240,6 +240,7 @@ struct fec_enet_private {
>         phy_interface_t phy_interface;
>         int     link;
>         int     full_duplex;
> +       int     speed;
>         struct  completion mdio_done;
>         int     irq[FEC_IRQ_NUM];
>         int     bufdesc_ex;
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - March 15, 2013, 12:47 p.m.
From: Lucas Stach <l.stach@pengutronix.de>
Date: Thu, 14 Mar 2013 16:12:01 +0100

> Proviously we would only restart the FEC when PHY link or duplex state
> changed. PHY does not always bring down the link for speed changes, in
> which case we would not detect any change and keep FEC running.
> 
> Switching link speed without restarting the FEC results in the FEC being
> stuck in an indefinite state, generating error conditions for every
> packet.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Applied.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
index 069a155..61d2e62 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -934,24 +934,28 @@  static void fec_enet_adjust_link(struct net_device *ndev)
 		goto spin_unlock;
 	}
 
-	/* Duplex link change */
 	if (phy_dev->link) {
-		if (fep->full_duplex != phy_dev->duplex) {
-			fec_restart(ndev, phy_dev->duplex);
-			/* prevent unnecessary second fec_restart() below */
+		if (!fep->link) {
 			fep->link = phy_dev->link;
 			status_change = 1;
 		}
-	}
 
-	/* Link on or off change */
-	if (phy_dev->link != fep->link) {
-		fep->link = phy_dev->link;
-		if (phy_dev->link)
+		if (fep->full_duplex != phy_dev->duplex)
+			status_change = 1;
+
+		if (phy_dev->speed != fep->speed) {
+			fep->speed = phy_dev->speed;
+			status_change = 1;
+		}
+
+		/* if any of the above changed restart the FEC */
+		if (status_change)
 			fec_restart(ndev, phy_dev->duplex);
-		else
+	} else {
+		if (fep->link) {
 			fec_stop(ndev);
-		status_change = 1;
+			status_change = 1;
+		}
 	}
 
 spin_unlock:
diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index f539007..eb43729 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -240,6 +240,7 @@  struct fec_enet_private {
 	phy_interface_t	phy_interface;
 	int	link;
 	int	full_duplex;
+	int	speed;
 	struct	completion mdio_done;
 	int	irq[FEC_IRQ_NUM];
 	int	bufdesc_ex;