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