On 6/3/21 11:04 AM, Andre Przywara wrote:
On Thu, 3 Jun 2021 09:46:48 +0200
Heinrich Schuchardt <xypron.g...@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.g...@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.przyw...@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


Reply via email to