Re: [PATCH v2] xhci: Bad Ethernet performance plugged in ASM1042A host

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

In addition to Felipes comments I have a few ones myself.

On 09.06.2017 05:33, Jiahau Chang wrote:
v2 : fix coding format

When USB Ethernet is plugged in ASMEDIA ASM1042A xHCI host, bad
performance was manifesting in Web browser use (like download
large file such as ISO image). It is known limitation of
ASM1042A that is not compatible with driver scheduling,
As a workaround we can modify flow control handling of ASM1042A.

Signed-off-by: Jiahau Chang <Lars_chang@xxxxxxxxxxxxxx>
---
  drivers/usb/host/pci-quirks.c | 62 +++++++++++++++++++++++++++++++++++++++++++
  drivers/usb/host/pci-quirks.h |  1 +
  drivers/usb/host/xhci-pci.c   |  5 ++++
  drivers/usb/host/xhci.c       |  3 +++
  drivers/usb/host/xhci.h       |  1 +
  5 files changed, 72 insertions(+)

diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index a9a1e4c..bdeaf75 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -77,6 +77,12 @@
  #define USB_INTEL_USB3_PSSEN   0xD8
  #define USB_INTEL_USB3PRM      0xDC

+/*ASMEDIA quirk use*/
+#define ASMT_DATA_WRITE0_REG	0xF8
+#define ASMT_DATA_WRITE1_REG	0xFC
+#define ASMT_CONTROL_REG	0xE0
+#define ASMT_CONTROL_WRITE_BIT	0x02
+
  /*
   * amd_chipset_gen values represent AMD different chipset generations
   */
@@ -412,6 +418,62 @@ void usb_amd_quirk_pll_disable(void)
  }
  EXPORT_SYMBOL_GPL(usb_amd_quirk_pll_disable);

+void usb_asmedia_modifyflowcontrol(struct pci_dev *pdev)
+{
+	u32 value_low, value_high;
+	unsigned char value;
+	unsigned long wait_time_count;
+
+	wait_time_count = 1000;
+	while (wait_time_count) {
+		pci_read_config_byte(pdev, ASMT_CONTROL_REG, &value);
+		if (value == 0xff) {
+			dev_dbg(&pdev->dev, "%s: wait_write_ready IO_ERROR, value=%x\n",
+				__func__, value);

Printing value=%x isn't very useful here. We know it's 0xff.

+			goto err_exit;
+		} else if ((value & ASMT_CONTROL_WRITE_BIT) == 0) {
+			break;
+		}
+		wait_time_count--;
+		udelay(50);

1000 * 50us = 50ms delay
plus another 50ms later on equals 100ms worst case.

If that is what it takes then it can't be helped.

+	}
+	if (wait_time_count == 0) {
+		dev_dbg(&pdev->dev, "%s: wait_write_ready timeout\n",
+			__func__);
+		goto err_exit;
+	}
+	value_low = 0x00010423;
+	value_high = 0xFA30;
+	pci_write_config_dword(pdev, ASMT_DATA_WRITE0_REG, value_low);
+	pci_write_config_dword(pdev, ASMT_DATA_WRITE1_REG, value_high);
+	pci_write_config_byte(pdev, ASMT_CONTROL_REG, ASMT_CONTROL_WRITE_BIT);

From here onwards:

+	wait_time_count = 1000;
+	while (wait_time_count) {
+		pci_read_config_byte(pdev, ASMT_CONTROL_REG, &value);
+		if (value == 0xff) {
+			dev_dbg(&pdev->dev, "%s: wait_write_ready IO_ERROR, value=%x\n",
+				__func__, value);
+		goto err_exit;
+		} else if ((value & ASMT_CONTROL_WRITE_BIT) == 0) {
+			break;
+		}
+		wait_time_count--;
+		udelay(50);
+	}
+	if (wait_time_count == 0) {
+		dev_dbg(&pdev->dev, "%s: wait_write_ready timeout\n",
+			__func__);
+		goto err_exit;

To here, we have the exact same code duplicated as the first loop.
Maybe restructure the code it a bit to avoid that.

Even the debug messages are exactly the same, easier to debug if they were a bit
different.

-Mathias

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux