On Thu, Jun 15, 2017 at 02:58:21PM +0800, Jiahau Chang wrote:
> 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.
> The register we modify is change the behavior
>
> Signed-off-by: Jiahau Chang <Lars_chang@xxxxxxxxxxxxxx>
> ---
> v3: add define value of setting ASMT write Reg
> modify dev_dbg message
> use usleep
> v2: fix coding format
>
> 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..bdf50bf 100644
> --- a/drivers/usb/host/pci-quirks.c
> +++ b/drivers/usb/host/pci-quirks.c
> @@ -77,6 +77,16 @@
> #define USB_INTEL_USB3_PSSEN 0xD8
> #define USB_INTEL_USB3PRM 0xDC
>
> +/*ASMEDIA quirk use*/
Please put ' ' characters in your comments around the text to make them
read better. This line should read:
/* ASMEDIA quirk use */
You do that in other places in this patch, please fix up.
> +#define ASMT_DATA_WRITE0_REG 0xF8
> +#define ASMT_DATA_WRITE1_REG 0xFC
> +#define ASMT_CONTROL_REG 0xE0
> +#define ASMT_CONTROL_WRITE_BIT 0x02
> +#define ASMT_WRITEREG_CMD 0x10423
> +#define ASMT_FLOWCTL_ADDR 0xFA30
> +#define ASMT_FLOWCTL_DATA 0xBA
> +#define ASMT_PSEUDO_DATA 0
> +
> /*
> * amd_chipset_gen values represent AMD different chipset generations
> */
> @@ -412,6 +422,58 @@ void usb_amd_quirk_pll_disable(void)
> }
> EXPORT_SYMBOL_GPL(usb_amd_quirk_pll_disable);
>
> +void usb_asmedia_modifyflowcontrol(struct pci_dev *pdev)
> +{
> + unsigned char value;
> + unsigned long wait_time_count;
> +
> + /*check device can accept command*/
Like there ^
> + 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: check_ready ERROR", __func__);
> + goto err_exit;
> + }
> + if ((value & ASMT_CONTROL_WRITE_BIT) == 0)
> + break;
> + wait_time_count--;
> + usleep(50);
> + }
> + if (wait_time_count == 0) {
> + dev_dbg(&pdev->dev, "%s: check_write_ready timeout", __func__);
Shouldn't this be an error? The hardware didn't respond correctly,
shouldn't users be able to report this?
> + goto err_exit;
> + }
> + /* send command and address to device */
> + pci_write_config_dword(pdev, ASMT_DATA_WRITE0_REG, ASMT_WRITEREG_CMD);
> + pci_write_config_dword(pdev, ASMT_DATA_WRITE1_REG, ASMT_FLOWCTL_ADDR);
> + pci_write_config_byte(pdev, ASMT_CONTROL_REG, ASMT_CONTROL_WRITE_BIT);
> + wait_time_count = 1000;
> + /* wait device receive the data */
> + while (wait_time_count) {
> + pci_read_config_byte(pdev, ASMT_CONTROL_REG, &value);
> + if (value == 0xff) {
> + dev_dbg(&pdev->dev, "%s: wait_ready ERROR", __func__);
> + goto err_exit;
> + }
> + if ((value & ASMT_CONTROL_WRITE_BIT) == 0)
> + break;
> + wait_time_count--;
> + usleep(50);
> + }
> + if (wait_time_count == 0) {
> + dev_dbg(&pdev->dev, "%s: wait_write_ready timeout", __func__);
Same here.
> + goto err_exit;
> + }
> + /* send data to device */
> + pci_write_config_dword(pdev, ASMT_DATA_WRITE0_REG, ASMT_FLOWCTL_DATA);
> + pci_write_config_dword(pdev, ASMT_DATA_WRITE1_REG, ASMT_PSEUDO_DATA);
> + pci_write_config_byte(pdev, ASMT_CONTROL_REG, ASMT_CONTROL_WRITE_BIT);
> +err_exit:
> + return;
> +}
> +EXPORT_SYMBOL_GPL(usb_asmedia_modifyflowcontrol);
> +
> void usb_amd_quirk_pll_enable(void)
> {
> usb_amd_quirk_pll(0);
> diff --git a/drivers/usb/host/pci-quirks.h b/drivers/usb/host/pci-quirks.h
> index 0222195..6ce1df1 100644
> --- a/drivers/usb/host/pci-quirks.h
> +++ b/drivers/usb/host/pci-quirks.h
> @@ -11,6 +11,7 @@ bool usb_amd_prefetch_quirk(void);
> void usb_amd_dev_put(void);
> void usb_amd_quirk_pll_disable(void);
> void usb_amd_quirk_pll_enable(void);
> +void usb_asmedia_modifyflowcontrol(struct pci_dev *pdev);
> void usb_enable_intel_xhci_ports(struct pci_dev *xhci_pdev);
> void usb_disable_xhci_ports(struct pci_dev *xhci_pdev);
> void sb800_prefetch(struct device *dev, int on);
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index fcf1f3f..fba52f0 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -53,6 +53,7 @@
> #define PCI_DEVICE_ID_INTEL_BROXTON_B_XHCI 0x1aa8
> #define PCI_DEVICE_ID_INTEL_APL_XHCI 0x5aa8
> #define PCI_DEVICE_ID_INTEL_DNV_XHCI 0x19d0
> +#define PCI_DEVICE_ID_ASMEDIA_1042A_XHCI 0x1142
>
> static const char hcd_name[] = "xhci_hcd";
>
> @@ -202,6 +203,10 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
> pdev->device == 0x1042)
> xhci->quirks |= XHCI_BROKEN_STREAMS;
>
> + if (pdev->vendor == PCI_VENDOR_ID_ASMEDIA &&
> + pdev->device == PCI_DEVICE_ID_ASMEDIA_1042A_XHCI)
> + xhci->quirks |= XHCI_ASMEDIA_MODIFY_FLOWCONTROL;
> +
> if (pdev->vendor == PCI_VENDOR_ID_TI && pdev->device == 0x8241)
> xhci->quirks |= XHCI_LIMIT_ENDPOINT_INTERVAL_7;
>
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 30f47d9..dcc310d 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -625,6 +625,9 @@ int xhci_run(struct usb_hcd *hcd)
> xhci_queue_vendor_command(xhci, command, 0, 0, 0,
> TRB_TYPE(TRB_NEC_GET_FW));
> }
> + if (xhci->quirks & XHCI_ASMEDIA_MODIFY_FLOWCONTROL)
> + usb_asmedia_modifyflowcontrol(to_pci_dev(hcd->self.controller));
> +
> xhci_dbg_trace(xhci, trace_xhci_dbg_init,
> "Finished xhci_run for USB2 roothub");
> return 0;
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index 73a28a9..ed58f87 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1819,6 +1819,7 @@ struct xhci_hcd {
> /* For controller with a broken Port Disable implementation */
> #define XHCI_BROKEN_PORT_PED (1 << 25)
> #define XHCI_LIMIT_ENDPOINT_INTERVAL_7 (1 << 26)
> +#define XHCI_ASMEDIA_MODIFY_FLOWCONTROL (1<<27)
BIT(27)?
At the least, follow the ' ' rules of the above #defines please.
thanks,
greg k-h
--
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
