Re: [PATCH] net: mv643xx_eth: Add missing phy_addr_set in DT mode

2013-11-05 Thread Arnaud Ebalard
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

2013-11-05 Thread Jason Cooper
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

2013-11-05 Thread Arnaud Ebalard
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

2013-11-05 Thread Sebastian Hesselbarth

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

2013-11-05 Thread Sebastian Hesselbarth

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

2013-11-05 Thread Sebastian Hesselbarth

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

2013-11-05 Thread Sebastian Hesselbarth

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

2013-11-05 Thread Arnaud Ebalard
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

2013-11-05 Thread Jason Cooper
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

2013-11-05 Thread Arnaud Ebalard
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

2013-11-04 Thread Jason Gunthorpe
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

2013-11-04 Thread Jason Cooper
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

2013-11-04 Thread Jason Cooper
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

2013-11-04 Thread Jason Gunthorpe
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/