Re: [PATCH net] net: phy: phy_caps: Don't skip better duplex macth on non-exact match

Russell King (Oracle)
2025-06-06T08:31:13Z

On Fri, Jun 06, 2025 at 10:19:23AM +0200, Maxime Chevallier wrote:
> In reality, the case you're mentioning would be a device that supports
> 1000/Full, 100/Full and 100/Half, user asks for 1000/Half, and 100/Full
> would be reported.
>
> That's unlikely to exist, but I'll document it as I've been surprised
> in the past with setups that shouldn't exist that actually do :)

That's not unlikely. Many MACs do not support 1000/Half but do support
100/Half and 10/Half.

Also... you're wrong about the result.

If phy_lookup_setting() is called with speed = 1000, duplex = half,
and we have a MAC that supports 1000/Full, 100/Half, 100/Full, then:

- We iterate down through the phy_settings[] to:
PHY_SETTING( 1000, FULL, 1000baseT_Full ),

if (p->speed == speed && p->duplex == duplex) {
the first condition is true, the second is false.
} else if (!exact) {
this is true.
if (!match && p->speed <= speed)
match is NULL, and p->speed == speed, so this is a candidate:
match = p;
We continue.
- The next entry that will be tested is:
PHY_SETTING( 100, FULL, 100baseT_Full ),

if (p->speed == speed && p->duplex == duplex) {
the first condition is false.
} else if (!exact) {
this is true.
if (!match && p->speed <= speed)
this is now false because match is set.
We continue.
- The next entry that will be tested is:
PHY_SETTING( 100, HALF, 100baseT_Half ),

if (p->speed == speed && p->duplex == duplex) {
the first condition is false.
} else if (!exact) {
this is true.
if (!match && p->speed <= speed)
this is now false because match is set.
We continue.

- We eventually get to the end of the list.
if (!match && !exact)
this is false.

- We return the entry for 1000/Full, which is exactly the behaviour I
was after when I wrote this matching.

If you're version doesn't come out with a matching speed, then I'm
afraid it's still broken.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!