Re: [PATCH] net: phy: genphy_loopback: fix loopback failed when speed is unknown

From: huangguangbin (A)
Date: Thu Apr 07 2022 - 09:54:53 EST




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.