PHY reset may still be asserted during MDIO probe

PHY reset may still be asserted during MDIO probe

[Geert Uytterhoeven]

Hi all,

I'm investigating a network failure after kexec on the Renesas Koelsch
and Salvator-XS development boards, using the sh-eth or ravb driver.

During normal boot, the Ethernet interface is working fine:

    libphy: get_phy_c22_id:814: sh_mii: mdiobus_read() MII_PHYSID1 returned 34
    libphy: get_phy_c22_id:824: sh_mii: mdiobus_read() MII_PHYSID2 returned 5431
    libphy: get_phy_c22_id:832: sh_mii: phy_id = 0x00221537
    libphy: get_phy_device:895: sh_mii: get_phy_c22_id() returned 0
    fwnode_mdiobus_register_phy:109: sh_mii: get_phy_device() returned (ptrval)
    fwnode_mdiobus_phy_device_register:46: sh_mii: fwnode_irq_get() returned 191
    libphy: mdiobus_register_gpiod:48: mdiodev->reset_gpio = (ptrval)
    mdio_bus ee700000.ethernet-ffffffff:01:
mdiobus_register_device:88: assert MDIO reset
    libphy: mdio_device_reset:124: calling gpiod_set_value_cansleep(..., 1)
    mdio_bus ee700000.ethernet-ffffffff:01: phy_device_register:931:
deassert PHY reset
    libphy: mdio_device_reset:124: calling gpiod_set_value_cansleep(..., 0)
    Micrel KSZ8041RNLI ee700000.ethernet-ffffffff:01: phy_probe:3026:
deassert PHY reset
    libphy: mdio_device_reset:124: calling gpiod_set_value_cansleep(..., 0)
    fwnode_mdiobus_phy_device_register:75: sh_mii:
phy_device_register() returned 0
    fwnode_mdiobus_register_phy:137: sh_mii:
fwnode_mdiobus_phy_device_register() returned 0
    of_mdiobus_register:188: of_mdiobus_register_phy(sh_mii,
/soc/ethernet@ee700000/ethernet-phy@1, 1) returned 0
    sh-eth ee700000.ethernet eth0: Base address at 0xee700000,
2e:09:0a:00:6d:85, IRQ 126.

When using kexec, the PHY reset is asserted before starting the
new kernel:

    Micrel KSZ8041RNLI ee700000.ethernet-ffffffff:01: phy_detach:1759:
assert PHY reset
    libphy: mdio_device_reset:124: calling gpiod_set_value_cansleep(..., 1)
    kexec_core: Starting new kernel
    Bye!

The new kernel fails to probe the PHY, as the PHY reset is still
asserted:

    libphy: get_phy_c22_id:814: sh_mii: mdiobus_read() MII_PHYSID1
returned 65535
    libphy: get_phy_c22_id:824: sh_mii: mdiobus_read() MII_PHYSID2
returned 65535
    libphy: get_phy_c22_id:832: sh_mii: phy_id = 0xffffffff
    libphy: get_phy_device:895: sh_mii: get_phy_c22_id() returned -19
    fwnode_mdiobus_register_phy:109: sh_mii: get_phy_device() returned -ENODEV
    of_mdiobus_register:188: of_mdiobus_register_phy(sh_mii,
/soc/ethernet@ee700000/ethernet-phy@1, 1) returned -19
    mdio_bus ee700000.ethernet-ffffffff: MDIO device at address 1 is missing.
    sh-eth ee700000.ethernet eth0: Base address at 0xee700000,
2e:09:0a:00:6d:85, IRQ 126.

This issue can also be reproduced using unbind:

    # echo ee700000.ethernet > /sys/bus/platform/drivers/sh-eth/unbind
    sh-eth ee700000.ethernet eth0: Link is Down
    Micrel KSZ8041RNLI ee700000.ethernet-ffffffff:01: phy_detach:1759:
assert PHY reset
    libphy: mdio_device_reset:124: calling gpiod_set_value_cansleep(..., 1)
    Micrel KSZ8041RNLI ee700000.ethernet-ffffffff:01: phy_remove:3120:
assert PHY reset
    libphy: mdio_device_reset:124: calling gpiod_set_value_cansleep(..., 1)
    mdio_bus ee700000.ethernet-ffffffff:01: phy_device_remove:974:
assert PHY reset
    libphy: mdio_device_reset:124: calling gpiod_set_value_cansleep(..., 1)

and bind:

    # echo ee700000.ethernet > /sys/bus/platform/drivers/sh-eth/bind
    (same log as kexec boot)

I think fwnode_mdiobus_register_phy() should do the PHY reset (assert +
deassert) before calling get_phy_device(), but currently that happens
in phy_device_register(), which is called later.

Thanks for your comments!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: PHY reset may still be asserted during MDIO probe

[Russell King (Oracle)]

On Fri, Jul 09, 2021 at 05:33:36PM +0200, Geert Uytterhoeven wrote:
> Hi all,
> 
> I'm investigating a network failure after kexec on the Renesas Koelsch
> and Salvator-XS development boards, using the sh-eth or ravb driver.

Personally, I've never liked the reset support at PHY device level due
to problems like the one you've identified here. I've tended to use the
bus-level reset in preference to the PHY-level reset, particularly
because when you have multiple PHYs on the bus all sharing a common
reset, it seems to be the most sensible approach - and I see a single
PHY as no different from multiple PHYs on the bus.

However, I can see the argument for using the PHY level, but as you
note, that can create chicken and egg issues. I'm not entirely sure
why we decide to hold a PHY in reset when we've found it but not
started to make use of it - we don't do that with other devices in
the system. Why are PHYs special?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: PHY reset may still be asserted during MDIO probe

[Andrew Lunn]

On Fri, Jul 09, 2021 at 05:33:36PM +0200, Geert Uytterhoeven wrote:
> Hi all,
> 
> I'm investigating a network failure after kexec on the Renesas Koelsch
> and Salvator-XS development boards, using the sh-eth or ravb driver.
> 
> During normal boot, the Ethernet interface is working fine:
> 
>     libphy: get_phy_c22_id:814: sh_mii: mdiobus_read() MII_PHYSID1 returned 34
>     libphy: get_phy_c22_id:824: sh_mii: mdiobus_read() MII_PHYSID2 returned 5431
>     libphy: get_phy_c22_id:832: sh_mii: phy_id = 0x00221537
>     libphy: get_phy_device:895: sh_mii: get_phy_c22_id() returned 0
>     fwnode_mdiobus_register_phy:109: sh_mii: get_phy_device() returned (ptrval)
>     fwnode_mdiobus_phy_device_register:46: sh_mii: fwnode_irq_get() returned 191
>     libphy: mdiobus_register_gpiod:48: mdiodev->reset_gpio = (ptrval)
>     mdio_bus ee700000.ethernet-ffffffff:01:
> mdiobus_register_device:88: assert MDIO reset
>     libphy: mdio_device_reset:124: calling gpiod_set_value_cansleep(..., 1)
>     mdio_bus ee700000.ethernet-ffffffff:01: phy_device_register:931:
> deassert PHY reset
>     libphy: mdio_device_reset:124: calling gpiod_set_value_cansleep(..., 0)
>     Micrel KSZ8041RNLI ee700000.ethernet-ffffffff:01: phy_probe:3026:
> deassert PHY reset
>     libphy: mdio_device_reset:124: calling gpiod_set_value_cansleep(..., 0)
>     fwnode_mdiobus_phy_device_register:75: sh_mii:
> phy_device_register() returned 0
>     fwnode_mdiobus_register_phy:137: sh_mii:
> fwnode_mdiobus_phy_device_register() returned 0
>     of_mdiobus_register:188: of_mdiobus_register_phy(sh_mii,
> /soc/ethernet@ee700000/ethernet-phy@1, 1) returned 0
>     sh-eth ee700000.ethernet eth0: Base address at 0xee700000,
> 2e:09:0a:00:6d:85, IRQ 126.
> 
> When using kexec, the PHY reset is asserted before starting the
> new kernel:
> 
>     Micrel KSZ8041RNLI ee700000.ethernet-ffffffff:01: phy_detach:1759:
> assert PHY reset
>     libphy: mdio_device_reset:124: calling gpiod_set_value_cansleep(..., 1)
>     kexec_core: Starting new kernel
>     Bye!
> 
> The new kernel fails to probe the PHY, as the PHY reset is still
> asserted:
> 
>     libphy: get_phy_c22_id:814: sh_mii: mdiobus_read() MII_PHYSID1
> returned 65535
>     libphy: get_phy_c22_id:824: sh_mii: mdiobus_read() MII_PHYSID2
> returned 65535

The per PHY reset is historically 'interesting'. It makes the
assumption the PHY can be detected when in reset, because the PHY it
was added for could be detected when in reset. And it turns out to be,
most PHYs cannot be detected when held in reset.

The simple solution is to make use of the MDIO bus reset property, as
Russell suggested. If you don't want to do that, you need to put the
PHY ID into DT. The core will then skip scanning the bus for the PHY,
and go straight to instantiating the PHY, and then it should be
brought out of reset.

	Andrew

Re: PHY reset may still be asserted during MDIO probe

[Grygorii Strashko]

On 09/07/2021 18:33, Geert Uytterhoeven wrote:
> Hi all,
> 
> I'm investigating a network failure after kexec on the Renesas Koelsch
> and Salvator-XS development boards, using the sh-eth or ravb driver.
> 
> During normal boot, the Ethernet interface is working fine:
> 
>      libphy: get_phy_c22_id:814: sh_mii: mdiobus_read() MII_PHYSID1 returned 34
>      libphy: get_phy_c22_id:824: sh_mii: mdiobus_read() MII_PHYSID2 returned 5431
>      libphy: get_phy_c22_id:832: sh_mii: phy_id = 0x00221537
>      libphy: get_phy_device:895: sh_mii: get_phy_c22_id() returned 0
>      fwnode_mdiobus_register_phy:109: sh_mii: get_phy_device() returned (ptrval)
>      fwnode_mdiobus_phy_device_register:46: sh_mii: fwnode_irq_get() returned 191
>      libphy: mdiobus_register_gpiod:48: mdiodev->reset_gpio = (ptrval)
>      mdio_bus ee700000.ethernet-ffffffff:01:
> mdiobus_register_device:88: assert MDIO reset
>      libphy: mdio_device_reset:124: calling gpiod_set_value_cansleep(..., 1)
>      mdio_bus ee700000.ethernet-ffffffff:01: phy_device_register:931:
> deassert PHY reset
>      libphy: mdio_device_reset:124: calling gpiod_set_value_cansleep(..., 0)
>      Micrel KSZ8041RNLI ee700000.ethernet-ffffffff:01: phy_probe:3026:
> deassert PHY reset
>      libphy: mdio_device_reset:124: calling gpiod_set_value_cansleep(..., 0)
>      fwnode_mdiobus_phy_device_register:75: sh_mii:
> phy_device_register() returned 0
>      fwnode_mdiobus_register_phy:137: sh_mii:
> fwnode_mdiobus_phy_device_register() returned 0
>      of_mdiobus_register:188: of_mdiobus_register_phy(sh_mii,
> /soc/ethernet@ee700000/ethernet-phy@1, 1) returned 0
>      sh-eth ee700000.ethernet eth0: Base address at 0xee700000,
> 2e:09:0a:00:6d:85, IRQ 126.
> 
> When using kexec, the PHY reset is asserted before starting the
> new kernel:
> 
>      Micrel KSZ8041RNLI ee700000.ethernet-ffffffff:01: phy_detach:1759:
> assert PHY reset
>      libphy: mdio_device_reset:124: calling gpiod_set_value_cansleep(..., 1)
>      kexec_core: Starting new kernel
>      Bye!
> 
> The new kernel fails to probe the PHY, as the PHY reset is still
> asserted:
> 
>      libphy: get_phy_c22_id:814: sh_mii: mdiobus_read() MII_PHYSID1
> returned 65535
>      libphy: get_phy_c22_id:824: sh_mii: mdiobus_read() MII_PHYSID2
> returned 65535
>      libphy: get_phy_c22_id:832: sh_mii: phy_id = 0xffffffff
>      libphy: get_phy_device:895: sh_mii: get_phy_c22_id() returned -19
>      fwnode_mdiobus_register_phy:109: sh_mii: get_phy_device() returned -ENODEV
>      of_mdiobus_register:188: of_mdiobus_register_phy(sh_mii,
> /soc/ethernet@ee700000/ethernet-phy@1, 1) returned -19
>      mdio_bus ee700000.ethernet-ffffffff: MDIO device at address 1 is missing.
>      sh-eth ee700000.ethernet eth0: Base address at 0xee700000,
> 2e:09:0a:00:6d:85, IRQ 126.
> 
> This issue can also be reproduced using unbind:
> 
>      # echo ee700000.ethernet > /sys/bus/platform/drivers/sh-eth/unbind
>      sh-eth ee700000.ethernet eth0: Link is Down
>      Micrel KSZ8041RNLI ee700000.ethernet-ffffffff:01: phy_detach:1759:
> assert PHY reset
>      libphy: mdio_device_reset:124: calling gpiod_set_value_cansleep(..., 1)
>      Micrel KSZ8041RNLI ee700000.ethernet-ffffffff:01: phy_remove:3120:
> assert PHY reset
>      libphy: mdio_device_reset:124: calling gpiod_set_value_cansleep(..., 1)
>      mdio_bus ee700000.ethernet-ffffffff:01: phy_device_remove:974:
> assert PHY reset
>      libphy: mdio_device_reset:124: calling gpiod_set_value_cansleep(..., 1)
> 
> and bind:
> 
>      # echo ee700000.ethernet > /sys/bus/platform/drivers/sh-eth/bind
>      (same log as kexec boot)
> 
> I think fwnode_mdiobus_register_phy() should do the PHY reset (assert +
> deassert) before calling get_phy_device(), but currently that happens
> in phy_device_register(), which is called later.

Seems like similar to [1], only PHY ID in DT compatible is current w/a.

[1] https://lkml.org/lkml/2020/10/23/750

-- 
Best regards,
grygorii