Re: [PATCH] net: mv643xx_eth: Add missing phy_addr_set in DT mode
Hi, Sebastian Hesselbarth writes: > On 11/05/2013 11:12 PM, Arnaud Ebalard wrote: >> Hi Jason, >> >> Jason Gunthorpe writes: >> >>> Commit cc9d4598 'net: mv643xx_eth: use of_phy_connect if phy_node >>> present' made the call to phy_scan optional, if the DT has a link to >>> the phy node. >>> >>> However phy_scan has the side effect of calling phy_addr_set, which >>> writes the phy MDIO address to the ethernet controller. If phy_addr_set >>> is not called, and the bootloader has not set the correct address then >>> the driver will fail to function. >> >> Thanks *a lot* for fixing this one! I had the issue on my ReadyNAS 102 >> (Armada 370 based) which I had put on a todo list and temporarily > > Erm, just to make sure: Armada 370 isn't using mv643xx_eth but mvneta, > are you sure it is (was) related to Jason's fix? Thanks for pointing this, Sebastian and my apologies for the noise. Jason's fix is indeed for a file which is not compiled for my RN102. As the problem perfectly matched the issue I had and current kernel w/ the patch applied does indeed fix it, I did not try and do the test w/o the patch applied. It would have showed the problem was fixed by something else in 3.12. Well, I spent some time digging the changes on mvneta.c and: commit 714086029116b6b0a34e67ba1dd2f0d1cf26770c Author: Thomas Petazzoni Date: Wed Sep 4 16:21:18 2013 +0200 net: mvneta: properly disable HW PHY polling and ensure adjust_link() works This commit fixes a long-standing bug that has been reported by many users: on some Armada 370 platforms, only the network interface that has been used in U-Boot to tftp the kernel works properly in Linux. The other network interfaces can see a 'link up', but are unable to transmit data. The reports were generally made on the Armada 370-based Mirabox, but have also been given on the Armada 370-RD board. [SNIP] $ git tag --contains 714086029116 v3.12 v3.12-rc1 v3.12-rc2 v3.12-rc3 v3.12-rc4 v3.12-rc5 v3.12-rc6 v3.12-rc7 So the problem was indeed fixed at the beginning of 3.12 series by Thomas. Anyway, my bad and thanks again for pointing it out. Cheers, a+ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net: mv643xx_eth: Add missing phy_addr_set in DT mode
On Tue, Nov 05, 2013 at 11:12:00PM +0100, Arnaud Ebalard wrote: > Hi Jason, > > Jason Gunthorpe writes: > > > Commit cc9d4598 'net: mv643xx_eth: use of_phy_connect if phy_node > > present' made the call to phy_scan optional, if the DT has a link to > > the phy node. > > > > However phy_scan has the side effect of calling phy_addr_set, which > > writes the phy MDIO address to the ethernet controller. If phy_addr_set > > is not called, and the bootloader has not set the correct address then > > the driver will fail to function. > > Thanks *a lot* for fixing this one! I had the issue on my ReadyNAS 102 > (Armada 370 based) which I had put on a todo list and temporarily readynas duo v2? thx, Jason. > workarounded by including a 'ping whatever' call in my u-boot env in > order to force it to do the init. Without it, I was unable to properly > use the interface. With your fix, after multiple reboots to test it, > everything works as expected. So, FWIW: > > Tested-by: Arnaud Ebalard > > Cheers, > > a+ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net: mv643xx_eth: Add missing phy_addr_set in DT mode
Hi Jason, Jason Gunthorpe writes: > Commit cc9d4598 'net: mv643xx_eth: use of_phy_connect if phy_node > present' made the call to phy_scan optional, if the DT has a link to > the phy node. > > However phy_scan has the side effect of calling phy_addr_set, which > writes the phy MDIO address to the ethernet controller. If phy_addr_set > is not called, and the bootloader has not set the correct address then > the driver will fail to function. Thanks *a lot* for fixing this one! I had the issue on my ReadyNAS 102 (Armada 370 based) which I had put on a todo list and temporarily workarounded by including a 'ping whatever' call in my u-boot env in order to force it to do the init. Without it, I was unable to properly use the interface. With your fix, after multiple reboots to test it, everything works as expected. So, FWIW: Tested-by: Arnaud Ebalard Cheers, a+ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net: mv643xx_eth: Add missing phy_addr_set in DT mode
On 11/05/2013 11:12 PM, Arnaud Ebalard wrote: Hi Jason, Jason Gunthorpe writes: Commit cc9d4598 'net: mv643xx_eth: use of_phy_connect if phy_node present' made the call to phy_scan optional, if the DT has a link to the phy node. However phy_scan has the side effect of calling phy_addr_set, which writes the phy MDIO address to the ethernet controller. If phy_addr_set is not called, and the bootloader has not set the correct address then the driver will fail to function. Thanks *a lot* for fixing this one! I had the issue on my ReadyNAS 102 (Armada 370 based) which I had put on a todo list and temporarily Erm, just to make sure: Armada 370 isn't using mv643xx_eth but mvneta, are you sure it is (was) related to Jason's fix? Sebastian workarounded by including a 'ping whatever' call in my u-boot env in order to force it to do the init. Without it, I was unable to properly use the interface. With your fix, after multiple reboots to test it, everything works as expected. So, FWIW: Tested-by: Arnaud Ebalard -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net: mv643xx_eth: Add missing phy_addr_set in DT mode
On 11/05/2013 01:27 AM, Jason Gunthorpe wrote: Commit cc9d4598 'net: mv643xx_eth: use of_phy_connect if phy_node present' made the call to phy_scan optional, if the DT has a link to the phy node. However phy_scan has the side effect of calling phy_addr_set, which writes the phy MDIO address to the ethernet controller. If phy_addr_set is not called, and the bootloader has not set the correct address then the driver will fail to function. Tested on Kirkwood. Signed-off-by: Jason Gunthorpe --- Jason, thanks for catching this! I do my kirkwood testing on Dockstar, which has PHY addr 0x0 - also the reset default, which may be why it slipped through. Acked-by: Sebastian Hesselbarth --- drivers/net/ethernet/marvell/mv643xx_eth.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c index 2c210ec..00e43b5 100644 --- a/drivers/net/ethernet/marvell/mv643xx_eth.c +++ b/drivers/net/ethernet/marvell/mv643xx_eth.c @@ -2890,6 +2890,7 @@ static int mv643xx_eth_probe(struct platform_device *pdev) PHY_INTERFACE_MODE_GMII); if (!mp->phy) err = -ENODEV; + phy_addr_set(mp, mp->phy->addr); } else if (pd->phy_addr != MV643XX_ETH_PHY_NONE) { mp->phy = phy_scan(mp, pd->phy_addr); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net: mv643xx_eth: Add missing phy_addr_set in DT mode
On 11/05/2013 01:27 AM, Jason Gunthorpe wrote: Commit cc9d4598 'net: mv643xx_eth: use of_phy_connect if phy_node present' made the call to phy_scan optional, if the DT has a link to the phy node. However phy_scan has the side effect of calling phy_addr_set, which writes the phy MDIO address to the ethernet controller. If phy_addr_set is not called, and the bootloader has not set the correct address then the driver will fail to function. Tested on Kirkwood. Signed-off-by: Jason Gunthorpe jguntho...@obsidianresearch.com --- Jason, thanks for catching this! I do my kirkwood testing on Dockstar, which has PHY addr 0x0 - also the reset default, which may be why it slipped through. Acked-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com --- drivers/net/ethernet/marvell/mv643xx_eth.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c index 2c210ec..00e43b5 100644 --- a/drivers/net/ethernet/marvell/mv643xx_eth.c +++ b/drivers/net/ethernet/marvell/mv643xx_eth.c @@ -2890,6 +2890,7 @@ static int mv643xx_eth_probe(struct platform_device *pdev) PHY_INTERFACE_MODE_GMII); if (!mp-phy) err = -ENODEV; + phy_addr_set(mp, mp-phy-addr); } else if (pd-phy_addr != MV643XX_ETH_PHY_NONE) { mp-phy = phy_scan(mp, pd-phy_addr); -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net: mv643xx_eth: Add missing phy_addr_set in DT mode
On 11/05/2013 11:12 PM, Arnaud Ebalard wrote: Hi Jason, Jason Gunthorpe jguntho...@obsidianresearch.com writes: Commit cc9d4598 'net: mv643xx_eth: use of_phy_connect if phy_node present' made the call to phy_scan optional, if the DT has a link to the phy node. However phy_scan has the side effect of calling phy_addr_set, which writes the phy MDIO address to the ethernet controller. If phy_addr_set is not called, and the bootloader has not set the correct address then the driver will fail to function. Thanks *a lot* for fixing this one! I had the issue on my ReadyNAS 102 (Armada 370 based) which I had put on a todo list and temporarily Erm, just to make sure: Armada 370 isn't using mv643xx_eth but mvneta, are you sure it is (was) related to Jason's fix? Sebastian workarounded by including a 'ping whatever' call in my u-boot env in order to force it to do the init. Without it, I was unable to properly use the interface. With your fix, after multiple reboots to test it, everything works as expected. So, FWIW: Tested-by: Arnaud Ebalard a...@natisbad.org -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net: mv643xx_eth: Add missing phy_addr_set in DT mode
Hi Jason, Jason Gunthorpe jguntho...@obsidianresearch.com writes: Commit cc9d4598 'net: mv643xx_eth: use of_phy_connect if phy_node present' made the call to phy_scan optional, if the DT has a link to the phy node. However phy_scan has the side effect of calling phy_addr_set, which writes the phy MDIO address to the ethernet controller. If phy_addr_set is not called, and the bootloader has not set the correct address then the driver will fail to function. Thanks *a lot* for fixing this one! I had the issue on my ReadyNAS 102 (Armada 370 based) which I had put on a todo list and temporarily workarounded by including a 'ping whatever' call in my u-boot env in order to force it to do the init. Without it, I was unable to properly use the interface. With your fix, after multiple reboots to test it, everything works as expected. So, FWIW: Tested-by: Arnaud Ebalard a...@natisbad.org Cheers, a+ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net: mv643xx_eth: Add missing phy_addr_set in DT mode
On Tue, Nov 05, 2013 at 11:12:00PM +0100, Arnaud Ebalard wrote: Hi Jason, Jason Gunthorpe jguntho...@obsidianresearch.com writes: Commit cc9d4598 'net: mv643xx_eth: use of_phy_connect if phy_node present' made the call to phy_scan optional, if the DT has a link to the phy node. However phy_scan has the side effect of calling phy_addr_set, which writes the phy MDIO address to the ethernet controller. If phy_addr_set is not called, and the bootloader has not set the correct address then the driver will fail to function. Thanks *a lot* for fixing this one! I had the issue on my ReadyNAS 102 (Armada 370 based) which I had put on a todo list and temporarily readynas duo v2? thx, Jason. workarounded by including a 'ping whatever' call in my u-boot env in order to force it to do the init. Without it, I was unable to properly use the interface. With your fix, after multiple reboots to test it, everything works as expected. So, FWIW: Tested-by: Arnaud Ebalard a...@natisbad.org Cheers, a+ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net: mv643xx_eth: Add missing phy_addr_set in DT mode
Hi, Sebastian Hesselbarth sebastian.hesselba...@gmail.com writes: On 11/05/2013 11:12 PM, Arnaud Ebalard wrote: Hi Jason, Jason Gunthorpe jguntho...@obsidianresearch.com writes: Commit cc9d4598 'net: mv643xx_eth: use of_phy_connect if phy_node present' made the call to phy_scan optional, if the DT has a link to the phy node. However phy_scan has the side effect of calling phy_addr_set, which writes the phy MDIO address to the ethernet controller. If phy_addr_set is not called, and the bootloader has not set the correct address then the driver will fail to function. Thanks *a lot* for fixing this one! I had the issue on my ReadyNAS 102 (Armada 370 based) which I had put on a todo list and temporarily Erm, just to make sure: Armada 370 isn't using mv643xx_eth but mvneta, are you sure it is (was) related to Jason's fix? Thanks for pointing this, Sebastian and my apologies for the noise. Jason's fix is indeed for a file which is not compiled for my RN102. As the problem perfectly matched the issue I had and current kernel w/ the patch applied does indeed fix it, I did not try and do the test w/o the patch applied. It would have showed the problem was fixed by something else in 3.12. Well, I spent some time digging the changes on mvneta.c and: commit 714086029116b6b0a34e67ba1dd2f0d1cf26770c Author: Thomas Petazzoni thomas.petazz...@free-electrons.com Date: Wed Sep 4 16:21:18 2013 +0200 net: mvneta: properly disable HW PHY polling and ensure adjust_link() works This commit fixes a long-standing bug that has been reported by many users: on some Armada 370 platforms, only the network interface that has been used in U-Boot to tftp the kernel works properly in Linux. The other network interfaces can see a 'link up', but are unable to transmit data. The reports were generally made on the Armada 370-based Mirabox, but have also been given on the Armada 370-RD board. [SNIP] $ git tag --contains 714086029116 v3.12 v3.12-rc1 v3.12-rc2 v3.12-rc3 v3.12-rc4 v3.12-rc5 v3.12-rc6 v3.12-rc7 So the problem was indeed fixed at the beginning of 3.12 series by Thomas. Anyway, my bad and thanks again for pointing it out. Cheers, a+ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] net: mv643xx_eth: Add missing phy_addr_set in DT mode
Commit cc9d4598 'net: mv643xx_eth: use of_phy_connect if phy_node present' made the call to phy_scan optional, if the DT has a link to the phy node. However phy_scan has the side effect of calling phy_addr_set, which writes the phy MDIO address to the ethernet controller. If phy_addr_set is not called, and the bootloader has not set the correct address then the driver will fail to function. Tested on Kirkwood. Signed-off-by: Jason Gunthorpe --- Cc: David Miller Cc: Lennert Buytenhek Cc: Jason Cooper Cc: Andrew Lunn Cc: Benjamin Herrenschmidt Cc: net...@vger.kernel.org Cc: linux-arm-ker...@lists.infradead.org Cc: linuxppc-...@lists.ozlabs.org Cc: linux-kernel@vger.kernel.org --- drivers/net/ethernet/marvell/mv643xx_eth.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c index 2c210ec..00e43b5 100644 --- a/drivers/net/ethernet/marvell/mv643xx_eth.c +++ b/drivers/net/ethernet/marvell/mv643xx_eth.c @@ -2890,6 +2890,7 @@ static int mv643xx_eth_probe(struct platform_device *pdev) PHY_INTERFACE_MODE_GMII); if (!mp->phy) err = -ENODEV; + phy_addr_set(mp, mp->phy->addr); } else if (pd->phy_addr != MV643XX_ETH_PHY_NONE) { mp->phy = phy_scan(mp, pd->phy_addr); -- 1.8.1.2 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net: mv643xx_eth: Add missing phy_addr_set in DT mode
Jason, On Mon, Nov 04, 2013 at 05:27:19PM -0700, Jason Gunthorpe wrote: > Commit cc9d4598 'net: mv643xx_eth: use of_phy_connect if phy_node fyi: set core.abbrev = 12 in your git config, according to Linus, 7/8 was a bad decision... > present' made the call to phy_scan optional, if the DT has a link to > the phy node. > > However phy_scan has the side effect of calling phy_addr_set, which > writes the phy MDIO address to the ethernet controller. If phy_addr_set > is not called, and the bootloader has not set the correct address then > the driver will fail to function. > > Tested on Kirkwood. > > Signed-off-by: Jason Gunthorpe Fixes: cc9d459894b0 "net: mv643xx_eth: use of_phy_connect if phy_node present" Acked-by: Jason Cooper And it should be suitable for v3.11+ thx, Jason. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] net: mv643xx_eth: Add missing phy_addr_set in DT mode
Jason, On Mon, Nov 04, 2013 at 05:27:19PM -0700, Jason Gunthorpe wrote: Commit cc9d4598 'net: mv643xx_eth: use of_phy_connect if phy_node fyi: set core.abbrev = 12 in your git config, according to Linus, 7/8 was a bad decision... present' made the call to phy_scan optional, if the DT has a link to the phy node. However phy_scan has the side effect of calling phy_addr_set, which writes the phy MDIO address to the ethernet controller. If phy_addr_set is not called, and the bootloader has not set the correct address then the driver will fail to function. Tested on Kirkwood. Signed-off-by: Jason Gunthorpe jguntho...@obsidianresearch.com Fixes: cc9d459894b0 net: mv643xx_eth: use of_phy_connect if phy_node present Acked-by: Jason Cooper ja...@lakedaemon.net And it should be suitable for v3.11+ thx, Jason. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] net: mv643xx_eth: Add missing phy_addr_set in DT mode
Commit cc9d4598 'net: mv643xx_eth: use of_phy_connect if phy_node present' made the call to phy_scan optional, if the DT has a link to the phy node. However phy_scan has the side effect of calling phy_addr_set, which writes the phy MDIO address to the ethernet controller. If phy_addr_set is not called, and the bootloader has not set the correct address then the driver will fail to function. Tested on Kirkwood. Signed-off-by: Jason Gunthorpe jguntho...@obsidianresearch.com --- Cc: David Miller da...@davemloft.net Cc: Lennert Buytenhek buyt...@wantstofly.org Cc: Jason Cooper ja...@lakedaemon.net Cc: Andrew Lunn and...@lunn.ch Cc: Benjamin Herrenschmidt b...@kernel.crashing.org Cc: net...@vger.kernel.org Cc: linux-arm-ker...@lists.infradead.org Cc: linuxppc-...@lists.ozlabs.org Cc: linux-kernel@vger.kernel.org --- drivers/net/ethernet/marvell/mv643xx_eth.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c index 2c210ec..00e43b5 100644 --- a/drivers/net/ethernet/marvell/mv643xx_eth.c +++ b/drivers/net/ethernet/marvell/mv643xx_eth.c @@ -2890,6 +2890,7 @@ static int mv643xx_eth_probe(struct platform_device *pdev) PHY_INTERFACE_MODE_GMII); if (!mp-phy) err = -ENODEV; + phy_addr_set(mp, mp-phy-addr); } else if (pd-phy_addr != MV643XX_ETH_PHY_NONE) { mp-phy = phy_scan(mp, pd-phy_addr); -- 1.8.1.2 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/