diff mbox

[PATCHv2,net] phy: micrel: Fix finding PHY properties in MAC node.

Message ID 1449687391-15331-1-git-send-email-andrew@lunn.ch
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Andrew Lunn Dec. 9, 2015, 6:56 p.m. UTC
commit 8b63ec1837fa ("phylib: Make PHYs children of their MDIO bus,
not the bus' parent.")  changed the parenting of PHY devices, making
them a child of the MDIO bus, instead of the MAC device. This broken
the Micrel PHY driver which has a deprecated feature of allowing PHY
properties to be placed into the MAC node.

In order to find the MAC node, we need to walk up the tree of devices
until we find one with an OF node attached.

Reported-by: Dinh Nguyen <dinguyen@opensource.altera.com>
Suggested-by: David Daney <david.daney@cavium.com>
Acked-by: David Daney <david.daney@cavium.com>
Fixes: 8b63ec1837fa ("phylib: Make PHYs children of their MDIO bus, not the bus' parent.")
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
v2: Remove include of netdev.h
---
 drivers/net/phy/micrel.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Comments

Dinh Nguyen Dec. 9, 2015, 7:39 p.m. UTC | #1
On 12/09/2015 12:56 PM, Andrew Lunn wrote:
> commit 8b63ec1837fa ("phylib: Make PHYs children of their MDIO bus,
> not the bus' parent.")  changed the parenting of PHY devices, making
> them a child of the MDIO bus, instead of the MAC device. This broken
> the Micrel PHY driver which has a deprecated feature of allowing PHY
> properties to be placed into the MAC node.
> 
> In order to find the MAC node, we need to walk up the tree of devices
> until we find one with an OF node attached.
> 
> Reported-by: Dinh Nguyen <dinguyen@opensource.altera.com>
> Suggested-by: David Daney <david.daney@cavium.com>
> Acked-by: David Daney <david.daney@cavium.com>
> Fixes: 8b63ec1837fa ("phylib: Make PHYs children of their MDIO bus, not the bus' parent.")
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
> v2: Remove include of netdev.h
> ---

Feel free to add:

Tested-by: Dinh Nguyen <dinguyen@opensource.altera.com>

Thanks,
Dinh

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Florian Fainelli Dec. 9, 2015, 9:40 p.m. UTC | #2
On December 9, 2015 10:56:31 AM PST, Andrew Lunn <andrew@lunn.ch> wrote:
>commit 8b63ec1837fa ("phylib: Make PHYs children of their MDIO bus,
>not the bus' parent.")  changed the parenting of PHY devices, making
>them a child of the MDIO bus, instead of the MAC device. This broken
>the Micrel PHY driver which has a deprecated feature of allowing PHY
>properties to be placed into the MAC node.
>
>In order to find the MAC node, we need to walk up the tree of devices
>until we find one with an OF node attached.
>
>Reported-by: Dinh Nguyen <dinguyen@opensource.altera.com>
>Suggested-by: David Daney <david.daney@cavium.com>
>Acked-by: David Daney <david.daney@cavium.com>
>Fixes: 8b63ec1837fa ("phylib: Make PHYs children of their MDIO bus, not
>the bus' parent.")
>Signed-off-by: Andrew Lunn <andrew@lunn.ch>
>---

>-	if (!of_node && dev->parent->of_node)
>-		of_node = dev->parent->of_node;
>+	/* The Micrel driver has a deprecated option to place phy OF
>+	 * properties in the MAC node. Walk up the tree of devices to
>+	 * find a device with an OF node.
>+	 */
>+	dev_walker = &phydev->dev;
>+	do {
>+		of_node = dev_walker->of_node;
>+		dev_walker = dev_walker->parent;
>+
>+	} while (!of_node && dev_walker);

Looks good to me, can we also issuing à warning so people get a chance to update their Device Tree?

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> 
> 	if (of_node) {
> 		ksz9021_load_values_from_of(phydev, of_node,
Andrew Lunn Dec. 9, 2015, 9:57 p.m. UTC | #3
> >-	if (!of_node && dev->parent->of_node)
> >-		of_node = dev->parent->of_node;
> >+	/* The Micrel driver has a deprecated option to place phy OF
> >+	 * properties in the MAC node. Walk up the tree of devices to
> >+	 * find a device with an OF node.
> >+	 */
> >+	dev_walker = &phydev->dev;
> >+	do {
> >+		of_node = dev_walker->of_node;
> >+		dev_walker = dev_walker->parent;
> >+
> >+	} while (!of_node && dev_walker);
> 
> Looks good to me, can we also issuing à warning so people get a chance to update their Device Tree?

I did wounder about that.

But i think it might be better to wait until there are updated dts
files. No point telling users they need to update when there are no
updated DT files to use.

If Dinh updates the SoCFPGA .dts files then i think it makes sense to
add the warning.

    Andrew
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Dec. 12, 2015, 1:32 a.m. UTC | #4
From: Andrew Lunn <andrew@lunn.ch>
Date: Wed,  9 Dec 2015 19:56:31 +0100

> commit 8b63ec1837fa ("phylib: Make PHYs children of their MDIO bus,
> not the bus' parent.")  changed the parenting of PHY devices, making
> them a child of the MDIO bus, instead of the MAC device. This broken
> the Micrel PHY driver which has a deprecated feature of allowing PHY
> properties to be placed into the MAC node.
> 
> In order to find the MAC node, we need to walk up the tree of devices
> until we find one with an OF node attached.
> 
> Reported-by: Dinh Nguyen <dinguyen@opensource.altera.com>
> Suggested-by: David Daney <david.daney@cavium.com>
> Acked-by: David Daney <david.daney@cavium.com>
> Fixes: 8b63ec1837fa ("phylib: Make PHYs children of their MDIO bus, not the bus' parent.")
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Applied and queued up for -stable, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index cf6312fafea5..e13ad6cdcc22 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -339,9 +339,18 @@  static int ksz9021_config_init(struct phy_device *phydev)
 {
 	const struct device *dev = &phydev->dev;
 	const struct device_node *of_node = dev->of_node;
+	const struct device *dev_walker;
 
-	if (!of_node && dev->parent->of_node)
-		of_node = dev->parent->of_node;
+	/* The Micrel driver has a deprecated option to place phy OF
+	 * properties in the MAC node. Walk up the tree of devices to
+	 * find a device with an OF node.
+	 */
+	dev_walker = &phydev->dev;
+	do {
+		of_node = dev_walker->of_node;
+		dev_walker = dev_walker->parent;
+
+	} while (!of_node && dev_walker);
 
 	if (of_node) {
 		ksz9021_load_values_from_of(phydev, of_node,