[PATCH 0/1] [SRU] [B/raspi2] arm64: lan78xx fails to initialize

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/1] [SRU] [B/raspi2] arm64: lan78xx fails to initialize

Paolo Pisati-5
BugLink: https://bugs.launchpad.net/bugs/1806108

[Impact]

Booting this image:

http://cdimage.ubuntu.com/releases/18.04/beta/ubuntu-18.04-beta-preinstalled-server-arm64+raspi3.img.xz

on a RaspberryPi 3B+, the ethernet port is not working.

Moving the EEE enabling into the PHY initialisation function,
immediately after the connection to the PHY is established, which is
long before phy_start is called, prevents the renegotiation and avoids
the slowdown and failure.

QA and Foundation have confirmed applying this patches fix the problem listed
in #LP1806108.

[Fix]

Apply the attached patch and recompile

[Test]

Install a patched kernel and reboot, if the image boots and eth0 is
alive, the fix is working

[Regression potential]

The patch is a cherry-pick (with some mechanical modification) from Cosmic and
it fixes an isolated bug, so i say it's low to none.

Phil Elwell (1):
  lan78xx: Move enabling of EEE into PHY init code

 drivers/net/usb/lan78xx.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/1] lan78xx: Move enabling of EEE into PHY init code

Paolo Pisati-5
From: Phil Elwell <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1806108

Enable EEE mode as soon as possible after connecting to the PHY, and
before phy_start. This avoids a second link negotiation, which speeds
up booting and stops the interface failing to become ready.

See: https://github.com/raspberrypi/linux/issues/2437

Signed-off-by: Phil Elwell <[hidden email]>
(cherry picked from commit 770de6c8d6b771933c5cfb24708b6027e6ed8d43)
Signed-off-by: Paolo Pisati <[hidden email]>
(cherry picked from commit 66524920fa205f6670171e8a38e582b7d9abea38)
Signed-off-by: Paolo Pisati <[hidden email]>
---
 drivers/net/usb/lan78xx.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 77a186befd07..4e0f223435d4 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -2093,6 +2093,22 @@ static int lan78xx_phy_init(struct lan78xx_net *dev)
  mii_adv = (u32)mii_advertise_flowctrl(dev->fc_request_control);
  phydev->advertising |= mii_adv_to_ethtool_adv_t(mii_adv);
 
+ if (of_property_read_bool(dev->udev->dev.of_node,
+  "microchip,eee-enabled")) {
+ struct ethtool_eee edata;
+ memset(&edata, 0, sizeof(edata));
+ edata.cmd = ETHTOOL_SEEE;
+ edata.advertised = ADVERTISED_1000baseT_Full |
+   ADVERTISED_100baseT_Full;
+ edata.eee_enabled = true;
+ edata.tx_lpi_enabled = true;
+ if (of_property_read_u32(dev->udev->dev.of_node,
+ "microchip,tx-lpi-timer",
+ &edata.tx_lpi_timer))
+ edata.tx_lpi_timer = 600; /* non-aggressive */
+ (void)lan78xx_set_eee(dev->net, &edata);
+ }
+
  /* Set LED modes:
  * led: 0=link/activity          1=link1000/activity
  *      2=link100/activity       3=link10/activity
@@ -2566,22 +2582,6 @@ static int lan78xx_open(struct net_device *net)
 
  netif_dbg(dev, ifup, dev->net, "phy initialised successfully");
 
- if (of_property_read_bool(dev->udev->dev.of_node,
-  "microchip,eee-enabled")) {
- struct ethtool_eee edata;
- memset(&edata, 0, sizeof(edata));
- edata.cmd = ETHTOOL_SEEE;
- edata.advertised = ADVERTISED_1000baseT_Full |
-   ADVERTISED_100baseT_Full;
- edata.eee_enabled = true;
- edata.tx_lpi_enabled = true;
- if (of_property_read_u32(dev->udev->dev.of_node,
- "microchip,tx-lpi-timer",
- &edata.tx_lpi_timer))
- edata.tx_lpi_timer = 600; /* non-aggressive */
- (void)lan78xx_set_eee(net, &edata);
- }
-
  /* for Link Check */
  if (dev->urb_intr) {
  ret = usb_submit_urb(dev->urb_intr, GFP_KERNEL);
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/1] lan78xx: Move enabling of EEE into PHY init code

Colin Ian King-2
On 05/12/2018 16:42, Paolo Pisati wrote:

> From: Phil Elwell <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1806108
>
> Enable EEE mode as soon as possible after connecting to the PHY, and
> before phy_start. This avoids a second link negotiation, which speeds
> up booting and stops the interface failing to become ready.
>
> See: https://github.com/raspberrypi/linux/issues/2437
>
> Signed-off-by: Phil Elwell <[hidden email]>
> (cherry picked from commit 770de6c8d6b771933c5cfb24708b6027e6ed8d43)
> Signed-off-by: Paolo Pisati <[hidden email]>
> (cherry picked from commit 66524920fa205f6670171e8a38e582b7d9abea38)
> Signed-off-by: Paolo Pisati <[hidden email]>

I don't understand the double cherry pick info above

> ---
>  drivers/net/usb/lan78xx.c | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index 77a186befd07..4e0f223435d4 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> @@ -2093,6 +2093,22 @@ static int lan78xx_phy_init(struct lan78xx_net *dev)
>   mii_adv = (u32)mii_advertise_flowctrl(dev->fc_request_control);
>   phydev->advertising |= mii_adv_to_ethtool_adv_t(mii_adv);
>  
> + if (of_property_read_bool(dev->udev->dev.of_node,
> +  "microchip,eee-enabled")) {
> + struct ethtool_eee edata;
> + memset(&edata, 0, sizeof(edata));
> + edata.cmd = ETHTOOL_SEEE;
> + edata.advertised = ADVERTISED_1000baseT_Full |
> +   ADVERTISED_100baseT_Full;
> + edata.eee_enabled = true;
> + edata.tx_lpi_enabled = true;
> + if (of_property_read_u32(dev->udev->dev.of_node,
> + "microchip,tx-lpi-timer",
> + &edata.tx_lpi_timer))
> + edata.tx_lpi_timer = 600; /* non-aggressive */
> + (void)lan78xx_set_eee(dev->net, &edata);
> + }
> +
>   /* Set LED modes:
>   * led: 0=link/activity          1=link1000/activity
>   *      2=link100/activity       3=link10/activity
> @@ -2566,22 +2582,6 @@ static int lan78xx_open(struct net_device *net)
>  
>   netif_dbg(dev, ifup, dev->net, "phy initialised successfully");
>  
> - if (of_property_read_bool(dev->udev->dev.of_node,
> -  "microchip,eee-enabled")) {
> - struct ethtool_eee edata;
> - memset(&edata, 0, sizeof(edata));
> - edata.cmd = ETHTOOL_SEEE;
> - edata.advertised = ADVERTISED_1000baseT_Full |
> -   ADVERTISED_100baseT_Full;
> - edata.eee_enabled = true;
> - edata.tx_lpi_enabled = true;
> - if (of_property_read_u32(dev->udev->dev.of_node,
> - "microchip,tx-lpi-timer",
> - &edata.tx_lpi_timer))
> - edata.tx_lpi_timer = 600; /* non-aggressive */
> - (void)lan78xx_set_eee(net, &edata);
> - }
> -
>   /* for Link Check */
>   if (dev->urb_intr) {
>   ret = usb_submit_urb(dev->urb_intr, GFP_KERNEL);
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/1] lan78xx: Move enabling of EEE into PHY init code

Colin Ian King-2
In reply to this post by Paolo Pisati-5
On 05/12/2018 16:42, Paolo Pisati wrote:

> From: Phil Elwell <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1806108
>
> Enable EEE mode as soon as possible after connecting to the PHY, and
> before phy_start. This avoids a second link negotiation, which speeds
> up booting and stops the interface failing to become ready.
>
> See: https://github.com/raspberrypi/linux/issues/2437
>
> Signed-off-by: Phil Elwell <[hidden email]>
> (cherry picked from commit 770de6c8d6b771933c5cfb24708b6027e6ed8d43)
> Signed-off-by: Paolo Pisati <[hidden email]>
> (cherry picked from commit 66524920fa205f6670171e8a38e582b7d9abea38)
> Signed-off-by: Paolo Pisati <[hidden email]>

Ah, the 2nd cherry pick is from a cherry pick. Got it.

> ---
>  drivers/net/usb/lan78xx.c | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index 77a186befd07..4e0f223435d4 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> @@ -2093,6 +2093,22 @@ static int lan78xx_phy_init(struct lan78xx_net *dev)
>   mii_adv = (u32)mii_advertise_flowctrl(dev->fc_request_control);
>   phydev->advertising |= mii_adv_to_ethtool_adv_t(mii_adv);
>  
> + if (of_property_read_bool(dev->udev->dev.of_node,
> +  "microchip,eee-enabled")) {
> + struct ethtool_eee edata;
> + memset(&edata, 0, sizeof(edata));
> + edata.cmd = ETHTOOL_SEEE;
> + edata.advertised = ADVERTISED_1000baseT_Full |
> +   ADVERTISED_100baseT_Full;
> + edata.eee_enabled = true;
> + edata.tx_lpi_enabled = true;
> + if (of_property_read_u32(dev->udev->dev.of_node,
> + "microchip,tx-lpi-timer",
> + &edata.tx_lpi_timer))
> + edata.tx_lpi_timer = 600; /* non-aggressive */
> + (void)lan78xx_set_eee(dev->net, &edata);
> + }
> +
>   /* Set LED modes:
>   * led: 0=link/activity          1=link1000/activity
>   *      2=link100/activity       3=link10/activity
> @@ -2566,22 +2582,6 @@ static int lan78xx_open(struct net_device *net)
>  
>   netif_dbg(dev, ifup, dev->net, "phy initialised successfully");
>  
> - if (of_property_read_bool(dev->udev->dev.of_node,
> -  "microchip,eee-enabled")) {
> - struct ethtool_eee edata;
> - memset(&edata, 0, sizeof(edata));
> - edata.cmd = ETHTOOL_SEEE;
> - edata.advertised = ADVERTISED_1000baseT_Full |
> -   ADVERTISED_100baseT_Full;
> - edata.eee_enabled = true;
> - edata.tx_lpi_enabled = true;
> - if (of_property_read_u32(dev->udev->dev.of_node,
> - "microchip,tx-lpi-timer",
> - &edata.tx_lpi_timer))
> - edata.tx_lpi_timer = 600; /* non-aggressive */
> - (void)lan78xx_set_eee(net, &edata);
> - }
> -
>   /* for Link Check */
>   if (dev->urb_intr) {
>   ret = usb_submit_urb(dev->urb_intr, GFP_KERNEL);
>

Seems sane to me.

Acked-by: Colin Ian King <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/1] lan78xx: Move enabling of EEE into PHY init code

Paolo Pisati-5
In reply to this post by Colin Ian King-2
This is a cherry-pick from Cosmic, so the first cherry-pick line is
when the patch originally landed in Cosmic from the Raspberry Github
tree, and the second cherry-pick line is me picking the patch from
Cosmic and landing it in Bionic for this specific bug.
On Wed, Dec 5, 2018 at 5:50 PM Colin Ian King <[hidden email]> wrote:

>
> On 05/12/2018 16:42, Paolo Pisati wrote:
> > From: Phil Elwell <[hidden email]>
> >
> > BugLink: https://bugs.launchpad.net/bugs/1806108
> >
> > Enable EEE mode as soon as possible after connecting to the PHY, and
> > before phy_start. This avoids a second link negotiation, which speeds
> > up booting and stops the interface failing to become ready.
> >
> > See: https://github.com/raspberrypi/linux/issues/2437
> >
> > Signed-off-by: Phil Elwell <[hidden email]>
> > (cherry picked from commit 770de6c8d6b771933c5cfb24708b6027e6ed8d43)
> > Signed-off-by: Paolo Pisati <[hidden email]>
> > (cherry picked from commit 66524920fa205f6670171e8a38e582b7d9abea38)
> > Signed-off-by: Paolo Pisati <[hidden email]>
>
> I don't understand the double cherry pick info above
>
> > ---
> >  drivers/net/usb/lan78xx.c | 32 ++++++++++++++++----------------
> >  1 file changed, 16 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> > index 77a186befd07..4e0f223435d4 100644
> > --- a/drivers/net/usb/lan78xx.c
> > +++ b/drivers/net/usb/lan78xx.c
> > @@ -2093,6 +2093,22 @@ static int lan78xx_phy_init(struct lan78xx_net *dev)
> >       mii_adv = (u32)mii_advertise_flowctrl(dev->fc_request_control);
> >       phydev->advertising |= mii_adv_to_ethtool_adv_t(mii_adv);
> >
> > +     if (of_property_read_bool(dev->udev->dev.of_node,
> > +                               "microchip,eee-enabled")) {
> > +             struct ethtool_eee edata;
> > +             memset(&edata, 0, sizeof(edata));
> > +             edata.cmd = ETHTOOL_SEEE;
> > +             edata.advertised = ADVERTISED_1000baseT_Full |
> > +                                ADVERTISED_100baseT_Full;
> > +             edata.eee_enabled = true;
> > +             edata.tx_lpi_enabled = true;
> > +             if (of_property_read_u32(dev->udev->dev.of_node,
> > +                                      "microchip,tx-lpi-timer",
> > +                                      &edata.tx_lpi_timer))
> > +                     edata.tx_lpi_timer = 600; /* non-aggressive */
> > +             (void)lan78xx_set_eee(dev->net, &edata);
> > +     }
> > +
> >       /* Set LED modes:
> >        * led: 0=link/activity          1=link1000/activity
> >        *      2=link100/activity       3=link10/activity
> > @@ -2566,22 +2582,6 @@ static int lan78xx_open(struct net_device *net)
> >
> >       netif_dbg(dev, ifup, dev->net, "phy initialised successfully");
> >
> > -     if (of_property_read_bool(dev->udev->dev.of_node,
> > -                               "microchip,eee-enabled")) {
> > -             struct ethtool_eee edata;
> > -             memset(&edata, 0, sizeof(edata));
> > -             edata.cmd = ETHTOOL_SEEE;
> > -             edata.advertised = ADVERTISED_1000baseT_Full |
> > -                                ADVERTISED_100baseT_Full;
> > -             edata.eee_enabled = true;
> > -             edata.tx_lpi_enabled = true;
> > -             if (of_property_read_u32(dev->udev->dev.of_node,
> > -                                      "microchip,tx-lpi-timer",
> > -                                      &edata.tx_lpi_timer))
> > -                     edata.tx_lpi_timer = 600; /* non-aggressive */
> > -             (void)lan78xx_set_eee(net, &edata);
> > -     }
> > -
> >       /* for Link Check */
> >       if (dev->urb_intr) {
> >               ret = usb_submit_urb(dev->urb_intr, GFP_KERNEL);
> >
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team



--
bye,
p.

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK/Cmnt: [PATCH 1/1] lan78xx: Move enabling of EEE into PHY init code

Stefan Bader-2
In reply to this post by Paolo Pisati-5
On 05.12.18 17:42, Paolo Pisati wrote:

> From: Phil Elwell <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1806108
>
> Enable EEE mode as soon as possible after connecting to the PHY, and
> before phy_start. This avoids a second link negotiation, which speeds
> up booting and stops the interface failing to become ready.
>
> See: https://github.com/raspberrypi/linux/issues/2437
>
> Signed-off-by: Phil Elwell <[hidden email]>
> (cherry picked from commit 770de6c8d6b771933c5cfb24708b6027e6ed8d43)
> Signed-off-by: Paolo Pisati <[hidden email]>
> (cherry picked from commit 66524920fa205f6670171e8a38e582b7d9abea38)
> Signed-off-by: Paolo Pisati <[hidden email]>
Acked-by: Stefan Bader <[hidden email]>
> ---

I would add "cosmic" to the second pick on commit, but also where is the first
pick from? Does not seem to be linus tree.

-Stefan

>  drivers/net/usb/lan78xx.c | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index 77a186befd07..4e0f223435d4 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> @@ -2093,6 +2093,22 @@ static int lan78xx_phy_init(struct lan78xx_net *dev)
>   mii_adv = (u32)mii_advertise_flowctrl(dev->fc_request_control);
>   phydev->advertising |= mii_adv_to_ethtool_adv_t(mii_adv);
>  
> + if (of_property_read_bool(dev->udev->dev.of_node,
> +  "microchip,eee-enabled")) {
> + struct ethtool_eee edata;
> + memset(&edata, 0, sizeof(edata));
> + edata.cmd = ETHTOOL_SEEE;
> + edata.advertised = ADVERTISED_1000baseT_Full |
> +   ADVERTISED_100baseT_Full;
> + edata.eee_enabled = true;
> + edata.tx_lpi_enabled = true;
> + if (of_property_read_u32(dev->udev->dev.of_node,
> + "microchip,tx-lpi-timer",
> + &edata.tx_lpi_timer))
> + edata.tx_lpi_timer = 600; /* non-aggressive */
> + (void)lan78xx_set_eee(dev->net, &edata);
> + }
> +
>   /* Set LED modes:
>   * led: 0=link/activity          1=link1000/activity
>   *      2=link100/activity       3=link10/activity
> @@ -2566,22 +2582,6 @@ static int lan78xx_open(struct net_device *net)
>  
>   netif_dbg(dev, ifup, dev->net, "phy initialised successfully");
>  
> - if (of_property_read_bool(dev->udev->dev.of_node,
> -  "microchip,eee-enabled")) {
> - struct ethtool_eee edata;
> - memset(&edata, 0, sizeof(edata));
> - edata.cmd = ETHTOOL_SEEE;
> - edata.advertised = ADVERTISED_1000baseT_Full |
> -   ADVERTISED_100baseT_Full;
> - edata.eee_enabled = true;
> - edata.tx_lpi_enabled = true;
> - if (of_property_read_u32(dev->udev->dev.of_node,
> - "microchip,tx-lpi-timer",
> - &edata.tx_lpi_timer))
> - edata.tx_lpi_timer = 600; /* non-aggressive */
> - (void)lan78xx_set_eee(net, &edata);
> - }
> -
>   /* for Link Check */
>   if (dev->urb_intr) {
>   ret = usb_submit_urb(dev->urb_intr, GFP_KERNEL);
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: ACK/Cmnt: [PATCH 1/1] lan78xx: Move enabling of EEE into PHY init code

Paolo Pisati-5
On Wed, Dec 5, 2018 at 5:57 PM Stefan Bader <[hidden email]> wrote:
>
> I would add "cosmic" to the second pick on commit, but also where is the first
> pick from? Does not seem to be linus tree.

For the Bionic point of view, it comes from Cosmic/raspi2 (since
that's where i'm cherry-picking), but originally it came from
Raspberry Pi's github tree / rpi-4.18.y branch

They keep rebasing so that sha1 is no more, but here it is:
https://github.com/raspberrypi/linux/commit/4efb959af4904f09810f4f98a65a4dbc91f0f0d2

commit 4efb959af4904f09810f4f98a65a4dbc91f0f0d2
Author: Phil Elwell <[hidden email]>
Date:   Thu Apr 5 14:46:11 2018 +0100

    lan78xx: Move enabling of EEE into PHY init code

    Enable EEE mode as soon as possible after connecting to the PHY, and
    before phy_start. This avoids a second link negotiation, which speeds
    up booting and stops the interface failing to become ready.

    See: https://github.com/raspberrypi/linux/issues/2437

    Signed-off-by: Phil Elwell <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED: [PATCH 0/1] [SRU] [B/raspi2] arm64: lan78xx fails to initialize

Stefan Bader-2
In reply to this post by Paolo Pisati-5
On 05.12.18 17:41, Paolo Pisati wrote:

> BugLink: https://bugs.launchpad.net/bugs/1806108
>
> [Impact]
>
> Booting this image:
>
> http://cdimage.ubuntu.com/releases/18.04/beta/ubuntu-18.04-beta-preinstalled-server-arm64+raspi3.img.xz
>
> on a RaspberryPi 3B+, the ethernet port is not working.
>
> Moving the EEE enabling into the PHY initialisation function,
> immediately after the connection to the PHY is established, which is
> long before phy_start is called, prevents the renegotiation and avoids
> the slowdown and failure.
>
> QA and Foundation have confirmed applying this patches fix the problem listed
> in #LP1806108.
>
> [Fix]
>
> Apply the attached patch and recompile
>
> [Test]
>
> Install a patched kernel and reboot, if the image boots and eth0 is
> alive, the fix is working
>
> [Regression potential]
>
> The patch is a cherry-pick (with some mechanical modification) from Cosmic and
> it fixes an isolated bug, so i say it's low to none.
>
> Phil Elwell (1):
>   lan78xx: Move enabling of EEE into PHY init code
>
>  drivers/net/usb/lan78xx.c | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
>
Applied (with s-o-b updates to bionic/raspi2. Thanks.

-Stefan


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment