[PATCH 0/3] mdio: Parse DT nodes for auto-probed PHYs

[PATCH 0/3] mdio: Parse DT nodes for auto-probed PHYs

[Daniel Mack]

Hi Florian,

here's another aproach of solving the issue with auto-probed buses and
DT device nodes. In short, this patch set introduces a new hook which
sets the of_node pointer of a phy device to a subnode of the bus, if
there is any which matches the phy's address. This function is called
from mdiobus_scan(), and is a no-op for !CONFIG_OF.

Unlike with with the 1st version, dev->of_node is now available in the
PHY driver's probe() callback already.

Let me know what you think, I'm open to other ideas :)


Daniel


Daniel Mack (3):
  net: of_mdio: factor out code to parse a phy's 'reg' property
  net: of_mdio: add of_mdiobus_link_phydev()
  net: of_mdio: don't store the length of a property if we don't need to

 drivers/net/phy/mdio_bus.c |  6 ++++
 drivers/of/of_mdio.c       | 74 +++++++++++++++++++++++++++++++++++++---------
 include/linux/of_mdio.h    |  8 +++++
 3 files changed, 74 insertions(+), 14 deletions(-)

-- 
1.9.0

[PATCH 1/3] net: of_mdio: factor out code to parse a phy's 'reg' property

[Daniel Mack]

Factor out some logic into of_mdio_parse_addr() so it can be reused
later.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 drivers/of/of_mdio.c | 37 +++++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 9a95831..171f9d5 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -114,6 +114,29 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio, struct device_node *chi
 	return 0;
 }
 
+static int of_mdio_parse_addr(struct device *dev, const struct device_node *np)
+{
+	const __be32 *paddr;
+	u32 addr;
+	int len;
+
+	/* A PHY must have a reg property in the range [0-31] */
+	paddr = of_get_property(np, "reg", &len);
+	if (!paddr || len < sizeof(*paddr)) {
+		dev_err(dev, "%s has invalid PHY address\n", np->full_name);
+		return -EINVAL;
+	}
+
+	addr = be32_to_cpup(paddr);
+	if (addr >= PHY_MAX_ADDR) {
+		dev_err(dev, "%s PHY address %i is too large\n",
+			np->full_name, addr);
+		return -EINVAL;
+	}
+
+	return addr;
+}
+
 /**
  * of_mdiobus_register - Register mii_bus and create PHYs from the device tree
  * @mdio: pointer to mii_bus structure
@@ -148,19 +171,9 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
 
 	/* Loop over the child nodes and register a phy_device for each one */
 	for_each_available_child_of_node(np, child) {
-		/* A PHY must have a reg property in the range [0-31] */
-		paddr = of_get_property(child, "reg", &len);
-		if (!paddr || len < sizeof(*paddr)) {
+		addr = of_mdio_parse_addr(&mdio->dev, child);
+		if (addr < 0) {
 			scanphys = true;
-			dev_err(&mdio->dev, "%s has invalid PHY address\n",
-				child->full_name);
-			continue;
-		}
-
-		addr = be32_to_cpup(paddr);
-		if (addr >= PHY_MAX_ADDR) {
-			dev_err(&mdio->dev, "%s PHY address %i is too large\n",
-				child->full_name, addr);
 			continue;
 		}
 
-- 
1.9.0

[PATCH 2/3] net: of_mdio: add of_mdiobus_link_phydev()

[Daniel Mack]

Add a function to Walk the list of subnodes of a mdio bus and look for
a node that matches the phy's address with its 'reg' property. If found,
set the of_node pointer for the phy. This allows auto-probed pyh
devices to be augmented by information passed in via DT.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 drivers/net/phy/mdio_bus.c |  6 ++++++
 drivers/of/of_mdio.c       | 33 +++++++++++++++++++++++++++++++++
 include/linux/of_mdio.h    |  8 ++++++++
 3 files changed, 47 insertions(+)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 76f54b3..e1444b2 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -233,6 +233,12 @@ struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr)
 	if (IS_ERR(phydev) || phydev == NULL)
 		return phydev;
 
+	/*
+	 * For DT, see if the auto-probed phy has a correspoding child
+	 * in the bus node, and set the of_node pointer in this case.
+	 */
+	of_mdiobus_link_phydev(bus, phydev);
+
 	err = phy_device_register(phydev);
 	if (err) {
 		phy_device_free(phydev);
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 171f9d5..931a977 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -211,6 +211,39 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
 }
 EXPORT_SYMBOL(of_mdiobus_register);
 
+/**
+ * of_mdiobus_link_phydev - Find a device node for a phy
+ * @mdio: pointer to mii_bus structure
+ * @phydev: phydev for which the of_node pointer should be set
+ *
+ * Walk the list of subnodes of a mdio bus and look for a node that matches the
+ * phy's address with its 'reg' property. If found, set the of_node pointer for
+ * the phy. This allows auto-probed pyh devices to be supplied with information
+ * passed in via DT.
+ */
+void of_mdiobus_link_phydev(struct mii_bus *mdio,
+			    struct phy_device *phydev)
+{
+	struct device *dev = &phydev->dev;
+	struct device_node *child;
+
+	if (dev->of_node)
+		return;
+
+	for_each_available_child_of_node(mdio->dev.of_node, child) {
+		int addr;
+
+		addr = of_mdio_parse_addr(&mdio->dev, child);
+		if (addr < 0)
+			continue;
+
+		if (addr == phydev->addr) {
+			dev->of_node = child;
+			return;
+		}
+	}
+}
+
 /* Helper function for of_phy_find_device */
 static int of_phy_match(struct device *dev, void *phy_np)
 {
diff --git a/include/linux/of_mdio.h b/include/linux/of_mdio.h
index 881a7c3..de72206 100644
--- a/include/linux/of_mdio.h
+++ b/include/linux/of_mdio.h
@@ -28,6 +28,9 @@ extern struct phy_device *of_phy_connect_fixed_link(struct net_device *dev,
 
 extern struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np);
 
+extern void of_mdiobus_link_phydev(struct mii_bus *mdio,
+				   struct phy_device *phydev);
+
 #else /* CONFIG_OF */
 static inline int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
 {
@@ -70,6 +73,11 @@ static inline struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np)
 {
 	return NULL;
 }
+
+static inline void of_mdiobus_link_phydev(struct mii_bus *mdio,
+					  struct phy_device *phydev)
+{
+}
 #endif /* CONFIG_OF */
 
 #endif /* __LINUX_OF_MDIO_H */
-- 
1.9.0

[PATCH 3/3] net: of_mdio: don't store the length of a property if we don't need to

[Daniel Mack]

of_get_property() can be called with NULL as 2nd argument if the caller
is not interested in the length of a property. Use that here so we can
get rid of a variable.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 drivers/of/of_mdio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 931a977..f567210 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -151,7 +151,7 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
 	const __be32 *paddr;
 	u32 addr;
 	bool scanphys = false;
-	int rc, i, len;
+	int rc, i;
 
 	/* Mask out all PHYs from auto probing.  Instead the PHYs listed in
 	 * the device tree are populated after the bus has been registered */
@@ -188,7 +188,7 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
 	/* auto scan for PHYs with empty reg property */
 	for_each_available_child_of_node(np, child) {
 		/* Skip PHYs with reg property set */
-		paddr = of_get_property(child, "reg", &len);
+		paddr = of_get_property(child, "reg", NULL);
 		if (paddr)
 			continue;
 
-- 
1.9.0

Re: [PATCH 1/3] net: of_mdio: factor out code to parse a phy's 'reg' property

[Sergei Shtylyov]

Hello.

On 05/10/2014 08:37 PM, Daniel Mack wrote:

> Factor out some logic into of_mdio_parse_addr() so it can be reused
> later.

> Signed-off-by: Daniel Mack <zonque@gmail.com>
> ---
>   drivers/of/of_mdio.c | 37 +++++++++++++++++++++++++------------
>   1 file changed, 25 insertions(+), 12 deletions(-)

> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index 9a95831..171f9d5 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -114,6 +114,29 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio, struct device_node *chi
>   	return 0;
>   }
>
> +static int of_mdio_parse_addr(struct device *dev, const struct device_node *np)
> +{
> +	const __be32 *paddr;
> +	u32 addr;
> +	int len;
> +
> +	/* A PHY must have a reg property in the range [0-31] */
> +	paddr = of_get_property(np, "reg", &len);
> +	if (!paddr || len < sizeof(*paddr)) {
> +		dev_err(dev, "%s has invalid PHY address\n", np->full_name);
> +		return -EINVAL;
> +	}
> +
> +	addr = be32_to_cpup(paddr);

    Why not just use of_property_read_u32() instead of the above? At least 
that's a material for a follow-up patch...

> +	if (addr >= PHY_MAX_ADDR) {
> +		dev_err(dev, "%s PHY address %i is too large\n",
> +			np->full_name, addr);
> +		return -EINVAL;
> +	}
> +
> +	return addr;
> +}
> +
>   /**
>    * of_mdiobus_register - Register mii_bus and create PHYs from the device tree
>    * @mdio: pointer to mii_bus structure
[...]

WBR, Sergei

Re: [PATCH 0/3] mdio: Parse DT nodes for auto-probed PHYs

[Florian Fainelli]

Hi Daniel,

Le 10/05/2014 09:37, Daniel Mack a écrit :
> Hi Florian,
>
> here's another aproach of solving the issue with auto-probed buses and
> DT device nodes. In short, this patch set introduces a new hook which
> sets the of_node pointer of a phy device to a subnode of the bus, if
> there is any which matches the phy's address. This function is called
> from mdiobus_scan(), and is a no-op for !CONFIG_OF.
>
> Unlike with with the 1st version, dev->of_node is now available in the
> PHY driver's probe() callback already.
>
> Let me know what you think, I'm open to other ideas :)

I like it so far, I will give this a try on some of my quirky hardware. 
Thanks!

>
>
> Daniel
>
>
> Daniel Mack (3):
>    net: of_mdio: factor out code to parse a phy's 'reg' property
>    net: of_mdio: add of_mdiobus_link_phydev()
>    net: of_mdio: don't store the length of a property if we don't need to
>
>   drivers/net/phy/mdio_bus.c |  6 ++++
>   drivers/of/of_mdio.c       | 74 +++++++++++++++++++++++++++++++++++++---------
>   include/linux/of_mdio.h    |  8 +++++
>   3 files changed, 74 insertions(+), 14 deletions(-)
>

Re: [PATCH 1/3] net: of_mdio: factor out code to parse a phy's 'reg' property

[David Miller]

From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Sun, 11 May 2014 00:29:02 +0400

> Hello.
> 
> On 05/10/2014 08:37 PM, Daniel Mack wrote:
> 
>> Factor out some logic into of_mdio_parse_addr() so it can be reused
>> later.
> 
>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>> ---
>>   drivers/of/of_mdio.c | 37 +++++++++++++++++++++++++------------
>>   1 file changed, 25 insertions(+), 12 deletions(-)
> 
>> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
>> index 9a95831..171f9d5 100644
>> --- a/drivers/of/of_mdio.c
>> +++ b/drivers/of/of_mdio.c
>> @@ -114,6 +114,29 @@ static int of_mdiobus_register_phy(struct mii_bus
>> *mdio, struct device_node *chi
>>   	return 0;
>>   }
>>
>> +static int of_mdio_parse_addr(struct device *dev, const struct
>> device_node *np)
>> +{
>> +	const __be32 *paddr;
>> +	u32 addr;
>> +	int len;
>> +
>> +	/* A PHY must have a reg property in the range [0-31] */
>> +	paddr = of_get_property(np, "reg", &len);
>> +	if (!paddr || len < sizeof(*paddr)) {
>> + dev_err(dev, "%s has invalid PHY address\n", np->full_name);
>> +		return -EINVAL;
>> +	}
>> +
>> +	addr = be32_to_cpup(paddr);
> 
>    Why not just use of_property_read_u32() instead of the above? At least
>    that's a material for a follow-up patch...

Agreed, I would prefer if of_property_read_u32() is used here instead
of this by hand expansion.

Re: [PATCH 1/3] net: of_mdio: factor out code to parse a phy's 'reg' property

[Daniel Mack]

On 05/13/2014 07:07 PM, David Miller wrote:
> From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Date: Sun, 11 May 2014 00:29:02 +0400
> 
>> Hello.
>>
>> On 05/10/2014 08:37 PM, Daniel Mack wrote:
>>
>>> Factor out some logic into of_mdio_parse_addr() so it can be reused
>>> later.
>>
>>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>>> ---
>>>   drivers/of/of_mdio.c | 37 +++++++++++++++++++++++++------------
>>>   1 file changed, 25 insertions(+), 12 deletions(-)
>>
>>> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
>>> index 9a95831..171f9d5 100644
>>> --- a/drivers/of/of_mdio.c
>>> +++ b/drivers/of/of_mdio.c
>>> @@ -114,6 +114,29 @@ static int of_mdiobus_register_phy(struct mii_bus
>>> *mdio, struct device_node *chi
>>>   	return 0;
>>>   }
>>>
>>> +static int of_mdio_parse_addr(struct device *dev, const struct
>>> device_node *np)
>>> +{
>>> +	const __be32 *paddr;
>>> +	u32 addr;
>>> +	int len;
>>> +
>>> +	/* A PHY must have a reg property in the range [0-31] */
>>> +	paddr = of_get_property(np, "reg", &len);
>>> +	if (!paddr || len < sizeof(*paddr)) {
>>> + dev_err(dev, "%s has invalid PHY address\n", np->full_name);
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	addr = be32_to_cpup(paddr);
>>
>>    Why not just use of_property_read_u32() instead of the above? At least
>>    that's a material for a follow-up patch...
> 
> Agreed, I would prefer if of_property_read_u32() is used here instead
> of this by hand expansion.
> 

Yes, I agree as well, but I guess I can add this as a follow-up patch,
so we have it separate from the code move?


Daniel

Re: [PATCH 1/3] net: of_mdio: factor out code to parse a phy's 'reg' property

[David Miller]

From: Daniel Mack <zonque@gmail.com>
Date: Tue, 13 May 2014 19:21:54 +0200

> On 05/13/2014 07:07 PM, David Miller wrote:
>> From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>> Date: Sun, 11 May 2014 00:29:02 +0400
>> 
>>> Hello.
>>>
>>> On 05/10/2014 08:37 PM, Daniel Mack wrote:
>>>
>>>> Factor out some logic into of_mdio_parse_addr() so it can be reused
>>>> later.
>>>
>>>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>>>> ---
>>>>   drivers/of/of_mdio.c | 37 +++++++++++++++++++++++++------------
>>>>   1 file changed, 25 insertions(+), 12 deletions(-)
>>>
>>>> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
>>>> index 9a95831..171f9d5 100644
>>>> --- a/drivers/of/of_mdio.c
>>>> +++ b/drivers/of/of_mdio.c
>>>> @@ -114,6 +114,29 @@ static int of_mdiobus_register_phy(struct mii_bus
>>>> *mdio, struct device_node *chi
>>>>   	return 0;
>>>>   }
>>>>
>>>> +static int of_mdio_parse_addr(struct device *dev, const struct
>>>> device_node *np)
>>>> +{
>>>> +	const __be32 *paddr;
>>>> +	u32 addr;
>>>> +	int len;
>>>> +
>>>> +	/* A PHY must have a reg property in the range [0-31] */
>>>> +	paddr = of_get_property(np, "reg", &len);
>>>> +	if (!paddr || len < sizeof(*paddr)) {
>>>> + dev_err(dev, "%s has invalid PHY address\n", np->full_name);
>>>> +		return -EINVAL;
>>>> +	}
>>>> +
>>>> +	addr = be32_to_cpup(paddr);
>>>
>>>    Why not just use of_property_read_u32() instead of the above? At least
>>>    that's a material for a follow-up patch...
>> 
>> Agreed, I would prefer if of_property_read_u32() is used here instead
>> of this by hand expansion.
>> 
> 
> Yes, I agree as well, but I guess I can add this as a follow-up patch,
> so we have it separate from the code move?

Please repost the patch series with the requested changes made, no need
to start with excessive code from the beginning only to remove it.

Re: [PATCH 1/3] net: of_mdio: factor out code to parse a phy's 'reg' property

[Daniel Mack]

On 05/13/2014 07:25 PM, David Miller wrote:
> From: Daniel Mack <zonque@gmail.com>

>> Yes, I agree as well, but I guess I can add this as a follow-up patch,
>> so we have it separate from the code move?
> 
> Please repost the patch series with the requested changes made, no need
> to start with excessive code from the beginning only to remove it.

Ok, no problem. I'll wait for Florian's test results and repost.


Thanks for the input, everyone!
Daniel

Re: [PATCH 0/3] mdio: Parse DT nodes for auto-probed PHYs

[Daniel Mack]

Hi Florian,

On 05/13/2014 06:33 AM, Florian Fainelli wrote:
> Le 10/05/2014 09:37, Daniel Mack a écrit :
>> Hi Florian,
>>
>> here's another aproach of solving the issue with auto-probed buses and
>> DT device nodes. In short, this patch set introduces a new hook which
>> sets the of_node pointer of a phy device to a subnode of the bus, if
>> there is any which matches the phy's address. This function is called
>> from mdiobus_scan(), and is a no-op for !CONFIG_OF.
>>
>> Unlike with with the 1st version, dev->of_node is now available in the
>> PHY driver's probe() callback already.
>>
>> Let me know what you think, I'm open to other ideas :)
> 
> I like it so far, I will give this a try on some of my quirky hardware. 
> Thanks!

Did you have a chance yet to test these patches? I can submit a v2 which
does the requested of_property_read_u32() change.


Thanks and best regards,
Daniel

Re: [PATCH 0/3] mdio: Parse DT nodes for auto-probed PHYs

[Florian Fainelli]

Hi Daniel,

2014-05-22 11:18 GMT-07:00 Daniel Mack <zonque@gmail.com>:
> Hi Florian,
>
> On 05/13/2014 06:33 AM, Florian Fainelli wrote:
>> Le 10/05/2014 09:37, Daniel Mack a écrit :
>>> Hi Florian,
>>>
>>> here's another aproach of solving the issue with auto-probed buses and
>>> DT device nodes. In short, this patch set introduces a new hook which
>>> sets the of_node pointer of a phy device to a subnode of the bus, if
>>> there is any which matches the phy's address. This function is called
>>> from mdiobus_scan(), and is a no-op for !CONFIG_OF.
>>>
>>> Unlike with with the 1st version, dev->of_node is now available in the
>>> PHY driver's probe() callback already.
>>>
>>> Let me know what you think, I'm open to other ideas :)
>>
>> I like it so far, I will give this a try on some of my quirky hardware.
>> Thanks!
>
> Did you have a chance yet to test these patches? I can submit a v2 which
> does the requested of_property_read_u32() change.

I just tested those patches, and they look good. Please resubmit a v2
with the of_property_read_u32() changes and feel free to add my:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

tag, thanks!
-- 
Florian

Re: [PATCH 0/3] mdio: Parse DT nodes for auto-probed PHYs

[Florian Fainelli]

2014-05-23 12:24 GMT-07:00 Florian Fainelli <f.fainelli@gmail.com>:
> Hi Daniel,
>
> 2014-05-22 11:18 GMT-07:00 Daniel Mack <zonque@gmail.com>:
>> Hi Florian,
>>
>> On 05/13/2014 06:33 AM, Florian Fainelli wrote:
>>> Le 10/05/2014 09:37, Daniel Mack a écrit :
>>>> Hi Florian,
>>>>
>>>> here's another aproach of solving the issue with auto-probed buses and
>>>> DT device nodes. In short, this patch set introduces a new hook which
>>>> sets the of_node pointer of a phy device to a subnode of the bus, if
>>>> there is any which matches the phy's address. This function is called
>>>> from mdiobus_scan(), and is a no-op for !CONFIG_OF.
>>>>
>>>> Unlike with with the 1st version, dev->of_node is now available in the
>>>> PHY driver's probe() callback already.
>>>>
>>>> Let me know what you think, I'm open to other ideas :)
>>>
>>> I like it so far, I will give this a try on some of my quirky hardware.
>>> Thanks!
>>
>> Did you have a chance yet to test these patches? I can submit a v2 which
>> does the requested of_property_read_u32() change.
>
> I just tested those patches, and they look good. Please resubmit a v2
> with the of_property_read_u32() changes and feel free to add my:
>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

I just found a corner case we need to address. I will follow-up with
an email explaining what is happening.
-- 
Florian

Re: [PATCH 2/3] net: of_mdio: add of_mdiobus_link_phydev()

[Florian Fainelli]

2014-05-10 9:37 GMT-07:00 Daniel Mack <zonque@gmail.com>:
> Add a function to Walk the list of subnodes of a mdio bus and look for
> a node that matches the phy's address with its 'reg' property. If found,
> set the of_node pointer for the phy. This allows auto-probed pyh
> devices to be augmented by information passed in via DT.
>
> Signed-off-by: Daniel Mack <zonque@gmail.com>
> ---
>  drivers/net/phy/mdio_bus.c |  6 ++++++
>  drivers/of/of_mdio.c       | 33 +++++++++++++++++++++++++++++++++
>  include/linux/of_mdio.h    |  8 ++++++++
>  3 files changed, 47 insertions(+)
>
> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> index 76f54b3..e1444b2 100644
> --- a/drivers/net/phy/mdio_bus.c
> +++ b/drivers/net/phy/mdio_bus.c
> @@ -233,6 +233,12 @@ struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr)
>         if (IS_ERR(phydev) || phydev == NULL)
>                 return phydev;
>
> +       /*
> +        * For DT, see if the auto-probed phy has a correspoding child
> +        * in the bus node, and set the of_node pointer in this case.
> +        */
> +       of_mdiobus_link_phydev(bus, phydev);
> +
>         err = phy_device_register(phydev);
>         if (err) {
>                 phy_device_free(phydev);
> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index 171f9d5..931a977 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -211,6 +211,39 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
>  }
>  EXPORT_SYMBOL(of_mdiobus_register);
>
> +/**
> + * of_mdiobus_link_phydev - Find a device node for a phy
> + * @mdio: pointer to mii_bus structure
> + * @phydev: phydev for which the of_node pointer should be set
> + *
> + * Walk the list of subnodes of a mdio bus and look for a node that matches the
> + * phy's address with its 'reg' property. If found, set the of_node pointer for
> + * the phy. This allows auto-probed pyh devices to be supplied with information
> + * passed in via DT.
> + */
> +void of_mdiobus_link_phydev(struct mii_bus *mdio,
> +                           struct phy_device *phydev)
> +{
> +       struct device *dev = &phydev->dev;
> +       struct device_node *child;
> +
> +       if (dev->of_node)
> +               return;

This should be:

            if (dev->of_node || !mdio->dev_of_node)
                    return;

a system with for instance non-DT probed MDIO busses such as the
special fixed MDIO bus will not have such an of_node pointer and we
will have a null pointer during the first iteration of the for_each_*
loop.

arguably of_get_next_available_child() could be updated to handle a
NULL parent device_node, but I think it clearer to add this check
here.

> +
> +       for_each_available_child_of_node(mdio->dev.of_node, child) {
> +               int addr;
> +
> +               addr = of_mdio_parse_addr(&mdio->dev, child);
> +               if (addr < 0)
> +                       continue;
> +
> +               if (addr == phydev->addr) {
> +                       dev->of_node = child;
> +                       return;
> +               }
> +       }
> +}
> +
>  /* Helper function for of_phy_find_device */
>  static int of_phy_match(struct device *dev, void *phy_np)
>  {
> diff --git a/include/linux/of_mdio.h b/include/linux/of_mdio.h
> index 881a7c3..de72206 100644
> --- a/include/linux/of_mdio.h
> +++ b/include/linux/of_mdio.h
> @@ -28,6 +28,9 @@ extern struct phy_device *of_phy_connect_fixed_link(struct net_device *dev,
>
>  extern struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np);
>
> +extern void of_mdiobus_link_phydev(struct mii_bus *mdio,
> +                                  struct phy_device *phydev);
> +
>  #else /* CONFIG_OF */
>  static inline int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
>  {
> @@ -70,6 +73,11 @@ static inline struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np)
>  {
>         return NULL;
>  }
> +
> +static inline void of_mdiobus_link_phydev(struct mii_bus *mdio,
> +                                         struct phy_device *phydev)
> +{
> +}
>  #endif /* CONFIG_OF */
>
>  #endif /* __LINUX_OF_MDIO_H */
> --
> 1.9.0
>



-- 
Florian

Re: [PATCH 2/3] net: of_mdio: add of_mdiobus_link_phydev()

[Daniel Mack]

On 05/23/2014 09:35 PM, Florian Fainelli wrote:
> 2014-05-10 9:37 GMT-07:00 Daniel Mack <zonque@gmail.com>:

>> +/**
>> + * of_mdiobus_link_phydev - Find a device node for a phy
>> + * @mdio: pointer to mii_bus structure
>> + * @phydev: phydev for which the of_node pointer should be set
>> + *
>> + * Walk the list of subnodes of a mdio bus and look for a node that matches the
>> + * phy's address with its 'reg' property. If found, set the of_node pointer for
>> + * the phy. This allows auto-probed pyh devices to be supplied with information
>> + * passed in via DT.
>> + */
>> +void of_mdiobus_link_phydev(struct mii_bus *mdio,
>> +                           struct phy_device *phydev)
>> +{
>> +       struct device *dev = &phydev->dev;
>> +       struct device_node *child;
>> +
>> +       if (dev->of_node)
>> +               return;
> 
> This should be:
> 
>             if (dev->of_node || !mdio->dev_of_node)
>                     return;
> 
> a system with for instance non-DT probed MDIO busses such as the
> special fixed MDIO bus will not have such an of_node pointer and we
> will have a null pointer during the first iteration of the for_each_*
> loop.
> 
> arguably of_get_next_available_child() could be updated to handle a
> NULL parent device_node, but I think it clearer to add this check
> here.

Good catch! I'll change that, add your Reviewed-by and resend!


Thanks,
Daniel