| Message ID | 20190121091318.20079-5-vkoul@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | net: Add support for Qualcomm ethqos | expand |
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 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
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. 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. 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
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
Hi again, On 12-02-19, 17:01, 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. > > 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. Relooking at this again and seeing the mode definitions in Documentation/devicetree/bindings/net/ethernet.txt, I think I have got it wrong.. (thanks to Niklas for discussion) So in case of rgmii mode delay should be disabled and in case of rgmii-id/rxid/txid it should be enabled. I will send a patch to address this shortly, please do test. Thanks
On 12/02/2019 11:55, Peter Ujfalusi wrote: > 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'm having flashbacks: [PATCH 1/2] net: phy: at803x: Fix RGMII RX and TX clock delays setup https://www.spinics.net/lists/netdev/msg445053.html Quirks of the Atheros 8035 PHY https://www.spinics.net/lists/netdev/msg444527.html See also http://patchwork.ozlabs.org/project/netdev/list/?submitter=67482&state=* Regards.
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; }
For RGMII mode, phy delay should be disabled. Add this case along with disable delay routines. Signed-off-by: Vinod Koul <vkoul@kernel.org> --- drivers/net/phy/at803x.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-)