Patchwork net: Fix ucc_geth.c handling of phy 'interface' property.

login
register
mail settings
Submitter Grant Likely
Date April 27, 2009, 3:36 p.m.
Message ID <20090427153612.5108.10877.stgit@localhost.localdomain>
Download mbox | patch
Permalink /patch/26504/
State Accepted
Delegated to: Grant Likely
Headers show

Comments

Grant Likely - April 27, 2009, 3:36 p.m.
From: Grant Likely <grant.likely@secretlab.ca>

Previous rework to ucc_geth.c to add of_mdio support (net: Rework
ucc_geth driver to use of_mdio infrastructure) added a block of
code which broke older openfirmware device trees which use an
'interface' property in the phy node to describe the connection
between the MAC and the PHY.  This patch removes the offending blurb.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---

 drivers/net/ucc_geth.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)
Scott Wood - April 27, 2009, 5:36 p.m.
On Mon, Apr 27, 2009 at 09:36:13AM -0600, Grant Likely wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
> 
> Previous rework to ucc_geth.c to add of_mdio support (net: Rework
> ucc_geth driver to use of_mdio infrastructure) added a block of
> code which broke older openfirmware device trees which use an
> 'interface' property in the phy node to describe the connection
> between the MAC and the PHY.  This patch removes the offending blurb.
[snip]
>  	prop = of_get_property(np, "phy-connection-type", NULL);
>  	if (!prop) {
>  		/* handle interface property present in old trees */
> -		if (!phy)
> -			return -ENODEV;
> -
>  		prop = of_get_property(phy, "interface", NULL);
>  		if (prop != NULL) {
>  			phy_interface = enet_to_phy_interface[*prop];

The above test only makes a difference when there is no phy node -- so I
don't see how it was breaking device trees that had a phy node (with or
without an "interface" property).  I can see it breaking fixed link
device trees, though.

-Scott
Grant Likely - April 27, 2009, 5:50 p.m.
On Mon, Apr 27, 2009 at 11:36 AM, Scott Wood <scottwood@freescale.com> wrote:
> On Mon, Apr 27, 2009 at 09:36:13AM -0600, Grant Likely wrote:
>> From: Grant Likely <grant.likely@secretlab.ca>
>>
>> Previous rework to ucc_geth.c to add of_mdio support (net: Rework
>> ucc_geth driver to use of_mdio infrastructure) added a block of
>> code which broke older openfirmware device trees which use an
>> 'interface' property in the phy node to describe the connection
>> between the MAC and the PHY.  This patch removes the offending blurb.
> [snip]
>>       prop = of_get_property(np, "phy-connection-type", NULL);
>>       if (!prop) {
>>               /* handle interface property present in old trees */
>> -             if (!phy)
>> -                     return -ENODEV;
>> -
>>               prop = of_get_property(phy, "interface", NULL);
>>               if (prop != NULL) {
>>                       phy_interface = enet_to_phy_interface[*prop];
>
> The above test only makes a difference when there is no phy node -- so I
> don't see how it was breaking device trees that had a phy node (with or
> without an "interface" property).  I can see it breaking fixed link
> device trees, though.

Sorry, you're right.  I got myself confused when I was writing the
description.  This patch fixes breakage when using a fixed-link and
there is no phy-connection-type property.

g.
David Miller - April 28, 2009, 9:12 a.m.
From: Grant Likely <grant.likely@secretlab.ca>
Date: Mon, 27 Apr 2009 11:50:20 -0600

> On Mon, Apr 27, 2009 at 11:36 AM, Scott Wood <scottwood@freescale.com> wrote:
>> On Mon, Apr 27, 2009 at 09:36:13AM -0600, Grant Likely wrote:
>>> From: Grant Likely <grant.likely@secretlab.ca>
>>>
>>> Previous rework to ucc_geth.c to add of_mdio support (net: Rework
>>> ucc_geth driver to use of_mdio infrastructure) added a block of
>>> code which broke older openfirmware device trees which use an
>>> 'interface' property in the phy node to describe the connection
>>> between the MAC and the PHY.  This patch removes the offending blurb.
>> [snip]
>>>       prop = of_get_property(np, "phy-connection-type", NULL);
>>>       if (!prop) {
>>>               /* handle interface property present in old trees */
>>> -             if (!phy)
>>> -                     return -ENODEV;
>>> -
>>>               prop = of_get_property(phy, "interface", NULL);
>>>               if (prop != NULL) {
>>>                       phy_interface = enet_to_phy_interface[*prop];
>>
>> The above test only makes a difference when there is no phy node -- so I
>> don't see how it was breaking device trees that had a phy node (with or
>> without an "interface" property).  I can see it breaking fixed link
>> device trees, though.
> 
> Sorry, you're right.  I got myself confused when I was writing the
> description.  This patch fixes breakage when using a fixed-link and
> there is no phy-connection-type property.

Patch applied with commit message fixed up.
Grant Likely - April 28, 2009, 1:37 p.m.
On Tue, Apr 28, 2009 at 3:12 AM, David Miller <davem@davemloft.net> wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
> Date: Mon, 27 Apr 2009 11:50:20 -0600
>
>> On Mon, Apr 27, 2009 at 11:36 AM, Scott Wood <scottwood@freescale.com> wrote:
>>> On Mon, Apr 27, 2009 at 09:36:13AM -0600, Grant Likely wrote:
>>>> From: Grant Likely <grant.likely@secretlab.ca>
>>>>
>>>> Previous rework to ucc_geth.c to add of_mdio support (net: Rework
>>>> ucc_geth driver to use of_mdio infrastructure) added a block of
>>>> code which broke older openfirmware device trees which use an
>>>> 'interface' property in the phy node to describe the connection
>>>> between the MAC and the PHY.  This patch removes the offending blurb.
>>> [snip]
>>>>       prop = of_get_property(np, "phy-connection-type", NULL);
>>>>       if (!prop) {
>>>>               /* handle interface property present in old trees */
>>>> -             if (!phy)
>>>> -                     return -ENODEV;
>>>> -
>>>>               prop = of_get_property(phy, "interface", NULL);
>>>>               if (prop != NULL) {
>>>>                       phy_interface = enet_to_phy_interface[*prop];
>>>
>>> The above test only makes a difference when there is no phy node -- so I
>>> don't see how it was breaking device trees that had a phy node (with or
>>> without an "interface" property).  I can see it breaking fixed link
>>> device trees, though.
>>
>> Sorry, you're right.  I got myself confused when I was writing the
>> description.  This patch fixes breakage when using a fixed-link and
>> there is no phy-connection-type property.
>
> Patch applied with commit message fixed up.

Thanks David

g.

Patch

diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index 195b490..d805d60 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -3639,9 +3639,6 @@  static int ucc_geth_probe(struct of_device* ofdev, const struct of_device_id *ma
 	prop = of_get_property(np, "phy-connection-type", NULL);
 	if (!prop) {
 		/* handle interface property present in old trees */
-		if (!phy)
-			return -ENODEV;
-
 		prop = of_get_property(phy, "interface", NULL);
 		if (prop != NULL) {
 			phy_interface = enet_to_phy_interface[*prop];