On Sat, 2 Nov 2019 at 16:12, Tom Rini <tr...@konsulko.com> wrote: > > On Sat, Nov 02, 2019 at 02:17:28PM +0100, Michael Walle wrote: > > Am 2019-05-15 16:58, schrieb Tom Rini: > > > On Fri, May 10, 2019 at 09:50:45PM +0000, Joe Hershberger wrote: > > > > Hi Vladimir and Tom, > > > > > > > > On Thu, May 9, 2019 at 7:51 AM Vladimir Oltean > > > > <vladimir.olt...@nxp.com> wrote: > > > > > > > > > > On 09.05.2019 02:05, Vladimir Oltean wrote: > > > > > > On 5/9/19 1:55 AM, Tom Rini wrote: > > > > > >> On Wed, May 08, 2019 at 10:52:28PM +0000, Vladimir Oltean wrote: > > > > > >>> On 5/9/19 1:48 AM, Tom Rini wrote: > > > > > >>>> On Wed, May 08, 2019 at 10:45:50PM +0000, Vladimir Oltean wrote: > > > > > >>>>> On 5/9/19 1:42 AM, Tom Rini wrote: > > > > > >>>>>> On Wed, May 08, 2019 at 10:40:57PM +0000, Vladimir Oltean > > > > > >>>>>> wrote: > > > > > >>>>>>> On 5/9/19 1:24 AM, Joe Hershberger wrote: > > > > > >>>>>>>> On Tue, May 7, 2019 at 5:15 PM Joe Hershberger > > > > > >>>>>>>> <joe.hershber...@ni.com> wrote: > > > > > >>>>>>>>> > > > > > >>>>>>>>> Hi Tom, > > > > > >>>>>>>>> > > > > > >>>>>>>>> The following changes since commit > > > > > >>>>>>>>> 8d7f06bbbef16f172cd5e9c4923cdcebe16b8980: > > > > > >>>>>>>>> > > > > > >>>>>>>>> I rebased on your master and built for BB Black. DHCP seems > > > > > >>>>>>>>> to work fine. > > > > > >>>>>>>>> MLO also now fits again. > > > > > >>>>>>>>> > > > > > >>>>>>>>> Merge branch 'master' of git://git.denx.de/u-boot-sh > > > > > >>>>>>>>> (2019-05-07 09:38:00 -0400) > > > > > >>>>>>>>> > > > > > >>>>>>>>> are available in the git repository at: > > > > > >>>>>>>>> > > > > > >>>>>>>>> git://git.denx.de/u-boot-net.git master > > > > > >>>>>>>>> > > > > > >>>>>>>>> for you to fetch changes up to > > > > > >>>>>>>>> 8d0c6858455e89b089222a08d55ff711681ca011: > > > > > >>>>>>>>> > > > > > >>>>>>>>> net: phy: micrel: Find Micrel PHY node correctly > > > > > >>>>>>>>> (2019-05-07 14:51:55 -0500) > > > > > >>>>>>>>> > > > > > >>>>>>>>> ---------------------------------------------------------------- > > > > > >>>>>>>>> Carlo Caione (4): > > > > > >>>>>>>>> net: phy: Add generic helpers to access MMD PHY > > > > > >>>>>>>>> registers > > > > > >>>>>>>>> net: phy: ti: use generic helpers to access MMD > > > > > >>>>>>>>> registers > > > > > >>>>>>>>> cmd: mdio: Switch to generic helpers when > > > > > >>>>>>>>> accessing the registers > > > > > >>>>>>>>> net: phy: realtek: Introduce quirk to mark RXC > > > > > >>>>>>>>> not stoppable > > > > > >>>>>>>>> > > > > > >>>>>>>>> James Byrne (2): > > > > > >>>>>>>>> net: phy: micrel: Use correct skew values on > > > > > >>>>>>>>> KSZ9021 > > > > > >>>>>>>>> net: phy: micrel: Find Micrel PHY node correctly > > > > > >>>>>>>>> > > > > > >>>>>>>>> Murali Karicheri (2): > > > > > >>>>>>>>> ARM: k2g-gp-evm: update to rgmii pinmux > > > > > >>>>>>>>> configuration > > > > > >>>>>>>>> ARM: k2g-ice: Add pinmux support for rgmii > > > > > >>>>>>>>> interface > > > > > >>>>>>>>> > > > > > >>>>>>>>> Pankaj Bansal (1): > > > > > >>>>>>>>> drivers: net: ldpaa_eth: fix resource leak > > > > > >>>>>>>>> > > > > > >>>>>>>>> Siva Durga Prasad Paladugu (2): > > > > > >>>>>>>>> net: phy: Reloc next and prev pointers inside > > > > > >>>>>>>>> phy_drivers > > > > > >>>>>>>>> net: phy: Fix return value check phy_probe > > > > > >>>>>>>>> > > > > > >>>>>>>>> Valentin-catalin Neacsu (1): > > > > > >>>>>>>>> net: phy: aquantia: Set only autoneg on in > > > > > >>>>>>>>> register 4.c441 > > > > > >>>>>>>>> > > > > > >>>>>>>>> Vladimir Oltean (6): > > > > > >>>>>>>>> net: phy: ar803x: Address packet drops at low > > > > > >>>>>>>>> traffic rate due to SmartEEE feature > > > > > >>>>>>>>> net: phy: ar803x: Make RGMII Tx delays actually > > > > > >>>>>>>>> configurable for AR8035 > > > > > >>>>>>>>> net: phy: ar803x: Use common functions for RGMII > > > > > >>>>>>>>> internal delays > > > > > >>>>>>>>> net: phy: ar803x: Clarify the configuration of > > > > > >>>>>>>>> the CLK_25M output pin > > > > > >>>>>>>>> net: phy: ar803x: Explicitly disable RGMII delays > > > > > >>>>>>>> > > > > > >>>>>>>> Tom, this [1] is the patch that is breaking the evm. It > > > > > >>>>>>>> doesn't affect > > > > > >>>>>>>> BB Black because it uses an SMSC phy, where as this evm uses > > > > > >>>>>>>> an > > > > > >>>>>>>> AR8031/AR8033. > > > > > >>>>>>>> > > > > > >>>>>>>> Is it possible the device tree [2] is wrong for the board? > > > > > >>>>>>>> It lists > > > > > >>>>>>>> 'phy-mode = "rgmii-txid";', so that means that with this > > > > > >>>>>>>> patch the RX > > > > > >>>>>>>> delay is now being disabled. > > > > > >>>>>>>> > > > > > >>>>>>>> Any thoughts, Vladimir? > > > > > >>>>>>>> > > > > > >>>>>>>> Thanks, > > > > > >>>>>>>> -Joe > > > > > >>>>>>>> > > > > > >>>>>>>> [1] b3224e0f7e - "net: phy: ar803x: Explicitly disable RGMII > > > > > >>>>>>>> delays" > > > > > >>>>>>>> [2] arch/arm/dts/am335x-evm.dts > > > > > >>>>>>>> > > > > > >>>>>>>>> net: phy: ar803x: Clarify the intention of > > > > > >>>>>>>>> ar8021_config > > > > > >>>>>>>>> > > > > > >>>>>>>>> arch/arm/dts/sama5d3xcm.dtsi | 32 > > > > > >>>>>>>>> +++--- > > > > > >>>>>>>>> arch/arm/dts/sama5d3xcm_cmp.dtsi | 32 > > > > > >>>>>>>>> +++--- > > > > > >>>>>>>>> arch/arm/dts/socfpga_arria5_socdk.dts | 4 > > > > > >>>>>>>>> +- > > > > > >>>>>>>>> arch/arm/dts/socfpga_cyclone5_is1.dts | 4 > > > > > >>>>>>>>> +- > > > > > >>>>>>>>> arch/arm/dts/socfpga_cyclone5_socdk.dts | 4 > > > > > >>>>>>>>> +- > > > > > >>>>>>>>> arch/arm/dts/socfpga_cyclone5_sockit.dts | 4 > > > > > >>>>>>>>> +- > > > > > >>>>>>>>> arch/arm/dts/socfpga_cyclone5_vining_fpga.dts | 4 > > > > > >>>>>>>>> +- > > > > > >>>>>>>>> board/ti/ks2_evm/mux-k2g.h | 36 > > > > > >>>>>>>>> +++---- > > > > > >>>>>>>>> cmd/mdio.c | 27 > > > > > >>>>>>>>> +++-- > > > > > >>>>>>>>> doc/device-tree-bindings/net/micrel-ksz90x1.txt | 27 > > > > > >>>>>>>>> +++++ > > > > > >>>>>>>>> drivers/net/ldpaa_eth/ldpaa_eth.c | 1 > > > > > >>>>>>>>> + > > > > > >>>>>>>>> drivers/net/phy/Kconfig | 41 > > > > > >>>>>>>>> ++++++++ > > > > > >>>>>>>>> drivers/net/phy/aquantia.c | 7 > > > > > >>>>>>>>> +- > > > > > >>>>>>>>> drivers/net/phy/atheros.c | 128 > > > > > >>>>>>>>> ++++++++++++++++------- > > > > > >>>>>>>>> drivers/net/phy/micrel_ksz90x1.c | 24 > > > > > >>>>>>>>> ++++- > > > > > >>>>>>>>> drivers/net/phy/phy.c | 21 > > > > > >>>>>>>>> +++- > > > > > >>>>>>>>> drivers/net/phy/realtek.c | 19 > > > > > >>>>>>>>> ++++ > > > > > >>>>>>>>> drivers/net/phy/ti.c | 130 > > > > > >>>>>>>>> +++++------------------- > > > > > >>>>>>>>> include/phy.h | 70 > > > > > >>>>>>>>> +++++++++++++ > > > > > >>>>>>>>> 19 files changed, 394 insertions(+), 221 deletions(-) > > > > > >>>>>>>>> > > > > > >>>>>>>>> Thanks! > > > > > >>>>>>>>> -Joe > > > > > >>>>>>>>> _______________________________________________ > > > > > >>>>>>>>> U-Boot mailing list > > > > > >>>>>>>>> U-Boot@lists.denx.de > > > > > >>>>>>>>> https://lists.denx.de/listinfo/u-boot > > > > > >>>>>>>> > > > > > >>>>>>> > > > > > >>>>>>> Hi Joe, Tom, > > > > > >>>>>>> > > > > > >>>>>>> It sounds like what Joe pointed to (my patch) has a high > > > > > >>>>>>> chance of > > > > > >>>>>>> causing link failure. > > > > > >>>>>>> If the board is relying on RX delays in the Atheros PHY to > > > > > >>>>>>> ensure > > > > > >>>>>>> correct RGMII timing budget, then for sure it was working > > > > > >>>>>>> before and now > > > > > >>>>>>> it is broken. In that case, it was working by mistake; the DT > > > > > >>>>>>> blob is > > > > > >>>>>>> broken and should be corrected. > > > > > >>>>>>> Sorry for the trouble this has caused. > > > > > >>>>>> > > > > > >>>>>> How is this handled in the Linux kernel and/or why doesn't it > > > > > >>>>>> fail > > > > > >>>>>> there? > > > > > >>>>> > > > > > >>>>> On the latest net-next, it's handled the same: disable > > > > > >>>>> everything and > > > > > >>>>> enable only what's specified in phy-mode. I didn't keep track > > > > > >>>>> of older > > > > > >>>>> versions. > > > > > >>>> > > > > > >>>> So you would expect the network to be broken on net-next with > > > > > >>>> this DTS? > > > > > >>>> Because that's not allowed... > > > > > >>>> > > > > > >>> > > > > > >>> Yes, if it's the same problem, the behavior should be the same. > > > > > >>> I see net-next is only rejecting the bad DT since 2019-02-21: > > > > > >>> https://patchwork.kernel.org/patch/10819279/ > > > > > >>> So it depends when you checked it last time. > > > > > >> > > > > > >> I'll see about testing eth in Linux with net-next tomorrow, but, > > > > > >> uh, is > > > > > >> there some work-around for the DTS so that ethernet works? > > > > > > > > > > > > Work-around in Linux or in U-boot? > > > > > > U-boot can fix up the DT for Linux and replace "rgmii-txid" with > > > > > > "rgmii-id", whereas neither Linux nor U-boot can do anything for > > > > > > themselves except either load a correct DT blob, or not use > > > > > > (revert, not > > > > > > upgrade to, etc) the enforcing patch. > > > > > > > > > > > >> You cannot break existing DTs > > > > > > RGMII delay breakages happen all the time. To play the devil's > > > > > > advocate, > > > > > > protecting against them should be done by observing the semantics > > > > > > described in the devicetree/bindings document. > > > > > > If you follow Linux net-next, a breakage for RTL8211F is planned out > > > > > > right as we speak. > > > > > > > > > > > > But let's wait until we know for sure that this is what's causing > > > > > > the > > > > > > issues. > > > > > > > > > > > > Regards, > > > > > > -Vladimir > > > > > > > > > > > > > > > > Hi Tom, > > > > > > > > > > I understand now what you mean by "workaround". > > > > > Yes you can enable RGMII delays by hand in U-boot: > > > > > > > > > > Check if RX delay is enabled (bit 15 of debug register 0): > > > > > => mdio write eTSEC1 0x1D 0 > > > > > => mdio read eTSEC1 0x1E > > > > > => mdio write eTSEC1 0x1E <new value> > > > > > > > > > > Check if TX delay is enabled (bit 8 of debug register 5): > > > > > => mdio write eTSEC1 0x1D 5 > > > > > => mdio read eTSEC1 0x1E > > > > > => mdio write eTSEC1 0x1E <new value> > > > > > > > > > > Replace eTSEC1 with your net device from "mdio list". > > > > > > > > I'm not sure what the follow-on action is here. I'll hold off on > > > > Vladimir's series until there is a resolution that everyone is > > > > satisfied with. > > > > > > While I personally think this breaks the "DT is ABI" model, the required > > > change has already been merged into Linus' tree so we just need to > > > re-sync the am335x DTS files again and then we can apply these changes. > > > I'll put it on my TODO list. > > > > Hi, > > > > sorry for pulling up such an old thread. Are there any news on this? Because > > I've had the same fix for the atheros PHYs posted to the u-boot mailing list > > lately (without knowing anything about this thread). > > > > So with that rule "don't break old (and very wrong!) DT blobs" we seem not > > to be able to fix any serious errors. Because while it works for the most > > cases eg. phy-mode = rgmii-id, it wont work (and cant be fixed!) for > > rgmii-txid (or was it rxid? the one where the hardware default is enabled). > > > > I could also post my series without fixing that issue and just adding clock > > support. But I'd rather see that fixed. > > So, there's patches to fix / keep cpsw working in U-Boot. So while I > believe that Linux broke the rule of "DT is ABI" I've also gone > (virtually) hoarse pointing this out to the people that were supposed to > care. So it's on my short list to grab the changes that are needed for > the TI platforms and we can grab all of the other changes as well. > > --
Tom, How come the people who proclaim that DT is ABI (which it is, nobody is saying otherwise) are the same as (or do it in the name of) those who don't follow it? Had your boards been written with the correct phy-mode in the first place, no breakage would have happened, and maybe the Atheros PHY driver bugs would have been caught earlier. I agree with Michael that interpreting the "DT is ABI" phrase as "be compatible with past mistakes" (which are not in the ABI specification, but in its interpretation, at that!) is only going to lead to a cacophony of incoherent interpretations, which at some point will become impossible to reconcile. And for U-Boot, I guess the point is moot, since the DT blob is typically packaged with the bootloader binary anyway. > Tom > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot Thanks, -Vladimir _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
