* [PATCH] net: phy: genphy_loopback: fix loopback failed when speed is unknown
@ 2022-03-31 11:48 Guangbin Huang
2022-03-31 12:23 ` Andrew Lunn
0 siblings, 1 reply; 10+ messages in thread
From: Guangbin Huang @ 2022-03-31 11:48 UTC (permalink / raw)
To: andrew, davem, kuba, o.rempel
Cc: netdev, linux-kernel, lipeng321, huangguangbin2, chenhao288
If phy link status is down because link partner goes down, the phy speed
will be updated to SPEED_UNKNOWN when autoneg on with general phy driver.
If test loopback in this case, the phy speed will be set to 10M. However,
the speed of mac may not be 10M, it causes loopback test failed.
To fix this problem, if speed is SPEED_UNKNOWN, don't configure link speed.
Fixes: 014068dcb5b1 ("net: phy: genphy_loopback: add link speed configuration")
Signed-off-by: Guangbin Huang <huangguangbin2@huawei.com>
---
drivers/net/phy/phy_device.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 8406ac739def..5001bb1a019c 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -2618,6 +2618,9 @@ int genphy_loopback(struct phy_device *phydev, bool enable)
ctl |= BMCR_SPEED1000;
else if (phydev->speed == SPEED_100)
ctl |= BMCR_SPEED100;
+ else if (phydev->speed == SPEED_UNKNOWN)
+ return phy_modify(phydev, MII_BMCR, BMCR_LOOPBACK,
+ BMCR_LOOPBACK);
if (phydev->duplex == DUPLEX_FULL)
ctl |= BMCR_FULLDPLX;
--
2.33.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] net: phy: genphy_loopback: fix loopback failed when speed is unknown
2022-03-31 11:48 [PATCH] net: phy: genphy_loopback: fix loopback failed when speed is unknown Guangbin Huang
@ 2022-03-31 12:23 ` Andrew Lunn
2022-03-31 13:57 ` huangguangbin (A)
0 siblings, 1 reply; 10+ messages in thread
From: Andrew Lunn @ 2022-03-31 12:23 UTC (permalink / raw)
To: Guangbin Huang
Cc: davem, kuba, o.rempel, netdev, linux-kernel, lipeng321, chenhao288
On Thu, Mar 31, 2022 at 07:48:19PM +0800, Guangbin Huang wrote:
> If phy link status is down because link partner goes down, the phy speed
> will be updated to SPEED_UNKNOWN when autoneg on with general phy driver.
> If test loopback in this case, the phy speed will be set to 10M. However,
> the speed of mac may not be 10M, it causes loopback test failed.
>
> To fix this problem, if speed is SPEED_UNKNOWN, don't configure link speed.
I don't think this explanation is correct.
If speed is UNKNOWN, ctl is just going to have BMCR_LOOPBACK set. That
is very similar to what you are doing. The code then waits for the
link to establish. This is where i guess your problem is. Are you
seeing ETIMEDOUT? Does the link not establish?
Thanks
Andrew
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] net: phy: genphy_loopback: fix loopback failed when speed is unknown
2022-03-31 12:23 ` Andrew Lunn
@ 2022-03-31 13:57 ` huangguangbin (A)
2022-03-31 14:26 ` Andrew Lunn
0 siblings, 1 reply; 10+ messages in thread
From: huangguangbin (A) @ 2022-03-31 13:57 UTC (permalink / raw)
To: Andrew Lunn
Cc: davem, kuba, o.rempel, netdev, linux-kernel, lipeng321, chenhao288
On 2022/3/31 20:23, Andrew Lunn wrote:
> On Thu, Mar 31, 2022 at 07:48:19PM +0800, Guangbin Huang wrote:
>> If phy link status is down because link partner goes down, the phy speed
>> will be updated to SPEED_UNKNOWN when autoneg on with general phy driver.
>> If test loopback in this case, the phy speed will be set to 10M. However,
>> the speed of mac may not be 10M, it causes loopback test failed.
>>
>> To fix this problem, if speed is SPEED_UNKNOWN, don't configure link speed.
>
> I don't think this explanation is correct.
>
> If speed is UNKNOWN, ctl is just going to have BMCR_LOOPBACK set. That
> is very similar to what you are doing. The code then waits for the
> link to establish. This is where i guess your problem is. Are you
> seeing ETIMEDOUT? Does the link not establish?
>
> Thanks
> Andrew
> .
>
Hi Andrew
This problem is not timeout, I have print return value of phy_read_poll_timeout()
and it is 0.
In this case, as speed and duplex both are unknown, ctl is just set to 0x4000.
However, the follow code sets mask to ~0 for function phy_modify():
int genphy_loopback(struct phy_device *phydev, bool enable)
{
if (enable) {
...
phy_modify(phydev, MII_BMCR, ~0, ctl);
...
}
so all other bits of BMCR will be cleared and just set bit 14, I use phy trace to
prove that:
$ cat /sys/kernel/debug/tracing/trace
# tracer: nop
#
# entries-in-buffer/entries-written: 923/923 #P:128
#
# _-----=> irqs-off/BH-disabled
# / _----=> need-resched
# | / _---=> hardirq/softirq
# || / _--=> preempt-depth
# ||| / _-=> migrate-disable
# |||| / delay
# TASK-PID CPU# ||||| TIMESTAMP FUNCTION
# | | | ||||| | |
kworker/u257:2-694 [015] ..... 209.263912: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x00 val:0x1040
kworker/u257:2-694 [015] ..... 209.263951: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
kworker/u257:2-694 [015] ..... 209.263990: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
kworker/u257:2-694 [015] ..... 209.264028: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x09 val:0x0200
kworker/u257:2-694 [015] ..... 209.264067: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x0a val:0x0000
ethtool-1148 [007] ..... 209.665693: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x00 val:0x1040
ethtool-1148 [007] ..... 209.665706: mdio_access: mii-0000:bd:00.1 write phy:0x03 reg:0x00 val:0x1840
ethtool-1148 [007] ..... 210.588139: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x00 val:0x1840
ethtool-1148 [007] ..... 210.588152: mdio_access: mii-0000:bd:00.1 write phy:0x03 reg:0x00 val:0x1040
ethtool-1148 [007] ..... 210.615900: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x00 val:0x1040
ethtool-1148 [007] ..... 210.615912: mdio_access: mii-0000:bd:00.1 write phy:0x03 reg:0x00 val:0x4000 //here just set bit 14
ethtool-1148 [007] ..... 210.620952: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
ethtool-1148 [007] ..... 210.625992: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
ethtool-1148 [007] ..... 210.631034: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
ethtool-1148 [007] ..... 210.636075: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
ethtool-1148 [007] ..... 210.641116: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
ethtool-1148 [007] ..... 210.646159: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
ethtool-1148 [007] ..... 210.651215: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
ethtool-1148 [007] ..... 210.656256: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
ethtool-1148 [007] ..... 210.661296: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
ethtool-1148 [007] ..... 210.666338: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
ethtool-1148 [007] ..... 210.671378: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x798d
ethtool-1148 [007] ..... 210.679016: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x00 val:0x4000
ethtool-1148 [007] ..... 210.679053: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x798d
ethtool-1148 [007] ..... 210.679091: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x09 val:0x0200
ethtool-1148 [007] ..... 210.679129: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x0a val:0x0000
ethtool-1148 [007] ..... 210.695902: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x00 val:0x4000
ethtool-1148 [007] ..... 210.695939: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x798d
ethtool-1148 [007] ..... 210.695977: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x09 val:0x0200
ethtool-1148 [007] ..... 210.696014: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x0a val:0x0000
So phy speed will be set to 10M in this case, if previous speed of device before going
down is 10M, loopback test is pass. Only previous speed is 100M or 1000M, loopback test is failed.
Thanks
Guangbin
.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] net: phy: genphy_loopback: fix loopback failed when speed is unknown
2022-03-31 13:57 ` huangguangbin (A)
@ 2022-03-31 14:26 ` Andrew Lunn
2022-04-01 6:40 ` Oleksij Rempel
0 siblings, 1 reply; 10+ messages in thread
From: Andrew Lunn @ 2022-03-31 14:26 UTC (permalink / raw)
To: huangguangbin (A)
Cc: davem, kuba, o.rempel, netdev, linux-kernel, lipeng321, chenhao288
> In this case, as speed and duplex both are unknown, ctl is just set to 0x4000.
> However, the follow code sets mask to ~0 for function phy_modify():
> int genphy_loopback(struct phy_device *phydev, bool enable)
> {
> if (enable) {
> ...
> phy_modify(phydev, MII_BMCR, ~0, ctl);
> ...
> }
> so all other bits of BMCR will be cleared and just set bit 14, I use phy trace to
> prove that:
>
> $ cat /sys/kernel/debug/tracing/trace
> # tracer: nop
> #
> # entries-in-buffer/entries-written: 923/923 #P:128
> #
> # _-----=> irqs-off/BH-disabled
> # / _----=> need-resched
> # | / _---=> hardirq/softirq
> # || / _--=> preempt-depth
> # ||| / _-=> migrate-disable
> # |||| / delay
> # TASK-PID CPU# ||||| TIMESTAMP FUNCTION
> # | | | ||||| | |
> kworker/u257:2-694 [015] ..... 209.263912: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x00 val:0x1040
> kworker/u257:2-694 [015] ..... 209.263951: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
> kworker/u257:2-694 [015] ..... 209.263990: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
> kworker/u257:2-694 [015] ..... 209.264028: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x09 val:0x0200
> kworker/u257:2-694 [015] ..... 209.264067: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x0a val:0x0000
> ethtool-1148 [007] ..... 209.665693: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x00 val:0x1040
> ethtool-1148 [007] ..... 209.665706: mdio_access: mii-0000:bd:00.1 write phy:0x03 reg:0x00 val:0x1840
> ethtool-1148 [007] ..... 210.588139: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x00 val:0x1840
> ethtool-1148 [007] ..... 210.588152: mdio_access: mii-0000:bd:00.1 write phy:0x03 reg:0x00 val:0x1040
> ethtool-1148 [007] ..... 210.615900: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x00 val:0x1040
> ethtool-1148 [007] ..... 210.615912: mdio_access: mii-0000:bd:00.1 write phy:0x03 reg:0x00 val:0x4000 //here just set bit 14
>
> So phy speed will be set to 10M in this case, if previous speed of
> device before going down is 10M, loopback test is pass. Only
> previous speed is 100M or 1000M, loopback test is failed.
O.K. So it should be set into 10M half duplex. But why does this cause
it not to loopback packets? Does the PHY you are using not actually
support 10 Half? Why does it need to be the same speed as when the
link was up? And why does it actually set LSTATUS indicating there is
link?
Is this a generic problem, all PHYs are like this, or is this specific
to the PHY you are using? Maybe this PHY needs its own loopback
function because it does something odd?
Andrew
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] net: phy: genphy_loopback: fix loopback failed when speed is unknown
2022-03-31 14:26 ` Andrew Lunn
@ 2022-04-01 6:40 ` Oleksij Rempel
2022-04-01 12:14 ` Andrew Lunn
0 siblings, 1 reply; 10+ messages in thread
From: Oleksij Rempel @ 2022-04-01 6:40 UTC (permalink / raw)
To: Andrew Lunn
Cc: huangguangbin (A),
davem, kuba, netdev, linux-kernel, lipeng321, chenhao288
On Thu, Mar 31, 2022 at 04:26:47PM +0200, Andrew Lunn wrote:
> > In this case, as speed and duplex both are unknown, ctl is just set to 0x4000.
> > However, the follow code sets mask to ~0 for function phy_modify():
> > int genphy_loopback(struct phy_device *phydev, bool enable)
> > {
> > if (enable) {
> > ...
> > phy_modify(phydev, MII_BMCR, ~0, ctl);
> > ...
> > }
> > so all other bits of BMCR will be cleared and just set bit 14, I use phy trace to
> > prove that:
> >
> > $ cat /sys/kernel/debug/tracing/trace
> > # tracer: nop
> > #
> > # entries-in-buffer/entries-written: 923/923 #P:128
> > #
> > # _-----=> irqs-off/BH-disabled
> > # / _----=> need-resched
> > # | / _---=> hardirq/softirq
> > # || / _--=> preempt-depth
> > # ||| / _-=> migrate-disable
> > # |||| / delay
> > # TASK-PID CPU# ||||| TIMESTAMP FUNCTION
> > # | | | ||||| | |
> > kworker/u257:2-694 [015] ..... 209.263912: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x00 val:0x1040
> > kworker/u257:2-694 [015] ..... 209.263951: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
> > kworker/u257:2-694 [015] ..... 209.263990: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
> > kworker/u257:2-694 [015] ..... 209.264028: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x09 val:0x0200
> > kworker/u257:2-694 [015] ..... 209.264067: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x0a val:0x0000
> > ethtool-1148 [007] ..... 209.665693: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x00 val:0x1040
> > ethtool-1148 [007] ..... 209.665706: mdio_access: mii-0000:bd:00.1 write phy:0x03 reg:0x00 val:0x1840
> > ethtool-1148 [007] ..... 210.588139: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x00 val:0x1840
> > ethtool-1148 [007] ..... 210.588152: mdio_access: mii-0000:bd:00.1 write phy:0x03 reg:0x00 val:0x1040
> > ethtool-1148 [007] ..... 210.615900: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x00 val:0x1040
> > ethtool-1148 [007] ..... 210.615912: mdio_access: mii-0000:bd:00.1 write phy:0x03 reg:0x00 val:0x4000 //here just set bit 14
> >
> > So phy speed will be set to 10M in this case, if previous speed of
> > device before going down is 10M, loopback test is pass. Only
> > previous speed is 100M or 1000M, loopback test is failed.
>
> O.K. So it should be set into 10M half duplex. But why does this cause
> it not to loopback packets? Does the PHY you are using not actually
> support 10 Half? Why does it need to be the same speed as when the
> link was up? And why does it actually set LSTATUS indicating there is
> link?
>
> Is this a generic problem, all PHYs are like this, or is this specific
> to the PHY you are using? Maybe this PHY needs its own loopback
> function because it does something odd?
It looks for me like attempt to fix loopback test for setup without active
link partner. Correct?
Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] net: phy: genphy_loopback: fix loopback failed when speed is unknown
2022-04-01 6:40 ` Oleksij Rempel
@ 2022-04-01 12:14 ` Andrew Lunn
2022-04-07 13:54 ` huangguangbin (A)
0 siblings, 1 reply; 10+ messages in thread
From: Andrew Lunn @ 2022-04-01 12:14 UTC (permalink / raw)
To: Oleksij Rempel
Cc: huangguangbin (A),
davem, kuba, netdev, linux-kernel, lipeng321, chenhao288
> > O.K. So it should be set into 10M half duplex. But why does this cause
> > it not to loopback packets? Does the PHY you are using not actually
> > support 10 Half? Why does it need to be the same speed as when the
> > link was up? And why does it actually set LSTATUS indicating there is
> > link?
> >
> > Is this a generic problem, all PHYs are like this, or is this specific
> > to the PHY you are using? Maybe this PHY needs its own loopback
> > function because it does something odd?
>
> It looks for me like attempt to fix loopback test for setup without active
> link partner. Correct?
You should not need a link partner for loopback to work. This is local
loopback. The PHY is also saying it has link, if the LSTATUS bit is
set. So i don't see why previous speed is relevant hear. This seems to
me to be an issue for this particular PHY.
What i don't like about this patch is that it is not deterministic
what mode the PHY will end up in if speed is unknown. Without the
patch, it is 10Mbps, which is historically a sensible default.
If this PHY has never had link, what speed does it use? Does it still
work in that case?
Andrew
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] net: phy: genphy_loopback: fix loopback failed when speed is unknown
2022-04-01 12:14 ` Andrew Lunn
@ 2022-04-07 13:54 ` huangguangbin (A)
2022-04-07 14:45 ` Andrew Lunn
0 siblings, 1 reply; 10+ messages in thread
From: huangguangbin (A) @ 2022-04-07 13:54 UTC (permalink / raw)
To: Andrew Lunn, Oleksij Rempel
Cc: davem, kuba, netdev, linux-kernel, lipeng321, chenhao288
On 2022/4/1 20:14, Andrew Lunn wrote:
>>> O.K. So it should be set into 10M half duplex. But why does this cause
>>> it not to loopback packets? Does the PHY you are using not actually
>>> support 10 Half? Why does it need to be the same speed as when the
>>> link was up? And why does it actually set LSTATUS indicating there is
>>> link?
>>>
>>> Is this a generic problem, all PHYs are like this, or is this specific
>>> to the PHY you are using? Maybe this PHY needs its own loopback
>>> function because it does something odd?
>>
>> It looks for me like attempt to fix loopback test for setup without active
>> link partner. Correct?
>
> You should not need a link partner for loopback to work. This is local
> loopback. The PHY is also saying it has link, if the LSTATUS bit is
> set. So i don't see why previous speed is relevant hear. This seems to
> me to be an issue for this particular PHY.
>
> What i don't like about this patch is that it is not deterministic
> what mode the PHY will end up in if speed is unknown. Without the
> patch, it is 10Mbps, which is historically a sensible default.
>
> If this PHY has never had link, what speed does it use? Does it still
> work in that case?
>
> Andrew
> .
>
Hi Andrew,
The PHY we test is RTL8211F, it supports 10 half. This problem actually is,
our board has MAC connected with PHY, when loopback test, packet flow is
MAC->PHY->MAC, it needs speed of MAC and PHY should be same when they work.
If PHY speed is unknown when PHY goes down, we will not set MAC speed in
adjust_link interface. In this case, we hope that PHY speed should not be
changed, as the old code of function genphy_loopback() before patch
"net: phy: genphy_loopback: add link speed configuration".
If PHY has never link, MAC speed has never be set in adjust_link interface,
yeah, in this case, MAC and PHY may has different speed, and they can not work.
I think we can accept this situation.
I think it is general problem if there is MAC connected with PHY.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] net: phy: genphy_loopback: fix loopback failed when speed is unknown
2022-04-07 13:54 ` huangguangbin (A)
@ 2022-04-07 14:45 ` Andrew Lunn
2022-04-08 8:18 ` Oleksij Rempel
0 siblings, 1 reply; 10+ messages in thread
From: Andrew Lunn @ 2022-04-07 14:45 UTC (permalink / raw)
To: huangguangbin (A)
Cc: Oleksij Rempel, davem, kuba, netdev, linux-kernel, lipeng321, chenhao288
> Hi Andrew,
> The PHY we test is RTL8211F, it supports 10 half. This problem actually is,
> our board has MAC connected with PHY, when loopback test, packet flow is
> MAC->PHY->MAC, it needs speed of MAC and PHY should be same when they work.
>
> If PHY speed is unknown when PHY goes down, we will not set MAC speed in
> adjust_link interface. In this case, we hope that PHY speed should not be
> changed, as the old code of function genphy_loopback() before patch
> "net: phy: genphy_loopback: add link speed configuration".
>
> If PHY has never link, MAC speed has never be set in adjust_link interface,
> yeah, in this case, MAC and PHY may has different speed, and they can not work.
> I think we can accept this situation.
>
> I think it is general problem if there is MAC connected with PHY.
Thanks for investigating. Looks like we are getting close the real
solution. And it is a generic problem, that the MAC and PHY might not
be using the same configuration.
So it looks like if the link is down, or speed is UNKNOWN, we need to
set phydev->link true, speed 10, duplex half, the PHY into 10/Half and
call the adjust_link callback. That should get the MAC and PHY to talk
to each other.
The open question is what to do when we disable loopback. Maybe we
need to always set link false, speed unknown and call phy_start_aneg()
to restart the link?
Andrew
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] net: phy: genphy_loopback: fix loopback failed when speed is unknown
2022-04-07 14:45 ` Andrew Lunn
@ 2022-04-08 8:18 ` Oleksij Rempel
2022-04-08 13:04 ` Andrew Lunn
0 siblings, 1 reply; 10+ messages in thread
From: Oleksij Rempel @ 2022-04-08 8:18 UTC (permalink / raw)
To: Andrew Lunn
Cc: huangguangbin (A),
davem, kuba, netdev, linux-kernel, lipeng321, chenhao288
On Thu, Apr 07, 2022 at 04:45:03PM +0200, Andrew Lunn wrote:
> > Hi Andrew,
> > The PHY we test is RTL8211F, it supports 10 half. This problem actually is,
> > our board has MAC connected with PHY, when loopback test, packet flow is
> > MAC->PHY->MAC, it needs speed of MAC and PHY should be same when they work.
> >
> > If PHY speed is unknown when PHY goes down, we will not set MAC speed in
> > adjust_link interface. In this case, we hope that PHY speed should not be
> > changed, as the old code of function genphy_loopback() before patch
> > "net: phy: genphy_loopback: add link speed configuration".
> >
> > If PHY has never link, MAC speed has never be set in adjust_link interface,
> > yeah, in this case, MAC and PHY may has different speed, and they can not work.
> > I think we can accept this situation.
> >
> > I think it is general problem if there is MAC connected with PHY.
>
> Thanks for investigating. Looks like we are getting close the real
> solution. And it is a generic problem, that the MAC and PHY might not
> be using the same configuration.
>
> So it looks like if the link is down, or speed is UNKNOWN, we need to
> set phydev->link true, speed 10, duplex half...
Hm, i was thinking, is it impossible to run loopback test on half duplex link.
Do I missing something?
Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] net: phy: genphy_loopback: fix loopback failed when speed is unknown
2022-04-08 8:18 ` Oleksij Rempel
@ 2022-04-08 13:04 ` Andrew Lunn
0 siblings, 0 replies; 10+ messages in thread
From: Andrew Lunn @ 2022-04-08 13:04 UTC (permalink / raw)
To: Oleksij Rempel
Cc: huangguangbin (A),
davem, kuba, netdev, linux-kernel, lipeng321, chenhao288
On Fri, Apr 08, 2022 at 10:18:24AM +0200, Oleksij Rempel wrote:
> On Thu, Apr 07, 2022 at 04:45:03PM +0200, Andrew Lunn wrote:
> > > Hi Andrew,
> > > The PHY we test is RTL8211F, it supports 10 half. This problem actually is,
> > > our board has MAC connected with PHY, when loopback test, packet flow is
> > > MAC->PHY->MAC, it needs speed of MAC and PHY should be same when they work.
> > >
> > > If PHY speed is unknown when PHY goes down, we will not set MAC speed in
> > > adjust_link interface. In this case, we hope that PHY speed should not be
> > > changed, as the old code of function genphy_loopback() before patch
> > > "net: phy: genphy_loopback: add link speed configuration".
> > >
> > > If PHY has never link, MAC speed has never be set in adjust_link interface,
> > > yeah, in this case, MAC and PHY may has different speed, and they can not work.
> > > I think we can accept this situation.
> > >
> > > I think it is general problem if there is MAC connected with PHY.
> >
> > Thanks for investigating. Looks like we are getting close the real
> > solution. And it is a generic problem, that the MAC and PHY might not
> > be using the same configuration.
> >
> > So it looks like if the link is down, or speed is UNKNOWN, we need to
> > set phydev->link true, speed 10, duplex half...
>
> Hm, i was thinking, is it impossible to run loopback test on half duplex link.
> Do I missing something?
Ah, you might be right. I was just thinking of the lowest common
denominator. So 10 Full. Or maybe even look at phydev->supported and
pick one from there.
Andrew
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-04-08 13:05 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-31 11:48 [PATCH] net: phy: genphy_loopback: fix loopback failed when speed is unknown Guangbin Huang
2022-03-31 12:23 ` Andrew Lunn
2022-03-31 13:57 ` huangguangbin (A)
2022-03-31 14:26 ` Andrew Lunn
2022-04-01 6:40 ` Oleksij Rempel
2022-04-01 12:14 ` Andrew Lunn
2022-04-07 13:54 ` huangguangbin (A)
2022-04-07 14:45 ` Andrew Lunn
2022-04-08 8:18 ` Oleksij Rempel
2022-04-08 13:04 ` Andrew Lunn
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).