Skip to contentSkip to conversation

usb: gadget: USB3 support to the legacy printer driver

PATCH In review Jorge Ramirez-Ortiz 4 participants 14 messages Nov 17, 2014 – Nov 19, 2014 USB

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 usb: gadget: USB3 support to the legacy printer driver

Hi,

This patch adds USB3 support to the legacy gadget printer driver. Applies cleanly on fc14f9c Linux 3.18-rc5.

Please could it be considered for inclusion?

regards,
Jorge

Quoted 1 line

From f46d9b0d2160b30f14dee104657de865e9e2bc38 Mon Sep 17 00:00:00 2001

From: Jorge Ramirez-Ortiz <[email protected]>
Date: Thu, 25 Sep 2014 16:17:20 -0400
Subject: [PATCH] usb: gadget: add USB3 support to the printer driver

Add SS descriptors to support the capabilities provided by USB3 controller drivers; unit tests run using PLX 3380 [max transfer speed measured of 1Gbps]

This driver will fallback to lower operating modes when the higher ones are not available.

---
 drivers/usb/gadget/legacy/printer.c | 66 +++++++++++++++++++++++++++++++++----
 1 file changed, 60 insertions(+), 6 deletions(-)
drivers/usb/gadget/legacy/printer.c +60 -6
index 6474081..625d905 100644
@@ -3,6 +3,7 @@
  *
  * Copyright (C) 2003-2005 David Brownell
  * Copyright (C) 2006 Craig W. Nadler
+ * Copyright (C) 2014 Linaro.org
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -208,6 +209,43 @@ static struct usb_descriptor_header *hs_printer_function[] = {
        NULL
 };
 
+/*
+ * Added endpoint descriptors for 3.0 devices
+ */
+
+static struct usb_endpoint_descriptor ss_ep_in_desc = {
+       .bLength =              USB_DT_ENDPOINT_SIZE,
+       .bDescriptorType =      USB_DT_ENDPOINT,
+       .bmAttributes =         USB_ENDPOINT_XFER_BULK,
+       .wMaxPacketSize =       cpu_to_le16(1024),
+};
+
+struct usb_ss_ep_comp_descriptor ss_ep_in_comp_desc = {
+       .bLength =              sizeof(ss_ep_in_comp_desc),
+       .bDescriptorType =      USB_DT_SS_ENDPOINT_COMP,
+};
+
+static struct usb_endpoint_descriptor ss_ep_out_desc = {
+       .bLength =              USB_DT_ENDPOINT_SIZE,
+       .bDescriptorType =      USB_DT_ENDPOINT,
+       .bmAttributes =         USB_ENDPOINT_XFER_BULK,
+       .wMaxPacketSize =       cpu_to_le16(1024),
+};
+
+struct usb_ss_ep_comp_descriptor ss_ep_out_comp_desc = {
+       .bLength =              sizeof(ss_ep_out_comp_desc),
+       .bDescriptorType =      USB_DT_SS_ENDPOINT_COMP,
+};
+
+static struct usb_descriptor_header *ss_printer_function[] = {
+       (struct usb_descriptor_header *) &intf_desc,
+       (struct usb_descriptor_header *) &ss_ep_in_desc,
+       (struct usb_descriptor_header *) &ss_ep_in_comp_desc,
+       (struct usb_descriptor_header *) &ss_ep_out_desc,
+       (struct usb_descriptor_header *) &ss_ep_out_comp_desc,
+       NULL
+};
+
 static struct usb_otg_descriptor otg_descriptor = {
        .bLength =              sizeof otg_descriptor,
        .bDescriptorType =      USB_DT_OTG,
@@ -220,7 +258,20 @@ static const struct usb_descriptor_header *otg_desc[] = {
 };
 
 /* maxpacket and other transfer characteristics vary by speed. */
-#define ep_desc(g, hs, fs) (((g)->speed == USB_SPEED_HIGH)?(hs):(fs))
+static inline struct usb_endpoint_descriptor *ep_desc(struct usb_gadget *gadget,
+                                       struct usb_endpoint_descriptor *fs,
+                                       struct usb_endpoint_descriptor *hs,
+                                       struct usb_endpoint_descriptor *ss)
+{
+       struct usb_endpoint_descriptor *d = fs;
+
+       if (gadget->speed == USB_SPEED_SUPER)
+               d = ss;
+       else if (gadget->speed == USB_SPEED_HIGH)
+               d = hs;
+
+       return d;
+}
 
 /*-------------------------------------------------------------------------*/
 
@@ -793,11 +844,12 @@ set_printer_interface(struct printer_dev *dev)
 {
        int                     result = 0;
 
-       dev->in_ep->desc = ep_desc(dev->gadget, &hs_ep_in_desc, &fs_ep_in_desc);
+       dev->in_ep->desc = ep_desc(dev->gadget, &fs_ep_in_desc, &hs_ep_in_desc,
+                                                               &ss_ep_in_desc);
        dev->in_ep->driver_data = dev;
 
-       dev->out_ep->desc = ep_desc(dev->gadget, &hs_ep_out_desc,
-                                   &fs_ep_out_desc);
+       dev->out_ep->desc = ep_desc(dev->gadget, &fs_ep_out_desc,
+                                       &hs_ep_out_desc, &ss_ep_out_desc);
        dev->out_ep->driver_data = dev;
 
        result = usb_ep_enable(dev->in_ep);
@@ -1016,9 +1068,11 @@ autoconf_fail:
        /* assumes that all endpoints are dual-speed */
        hs_ep_in_desc.bEndpointAddress = fs_ep_in_desc.bEndpointAddress;
        hs_ep_out_desc.bEndpointAddress = fs_ep_out_desc.bEndpointAddress;
+       ss_ep_in_desc.bEndpointAddress = fs_ep_in_desc.bEndpointAddress;
+       ss_ep_out_desc.bEndpointAddress = fs_ep_out_desc.bEndpointAddress;
 
        ret = usb_assign_descriptors(f, fs_printer_function,
-                       hs_printer_function, NULL);
+                                    hs_printer_function, ss_printer_function);
        if (ret)
                return ret;
 
@@ -1253,7 +1307,7 @@ static __refdata struct usb_composite_driver printer_driver = {
        .name           = shortname,
        .dev            = &device_desc,
        .strings        = dev_strings,
-       .max_speed      = USB_SPEED_HIGH,
+       .max_speed      = USB_SPEED_SUPER,
        .bind           = printer_bind,
        .unbind         = printer_unbind,
 };
Signature · 1 line
1.9.1
#1 lore raw

Hi,

Quoted 6 lines · Jorge Ramirez-Ortiz

Hi,

This patch adds USB3 support to the legacy gadget printer driver. Applies cleanly on fc14f9c Linux 3.18-rc5.

Please could it be considered for inclusion?

sure, if you send it properly (see Documentation/SubmittingPatches), provide logs of your tests with a recent kernel (v3.18-rc5 would be just awesome) and Cc myself on your resubmission.

drivers/usb/gadget/legacy/printer.c line 6

Quoted 9 lines
drivers/usb/gadget/legacy/printer.c +1 -0
index 6474081..625d905 100644
@@ -3,6 +3,7 @@
  *
  * Copyright (C) 2003-2005 David Brownell
  * Copyright (C) 2006 Craig W. Nadler
+ * Copyright (C) 2014 Linaro.org

I don't think the minimal change below constitutes enough to merit the copyright. If your lawyers tell you otherwise, let me know.

Greg, what's Linux Foundation's lawyers take on this ?

Quoted 50 lines
drivers/usb/gadget/legacy/printer.c +38 -1
@@ -208,6 +209,43 @@ static struct usb_descriptor_header *hs_printer_function[] = {
        NULL
 };
 
+/*
+ * Added endpoint descriptors for 3.0 devices
+ */
+
+static struct usb_endpoint_descriptor ss_ep_in_desc = {
+       .bLength =              USB_DT_ENDPOINT_SIZE,
+       .bDescriptorType =      USB_DT_ENDPOINT,
+       .bmAttributes =         USB_ENDPOINT_XFER_BULK,
+       .wMaxPacketSize =       cpu_to_le16(1024),
+};
+
+struct usb_ss_ep_comp_descriptor ss_ep_in_comp_desc = {
+       .bLength =              sizeof(ss_ep_in_comp_desc),
+       .bDescriptorType =      USB_DT_SS_ENDPOINT_COMP,
+};
+
+static struct usb_endpoint_descriptor ss_ep_out_desc = {
+       .bLength =              USB_DT_ENDPOINT_SIZE,
+       .bDescriptorType =      USB_DT_ENDPOINT,
+       .bmAttributes =         USB_ENDPOINT_XFER_BULK,
+       .wMaxPacketSize =       cpu_to_le16(1024),
+};
+
+struct usb_ss_ep_comp_descriptor ss_ep_out_comp_desc = {
+       .bLength =              sizeof(ss_ep_out_comp_desc),
+       .bDescriptorType =      USB_DT_SS_ENDPOINT_COMP,
+};
+
+static struct usb_descriptor_header *ss_printer_function[] = {
+       (struct usb_descriptor_header *) &intf_desc,
+       (struct usb_descriptor_header *) &ss_ep_in_desc,
+       (struct usb_descriptor_header *) &ss_ep_in_comp_desc,
+       (struct usb_descriptor_header *) &ss_ep_out_desc,
+       (struct usb_descriptor_header *) &ss_ep_out_comp_desc,
+       NULL
+};
+
 static struct usb_otg_descriptor otg_descriptor = {
        .bLength =              sizeof otg_descriptor,
        .bDescriptorType =      USB_DT_OTG,
@@ -220,7 +258,20 @@ static const struct usb_descriptor_header *otg_desc[] = {
 };
 
 /* maxpacket and other transfer characteristics vary by speed. */
-#define ep_desc(g, hs, fs) (((g)->speed == USB_SPEED_HIGH)?(hs):(fs))
+static inline struct usb_endpoint_descriptor *ep_desc(struct usb_gadget *gadget,

drivers/usb/gadget/legacy/printer.c static const struct usb_descriptor_header *otg_desc[] = { · line 271

Quoted 10 lines
drivers/usb/gadget/legacy/printer.c +10 -0
+                                       struct usb_endpoint_descriptor *fs,
+                                       struct usb_endpoint_descriptor *hs,
+                                       struct usb_endpoint_descriptor *ss)
+{
+       struct usb_endpoint_descriptor *d = fs;
+
+       if (gadget->speed == USB_SPEED_SUPER)
+               d = ss;
+       else if (gadget->speed == USB_SPEED_HIGH)
+               d = hs;

what happened to good old switch ?

drivers/usb/gadget/legacy/printer.c set_printer_interface(struct printer_dev *dev) · line 848

Quoted 12 lines
drivers/usb/gadget/legacy/printer.c +4 -1
+       return d;
+}
 
 /*-------------------------------------------------------------------------*/
 
@@ -793,11 +844,12 @@ set_printer_interface(struct printer_dev *dev)
 {
        int                     result = 0;
 
-       dev->in_ep->desc = ep_desc(dev->gadget, &hs_ep_in_desc, &fs_ep_in_desc);
+       dev->in_ep->desc = ep_desc(dev->gadget, &fs_ep_in_desc, &hs_ep_in_desc,
+                                                               &ss_ep_in_desc);

Fix your indentation.

Quoted 10 lines
drivers/usb/gadget/legacy/printer.c +2 -2
        dev->in_ep->driver_data = dev;
 
-       dev->out_ep->desc = ep_desc(dev->gadget, &hs_ep_out_desc,
-                                   &fs_ep_out_desc);
+       dev->out_ep->desc = ep_desc(dev->gadget, &fs_ep_out_desc,
+                                       &hs_ep_out_desc, &ss_ep_out_desc);
        dev->out_ep->driver_data = dev;
 
        result = usb_ep_enable(dev->in_ep);

drivers/usb/gadget/legacy/printer.c autoconf_fail: · line 1075

Quoted 9 lines
drivers/usb/gadget/legacy/printer.c +3 -1
@@ -1016,9 +1068,11 @@ autoconf_fail:
        /* assumes that all endpoints are dual-speed */
        hs_ep_in_desc.bEndpointAddress = fs_ep_in_desc.bEndpointAddress;
        hs_ep_out_desc.bEndpointAddress = fs_ep_out_desc.bEndpointAddress;
+       ss_ep_in_desc.bEndpointAddress = fs_ep_in_desc.bEndpointAddress;
+       ss_ep_out_desc.bEndpointAddress = fs_ep_out_desc.bEndpointAddress;
 
        ret = usb_assign_descriptors(f, fs_printer_function,
-                       hs_printer_function, NULL);
+                                    hs_printer_function, ss_printer_function);

why do you change indentation when adding just one extra argument ?

Signature · 1 line
balbi
maintainer
#2 lore raw
Quoted 27 lines · Felipe Balbi

Hi,

Quoted 6 lines · Jorge Ramirez-Ortiz

Hi,

This patch adds USB3 support to the legacy gadget printer driver. Applies cleanly on fc14f9c Linux 3.18-rc5.

Please could it be considered for inclusion?

sure, if you send it properly (see Documentation/SubmittingPatches), provide logs of your tests with a recent kernel (v3.18-rc5 would be just awesome) and Cc myself on your resubmission.

drivers/usb/gadget/legacy/printer.c line 6

Quoted 9 lines
drivers/usb/gadget/legacy/printer.c +1 -0
index 6474081..625d905 100644
@@ -3,6 +3,7 @@
  *
  * Copyright (C) 2003-2005 David Brownell
  * Copyright (C) 2006 Craig W. Nadler
+ * Copyright (C) 2014 Linaro.org

I don't think the minimal change below constitutes enough to merit the copyright. If your lawyers tell you otherwise, let me know.

Quoted 1 line

Greg, what's Linux Foundation's lawyers take on this ?

I'm not going to speak for the Linux Foundation here (we have no in-house lawyers), but I have been advised that you need to modify/add at least 1/3 of the file before you can add your copyright notice to a file. That's the rule I go by, and numerous lawyers I have asked about this say it is a safe rule to follow.

Linaro should also be following those rules, last I checked with them, so Jorge, you should go back and ask for clarification.

But then there's the issue that copyright notices in files really don't mean anything, but I'm not going to get into that discussion right now :)

thanks,

greg k-h

#3 lore raw
Quoted 27 lines · Greg KH
Quoted 25 lines · Felipe Balbi

Hi,

Quoted 6 lines · Jorge Ramirez-Ortiz

Hi,

This patch adds USB3 support to the legacy gadget printer driver. Applies cleanly on fc14f9c Linux 3.18-rc5.

Please could it be considered for inclusion?

sure, if you send it properly (see Documentation/SubmittingPatches), provide logs of your tests with a recent kernel (v3.18-rc5 would be just awesome) and Cc myself on your resubmission.

drivers/usb/gadget/legacy/printer.c line 6

Quoted 9 lines
drivers/usb/gadget/legacy/printer.c +1 -0
index 6474081..625d905 100644
@@ -3,6 +3,7 @@
  *
  * Copyright (C) 2003-2005 David Brownell
  * Copyright (C) 2006 Craig W. Nadler
+ * Copyright (C) 2014 Linaro.org

I don't think the minimal change below constitutes enough to merit the copyright. If your lawyers tell you otherwise, let me know.

Quoted 1 line

Greg, what's Linux Foundation's lawyers take on this ?

Quoted 8 lines

I'm not going to speak for the Linux Foundation here (we have no in-house lawyers), but I have been advised that you need to modify/add at least 1/3 of the file before you can add your copyright notice to a file. That's the rule I go by, and numerous lawyers I have asked about this say it is a safe rule to follow.

Linaro should also be following those rules, last I checked with them, so Jorge, you should go back and ask for clarification.

Yes, let's drop the copyright - my fault.

Quoted 7 lines

But then there's the issue that copyright notices in files really don't mean anything, but I'm not going to get into that discussion right now :)

thanks,

greg k-h

#4 lore raw

PATCH 1/1 usb: gadget: USB3 support to the legacy printer driver

In reply to Felipe Balbi

Hi Felipe/Greg

Thanks for your comments on my previous attempt.
I think I addressed them here.
I added some logs of a run captured on a recent kernel, fixed the indentations, replaced the if/else with a switch statement and removed the copyright.

Note that the tests seem to indicate a performance issue on the net2280 driver: the exact same tests using the usb338x.c driver from PLX allows for transfer speeds of 1Gbps.

cheers
Jorge

File transfer test using g_printer on 10b5:3380
================================================

The host will transfer a file to the device using the g_printer driver.

0) enable the net2280 on the g_printer:
--------------------------------------

From 8e306693839a77bfe3411a842d4d20acb9dae9e3 Mon Sep 17 00:00:00 2001
From: Jorge Ramirez-Ortiz <[email protected]>
Date: Mon, 17 Nov 2014 22:31:59 -0500
Subject: [PATCH] use the 338x

---
drivers/usb/gadget/legacy/printer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c
b/drivers/usb/gadget/legacy/printer.c

       index 456730b..ba919a0 100644
       --- a/drivers/usb/gadget/legacy/printer.c
       +++ b/drivers/usb/gadget/legacy/printer.c
       @@ -99,8 +99,8 @@ static struct printer_dev usb_printer_gadget;
      
        /* Thanks to NetChip Technologies for donating this product ID.
         */
       -#define PRINTER_VENDOR_NUM     0x0525          /* NetChip */
       -#define PRINTER_PRODUCT_NUM    0xa4a8          /* Linux-USB Printer Gadget */
       +#define PRINTER_VENDOR_NUM     0x10B5        
       +#define PRINTER_PRODUCT_NUM    0x3380        
      
        /* Some systems will want different product identifiers published in the
         * device descriptor, either numbers or strings or both.  These string
       --
       1.9.1

1) Host logs:
-------------

[jramirez@miro ~]$ lsusb
Bus 002 Device 006: ID 05ac:1303 Apple, Inc. iPod Shuffle 4.Gen
Bus 002 Device 002: ID 8087:0024 Intel Corp. Integrated Rate Matching Hub Bus 002 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 004 Device 007: ID 10b5:3380 Comodo (PLX?)
Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 003 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 001 Device 005: ID 0403:6001 Future Technology Devices International, Ltd FT232 USB-Serial (UART) IC
Bus 001 Device 003: ID 046d:0990 Logitech, Inc. QuickCam Pro 9000
Bus 001 Device 002: ID 8087:0024 Intel Corp. Integrated Rate Matching Hub Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub

[jramirez@miro ~]$ lsusb -s 004:007 -v

Bus 004 Device 007: ID 10b5:3380 Comodo (PLX?) Couldn't open device, some information will be missing Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 3.00

  bDeviceClass            0 (Defined at Interface level)
  bDeviceSubClass         0
  bDeviceProtocol         0
  bMaxPacketSize0         9
  idVendor           0x10b5 Comodo (PLX?)
  idProduct          0x3380
  bcdDevice            3.18
  iManufacturer           1
  iProduct                2
  iSerial                 3
  bNumConfigurations      1

Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 44
bNumInterfaces 1
bConfigurationValue 1
iConfiguration 0
bmAttributes 0xc0
Self Powered
MaxPower 2mA
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 0
bAlternateSetting 0
bNumEndpoints 2
bInterfaceClass 7 Printer
bInterfaceSubClass 1 Printer
bInterfaceProtocol 2 Bidirectional
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81 EP 1 IN
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0400 1x 1024 bytes
bInterval 0
bMaxBurst 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x01 EP 1 OUT
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0400 1x 1024 bytes
bInterval 0
bMaxBurst 0

[jramirez@miro host.git (master%)]$ sudo ./usbhost file.wav [sudo] password for jramirez:
Opening device 10B5:3380...

Device properties:
bus number: 4
port path: 2 (from root hub)
device speed: 5000 Mbit/s (USB SuperSpeed)

Reading device descriptor:

            length: 18
      device class: 0
               S/N: 3
           VID:PID: 10B5:3380
         bcdDevice: 0318

iMan:iProd:iSer: 1:2:3
nb confs: 1

Reading BOS descriptor: 2 caps
USB 2.0 extension:
attributes : 06
USB 3.0 capabilities:
attributes : 00
supported speeds : 000F
supported functionality: 01

Reading first configuration descriptor:
nb interfaces: 1
interface[0]: id = 0
interface[0].altsetting[0]: num endpoints = 2
Class.SubClass.Protocol: 07.01.02

       endpoint[0].address: 81
           max packet size: 0400
          polling interval: 00
                 max burst: 00   (USB 3.0)
        bytes per interval: 0000 (USB 3.0)
       endpoint[1].address: 01
           max packet size: 0400
          polling interval: 00
                 max burst: 00   (USB 3.0)
        bytes per interval: 0000 (USB 3.0)

Claiming interface 0...

Reading string descriptors:
String (0x01): "Linux 3.18.0-rc5+ with net2280" String (0x02): "Printer Gadget"
Transfering: endpoint_out 1, size 61387314
- number of bulk transfers : 7494
- max user transfer size : 8192 bytes
- max usb transfer size : 1024 bytes

2) Device Logs:
--------------

[jramirez@dali ~]$ cat /proc/version
Linux version 3.18.0-rc5+ ([email protected]) (gcc version 4.8.3 20140911 (Red Hat 4.8.3-7) (GCC) ) #1 SMP Mon Nov 17 21:59:22 EST 2014

[   72.345683] net2280 0000:02:00.0: usb_reset_338x: Defect 7374 FsmValue 0xf0000000
[   72.345706] net2280 0000:02:00.0: usb_reinit_338x: Defect 7374 FsmValue f0000000
[   72.345748] net2280 0000:02:00.0: irq 35 for MSI/MSI-X
[   72.345798] net2280 0000:02:00.0: PLX NET228x/USB338x USB Peripheral Controller
[   72.345801] net2280 0000:02:00.0: irq 35, pci mem ffffc90004960000, chip rev 00ab
[   72.345803] net2280 0000:02:00.0: version: 2005 Sept 27/v3.0; dma enabled legacy mode
[   86.630589] printer gadget: Printer Gadget, version: 2007 OCT 06
[   86.630593] printer gadget: printer ready
[   86.630599] net2280 0000:02:00.0: Operate Defect 7374 workaround soft this time
[   86.630600] net2280 0000:02:00.0: It will operate on cold-reboot and SS connect
[   86.630709] net2280 0000:02:00.0: ep0_start_338x: Defect 7374 FsmValue 10000000
[   86.870669] net2280 0000:02:00.0: INFO: Defect 7374 workaround waited about
[   86.875065] printer gadget: super-speed config #1: printer
[   86.875077] printer gadget: Using interface 0

[jramirez@dali device.git (master%)]$ sudo ./usbdevice Receiving file

Transfer rate => 462 Mbits/sec [57MB/sec]
- file size : 58 MB
- time : 1.12 sec

As mentioned above, using the usb338x driver from PLX instead of the net2280 from kernel.org, the effective file transfer rate increases 1Gbps.

3) Patch:
---------

Quoted 1 line

From 9b5ee9330c5c02cf51328c350036c1dac998b732 Mon Sep 17 00:00:00 2001

From: Jorge Ramirez-Ortiz <[email protected]>
Date: Thu, 25 Sep 2014 16:17:20 -0400
Subject: [PATCH 2/3] usb: gadget: add USB3 support to the printer driver

Add SS descriptors to support the capabilities provided by USB3 controller drivers; unit tests run using a PLX 3380 [max transfer speed measured of 1Gbps]

This driver shall fallback to lower operating modes when the higher ones are not available.

---
 drivers/usb/gadget/legacy/printer.c | 65 +++++++++++++++++++++++++++++++++----
 1 file changed, 59 insertions(+), 6 deletions(-)
drivers/usb/gadget/legacy/printer.c +59 -6
index 6474081..456730b 100644
@@ -208,6 +208,43 @@ static struct usb_descriptor_header *hs_printer_function[] = {
     NULL
 };
 
+/*
+ * Added endpoint descriptors for 3.0 devices
+ */
+
+static struct usb_endpoint_descriptor ss_ep_in_desc = {
+    .bLength =              USB_DT_ENDPOINT_SIZE,
+    .bDescriptorType =      USB_DT_ENDPOINT,
+    .bmAttributes =         USB_ENDPOINT_XFER_BULK,
+    .wMaxPacketSize =       cpu_to_le16(1024),
+};
+
+struct usb_ss_ep_comp_descriptor ss_ep_in_comp_desc = {
+    .bLength =              sizeof(ss_ep_in_comp_desc),
+    .bDescriptorType =      USB_DT_SS_ENDPOINT_COMP,
+};
+
+static struct usb_endpoint_descriptor ss_ep_out_desc = {
+    .bLength =              USB_DT_ENDPOINT_SIZE,
+    .bDescriptorType =      USB_DT_ENDPOINT,
+    .bmAttributes =         USB_ENDPOINT_XFER_BULK,
+    .wMaxPacketSize =       cpu_to_le16(1024),
+};
+
+struct usb_ss_ep_comp_descriptor ss_ep_out_comp_desc = {
+    .bLength =              sizeof(ss_ep_out_comp_desc),
+    .bDescriptorType =      USB_DT_SS_ENDPOINT_COMP,
+};
+
+static struct usb_descriptor_header *ss_printer_function[] = {
+    (struct usb_descriptor_header *) &intf_desc,
+    (struct usb_descriptor_header *) &ss_ep_in_desc,
+    (struct usb_descriptor_header *) &ss_ep_in_comp_desc,
+    (struct usb_descriptor_header *) &ss_ep_out_desc,
+    (struct usb_descriptor_header *) &ss_ep_out_comp_desc,
+    NULL
+};
+
 static struct usb_otg_descriptor otg_descriptor = {
     .bLength =              sizeof otg_descriptor,
     .bDescriptorType =      USB_DT_OTG,
@@ -220,7 +257,20 @@ static const struct usb_descriptor_header *otg_desc[] = {
 };
 
 /* maxpacket and other transfer characteristics vary by speed. */
-#define ep_desc(g, hs, fs) (((g)->speed == USB_SPEED_HIGH)?(hs):(fs))
+static inline struct usb_endpoint_descriptor *ep_desc(struct usb_gadget *gadget,
+                    struct usb_endpoint_descriptor *fs,
+                    struct usb_endpoint_descriptor *hs,
+                    struct usb_endpoint_descriptor *ss)
+{
+    switch(gadget->speed) {
+    case USB_SPEED_SUPER:
+        return ss;
+    case USB_SPEED_HIGH:
+        return hs;
+    default:
+        return fs;
+    }
+}
 
 /*-------------------------------------------------------------------------*/
 
@@ -793,11 +843,12 @@ set_printer_interface(struct printer_dev *dev)
 {
     int            result = 0;
 
-    dev->in_ep->desc = ep_desc(dev->gadget, &hs_ep_in_desc, &fs_ep_in_desc);
+    dev->in_ep->desc = ep_desc(dev->gadget, &fs_ep_in_desc, &hs_ep_in_desc,
+                &ss_ep_in_desc);
     dev->in_ep->driver_data = dev;
 
-    dev->out_ep->desc = ep_desc(dev->gadget, &hs_ep_out_desc,
-                    &fs_ep_out_desc);
+    dev->out_ep->desc = ep_desc(dev->gadget, &fs_ep_out_desc,
+                    &hs_ep_out_desc, &ss_ep_out_desc);
     dev->out_ep->driver_data = dev;
 
     result = usb_ep_enable(dev->in_ep);
@@ -1016,9 +1067,11 @@ autoconf_fail:
     /* assumes that all endpoints are dual-speed */
     hs_ep_in_desc.bEndpointAddress = fs_ep_in_desc.bEndpointAddress;
     hs_ep_out_desc.bEndpointAddress = fs_ep_out_desc.bEndpointAddress;
+    ss_ep_in_desc.bEndpointAddress = fs_ep_in_desc.bEndpointAddress;
+    ss_ep_out_desc.bEndpointAddress = fs_ep_out_desc.bEndpointAddress;
 
     ret = usb_assign_descriptors(f, fs_printer_function,
-            hs_printer_function, NULL);
+            hs_printer_function, ss_printer_function);
     if (ret)
         return ret;
 
@@ -1253,7 +1306,7 @@ static __refdata struct usb_composite_driver printer_driver = {
     .name           = shortname,
     .dev            = &device_desc,
     .strings        = dev_strings,
-    .max_speed      = USB_SPEED_HIGH,
+    .max_speed      = USB_SPEED_SUPER,
     .bind        = printer_bind,
     .unbind        = printer_unbind,
 };
Signature · 1 line
1.9.1
#5 lore raw

Hi,

Quoted 4 lines · Jorge Ramirez-Ortiz

Hi Felipe/Greg

Thanks for your comments on my previous attempt.
I think I addressed them here.

no you haven't. Read Documentation/SubmittingPatches, read the mailing list archives and you'll see your basic mistake.

Quoted 28 lines

I added some logs of a run captured on a recent kernel, fixed the indentations, replaced the if/else with a switch statement and removed the copyright.

Note that the tests seem to indicate a performance issue on the net2280 driver: the exact same tests using the usb338x.c driver from PLX allows for transfer speeds of 1Gbps.

cheers
Jorge

File transfer test using g_printer on 10b5:3380
================================================

The host will transfer a file to the device using the g_printer driver.

0) enable the net2280 on the g_printer:
--------------------------------------

From 8e306693839a77bfe3411a842d4d20acb9dae9e3 Mon Sep 17 00:00:00 2001
From: Jorge Ramirez-Ortiz <[email protected]>
Date: Mon, 17 Nov 2014 22:31:59 -0500
Subject: [PATCH] use the 338x

---
drivers/usb/gadget/legacy/printer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/legacy/printer.c
b/drivers/usb/gadget/legacy/printer.c

Quoted 11 lines
       index 456730b..ba919a0 100644
       --- a/drivers/usb/gadget/legacy/printer.c
       +++ b/drivers/usb/gadget/legacy/printer.c
       @@ -99,8 +99,8 @@ static struct printer_dev usb_printer_gadget;
      
        /* Thanks to NetChip Technologies for donating this product ID.
         */
       -#define PRINTER_VENDOR_NUM     0x0525          /* NetChip */
       -#define PRINTER_PRODUCT_NUM    0xa4a8          /* Linux-USB Printer Gadget */
       +#define PRINTER_VENDOR_NUM     0x10B5        
       +#define PRINTER_PRODUCT_NUM    0x3380        

you have no clue what these mean, do you ? How about reading the USB specification of even http://www.beyondlogic.org/usbnutshell/usb1.shtml

Changing these will not cause you to use usb338x driver, you're just using a vendor/product ID you don't own.

Quoted 165 lines
      
        /* Some systems will want different product identifiers published in the
         * device descriptor, either numbers or strings or both.  These string
       --
       1.9.1

1) Host logs:
-------------

[jramirez@miro ~]$ lsusb
Bus 002 Device 006: ID 05ac:1303 Apple, Inc. iPod Shuffle 4.Gen
Bus 002 Device 002: ID 8087:0024 Intel Corp. Integrated Rate Matching Hub Bus 002 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 004 Device 007: ID 10b5:3380 Comodo (PLX?)
Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
Bus 003 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 001 Device 005: ID 0403:6001 Future Technology Devices International, Ltd FT232 USB-Serial (UART) IC
Bus 001 Device 003: ID 046d:0990 Logitech, Inc. QuickCam Pro 9000
Bus 001 Device 002: ID 8087:0024 Intel Corp. Integrated Rate Matching Hub Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub

[jramirez@miro ~]$ lsusb -s 004:007 -v

Bus 004 Device 007: ID 10b5:3380 Comodo (PLX?) Couldn't open device, some information will be missing Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 3.00

  bDeviceClass            0 (Defined at Interface level)
  bDeviceSubClass         0
  bDeviceProtocol         0
  bMaxPacketSize0         9
  idVendor           0x10b5 Comodo (PLX?)
  idProduct          0x3380
  bcdDevice            3.18
  iManufacturer           1
  iProduct                2
  iSerial                 3
  bNumConfigurations      1

Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 44
bNumInterfaces 1
bConfigurationValue 1
iConfiguration 0
bmAttributes 0xc0
Self Powered
MaxPower 2mA
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 0
bAlternateSetting 0
bNumEndpoints 2
bInterfaceClass 7 Printer
bInterfaceSubClass 1 Printer
bInterfaceProtocol 2 Bidirectional
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81 EP 1 IN
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0400 1x 1024 bytes
bInterval 0
bMaxBurst 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x01 EP 1 OUT
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0400 1x 1024 bytes
bInterval 0
bMaxBurst 0

[jramirez@miro host.git (master%)]$ sudo ./usbhost file.wav [sudo] password for jramirez:
Opening device 10B5:3380...

Device properties:
bus number: 4
port path: 2 (from root hub)
device speed: 5000 Mbit/s (USB SuperSpeed)

Reading device descriptor:

            length: 18
      device class: 0
               S/N: 3
           VID:PID: 10B5:3380
         bcdDevice: 0318

iMan:iProd:iSer: 1:2:3
nb confs: 1

Reading BOS descriptor: 2 caps
USB 2.0 extension:
attributes : 06
USB 3.0 capabilities:
attributes : 00
supported speeds : 000F
supported functionality: 01

Reading first configuration descriptor:
nb interfaces: 1
interface[0]: id = 0
interface[0].altsetting[0]: num endpoints = 2
Class.SubClass.Protocol: 07.01.02

       endpoint[0].address: 81
           max packet size: 0400
          polling interval: 00
                 max burst: 00   (USB 3.0)
        bytes per interval: 0000 (USB 3.0)
       endpoint[1].address: 01
           max packet size: 0400
          polling interval: 00
                 max burst: 00   (USB 3.0)
        bytes per interval: 0000 (USB 3.0)

Claiming interface 0...

Reading string descriptors:

   String (0x01): "Linux 3.18.0-rc5+ with net2280"
   String (0x02): "Printer Gadget"

Transfering: endpoint_out 1, size 61387314
- number of bulk transfers : 7494
- max user transfer size : 8192 bytes
- max usb transfer size : 1024 bytes

2) Device Logs:
--------------

[jramirez@dali ~]$ cat /proc/version
Linux version 3.18.0-rc5+ ([email protected]) (gcc version 4.8.3 20140911 (Red Hat 4.8.3-7) (GCC) ) #1 SMP Mon Nov 17 21:59:22 EST 2014

[   72.345683] net2280 0000:02:00.0: usb_reset_338x: Defect 7374 FsmValue 0xf0000000
[   72.345706] net2280 0000:02:00.0: usb_reinit_338x: Defect 7374 FsmValue f0000000
[   72.345748] net2280 0000:02:00.0: irq 35 for MSI/MSI-X
[   72.345798] net2280 0000:02:00.0: PLX NET228x/USB338x USB Peripheral Controller
[   72.345801] net2280 0000:02:00.0: irq 35, pci mem ffffc90004960000, chip rev 00ab
[   72.345803] net2280 0000:02:00.0: version: 2005 Sept 27/v3.0; dma enabled legacy mode
[   86.630589] printer gadget: Printer Gadget, version: 2007 OCT 06
[   86.630593] printer gadget: printer ready
[   86.630599] net2280 0000:02:00.0: Operate Defect 7374 workaround soft this time
[   86.630600] net2280 0000:02:00.0: It will operate on cold-reboot and SS connect
[   86.630709] net2280 0000:02:00.0: ep0_start_338x: Defect 7374 FsmValue 10000000
[   86.870669] net2280 0000:02:00.0: INFO: Defect 7374 workaround waited about
[   86.875065] printer gadget: super-speed config #1: printer
[   86.875077] printer gadget: Using interface 0

[jramirez@dali device.git (master%)]$ sudo ./usbdevice Receiving file

Quoted 7 lines

Transfer rate => 462 Mbits/sec [57MB/sec]
- file size : 58 MB
- time : 1.12 sec

As mentioned above, using the usb338x driver from PLX instead of the net2280 from kernel.org, the effective file transfer rate increases 1Gbps.

do you want to debug that and find the culprit since you're already at it ?

Quoted 31 lines

3) Patch:
---------

From 9b5ee9330c5c02cf51328c350036c1dac998b732 Mon Sep 17 00:00:00 2001
From: Jorge Ramirez-Ortiz <[email protected]>
Date: Thu, 25 Sep 2014 16:17:20 -0400
Subject: [PATCH 2/3] usb: gadget: add USB3 support to the printer driver

Add SS descriptors to support the capabilities provided by USB3 controller drivers; unit tests run using a PLX 3380 [max transfer speed measured of 1Gbps]

This driver shall fallback to lower operating modes when the higher ones are not available.

---
 drivers/usb/gadget/legacy/printer.c | 65 +++++++++++++++++++++++++++++++++----
 1 file changed, 59 insertions(+), 6 deletions(-)
drivers/usb/gadget/legacy/printer.c +0 -0
index 6474081..456730b 100644
@@ -208,6 +208,43 @@ static struct usb_descriptor_header *hs_printer_function[] = {
     NULL
 };
 

drivers/usb/gadget/legacy/printer.c static struct usb_descriptor_header *hs_printer_function[] = { · line 220

Quoted 10 lines
drivers/usb/gadget/legacy/printer.c +10 -0
+/*
+ * Added endpoint descriptors for 3.0 devices
+ */
+
+static struct usb_endpoint_descriptor ss_ep_in_desc = {
+    .bLength =              USB_DT_ENDPOINT_SIZE,
+    .bDescriptorType =      USB_DT_ENDPOINT,
+    .bmAttributes =         USB_ENDPOINT_XFER_BULK,
+    .wMaxPacketSize =       cpu_to_le16(1024),
+};

all your tabs have been converted into spaces. Perhaps try:

	$ git help send-email

and figure out how to use that ?

Signature · 1 line
balbi
#6 lore raw
Quoted 9 lines · Felipe Balbi

Hi,

Quoted 4 lines · Jorge Ramirez-Ortiz

Hi Felipe/Greg

Thanks for your comments on my previous attempt.
I think I addressed them here.

no you haven't. Read Documentation/SubmittingPatches, read the mailing list archives and you'll see your basic mistake.

For this fundamental mistake, could you not just share with me what I am missing?
I don't see anything wrong other than maybe adding more maintainers to the DL as per the scripts since the feature itself is self explanatory.

Quoted 2 lines

you have no clue what these mean, do you ? How about reading the USB specification of even http://www.beyondlogic.org/usbnutshell/usb1.shtml

Unfortunately I do.
It was easier to temporarily hack the driver code for a test - while I was at it - rather than modifying the host code. Since you asked for them, I though you would read the logs and wonder where the funny ids where coming from. That hack above would have given you an answer: so I kind of know what the ids are for. honestly. anyway, will send the new logs - it took me a while to find and modify the host test code.

Quoted 3 lines

do you want to debug that and find the culprit since you're already at it ?

probably: I still need to get used to this process, thanks for bearing with me on this.

I spoke to Ricardo Ribalda three months ago while I was doing this stuff.
but yes, I might work on this -after I finish with this patch!- since I have access to the hardware locally.

Quoted 46 lines
Quoted 41 lines

3) Patch:
---------

From 9b5ee9330c5c02cf51328c350036c1dac998b732 Mon Sep 17 00:00:00 2001
From: Jorge Ramirez-Ortiz <[email protected]>
Date: Thu, 25 Sep 2014 16:17:20 -0400
Subject: [PATCH 2/3] usb: gadget: add USB3 support to the printer driver

Add SS descriptors to support the capabilities provided by USB3 controller drivers; unit tests run using a PLX 3380 [max transfer speed measured of 1Gbps]

This driver shall fallback to lower operating modes when the higher ones are not available.

---
 drivers/usb/gadget/legacy/printer.c | 65 +++++++++++++++++++++++++++++++++----
 1 file changed, 59 insertions(+), 6 deletions(-)
drivers/usb/gadget/legacy/printer.c +10 -0
index 6474081..456730b 100644
@@ -208,6 +208,43 @@ static struct usb_descriptor_header *hs_printer_function[] = {
     NULL
 };
 
+/*
+ * Added endpoint descriptors for 3.0 devices
+ */
+
+static struct usb_endpoint_descriptor ss_ep_in_desc = {
+    .bLength =              USB_DT_ENDPOINT_SIZE,
+    .bDescriptorType =      USB_DT_ENDPOINT,
+    .bmAttributes =         USB_ENDPOINT_XFER_BULK,
+    .wMaxPacketSize =       cpu_to_le16(1024),
+};
Quoted 3 lines

As mentioned above, using the usb338x driver from PLX instead of the net2280 from kernel.org, the effective file transfer rate increases 1Gbps.

all your tabs have been converted into spaces. Perhaps try:

Quoted 4 lines
	$ git help send-email

and figure out how to use that ?

crap. really sorry about this!

#7 lore raw

Hi,

(fix your mailer, lines should be broken at 80-characters. Documentation/email-clients.txt has tips)

Quoted 8 lines · Jorge Ramirez-Ortiz
Quoted 7 lines · Felipe Balbi

Hi,

Quoted 4 lines · Jorge Ramirez-Ortiz

Hi Felipe/Greg

Thanks for your comments on my previous attempt.
I think I addressed them here.

Quoted 8 lines
Quoted 2 lines

no you haven't. Read Documentation/SubmittingPatches, read the mailing list archives and you'll see your basic mistake.

For this fundamental mistake, could you not just share with me what I am missing?
I don't see anything wrong other than maybe adding more maintainers to the DL as per the scripts since the feature itself is self explanatory.

Try to save your email from the mailing list and apply it with git am, it should be very easy to see the problem.

If you also look at the mailing list archives for other patch submissions (as I suggested) you'll easily see what your mistake is.

Quoted 9 lines
Quoted 2 lines

you have no clue what these mean, do you ? How about reading the USB specification of even http://www.beyondlogic.org/usbnutshell/usb1.shtml

Unfortunately I do.
It was easier to temporarily hack the driver code for a test - while I was at it - rather than modifying the host code.
Since you asked for them, I though you would read the logs and wonder where the funny ids where coming from.

why do you even need to hack the host driver for these ? The driver shows a Printer Class interface and the linux host side driver should bind to it without any issues.

Quoted 3 lines

That hack above would have given you an answer: so I kind of know what the ids are for. honestly. anyway, will send the new logs - it took me a while to find and modify the host test code.

Which host test code ? Why don't you just use lpr or even cat file > /dev/lp0 or something like that ?

Quoted 5 lines
Quoted 2 lines

do you want to debug that and find the culprit since you're already at it ?

probably: I still need to get used to this process, thanks for bearing with me on this.

no problem.

Quoted 3 lines

I spoke to Ricardo Ribalda three months ago while I was doing this stuff. but yes, I might work on this -after I finish with this patch!- since I have access to the hardware locally.

cool, that'll help.

Quoted 10 lines
Quoted 10 lines
Quoted 10 lines

3) Patch:
---------

From 9b5ee9330c5c02cf51328c350036c1dac998b732 Mon Sep 17 00:00:00 2001
From: Jorge Ramirez-Ortiz <[email protected]>
Date: Thu, 25 Sep 2014 16:17:20 -0400
Subject: [PATCH 2/3] usb: gadget: add USB3 support to the printer driver

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

Add SS descriptors to support the capabilities provided by USB3 controller drivers; unit tests run using a PLX 3380 [max transfer speed measured of 1Gbps]

This driver shall fallback to lower operating modes when the higher ones are not available.

hint:

Everything up until this point will go to commit log.

Quoted 27 lines
Quoted 27 lines
Quoted 22 lines
---
 drivers/usb/gadget/legacy/printer.c | 65 +++++++++++++++++++++++++++++++++----
 1 file changed, 59 insertions(+), 6 deletions(-)
drivers/usb/gadget/legacy/printer.c +10 -0
index 6474081..456730b 100644
@@ -208,6 +208,43 @@ static struct usb_descriptor_header *hs_printer_function[] = {
     NULL
 };
 
+/*
+ * Added endpoint descriptors for 3.0 devices
+ */
+
+static struct usb_endpoint_descriptor ss_ep_in_desc = {
+    .bLength =              USB_DT_ENDPOINT_SIZE,
+    .bDescriptorType =      USB_DT_ENDPOINT,
+    .bmAttributes =         USB_ENDPOINT_XFER_BULK,
+    .wMaxPacketSize =       cpu_to_le16(1024),
+};
Quoted 3 lines

As mentioned above, using the usb338x driver from PLX instead of the net2280 from kernel.org, the effective file transfer rate increases 1Gbps.

all your tabs have been converted into spaces. Perhaps try:

Quoted 6 lines
Quoted 4 lines
	$ git help send-email

and figure out how to use that ?

crap. really sorry about this!

np

Signature · 1 line
balbi
#8 lore raw
Quoted 21 lines · Felipe Balbi

Hi,

(fix your mailer, lines should be broken at 80-characters. Documentation/email-clients.txt has tips)

Quoted 8 lines · Jorge Ramirez-Ortiz
Quoted 7 lines · Felipe Balbi

Hi,

Quoted 4 lines · Jorge Ramirez-Ortiz

Hi Felipe/Greg

Thanks for your comments on my previous attempt.
I think I addressed them here.

Quoted 7 lines
Quoted 2 lines

no you haven't. Read Documentation/SubmittingPatches, read the mailing list archives and you'll see your basic mistake.

For this fundamental mistake, could you not just share with me what I am missing?
I don't see anything wrong other than maybe adding more maintainers to the DL as per the scripts since the feature itself is self explanatory.

Quoted 5 lines

Try to save your email from the mailing list and apply it with git am, it should be very easy to see the problem.

If you also look at the mailing list archives for other patch submissions (as I suggested) you'll easily see what your mistake is.

thanks, I get it now.

Quoted 4 lines
Quoted 4 lines
Quoted 3 lines

you have no clue what these mean, do you ? How about reading the USB specification of even http://www.beyondlogic.org/usbnutshell/usb1.shtml

Quoted 8 lines
Quoted 5 lines

Unfortunately I do.
It was easier to temporarily hack the driver code for a test - while I was at it - rather than modifying the host code.
Since you asked for them, I though you would read the logs and wonder where the funny ids where coming from.

why do you even need to hack the host driver for these ? The driver shows a Printer Class interface and the linux host side driver should bind to it without any issues.

the hack was on the gadget side.

the usbhost test code in charge of sending the file to the device had the wrong ids. to save time -since I was modifying the gadget driver code and only for the tests (it is not part of the final patch) - I hacked those ids on the printer.c file.
but anyway. lets move on. I removed those, recompiled the usb host code and sent the new traces.

Quoted 6 lines
Quoted 4 lines

That hack above would have given you an answer: so I kind of know what the ids are for. honestly. anyway, will send the new logs - it took me a while to find and modify the host test code.

Which host test code ? Why don't you just use lpr or even cat file > /dev/lp0 or something like that ?

it is some proprietary code that links libusb -part of a different project: it was useful as it generated some metrics I was interested in.

Quoted 11 lines
Quoted 5 lines
Quoted 3 lines

do you want to debug that and find the culprit since you're already at it ?

probably: I still need to get used to this process, thanks for bearing with me on this.

no problem.

Quoted 3 lines

I spoke to Ricardo Ribalda three months ago while I was doing this stuff. but yes, I might work on this -after I finish with this patch!- since I have access to the hardware locally.

cool, that'll help.

notice that the original PLX driver was still far from the theoretical 5Gbps target (I was expecting to measure at least 3Gbps and could only get 1Gbps). So 1Gbps should be the target to meet on the kernel.org net2280 - do you agree?

Quoted 49 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines

3) Patch:
---------

From 9b5ee9330c5c02cf51328c350036c1dac998b732 Mon Sep 17 00:00:00 2001
From: Jorge Ramirez-Ortiz <[email protected]>
Date: Thu, 25 Sep 2014 16:17:20 -0400
Subject: [PATCH 2/3] usb: gadget: add USB3 support to the printer driver

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines

Add SS descriptors to support the capabilities provided by USB3 controller drivers; unit tests run using a PLX 3380 [max transfer speed measured of 1Gbps]

This driver shall fallback to lower operating modes when the higher ones are not available.

hint:

Everything up until this point will go to commit log.

Quoted 27 lines
Quoted 27 lines
Quoted 22 lines
---
 drivers/usb/gadget/legacy/printer.c | 65 +++++++++++++++++++++++++++++++++----
 1 file changed, 59 insertions(+), 6 deletions(-)
drivers/usb/gadget/legacy/printer.c +10 -0
index 6474081..456730b 100644
@@ -208,6 +208,43 @@ static struct usb_descriptor_header *hs_printer_function[] = {
     NULL
 };
 
+/*
+ * Added endpoint descriptors for 3.0 devices
+ */
+
+static struct usb_endpoint_descriptor ss_ep_in_desc = {
+    .bLength =              USB_DT_ENDPOINT_SIZE,
+    .bDescriptorType =      USB_DT_ENDPOINT,
+    .bmAttributes =         USB_ENDPOINT_XFER_BULK,
+    .wMaxPacketSize =       cpu_to_le16(1024),
+};
Quoted 3 lines

As mentioned above, using the usb338x driver from PLX instead of the net2280 from kernel.org, the effective file transfer rate increases 1Gbps.

all your tabs have been converted into spaces. Perhaps try:

Quoted 7 lines
Quoted 5 lines
Quoted 4 lines
	$ git help send-email

and figure out how to use that ?

crap. really sorry about this!

np

it should be fine now.

#9 lore raw

Hi,

Quoted 12 lines · Jorge Ramirez-Ortiz
Quoted 11 lines
Quoted 8 lines
Quoted 2 lines

you have no clue what these mean, do you ? How about reading the USB specification of even http://www.beyondlogic.org/usbnutshell/usb1.shtml

Unfortunately I do.
It was easier to temporarily hack the driver code for a test - while I was at it - rather than modifying the host code.
Since you asked for them, I though you would read the logs and wonder where the funny ids where coming from.

why do you even need to hack the host driver for these ? The driver shows a Printer Class interface and the linux host side driver should bind to it without any issues.

Quoted 8 lines

the hack was on the gadget side.

the usbhost test code in charge of sending the file to the device had the wrong ids. to save time -since I was modifying the gadget driver code and only for the tests (it is not part of the final patch) - I hacked those ids on the printer.c file.
but anyway. lets move on. I removed those, recompiled the usb host code and sent the new traces.

then the host side needs a fix because it shouldn't really care about the device ID, rather it should care about the class being printer.

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

That hack above would have given you an answer: so I kind of know what the ids are for. honestly. anyway, will send the new logs - it took me a while to find and modify the host test code.

Which host test code ? Why don't you just use lpr or even cat file > /dev/lp0 or something like that ?

it is some proprietary code that links libusb -part of a different project: it was useful as it generated some metrics I was interested in.

I would be surprised if lpr doesn't work for the same purpose.

Quoted 6 lines
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

do you want to debug that and find the culprit since you're already at it ?

probably: I still need to get used to this process, thanks for bearing with me on this.

no problem.

Quoted 8 lines
Quoted 4 lines
Quoted 3 lines

I spoke to Ricardo Ribalda three months ago while I was doing this stuff. but yes, I might work on this -after I finish with this patch!- since I have access to the hardware locally.

cool, that'll help.

notice that the original PLX driver was still far from the theoretical 5Gbps target (I was expecting to measure at least 3Gbps and could only get 1Gbps). So 1Gbps should be the target to meet on the kernel.org net2280 - do you agree?

this depends on a whole bunch of things. Mainline is a lot different from PLX's kernel tree, I'm sure.

It also depends on how many PCIe lanes you're using. Just because USB3 guarantees 5Gbps bandwidth, if you use a 1x PCIe connector, you'll never get that ;-)

Signature · 1 line
balbi
#10 lore raw
Quoted 15 lines · Felipe Balbi

Hi,

Quoted 12 lines · Jorge Ramirez-Ortiz
Quoted 10 lines
Quoted 7 lines
Quoted 2 lines

you have no clue what these mean, do you ? How about reading the USB specification of even http://www.beyondlogic.org/usbnutshell/usb1.shtml

Unfortunately I do.
It was easier to temporarily hack the driver code for a test - while I was at it - rather than modifying the host code.
Since you asked for them, I though you would read the logs and wonder where the funny ids where coming from.

why do you even need to hack the host driver for these ? The driver shows a Printer Class interface and the linux host side driver should bind to it without any issues.

the hack was on the gadget side.

Quoted 8 lines
Quoted 6 lines

the usbhost test code in charge of sending the file to the device had the wrong ids. to save time -since I was modifying the gadget driver code and only for the tests (it is not part of the final patch) - I hacked those ids on the printer.c file.
but anyway. lets move on. I removed those, recompiled the usb host code and sent the new traces.

then the host side needs a fix because it shouldn't really care about the device ID, rather it should care about the class being printer.

absolutely.
however if you use libusb_open_device_with_vid_pid then well, these things happen...

Quoted 23 lines
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines

That hack above would have given you an answer: so I kind of know what the ids are for. honestly. anyway, will send the new logs - it took me a while to find and modify the host test code.

Which host test code ? Why don't you just use lpr or even cat file > /dev/lp0 or something like that ?

it is some proprietary code that links libusb -part of a different project: it was useful as it generated some metrics I was interested in.

I would be surprised if lpr doesn't work for the same purpose.

Quoted 6 lines
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

do you want to debug that and find the culprit since you're already at it ?

probably: I still need to get used to this process, thanks for bearing with me on this.

no problem.

Quoted 7 lines
Quoted 4 lines
Quoted 3 lines

I spoke to Ricardo Ribalda three months ago while I was doing this stuff. but yes, I might work on this -after I finish with this patch!- since I have access to the hardware locally.

cool, that'll help.

notice that the original PLX driver was still far from the theoretical 5Gbps target (I was expecting to measure at least 3Gbps and could only get 1Gbps). So 1Gbps should be the target to meet on the kernel.org net2280 - do you agree?

Quoted 7 lines

this depends on a whole bunch of things. Mainline is a lot different from PLX's kernel tree, I'm sure.

It also depends on how many PCIe lanes you're using. Just because USB3 guarantees 5Gbps bandwidth, if you use a 1x PCIe connector, you'll never get that ;-)

yes, that is why I purchased a Lenovo ThinkServer TS140 just for this integration. it has one x16 Gen3 PCIe slot, one x1 Gen2 PCIe and one x16 Gen2 PCIe (x4 signal). so this should be enough.

on the host side, I have good server as well.
So really, there is no excuse to not get this right unless there is a problem in the plx silicon but from the Windows based metrics that I saw I dont think so. The only think I am missing is the USB3 analyzer I used to have in my previous company.

#11 lore raw
Quoted 22 lines · Jorge Ramirez-Ortiz
Quoted 21 lines · Felipe Balbi

Hi,

Quoted 18 lines · Jorge Ramirez-Ortiz
Quoted 10 lines
Quoted 7 lines
Quoted 2 lines

you have no clue what these mean, do you ? How about reading the USB specification of even http://www.beyondlogic.org/usbnutshell/usb1.shtml

Unfortunately I do.
It was easier to temporarily hack the driver code for a test - while I was at it - rather than modifying the host code.
Since you asked for them, I though you would read the logs and wonder where the funny ids where coming from.

why do you even need to hack the host driver for these ? The driver shows a Printer Class interface and the linux host side driver should bind to it without any issues.

the hack was on the gadget side.

the usbhost test code in charge of sending the file to the device had the wrong ids. to save time -since I was modifying the gadget driver code and only for the tests (it is not part of the final patch) - I hacked those ids on the printer.c file.
but anyway. lets move on. I removed those, recompiled the usb host code and sent the new traces.

Quoted 5 lines
Quoted 2 lines

then the host side needs a fix because it shouldn't really care about the device ID, rather it should care about the class being printer.

absolutely.
however if you use libusb_open_device_with_vid_pid then well, these things happen...

heh :-)

Quoted 25 lines
Quoted 25 lines
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines

That hack above would have given you an answer: so I kind of know what the ids are for. honestly. anyway, will send the new logs - it took me a while to find and modify the host test code.

Which host test code ? Why don't you just use lpr or even cat file > /dev/lp0 or something like that ?

it is some proprietary code that links libusb -part of a different project: it was useful as it generated some metrics I was interested in.

I would be surprised if lpr doesn't work for the same purpose.

Quoted 6 lines
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

do you want to debug that and find the culprit since you're already at it ?

probably: I still need to get used to this process, thanks for bearing with me on this.

no problem.

Quoted 7 lines
Quoted 4 lines
Quoted 3 lines

I spoke to Ricardo Ribalda three months ago while I was doing this stuff. but yes, I might work on this -after I finish with this patch!- since I have access to the hardware locally.

cool, that'll help.

notice that the original PLX driver was still far from the theoretical 5Gbps target (I was expecting to measure at least 3Gbps and could only get 1Gbps). So 1Gbps should be the target to meet on the kernel.org net2280 - do you agree?

this depends on a whole bunch of things. Mainline is a lot different from PLX's kernel tree, I'm sure.

Quoted 8 lines
Quoted 4 lines

It also depends on how many PCIe lanes you're using. Just because USB3 guarantees 5Gbps bandwidth, if you use a 1x PCIe connector, you'll never get that ;-)

yes, that is why I purchased a Lenovo ThinkServer TS140 just for this integration. it has one x16 Gen3 PCIe slot, one x1 Gen2 PCIe and one x16 Gen2 PCIe (x4 signal). so this should be enough.

right, assuming PLX PCIe card actually supports that :-)

Quoted 5 lines

on the host side, I have good server as well. So really, there is no excuse to not get this right unless there is a problem in the plx silicon but from the Windows based metrics that I saw I dont think so. The only think I am missing is the USB3 analyzer I used to have in my previous company.

yeah, that helps a lot indeed. I'm always using my trusty beagle 5000.

Signature · 1 line
balbi
#12 lore raw
In reply to Felipe Balbi
Quoted 9 lines

From: [email protected] [mailto:[email protected]] On Behalf Of Felipe Balbi
Sent: Tuesday, November 18, 2014 12:47 PM

Quoted 4 lines · Jorge Ramirez-Ortiz

notice that the original PLX driver was still far from the theoretical 5Gbps target (I was expecting to measure at least 3Gbps and could only get 1Gbps). So 1Gbps should be the target to meet on the kernel.org net2280 - do you agree?

Quoted 6 lines

this depends on a whole bunch of things. Mainline is a lot different from PLX's kernel tree, I'm sure.

It also depends on how many PCIe lanes you're using. Just because USB3 guarantees 5Gbps bandwidth, if you use a 1x PCIe connector, you'll never get that ;-)

Being pedantic, USB3 runs at 4Gbps, not 5Gbps. The signal transitions on the bus are at 5GT/s (5 giga-transitions per second), but due to the 8b/10b encoding, that equates to 4Gbps data rate.

Signature · 1 line
Paul
#13 lore raw
Quoted 12 lines · Paul Zimmerman
Quoted 12 lines

From: [email protected] [mailto:[email protected]] On Behalf Of Felipe Balbi
Sent: Tuesday, November 18, 2014 12:47 PM

Quoted 4 lines · Jorge Ramirez-Ortiz

notice that the original PLX driver was still far from the theoretical 5Gbps target (I was expecting to measure at least 3Gbps and could only get 1Gbps). So 1Gbps should be the target to meet on the kernel.org net2280 - do you agree?

this depends on a whole bunch of things. Mainline is a lot different from PLX's kernel tree, I'm sure.

Quoted 7 lines
Quoted 3 lines

It also depends on how many PCIe lanes you're using. Just because USB3 guarantees 5Gbps bandwidth, if you use a 1x PCIe connector, you'll never get that ;-)

Being pedantic, USB3 runs at 4Gbps, not 5Gbps. The signal transitions on the bus are at 5GT/s (5 giga-transitions per second), but due to the 8b/10b encoding, that equates to 4Gbps data rate.

I stand corrected :-)

Signature · 1 line
balbi