Re: [PATCH] usb: gadget: USB3 support to the legacy printer driver

From: Felipe Balbi
Date: Tue Nov 18 2014 - 10:19:20 EST

Hi,

On Tue, Nov 18, 2014 at 09:19:36AM -0500, Jorge Ramirez-Ortiz wrote:
> 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.

> 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 <jorge.ramirez-ortiz@xxxxxxxxxx>
> 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

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.

>
> /* 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+ (jramirez@xxxxxxxxxxxxxxxxxxxxx) (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.

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

>
>
> 3) Patch:
> ---------
>
>
>
> From 9b5ee9330c5c02cf51328c350036c1dac998b732 Mon Sep 17 00:00:00 2001
> From: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@xxxxxxxxxx>
> 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.
>
> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@xxxxxxxxxx>
> ---
> drivers/usb/gadget/legacy/printer.c | 65 +++++++++++++++++++++++++++++++++----
> 1 file changed, 59 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/gadget/legacy/printer.c b/drivers/usb/gadget/legacy/printer.c
> index 6474081..456730b 100644
> --- a/drivers/usb/gadget/legacy/printer.c
> +++ b/drivers/usb/gadget/legacy/printer.c
> @@ -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),
> +};

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

$ git help send-email

and figure out how to use that ?

--
balbi

Attachment: signature.asc
Description: Digital signature