Re: [PATCH net-next v4 0/4] usbnet: speed reporting for devices without MDIO

[Key part of Andew's reply: "Yes, this discussion should not prevent
this patchset from being merged."]

On Tue, Apr 6, 2021 at 1:00 PM Andrew Lunn [off-list ref] wrote:

quoted

quoted

Speed: 2500Mb/s and Duplex: Half is very unlikely. You really only
ever see 10 Half and occasionally 100 Half. Anything above that will
be full duplex.

It is probably best to admit the truth and use DUPLEX_UNKNOWN.

Agreed. I didn't notice this "lie" until I was writing the commit
message and wasn't sure off-hand how to fix it. Decided a follow on
patch could fix it up once this series lands.

You are right that DUPLEX_UNKNOWN is the safest (and usually correct) default.
Additionally, if RX and TX speed are equal, I am willing to assume
this is DUPLEX_FULL.

Is this same interface used by WiFi?

I doubt WiFi could work with this driver interface (though maybe with
"SendEncapsulatedCommand").
All the Wifi Devices I'm familiar with need WPA support and
communicate through 80211 kernel subsystem.

I was thinking of just about everything else: Cellular modem
(cdc_ether), xDSL, or other broadband.

Ethernet does not support
different rates in each direction. So if RX and TX are different, i
would actually say something is broken.

Agreed. The question is: Is there data or some heuristics we can use
to determine if the physical layer/link is ethernet?
I'm pessimistic we will be able to since this is at odds with the
intent of the CDC spec.

10 Half is still doing 10Mbps
in each direction, it just cannot do both at the same time.
WiFi can have asymmetric speeds.

*nod*

quoted

I can propose something like this in a patch:

grundler <1637>git diff

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 86eb1d107433..a7ad9a0fb6ae 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c

@@ -978,6 +978,11 @@ int usbnet_get_link_ksettings_internal(struct

net_device *net,
        else
                cmd->base.speed = SPEED_UNKNOWN;

+       if (dev->rx_speed == dev->tx_speed)
+               cmd->base.duplex = DUPLEX_FULL;
+       else
+               cmd->base.duplex =DUPLEX_UNKNOWN;
+
        return 0;
 }
 EXPORT_SYMBOL_GPL(usbnet_get_link_ksettings_internal);

So i would say this is wrong. I would just set DUPLEX_UNKNOWN and be
done.

Ok.

quoted

I can send this out later once this series lands or you are welcome to
post this with additional checks if you like.

Yes, this discussion should not prevent this patchset from being
merged.

Good. That's what I'm hoping for.

quoted

If we want to assume autoneg is always on (regardless of which type of
media cdc_ncm/cdc_ether are talking to), we could set both supported
and advertising to AUTO and lp_advertising to UNKNOWN.

I pretty much agree autoneg has to be on. If it is not, and it is
using a forced speed, there would need to be an additional API to set
what it is forced to. There could be such proprietary calls, but the
generic cdc_ncm/cdc_ether won't support them.

Good observation. Agreed.

But i also don't know how setting autoneg actually helps the user.

Just to let them know the link rate can change and is dynamically determined.

Everybody just assumes it is supported. If you really know auto-neg is
not supported and you can reliably indicate that autoneg is not
supported, that would be useful. But i expect most users want to know
if their USB 2.0 device is just doing 100Mbps, or if their USB 3.0
device can do 2.5G. For that, you need to see what is actually
supported.

Yes. Other than using a table to look up USB VID:PID, I don't see
anything in the spec which provides "media-specific" information.

I was curious about the "can do 2.5Gbps?" question by looking at the
CDC Ethernet Networking Functional Descriptor (USBECM12) and other CDC
specs. The spec feels like a "compatibility wrapper" to make a
cellular modem look like an ethernet device. This statement in the
ECM120.pdf I have suggests we can not determine media layer:
    The effect of a "reset" on the device physical layer is
media-dependent and beyond the scope of this specification.

cheers,
grant