* [PATCH] drivers/of/of_mdio.c:fix of_mdiobus_register()
@ 2020-03-01 16:41 Dajun Jin
2020-03-01 16:50 ` Andrew Lunn
0 siblings, 1 reply; 8+ messages in thread
From: Dajun Jin @ 2020-03-01 16:41 UTC (permalink / raw)
To: andrew, hkallweit1, linux, robh+dt, frowand.list
Cc: netdev, devicetree, Dajun Jin
when registers a phy_device successful, should terminate the loop
or the phy_device would be registered in other addr.
Signed-off-by: Dajun Jin <adajunjin@gmail.com>
---
drivers/of/of_mdio.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 8270bbf505fb..9f982c0627a0 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -306,6 +306,7 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
rc = of_mdiobus_register_phy(mdio, child, addr);
if (rc && rc != -ENODEV)
goto unregister;
+ break;
}
}
}
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] drivers/of/of_mdio.c:fix of_mdiobus_register()
2020-03-01 16:41 [PATCH] drivers/of/of_mdio.c:fix of_mdiobus_register() Dajun Jin
@ 2020-03-01 16:50 ` Andrew Lunn
2020-03-02 17:29 ` Dajun Jin
0 siblings, 1 reply; 8+ messages in thread
From: Andrew Lunn @ 2020-03-01 16:50 UTC (permalink / raw)
To: Dajun Jin; +Cc: hkallweit1, linux, robh+dt, frowand.list, netdev, devicetree
On Mon, Mar 02, 2020 at 12:41:38AM +0800, Dajun Jin wrote:
> when registers a phy_device successful, should terminate the loop
> or the phy_device would be registered in other addr.
>
> Signed-off-by: Dajun Jin <adajunjin@gmail.com>
> ---
> drivers/of/of_mdio.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index 8270bbf505fb..9f982c0627a0 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -306,6 +306,7 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
> rc = of_mdiobus_register_phy(mdio, child, addr);
> if (rc && rc != -ENODEV)
> goto unregister;
> + break;
> }
> }
> }
Hi Dajun
What problem are you seeing? You explanation needs to be better.
I'm guessing you have two or more PHYs on the bus, without reg
properties?
Andrew
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drivers/of/of_mdio.c:fix of_mdiobus_register()
2020-03-01 16:50 ` Andrew Lunn
@ 2020-03-02 17:29 ` Dajun Jin
2020-03-02 17:57 ` Andrew Lunn
0 siblings, 1 reply; 8+ messages in thread
From: Dajun Jin @ 2020-03-02 17:29 UTC (permalink / raw)
To: andrew, hkallweit1, linux, robh+dt, frowand.list; +Cc: netdev, devicetree
>On Mon, Mar 02, 2020 at 12:41:38AM +0800, Dajun Jin wrote:
>> when registers a phy_device successful, should terminate the loop
>> or the phy_device would be registered in other addr.
>>
>> Signed-off-by: Dajun Jin <adajunjin@xxxxxxxxx>
>> ---
>> drivers/of/of_mdio.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
>> index 8270bbf505fb..9f982c0627a0 100644
>> --- a/drivers/of/of_mdio.c
>> +++ b/drivers/of/of_mdio.c
>> @@ -306,6 +306,7 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
>> rc = of_mdiobus_register_phy(mdio, child, addr);
>> if (rc && rc != -ENODEV)
>> goto unregister;
>> + break;
>> }
>> }
>> }
>
>Hi Dajun
>
>What problem are you seeing? You explanation needs to be better.
>
>I'm guessing you have two or more PHYs on the bus, without reg
>properties?
>
> Andrew
Hi Andrew
If a phy without reg property would be registered to all unoccupied addr.
This is my test in Xilinx zcu106 board.
dts is liks this:
ethernet@ff0e0000 {
compatible = "cdns,zynqmp-gem", "cdns,gem";
status = "okay";
...
phy@0 {
ti,rx-internal-delay = <0x8>;
ti,tx-internal-delay = <0xa>;
ti,fifo-depth = <0x1>;
ti,rxctrl-strap-worka;
linux,phandle = <0x12>;
phandle = <0x12>;
};
};
then when borad is booting,the dmesg is like this:
[ 4.600035] mdio_bus ff0e0000.ethernet-ffffffff: /amba/ethernet@ff0e0000/phy@0 has invalid PHY address
[ 4.600050] mdio_bus ff0e0000.ethernet-ffffffff: scan phy phy at address 0
[ 4.602076] mdio_bus ff0e0000.ethernet-ffffffff: scan phy phy at address 1
[ 4.603849] mdio_bus ff0e0000.ethernet-ffffffff: scan phy phy at address 2
[ 4.605574] mdio_bus ff0e0000.ethernet-ffffffff: scan phy phy at address 4
[ 4.607312] mdio_bus ff0e0000.ethernet-ffffffff: scan phy phy at address 5
...
[ 4.636155] mdio_bus ff0e0000.ethernet-ffffffff: scan phy phy at address 28
[ 4.637335] mdio_bus ff0e0000.ethernet-ffffffff: scan phy phy at address 29
[ 4.638504] mdio_bus ff0e0000.ethernet-ffffffff: scan phy phy at address 30
[ 4.639666] mdio_bus ff0e0000.ethernet-ffffffff: scan phy phy at address 31
Dajun
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] drivers/of/of_mdio.c:fix of_mdiobus_register()
2020-03-02 17:29 ` Dajun Jin
@ 2020-03-02 17:57 ` Andrew Lunn
2020-03-02 18:38 ` Russell King - ARM Linux admin
2020-03-03 4:24 ` Dajun Jin
0 siblings, 2 replies; 8+ messages in thread
From: Andrew Lunn @ 2020-03-02 17:57 UTC (permalink / raw)
To: Dajun Jin; +Cc: hkallweit1, linux, robh+dt, frowand.list, netdev, devicetree
Hi Dajun
> This is my test in Xilinx zcu106 board.
>
> dts is liks this:
> ethernet@ff0e0000 {
> compatible = "cdns,zynqmp-gem", "cdns,gem";
> status = "okay";
> ...
>
> phy@0 {
> ti,rx-internal-delay = <0x8>;
> ti,tx-internal-delay = <0xa>;
> ti,fifo-depth = <0x1>;
> ti,rxctrl-strap-worka;
> linux,phandle = <0x12>;
> phandle = <0x12>;
> };
> };
>
> then when borad is booting,the dmesg is like this:
> [ 4.600035] mdio_bus ff0e0000.ethernet-ffffffff: /amba/ethernet@ff0e0000/phy@0 has invalid PHY address
> [ 4.600050] mdio_bus ff0e0000.ethernet-ffffffff: scan phy phy at address 0
> [ 4.602076] mdio_bus ff0e0000.ethernet-ffffffff: scan phy phy at address 1
> [ 4.603849] mdio_bus ff0e0000.ethernet-ffffffff: scan phy phy at address 2
> [ 4.605574] mdio_bus ff0e0000.ethernet-ffffffff: scan phy phy at address 4
> [ 4.607312] mdio_bus ff0e0000.ethernet-ffffffff: scan phy phy at address 5
> ...
> [ 4.636155] mdio_bus ff0e0000.ethernet-ffffffff: scan phy phy at address 28
> [ 4.637335] mdio_bus ff0e0000.ethernet-ffffffff: scan phy phy at address 29
> [ 4.638504] mdio_bus ff0e0000.ethernet-ffffffff: scan phy phy at address 30
> [ 4.639666] mdio_bus ff0e0000.ethernet-ffffffff: scan phy phy at address 31
For a single PHY without a reg properties, it should do the right
thing. But it will go wrong if there are multiple PHYs without reg
properties. In that case, the break helps.
However, as the comment suggests, you really should have a reg
property.
Please make the commit message better, and then i will give a
reviewed-by.
Thanks
Andrew
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] drivers/of/of_mdio.c:fix of_mdiobus_register()
2020-03-02 17:57 ` Andrew Lunn
@ 2020-03-02 18:38 ` Russell King - ARM Linux admin
2020-03-03 4:24 ` Dajun Jin
1 sibling, 0 replies; 8+ messages in thread
From: Russell King - ARM Linux admin @ 2020-03-02 18:38 UTC (permalink / raw)
To: Andrew Lunn
Cc: Dajun Jin, hkallweit1, robh+dt, frowand.list, netdev, devicetree
On Mon, Mar 02, 2020 at 06:57:59PM +0100, Andrew Lunn wrote:
> Hi Dajun
>
> > This is my test in Xilinx zcu106 board.
> >
> > dts is liks this:
> > ethernet@ff0e0000 {
> > compatible = "cdns,zynqmp-gem", "cdns,gem";
> > status = "okay";
> > ...
> >
> > phy@0 {
> > ti,rx-internal-delay = <0x8>;
> > ti,tx-internal-delay = <0xa>;
> > ti,fifo-depth = <0x1>;
> > ti,rxctrl-strap-worka;
> > linux,phandle = <0x12>;
> > phandle = <0x12>;
Isn't dtc going to complain about this? The node name has an address,
but there's no reg property. If there's no reg property, shouldn't it
be just "phy" ?
The above doesn't look like the original .dts file itself either, but
a .dtb translated back to a .dts - note the numeric phandle properties
and presence of "linux,phandle".
ethernet-phy.yaml also says:
required:
- reg
so arguably the above doesn't conform to what we expect?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH] drivers/of/of_mdio.c:fix of_mdiobus_register()
2020-03-02 17:57 ` Andrew Lunn
2020-03-02 18:38 ` Russell King - ARM Linux admin
@ 2020-03-03 4:24 ` Dajun Jin
2020-03-03 22:02 ` Andrew Lunn
2020-03-04 3:02 ` David Miller
1 sibling, 2 replies; 8+ messages in thread
From: Dajun Jin @ 2020-03-03 4:24 UTC (permalink / raw)
To: andrew, hkallweit1, linux, robh+dt, frowand.list; +Cc: netdev, devicetree
When registers a phy_device successful, should terminate the loop
or the phy_device would be registered in other addr. If there are
multiple PHYs without reg properties, it will go wrong.
Signed-off-by: Dajun Jin <adajunjin@gmail.com>
---
Hi Andrew, Thanks for your review.
drivers/of/of_mdio.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 8270bbf505fb..9f982c0627a0 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -306,6 +306,7 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
rc = of_mdiobus_register_phy(mdio, child, addr);
if (rc && rc != -ENODEV)
goto unregister;
+ break;
}
}
}
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] drivers/of/of_mdio.c:fix of_mdiobus_register()
2020-03-03 4:24 ` Dajun Jin
@ 2020-03-03 22:02 ` Andrew Lunn
2020-03-04 3:02 ` David Miller
1 sibling, 0 replies; 8+ messages in thread
From: Andrew Lunn @ 2020-03-03 22:02 UTC (permalink / raw)
To: Dajun Jin; +Cc: hkallweit1, linux, robh+dt, frowand.list, netdev, devicetree
On Mon, Mar 02, 2020 at 08:24:21PM -0800, Dajun Jin wrote:
> When registers a phy_device successful, should terminate the loop
> or the phy_device would be registered in other addr. If there are
> multiple PHYs without reg properties, it will go wrong.
>
> Signed-off-by: Dajun Jin <adajunjin@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] drivers/of/of_mdio.c:fix of_mdiobus_register()
2020-03-03 4:24 ` Dajun Jin
2020-03-03 22:02 ` Andrew Lunn
@ 2020-03-04 3:02 ` David Miller
1 sibling, 0 replies; 8+ messages in thread
From: David Miller @ 2020-03-04 3:02 UTC (permalink / raw)
To: adajunjin
Cc: andrew, hkallweit1, linux, robh+dt, frowand.list, netdev, devicetree
From: Dajun Jin <adajunjin@gmail.com>
Date: Mon, 2 Mar 2020 20:24:21 -0800
> When registers a phy_device successful, should terminate the loop
> or the phy_device would be registered in other addr. If there are
> multiple PHYs without reg properties, it will go wrong.
>
> Signed-off-by: Dajun Jin <adajunjin@gmail.com>
Applied, thank you.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-03-04 3:02 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-01 16:41 [PATCH] drivers/of/of_mdio.c:fix of_mdiobus_register() Dajun Jin
2020-03-01 16:50 ` Andrew Lunn
2020-03-02 17:29 ` Dajun Jin
2020-03-02 17:57 ` Andrew Lunn
2020-03-02 18:38 ` Russell King - ARM Linux admin
2020-03-03 4:24 ` Dajun Jin
2020-03-03 22:02 ` Andrew Lunn
2020-03-04 3:02 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).