Re: USB scanner stops working with xhci_hcd URB transfer length is wrong, xHC issue? req. len = 0, act. len = 4294967288

[Orion Poplawski]

On 11/09/2015 06:46 AM, Mathias Nyman wrote:

On 07.11.2015 05:50, Orion Poplawski wrote:

On 11/06/2015 10:34 AM, Felipe Balbi wrote:


Hi,

Orion Poplawski  writes:

See https://bugzilla.kernel.org/show_bug.cgi?id=107331

Trying to use my scanner.  Worked for a while, but now access
fails.  Get this
in log:


okay, first things first. Which kernel version are you using ? Care to
capture a usbmon trace ? (see Documentation/usb/usbmon.txt for
instructions)


kernel is 4.2.5-201.fc22.x86_64

Output from:

cat /sys/kernel/debug/usb/usbmon/1u > usb-scanner.log

is attached.  Thanks.



The following part from the logs says that a "Clear Endpoint Halt"
request was issued
after the first returned EPROTO URB status (-71)

fff8801932f0cc0 1015040427 S Bo:1:005:3 -115 5 = 00590001 66
8801932f0cc0 1015040515 C Bo:1:005:3 -71 0
8801932f0cc0 1015040598 S Co:1:005:0 s 02 01  0003  0
8801932f0cc0 1015040690 C Co:1:005:0 -71 0

after this we get only EPROTO returns.

Which brings back some earlier scanner issues we had about clearing
halts for scanners
when endpoint was not really halted.

The toggles get out of sync as the device side of the endpoint gets its
toggle reset
during clear endpoint halt, but host side never resets its toggle as
endpoint is not
really halted.

Hmm.. turns out a proper fix was never upsrteamed. The initial fix  was
inclomplete and
reverted as it had some issues.

Could you test the attached patch and see if it helps?

-Mathias


Sorry for the delay.  I'm afraid I don't see any change.  I'm attaching 
a new usb-scanner.log.  Also, on the second attempt while running the 
usbmon log I got the following kernel oops:


Nov 27 09:32:05 pacas.cora.nwra.com kernel: WARNING: CPU: 0 PID: 21651 
at lib/list_debug.c:36 __list_add+0xb4/0xc0()
Nov 27 09:32:05 pacas.cora.nwra.com kernel: list_add double add: 
new=8801f793e2b8, prev=8801f793e2b8, next=8801f793e2b8.
Nov 27 09:32:05 pacas.cora.nwra.com kernel: Modules linked in: 
xt_CHECKSUM iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4 
iptable_nat nf_nat_ipv4 nf_nat nf_conntr
Nov 27 09:32:05 pacas.cora.nwra.com kernel:  uinput xfs libcrc32c i915 
i2c_algo_bit drm_kms_helper tg3 drm ptp pps_core video
Nov 27 09:32:05 pacas.cora.nwra.com colord[917]: Device added: 
sysfs-Canon-CanoScan

Nov 27 09:32:05 pacas.cora.nwra.com colord[917]: Device added: sysfs-(null)
Nov 27 09:32:05 pacas.cora.nwra.com kernel: CPU: 0 PID: 21651 Comm: 
kworker/0:1 Not tainted 4.2.6-201.xhci.1.fc22.x86_64 #1
Nov 27 09:32:05 pacas.cora.nwra.com kernel: Hardware name: Acer 
TravelMate B113/BA10_HX   , BIOS V2.21 11/26/2013

Nov 27 09:32:05 pacas.cora.nwra.com kernel: Workqueue: usb_hub_wq hub_event
Nov 27 09:32:05 pacas.cora.nwra.com kernel:   
b3848654 88020082b458 81772d3a
Nov 27 09:32:05 pacas.cora.nwra.com kernel:   
88020082b4b0 88020082b498 8109e4b6
Nov 27 09:32:05 pacas.cora.nwra.com kernel:  88024f5e3b00 
8801f793e2b8 8801f793e2b8 8801f793e2b8

Nov 27 09:32:05 pacas.cora.nwra.com kernel: Call Trace:
Nov 27 09:32:05 pacas.cora.nwra.com kernel:  [] 
dump_stack+0x45/0x57
Nov 27 09:32:05 pacas.cora.nwra.com kernel:  [] 
warn_slowpath_common+0x86/0xc0
Nov 27 09:32:05 pacas.cora.nwra.com kernel:  [] 
warn_slowpath_fmt+0x55/0x70
Nov 27 09:32:05 pacas.cora.nwra.com kernel:  [] ? 
find_next_bit+0x15/0x20
Nov 27 09:32:05 pacas.cora.nwra.com kernel:  [] 
__list_add+0xb4/0xc0
Nov 27 09:32:05 pacas.cora.nwra.com kernel:  [] 
xhci_add_ep_to_interval_table.isra.42+0x123/0x1a0
Nov 27 09:32:05 pacas.cora.nwra.com kernel:  [] 
xhci_reserve_bandwidth+0x1e0/0x780
Nov 27 09:32:05 pacas.cora.nwra.com kernel:  [] ? 
find_next_bit+0x15/0x20
Nov 27 09:32:05 pacas.cora.nwra.com kernel:  [] ? 
find_busiest_group+0x47/0x4e0
Nov 27 09:32:05 pacas.cora.nwra.com kernel:  [] ? 
find_next_bit+0x15/0x20
Nov 27 09:32:05 pacas.cora.nwra.com kernel:  [] ? 
_raw_spin_unlock_irqrestore+0xe/0x10
Nov 27 09:32:05 pacas.cora.nwra.com kernel:  [] ? 
dma_pool_alloc+0x150/0x1a0
Nov 27 09:32:05 pacas.cora.nwra.com kernel:  [] ? 
xhci_segment_alloc.isra.20+0x4e/0x120
Nov 27 09:32:05 pacas.cora.nwra.com kernel:  [] ? 
xhci_segment_alloc.isra.20+0x69/0x120
Nov 27 09:32:05 pacas.cora.nwra.com kernel:  [] ? 
xhci_alloc_segments_for_ring+0xd1/0x100
Nov 27 09:32:05 pacas.cora.nwra.com kernel:  [] ? 
xhci_dbg_trace+0x5e/0xa0
Nov 27 09:32:05 pacas.cora.nwra.com kernel:  [] 
xhci_configure_endpoint+0x186/0x500
Nov 27 09:32:05 pacas.cora.nwra.com kernel:  [] 
xhci_check_bandwidth+0x1a1/0x380
Nov 27 09:32:05 pacas.cora.nwra.com kernel:  [] 
usb_hcd_alloc_bandwidth+0x278/0x320
Nov 27 09:32:05 pacas.cora.nwra.com kernel:  [] 
usb_set_configuration+0x1b7/0x8d0
Nov 27 09:32:05 pacas.cora.nwra.com kernel:  [] ? 
kernfs_add_one+0xee/0x140
Nov 27 09:32:05 pacas.cora.nwra.com kernel:  [] ? 
sysfs_do_create_link_sd.isra.2+0x6d/0xb0
Nov 27 09:32:05 pacas.cora.nwra.com kernel:  [] 

Re: [PATCH v5 7/7] usb: gadget: f_midi: pre-allocate IN requests

[Felipe Ferreri Tonello]

Hi Clemens

On 27/11/15 09:05, Clemens Ladisch wrote:
> Felipe Ferreri Tonello wrote:
>> On 13/11/15 08:55, Clemens Ladisch wrote:
>>> Felipe F. Tonello wrote:
 +static void f_midi_transmit(struct f_midi *midi)
 +{
 +...
 +  len = kfifo_peek(>in_req_fifo, );
 +  ...
 +  if (req->length > 0) {
 +  WARNING(midi, "%s: All USB requests have been used. 
 Current queue size "
 +  "is %u, consider increasing it.\n", __func__, 
 midi->in_req_num);
 +  goto drop_out;
 +  }
>>>
>>> There are two cases where the in_req FIFO might overflow:
>>> 1) the gadget is trying to send too much data at once; or
>>> 2) the host does not bother to read any of the data.
>>>
>>> In case 1), the appropriate action would be to do nothing, so that the
>>> remaining data is sent after some currently queued packets have been
>>> transmitted.  In case 2), the appropriate action would be to drop the
>>> data (even better, the _oldest_ data), and spamming the log with error
>>> messages would not help.
>>
>> True. In this case the log will be spammed.
>>
>> How would you suggest to drop the oldest data? That doesn't really seem
>> to be feasible.
> 
> There is usb_ep_dequeue().  Its documentation warns about some hardware,
> but it would be possible to at least try it.
> 
>>> I'm not quite sure if trying to detect which of these cases we have is
>>> possible, or worthwhile.  Anyway, with a packet size of 64, the queue
>>> size would be 32*64 = 2KB, which should be enough for everyone.  So I
>>> propose to ignore case 1), and to drop the error message.
> 
> After some thought, I'm not so sure anymore -- the ability to buffer
> more than 2 KB of data is part of the snd_rawmidi_write() API, so this
> could introduce a regression.  And I can imagine cases where one would
> actually want to transmit large amounts data.

One thing to consider is that the ALSA rawmidi device buffer is
sequential and our USB request buffer is not. This means that our 32
(qlen) * 256 (buflen) = 8KB of data is non-linear. Some requests might
have 3 or 4 bytes (average size of a normal MIDI message) of data and
some others might contain the full 256 bytes (for SysEx messages).

I am considering this especially for MPE (Multidimensional Polyphonic
Expression) MIDI protocol. On few benchmarks I did, a device that
implements this protocol generates around 500-2000 b/s of *raw* MIDI
data. And in practice only 4 (average MIDI message) * 32 (USB requests
defined by qlen) bytes will be used. Which means that the 8KB USB
request buffer will be under used.

So I think we have to treat the ALSA buffers and the USB request buffers
differently.

That's why I think this approach is fine by allowing the user to
increase that number of requests and its size if it needs to deal with a
higher throughput devices.

> 
> I think the safest approach would be to behave similar to the old driver,
> i.e., when the queue overflows, do nothing (not even dropping data), and
> rely on the transmit completion handler to continue.  (This implies that
> ALSA's buffer can fill up, and that snd_rawmidi_write() can block.)
> 

The previous implementation would not block, even though
snd_rawmidi_write() can block, because it was been created a new USB
request for each write call and data was been consumed even if this
request would not be enqueued to the endpoint.

But, anyway, I agree with your suggestion.

> 
> It you want to dequeue outdated data, I think this should be done with
> a timeout, i.e., when the host did not read anything for some tens of
> milliseconds or so.  This would be independent of the fill level of the
> queue, and could be done either for individual packets, or just on the
> entire endpoint queue.

That can be done. But I believe in another patch since it is not
required to work for this patch.

== Conclusion ==

Based on our conversation and your suggestions, I think that to just
ignore if an overrun occurs to the USB requests is fine. Upon completion
the request will be reused.
Important to note that if the overrun occurs, it will cause user-space
to block until a) the completion function is called successfully or b)
snd_rawmidi_write() times out. Which I think this is expected by ALSA users.

Does that make sense?

If yes then I will send the v6 of this patch.

Felipe


0x92698E6A.asc
Description: application/pgp-keys

Re: [PATCH v5 7/7] usb: gadget: f_midi: pre-allocate IN requests

[Clemens Ladisch]

Felipe Ferreri Tonello wrote:
> One thing to consider is that the ALSA rawmidi device buffer is
> sequential and our USB request buffer is not. This means that our 32
> (qlen) * 256 (buflen) = 8KB of data is non-linear. Some requests might
> have 3 or 4 bytes (average size of a normal MIDI message) of data and
> some others might contain the full 256 bytes (for SysEx messages).

f_midi_transmit() always fills up the USB packet as much as possible, so
the number of MIDI messages per request will increase automatically when
the ALSA buffer fills up faster that it is emptied by f_midi.

> == Conclusion ==
>
> Based on our conversation and your suggestions, I think that to just
> ignore if an overrun occurs to the USB requests is fine. Upon completion
> the request will be reused.
> Important to note that if the overrun occurs, it will cause user-space
> to block until a) the completion function is called successfully or b)
> snd_rawmidi_write() times out. Which I think this is expected by ALSA users.
>
> Does that make sense?

Yes.


Regards,
Clemens
--
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 28/29] usb: gadget: f_printer: conversion to new API

[Robert Baldyga]

Generate descriptors in new format and attach them to USB function in
prep_descs(). Implement prep_vendor_descs() to supply class specific
descriptors. Change set_alt() implementation and implement clear_alt()
operation. Remove boilerplate code.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/function/f_printer.c | 300 ++--
 1 file changed, 88 insertions(+), 212 deletions(-)

diff --git a/drivers/usb/gadget/function/f_printer.c 
b/drivers/usb/gadget/function/f_printer.c
index 0fbfb2b..f5cfea3 100644
--- a/drivers/usb/gadget/function/f_printer.c
+++ b/drivers/usb/gadget/function/f_printer.c
@@ -135,13 +135,6 @@ static struct usb_endpoint_descriptor fs_ep_out_desc = {
.bmAttributes = USB_ENDPOINT_XFER_BULK
 };
 
-static struct usb_descriptor_header *fs_printer_function[] = {
-   (struct usb_descriptor_header *) _desc,
-   (struct usb_descriptor_header *) _ep_in_desc,
-   (struct usb_descriptor_header *) _ep_out_desc,
-   NULL
-};
-
 /*
  * usb 2.0 devices need to expose both high speed and full speed
  * descriptors, unless they only run at full speed.
@@ -169,13 +162,6 @@ static struct usb_qualifier_descriptor dev_qualifier = {
.bNumConfigurations =   1
 };
 
-static struct usb_descriptor_header *hs_printer_function[] = {
-   (struct usb_descriptor_header *) _desc,
-   (struct usb_descriptor_header *) _ep_in_desc,
-   (struct usb_descriptor_header *) _ep_out_desc,
-   NULL
-};
-
 /*
  * Added endpoint descriptors for 3.0 devices
  */
@@ -204,14 +190,16 @@ static struct usb_ss_ep_comp_descriptor 
ss_ep_out_comp_desc = {
.bDescriptorType =  USB_DT_SS_ENDPOINT_COMP,
 };
 
-static struct usb_descriptor_header *ss_printer_function[] = {
-   (struct usb_descriptor_header *) _desc,
-   (struct usb_descriptor_header *) _ep_in_desc,
-   (struct usb_descriptor_header *) _ep_in_comp_desc,
-   (struct usb_descriptor_header *) _ep_out_desc,
-   (struct usb_descriptor_header *) _ep_out_comp_desc,
-   NULL
-};
+USB_COMPOSITE_ENDPOINT(ep_in, _ep_in_desc, _ep_in_desc,
+   _ep_in_desc, _ep_in_comp_desc);
+USB_COMPOSITE_ENDPOINT(ep_out, _ep_out_desc, _ep_out_desc,
+   _ep_out_desc, _ep_out_comp_desc);
+
+USB_COMPOSITE_ALTSETTING(intf0alt0, _desc, _in, _out);
+
+USB_COMPOSITE_INTERFACE(intf0, );
+
+USB_COMPOSITE_DESCRIPTORS(printer_descs, );
 
 /* maxpacket and other transfer characteristics vary by speed. */
 static inline struct usb_endpoint_descriptor *ep_desc(struct usb_gadget 
*gadget,
@@ -764,86 +752,6 @@ static const struct file_operations printer_io_operations 
= {
 
 /*-*/
 
-static int
-set_printer_interface(struct printer_dev *dev)
-{
-   int result = 0;
-
-   dev->in_ep->desc = ep_desc(dev->gadget, _ep_in_desc, _ep_in_desc,
-   _ep_in_desc);
-   dev->in_ep->driver_data = dev;
-
-   dev->out_ep->desc = ep_desc(dev->gadget, _ep_out_desc,
-   _ep_out_desc, _ep_out_desc);
-   dev->out_ep->driver_data = dev;
-
-   result = usb_ep_enable(dev->in_ep);
-   if (result != 0) {
-   DBG(dev, "enable %s --> %d\n", dev->in_ep->name, result);
-   goto done;
-   }
-
-   result = usb_ep_enable(dev->out_ep);
-   if (result != 0) {
-   DBG(dev, "enable %s --> %d\n", dev->in_ep->name, result);
-   goto done;
-   }
-
-done:
-   /* on error, disable any endpoints  */
-   if (result != 0) {
-   (void) usb_ep_disable(dev->in_ep);
-   (void) usb_ep_disable(dev->out_ep);
-   dev->in_ep->desc = NULL;
-   dev->out_ep->desc = NULL;
-   }
-
-   /* caller is responsible for cleanup on error */
-   return result;
-}
-
-static void printer_reset_interface(struct printer_dev *dev)
-{
-   unsigned long   flags;
-
-   if (dev->interface < 0)
-   return;
-
-   DBG(dev, "%s\n", __func__);
-
-   if (dev->in_ep->desc)
-   usb_ep_disable(dev->in_ep);
-
-   if (dev->out_ep->desc)
-   usb_ep_disable(dev->out_ep);
-
-   spin_lock_irqsave(>lock, flags);
-   dev->in_ep->desc = NULL;
-   dev->out_ep->desc = NULL;
-   dev->interface = -1;
-   spin_unlock_irqrestore(>lock, flags);
-}
-
-/* Change our operational Interface. */
-static int set_interface(struct printer_dev *dev, unsigned number)
-{
-   int result = 0;
-
-   /* Free the current interface */
-   printer_reset_interface(dev);
-
-   result = set_printer_interface(dev);
-   if (result)
-   printer_reset_interface(dev);
-   else
-   dev->interface = number;
-
-   if (!result)
-   INFO(dev, "Using interface %x\n", number);
-
-   return result;
-}
-
 static void 

[PATCH v2 25/29] usb: gadget: f_acm: conversion to new API

[Robert Baldyga]

Generate descriptors in new format and attach them to USB function in
prep_descs(). Implement prep_vendor_descs() to supply class specific
descriptors. Change set_alt() implementation and implement clear_alt()
operation. Remove boilerplate code.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/function/f_acm.c | 248 
 1 file changed, 78 insertions(+), 170 deletions(-)

diff --git a/drivers/usb/gadget/function/f_acm.c 
b/drivers/usb/gadget/function/f_acm.c
index 2fa1e80..0d3fe1a 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -185,20 +185,6 @@ static struct usb_endpoint_descriptor acm_fs_out_desc = {
.bmAttributes = USB_ENDPOINT_XFER_BULK,
 };
 
-static struct usb_descriptor_header *acm_fs_function[] = {
-   (struct usb_descriptor_header *) _iad_descriptor,
-   (struct usb_descriptor_header *) _control_interface_desc,
-   (struct usb_descriptor_header *) _header_desc,
-   (struct usb_descriptor_header *) _call_mgmt_descriptor,
-   (struct usb_descriptor_header *) _descriptor,
-   (struct usb_descriptor_header *) _union_desc,
-   (struct usb_descriptor_header *) _fs_notify_desc,
-   (struct usb_descriptor_header *) _data_interface_desc,
-   (struct usb_descriptor_header *) _fs_in_desc,
-   (struct usb_descriptor_header *) _fs_out_desc,
-   NULL,
-};
-
 /* high speed support: */
 static struct usb_endpoint_descriptor acm_hs_notify_desc = {
.bLength =  USB_DT_ENDPOINT_SIZE,
@@ -223,20 +209,6 @@ static struct usb_endpoint_descriptor acm_hs_out_desc = {
.wMaxPacketSize =   cpu_to_le16(512),
 };
 
-static struct usb_descriptor_header *acm_hs_function[] = {
-   (struct usb_descriptor_header *) _iad_descriptor,
-   (struct usb_descriptor_header *) _control_interface_desc,
-   (struct usb_descriptor_header *) _header_desc,
-   (struct usb_descriptor_header *) _call_mgmt_descriptor,
-   (struct usb_descriptor_header *) _descriptor,
-   (struct usb_descriptor_header *) _union_desc,
-   (struct usb_descriptor_header *) _hs_notify_desc,
-   (struct usb_descriptor_header *) _data_interface_desc,
-   (struct usb_descriptor_header *) _hs_in_desc,
-   (struct usb_descriptor_header *) _hs_out_desc,
-   NULL,
-};
-
 static struct usb_endpoint_descriptor acm_ss_in_desc = {
.bLength =  USB_DT_ENDPOINT_SIZE,
.bDescriptorType =  USB_DT_ENDPOINT,
@@ -256,22 +228,20 @@ static struct usb_ss_ep_comp_descriptor 
acm_ss_bulk_comp_desc = {
.bDescriptorType =  USB_DT_SS_ENDPOINT_COMP,
 };
 
-static struct usb_descriptor_header *acm_ss_function[] = {
-   (struct usb_descriptor_header *) _iad_descriptor,
-   (struct usb_descriptor_header *) _control_interface_desc,
-   (struct usb_descriptor_header *) _header_desc,
-   (struct usb_descriptor_header *) _call_mgmt_descriptor,
-   (struct usb_descriptor_header *) _descriptor,
-   (struct usb_descriptor_header *) _union_desc,
-   (struct usb_descriptor_header *) _hs_notify_desc,
-   (struct usb_descriptor_header *) _ss_bulk_comp_desc,
-   (struct usb_descriptor_header *) _data_interface_desc,
-   (struct usb_descriptor_header *) _ss_in_desc,
-   (struct usb_descriptor_header *) _ss_bulk_comp_desc,
-   (struct usb_descriptor_header *) _ss_out_desc,
-   (struct usb_descriptor_header *) _ss_bulk_comp_desc,
-   NULL,
-};
+USB_COMPOSITE_ENDPOINT(ep_notify, _fs_notify_desc, _hs_notify_desc,
+   _hs_notify_desc, _ss_bulk_comp_desc);
+USB_COMPOSITE_ENDPOINT(ep_in, _fs_in_desc, _hs_in_desc,
+   _ss_in_desc, _ss_bulk_comp_desc);
+USB_COMPOSITE_ENDPOINT(ep_out, _fs_out_desc, _hs_out_desc,
+   _ss_out_desc, _ss_bulk_comp_desc);
+
+USB_COMPOSITE_ALTSETTING(intf0alt0, _control_interface_desc, _notify);
+USB_COMPOSITE_ALTSETTING(intf1alt0, _data_interface_desc, _in, _out);
+
+USB_COMPOSITE_INTERFACE(intf0, );
+USB_COMPOSITE_INTERFACE(intf1, );
+
+USB_COMPOSITE_DESCRIPTORS(acm_descs, , );
 
 /* string descriptors: */
 
@@ -420,6 +390,8 @@ invalid:
return value;
 }
 
+static void acm_cdc_notify_complete(struct usb_ep *ep, struct usb_request 
*req);
+
 static int acm_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 {
struct f_acm*acm = func_to_acm(f);
@@ -427,51 +399,49 @@ static int acm_set_alt(struct usb_function *f, unsigned 
intf, unsigned alt)
 
/* we know alt == 0, so this is an activation or a reset */
 
-   if (intf == acm->ctrl_id) {
+   if (intf == 0) {
dev_vdbg(>gadget->dev,
"reset acm control interface %d\n", intf);
-   usb_ep_disable(acm->notify);
-
-   if (!acm->notify->desc)
-   if (config_ep_by_speed(cdev->gadget, f, acm->notify))
-   return 

[PATCH v2 26/29] usb: gadget: f_eem: conversion to new API

[Robert Baldyga]

Generate descriptors in new format and attach them to USB function in
prep_descs(). Implement prep_vendor_descs() to supply class specific
descriptors. Change set_alt() implementation and implement clear_alt()
operation. Remove boilerplate code.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/function/f_eem.c | 154 
 1 file changed, 33 insertions(+), 121 deletions(-)

diff --git a/drivers/usb/gadget/function/f_eem.c 
b/drivers/usb/gadget/function/f_eem.c
index cad35a5..8896419 100644
--- a/drivers/usb/gadget/function/f_eem.c
+++ b/drivers/usb/gadget/function/f_eem.c
@@ -73,14 +73,6 @@ static struct usb_endpoint_descriptor eem_fs_out_desc = {
.bmAttributes = USB_ENDPOINT_XFER_BULK,
 };
 
-static struct usb_descriptor_header *eem_fs_function[] = {
-   /* CDC EEM control descriptors */
-   (struct usb_descriptor_header *) _intf,
-   (struct usb_descriptor_header *) _fs_in_desc,
-   (struct usb_descriptor_header *) _fs_out_desc,
-   NULL,
-};
-
 /* high speed support: */
 
 static struct usb_endpoint_descriptor eem_hs_in_desc = {
@@ -101,14 +93,6 @@ static struct usb_endpoint_descriptor eem_hs_out_desc = {
.wMaxPacketSize =   cpu_to_le16(512),
 };
 
-static struct usb_descriptor_header *eem_hs_function[] = {
-   /* CDC EEM control descriptors */
-   (struct usb_descriptor_header *) _intf,
-   (struct usb_descriptor_header *) _hs_in_desc,
-   (struct usb_descriptor_header *) _hs_out_desc,
-   NULL,
-};
-
 /* super speed support: */
 
 static struct usb_endpoint_descriptor eem_ss_in_desc = {
@@ -138,15 +122,16 @@ static struct usb_ss_ep_comp_descriptor 
eem_ss_bulk_comp_desc = {
/* .bmAttributes =  0, */
 };
 
-static struct usb_descriptor_header *eem_ss_function[] = {
-   /* CDC EEM control descriptors */
-   (struct usb_descriptor_header *) _intf,
-   (struct usb_descriptor_header *) _ss_in_desc,
-   (struct usb_descriptor_header *) _ss_bulk_comp_desc,
-   (struct usb_descriptor_header *) _ss_out_desc,
-   (struct usb_descriptor_header *) _ss_bulk_comp_desc,
-   NULL,
-};
+USB_COMPOSITE_ENDPOINT(ep_in, _fs_in_desc, _hs_in_desc,
+   _ss_in_desc, _ss_bulk_comp_desc);
+USB_COMPOSITE_ENDPOINT(ep_out, _fs_out_desc, _hs_out_desc,
+   _ss_out_desc, _ss_bulk_comp_desc);
+
+USB_COMPOSITE_ALTSETTING(intf0alt0, _intf, _in, _out);
+
+USB_COMPOSITE_INTERFACE(intf0, );
+
+USB_COMPOSITE_DESCRIPTORS(eem_descs, );
 
 /* string descriptors: */
 
@@ -190,65 +175,46 @@ static int eem_set_alt(struct usb_function *f, unsigned 
intf, unsigned alt)
struct usb_composite_dev *cdev = f->config->cdev;
struct net_device   *net;
 
-   /* we know alt == 0, so this is an activation or a reset */
-   if (alt != 0)
-   goto fail;
-
-   if (intf == eem->ctrl_id) {
-   DBG(cdev, "reset eem\n");
-   gether_disconnect(>port);
-
-   if (!eem->port.in_ep->desc || !eem->port.out_ep->desc) {
-   DBG(cdev, "init eem\n");
-   if (config_ep_by_speed(cdev->gadget, f,
-  eem->port.in_ep) ||
-   config_ep_by_speed(cdev->gadget, f,
-  eem->port.out_ep)) {
-   eem->port.in_ep->desc = NULL;
-   eem->port.out_ep->desc = NULL;
-   goto fail;
-   }
-   }
+   eem->port.in_ep = usb_function_get_ep(f, intf, 0);
+   if (!eem->port.in_ep)
+   return -ENODEV;
 
-   /* zlps should not occur because zero-length EEM packets
-* will be inserted in those cases where they would occur
-*/
-   eem->port.is_zlp_ok = 1;
-   eem->port.cdc_filter = DEFAULT_FILTER;
-   DBG(cdev, "activate eem\n");
-   net = gether_connect(>port);
-   if (IS_ERR(net))
-   return PTR_ERR(net);
-   } else
-   goto fail;
+   eem->port.out_ep = usb_function_get_ep(f, intf, 1);
+   if (!eem->port.out_ep)
+   return -ENODEV;
+
+   /* zlps should not occur because zero-length EEM packets
+* will be inserted in those cases where they would occur
+*/
+   eem->port.is_zlp_ok = 1;
+   eem->port.cdc_filter = DEFAULT_FILTER;
+   DBG(cdev, "activate eem\n");
+   net = gether_connect(>port);
+   if (IS_ERR(net))
+   return PTR_ERR(net);
 
return 0;
-fail:
-   return -EINVAL;
 }
 
-static void eem_disable(struct usb_function *f)
+static void eem_clear_alt(struct usb_function *f, unsigned intf, unsigned alt)
 {
struct f_eem*eem = func_to_eem(f);
struct usb_composite_dev *cdev = f->config->cdev;
 

[PATCH v2 03/29] usb: gadget: f_loopback: free requests in loopback_disable()

[Robert Baldyga]

USB requests in Loopback function are allocated in loopback_get_alt()
function, so we prefer to free them rather in loopback_disable() than
in loopback_complete() when request is completed with error. It provides
better symetry in resource management and improves code readability.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/function/f_loopback.c | 58 +---
 1 file changed, 23 insertions(+), 35 deletions(-)

diff --git a/drivers/usb/gadget/function/f_loopback.c 
b/drivers/usb/gadget/function/f_loopback.c
index 23933bd..7d1fa10 100644
--- a/drivers/usb/gadget/function/f_loopback.c
+++ b/drivers/usb/gadget/function/f_loopback.c
@@ -35,6 +35,9 @@ struct f_loopback {
struct usb_ep   *in_ep;
struct usb_ep   *out_ep;
 
+   struct usb_request  *in_req;
+   struct usb_request  *out_req;
+
unsignedqlen;
unsignedbuflen;
 };
@@ -249,30 +252,25 @@ static void loopback_complete(struct usb_ep *ep, struct 
usb_request *req)
 * We received some data from the host so let's
 * queue it so host can read the from our in ep
 */
-   struct usb_request *in_req = req->context;
-
-   in_req->zero = (req->actual < req->length);
-   in_req->length = req->actual;
+   loop->in_req->zero = (req->actual < req->length);
+   loop->in_req->length = req->actual;
+   req = loop->in_req;
ep = loop->in_ep;
-   req = in_req;
} else {
/*
 * We have just looped back a bunch of data
 * to host. Now let's wait for some more data.
 */
-   req = req->context;
+   req = loop->out_req;
ep = loop->out_ep;
}
 
/* queue the buffer back to host or for next bunch of data */
status = usb_ep_queue(ep, req, GFP_ATOMIC);
-   if (status == 0) {
-   return;
-   } else {
+   if (status < 0)
ERROR(cdev, "Unable to loop back buffer to %s: %d\n",
  ep->name, status);
-   goto free_req;
-   }
+   break;
 
/* "should never get here" */
default:
@@ -280,20 +278,10 @@ static void loopback_complete(struct usb_ep *ep, struct 
usb_request *req)
status, req->actual, req->length);
/* FALLTHROUGH */
 
-   /* NOTE:  since this driver doesn't maintain an explicit record
-* of requests it submitted (just maintains qlen count), we
-* rely on the hardware driver to clean up on disconnect or
-* endpoint disable.
-*/
case -ECONNABORTED: /* hardware forced ep reset */
case -ECONNRESET:   /* request dequeued */
case -ESHUTDOWN:/* disconnect from host */
-free_req:
-   usb_ep_free_request(ep == loop->in_ep ?
-   loop->out_ep : loop->in_ep,
-   req->context);
-   free_ep_req(ep, req);
-   return;
+   break;
}
 }
 
@@ -316,7 +304,6 @@ static inline struct usb_request *lb_alloc_ep_req(struct 
usb_ep *ep, int len)
 static int alloc_requests(struct usb_composite_dev *cdev,
  struct f_loopback *loop)
 {
-   struct usb_request *in_req, *out_req;
int i;
int result = 0;
 
@@ -329,23 +316,21 @@ static int alloc_requests(struct usb_composite_dev *cdev,
for (i = 0; i < loop->qlen && result == 0; i++) {
result = -ENOMEM;
 
-   in_req = usb_ep_alloc_request(loop->in_ep, GFP_KERNEL);
-   if (!in_req)
+   loop->in_req = usb_ep_alloc_request(loop->in_ep, GFP_KERNEL);
+   if (!loop->in_req)
goto fail;
 
-   out_req = lb_alloc_ep_req(loop->out_ep, 0);
-   if (!out_req)
+   loop->out_req = lb_alloc_ep_req(loop->out_ep, 0);
+   if (!loop->out_req)
goto fail_in;
 
-   in_req->complete = loopback_complete;
-   out_req->complete = loopback_complete;
+   loop->in_req->complete = loopback_complete;
+   loop->out_req->complete = loopback_complete;
 
-   in_req->buf = out_req->buf;
+   loop->in_req->buf = loop->out_req->buf;
/* length will be set in complete routine */
-   in_req->context = out_req;
-   out_req->context = in_req;
 
-   result 

Re: [PATCH 1/3] usb: musb: convert printk to pr_*

[Sergei Shtylyov]

Hello.

On 11/27/2015 1:38 PM, Rasmus Villemoes wrote:


This file already uses pr_debug in a few places; this converts the
remaining printks.


   Are you aware that printk(KERN_DEBUG, ...) and pr_debug() are not equivalent?


Signed-off-by: Rasmus Villemoes 


[...]

MBR, Sergei


--
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 02/29] usb: gadget: f_sourcesink: free requests in sourcesink_disable()

[Robert Baldyga]

USB requests in SourceSink function are allocated in sourcesink_get_alt()
function, so we prefer to free them rather in sourcesink_disable() than
in source_sink_complete() when request is completed with error. It provides
better symetry in resource management and improves code readability.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/function/f_sourcesink.c | 63 --
 1 file changed, 51 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/gadget/function/f_sourcesink.c 
b/drivers/usb/gadget/function/f_sourcesink.c
index e950031..6193b47 100644
--- a/drivers/usb/gadget/function/f_sourcesink.c
+++ b/drivers/usb/gadget/function/f_sourcesink.c
@@ -44,6 +44,11 @@ struct f_sourcesink {
struct usb_ep   *iso_out_ep;
int cur_alt;
 
+   struct usb_request  **in_reqs;
+   struct usb_request  **out_reqs;
+   struct usb_request  **iso_in_reqs;
+   struct usb_request  **iso_out_reqs;
+
unsigned pattern;
unsigned isoc_interval;
unsigned isoc_maxpacket;
@@ -550,7 +555,6 @@ static void source_sink_complete(struct usb_ep *ep, struct 
usb_request *req)
req->actual, req->length);
if (ep == ss->out_ep)
check_read_data(ss, req);
-   free_ep_req(ep, req);
return;
 
case -EOVERFLOW:/* buffer overrun on read means that
@@ -579,7 +583,7 @@ static int source_sink_start_ep(struct f_sourcesink *ss, 
bool is_in,
bool is_iso, int speed)
 {
struct usb_ep   *ep;
-   struct usb_request  *req;
+   struct usb_request  **reqs;
int i, size, qlen, status = 0;
 
if (is_iso) {
@@ -604,19 +608,23 @@ static int source_sink_start_ep(struct f_sourcesink *ss, 
bool is_in,
qlen = ss->bulk_qlen;
size = 0;
}
-
+   
+   reqs = kzalloc(qlen * sizeof(*reqs), GFP_ATOMIC);
+   
for (i = 0; i < qlen; i++) {
-   req = ss_alloc_ep_req(ep, size);
-   if (!req)
-   return -ENOMEM;
+   reqs[i] = ss_alloc_ep_req(ep, size);
+   if (!reqs[i]) {
+   status = -ENOMEM;
+   goto err;
+   }
 
-   req->complete = source_sink_complete;
+   reqs[i]->complete = source_sink_complete;
if (is_in)
-   reinit_write_data(ep, req);
+   reinit_write_data(ep, reqs[i]);
else if (ss->pattern != 2)
-   memset(req->buf, 0x55, req->length);
+   memset(reqs[i]->buf, 0x55, reqs[i]->length);
 
-   status = usb_ep_queue(ep, req, GFP_ATOMIC);
+   status = usb_ep_queue(ep, reqs[i], GFP_ATOMIC);
if (status) {
struct usb_composite_dev*cdev;
 
@@ -624,12 +632,30 @@ static int source_sink_start_ep(struct f_sourcesink *ss, 
bool is_in,
ERROR(cdev, "start %s%s %s --> %d\n",
  is_iso ? "ISO-" : "", is_in ? "IN" : "OUT",
  ep->name, status);
-   free_ep_req(ep, req);
-   return status;
+   free_ep_req(ep, reqs[i]);
+   goto err;
+   }
+
+   if (is_iso) {
+   if (is_in)
+   ss->iso_in_reqs = reqs;
+   else
+   ss->iso_out_reqs = reqs;
+   } else {
+   if (is_in)
+   ss->in_reqs = reqs;
+   else
+   ss->out_reqs = reqs;
}
}
 
return status;
+
+err:
+   while (--i)
+   free_ep_req(ep, reqs[i]);
+   kfree(reqs);
+   return status;
 }
 
 static void disable_source_sink(struct f_sourcesink *ss)
@@ -754,8 +780,21 @@ static int sourcesink_get_alt(struct usb_function *f, 
unsigned intf)
 static void sourcesink_disable(struct usb_function *f)
 {
struct f_sourcesink *ss = func_to_ss(f);
+   int i;
 
disable_source_sink(ss);
+
+   for (i = 0; i < ss->bulk_qlen; ++i) {
+   free_ep_req(ss->in_ep, ss->in_reqs[i]);
+   free_ep_req(ss->out_ep, ss->out_reqs[i]);
+   }
+
+   if (ss->iso_in_ep) {
+   for (i = 0; i < ss->iso_qlen; ++i) {
+   free_ep_req(ss->iso_in_ep, ss->iso_in_reqs[i]);
+   free_ep_req(ss->iso_out_ep, ss->iso_out_reqs[i]);
+   }
+   }
 }
 
 /*-*/
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 

[PATCH v2 00/29] usb: gadget: composite: introduce new function API

[Robert Baldyga]

Hi Felipe,

Here is my new patch series doing some changes in composite framework
and modifying USB Function API. Some of concepts changed significantly,
for example bind process is done automatically inside composite framework
after collecting descriptors from all Functions. Hence bind() operation
of USB Function has been replaced with prep_descs(). Besides the other
benefits, such as simple implementation of gadget-level autoconfig solver,
changes in API allowed to simplify code of USB Functions, which contain
lots of boilerplate code.

First five patches of this series are fixes or improvements, being
preparation for further changes. Patches from 6 to 19 add implementation
of new features. Some code allowing coexistence of both old and new API
is added. This code will be removed after converting all USB Functions
present in kernel to new API.
Last ten patches converts Functions: loopback, sourcesink, ecm, rndis,
hid, acm, eem, ncm, printer and serial to new API. Conversion of another
Functions will be done soon.

*** What has changed? ***

The main changes are listed below:
A. Introduce new descriptors format. It makes descriptors creation process
   simpler plus creates good place to contain additional information such
   as result of automatic bind or actually selected altsetting.
B. Split descriptors creation process into two stages, implemented by
   two new operations:
   - prep_descs() provide entity descriptors (interfaces, altsettings
 and endpoints)
   - prep_vendor_descs() provide class and vendor specific descriptors.
   The first one is called before binding funciton to UDC and it's
   mandatory, because it provides information needed during bind process.
   The second one is optional and a Function can implement it if it wants
   to attach some class or vendor specific descriptors. It's called after
   bind process, so from it's context all information about interface
   numbers and endpoint addresses is accessible.
C. Perform bind automatically inside composite framework after collecting
   descriptors from all USB Functions. Besides removing lots of repetitive
   code from USB Functions, it gives us two main advantages:
   - We can have gadget-level autoconfig solver providing better endpoint
 resources usage. We can choose best endpoint configuration for all
 Functions in all configurations.
   - We have composite driver structure creation process separated from
 bind process which allows to modify configfs to operate directly
 on composite driver state - both legacy gadgets and configfs can
 use common composite driver creation process.
   Function allowing to obtain endpoints after bind process is provided,
   and it should be called in set_alt().
D. Replace disable() operation with more powerful clear_alt(). It is
   called when Function is being disabled or when altsetting being
   selected on interface which already has active altsetting. It makes
   API more symmetric, which greatly simplifies resource management.
E. Handle endpoint enable/disable automatically, which means, that in
   set_alt() we obtain set of already enabled endpoints for current
   altsetting. Likewise in clear_alt() endpoints are already disabled.
F. Change meaning of second parameter of set_alt() operation. Now it
   contains index of interface within desctiptors array of given USB
   Function instead of bInterfaceNumber of this interface, which
   simplifies altsetting handling (so far it was necessary to compare
   this value with bInterfaceNumber of each interface to find out which
   altsetting of which interface is being selected).
G. Handle get_alt() automatically. Currently selected altsetting number
   is stored for each interface.

*** How did it work before? ***

So far USB Functions had to handle bind process manually and deal with
endpoints state explicitly, which has been making code lengthy and
bug-prone. USB Functions contained lots of repetitive code which was
usually copied while creating new USB Function module. This resulted
with lots of boilerplate code scattered across all Functions present
in Linux kernel.

BIND:

During bind process we had to obtain interface id manually and assign
it to each interface descriptor (altsetting) of given interface. We also
had to obtain endpoints manually using usb_ep_autoconfig(). Beside its
verbosity, this solution resulted with suboptimal endpoints distribution,
because autoconfig algorithm was aware of requirements of only single
endpoint at a time.

udc_bind_to_driver() {
  composite_bind() {
configuration1->bind() {
  function1->bind() {
intf1_id = usb_interface_id(); // Obtain intf id manually
ep1 = usb_ep_autoconfig(); // Endpoint-level autoconfig
ep2 = usb_ep_autoconfig();
intf2_id = usb_interface_id();
ep3 = usb_ep_autoconfig();
ep4 = usb_ep_autoconfig();
  }
  function2->bind() {
intf1_id = usb_interface_id();
ep1 = usb_ep_autoconfig();
  

[PATCH v2 01/29] usb: gadget: f_sourcesink: make ISO altset user-selectable

[Robert Baldyga]

So far it was decided during the bind process whether is iso altsetting
included to f_sourcesink function or not. This decision was based on
availability of isochronous endpoints.

Since we can assemble gadget driver using composite framework and configfs
from many different functions, availability of given type of endpoint
can depend on selected components or even on their order in given
configuration.

This can result with non-obvious behavior - even small, seemingly unrelated
change in gadget configuration can decide if we have second altsetting with
iso endpoints in given sourcesink function instance or not.

Because of this it's way better to have additional parameter allowing user
to decide if he/she wants to have iso altsetting, and if iso altsetting is
included, and there are no iso endpoints available, function bind will fail
instead of silently allowing to have non-complete function bound.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/function/f_sourcesink.c | 98 --
 drivers/usb/gadget/function/g_zero.h   |  3 +
 drivers/usb/gadget/legacy/zero.c   |  6 ++
 3 files changed, 76 insertions(+), 31 deletions(-)

diff --git a/drivers/usb/gadget/function/f_sourcesink.c 
b/drivers/usb/gadget/function/f_sourcesink.c
index 242ba5c..e950031 100644
--- a/drivers/usb/gadget/function/f_sourcesink.c
+++ b/drivers/usb/gadget/function/f_sourcesink.c
@@ -49,6 +49,7 @@ struct f_sourcesink {
unsigned isoc_maxpacket;
unsigned isoc_mult;
unsigned isoc_maxburst;
+   unsigned isoc_enabled;
unsigned buflen;
unsigned bulk_qlen;
unsigned iso_qlen;
@@ -336,17 +337,28 @@ sourcesink_bind(struct usb_configuration *c, struct 
usb_function *f)
 
/* allocate bulk endpoints */
ss->in_ep = usb_ep_autoconfig(cdev->gadget, _source_desc);
-   if (!ss->in_ep) {
-autoconf_fail:
-   ERROR(cdev, "%s: can't autoconfigure on %s\n",
-   f->name, cdev->gadget->name);
-   return -ENODEV;
-   }
+   if (!ss->in_ep)
+   goto autoconf_fail;
 
ss->out_ep = usb_ep_autoconfig(cdev->gadget, _sink_desc);
if (!ss->out_ep)
goto autoconf_fail;
 
+   /* support high speed hardware */
+   hs_source_desc.bEndpointAddress = fs_source_desc.bEndpointAddress;
+   hs_sink_desc.bEndpointAddress = fs_sink_desc.bEndpointAddress;
+
+   /* support super speed hardware */
+   ss_source_desc.bEndpointAddress = fs_source_desc.bEndpointAddress;
+   ss_sink_desc.bEndpointAddress = fs_sink_desc.bEndpointAddress;
+
+   if (!ss->isoc_enabled) {
+   fs_source_sink_descs[FS_ALT_IFC_1_OFFSET] = NULL;
+   hs_source_sink_descs[HS_ALT_IFC_1_OFFSET] = NULL;
+   ss_source_sink_descs[SS_ALT_IFC_1_OFFSET] = NULL;
+   goto no_iso;
+   }
+
/* sanity check the isoc module parameters */
if (ss->isoc_interval < 1)
ss->isoc_interval = 1;
@@ -368,30 +380,14 @@ autoconf_fail:
/* allocate iso endpoints */
ss->iso_in_ep = usb_ep_autoconfig(cdev->gadget, _iso_source_desc);
if (!ss->iso_in_ep)
-   goto no_iso;
+   goto autoconf_fail;
 
ss->iso_out_ep = usb_ep_autoconfig(cdev->gadget, _iso_sink_desc);
-   if (!ss->iso_out_ep) {
-   usb_ep_autoconfig_release(ss->iso_in_ep);
-   ss->iso_in_ep = NULL;
-no_iso:
-   /*
-* We still want to work even if the UDC doesn't have isoc
-* endpoints, so null out the alt interface that contains
-* them and continue.
-*/
-   fs_source_sink_descs[FS_ALT_IFC_1_OFFSET] = NULL;
-   hs_source_sink_descs[HS_ALT_IFC_1_OFFSET] = NULL;
-   ss_source_sink_descs[SS_ALT_IFC_1_OFFSET] = NULL;
-   }
+   if (!ss->iso_out_ep)
+   goto autoconf_fail;
 
if (ss->isoc_maxpacket > 1024)
ss->isoc_maxpacket = 1024;
-
-   /* support high speed hardware */
-   hs_source_desc.bEndpointAddress = fs_source_desc.bEndpointAddress;
-   hs_sink_desc.bEndpointAddress = fs_sink_desc.bEndpointAddress;
-
/*
 * Fill in the HS isoc descriptors from the module parameters.
 * We assume that the user knows what they are doing and won't
@@ -408,12 +404,6 @@ no_iso:
hs_iso_sink_desc.bInterval = ss->isoc_interval;
hs_iso_sink_desc.bEndpointAddress = fs_iso_sink_desc.bEndpointAddress;
 
-   /* support super speed hardware */
-   ss_source_desc.bEndpointAddress =
-   fs_source_desc.bEndpointAddress;
-   ss_sink_desc.bEndpointAddress =
-   fs_sink_desc.bEndpointAddress;
-
/*
 * Fill in the SS isoc descriptors from the module parameters.
 * We assume that the user knows what they are doing and won't
@@ -436,6 +426,7 @@ 

[PATCH v2 06/29] usb: gadget: composite: add functions for descriptors handling

[Robert Baldyga]

Introduce functions and macros allowing to create and assign descriptors
to function easily. Macros build structure hierarchy using pointers to
USB descriptors, while functions assigning them to gadget make a deep
copy. It allows for easy conversion of USB functions to make them using
new descriptors format.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/composite.c | 344 +
 include/linux/usb/composite.h  |  52 +++
 2 files changed, 396 insertions(+)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 8b14c2a..3ecfaca 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -327,6 +327,315 @@ int usb_function_activate(struct usb_function *function)
 EXPORT_SYMBOL_GPL(usb_function_activate);
 
 /**
+ * usb_function_set_descs - assing descriptors to USB function
+ * @f: USB function
+ * @descs: USB descriptors to be assigned to function
+ *
+ * This function is to be called from prep_desc() callback to provide
+ * descriptors needed during bind process. It does a deep copy of
+ * descriptors hierarchy.
+ *
+ * Returns zero on success, else negative errno.
+ */
+int usb_function_set_descs(struct usb_function *f,
+   struct usb_composite_descs *descs)
+{
+   struct usb_composite_descs *descs_c;
+   struct usb_composite_intf *intf, *intf_c;
+   struct usb_composite_altset *altset, *altset_c;
+   struct usb_composite_ep *ep, *ep_c;
+   int i, a, e;
+   size_t size;
+   void *mem;
+
+   size = sizeof(*descs);
+
+   if (!descs->intfs_num)
+   return -EINVAL;
+
+   if (!f->config)
+   return -ENODEV;
+
+   size += descs->intfs_num *
+   (sizeof(*descs->intfs) + sizeof(**descs->intfs));
+   for (i = 0; i < descs->intfs_num; ++i) {
+   intf = descs->intfs[i];
+   if (!intf->altsets_num)
+   return -EINVAL;
+   size += intf->altsets_num *
+   (sizeof(*intf->altsets) + sizeof(**intf->altsets));
+   for (a = 0; a < intf->altsets_num; ++a) {
+   altset = intf->altsets[a];
+   size += sizeof(*altset->alt.desc);
+   size += altset->eps_num *
+   (sizeof(*altset->eps) + sizeof(**altset->eps));
+   for (e = 0; e < altset->eps_num; ++e) {
+   ep = altset->eps[e];
+   if (ep->fs.desc) {
+   size += sizeof(*ep->fs.desc);
+   f->config->fullspeed = true;
+   }
+   if (ep->hs.desc) {
+   size += sizeof(*ep->hs.desc);
+   f->config->highspeed = true;
+   }
+   if (ep->ss.desc) {
+   size += sizeof(*ep->ss.desc);
+   f->config->superspeed = true;
+   }
+   if (ep->ss_comp.desc)
+   size += sizeof(*ep->ss_comp.desc);
+   }
+   }
+   }
+
+   mem = kzalloc(size, GFP_KERNEL);
+   if (!mem)
+   return -ENOMEM;
+
+   f->descs = descs_c = mem;
+   mem += sizeof(*descs_c);
+   INIT_LIST_HEAD(_c->vendor_descs);
+   descs_c->intfs_num = descs->intfs_num;
+   descs_c->intfs = mem;
+   mem += descs_c->intfs_num * sizeof(*descs_c->intfs);
+
+   for (i = 0; i < f->descs->intfs_num; ++i) {
+   intf = descs->intfs[i];
+   descs_c->intfs[i] = intf_c = mem;
+   mem += sizeof(*intf_c);
+   intf_c->altsets_num = intf->altsets_num;
+   intf_c->altsets = mem;
+   mem += intf_c->altsets_num * sizeof(*intf_c->altsets);
+
+   for (a = 0; a < intf->altsets_num; ++a) {
+   altset = intf->altsets[a];
+   intf_c->altsets[a] = altset_c = mem;
+   mem += sizeof(*altset_c);
+   INIT_LIST_HEAD(_c->vendor_descs);
+   altset_c->alt.desc = mem;
+   mem += sizeof(*altset->alt.desc);
+   memcpy(altset_c->alt.desc, altset->alt.desc,
+   sizeof(*altset->alt.desc));
+   altset_c->eps_num = altset->eps_num;
+   altset_c->eps = mem;
+   mem += altset_c->eps_num * sizeof(*altset_c->eps);
+
+   for (e = 0; e < altset->eps_num; ++e) {
+   ep = altset->eps[e];
+   altset_c->eps[e] = ep_c = mem;
+   

[PATCH v2 11/29] usb: gadget: composite: disable eps before calling disable() callback

[Robert Baldyga]

Changes meaning of disable() operation for functions using new API.
Before calling disable() callback composite automatically disables
endpoints of active altsettings of given USB function. This reduces
amount of boilerplate code in USB functions.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/composite.c | 51 --
 1 file changed, 44 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index b50ebdb..a965fbc 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -246,12 +246,12 @@ done:
 }
 EXPORT_SYMBOL_GPL(usb_add_function);
 
+static void disable_function(struct usb_function *f);
+
 void usb_remove_function(struct usb_configuration *c, struct usb_function *f)
 {
-   if (f->disable)
-   f->disable(f);
+   disable_function(f);
 
-   bitmap_zero(f->endpoints, 32);
list_del(>list);
if (f->unbind)
f->unbind(c, f);
@@ -943,6 +943,46 @@ static void device_qual(struct usb_composite_dev *cdev)
 
 /*-*/
 
+/**
+ * disable_interface - disable all endpoints in given interface
+ * @f: USB function
+ * @i: interface index in function
+ */
+static void disable_interface(struct usb_function *f, unsigned i)
+{
+   struct usb_composite_intf *intf;
+   struct usb_composite_altset *alt;
+   int e;
+
+   intf = f->descs->intfs[i];
+   if (intf->cur_altset < 0)
+   return;
+
+   alt = intf->altsets[intf->cur_altset];
+   for (e = 0; e < alt->eps_num; ++e)
+   usb_ep_disable(alt->eps[e]->ep);
+
+   intf->cur_altset = -1;
+}
+
+/**
+ * disable_function - disable all endpoints in given function
+ * @f: USB function
+ */
+static void disable_function(struct usb_function *f)
+{
+   int i;
+
+   if (usb_function_is_new_api(f))
+   for (i = 0; i < f->descs->intfs_num; ++i)
+   disable_interface(f, i);
+
+   if (f->disable)
+   f->disable(f);
+
+   bitmap_zero(f->endpoints, 32);
+}
+
 static void reset_config(struct usb_composite_dev *cdev)
 {
struct usb_function *f;
@@ -950,10 +990,7 @@ static void reset_config(struct usb_composite_dev *cdev)
DBG(cdev, "reset config\n");
 
list_for_each_entry(f, >config->functions, list) {
-   if (f->disable)
-   f->disable(f);
-
-   bitmap_zero(f->endpoints, 32);
+   disable_function(f);
}
cdev->config = NULL;
cdev->delayed_status = 0;
-- 
1.9.1

--
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 09/29] usb: gadget: composite: handle vendor descs

[Robert Baldyga]

After binding all configurations in gadget, call prep_vendor_descs()
for each function which uses new API and implements this callback.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/composite.c | 31 +++
 include/linux/usb/composite.h  |  2 ++
 2 files changed, 33 insertions(+)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index f4189b1..9b4fcfe 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -2566,6 +2566,33 @@ int usb_config_do_bind(struct usb_configuration *c)
 }
 EXPORT_SYMBOL_GPL(usb_config_do_bind);
 
+/**
+ * composite_prep_vendor_descs - for each function in each configuration call
+ * prep_vendor_descs() callback.
+ * @cdev: composite device
+ */
+int composite_prep_vendor_descs(struct usb_composite_dev *cdev)
+{
+   struct usb_configuration *c;
+   struct usb_function *f;
+   int ret;
+
+   list_for_each_entry(c, >configs, list)
+   list_for_each_entry(f, >functions, list) {
+   if (!usb_function_is_new_api(f))
+   continue;
+   if (f->prep_vendor_descs) {
+   ret = f->prep_vendor_descs(f);
+   if (ret)
+   return ret;
+   }
+
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(composite_prep_vendor_descs);
+
 static int composite_bind(struct usb_gadget *gadget,
struct usb_gadget_driver *gdriver)
 {
@@ -2595,6 +2622,10 @@ static int composite_bind(struct usb_gadget *gadget,
if (status < 0)
goto fail;
 
+   status = composite_prep_vendor_descs(cdev);
+   if (status < 0)
+   goto fail;
+
if (cdev->use_os_string) {
status = composite_os_desc_req_prepare(cdev, gadget->ep0);
if (status)
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index a92da38..dc0ac28c 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -592,6 +592,8 @@ extern int composite_os_desc_req_prepare(struct 
usb_composite_dev *cdev,
 struct usb_ep *ep0);
 void composite_dev_cleanup(struct usb_composite_dev *cdev);
 
+int composite_prep_vendor_descs(struct usb_composite_dev *cdev);
+
 void composite_free_descs(struct usb_composite_dev *cdev);
 void composite_free_vendor_descs(struct usb_composite_dev *cdev);
 
-- 
1.9.1

--
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 05/29] usb: gadget: composite: introduce new descriptors format

[Robert Baldyga]

Introduce new structures designed to contain information about
descriptors. It splits descriptors in two categories:
1. Entity descs - interface and endpoint descriptors
2. Vendor descs - all other vendor and class specific descriptors

Entity descriptors are embedded in hierarchy of structures while vendor
descriptors are contained in linked lists. This distinction is caused
by fact, that entity descriptors are needed during gadget bind procedure,
while vendor descriptors can be supplied later, which is usually desired,
as these descriptors may need to be filled with interface numbers and
endpoint addresses which are assigned during gadget bind.

In result we can split descriptors creation process in two steps - first
collecs entity descriptors, perform the bind and then update and attach
all other descriptors. This process can be done this way not only for
each function separately, but also for entire gadget at once, which means
we can first gather descriptors from all functions in gadget, next perform
bind procedure, and then allow functions to supply additional descriptors.

It allows us to have autoconfig solver capable to better distibute ep
resources, and additionally, because we now store information about
endpoints, allows us to handle endpoint state inside composite framework,
and in result remove lots of boilerplate code from USB functions.

Signed-off-by: Robert Baldyga 
---
 include/linux/usb/composite.h | 119 ++
 1 file changed, 119 insertions(+)

diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 1074b89..686c5f7 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -57,6 +57,121 @@
 struct usb_configuration;
 
 /**
+ * struct usb_composite_vendor_desc - vendor specific descriptor
+ * @desc: pointer to vendor specific descriptor
+ * @list: descriptor list element
+ *
+ * It's designed to be element of vendor specific descriptor list,
+ * which can be attached to function, interface (per altsetting) or
+ * endpoint.
+ */
+struct usb_composite_vendor_desc {
+   struct usb_descriptor_header *desc;
+   struct list_head list;
+};
+
+/**
+ * struct usb_composite_ep - representation of USB endpoint
+ * @fs.desc: FullSpeed descriptor
+ * @hs.desc: HighSpeed descriptor
+ * @ss.desc: SuperSpeed descriptor
+ * @ss_comp.desc: SuperSpeed Companion descriptor
+ * @vendor_descs: list of vendor specific descriptors
+ * @vendor_descs_num: count of vendor specific descriptors
+ * @ep: pointer to endpoint obtained during bind process
+ *
+ * We have pointer to each descriptor in union with pointer to descriptor
+ * header in order to avoid casting in many places in code, because in
+ * some situations we want to have access to fields of particular type
+ * of descriptor, while in other situations we want to treat all types
+ * of descriptors in the same way.
+ */
+struct usb_composite_ep {
+   union {
+   struct usb_descriptor_header *header;
+   struct usb_endpoint_descriptor *desc;
+   } fs;
+
+   union {
+   struct usb_descriptor_header *header;
+   struct usb_endpoint_descriptor *desc;
+   } hs;
+
+   union {
+   struct usb_descriptor_header *header;
+   struct usb_endpoint_descriptor *desc;
+   } ss;
+
+   union {
+   struct usb_descriptor_header *header;
+   struct usb_ss_ep_comp_descriptor *desc;
+   } ss_comp;
+
+   struct list_head vendor_descs;
+   int vendor_descs_num;
+
+   struct usb_ep *ep;
+};
+
+/**
+ * struct usb_composite_altset - representation of USB altsetting
+ * @alt.desc: interface (altsetting) descriptor
+ * @eps: array of endpoints in altsetting
+ * @eps_num: number of endpoints
+ * @vendor_descs: list of vendor specific descriptors
+ * @vendor_descs_num: count of vendor specific descriptors
+ *
+ * We have pointer to alt descriptor in union with pointer to descriptor
+ * header in order to avoid casting in many places in code, because in
+ * some situations we want to have access to fields of particular type
+ * of descriptor, while in other situations we want to treat all types
+ * of descriptors in the same way.
+ */
+struct usb_composite_altset {
+   union {
+   struct usb_descriptor_header *header;
+   struct usb_interface_descriptor *desc;
+   } alt;
+
+   struct usb_composite_ep **eps;
+   int eps_num;
+
+   struct list_head vendor_descs;
+   int vendor_descs_num;
+};
+
+/**
+ * struct usb_composite_intf - representation of USB interface
+ * @altsets: array of altsettings in interface
+ * @altsets_num: number of altsettings
+ * @cur_altset: number of currently selected altsetting
+ * @id: id number of interface in configuraion (value of
+ * bInterfaceNumber in interface descriptor)
+ */
+struct usb_composite_intf {
+   struct usb_composite_altset **altsets;

[PATCH v2 10/29] usb: gadget: composite: generate old descs for compatibility

[Robert Baldyga]

For now we generate descriptor arrays for each speed as it is done by old
API functions, to allow use mixed new and old API based functions in single
configurations.

This will be removed after complete switch to new API.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/composite.c | 175 +
 include/linux/usb/composite.h  |   2 +
 2 files changed, 177 insertions(+)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 9b4fcfe..b50ebdb 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -2255,6 +2255,30 @@ void composite_free_vendor_descs(struct 
usb_composite_dev *cdev)
 }
 EXPORT_SYMBOL_GPL(composite_free_vendor_descs);
 
+/**
+ * composite_free_old_descs - for all functions implementing new API free old
+ * descriptors arrays.
+ * @cdev: composite device
+ */
+void composite_free_old_descs(struct usb_composite_dev *cdev)
+{
+   struct usb_configuration *c;
+   struct usb_function *f;
+
+   list_for_each_entry(c, >configs, list)
+   list_for_each_entry(f, >functions, list) {
+   if (!usb_function_is_new_api(f))
+   continue;
+   kfree(f->fs_descriptors);
+   kfree(f->hs_descriptors);
+   kfree(f->ss_descriptors);
+   f->fs_descriptors = NULL;
+   f->hs_descriptors = NULL;
+   f->ss_descriptors = NULL;
+   }
+}
+EXPORT_SYMBOL_GPL(composite_free_old_descs);
+
 static void __composite_unbind(struct usb_gadget *gadget, bool unbind_driver)
 {
struct usb_composite_dev*cdev = get_gadget_data(gadget);
@@ -2266,6 +2290,7 @@ static void __composite_unbind(struct usb_gadget *gadget, 
bool unbind_driver)
 */
WARN_ON(cdev->config);
 
+   composite_free_old_descs(cdev);
composite_free_vendor_descs(cdev);
composite_free_descs(cdev);
 
@@ -2593,6 +2618,152 @@ int composite_prep_vendor_descs(struct 
usb_composite_dev *cdev)
 }
 EXPORT_SYMBOL_GPL(composite_prep_vendor_descs);
 
+/*
+ * function_add_desc() - Add given descriptor to descriptor arrays for
+ * each supported speed, in proper version for each speed.
+ *
+ * @f - USB function
+ * @idx - pointer to index of descriptor in fs and hs array
+ * @idx_ss - pointer to index of descriptor in ss array
+ * @fs - descriptor for FS
+ * @hs - descriptor for HS
+ * @ss - descriptor for SS
+ *
+ * Indexes are automatically incremented.
+ *
+ * This function will be removed after converting all USB functions
+ * in kernel to new API.
+ */
+static inline void function_add_desc(struct usb_function *f,
+   int *idx, int *idx_ss,
+   struct usb_descriptor_header *fs,
+   struct usb_descriptor_header *hs,
+   struct usb_descriptor_header *ss) {
+   if (f->config->fullspeed)
+   f->fs_descriptors[*idx] = fs;
+   if (f->config->highspeed)
+   f->hs_descriptors[*idx] = hs;
+   if (f->config->superspeed)
+   f->ss_descriptors[*idx_ss] = ss;
+   ++(*idx);
+   ++(*idx_ss);
+}
+
+/*
+ * function_generate_old_descs() - generate descriptors array for each speed
+ *
+ * Allocate arrays of needed size and assign to them USB descriptors.
+ *
+ * This is temporary solution allowing coexistence to both old and new function
+ * API. It will be removed after converting all functions in kernel to new API.
+ */
+static int function_generate_old_descs(struct usb_function *f)
+{
+   struct usb_composite_intf *intf;
+   struct usb_composite_altset *alt;
+   struct usb_composite_ep *ep;
+   struct usb_composite_vendor_desc *vd;
+   int cnt, eps, i, a, e, idx, idx_ss;
+
+   cnt = f->descs->vendor_descs_num;
+   eps = 0;
+
+   for (i = 0; i < f->descs->intfs_num; ++i) {
+   intf = f->descs->intfs[i];
+   for (a = 0; a < intf->altsets_num; ++a) {
+   alt = intf->altsets[a];
+   cnt += alt->vendor_descs_num + 1;
+   eps += alt->eps_num;
+   for (e = 0; e < alt->eps_num; ++e)
+   cnt += alt->eps[e]->vendor_descs_num + 1;
+   }
+   }
+
+   if (f->config->fullspeed) {
+   f->fs_descriptors = kzalloc((cnt + 1) *
+   sizeof(*f->fs_descriptors), GFP_KERNEL);
+   if (!f->fs_descriptors)
+   return -ENOMEM;
+   }
+   if (f->config->highspeed) {
+   f->hs_descriptors = kzalloc((cnt + 1) *
+   sizeof(*f->hs_descriptors), GFP_KERNEL);
+   if (!f->hs_descriptors)
+   goto err;
+   }
+   if (f->config->superspeed) {
+   f->ss_descriptors = kzalloc((cnt + eps + 1) *
+ 

[PATCH v2 08/29] usb: gadget: composite: handle function bind

[Robert Baldyga]

As now USB function supplies entity descriptors to composite in
prep_descs() callback, we can perform bind inside composite framework
without involving bind() callback (which now is unused and will be
removed after converting all functions in kernel to new API).

For now we bind each configuration when it's added, because we have
to support functions based on old API, but after completing conversion
of functions, we will be able to do bind after adding all configurations.
Also more sophisticated autoconfig solver will be provided to improve
utilization of available hardware endpoints.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/composite.c | 162 +
 include/linux/usb/composite.h  |   3 +
 2 files changed, 165 insertions(+)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 3ecfaca..f4189b1 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -327,6 +327,21 @@ int usb_function_activate(struct usb_function *function)
 EXPORT_SYMBOL_GPL(usb_function_activate);
 
 /**
+ * usb_function_is_new_api - checks if USB function uses new API
+ * @f: USB function
+ *
+ * This function is added temporarily to allow both old and new function API
+ * to coexist. It function will be removed after converting all USB functions
+ * in kernel to new API.
+ *
+ * Returns true if function uses new API.
+ */
+static inline bool usb_function_is_new_api(struct usb_function *f)
+{
+   return !!f->prep_descs;
+}
+
+/**
  * usb_function_set_descs - assing descriptors to USB function
  * @f: USB function
  * @descs: USB descriptors to be assigned to function
@@ -1109,6 +1124,12 @@ int usb_add_config(struct usb_composite_dev *cdev,
goto done;
 
status = bind(config);
+   if (status < 0)
+   goto out;
+
+   status = usb_config_do_bind(config);
+
+out:
if (status < 0) {
while (!list_empty(>functions)) {
struct usb_function *f;
@@ -2404,6 +2425,147 @@ void composite_dev_cleanup(struct usb_composite_dev 
*cdev)
device_remove_file(>gadget->dev, _attr_suspended);
 }
 
+/**
+ * usb_cmp_ep_descs - compare descriptors of two endpoints
+ *
+ * As currently during autoconfig procedure we take into consideration only
+ * FullSpeed and SuperSpeed Companion descriptors, we need to compare only
+ * these descriptors. It they are the same, endpoints are identical from
+ * autoconfig point of view.
+ */
+static int usb_cmp_ep_descs(struct usb_composite_ep *ep1,
+   struct usb_composite_ep *ep2)
+{
+   if (ep1->fs.desc->bLength != ep2->fs.desc->bLength)
+   return 0;
+   if (usb_endpoint_dir_in(ep1->fs.desc) ^
+   usb_endpoint_dir_in(ep2->fs.desc))
+   return 0;
+   if (ep1->fs.desc->bmAttributes != ep2->fs.desc->bmAttributes)
+   return 0;
+   if (ep1->fs.desc->wMaxPacketSize != ep2->fs.desc->wMaxPacketSize)
+   return 0;
+   if (ep1->fs.desc->bInterval != ep2->fs.desc->bInterval)
+   return 0;
+
+   if (ep1->fs.desc->bLength != USB_DT_ENDPOINT_AUDIO_SIZE)
+   goto ss_comp;
+
+   if (ep1->fs.desc->bRefresh != ep2->fs.desc->bRefresh)
+   return 0;
+   if (ep1->fs.desc->bSynchAddress != ep2->fs.desc->bSynchAddress)
+   return 0;
+
+ss_comp:
+   if (!ep1->ss_comp.desc ^ !ep2->ss_comp.desc)
+   return 0;
+   if (!ep1->ss_comp.desc)
+   return 1;
+
+   if (ep1->ss_comp.desc->bMaxBurst != ep2->ss_comp.desc->bMaxBurst)
+   return 0;
+   if (ep1->ss_comp.desc->bmAttributes != ep2->ss_comp.desc->bmAttributes)
+   return 0;
+   if (ep1->ss_comp.desc->wBytesPerInterval !=
+   ep2->ss_comp.desc->wBytesPerInterval)
+   return 0;
+
+   return 1;
+}
+
+/**
+ * ep_update_address() - update endpoint address in descriptors
+ * @ep: composite endpoint with assigned hardware ep
+ *
+ * This function should be called after setting ep->ep to endpoint obtained
+ * from usb_ep_autoconfig_ss(), to update endpoint address in descriptors for
+ * all supported speeds.
+ */
+static inline void ep_update_address(struct usb_composite_ep *ep)
+{
+   if (ep->fs.desc)
+   ep->hs.desc->bEndpointAddress = ep->ep->address;
+   if (ep->hs.desc)
+   ep->hs.desc->bEndpointAddress = ep->ep->address;
+   if (ep->ss.desc)
+   ep->ss.desc->bEndpointAddress = ep->ep->address;
+}
+
+/**
+ * interface_do_bind() - bind interface to UDC
+ * @c: USB configuration
+ * @f: USB function in configuration c
+ * @intf: USB interface in function f
+ *
+ * For now we use only simple interface-level ep aucoconfig solver.
+ * We share endpoints between altsettings where it's possible.
+ */
+static int interface_do_bind(struct usb_configuration *c,
+ 

[PATCH v2 07/29] usb: gadget: composite: introduce new USB function ops

[Robert Baldyga]

Introduce two new USB function operations:

1. prep_descs() prepares and assigns entity (interface and endpoint)
descriptors to USB function. It's mandatory, in the new function API,
as each USB function should have at least minimalistic set of entity
descriptors. The minimum is single inferface with one altsetting with
no endpoins (ep0 only). Descriptors assigned to function in prep_descs()
callback are used during bind procedure.

2. prep_vendor_descs() - prepares and assigns class and vendor specific
descriptors to function. This function is called after binding function
to UDC hardware, which means that interface numbers and endpoint addresses
are already assigned so that function can use these values to prepare
class or vendor specific descriptors and attach them to function.

Signed-off-by: Robert Baldyga 
---
 include/linux/usb/composite.h | 8 
 1 file changed, 8 insertions(+)

diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index b778d4d..58d2929 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -286,6 +286,10 @@ struct usb_os_desc_table {
  * can expose more than one interface. If an interface is a member of
  * an IAD, only the first interface of IAD has its entry in the table.
  * @os_desc_n: Number of entries in os_desc_table
+ * @prep_descs: Returns standard function descriptors (interface and endpoint
+ * descritptors).
+ * @prep_vendor_descs: Attaches vendor or class specific descriptors to
+ * standard descriptors.
  * @bind: Before the gadget can register, all of its functions bind() to the
  * available resources including string and interface identifiers used
  * in interface or class descriptors; endpoints; I/O buffers; and so on.
@@ -354,6 +358,10 @@ struct usb_function {
 * Related:  unbind() may kfree() but bind() won't...
 */
 
+   /* new function API*/
+   int (*prep_descs)(struct usb_function *);
+   int (*prep_vendor_descs)(struct usb_function *);
+
/* configuration management:  bind/unbind */
int (*bind)(struct usb_configuration *,
struct usb_function *);
-- 
1.9.1

--
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 17/29] usb: gadget: composite: enable adding USB functions using new API

[Robert Baldyga]

Enable adding USB functions which use new API. Check if all necessary
function ops are supplied and call prep_descs() to allow function register
it's entity descriptors. Notice that bind() function is not called for
USB functions using new API, as now bind procedure is handled for them
in composite framework.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/composite.c | 40 +++-
 1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 9db8924..91ed45d 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -181,6 +181,8 @@ ep_found:
 }
 EXPORT_SYMBOL_GPL(config_ep_by_speed);
 
+static inline bool usb_function_is_new_api(struct usb_function *f);
+
 /**
  * usb_add_function() - add a function to a configuration
  * @config: the configuration
@@ -198,15 +200,12 @@ EXPORT_SYMBOL_GPL(config_ep_by_speed);
 int usb_add_function(struct usb_configuration *config,
struct usb_function *function)
 {
-   int value = -EINVAL;
+   int value;
 
DBG(config->cdev, "adding '%s'/%p to config '%s'/%p\n",
function->name, function,
config->label, config);
 
-   if (!function->set_alt || !function->disable)
-   goto done;
-
function->config = config;
list_add_tail(>list, >functions);
 
@@ -216,13 +215,22 @@ int usb_add_function(struct usb_configuration *config,
goto done;
}
 
+   value = -EINVAL;
+
+   if (!function->set_alt)
+   goto done;
+
+   if (usb_function_is_new_api(function))
+   goto new_api;
+
+   if (!function->disable)
+   goto done;
+
/* REVISIT *require* function->bind? */
if (function->bind) {
value = function->bind(config, function);
-   if (value < 0) {
-   list_del(>list);
-   function->config = NULL;
-   }
+   if (value < 0)
+   goto done;
} else
value = 0;
 
@@ -238,10 +246,24 @@ int usb_add_function(struct usb_configuration *config,
if (!config->superspeed && function->ss_descriptors)
config->superspeed = true;
 
+   goto done;
+
+new_api:
+   if (!function->prep_descs)
+   goto done;
+
+   if (!function->clear_alt)
+   goto done;
+
+   value = function->prep_descs(function);
+
 done:
-   if (value)
+   if (value) {
+   list_del(>list);
+   function->config = NULL;
DBG(config->cdev, "adding '%s'/%p --> %d\n",
function->name, function, value);
+   }
return value;
 }
 EXPORT_SYMBOL_GPL(usb_add_function);
-- 
1.9.1

--
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 15/29] usb: gadget: composite: add usb_function_get_ep() function

[Robert Baldyga]

Introduce function returning endpoint of given index in active altsetting
of specified interface. It's intended to be used in set_alt() callback
to obtain endpoints of currently selected altsetting.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/composite.c | 33 +
 include/linux/usb/composite.h  |  2 ++
 2 files changed, 35 insertions(+)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 21f8c51..0e264c5 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -671,6 +671,39 @@ static int usb_interface_id_to_index(struct usb_function 
*f, u8 id)
 }
 
 /**
+ * usb_function_get_ep - obtains endpoint of given index from active
+ * altsetting of given interface
+ * @f: USB function
+ * @i: index of interface in given function
+ * @e: index of endpoint in active altsetting of given interface
+ *
+ * This function is designed to be used in set_alt() callback, when
+ * new altsetting is selected. It allows to obtain endpoints assigned
+ * to altsetting during autoconfig process.
+ *
+ * Returns pointer to endpoint on success or NULL on failure.
+ */
+struct usb_ep *usb_function_get_ep(struct usb_function *f, int i, int e)
+{
+   struct usb_composite_altset *altset;
+   int selected_altset;
+
+   if (!f->descs)
+   return NULL;
+   if (i >= f->descs->intfs_num)
+   return NULL;
+
+   selected_altset = f->descs->intfs[i]->cur_altset;
+   altset = f->descs->intfs[i]->altsets[selected_altset];
+
+   if (e >= altset->eps_num)
+   return NULL;
+
+   return altset->eps[e]->ep;
+}
+EXPORT_SYMBOL_GPL(usb_function_get_ep);
+
+/**
  * usb_interface_id() - allocate an unused interface ID
  * @config: configuration associated with the interface
  * @function: function handling the interface
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 3838eb6..e12921c 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -418,6 +418,8 @@ int usb_ep_add_vendor_desc(struct usb_function *f, int i, 
int a, int e,
struct usb_descriptor_header *desc);
 
 
+struct usb_ep *usb_function_get_ep(struct usb_function *f, int intf, int ep);
+
 int usb_interface_id(struct usb_configuration *, struct usb_function *);
 
 int config_ep_by_speed(struct usb_gadget *g, struct usb_function *f,
-- 
1.9.1

--
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 16/29] usb: gadget: composite: add usb_get_interface_id() function

[Robert Baldyga]

Introduce function returning id of interface at given index in function.
The id value is equal bInterfaceNumber field in interface descriptor.
This value can be useful during preparation of class or vendor specific
descriptors in prep_vendor_descs() callback. It can be also necessary
to handle some class or vendor specific setup requests.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/composite.c | 19 +++
 include/linux/usb/composite.h  |  2 ++
 2 files changed, 21 insertions(+)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 0e264c5..9db8924 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -671,6 +671,25 @@ static int usb_interface_id_to_index(struct usb_function 
*f, u8 id)
 }
 
 /**
+ * usb_get_interface_id - get id number of interface at given index in
+ * USB function
+ * @f: USB function
+ * @i: index of interface in function
+ *
+ * Returns interface id on success, else negative errno.
+ */
+int usb_get_interface_id(struct usb_function *f, int i)
+{
+   if (!f->descs)
+   return -ENODEV;
+   if (f->descs->intfs_num <= i)
+   return -ENODEV;
+
+   return f->descs->intfs[i]->id;
+}
+EXPORT_SYMBOL_GPL(usb_get_interface_id);
+
+/**
  * usb_function_get_ep - obtains endpoint of given index from active
  * altsetting of given interface
  * @f: USB function
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index e12921c..b6f5447 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -418,6 +418,8 @@ int usb_ep_add_vendor_desc(struct usb_function *f, int i, 
int a, int e,
struct usb_descriptor_header *desc);
 
 
+int usb_get_interface_id(struct usb_function *f, int i);
+
 struct usb_ep *usb_function_get_ep(struct usb_function *f, int intf, int ep);
 
 int usb_interface_id(struct usb_configuration *, struct usb_function *);
-- 
1.9.1

--
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 22/29] usb: gadget: f_rndis: conversion to new API

[Robert Baldyga]

Generate descriptors in new format and attach them to USB function in
prep_descs(). Implement prep_vendor_descs() to supply class specific
descriptors. Change set_alt() implementation and implement clear_alt()
operation. Remove boilerplate code. Change USB request lifetime
management - now it's allocated in set_alt() and freed in clear_alt().

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/function/f_rndis.c | 321 --
 1 file changed, 112 insertions(+), 209 deletions(-)

diff --git a/drivers/usb/gadget/function/f_rndis.c 
b/drivers/usb/gadget/function/f_rndis.c
index e587767..abe11a7 100644
--- a/drivers/usb/gadget/function/f_rndis.c
+++ b/drivers/usb/gadget/function/f_rndis.c
@@ -212,24 +212,6 @@ static struct usb_endpoint_descriptor fs_out_desc = {
.bmAttributes = USB_ENDPOINT_XFER_BULK,
 };
 
-static struct usb_descriptor_header *eth_fs_function[] = {
-   (struct usb_descriptor_header *) _iad_descriptor,
-
-   /* control interface matches ACM, not Ethernet */
-   (struct usb_descriptor_header *) _control_intf,
-   (struct usb_descriptor_header *) _desc,
-   (struct usb_descriptor_header *) _mgmt_descriptor,
-   (struct usb_descriptor_header *) _acm_descriptor,
-   (struct usb_descriptor_header *) _union_desc,
-   (struct usb_descriptor_header *) _notify_desc,
-
-   /* data interface has no altsetting */
-   (struct usb_descriptor_header *) _data_intf,
-   (struct usb_descriptor_header *) _in_desc,
-   (struct usb_descriptor_header *) _out_desc,
-   NULL,
-};
-
 /* high speed support: */
 
 static struct usb_endpoint_descriptor hs_notify_desc = {
@@ -260,24 +242,6 @@ static struct usb_endpoint_descriptor hs_out_desc = {
.wMaxPacketSize =   cpu_to_le16(512),
 };
 
-static struct usb_descriptor_header *eth_hs_function[] = {
-   (struct usb_descriptor_header *) _iad_descriptor,
-
-   /* control interface matches ACM, not Ethernet */
-   (struct usb_descriptor_header *) _control_intf,
-   (struct usb_descriptor_header *) _desc,
-   (struct usb_descriptor_header *) _mgmt_descriptor,
-   (struct usb_descriptor_header *) _acm_descriptor,
-   (struct usb_descriptor_header *) _union_desc,
-   (struct usb_descriptor_header *) _notify_desc,
-
-   /* data interface has no altsetting */
-   (struct usb_descriptor_header *) _data_intf,
-   (struct usb_descriptor_header *) _in_desc,
-   (struct usb_descriptor_header *) _out_desc,
-   NULL,
-};
-
 /* super speed support: */
 
 static struct usb_endpoint_descriptor ss_notify_desc = {
@@ -327,26 +291,20 @@ static struct usb_ss_ep_comp_descriptor ss_bulk_comp_desc 
= {
/* .bmAttributes =  0, */
 };
 
-static struct usb_descriptor_header *eth_ss_function[] = {
-   (struct usb_descriptor_header *) _iad_descriptor,
-
-   /* control interface matches ACM, not Ethernet */
-   (struct usb_descriptor_header *) _control_intf,
-   (struct usb_descriptor_header *) _desc,
-   (struct usb_descriptor_header *) _mgmt_descriptor,
-   (struct usb_descriptor_header *) _acm_descriptor,
-   (struct usb_descriptor_header *) _union_desc,
-   (struct usb_descriptor_header *) _notify_desc,
-   (struct usb_descriptor_header *) _intr_comp_desc,
-
-   /* data interface has no altsetting */
-   (struct usb_descriptor_header *) _data_intf,
-   (struct usb_descriptor_header *) _in_desc,
-   (struct usb_descriptor_header *) _bulk_comp_desc,
-   (struct usb_descriptor_header *) _out_desc,
-   (struct usb_descriptor_header *) _bulk_comp_desc,
-   NULL,
-};
+USB_COMPOSITE_ENDPOINT(ep_notify, _notify_desc, _notify_desc,
+   _notify_desc, _intr_comp_desc);
+USB_COMPOSITE_ENDPOINT(ep_in, _in_desc, _in_desc,
+   _in_desc, _bulk_comp_desc);
+USB_COMPOSITE_ENDPOINT(ep_out, _out_desc, _out_desc,
+   _out_desc, _bulk_comp_desc);
+
+USB_COMPOSITE_ALTSETTING(intf0alt0, _control_intf, _notify);
+USB_COMPOSITE_ALTSETTING(intf1alt0, _data_intf, _in, _out);
+
+USB_COMPOSITE_INTERFACE(intf0, );
+USB_COMPOSITE_INTERFACE(intf1, );
+
+USB_COMPOSITE_DESCRIPTORS(rndis_descs, , );
 
 /* string descriptors: */
 
@@ -542,36 +500,35 @@ static int rndis_set_alt(struct usb_function *f, unsigned 
intf, unsigned alt)
 
/* we know alt == 0 */
 
-   if (intf == rndis->ctrl_id) {
+   if (intf == 0) {
VDBG(cdev, "reset rndis control %d\n", intf);
-   usb_ep_disable(rndis->notify);
 
-   if (!rndis->notify->desc) {
-   VDBG(cdev, "init rndis ctrl %d\n", intf);
-   if (config_ep_by_speed(cdev->gadget, f, rndis->notify))
-   goto fail;
-   }
-   usb_ep_enable(rndis->notify);
-
-   } else if (intf == rndis->data_id) {
-   struct net_device   *net;
+   

[PATCH v2 21/29] usb: gadget: f_ecm: conversion to new API

[Robert Baldyga]

Generate descriptors in new format and attach them to USB function in
prep_descs(). Implement prep_vendor_descs() to supply class specific
descriptors. Change set_alt() implementation and implement clear_alt()
operation. Get rid of get_alt() which now is handled automatically.
Remove boilerplate code. Change USB request lifetime management - now
it's allocated in set_alt() and freed in clear_alt().

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/function/f_ecm.c | 321 +++-
 1 file changed, 92 insertions(+), 229 deletions(-)

diff --git a/drivers/usb/gadget/function/f_ecm.c 
b/drivers/usb/gadget/function/f_ecm.c
index 7ad60ee..4627f20 100644
--- a/drivers/usb/gadget/function/f_ecm.c
+++ b/drivers/usb/gadget/function/f_ecm.c
@@ -214,25 +214,6 @@ static struct usb_endpoint_descriptor fs_ecm_out_desc = {
.bmAttributes = USB_ENDPOINT_XFER_BULK,
 };
 
-static struct usb_descriptor_header *ecm_fs_function[] = {
-   /* CDC ECM control descriptors */
-   (struct usb_descriptor_header *) _iad_descriptor,
-   (struct usb_descriptor_header *) _control_intf,
-   (struct usb_descriptor_header *) _header_desc,
-   (struct usb_descriptor_header *) _union_desc,
-   (struct usb_descriptor_header *) _desc,
-
-   /* NOTE: status endpoint might need to be removed */
-   (struct usb_descriptor_header *) _ecm_notify_desc,
-
-   /* data interface, altsettings 0 and 1 */
-   (struct usb_descriptor_header *) _data_nop_intf,
-   (struct usb_descriptor_header *) _data_intf,
-   (struct usb_descriptor_header *) _ecm_in_desc,
-   (struct usb_descriptor_header *) _ecm_out_desc,
-   NULL,
-};
-
 /* high speed support: */
 
 static struct usb_endpoint_descriptor hs_ecm_notify_desc = {
@@ -263,25 +244,6 @@ static struct usb_endpoint_descriptor hs_ecm_out_desc = {
.wMaxPacketSize =   cpu_to_le16(512),
 };
 
-static struct usb_descriptor_header *ecm_hs_function[] = {
-   /* CDC ECM control descriptors */
-   (struct usb_descriptor_header *) _iad_descriptor,
-   (struct usb_descriptor_header *) _control_intf,
-   (struct usb_descriptor_header *) _header_desc,
-   (struct usb_descriptor_header *) _union_desc,
-   (struct usb_descriptor_header *) _desc,
-
-   /* NOTE: status endpoint might need to be removed */
-   (struct usb_descriptor_header *) _ecm_notify_desc,
-
-   /* data interface, altsettings 0 and 1 */
-   (struct usb_descriptor_header *) _data_nop_intf,
-   (struct usb_descriptor_header *) _data_intf,
-   (struct usb_descriptor_header *) _ecm_in_desc,
-   (struct usb_descriptor_header *) _ecm_out_desc,
-   NULL,
-};
-
 /* super speed support: */
 
 static struct usb_endpoint_descriptor ss_ecm_notify_desc = {
@@ -331,27 +293,21 @@ static struct usb_ss_ep_comp_descriptor 
ss_ecm_bulk_comp_desc = {
/* .bmAttributes =  0, */
 };
 
-static struct usb_descriptor_header *ecm_ss_function[] = {
-   /* CDC ECM control descriptors */
-   (struct usb_descriptor_header *) _iad_descriptor,
-   (struct usb_descriptor_header *) _control_intf,
-   (struct usb_descriptor_header *) _header_desc,
-   (struct usb_descriptor_header *) _union_desc,
-   (struct usb_descriptor_header *) _desc,
-
-   /* NOTE: status endpoint might need to be removed */
-   (struct usb_descriptor_header *) _ecm_notify_desc,
-   (struct usb_descriptor_header *) _ecm_intr_comp_desc,
-
-   /* data interface, altsettings 0 and 1 */
-   (struct usb_descriptor_header *) _data_nop_intf,
-   (struct usb_descriptor_header *) _data_intf,
-   (struct usb_descriptor_header *) _ecm_in_desc,
-   (struct usb_descriptor_header *) _ecm_bulk_comp_desc,
-   (struct usb_descriptor_header *) _ecm_out_desc,
-   (struct usb_descriptor_header *) _ecm_bulk_comp_desc,
-   NULL,
-};
+USB_COMPOSITE_ENDPOINT(ep_notify, _ecm_notify_desc, _ecm_notify_desc,
+   _ecm_notify_desc, _ecm_intr_comp_desc);
+USB_COMPOSITE_ENDPOINT(ep_in, _ecm_in_desc, _ecm_in_desc,
+   _ecm_in_desc, _ecm_bulk_comp_desc);
+USB_COMPOSITE_ENDPOINT(ep_out, _ecm_out_desc, _ecm_out_desc,
+   _ecm_out_desc, _ecm_bulk_comp_desc);
+
+USB_COMPOSITE_ALTSETTING(intf0alt0, _control_intf, _notify);
+USB_COMPOSITE_ALTSETTING(intf1alt0, _data_nop_intf);
+USB_COMPOSITE_ALTSETTING(intf1alt1, _data_intf, _in, _out);
+
+USB_COMPOSITE_INTERFACE(intf0, );
+USB_COMPOSITE_INTERFACE(intf1, , );
+
+USB_COMPOSITE_DESCRIPTORS(ecm_descs, , );
 
 /* string descriptors: */
 
@@ -537,47 +493,43 @@ static int ecm_set_alt(struct usb_function *f, unsigned 
intf, unsigned alt)
struct usb_composite_dev *cdev = f->config->cdev;
 
/* Control interface has only altsetting 0 */
-   if (intf == ecm->ctrl_id) {
-   if (alt != 0)
-   goto fail;
+   if (intf == 0) {
+   /* NOTE:  a 

[PATCH v2 04/29] usb: gadget: configfs: fix error path

[Robert Baldyga]

As usb_gstrings_attach() failure can happen when some USB functions are
are already added to some configurations (in previous loop iterations),
we should always call purge_configs_funcs() to be sure that failure is
be handled properly.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/configfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 163d305..0557f80 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -1342,7 +1342,7 @@ static int configfs_composite_bind(struct usb_gadget 
*gadget,
s = usb_gstrings_attach(>cdev, cfg->gstrings, 1);
if (IS_ERR(s)) {
ret = PTR_ERR(s);
-   goto err_comp_cleanup;
+   goto err_purge_funcs;
}
c->iConfiguration = s[0].id;
}
-- 
1.9.1

--
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 12/29] usb: gadget: composite: enable eps before calling set_alt() callback

[Robert Baldyga]

Change set_alt() behavior for functions using new API. Before we call
set_alt() callback, we disable endpoints of previously selected altsetting,
and enable endpoints of currently selected altsetting, which reduces
amount of boilerplate code in USB functions.

We also calculate index of interface in function and pass it to set_alt()
callback instead of passing index of interface in configuration which has
to be obtained from interface descriptor. This simplifies altsetting
changes handling in code of USB functions.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/composite.c | 80 --
 1 file changed, 78 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index a965fbc..439beca 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -651,6 +651,26 @@ static void usb_function_free_vendor_descs(struct 
usb_function *f)
 }
 
 /**
+ * usb_interface_id_to_index - if interface with a specified id belongs
+ * to given USB function, return its index within descriptors array
+ * of this function
+ * @f: USB function
+ * @id: id number of interface
+ *
+ * Returns interface index on success, else negative errno.
+ */
+static int usb_interface_id_to_index(struct usb_function *f, u8 id)
+{
+   int i;
+
+   for (i = 0; i < f->descs->intfs_num; ++i)
+   if (f->descs->intfs[i]->id == id)
+   return i;
+
+   return -EINVAL;
+}
+
+/**
  * usb_interface_id() - allocate an unused interface ID
  * @config: configuration associated with the interface
  * @function: function handling the interface
@@ -996,6 +1016,62 @@ static void reset_config(struct usb_composite_dev *cdev)
cdev->delayed_status = 0;
 }
 
+/**
+ * set_alt() - select specified altsetting in given interface
+ * @f: USB function
+ * @i: interface id number
+ * @a: altsetting number
+ *
+ * This function has different behavior depending on which API is used by
+ * given USB function. For functions using old API behavior stays unchanged,
+ * while for functions using new API index of interface in function is
+ * calculated and endpoints are configured and enabled before calling
+ * set_alt() callback.
+ */
+static int set_alt(struct usb_function *f, unsigned i, unsigned a)
+{
+   struct usb_composite_dev *cdev = f->config->cdev;
+   struct usb_composite_altset *alt;
+   struct usb_composite_ep *ep;
+   int e, ret = -EINVAL;
+
+   /* To be removed after switch to new API */
+   if (!usb_function_is_new_api(f))
+   return f->set_alt(f, i, a);
+
+   i = usb_interface_id_to_index(f, i);
+   if (i < 0)
+   return i;
+
+   disable_interface(f, i);
+
+   if (a >= f->descs->intfs[i]->altsets_num)
+   return -EINVAL;
+
+   alt = f->descs->intfs[i]->altsets[a];
+   for (e = 0; e < alt->eps_num; ++e) {
+   ep = alt->eps[e];
+   ret = config_ep_by_speed(cdev->gadget, f, ep->ep);
+   if (ret)
+   goto err;
+   ret = usb_ep_enable(ep->ep);
+   if (ret)
+   goto err;
+   }
+
+   f->descs->intfs[i]->cur_altset = a;
+   ret = f->set_alt(f, i, a);
+   if (ret)
+   goto err;
+
+   return 0;
+err:
+   for (e = 0; e < alt->eps_num; ++e)
+   usb_ep_disable(alt->eps[e]->ep);
+   f->descs->intfs[i]->cur_altset = -1;
+   return ret;
+}
+
 static int set_config(struct usb_composite_dev *cdev,
const struct usb_ctrlrequest *ctrl, unsigned number)
 {
@@ -1075,7 +1151,7 @@ static int set_config(struct usb_composite_dev *cdev,
set_bit(addr, f->endpoints);
}
 
-   result = f->set_alt(f, tmp, 0);
+   result = set_alt(f, tmp, 0);
if (result < 0) {
DBG(cdev, "interface %d (%s/%p) alt 0 --> %d\n",
tmp, f->name, f, result);
@@ -1976,7 +2052,7 @@ composite_setup(struct usb_gadget *gadget, const struct 
usb_ctrlrequest *ctrl)
break;
if (w_value && !f->set_alt)
break;
-   value = f->set_alt(f, w_index, w_value);
+   value = set_alt(f, w_index, w_value);
if (value == USB_GADGET_DELAYED_STATUS) {
DBG(cdev,
 "%s: interface %d (%s) requested delayed status\n",
-- 
1.9.1

--
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 14/29] usb: gadget: composite: handle get_alt() automatically

[Robert Baldyga]

As now we store current altsetting number for each interface, we can
handle USB_REQ_GET_INTERFACE automatically.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/composite.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 5ce95d2..21f8c51 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -2075,7 +2075,13 @@ composite_setup(struct usb_gadget *gadget, const struct 
usb_ctrlrequest *ctrl)
if (!f)
break;
/* lots of interfaces only need altsetting zero... */
-   value = f->get_alt ? f->get_alt(f, w_index) : 0;
+   if (usb_function_is_new_api(f)) {
+   value = usb_interface_id_to_index(f, intf);
+   if (value >= 0)
+   value = f->descs->intfs[value]->cur_altset;
+   } else {
+   value = f->get_alt ? f->get_alt(f, w_index) : 0;
+   }
if (value < 0)
break;
*((u8 *)req->buf) = value;
-- 
1.9.1

--
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 13/29] usb: gadget: composite: introduce clear_alt() operation

[Robert Baldyga]

Introduce clear_alt() callback, which is called when prevoiusly set
altsetting is cleared. This can take place in two situations:
- when another altsetting is selected,
- during function disable.

Thanks to symetry to set_alt(), clear_alt() simplifies managing of
resources allocated in set_alt(). It also takes over the function of
disable() opetarion, so it can be removed after converting all USB
functions to new API.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/composite.c | 12 
 include/linux/usb/composite.h  |  4 
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 439beca..5ce95d2 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -982,6 +982,9 @@ static void disable_interface(struct usb_function *f, 
unsigned i)
for (e = 0; e < alt->eps_num; ++e)
usb_ep_disable(alt->eps[e]->ep);
 
+   if (f->clear_alt)
+   f->clear_alt(f, i, intf->cur_altset);
+
intf->cur_altset = -1;
 }
 
@@ -993,12 +996,13 @@ static void disable_function(struct usb_function *f)
 {
int i;
 
-   if (usb_function_is_new_api(f))
+   if (usb_function_is_new_api(f)) {
for (i = 0; i < f->descs->intfs_num; ++i)
disable_interface(f, i);
-
-   if (f->disable)
-   f->disable(f);
+   } else {
+   if (f->disable)
+   f->disable(f);
+   }
 
bitmap_zero(f->endpoints, 32);
 }
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 7cef8c0..3838eb6 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -301,6 +301,8 @@ struct usb_os_desc_table {
  * initialize usb_ep.driver data at this time (when it is used).
  * Note that setting an interface to its current altsetting resets
  * interface state, and that all interfaces have a disabled state.
+ * @clear_alt: (REQUIRED) Clears altsetting, frees all ep requiests and other
+ * resources allocated by set_alt.
  * @get_alt: Returns the active altsetting.  If this is not provided,
  * then only altsetting zero is supported.
  * @disable: (REQUIRED) Indicates the function should be disabled.  Reasons
@@ -373,6 +375,8 @@ struct usb_function {
/* runtime state management */
int (*set_alt)(struct usb_function *,
unsigned interface, unsigned alt);
+   void(*clear_alt)(struct usb_function *,
+   unsigned interface, unsigned alt);
int (*get_alt)(struct usb_function *,
unsigned interface);
void(*disable)(struct usb_function *);
-- 
1.9.1

--
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 29/29] usb: gadget: f_serial: conversion to new API

[Robert Baldyga]

Generate descriptors in new format and attach them to USB function in
prep_descs(). Change set_alt() implementation and implement clear_alt()
operation. Remove boilerplate code.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/function/f_serial.c | 122 +++--
 1 file changed, 25 insertions(+), 97 deletions(-)

diff --git a/drivers/usb/gadget/function/f_serial.c 
b/drivers/usb/gadget/function/f_serial.c
index 6bb44d61..526e664 100644
--- a/drivers/usb/gadget/function/f_serial.c
+++ b/drivers/usb/gadget/function/f_serial.c
@@ -69,13 +69,6 @@ static struct usb_endpoint_descriptor gser_fs_out_desc = {
.bmAttributes = USB_ENDPOINT_XFER_BULK,
 };
 
-static struct usb_descriptor_header *gser_fs_function[] = {
-   (struct usb_descriptor_header *) _interface_desc,
-   (struct usb_descriptor_header *) _fs_in_desc,
-   (struct usb_descriptor_header *) _fs_out_desc,
-   NULL,
-};
-
 /* high speed support: */
 
 static struct usb_endpoint_descriptor gser_hs_in_desc = {
@@ -92,13 +85,6 @@ static struct usb_endpoint_descriptor gser_hs_out_desc = {
.wMaxPacketSize =   cpu_to_le16(512),
 };
 
-static struct usb_descriptor_header *gser_hs_function[] = {
-   (struct usb_descriptor_header *) _interface_desc,
-   (struct usb_descriptor_header *) _hs_in_desc,
-   (struct usb_descriptor_header *) _hs_out_desc,
-   NULL,
-};
-
 static struct usb_endpoint_descriptor gser_ss_in_desc = {
.bLength =  USB_DT_ENDPOINT_SIZE,
.bDescriptorType =  USB_DT_ENDPOINT,
@@ -118,14 +104,16 @@ static struct usb_ss_ep_comp_descriptor 
gser_ss_bulk_comp_desc = {
.bDescriptorType =  USB_DT_SS_ENDPOINT_COMP,
 };
 
-static struct usb_descriptor_header *gser_ss_function[] = {
-   (struct usb_descriptor_header *) _interface_desc,
-   (struct usb_descriptor_header *) _ss_in_desc,
-   (struct usb_descriptor_header *) _ss_bulk_comp_desc,
-   (struct usb_descriptor_header *) _ss_out_desc,
-   (struct usb_descriptor_header *) _ss_bulk_comp_desc,
-   NULL,
-};
+USB_COMPOSITE_ENDPOINT(ep_in, _fs_in_desc, _hs_in_desc,
+   _ss_in_desc, _ss_bulk_comp_desc);
+USB_COMPOSITE_ENDPOINT(ep_out, _fs_out_desc, _hs_out_desc,
+   _ss_out_desc, _ss_bulk_comp_desc);
+
+USB_COMPOSITE_ALTSETTING(intf0alt0, _interface_desc, _in, _out);
+
+USB_COMPOSITE_INTERFACE(intf0, );
+
+USB_COMPOSITE_DESCRIPTORS(serial_descs, );
 
 /* string descriptors: */
 
@@ -151,28 +139,21 @@ static int gser_set_alt(struct usb_function *f, unsigned 
intf, unsigned alt)
struct f_gser   *gser = func_to_gser(f);
struct usb_composite_dev *cdev = f->config->cdev;
 
-   /* we know alt == 0, so this is an activation or a reset */
-
-   if (gser->port.in->enabled) {
-   dev_dbg(>gadget->dev,
-   "reset generic ttyGS%d\n", gser->port_num);
-   gserial_disconnect(>port);
-   }
-   if (!gser->port.in->desc || !gser->port.out->desc) {
-   dev_dbg(>gadget->dev,
+   dev_dbg(>gadget->dev,
"activate generic ttyGS%d\n", gser->port_num);
-   if (config_ep_by_speed(cdev->gadget, f, gser->port.in) ||
-   config_ep_by_speed(cdev->gadget, f, gser->port.out)) {
-   gser->port.in->desc = NULL;
-   gser->port.out->desc = NULL;
-   return -EINVAL;
-   }
-   }
+
+   gser->port.in  = usb_function_get_ep(f, intf, 0);
+   if (!gser->port.in)
+   return -ENODEV;
+   gser->port.out = usb_function_get_ep(f, intf, 0);
+   if (!gser->port.out)
+   return -ENODEV;
+
gserial_connect(>port, gser->port_num);
return 0;
 }
 
-static void gser_disable(struct usb_function *f)
+static void gser_clear_alt(struct usb_function *f, unsigned intf, unsigned alt)
 {
struct f_gser   *gser = func_to_gser(f);
struct usb_composite_dev *cdev = f->config->cdev;
@@ -186,12 +167,9 @@ static void gser_disable(struct usb_function *f)
 
 /* serial function driver setup/binding */
 
-static int gser_bind(struct usb_configuration *c, struct usb_function *f)
+static int gser_prep_descs(struct usb_function *f)
 {
-   struct usb_composite_dev *cdev = c->cdev;
-   struct f_gser   *gser = func_to_gser(f);
int status;
-   struct usb_ep   *ep;
 
/* REVISIT might want instance-specific strings to help
 * distinguish instances ...
@@ -199,57 +177,13 @@ static int gser_bind(struct usb_configuration *c, struct 
usb_function *f)
 
/* maybe allocate device-global string ID */
if (gser_string_defs[0].id == 0) {
-   status = usb_string_id(c->cdev);
+   status = usb_string_id(f->config->cdev);
if (status < 0)
return status;

[PATCH v2 27/29] usb: gadget: f_ncm: conversion to new API

[Robert Baldyga]

Generate descriptors in new format and attach them to USB function in
prep_descs(). Implement prep_vendor_descs() to supply class specific
descriptors. Change set_alt() implementation and implement clear_alt()
operation. Remove boilerplate code.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/function/f_ncm.c | 320 
 1 file changed, 105 insertions(+), 215 deletions(-)

diff --git a/drivers/usb/gadget/function/f_ncm.c 
b/drivers/usb/gadget/function/f_ncm.c
index 7ad798a..a681895 100644
--- a/drivers/usb/gadget/function/f_ncm.c
+++ b/drivers/usb/gadget/function/f_ncm.c
@@ -32,8 +32,7 @@
  * NCM is intended to be used with high-speed network attachments.
  *
  * Note that NCM requires the use of "alternate settings" for its data
- * interface.  This means that the set_alt() method has real work to do,
- * and also means that a get_alt() method is required.
+ * interface.
  */
 
 /* to trigger crc/non-crc ndp signature */
@@ -270,23 +269,6 @@ static struct usb_endpoint_descriptor fs_ncm_out_desc = {
.bmAttributes = USB_ENDPOINT_XFER_BULK,
 };
 
-static struct usb_descriptor_header *ncm_fs_function[] = {
-   (struct usb_descriptor_header *) _iad_desc,
-   /* CDC NCM control descriptors */
-   (struct usb_descriptor_header *) _control_intf,
-   (struct usb_descriptor_header *) _header_desc,
-   (struct usb_descriptor_header *) _union_desc,
-   (struct usb_descriptor_header *) _desc,
-   (struct usb_descriptor_header *) _desc,
-   (struct usb_descriptor_header *) _ncm_notify_desc,
-   /* data interface, altsettings 0 and 1 */
-   (struct usb_descriptor_header *) _data_nop_intf,
-   (struct usb_descriptor_header *) _data_intf,
-   (struct usb_descriptor_header *) _ncm_in_desc,
-   (struct usb_descriptor_header *) _ncm_out_desc,
-   NULL,
-};
-
 /* high speed support: */
 
 static struct usb_endpoint_descriptor hs_ncm_notify_desc = {
@@ -316,22 +298,21 @@ static struct usb_endpoint_descriptor hs_ncm_out_desc = {
.wMaxPacketSize =   cpu_to_le16(512),
 };
 
-static struct usb_descriptor_header *ncm_hs_function[] = {
-   (struct usb_descriptor_header *) _iad_desc,
-   /* CDC NCM control descriptors */
-   (struct usb_descriptor_header *) _control_intf,
-   (struct usb_descriptor_header *) _header_desc,
-   (struct usb_descriptor_header *) _union_desc,
-   (struct usb_descriptor_header *) _desc,
-   (struct usb_descriptor_header *) _desc,
-   (struct usb_descriptor_header *) _ncm_notify_desc,
-   /* data interface, altsettings 0 and 1 */
-   (struct usb_descriptor_header *) _data_nop_intf,
-   (struct usb_descriptor_header *) _data_intf,
-   (struct usb_descriptor_header *) _ncm_in_desc,
-   (struct usb_descriptor_header *) _ncm_out_desc,
-   NULL,
-};
+USB_COMPOSITE_ENDPOINT(ep_notify, _ncm_notify_desc,
+   _ncm_notify_desc, NULL, NULL);
+USB_COMPOSITE_ENDPOINT(ep_in, _ncm_in_desc,
+   _ncm_in_desc, NULL, NULL);
+USB_COMPOSITE_ENDPOINT(ep_out, _ncm_out_desc,
+   _ncm_out_desc, NULL, NULL);
+
+USB_COMPOSITE_ALTSETTING(intf0alt0, _control_intf, _notify);
+USB_COMPOSITE_ALTSETTING(intf1alt0, _data_nop_intf);
+USB_COMPOSITE_ALTSETTING(intf1alt1, _data_intf, _in, _out);
+
+USB_COMPOSITE_INTERFACE(intf0, );
+USB_COMPOSITE_INTERFACE(intf1, , );
+
+USB_COMPOSITE_DESCRIPTORS(ncm_descs, , );
 
 /* string descriptors: */
 
@@ -792,6 +773,8 @@ invalid:
return value;
 }
 
+static void ncm_tx_tasklet(unsigned long data);
+static enum hrtimer_restart ncm_tx_timeout(struct hrtimer *data);
 
 static int ncm_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 {
@@ -799,52 +782,44 @@ static int ncm_set_alt(struct usb_function *f, unsigned 
intf, unsigned alt)
struct usb_composite_dev *cdev = f->config->cdev;
 
/* Control interface has only altsetting 0 */
-   if (intf == ncm->ctrl_id) {
-   if (alt != 0)
-   goto fail;
-
+   if (intf == 0) {
DBG(cdev, "reset ncm control %d\n", intf);
-   usb_ep_disable(ncm->notify);
 
-   if (!(ncm->notify->desc)) {
-   DBG(cdev, "init ncm ctrl %d\n", intf);
-   if (config_ep_by_speed(cdev->gadget, f, ncm->notify))
-   goto fail;
+   ncm->notify = usb_function_get_ep(f, intf, 0);
+   if (!ncm->notify)
+   return -ENODEV;
+
+   /* allocate notification request and buffer */
+   ncm->notify_req = usb_ep_alloc_request(ncm->notify, GFP_KERNEL);
+   if (!ncm->notify_req)
+   return -ENOMEM;
+   ncm->notify_req->buf = kmalloc(NCM_STATUS_BYTECOUNT, 
GFP_KERNEL);
+   if (!ncm->notify_req->buf) {
+   usb_ep_free_request(ncm->notify, ncm->notify_req);
+  

[PATCH v2 20/29] usb: gadget: f_sourcesink: convert to new API

[Robert Baldyga]

Generate descriptors in new format and attach them to USB function in
prep_descs(). Change set_alt() implementation and implement clear_alt()
operation. Get rid of get_alt() callback, as now USB_REQ_GET_INTERFACE
is handled automatically by composite framwework. Remove unnecessary
boilerplate code.

Call usb_config_do_bind() in legacy gadget zero, because it uses
usb_add_config_only() instead of usb_add_config() and prepares
configuration manually.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/function/f_sourcesink.c | 314 ++---
 drivers/usb/gadget/function/g_zero.h   |   3 -
 drivers/usb/gadget/legacy/zero.c   |   3 +
 3 files changed, 65 insertions(+), 255 deletions(-)

diff --git a/drivers/usb/gadget/function/f_sourcesink.c 
b/drivers/usb/gadget/function/f_sourcesink.c
index 6193b47..262dae8 100644
--- a/drivers/usb/gadget/function/f_sourcesink.c
+++ b/drivers/usb/gadget/function/f_sourcesink.c
@@ -42,7 +42,6 @@ struct f_sourcesink {
struct usb_ep   *out_ep;
struct usb_ep   *iso_in_ep;
struct usb_ep   *iso_out_ep;
-   int cur_alt;
 
struct usb_request  **in_reqs;
struct usb_request  **out_reqs;
@@ -125,19 +124,6 @@ static struct usb_endpoint_descriptor fs_iso_sink_desc = {
.bInterval =4,
 };
 
-static struct usb_descriptor_header *fs_source_sink_descs[] = {
-   (struct usb_descriptor_header *) _sink_intf_alt0,
-   (struct usb_descriptor_header *) _sink_desc,
-   (struct usb_descriptor_header *) _source_desc,
-   (struct usb_descriptor_header *) _sink_intf_alt1,
-#define FS_ALT_IFC_1_OFFSET3
-   (struct usb_descriptor_header *) _sink_desc,
-   (struct usb_descriptor_header *) _source_desc,
-   (struct usb_descriptor_header *) _iso_sink_desc,
-   (struct usb_descriptor_header *) _iso_source_desc,
-   NULL,
-};
-
 /* high speed support: */
 
 static struct usb_endpoint_descriptor hs_source_desc = {
@@ -174,19 +160,6 @@ static struct usb_endpoint_descriptor hs_iso_sink_desc = {
.bInterval =4,
 };
 
-static struct usb_descriptor_header *hs_source_sink_descs[] = {
-   (struct usb_descriptor_header *) _sink_intf_alt0,
-   (struct usb_descriptor_header *) _source_desc,
-   (struct usb_descriptor_header *) _sink_desc,
-   (struct usb_descriptor_header *) _sink_intf_alt1,
-#define HS_ALT_IFC_1_OFFSET3
-   (struct usb_descriptor_header *) _source_desc,
-   (struct usb_descriptor_header *) _sink_desc,
-   (struct usb_descriptor_header *) _iso_source_desc,
-   (struct usb_descriptor_header *) _iso_sink_desc,
-   NULL,
-};
-
 /* super speed support: */
 
 static struct usb_endpoint_descriptor ss_source_desc = {
@@ -259,24 +232,24 @@ static struct usb_ss_ep_comp_descriptor 
ss_iso_sink_comp_desc = {
.wBytesPerInterval =cpu_to_le16(1024),
 };
 
-static struct usb_descriptor_header *ss_source_sink_descs[] = {
-   (struct usb_descriptor_header *) _sink_intf_alt0,
-   (struct usb_descriptor_header *) _source_desc,
-   (struct usb_descriptor_header *) _source_comp_desc,
-   (struct usb_descriptor_header *) _sink_desc,
-   (struct usb_descriptor_header *) _sink_comp_desc,
-   (struct usb_descriptor_header *) _sink_intf_alt1,
-#define SS_ALT_IFC_1_OFFSET5
-   (struct usb_descriptor_header *) _source_desc,
-   (struct usb_descriptor_header *) _source_comp_desc,
-   (struct usb_descriptor_header *) _sink_desc,
-   (struct usb_descriptor_header *) _sink_comp_desc,
-   (struct usb_descriptor_header *) _iso_source_desc,
-   (struct usb_descriptor_header *) _iso_source_comp_desc,
-   (struct usb_descriptor_header *) _iso_sink_desc,
-   (struct usb_descriptor_header *) _iso_sink_comp_desc,
-   NULL,
-};
+USB_COMPOSITE_ENDPOINT(ep_source, _source_desc, _source_desc,
+   _source_desc, _source_comp_desc);
+USB_COMPOSITE_ENDPOINT(ep_sink, _sink_desc, _sink_desc,
+   _sink_desc, _sink_comp_desc);
+USB_COMPOSITE_ENDPOINT(ep_iso_source, _iso_source_desc, _iso_source_desc,
+   _iso_source_desc, _iso_source_comp_desc);
+USB_COMPOSITE_ENDPOINT(ep_iso_sink, _iso_sink_desc, _iso_sink_desc,
+   _iso_sink_desc, _iso_sink_comp_desc);
+
+USB_COMPOSITE_ALTSETTING(altset0, _sink_intf_alt0, _source, 
_sink);
+USB_COMPOSITE_ALTSETTING(altset1, _sink_intf_alt1, _source, _sink,
+   _iso_source, _iso_sink);
+
+USB_COMPOSITE_INTERFACE(intf0, , );
+USB_COMPOSITE_INTERFACE(intf0_no_iso, );
+
+USB_COMPOSITE_DESCRIPTORS(source_sink_descs, );
+USB_COMPOSITE_DESCRIPTORS(source_sink_descs_no_iso, _no_iso);
 
 /* function-specific strings: */
 
@@ -304,65 +277,12 @@ static inline struct usb_request *ss_alloc_ep_req(struct 
usb_ep *ep, int len)
return alloc_ep_req(ep, len, ss->buflen);
 }
 
-static void disable_ep(struct usb_composite_dev 

[PATCH v2 18/29] usb: gadget: configfs: add new composite API support

[Robert Baldyga]

Handle functions using new API properly.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/configfs.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 0557f80..153adf7 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -1355,6 +1355,11 @@ static int configfs_composite_bind(struct usb_gadget 
*gadget,
goto err_purge_funcs;
}
}
+
+   ret = usb_config_do_bind(c);
+   if (ret)
+   goto err_purge_funcs;
+
usb_ep_autoconfig_reset(cdev->gadget);
}
if (cdev->use_os_string) {
@@ -1363,10 +1368,23 @@ static int configfs_composite_bind(struct usb_gadget 
*gadget,
goto err_purge_funcs;
}
 
+   ret = composite_prep_vendor_descs(cdev);
+   if (ret < 0)
+   goto err_free_vendor_descs;
+
+   ret = composite_generate_old_descs(cdev);
+   if (ret < 0)
+   goto err_free_old_descs;
+
usb_ep_autoconfig_reset(cdev->gadget);
return 0;
 
+err_free_old_descs:
+   composite_free_old_descs(cdev);
+err_free_vendor_descs:
+   composite_free_vendor_descs(cdev);
 err_purge_funcs:
+   composite_free_descs(cdev);
purge_configs_funcs(gi);
 err_comp_cleanup:
composite_dev_cleanup(cdev);
@@ -1383,6 +1401,10 @@ static void configfs_composite_unbind(struct usb_gadget 
*gadget)
cdev = get_gadget_data(gadget);
gi = container_of(cdev, struct gadget_info, cdev);
 
+   composite_free_old_descs(cdev);
+   composite_free_vendor_descs(cdev);
+   composite_free_descs(cdev);
+
kfree(otg_desc[0]);
otg_desc[0] = NULL;
purge_configs_funcs(gi);
-- 
1.9.1

--
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 24/29] usb: gadget: f_hid: conversion to new API

[Robert Baldyga]

Generate descriptors in new format and attach them to USB function in
prep_descs(). Implement prep_vendor_descs() to supply class specific
descriptors. Change set_alt() implementation and implement clear_alt()
operation. Move cdev initialization to hidg_alloc(). Remove boilerplate
code.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/function/f_hid.c | 305 ++--
 1 file changed, 119 insertions(+), 186 deletions(-)

diff --git a/drivers/usb/gadget/function/f_hid.c 
b/drivers/usb/gadget/function/f_hid.c
index 0456a53..8770289 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -125,14 +125,6 @@ static struct usb_endpoint_descriptor hidg_hs_out_ep_desc 
= {
  */
 };
 
-static struct usb_descriptor_header *hidg_hs_descriptors[] = {
-   (struct usb_descriptor_header *)_interface_desc,
-   (struct usb_descriptor_header *)_desc,
-   (struct usb_descriptor_header *)_hs_in_ep_desc,
-   (struct usb_descriptor_header *)_hs_out_ep_desc,
-   NULL,
-};
-
 /* Full-Speed Support */
 
 static struct usb_endpoint_descriptor hidg_fs_in_ep_desc = {
@@ -159,13 +151,16 @@ static struct usb_endpoint_descriptor hidg_fs_out_ep_desc 
= {
   */
 };
 
-static struct usb_descriptor_header *hidg_fs_descriptors[] = {
-   (struct usb_descriptor_header *)_interface_desc,
-   (struct usb_descriptor_header *)_desc,
-   (struct usb_descriptor_header *)_fs_in_ep_desc,
-   (struct usb_descriptor_header *)_fs_out_ep_desc,
-   NULL,
-};
+USB_COMPOSITE_ENDPOINT(ep_in, _fs_in_ep_desc,
+   _hs_in_ep_desc, NULL, NULL);
+USB_COMPOSITE_ENDPOINT(ep_out, _fs_out_ep_desc,
+   _hs_out_ep_desc, NULL, NULL);
+
+USB_COMPOSITE_ALTSETTING(intf0alt0, _interface_desc, _in, _out);
+
+USB_COMPOSITE_INTERFACE(intf0, );
+
+USB_COMPOSITE_DESCRIPTORS(hidg_descs, );
 
 /*-*/
 /* Strings */
@@ -487,27 +482,6 @@ respond:
return status;
 }
 
-static void hidg_disable(struct usb_function *f)
-{
-   struct f_hidg *hidg = func_to_hidg(f);
-   struct f_hidg_req_list *list, *next;
-   int i;
-
-   usb_ep_disable(hidg->in_ep);
-   usb_ep_disable(hidg->out_ep);
-
-   list_for_each_entry_safe(list, next, >completed_out_req, list) {
-   list_del(>list);
-   kfree(list);
-   }
-
-   for (i = 0; i < hidg->qlen; ++i) {
-   kfree(hidg->out_reqs[i]->buf);
-   kfree(hidg->out_reqs[i]);
-   }
-   kfree(hidg->out_reqs);
-}
-
 static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 {
struct usb_composite_dev*cdev = f->config->cdev;
@@ -516,65 +490,46 @@ static int hidg_set_alt(struct usb_function *f, unsigned 
intf, unsigned alt)
 
VDBG(cdev, "hidg_set_alt intf:%d alt:%d\n", intf, alt);
 
-   if (hidg->in_ep != NULL) {
-   /* restart endpoint */
-   usb_ep_disable(hidg->in_ep);
+   hidg->in_ep = usb_function_get_ep(f, intf, 0);
+   if (!hidg->in_ep)
+   return -ENODEV;
+   hidg->in_ep->driver_data = hidg;
 
-   status = config_ep_by_speed(f->config->cdev->gadget, f,
-   hidg->in_ep);
-   if (status) {
-   ERROR(cdev, "config_ep_by_speed FAILED!\n");
-   goto fail;
-   }
-   status = usb_ep_enable(hidg->in_ep);
-   if (status < 0) {
-   ERROR(cdev, "Enable IN endpoint FAILED!\n");
-   goto fail;
-   }
-   hidg->in_ep->driver_data = hidg;
-   }
+   hidg->out_ep = usb_function_get_ep(f, intf, 1);
+   if (!hidg->out_ep)
+   return -ENODEV;
+   hidg->out_ep->driver_data = hidg;
 
+   /* preallocate request and buffer */
+   hidg->req = usb_ep_alloc_request(hidg->in_ep, GFP_KERNEL);
+   if (!hidg->req)
+   return -ENOMEM;
 
-   if (hidg->out_ep != NULL) {
-   /* restart endpoint */
-   usb_ep_disable(hidg->out_ep);
+   hidg->req->buf = kmalloc(hidg->report_length, GFP_KERNEL);
+   if (!hidg->req->buf)
+   return -ENOMEM;
 
-   status = config_ep_by_speed(f->config->cdev->gadget, f,
-   hidg->out_ep);
-   if (status) {
-   ERROR(cdev, "config_ep_by_speed FAILED!\n");
-   goto fail;
-   }
-   status = usb_ep_enable(hidg->out_ep);
-   if (status < 0) {
-   ERROR(cdev, "Enable IN endpoint FAILED!\n");
-   goto fail;
-   }
-   

[PATCH v2 19/29] usb: gadget: f_loopback: convert to new API

[Robert Baldyga]

Generate descriptors in new format and attach them to USB function in
prep_descs(). Change set_alt() implementation and implement clear_alt()
operation. Remove unnecessary boilerplate code.

Call usb_config_do_bind() in legacy gadget zero, because it uses
usb_add_config_only() instead of usb_add_config() and prepares
configuration manually.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/function/f_loopback.c | 162 ++-
 drivers/usb/gadget/legacy/zero.c |   3 +
 2 files changed, 33 insertions(+), 132 deletions(-)

diff --git a/drivers/usb/gadget/function/f_loopback.c 
b/drivers/usb/gadget/function/f_loopback.c
index 7d1fa10..14ba8a9 100644
--- a/drivers/usb/gadget/function/f_loopback.c
+++ b/drivers/usb/gadget/function/f_loopback.c
@@ -76,13 +76,6 @@ static struct usb_endpoint_descriptor fs_loop_sink_desc = {
.bmAttributes = USB_ENDPOINT_XFER_BULK,
 };
 
-static struct usb_descriptor_header *fs_loopback_descs[] = {
-   (struct usb_descriptor_header *) _intf,
-   (struct usb_descriptor_header *) _loop_sink_desc,
-   (struct usb_descriptor_header *) _loop_source_desc,
-   NULL,
-};
-
 /* high speed support: */
 
 static struct usb_endpoint_descriptor hs_loop_source_desc = {
@@ -101,13 +94,6 @@ static struct usb_endpoint_descriptor hs_loop_sink_desc = {
.wMaxPacketSize =   cpu_to_le16(512),
 };
 
-static struct usb_descriptor_header *hs_loopback_descs[] = {
-   (struct usb_descriptor_header *) _intf,
-   (struct usb_descriptor_header *) _loop_source_desc,
-   (struct usb_descriptor_header *) _loop_sink_desc,
-   NULL,
-};
-
 /* super speed support: */
 
 static struct usb_endpoint_descriptor ss_loop_source_desc = {
@@ -142,14 +128,17 @@ static struct usb_ss_ep_comp_descriptor 
ss_loop_sink_comp_desc = {
.wBytesPerInterval =0,
 };
 
-static struct usb_descriptor_header *ss_loopback_descs[] = {
-   (struct usb_descriptor_header *) _intf,
-   (struct usb_descriptor_header *) _loop_source_desc,
-   (struct usb_descriptor_header *) _loop_source_comp_desc,
-   (struct usb_descriptor_header *) _loop_sink_desc,
-   (struct usb_descriptor_header *) _loop_sink_comp_desc,
-   NULL,
-};
+USB_COMPOSITE_ENDPOINT(ep_source, _loop_source_desc, _loop_source_desc,
+   _loop_source_desc, _loop_source_comp_desc);
+USB_COMPOSITE_ENDPOINT(ep_sink, _loop_sink_desc, _loop_sink_desc,
+   _loop_sink_desc, _loop_sink_comp_desc);
+
+USB_COMPOSITE_ALTSETTING(altset0, _intf, _source, _sink);
+
+USB_COMPOSITE_INTERFACE(intf0, );
+
+USB_COMPOSITE_DESCRIPTORS(loopback_descs, );
+
 
 /* function-specific strings: */
 
@@ -170,18 +159,10 @@ static struct usb_gadget_strings *loopback_strings[] = {
 
 /*-*/
 
-static int loopback_bind(struct usb_configuration *c, struct usb_function *f)
+static int loopback_prep_descs(struct usb_function *f)
 {
-   struct usb_composite_dev *cdev = c->cdev;
-   struct f_loopback   *loop = func_to_loop(f);
-   int id;
-   int ret;
-
-   /* allocate interface ID(s) */
-   id = usb_interface_id(c, f);
-   if (id < 0)
-   return id;
-   loopback_intf.bInterfaceNumber = id;
+   struct usb_composite_dev *cdev = f->config->cdev;
+   int id;
 
id = usb_string_id(cdev);
if (id < 0)
@@ -189,40 +170,7 @@ static int loopback_bind(struct usb_configuration *c, 
struct usb_function *f)
strings_loopback[0].id = id;
loopback_intf.iInterface = id;
 
-   /* allocate endpoints */
-
-   loop->in_ep = usb_ep_autoconfig(cdev->gadget, _loop_source_desc);
-   if (!loop->in_ep) {
-autoconf_fail:
-   ERROR(cdev, "%s: can't autoconfigure on %s\n",
-   f->name, cdev->gadget->name);
-   return -ENODEV;
-   }
-
-   loop->out_ep = usb_ep_autoconfig(cdev->gadget, _loop_sink_desc);
-   if (!loop->out_ep)
-   goto autoconf_fail;
-
-   /* support high speed hardware */
-   hs_loop_source_desc.bEndpointAddress =
-   fs_loop_source_desc.bEndpointAddress;
-   hs_loop_sink_desc.bEndpointAddress = fs_loop_sink_desc.bEndpointAddress;
-
-   /* support super speed hardware */
-   ss_loop_source_desc.bEndpointAddress =
-   fs_loop_source_desc.bEndpointAddress;
-   ss_loop_sink_desc.bEndpointAddress = fs_loop_sink_desc.bEndpointAddress;
-
-   ret = usb_assign_descriptors(f, fs_loopback_descs, hs_loopback_descs,
-   ss_loopback_descs);
-   if (ret)
-   return ret;
-
-   DBG(cdev, "%s speed %s: IN/%s, OUT/%s\n",
-   (gadget_is_superspeed(c->cdev->gadget) ? "super" :
-(gadget_is_dualspeed(c->cdev->gadget) ? "dual" : "full")),
-   f->name, loop->in_ep->name, loop->out_ep->name);
-   return 

[PATCH v2 23/29] usb: gadget: f_hid: handle requests lifetime properly

[Robert Baldyga]

So far USB requests allocated in hidg_set_alt() were not freed. Now we
free them in case of hidg_set_alt() failure (when we are not able to
allocate and enqueue all the requests) or in hidg_disable() function.

Signed-off-by: Robert Baldyga 
---
 drivers/usb/gadget/function/f_hid.c | 22 +-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_hid.c 
b/drivers/usb/gadget/function/f_hid.c
index 99285b4..0456a53 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -59,6 +59,7 @@ struct f_hidg {
boolwrite_pending;
wait_queue_head_t   write_queue;
struct usb_request  *req;
+   struct usb_request  **out_reqs;
 
int minor;
struct cdev cdev;
@@ -490,6 +491,7 @@ static void hidg_disable(struct usb_function *f)
 {
struct f_hidg *hidg = func_to_hidg(f);
struct f_hidg_req_list *list, *next;
+   int i;
 
usb_ep_disable(hidg->in_ep);
usb_ep_disable(hidg->out_ep);
@@ -498,6 +500,12 @@ static void hidg_disable(struct usb_function *f)
list_del(>list);
kfree(list);
}
+
+   for (i = 0; i < hidg->qlen; ++i) {
+   kfree(hidg->out_reqs[i]->buf);
+   kfree(hidg->out_reqs[i]);
+   }
+   kfree(hidg->out_reqs);
 }
 
 static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
@@ -547,11 +555,14 @@ static int hidg_set_alt(struct usb_function *f, unsigned 
intf, unsigned alt)
/*
 * allocate a bunch of read buffers and queue them all at once.
 */
+   hidg->out_reqs = kzalloc(hidg->qlen *
+   sizeof(*hidg->out_reqs), GFP_KERNEL);
for (i = 0; i < hidg->qlen && status == 0; i++) {
struct usb_request *req =
hidg_alloc_ep_req(hidg->out_ep,
  hidg->report_length);
if (req) {
+   hidg->out_reqs[i] = req;
req->complete = hidg_set_report_complete;
req->context  = hidg;
status = usb_ep_queue(hidg->out_ep, req,
@@ -562,11 +573,20 @@ static int hidg_set_alt(struct usb_function *f, unsigned 
intf, unsigned alt)
} else {
usb_ep_disable(hidg->out_ep);
status = -ENOMEM;
-   goto fail;
+   goto free_req;
}
}
}
 
+free_req:
+   if (status < 0) {
+   while (i--) {
+   kfree(hidg->out_reqs[i]->buf);
+   kfree(hidg->out_reqs[i]);
+   }
+   kfree(hidg->out_reqs);
+   }
+
 fail:
return status;
 }
-- 
1.9.1

--
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] xhci: Fix memory leak in xhci_pme_acpi_rtd3_enable()

[Mika Westerberg]

There is a memory leak because acpi_evaluate_dsm() actually returns an
object which the caller is supposed to release. Fix this by calling
ACPI_FREE() for the returned object (this expands to kfree() so passing
NULL there is fine as well).

While there correct indentation in !CONFIG_ACPI case.

Signed-off-by: Mika Westerberg 
---
 drivers/usb/host/xhci-pci.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 17f6897acde2..c62109091d12 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -188,10 +188,14 @@ static void xhci_pme_acpi_rtd3_enable(struct pci_dev *dev)
0xb7, 0x0c, 0x34, 0xac, 0x01, 0xe9, 0xbf, 0x45,
0xb7, 0xe6, 0x2b, 0x34, 0xec, 0x93, 0x1e, 0x23,
};
-   acpi_evaluate_dsm(ACPI_HANDLE(>dev), intel_dsm_uuid, 3, 1, NULL);
+   union acpi_object *obj;
+
+   obj = acpi_evaluate_dsm(ACPI_HANDLE(>dev), intel_dsm_uuid, 3, 1,
+   NULL);
+   ACPI_FREE(obj);
 }
 #else
-   static void xhci_pme_acpi_rtd3_enable(struct pci_dev *dev) { }
+static void xhci_pme_acpi_rtd3_enable(struct pci_dev *dev) { }
 #endif /* CONFIG_ACPI */
 
 /* called during probe() after chip reset completes */
-- 
2.6.2

--
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 - VID/PID for Yaesu SCU-18 cable for ftdi-usb-sio

[Harald Linden]

Add PID for Yaesu SCU-18 data cable, modify definitions for RATOC 
products to fit general scheme.


--- ./drivers/usb/serial/ftdi_sio.c.orig	2015-11-28 00:14:58.411681897 +0100
+++ ./drivers/usb/serial/ftdi_sio.c	2015-11-28 00:15:28.112310588 +0100
@@ -823,7 +823,8 @@ static const struct usb_device_id id_tab
 		.driver_info = (kernel_ulong_t)_jtag_quirk },
 	{ USB_DEVICE(FTDI_VID, FTDI_TURTELIZER_PID),
 		.driver_info = (kernel_ulong_t)_jtag_quirk },
-	{ USB_DEVICE(RATOC_VENDOR_ID, RATOC_PRODUCT_ID_USB60F) },
+	{ USB_DEVICE(RATOC_VID, RATOC_USB60F_PID) },
+	{ USB_DEVICE(RATOC_VID, RATOC_YAESUSCU18_PID) },
 	{ USB_DEVICE(FTDI_VID, FTDI_REU_TINY_PID) },
 
 	/* Papouch devices based on FTDI chip */
--- ./drivers/usb/serial/ftdi_sio_ids.h.orig	2015-11-28 00:15:11.446518939 +0100
+++ ./drivers/usb/serial/ftdi_sio_ids.h	2015-11-28 00:15:33.779239742 +0100
@@ -613,8 +613,9 @@
 /*
  * RATOC REX-USB60F
  */
-#define RATOC_VENDOR_ID		0x0584
-#define RATOC_PRODUCT_ID_USB60F	0xb020
+#define RATOC_VID		0x0584
+#define RATOC_USB60F_PID	0xB020
+#define RATOC_YAESUSCU18_PID	0xB03A /* Yaesu SCU-18 data cable made by RATOC */
 
 /*
  * Infineon Technologies

Re: [PATCH v1 0/1] ioctl to disallow detaching kernel USB drivers

[Greg KH]

On Fri, Nov 27, 2015 at 09:44:45AM +0100, Krzysztof Opasiak wrote:
> 
> 
> On 11/26/2015 06:29 PM, Greg KH wrote:
> >On Thu, Nov 26, 2015 at 10:19:29AM +0100, Krzysztof Opasiak wrote:
> >>
> >>
> >>On 11/25/2015 04:45 PM, Emilio López wrote:
> >>>Hi everyone,
> >>>
> >>>This patch introduces a new ioctl, USBDEVFS_DROP_PRIVILEGES,
> >>>to voluntarily forgo the ability to issue ioctls which may
> >>>interfere with other users of the USB device.
> >>>
> >>>This feature allows a privileged process (in the case of Chrome OS,
> >>>permission_broker) to open a USB device node and then drop a number
> >>>of capabilities that are considered "privileged".
> >>
> >>We had the same idea in Tizen but for now we didn't have time to implement
> >>it.
> >>
> >>  These privileges
> >>>include the ability to reset the device if there are other users
> >>>(most notably a kernel driver) or to disconnect a kernel driver
> >>>from the device. The file descriptor can then be passed to an
> >>>unprivileged process.
> >>
> >>And how about switching configuration? This can be also harmful even if the
> >>are no other users for any interface in this configuration.
> >>(Just imagine the situation in which only second config contains an HID
> >>function and when app switch configuration it is activated without user
> >>knowing about this;))
> >
> >Adding this option might be nice.
> >
> >>>This is useful for granting a process access to a device with
> >>>multiple functions. It won't be able to use its access to one
> >>>function to disrupt or take over control of another function.
> >>
> >>I run through your code and as far as I understand above is not exactly
> >>true. Your patch allows only to prevent userspace from accessing interfaces
> >>which has kernel drivers, there is no way to stop an application from taking
> >>control over all free interfaces.
> >>
> >>Let's say that your device has 3 interfaces. First of them has a kernel
> >>driver but second and third doesn't. You have 2 apps. One should communicate
> >>using second interface and another one third. But first app is malicious and
> >>it claims all free interfaces of received device (your patch doesn't prevent
> >>this). And when second app starts it is unable to do anything with the
> >>device because all interfaces are taken. How would you like to handle this?
> >
> >You can't, and why would you ever want to, as you can't tell what an app
> >"should" or "should not" do.  If you really care about this, then use a
> >LSM policy to prevent this.
> 
> Well, an app can declare what it does and what it needs in it's manifest
> file (or some equivalent of this) and the platform should ensure that app
> can do only what it has declared.

"should"?  Depending on what?  :)

> I would really like to use LSM policy in here but currently it is impossible
> as one device node represents whole device. Permissions (even those from
> LSM) are being checked only on open() not on each ioctl() so as far as I
> know there is nothing which prevents any owner of opened fd to claim all
> available (not taken by someone else) interfaces and LSM policy is unable to
> filter those calls (unless we add some LSM hooks over there).

Yes, it's tough, I know, good luck.

Also deal with multiple devices, busses that are ordered differently
depending on the phase of the moon, and other fun things with dynamic
devices and ioctls.  It's a loosing battle :)

> >>Moreover I'm not convinced to this patch as it hardcodes the *policy* in
> >>kernel code.
> >
> >What policy is that?
> 
> It's a policy which defines set of ioctls which cannot be issued in
> "restricted mode".
> 
> >
> >>Generally our approach (with passing fd from broker to
> >>unprivileged process) was similar but we found out that if we would like to
> >>do this correctly there is much more things to filter than in this patch. We
> >>had two main ideas:
> >>
> >>- implement some LSM hooks in ioctls() but this leads to a lot of additional
> >>callbacks in lsm ops struct which even now is very big. But as a benefit we
> >>would get a very flexible policy consistent with other system policies
> >>
> >>- split single usb device node into multiple files which could represent
> >>single endpoins only for io and separate control file for privileged but
> >>it's quite a lot of work and I don't know if any one is going to accept such
> >>a change
> >
> >I've been asking for that for well over a decade, but no one ever did
> >the work.  I think if you work through the options, it ends up not being
> >a viable solution...
> >
> 
> I'm not surprised that no one ever did this as it looks like quite a lot of
> work and current interface is still working;) Do you have some link to a
> discussion or sth which shows why it's not a good solution?

Dig through the archives, I think the last time this was brought up was
way before USB 3 came out.  As for "not a good solution", you have to
map endpoints together somehow in some way in userspace, which gets
messy 

Re: [PATCH 1/3] usb: musb: convert printk to pr_*

[Greg Kroah-Hartman]

On Fri, Nov 27, 2015 at 03:23:33PM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 11/27/2015 1:38 PM, Rasmus Villemoes wrote:
> 
> >This file already uses pr_debug in a few places; this converts the
> >remaining printks.
> 
>Are you aware that printk(KERN_DEBUG, ...) and pr_debug() are not 
> equivalent?

Yes, and that is a good thing, you should be using pr_debug() instead of
printk(KERN_DEBUG...).

Why object to something like this?

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

Reply Me For Details

[HS09912HH3]

Good day,

I wish to contact you personally for an important proposal that might be 
of interest to you. I am sending this mail just to know if this email address
is functional.

I have something absolutely essential to discuss with you. Contact me for 
details through my private email: shu...@qq.com

Regards,
shu...@qq.com
--
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 - VID/PID for Yaesu SCU-18 cable for ftdi-usb-sio

[Greg KH]

On Sat, Nov 28, 2015 at 12:20:59AM +0100, Harald Linden wrote:
> Add PID for Yaesu SCU-18 data cable, modify definitions for RATOC products
> to fit general scheme.
> 

> --- ./drivers/usb/serial/ftdi_sio.c.orig  2015-11-28 00:14:58.411681897 
> +0100
> +++ ./drivers/usb/serial/ftdi_sio.c   2015-11-28 00:15:28.112310588 +0100
> @@ -823,7 +823,8 @@ static const struct usb_device_id id_tab
>   .driver_info = (kernel_ulong_t)_jtag_quirk },
>   { USB_DEVICE(FTDI_VID, FTDI_TURTELIZER_PID),
>   .driver_info = (kernel_ulong_t)_jtag_quirk },
> - { USB_DEVICE(RATOC_VENDOR_ID, RATOC_PRODUCT_ID_USB60F) },
> + { USB_DEVICE(RATOC_VID, RATOC_USB60F_PID) },
> + { USB_DEVICE(RATOC_VID, RATOC_YAESUSCU18_PID) },
>   { USB_DEVICE(FTDI_VID, FTDI_REU_TINY_PID) },
>  
>   /* Papouch devices based on FTDI chip */

> --- ./drivers/usb/serial/ftdi_sio_ids.h.orig  2015-11-28 00:15:11.446518939 
> +0100
> +++ ./drivers/usb/serial/ftdi_sio_ids.h   2015-11-28 00:15:33.779239742 
> +0100
> @@ -613,8 +613,9 @@
>  /*
>   * RATOC REX-USB60F
>   */
> -#define RATOC_VENDOR_ID  0x0584
> -#define RATOC_PRODUCT_ID_USB60F  0xb020
> +#define RATOC_VID0x0584

Why change this #define?

> +#define RATOC_USB60F_PID 0xB020

Why change this #define?

> +#define RATOC_YAESUSCU18_PID 0xB03A /* Yaesu SCU-18 data cable made by RATOC 
> */
>  
>  /*
>   * Infineon Technologies

Can you combine this into the same patch, and provide a "Signed-off-by:"
line as described in Documentation/SubmittingPatches?  Then we can apply
the patch.

thanks,

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

Re: [PATCH v5 7/7] usb: gadget: f_midi: pre-allocate IN requests

[Clemens Ladisch]

Felipe Ferreri Tonello wrote:
> On 13/11/15 08:55, Clemens Ladisch wrote:
>> Felipe F. Tonello wrote:
>>> +static void f_midi_transmit(struct f_midi *midi)
>>> +{
>>> +...
>>> +   len = kfifo_peek(>in_req_fifo, );
>>> +   ...
>>> +   if (req->length > 0) {
>>> +   WARNING(midi, "%s: All USB requests have been used. 
>>> Current queue size "
>>> +   "is %u, consider increasing it.\n", __func__, 
>>> midi->in_req_num);
>>> +   goto drop_out;
>>> +   }
>>
>> There are two cases where the in_req FIFO might overflow:
>> 1) the gadget is trying to send too much data at once; or
>> 2) the host does not bother to read any of the data.
>>
>> In case 1), the appropriate action would be to do nothing, so that the
>> remaining data is sent after some currently queued packets have been
>> transmitted.  In case 2), the appropriate action would be to drop the
>> data (even better, the _oldest_ data), and spamming the log with error
>> messages would not help.
>
> True. In this case the log will be spammed.
>
> How would you suggest to drop the oldest data? That doesn't really seem
> to be feasible.

There is usb_ep_dequeue().  Its documentation warns about some hardware,
but it would be possible to at least try it.

>> I'm not quite sure if trying to detect which of these cases we have is
>> possible, or worthwhile.  Anyway, with a packet size of 64, the queue
>> size would be 32*64 = 2KB, which should be enough for everyone.  So I
>> propose to ignore case 1), and to drop the error message.

After some thought, I'm not so sure anymore -- the ability to buffer
more than 2 KB of data is part of the snd_rawmidi_write() API, so this
could introduce a regression.  And I can imagine cases where one would
actually want to transmit large amounts data.

I think the safest approach would be to behave similar to the old driver,
i.e., when the queue overflows, do nothing (not even dropping data), and
rely on the transmit completion handler to continue.  (This implies that
ALSA's buffer can fill up, and that snd_rawmidi_write() can block.)


It you want to dequeue outdated data, I think this should be done with
a timeout, i.e., when the host did not read anything for some tens of
milliseconds or so.  This would be independent of the fill level of the
queue, and could be done either for individual packets, or just on the
entire endpoint queue.


Regards,
Clemens
--
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 v1 0/1] ioctl to disallow detaching kernel USB drivers

[Krzysztof Opasiak]

On 11/26/2015 06:29 PM, Greg KH wrote:

On Thu, Nov 26, 2015 at 10:19:29AM +0100, Krzysztof Opasiak wrote:



On 11/25/2015 04:45 PM, Emilio López wrote:

Hi everyone,

This patch introduces a new ioctl, USBDEVFS_DROP_PRIVILEGES,
to voluntarily forgo the ability to issue ioctls which may
interfere with other users of the USB device.

This feature allows a privileged process (in the case of Chrome OS,
permission_broker) to open a USB device node and then drop a number
of capabilities that are considered "privileged".


We had the same idea in Tizen but for now we didn't have time to implement
it.

  These privileges

include the ability to reset the device if there are other users
(most notably a kernel driver) or to disconnect a kernel driver

>from the device. The file descriptor can then be passed to an

unprivileged process.


And how about switching configuration? This can be also harmful even if the
are no other users for any interface in this configuration.
(Just imagine the situation in which only second config contains an HID
function and when app switch configuration it is activated without user
knowing about this;))


Adding this option might be nice.


This is useful for granting a process access to a device with
multiple functions. It won't be able to use its access to one
function to disrupt or take over control of another function.


I run through your code and as far as I understand above is not exactly
true. Your patch allows only to prevent userspace from accessing interfaces
which has kernel drivers, there is no way to stop an application from taking
control over all free interfaces.

Let's say that your device has 3 interfaces. First of them has a kernel
driver but second and third doesn't. You have 2 apps. One should communicate
using second interface and another one third. But first app is malicious and
it claims all free interfaces of received device (your patch doesn't prevent
this). And when second app starts it is unable to do anything with the
device because all interfaces are taken. How would you like to handle this?


You can't, and why would you ever want to, as you can't tell what an app
"should" or "should not" do.  If you really care about this, then use a
LSM policy to prevent this.


Well, an app can declare what it does and what it needs in it's manifest 
file (or some equivalent of this) and the platform should ensure that 
app can do only what it has declared.


I would really like to use LSM policy in here but currently it is 
impossible as one device node represents whole device. Permissions (even 
those from LSM) are being checked only on open() not on each ioctl() so 
as far as I know there is nothing which prevents any owner of opened fd 
to claim all available (not taken by someone else) interfaces and LSM 
policy is unable to filter those calls (unless we add some LSM hooks 
over there).





Moreover I'm not convinced to this patch as it hardcodes the *policy* in
kernel code.


What policy is that?


It's a policy which defines set of ioctls which cannot be issued in 
"restricted mode".





Generally our approach (with passing fd from broker to
unprivileged process) was similar but we found out that if we would like to
do this correctly there is much more things to filter than in this patch. We
had two main ideas:

- implement some LSM hooks in ioctls() but this leads to a lot of additional
callbacks in lsm ops struct which even now is very big. But as a benefit we
would get a very flexible policy consistent with other system policies

- split single usb device node into multiple files which could represent
single endpoins only for io and separate control file for privileged but
it's quite a lot of work and I don't know if any one is going to accept such
a change


I've been asking for that for well over a decade, but no one ever did
the work.  I think if you work through the options, it ends up not being
a viable solution...



I'm not surprised that no one ever did this as it looks like quite a lot 
of work and current interface is still working;) Do you have some link 
to a discussion or sth which shows why it's not a good solution?


--
Krzysztof Opasiak
Samsung R Institute Poland
Samsung Electronics
--
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: [linux-sunxi] Re: [PATCH v3 1/2] phy-sun4i-usb: Use of_match_node to get model specific config data

[Maxime Ripard]

On Thu, Nov 26, 2015 at 01:11:32PM +0100, Hans de Goede wrote:
> >>+enum sun4i_usb_phy_type {
> >>+   sun4i_a10_phy,
> >>+   sun8i_a33_phy,
> >>+};
> >>+
> >>+struct sun4i_usb_phy_cfg {
> >>+   int num_phys;
> >>+   u32 disc_thresh;
> >>+   enum sun4i_usb_phy_type type;
> >>+   bool dedicated_clocks;
> >>+};
> >>+
> >>  struct sun4i_usb_phy_data {
> >> void __iomem *base;
> >>+   const struct sun4i_usb_phy_cfg *cfg;
> >> struct mutex mutex;
> >>-   int num_phys;
> >>-   u32 disc_thresh;
> >>-   bool has_a33_phyctl;
> >> struct sun4i_usb_phy {
> >> struct phy *phy;
> >> void __iomem *pmu;
> >>@@ -164,12 +174,15 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy 
> >>*phy, u32 addr, u32 data,
> >>
> >> mutex_lock(_data->mutex);
> >>
> >>-   if (phy_data->has_a33_phyctl) {
> >>+   switch (phy_data->cfg->type) {
> >>+   case sun4i_a10_phy:
> >>+   phyctl = phy_data->base + REG_PHYCTL_A10;
> >
> >Any reason why this offset isn't incorporated into phy_data?
> 
> You mean in phy_data->cfg I assume, the difference needed for
> the "sun4i_usb_phy_write" functionality are not just the phyctl
> register offset...
> 
> >
> >>+   break;
> >>+   case sun8i_a33_phy:
> >> phyctl = phy_data->base + REG_PHYCTL_A33;
> >> /* A33 needs us to set phyctl to 0 explicitly */
> >> writel(0, phyctl);
> 
> e.g. the A33 needs this extra write, and on the H3 we need to do
> similar bitbanging, but slightly different, see:
> 
> https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/drivers/usb/host/sunxi_hci.c#L899
> 
> Notice how it uses different addr and write register addresses
> their through the usb_phy_csr_add and usb_phy_csr_write helper
> functions as well as directly poking offset 0x20.

Then it easy to support: one u8 for each register that changes, one
bool to tell if you need to clear the phyctl register or not, And you
don't have to duplicate the switch everywhere, and basically just
reimplement of_device_is_compatible without an actual compatible to
workaround the review ;)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature

[PATCH 2/3] usb: musb: remove always-empty string from debug output

[Rasmus Villemoes]

aDate is always empty, hence pointless.

Signed-off-by: Rasmus Villemoes 
---
 drivers/usb/musb/musb_core.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 1ac976332060..86dce9635b14 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1458,7 +1458,7 @@ static int musb_core_init(u16 musb_type, struct musb 
*musb)
 {
u8 reg;
char *type;
-   char aInfo[90], aRevision[32], aDate[12];
+   char aInfo[90], aRevision[32];
void __iomem*mbase = musb->mregs;
int status = 0;
int i;
@@ -1492,7 +1492,6 @@ static int musb_core_init(u16 musb_type, struct musb 
*musb)
 
pr_debug("%s: ConfigData=0x%02x (%s)\n", musb_driver_name, reg, aInfo);
 
-   aDate[0] = 0;
if (MUSB_CONTROLLER_MHDRC == musb_type) {
musb->is_multipoint = 1;
type = "M";
@@ -1510,8 +1509,8 @@ static int musb_core_init(u16 musb_type, struct musb 
*musb)
snprintf(aRevision, 32, "%d.%d%s", MUSB_HWVERS_MAJOR(musb->hwvers),
MUSB_HWVERS_MINOR(musb->hwvers),
(musb->hwvers & MUSB_HWVERS_RC) ? "RC" : "");
-   pr_debug("%s: %sHDRC RTL version %s %s\n",
-musb_driver_name, type, aRevision, aDate);
+   pr_debug("%s: %sHDRC RTL version %s\n",
+musb_driver_name, type, aRevision);
 
/* configure ep0 */
musb_configure_ep0(musb);
-- 
2.6.1

--
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 3/3] usb: musb: remove redundant stack buffer

[Rasmus Villemoes]

aRevision is only used once, so we might as well do the formatting as
part of the pr_debug. This eliminates the stack buffer, and avoids
doing the formatting at all when pr_debug is compiled out.

Signed-off-by: Rasmus Villemoes 
---
 drivers/usb/musb/musb_core.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 86dce9635b14..3e8d1bcfc1b5 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1458,7 +1458,7 @@ static int musb_core_init(u16 musb_type, struct musb 
*musb)
 {
u8 reg;
char *type;
-   char aInfo[90], aRevision[32];
+   char aInfo[90];
void __iomem*mbase = musb->mregs;
int status = 0;
int i;
@@ -1506,11 +1506,11 @@ static int musb_core_init(u16 musb_type, struct musb 
*musb)
 
/* log release info */
musb->hwvers = musb_read_hwvers(mbase);
-   snprintf(aRevision, 32, "%d.%d%s", MUSB_HWVERS_MAJOR(musb->hwvers),
-   MUSB_HWVERS_MINOR(musb->hwvers),
-   (musb->hwvers & MUSB_HWVERS_RC) ? "RC" : "");
-   pr_debug("%s: %sHDRC RTL version %s\n",
-musb_driver_name, type, aRevision);
+   pr_debug("%s: %sHDRC RTL version %d.%d%s\n",
+musb_driver_name, type,
+MUSB_HWVERS_MAJOR(musb->hwvers),
+MUSB_HWVERS_MINOR(musb->hwvers),
+(musb->hwvers & MUSB_HWVERS_RC) ? "RC" : "");
 
/* configure ep0 */
musb_configure_ep0(musb);
-- 
2.6.1

--
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: [linux-sunxi] Re: [PATCH v3 1/2] phy-sun4i-usb: Use of_match_node to get model specific config data

[Hans de Goede]

Hi,

On 27-11-15 09:53, Maxime Ripard wrote:

On Thu, Nov 26, 2015 at 01:11:32PM +0100, Hans de Goede wrote:

+enum sun4i_usb_phy_type {
+   sun4i_a10_phy,
+   sun8i_a33_phy,
+};
+
+struct sun4i_usb_phy_cfg {
+   int num_phys;
+   u32 disc_thresh;
+   enum sun4i_usb_phy_type type;
+   bool dedicated_clocks;
+};
+
  struct sun4i_usb_phy_data {
 void __iomem *base;
+   const struct sun4i_usb_phy_cfg *cfg;
 struct mutex mutex;
-   int num_phys;
-   u32 disc_thresh;
-   bool has_a33_phyctl;
 struct sun4i_usb_phy {
 struct phy *phy;
 void __iomem *pmu;
@@ -164,12 +174,15 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy 
*phy, u32 addr, u32 data,

 mutex_lock(_data->mutex);

-   if (phy_data->has_a33_phyctl) {
+   switch (phy_data->cfg->type) {
+   case sun4i_a10_phy:
+   phyctl = phy_data->base + REG_PHYCTL_A10;


Any reason why this offset isn't incorporated into phy_data?


You mean in phy_data->cfg I assume, the difference needed for
the "sun4i_usb_phy_write" functionality are not just the phyctl
register offset...




+   break;
+   case sun8i_a33_phy:
 phyctl = phy_data->base + REG_PHYCTL_A33;
 /* A33 needs us to set phyctl to 0 explicitly */
 writel(0, phyctl);


e.g. the A33 needs this extra write, and on the H3 we need to do
similar bitbanging, but slightly different, see:

https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/drivers/usb/host/sunxi_hci.c#L899

Notice how it uses different addr and write register addresses
their through the usb_phy_csr_add and usb_phy_csr_write helper
functions as well as directly poking offset 0x20.


Then it easy to support: one u8 for each register that changes, one
bool to tell if you need to clear the phyctl register or not, And you
don't have to duplicate the switch everywhere, and basically just
reimplement of_device_is_compatible without an actual compatible to
workaround the review ;)


You clearly have not looked at the actual code I've linked to, the
entire "algorithm" for sun4i_usb_phy_write is different on the H3.

Moreover, this has nothing to do with this patch, the code coding the
difference behavior for the a10 style phy and the a33 style phy is already
there before this patch-set, and this is not something Kishon asked
me to change.

Regards,

Hans
--
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 1/3] usb: musb: convert printk to pr_*

[Rasmus Villemoes]

This file already uses pr_debug in a few places; this converts the
remaining printks.

Signed-off-by: Rasmus Villemoes 
---
 drivers/usb/musb/musb_core.c | 17 +++--
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 18cfc0a361cb..1ac976332060 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1360,8 +1360,7 @@ static int ep_config_from_table(struct musb *musb)
break;
}
 
-   printk(KERN_DEBUG "%s: setup fifo_mode %d\n",
-   musb_driver_name, fifo_mode);
+   pr_debug("%s: setup fifo_mode %d\n", musb_driver_name, fifo_mode);
 
 
 done:
@@ -1390,7 +1389,7 @@ done:
musb->nr_endpoints = max(epn, musb->nr_endpoints);
}
 
-   printk(KERN_DEBUG "%s: %d/%d max ep, %d/%d memory\n",
+   pr_debug("%s: %d/%d max ep, %d/%d memory\n",
musb_driver_name,
n + 1, musb->config->num_eps * 2 - 1,
offset, (1 << (musb->config->ram_bits + 2)));
@@ -1491,8 +1490,7 @@ static int musb_core_init(u16 musb_type, struct musb 
*musb)
if (reg & MUSB_CONFIGDATA_SOFTCONE)
strcat(aInfo, ", SoftConn");
 
-   printk(KERN_DEBUG "%s: ConfigData=0x%02x (%s)\n",
-   musb_driver_name, reg, aInfo);
+   pr_debug("%s: ConfigData=0x%02x (%s)\n", musb_driver_name, reg, aInfo);
 
aDate[0] = 0;
if (MUSB_CONTROLLER_MHDRC == musb_type) {
@@ -1502,9 +1500,8 @@ static int musb_core_init(u16 musb_type, struct musb 
*musb)
musb->is_multipoint = 0;
type = "";
 #ifndefCONFIG_USB_OTG_BLACKLIST_HUB
-   printk(KERN_ERR
-   "%s: kernel must blacklist external hubs\n",
-   musb_driver_name);
+   pr_err("%s: kernel must blacklist external hubs\n",
+  musb_driver_name);
 #endif
}
 
@@ -1513,8 +1510,8 @@ static int musb_core_init(u16 musb_type, struct musb 
*musb)
snprintf(aRevision, 32, "%d.%d%s", MUSB_HWVERS_MAJOR(musb->hwvers),
MUSB_HWVERS_MINOR(musb->hwvers),
(musb->hwvers & MUSB_HWVERS_RC) ? "RC" : "");
-   printk(KERN_DEBUG "%s: %sHDRC RTL version %s %s\n",
-   musb_driver_name, type, aRevision, aDate);
+   pr_debug("%s: %sHDRC RTL version %s %s\n",
+musb_driver_name, type, aRevision, aDate);
 
/* configure ep0 */
musb_configure_ep0(musb);
-- 
2.6.1

--
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: [linux-sunxi] Re: [PATCH v3 1/2] phy-sun4i-usb: Use of_match_node to get model specific config data

[Hans de Goede]

Hi,

On 27-11-15 11:37, Hans de Goede wrote:

Hi,

On 27-11-15 09:53, Maxime Ripard wrote:

On Thu, Nov 26, 2015 at 01:11:32PM +0100, Hans de Goede wrote:

+enum sun4i_usb_phy_type {
+   sun4i_a10_phy,
+   sun8i_a33_phy,
+};
+
+struct sun4i_usb_phy_cfg {
+   int num_phys;
+   u32 disc_thresh;
+   enum sun4i_usb_phy_type type;
+   bool dedicated_clocks;
+};
+
  struct sun4i_usb_phy_data {
 void __iomem *base;
+   const struct sun4i_usb_phy_cfg *cfg;
 struct mutex mutex;
-   int num_phys;
-   u32 disc_thresh;
-   bool has_a33_phyctl;
 struct sun4i_usb_phy {
 struct phy *phy;
 void __iomem *pmu;
@@ -164,12 +174,15 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy 
*phy, u32 addr, u32 data,

 mutex_lock(_data->mutex);

-   if (phy_data->has_a33_phyctl) {
+   switch (phy_data->cfg->type) {
+   case sun4i_a10_phy:
+   phyctl = phy_data->base + REG_PHYCTL_A10;


Any reason why this offset isn't incorporated into phy_data?


You mean in phy_data->cfg I assume, the difference needed for
the "sun4i_usb_phy_write" functionality are not just the phyctl
register offset...




+   break;
+   case sun8i_a33_phy:
 phyctl = phy_data->base + REG_PHYCTL_A33;
 /* A33 needs us to set phyctl to 0 explicitly */
 writel(0, phyctl);


e.g. the A33 needs this extra write, and on the H3 we need to do
similar bitbanging, but slightly different, see:

https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/drivers/usb/host/sunxi_hci.c#L899

Notice how it uses different addr and write register addresses
their through the usb_phy_csr_add and usb_phy_csr_write helper
functions as well as directly poking offset 0x20.


Then it easy to support: one u8 for each register that changes, one
bool to tell if you need to clear the phyctl register or not, And you
don't have to duplicate the switch everywhere, and basically just
reimplement of_device_is_compatible without an actual compatible to
workaround the review ;)


You clearly have not looked at the actual code I've linked to, the
entire "algorithm" for sun4i_usb_phy_write is different on the H3.

Moreover, this has nothing to do with this patch, the code coding the
difference behavior for the a10 style phy and the a33 style phy is already
there before this patch-set, and this is not something Kishon asked
me to change.


And on top of that phy_data->cfg->type is also used in different places,
where the behavior for h3 is again completely different then for the other
phy-s. There are just some differences which cannot be encoded in the form
of register offsets or bools between the old a10 usb phys and the newer ones.

And yes I've considered to just do a whole new phy driver, but there is enough
overlap to make doing this in one driver more sensible. This may change when
we add otg support for the H3. But we can always do a new driver then, and
remove the limited (see the 2nd patch in this series) amount of changes needed
for the H3 from phy-sun4i-usb.c again.

Regards,

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