Re: [BUG] network is broken on Orange Pi PC

[Andre Przywara <andre.przywara@arm.com>]

On Thu, 3 Jun 2021 09:46:48 +0200
Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

Hi Heinrich,

> On 6/2/21 3:08 PM, Ramon Fried wrote:
> > On Tue, Jun 1, 2021 at 12:35 AM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:  
> >>
> >> Dear all,
> >>
> >> network is broken in U-Boot on orangepi_pc_defconfig:

Thanks for the report!

> >>
> >> U-Boot 2021.07-rc3-00059-gd8729a114e (May 31 2021 - 21:26:56 +0000)
> >> Allwinner Technology
> >> eth0: ethernet@1c30000  
> >> => dhcp  
> >> sun8i_emac_eth_start: Timeout
> >>
> >> Best regards
> >>
> >> Heinrich
> >>  
> > Hi Heinrich, I don't have OrangePi. can you bisect and tell me when it broke ?
> > Thanks.
> > Ramon.
> >  
> 
> Git bisect points to:
> 
> commit 4f0278dac56a658ef1e0967fec0bb95372a875bd
> Author: Andre Przywara <andre.przywara@arm.com>
> Date:   Mon Jul 6 01:40:45 2020 +0100
> 
>      net: sun8i-emac: Lower MDIO frequency
> 
> Reverting the patch solves the problem for the OrangePi PC.
> 
> According to the commit message the change was only needed for needed
> for external PHYs.

The external PHY on the Pine64 (non-plus) was the trigger, however
both the manual and the Linux driver point to that we definitely need a
higher divider. From what I can see, AHB2 (the EMAC clock) runs at 200
MHz (AHB2=AHB1/1=PLL6/3=200 MHz). So just having "/ 16" results in 12.5
MHz MDIO frequency. Can you check whether any other divider values fix
this for you as well?

> Can't we let the change depend on priv->use_internal_phy?
> 
> diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
> index 5a1b38bf80..d7553fe163 100644
> --- a/drivers/net/sun8i_emac.c
> +++ b/drivers/net/sun8i_emac.c
> @@ -211,7 +211,9 @@ static int sun8i_mdio_read(struct mii_dev *bus, int
> addr, int devad, int reg)
>           * The EMAC clock is either 200 or 300 MHz, so we need a divider
>           * of 128 to get the MDIO frequency below the required 2.5 MHz.
>           */
> -       mii_cmd |= MDIO_CMD_MII_CLK_CSR_DIV_128 <<
> MDIO_CMD_MII_CLK_CSR_SHIFT;
> +       if (!priv->use_internal_phy)
> +               mii_cmd |= MDIO_CMD_MII_CLK_CSR_DIV_128 <<
> +                          MDIO_CMD_MII_CLK_CSR_SHIFT;
> 
>          mii_cmd |= MDIO_CMD_MII_BUSY;
> 
> @@ -242,7 +244,9 @@ static int sun8i_mdio_write(struct mii_dev *bus, int
> addr, int devad, int reg,
>           * The EMAC clock is either 200 or 300 MHz, so we need a divider
>           * of 128 to get the MDIO frequency below the required 2.5 MHz.
>           */
> -       mii_cmd |= MDIO_CMD_MII_CLK_CSR_DIV_128 <<
> MDIO_CMD_MII_CLK_CSR_SHIFT;
> +       if (!priv->use_internal_phy)
> +               mii_cmd |= MDIO_CMD_MII_CLK_CSR_DIV_128 <<
> +                          MDIO_CMD_MII_CLK_CSR_SHIFT;
> 
>          mii_cmd |= MDIO_CMD_MII_WRITE;
>          mii_cmd |= MDIO_CMD_MII_BUSY;
> 
> Best regards
> 
> Heinrich
> 
> I would assume the problem hits all H3 boards.

And that's the confusing part: it does not. I tested this on my
OrangePi Zero (H2+ with internal PHY), both back then with the original
MDIO frequency patch and also now after your report. It always worked
reliably for me.
Also: I am still puzzled how one influences the other: The error you
get is from the *MAC* soft reset: I would think this is an independent
operation from any communication attempts with the PHY.

There is this thread here about the same symptom on a V3s:
https://lists.denx.de/pipermail/u-boot/2021-May/450315.html
v2 of this original patch (in the same thread) proposes some other
solution, which I am also not very happy with. But just to get some more
data points: can you check whether skipping the soft reset fixes this
for you? I will have a closer look tonight to check the order of soft
reset and PHY communication. Maybe we should only do the soft reset
*once* when the MAC probes, and not on every .start call?

Thanks,
Andre