All of lore.kernel.org
 help / color / mirror / Atom feed
From: arno@natisbad.org (Arnaud Ebalard)
To: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>,
	Andrew Lunn <andrew@lunn.ch>, Jason Cooper <jason@lakedaemon.net>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Jason Gunthorpe <jgunthorpe@obsidianresearch.com>,
	Lennert Buytenhek <buytenh@wantstofly.org>,
	linuxppc-dev@lists.ozlabs.org, David Miller <davem@davemloft.net>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] net: mv643xx_eth: Add missing phy_addr_set in DT mode
Date: Wed, 06 Nov 2013 00:14:34 +0100	[thread overview]
Message-ID: <87iow6cug5.fsf@natisbad.org> (raw)
In-Reply-To: <52796E82.5010800@gmail.com> (Sebastian Hesselbarth's message of "Tue, 05 Nov 2013 23:17:38 +0100")

Hi,

Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> writes:

> On 11/05/2013 11:12 PM, Arnaud Ebalard wrote:
>> Hi Jason,
>>
>> Jason Gunthorpe <jgunthorpe@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.petazzoni@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+

WARNING: multiple messages have this Message-ID (diff)
From: arno@natisbad.org (Arnaud Ebalard)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] net: mv643xx_eth: Add missing phy_addr_set in DT mode
Date: Wed, 06 Nov 2013 00:14:34 +0100	[thread overview]
Message-ID: <87iow6cug5.fsf@natisbad.org> (raw)
In-Reply-To: <52796E82.5010800@gmail.com> (Sebastian Hesselbarth's message of "Tue, 05 Nov 2013 23:17:38 +0100")

Hi,

Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> writes:

> On 11/05/2013 11:12 PM, Arnaud Ebalard wrote:
>> Hi Jason,
>>
>> Jason Gunthorpe <jgunthorpe@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.petazzoni@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+

WARNING: multiple messages have this Message-ID (diff)
From: arno@natisbad.org (Arnaud Ebalard)
To: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>,
	Andrew Lunn <andrew@lunn.ch>, Jason Cooper <jason@lakedaemon.net>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	linuxppc-dev@lists.ozlabs.org, David Miller <davem@davemloft.net>,
	Lennert Buytenhek <buytenh@wantstofly.org>,
	Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Subject: Re: [PATCH] net: mv643xx_eth: Add missing phy_addr_set in DT mode
Date: Wed, 06 Nov 2013 00:14:34 +0100	[thread overview]
Message-ID: <87iow6cug5.fsf@natisbad.org> (raw)
In-Reply-To: <52796E82.5010800@gmail.com> (Sebastian Hesselbarth's message of "Tue, 05 Nov 2013 23:17:38 +0100")

Hi,

Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> writes:

> On 11/05/2013 11:12 PM, Arnaud Ebalard wrote:
>> Hi Jason,
>>
>> Jason Gunthorpe <jgunthorpe@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.petazzoni@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+

  reply	other threads:[~2013-11-05 23:14 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-05  0:27 [PATCH] net: mv643xx_eth: Add missing phy_addr_set in DT mode Jason Gunthorpe
2013-11-05  0:27 ` Jason Gunthorpe
2013-11-05  0:27 ` Jason Gunthorpe
2013-11-05  0:36 ` Jason Cooper
2013-11-05  0:36   ` Jason Cooper
2013-11-05  0:36   ` Jason Cooper
2013-11-05  0:36   ` Jason Cooper
2013-11-05  9:04 ` Sebastian Hesselbarth
2013-11-05  9:04   ` Sebastian Hesselbarth
2013-11-05  9:04   ` Sebastian Hesselbarth
2013-11-05 22:12 ` Arnaud Ebalard
2013-11-05 22:12   ` Arnaud Ebalard
2013-11-05 22:12   ` Arnaud Ebalard
2013-11-05 22:12   ` Arnaud Ebalard
2013-11-05 22:17   ` Sebastian Hesselbarth
2013-11-05 22:17     ` Sebastian Hesselbarth
2013-11-05 22:17     ` Sebastian Hesselbarth
2013-11-05 23:14     ` Arnaud Ebalard [this message]
2013-11-05 23:14       ` Arnaud Ebalard
2013-11-05 23:14       ` Arnaud Ebalard
2013-11-05 23:00   ` Jason Cooper
2013-11-05 23:00     ` Jason Cooper
2013-11-05 23:00     ` Jason Cooper

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=87iow6cug5.fsf@natisbad.org \
    --to=arno@natisbad.org \
    --cc=andrew@lunn.ch \
    --cc=buytenh@wantstofly.org \
    --cc=davem@davemloft.net \
    --cc=jason@lakedaemon.net \
    --cc=jgunthorpe@obsidianresearch.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=netdev@vger.kernel.org \
    --cc=sebastian.hesselbarth@gmail.com \
    --cc=thomas.petazzoni@free-electrons.com \
    /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.