netdev - Re: [v3,4/5] net: phy: at803x: Disable phy delay for RGMII mode

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:   Tue, 12 Feb 2019 14:09:39 +0200
From:   Peter Ujfalusi <peter.ujfalusi@...com>
To:     Vinod Koul <vkoul@...nel.org>
CC:     David S Miller <davem@...emloft.net>,
        Roger Quadros <rogerq@...com>, <netdev@...r.kernel.org>,
        <linux-arm-msm@...r.kernel.org>,
        Niklas Cassel <niklas.cassel@...aro.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        "Nori, Sekhar" <nsekhar@...com>
Subject: Re: [v3,4/5] net: phy: at803x: Disable phy delay for RGMII mode

Hi Vinod,

On 12/02/2019 13.31, Vinod Koul wrote:
> Hi Peter,
> 
> On 12-02-19, 12:55, Peter Ujfalusi wrote:
>> Vinod,
>>
>> On 21/01/2019 11.13, Vinod Koul wrote:
>>> For RGMII mode, phy delay should be disabled. Add this case along
>>> with disable delay routines.
>>
>> In next-20190211 I need to revert this patch to get cpsw networking to
>> work on am335x-evmsk. The board uses AR8031_AL1A PHY, which is handled
>> by the phy/at803x.c
> 
> I see that DTS specifies that you are using phy-mode = "rgmii-txid".
> RGMII mode implies that we should not have any delay in the
> phy, so this patch does the right thing.

Right, on am335x-evmsk:
[    3.393406] at803x_config_init: phydev->interface: 11
So PHY_INTERFACE_MODE_RGMII_TXID.

> In the previous version of the patch I did propose to add a DT entry so
> that current users who are wrongly using this would not be impacted but
> the suggestion was to get them fixed.
> 
> So in you case do you need rgmii-txd mode if so why should the delay be
> enabled for this? We can add a patch that enabled delay for your
> controller but that cant be rgmii mode.

I'm not too familiar with the networking, but the AR8031 datasheet does
tell that the debug_5:bit8 is 0 by default (rgmii tx clock delay disable).

In the errata document of am335x (http://www.ti.com/lit/pdf/sprz360),
1.0.10 I believe tells that the delay needs to be enabled on the rgmii
ethernet PHY because the internal delay is not working (?) and advises
that the delay might need to be enabled on the rgmii PHY.

The PHY_INTERFACE_MODE_RGMII_TXID delay was enabled in the PHY driver
since 2013 (1ca6d1b1aef4628ff0fe458135ddb008d134ad8f)

To me it looks like that when AR8031 is used with am335x the 'rgmii tx
clock delay' needs to be enabled.

Sekhar, Roger: any insights, thoughts?

> 
> Thanks
> 
> 
>>
>> On next-20190211:
>> [    3.374601] net eth0: initializing cpsw version 1.12 (0)
>> [    3.384484] Atheros 8031 ethernet 4a101000.mdio:00: attached PHY driver [Atheros 8031 ethernet] (mii_bus:phy_addr=4a101000.mdio:00, irq=POLL)
>> [    3.400041] cpsw 4a100000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
>> [    3.410813] mmc1: new SDIO card at address 0001
>> [    3.439362] IP-Config: Complete:
>> [    3.442649]      device=eth0, hwaddr=bc:6a:29:7d:2c:a9, ipaddr=10.0.0.90, mask=255.255.255.0, gw=10.0.0.1
>> [    3.452840]      host=10.0.0.90, domain=, nis-domain=(none)
>> [    3.458462]      bootserver=10.0.0.30, rootserver=10.0.0.30, rootpath=
>> [    3.466296] vwl1271: disabling
>> [    3.470195] ALSA device list:
>> [    3.473189]   #0: AM335x-EVMSK
>>
>> After reverting this patch:
>> [    3.374636] net eth0: initializing cpsw version 1.12 (0)
>> [    3.384534] Atheros 8031 ethernet 4a101000.mdio:00: attached PHY driver [Atheros 8031 ethernet] (mii_bus:phy_addr=4a101000.mdio:00, irq=POLL)
>> [    3.400125] cpsw 4a100000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
>> [    3.410866] mmc1: new SDIO card at address 0001
>> [    3.439379] IP-Config: Complete:
>> [    3.442666]      device=eth0, hwaddr=bc:6a:29:7d:2c:a9, ipaddr=10.0.0.90, mask=255.255.255.0, gw=10.0.0.1
>> [    3.452865]      host=10.0.0.90, domain=, nis-domain=(none)
>> [    3.458482]      bootserver=10.0.0.30, rootserver=10.0.0.30, rootpath=
>> [    3.466334] vwl1271: disabling
>> [    3.470245] ALSA device list:
>> [    3.473241]   #0: AM335x-EVMSK
>> [    3.501052] VFS: Mounted root (nfs filesystem) readonly on device 0:15.
>> [    3.508694] devtmpfs: mounted
>> [    3.514546] Freeing unused kernel memory: 1024K
>> [    3.520567] Run /sbin/init as init process
>>
>> and the board boots to nfsroot fine.
>>
>>  
>>> Signed-off-by: Vinod Koul <vkoul@...nel.org>
>>> ---
>>>  drivers/net/phy/at803x.c | 22 ++++++++++++----------
>>>  1 file changed, 12 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
>>> index f9432d053a22..8ff12938ab47 100644
>>> --- a/drivers/net/phy/at803x.c
>>> +++ b/drivers/net/phy/at803x.c
>>> @@ -110,16 +110,16 @@ static int at803x_debug_reg_mask(struct phy_device *phydev, u16 reg,
>>>  	return phy_write(phydev, AT803X_DEBUG_DATA, val);
>>>  }
>>>  
>>> -static inline int at803x_enable_rx_delay(struct phy_device *phydev)
>>> +static inline int at803x_disable_rx_delay(struct phy_device *phydev)
>>>  {
>>> -	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0, 0,
>>> -					AT803X_DEBUG_RX_CLK_DLY_EN);
>>> +	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
>>> +				     AT803X_DEBUG_RX_CLK_DLY_EN, 0);
>>>  }
>>>  
>>> -static inline int at803x_enable_tx_delay(struct phy_device *phydev)
>>> +static inline int at803x_disable_tx_delay(struct phy_device *phydev)
>>>  {
>>> -	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_5, 0,
>>> -					AT803X_DEBUG_TX_CLK_DLY_EN);
>>> +	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_5,
>>> +				     AT803X_DEBUG_TX_CLK_DLY_EN, 0);
>>>  }
>>>  
>>>  /* save relevant PHY registers to private copy */
>>> @@ -256,15 +256,17 @@ static int at803x_config_init(struct phy_device *phydev)
>>>  		return ret;
>>>  
>>>  	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
>>> -			phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
>>> -		ret = at803x_enable_rx_delay(phydev);
>>> +			phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
>>> +			phydev->interface == PHY_INTERFACE_MODE_RGMII) {
>>> +		ret = at803x_disable_rx_delay(phydev);
>>>  		if (ret < 0)
>>>  			return ret;
>>>  	}
>>>  
>>>  	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
>>> -			phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
>>> -		ret = at803x_enable_tx_delay(phydev);
>>> +			phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
>>> +			phydev->interface == PHY_INTERFACE_MODE_RGMII) {
>>> +		ret = at803x_disable_tx_delay(phydev);
>>>  		if (ret < 0)
>>>  			return ret;
>>>  	}
>>>
>>
>> - Péter
>>
>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki