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

2017-06-12 Thread David Laight 
From: Mathias Nyman
> Sent: 12 June 2017 15:49

Commenting on this copy because it is handy...
> 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.

I think you need to be more explicit both in the commit message
and in the code about what the actual problem is, and what the
actual effect of the control register writes is.

> >
> > Signed-off-by: Jiahau Chang 
> > ---
> >   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, );
> > +   if (value == 0xff) {
> > +   dev_dbg(>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;

No need for else after goto.

> > +   } 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 this is a probe function, can it at least be arranged to sleep?

> 
> > +   }
> > +   if (wait_time_count == 0) {
> > +   dev_dbg(>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, );
> > +   if (value == 0xff) {
> > +   dev_dbg(>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(>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.

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

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

2017-06-12 Thread Mathias Nyman 

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 
---
  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, );
+   if (value == 0xff) {
+   dev_dbg(>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(>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, );
+   if (value == 0xff) {
+   dev_dbg(>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(>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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2017-06-12 Thread Mario.Limonciello 
> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Monday, June 12, 2017 9:02 AM
> To: Limonciello, Mario <mario_limoncie...@dell.com>
> Cc: lars_ch...@asmedia.com.tw; felipe.ba...@linux.intel.com;
> jia...@gmail.com; linux-usb@vger.kernel.org; mathias.ny...@intel.com;
> brain_...@asmedia.com.tw; justin_cy_c...@asmedia.com.tw; Wang, Keith
> <keith_w...@dell.com>; yd_ts...@asmedia.com.tw
> Subject: Re: [PATCH v2] xhci: Bad Ethernet performance plugged in ASM1042A 
> host
> 
> On Mon, Jun 12, 2017 at 01:52:36PM +, mario.limoncie...@dell.com wrote:
> > > -Original Message-
> > > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > > Sent: Monday, June 12, 2017 8:42 AM
> > > To: Limonciello, Mario <mario_limoncie...@dell.com>
> > > Cc: lars_ch...@asmedia.com.tw; felipe.ba...@linux.intel.com;
> > > jia...@gmail.com; linux-usb@vger.kernel.org; mathias.ny...@intel.com;
> > > brain_...@asmedia.com.tw; justin_cy_c...@asmedia.com.tw; Wang, Keith
> > > <keith_w...@dell.com>; yd_ts...@asmedia.com.tw
> > > Subject: Re: [PATCH v2] xhci: Bad Ethernet performance plugged in ASM1042A
> host
> > >
> > > On Mon, Jun 12, 2017 at 01:28:57PM +, mario.limoncie...@dell.com
> wrote:
> > > > > -Original Message-
> > > > > From: Lars Chang(張家豪) [mailto:lars_ch...@asmedia.com.tw]
> > > > > Sent: Friday, June 9, 2017 1:34 AM
> > > > > To: 'Felipe Balbi' <felipe.ba...@linux.intel.com>; Jiahau Chang
> > > > > <jia...@gmail.com>; linux-usb@vger.kernel.org;
> mathias.ny...@intel.com
> > > > > Cc: Brain Lee(李魁中) <brain_...@asmedia.com.tw>; Limonciello, Mario
> > > > > <mario_limoncie...@dell.com>; Justin_CY Chen(陳志勇)
> > > > > <justin_cy_c...@asmedia.com.tw>; Wang, Keith
> <keith_w...@dell.com>;
> > > Yd
> > > > > Tseng(曾裕達) <yd_ts...@asmedia.com.tw>; Jiahau Chang
> > > <jia...@gmail.com>
> > > > > Subject: RE: [PATCH v2] xhci: Bad Ethernet performance plugged in
> ASM1042A
> > > host
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-
> > > > > > From: Felipe Balbi [mailto:felipe.ba...@linux.intel.com]
> > > > > > Sent: Friday, June 09, 2017 2:04 PM
> > > > > > To: Jiahau Chang; linux-usb@vger.kernel.org; mathias.ny...@intel.com
> > > > > > Cc: Brain Lee(李魁中); mario.limoncie...@dell.com; Justin_CY Chen(陳志
> 勇);
> > > > > > keith_w...@dell.com; Yd Tseng(曾裕達); Jiahau Chang; Lars Chang(張家
> 豪)
> > > > > > Subject: Re: [PATCH v2] xhci: Bad Ethernet performance plugged in
> ASM1042A
> > > > > > host
> > > > > >
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > Jiahau Chang <jia...@gmail.com> writes:
> > > > > > > v2 : fix coding format
> > > > > >
> > > > > > this doesn't need to be in the commit log. You should place such 
> > > > > > notes
> > > > > > after the tearline (---) below
> > > > >
> > > > > I will move it after the tearline. Should I need to resend the patch?
> > > > > >
> > > > > > > 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_ch...@asmedia.com.tw>
> > > > > > > ---
> > > > > > >  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

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

2017-06-12 Thread Greg KH 
On Mon, Jun 12, 2017 at 01:52:36PM +, mario.limoncie...@dell.com wrote:
> > -Original Message-
> > From: Greg KH [mailto:gre...@linuxfoundation.org]
> > Sent: Monday, June 12, 2017 8:42 AM
> > To: Limonciello, Mario <mario_limoncie...@dell.com>
> > Cc: lars_ch...@asmedia.com.tw; felipe.ba...@linux.intel.com;
> > jia...@gmail.com; linux-usb@vger.kernel.org; mathias.ny...@intel.com;
> > brain_...@asmedia.com.tw; justin_cy_c...@asmedia.com.tw; Wang, Keith
> > <keith_w...@dell.com>; yd_ts...@asmedia.com.tw
> > Subject: Re: [PATCH v2] xhci: Bad Ethernet performance plugged in ASM1042A 
> > host
> > 
> > On Mon, Jun 12, 2017 at 01:28:57PM +, mario.limoncie...@dell.com wrote:
> > > > -Original Message-
> > > > From: Lars Chang(張家豪) [mailto:lars_ch...@asmedia.com.tw]
> > > > Sent: Friday, June 9, 2017 1:34 AM
> > > > To: 'Felipe Balbi' <felipe.ba...@linux.intel.com>; Jiahau Chang
> > > > <jia...@gmail.com>; linux-usb@vger.kernel.org; mathias.ny...@intel.com
> > > > Cc: Brain Lee(李魁中) <brain_...@asmedia.com.tw>; Limonciello, Mario
> > > > <mario_limoncie...@dell.com>; Justin_CY Chen(陳志勇)
> > > > <justin_cy_c...@asmedia.com.tw>; Wang, Keith <keith_w...@dell.com>;
> > Yd
> > > > Tseng(曾裕達) <yd_ts...@asmedia.com.tw>; Jiahau Chang
> > <jia...@gmail.com>
> > > > Subject: RE: [PATCH v2] xhci: Bad Ethernet performance plugged in 
> > > > ASM1042A
> > host
> > > >
> > > >
> > > >
> > > > > -Original Message-
> > > > > From: Felipe Balbi [mailto:felipe.ba...@linux.intel.com]
> > > > > Sent: Friday, June 09, 2017 2:04 PM
> > > > > To: Jiahau Chang; linux-usb@vger.kernel.org; mathias.ny...@intel.com
> > > > > Cc: Brain Lee(李魁中); mario.limoncie...@dell.com; Justin_CY Chen(陳志勇);
> > > > > keith_w...@dell.com; Yd Tseng(曾裕達); Jiahau Chang; Lars Chang(張家豪)
> > > > > Subject: Re: [PATCH v2] xhci: Bad Ethernet performance plugged in 
> > > > > ASM1042A
> > > > > host
> > > > >
> > > > >
> > > > > Hi,
> > > > >
> > > > > Jiahau Chang <jia...@gmail.com> writes:
> > > > > > v2 : fix coding format
> > > > >
> > > > > this doesn't need to be in the commit log. You should place such notes
> > > > > after the tearline (---) below
> > > >
> > > > I will move it after the tearline. Should I need to resend the patch?
> > > > >
> > > > > > 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_ch...@asmedia.com.tw>
> > > > > > ---
> > > > > >  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_chipse

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

2017-06-12 Thread Mario.Limonciello 
> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Monday, June 12, 2017 8:42 AM
> To: Limonciello, Mario <mario_limoncie...@dell.com>
> Cc: lars_ch...@asmedia.com.tw; felipe.ba...@linux.intel.com;
> jia...@gmail.com; linux-usb@vger.kernel.org; mathias.ny...@intel.com;
> brain_...@asmedia.com.tw; justin_cy_c...@asmedia.com.tw; Wang, Keith
> <keith_w...@dell.com>; yd_ts...@asmedia.com.tw
> Subject: Re: [PATCH v2] xhci: Bad Ethernet performance plugged in ASM1042A 
> host
> 
> On Mon, Jun 12, 2017 at 01:28:57PM +, mario.limoncie...@dell.com wrote:
> > > -Original Message-
> > > From: Lars Chang(張家豪) [mailto:lars_ch...@asmedia.com.tw]
> > > Sent: Friday, June 9, 2017 1:34 AM
> > > To: 'Felipe Balbi' <felipe.ba...@linux.intel.com>; Jiahau Chang
> > > <jia...@gmail.com>; linux-usb@vger.kernel.org; mathias.ny...@intel.com
> > > Cc: Brain Lee(李魁中) <brain_...@asmedia.com.tw>; Limonciello, Mario
> > > <mario_limoncie...@dell.com>; Justin_CY Chen(陳志勇)
> > > <justin_cy_c...@asmedia.com.tw>; Wang, Keith <keith_w...@dell.com>;
> Yd
> > > Tseng(曾裕達) <yd_ts...@asmedia.com.tw>; Jiahau Chang
> <jia...@gmail.com>
> > > Subject: RE: [PATCH v2] xhci: Bad Ethernet performance plugged in ASM1042A
> host
> > >
> > >
> > >
> > > > -Original Message-
> > > > From: Felipe Balbi [mailto:felipe.ba...@linux.intel.com]
> > > > Sent: Friday, June 09, 2017 2:04 PM
> > > > To: Jiahau Chang; linux-usb@vger.kernel.org; mathias.ny...@intel.com
> > > > Cc: Brain Lee(李魁中); mario.limoncie...@dell.com; Justin_CY Chen(陳志勇);
> > > > keith_w...@dell.com; Yd Tseng(曾裕達); Jiahau Chang; Lars Chang(張家豪)
> > > > Subject: Re: [PATCH v2] xhci: Bad Ethernet performance plugged in 
> > > > ASM1042A
> > > > host
> > > >
> > > >
> > > > Hi,
> > > >
> > > > Jiahau Chang <jia...@gmail.com> writes:
> > > > > v2 : fix coding format
> > > >
> > > > this doesn't need to be in the commit log. You should place such notes
> > > > after the tearline (---) below
> > >
> > > I will move it after the tearline. Should I need to resend the patch?
> > > >
> > > > > 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_ch...@asmedia.com.tw>
> > > > > ---
> > > > >  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;
&g

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

2017-06-12 Thread Greg KH 
On Mon, Jun 12, 2017 at 01:28:57PM +, mario.limoncie...@dell.com wrote:
> > -Original Message-
> > From: Lars Chang(張家豪) [mailto:lars_ch...@asmedia.com.tw]
> > Sent: Friday, June 9, 2017 1:34 AM
> > To: 'Felipe Balbi' <felipe.ba...@linux.intel.com>; Jiahau Chang
> > <jia...@gmail.com>; linux-usb@vger.kernel.org; mathias.ny...@intel.com
> > Cc: Brain Lee(李魁中) <brain_...@asmedia.com.tw>; Limonciello, Mario
> > <mario_limoncie...@dell.com>; Justin_CY Chen(陳志勇)
> > <justin_cy_c...@asmedia.com.tw>; Wang, Keith <keith_w...@dell.com>; Yd
> > Tseng(曾裕達) <yd_ts...@asmedia.com.tw>; Jiahau Chang <jia...@gmail.com>
> > Subject: RE: [PATCH v2] xhci: Bad Ethernet performance plugged in ASM1042A 
> > host
> > 
> > 
> > 
> > > -Original Message-
> > > From: Felipe Balbi [mailto:felipe.ba...@linux.intel.com]
> > > Sent: Friday, June 09, 2017 2:04 PM
> > > To: Jiahau Chang; linux-usb@vger.kernel.org; mathias.ny...@intel.com
> > > Cc: Brain Lee(李魁中); mario.limoncie...@dell.com; Justin_CY Chen(陳志勇);
> > > keith_w...@dell.com; Yd Tseng(曾裕達); Jiahau Chang; Lars Chang(張家豪)
> > > Subject: Re: [PATCH v2] xhci: Bad Ethernet performance plugged in ASM1042A
> > > host
> > >
> > >
> > > Hi,
> > >
> > > Jiahau Chang <jia...@gmail.com> writes:
> > > > v2 : fix coding format
> > >
> > > this doesn't need to be in the commit log. You should place such notes
> > > after the tearline (---) below
> > 
> > I will move it after the tearline. Should I need to resend the patch?
> > >
> > > > 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_ch...@asmedia.com.tw>
> > > > ---
> > > >  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, );
> > > > +   if (value == 0xff) {
> > > > +   dev_dbg(>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);
> > > > +   }
> > > > +

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

2017-06-12 Thread Mario.Limonciello 
> -Original Message-
> From: Lars Chang(張家豪) [mailto:lars_ch...@asmedia.com.tw]
> Sent: Friday, June 9, 2017 1:34 AM
> To: 'Felipe Balbi' <felipe.ba...@linux.intel.com>; Jiahau Chang
> <jia...@gmail.com>; linux-usb@vger.kernel.org; mathias.ny...@intel.com
> Cc: Brain Lee(李魁中) <brain_...@asmedia.com.tw>; Limonciello, Mario
> <mario_limoncie...@dell.com>; Justin_CY Chen(陳志勇)
> <justin_cy_c...@asmedia.com.tw>; Wang, Keith <keith_w...@dell.com>; Yd
> Tseng(曾裕達) <yd_ts...@asmedia.com.tw>; Jiahau Chang <jia...@gmail.com>
> Subject: RE: [PATCH v2] xhci: Bad Ethernet performance plugged in ASM1042A 
> host
> 
> 
> 
> > -Original Message-
> > From: Felipe Balbi [mailto:felipe.ba...@linux.intel.com]
> > Sent: Friday, June 09, 2017 2:04 PM
> > To: Jiahau Chang; linux-usb@vger.kernel.org; mathias.ny...@intel.com
> > Cc: Brain Lee(李魁中); mario.limoncie...@dell.com; Justin_CY Chen(陳志勇);
> > keith_w...@dell.com; Yd Tseng(曾裕達); Jiahau Chang; Lars Chang(張家豪)
> > Subject: Re: [PATCH v2] xhci: Bad Ethernet performance plugged in ASM1042A
> > host
> >
> >
> > Hi,
> >
> > Jiahau Chang <jia...@gmail.com> writes:
> > > v2 : fix coding format
> >
> > this doesn't need to be in the commit log. You should place such notes
> > after the tearline (---) below
> 
> I will move it after the tearline. Should I need to resend the patch?
> >
> > > 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_ch...@asmedia.com.tw>
> > > ---
> > >  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, );
> > > + if (value == 0xff) {
> > > + dev_dbg(>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(>dev, "%s: wait_write_ready timeout\n",
> > > + __func__);
> > > + goto err_exit;
> > > + }
> > > + value_low = 0x00010423;
> > > + value_high = 0xFA30;
> >
> > sorry, no magic constants
> >
> > > + 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);
> > > + wait_time_count = 1000;
> > > + while (wait_tim

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

2017-06-09 Thread Greg KH 
On Fri, Jun 09, 2017 at 06:33:39AM +, Lars Chang(張家豪) wrote:
> ==
> This email and any attachments to it contain confidential information and are 
> intended solely for the use of the individual to whom it 
> is addressed.If you are not the intended recipient or receive it 
> accidentally, please immediately notify the sender by e-mail and delete 
> the message and any attachments from your computer system, and destroy all 
> hard copies. If any, please be advised that any unauthorized 
> disclosure, copying, distribution or any action taken or omitted in reliance 
> on this, is illegal and prohibited. Furthermore, any views 
> or opinions expressed are solely those of the author and do not represent 
> those of ASMedia Technology Inc. Thank you for your cooperation.
> ==

As per your instructions, I am deleting this.  Note, I'm really not even
supposed to be telling you I am doing so.

Hint, you have to remove this footer if you wish to participate in
kernel development...

thanks,

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

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

2017-06-09 Thread 張家豪 


> -Original Message-
> From: Felipe Balbi [mailto:felipe.ba...@linux.intel.com]
> Sent: Friday, June 09, 2017 2:04 PM
> To: Jiahau Chang; linux-usb@vger.kernel.org; mathias.ny...@intel.com
> Cc: Brain Lee(李魁中); mario.limoncie...@dell.com; Justin_CY Chen(陳志勇);
> keith_w...@dell.com; Yd Tseng(曾裕達); Jiahau Chang; Lars Chang(張家豪)
> Subject: Re: [PATCH v2] xhci: Bad Ethernet performance plugged in ASM1042A
> host
> 
> 
> Hi,
> 
> Jiahau Chang <jia...@gmail.com> writes:
> > v2 : fix coding format
> 
> this doesn't need to be in the commit log. You should place such notes
> after the tearline (---) below

I will move it after the tearline. Should I need to resend the patch?
> 
> > 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_ch...@asmedia.com.tw>
> > ---
> >  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, );
> > +   if (value == 0xff) {
> > +   dev_dbg(>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(>dev, "%s: wait_write_ready timeout\n",
> > +   __func__);
> > +   goto err_exit;
> > +   }
> > +   value_low = 0x00010423;
> > +   value_high = 0xFA30;
> 
> sorry, no magic constants
> 
> > +   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);
> > +   wait_time_count = 1000;
> > +   while (wait_time_count) {
> > +   pci_read_config_byte(pdev, ASMT_CONTROL_REG, );
> > +   if (value == 0xff) {
> > +   dev_dbg(>dev, "%s: wait_write_ready IO_ERROR,
> value=%x\n",
> > +   __func__, value);
> > +   goto err_exit;
> 
> indentation
> 
> > +   } else if ((value & ASMT_CONTROL_WRITE_BIT) == 0) {
> > +   break;
> > +   }
> > +   wait_time_count--;
> > +   udelay(50);
> > +   }
> > +   if (wait_time_count == 0) {
> > +   dev_dbg(>dev, "%s: wait_write_ready timeout\n",
> > +   __func__);
> > +   goto err_exit;
> > +   }
> > +   pci_write_config_dword(pdev, ASMT_DATA_WRITE0_REG, 0xBA);
> > +   pci_write_config_dword(pdev, ASMT_DATA_WRITE1_REG, 0);
> 
> likewise
> 
> > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> > index 73a28a9..ed58f87 100644
> > --- a/drive

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

2017-06-09 Thread Felipe Balbi 

Hi,

"Lars Chang(張家豪)" <lars_ch...@asmedia.com.tw> writes:
>> -Original Message-
>> From: Felipe Balbi [mailto:felipe.ba...@linux.intel.com]
>> Sent: Friday, June 09, 2017 2:04 PM
>> To: Jiahau Chang; linux-usb@vger.kernel.org; mathias.ny...@intel.com
>> Cc: Brain Lee(李魁中); mario.limoncie...@dell.com; Justin_CY Chen(陳志勇);
>> keith_w...@dell.com; Yd Tseng(曾裕達); Jiahau Chang; Lars Chang(張家豪)
>> Subject: Re: [PATCH v2] xhci: Bad Ethernet performance plugged in ASM1042A
>> host
>> 
>> 
>> Hi,
>> 
>> Jiahau Chang <jia...@gmail.com> writes:
>> > v2 : fix coding format
>> 
>> this doesn't need to be in the commit log. You should place such notes
>> after the tearline (---) below
>
> I will move it after the tearline. Should I need to resend the patch?

it's best to wait for more review comments. You don't want to spam the
list.

> ==
> This email and any attachments to it contain confidential information and are 
> intended solely for the use of the individual to whom it 
> is addressed.If you are not the intended recipient or receive it 
> accidentally, please immediately notify the sender by e-mail and delete 
> the message and any attachments from your computer system, and destroy all 
> hard copies. If any, please be advised that any unauthorized 
> disclosure, copying, distribution or any action taken or omitted in reliance 
> on this, is illegal and prohibited. Furthermore, any views 
> or opinions expressed are solely those of the author and do not represent 
> those of ASMedia Technology Inc. Thank you for your cooperation.
> ==

make sure these footers don't show up when sending mails to the mailing
list. I'm now deleting this email

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

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

2017-06-09 Thread Felipe Balbi 

Hi,

Jiahau Chang  writes:
> v2 : fix coding format

this doesn't need to be in the commit log. You should place such notes
after the tearline (---) below

> 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 
> ---
>  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, );
> + if (value == 0xff) {
> + dev_dbg(>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(>dev, "%s: wait_write_ready timeout\n",
> + __func__);
> + goto err_exit;
> + }
> + value_low = 0x00010423;
> + value_high = 0xFA30;

sorry, no magic constants

> + 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);
> + wait_time_count = 1000;
> + while (wait_time_count) {
> + pci_read_config_byte(pdev, ASMT_CONTROL_REG, );
> + if (value == 0xff) {
> + dev_dbg(>dev, "%s: wait_write_ready IO_ERROR, 
> value=%x\n",
> + __func__, value);
> + goto err_exit;

indentation

> + } else if ((value & ASMT_CONTROL_WRITE_BIT) == 0) {
> + break;
> + }
> + wait_time_count--;
> + udelay(50);
> + }
> + if (wait_time_count == 0) {
> + dev_dbg(>dev, "%s: wait_write_ready timeout\n",
> + __func__);
> + goto err_exit;
> + }
> + pci_write_config_dword(pdev, ASMT_DATA_WRITE0_REG, 0xBA);
> + pci_write_config_dword(pdev, ASMT_DATA_WRITE1_REG, 0);

likewise

> 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)

spaces around <<

-- 
balbi


signature.asc
Description: PGP signature