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

[Heinrich Schuchardt <xypron.glpk@gmx.de>]

On 6/3/21 11:04 AM, Andre Przywara wrote:
> 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?

MDIO_CMD_MII_CLK_CSR_DIV_16 = 0 and this is the value that was used
before your patch.

MDIO_CMD_MII_CLK_CSR_DIV_16 = 0 =>
	Waiting for PHY auto negotiation to complete. done
MDIO_CMD_MII_CLK_CSR_DIV_32 = 1 => sun8i_emac_eth_start: Timeout
MDIO_CMD_MII_CLK_CSR_DIV_64 = 2 => sun8i_emac_eth_start: Timeout
MDIO_CMD_MII_CLK_CSR_DIV_128 = 3 => sun8i_emac_eth_start: Timeout

What is wrong about the approach in

[PATCH 1/1] net: sun8i-emac: fix MDIO frequency
https://lists.denx.de/pipermail/u-boot/2021-June/451305.html ?

The Linux 5.10 driver runs fine no matter what value we choose for the
divider in U-Boot. But I see a message:

dwmac-sun8i 1c30000.ethernet:
Current syscon value is not the default 148000 (expect 58000)

@default_syscon_value:
The default value of the EMAC register in syscon
This value is used for disabling properly EMAC
and used as a good starting value in case of the
boot process(uboot) leave some stuff.

Best regards

Heinrich

>
>> 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
>