Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1654588 > unrolled thread

[PATCH net-next] net: phy: use of_mdio_parse_addr

Back to article view | Back to linux.kernel

Contents

  [PATCH net-next] net: phy: use of_mdio_parse_addr Jon Mason <jon.mason@broadcom.com> - 2017-05-31 21:50 +0200
    Re: [PATCH net-next] net: phy: use of_mdio_parse_addr Andrew Lunn <andrew@lunn.ch> - 2017-05-31 22:10 +0200
    Re: [PATCH net-next] net: phy: use of_mdio_parse_addr Florian Fainelli <florian.fainelli@broadcom.com> - 2017-05-31 22:10 +0200
    Re: [PATCH net-next] net: phy: use of_mdio_parse_addr David Miller <davem@davemloft.net> - 2017-06-02 20:30 +0200
      Re: [PATCH net-next] net: phy: use of_mdio_parse_addr Liviu Dudau <liviu@dudau.co.uk> - 2017-06-07 18:30 +0200
        Re: [PATCH net-next] net: phy: use of_mdio_parse_addr Florian Fainelli <f.fainelli@gmail.com> - 2017-06-07 23:00 +0200
        Re: [PATCH net-next] net: phy: use of_mdio_parse_addr Jon Mason <jon.mason@broadcom.com> - 2017-06-07 23:00 +0200
        Re: [PATCH net-next] net: phy: use of_mdio_parse_addr Michael Ellerman <mpe@ellerman.id.au> - 2017-06-09 11:10 +0200

#1654588 — [PATCH net-next] net: phy: use of_mdio_parse_addr

use of_mdio_parse_addr() in place of an OF read of reg and a bounds
check (which is litterally the exact same thing that
of_mdio_parse_addr() does)

Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
 drivers/net/phy/mdio_bus.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 8e73f5f36e71..d4782e902e2e 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -263,21 +263,10 @@ static void of_mdiobus_link_mdiodev(struct mii_bus *bus,
 
 	for_each_available_child_of_node(bus->dev.of_node, child) {
 		int addr;
-		int ret;
 
-		ret = of_property_read_u32(child, "reg", &addr);
-		if (ret < 0) {
-			dev_err(dev, "%s has invalid MDIO address\n",
-				child->full_name);
+		addr = of_mdio_parse_addr(dev, child);
+		if (addr < 0)
 			continue;
-		}
-
-		/* A MDIO device must have a reg property in the range [0-31] */
-		if (addr >= PHY_MAX_ADDR) {
-			dev_err(dev, "%s MDIO address %i is too large\n",
-				child->full_name, addr);
-			continue;
-		}
 
 		if (addr == mdiodev->addr) {
 			dev->of_node = child;
-- 
2.7.4

[toc] | [next] | [standalone]

#1654599

On Wed, May 31, 2017 at 03:43:30PM -0400, Jon Mason wrote:
> use of_mdio_parse_addr() in place of an OF read of reg and a bounds
> check (which is litterally the exact same thing that
> of_mdio_parse_addr() does)
> 
> Signed-off-by: Jon Mason <jon.mason@broadcom.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[toc] | [prev] | [next] | [standalone]

#1654600

On 05/31/2017 12:43 PM, Jon Mason wrote:
> use of_mdio_parse_addr() in place of an OF read of reg and a bounds
> check (which is litterally the exact same thing that
> of_mdio_parse_addr() does)
> 
> Signed-off-by: Jon Mason <jon.mason@broadcom.com>

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

[toc] | [prev] | [next] | [standalone]

#1656456

From: Jon Mason <jon.mason@broadcom.com>
Date: Wed, 31 May 2017 15:43:30 -0400

> use of_mdio_parse_addr() in place of an OF read of reg and a bounds
> check (which is litterally the exact same thing that
> of_mdio_parse_addr() does)
> 
> Signed-off-by: Jon Mason <jon.mason@broadcom.com>

Applied, thanks Jon.

[toc] | [prev] | [next] | [standalone]

#1659999

On Fri, Jun 02, 2017 at 02:22:51PM -0400, David Miller wrote:
> From: Jon Mason <jon.mason@broadcom.com>
> Date: Wed, 31 May 2017 15:43:30 -0400
> 
> > use of_mdio_parse_addr() in place of an OF read of reg and a bounds
> > check (which is litterally the exact same thing that
> > of_mdio_parse_addr() does)
> > 
> > Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> 
> Applied, thanks Jon.

This makes linux-next fail the modules_install target as depmod detects 2 circular
dependencies. Reverting this patch fixes the issue.

depmod: ERROR: Cycle detected: libphy -> of_mdio -> fixed_phy -> libphy
depmod: ERROR: Cycle detected: libphy -> of_mdio -> libphy
depmod: ERROR: Found 3 modules in dependency cycles!
make[1]: *** [/home/dliviu/devel/kernel/Makefile:1245: _modinst_post] Error 1

This is on an ARCH=arm build, build I doubt it makes a difference. Let me know if
you need some .config values in order to reproduce.

Best regards,
Liviu

[toc] | [prev] | [next] | [standalone]

#1660215

On 06/07/2017 01:53 PM, Jon Mason wrote:
> On Wed, Jun 7, 2017 at 12:18 PM, Liviu Dudau <liviu@dudau.co.uk> wrote:
>> On Fri, Jun 02, 2017 at 02:22:51PM -0400, David Miller wrote:
>>> From: Jon Mason <jon.mason@broadcom.com>
>>> Date: Wed, 31 May 2017 15:43:30 -0400
>>>
>>>> use of_mdio_parse_addr() in place of an OF read of reg and a bounds
>>>> check (which is litterally the exact same thing that
>>>> of_mdio_parse_addr() does)
>>>>
>>>> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
>>>
>>> Applied, thanks Jon.
>>
>> This makes linux-next fail the modules_install target as depmod detects 2 circular
>> dependencies. Reverting this patch fixes the issue.
>>
>> depmod: ERROR: Cycle detected: libphy -> of_mdio -> fixed_phy -> libphy
>> depmod: ERROR: Cycle detected: libphy -> of_mdio -> libphy
>> depmod: ERROR: Found 3 modules in dependency cycles!
>> make[1]: *** [/home/dliviu/devel/kernel/Makefile:1245: _modinst_post] Error 1
> 
> I did not test this as modules.  Sorry.
> 
> It would be ugly to duplicate the code in both place, and the code in
> question does not seem to really need to be in a C file.  Perhaps it
> can be moved to a header file as an inline function, which would solve
> this dependency.  Would this be acceptable?

You read my mind, was just going to suggest doing that.
-- 
Florian

[toc] | [prev] | [next] | [standalone]

#1660223

On Wed, Jun 7, 2017 at 12:18 PM, Liviu Dudau <liviu@dudau.co.uk> wrote:
> On Fri, Jun 02, 2017 at 02:22:51PM -0400, David Miller wrote:
>> From: Jon Mason <jon.mason@broadcom.com>
>> Date: Wed, 31 May 2017 15:43:30 -0400
>>
>> > use of_mdio_parse_addr() in place of an OF read of reg and a bounds
>> > check (which is litterally the exact same thing that
>> > of_mdio_parse_addr() does)
>> >
>> > Signed-off-by: Jon Mason <jon.mason@broadcom.com>
>>
>> Applied, thanks Jon.
>
> This makes linux-next fail the modules_install target as depmod detects 2 circular
> dependencies. Reverting this patch fixes the issue.
>
> depmod: ERROR: Cycle detected: libphy -> of_mdio -> fixed_phy -> libphy
> depmod: ERROR: Cycle detected: libphy -> of_mdio -> libphy
> depmod: ERROR: Found 3 modules in dependency cycles!
> make[1]: *** [/home/dliviu/devel/kernel/Makefile:1245: _modinst_post] Error 1

I did not test this as modules.  Sorry.

It would be ugly to duplicate the code in both place, and the code in
question does not seem to really need to be in a C file.  Perhaps it
can be moved to a header file as an inline function, which would solve
this dependency.  Would this be acceptable?

Thanks,
Jon

>
> This is on an ARCH=arm build, build I doubt it makes a difference. Let me know if
> you need some .config values in order to reproduce.
>
> Best regards,
> Liviu
>

[toc] | [prev] | [next] | [standalone]

#1662138

Liviu Dudau <liviu@dudau.co.uk> writes:

> On Fri, Jun 02, 2017 at 02:22:51PM -0400, David Miller wrote:
>> From: Jon Mason <jon.mason@broadcom.com>
>> Date: Wed, 31 May 2017 15:43:30 -0400
>> 
>> > use of_mdio_parse_addr() in place of an OF read of reg and a bounds
>> > check (which is litterally the exact same thing that
>> > of_mdio_parse_addr() does)
>> > 
>> > Signed-off-by: Jon Mason <jon.mason@broadcom.com>
>> 
>> Applied, thanks Jon.
>
> This makes linux-next fail the modules_install target as depmod detects 2 circular
> dependencies. Reverting this patch fixes the issue.
>
> depmod: ERROR: Cycle detected: libphy -> of_mdio -> fixed_phy -> libphy
> depmod: ERROR: Cycle detected: libphy -> of_mdio -> libphy
> depmod: ERROR: Found 3 modules in dependency cycles!
> make[1]: *** [/home/dliviu/devel/kernel/Makefile:1245: _modinst_post] Error 1
>
> This is on an ARCH=arm build, build I doubt it makes a difference. Let me know if
> you need some .config values in order to reproduce.

Same happens on 32-bit powerpc.

cheers

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web