David Hollis wrote:
On Sun, 2005-02-13 at 14:07 -0700, Jamie Painter wrote:
  
I've observed poor receive performance in 100baseTx-FD mode from ax8817x 
devices in all versions of usbnet.c.  Half-duplex received performance 
is actually better than full duplex.  After some investigation, I found 
that the MEDIUM_MODE wasn't being written, and this seems to affect 
"Pause frame" full duplex flow control.

The attached patch to 2.6.10, which sets the medium mode in an 
ax8817x_reset function, doubled my full duplex performance (from ~45Mbps 
to >90Mbps).
    

The only issue that I see is that the usbnet_open() function (which
calls the ax8817x_reset) is only called when the interface is brought
from DOWN to UP state.  If I initially connect to a 100FD switch, then
unplug and plug into a 10HD hub, things may get squirrely.  If I bring
the device down and back up, things will reset and I'll be fine.  Would
it possibly be better to handle this within the interrupt handler that
sees when link changes?  That is an in_interrupt() call so you'd have to
use the async read/write_cmd calls and implement an FSM to handle it.
But there weren't too many calls so it shouldn't be a major issue.
  

Here's a second try at the patch that fixes ax88172 receive performance in full duplex mode.

Dealing with the async read/write_cmd calls  looked to me to be messy, with four calls needed to read the link partner capabilities then set the medium mode.  I instead opted for a kevent, so I could use the blocking calls in process context.   If this is a bad decision for some reason, please educate me -- I'm not experienced with linux driver hacking.

I'd also like to point out that the usbnet.c patch for AX88772 support recently applied also issues AX_CMD_WRITE_MEDIUM_MODE.  It does it in the bind function and does not take into account the state of the link (10hd,100fd, etc).  It's possible the same performance issue I observed with the ax88172 applies there too.  It might be worth testing link changes between fd and hd  to make sure it behaves well.  I don't have a device with an AX88772 or I'd try it myself.

--Jamie




--- linux-2.6.10/drivers/usb/net/usbnet.c       2004-12-24 14:34:58.000000000 
-0700
+++ new-2.6.10/drivers/usb/net/usbnet.c 2005-02-20 12:34:23.288010642 -0700
@@ -206,6 +206,7 @@
 #              define EVENT_TX_HALT    0
 #              define EVENT_RX_HALT    1
 #              define EVENT_RX_MEMORY  2
+#              define EVENT_LINK_RESET 3
 };
 
 // device-specific info used by the driver
@@ -300,6 +301,7 @@
 static u32 usbnet_get_link (struct net_device *);
 static u32 usbnet_get_msglevel (struct net_device *);
 static void usbnet_set_msglevel (struct net_device *, u32);
+static void defer_kevent (struct usbnet *, int);
 
 /* mostly for PDA style devices, which are always connected if present */
 static int always_connected (struct usbnet *dev)
@@ -443,6 +445,7 @@
 #define AX_CMD_WRITE_MULTI_FILTER      0x16
 #define AX_CMD_READ_NODE_ID            0x17
 #define AX_CMD_READ_PHY_ID             0x19
+#define AX_CMD_READ_MEDIUM_STATUS       0x1a
 #define AX_CMD_WRITE_MEDIUM_MODE       0x1b
 #define AX_CMD_READ_MONITOR_MODE       0x1c
 #define AX_CMD_WRITE_MONITOR_MODE      0x1d
@@ -453,6 +456,11 @@
 #define AX_MONITOR_MAGIC               0x04
 #define AX_MONITOR_HSFS                        0x10
 
+// BitMask for MEDIUM_STATUS and MEDIUM_MODE
+#define AX_MEDIUM_FULL_DUPLEX          0x02
+#define AX_MEDIUM_TX_ABORT_ALLOW       0x04
+#define AX_MEDIUM_FLOW_CONTROL_EN      0x10
+
 #define AX_MCAST_FILTER_SIZE           8
 #define AX_MAX_MCAST                   64
 
@@ -520,9 +528,10 @@
                if (data->int_buf[5] == 0x90) {
                        link = data->int_buf[2] & 0x01;
                        if (netif_carrier_ok(dev->net) != link) {
-                               if (link)
+                               if (link) {
                                        netif_carrier_on(dev->net);
-                               else
+                                       defer_kevent (dev, EVENT_LINK_RESET );
+                               } else
                                        netif_carrier_off(dev->net);
                                devdbg(dev, "ax8817x - Link Status is: %d", 
link);
                        }
@@ -832,10 +841,31 @@
        kfree(data->int_buf);
 }
 
+static int ax8817x_reset_mm(struct usbnet *dev) {
+       int lpa;
+       int mode;
+
+       /* Initialize fullduplex flow control */
+       mode = 0;
+       ax8817x_read_cmd(dev, AX_CMD_READ_MEDIUM_STATUS, 0, 0, 1, &mode);
+       devdbg( dev, "Medium status before: 0x%02x", mode );
+
+       mode = AX_MEDIUM_TX_ABORT_ALLOW|AX_MEDIUM_FLOW_CONTROL_EN;
+       lpa = ax8817x_mdio_read(dev->net,dev->mii.phy_id, MII_LPA);
+       if ((lpa & (LPA_100FULL|LPA_100BASE4)) != 0) /* 100baseTx-FD */
+               mode |= AX_MEDIUM_FULL_DUPLEX;
+       ax8817x_write_cmd(dev, AX_CMD_WRITE_MEDIUM_MODE, mode, 0, 0, NULL);
+
+       ax8817x_read_cmd(dev, AX_CMD_READ_MEDIUM_STATUS, 0, 0, 1, &mode);
+       devdbg( dev, "Medium status after: 0x%02x", mode );
+       return 0;
+}
+
 static const struct driver_info ax8817x_info = {
        .description = "ASIX AX8817x USB 2.0 Ethernet",
        .bind = ax8817x_bind,
        .unbind = ax8817x_unbind,
+       .reset = ax8817x_reset_mm,
        .flags =  FLAG_ETHER,
        .data = 0x00130103,
 };
@@ -844,6 +874,7 @@
        .description = "DLink DUB-E100 USB Ethernet",
        .bind = ax8817x_bind,
        .unbind = ax8817x_unbind,
+       .reset = ax8817x_reset_mm,
        .flags =  FLAG_ETHER,
        .data = 0x009f9d9f,
 };
@@ -852,6 +883,7 @@
        .description = "Netgear FA-120 USB Ethernet",
        .bind = ax8817x_bind,
        .unbind = ax8817x_unbind,
+       .reset = ax8817x_reset_mm,
        .flags =  FLAG_ETHER,
        .data = 0x00130103,
 };
@@ -860,6 +892,7 @@
        .description = "Hawking UF200 USB Ethernet",
        .bind = ax8817x_bind,
        .unbind = ax8817x_unbind,
+       .reset = ax8817x_reset_mm,
        .flags =  FLAG_ETHER,
        .data = 0x001f1d1f,
 };
@@ -2776,6 +2809,18 @@
                }
        }
 
+       if (test_bit (EVENT_LINK_RESET, &dev->flags)) {
+               struct driver_info      *info = dev->driver_info;
+               int                     retval = 0;
+               clear_bit (EVENT_LINK_RESET, &dev->flags);
+               if (info->reset && (retval = info->reset (dev)) < 0) {
+                       devinfo (dev, "link reset fail (%d) usbnet usb-%s-%s, 
%s",
+                                retval,
+                                dev->udev->bus->bus_name, dev->udev->devpath,
+                                info->description);
+               }
+       }
+
        if (dev->flags)
                devdbg (dev, "kevent done, flags = 0x%lx",
                        dev->flags);

Reply via email to