lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 26 Apr 2019 22:19:36 +0300
From:   Serge Semin <fancer.lancer@...il.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Serge Semin <Sergey.Semin@...latforms.ru>,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net: phy: realtek: Add rtl8211e rx/tx delays config

On Fri, Apr 26, 2019 at 03:28:21PM +0200, Andrew Lunn wrote:
> On Fri, Apr 26, 2019 at 12:30:10PM +0300, Serge Semin wrote:
> > The hidden RGMII configs register utilization was found in the
> > rtl8211e U-boot driver:
> > https://elixir.bootlin.com/u-boot/v2019.01/source/drivers/net/phy/realtek.c#L99
> > 
> > It confirms that the register bits field must control the so called
> > configuration pins described in the table 12-13 of the official PHY
> > datasheet:
> > 8:6 = PHY Address
> > 5:4 = Auto-Negotiation
> > 3 = Interface Mode Select
> > 2 = RX Delay
> > 1 = TX Delay
> > 0 = SELRGV
> 
> > +static int rtl8211e_config_init(struct phy_device *phydev)
> > +{
> > +	int ret, oldpage;
> > +	u16 val = 0;
> > +
> > +	ret = genphy_config_init(phydev);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	/* enable TX/RX delay for rgmii-* modes, otherwise disable it */
> > +	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
> > +		val = RTL8211E_TX_DELAY | RTL8211E_RX_DELAY;
> > +	else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
> > +		val = RTL8211E_TX_DELAY;
> > +	else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
> > +		val = RTL8211E_RX_DELAY;
> 
> Hi Serge
> 
> You need to look for PHY_INTERFACE_MODE_RGMII and disable the bits.
> For any other value, and in particular PHY_INTERFACE_MODE_NA, you
> should leave the bits alone.
> 
> As you found out, u-boot knows how to program these bits. There are
> probably boards out there which rely on u-boot doing this, and the PHY
> driver then not messing with the bits. The way to indicate it should
> not mess with the bits is to not have a phy-mode property in DT, which
> results in PHY_INTERFACE_MODE_NA.
> 

Hello Andrew

Thanks for the comment. I'll alter the code the way you said. The mode will
be changed by the config_init-function only if the interface is selected to be
rgmii-like (rgmii, rgmii-id, rgmii-txid and rgmii-rxid), otherwise it will
be left as is.

But I've got a question regarding this then. What about for instance rtl8211f
phy config_init-method? It setups the delay config in any case, no matter
whether interface is configured to be of rgmii or another mode. Is it correct
to configure rtl8211e and rtl8211f differently? Especially seeing the U-boot
driver also perform the rtl8211f phy configuration.

-Sergey

> 	Andrew

Powered by blists - more mailing lists