Hello Vincente, On Sat, May 11, 2019 at 05:06:04PM +0200, Vicente Bergas wrote: > On Saturday, May 11, 2019 4:56:56 PM CEST, Heiner Kallweit wrote: > > On 11.05.2019 16:46, Vicente Bergas wrote: > > > On Friday, May 10, 2019 10:28:06 PM CEST, Heiner Kallweit wrote: > > > > On 10.05.2019 17:05, Vicente Bergas wrote: ... > > > > > > Hello Heiner, > > > just tried your patch and indeed the NPE is gone. But still no network... > > > The MAC <-> PHY link was working before, so, maybe the rgmii delays > > > are not > > > correctly configured. > > > > That's a question to the author of the original patch. My patch was just > > meant to fix the NPE. In which configuration are you using the RTL8211E? > > As a standalone PHY (with which MAC/driver?) or is it the integrated PHY > > in a member of the RTL8168 family? > > It is the one on the Sapphire board, so is connected to the MAC on the > RK3399 SoC. It is on page 8 of the schematics: > http://dl.vamrs.com/products/sapphire_excavator/RK_SAPPHIRE_SOCBOARD_RK3399_LPDDR3D178P232SD8_V12_20161109HXS.pdf >
Thanks for sending this bug report. As I said in the commit-message. The idea of this patch is to provide a way to setup the RGMII delays in the PHY drivers (similar to the most of the PHY drivers). Before this commit phy-mode dts-node hadn't been taked into account by the PHY driver, so any PHY-delay setups provided via external pins strapping were accepted as is. But now rtl8211e phy-mode is parsed as follows: phy-mode="rgmii" - delays aren't set by PHY (current dts setting in rk3399-sapphire.dtsi) phy-mode="rgmii-id" - both RX and TX delays are setup on the PHY side, phy-mode="rgmii-rxid" - only RX delay is setup on the PHY side, phy-mode="rgmii-txid" - only TX delay is setup on the PHY side. It means, that now matter what the rtl8211e TXDLY/RXDLY pins are grounded or pulled high, the delays are going to be setup in accordance with the dts phy-mode settings, which is supposed to reflect the real hardware setup. So since you get the problem with MAC<->PHY link, it means your dts-file didn't provide a correct interface mode. Indeed seeing the sheet on page 7 in the sepphire pdf-file your rtl8211e PHY is setup to have TXDLY/RXDLY being pulled high, which means to add 2ns delays by the PHY. This setup corresponds to phy-mode="rgmii-id". As soon as you set it this way in the rk3399 dts-file, the MAC-PHY link shall work correctly as before. -Sergey > > Serge: The issue with the NPE gave a hint already that you didn't test your > > patch. Was your patch based on an actual issue on some board and did you > > test it? We may have to consider reverting the patch. > > > > > With this change it is back to working: > > > --- a/drivers/net/phy/realtek.c > > > +++ b/drivers/net/phy/realtek.c > > > @@ -300,7 +300,6 @@ > > > }, { > > > PHY_ID_MATCH_EXACT(0x001cc915), > > > .name = "RTL8211E Gigabit Ethernet", > > > - .config_init = &rtl8211e_config_init, > > > .ack_interrupt = &rtl821x_ack_interrupt, > > > .config_intr = &rtl8211e_config_intr, > > > .suspend = genphy_suspend, > > > That is basically reverting the patch. > > > > > > Regards, > > > Vicenç. > > > > > > > Nevertheless your proposed patch looks good to me, just one small change > > > > would be needed and it should be splitted. > > > > > > > > The change to phy-core I would consider a fix and it should be fine to > > > > submit it to net (net-next is closed currently). > > > > > > > > Adding the warning to the Realtek driver is fine, but this would be ... >