All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Ujfalusi <peter.ujfalusi@ti.com>
To: Vinod Koul <vkoul@kernel.org>
Cc: David S Miller <davem@davemloft.net>,
	Roger Quadros <rogerq@ti.com>,
	netdev@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	Niklas Cassel <niklas.cassel@linaro.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Andrew Lunn <andrew@lunn.ch>,
	Florian Fainelli <f.fainelli@gmail.com>,
	"Nori, Sekhar" <nsekhar@ti.com>
Subject: Re: [v3,4/5] net: phy: at803x: Disable phy delay for RGMII mode
Date: Tue, 12 Feb 2019 14:09:39 +0200	[thread overview]
Message-ID: <06680a9b-70ec-df76-b632-d8c467b3a5f7@ti.com> (raw)
In-Reply-To: <20190212113114.GZ4296@vkoul-mobl>

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@kernel.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

WARNING: multiple messages have this Message-ID (diff)
From: Peter Ujfalusi <peter.ujfalusi@ti.com>
To: Vinod Koul <vkoul@kernel.org>
Cc: David S Miller <davem@davemloft.net>,
	Roger Quadros <rogerq@ti.com>, <netdev@vger.kernel.org>,
	<linux-arm-msm@vger.kernel.org>,
	Niklas Cassel <niklas.cassel@linaro.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Andrew Lunn <andrew@lunn.ch>,
	Florian Fainelli <f.fainelli@gmail.com>,
	"Nori, Sekhar" <nsekhar@ti.com>
Subject: Re: [v3,4/5] net: phy: at803x: Disable phy delay for RGMII mode
Date: Tue, 12 Feb 2019 14:09:39 +0200	[thread overview]
Message-ID: <06680a9b-70ec-df76-b632-d8c467b3a5f7@ti.com> (raw)
In-Reply-To: <20190212113114.GZ4296@vkoul-mobl>

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@kernel.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

  reply	other threads:[~2019-02-12 12:09 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-21  9:13 [PATCH v3 0/5] net: Add support for Qualcomm ethqos Vinod Koul
2019-01-21  9:13 ` [PATCH v3 1/5] dt-bindings: net: Add Qualcomm ethqos binding Vinod Koul
2019-01-21 15:31   ` Rob Herring
2019-01-21 15:31     ` Rob Herring
2019-01-21  9:13 ` [PATCH v3 2/5] net: stmmac: Add driver for Qualcomm ethqos Vinod Koul
2019-01-21  9:13 ` [PATCH v3 3/5] MAINTAINER: Add entry for Qualcomm ETHQOS ethernet driver Vinod Koul
2019-01-21  9:13 ` [PATCH v3 4/5] net: phy: at803x: Disable phy delay for RGMII mode Vinod Koul
2019-02-12 10:55   ` [v3,4/5] " Peter Ujfalusi
2019-02-12 10:55     ` Peter Ujfalusi
2019-02-12 11:31     ` Vinod Koul
2019-02-12 12:09       ` Peter Ujfalusi [this message]
2019-02-12 12:09         ` Peter Ujfalusi
2019-02-12 12:17       ` Vinod Koul
2019-02-12 13:34     ` Marc Gonzalez
2019-01-21  9:13 ` [PATCH v3 5/5] net: dsa: qca8k: disable " Vinod Koul
2019-01-23  3:38 ` [PATCH v3 0/5] net: Add support for Qualcomm ethqos David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=06680a9b-70ec-df76-b632-d8c467b3a5f7@ti.com \
    --to=peter.ujfalusi@ti.com \
    --cc=andrew@lunn.ch \
    --cc=bjorn.andersson@linaro.org \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=niklas.cassel@linaro.org \
    --cc=nsekhar@ti.com \
    --cc=rogerq@ti.com \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.