[PATCH] Input: gtco - bounds check collection indent level

2019-07-11 Thread Grant Hernandez
The GTCO tablet input driver configures itself from an HID report sent
via USB during the initial enumeration process. Some debugging messages
are generated during the parsing. A debugging message indentation
counter is not bounds checked, leading to the ability for a specially
crafted HID report to cause '-' and null bytes be written past the end
of the indentation array. As long as the kernel has CONFIG_DYNAMIC_DEBUG
enabled, this code will not be optimized out.  This was discovered
during code review after a previous syzkaller bug was found in this
driver.

Cc: sta...@vger.kernel.org
Signed-off-by: Grant Hernandez 
---
 drivers/input/tablet/gtco.c | 19 ---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/input/tablet/gtco.c b/drivers/input/tablet/gtco.c
index 4b8b9d7aa75e..9771052ed027 100644
--- a/drivers/input/tablet/gtco.c
+++ b/drivers/input/tablet/gtco.c
@@ -78,6 +78,7 @@ Scott Hill sh...@gtcocalcomp.com
 
 /* Max size of a single report */
 #define REPORT_MAX_SIZE   10
+#define MAX_COLLECTION_LEVELS  10
 
 
 /* Bitmask whether pen is in range */
@@ -223,8 +224,7 @@ static void parse_hid_report_descriptor(struct gtco 
*device, char * report,
char  maintype = 'x';
char  globtype[12];
int   indent = 0;
-   char  indentstr[10] = "";
-
+   char  indentstr[MAX_COLLECTION_LEVELS+1] = {0};
 
dev_dbg(ddev, "==>>>>>>PARSE<<<<<<==\n");
 
@@ -350,6 +350,12 @@ static void parse_hid_report_descriptor(struct gtco 
*device, char * report,
case TAG_MAIN_COL_START:
maintype = 'S';
 
+   if (indent == MAX_COLLECTION_LEVELS) {
+   dev_err(ddev, "Collection level %d 
would exceed limit of %d\n",
+   indent+1, 
MAX_COLLECTION_LEVELS);
+   break;
+   }
+
if (data == 0) {
dev_dbg(ddev, "==>>>>>> 
Physical\n");
strcpy(globtype, "Physical");
@@ -369,8 +375,15 @@ static void parse_hid_report_descriptor(struct gtco 
*device, char * report,
break;
 
case TAG_MAIN_COL_END:
-   dev_dbg(ddev, "<<<<<<==\n");
maintype = 'E';
+
+   if (indent == 0) {
+   dev_err(ddev, "Collection level already 
at zero\n");
+   break;
+   }
+
+   dev_dbg(ddev, "<<<<<<==\n");
+
indent--;
for (x = 0; x < indent; x++)
indentstr[x] = '-';
-- 
2.22.0.410.gd8fdbe21b5-goog



Re: [PATCH] usb: gadget: udc: lpc32xx: allocate descriptor with GFP_ATOMIC

2019-05-13 Thread James Grant
Tested with a board containing LPC3250 SOC and STOTG04 PHY by using 
serial gadget.


Needed patch series starting with "[PATCH 0/5] usb: gadget: udc: 
lpc32xx: add stotg04 phy support" also.


Tested-by: James Grant 

Regards,
James Grant.

On 11/05/2019 00:42, Alexandre Belloni wrote:

Gadget drivers may queue request in interrupt context. This would lead to
a descriptor allocation in that context. In that case we would hit
BUG_ON(in_interrupt()) in __get_vm_area_node.

Signed-off-by: Alexandre Belloni
---
  drivers/usb/gadget/udc/lpc32xx_udc.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/lpc32xx_udc.c 
b/drivers/usb/gadget/udc/lpc32xx_udc.c
index d8f1c60793ed..b706d9c85a35 100644
--- a/drivers/usb/gadget/udc/lpc32xx_udc.c
+++ b/drivers/usb/gadget/udc/lpc32xx_udc.c
@@ -938,7 +938,7 @@ static struct lpc32xx_usbd_dd_gad *udc_dd_alloc(struct 
lpc32xx_udc *udc)
struct lpc32xx_usbd_dd_gad  *dd;
  
  	dd = (struct lpc32xx_usbd_dd_gad *) dma_pool_alloc(

-   udc->dd_cache, (GFP_KERNEL | GFP_DMA), &dma);
+   udc->dd_cache, (GFP_ATOMIC | GFP_DMA), &dma);
if (dd)
dd->this_dma = dma;
  


Re: [PATCH 0/5] usb: gadget: udc: lpc32xx: add stotg04 phy support

2019-05-13 Thread James Grant
Tested with a board containing LPC3250 SOC and STOTG04 PHY by using 
serial gadget.


Needed patch "[PATCH] usb: gadget: udc: lpc32xx: allocate descriptor 
with GFP_ATOMIC" also.


Tested-by: James Grant 

Regards,
James Grant.

On 09/04/2019 14:09, Alexandre Belloni Wrote:

Hi,

This series starts to clean up the lpc32xx udc driver and also repairs
interrupt handling so hotplugging actually works. The design I tested
that on uses an stotg04 instead of the usual isp1301 so support for that
is also added.

A bit more work is needed to get the RNDIS gadget driver to work.

Alexandre Belloni (5):
   usb: gadget: udc: lpc32xx: simplify probe
   usb: gadget: udc: lpc32xx: simplify vbus handling
   usb: gadget: udc: lpc32xx: properly setup phy interrupts
   usb: gadget: udc: lpc32xx: add support for stotg04 phy
   usb: gadget: udc: lpc32xx: rework interrupt handling

  drivers/usb/gadget/udc/lpc32xx_udc.c | 167 +++
  1 file changed, 66 insertions(+), 101 deletions(-)


Re: Particular USB port does not work with certain wifi usb device xhci_hcd

2018-07-03 Thread Jonny Grant

Hi here

There are a lot of emails on this list. Do you use a bug tracker?

Otherwise, I can unsubscribe, and anyone can email me on j...@jguk.org if 
they want to work on it.


Regards
Jonny


On 02/07/18 21:20, Jonny Grant wrote:

Hi!

My ZyDAS USB wifi works well on other laptop, and this laptop in 2 ports, but 
not a particular 1 port. That port does work with USB mouse tough - so I don't 
think it is a hardware issue.

4.15.0-23-generic #25-Ubuntu SMP Wed May 23 18:02:16 UTC 2018 x86_64 x86_64 
x86_64 GNU/Linux

Is there a problem with the driver?

let me know if any other info needed!


Jul  2 10:30:46 asus kernel: [40466.301834] usb 3-2: new high-speed USB device 
number 24 using xhci_hcd
Jul  2 10:30:46 asus kernel: [40466.429772] usb 3-2: device descriptor read/64, 
error -71
Jul  2 10:30:46 asus kernel: [40466.665802] usb 3-2: device descriptor read/64, 
error -71
Jul  2 10:30:46 asus kernel: [40466.901783] usb 3-2: new high-speed USB device 
number 25 using xhci_hcd
Jul  2 10:30:47 asus kernel: [40467.029797] usb 3-2: device descriptor read/64, 
error -71
Jul  2 10:30:47 asus kernel: [40467.265769] usb 3-2: device descriptor read/64, 
error -71
Jul  2 10:30:47 asus kernel: [40467.373859] usb usb3-port2: attempt power cycle
Jul  2 10:30:48 asus kernel: [40468.025748] usb 3-2: new high-speed USB device 
number 26 using xhci_hcd
Jul  2 10:30:48 asus kernel: [40468.025842] usb 3-2: Device not responding to 
setup address.
Jul  2 10:30:48 asus kernel: [40468.233839] usb 3-2: Device not responding to 
setup address.
Jul  2 10:30:48 asus kernel: [40468.441770] usb 3-2: device not accepting 
address 26, error -71
Jul  2 10:30:48 asus kernel: [40468.569748] usb 3-2: new high-speed USB device 
number 27 using xhci_hcd
Jul  2 10:30:48 asus kernel: [40468.569868] usb 3-2: Device not responding to 
setup address.
Jul  2 10:30:48 asus kernel: [40468.777897] usb 3-2: Device not responding to 
setup address.
Jul  2 10:30:49 asus kernel: [40468.985722] usb 3-2: device not accepting 
address 27, error -71
Jul  2 10:30:49 asus kernel: [40468.985788] usb usb3-port2: unable to enumerate 
USB device



I had opened bug here before
https://bugzilla.kernel.org/show_bug.cgi?id=200391

--
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 stable v4.4+] r8152: add Linksys USB3GIGV1 id

2018-04-26 Thread Grant Grundler
On Thu, Apr 26, 2018 at 12:56 AM, Krzysztof Kozlowski  wrote:
> On Thu, Apr 26, 2018 at 2:40 AM, Grant Grundler  wrote:
>> On Wed, Apr 25, 2018 at 2:54 AM, Krzysztof Kozlowski 
>> wrote:
>>>
>>> commit 90841047a01b452cc8c3f9b990698b264143334a upstream
>>>
>>> This linksys dongle by default comes up in cdc_ether mode.
>>> This patch allows r8152 to claim the device:
>>>Bus 002 Device 002: ID 13b1:0041 Linksys
>>>
>>> Signed-off-by: Grant Grundler 
>>> Reviewed-by: Douglas Anderson 
>>> Signed-off-by: David S. Miller 
>>> [krzk: Rebase on v4.4]'
>>
>>
>> thanks krzk!
>>
>> FTR, to support RTL8153B (HW ID 0x6010), the follow patch series to bring
>> r8152 v1.09.9 driver from 4.14 kernel.org to 3 (of 5) older Chrome OS
>> kernels:
>>
>> 3.14:
>> https://chromium-review.googlesource.com/q/topic:%22update_r8152-3.14%22+(status:open%20OR%20status:merged)
>> 3.18:
>> https://chromium-review.googlesource.com/q/topic:%2522update-r8152-3.18%2522+(status:open+OR+status:merged)
>> 4.4:
>> https://chromium-review.googlesource.com/q/topic:%2522update_r8152-4.4%2522+(status:open+OR+status:merged)
>>
>> caveat: These series are not suitable directly for kernel.org submission
>> (extraneous stuff in the commit messages, order is different). Using the
>> original SHA1 (in each commit message), this can all be fixed up by
>> hand/simple scripts.
>
> Hi Grant,
>
> These are regular feature/patch backports so they do not fit into
> stable process. Only new quirks and IDs are accepted for stable.

Hi Krzysztof!
Sorry, I wasn't advocating for -stable inclusion. I shared in case
someone has unusually high USB ethernet requirements similar to Chrome
OS test lab which nearly all dongles I've tested can't provide.

Chrome OS test lab needs a USB ethernet dongle that reliably
negotiates a link (e.g. 10k iterations in a row). RTL8153 in general
are good (> 99.99% gets GigE link speed) but RTL8153B is the first
dongle that meets Chrome OS test lab requirements. The patch series
above is required to support RTL8153B.

cheers,
grant
--
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 stable v4.4+] r8152: add Linksys USB3GIGV1 id

2018-04-25 Thread Grant Grundler
On Wed, Apr 25, 2018 at 2:54 AM, Krzysztof Kozlowski  wrote:
> commit 90841047a01b452cc8c3f9b990698b264143334a upstream
>
> This linksys dongle by default comes up in cdc_ether mode.
> This patch allows r8152 to claim the device:
>Bus 002 Device 002: ID 13b1:0041 Linksys
>
> Signed-off-by: Grant Grundler 
> Reviewed-by: Douglas Anderson 
> Signed-off-by: David S. Miller 
> [krzk: Rebase on v4.4]
> Signed-off-by: Krzysztof Kozlowski 

thanks krzk!

FTR, to support RTL8153B (HW ID 0x6010), the follow patch series to
bring r8152 v1.09.9 driver from 4.14 kernel.org to 3 (of 5) older
Chrome OS kernels:

3.14: 
https://chromium-review.googlesource.com/q/topic:%22update_r8152-3.14%22+(status:open%20OR%20status:merged)
3.18: 
https://chromium-review.googlesource.com/q/topic:%2522update-r8152-3.18%2522+(status:open+OR+status:merged)
4.4: 
https://chromium-review.googlesource.com/q/topic:%2522update_r8152-4.4%2522+(status:open+OR+status:merged)

caveat: These series are not suitable directly for kernel.org
submission (extraneous stuff in the commit messages, order is
different). Using the original SHA1 (in each commit message), this can
all be fixed up by hand/simple scripts.

cheers,
grant


> ---
>  drivers/net/usb/cdc_ether.c | 10 ++
>  drivers/net/usb/r8152.c |  2 ++
>  2 files changed, 12 insertions(+)
>
> diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
> index 6578127db847..f71abe50ea6f 100644
> --- a/drivers/net/usb/cdc_ether.c
> +++ b/drivers/net/usb/cdc_ether.c
> @@ -461,6 +461,7 @@ static const struct driver_info wwan_info = {
>  #define REALTEK_VENDOR_ID  0x0bda
>  #define SAMSUNG_VENDOR_ID  0x04e8
>  #define LENOVO_VENDOR_ID   0x17ef
> +#define LINKSYS_VENDOR_ID  0x13b1
>  #define NVIDIA_VENDOR_ID   0x0955
>  #define HP_VENDOR_ID   0x03f0
>
> @@ -650,6 +651,15 @@ static const struct usb_device_id  products[] = {
> .driver_info = 0,
>  },
>
> +#if IS_ENABLED(CONFIG_USB_RTL8152)
> +/* Linksys USB3GIGV1 Ethernet Adapter */
> +{
> +   USB_DEVICE_AND_INTERFACE_INFO(LINKSYS_VENDOR_ID, 0x0041, 
> USB_CLASS_COMM,
> +   USB_CDC_SUBCLASS_ETHERNET, USB_CDC_PROTO_NONE),
> +   .driver_info = 0,
> +},
> +#endif
> +
>  /* Lenovo Thinkpad USB 3.0 Ethernet Adapters (based on Realtek RTL8153) */
>  {
> USB_DEVICE_AND_INTERFACE_INFO(LENOVO_VENDOR_ID, 0x7205, 
> USB_CLASS_COMM,
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 89950f5cea71..b2c1a435357f 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -506,6 +506,7 @@ enum rtl8152_flags {
>  #define VENDOR_ID_REALTEK  0x0bda
>  #define VENDOR_ID_SAMSUNG  0x04e8
>  #define VENDOR_ID_LENOVO   0x17ef
> +#define VENDOR_ID_LINKSYS  0x13b1
>  #define VENDOR_ID_NVIDIA   0x0955
>
>  #define MCU_TYPE_PLA   0x0100
> @@ -4376,6 +4377,7 @@ static struct usb_device_id rtl8152_table[] = {
> {REALTEK_USB_DEVICE(VENDOR_ID_SAMSUNG, 0xa101)},
> {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x7205)},
> {REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x304f)},
> +   {REALTEK_USB_DEVICE(VENDOR_ID_LINKSYS, 0x0041)},
> {REALTEK_USB_DEVICE(VENDOR_ID_NVIDIA,  0x09ff)},
> {}
>  };
> --
> 2.7.4
>
--
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 V4] r8152: add Linksys USB3GIGV1 id

2017-10-02 Thread Grant Grundler
On Sun, Oct 1, 2017 at 10:39 PM, David Miller  wrote:
> From: Grant Grundler 
> Date: Thu, 28 Sep 2017 11:35:00 -0700
>
>> This linksys dongle by default comes up in cdc_ether mode.
>> This patch allows r8152 to claim the device:
>>Bus 002 Device 002: ID 13b1:0041 Linksys
>>
>> Signed-off-by: Grant Grundler 
>
> Applied, thanks.

thanks David, Doug, Oliver! :)

cheers,
grant
--
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


[PATCH V4] r8152: add Linksys USB3GIGV1 id

2017-09-28 Thread Grant Grundler
This linksys dongle by default comes up in cdc_ether mode.
This patch allows r8152 to claim the device:
   Bus 002 Device 002: ID 13b1:0041 Linksys

Signed-off-by: Grant Grundler 
---
 drivers/net/usb/cdc_ether.c | 10 ++
 drivers/net/usb/r8152.c |  2 ++
 2 files changed, 12 insertions(+)

V4: use IS_ENABLED() to check CONFIG_USB_RTL8152 is m or y.
(verified by adding #error to the new code and trying to compile
 Thanks Doug for the tip!)
Add LINKSYS vendor #define in same order for both drivers.

V3: for backwards compat, add #ifdef CONFIG_USB_RTL8152 around
the cdc_ether blacklist entry so the cdc_ether driver can
still claim the device if r8152 driver isn't configured.

V2: add LINKSYS_VENDOR_ID to cdc_ether blacklist



diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index 8ab281b478f2..677a85360db1 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -547,6 +547,7 @@ static const struct driver_info wwan_info = {
 #define REALTEK_VENDOR_ID  0x0bda
 #define SAMSUNG_VENDOR_ID  0x04e8
 #define LENOVO_VENDOR_ID   0x17ef
+#define LINKSYS_VENDOR_ID  0x13b1
 #define NVIDIA_VENDOR_ID   0x0955
 #define HP_VENDOR_ID   0x03f0
 #define MICROSOFT_VENDOR_ID0x045e
@@ -737,6 +738,15 @@ static const struct usb_device_id  products[] = {
.driver_info = 0,
 },
 
+#if IS_ENABLED(CONFIG_USB_RTL8152)
+/* Linksys USB3GIGV1 Ethernet Adapter */
+{
+   USB_DEVICE_AND_INTERFACE_INFO(LINKSYS_VENDOR_ID, 0x0041, USB_CLASS_COMM,
+   USB_CDC_SUBCLASS_ETHERNET, USB_CDC_PROTO_NONE),
+   .driver_info = 0,
+},
+#endif
+
 /* ThinkPad USB-C Dock (based on Realtek RTL8153) */
 {
USB_DEVICE_AND_INTERFACE_INFO(LENOVO_VENDOR_ID, 0x3062, USB_CLASS_COMM,
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index ceb78e2ea4f0..941ece08ba78 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -613,6 +613,7 @@ enum rtl8152_flags {
 #define VENDOR_ID_MICROSOFT0x045e
 #define VENDOR_ID_SAMSUNG  0x04e8
 #define VENDOR_ID_LENOVO   0x17ef
+#define VENDOR_ID_LINKSYS  0x13b1
 #define VENDOR_ID_NVIDIA   0x0955
 
 #define MCU_TYPE_PLA   0x0100
@@ -5316,6 +5317,7 @@ static const struct usb_device_id rtl8152_table[] = {
{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x7205)},
{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x720c)},
{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x7214)},
+   {REALTEK_USB_DEVICE(VENDOR_ID_LINKSYS, 0x0041)},
{REALTEK_USB_DEVICE(VENDOR_ID_NVIDIA,  0x09ff)},
{}
 };
-- 
2.14.2.822.g60be5d43e6-goog

--
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 V3] r8152: add Linksys USB3GIGV1 id

2017-09-27 Thread Grant Grundler
Hi Doug!

On Wed, Sep 27, 2017 at 4:47 PM, Doug Anderson  wrote:
> Hi,
>
> On Wed, Sep 27, 2017 at 10:28 AM, Grant Grundler  
> wrote:
>> This linksys dongle by default comes up in cdc_ether mode.
>> This patch allows r8152 to claim the device:
>>Bus 002 Device 002: ID 13b1:0041 Linksys
>>
>> Signed-off-by: Grant Grundler 
>> ---
>>  drivers/net/usb/cdc_ether.c | 10 ++
>>  drivers/net/usb/r8152.c |  2 ++
>>  2 files changed, 12 insertions(+)
>>
>> V3: for backwards compat, add #ifdef CONFIG_USB_RTL8152 around
>> the cdc_ether blacklist entry so the cdc_ether driver can
>> still claim the device if r8152 driver isn't configured.
>>
>> V2: add LINKSYS_VENDOR_ID to cdc_ether blacklist
>>
>> diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
>> index 8ab281b478f2..446dcc0f1f70 100644
>> --- a/drivers/net/usb/cdc_ether.c
>> +++ b/drivers/net/usb/cdc_ether.c
>> @@ -546,6 +546,7 @@ static const struct driver_info wwan_info = {
>>  #define DELL_VENDOR_ID 0x413C
>>  #define REALTEK_VENDOR_ID  0x0bda
>>  #define SAMSUNG_VENDOR_ID  0x04e8
>> +#define LINKSYS_VENDOR_ID  0x13b1
>>  #define LENOVO_VENDOR_ID   0x17ef
>
> Slight nit that "LI" sorts after "LE".  You got it right in the other case...

The list isn't sorted by any rational thing I can see.  I managed to
check my OCD reaction to sort the list numerically. :)

>>  #define NVIDIA_VENDOR_ID   0x0955
>>  #define HP_VENDOR_ID   0x03f0
>> @@ -737,6 +738,15 @@ static const struct usb_device_id  products[] = {
>> .driver_info = 0,
>>  },
>>
>> +#ifdef CONFIG_USB_RTL8152
>> +/* Linksys USB3GIGV1 Ethernet Adapter */
>> +{
>> +   USB_DEVICE_AND_INTERFACE_INFO(LINKSYS_VENDOR_ID, 0x0041, 
>> USB_CLASS_COMM,
>> +   USB_CDC_SUBCLASS_ETHERNET, USB_CDC_PROTO_NONE),
>> +   .driver_info = 0,
>> +},
>> +#endif
>
> I believe you want to use IS_ENABLED(), don't you?

Ah yes - I wasn't aware IS_ENABLED existed.  Will respin V4 with this
if there isn't any other feedback.


> There's still a weird esoteric side case where kernel modules don't
> all need to be included in the filesystem just because they were built
> at the same time.  ...but IMHO that seems like enough of a nit that we
> can probably ignore it unless someone has a better idea.

I think that would require a run time check. I'm perfectly willing to
ignore that case. :)

thanks!
grant

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


[PATCH V3] r8152: add Linksys USB3GIGV1 id

2017-09-27 Thread Grant Grundler
This linksys dongle by default comes up in cdc_ether mode.
This patch allows r8152 to claim the device:
   Bus 002 Device 002: ID 13b1:0041 Linksys

Signed-off-by: Grant Grundler 
---
 drivers/net/usb/cdc_ether.c | 10 ++
 drivers/net/usb/r8152.c |  2 ++
 2 files changed, 12 insertions(+)

V3: for backwards compat, add #ifdef CONFIG_USB_RTL8152 around
the cdc_ether blacklist entry so the cdc_ether driver can
still claim the device if r8152 driver isn't configured.

V2: add LINKSYS_VENDOR_ID to cdc_ether blacklist

diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index 8ab281b478f2..446dcc0f1f70 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -546,6 +546,7 @@ static const struct driver_info wwan_info = {
 #define DELL_VENDOR_ID 0x413C
 #define REALTEK_VENDOR_ID  0x0bda
 #define SAMSUNG_VENDOR_ID  0x04e8
+#define LINKSYS_VENDOR_ID  0x13b1
 #define LENOVO_VENDOR_ID   0x17ef
 #define NVIDIA_VENDOR_ID   0x0955
 #define HP_VENDOR_ID   0x03f0
@@ -737,6 +738,15 @@ static const struct usb_device_id  products[] = {
.driver_info = 0,
 },
 
+#ifdef CONFIG_USB_RTL8152
+/* Linksys USB3GIGV1 Ethernet Adapter */
+{
+   USB_DEVICE_AND_INTERFACE_INFO(LINKSYS_VENDOR_ID, 0x0041, USB_CLASS_COMM,
+   USB_CDC_SUBCLASS_ETHERNET, USB_CDC_PROTO_NONE),
+   .driver_info = 0,
+},
+#endif
+
 /* ThinkPad USB-C Dock (based on Realtek RTL8153) */
 {
USB_DEVICE_AND_INTERFACE_INFO(LENOVO_VENDOR_ID, 0x3062, USB_CLASS_COMM,
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index ceb78e2ea4f0..941ece08ba78 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -613,6 +613,7 @@ enum rtl8152_flags {
 #define VENDOR_ID_MICROSOFT0x045e
 #define VENDOR_ID_SAMSUNG  0x04e8
 #define VENDOR_ID_LENOVO   0x17ef
+#define VENDOR_ID_LINKSYS  0x13b1
 #define VENDOR_ID_NVIDIA   0x0955
 
 #define MCU_TYPE_PLA   0x0100
@@ -5316,6 +5317,7 @@ static const struct usb_device_id rtl8152_table[] = {
{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x7205)},
{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x720c)},
{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x7214)},
+   {REALTEK_USB_DEVICE(VENDOR_ID_LINKSYS, 0x0041)},
{REALTEK_USB_DEVICE(VENDOR_ID_NVIDIA,  0x09ff)},
{}
 };
-- 
2.14.2.822.g60be5d43e6-goog

--
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] r8152: add Linksys USB3GIGV1 id

2017-09-27 Thread Grant Grundler
On Wed, Sep 27, 2017 at 12:15 AM, Oliver Neukum  wrote:
> Am Dienstag, den 26.09.2017, 08:19 -0700 schrieb Doug Anderson:
>>
>> I know that for at least some of the adapters in the CDC Ethernet
>> blacklist it was claimed that the CDC Ethernet support in the adapter
>> was kinda broken anyway so the blacklist made sense.  ...but for the
>> Linksys Gigabit adapter the CDC Ethernet driver seems to work OK, it's
>> just not quite as full featured / efficient as the R8152 driver.
>>
>> Is that not a concern?  I guess you could tell people in this
>> situation that they simply need to enable the R8152 driver to get
>> continued support for their Ethernet adapter?
>
> Hi,
>
> yes, it is a valid concern. An #ifdef will be needed.

Good idea - I will post V3 shortly.

I'm assuming you mean to add #ifdef CONFIG_USB_RTL8152 around the
blacklist entry in cdc_ether driver.

cheers,
grant
--
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] r8152: add Linksys USB3GIGV1 id

2017-09-25 Thread Grant Grundler
On Mon, Sep 25, 2017 at 1:17 PM, Grant Grundler  wrote:
...
> I didn't realize cdc_ether has a blacklist to make sure
> RTL8152|RTL8153 devices are not picked up by cdc_ether. Would you
> prefer I add this device to the blacklist in the same patch?

I've sent a V2 which also updates the blacklist in cdc_ether.

cheers,
grant
--
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


[PATCH V2] r8152: add Linksys USB3GIGV1 id

2017-09-25 Thread Grant Grundler
This linksys dongle by default comes up in cdc_ether mode.
This patch allows r8152 to claim the device:
   Bus 002 Device 002: ID 13b1:0041 Linksys

Signed-off-by: Grant Grundler 
---
 drivers/net/usb/cdc_ether.c | 8 
 drivers/net/usb/r8152.c | 2 ++
 2 files changed, 10 insertions(+)

V2: add LINKSYS_VENDOR_ID to cdc_ether blacklist

diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index 8ab281b478f2..fa5c2e7aff1a 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -546,6 +546,7 @@ static const struct driver_info wwan_info = {
 #define DELL_VENDOR_ID 0x413C
 #define REALTEK_VENDOR_ID  0x0bda
 #define SAMSUNG_VENDOR_ID  0x04e8
+#define LINKSYS_VENDOR_ID  0x13b1
 #define LENOVO_VENDOR_ID   0x17ef
 #define NVIDIA_VENDOR_ID   0x0955
 #define HP_VENDOR_ID   0x03f0
@@ -737,6 +738,13 @@ static const struct usb_device_id  products[] = {
.driver_info = 0,
 },
 
+/* Linksys USB3GIGV1 Ethernet Adapter */
+{
+   USB_DEVICE_AND_INTERFACE_INFO(LINKSYS_VENDOR_ID, 0x0041, USB_CLASS_COMM,
+   USB_CDC_SUBCLASS_ETHERNET, USB_CDC_PROTO_NONE),
+   .driver_info = 0,
+},
+
 /* ThinkPad USB-C Dock (based on Realtek RTL8153) */
 {
USB_DEVICE_AND_INTERFACE_INFO(LENOVO_VENDOR_ID, 0x3062, USB_CLASS_COMM,
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index ceb78e2ea4f0..941ece08ba78 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -613,6 +613,7 @@ enum rtl8152_flags {
 #define VENDOR_ID_MICROSOFT0x045e
 #define VENDOR_ID_SAMSUNG  0x04e8
 #define VENDOR_ID_LENOVO   0x17ef
+#define VENDOR_ID_LINKSYS  0x13b1
 #define VENDOR_ID_NVIDIA   0x0955
 
 #define MCU_TYPE_PLA   0x0100
@@ -5316,6 +5317,7 @@ static const struct usb_device_id rtl8152_table[] = {
{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x7205)},
{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x720c)},
{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x7214)},
+   {REALTEK_USB_DEVICE(VENDOR_ID_LINKSYS, 0x0041)},
{REALTEK_USB_DEVICE(VENDOR_ID_NVIDIA,  0x09ff)},
{}
 };
-- 
2.14.1.821.g8fa685d3b7-goog

--
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] r8152: add Linksys USB3GIGV1 id

2017-09-25 Thread Grant Grundler
[grrhmail...sorry! resending as plain text]

Hallo Oliver!

On Mon, Sep 25, 2017 at 7:51 AM, Oliver Neukum  wrote:
> Am Freitag, den 22.09.2017, 12:06 -0700 schrieb Grant Grundler:
> > This Linksys dongle by default comes up in cdc_ether mode.
> > This patch allows r8152 to claim the device:
> >Bus 002 Device 002: ID 13b1:0041 Linksys
>
> Hi,
>
> have you tested this in case cdc_ether is for some reason already
> loaded?

I did not consider testing this case since it's not possible on a
normal chromeos system (the entire root file system is signed for
normal users and get's rebooted after an update).  I could test this
in developer mode of course.

Did you expect both driver probe routines to claim the device and
wreak havoc with the device?

> The patch seems to enable r8152 but does not disable cdc_ether.

Correct. r8152 happens to claim the device before cdc_ether does - I
thought because cdc_ether is a class driver and only gets picked up
after vendor specific drivers are probed.  Is that correct?

I didn't realize cdc_ether has a blacklist to make sure
RTL8152|RTL8153 devices are not picked up by cdc_ether. Would you
prefer I add this device to the blacklist in the same patch?

cheers,
grant

>
> Regards
> Oliver
>
--
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


[PATCH] r8152: add Linksys USB3GIGV1 id

2017-09-22 Thread Grant Grundler
This Linksys dongle by default comes up in cdc_ether mode.
This patch allows r8152 to claim the device:
   Bus 002 Device 002: ID 13b1:0041 Linksys

Signed-off-by: Grant Grundler 
---
 drivers/net/usb/r8152.c | 2 ++
 1 file changed, 2 insertions(+)

This was tested on chromeos-3.14, chromeos-3.18, and chromeos-4.4 kernels
with a mix of ARM/x86-64 systems.

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index ceb78e2ea4f0..941ece08ba78 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -613,6 +613,7 @@ enum rtl8152_flags {
 #define VENDOR_ID_MICROSOFT0x045e
 #define VENDOR_ID_SAMSUNG  0x04e8
 #define VENDOR_ID_LENOVO   0x17ef
+#define VENDOR_ID_LINKSYS  0x13b1
 #define VENDOR_ID_NVIDIA   0x0955
 
 #define MCU_TYPE_PLA   0x0100
@@ -5316,6 +5317,7 @@ static const struct usb_device_id rtl8152_table[] = {
{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x7205)},
{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x720c)},
{REALTEK_USB_DEVICE(VENDOR_ID_LENOVO,  0x7214)},
+   {REALTEK_USB_DEVICE(VENDOR_ID_LINKSYS, 0x0041)},
{REALTEK_USB_DEVICE(VENDOR_ID_NVIDIA,  0x09ff)},
{}
 };
-- 
2.14.1.821.g8fa685d3b7-goog

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


[PATCH] HID: remove use of DRIVER_LICENSE

2017-01-05 Thread Grant Grundler
Local "#define DRIVER_LICENSE" obfuscates which license is used
in MODULE_LICENSE().  "fgrep -R MODULE_LICENSE" is more informative
when the string is hard coded in MODULE_LICENSE.

Signed-off-by: Grant Grundler 
---
Most of the kernel already uses hard coded strings.
The few places that don't are easy to fix.

 drivers/hid/hid-core.c| 3 +--
 drivers/hid/usbhid/hid-core.c | 3 +--
 drivers/hid/usbhid/usbkbd.c   | 3 +--
 drivers/hid/usbhid/usbmouse.c | 3 +--
 drivers/hid/wacom.h   | 1 -
 drivers/hid/wacom_sys.c   | 2 +-
 6 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 08f53c7fd513..996b45dff76d 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -43,7 +43,6 @@
  */
 
 #define DRIVER_DESC "HID core driver"
-#define DRIVER_LICENSE "GPL"
 
 int hid_debug = 0;
 module_param_named(debug, hid_debug, int, 0600);
@@ -2848,5 +2847,5 @@ module_exit(hid_exit);
 MODULE_AUTHOR("Andreas Gal");
 MODULE_AUTHOR("Vojtech Pavlik");
 MODULE_AUTHOR("Jiri Kosina");
-MODULE_LICENSE(DRIVER_LICENSE);
+MODULE_LICENSE("GPL");
 
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index ae83af649a60..1847f64ac7dc 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -43,7 +43,6 @@
  */
 
 #define DRIVER_DESC "USB HID core driver"
-#define DRIVER_LICENSE "GPL"
 
 /*
  * Module parameters.
@@ -1660,4 +1659,4 @@ MODULE_AUTHOR("Andreas Gal");
 MODULE_AUTHOR("Vojtech Pavlik");
 MODULE_AUTHOR("Jiri Kosina");
 MODULE_DESCRIPTION(DRIVER_DESC);
-MODULE_LICENSE(DRIVER_LICENSE);
+MODULE_LICENSE("GPL");
diff --git a/drivers/hid/usbhid/usbkbd.c b/drivers/hid/usbhid/usbkbd.c
index 9a332e683db7..7fb2d1e4f5dd 100644
--- a/drivers/hid/usbhid/usbkbd.c
+++ b/drivers/hid/usbhid/usbkbd.c
@@ -39,11 +39,10 @@
 #define DRIVER_VERSION ""
 #define DRIVER_AUTHOR "Vojtech Pavlik "
 #define DRIVER_DESC "USB HID Boot Protocol keyboard driver"
-#define DRIVER_LICENSE "GPL"
 
 MODULE_AUTHOR(DRIVER_AUTHOR);
 MODULE_DESCRIPTION(DRIVER_DESC);
-MODULE_LICENSE(DRIVER_LICENSE);
+MODULE_LICENSE("GPL");
 
 static const unsigned char usb_kbd_keycode[256] = {
  0,  0,  0,  0, 30, 48, 46, 32, 18, 33, 34, 35, 23, 36, 37, 38,
diff --git a/drivers/hid/usbhid/usbmouse.c b/drivers/hid/usbhid/usbmouse.c
index bf16d72dc370..dd911c5241d8 100644
--- a/drivers/hid/usbhid/usbmouse.c
+++ b/drivers/hid/usbhid/usbmouse.c
@@ -42,11 +42,10 @@
 #define DRIVER_VERSION "v1.6"
 #define DRIVER_AUTHOR "Vojtech Pavlik "
 #define DRIVER_DESC "USB HID Boot Protocol mouse driver"
-#define DRIVER_LICENSE "GPL"
 
 MODULE_AUTHOR(DRIVER_AUTHOR);
 MODULE_DESCRIPTION(DRIVER_DESC);
-MODULE_LICENSE(DRIVER_LICENSE);
+MODULE_LICENSE("GPL");
 
 struct usb_mouse {
char name[128];
diff --git a/drivers/hid/wacom.h b/drivers/hid/wacom.h
index 4681a65a4579..eb24bf823252 100644
--- a/drivers/hid/wacom.h
+++ b/drivers/hid/wacom.h
@@ -100,7 +100,6 @@
 #define DRIVER_VERSION "v2.00"
 #define DRIVER_AUTHOR "Vojtech Pavlik "
 #define DRIVER_DESC "USB Wacom tablet driver"
-#define DRIVER_LICENSE "GPL"
 
 #define USB_VENDOR_ID_WACOM0x056a
 #define USB_VENDOR_ID_LENOVO   0x17ef
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 499cc8213cfe..f79627df9597 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1959,4 +1959,4 @@ module_hid_driver(wacom_driver);
 MODULE_VERSION(DRIVER_VERSION);
 MODULE_AUTHOR(DRIVER_AUTHOR);
 MODULE_DESCRIPTION(DRIVER_DESC);
-MODULE_LICENSE(DRIVER_LICENSE);
+MODULE_LICENSE("GPL");
-- 
2.11.0.390.gc69c2f50cf-goog

--
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 v3 5/5] net: asix: autoneg will set WRITE_MEDIUM reg

2016-09-06 Thread Grant Grundler
On Thu, Sep 1, 2016 at 10:02 AM, Eric Dumazet  wrote:
> On Thu, 2016-09-01 at 12:47 -0400, Robert Foss wrote:
>
>> I'm not quite sure how the first From line was added, it
>> should not have been.
>> Grant Grundler is most definitely the author.
>>
>> Would you like me to resubmit in v++ and make sure that it has been
>> corrected?
>
> This is too late, patches are already merged in David Miller net-next
> tree.
>
> These kinds of errors can not be fixed, we have to be very careful at
> submission/review time.
>
> I guess Grant does not care, but some contributors, especially new ones
> would like to get proper attribution.

I do not mind. Robert will get email about bugs instead of me. :D

cheers,
grant

>
> Thanks.
>
>
--
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 1/3] net: asix: Add in_pm parameter

2016-07-26 Thread Grant Grundler
On Tue, Jul 26, 2016 at 2:14 PM, Robert Foss  wrote:
...
> Thanks for the feedback (for this patch and the other ones)!
> I'm preparing a v2 and will submit it withing a day or two.

Excellent! very welcome and thanks again for picking this up.

...
>> FTR, current  drivers/net/usb/ax88179_178a.c uses "in_pm" as well.
>
> Ah! Would you like that to be noted in the commit message?

No - I generally do not muck with commit messages that I didn't write.
It's part of the "finger print" of a patch (so one can trace origin
across distro's and kernel versions).

My comment was intended for reviewers and could be included in the
"comments after the commit message". (ie below the "---" marker in the
patch email).

...
>> BTW, I have two additional changes for AX88772x support sitting in my
>> "needs more work" queue (for quite a while already):
>>https://chromium-review.googlesource.com/#/c/229620/
>>"asix: autoneg will set WRITE_MEDIUM reg"
>>
>>https://chromium-review.googlesource.com/#/c/231162/
>>"net: asix: see 802.3 spec for phy reset"
>>
>> I would certainly approve if _anyone_ picked these up, tested them,
>> and then submitted them to netdev.
>
> Unfortunately I am without appropriate hardware at the moment.

That's a risky thing to point out for two reasons:

1) I can send you more HW than you could possibly ever use. :)

2) Reviewers of this patch series are already wondering how you
verified the AX88772x patches work on current kernel.org (or
netdev-next) branches.

Please email me with your shipping address (OFFLIST!) and I'll try to
send you some Asix HW to test with.  I'm located in "Silicon Valley"
(USA) and can easily ship in the US. Other countries is a bit harder
and in some cases not possible.

cheers,
grant
--
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 3/3] net: asix: Fix AX88772x resume failures

2016-07-25 Thread Grant Grundler
Robert,
This patch content LGTM but isn't including the correct "meta data"
regarding it's origin.

On Mon, Jul 25, 2016 at 10:40 AM,   wrote:
> From: WK Tsai 

WK Tsai is not the author of this patch.
I believe Allan Chou  is the author.

> The change fixes AX88772x resume failure by
> - Restore incorrect AX88772A PHY registers when resetting
> - Need to stop MAC operation when suspending
> - Need to restart MII when restoring PHY
>
> Signed-off-by: WK Tsai 

This patch was applied to four different chromium.org kernel branches:
   https://chromium-review.googlesource.com/#/q/Fix+AX88772x+resume+failures

And has substantial meta data regarding author and reviewers:
  (summarized from https://chromium-review.googlesource.com/#/c/314520/)

Signed-off-by: Allan Chou 
Signed-off-by: WK Tsai 
Tested-by: Grant Grundler 
Reviewed-by: Wang-Kai Tsai 
Reviewed-by: Mark Kuo 
Reviewed-by: Grant Grundler 
Reviewed-by: Allan Chou 
Reviewed-by: Vincent Palatin 

cheers,
grant

> ---
>  drivers/net/usb/asix_devices.c | 47 
> +-
>  1 file changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
> index ebeb730..083dc2e 100644
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -35,6 +35,15 @@
>
>  #definePHY_MODE_RTL8211CL  0x000C
>
> +#define AX88772A_PHY14H0x14
> +#define AX88772A_PHY14H_DEFAULT 0x442C
> +
> +#define AX88772A_PHY15H0x15
> +#define AX88772A_PHY15H_DEFAULT 0x03C8
> +
> +#define AX88772A_PHY16H0x16
> +#define AX88772A_PHY16H_DEFAULT 0x4044
> +
>  struct ax88172_int_data {
> __le16 res1;
> u8 link;
> @@ -424,7 +433,7 @@ static int ax88772a_hw_reset(struct usbnet *dev, int 
> in_pm)
>  {
> struct asix_data *data = (struct asix_data *)&dev->data;
> int ret, embd_phy;
> -   u16 rx_ctl;
> +   u16 rx_ctl, phy14h, phy15h, phy16h;
> u8 chipcode = 0;
>
> ret = asix_write_gpio(dev, AX_GPIO_RSE, 5, in_pm);
> @@ -482,6 +491,32 @@ static int ax88772a_hw_reset(struct usbnet *dev, int 
> in_pm)
>ret);
> goto out;
> }
> +   } else if ((chipcode & AX_CHIPCODE_MASK) == AX_AX88772A_CHIPCODE) {
> +   /* Check if the PHY registers have default settings */
> +   phy14h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id,
> +AX88772A_PHY14H);
> +   phy15h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id,
> +AX88772A_PHY15H);
> +   phy16h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id,
> +AX88772A_PHY16H);
> +
> +   netdev_dbg(dev->net,
> +  "772a_hw_reset: MR20=0x%x MR21=0x%x MR22=0x%x\n",
> +  phy14h, phy15h, phy16h);
> +
> +   /* Restore PHY registers default setting if not */
> +   if (phy14h != AX88772A_PHY14H_DEFAULT)
> +   asix_mdio_write_nopm(dev->net, dev->mii.phy_id,
> +AX88772A_PHY14H,
> +AX88772A_PHY14H_DEFAULT);
> +   if (phy15h != AX88772A_PHY15H_DEFAULT)
> +   asix_mdio_write_nopm(dev->net, dev->mii.phy_id,
> +AX88772A_PHY15H,
> +AX88772A_PHY15H_DEFAULT);
> +   if (phy16h != AX88772A_PHY16H_DEFAULT)
> +   asix_mdio_write_nopm(dev->net, dev->mii.phy_id,
> +AX88772A_PHY16H,
> +AX88772A_PHY16H_DEFAULT);
> }
>
> ret = asix_write_cmd(dev, AX_CMD_WRITE_IPG0,
> @@ -543,6 +578,15 @@ static const struct net_device_ops ax88772_netdev_ops = {
>  static void ax88772_suspend(struct usbnet *dev)
>  {
> struct asix_common_private *priv = dev->driver_priv;
> +   u16 medium;
> +
> +   /* Stop MAC operation */
> +   medium = asix_read_medium_status(dev, 0);
> +   medium &= ~AX_MEDIUM_RE;
> +   asix_write_medium_mode(dev, medium, 0);
> +
> +   netdev_dbg(dev->net, "ax88772_suspend: medium=0x%04x\n",
> +  asix_read_medium_status(dev, 0));
>
> /* Preserve BMCR for restoring */
> priv->presvd_phy_bmcr =
> @@ -577,6 +621,7 @@ static void ax88772_restore

Re: [PATCH 2/3] net: asix: Avoid looping when the device is disconnected

2016-07-25 Thread Grant Grundler
On Mon, Jul 25, 2016 at 10:40 AM,   wrote:
> From: Vincent Palatin 
>
> Check the answers from the USB stack and avoid re-sending multiple times
> the request if the device has disappeared.
>
> Signed-off-by: Vincent Palatin 

The original chromium.org code review:
https://chromium-review.googlesource.com/255931

resulted in some additional commit message meta-data that would be
nice to include in the kernel.org commit message:
Reviewed-by: Julius Werner 
Reviewed-by: Douglas Anderson 
Tested-by: Douglas Anderson 

And yes, patch LGTM too. :)

cheers,
grant

> ---
>  drivers/net/usb/asix_common.c  | 56 
> +-
>  drivers/net/usb/asix_devices.c |  2 ++
>  2 files changed, 46 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
> index f0ccf76..0339560 100644
> --- a/drivers/net/usb/asix_common.c
> +++ b/drivers/net/usb/asix_common.c
> @@ -428,13 +428,21 @@ int asix_mdio_read(struct net_device *netdev, int 
> phy_id, int loc)
> __le16 res;
> u8 smsr;
> int i = 0;
> +   int ret;
>
> mutex_lock(&dev->phy_mutex);
> do {
> -   asix_set_sw_mii(dev, 0);
> +   ret = asix_set_sw_mii(dev, 0);
> +   if (ret == -ENODEV)
> +   break;
> usleep_range(1000, 1100);
> -   asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG, 0, 0, 1, &smsr, 0);
> -   } while (!(smsr & AX_HOST_EN) && (i++ < 30));
> +   ret = asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG,
> +   0, 0, 1, &smsr, 0);
> +   } while (!(smsr & AX_HOST_EN) && (i++ < 30) && (ret != -ENODEV));
> +   if (ret == -ENODEV) {
> +   mutex_unlock(&dev->phy_mutex);
> +   return ret;
> +   }
>
> asix_read_cmd(dev, AX_CMD_READ_MII_REG, phy_id,
> (__u16)loc, 2, &res, 0);
> @@ -453,16 +461,24 @@ void asix_mdio_write(struct net_device *netdev, int 
> phy_id, int loc, int val)
> __le16 res = cpu_to_le16(val);
> u8 smsr;
> int i = 0;
> +   int ret;
>
> netdev_dbg(dev->net, "asix_mdio_write() phy_id=0x%02x, loc=0x%02x, 
> val=0x%04x\n",
> phy_id, loc, val);
>
> mutex_lock(&dev->phy_mutex);
> do {
> -   asix_set_sw_mii(dev, 0);
> +   ret = asix_set_sw_mii(dev, 0);
> +   if (ret == -ENODEV)
> +   break;
> usleep_range(1000, 1100);
> -   asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG, 0, 0, 1, &smsr, 0);
> -   } while (!(smsr & AX_HOST_EN) && (i++ < 30));
> +   ret = asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG,
> +   0, 0, 1, &smsr, 0);
> +   } while (!(smsr & AX_HOST_EN) && (i++ < 30) && (ret != -ENODEV));
> +   if (ret == -ENODEV) {
> +   mutex_unlock(&dev->phy_mutex);
> +   return;
> +   }
>
> asix_write_cmd(dev, AX_CMD_WRITE_MII_REG, phy_id,
>(__u16)loc, 2, &res, 0);
> @@ -476,13 +492,21 @@ int asix_mdio_read_nopm(struct net_device *netdev, int 
> phy_id, int loc)
> __le16 res;
> u8 smsr;
> int i = 0;
> +   int ret;
>
> mutex_lock(&dev->phy_mutex);
> do {
> -   asix_set_sw_mii(dev, 1);
> +   ret = asix_set_sw_mii(dev, 1);
> +   if (ret == -ENODEV)
> +   break;
> usleep_range(1000, 1100);
> -   asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG, 0, 0, 1, &smsr, 1);
> -   } while (!(smsr & AX_HOST_EN) && (i++ < 30));
> +   ret = asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG,
> +   0, 0, 1, &smsr, 1);
> +   } while (!(smsr & AX_HOST_EN) && (i++ < 30) && (ret != -ENODEV));
> +   if (ret == -ENODEV) {
> +   mutex_unlock(&dev->phy_mutex);
> +   return ret;
> +   }
>
> asix_read_cmd(dev, AX_CMD_READ_MII_REG, phy_id,
>   (__u16)loc, 2, &res, 1);
> @@ -502,16 +526,24 @@ asix_mdio_write_nopm(struct net_device *netdev, int 
> phy_id, int loc, int val)
> __le16 res = cpu_to_le16(val);
> u8 smsr;
> int i = 0;
> +   int ret;
>
> netdev_dbg(dev->net, "asix_mdio_write() phy_id=0x%02x, loc=0x%

Re: [PATCH 1/3] net: asix: Add in_pm parameter

2016-07-25 Thread Grant Grundler
[as plain text this time...]

Robert,

On Mon, Jul 25, 2016 at 10:40 AM,   wrote:
> From: Grant Grundler 

For the record, I believe I am not the author of these patches.

I believe the original author is
Signed-off-by: Freddy Xin 

as recorded in the following code reviews (and testing) that I was
responsible for:

https://chromium-review.googlesource.com/#/q/owner:%22Grant+Grundler%22+status:merged+asix+in_pm

And I would certainly be happy to see this code go upstream and
expected ASIX would submit this change upstream.

> In order to R/W registers in suspend/resume functions, in_pm flags are
> added to some functions to determine whether the nopm version of usb
> functions is called.

FTR, current  drivers/net/usb/ax88179_178a.c uses "in_pm" as well.

> Save BMCR and ANAR PHY registers in suspend function and restore them
> in resume function.
>
> Reset HW in resume function to ensure the PHY works correctly.
>
> Signed-off-by: Grant Grundler 

BTW, I have two additional changes for AX88772x support sitting in my
"needs more work" queue (for quite a while already):
   https://chromium-review.googlesource.com/#/c/229620/
   "asix: autoneg will set WRITE_MEDIUM reg"

   https://chromium-review.googlesource.com/#/c/231162/
   "net: asix: see 802.3 spec for phy reset"

I would certainly approve if _anyone_ picked these up, tested them,
and then submitted them to netdev.

cheers,
grant

> ---
>  drivers/net/usb/asix.h |  40 +++--
>  drivers/net/usb/asix_common.c  | 180 +++-
>  drivers/net/usb/asix_devices.c | 373 
> -
>  drivers/net/usb/ax88172a.c |  29 ++--
>  4 files changed, 472 insertions(+), 150 deletions(-)
>
> diff --git a/drivers/net/usb/asix.h b/drivers/net/usb/asix.h
> index a2d3ea6..d109242 100644
> --- a/drivers/net/usb/asix.h
> +++ b/drivers/net/usb/asix.h
> @@ -46,6 +46,7 @@
>  #define AX_CMD_SET_SW_MII  0x06
>  #define AX_CMD_READ_MII_REG0x07
>  #define AX_CMD_WRITE_MII_REG   0x08
> +#define AX_CMD_STATMNGSTS_REG  0x09
>  #define AX_CMD_SET_HW_MII  0x0a
>  #define AX_CMD_READ_EEPROM 0x0b
>  #define AX_CMD_WRITE_EEPROM0x0c
> @@ -71,6 +72,17 @@
>  #define AX_CMD_SW_RESET0x20
>  #define AX_CMD_SW_PHY_STATUS   0x21
>  #define AX_CMD_SW_PHY_SELECT   0x22
> +#define AX_QCTCTRL 0x2A
> +
> +#define AX_CHIPCODE_MASK   0x70
> +#define AX_AX88772_CHIPCODE0x00
> +#define AX_AX88772A_CHIPCODE   0x10
> +#define AX_AX88772B_CHIPCODE   0x20
> +#define AX_HOST_EN 0x01
> +
> +#define AX_PHYSEL_PSEL 0x01
> +#define AX_PHYSEL_SSMII0
> +#define AX_PHYSEL_SSEN 0x10
>
>  #define AX_PHY_SELECT_MASK (BIT(3) | BIT(2))
>  #define AX_PHY_SELECT_INTERNAL 0
> @@ -173,6 +185,10 @@ struct asix_rx_fixup_info {
>  };
>
>  struct asix_common_private {
> +   void (*resume)(struct usbnet *dev);
> +   void (*suspend)(struct usbnet *dev);
> +   u16 presvd_phy_advertise;
> +   u16 presvd_phy_bmcr;
> struct asix_rx_fixup_info rx_fixup_info;
>  };
>
> @@ -182,10 +198,10 @@ extern const struct driver_info ax88172a_info;
>  #define FLAG_EEPROM_MAC(1UL << 0)  /* init device MAC from 
> eeprom */
>
>  int asix_read_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
> - u16 size, void *data);
> + u16 size, void *data, int in_pm);
>
>  int asix_write_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
> -  u16 size, void *data);
> +  u16 size, void *data, int in_pm);
>
>  void asix_write_cmd_async(struct usbnet *dev, u8 cmd, u16 value,
>   u16 index, u16 size, void *data);
> @@ -197,27 +213,31 @@ int asix_rx_fixup_common(struct usbnet *dev, struct 
> sk_buff *skb);
>  struct sk_buff *asix_tx_fixup(struct usbnet *dev, struct sk_buff *skb,
>   gfp_t flags);
>
> -int asix_set_sw_mii(struct usbnet *dev);
> -int asix_set_hw_mii(struct usbnet *dev);
> +int asix_set_sw_mii(struct usbnet *dev, int in_pm);
> +int asix_set_hw_mii(struct usbnet *dev, int in_pm);
>
>  int asix_read_phy_addr(struct usbnet *dev, int internal);
>  int asix_get_phy_addr(struct usbnet *dev);
>
> -int asix_sw_reset(struct usbnet *dev, u8 flags);
> +int asix_sw_reset(struct usbnet *dev, u8 flags, int in_pm);
>
> -u16 asix_read_rx_ctl(struct usbnet *dev);
> -int asix_write_rx_ctl(struct usbnet *dev, u16 mode);
> +u16 asix_read_rx_ctl(struct usbnet *dev, int 

Re: [PATCH] r8152: add MODULE_VERSION

2016-07-15 Thread Grant Grundler
On Fri, Jul 15, 2016 at 2:25 PM, David Miller  wrote:
> From: Grant Grundler 
> Date: Thu, 14 Jul 2016 11:27:16 -0700
>
>> ethtool -i provides a driver version that is hard coded.
>> Export the same value via "modinfo".
>>
>> Signed-off-by: Grant Grundler 
>
> Applied.

Excellent - thank you. :)

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


[PATCH] r8152: add MODULE_VERSION

2016-07-14 Thread Grant Grundler
ethtool -i provides a driver version that is hard coded.
Export the same value via "modinfo".

Signed-off-by: Grant Grundler 
---
 drivers/net/usb/r8152.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 0da72d3..1c01ed5 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -4359,3 +4359,4 @@ module_usb_driver(rtl8152_driver);
 MODULE_AUTHOR(DRIVER_AUTHOR);
 MODULE_DESCRIPTION(DRIVER_DESC);
 MODULE_LICENSE("GPL");
+MODULE_VERSION(DRIVER_VERSION);
-- 
2.8.0.rc3.226.g39d4020

--
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 0/2] FTDI CBUS GPIO support

2015-08-23 Thread Grant Likely
On Sun, 21 Jun 2015 21:44:30 +0200
, Stefan Agner 
 wrote:
> On 2015-06-21 01:49, Peter Stuge wrote:
> > Stefan Agner wrote:
> >> libftdi requires to detach the kernel driver to get access to the device
> > 
> > Control transfers ought to be possible without a detach.
> 
> Good to know, thanks for this input. The detach is probably a default
> behavior of libftdi... Will have a look at that.

libftdi is built on libusb. libusb detaches the device from ftdi_sio and
binds it to a special driver for userspace access. There may indeed be
another way to do it, but libusb doesn't know how to do it AFAIKS.

We can't avoid the detach on current libftdi, but I do have a patch to
libftdi that will at least reconnect the ftdi_sio driver after userspace
has finished. I'll be posting it to the libftdi list in the next week,
but I've attached it below for anyone who wants to play.

> Having kernel level gpiolib would still have advantages: It would make
> the matching of the GPIO's and tty device easier, since with this patch
> the gpiolib device is a sub-node of the usb-serial device in sysfs and
> the user would have to use kernel interfaces only (no libftdi)...

NAK on the sysfs interface. Don't add a new ABI. gpiolib is the right
way to expose these pins.

g.

---
Subject: [PATCH] Automatically reattach kernel driver on close

The built-in libusb auto detach feature supports automatically
reattaching the kernel driver when the device is closed. Switch libftdi
to use the libusb auto-detach code so that we can get the Linux ftdi_sio
driver reattached automatically when finished with the device.
---
 src/ftdi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/ftdi.c b/src/ftdi.c
index 68489ea096be..0d043690aeca 100644
--- a/src/ftdi.c
+++ b/src/ftdi.c
@@ -548,7 +548,7 @@ int ftdi_usb_open_dev(struct ftdi_context *ftdi, 
libusb_device *dev)
 // Likely scenario is a static ftdi_sio kernel module.
 if (ftdi->module_detach_mode == AUTO_DETACH_SIO_MODULE)
 {
-if (libusb_detach_kernel_driver(ftdi->usb_dev, ftdi->interface) !=0)
+if (libusb_set_auto_detach_kernel_driver(ftdi->usb_dev, 1))
 detach_errno = errno;
 }
 
-- 
2.1.4

--
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] USB: ftdi_sio: add GPIO support

2015-07-03 Thread Grant Likely
On Tue, Jun 2, 2015 at 2:18 PM, Linus Walleij  wrote:
> On Sat, May 30, 2015 at 10:29 PM, Grant Likely
>  wrote:
>> On Mon, Jul 7, 2014 at 6:31 PM, Greg Kroah-Hartman
>>  wrote:
>
>>>> However is the MFD cell approach acceptable?
>>>
>>> Yes it is.
>>
>> Going back to this old conversation... Actually, I disagree. There is
>> absolutely no need to go the MFD approach for this driver. That just
>> adds layers of abstraction for no purpose. GPIOLIB is an interface,
>> and it is completely fine for a driver to hook up to the GPIOLIB
>> interface at the same time as exposing a serial port. MFD doesn't buy
>> the driver anything useful here.
>
> What is buys is centralizing code into the proper drivers/gpio
> folder of the kernel. So more of a maintenance point than a
> mechanics/performance point.
>
> We do have GPIO drivers scattered all over the kernel so one
> more or less wouldn't matter so much...

Yeah, I would say that's a non-reason. When it comes to a single
device, it is far better in my opinion to have the entire driver
located together rather than splitting it up into parts so that each
part lives with it's subsystem. We've got tools for find users of
interfaces, whereas spliting a driver up can make maintenance a lot
more complicated.

g.
--
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 v8 4/9] mfd: Add binding document for NVIDIA Tegra XUSB

2015-06-30 Thread Grant Likely
On Tue, 26 May 2015 16:18:54 +0100
, Lee Jones 
 wrote:
> On Thu, 21 May 2015, Thierry Reding wrote:
> 
> > On Thu, May 21, 2015 at 09:40:01AM +0100, Lee Jones wrote:
> > > On Wed, 20 May 2015, Thierry Reding wrote:
> > > > On Wed, May 20, 2015 at 07:35:51AM +0100, Lee Jones wrote:
> > > > > On Tue, 19 May 2015, Andrew Bresticker wrote:
> > > > > > On Thu, May 14, 2015 at 10:38 AM, Andrew Bresticker
> > > > > >  wrote:
> > > > > > > On Thu, May 14, 2015 at 12:40 AM, Lee Jones 
> > > > > > >  wrote:
> > > > > > >> On Thu, 14 May 2015, Jon Hunter wrote:
> > > > > > >>> On 13/05/15 15:39, Lee Jones wrote:
> > > > > > >>> > On Mon, 04 May 2015, Andrew Bresticker wrote:
> > > > > > >>> >
> > > > > > >>> >> Add a binding document for the XUSB host complex on NVIDIA 
> > > > > > >>> >> Tegra124
> > > > > > >>> >> and later SoCs.  The XUSB host complex includes a mailbox for
> > > > > > >>> >> communication with the XUSB micro-controller and an xHCI 
> > > > > > >>> >> host-controller.
> > > > > > >>> >>
> > > > > > >>> >> Signed-off-by: Andrew Bresticker 
> > > > > > >>> >> Cc: Rob Herring 
> > > > > > >>> >> Cc: Pawel Moll 
> > > > > > >>> >> Cc: Mark Rutland 
> > > > > > >>> >> Cc: Ian Campbell 
> > > > > > >>> >> Cc: Kumar Gala 
> > > > > > >>> >> Cc: Samuel Ortiz 
> > > > > > >>> >> Cc: Lee Jones 
> > > > > > >>> >> ---
> > > > > > >>> >> Changes from v7:
> > > > > > >>> >>  - Move non-shared resources into child nodes.
> > > > > > >>> >> New for v7.
> > > > > > >>> >> ---
> > > > > > >>> >>  .../bindings/mfd/nvidia,tegra124-xusb.txt  | 37 
> > > > > > >>> >> ++
> > > > > > >>> >>  1 file changed, 37 insertions(+)
> > > > > > >>> >>  create mode 100644 
> > > > > > >>> >> Documentation/devicetree/bindings/mfd/nvidia,tegra124-xusb.txt
> > > > > > >>> >>
> > > > > > >>> >> diff --git 
> > > > > > >>> >> a/Documentation/devicetree/bindings/mfd/nvidia,tegra124-xusb.txt
> > > > > > >>> >>  
> > > > > > >>> >> b/Documentation/devicetree/bindings/mfd/nvidia,tegra124-xusb.txt
> > > > > > >>> >> new file mode 100644
> > > > > > >>> >> index 000..bc50110
> > > > > > >>> >> --- /dev/null
> > > > > > >>> >> +++ 
> > > > > > >>> >> b/Documentation/devicetree/bindings/mfd/nvidia,tegra124-xusb.txt
> > > > > > >>> >> @@ -0,0 +1,37 @@
> > > > > > >>> >> +NVIDIA Tegra XUSB host copmlex
> > > > > > >>> >> +==
> > > > > > >>> >> +
> > > > > > >>> >> +The XUSB host complex on Tegra124 and later SoCs contains 
> > > > > > >>> >> an xHCI host
> > > > > > >>> >> +controller and a mailbox for communication with the XUSB 
> > > > > > >>> >> micro-controller.
> > > > > > >>> >> +
> > > > > > >>> >> +Required properties:
> > > > > > >>> >> +
> > > > > > >>> >> + - compatible: For Tegra124, must contain 
> > > > > > >>> >> "nvidia,tegra124-xusb".
> > > > > > >>> >> +   Otherwise, must contain '"nvidia,-xusb", 
> > > > > > >>> >> "nvidia,tegra124-xusb"'
> > > > > > >>> >> +   where  is tegra132.
> > > > > > >>> >> + - reg: Must contain the base and length of the XUSB FPCI 
> > > > > > >>> >> registers.
> > > > > > >>> >> + - ranges: Bus address mapping for the XUSB block.  Can be 
> > > > > > >>> >> empty since the
> > > > > > >>> >> +   mapping is 1:1.
> > > > > > >>> >> + - #address-cells: Must be 2.
> > > > > > >>> >> + - #size-cells: Must be 2.
> > > > > > >>> >> +
> > > > > > >>> >> +Example:
> > > > > > >>> >> +
> > > > > > >>> >> +  usb@0,70098000 {
> > > > > > >>> >> +  compatible = "nvidia,tegra124-xusb";
> > > > > > >>> >> +  reg = <0x0 0x70098000 0x0 0x1000>;
> > > > > > >>> >> +  ranges;
> > > > > > >>> >> +
> > > > > > >>> >> +  #address-cells = <2>;
> > > > > > >>> >> +  #size-cells = <2>;
> > > > > > >>> >> +
> > > > > > >>> >> +  usb-host@0,7009 {
> > > > > > >>> >> +  compatible = "nvidia,tegra124-xhci";
> > > > > > >>> >> +  ...
> > > > > > >>> >> +  };
> > > > > > >>> >> +
> > > > > > >>> >> +  mailbox {
> > > > > > >>> >> +  compatible = "nvidia,tegra124-xusb-mbox";
> > > > > > >>> >> +  ...
> > > > > > >>> >> +  };
> > > > > > >>> >
> > > > > > >>> > This doesn't appear to be a proper MFD.  I would have the USB 
> > > > > > >>> > and
> > > > > > >>> > Mailbox devices probe seperately and use a phandle to point 
> > > > > > >>> > the USB
> > > > > > >>> > device to its Mailbox.
> > > > > > >>> >
> > > > > > >>> > usb@xyz {
> > > > > > >>> > mboxes = <&xusb-mailbox, [chan]>;
> > > > > > >>> > };
> > > > > > >>> >
> > > > > > >>>
> > > > > > >>> I am assuming that Andrew had laid it out like this to reflect 
> > > > > > >>> the hw
> > > > > > >>> structure. The mailbox and xhci controller are part of the xusb
> > > > > > >>> sub-system and hence appear as child nodes. My understanding is 
> > > > > > >>> that for
> > > > > > >>> device-tree we want the device-tree structure to reflect the 
> >

Re: [PATCH] USB: ftdi_sio: add GPIO support

2015-05-30 Thread Grant Likely
On Mon, Jul 7, 2014 at 6:31 PM, Greg Kroah-Hartman
 wrote:
> On Mon, Jul 07, 2014 at 12:44:28PM +0200, Linus Walleij wrote:
>> On Fri, Jun 13, 2014 at 8:31 PM, Greg Kroah-Hartman
>>  wrote:
>> > On Fri, Jun 13, 2014 at 09:25:07AM +0200, Linus Walleij wrote:
>>
>> >> But I also want to bring the device model into question: normally
>> >> when a mother device spawns children across different subsystems
>> >> we model them as MFD devices (drivers/mfd) that instantiate
>> >> children for the different subsystems. So you could spawn a
>> >> serial and a GPIO device from a USB-based hub device there.
>> >>
>> >> I do not know if that is really apropriate in this case. It seems the
>> >> device is first and foremost FTDI.
>> >>
>> >> But it could still spawn a child platform device for the GPIO stuff
>> >> so that this can live as a separate driver under drivers/gpio/gpio-ftdi.c
>> >> or similar.
>> >>
>> >> You could then use something like:
>> >>
>> >> struct platform_device *gdev;
>> >
>> > Ick, no, it's a USB device, do not abuse the platform_device code any
>> > more than it currently is (note, I HATE the platform device code,
>> > someday I'll delete it entirely...  Well, I can dream...)
>>
>> Haha yeah :-)
>>
>> However is the MFD cell approach acceptable?
>
> Yes it is.

Going back to this old conversation... Actually, I disagree. There is
absolutely no need to go the MFD approach for this driver. That just
adds layers of abstraction for no purpose. GPIOLIB is an interface,
and it is completely fine for a driver to hook up to the GPIOLIB
interface at the same time as exposing a serial port. MFD doesn't buy
the driver anything useful here.

g.
--
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: Mouse works with eHCI, fails with xHCI - can't set config #1, error -110

2015-04-20 Thread Alistair Grant
On Tue, Apr 14, 2015 at 10:52:21AM -0400, Alan Stern wrote:
> On Tue, 14 Apr 2015, Alistair Grant wrote:
> 
> > Hi Mathias and Alan,
> > 
> > As Mathias requested, I've included the usbmon output with the patch
> > applied.
> > 
> > It didn't make any difference to the end result, the mouse still fails
> > to initialise correctly (no real surprise, I think), but is getting the
> > same string that Alan reported earlier ("Laser 0").  I've also included
> > the relevant lines from syslog below. (I also have a pcap file
> > captured with tshark if anyone is interested).
> > 
> > As always, thanks for your help,
> > Alistair
> > 
> > usbmon output:
> ...
> > 880024c51b40 591168422 S Ci:1:006:0 s 80 06 0302 0409 00ff 255 <
> > 880024c51b40 591169070 C Ci:1:006:0 -32 15 = 18034c00 61007300 65007200 
> > 200030
> > 880024c51b40 591169098 S Ci:1:006:0 s 80 06 0302 0409 00ff 255 <
> > 880024c51b40 591169854 C Ci:1:006:0 -32 15 = 18034c00 61007300 65007200 
> > 200030
> > 880024c51b40 591169884 S Ci:1:006:0 s 80 06 0302 0409 00ff 255 <
> > 880024c51b40 591170684 C Ci:1:006:0 -32 15 = 18034c00 61007300 65007200 
> > 200030
> > 880024c51b40 591170702 S Ci:1:006:0 s 80 06 0302 0409 0002 2 <
> > 880024c51b40 591171025 C Ci:1:006:0 0 2 = 1803
> > 880024c51b40 591171037 S Ci:1:006:0 s 80 06 0302 0409 0018 24 <
> > 880024c51b40 591171668 C Ci:1:006:0 -32 15 = 18034c00 61007300 65007200 
> > 200030
> > 880024c51b40 591171679 S Ci:1:006:0 s 80 06 0302 0409 0018 24 <
> > 880024c51b40 591172455 C Ci:1:006:0 -32 15 = 18034c00 61007300 65007200 
> > 200030
> > 880024c51b40 591172466 S Ci:1:006:0 s 80 06 0302 0409 0018 24 <
> > 880024c51b40 596169148 C Ci:1:006:0 -2 0
> 
> This is practically identical to the EHCI trace from before, right up 
> to the point where the mouse's firmware crashes.  I can't explain the 
> difference in behavior; it must be some very subtle timing-related 
> issue.
> 
> There doesn't seem to be anything more to try.  :-(

Hi Alan,

As always, thanks very much for your assistance.  The machine that it
works on is quite a bit slower than my laptop, I might continue to
have a play with this and see if it is a less subtle timing issue, i.e.
whether just slowing down the host interactions allows the mouse to keep
up.

Thanks again,
Alistair

--
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: Mouse works with eHCI, fails with xHCI - can't set config #1, error -110

2015-04-14 Thread Alistair Grant
On Mon, Apr 13, 2015 at 04:24:50PM -0400, Alan Stern wrote:
> On Mon, 13 Apr 2015, Mathias Nyman wrote:
> 
> > Another difference between EHCI and xHCI iss that xHCI needs to reset (the 
> > host side)
> > of a control endpoint if it stalled.  
> > 
> > From xHCI 1.0 4.8.3:
> > 
> > "A STALL detected on any stage (Setup, Data, or Status) of a Default 
> > Control Endpoint request
> > shall transition the Endpoint Context to the Halted state. A Default 
> > Control Endpoint STALL
> > condition is cleared by a Reset Endpoint Command which transitions the 
> > endpoint from the Halted
> > to the Stopped state. The Default Control Endpoint shall return to the 
> > Running state when the
> > Doorbell is rung for the next Setup Stage TD sent to the endpoint.
> > ection 8.5.3.4 of the USB2 spec and section 8.12.2.3 of the USB3 spec state 
> > of Control pipes,
> > ?Unlike the case of a functional stall, protocol stall does not indicate an 
> > error with the device.? The
> > xHC treats a functional stall and protocol stall identically, by Halting 
> > the endpoint and requiring
> > software to clear the condition by issuing a Reset Endpoint Command."
> 
> That sounds like something the xhci-hcd driver needs to do
> automatically.  Higher-level drivers all assume that a protocol stall
> does not need to be cleared.


Hi Mathias and Alan,

As Mathias requested, I've included the usbmon output with the patch
applied.

It didn't make any difference to the end result, the mouse still fails
to initialise correctly (no real surprise, I think), but is getting the
same string that Alan reported earlier ("Laser 0").  I've also included
the relevant lines from syslog below. (I also have a pcap file
captured with tshark if anyone is interested).

As always, thanks for your help,
Alistair

usbmon output:

8800bc095180 590743316 C Ii:1:001:1 0:2048 2 = 0400
8800bc095180 590743354 S Ii:1:001:1 -115:2048 4 <
880024c51b40 590743380 S Ci:1:001:0 s a3 00  0002 0004 4 <
880024c51b40 590743399 C Ci:1:001:0 0 4 = 01030100
880024c51b40 590743408 S Co:1:001:0 s 23 01 0010 0002  0
880024c51b40 590743417 C Co:1:001:0 0 0
880024c51b40 590743423 S Ci:1:001:0 s a3 00  0002 0004 4 <
880024c51b40 590743431 C Ci:1:001:0 0 4 = 0103
880024c51b40 590773073 S Ci:1:001:0 s a3 00  0002 0004 4 <
880024c51b40 590773106 C Ci:1:001:0 0 4 = 0103
880024c51b40 590805052 S Ci:1:001:0 s a3 00  0002 0004 4 <
880024c51b40 590805089 C Ci:1:001:0 0 4 = 0103
880024c51b40 590837037 S Ci:1:001:0 s a3 00  0002 0004 4 <
880024c51b40 590837077 C Ci:1:001:0 0 4 = 0103
880024c51b40 590869051 S Ci:1:001:0 s a3 00  0002 0004 4 <
880024c51b40 590869079 C Ci:1:001:0 0 4 = 0103
880024c51b40 590869151 S Co:1:001:0 s 23 03 0004 0002  0
880024c51b40 590869176 C Co:1:001:0 0 0
880024c51b40 590925077 S Ci:1:001:0 s a3 00  0002 0004 4 <
880024c51b40 590925113 C Ci:1:001:0 0 4 = 03031000
880024c51b40 590981102 S Co:1:001:0 s 23 01 0014 0002  0
880024c51b40 590981134 C Co:1:001:0 0 0
880024c51b40 590981217 S Ci:1:000:0 s 80 06 0100  0040 64 <
880024c51b40 590981978 C Ci:1:000:0 0 18 = 12011001 0008 58043a00 
0102 0001
880024c51b40 590982014 S Co:1:001:0 s 23 03 0004 0002  0
880024c51b40 590982031 C Co:1:001:0 0 0
880024c51b40 591037084 S Ci:1:001:0 s a3 00  0002 0004 4 <
880024c51b40 591037110 C Ci:1:001:0 0 4 = 1103
880024c51b40 591093083 S Ci:1:001:0 s a3 00  0002 0004 4 <
880024c51b40 591093108 C Ci:1:001:0 0 4 = 03031000
880024c51b40 591149105 S Co:1:001:0 s 23 01 0014 0002  0
880024c51b40 591149128 C Co:1:001:0 0 0
880024c51b40 591165056 S Ci:1:006:0 s 80 06 0100  0012 18 <
880024c51b40 591165918 C Ci:1:006:0 0 18 = 12011001 0008 58043a00 
0102 0001
880024c51b40 591165954 S Ci:1:006:0 s 80 06 0200  0009 9 <
880024c51b40 591166545 C Ci:1:006:0 0 9 = 09022200 010100a0 32
880024c51b40 591166579 S Ci:1:006:0 s 80 06 0200  0022 34 <
880024c51b40 591167857 C Ci:1:006:0 0 34 = 09022200 010100a0 32090400 
00010301 02000921 10010001 22340007 05810304
880024c51b40 591167895 S Ci:1:006:0 s 80 06 0300  00ff 255 <
880024c51b40 591168392 C Ci:1:006:0 0 4 = 04030904
880024c51b40 591168422 S Ci:1:006:0 s 80 06 0302 0409 00ff 255 <
880024c51b40 591169070 C Ci:1:006:0 -32 15 = 18034c00 61007300 65007200 
200030
880024c51b40 591169098 S Ci:1:006:0 s 80 06 0302 0409 00ff 255 <
880024c51b40 591169854 C Ci:1:006:0 -32 15 = 18034c00 61007300 65007200 
200030
880024c51b40 591169884 S Ci:1:006:0 s 80 06 0302 0409 00ff 255 <
880024c51b40 591170684 C Ci:1:006:0 -32 15 = 18034c00 61007300 65007200 
200030
880024c51b40 591170702 S Ci:1:006:0 s 80 06 0302 0409 0002 2 <
880024c51b40 591171025 C Ci:1:006:0 0 2 = 1803
880024c51b40 591171037 S Ci:1:006:0 s 80 06 0302 0409 0018 24 <
880024c51b40 5911716

Re: Mouse works with eHCI, fails with xHCI - can't set config #1, error -110

2015-04-10 Thread Alistair Grant
Hi Alan,

On Fri, Apr 10, 2015 at 7:23 PM, Alan Stern  wrote:
> On Fri, 10 Apr 2015, Alistair Grant wrote:
>> On Fri, Apr 10, 2015 at 5:29 PM, Alan Stern  
>> wrote:
>> > On Fri, 10 Apr 2015, Alistair Grant wrote:
>> > ...
>> >> i.e. the mouse works reliably in eHCI controllers, but not in xHCI
>> >> controllers (I've tried two different Intel xHCI controllers).  The
>> >
>> > Have you tried testing a different mouse?
>>
>> I've got two other mice that work(ed) with xHCI (the first has died,
>> which led to buying the one with the problems, and a second one which
>> was purchased to tide me over while I try and figure out the problem
>> with the problem mouse).
>
> It does sound as though the mouse is the major part of the problem.
>
>> > Can you post the usbmon log for an EHCI controller?  Comparing the two
>> > logs may be helpful.
>
> Here's the relevant part of the xHCI trace:
> ...

Thanks very much for your detailed analysis, I really appreciate it
(as someone trying to learn a little about the USB protocol).

I was hoping that this was hitting an edge case with the xHCI driver,
but as you say, it looks like the mouse is at fault.

Thanks again,
Alistair
--
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: xhci_hcd error Transfer event TRB DMA ptr not part of current TD ep_index 8 comp_code 1

2015-03-26 Thread Alistair Grant
On Mon, Mar 23, 2015 at 10:14 PM, Devin Heitmueller
 wrote:
>
> My guess would be that there's some bug in the cx231xx that
> exacerbates an edge case in the XHCI core - like prematurely setting
> the USB alternate back to zero when stopping streaming and not
> canceling all the URBs first.
>
> ...
>
> As a test, you may wish to try disabling the cx231xx-audio module.
> That will help narrow down an interaction with ALSA, pulseaudio, and
> one of the two sources of URB queueing.


I think you're on the right track here - blacklisting the audio module
(cx231xxx_alsa) allowed successful video recording with the device inserted
during boot.

I added some logging to drivers/usb/core/message.c to check that the URB
list is empty before changing the alternate interface.  While it wasn't
ever called with a non-empty list, it did perhaps help narrow down the area
of code causing trouble.

The normal sequence of log messages when stopping recoring is:

Mar 26 08:50:41 alistair-XPS13 kernel: [  771.866435] cx231xx 1-2:1.1:
urb_init=0 dev->video_mode.max_pkt_size=2892
Mar 26 08:50:41 alistair-XPS13 kernel: [  771.900280] cx231xx 1-2:1.1:
urb_init=0 dev->video_mode.max_pkt_size=2892
Mar 26 08:50:41 alistair-XPS13 kernel: [  771.943487] cx231xx 1-2:1.1:
urb_init=0 dev->video_mode.max_pkt_size=2892
Mar 26 08:50:41 alistair-XPS13 kernel: [  771.988004] cx231xx 1-2:1.1:
stopping capture
Mar 26 08:50:41 alistair-XPS13 kernel: [  771.988009] cx231xx 1-2:1.1:
Stopping isoc
Mar 26 08:50:41 alistair-XPS13 kernel: [  771.988022] cx231xx 1-2:1.1:
Stop capture, if needed
Mar 26 08:50:41 alistair-XPS13 kernel: [  771.988056] cx231xx 1-2:1.1:
Stop capture, if needed
Mar 26 08:50:41 alistair-XPS13 kernel: [  771.988058] cx231xx 1-2:1.1:
closing device
Mar 26 08:50:41 alistair-XPS13 kernel: [  771.988062] cx231xx 1-2:1.1:
cx231xx_stop_stream: ep_mask = 4
Mar 26 08:50:41 alistair-XPS13 kernel: [  771.988167] usb 1-2: akg URB
list: next=0x880073299d90, prev=0x880073299d90, eq=1, empty=1
Mar 26 08:50:41 alistair-XPS13 kernel: [  771.993115] cx231xx 1-2:1.1:
cx231xx_stop_stream: ep_mask = 8
Mar 26 08:50:41 alistair-XPS13 kernel: [  771.993192] usb 1-2: akg URB
list: next=0x8800732992b0, prev=0x8800732992b0, eq=1, empty=1

The "akg URB" record is the log I added to usb_set_interface().

The failures appear to have several different paths, however one of them is:

Mar 26 08:57:17 alistair-XPS13 kernel: [   87.915849] cx231xx 1-2:1.1:
urb_init=0 dev->video_mode.max_pkt_size=2892
Mar 26 08:57:17 alistair-XPS13 kernel: [   87.960923] cx231xx 1-2:1.1:
urb_init=0 dev->video_mode.max_pkt_size=2892
Mar 26 08:57:17 alistair-XPS13 kernel: [   88.004771] cx231xx 1-2:1.1:
urb_init=0 dev->video_mode.max_pkt_size=2892
Mar 26 08:57:17 alistair-XPS13 kernel: [   88.032245] cx231xx 1-2:1.1:
stopping capture
Mar 26 08:57:17 alistair-XPS13 kernel: [   88.032253] cx231xx 1-2:1.1:
Stopping isoc
Mar 26 08:57:17 alistair-XPS13 kernel: [   88.032273] cx231xx 1-2:1.1:
Stop capture, if needed
Mar 26 08:57:17 alistair-XPS13 kernel: [   88.032326] cx231xx 1-2:1.1:
Stop capture, if needed
Mar 26 08:57:17 alistair-XPS13 kernel: [   88.032331] cx231xx 1-2:1.1:
closing device
Mar 26 08:57:17 alistair-XPS13 kernel: [   88.032337] cx231xx 1-2:1.1:
cx231xx_stop_stream: ep_mask = 4
Mar 26 08:57:17 alistair-XPS13 kernel: [   88.032526] xhci_hcd
:00:14.0: ERROR Transfer event TRB DMA ptr not part of current TD
ep_index 8 comp_code 1
Mar 26 08:57:17 alistair-XPS13 kernel: [   88.032542] xhci_hcd
:00:14.0: Looking for event-dma 0001f8671400 trb-start
0001facb2090 trb-end 0001facb2090 seg-start 0001facb2000
seg-end 0001facb23f0
Mar 26 08:57:17 alistair-XPS13 kernel: [   88.032565] usb 1-2: akg URB
list: next=0x880215504eb0, prev=0x880215504eb0, eq=1, empty=1
Mar 26 08:57:17 alistair-XPS13 kernel: [   88.032579] xhci_hcd
:00:14.0: ERROR Transfer event TRB DMA ptr not part of current TD
ep_index 8 comp_code 1
Mar 26 08:57:17 alistair-XPS13 kernel: [   88.032596] xhci_hcd
:00:14.0: Looking for event-dma 0001f8671410 trb-start
0001facb2090 trb-end 0001facb2090 seg-start 0001facb2000
seg-end 0001facb23f0
Mar 26 08:57:17 alistair-XPS13 kernel: [   88.032703] xhci_hcd
:00:14.0: ERROR Transfer event TRB DMA ptr not part of current TD
ep_index 8 comp_code 1
Mar 26 08:57:17 alistair-XPS13 kernel: [   88.032712] xhci_hcd
:00:14.0: Looking for event-dma 0001f8671420 trb-start
0001facb2090 trb-end 0001facb2090 seg-start 0001facb2000
seg-end 0001facb23f0

Hopefully this will give some clues about where to look next.

Thanks again for all your assistance.

Cheers,
Alistair
--
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: xhci_hcd error Transfer event TRB DMA ptr not part of current TD ep_index 8 comp_code 1

2015-03-23 Thread Alistair Grant
Hi Devin,

On Mon, Mar 23, 2015 at 8:16 PM, Devin Heitmueller
 wrote:
> Hi Alistair,
>
>> There are some small differences in packet ordering, however the first
>> major difference is an isochronous in packet where the Live2 returns
>> "URB status: No such file or directory (-ENOENT) (-2)".
>>
>> Devin, I'm try to learn a bit about USB and the Live2 protocol, however I'm
>> not sure how far I'll get.  Are you able to offer any hints on what to do
>> next?
>
> I'm sorry if I'm asking something already answered in your original
> description of the problems - but have you confirmed the device works
> properly on a system with a standard EHCI controller (as opposed to
> XHCI)?
>
> The reason I ask is because that the cx231xx driver is pretty unstable
> in general, and I'm wondering if perhaps you're just hitting some
> issue completely unrelated to the recent XHCI problems (which
> obviously needed to be addressed in order for you to get this far into
> testing).
>
> If you haven't tried it yet on a standard ECHI controller because you
> don't have one in your PC, it might be worth spending the $20 to buy a
> PCI card with an USB 2.0 host controller on it for testing.

Thanks for your follow-up.  I've run the same series of tests over
an EHCI controller without any problems, i.e. I can record
successfully regardless of whether the Live2 is plugged in
during boot or after the system has settled down.

At the moment I only have access to laptops, not a desktop,
so the EHCI testing was on a different machine, with the Ubuntu
3.13 kernel.  If you'd like me to test on a more recent kernel,
please let me know.


> It's also worth mentioning that the process you're exercising isn't
> just the code path for device insertion.  The udev process and
> PulseAudio will both try to connect to the device as soon as the
> underlying device nodes appear, so it's possible there is some race
> condition where the device is being accessed and registers are being
> poked before initialization is complete.  I cannot say for certain
> this would be an issue with cx231xx, but I've definitely seen it with
> other V4L2 drivers.  That might also explain why you see different
> behavior at boot - the driver loads early enough in the boot process
> such that initialization completes before processes like udev and
> pulseaudio get a chance to interact with it.

Good point, although the problems occur when the device is present
during boot.  It works fine if it is plugged in after boot.  However it
does appear to be some sort of timing / race condition.

Any other suggestions?

Do you know if the cx23102 datasheet is available?  All I've been able
to find is a two page sheet with a block diagram.

Thanks again,
Alistair
--
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


xhci_hcd error Transfer event TRB DMA ptr not part of current TD ep_index 8 comp_code 1

2015-03-23 Thread Alistair Grant
Hi Mathias & Devin,

I've changed the subject to avoid any confusion with the patch series
that Mathias just posted for inclusion in 4.0-rc.

On Tue, Mar 17, 2015 at 4:21 PM, Alistair Grant  wrote:
>>>>>>> It looks like I may have signed-off a little too soon.  While the patch 
>>>>>>> is
>>>>>>> working correctly if the Hauppauge Live2 is plugged in after the system 
>>>>>>> has
>>>>>>> booted and settled down (my normal use case), it fails if the Live2 is
>>>>>>> plugged in while the system is booted up.
>>>>>>>
>>>>>>> Unplugging the Live2 after recording (which appears to succeed from the
>>>>>>> command line, but had no audio), executing lsusb just hangs.
>>>>>>>
>>>>>>> I've included what I think is the relevant portions of /var/log/syslog
>>>>>>> below.  If you'd like the entire log file posted somewhere please let me
>>>>>>> know.

I've been trying to narrow down the issues with the Hauppauge Live2 and
xhci.  I upgraded to 4.0rc5 + XHCI_AVOID_BEI and found a couple of things:

1) Plugging the Live2 in after booting produced the same error message as
earlier:

ERROR Transfer event TRB DMA ptr not part of current TD ep_index 8 comp_code 1I

I had only previously seen this if the Live2 was plugged in during boot.
Maybe this is a timing / race condition that has become a bit worse in
4.0rc5?

This only happened once, and I haven't been able to reproduce it, so I'll
leave it to you how useful this is.

I've included the usb related messages from syslog at the end of the message.



2) I captured a couple of traces using usbmon, tshark and wireshark
comparing video recording with:

a) The Live2 plugged in after boot (which succeeds), and
b) The Live2 plugging in during boot (which usually fails).

There are some small differences in packet ordering, however the first
major difference is an isochronous in packet where the Live2 returns
"URB status: No such file or directory (-ENOENT) (-2)".

Devin, I'm try to learn a bit about USB and the Live2 protocol, however I'm
not sure how far I'll get.  Are you able to offer any hints on what to do
next?

If you'd like me to post the complete traces or capture something else,
please let me know.

P.S.: I confirmed that the problem is still present if the
"usb: xhci: handle Config Error Change (CEC) in xhci driver"
patch is added to the kernel I used above (since Mathias requested
that it be added to 4.0).

I was also able to successfully record once with the Live2 plugged in during
boot, supporting the timing hypothesis mentioned above.


Thanks,
Alistair
--

The relevant packet is:

No. Time   SourceDestination
Protocol Length Info
290 16.005881000   2.3   host  USB
 1088   URB_ISOCHRONOUS in

Frame 290: 1088 bytes on wire (8704 bits), 1088 bytes captured (8704
bits) on interface 0
Interface id: 0 (usbmon1)
Encapsulation type: USB packets with Linux header and padding (115)
Arrival Time: Mar 23, 2015 10:03:26.691823000 CET
[Time shift for this packet: 0.0 seconds]
Epoch Time: 1427101406.691823000 seconds
[Time delta from previous captured frame: 15.868813000 seconds]
[Time delta from previous displayed frame: 15.868813000 seconds]
[Time since reference or first frame: 16.005881000 seconds]
Frame Number: 290
Frame Length: 1088 bytes (8704 bits)
Capture Length: 1088 bytes (8704 bits)
[Frame is marked: False]
[Frame is ignored: False]
[Protocols in frame: usb]
USB URB
URB id: 0x8801ee7a2000
URB type: URB_COMPLETE ('C')
URB transfer type: URB_ISOCHRONOUS (0x00)
Endpoint: 0x83, Direction: IN
1...  = Direction: IN (1)
.000 0011 = Endpoint value: 3
Device: 2
URB bus id: 1
Device setup request: not relevant ('-')
Data: present (0)
URB sec: 1427101406
URB usec: 691823
URB status: No such file or directory (-ENOENT) (-2)
URB length [bytes]: 0
Data length [bytes]: 1024
[Request in: 70]
[Time from request: 15.97074 seconds]
[bInterfaceClass: Unknown (0x)]
ISO error count: 0
Number of ISO descriptors: 64
Interval: 1
Start frame: 4468
Copy of Transfer Flags: 0x0202
Number of ISO descriptors: 64
Leftover Capture Data: eeffeeff1c00...

  00 20 7a ee 01 88 ff ff 43 00 83 02 01 00 2d 00   . z.C.-.
0010  de d6 0f 55 00 00 00 00 6f 8e 0a 00 fe ff ff ff   ...Uo...
0020  00 00 00 00 00 04 00 00 00 00 00 00 40 00 00 00   @...
0030  01 00 00 00 74 11 00 00 02 02 00 00 40 00 00 00   t...@...
0040

Re: [PATCH v2 1/1] usb: xhci: apply XHCI_AVOID_BEI quirk to all Intel xHCI controllers

2015-03-17 Thread Alistair Grant
On Mon, Mar 16, 2015 at 5:29 PM, Mathias Nyman  wrote:
> On 16.03.2015 17:21, Alistair Grant wrote:
>> On Mon, Mar 16, 2015 at 3:47 PM, Mathias Nyman  
>> wrote:
>>> On 16.03.2015 16:31, Alistair Grant wrote:
>>>> On Mon, Mar 16, 2015 at 1:55 PM, Mathias Nyman
>>>>  wrote:
>>>>> On 15.03.2015 21:18, Alistair Grant wrote:
>>>>>> On Sun, Mar 15, 2015 at 3:54 PM, Alistair Grant  
>>>>>> wrote:
>>>>>> ...
>>>>>> It looks like I may have signed-off a little too soon.  While the patch 
>>>>>> is
>>>>>> working correctly if the Hauppauge Live2 is plugged in after the system 
>>>>>> has
>>>>>> booted and settled down (my normal use case), it fails if the Live2 is
>>>>>> plugged in while the system is booted up.
>>>>>>
>>>>>> Unplugging the Live2 after recording (which appears to succeed from the
>>>>>> command line, but had no audio), executing lsusb just hangs.
>>>>>>
>>>>>> I've included what I think is the relevant portions of /var/log/syslog
>>>>>> below.  If you'd like the entire log file posted somewhere please let me
>>>>>> know.
>>>>>
>>>>> Hi
>>>>>
>>>>> What kernel did you try this patch on?
>>>>>
>>>>> The output look a bit like the regression in 4.0-rc3 caused by:
>>>>> commit 27082e2654dc148078b0abdfc3c8e5ccbde0ebfa
>>>>> xhci: Clear the host side toggle manually when endpoint is 'soft 
>>>>> reset'
>>>>>
>>>>> which will be reverted (in 4.0-rc5 I hope).
>>>>>
>>>>> If you boot the same base kernel without the patch does it work then?
>>>>>
>>>>> -Mathias
>>>>>
>>>>
>>>> Hi Mathias,
>>>>
>>>> This is on top of 3.19.1 with only the XHCI_AVOID_BEI quirk patch applied.
>>>>
>>>> If you'd like me to try it against 4.0-rc3, 4 or 5, please let me know.
>>>
>>> Thanks, no that's not needed.
>>> But did the patch cause regression on top of 3.19.1?
>>> I mean, did it make 3.19.1  worse, better or just different for you, while
>>> booting with the device connected?
>>
>> Recording doesn't work at all with 3.19.1 without the patch.  I've
>> included an extract from syslog below of attempting to record with a
>> clean 3.19.1 system (i.e. XHCI_AVOID_BEI quirk patch NOT applied, to
>> state the obvious).
>>
>> I did mean to say in my original message that I would still release
>> this patch even with the current known issue as it still improves the
>> overall stability of the system.
>>
>
> Ok, thanks
> I'll add it to the queue

Hi Mathias & Lu,

FYI...  I also tried booting with the Live2 plugged in with the
following kernel:

* 3.19.1
* usb: xhci: apply XHCI_AVOID_BEI quirk to all Intel xHCI controllers
* xhci: Allocate correct amount of scratchpad buffers
* xhci.h: Increase TRBS_PER_SEGMENT from 64 to 256

i.e. add the scratchpad buffers fix and increase TRBS_PER_SEGMENT,
however the problem still occurs if the Live2 is plugged in during
boot up.

Thanks,
Alistair
--
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 1/1] usb: xhci: apply XHCI_AVOID_BEI quirk to all Intel xHCI controllers

2015-03-16 Thread Alistair Grant
On Mon, Mar 16, 2015 at 3:47 PM, Mathias Nyman  wrote:
> On 16.03.2015 16:31, Alistair Grant wrote:
>> On Mon, Mar 16, 2015 at 1:55 PM, Mathias Nyman
>>  wrote:
>>> On 15.03.2015 21:18, Alistair Grant wrote:
>>>> On Sun, Mar 15, 2015 at 3:54 PM, Alistair Grant  
>>>> wrote:
>>>>> On Thu, Mar 12, 2015 at 8:14 PM, Alistair Grant  
>>>>> wrote:
>>>>>> On Thu, Mar 12, 2015 at 11:15 AM, Lu Baolu  
>>>>>> wrote:
>>>>>>> When a device with an isochronous endpoint is plugged into the Intel
>>>>>>> xHCI host controller, and the driver submits multiple frames per URB,
>>>>>>> the xHCI driver will set the Block Event Interrupt (BEI) flag on all
>>>>>>> but the last TD for the URB. This causes the host controller to place
>>>>>>> an event on the event ring, but not send an interrupt. When the last
>>>>>>> TD for the URB completes, BEI is cleared, and we get an interrupt for
>>>>>>> the whole URB.
>>>>>>>
>>>>>>> However, under Intel xHCI host controllers, if the event ring is full
>>>>>>> of events from transfers with BEI set,  an "Event Ring is Full" event
>>>>>>> will be posted to the last entry of the event ring,  but no interrupt
>>>>>>> is generated. Host will cease all transfer and command executions and
>>>>>>> wait until software completes handling the pending events in the event
>>>>>>> ring.  That means xHC stops, but event of "event ring is full" is not
>>>>>>> notified. As the result, the xHC looks like dead to user.
>>>>>>>
>>>>>>> This patch is to apply XHCI_AVOID_BEI quirk to Intel xHC devices. And
>>>>>>> it should be backported to kernels as old as 3.0, that contains the
>>>>>>> commit 69e848c2090a ("Intel xhci: Support EHCI/xHCI port switching.").
>>>>>>>
>>>>>>> Signed-off-by: Lu Baolu 
>>>>>>> Cc: sta...@vger.kernel.org
>>>>>>> ---
>>>>>>>  drivers/usb/host/xhci-pci.c | 2 +-
>>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
>>>>>>> index fd53c9e..2af32e2 100644
>>>>>>> --- a/drivers/usb/host/xhci-pci.c
>>>>>>> +++ b/drivers/usb/host/xhci-pci.c
>>>>>>> @@ -115,6 +115,7 @@ static void xhci_pci_quirks(struct device *dev, 
>>>>>>> struct xhci_hcd *xhci)
>>>>>>> if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
>>>>>>> xhci->quirks |= XHCI_LPM_SUPPORT;
>>>>>>> xhci->quirks |= XHCI_INTEL_HOST;
>>>>>>> +   xhci->quirks |= XHCI_AVOID_BEI;
>>>>>>> }
>>>>>>> if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
>>>>>>> pdev->device == 
>>>>>>> PCI_DEVICE_ID_INTEL_PANTHERPOINT_XHCI) {
>>>>>>> @@ -130,7 +131,6 @@ static void xhci_pci_quirks(struct device *dev, 
>>>>>>> struct xhci_hcd *xhci)
>>>>>>>  * PPT chipsets.
>>>>>>>  */
>>>>>>> xhci->quirks |= XHCI_SPURIOUS_REBOOT;
>>>>>>> -   xhci->quirks |= XHCI_AVOID_BEI;
>>>>>>> }
>>>>>>> if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
>>>>>>> pdev->device == PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI) {
>>>>>>> --
>>>>>>> 2.1.0
>>>>>>>
>>>>>>> --
>>>>>>
>>>>>> This works for me...
>>>>>>
>>>>>> Computer: Dell XPS13 9333
>>>>>> USB controller: Intel Corporation 8 Series USB xHCI HC (rev 04)
>>>>>> (prog-if 30 [XHCI])
>>>>>> Kernel: 3.19.1
>>>>>> USB Device: Hauppauge USB-Live2
>>>>>>
>>>>>> Please let me know if I can help in any other way.
>>>>>>
>>>>>> Tested-by: Alistair Grant 
>>>>>
>>>>> Just FYI

Re: [PATCH v2 1/1] usb: xhci: apply XHCI_AVOID_BEI quirk to all Intel xHCI controllers

2015-03-16 Thread Alistair Grant
On Mon, Mar 16, 2015 at 1:55 PM, Mathias Nyman
 wrote:
> On 15.03.2015 21:18, Alistair Grant wrote:
>> On Sun, Mar 15, 2015 at 3:54 PM, Alistair Grant  
>> wrote:
>>> On Thu, Mar 12, 2015 at 8:14 PM, Alistair Grant  
>>> wrote:
>>>> On Thu, Mar 12, 2015 at 11:15 AM, Lu Baolu  
>>>> wrote:
>>>>> When a device with an isochronous endpoint is plugged into the Intel
>>>>> xHCI host controller, and the driver submits multiple frames per URB,
>>>>> the xHCI driver will set the Block Event Interrupt (BEI) flag on all
>>>>> but the last TD for the URB. This causes the host controller to place
>>>>> an event on the event ring, but not send an interrupt. When the last
>>>>> TD for the URB completes, BEI is cleared, and we get an interrupt for
>>>>> the whole URB.
>>>>>
>>>>> However, under Intel xHCI host controllers, if the event ring is full
>>>>> of events from transfers with BEI set,  an "Event Ring is Full" event
>>>>> will be posted to the last entry of the event ring,  but no interrupt
>>>>> is generated. Host will cease all transfer and command executions and
>>>>> wait until software completes handling the pending events in the event
>>>>> ring.  That means xHC stops, but event of "event ring is full" is not
>>>>> notified. As the result, the xHC looks like dead to user.
>>>>>
>>>>> This patch is to apply XHCI_AVOID_BEI quirk to Intel xHC devices. And
>>>>> it should be backported to kernels as old as 3.0, that contains the
>>>>> commit 69e848c2090a ("Intel xhci: Support EHCI/xHCI port switching.").
>>>>>
>>>>> Signed-off-by: Lu Baolu 
>>>>> Cc: sta...@vger.kernel.org
>>>>> ---
>>>>>  drivers/usb/host/xhci-pci.c | 2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
>>>>> index fd53c9e..2af32e2 100644
>>>>> --- a/drivers/usb/host/xhci-pci.c
>>>>> +++ b/drivers/usb/host/xhci-pci.c
>>>>> @@ -115,6 +115,7 @@ static void xhci_pci_quirks(struct device *dev, 
>>>>> struct xhci_hcd *xhci)
>>>>> if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
>>>>> xhci->quirks |= XHCI_LPM_SUPPORT;
>>>>> xhci->quirks |= XHCI_INTEL_HOST;
>>>>> +   xhci->quirks |= XHCI_AVOID_BEI;
>>>>> }
>>>>> if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
>>>>> pdev->device == 
>>>>> PCI_DEVICE_ID_INTEL_PANTHERPOINT_XHCI) {
>>>>> @@ -130,7 +131,6 @@ static void xhci_pci_quirks(struct device *dev, 
>>>>> struct xhci_hcd *xhci)
>>>>>  * PPT chipsets.
>>>>>  */
>>>>> xhci->quirks |= XHCI_SPURIOUS_REBOOT;
>>>>> -   xhci->quirks |= XHCI_AVOID_BEI;
>>>>> }
>>>>> if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
>>>>> pdev->device == PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI) {
>>>>> --
>>>>> 2.1.0
>>>>>
>>>>> --
>>>>
>>>> This works for me...
>>>>
>>>> Computer: Dell XPS13 9333
>>>> USB controller: Intel Corporation 8 Series USB xHCI HC (rev 04)
>>>> (prog-if 30 [XHCI])
>>>> Kernel: 3.19.1
>>>> USB Device: Hauppauge USB-Live2
>>>>
>>>> Please let me know if I can help in any other way.
>>>>
>>>> Tested-by: Alistair Grant 
>>>
>>> Just FYI...
>>>
>>> I was able to test this on a slightly older laptop that had both USB2
>>> and USB3 ports and can confirm that it also works there:
>>>
>>> 00:14.0 USB controller: Intel Corporation 7 Series/C210 Series Chipset
>>> Family USB xHCI Host Controller (rev 04)
>>
>> It looks like I may have signed-off a little too soon.  While the patch is
>> working correctly if the Hauppauge Live2 is plugged in after the system has
>> booted and settled down (my normal use case), it fails if the Live2 is
>> plugged in while the system is booted up.
>>
>> Unplugging the Live2 after recording (which appears to succeed from the
>> command line, but had no audio), executing lsusb just hangs.
>>
>> I've included what I think is the relevant portions of /var/log/syslog
>> below.  If you'd like the entire log file posted somewhere please let me
>> know.
>
> Hi
>
> What kernel did you try this patch on?
>
> The output look a bit like the regression in 4.0-rc3 caused by:
> commit 27082e2654dc148078b0abdfc3c8e5ccbde0ebfa
> xhci: Clear the host side toggle manually when endpoint is 'soft reset'
>
> which will be reverted (in 4.0-rc5 I hope).
>
> If you boot the same base kernel without the patch does it work then?
>
> -Mathias
>

Hi Mathias,

This is on top of 3.19.1 with only the XHCI_AVOID_BEI quirk patch applied.

If you'd like me to try it against 4.0-rc3, 4 or 5, please let me know.

Thanks,
Alistair
--
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 1/1] usb: xhci: apply XHCI_AVOID_BEI quirk to all Intel xHCI controllers

2015-03-15 Thread Alistair Grant
On Sun, Mar 15, 2015 at 3:54 PM, Alistair Grant  wrote:
> On Thu, Mar 12, 2015 at 8:14 PM, Alistair Grant  wrote:
>> On Thu, Mar 12, 2015 at 11:15 AM, Lu Baolu  wrote:
>>> When a device with an isochronous endpoint is plugged into the Intel
>>> xHCI host controller, and the driver submits multiple frames per URB,
>>> the xHCI driver will set the Block Event Interrupt (BEI) flag on all
>>> but the last TD for the URB. This causes the host controller to place
>>> an event on the event ring, but not send an interrupt. When the last
>>> TD for the URB completes, BEI is cleared, and we get an interrupt for
>>> the whole URB.
>>>
>>> However, under Intel xHCI host controllers, if the event ring is full
>>> of events from transfers with BEI set,  an "Event Ring is Full" event
>>> will be posted to the last entry of the event ring,  but no interrupt
>>> is generated. Host will cease all transfer and command executions and
>>> wait until software completes handling the pending events in the event
>>> ring.  That means xHC stops, but event of "event ring is full" is not
>>> notified. As the result, the xHC looks like dead to user.
>>>
>>> This patch is to apply XHCI_AVOID_BEI quirk to Intel xHC devices. And
>>> it should be backported to kernels as old as 3.0, that contains the
>>> commit 69e848c2090a ("Intel xhci: Support EHCI/xHCI port switching.").
>>>
>>> Signed-off-by: Lu Baolu 
>>> Cc: sta...@vger.kernel.org
>>> ---
>>>  drivers/usb/host/xhci-pci.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
>>> index fd53c9e..2af32e2 100644
>>> --- a/drivers/usb/host/xhci-pci.c
>>> +++ b/drivers/usb/host/xhci-pci.c
>>> @@ -115,6 +115,7 @@ static void xhci_pci_quirks(struct device *dev, struct 
>>> xhci_hcd *xhci)
>>> if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
>>> xhci->quirks |= XHCI_LPM_SUPPORT;
>>> xhci->quirks |= XHCI_INTEL_HOST;
>>> +   xhci->quirks |= XHCI_AVOID_BEI;
>>> }
>>> if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
>>> pdev->device == 
>>> PCI_DEVICE_ID_INTEL_PANTHERPOINT_XHCI) {
>>> @@ -130,7 +131,6 @@ static void xhci_pci_quirks(struct device *dev, struct 
>>> xhci_hcd *xhci)
>>>  * PPT chipsets.
>>>  */
>>> xhci->quirks |= XHCI_SPURIOUS_REBOOT;
>>> -   xhci->quirks |= XHCI_AVOID_BEI;
>>> }
>>> if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
>>> pdev->device == PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI) {
>>> --
>>> 2.1.0
>>>
>>> --
>>
>> This works for me...
>>
>> Computer: Dell XPS13 9333
>> USB controller: Intel Corporation 8 Series USB xHCI HC (rev 04)
>> (prog-if 30 [XHCI])
>> Kernel: 3.19.1
>> USB Device: Hauppauge USB-Live2
>>
>> Please let me know if I can help in any other way.
>>
>> Tested-by: Alistair Grant 
>
> Just FYI...
>
> I was able to test this on a slightly older laptop that had both USB2
> and USB3 ports and can confirm that it also works there:
>
> 00:14.0 USB controller: Intel Corporation 7 Series/C210 Series Chipset
> Family USB xHCI Host Controller (rev 04)

It looks like I may have signed-off a little too soon.  While the patch is
working correctly if the Hauppauge Live2 is plugged in after the system has
booted and settled down (my normal use case), it fails if the Live2 is
plugged in while the system is booted up.

Unplugging the Live2 after recording (which appears to succeed from the
command line, but had no audio), executing lsusb just hangs.

I've included what I think is the relevant portions of /var/log/syslog
below.  If you'd like the entire log file posted somewhere please let me
know.

Thanks,
Alistair


Mar 15 19:50:42 alistair-XPS13 kernel: [   15.177240] Linux video
capture interface: v2.00
Mar 15 19:50:42 alistair-XPS13 kernel: [   15.221452] AVX2 version of
gcm_enc/dec engaged.
Mar 15 19:50:42 alistair-XPS13 kernel: [   15.221456] AES CTR mode by8
optimization enabled
Mar 15 19:50:42 alistair-XPS13 kernel: [   15.257091] cx231xx 1-2:1.1:
New device Hauppauge Hauppauge Device @ 480 Mbps (2040:c200) with 6
interfaces
Mar 15 19:50:42 alistair-XPS13 kernel: [   15.257790] cx231xx 1-2:1.1:
can't change interface 3 alt no. to 3: Max. Pkt size = 0
Mar 15 

Re: [PATCH v2 1/1] usb: xhci: apply XHCI_AVOID_BEI quirk to all Intel xHCI controllers

2015-03-15 Thread Alistair Grant
On Thu, Mar 12, 2015 at 8:14 PM, Alistair Grant  wrote:
> On Thu, Mar 12, 2015 at 11:15 AM, Lu Baolu  wrote:
>> When a device with an isochronous endpoint is plugged into the Intel
>> xHCI host controller, and the driver submits multiple frames per URB,
>> the xHCI driver will set the Block Event Interrupt (BEI) flag on all
>> but the last TD for the URB. This causes the host controller to place
>> an event on the event ring, but not send an interrupt. When the last
>> TD for the URB completes, BEI is cleared, and we get an interrupt for
>> the whole URB.
>>
>> However, under Intel xHCI host controllers, if the event ring is full
>> of events from transfers with BEI set,  an "Event Ring is Full" event
>> will be posted to the last entry of the event ring,  but no interrupt
>> is generated. Host will cease all transfer and command executions and
>> wait until software completes handling the pending events in the event
>> ring.  That means xHC stops, but event of "event ring is full" is not
>> notified. As the result, the xHC looks like dead to user.
>>
>> This patch is to apply XHCI_AVOID_BEI quirk to Intel xHC devices. And
>> it should be backported to kernels as old as 3.0, that contains the
>> commit 69e848c2090a ("Intel xhci: Support EHCI/xHCI port switching.").
>>
>> Signed-off-by: Lu Baolu 
>> Cc: sta...@vger.kernel.org
>> ---
>>  drivers/usb/host/xhci-pci.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
>> index fd53c9e..2af32e2 100644
>> --- a/drivers/usb/host/xhci-pci.c
>> +++ b/drivers/usb/host/xhci-pci.c
>> @@ -115,6 +115,7 @@ static void xhci_pci_quirks(struct device *dev, struct 
>> xhci_hcd *xhci)
>> if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
>> xhci->quirks |= XHCI_LPM_SUPPORT;
>> xhci->quirks |= XHCI_INTEL_HOST;
>> +   xhci->quirks |= XHCI_AVOID_BEI;
>> }
>> if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
>> pdev->device == 
>> PCI_DEVICE_ID_INTEL_PANTHERPOINT_XHCI) {
>> @@ -130,7 +131,6 @@ static void xhci_pci_quirks(struct device *dev, struct 
>> xhci_hcd *xhci)
>>  * PPT chipsets.
>>  */
>> xhci->quirks |= XHCI_SPURIOUS_REBOOT;
>> -   xhci->quirks |= XHCI_AVOID_BEI;
>> }
>> if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
>>     pdev->device == PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI) {
>> --
>> 2.1.0
>>
>> --
>
> This works for me...
>
> Computer: Dell XPS13 9333
> USB controller: Intel Corporation 8 Series USB xHCI HC (rev 04)
> (prog-if 30 [XHCI])
> Kernel: 3.19.1
> USB Device: Hauppauge USB-Live2
>
> Please let me know if I can help in any other way.
>
> Tested-by: Alistair Grant 

Just FYI...

I was able to test this on a slightly older laptop that had both USB2
and USB3 ports and can confirm that it also works there:

00:14.0 USB controller: Intel Corporation 7 Series/C210 Series Chipset
Family USB xHCI Host Controller (rev 04)

Thanks,
Alistair
--
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 v3 1/1] usb: xhci: apply XHCI_AVOID_BEI quirk to all Intel xHCI controllers

2015-03-15 Thread Alistair Grant
On Fri, Mar 13, 2015 at 1:54 AM, Lu Baolu  wrote:
> ...
> This patch is to apply XHCI_AVOID_BEI quirk to Intel xHC devices. And
> it should be backported to kernels as old as 3.0, that contains the
> commit 69e848c2090a ("Intel xhci: Support EHCI/xHCI port switching.").
>
> Signed-off-by: Lu Baolu 
> Tested-by: Alistair Grant 
> Tested-by: Devin Heitmueller 
> Cc: sta...@vger.kernel.org
> ...

As a workaround until this makes it to the various distributions,
would it be possible to create a config file that enables the quirk,
something like:

> cat /etc/modprobe.d/xhci.conf
# Enable XHCI_AVOID_BEI (= 1<<15)
options xhci-plat-hcd quirks=32768

I tried this using "xhci-plat-hcd" and "xhci-hcd", but neither appeared to work.

Thanks,
Alistair
--
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 1/1] usb: xhci: apply XHCI_AVOID_BEI quirk to all Intel xHCI controllers

2015-03-12 Thread Alistair Grant
On Thu, Mar 12, 2015 at 11:15 AM, Lu Baolu  wrote:
> When a device with an isochronous endpoint is plugged into the Intel
> xHCI host controller, and the driver submits multiple frames per URB,
> the xHCI driver will set the Block Event Interrupt (BEI) flag on all
> but the last TD for the URB. This causes the host controller to place
> an event on the event ring, but not send an interrupt. When the last
> TD for the URB completes, BEI is cleared, and we get an interrupt for
> the whole URB.
>
> However, under Intel xHCI host controllers, if the event ring is full
> of events from transfers with BEI set,  an "Event Ring is Full" event
> will be posted to the last entry of the event ring,  but no interrupt
> is generated. Host will cease all transfer and command executions and
> wait until software completes handling the pending events in the event
> ring.  That means xHC stops, but event of "event ring is full" is not
> notified. As the result, the xHC looks like dead to user.
>
> This patch is to apply XHCI_AVOID_BEI quirk to Intel xHC devices. And
> it should be backported to kernels as old as 3.0, that contains the
> commit 69e848c2090a ("Intel xhci: Support EHCI/xHCI port switching.").
>
> Signed-off-by: Lu Baolu 
> Cc: sta...@vger.kernel.org
> ---
>  drivers/usb/host/xhci-pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index fd53c9e..2af32e2 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -115,6 +115,7 @@ static void xhci_pci_quirks(struct device *dev, struct 
> xhci_hcd *xhci)
> if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
> xhci->quirks |= XHCI_LPM_SUPPORT;
> xhci->quirks |= XHCI_INTEL_HOST;
> +   xhci->quirks |= XHCI_AVOID_BEI;
> }
> if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
> pdev->device == 
> PCI_DEVICE_ID_INTEL_PANTHERPOINT_XHCI) {
> @@ -130,7 +131,6 @@ static void xhci_pci_quirks(struct device *dev, struct 
> xhci_hcd *xhci)
>  * PPT chipsets.
>  */
> xhci->quirks |= XHCI_SPURIOUS_REBOOT;
> -   xhci->quirks |= XHCI_AVOID_BEI;
> }
> if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
> pdev->device == PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI) {
> --
> 2.1.0
>
> --

This works for me...

Computer: Dell XPS13 9333
USB controller: Intel Corporation 8 Series USB xHCI HC (rev 04)
(prog-if 30 [XHCI])
Kernel: 3.19.1
USB Device: Hauppauge USB-Live2

Please let me know if I can help in any other way.

Tested-by: Alistair Grant 

Thanks!
Alistair
--
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


cp210x GPIO support

2015-03-10 Thread Grant Likely
Hi Preston,

Are you still maintaining GPIO support for the cp210x driver? I'm
looking at the last set of patches from 2012 that attempted to
mainline it, but it looks like it didn't get anywhere. I'd like to try
again.

Have you looked at CONFIG_GPIO support in the kernel? I think the
biggest complaint the last time had been the creation of a new ioctl
method. Using the GPIO library should probably get around that. The
GPIO userspace abi isn't fantastic, but it is already accepted, so
there shouldn't be any complaints there. It also makes it possible to
use the GPIO lines.

g.
--
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: Control message failures kill entire XHCI stack

2015-03-05 Thread Alistair Grant
Hi Mathias,

On Thu, Mar 5, 2015 at 4:25 PM, Mathias Nyman
 wrote:
> On 04.03.2015 15:27, Alistair Grant wrote:
>> On Tue, Mar 3, 2015 at 8:40 PM, Alistair Grant  wrote:
>>> On Tue, Mar 3, 2015 at 2:21 PM, Mathias Nyman
>>>  wrote:
>>>> On 28.02.2015 09:16, Alistair Grant wrote:
>>>>> ...
>>>>> * 3.19.0 with the following patches:
>>>>> * xhci: Allocate correct amount of scratchpad buffers
>>>>> * xhci: Don't touch TRBs memory if those are no longer on the endpoint 
>>>>> ring
>>>>> * xhci: fix invalid pointer in reset device debugging
>>>>> * xhci: add debugging for reset device and stop endpoint commands
>>>>> * xhci: add command ring stop and restart debug messages
>>>>>
>>>>
>>>> Does increasing the TRB count per segment help?
>>>
>>> Success!
>>>
>>> Increasing TRBS_PER_SEGMENT from 64 to 256 allowed me to successfully
>>> record two 30 second segments of video, i.e. start recording with
>>> mythffmpeg, Ctrl-C after 30 seconds, then repeat (this is on top of the
>>> patched kernel I reported in my last message).
>>>
>> ...
>> I assume that this is a workaround, and that the core problem of ring
>> expansion & cancelled URBs is still to be resolved.  Let me know if
>> you would like that tested when it is ready.
>>
>
> Hi
>
> yes, this is a workaround.
>
> The latest theory for the cause is that we fill up the event ring. This
> would be possible because we pick events from the event ring only on 
> interrupt.
>
> Isoc transfers are set to only interrupt at the last TD, with several isoc 
> transfer
> going on simultaneously, and especially with isoc transfer containing so many 
> TDs we need
> to increase the transfer ring (hence the ring expansion befora failure in the 
> log) we fill
> up the event ring and won't receive any stop endpoint event -> timeout -> 
> kill HC
>
> Increasing the TRBS_PER_SEGMENTS helps as it also increases the event ring.
>
> If you have time could you try forcing interrupts on every isoc TRB with the 
> following change:
>
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 151484e..dfad305 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -539,6 +539,8 @@ int xhci_init(struct usb_hcd *hcd)
> xhci_dbg_trace(xhci, trace_xhci_dbg_init,
> "xHCI doesn't need link TRB QUIRK");
> }
> +   xhci->quirks |= XHCI_AVOID_BEI;
> +
> retval = xhci_mem_init(xhci, GFP_KERNEL);
> xhci_dbg_trace(xhci, trace_xhci_dbg_init, "Finished xhci_init
>
> with the old TRBS_PER_SEGMENT size, and see if it helps

This worked without any problems also - I only recorded about 3
minutes of video, but I doubt that the length matters.

FYI, the following messages still appear in syslog:

Mar  5 19:56:19 alistair-XPS13 kernel: [   68.725309] xhci_hcd
:00:14.0: WARN Set TR Deq Ptr cmd failed due to incorrect slot or
ep state.

Mar  5 19:56:19 alistair-XPS13 kernel: [   68.725411] xhci_hcd
:00:14.0: Cancelled TD not on stopped ring (multiple times)

Mar  5 19:56:19 alistair-XPS13 kernel: [   68.725420] xhci_hcd
:00:14.0: Cancel URB NOT on current ring (multiple times)

Mar  5 20:00:17 alistair-XPS13 kernel: [  306.192925] xhci_hcd
:00:14.0: WARN Set TR Deq Ptr cmd failed due to incorrect slot or
ep state.

>From memory, the warning only appeared once with TRBS_PER_SEGMENT=256.

HTH,
Alistair
--
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: Control message failures kill entire XHCI stack

2015-03-04 Thread Alistair Grant
Hi Mathias,


On Tue, Mar 3, 2015 at 8:40 PM, Alistair Grant  wrote:
> Hi Mathias,
>
> On Tue, Mar 3, 2015 at 2:21 PM, Mathias Nyman
>  wrote:
>> On 28.02.2015 09:16, Alistair Grant wrote:
>>> ...
>>> * 3.19.0 with the following patches:
>>> * xhci: Allocate correct amount of scratchpad buffers
>>> * xhci: Don't touch TRBs memory if those are no longer on the endpoint ring
>>> * xhci: fix invalid pointer in reset device debugging
>>> * xhci: add debugging for reset device and stop endpoint commands
>>> * xhci: add command ring stop and restart debug messages
>>>
>>
>> Does increasing the TRB count per segment help?
>
> Success!
>
> Increasing TRBS_PER_SEGMENT from 64 to 256 allowed me to successfully
> record two 30 second segments of video, i.e. start recording with
> mythffmpeg, Ctrl-C after 30 seconds, then repeat (this is on top of the
> patched kernel I reported in my last message).
>
> This obviously is good news, it is also better than I typically saw using
> the ehci driver, as often the second attempt would fail with a "Device or
> Resource Busy" message (of course a single test is hardly conclusive, and
> it may still appear).
>
> It's getting a bit late here, so hopefully tomorrow I'll try recording for
> a longer period of time to make sure that succeeds as well.
>
> Included below is the syslog from the time I plugged the Live2 in to
> unplugging it after recording.  There are three types of messages which
> don't look completely normal to me:

I was able to record video for 1 hour today, and then stop and start
recording another 3 times - just a few seconds each, this was more
about ensuring it could stop and start multiple times.

I assume that this is a workaround, and that the core problem of ring
expansion & cancelled URBs is still to be resolved.  Let me know if
you would like that tested when it is ready.

Thanks!
Alistair
--
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: Control message failures kill entire XHCI stack

2015-03-03 Thread Alistair Grant
Hi Mathias,

On Tue, Mar 3, 2015 at 2:21 PM, Mathias Nyman
 wrote:
> On 28.02.2015 09:16, Alistair Grant wrote:
>> ...
>> * 3.19.0 with the following patches:
>> * xhci: Allocate correct amount of scratchpad buffers
>> * xhci: Don't touch TRBs memory if those are no longer on the endpoint ring
>> * xhci: fix invalid pointer in reset device debugging
>> * xhci: add debugging for reset device and stop endpoint commands
>> * xhci: add command ring stop and restart debug messages
>>
>
> Does increasing the TRB count per segment help?

Success!

Increasing TRBS_PER_SEGMENT from 64 to 256 allowed me to successfully
record two 30 second segments of video, i.e. start recording with
mythffmpeg, Ctrl-C after 30 seconds, then repeat (this is on top of the
patched kernel I reported in my last message).

This obviously is good news, it is also better than I typically saw using
the ehci driver, as often the second attempt would fail with a "Device or
Resource Busy" message (of course a single test is hardly conclusive, and
it may still appear).

It's getting a bit late here, so hopefully tomorrow I'll try recording for
a longer period of time to make sure that succeeds as well.

Included below is the syslog from the time I plugged the Live2 in to
unplugging it after recording.  There are three types of messages which
don't look completely normal to me:

Mar  3 20:20:45 alistair-XPS13 kernel: [   90.952271] xhci_hcd
:00:14.0: Cancel URB NOT on current ring
Mar  3 20:22:40 alistair-XPS13 kernel: [  205.582594] xhci_hcd
:00:14.0: Starting stop cmd watchdog timer for slot  4 ep index 6.
Mar  3 20:22:40 alistair-XPS13 kernel: [  205.582811] xhci_hcd
:00:14.0: WARN Set TR Deq Ptr cmd failed due to incorrect slot or
ep state.

Thanks!
Alistair

Mar  3 20:20:37 alistair-XPS13 kernel: [   83.375647] usb 1-2: new
high-speed USB device number 5 using xhci_hcd
Mar  3 20:20:37 alistair-XPS13 kernel: [   83.506190] usb 1-2: New USB
device found, idVendor=2040, idProduct=c200
Mar  3 20:20:37 alistair-XPS13 kernel: [   83.506194] usb 1-2: New USB
device strings: Mfr=1, Product=2, SerialNumber=3
Mar  3 20:20:37 alistair-XPS13 kernel: [   83.506197] usb 1-2:
Product: Hauppauge Device
Mar  3 20:20:37 alistair-XPS13 kernel: [   83.506199] usb 1-2:
Manufacturer: Hauppauge
Mar  3 20:20:37 alistair-XPS13 kernel: [   83.506200] usb 1-2:
SerialNumber: 0011446325
Mar  3 20:20:37 alistair-XPS13 mtp-probe: checking bus 1, device 5:
"/sys/devices/pci:00/:00:14.0/usb1/1-2"
Mar  3 20:20:37 alistair-XPS13 mtp-probe: bus: 1, device: 5 was not an
MTP device
Mar  3 20:20:38 alistair-XPS13 kernel: [   83.654357] cx231xx 1-2:1.1:
New device Hauppauge Hauppauge Device @ 480 Mbps (2040:c200) with 6
interfaces
Mar  3 20:20:38 alistair-XPS13 kernel: [   83.654398] cx231xx 1-2:1.1:
can't change interface 3 alt no. to 3: Max. Pkt size = 0
Mar  3 20:20:38 alistair-XPS13 kernel: [   83.659158] cx231xx 1-2:1.1:
can't change interface 4 alt no. to 1: Max. Pkt size = 0
Mar  3 20:20:38 alistair-XPS13 kernel: [   83.660371] cx231xx 1-2:1.1:
Identified as Hauppauge USB Live 2 (card=9)
Mar  3 20:20:38 alistair-XPS13 kernel: [   83.660587] i2c i2c-10:
Added multiplexed i2c bus 12
Mar  3 20:20:38 alistair-XPS13 kernel: [   83.660631] i2c i2c-10:
Added multiplexed i2c bus 13
Mar  3 20:20:38 alistair-XPS13 kernel: [   83.771556] cx25840 9-0044:
cx23102 A/V decoder found @ 0x88 (cx231xx #0-0)
Mar  3 20:20:40 alistair-XPS13 kernel: [   85.767254] cx25840 9-0044:
loaded v4l-cx231xx-avcore-01.fw firmware (16382 bytes)
Mar  3 20:20:40 alistair-XPS13 kernel: [   85.800371] cx231xx 1-2:1.1:
v4l2 driver version 0.0.3
Mar  3 20:20:40 alistair-XPS13 kernel: [   85.891907] cx231xx 1-2:1.1:
Registered video device video1 [v4l2]
Mar  3 20:20:40 alistair-XPS13 kernel: [   85.892063] cx231xx 1-2:1.1:
Registered VBI device vbi0
Mar  3 20:20:40 alistair-XPS13 kernel: [   85.892069] cx231xx 1-2:1.1:
video EndPoint Addr 0x84, Alternate settings: 5
Mar  3 20:20:40 alistair-XPS13 kernel: [   85.892073] cx231xx 1-2:1.1:
VBI EndPoint Addr 0x85, Alternate settings: 2
Mar  3 20:20:40 alistair-XPS13 kernel: [   85.892077] cx231xx 1-2:1.1:
sliced CC EndPoint Addr 0x86, Alternate settings: 2
Mar  3 20:20:40 alistair-XPS13 kernel: [   85.892274] usbcore:
registered new interface driver cx231xx
Mar  3 20:20:40 alistair-XPS13 kernel: [   85.908439] cx231xx 1-2:1.1:
audio EndPoint Addr 0x83, Alternate settings: 3
Mar  3 20:20:40 alistair-XPS13 kernel: [   85.908444] cx231xx 1-2:1.1:
Cx231xx Audio Extension initialized
Mar  3 20:20:40 alistair-XPS13 pulseaudio[3326]: [pulseaudio]
source.c: Default and alternate sample rates are the same.
Mar  3 20:20:40 alistair-XPS13 rtkit-daemon[2253]: Successfully made
thread 3993 of process 3326 (n/a) owned by '1000' RT at priority 5.
Mar  3 20:20:40 alistair-XPS13 rtkit-daemon[2253]: Supervising 5
threads of 1 processes of 1 users.
M

Re: Control message failures kill entire XHCI stack

2015-02-27 Thread Alistair Grant
Hi Mathias & Devin,

On Thu, Feb 19, 2015 at 3:18 PM, Mathias Nyman
 wrote:
>
> Got one more patch added to the for-usb-next-branch.
> It makes sure we allocate enough scratchpad memory for xhci.
>
> It's one possible cause.
> Patch will anyway go to 3.20, but you can try it out first to see if it helps

My apologies for my slow response, I've been in hospital for almost
two weeks having my gallbladder removed.

I tried recording with the Hauppauge USB Live2 using the following kernel:

* 3.19.0 with the following patches:
* xhci: Allocate correct amount of scratchpad buffers
* xhci: Don't touch TRBs memory if those are no longer on the endpoint ring
* xhci: fix invalid pointer in reset device debugging
* xhci: add debugging for reset device and stop endpoint commands
* xhci: add command ring stop and restart debug messages

Unfortunately it still fails with the xHCI host controller dying.
I've included an extract from syslog which will hopefully show you the
main activity.  The entire syslog (boot, test, shutdown) is available
from: 
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1412121/+attachment/4330313/+files/syslog

Feb 27 11:59:03 alistair-XPS13 kernel: [  154.081591] xhci_hcd
:00:14.0: Cached old ring, 26 rings cached
Feb 27 11:59:03 alistair-XPS13 kernel: [  154.081644] xhci_hcd
:00:14.0: Endpoint 0x83 ep reset callback called
Feb 27 11:59:03 alistair-XPS13 rtkit-daemon[2137]: Successfully made
thread 4191 of process 3296 (n/a) owned by '1000' RT at priority 5.
Feb 27 11:59:03 alistair-XPS13 rtkit-daemon[2137]: Supervising 5
threads of 1 processes of 1 users.
Feb 27 11:59:03 alistair-XPS13 kernel: [  154.087381] xhci_hcd
:00:14.0: ERROR no room on ep ring, try ring expansion
Feb 27 11:59:03 alistair-XPS13 kernel: [  154.087393] xhci_hcd
:00:14.0: ring expansion succeed, now has 4 segments
Feb 27 11:59:03 alistair-XPS13 kernel: [  154.087411] xhci_hcd
:00:14.0: ERROR no room on ep ring, try ring expansion
Feb 27 11:59:03 alistair-XPS13 kernel: [  154.087415] xhci_hcd
:00:14.0: ring expansion succeed, now has 8 segments
Feb 27 11:59:08 alistair-XPS13 kernel: [  159.090518] xhci_hcd
:00:14.0: Cancel URB 8801f949f000, dev 2, ep 0x83, starting at
offset 0x1ff9d03b0
Feb 27 11:59:08 alistair-XPS13 kernel: [  159.090531] xhci_hcd
:00:14.0: Starting stop cmd watchdog timer for slot  4 ep index 6.
Feb 27 11:59:08 alistair-XPS13 kernel: [  159.090544] xhci_hcd
:00:14.0: cmdring ctrl reg before ringing 0x8
Feb 27 11:59:08 alistair-XPS13 kernel: [  159.090547] xhci_hcd
:00:14.0: // Ding dong!
Feb 27 11:59:08 alistair-XPS13 kernel: [  159.090554] xhci_hcd
:00:14.0: cmdring ctrl reg after ringing 0x8
Feb 27 11:59:08 alistair-XPS13 kernel: [  159.090590] xhci_hcd
:00:14.0: Stopped on Transfer TRB
Feb 27 11:59:08 alistair-XPS13 kernel: [  159.090602] xhci_hcd
:00:14.0: Removing canceled TD starting at 0x1ff9d0690 (dma).
Feb 27 11:59:08 alistair-XPS13 kernel: [  159.090607] xhci_hcd
:00:14.0: Finding endpoint context
Feb 27 11:59:08 alistair-XPS13 kernel: [  159.090612] xhci_hcd
:00:14.0: Cycle state = 0x0
Feb 27 11:59:08 alistair-XPS13 kernel: [  159.090616] xhci_hcd
:00:14.0: New dequeue segment = 880214e25940 (virtual)
Feb 27 11:59:08 alistair-XPS13 kernel: [  159.090620] xhci_hcd
:00:14.0: New dequeue pointer = 0x1ff9d06a0 (DMA)
Feb 27 11:59:08 alistair-XPS13 kernel: [  159.090624] xhci_hcd
:00:14.0: Removing canceled TD starting at 0x1ff9d06a0 (dma).
Feb 27 11:59:08 alistair-XPS13 kernel: [  159.090628] xhci_hcd
:00:14.0: TRB to noop at offset 0x1ff9d06a0
Feb 27 11:59:08 alistair-XPS13 kernel: [  159.090631] xhci_hcd
:00:14.0: Removing canceled TD starting at 0x1ff9d06b0 (dma).
Feb 27 11:59:08 alistair-XPS13 kernel: [  159.090635] xhci_hcd
:00:14.0: TRB to noop at offset 0x1ff9d06b0

...

Feb 27 11:59:08 alistair-XPS13 kernel: [  159.090828] xhci_hcd
:00:14.0: event_trb is a no-op TRB. Skip it
Feb 27 11:59:08 alistair-XPS13 kernel: [  159.090833] xhci_hcd
:00:14.0: xhci_drop_endpoint called for udev 880208e61000
Feb 27 11:59:08 alistair-XPS13 kernel: [  159.090835] xhci_hcd
:00:14.0: Removing canceled TD starting at 0x1ff9d07b0 (dma).
Feb 27 11:59:08 alistair-XPS13 kernel: [  159.090838] xhci_hcd
:00:14.0: TRB to noop at offset 0x1ff9d07b0
Feb 27 11:59:08 alistair-XPS13 kernel: [  159.090842] xhci_hcd
:00:14.0: Removing canceled TD starting at 0x1ff9d07c0 (dma).
Feb 27 11:59:08 alistair-XPS13 kernel: [  159.090845] xhci_hcd
:00:14.0: TRB to noop at offset 0x1ff9d07c0

...

Feb 27 11:59:08 alistair-XPS13 kernel: [  159.094764] xhci_hcd
:00:14.0: Cached old ring, 27 rings cached
Feb 27 11:59:08 alistair-XPS13 kernel: [  159.094789] xhci_hcd
:00:14.0: Cancel URB 8801f949e000, dev 2, ep 0x83, starting at
offset 0x1ff9cf3d0
Feb 27 11:59:08 alistair-XPS13 kernel: [  159.094794] xhci_hcd
:00:14.0: Cancel URB NOT on current ring
Feb 27 11:59:08 alistair-XPS13 kernel: [  159.094

Re: Control message failures kill entire XHCI stack

2015-02-04 Thread Alistair Grant
Hi Mathias,

On Wed, Feb 4, 2015 at 5:26 PM, Mathias Nyman
 wrote:
> On 27.01.2015 00:20, Alistair Grant wrote:
>> I've come across what appears to be another xHCI issue - attempting to
>> format a disk with gparted is causing a kernel Oops.  This may not be
>> related to the issue you're currently investigating, but wanted to
>> pass it on in case it is (if it isn't let me know and I'll either keep
>> quiet or raise it separately, whatever you prefer).
>>
>> I can easily reproduce the crash running 3.19rc6 with Mathias
>> additional error and debugging messages (debugging switched off).  I
>> wasn't able to reproduce the issue with xhci debugging enabled, i.e.:
>>
>
> I got a some new patches to test, added to the same for-usb-next-test
> branch in git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git
>
> Alistair, did you manage to Oops the kernel without the debug patches?
> It might be caused by them (trying to access non-existing ep->ring->td_list 
> pointer).
>
> If you are able to reproduce the HDD gparted format oops on a normal (without 
> my testpaches)
> kernel then I think it should be reported as a separate issue.

You're correct, the Oops only happened with your original set of
patches.  After adding the latest patches it no longer occurs.

However I still get other errors, with vanilla 3.19rc7, your patched
3.19rc5 kernel and my 3.19rc7 with all your patches applied.  All of
the log reports look similar to me.

I'll submit a separate report for this issue.

Thanks,
Alistair
--
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: Control message failures kill entire XHCI stack

2015-01-26 Thread Alistair Grant
I've come across what appears to be another xHCI issue - attempting to
format a disk with gparted is causing a kernel Oops.  This may not be
related to the issue you're currently investigating, but wanted to
pass it on in case it is (if it isn't let me know and I'll either keep
quiet or raise it separately, whatever you prefer).

I can easily reproduce the crash running 3.19rc6 with Mathias
additional error and debugging messages (debugging switched off).  I
wasn't able to reproduce the issue with xhci debugging enabled, i.e.:

echo -n 'module xhci_hcd =p' > /sys/kernel/debug/dynamic_debug/control

Just enabling debugging for xhci-ring.c, i.e.:

echo -n 'file drivers/usb/host/xhci-ring.c =p' >
/sys/kernel/debug/dynamic_debug/control

allowed me to catch the crash with a bit more info.

I've attached the full log at:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1412121/+attachment/4306068/+files/syslog-3.19rc6-m0.1-gparted.log

The end of the log is:

Jan 26 22:33:27 alistair-XPS13 kernel: [  908.402917] xhci_hcd
:00:14.0: Stalled endpoint
Jan 26 22:33:27 alistair-XPS13 kernel: [  908.402933] xhci_hcd
:00:14.0: cmdring ctrl reg before ringing 0x8
Jan 26 22:33:27 alistair-XPS13 kernel: [  908.402937] xhci_hcd
:00:14.0: // Ding dong!
Jan 26 22:33:27 alistair-XPS13 kernel: [  908.402943] xhci_hcd
:00:14.0: cmdring ctrl reg after ringing 0x8
Jan 26 22:33:27 alistair-XPS13 kernel: [  908.402949] xhci_hcd
:00:14.0: Giveback URB 88020554dcc0, len = 0, expected = 512,
status = -32
Jan 26 22:33:28 alistair-XPS13 kernel: [  909.450041] xhci_hcd
:00:14.0: Stalled endpoint
Jan 26 22:33:28 alistair-XPS13 kernel: [  909.450049] xhci_hcd
:00:14.0: cmdring ctrl reg before ringing 0x8
Jan 26 22:33:28 alistair-XPS13 kernel: [  909.450050] xhci_hcd
:00:14.0: // Ding dong!
Jan 26 22:33:28 alistair-XPS13 kernel: [  909.450053] xhci_hcd
:00:14.0: cmdring ctrl reg after ringing 0x8
Jan 26 22:33:28 alistair-XPS13 kernel: [  909.450055] xhci_hcd
:00:14.0: Giveback URB 88020554d3c0, len = 0, expected = 512,
status = -32
Jan 26 22:33:28 alistair-XPS13 kernel: [  909.450256] xhci_hcd
:00:14.0: Stalled endpoint
Jan 26 22:33:28 alistair-XPS13 kernel: [  909.450258] xhci_hcd
:00:14.0: cmdring ctrl reg before ringing 0x8
Jan 26 22:33:28 alistair-XPS13 kernel: [  909.450259] xhci_hcd
:00:14.0: // Ding dong!
Jan 26 22:33:28 alistair-XPS13 kernel: [  909.450261] xhci_hcd
:00:14.0: cmdring ctrl reg after ringing 0x8
Jan 26 22:33:28 alistair-XPS13 kernel: [  909.450263] xhci_hcd
:00:14.0: Giveback URB 880203f6f000, len = 0, expected = 13,
status = -32
Jan 26 22:33:59 alistair-XPS13 kernel: [  940.174704] xhci_hcd
:00:14.0: Starting stop cmd watchdog timer for slot  6 ep index 2.
Jan 26 22:33:59 alistair-XPS13 kernel: [  940.174713] xhci_hcd
:00:14.0: cmdring ctrl reg before ringing 0x8
Jan 26 22:33:59 alistair-XPS13 kernel: [  940.174716] xhci_hcd
:00:14.0: // Ding dong!
Jan 26 22:33:59 alistair-XPS13 kernel: [  940.174721] xhci_hcd
:00:14.0: cmdring ctrl reg after ringing 0x8
Jan 26 22:33:59 alistair-XPS13 kernel: [  940.174728] xhci_hcd
:00:14.0: Stopped on Transfer TRB
Jan 26 22:33:59 alistair-XPS13 kernel: [  940.174738] xhci_hcd
:00:14.0: cmdring ctrl reg before ringing 0x8
Jan 26 22:33:59 alistair-XPS13 kernel: [  940.174739] xhci_hcd
:00:14.0: // Ding dong!
Jan 26 22:33:59 alistair-XPS13 kernel: [  940.174743] xhci_hcd
:00:14.0: cmdring ctrl reg after ringing 0x8
Jan 26 22:33:59 alistair-XPS13 kernel: [  940.229892] xhci_hcd
:00:14.0: Port Status Change Event for port 2
Jan 26 22:33:59 alistair-XPS13 kernel: [  940.229901] xhci_hcd
:00:14.0: handle_port_status: starting port polling.
Jan 26 22:33:59 alistair-XPS13 kernel: [  940.286659] xhci_hcd
:00:14.0: cmdring ctrl reg before ringing 0x8
Jan 26 22:33:59 alistair-XPS13 kernel: [  940.28] xhci_hcd
:00:14.0: // Ding dong!
Jan 26 22:33:59 alistair-XPS13 kernel: [  940.286672] xhci_hcd
:00:14.0: cmdring ctrl reg after ringing 0x8
Jan 26 22:33:59 alistair-XPS13 kernel: [  940.286817] xhci_hcd
:00:14.0: Completed reset device command.
Jan 26 22:33:59 alistair-XPS13 kernel: [  940.286871] BUG: unable to
handle kernel NULL pointer dereference at 0040
Jan 26 22:33:59 alistair-XPS13 kernel: [  940.286955] IP:
[] xhci_discover_or_reset_device+0x33d/0x520
Jan 26 22:33:59 alistair-XPS13 kernel: [  940.287030] PGD 0
Jan 26 22:33:59 alistair-XPS13 kernel: [  940.287054] Oops:  [#1] SMP
Jan 26 22:33:59 alistair-XPS13 kernel: [  940.287090] Modules linked
in: nls_iso8859_1 uas usb_storage ctr ccm pci_stub vboxpci(OE)
vboxnetadp(OE) vboxnetflt(OE) vboxdrv(OE) xt_CHECKSUM iptable_mangle
xt_tcpudp bridge stp llc iptable_filter ip_tables x_tables arc4 iwlmvm
mac80211 dm_crypt rpcsec_gss_krb5 nfsv4 intel_rapl iosf_mbi
x86_pkg_temp_thermal intel_powerclamp kvm_intel kvm iwlwifi uvcvideo
videobuf2_vmalloc videobuf2_memops snd_hda_codec_realtek
snd_hda_cod

Re: Control message failures kill entire XHCI stack

2015-01-26 Thread Alistair Grant
On Mon, Jan 26, 2015 at 4:37 AM, Devin Heitmueller
 wrote:
> Hi Mathias,
>
> Here's an interesting development:  as a result of a related thread on
> linux-media, I came across a patch they are distributing in openelec:
>
> https://github.com/OpenELEC/OpenELEC.tv/commit/b636927dec20652ff020e54ed7838a2e9be51e03
>
> Now I'm not saying that reverting the commit in question is the
> "right" thing to do, but I applied this patch and for the first time
> in 100+ tests it started to work (i.e. I'm not seeing the XHCI hcd
> tear down all the attached devices).
>
> Given what I've seen of the bug I cannot really explain why the
> scatter gather list sizes would have any bearing on TRBs for USB
> control messages to be added to the queue.  Perhaps we're hitting the
> upper bound of the list?  Any further speculation on my part would
> just make me look clueless...
>
> It would be great if you could offer any insight as to why the patch
> in question could be responsible for the behavior we're seeing.  I
> would really rather not just blindly check this patch into my local
> tree and declare "victory" without understanding the underlying issue
> and whether it's likely to cause other problems.

Just as another point of data...  I applied the patch to Mathias
for-usb-next-test branch and was able to record video and audio
through the Live2 for the first time using the xhci stack.  Attempting
to record a second video resulted in mythffmpeg returning:

[alsa @ 0xbf6560] cannot open audio device
plughw:CARD=Cx231xxAudio,DEV=0 (Device or resource busy)
plughw:CARD=Cx231xxAudio,DEV=0: Input/output error

I also get this error on the older hardware running ehci, although it
can be recovered by removing and re-inserting the Live2.  Attempting
to do this under xhci resulted in usb locking up somehow, lsusb never
returned.

syslog file at:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1412121/+attachment/4305645/+files/2015-01-26-syslog-14.10-3.19rc5akg0.1.log

Thanks again to you both,
Alistair
--
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: Control message failures kill entire XHCI stack

2015-01-22 Thread Alistair Grant
Hi Matthias,

On Thu, Jan 22, 2015 at 12:22 PM, Mathias Nyman
 wrote:
> On 21.01.2015 21:16, Alistair Grant wrote:
>> Hi Matthias,
>>
>> On Tue, Jan 20, 2015 at 10:22 AM, Mathias Nyman
>>  wrote:
>>> On 19.01.2015 22:02, Devin Heitmueller wrote:
>>>> Hi Mathias,
>>>>
>>>> Thanks for getting back to me.
>>>>
>>>>> There are a couple of xhci bugs triggered by dvb devices:
>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=75521
>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=65021
>>>>>
>>>>> The first one (75521) I believe is mostly fixed by patches in 3.18 and 
>>>>> early
>>>>> 3.19-rc, so work on a 3.19-rc kernel to eliminate those issues.
>>>> ...
>>>>> The second bug (65021) looks more like your case, it queues two 
>>>>> stop_endpoints
>>>>> commands almost simultaneously, which end up never completing, ->timeout 
>>>>> and tear down xhci.
>>>>> That bug has a debug patch for command ring status, you could try it out 
>>>>> to check if
>>>>> the command queue is running among other details.
>>>>
>>>> The second bug definitely looks like what I'm seeing.  I'll try your
>>>> tree and report back my findings.
>>>
>>> Ah, let me rebase that tree on top of a more current kernel first.
>>
>> I've been following this as it somewhat resembles the problems I'm
>> experiencing, which I've just formally reported in "Hauppauge
>> USB-Live2 recording fails on USB3 port".
>>
>> It looks like you did the rebase yesterday, so I tried running your
>> updated kernel.  It didn't solve the problems, but was an incremental
>> improvement since the mainline 3.19rc5 kernel hangs during shutdown,
>> while your modified kernel shutdown cleanly.
>>
>> I've got a syslog with verbose xhci logging (echo -n 'module xhci_hcd
>> =p' > /sys/kernel/debug/dynamic_debug/control as you suggested above)
>> if you would like to take a look.  It's 2.4M so please let me know how
>> you would like me to pass it on.
>
> I'm Interested yes, anyway is fine. Add it somewhere (bugreport, webpage) and 
> send me the link,
> or just sending it as an email attachent to me works fine as well.

You should be able to download it from:

https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1412121/+attachment/4303563/+files/syslog-14.10-3.19rc5mathias0.2-2.log

Please let me know if you have any trouble accessing it, or would like
more testing, information, etc.

Thanks,
Alistair
--
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: Control message failures kill entire XHCI stack

2015-01-21 Thread Alistair Grant
Hi Matthias,

On Tue, Jan 20, 2015 at 10:22 AM, Mathias Nyman
 wrote:
> On 19.01.2015 22:02, Devin Heitmueller wrote:
>> Hi Mathias,
>>
>> Thanks for getting back to me.
>>
>>> There are a couple of xhci bugs triggered by dvb devices:
>>> https://bugzilla.kernel.org/show_bug.cgi?id=75521
>>> https://bugzilla.kernel.org/show_bug.cgi?id=65021
>>>
>>> The first one (75521) I believe is mostly fixed by patches in 3.18 and early
>>> 3.19-rc, so work on a 3.19-rc kernel to eliminate those issues.
>> ...
>>> The second bug (65021) looks more like your case, it queues two 
>>> stop_endpoints
>>> commands almost simultaneously, which end up never completing, ->timeout 
>>> and tear down xhci.
>>> That bug has a debug patch for command ring status, you could try it out to 
>>> check if
>>> the command queue is running among other details.
>>
>> The second bug definitely looks like what I'm seeing.  I'll try your
>> tree and report back my findings.
>
> Ah, let me rebase that tree on top of a more current kernel first.

I've been following this as it somewhat resembles the problems I'm
experiencing, which I've just formally reported in "Hauppauge
USB-Live2 recording fails on USB3 port".

It looks like you did the rebase yesterday, so I tried running your
updated kernel.  It didn't solve the problems, but was an incremental
improvement since the mainline 3.19rc5 kernel hangs during shutdown,
while your modified kernel shutdown cleanly.

I've got a syslog with verbose xhci logging (echo -n 'module xhci_hcd
=p' > /sys/kernel/debug/dynamic_debug/control as you suggested above)
if you would like to take a look.  It's 2.4M so please let me know how
you would like me to pass it on.

Thanks very much,
Alistair
--
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: 3.17-rc6 on ODROID: ERROR: Bad of_node_put() on /ehci@12580000/port@1

2014-10-01 Thread Grant Likely
On Wed, Oct 1, 2014 at 4:12 PM, Daniel Drake  wrote:
> On Wed, Oct 1, 2014 at 12:36 AM, Vivek Gautam  
> wrote:
>> One reason i doubt why it could be coming is because we are
>> specifically putting the
>> child after doing everything with it.
>>
>> When we are getting the child node using for_each_available_child_of_node(),
>> which calls for of_get_next_available_child(). So 
>> of_get_next_available_child()
>> does a of_node_put() on the "prev" node, in case we have siblings to the 
>> child.
>>
>> Can you see if the below change helps ?
>>
>> 
>> diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
>> index 7189f2e..1b726bf 100644
>> --- a/drivers/usb/host/ehci-exynos.c
>> +++ b/drivers/usb/host/ehci-exynos.c
>> @@ -74,7 +74,6 @@ static int exynos_ehci_get_phy(struct device *dev,
>>
>> phy = devm_of_phy_get(dev, child, NULL);
>> exynos_ehci->phy[phy_number] = phy;
>> -   of_node_put(child);
>> if (IS_ERR(phy)) {
>> ret = PTR_ERR(phy);
>> if (ret == -EPROBE_DEFER) {
>> 
>>
>>
>> This is on top of usb-next.
>> If you are testing on rc6 only, then probably you will have to cherrypick two
>> patches each for ehci-exynos and ohci-exynos:
>> usb: host: ehci-exynos: Remove unnecessary usb-phy support
>> usb: host: ohci-exynos: Remove unnecessary usb-phy support
>
> I made the equivalent change to 3.17-rc7 (right now 3.17 is my main
> interest), i.e. removed all of_node_put calls from
> exynos_ehci_get_phy(). Same change is needed in exynos_ohci_get_phy().
> Now the warnings are gone.
> BTW, I think the warning only appeared when CONFIG_OF_SELFTEST=y
>
> I didn't check the implementation details like you did, but I looked
> at a few other users of for_each_available_child_of_node and it looks
> like indeed you do not need to call of_node_put() on the children in
> the normal case, or at least, nobody else does.

CONFIG_OF_SELFTEST enables CONFIG_OF_DYNAMIC, and reference counting
is only implemented when OF_DYNAMIC is enabled. That's probably why
selftest exposes the problem.

g.
--
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: disable VBUS?

2014-07-14 Thread Grant
>> >> I should have been more specific then.  I think I understand, but to
>> >> be sure, if I splice a USB cable, power VBUS with an external 5V power
>> >> supply, and connect a host and device, the device will work as if the
>> >> cable was connected to the host normally?
>> >
>> > Yes.  But why do you want to do this?
>>
>>
>> My application calls for low noise so I'm hoping to feed a clean 5V
>> from a battery.
>
> Okay.  Not much you can do about noise on the USB data lines, though.


Any ideas for minimizing that are most welcome.

- Grant
--
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: disable VBUS?

2014-07-14 Thread Grant
>> >> You are right, the device will get 5 volts. However it is still part
>> >> of the spec that 2ms without a SOF will force the device into suspend
>> >> mode, so it will use less than 2ma. So depending on your definition of
>> >> function, there will be power, but no activity. If it uses more than
>> >> 2ma, from the bus, problems will ensue.
>> >
>> > What you're saying is true, so long as the spliced USB cable isn't
>> > plugged into a host.  But of course, you wouldn't expect to get any
>> > activity on a device when it isn't plugged into a host -- even if you
>> > use a normal cable.
>> >
>> > If the spliced USB cable _is_ plugged into a host then the host will
>> > see it, issue a reset, and start sending SOF packets.  In that
>> > situation your comment doesn't apply.
>>
>>
>> I should have been more specific then.  I think I understand, but to
>> be sure, if I splice a USB cable, power VBUS with an external 5V power
>> supply, and connect a host and device, the device will work as if the
>> cable was connected to the host normally?
>
> Yes.  But why do you want to do this?


My application calls for low noise so I'm hoping to feed a clean 5V
from a battery.

- Grant
--
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: disable VBUS?

2014-07-08 Thread Grant
>> You are right, the device will get 5 volts. However it is still part
>> of the spec that 2ms without a SOF will force the device into suspend
>> mode, so it will use less than 2ma. So depending on your definition of
>> function, there will be power, but no activity. If it uses more than
>> 2ma, from the bus, problems will ensue.
>
> What you're saying is true, so long as the spliced USB cable isn't
> plugged into a host.  But of course, you wouldn't expect to get any
> activity on a device when it isn't plugged into a host -- even if you
> use a normal cable.
>
> If the spliced USB cable _is_ plugged into a host then the host will
> see it, issue a reset, and start sending SOF packets.  In that
> situation your comment doesn't apply.


I should have been more specific then.  I think I understand, but to
be sure, if I splice a USB cable, power VBUS with an external 5V power
supply, and connect a host and device, the device will work as if the
cable was connected to the host normally?

- Grant
--
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: disable VBUS?

2014-07-07 Thread Grant
> it isn't possible to disable Vbus on a USB host port and still use the
> port.  To do what you want, you would have to physically cut the Vbus
> wire in the USB cable and splice the device side of that wire to the
> external power supply.


I think I'll try that.  Can anyone confirm that it should work?  If I
connect a 5V external power supply to the VBUS wire in a USB cable,
the connected USB device should still function?

- Grant
--
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: Jetson TK1 USB support?

2014-06-28 Thread Grant
>> The following wiki page indicates that USB3 is not working on the Jetson TK1:
>>
>> http://elinux.org/Tegra/Boards/NVIDIA_Jetson_TK1
>>
>> Does that mean USB is not working at all or should USB2 work?
>
> USB-2 won't work if the board includes only a USB-3 controller.


Actually this thread seems to indicate there is an ehci controller on
the Jetson TK1 so that's good news:

https://devtalk.nvidia.com/default/topic/752246/jetson-tk1-usb-webcam-logitech-c260-is-working-via-hub/


>>  I would
>> have thought USB2 would still work but I think I tried disabling
>> xhci_hcd and enabling ehci_hcd/ohci_hcd on my USB3 laptop and it
>> didn't work.
>
> What exactly do you mean when you say it didn't work?  Does your USB-3
> laptop contain any EHCI or OHCI controllers?


>From memory, my Dell XPS 13 laptop wasn't able to detect any devices
without xhci enabled.  It looks like it does have ehci controllers so
maybe something else was going on:

00:14.0 USB controller: Intel Corporation 7 Series/C210 Series Chipset
Family USB xHCI Host Controller (rev 04)
00:1a.0 USB controller: Intel Corporation 7 Series/C210 Series Chipset
Family USB Enhanced Host Controller #2 (rev 04)
00:1d.0 USB controller: Intel Corporation 7 Series/C210 Series Chipset
Family USB Enhanced Host Controller #1 (rev 04)

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


Jetson TK1 USB support?

2014-06-27 Thread Grant
The following wiki page indicates that USB3 is not working on the Jetson TK1:

http://elinux.org/Tegra/Boards/NVIDIA_Jetson_TK1

Does that mean USB is not working at all or should USB2 work?  I would
have thought USB2 would still work but I think I tried disabling
xhci_hcd and enabling ehci_hcd/ohci_hcd on my USB3 laptop and it
didn't work.

http://www.newegg.com/Product/Product.aspx?Item=N82E16813190005

- Grant
--
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: disable VBUS?

2014-06-24 Thread Grant
>> Can I disable VBUS while keeping the rest of USB functional for a
>> device that does not require bus power?
>
> unfortunately not, your device would see a disconnection. The reason is
> that even though you don't really put any load on the bus, the PHY still
> samples VBUS levels to know when the session is valid (VBUS > 4.4V).


Is that part of the USB spec or is it a Linux driver convention?

- Grant
--
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: disable VBUS?

2014-06-10 Thread Grant
>> >> >> Can I disable VBUS while keeping the rest of USB functional for a
>> >> >> device that does not require bus power?
>> >> >
>> >> > unfortunately not, your device would see a disconnection. The reason is
>> >> > that even though you don't really put any load on the bus, the PHY still
>> >> > samples VBUS levels to know when the session is valid (VBUS > 4.4V).
>> >>
>> >> What if I use a USB cable with a separate DC input for the VBUS?
>> >
>> > What would be the point?  I mean, if you've got a separate DC input for
>> > Vbus then you aren't really saving any power or disabling anything.
>>
>> The point would be to minimize the amount of electrical noise the USB
>> device is exposed to by using a low-noise supply for this such as a
>> battery with a linear regulator.
>
> Okay.  Is Vbus noise really a problem for you?

I'm trying to see how far I can minimize the noise.

>> > If you did this and then turned off the separate DC power, your device
>> > wouldn't work afterward.  As Felipe pointed out, the device would think
>> > the cable had been disconnected.
>>
>> Understood.  Can you tell me how to disable VBUS on the USB
>> hub/controller?  I'll be attempting this on a Beaglebone.
>
> I don't know what controller the Beaglebone uses.  In general, however,
> it isn't possible to disable Vbus on a USB host port and still use the
> port.  To do what you want, you would have to physically cut the Vbus
> wire in the USB cable and splice the device side of that wire to the
> external power supply.

USB cables like that do exist for sale.  Is there a way to disable
VBUS on the host port if I use a cable like that?

- Grant
--
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: disable VBUS?

2014-06-10 Thread Grant
>> The reason is
>> that even though you don't really put any load on the bus, the PHY still
>> samples VBUS levels to know when the session is valid (VBUS > 4.4V).
>
> Hmmm that might be the cause of some 'random' USB disconnects
> we were seeing.
>
> We 'fixed' them by cutting the wire in the usb cable and driving the
> devices '5v sense' from its local 5v rail.
> (The USB device is permanently connected to the motherboard.)
>
> The ochi driver did recover from the disconnect, but xhci got
> terribly confused.

I've noticed this type of thing with xhci (only) as well.

- Grant
--
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: disable VBUS?

2014-06-10 Thread Grant
>> >> Can I disable VBUS while keeping the rest of USB functional for a
>> >> device that does not require bus power?
>> >
>> > unfortunately not, your device would see a disconnection. The reason is
>> > that even though you don't really put any load on the bus, the PHY still
>> > samples VBUS levels to know when the session is valid (VBUS > 4.4V).
>>
>> What if I use a USB cable with a separate DC input for the VBUS?
>
> What would be the point?  I mean, if you've got a separate DC input for
> Vbus then you aren't really saving any power or disabling anything.

The point would be to minimize the amount of electrical noise the USB
device is exposed to by using a low-noise supply for this such as a
battery with a linear regulator.

> If you did this and then turned off the separate DC power, your device
> wouldn't work afterward.  As Felipe pointed out, the device would think
> the cable had been disconnected.

Understood.  Can you tell me how to disable VBUS on the USB
hub/controller?  I'll be attempting this on a Beaglebone.

- Grant
--
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: disable VBUS?

2014-06-07 Thread Grant
>> Can I disable VBUS while keeping the rest of USB functional for a
>> device that does not require bus power?
>
> unfortunately not, your device would see a disconnection. The reason is
> that even though you don't really put any load on the bus, the PHY still
> samples VBUS levels to know when the session is valid (VBUS > 4.4V).

What if I use a USB cable with a separate DC input for the VBUS?

- Grant
--
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: disable VBUS?

2014-05-14 Thread Grant
>> Can I disable VBUS while keeping the rest of USB functional for a
>> device that does not require bus power?
>
>
> Can you please elaborate the question of why VBus should go off ? Is this
> question in the context of any new USB specification ?

It's not related to a USB spec, it's just part of a project I'm working on.

- Grant
--
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: disable VBUS?

2014-05-14 Thread Grant
>> Can I disable VBUS while keeping the rest of USB functional for a
>> device that does not require bus power?
>
> unfortunately not, your device would see a disconnection. The reason is
> that even though you don't really put any load on the bus, the PHY still
> samples VBUS levels to know when the session is valid (VBUS > 4.4V).


Darn.  My goals are to minimize power usage and the amount of
electrical noise traveling to my USB device so I was hoping to
eliminate the otherwise unused VBUS.  Any creative ways to pull this
off?

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


disable VBUS?

2014-05-13 Thread Grant
Can I disable VBUS while keeping the rest of USB functional for a
device that does not require bus power?

- Grant
--
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 net-next 12/12] r8152: modify the tx timeout funcfion

2014-03-25 Thread Grant Grundler
On Tue, Mar 4, 2014 at 4:01 AM, Hayes Wang  wrote:
> Reset and reinitialize the device when the tx timeout occurs.

Hayes,
I believe this patch was dropped after the series was split.
Can you please repost this patch by itself?

(and fix the "function" typo in the patch header)

>
> Signed-off-by: Hayes Wang 
> ---
>  drivers/net/usb/r8152.c | 41 +++--
>  1 file changed, 31 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index e04fcbd..23e03a6 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -1791,16 +1791,6 @@ static void rtl_drop_queued_tx(struct r8152 *tp)
> }
>  }
>
> -static void rtl8152_tx_timeout(struct net_device *netdev)
> -{
> -   struct r8152 *tp = netdev_priv(netdev);
> -   int i;
> -
> -   netif_warn(tp, tx_err, netdev, "Tx timeout\n");
> -   for (i = 0; i < RTL8152_MAX_TX; i++)
> -   usb_unlink_urb(tp->tx_info[i].urb);
> -}
> -
>  static void rtl8152_set_rx_mode(struct net_device *netdev)
>  {
> struct r8152 *tp = netdev_priv(netdev);
> @@ -3177,6 +3167,37 @@ out:
> return res;
>  }
>
> +static void rtl8152_tx_timeout(struct net_device *netdev)
> +{
> +   struct r8152 *tp = netdev_priv(netdev);
> +
> +   netif_warn(tp, tx_err, netdev, "Tx timeout\n");
> +
> +   if (usb_autopm_get_interface(tp->intf) < 0)
> +   return;
> +
> +   netif_stop_queue(netdev);
> +   clear_bit(WORK_ENABLE, &tp->flags);
> +   usb_kill_urb(tp->intr_urb);
> +   cancel_delayed_work_sync(&tp->schedule);
> +   tp->rtl_ops.down(tp);
> +
> +   usb_reset_device(tp->udev);
> +
> +   tp->rtl_ops.init(tp);
> +   tp->rtl_ops.up(tp);
> +   rtl8152_set_speed(tp, AUTONEG_ENABLE,
> + tp->mii.supports_gmii ? SPEED_1000 : SPEED_100,
> + DUPLEX_FULL);
> +   tp->speed = 0;

Nit: Could rtl_ops.up() set speed since it appears to be changing the
state of the link?

rtl8152_open() uses a remarkably similar code sequence. Is there an
opportunity to refactor and  make sure this sequence is consistent?
(different patch, not this one)

cheers,
grant

> +   netif_carrier_off(netdev);
> +   netif_start_queue(netdev);
> +   set_bit(WORK_ENABLE, &tp->flags);
> +   usb_submit_urb(tp->intr_urb, GFP_KERNEL);
> +
> +   usb_autopm_put_interface(tp->intf);
> +}
> +
>  static const struct net_device_ops rtl8152_netdev_ops = {
> .ndo_open   = rtl8152_open,
> .ndo_stop   = rtl8152_close,
> --
> 1.8.4.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>
>
--
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: usbnet: driver_info->stop required to stop USB interrupts?

2014-03-24 Thread Grant Grundler
On Fri, Mar 21, 2014 at 1:33 AM, Oliver Neukum  wrote:
...
> Very well. Thorough testing is good. I'll wait for the result. Could
> you notify me of the final outcome?

Ship it. :)

The testing so far has completed over 15K iterations of unload/reload
of the asix driver on the original failing system with no failures so
far. Previously I could reproduce the problem in 2K-8K iterations.

The caveat is due to various issues with my test environment, the test
never ran more than 7K iterations continuously. The ~15k is spread out
across four invocations (2K, 3K, 7K, 4K) of the previously shared
"for" loop.

Please add my Reported-by + Tested-by to this patch.

thanks
grant
--
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: usbnet: driver_info->stop required to stop USB interrupts?

2014-03-21 Thread Grant Grundler
On Fri, Mar 21, 2014 at 1:33 AM, Oliver Neukum  wrote:
...
> Very well. Thorough testing is good. I'll wait for the result. Could
> you notify me of the final outcome?

Ceratinly. Bad news is I had to restart my workstation that was
driving the test due to completely different issue (loopbacks hung -
known bug). reload_asix ran about 2000 iterations with no problems on
AX88178 part.
It's running again now on the same target machine.

> And, Julius, I owe you an apology. The check is necessary.

++oliver

Sorry to harp on this more, but I still think a comment is warranted:
  e.g. "Don't resend packets during shutdown of this USB net device."

And instead of naming the queue "wait", call the field "shutdown queue".

cheers,
grant
--
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: usbnet: driver_info->stop required to stop USB interrupts?

2014-03-20 Thread Grant Grundler
On Thu, Mar 20, 2014 at 9:35 AM, Grant Grundler  wrote:
> On Thu, Mar 20, 2014 at 12:15 AM, Oliver Neukum  wrote:
> ...
>> I have an idea. Could you test this patch?
> ...
>> -   if (dev->wait) {
> ..
>> +   if (waitqueue_active(&dev->wait)) {
>
> Yes - building new image now (and transfer to USB and boot from USB).
> Should know in an hour or so (doing other things in parallel).

Sorry...took a bit longer since my previous test method (bash
/tmp/reload_asix) was abusing a security exploit that is now
fixed...so had to move my script into a RO executable file system:

for i in `seq 1`; do echo -n "RELOAD $i  " ; ssh $T
"/bin/reload_asix eth0 1000_full" ; J=$? ; if [ $J -eq 255 ] ; then
echo; " SSH timeout" ; break ; fi ; ssh $T "cat
/var/log/reload-asix.out" ; if [ $J -ne 0 ] ; then echo "  ERROR $J" ;
fi ; sleep 3 ; done | tee ~/reload-AX88178-leon-192.168.1.100-06.out

This is running now and things look happy so far. :) This will take
more than 30h to complete.

So please add:
  "Tested-by: Grant Grundler "

> Can you please explain why we need to check if the waitqueue is active?

and add a comment that answers the above question.

thanks!
grant
--
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: usbnet: driver_info->stop required to stop USB interrupts?

2014-03-20 Thread Grant Grundler
On Thu, Mar 20, 2014 at 12:15 AM, Oliver Neukum  wrote:
...
> I have an idea. Could you test this patch?
...
> -   if (dev->wait) {
..
> +   if (waitqueue_active(&dev->wait)) {

Yes - building new image now (and transfer to USB and boot from USB).
Should know in an hour or so (doing other things in parallel).

I was sure the problem is in usbnet_bh() since that's the only code
change I'm actually exercising (so far). The way I was reading the
code, we might see extra wake_up calls...but there is clearly more
going on.

Can you please explain why we need to check if the waitqueue is active?

This patch should also add a comment to hint why waitqueue_active()
must be called.
Why? Several experienced people looked at the code and didn't see the
problem including the original author of the patch.

thanks,
grant
--
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: usbnet: driver_info->stop required to stop USB interrupts?

2014-03-19 Thread Grant Grundler
On Tue, Mar 18, 2014 at 6:40 PM, Grant Grundler  wrote:
> On Tue, Mar 18, 2014 at 6:09 PM, Julius Werner  wrote:
>> I think a better place for this would be in usbnet_probe() (together
>> with all the other dev->xxx initialization).
>
> Definitely better.
>
> @@ -1536,6 +1536,7 @@ usbnet_probe (struct usb_interface *udev, const struct 
> usb
> dev->driver_name = name;
> dev->msg_enable = netif_msg_init (msg_level, NETIF_MSG_DRV
> | NETIF_MSG_PROBE | NETIF_MSG_LINK);
> +   init_waitqueue_head(&dev->wait);
> skb_queue_head_init (&dev->rxq);
> skb_queue_head_init (&dev->txq);
> skb_queue_head_init (&dev->done);

Oliver,
So even with this additional change to usbnet_probe, the device is
reporting a link but can't transmit packets.
I've tried with three different USB dongles (AX88178, AX88772B, SMSC75xx).

In the last case, I ran "ifconfig eth0 192.168.1.100" and then tried
to ping 192.168.1.1. "ifconfig eth0" then reports "RX packets 0 bytes
0" and "TX packets 0 bytes 10796". The TX packets != bytes seems odd
and suggests (to me) that data getting discarded someplace in the
stack. I confirmed by running tcpdump on the other end of the link
(192.168.1.1 side) and got nothing from 192.168.1.100.

I believe the "10796" TX bytes is due to the connection manager
"trying really hard" to get an IP address (via DHCP).

Your patch looks like it should work. But it's missing something and I
don't have a clue what it is. Ideas?

cheers,
grant
--
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: usbnet: driver_info->stop required to stop USB interrupts?

2014-03-18 Thread Grant Grundler
On Tue, Mar 18, 2014 at 6:09 PM, Julius Werner  wrote:
> I think a better place for this would be in usbnet_probe() (together
> with all the other dev->xxx initialization).

Definitely better.

@@ -1536,6 +1536,7 @@ usbnet_probe (struct usb_interface *udev, const struct usb
dev->driver_name = name;
dev->msg_enable = netif_msg_init (msg_level, NETIF_MSG_DRV
| NETIF_MSG_PROBE | NETIF_MSG_LINK);
+   init_waitqueue_head(&dev->wait);
skb_queue_head_init (&dev->rxq);
skb_queue_head_init (&dev->txq);
skb_queue_head_init (&dev->done);


> I'm not quite sure how
> that could cause the transfer problems you are seeing, but at least
> you will no longer initialize the waitqueue multiple times on multiple
> usbnet_open (which is probably a bad idea).

Yeah, I agree. I don't expect usbnet_open would get called multiple
times but I'll try the above anyway.

thanks,
grant
--
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: usbnet: driver_info->stop required to stop USB interrupts?

2014-03-18 Thread Grant Grundler
On Tue, Mar 18, 2014 at 1:51 PM, Grant Grundler  wrote:
> On Mon, Mar 17, 2014 at 2:55 PM, Oliver Neukum  wrote:
>> I am hping for the reporter of the original bug to test it.
>
> Oliver,
> on a haswell system running ChromeOS-3.8 kernel, this patch as-is
> resulted in a "Bad Spinlock Magic" error and subsequent pagefault.
> I believe the sequence was:
>usbnet_open -> tasklet_schedule(dev->bh) -> usbnet_bh -> wake_up
> (&dev->wait) -> panic

BTW, full console output is available on this public patch:
https://code.google.com/p/chromium/issues/detail?id=353648

cheers,
grant

>
> I tried adding the following change on top of your patch but believe
> the plumbing still isn't quite correct since the USB device (eth0) is
> reporting a link but no TX or RX of traffic:
>  @@ -805,6 +807,9 @@ int usbnet_open (struct net_device *net)
> goto done;
> }
>
> +   /* usbnet_bh() expects the spinlock to be initialized. */
> +   init_waitqueue_head(&dev->wait);
> +
> /* hard_mtu or rx_urb_size may change in reset() */
> usbnet_update_max_qlen(dev);
>
> I suspect this hunk of your patch is now causing different problems at
> init time:
> @@ -1438,10 +1440,8 @@ static void usbnet_bh (unsigned long param)
> clear_bit(EVENT_RX_KILL, &dev->flags);
>
> // waiting for all pending urbs to complete?
> -   if (dev->wait) {
> -   if ((dev->txq.qlen + dev->rxq.qlen + dev->done.qlen) == 0) {
> -   wake_up (dev->wait);
> -   }
> +   if ((dev->txq.qlen + dev->rxq.qlen + dev->done.qlen) == 0) {
> +   wake_up (&dev->wait);
>
> // or are we maybe short a few urbs?
> } else if (netif_running (dev->net) &&
>
> Please advise on what you'd like me to try next.
>
> cheers,
> grant
--
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: usbnet: driver_info->stop required to stop USB interrupts?

2014-03-18 Thread Grant Grundler
On Mon, Mar 17, 2014 at 2:55 PM, Oliver Neukum  wrote:
> I am hping for the reporter of the original bug to test it.

Oliver,
on a haswell system running ChromeOS-3.8 kernel, this patch as-is
resulted in a "Bad Spinlock Magic" error and subsequent pagefault.
I believe the sequence was:
   usbnet_open -> tasklet_schedule(dev->bh) -> usbnet_bh -> wake_up
(&dev->wait) -> panic

I tried adding the following change on top of your patch but believe
the plumbing still isn't quite correct since the USB device (eth0) is
reporting a link but no TX or RX of traffic:
 @@ -805,6 +807,9 @@ int usbnet_open (struct net_device *net)
goto done;
}

+   /* usbnet_bh() expects the spinlock to be initialized. */
+   init_waitqueue_head(&dev->wait);
+
/* hard_mtu or rx_urb_size may change in reset() */
usbnet_update_max_qlen(dev);

I suspect this hunk of your patch is now causing different problems at
init time:
@@ -1438,10 +1440,8 @@ static void usbnet_bh (unsigned long param)
clear_bit(EVENT_RX_KILL, &dev->flags);

// waiting for all pending urbs to complete?
-   if (dev->wait) {
-   if ((dev->txq.qlen + dev->rxq.qlen + dev->done.qlen) == 0) {
-   wake_up (dev->wait);
-   }
+   if ((dev->txq.qlen + dev->rxq.qlen + dev->done.qlen) == 0) {
+   wake_up (&dev->wait);

// or are we maybe short a few urbs?
} else if (netif_running (dev->net) &&

Please advise on what you'd like me to try next.

cheers,
grant
--
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: usbnet: driver_info->stop required to stop USB interrupts?

2014-03-17 Thread Grant Grundler
On Mon, Mar 17, 2014 at 2:55 PM, Oliver Neukum  wrote:
> On Mon, 2014-03-17 at 14:53 -0700, Julius Werner wrote:
>> Hi Oliver,
>>
>> Any update on the state of this patch? Did it get picked up for merge
>> somewhere? Do you agree with my suggestion of sticking closer to the
>> original logic instead of adding that autopm_get(), or do you maybe
>> want to add some more reviewers to confirm your code? I don't really
>> care that much which way we choose in the end, I just want to make
>> sure this bug gets fixed and we don't forget about it.
>
> I am hping for the reporter of the original bug to test it.

Ok. I didn't realize I was part of the dependency chain here. :)
It seemed "obvious" to me once Julius described the race.

I'll try it on the offending system but fear I will run into some other problem.

cheers,
grant

>
> Regards
> Oliver
>
>
--
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: usbnet: driver_info->stop required to stop USB interrupts?

2014-03-11 Thread Grant Grundler
On Tue, Mar 11, 2014 at 10:31 AM, Grant Grundler  wrote:
> FWIW, I've reproduced this on "Samsung Chromebook" (Exynos 5250) with

FYA - I just noticed the system crashed on the 2048th iteration :)

...
RELOAD 2046  20140310202523  LINK 1000_full (3 sec)  IP 192.168.1.116 (1 Sec)
RELOAD 2047  20140310202533  LINK 1000_full (2 sec)  IP 192.168.1.116 (1 Sec)
RELOAD 2048   SSH timeout

thought folks would be amused by the coincidence.

grant
--
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: usbnet: driver_info->stop required to stop USB interrupts?

2014-03-11 Thread Grant Grundler
On Tue, Mar 11, 2014 at 10:31 AM, Grant Grundler  wrote:
...
> FWIW, I've reproduced this on "Samsung Chromebook" (Exynos 5250) with
> AX88772 USB dongle using the instructions I posted before (ie bouncing
> the USB port with reload_asix script).

Sorry - with AX88178 dongle - not AX88772
(5 machines on my desk right now).

grant

>
> cheers,
> grant
>
> [23231.533805] asix 3-1:1.0 eth0: link up, 1000Mbps, full-duplex, lpa 0xCDE1
> [23235.755652] usbcore: deregistering interface driver asix
> [23235.761722] asix 3-1:1.0 eth0: unregister 'asix'
> usb-1211.usb-1, ASIX AX88178 USB 2.0 Ethernet
> [23235.761763] Unable to handle kernel paging request at virtual
> address e24cb004
> [23235.761771] pgd = ebf7
> [23235.761777] [e24cb004] *pgd=6241141e(bad)
> [23235.761792] Internal error: Oops: 800d [#1] SMP ARM
> [23235.761798] Modules linked in: asix(-) exynos_gsc v4l2_mem2mem
> isl29018(C) sbs_battery i2c_dev uinput mwifiex_sdio mwifiex
> btmrvl_sdio btmrvl s5p_mfc videobuf2_dma_contig rtc_s3c bluetooth
> zram(C) zsmalloc(C) fuse cfg80211 nf_conntrack_ipv6 nf_defrag_ipv6
> ip6table_filter ip6_tables uvcvideo videobuf2_core videobuf2_vmalloc
> videobuf2_memops usbnet joydev [last unloaded: asix]
> [23235.761898] CPU: 0Tainted: G C(3.8.11 #25)
> [23235.761906] PC is at 0xe24cb004
> [23235.761916] LR is at __wake_up_common+0x5c/0x88
> [23235.761924] pc : []lr : []psr: 8093
> [23235.761924] sp : ef0e3e10  ip : e24cb004  fp : ef0e3e3c
> [23235.761931] r10: e1a0c00d  r9 :   r8 : 0003
> [23235.761938] r7 :   r6 : 0001  r5 : e92d3ff4  r4 : eab13d14
> [23235.761943] r3 :   r2 :   r1 : 0003  r0 : c060d0f4
> [23235.761951] Flags: Nzcv  IRQs off  FIQs on  Mode SVC_32  ISA ARM
> Segment kernel
> [23235.761957] Control: 10c5387d  Table: 6bf7006a  DAC: 0015
> [23235.761964] Process ksoftirqd/0 (pid: 3, stack limit = 0xef0e2240)
> [23235.761970] Stack: (0xef0e3e10 to 0xef0e4000)
> [23235.761977] 3e00: 
> eab13d04 4013 0001
> [23235.761986] 3e20: 0003  0100 3f6fdf7c ef0e3e6c
> ef0e3e40 c0151c30 c014f820
> [23235.761994] 3e40:  ef0e3e50 c052861c edaddd40 
> edadde4c  
> [23235.762001] 3e60: ef0e3e8c ef0e3e70 bf00a0e4 c0151bf4 bf009fa4
> edaddebc edaddec0 c084c790
> [23235.762009] 3e80: ef0e3eb4 ef0e3e90 c012bcb4 bf009fb0 c012bc1c
> ef0e2038 0009 c090209c
> [23235.762016] 3ea0: 0006 c09790c0 ef0e3f04 ef0e3eb8 c012b348
> c012bc28 c0934324 ef0e2000
> [23235.762024] 3ec0: 0001 ef0e2020   04208040
> 0005 c0153f94 
> [23235.762032] 3ee0: c0934324 ef0e2000 0001 ef0e2020 
>  ef0e3f1c ef0e3f08
> [23235.762039] 3f00: c012b48c c012b234 c012b44c ef0409c0 ef0e3f44
> ef0e3f20 c014f22c c012b458
> [23235.762046] 3f20: ef0dde48  ef0409c0 c014f0c0 
>  ef0e3fac ef0e3f48
> [23235.762054] 3f40: c01455b4 c014f0cc 0001  ef0409c0
>  00030003 dead4ead
> [23235.762061] 3f60:   ef0e3f68 ef0e3f68 
>  dead4ead 
> [23235.762068] 3f80:  ef0e3f84 ef0e3f84 271ae517 ef0dde48
> c01454ec  
> [23235.762075] 3fa0:  ef0e3fb0 c0106118 c01454f8 
>   
> [23235.762082] 3fc0:     
>   
> [23235.762089] 3fe0:     0013
>  f77e7f69 e1459824
> [23235.762094] Backtrace:
> [23235.762107] [] (__wake_up_common+0x5c/0x88) from
> [] (__wake_up+0x48/0x5c)
> [23235.762121] [] (__wake_up+0x48/0x5c) from []
> (usbnet_bh+0x140/0x210 [usbnet])
> [23235.762135] [] (usbnet_bh+0x140/0x210 [usbnet]) from
> [] (tasklet_action+0x98/0xf4)
> [23235.762148] [] (tasklet_action+0x98/0xf4) from
> [] (__do_softirq+0x120/0x224)
> [23235.762160] [] (__do_softirq+0x120/0x224) from
> [] (run_ksoftirqd+0x40/0x60)
> [23235.762170] [] (run_ksoftirqd+0x40/0x60) from
> [] (smpboot_thread_fn+0x16c/0x184)
> [23235.762180] [] (smpboot_thread_fn+0x16c/0x184) from
> [] (kthread+0xc8/0xd8)
> [23235.762191] [] (kthread+0xc8/0xd8) from []
> (ret_from_fork+0x14/0x20)
> [23235.762200] Code: efe8 3f15 eff0  (f004)
> [23235.762209] ---[ end trace 3ad68dc3731b37c5 ]---
> [23235.766529] Kernel panic - not syncing: Fatal exception in interrupt
> [23235.766539] CPU1: stopping
> [23235.766546] Backtrace:
> [23235.766564] [] (unwind_backtrace+0x0/0x118) from
> [] (dump_stack+0x28/0x30)
> [23235.766577] [] (dump_stack+0x28/0x30) from []
> (handle_IPI+0xf0/0x170)
> [23235.766588] [] (handle_IPI+0xf0/0x1

Re: usbnet: driver_info->stop required to stop USB interrupts?

2014-03-11 Thread Grant Grundler
On Mon, Mar 10, 2014 at 7:53 PM, Julius Werner  wrote:
> I think usbnet_stop() raced with the dev->bh tasklet, which by itself
> might not be a problem (usbnet_stop() later kills the tasklet itself,
> so it should expect that it can be running before that). The issue is
> that it calls usbnet_terminate_urbs() before that, which temporarily
> installs a waitqueue in dev->wait in order to be able to wait on the
> tasklet to run and finish up some queues. The waiting itself looks
> okay, but the access to 'dev->wait' is totally unprotected and can
> race arbitrarily. I think in this case usbnet_bh() managed to succeed
> it's dev->wait check just before usbnet_terminate_urbs() sets it back
> to NULL. The latter then finishes and the waitqueue_t structure on its
> stack gets overwritten by other functions halfway through the
> wake_up() call in usbnet_bh().

Awesome - thanks Julius! :)

FWIW, I've reproduced this on "Samsung Chromebook" (Exynos 5250) with
AX88772 USB dongle using the instructions I posted before (ie bouncing
the USB port with reload_asix script).

cheers,
grant

[23231.533805] asix 3-1:1.0 eth0: link up, 1000Mbps, full-duplex, lpa 0xCDE1
[23235.755652] usbcore: deregistering interface driver asix
[23235.761722] asix 3-1:1.0 eth0: unregister 'asix'
usb-1211.usb-1, ASIX AX88178 USB 2.0 Ethernet
[23235.761763] Unable to handle kernel paging request at virtual
address e24cb004
[23235.761771] pgd = ebf7
[23235.761777] [e24cb004] *pgd=6241141e(bad)
[23235.761792] Internal error: Oops: 800d [#1] SMP ARM
[23235.761798] Modules linked in: asix(-) exynos_gsc v4l2_mem2mem
isl29018(C) sbs_battery i2c_dev uinput mwifiex_sdio mwifiex
btmrvl_sdio btmrvl s5p_mfc videobuf2_dma_contig rtc_s3c bluetooth
zram(C) zsmalloc(C) fuse cfg80211 nf_conntrack_ipv6 nf_defrag_ipv6
ip6table_filter ip6_tables uvcvideo videobuf2_core videobuf2_vmalloc
videobuf2_memops usbnet joydev [last unloaded: asix]
[23235.761898] CPU: 0Tainted: G C(3.8.11 #25)
[23235.761906] PC is at 0xe24cb004
[23235.761916] LR is at __wake_up_common+0x5c/0x88
[23235.761924] pc : []lr : []psr: 8093
[23235.761924] sp : ef0e3e10  ip : e24cb004  fp : ef0e3e3c
[23235.761931] r10: e1a0c00d  r9 :   r8 : 0003
[23235.761938] r7 :   r6 : 0001  r5 : e92d3ff4  r4 : eab13d14
[23235.761943] r3 :   r2 :   r1 : 0003  r0 : c060d0f4
[23235.761951] Flags: Nzcv  IRQs off  FIQs on  Mode SVC_32  ISA ARM
Segment kernel
[23235.761957] Control: 10c5387d  Table: 6bf7006a  DAC: 0015
[23235.761964] Process ksoftirqd/0 (pid: 3, stack limit = 0xef0e2240)
[23235.761970] Stack: (0xef0e3e10 to 0xef0e4000)
[23235.761977] 3e00: 
eab13d04 4013 0001
[23235.761986] 3e20: 0003  0100 3f6fdf7c ef0e3e6c
ef0e3e40 c0151c30 c014f820
[23235.761994] 3e40:  ef0e3e50 c052861c edaddd40 
edadde4c  
[23235.762001] 3e60: ef0e3e8c ef0e3e70 bf00a0e4 c0151bf4 bf009fa4
edaddebc edaddec0 c084c790
[23235.762009] 3e80: ef0e3eb4 ef0e3e90 c012bcb4 bf009fb0 c012bc1c
ef0e2038 0009 c090209c
[23235.762016] 3ea0: 0006 c09790c0 ef0e3f04 ef0e3eb8 c012b348
c012bc28 c0934324 ef0e2000
[23235.762024] 3ec0: 0001 ef0e2020   04208040
0005 c0153f94 
[23235.762032] 3ee0: c0934324 ef0e2000 0001 ef0e2020 
 ef0e3f1c ef0e3f08
[23235.762039] 3f00: c012b48c c012b234 c012b44c ef0409c0 ef0e3f44
ef0e3f20 c014f22c c012b458
[23235.762046] 3f20: ef0dde48  ef0409c0 c014f0c0 
 ef0e3fac ef0e3f48
[23235.762054] 3f40: c01455b4 c014f0cc 0001  ef0409c0
 00030003 dead4ead
[23235.762061] 3f60:   ef0e3f68 ef0e3f68 
 dead4ead 
[23235.762068] 3f80:  ef0e3f84 ef0e3f84 271ae517 ef0dde48
c01454ec  
[23235.762075] 3fa0:  ef0e3fb0 c0106118 c01454f8 
  
[23235.762082] 3fc0:     
  
[23235.762089] 3fe0:     0013
 f77e7f69 e1459824
[23235.762094] Backtrace:
[23235.762107] [] (__wake_up_common+0x5c/0x88) from
[] (__wake_up+0x48/0x5c)
[23235.762121] [] (__wake_up+0x48/0x5c) from []
(usbnet_bh+0x140/0x210 [usbnet])
[23235.762135] [] (usbnet_bh+0x140/0x210 [usbnet]) from
[] (tasklet_action+0x98/0xf4)
[23235.762148] [] (tasklet_action+0x98/0xf4) from
[] (__do_softirq+0x120/0x224)
[23235.762160] [] (__do_softirq+0x120/0x224) from
[] (run_ksoftirqd+0x40/0x60)
[23235.762170] [] (run_ksoftirqd+0x40/0x60) from
[] (smpboot_thread_fn+0x16c/0x184)
[23235.762180] [] (smpboot_thread_fn+0x16c/0x184) from
[] (kthread+0xc8/0xd8)
[23235.762191] [] (kthread+0xc8/0xd8) from []
(ret_from_fork+0x14/0x20)
[23235.762200] Code: efe8 3f15 eff0  (f004)
[23235.762209] ---[ end t

Re: usbnet: driver_info->stop required to stop USB interrupts?

2014-03-10 Thread Grant Grundler
On Mon, Mar 10, 2014 at 11:33 AM, Grant Grundler  wrote:
> I've trying to unravel a page fault panic I've run into a few times
> while testing load/unload of asix driver with ChromeOS 3.8.11 based
> kernel.  I'm running into this crash on both ARM and X86.

Correction - I can only confirm I've seen this on ARM.

sorry,
grant

> Panic output below is from Exynos 5422 system. Test script attached.
>
> My _guess_ is usbnet_stop() is racing with a USB interrupt from the
> device and loses. First glance at the stack trace implies the
> interrupt handler is trying to access something that has previously
> been released.
>
> usbnet_stop() calls driver_info->stop() if provided by the driver.  If
> my guess above is correct, does that mean "stop()" call is expected
> (required?) to stop interrupts coming from that USB device?
> Or is something else supposed to stop RX (or other USB) traffic?
>
> ax88179_178a.c appears to be the only usbnet driver that provides a
> .stop call and was able to complete 10K iterations. asix driver
> completes 200-5000 iterations before failing for different causes.
>
> thanks,
> grant
>
> invoke the reload_asix script and monitor test ---
> scp reload_asix $T:/tmp
> for i in `seq 1`; do echo -n "RELOAD $i  " ; ssh $T ".
> /tmp/reload_asix eth0 100_full" ; J=$? ; if [ $J -eq 255 ] ; then echo
> " SSH timeout" ; break ; fi ; ssh $T "cat /var/log/reload-asix.out" ;
> if [ $J -ne 0 ] ; then echo "  ERROR $J" ; fi ; sleep 3 ; done | tee
> ~/reload-AX88772-$IP-04.out
>
>  tombstone from Exynos 5422 on asix driver unload 
> ...
> [28488.367522] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
> [28488.380574] asix 1-1:1.0 eth0: link up, 100Mbps, full-duplex, lpa 0xCDE1
> [28493.308354] usbcore: deregistering interface driver asix
> [28493.310775] asix 1-1:1.0 eth0: unregister 'asix'
> usb-xhci-hcd.4.auto-1, ASIX AX88772 USB 2.0 Ethernet
> [28494.369787] usbcore: registered new interface driver asix
> [28494.725186] asix 1-1:1.0 eth0: register 'asix' at
> usb-xhci-hcd.4.auto-1, ASIX AX88772 USB 2.0 Ethernet,
> c8:d7:19:d8:0b:d3
> [28494.725262] usb 1-1: authorized to connect
> [28495.545485] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready
> [28497.455518] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
> [28497.466586] asix 1-1:1.0 eth0: link up, 100Mbps, full-duplex, lpa 0xCDE1
> [28502.302851] usbcore: deregistering interface driver asix
> [28502.308652] asix 1-1:1.0 eth0: unregister 'asix'
> usb-xhci-hcd.4.auto-1, ASIX AX88772 USB 2.0 Ethernet
> [28502.308717] Unable to handle kernel paging request at virtual
> address e24cb004
> [28502.308739] pgd = ea514000
> [28502.308753] [e24cb004] *pgd=4241141e(bad)
> [28502.308782] Internal error: Oops: 800d [#1] SMP ARM
> [28502.308795] Modules linked in: asix(-) uvcvideo videobuf2_vmalloc
> i2c_dev uinput exynos_gsc v4l2_mem2mem btmrvl_sdio sbs_9018(C)
> mwifiex_sdio mwifiex btmrvl s5p_mfc videobuf2_core zram(C) bluetooth
> videobuf2_dma_contig videobuf2_memops rtc_s3c zuse cfg80211
> nf_conntrack_ipv6 nf_defrag_ipv6 ip6table_filter ip6_tables usbnet
> joydev [last unloaded: asix]
> [28502.308998] CPU: 0Tainted: G C(3.8.11 #6)
> [28502.309016] PC is at 0xe24cb004
> [28502.309039] LR is at __wake_up_common+0x5c/0x88
> [28502.309058] pc : []lr : []psr: 8093
> [28502.309058] sp : ef10be10  ip : e24cb004  fp : ef10be3c
> [28502.309076] r10: e1a0c00d  r9 :   r8 : 0003
> [28502.309091] r7 :   r6 : 0001  r5 : e92d3ff4  r4 : ea409d14
> [28502.309106] r3 :   r2 :   r1 : 0003  r0 : c060ced4
> [28502.309122] Flags: Nzcv  IRQs off  FIQs on  Mode SVC_32  ISA ARM
> Segment kernel
> [28502.309138] Control: 10c5387d  Table: 4a51406a  DAC: 0015
> [28502.309153] Process ksoftirqd/0 (pid: 3, stack limit = 0xef10a240)
> [28502.309168] Stack: (0xef10be10 to 0xef10c000)
> [28502.309186] be00: 
> ea409d04 4013 0001
> [28502.309209] be20: 0003  0100 3f6fdf7c ef10be6c
> ef10be40 c0151c08 c014f7f8
> [28502.309231] be40:  ef10be50 c0529a44 ea5ac540 
> ea5ac64c  
> [28502.309254] be60: ef10be8c ef10be70 bf00a0e4 c0151bcc bf009fa4
> ea5ac6bc ea5ac6c0 c084c790
> [28502.309277] be80: ef10beb4 ef10be90 c012bcb4 bf009fb0 c012bc1c
> ef10a038 0001 c090209c
> [28502.309300] bea0: 0006 c09795c0 ef10bf04 ef10beb8 c012b348
> c012bc28 c0934314 ef10a000
> [28502.309322] bec0: 0001 ef10a020   04208040
> 000a ef10bf04 
> [28502.309345] bee0: c0934314 ef10a000 0001 ef10

usbnet: driver_info->stop required to stop USB interrupts?

2014-03-10 Thread Grant Grundler
I've trying to unravel a page fault panic I've run into a few times
while testing load/unload of asix driver with ChromeOS 3.8.11 based
kernel.  I'm running into this crash on both ARM and X86. Panic output
below is from Exynos 5422 system. Test script attached.

My _guess_ is usbnet_stop() is racing with a USB interrupt from the
device and loses. First glance at the stack trace implies the
interrupt handler is trying to access something that has previously
been released.

usbnet_stop() calls driver_info->stop() if provided by the driver.  If
my guess above is correct, does that mean "stop()" call is expected
(required?) to stop interrupts coming from that USB device?
Or is something else supposed to stop RX (or other USB) traffic?

ax88179_178a.c appears to be the only usbnet driver that provides a
.stop call and was able to complete 10K iterations. asix driver
completes 200-5000 iterations before failing for different causes.

thanks,
grant

invoke the reload_asix script and monitor test ---
scp reload_asix $T:/tmp
for i in `seq 1`; do echo -n "RELOAD $i  " ; ssh $T ".
/tmp/reload_asix eth0 100_full" ; J=$? ; if [ $J -eq 255 ] ; then echo
" SSH timeout" ; break ; fi ; ssh $T "cat /var/log/reload-asix.out" ;
if [ $J -ne 0 ] ; then echo "  ERROR $J" ; fi ; sleep 3 ; done | tee
~/reload-AX88772-$IP-04.out

 tombstone from Exynos 5422 on asix driver unload 
...
[28488.367522] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
[28488.380574] asix 1-1:1.0 eth0: link up, 100Mbps, full-duplex, lpa 0xCDE1
[28493.308354] usbcore: deregistering interface driver asix
[28493.310775] asix 1-1:1.0 eth0: unregister 'asix'
usb-xhci-hcd.4.auto-1, ASIX AX88772 USB 2.0 Ethernet
[28494.369787] usbcore: registered new interface driver asix
[28494.725186] asix 1-1:1.0 eth0: register 'asix' at
usb-xhci-hcd.4.auto-1, ASIX AX88772 USB 2.0 Ethernet,
c8:d7:19:d8:0b:d3
[28494.725262] usb 1-1: authorized to connect
[28495.545485] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready
[28497.455518] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
[28497.466586] asix 1-1:1.0 eth0: link up, 100Mbps, full-duplex, lpa 0xCDE1
[28502.302851] usbcore: deregistering interface driver asix
[28502.308652] asix 1-1:1.0 eth0: unregister 'asix'
usb-xhci-hcd.4.auto-1, ASIX AX88772 USB 2.0 Ethernet
[28502.308717] Unable to handle kernel paging request at virtual
address e24cb004
[28502.308739] pgd = ea514000
[28502.308753] [e24cb004] *pgd=4241141e(bad)
[28502.308782] Internal error: Oops: 800d [#1] SMP ARM
[28502.308795] Modules linked in: asix(-) uvcvideo videobuf2_vmalloc
i2c_dev uinput exynos_gsc v4l2_mem2mem btmrvl_sdio sbs_9018(C)
mwifiex_sdio mwifiex btmrvl s5p_mfc videobuf2_core zram(C) bluetooth
videobuf2_dma_contig videobuf2_memops rtc_s3c zuse cfg80211
nf_conntrack_ipv6 nf_defrag_ipv6 ip6table_filter ip6_tables usbnet
joydev [last unloaded: asix]
[28502.308998] CPU: 0Tainted: G C(3.8.11 #6)
[28502.309016] PC is at 0xe24cb004
[28502.309039] LR is at __wake_up_common+0x5c/0x88
[28502.309058] pc : []lr : []psr: 8093
[28502.309058] sp : ef10be10  ip : e24cb004  fp : ef10be3c
[28502.309076] r10: e1a0c00d  r9 :   r8 : 0003
[28502.309091] r7 :   r6 : 0001  r5 : e92d3ff4  r4 : ea409d14
[28502.309106] r3 :   r2 :   r1 : 0003  r0 : c060ced4
[28502.309122] Flags: Nzcv  IRQs off  FIQs on  Mode SVC_32  ISA ARM
Segment kernel
[28502.309138] Control: 10c5387d  Table: 4a51406a  DAC: 0015
[28502.309153] Process ksoftirqd/0 (pid: 3, stack limit = 0xef10a240)
[28502.309168] Stack: (0xef10be10 to 0xef10c000)
[28502.309186] be00: 
ea409d04 4013 0001
[28502.309209] be20: 0003  0100 3f6fdf7c ef10be6c
ef10be40 c0151c08 c014f7f8
[28502.309231] be40:  ef10be50 c0529a44 ea5ac540 
ea5ac64c  
[28502.309254] be60: ef10be8c ef10be70 bf00a0e4 c0151bcc bf009fa4
ea5ac6bc ea5ac6c0 c084c790
[28502.309277] be80: ef10beb4 ef10be90 c012bcb4 bf009fb0 c012bc1c
ef10a038 0001 c090209c
[28502.309300] bea0: 0006 c09795c0 ef10bf04 ef10beb8 c012b348
c012bc28 c0934314 ef10a000
[28502.309322] bec0: 0001 ef10a020   04208040
000a ef10bf04 
[28502.309345] bee0: c0934314 ef10a000 0001 ef10a020 
 ef10bf1c ef10bf08
[28502.309368] bf00: c012b48c c012b234 c012b44c ef056d00 ef10bf44
ef10bf20 c014f204 c012b458
[28502.309391] bf20: ef101e48  ef056d00 c014f098 
 ef10bfac ef10bf48
[28502.309413] bf40: c01455b4 c014f0a4 0001  ef056d00
 00030003 dead4ead
[28502.309436] bf60:   ef10bf68 ef10bf68 
 dead4ead 
[28502.309459] bf80:  ef10bf84 ef10bf84 271ae517 ef101e48
c01454ec  
[28502.309480] bfa0:  ef10bfb0 c0106118 c0

Re: [PATCH 1/1] Workaround for Suspend/Resume issue of AX88772B under ChromeOS

2013-11-20 Thread Grant Grundler
On Wed, Nov 20, 2013 at 10:54 AM, David Miller  wrote:
> From: Grant Grundler 
> Date: Wed, 20 Nov 2013 09:42:42 -0800
>
>> While this patch raises a new issue, can you apply this patch anyway
>> since in most cases (default settings) it improves "the user
>> experience" for most users?
>
> I'm not applying a patch which knowingly is not implemented correctly.

Ok - fair enough. I'll figure out what needs to be done to
save/restore phy values.

Seems like this should be part of usbnet_resume code based on whether
the driver provides mdio_write hook (most USBNET drivers do).

thanks,
grant
--
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 1/1] Workaround for Suspend/Resume issue of AX88772B under ChromeOS

2013-11-20 Thread Grant Grundler
On Tue, Nov 19, 2013 at 9:40 PM, David Miller  wrote:
> From: fre...@asix.com.tw
> Date: Wed, 20 Nov 2013 10:11:36 +0800
>
>> From: Freddy Xin 
>>
>> This patch adds a workaroud to solve Suspend/Resume issue that AX88772B turns
>> off its Ethernet PHY power in the case that REMOTE_WAKEUP feature doesn't
>> be set when system suspend. In this case, the PHY power will not be turned
>> on again when system resume, so the HW reset must be added in the resume 
>> function.
>>
>> Signed-off-by: Freddy Xin 
>> Tested-by: Grant Grundler 
>
> Like many similar patches I've seen recently, you cannot do this.
>
> Now the the configuration that software things the PHY has is no
> longer accurate.

Dave,
Sorry - I didn't think of that.  I agree the phy settings should be
reprograming in the phy.

The kernel state of the phy is not accurate in any case since the phy
was powered off and left powered off.
While this patch raises a new issue, can you apply this patch anyway
since in most cases (default settings) it improves "the user
experience" for most users?

> If you absolutely must reset the chip on resume, you have to reprogram
> the PHY with whatever settings were existed at the time of the suspend.

AFAICT, the reset is required.

Since you've seen so many of these patches that do NOT reprogram the
phy on reset, would you consider a patch that added "reprogram phy" to
usbnet_resume() code path?

thanks!
grant
--
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 22/51] DMA-API: amba: get rid of separate dma_mask

2013-09-22 Thread Grant Likely
On Thu, 19 Sep 2013 22:47:01 +0100, Russell King  
wrote:
> AMBA Primecell devices always treat streaming and coherent DMA exactly
> the same, so there's no point in having the masks separated.
> 
> Signed-off-by: Russell King 

for the drivers/of/platform.c portion:
Acked-by: Grant Likely 

g.
--
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: Non-enumerable devices on USB and other enumerable buses

2013-08-11 Thread Grant Likely
On Sun, Aug 11, 2013 at 8:08 PM, Mark Brown  wrote:
> I know there's been some discussion of this topic but do we have any
> general consensus on how to handle such things both from a Linux driver
> model point of view and from a DT/ACPI point of view?

There is precedence for describing enumerated device in the device
tree. Real OpenFirmware platforms can and will enumerate the PCI and
USB busses and pass a full tree to the OS. I don't think we want to
full enumerating like that with either ACPI or FDT, but we could allow
for sparse population of devices when something is fixed like a
soldered down USB hub or USB Ethernet MAC.

To make it work would probably require a hook in the USB enumeration
path to look for matching nodes in DT/ACPI and attach it to the struct
device.

g.
--
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 v3 4/4] USBNET: ax88179_178a: enable tso if usb host supports sg dma

2013-08-08 Thread Grant Grundler
Ming,
We are splitting hairs now. :) I want to be clear I think your changes
are good and the rest of this conversation is just to learn something
new.

On Thu, Aug 8, 2013 at 4:48 PM, Ming Lei  wrote:
> On Fri, Aug 9, 2013 at 1:25 AM, Grant Grundler  wrote:
...
>>> I am afraid that PCI network devices' setting still won't survive unbound&
>>> re-probed, will they?
>>
>> Correct - but PCI isn't as prone to "dropping off the bus" like USB
>
> As far as I know, USB device still won't be disconnected easily, and
> reset is possible, but we can make setting survive reset by implementing
> .pre_reset() and .post_reset() callback. Or do you have other situation
> of USB 'dropping off the bus'?

So far only older USB core bugs like this one:
https://codereview.chromium.org/4687002/show

I agree USB won't be disconnected easily.

>> is. Master aborts on some PCI systems is a "Fatal Exception" and AFAIK
>> that's never been true for any USB device.
>
> I mean rmmod & modprobe still can reset setting of one PCI network
> device after powering on the device, can't it?

Definitely. But this isn't something that will "randomly" happen and
will leave tracks all over the place of it happening. So I'm not
worried about trying to debug this scenario.

thanks,
grant

>
>
> Thanks,
> --
> Ming Lei
--
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 v3 4/4] USBNET: ax88179_178a: enable tso if usb host supports sg dma

2013-08-08 Thread Grant Grundler
On Tue, Aug 6, 2013 at 5:41 PM, Ming Lei  wrote:
> On Wed, Aug 7, 2013 at 1:09 AM, Grant Grundler  wrote:
...
>> Following that logic, shouldn't all the features/hw_features settings
>> be removed from reset code path?
>
> This patch won't touch other settings because that isn't related with
> this patch.

Sorry - I didn't mean to imply your patch should do this. I was hoping
for a yes/no answer from you, Eric, or Dave.

...
>> I'll note that any "hiccup" in the USB side that causes the device to
>> get dropped and re-probed will cause the same symptom. There is
>
> I am afraid that PCI network devices' setting still won't survive unbound&
> re-probed, will they?

Correct - but PCI isn't as prone to "dropping off the bus" like USB
is. Master aborts on some PCI systems is a "Fatal Exception" and AFAIK
that's never been true for any USB device.

>> nothing the driver can do about it in this case. Perhaps add some udev
>> rules to preserve ethtool settings the same way I've seen udev rules
>> to record MAC address to enumerate devices (eth0, eth1, etc.)
>
> Some usbnet devices may have random MAC address assigned in every
> probe().

Ugh - ok. Then those scripts are broken for those devices. C'est la Vie.

My point is the mechanism (udev) exists to preserve any settings
ethtool can read/record the state of.

cheers,
grant

ps. thanks again for posting the USBNET sg dma series - I will likely
back port those to our chromeos-3.8 tree in the near future.
--
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 v3 4/4] USBNET: ax88179_178a: enable tso if usb host supports sg dma

2013-08-06 Thread Grant Grundler
On Tue, Aug 6, 2013 at 5:22 AM, Eric Dumazet  wrote:
...
>> @@ -1310,6 +1318,10 @@ static int ax88179_reset(struct usbnet *dev)
>>
>>   dev->net->hw_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
>>NETIF_F_RXCSUM;
>> + if (dev->can_dma_sg) {
>> + dev->net->features |= NETIF_F_SG | NETIF_F_TSO;
>> + dev->net->hw_features |= NETIF_F_SG | NETIF_F_TSO;
>> + }
>>
>
> My concern with setting TSO on reset() is the following :
>
> Admin can disable TSO with
>
> ethtool -K ethX tso off
>
>
> Then, one hour later, or one month later, a reset happens, and this code
> magically re-enables TSO
>
> So, I really think this part should be removed from your patch.

Following that logic, shouldn't all the features/hw_features settings
be removed from reset code path?

hw_features shouldn't change since power up.

FWIW, I do agree with you.

I'll note that any "hiccup" in the USB side that causes the device to
get dropped and re-probed will cause the same symptom. There is
nothing the driver can do about it in this case. Perhaps add some udev
rules to preserve ethtool settings the same way I've seen udev rules
to record MAC address to enumerate devices (eth0, eth1, etc.)

cheers,
grant
--
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 4/4] USBNET: ax88179_178a: enable tso if host supports sg dma

2013-08-01 Thread Grant Grundler
On Thu, Aug 1, 2013 at 8:36 AM, Eric Dumazet  wrote:
...
>> IIRC, cpu_to_leXX() macros return the endian "corrected" value.
>> In other words, they need to be assigned to something, no?
>
> Nope, this in in-place byte swapping (for Big Endian only)

Ah ok - thanks for clarifying. I couldn't find the implementation and
this documentation also helped:
   http://kernelnewbies.org/EndianIssues

I don't remember implementing in-place endian correction for
arch/parisc (which is BE). I was expecting to find a generic
definition that did a load/swap/store and didn't.

thanks,
grant
--
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 4/4] USBNET: ax88179_178a: enable tso if host supports sg dma

2013-08-01 Thread Grant Grundler
On Wed, Jul 31, 2013 at 3:51 AM, Ming Lei  wrote:
> This patch enables 'can_dma_sg' flag for ax88179_178a device
> if the attached host controller supports building packet from
> discontinuous buffers(DMA SG is possible), so both frame header
> and skb data buffers can be passed to usb stack via urb->sg,
> then skb data copy can be saved.
>
> With the patch, CPU utilization decreased much in iperf test at
> client mode.
>
> Signed-off-by: Ming Lei 
> ---
>  drivers/net/usb/ax88179_178a.c |   30 ++
>  1 file changed, 30 insertions(+)
>
> diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
> index 5a468f3..c75bded 100644
> --- a/drivers/net/usb/ax88179_178a.c
> +++ b/drivers/net/usb/ax88179_178a.c
> @@ -1031,12 +1031,20 @@ static int ax88179_bind(struct usbnet *dev, struct 
> usb_interface *intf)
> dev->mii.phy_id = 0x03;
> dev->mii.supports_gmii = 1;
>
> +   if (dev->udev->bus->no_sg_limit)
> +   dev->can_dma_sg = 1;
> +
> dev->net->features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
>   NETIF_F_RXCSUM;
>
> dev->net->hw_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
>  NETIF_F_RXCSUM;
>
> +   if (dev->can_dma_sg) {
> +   dev->net->features |= NETIF_F_SG | NETIF_F_TSO;
> +   dev->net->hw_features |= NETIF_F_SG | NETIF_F_TSO;
> +   }
> +
> /* Enable checksum offload */
> *tmp = AX_RXCOE_IP | AX_RXCOE_TCP | AX_RXCOE_UDP |
>AX_RXCOE_TCPV6 | AX_RXCOE_UDPV6;
> @@ -1170,12 +1178,30 @@ ax88179_tx_fixup(struct usbnet *dev, struct sk_buff 
> *skb, gfp_t flags)
> int mss = skb_shinfo(skb)->gso_size;
> int headroom;
> int tailroom;
> +   struct skb_data *entry = (struct skb_data *)skb->cb;
>
> tx_hdr1 = skb->len;
> tx_hdr2 = mss;
> if (((skb->len + 8) % frame_size) == 0)
> tx_hdr2 |= 0x80008000;  /* Enable padding */
>
> +   if (dev->can_dma_sg) {
> +   if (!(entry->header = kmalloc(8, GFP_ATOMIC)))
> +   goto no_sg;
> +
> +   entry->length = 8;
> +   cpu_to_le32s(&tx_hdr1);
> +   cpu_to_le32s(&tx_hdr2);

http://lxr.free-electrons.com/source/include/linux/byteorder/generic.h#L111
http://lxr.free-electrons.com/ident?i=__cpu_to_le32s

IIRC, cpu_to_leXX() macros return the endian "corrected" value.
In other words, they need to be assigned to something, no?

thanks,
grant

> +   memcpy(entry->header, &tx_hdr1, 4);
> +   memcpy(entry->header + 4, &tx_hdr2, 4);
> +
> +   return skb;
> +   } else {
> +no_sg:
> +   entry->header = NULL;
> +   entry->length = 0;
> +   }
> +
> headroom = skb_headroom(skb);
> tailroom = skb_tailroom(skb);
>
> @@ -1323,6 +1349,10 @@ static int ax88179_reset(struct usbnet *dev)
>
> dev->net->hw_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
>  NETIF_F_RXCSUM;
> +   if (dev->can_dma_sg) {
> +   dev->net->features |= NETIF_F_SG | NETIF_F_TSO;
> +   dev->net->hw_features |= NETIF_F_SG | NETIF_F_TSO;
> +   }
>
> /* Enable checksum offload */
> *tmp = AX_RXCOE_IP | AX_RXCOE_TCP | AX_RXCOE_UDP |
> --
> 1.7.9.5
>
--
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 1/1] TX throttling bug-fixing patch of AX88179_178A

2013-07-23 Thread Grant Grundler
On Tue, Jul 23, 2013 at 7:29 PM, Grant Grundler  wrote:
> On Tue, Jul 23, 2013 at 4:46 PM, David Miller  wrote:
> ...
>> A quick scan shows that smsc75xx, smsc95xx, and ax88179_178a all have
>> this problem.
>>
>> Instead of the patch starting this thread, I'd like to see one that
>> hits all three drivers and removes all SG and TSO features bits from
>> both the ->features _and_ ->hw_features settings.
>
> Since you are asking to remove TSO, do you also want skb_linearize()
> calls in ax88179_178a.c and smsc75xx.c removed as well?

Nevermind...Eric already removed skb_linearize calls in his patch.

cheers,
grant

>
> Not part of the original patch - but based on this thread, Eric seems
> to think calling skb_linearize isn't necessary or helpful either.
>
> cheers,
> grant
--
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 1/1] TX throttling bug-fixing patch of AX88179_178A

2013-07-23 Thread Grant Grundler
On Tue, Jul 23, 2013 at 4:46 PM, David Miller  wrote:
...
> A quick scan shows that smsc75xx, smsc95xx, and ax88179_178a all have
> this problem.
>
> Instead of the patch starting this thread, I'd like to see one that
> hits all three drivers and removes all SG and TSO features bits from
> both the ->features _and_ ->hw_features settings.

Since you are asking to remove TSO, do you also want skb_linearize()
calls in ax88179_178a.c and smsc75xx.c removed as well?

Not part of the original patch - but based on this thread, Eric seems
to think calling skb_linearize isn't necessary or helpful either.

cheers,
grant
--
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 1/1] TX throttling bug-fixing patch of AX88179_178A

2013-07-22 Thread Grant Grundler
On Mon, Jul 22, 2013 at 10:07 AM, Eric Dumazet  wrote:
...
> I guess that if a driver does not advertise NETIF_F_SG, this
> skb_linearize() call is not needed : All frames reaching your xmit
> function should already be linear

As Ben Hutchings pointed out, hw_features is still setting this...but
I'm not sure how that matters.

ax88179_set_features() doesn't allow setting SG or TSO features.  But
I expect it would be "not too difficult" to add such that ethtool
could set those features after boot.  Or perhaps add a driver module
parameter to set these features.  I just guessing the skb_linearize()
could be removed until set_features support for SG and/or TSO is
added. Is that correct?

thanks,
grant
--
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] xhci: Compute last_ctx from complete set of configured endpoints.

2013-06-24 Thread Reilly Grant
On Mon, Jun 24, 2013 at 08:59AM -0700, Sarah Sharp wrote:
> On Tue, Jun 18, 2013 at 02:09:13PM -0700, Reilly Grant wrote:
> > The context entries field of the slot context must be set to one more
> > than the highest endpoint index currently active. The previous logic
> > only included the set of endpoints currently being added, meaning that
> > if an endpoint where dropped then the field would be reset to 1,
> > deactivating all configured endpoints.
> > 
> > The xHCI spec is decidedly unclear on whether this field includes all
> > configured endpoints or only those being modified by a configure
> > endpoint command. My interpretation is the former as when the xHC writes
> > changes into the output device context array it must know how large it
> > is. It does not make sense for this field to only refer to the input
> > context.
> 
> My interpretation of the spec is that the last valid context index in
> the input context was supposed to be used as the last endpoint context
> that is valid for this command, not for the output context.
> 
> Looking at the spec revision from 08/2012, there is an amendment to
> section 4.6.6.1 that makes this even more clear.  The part of the
> section that talks about what the hardware must do in order to evaluate
> each endpoint and update the output context says:
> 
> "Set the Context Entries field in the Output Slot Context to the index
> of the last valid Endpoint Context in its Output Device Context
> structure, which shall be greater to or equal to the value of the
> Context Entries field in the Input Slot Context."
> 
> That makes it fairly clear that software needs to set the last valid
> endpoint context index based on what endpoints are added or dropped in
> the input context, not which endpoints are still valid in the output
> context.
> 
> So, no, I can't accept this patch.  If this fixes a real problem in some
> hardware, we'll add a quirk for that hardware.

Thank you for the clarification. I was not aware of this addendum. Could you 
forward me a link to it? The last revision I can find on the Intel web site is 
from 05/2010. This fixed a problem on VMware's virtual hardware. Assuming 
changing behavior will not affect any other systems I will fix it based on this 
information.
 
> > ---
> >  drivers/usb/host/xhci.c | 52
> >  +
> >  1 file changed, 18 insertions(+), 34 deletions(-)
> > 
> > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> > index d8f640b..aa117d1 100644
> > --- a/drivers/usb/host/xhci.c
> > +++ b/drivers/usb/host/xhci.c
> > @@ -1571,12 +1571,10 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct
> > usb_device *udev,
> > struct xhci_hcd *xhci;
> > struct xhci_container_ctx *in_ctx, *out_ctx;
> > struct xhci_input_control_ctx *ctrl_ctx;
> > -   struct xhci_slot_ctx *slot_ctx;
> > -   unsigned int last_ctx;
> > unsigned int ep_index;
> > struct xhci_ep_ctx *ep_ctx;
> > u32 drop_flag;
> > -   u32 new_add_flags, new_drop_flags, new_slot_info;
> > +   u32 new_add_flags, new_drop_flags;
> > int ret;
> >  
> > ret = xhci_check_args(hcd, udev, ep, 1, true, __func__);
> > @@ -1617,24 +1615,13 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct
> > usb_device *udev,
> > ctrl_ctx->add_flags &= cpu_to_le32(~drop_flag);
> > new_add_flags = le32_to_cpu(ctrl_ctx->add_flags);
> >  
> > -   last_ctx = xhci_last_valid_endpoint(le32_to_cpu(ctrl_ctx->add_flags));
> > -   slot_ctx = xhci_get_slot_ctx(xhci, in_ctx);
> > -   /* Update the last valid endpoint context, if we deleted the last one */
> > -   if ((le32_to_cpu(slot_ctx->dev_info) & LAST_CTX_MASK) >
> > -   LAST_CTX(last_ctx)) {
> > -   slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
> > -   slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(last_ctx));
> > -   }
> > -   new_slot_info = le32_to_cpu(slot_ctx->dev_info);
> > -
> > xhci_endpoint_zero(xhci, xhci->devs[udev->slot_id], ep);
> >  
> > -   xhci_dbg(xhci, "drop ep 0x%x, slot id %d, new drop flags = %#x, new add
> > flags = %#x, new slot info = %#x\n",
> > +   xhci_dbg(xhci, "drop ep 0x%x, slot id %d, new drop flags = %#x, new add
> > flags = %#x\n",
> > (unsigned int) ep->desc.bEndpointAddress,
> > udev->slot_id,
> > (unsigned int) new_drop_flags,
> > -   (unsigned int) new_add_flags,
> > -   (unsigned 

[PATCH] xhci: Compute last_ctx from complete set of configured endpoints.

2013-06-18 Thread Reilly Grant
The context entries field of the slot context must be set to one more
than the highest endpoint index currently active. The previous logic
only included the set of endpoints currently being added, meaning that
if an endpoint where dropped then the field would be reset to 1,
deactivating all configured endpoints.

The xHCI spec is decidedly unclear on whether this field includes all
configured endpoints or only those being modified by a configure
endpoint command. My interpretation is the former as when the xHC writes
changes into the output device context array it must know how large it
is. It does not make sense for this field to only refer to the input
context.
---
 drivers/usb/host/xhci.c | 52 +
 1 file changed, 18 insertions(+), 34 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index d8f640b..aa117d1 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1571,12 +1571,10 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct 
usb_device *udev,
struct xhci_hcd *xhci;
struct xhci_container_ctx *in_ctx, *out_ctx;
struct xhci_input_control_ctx *ctrl_ctx;
-   struct xhci_slot_ctx *slot_ctx;
-   unsigned int last_ctx;
unsigned int ep_index;
struct xhci_ep_ctx *ep_ctx;
u32 drop_flag;
-   u32 new_add_flags, new_drop_flags, new_slot_info;
+   u32 new_add_flags, new_drop_flags;
int ret;
 
ret = xhci_check_args(hcd, udev, ep, 1, true, __func__);
@@ -1617,24 +1615,13 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct 
usb_device *udev,
ctrl_ctx->add_flags &= cpu_to_le32(~drop_flag);
new_add_flags = le32_to_cpu(ctrl_ctx->add_flags);
 
-   last_ctx = xhci_last_valid_endpoint(le32_to_cpu(ctrl_ctx->add_flags));
-   slot_ctx = xhci_get_slot_ctx(xhci, in_ctx);
-   /* Update the last valid endpoint context, if we deleted the last one */
-   if ((le32_to_cpu(slot_ctx->dev_info) & LAST_CTX_MASK) >
-   LAST_CTX(last_ctx)) {
-   slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
-   slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(last_ctx));
-   }
-   new_slot_info = le32_to_cpu(slot_ctx->dev_info);
-
xhci_endpoint_zero(xhci, xhci->devs[udev->slot_id], ep);
 
-   xhci_dbg(xhci, "drop ep 0x%x, slot id %d, new drop flags = %#x, new add 
flags = %#x, new slot info = %#x\n",
+   xhci_dbg(xhci, "drop ep 0x%x, slot id %d, new drop flags = %#x, new add 
flags = %#x\n",
(unsigned int) ep->desc.bEndpointAddress,
udev->slot_id,
(unsigned int) new_drop_flags,
-   (unsigned int) new_add_flags,
-   (unsigned int) new_slot_info);
+   (unsigned int) new_add_flags);
return 0;
 }
 
@@ -1657,11 +1644,9 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct 
usb_device *udev,
struct xhci_hcd *xhci;
struct xhci_container_ctx *in_ctx, *out_ctx;
unsigned int ep_index;
-   struct xhci_slot_ctx *slot_ctx;
struct xhci_input_control_ctx *ctrl_ctx;
u32 added_ctxs;
-   unsigned int last_ctx;
-   u32 new_add_flags, new_drop_flags, new_slot_info;
+   u32 new_add_flags, new_drop_flags;
struct xhci_virt_device *virt_dev;
int ret = 0;
 
@@ -1676,7 +1661,6 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct 
usb_device *udev,
return -ENODEV;
 
added_ctxs = xhci_get_endpoint_flag(&ep->desc);
-   last_ctx = xhci_last_valid_endpoint(added_ctxs);
if (added_ctxs == SLOT_FLAG || added_ctxs == EP0_FLAG) {
/* FIXME when we have to issue an evaluate endpoint command to
 * deal with ep0 max packet size changing once we get the
@@ -1737,24 +1721,14 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct 
usb_device *udev,
 */
new_drop_flags = le32_to_cpu(ctrl_ctx->drop_flags);
 
-   slot_ctx = xhci_get_slot_ctx(xhci, in_ctx);
-   /* Update the last valid endpoint context, if we just added one past */
-   if ((le32_to_cpu(slot_ctx->dev_info) & LAST_CTX_MASK) <
-   LAST_CTX(last_ctx)) {
-   slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
-   slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(last_ctx));
-   }
-   new_slot_info = le32_to_cpu(slot_ctx->dev_info);
-
/* Store the usb_device pointer for later use */
ep->hcpriv = udev;
 
-   xhci_dbg(xhci, "add ep 0x%x, slot id %d, new drop flags = %#x, new add 
flags = %#x, new slot info = %#x\n",
+   xhci_dbg(xhci, "add ep 0x%x, slot id %d, new drop flags = %#x, new add 
flags = %#x\n",
(unsigned int) ep->desc.bEndpointAddress,
udev->slot_id,
(unsigned int) new_drop_flags,
-   (unsigned int) new_add_flags,
- 

Re: USB3 SSD/HD device file disappears after "eject"

2013-05-29 Thread Grant
> [...]
>
>> sd 8:0:0:0: [sdb] Synchronizing SCSI cache
>> sd 8:0:0:0: Device offlined - not ready after error recovery
>>
>> Unplugging and replugging the device does not result in the
>> reappearance of the /dev/sdb device file like it does with my other
>> external drives.  The only way to bring the /dev/sdb device file back
>> is to 'modprobe -r xhci_hcd && modprobe xhci_hcd'.
>
> [...]
>
> I'm not sure where the problem is exactly but I think it is really
> a problem with the power down of some USB hardware in the chain.
> I am wondering why this still exists on your quite new hardware.

Do you think it must be a problem with power down of the USB3 SSD
since this doesn't happen with a USB3 HD I have?

> I see this here on my own (a few years old) laptop with experimenting
> some power control settings.
> I think your USB port got power switched off and so can't detect that
> you have re-plugged a new device.
> So my idea is, with reloading the kernel module the power is switched
> on again.
> Maybe there should be a config option in userspace to disable
> powerdown and only unmount filesystems if it is not possible
> to detect powerdown support of hardware?!

I'm not sure I follow.  Why only unmount filesystems if it isn't
possible to detect powerdown support of hardware (is that the port or
the drive)?

I noticed this in linux-3.10-rc2 for the first time but I think it is
always enabled on previous kernels and this option only allows it to
be disabled:

CONFIG_USB_DEFAULT_PERSIST:

Say N here if you don't want USB power session persistance
enabled by default.  If you say N it will make suspended USB
devices that lose power get reenumerated as if they had been
unplugged, causing any mounted filesystems to be lost.  The
persist feature can still be enabled for individual devices
through the power/persist sysfs node. See
Documentation/usb/persist.txt for more info.

Should I file a kernel bug?

- Grant
--
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: USB3 SSD/HD device file disappears after "eject"

2013-05-28 Thread Grant
>> I just got one of these:
>>
>> http://www.newegg.com/Product/Product.aspx?Item=N82E16820208913
>>
>> It seems to work fine except I get the following after ejecting it in Thunar:
>>
>> sd 8:0:0:0: [sdb] Synchronizing SCSI cache
>> sd 8:0:0:0: Device offlined - not ready after error recovery
>
> That's to be expected.  Lots of devices will shut down after userspace
> told it to power down.  It was a feature request that Thunar and
> Nautilus added a year or so ago.
>
> Not all devices will do this, as their firmware does not support the
> powerdown command, which is why older devices do not act like this.
>
> If you don't like this, don't eject the device :)

OK good point.  In light of this, here's the real problem.  Ejecting
the SSD results in the "Writing data to device" desktop notification
persisting on the desktop instead of disappearing after a few seconds
like it does with my other external drives.  The following messages
appears in dmesg which does not appear when ejecting my other external
drives:

sd 8:0:0:0: [sdb] Synchronizing SCSI cache
sd 8:0:0:0: Device offlined - not ready after error recovery

Unplugging and replugging the device does not result in the
reappearance of the /dev/sdb device file like it does with my other
external drives.  The only way to bring the /dev/sdb device file back
is to 'modprobe -r xhci_hcd && modprobe xhci_hcd'.

I originally wrote a problem report like this but then I convinced
myself that the *real* problem was the one I described in my previous
message which Greg proved it is not.

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


USB3 SSD/HD device file disappears after "eject"

2013-05-28 Thread Grant
I just got one of these:

http://www.newegg.com/Product/Product.aspx?Item=N82E16820208913

It seems to work fine except I get the following after ejecting it in Thunar:

sd 8:0:0:0: [sdb] Synchronizing SCSI cache
sd 8:0:0:0: Device offlined - not ready after error recovery

The only way I've found to get the /dev/sdb device file back right
away is to 'modprobe -r xhci_hcd && modprobe xhci_hcd'.  It comes back
on its own after about 30 minutes (rough estimate) or so.  I've tried
two different USB3 cables.  I have a 1TB USB3 HD that also requires
modprobe after ejecting but it doesn't print anything interesting to
dmesg.  My laptop is a Dell XPS13 and I'm on the latest BIOS:

# lspci -v
...
02:00.0 USB controller: Fresco Logic FL1009 USB 3.0 Host Controller
(rev 02) (prog-if 30 [XHCI])
Subsystem: Dell Device 052e
Flags: bus master, fast devsel, latency 0, IRQ 19
Memory at f040 (64-bit, non-prefetchable) [size=64K]
Memory at f041 (64-bit, non-prefetchable) [size=4K]
Memory at f0411000 (64-bit, non-prefetchable) [size=4K]
Capabilities: [40] Power Management version 3
Capabilities: [50] MSI: Enable- Count=1/8 Maskable- 64bit+
Capabilities: [70] Express Endpoint, MSI 00
Capabilities: [b0] MSI-X: Enable+ Count=8 Masked-
Capabilities: [100] Advanced Error Reporting
Kernel driver in use: xhci_hcd
Kernel modules: xhci_hcd

Should I file a bug?

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


Resend: [PATCH] xhci: Compute last_ctx from complete set of endpoints.

2013-05-24 Thread Reilly Grant
The context entries field of the slot context must be set to one more
than the highest endpoint index currently active. The previous logic
only included the set of endpoints currently being added, meaning that
if an endpoint where dropped then the field would be reset to 1,
deactivating all configured endpoints.

The xHCI spec is decidedly unclear on whether this field includes all
configured endpoints or only those being modified by a configure
endpoint command. My interpretation is the former and is the behavior
observed in the Apple's xHCI driver.
---
 drivers/usb/host/xhci.c | 52 +
 1 file changed, 18 insertions(+), 34 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 53b8f89..5c40d3e 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1557,12 +1557,10 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct 
usb_device *udev,
struct xhci_hcd *xhci;
struct xhci_container_ctx *in_ctx, *out_ctx;
struct xhci_input_control_ctx *ctrl_ctx;
-   struct xhci_slot_ctx *slot_ctx;
-   unsigned int last_ctx;
unsigned int ep_index;
struct xhci_ep_ctx *ep_ctx;
u32 drop_flag;
-   u32 new_add_flags, new_drop_flags, new_slot_info;
+   u32 new_add_flags, new_drop_flags;
int ret;
 
ret = xhci_check_args(hcd, udev, ep, 1, true, __func__);
@@ -1603,24 +1601,13 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct 
usb_device *udev,
ctrl_ctx->add_flags &= cpu_to_le32(~drop_flag);
new_add_flags = le32_to_cpu(ctrl_ctx->add_flags);
 
-   last_ctx = xhci_last_valid_endpoint(le32_to_cpu(ctrl_ctx->add_flags));
-   slot_ctx = xhci_get_slot_ctx(xhci, in_ctx);
-   /* Update the last valid endpoint context, if we deleted the last one */
-   if ((le32_to_cpu(slot_ctx->dev_info) & LAST_CTX_MASK) >
-   LAST_CTX(last_ctx)) {
-   slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
-   slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(last_ctx));
-   }
-   new_slot_info = le32_to_cpu(slot_ctx->dev_info);
-
xhci_endpoint_zero(xhci, xhci->devs[udev->slot_id], ep);
 
-   xhci_dbg(xhci, "drop ep 0x%x, slot id %d, new drop flags = %#x, new add 
flags = %#x, new slot info = %#x\n",
+   xhci_dbg(xhci, "drop ep 0x%x, slot id %d, new drop flags = %#x, new add 
flags = %#x\n",
(unsigned int) ep->desc.bEndpointAddress,
udev->slot_id,
(unsigned int) new_drop_flags,
-   (unsigned int) new_add_flags,
-   (unsigned int) new_slot_info);
+   (unsigned int) new_add_flags);
return 0;
 }
 
@@ -1643,11 +1630,9 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct 
usb_device *udev,
struct xhci_hcd *xhci;
struct xhci_container_ctx *in_ctx, *out_ctx;
unsigned int ep_index;
-   struct xhci_slot_ctx *slot_ctx;
struct xhci_input_control_ctx *ctrl_ctx;
u32 added_ctxs;
-   unsigned int last_ctx;
-   u32 new_add_flags, new_drop_flags, new_slot_info;
+   u32 new_add_flags, new_drop_flags;
struct xhci_virt_device *virt_dev;
int ret = 0;
 
@@ -1662,7 +1647,6 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct 
usb_device *udev,
return -ENODEV;
 
added_ctxs = xhci_get_endpoint_flag(&ep->desc);
-   last_ctx = xhci_last_valid_endpoint(added_ctxs);
if (added_ctxs == SLOT_FLAG || added_ctxs == EP0_FLAG) {
/* FIXME when we have to issue an evaluate endpoint command to
 * deal with ep0 max packet size changing once we get the
@@ -1723,24 +1707,14 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct 
usb_device *udev,
 */
new_drop_flags = le32_to_cpu(ctrl_ctx->drop_flags);
 
-   slot_ctx = xhci_get_slot_ctx(xhci, in_ctx);
-   /* Update the last valid endpoint context, if we just added one past */
-   if ((le32_to_cpu(slot_ctx->dev_info) & LAST_CTX_MASK) <
-   LAST_CTX(last_ctx)) {
-   slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
-   slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(last_ctx));
-   }
-   new_slot_info = le32_to_cpu(slot_ctx->dev_info);
-
/* Store the usb_device pointer for later use */
ep->hcpriv = udev;
 
-   xhci_dbg(xhci, "add ep 0x%x, slot id %d, new drop flags = %#x, new add 
flags = %#x, new slot info = %#x\n",
+   xhci_dbg(xhci, "add ep 0x%x, slot id %d, new drop flags = %#x, new add 
flags = %#x\n",
(unsigned int) ep->desc.bEndpointAddress,
udev->slot_id,
(unsigned int) new_drop_flags,
-   (unsigned int) new_add_flags,
-   (unsigned int) new_slot_info);
+   (unsigned int) new_add_flags);
return 0;
 }
 

[PATCH] xhci: Compute last_ctx from complete set of endpoints.

2013-05-08 Thread Reilly Grant
The context entries field of the slot context must be set to one more
than the highest endpoint index currently active. The previous logic
only included the set of endpoints currently being added, meaning that
if an endpoint where dropped then the field would be reset to 1,
deactivating all configured endpoints.

The xHCI spec is decidedly unclear on whether this field includes all
configured endpoints or only those being modified by a configure
endpoint command. My interpretation is the former and is the behavior
observed in the Apple's xHCI driver.
---
 drivers/usb/host/xhci.c | 52 +
 1 file changed, 18 insertions(+), 34 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 53b8f89..5c40d3e 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1557,12 +1557,10 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct 
usb_device *udev,
struct xhci_hcd *xhci;
struct xhci_container_ctx *in_ctx, *out_ctx;
struct xhci_input_control_ctx *ctrl_ctx;
-   struct xhci_slot_ctx *slot_ctx;
-   unsigned int last_ctx;
unsigned int ep_index;
struct xhci_ep_ctx *ep_ctx;
u32 drop_flag;
-   u32 new_add_flags, new_drop_flags, new_slot_info;
+   u32 new_add_flags, new_drop_flags;
int ret;
 
ret = xhci_check_args(hcd, udev, ep, 1, true, __func__);
@@ -1603,24 +1601,13 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct 
usb_device *udev,
ctrl_ctx->add_flags &= cpu_to_le32(~drop_flag);
new_add_flags = le32_to_cpu(ctrl_ctx->add_flags);
 
-   last_ctx = xhci_last_valid_endpoint(le32_to_cpu(ctrl_ctx->add_flags));
-   slot_ctx = xhci_get_slot_ctx(xhci, in_ctx);
-   /* Update the last valid endpoint context, if we deleted the last one */
-   if ((le32_to_cpu(slot_ctx->dev_info) & LAST_CTX_MASK) >
-   LAST_CTX(last_ctx)) {
-   slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
-   slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(last_ctx));
-   }
-   new_slot_info = le32_to_cpu(slot_ctx->dev_info);
-
xhci_endpoint_zero(xhci, xhci->devs[udev->slot_id], ep);
 
-   xhci_dbg(xhci, "drop ep 0x%x, slot id %d, new drop flags = %#x, new add 
flags = %#x, new slot info = %#x\n",
+   xhci_dbg(xhci, "drop ep 0x%x, slot id %d, new drop flags = %#x, new add 
flags = %#x\n",
(unsigned int) ep->desc.bEndpointAddress,
udev->slot_id,
(unsigned int) new_drop_flags,
-   (unsigned int) new_add_flags,
-   (unsigned int) new_slot_info);
+   (unsigned int) new_add_flags);
return 0;
 }
 
@@ -1643,11 +1630,9 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct 
usb_device *udev,
struct xhci_hcd *xhci;
struct xhci_container_ctx *in_ctx, *out_ctx;
unsigned int ep_index;
-   struct xhci_slot_ctx *slot_ctx;
struct xhci_input_control_ctx *ctrl_ctx;
u32 added_ctxs;
-   unsigned int last_ctx;
-   u32 new_add_flags, new_drop_flags, new_slot_info;
+   u32 new_add_flags, new_drop_flags;
struct xhci_virt_device *virt_dev;
int ret = 0;
 
@@ -1662,7 +1647,6 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct 
usb_device *udev,
return -ENODEV;
 
added_ctxs = xhci_get_endpoint_flag(&ep->desc);
-   last_ctx = xhci_last_valid_endpoint(added_ctxs);
if (added_ctxs == SLOT_FLAG || added_ctxs == EP0_FLAG) {
/* FIXME when we have to issue an evaluate endpoint command to
 * deal with ep0 max packet size changing once we get the
@@ -1723,24 +1707,14 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct 
usb_device *udev,
 */
new_drop_flags = le32_to_cpu(ctrl_ctx->drop_flags);
 
-   slot_ctx = xhci_get_slot_ctx(xhci, in_ctx);
-   /* Update the last valid endpoint context, if we just added one past */
-   if ((le32_to_cpu(slot_ctx->dev_info) & LAST_CTX_MASK) <
-   LAST_CTX(last_ctx)) {
-   slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
-   slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(last_ctx));
-   }
-   new_slot_info = le32_to_cpu(slot_ctx->dev_info);
-
/* Store the usb_device pointer for later use */
ep->hcpriv = udev;
 
-   xhci_dbg(xhci, "add ep 0x%x, slot id %d, new drop flags = %#x, new add 
flags = %#x, new slot info = %#x\n",
+   xhci_dbg(xhci, "add ep 0x%x, slot id %d, new drop flags = %#x, new add 
flags = %#x\n",
(unsigned int) ep->desc.bEndpointAddress,
udev->slot_id,
(unsigned int) new_drop_flags,
-   (unsigned int) new_add_flags,
-   (unsigned int) new_slot_info);
+   (unsigned int) new_add_flags);
return 0;
 }
 

[PATCH] xhci: Compute last_ctx from complete set of endpoints.

2013-04-30 Thread Reilly Grant
The context entries field of the slot context must be set to one more
than the highest endpoint index currently active. The previous logic
only included the set of endpoints currently being added, meaning that
if an endpoint where dropped then the field would be reset to 1,
deactivating all configured endpoints.

The xHCI spec is decidedly unclear on whether this field includes all
configured endpoints or only those being modified by a configure
endpoint command. My interpretation is the former and is the behavior
observed in the Apple's xHCI driver.

Signed-off-by: Reilly Grant 
---
 drivers/usb/host/xhci.c | 52 +
 1 file changed, 18 insertions(+), 34 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 53b8f89..5c40d3e 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1557,12 +1557,10 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct 
usb_device *udev,
struct xhci_hcd *xhci;
struct xhci_container_ctx *in_ctx, *out_ctx;
struct xhci_input_control_ctx *ctrl_ctx;
-   struct xhci_slot_ctx *slot_ctx;
-   unsigned int last_ctx;
unsigned int ep_index;
struct xhci_ep_ctx *ep_ctx;
u32 drop_flag;
-   u32 new_add_flags, new_drop_flags, new_slot_info;
+   u32 new_add_flags, new_drop_flags;
int ret;
 
ret = xhci_check_args(hcd, udev, ep, 1, true, __func__);
@@ -1603,24 +1601,13 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct 
usb_device *udev,
ctrl_ctx->add_flags &= cpu_to_le32(~drop_flag);
new_add_flags = le32_to_cpu(ctrl_ctx->add_flags);
 
-   last_ctx = xhci_last_valid_endpoint(le32_to_cpu(ctrl_ctx->add_flags));
-   slot_ctx = xhci_get_slot_ctx(xhci, in_ctx);
-   /* Update the last valid endpoint context, if we deleted the last one */
-   if ((le32_to_cpu(slot_ctx->dev_info) & LAST_CTX_MASK) >
-   LAST_CTX(last_ctx)) {
-   slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
-   slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(last_ctx));
-   }
-   new_slot_info = le32_to_cpu(slot_ctx->dev_info);
-
xhci_endpoint_zero(xhci, xhci->devs[udev->slot_id], ep);
 
-   xhci_dbg(xhci, "drop ep 0x%x, slot id %d, new drop flags = %#x, new add 
flags = %#x, new slot info = %#x\n",
+   xhci_dbg(xhci, "drop ep 0x%x, slot id %d, new drop flags = %#x, new add 
flags = %#x\n",
(unsigned int) ep->desc.bEndpointAddress,
udev->slot_id,
(unsigned int) new_drop_flags,
-   (unsigned int) new_add_flags,
-   (unsigned int) new_slot_info);
+   (unsigned int) new_add_flags);
return 0;
 }
 
@@ -1643,11 +1630,9 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct 
usb_device *udev,
struct xhci_hcd *xhci;
struct xhci_container_ctx *in_ctx, *out_ctx;
unsigned int ep_index;
-   struct xhci_slot_ctx *slot_ctx;
struct xhci_input_control_ctx *ctrl_ctx;
u32 added_ctxs;
-   unsigned int last_ctx;
-   u32 new_add_flags, new_drop_flags, new_slot_info;
+   u32 new_add_flags, new_drop_flags;
struct xhci_virt_device *virt_dev;
int ret = 0;
 
@@ -1662,7 +1647,6 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct 
usb_device *udev,
return -ENODEV;
 
added_ctxs = xhci_get_endpoint_flag(&ep->desc);
-   last_ctx = xhci_last_valid_endpoint(added_ctxs);
if (added_ctxs == SLOT_FLAG || added_ctxs == EP0_FLAG) {
/* FIXME when we have to issue an evaluate endpoint command to
 * deal with ep0 max packet size changing once we get the
@@ -1723,24 +1707,14 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct 
usb_device *udev,
 */
new_drop_flags = le32_to_cpu(ctrl_ctx->drop_flags);
 
-   slot_ctx = xhci_get_slot_ctx(xhci, in_ctx);
-   /* Update the last valid endpoint context, if we just added one past */
-   if ((le32_to_cpu(slot_ctx->dev_info) & LAST_CTX_MASK) <
-   LAST_CTX(last_ctx)) {
-   slot_ctx->dev_info &= cpu_to_le32(~LAST_CTX_MASK);
-   slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(last_ctx));
-   }
-   new_slot_info = le32_to_cpu(slot_ctx->dev_info);
-
/* Store the usb_device pointer for later use */
ep->hcpriv = udev;
 
-   xhci_dbg(xhci, "add ep 0x%x, slot id %d, new drop flags = %#x, new add 
flags = %#x, new slot info = %#x\n",
+   xhci_dbg(xhci, "add ep 0x%x, slot id %d, new drop flags = %#x, new add 
flags = %#x\n",
(unsigned int) ep->desc.bEndpointAddress,
udev->slot_id,
(unsigned int) new_drop_flags,
-  

  1   2   >