[PATCH 03/28] usb: dwc3: gadget: move isoc endpoint check to unlocked set_halt

[Felipe Balbi]

__dwc3_gadget_ep_set_halt() is the function which
handles the actual halt feature. In order to cope
with some extra cleanup comming as a follow-up patch
let's move the isochronous endpoint check there too.

Signed-off-by: Felipe Balbi <balbi-***@public.gmane.org>
---
drivers/usb/dwc3/gadget.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 3818b26b..fd92f63 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1208,6 +1208,11 @@ int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value)
struct dwc3 *dwc = dep->dwc;
int ret;

+ if (usb_endpoint_xfer_isoc(dep->endpoint.desc)) {
+ dev_err(dwc->dev, "%s is of Isochronous type\n", dep->name);
+ return -EINVAL;
+ }
+
memset(&params, 0x00, sizeof(params));

if (value) {
@@ -1241,15 +1246,7 @@ static int dwc3_gadget_ep_set_halt(struct usb_ep *ep, int value)
int ret;

spin_lock_irqsave(&dwc->lock, flags);
-
- if (usb_endpoint_xfer_isoc(dep->endpoint.desc)) {
- dev_err(dwc->dev, "%s is of Isochronous type\n", dep->name);
- ret = -EINVAL;
- goto out;
- }

[Felipe Balbi]

The way trace works is that it won't decode strings
until we read the actual trace. Because of that, we
can't make assumptions of pointers still being valid
at the time we read the trace. In order to avoid that,
just copy all fields from every struct pointer we need
for our traces.

Ths patch fixes the following bug:

[ 2940.039229] Unable to handle kernel paging request at virtual address 814efa9e
[ 2940.046904] pgd = ec3dc000
[ 2940.049737] [814efa9e] *pgd=00000000
[ 2940.053552] Internal error: Oops: 5 [#1] SMP ARM
[ 2940.058379] Modules linked in: usb_f_acm u_serial g_serial usb_f_uac2 libcomposite configfs xhci_hcd dwc3 udc_core matrix_keypad dwc3_omap lis3lv02d_i2c lis3lv02d input_polldev [last unloaded: g_audio]
[ 2940.077238] CPU: 0 PID: 3020 Comm: tail Tainted: G W
3.17.0-rc5-dirty #1097
[ 2940.085596] task: ed1b1040 ti: ed07c000 task.ti: ed07c000
[ 2940.091258] PC is at strnlen+0x18/0x68
[ 2940.095177] LR is at 0xfffffffe
[ 2940.098454] pc : [<c0356df8>] lr : [<fffffffe>] psr: a0000013
[ 2940.098454] sp : ed07ddb0 ip : ed07ddc0 fp : ed07ddbc
[ 2940.110445] r10: c070ff70 r9 : ed07de70 r8 : 00000000
[ 2940.115906] r7 : 814efa9e r6 : ffffffff r5 : ed4b6087 r4 : ed4b50c7
[ 2940.122726] r3 : 00000000 r2 : 814efa9e r1 : ffffffff r0 : 814efa9e
[ 2940.129546] Flags: NzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
[ 2940.137000] Control: 10c5387d Table: ac3dc059 DAC: 00000015
[ 2940.143006] Process tail (pid: 3020, stack limit = 0xed07c248)
[ 2940.149098] Stack: (0xed07ddb0 to 0xed07e000)
[ 2940.153660] dda0: ed07dde4 ed07ddc0 c0359628 c0356dec
[ 2940.162203] ddc0: 00000000 ed4b50c7 bf03ae9c ed4b6087 bf03ae9e 00000002 ed07de3c ed07dde8
[ 2940.170740] dde0: c035ab50 c0359600 ffffffff ffffffff ff0a0000 ffffffff ed07de30 ed4b5088
[ 2940.179275] de00: ed4b50c7 00000fc0 ff0a0004 ffffffff ed4b5088 ed4b5088 00000000 00001000
[ 2940.187810] de20: 00001008 00000fc0 ed4b5088 00000000 ed07de68 ed07de40 c00f1e64 c035a9c4
[ 2940.196341] de40: bf03dae0 ed07de70 ed4b4000 ec25b280 ed4b4000 ec25b280 bf03dae0 ed07de9c
[ 2940.204886] de60: ed07de78 bf033324 c00f1e0c bf03ae9c 814efa9e ed428bc0 814eca3e 00000000
[ 2940.213428] de80: 814eba3e ed4b4000 03bd1201 c0c34790 ed07ded4 ed07dea0 c00edc0c bf0332d0
[ 2940.221994] dea0: 000002c7 ed07df10 ed07decc ed07deb8 ed4b4000 0000209c ec278ac0 00000000
[ 2940.230536] dec0: 00002000 ec0db340 ed07def4 ed07ded8 c00ee7ec c00eda90 c00ee7b0 ec278ac0
[ 2940.239075] dee0: ed4b4000 000002d5 ed07df44 ed07def8 c018b8d0 c00ee7bc c0166d3c ec278af0
[ 2940.247621] df00: 0001f090 ed07df78 000002c7 00000000 000002c8 00000000 00000000 ec0db340
[ 2940.256173] df20: 0001f090 ed07df78 ec0db340 00002000 0001f090 00000000 ed07df74 ed07df48
[ 2940.264729] df40: c0166e98 c018b5f4 00000001 c018535c 000168c1 00000000 ec0db340 ec0db340
[ 2940.273284] df60: 00002000 0001f090 ed07dfa4 ed07df78 c01675c4 c0166e0c 000168c1 00000000
[ 2940.281829] df80: 00002000 0000000a 0001f090 00000003 c000f064 ed07c000 00000000 ed07dfa8
[ 2940.290365] dfa0: c000ede0 c0167584 00002000 0000000a 00000003 0001f090 00002000 00000000
[ 2940.298909] dfc0: 00002000 0000000a 0001f090 00000003 7fffe000 0001e1e0 00002004 0000002f
[ 2940.307445] dfe0: 00000000 beed38ec 000104c8 b6e6397c 40000010 00000003 00000000 00000000
[ 2940.315992] [<c0356df8>] (strnlen) from [<c0359628>] (string.isra.8+0x34/0xe8)
[ 2940.323534] [<c0359628>] (string.isra.8) from [<c035ab50>] (vsnprintf+0x198/0x3fc)
[ 2940.331461] [<c035ab50>] (vsnprintf) from [<c00f1e64>] (trace_seq_printf+0x68/0x94)
[ 2940.339494] [<c00f1e64>] (trace_seq_printf) from [<bf033324>] (ftrace_raw_output_dwc3_log_request+0x60/0x78 [dwc3])
[ 2940.350424] [<bf033324>] (ftrace_raw_output_dwc3_log_request [dwc3]) from [<c00edc0c>] (print_trace_line+0x188/0x418)
[ 2940.361507] [<c00edc0c>] (print_trace_line) from [<c00ee7ec>] (s_show+0x3c/0x12c)
[ 2940.369330] [<c00ee7ec>] (s_show) from [<c018b8d0>] (seq_read+0x2e8/0x4a0)
[ 2940.376519] [<c018b8d0>] (seq_read) from [<c0166e98>] (vfs_read+0x98/0x158)
[ 2940.383796] [<c0166e98>] (vfs_read) from [<c01675c4>] (SyS_read+0x4c/0xa0)
[ 2940.390981] [<c01675c4>] (SyS_read) from [<c000ede0>] (ret_fast_syscall+0x0/0x48)
[ 2940.398792] Code: e24cb004 e3510000 e241e001 0a000011 (e5d01000)
[ 2940.406980] ---[ end trace d8b38370fbb531f3 ]---

Signed-off-by: Felipe Balbi <balbi-***@public.gmane.org>
---
drivers/usb/dwc3/trace.h | 53 ++++++++++++++++++++++++++++++++++--------------
1 file changed, 38 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/dwc3/trace.h b/drivers/usb/dwc3/trace.h
index 78aff1d..60b0f41 100644
--- a/drivers/usb/dwc3/trace.h
+++ b/drivers/usb/dwc3/trace.h
@@ -73,15 +73,23 @@ DECLARE_EVENT_CLASS(dwc3_log_ctrl,
TP_PROTO(struct usb_ctrlrequest *ctrl),
TP_ARGS(ctrl),
TP_STRUCT__entry(
- __field(struct usb_ctrlrequest *, ctrl)
+ __field(__u8, bRequestType)
+ __field(__u8, bRequest)
+ __field(__le16, wValue)
+ __field(__le16, wIndex)
+ __field(__le16, wLength)
),
TP_fast_assign(
- __entry->ctrl = ctrl;
+ __entry->bRequestType = ctrl->bRequestType;
+ __entry->bRequest = ctrl->bRequest;
+ __entry->wValue = ctrl->wValue;
+ __entry->wIndex = ctrl->wIndex;
+ __entry->wLength = ctrl->wLength;
),
TP_printk("bRequestType %02x bRequest %02x wValue %04x wIndex %04x wLength %d",
- __entry->ctrl->bRequestType, __entry->ctrl->bRequest,
- le16_to_cpu(__entry->ctrl->wValue), le16_to_cpu(__entry->ctrl->wIndex),
- le16_to_cpu(__entry->ctrl->wLength)
+ __entry->bRequestType, __entry->bRequest,
+ le16_to_cpu(__entry->wValue), le16_to_cpu(__entry->wIndex),
+ le16_to_cpu(__entry->wLength)
)
);

@@ -94,15 +102,22 @@ DECLARE_EVENT_CLASS(dwc3_log_request,
TP_PROTO(struct dwc3_request *req),
TP_ARGS(req),
TP_STRUCT__entry(
+ __dynamic_array(char, name, DWC3_MSG_MAX)
__field(struct dwc3_request *, req)
+ __field(unsigned, actual)
+ __field(unsigned, length)
+ __field(int, status)
),
TP_fast_assign(
+ snprintf(__get_str(name), DWC3_MSG_MAX, "%s", req->dep->name);
__entry->req = req;
+ __entry->actual = req->request.actual;
+ __entry->length = req->request.length;
+ __entry->status = req->request.status;
),
TP_printk("%s: req %p length %u/%u ==> %d",
- __entry->req->dep->name, __entry->req,
- __entry->req->request.actual, __entry->req->request.length,
- __entry->req->request.status
+ __get_str(name), __entry->req, __entry->actual, __entry->length,
+ __entry->status
)
);

@@ -158,17 +173,17 @@ DECLARE_EVENT_CLASS(dwc3_log_gadget_ep_cmd,
struct dwc3_gadget_ep_cmd_params *params),
TP_ARGS(dep, cmd, params),
TP_STRUCT__entry(
- __field(struct dwc3_ep *, dep)
+ __dynamic_array(char, name, DWC3_MSG_MAX)
__field(unsigned int, cmd)
__field(struct dwc3_gadget_ep_cmd_params *, params)
),
TP_fast_assign(
- __entry->dep = dep;
+ snprintf(__get_str(name), DWC3_MSG_MAX, "%s", dep->name);
__entry->cmd = cmd;
__entry->params = params;
),
TP_printk("%s: cmd '%s' [%d] params %08x %08x %08x\n",
- __entry->dep->name, dwc3_gadget_ep_cmd_string(__entry->cmd),
+ __get_str(name), dwc3_gadget_ep_cmd_string(__entry->cmd),
__entry->cmd, __entry->params->param0,
__entry->params->param1, __entry->params->param2
)
@@ -184,16 +199,24 @@ DECLARE_EVENT_CLASS(dwc3_log_trb,
TP_PROTO(struct dwc3_ep *dep, struct dwc3_trb *trb),
TP_ARGS(dep, trb),
TP_STRUCT__entry(
- __field(struct dwc3_ep *, dep)
+ __dynamic_array(char, name, DWC3_MSG_MAX)
__field(struct dwc3_trb *, trb)
+ __field(u32, bpl)
+ __field(u32, bph)
+ __field(u32, size)
+ __field(u32, ctrl)
),
TP_fast_assign(
- __entry->dep = dep;
+ snprintf(__get_str(name), DWC3_MSG_MAX, "%s", dep->name);
__entry->trb = trb;
+ __entry->bpl = trb->bpl;
+ __entry->bph = trb->bph;
+ __entry->size = trb->size;
+ __entry->ctrl = trb->ctrl;
),
TP_printk("%s: trb %p bph %08x bpl %08x size %08x ctrl %08x\n",
- __entry->dep->name, __entry->trb, __entry->trb->bph,
- __entry->trb->bpl, __entry->trb->size, __entry->trb->ctrl
+ __get_str(name), __entry->trb, __entry->bph, __entry->bpl,
+ __entry->size, __entry->ctrl
)
);

[Felipe Balbi]

According to our Gadget Framework API documentation,
->set_halt() *must* return -EAGAIN if we have pending
transfers (on either direction) or FIFO isn't empty (on
TX endpoints).

Fix this bug so that the mass storage gadget can be used
without stall=0 parameter.

This patch should be backported to all kernels since v3.2.

Cc: <stable-***@public.gmane.org> # v3.2+
Suggested-by: Alan Stern <stern-nwvwT67g6+6dFdvTe/***@public.gmane.org>
Signed-off-by: Felipe Balbi <balbi-***@public.gmane.org>
---
drivers/usb/dwc3/ep0.c | 4 ++--
drivers/usb/dwc3/gadget.c | 16 ++++++++++++----
drivers/usb/dwc3/gadget.h | 2 +-
3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 36f6158..ae6b575 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -256,7 +256,7 @@ static void dwc3_ep0_stall_and_restart(struct dwc3 *dwc)

/* stall is always issued on EP0 */
dep = dwc->eps[0];
- __dwc3_gadget_ep_set_halt(dep, 1);
+ __dwc3_gadget_ep_set_halt(dep, 1, false);
dep->flags = DWC3_EP_ENABLED;
dwc->delayed_status = false;

@@ -480,7 +480,7 @@ static int dwc3_ep0_handle_feature(struct dwc3 *dwc,
return -EINVAL;
if (set == 0 && (dep->flags & DWC3_EP_WEDGE))
break;
- ret = __dwc3_gadget_ep_set_halt(dep, set);
+ ret = __dwc3_gadget_ep_set_halt(dep, set, true);
if (ret)
return -EINVAL;
break;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index b98295e..f6d1dba 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -581,7 +581,7 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep)

/* make sure HW endpoint isn't stalled */
if (dep->flags & DWC3_EP_STALL)
- __dwc3_gadget_ep_set_halt(dep, 0);
+ __dwc3_gadget_ep_set_halt(dep, 0, false);

reg = dwc3_readl(dwc->regs, DWC3_DALEPENA);
reg &= ~DWC3_DALEPENA_EP(dep->number);
@@ -1202,7 +1202,7 @@ out0:
return ret;
}

-int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value)
+int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value, int protocol)
{
struct dwc3_gadget_ep_cmd_params params;
struct dwc3 *dwc = dep->dwc;
@@ -1216,6 +1216,14 @@ int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value)
memset(&params, 0x00, sizeof(params));

if (value) {
+ if (!protocol && ((dep->direction && dep->flags & DWC3_EP_BUSY) ||
+ (!list_empty(&dep->req_queued) ||
+ !list_empty(&dep->request_list)))) {
+ dev_dbg(dwc->dev, "%s: pending request, cannot halt\n",
+ dep->name);
+ return -EAGAIN;
+ }
+
ret = dwc3_send_gadget_ep_cmd(dwc, dep->number,
DWC3_DEPCMD_SETSTALL, &params);
if (ret)
@@ -1246,7 +1254,7 @@ static int dwc3_gadget_ep_set_halt(struct usb_ep *ep, int value)
int ret;

spin_lock_irqsave(&dwc->lock, flags);
- ret = __dwc3_gadget_ep_set_halt(dep, value);
+ ret = __dwc3_gadget_ep_set_halt(dep, value, false);
spin_unlock_irqrestore(&dwc->lock, flags);

return ret;
@@ -1265,7 +1273,7 @@ static int dwc3_gadget_ep_set_wedge(struct usb_ep *ep)
if (dep->number == 0 || dep->number == 1)
ret = __dwc3_gadget_ep0_set_halt(ep, 1);
else
- ret = __dwc3_gadget_ep_set_halt(dep, 1);
+ ret = __dwc3_gadget_ep_set_halt(dep, 1, false);
spin_unlock_irqrestore(&dwc->lock, flags);

return ret;
diff --git a/drivers/usb/dwc3/gadget.h b/drivers/usb/dwc3/gadget.h
index f889008..18ae3ea 100644
--- a/drivers/usb/dwc3/gadget.h
+++ b/drivers/usb/dwc3/gadget.h
@@ -86,7 +86,7 @@ int __dwc3_gadget_ep0_set_halt(struct usb_ep *ep, int value);
int dwc3_gadget_ep0_set_halt(struct usb_ep *ep, int value);
int dwc3_gadget_ep0_queue(struct usb_ep *ep, struct usb_request *request,
gfp_t gfp_flags);
-int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value);
+int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value, int protocol);

/**
* dwc3_gadget_ep_get_transfer_index - Gets transfer index from HW

[Felipe Balbi]

During Halt Endpoint Test, our interrupt endpoint
will be disabled, which will clear out ep->desc
to NULL. Unless we call config_ep_by_speed() again,
we will not be able to enable this endpoint which
will make us fail that test.

Fixes: f9c56cd (usb: gadget: Clear usb_endpoint_descriptor
inside the struct usb_ep on disable)
Cc: <***@vger.kernel.org> # v3.4+
Signed-off-by: Felipe Balbi <***@ti.com>
---
drivers/usb/gadget/function/f_acm.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/function/f_acm.c b/drivers/usb/gadget/function/f_acm.c
index 6da4685..aad8165 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -433,12 +433,12 @@ static int acm_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
dev_vdbg(&cdev->gadget->dev,
"reset acm control interface %d\n", intf);
usb_ep_disable(acm->notify);
- } else {
- dev_vdbg(&cdev->gadget->dev,
- "init acm ctrl interface %d\n", intf);
+ }
+
+ if (!acm->notify->desc)
if (config_ep_by_speed(cdev->gadget, f, acm->notify))
return -EINVAL;
- }
+
usb_ep_enable(acm->notify);
acm->notify->driver_data = acm;

[Felipe Balbi]

just like any other endpoint, we must enable/disable
our video control endpoint based on calls to our
->set_alt() method.

Signed-off-by: Felipe Balbi <balbi-***@public.gmane.org>
---
drivers/usb/gadget/function/f_uvc.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 4138ad5..413a09f 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -297,6 +297,19 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
if (alt)
return -EINVAL;

+ if (uvc->control_ep->driver_data) {
+ INFO(cdev, "reset UVC Control\n");
+ usb_ep_disable(uvc->control_ep);
+ uvc->control_ep->driver_data = NULL;
+ }
+
+ if (!uvc->control_ep->desc)
+ if (config_ep_by_speed(cdev->gadget, f, uvc->control_ep))
+ return -EINVAL;
+
+ usb_ep_enable(uvc->control_ep);
+ uvc->control_ep->driver_data = uvc;
+
if (uvc->state == UVC_STATE_DISCONNECTED) {
memset(&v4l2_event, 0, sizeof(v4l2_event));
v4l2_event.type = UVC_EVENT_CONNECT;

[Felipe Balbi]

Endpoint descriptors should pass wMaxPacketSize. Note
that this also fixes USB20CV Other Speed Endpoint
Descriptor Tests.

Signed-off-by: Felipe Balbi <balbi-***@public.gmane.org>
---
drivers/usb/gadget/function/f_uac2.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
index a5a27a5..fa51118 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -772,6 +772,7 @@ struct usb_endpoint_descriptor fs_epout_desc = {

.bEndpointAddress = USB_DIR_OUT,
.bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
+ .wMaxPacketSize = cpu_to_le16(1023),
.bInterval = 1,
};

@@ -780,6 +781,7 @@ struct usb_endpoint_descriptor hs_epout_desc = {
.bDescriptorType = USB_DT_ENDPOINT,

.bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
+ .wMaxPacketSize = cpu_to_le16(1024),
.bInterval = 4,
};

@@ -847,6 +849,7 @@ struct usb_endpoint_descriptor fs_epin_desc = {

.bEndpointAddress = USB_DIR_IN,
.bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
+ .wMaxPacketSize = cpu_to_le16(1023),
.bInterval = 1,
};

@@ -855,6 +858,7 @@ struct usb_endpoint_descriptor hs_epin_desc = {
.bDescriptorType = USB_DT_ENDPOINT,

.bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
+ .wMaxPacketSize = cpu_to_le16(1024),
.bInterval = 4,
};

[Felipe Balbi]

devices are required to provide a release method. This
patch fixes the following WARN():

[ 42.611159] ------------[ cut here ]------------
[ 42.616025] WARNING: CPU: 0 PID: 1453 at drivers/base/core.c:250 device_release+0x94/0xa0()
[ 42.624820] Device 'snd_uac2.0' does not have a release() function, it is broken and must be fixed.
[ 42.634328] Modules linked in: usb_f_uac2 g_audio(-) libcomposite configfs xhci_hcd snd_soc_davinci_mcasp snd_soc_edma snd_soc_tlv320aic3x snd_soc_omap snd_soc_evm snd_soc_core dwc3 snd_compress omapdrm snd_pcm_dmaengine snd_pcm snd_timer snd fb_sys_fops lis3lv02d_i2c matrix_keypad dwc3_omap lis3lv02d panel_dpi input_polldev soundcore
[ 42.665687] CPU: 0 PID: 1453 Comm: modprobe Tainted: G D 3.17.0-rc6-00448-g9f3d0ec-dirty #188
[ 42.675756] [<c0017338>] (unwind_backtrace) from [<c0012fdc>] (show_stack+0x20/0x24)
[ 42.683911] [<c0012fdc>] (show_stack) from [<c0647fbc>] (dump_stack+0x8c/0xa4)
[ 42.691526] [<c0647fbc>] (dump_stack) from [<c0049950>] (warn_slowpath_common+0x7c/0xa0)
[ 42.700004] [<c0049950>] (warn_slowpath_common) from [<c00499b4>] (warn_slowpath_fmt+0x40/0x48)
[ 42.709194] [<c00499b4>] (warn_slowpath_fmt) from [<c0405f7c>] (device_release+0x94/0xa0)
[ 42.717794] [<c0405f7c>] (device_release) from [<c032e8e8>] (kobject_cleanup+0x4c/0x7c)
[ 42.726189] [<c032e8e8>] (kobject_cleanup) from [<c032e7c8>] (kobject_put+0x60/0x90)
[ 42.734316] [<c032e7c8>] (kobject_put) from [<c0406320>] (put_device+0x24/0x28)
[ 42.741995] [<c0406320>] (put_device) from [<c040c008>] (platform_device_unregister+0x2c/0x30)
[ 42.751061] [<c040c008>] (platform_device_unregister) from [<bf2b6b70>] (afunc_unbind+0x2c/0x68 [usb_f_uac2])
[ 42.761523] [<bf2b6b70>] (afunc_unbind [usb_f_uac2]) from [<bf29dbec>] (remove_config.isra.8+0xe8/0x100 [libcomposite])
[ 42.772868] [<bf29dbec>] (remove_config.isra.8 [libcomposite]) from [<bf29f9a4>] (__composite_unbind+0x48/0xb0 [libcomposite])
[ 42.784855] [<bf29f9a4>] (__composite_unbind [libcomposite]) from [<bf29fa28>] (composite_unbind+0x1c/0x20 [libcomposite])
[ 42.796446] [<bf29fa28>] (composite_unbind [libcomposite]) from [<c04d229c>] (usb_gadget_remove_driver+0x78/0xb0)
[ 42.807224] [<c04d229c>] (usb_gadget_remove_driver) from [<c04d2348>] (usb_gadget_unregister_driver+0x74/0xb8)
[ 42.817742] [<c04d2348>] (usb_gadget_unregister_driver) from [<bf29db00>] (usb_composite_unregister+0x1c/0x20 [libcomposite])
[ 42.829632] [<bf29db00>] (usb_composite_unregister [libcomposite]) from [<bf2b1084>] (audio_driver_exit+0x14/0x1c [g_audio])
[ 42.841430] [<bf2b1084>] (audio_driver_exit [g_audio]) from [<c00c0fe0>] (SyS_delete_module+0x120/0x1b0)
[ 42.851415] [<c00c0fe0>] (SyS_delete_module) from [<c000ed40>] (ret_fast_syscall+0x0/0x48)
[ 42.860075] ---[ end trace bb22e678d8d6db7b ]---
***@saruman:~#

Signed-off-by: Felipe Balbi <balbi-***@public.gmane.org>
---
drivers/usb/gadget/function/f_uac2.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
index 1146f4d..9296e59 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -512,6 +512,11 @@ static int snd_uac2_remove(struct platform_device *pdev)
return 0;
}

+static void snd_uac2_release(struct device *dev)
+{
+ dev_dbg(dev, "releasing '%s'\n", dev_name(dev));
+}
+
static int alsa_uac2_init(struct audio_dev *agdev)
{
struct snd_uac2_chip *uac2 = &agdev->uac2;
@@ -523,6 +528,7 @@ static int alsa_uac2_init(struct audio_dev *agdev)

uac2->pdev.id = 0;
uac2->pdev.name = uac2_name;
+ uac2->pdev.dev.release = snd_uac2_release;

/* Register snd_uac2 driver */
err = platform_driver_register(&uac2->pdrv);

[Felipe Balbi]

when our ->disable() method is called, we must
make sure to teardown all our resources, including
endpoints.

Signed-off-by: Felipe Balbi <balbi-***@public.gmane.org>
---
drivers/usb/gadget/function/f_uvc.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 413a09f..945b3bd 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -390,6 +390,16 @@ uvc_function_disable(struct usb_function *f)
v4l2_event_queue(uvc->vdev, &v4l2_event);

uvc->state = UVC_STATE_DISCONNECTED;
+
+ if (uvc->video.ep->driver_data) {
+ usb_ep_disable(uvc->video.ep);
+ uvc->video.ep->driver_data = NULL;
+ }
+
+ if (uvc->control_ep->driver_data) {
+ usb_ep_disable(uvc->control_ep);
+ uvc->control_ep->driver_data = NULL;
+ }
}

/* --------------------------------------------------------------------------

[Felipe Balbi]

On USB20CV's Interface Descriptor Test, a series
of SetInterface/GetInterface requests are issued
and gadget driver is required to always return
correct alternate setting.

In one step of the test, g_serial with f_obex
was returning the wrong value (1 instead of 0).

In order to fix this, we will now hold currently
selected alternate setting inside our struct f_obex
and just return that from our ->get_alt()
implementation.

Note that his also simplifies the code a bit.

Signed-off-by: Felipe Balbi <balbi-***@public.gmane.org>
---
drivers/usb/gadget/function/f_obex.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/function/f_obex.c b/drivers/usb/gadget/function/f_obex.c
index 5f40080..1a1a490 100644
--- a/drivers/usb/gadget/function/f_obex.c
+++ b/drivers/usb/gadget/function/f_obex.c
@@ -35,6 +35,7 @@ struct f_obex {
struct gserial port;
u8 ctrl_id;
u8 data_id;
+ u8 cur_alt;
u8 port_num;
u8 can_activate;
};
@@ -235,6 +236,8 @@ static int obex_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
} else
goto fail;

+ obex->cur_alt = alt;
+
return 0;

fail:
@@ -245,10 +248,7 @@ static int obex_get_alt(struct usb_function *f, unsigned intf)
{
struct f_obex *obex = func_to_obex(f);

- if (intf == obex->ctrl_id)
- return 0;

[Felipe Balbi]

if our list of requests is empty, return early.

There's really nothing to be done in case our
request list is empty anyway because the only
situation where we our list is empty, is when
we're transferring ZLPs.

Signed-off-by: Felipe Balbi <balbi-***@public.gmane.org>
---
drivers/usb/dwc3/ep0.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index ae6b575..a47cc1e 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -789,9 +789,6 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,

dwc->ep0_next_event = DWC3_EP0_NRDY_STATUS;

- r = next_request(&ep0->request_list);
- ur = &r->request;

[Felipe Balbi]

According to USB 2.0 ECN Errata for Link Power
Management (USB2-LPM-Errata-final.pdf), BESL
must be enabled if LPM is enabled.

This helps with USB30CV TD 9.21 LPM L1
Suspend Resume Test.

Signed-off-by: Felipe Balbi <balbi-***@public.gmane.org>
---
drivers/usb/gadget/composite.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index a8c18df..f6a51fd 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -560,7 +560,7 @@ static int bos_desc(struct usb_composite_dev *cdev)
usb_ext->bLength = USB_DT_USB_EXT_CAP_SIZE;
usb_ext->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
usb_ext->bDevCapabilityType = USB_CAP_TYPE_EXT;
- usb_ext->bmAttributes = cpu_to_le32(USB_LPM_SUPPORT);
+ usb_ext->bmAttributes = cpu_to_le32(USB_LPM_SUPPORT | USB_BESL_SUPPORT);

/*
* The Superspeed USB Capability descriptor shall be implemented by all

[Felipe Balbi]

If our alternate setting has been selected, we must
return that on a subsequent Get Interface request
even if we're not streaming.

Signed-off-by: Felipe Balbi <balbi-***@public.gmane.org>
---
drivers/usb/gadget/function/f_uvc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index e00e8b7..4138ad5 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -279,7 +279,7 @@ uvc_function_get_alt(struct usb_function *f, unsigned interface)
else if (interface != uvc->streaming_intf)
return -EINVAL;
else
- return uvc->state == UVC_STATE_STREAMING ? 1 : 0;
+ return uvc->video.ep->driver_data ? 1 : 0;
}

static int

[Felipe Balbi]

According to Section 8.5.3.2 of the USB 2.0 specification,
a USB device must terminate a Data Phase with either a
short packet or a ZLP (if the previous transfer was
a multiple of wMaxPacketSize).

For reference, here's what the USB 2.0 specification, section
8.5.3.2 says:

"
8.5.3.2 Variable-length Data Stage

A control pipe may have a variable-length data phase
in which the host requests more data than is contained
in the specified data structure. When all of the data
structure is returned to the host, the function should
indicate that the Data stage is ended by returning a
packet that is shorter than the MaxPacketSize for the
pipe. If the data structure is an exact multiple of
wMaxPacketSize for the pipe, the function will return
a zero-length packet to indicate the end of the Data
stage.
"

Signed-off-by: Felipe Balbi <balbi-***@public.gmane.org>
---
drivers/usb/dwc3/ep0.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index a47cc1e..0a34e71 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -828,12 +828,18 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,

dwc3_ep0_stall_and_restart(dwc);
} else {
- /*
- * handle the case where we have to send a zero packet. This
- * seems to be case when req.length > maxpacket. Could it be?
- */
- if (r)
- dwc3_gadget_giveback(ep0, r, 0);
+ dwc3_gadget_giveback(ep0, r, 0);
+
+ if (IS_ALIGNED(ur->length, ep0->endpoint.maxpacket)) {
+ int ret;
+
+ dwc->ep0_next_event = DWC3_EP0_COMPLETE;
+
+ ret = dwc3_ep0_start_trans(dwc, epnum,
+ dwc->ctrl_req_addr, 0,
+ DWC3_TRBCTL_CONTROL_DATA);
+ WARN_ON(ret < 0);
+ }
}
}

[David Laight]

From: Felipe Balbi

Post by Felipe Balbi
According to Section 8.5.3.2 of the USB 2.0 specification,
a USB device must terminate a Data Phase with either a
short packet or a ZLP (if the previous transfer was
a multiple of wMaxPacketSize).
For reference, here's what the USB 2.0 specification, section
"
8.5.3.2 Variable-length Data Stage
A control pipe may have a variable-length data phase
in which the host requests more data than is contained
in the specified data structure. When all of the data
structure is returned to the host, the function should
indicate that the Data stage is ended by returning a
packet that is shorter than the MaxPacketSize for the
pipe. If the data structure is an exact multiple of
wMaxPacketSize for the pipe, the function will return
a zero-length packet to indicate the end of the Data
stage.
"

If that is the same as my understanding of the USB3 spec then the
requirement for a ZLP isn't unconditional.

In particular if the data phase isn't variable length then a ZLP
must not be added.

David

Post by Felipe Balbi
---
drivers/usb/dwc3/ep0.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index a47cc1e..0a34e71 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -828,12 +828,18 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
dwc3_ep0_stall_and_restart(dwc);
} else {
- /*
- * handle the case where we have to send a zero packet. This
- * seems to be case when req.length > maxpacket. Could it be?
- */
- if (r)
- dwc3_gadget_giveback(ep0, r, 0);
+ dwc3_gadget_giveback(ep0, r, 0);
+
+ if (IS_ALIGNED(ur->length, ep0->endpoint.maxpacket)) {
+ int ret;
+
+ dwc->ep0_next_event = DWC3_EP0_COMPLETE;
+
+ ret = dwc3_ep0_start_trans(dwc, epnum,
+ dwc->ctrl_req_addr, 0,
+ DWC3_TRBCTL_CONTROL_DATA);
+ WARN_ON(ret < 0);
+ }
}
}
--
2.1.0.GIT
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

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

[Paul Zimmerman]

Sent: Monday, October 20, 2014 2:48 AM
From: Felipe Balbi

Post by Felipe Balbi
According to Section 8.5.3.2 of the USB 2.0 specification,
a USB device must terminate a Data Phase with either a
short packet or a ZLP (if the previous transfer was
a multiple of wMaxPacketSize).
For reference, here's what the USB 2.0 specification, section
"
8.5.3.2 Variable-length Data Stage
A control pipe may have a variable-length data phase
in which the host requests more data than is contained
in the specified data structure. When all of the data
structure is returned to the host, the function should
indicate that the Data stage is ended by returning a
packet that is shorter than the MaxPacketSize for the
pipe. If the data structure is an exact multiple of
wMaxPacketSize for the pipe, the function will return
a zero-length packet to indicate the end of the Data
stage.
"

If that is the same as my understanding of the USB3 spec then the
requirement for a ZLP isn't unconditional.
In particular if the data phase isn't variable length then a ZLP
must not be added.

Also, in the USB 3.0 spec, the corresponding section has been modified
a bit. The last sentence has been changed to this:

"Note that if the amount of data in the data structure that is returned
to the host *is less than the amount requested by the host* and is an
exact multiple of maximum packet size then a control endpoint shall send
a zero length DP to terminate the data stage." (emphasis mine)

So I think you also need to take into account the wLength field of the
request, and only send the ZLP if the amount of data returned is less
than wLength.

[Alan Stern]

Post by Paul Zimmerman

Sent: Monday, October 20, 2014 2:48 AM
From: Felipe Balbi

Post by Felipe Balbi
According to Section 8.5.3.2 of the USB 2.0 specification,
a USB device must terminate a Data Phase with either a
short packet or a ZLP (if the previous transfer was
a multiple of wMaxPacketSize).
For reference, here's what the USB 2.0 specification, section
"
8.5.3.2 Variable-length Data Stage
A control pipe may have a variable-length data phase
in which the host requests more data than is contained
in the specified data structure. When all of the data
structure is returned to the host, the function should
indicate that the Data stage is ended by returning a
packet that is shorter than the MaxPacketSize for the
pipe. If the data structure is an exact multiple of
wMaxPacketSize for the pipe, the function will return
a zero-length packet to indicate the end of the Data
stage.
"

If that is the same as my understanding of the USB3 spec then the
requirement for a ZLP isn't unconditional.
In particular if the data phase isn't variable length then a ZLP
must not be added.

Also, in the USB 3.0 spec, the corresponding section has been modified
"Note that if the amount of data in the data structure that is returned
to the host *is less than the amount requested by the host* and is an
exact multiple of maximum packet size then a control endpoint shall send
a zero length DP to terminate the data stage." (emphasis mine)
So I think you also need to take into account the wLength field of the
request, and only send the ZLP if the amount of data returned is less
than wLength.

That's right, and it's true for USB-2 as well. A ZLP is needed only in
cases where the host otherwise wouldn't know the transfer is over,
i.e., when the transfer length is a nonzero multiple of the maxpacket
size and is smaller than wLength.

It's not clear what a "variable length" control transfer is supposed to
be. I guess it means that sometimes the device will send back less
than wLength bytes of data.

Alan Stern

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

[David Laight]

From: Alan Stern

Post by Alan Stern

Post by Paul Zimmerman

Sent: Monday, October 20, 2014 2:48 AM
From: Felipe Balbi

Post by Felipe Balbi
According to Section 8.5.3.2 of the USB 2.0 specification,
a USB device must terminate a Data Phase with either a
short packet or a ZLP (if the previous transfer was
a multiple of wMaxPacketSize).
For reference, here's what the USB 2.0 specification, section
"
8.5.3.2 Variable-length Data Stage
A control pipe may have a variable-length data phase
in which the host requests more data than is contained
in the specified data structure. When all of the data
structure is returned to the host, the function should
indicate that the Data stage is ended by returning a
packet that is shorter than the MaxPacketSize for the
pipe. If the data structure is an exact multiple of
wMaxPacketSize for the pipe, the function will return
a zero-length packet to indicate the end of the Data
stage.
"

If that is the same as my understanding of the USB3 spec then the
requirement for a ZLP isn't unconditional.
In particular if the data phase isn't variable length then a ZLP
must not be added.

Also, in the USB 3.0 spec, the corresponding section has been modified
"Note that if the amount of data in the data structure that is returned
to the host *is less than the amount requested by the host* and is an
exact multiple of maximum packet size then a control endpoint shall send
a zero length DP to terminate the data stage." (emphasis mine)
So I think you also need to take into account the wLength field of the
request, and only send the ZLP if the amount of data returned is less
than wLength.

That's right, and it's true for USB-2 as well. A ZLP is needed only in
cases where the host otherwise wouldn't know the transfer is over,
i.e., when the transfer length is a nonzero multiple of the maxpacket
size and is smaller than wLength.
It's not clear what a "variable length" control transfer is supposed to
be. I guess it means that sometimes the device will send back less
than wLength bytes of data.

I take it as being one where the amount of data returned isn't known
by the host when it performs the 'read' request.

So the response to a 'disk read' request is known and a ZLP isn't required
(even though the transfer request is likely to be a multiple of the packet size).

The length that matters is that of the buffer(s) supplied by the receiving driver.
From the USB driver's point of view, only the 'application' (another driver)
knows whether the next receive message is 'variable length', so the onus
has to be on the 'application' sending the data to say whether it needs a ZLP.

Has anyone fixed xhci to send ZLP yet ?

David



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

[Anton Tikhomirov]

Hi,

Post by Paul Zimmerman

Post by Paul Zimmerman

Sent: Monday, October 20, 2014 2:48 AM
From: Felipe Balbi

Post by Felipe Balbi
According to Section 8.5.3.2 of the USB 2.0 specification,
a USB device must terminate a Data Phase with either a
short packet or a ZLP (if the previous transfer was
a multiple of wMaxPacketSize).
For reference, here's what the USB 2.0 specification, section
"
8.5.3.2 Variable-length Data Stage
A control pipe may have a variable-length data phase
in which the host requests more data than is contained
in the specified data structure. When all of the data
structure is returned to the host, the function should
indicate that the Data stage is ended by returning a
packet that is shorter than the MaxPacketSize for the
pipe. If the data structure is an exact multiple of
wMaxPacketSize for the pipe, the function will return
a zero-length packet to indicate the end of the Data
stage.
"

If that is the same as my understanding of the USB3 spec then the
requirement for a ZLP isn't unconditional.
In particular if the data phase isn't variable length then a ZLP
must not be added.

Also, in the USB 3.0 spec, the corresponding section has been

modified

Post by Paul Zimmerman
"Note that if the amount of data in the data structure that is

returned

Post by Paul Zimmerman
to the host *is less than the amount requested by the host* and is an
exact multiple of maximum packet size then a control endpoint shall

send

Post by Paul Zimmerman
a zero length DP to terminate the data stage." (emphasis mine)
So I think you also need to take into account the wLength field of

the

Post by Paul Zimmerman
request, and only send the ZLP if the amount of data returned is less
than wLength.

That's right, and it's true for USB-2 as well. A ZLP is needed only in
cases where the host otherwise wouldn't know the transfer is over,
i.e., when the transfer length is a nonzero multiple of the maxpacket
size and is smaller than wLength.

Shall we use/check struct usb_request's zero flag for this?

Post by Paul Zimmerman
It's not clear what a "variable length" control transfer is supposed to
be. I guess it means that sometimes the device will send back less
than wLength bytes of data.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

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

[Alan Stern]

Post by Anton Tikhomirov

Post by Alan Stern
That's right, and it's true for USB-2 as well. A ZLP is needed only in
cases where the host otherwise wouldn't know the transfer is over,
i.e., when the transfer length is a nonzero multiple of the maxpacket
size and is smaller than wLength.

Shall we use/check struct usb_request's zero flag for this?

Of course; we have to. There's no other way for the UDC driver to know
whether the transfer is shorter than the host expects.

Alan Stern

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

[Felipe Balbi]

From: Alan Cox <alan-VuQAYsv1563Yd54FQh9/***@public.gmane.org>

The device controller is the same but it has different PCI ID. Add this new
ID to the driver's list of supported IDs.

Signed-off-by: Alan Cox <alan-VuQAYsv1563Yd54FQh9/***@public.gmane.org>
Signed-off-by: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/***@public.gmane.org>
Signed-off-by: Heikki Krogerus <heikki.krogerus-VuQAYsv1563Yd54FQh9/***@public.gmane.org>
Signed-off-by: Felipe Balbi <balbi-***@public.gmane.org>
---
drivers/usb/dwc3/dwc3-pci.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index 436fb08..a36cf66 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -30,6 +30,7 @@
#define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3 0xabcd
#define PCI_DEVICE_ID_INTEL_BYT 0x0f37
#define PCI_DEVICE_ID_INTEL_MRFLD 0x119e
+#define PCI_DEVICE_ID_INTEL_BSW 0x22B7

struct dwc3_pci {
struct device *dev;
@@ -181,6 +182,7 @@ static const struct pci_device_id dwc3_pci_id_table[] = {
PCI_DEVICE(PCI_VENDOR_ID_SYNOPSYS,
PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3),
},
+ { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BSW), },
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BYT), },
{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MRFLD), },
{ } /* Terminating Entry */

[Felipe Balbi]

From: David Cohen <david.a.cohen-VuQAYsv1563Yd54FQh9/***@public.gmane.org>

The commit '2e4c7553cd usb: gadget: f_fs: add aio support' broke the
quirk implemented to align buffer size to maxpacketsize on out endpoint.
As result, functionfs does not work on Intel platforms using dwc3 driver
(i.e. Bay Trail and Merrifield). This patch fixes the issue.

This code is based on a previous Qiuxu's patch.

Fixes: 2e4c7553cd (usb: gadget: f_fs: add aio support)
Cc: <stable-***@public.gmane.org> # v3.16+
Signed-off-by: David Cohen <david.a.cohen-VuQAYsv1563Yd54FQh9/***@public.gmane.org>
Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo-***@public.gmane.org>
Acked-by: Michal Nazarewicz <mina86-deATy8a+UHjQT0dZR+***@public.gmane.org>
Signed-off-by: Felipe Balbi <balbi-***@public.gmane.org>
---
drivers/usb/gadget/function/f_fs.c | 40 ++++++++++++++++++++++++++++++++------
1 file changed, 34 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 12dbdaf..63314ed 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -647,15 +647,26 @@ static void ffs_user_copy_worker(struct work_struct *work)
if (io_data->read && ret > 0) {
int i;
size_t pos = 0;
+
+ /*
+ * Since req->length may be bigger than io_data->len (after
+ * being rounded up to maxpacketsize), we may end up with more
+ * data then user space has space for.
+ */
+ ret = min_t(int, ret, io_data->len);
+
use_mm(io_data->mm);
for (i = 0; i < io_data->nr_segs; i++) {
+ size_t len = min_t(size_t, ret - pos,
+ io_data->iovec[i].iov_len);
+ if (!len)
+ break;
if (unlikely(copy_to_user(io_data->iovec[i].iov_base,
- &io_data->buf[pos],
- io_data->iovec[i].iov_len))) {
+ &io_data->buf[pos], len))) {
ret = -EFAULT;
break;
}
- pos += io_data->iovec[i].iov_len;
+ pos += len;
}
unuse_mm(io_data->mm);
}
@@ -687,7 +698,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
struct ffs_epfile *epfile = file->private_data;
struct ffs_ep *ep;
char *data = NULL;
- ssize_t ret, data_len;
+ ssize_t ret, data_len = -EINVAL;
int halt;

/* Are we still active? */
@@ -787,13 +798,30 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
/* Fire the request */
struct usb_request *req;

+ /*
+ * Sanity Check: even though data_len can't be used
+ * uninitialized at the time I write this comment, some
+ * compilers complain about this situation.
+ * In order to keep the code clean from warnings, data_len is
+ * being initialized to -EINVAL during its declaration, which
+ * means we can't rely on compiler anymore to warn no future
+ * changes won't result in data_len being used uninitialized.
+ * For such reason, we're adding this redundant sanity check
+ * here.
+ */
+ if (unlikely(data_len == -EINVAL)) {
+ WARN(1, "%s: data_len == -EINVAL\n", __func__);
+ ret = -EINVAL;
+ goto error_lock;
+ }
+
if (io_data->aio) {
req = usb_ep_alloc_request(ep->ep, GFP_KERNEL);
if (unlikely(!req))
goto error_lock;

req->buf = data;
- req->length = io_data->len;
+ req->length = data_len;

io_data->buf = data;
io_data->ep = ep->ep;
@@ -815,7 +843,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)

req = ep->req;
req->buf = data;
- req->length = io_data->len;
+ req->length = data_len;

req->context = &done;
req->complete = ffs_epfile_io_complete;

[Felipe Balbi]

From: Roger Quadros <rogerq-***@public.gmane.org>

This reverts commit 02dae36aa649a66c5c6181157ddd806e7b4913fc.

That commit is bogus in two ways:

1) There's no way dwc3-omap's ->suspend() can cause any effect
on xhci's ->suspend(). Linux device driver model guarantees
that a parent's ->suspend() will only be called after all
children are suspended. dwc3-omap is the parent of the
parent of xhci.

2) When implementing Deep Sleep states where context is lost,
USBOTGSS_IRQ0 register, well, looses context so we
_must_ rewrite it otherwise core IRQs will never be
reenabled and USB will appear to be dead.

Fixes: 02dae36 (usb: dwc3: dwc3-omap: Disable/Enable only
wrapper interrupts in prepare/complete)
Cc: <stable-***@public.gmane.org> # v3.17
Cc: George Cherian <george.cherian-***@public.gmane.org>
Signed-off-by: Roger Quadros <rogerq-***@public.gmane.org>
Signed-off-by: Felipe Balbi <balbi-***@public.gmane.org>
---
drivers/usb/dwc3/dwc3-omap.c | 15 ++-------------
1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index 2f537d5..a0aa9f3 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -597,7 +597,7 @@ static int dwc3_omap_prepare(struct device *dev)
{
struct dwc3_omap *omap = dev_get_drvdata(dev);

- dwc3_omap_write_irqmisc_set(omap, 0x00);
+ dwc3_omap_disable_irqs(omap);

return 0;
}
@@ -605,19 +605,8 @@ static int dwc3_omap_prepare(struct device *dev)
static void dwc3_omap_complete(struct device *dev)
{
struct dwc3_omap *omap = dev_get_drvdata(dev);
- u32 reg;

- reg = (USBOTGSS_IRQMISC_OEVT |
- USBOTGSS_IRQMISC_DRVVBUS_RISE |
- USBOTGSS_IRQMISC_CHRGVBUS_RISE |
- USBOTGSS_IRQMISC_DISCHRGVBUS_RISE |
- USBOTGSS_IRQMISC_IDPULLUP_RISE |
- USBOTGSS_IRQMISC_DRVVBUS_FALL |
- USBOTGSS_IRQMISC_CHRGVBUS_FALL |
- USBOTGSS_IRQMISC_DISCHRGVBUS_FALL |
- USBOTGSS_IRQMISC_IDPULLUP_FALL);

[Felipe Balbi]

A request allocated from e.g. ep1out cannot
be queued to any other endpoint. This bug has
been in f_loopback at least since mid-2011 and
since nobody has really screamed about it, we're
not backporting to any stable kernels.

Debugged with MUSB since that's the only controller
which complains about this case.

Signed-off-by: Felipe Balbi <balbi-***@public.gmane.org>
---
drivers/usb/gadget/function/f_loopback.c | 87 +++++++++++++++-----------------
1 file changed, 42 insertions(+), 45 deletions(-)

diff --git a/drivers/usb/gadget/function/f_loopback.c b/drivers/usb/gadget/function/f_loopback.c
index bf04389..298b461 100644
--- a/drivers/usb/gadget/function/f_loopback.c
+++ b/drivers/usb/gadget/function/f_loopback.c
@@ -253,22 +253,13 @@ static void loopback_complete(struct usb_ep *ep, struct usb_request *req)

case 0: /* normal completion? */
if (ep == loop->out_ep) {
- /* loop this OUT packet back IN to the host */
req->zero = (req->actual < req->length);
req->length = req->actual;
- status = usb_ep_queue(loop->in_ep, req, GFP_ATOMIC);
- if (status == 0)
- return;
-
- /* "should never get here" */
- ERROR(cdev, "can't loop %s to %s: %d\n",
- ep->name, loop->in_ep->name,
- status);
}

/* queue the buffer for some later OUT packet */
req->length = buflen;
- status = usb_ep_queue(loop->out_ep, req, GFP_ATOMIC);
+ status = usb_ep_queue(ep, req, GFP_ATOMIC);
if (status == 0)
return;

@@ -308,60 +299,66 @@ static inline struct usb_request *lb_alloc_ep_req(struct usb_ep *ep, int len)
return alloc_ep_req(ep, len, buflen);
}

-static int
-enable_loopback(struct usb_composite_dev *cdev, struct f_loopback *loop)
+static int enable_endpoint(struct usb_composite_dev *cdev, struct f_loopback *loop,
+ struct usb_ep *ep)
{
- int result = 0;
- struct usb_ep *ep;
struct usb_request *req;
unsigned i;
+ int result;

- /* one endpoint writes data back IN to the host */
- ep = loop->in_ep;
+ /*
+ * one endpoint writes data back IN to the host while another endpoint
+ * just reads OUT packets
+ */
result = config_ep_by_speed(cdev->gadget, &(loop->function), ep);
if (result)
- return result;
+ goto fail0;
result = usb_ep_enable(ep);
if (result < 0)
- return result;
- ep->driver_data = loop;
-
- /* one endpoint just reads OUT packets */
- ep = loop->out_ep;
- result = config_ep_by_speed(cdev->gadget, &(loop->function), ep);
- if (result)
goto fail0;
-
- result = usb_ep_enable(ep);
- if (result < 0) {
-fail0:
- ep = loop->in_ep;
- usb_ep_disable(ep);
- ep->driver_data = NULL;
- return result;
- }
ep->driver_data = loop;

- /* allocate a bunch of read buffers and queue them all at once.
+ /*
+ * allocate a bunch of read buffers and queue them all at once.
* we buffer at most 'qlen' transfers; fewer if any need more
* than 'buflen' bytes each.
*/
for (i = 0; i < qlen && result == 0; i++) {
req = lb_alloc_ep_req(ep, 0);
- if (req) {
- req->complete = loopback_complete;
- result = usb_ep_queue(ep, req, GFP_ATOMIC);
- if (result)
- ERROR(cdev, "%s queue req --> %d\n",
- ep->name, result);
- } else {
- usb_ep_disable(ep);
- ep->driver_data = NULL;
- result = -ENOMEM;
- goto fail0;
+ if (!req)
+ goto fail1;
+
+ req->complete = loopback_complete;
+ result = usb_ep_queue(ep, req, GFP_ATOMIC);
+ if (result) {
+ ERROR(cdev, "%s queue req --> %d\n",
+ ep->name, result);
+ goto fail1;
}
}

+ return 0;
+
+fail1:
+ usb_ep_disable(ep);
+
+fail0:
+ return result;
+}
+
+static int
+enable_loopback(struct usb_composite_dev *cdev, struct f_loopback *loop)
+{
+ int result = 0;
+
+ result = enable_endpoint(cdev, loop, loop->in_ep);
+ if (result)
+ return result;
+
+ result = enable_endpoint(cdev, loop, loop->out_ep);
+ if (result)
+ return result;
+
DBG(cdev, "%s enabled\n", loop->function.name);
return result;
}

[Felipe Balbi]

From: Sebastian Andrzej Siewior <bigeasy-***@public.gmane.org>

So testing managed to configure musb in DMA mode but not load the
matching cppi41 driver for DMA. This results in

|musb-hdrc musb-hdrc.0.auto: Failed to request rx1.
|musb-hdrc musb-hdrc.0.auto: musb_init_controller failed with status -517
|platform musb-hdrc.0.auto: Driver musb-hdrc requests probe deferral

which is "okay". Once the driver is loaded we re-try probing and
everyone is happy. Until then if you try suspend say
echo mem > /sys/power/state
then you go boom

|Unable to handle kernel NULL pointer dereference at virtual address 000003a4
|pgd = cf50c000
|[000003a4] *pgd=8f6a3831, *pte=00000000, *ppte=00000000
|Internal error: Oops: 17 [#1] ARM
|PC is at dsps_suspend+0x18/0x9c [musb_dsps]
|LR is at dsps_suspend+0x18/0x9c [musb_dsps]
|pc : [<bf08e268>] lr : [<bf08e268>] psr: a0000013
|sp : cbd97e00 ip : c0af4394 fp : 00000000
|r10: c0831d90 r9 : 00000002 r8 : cf6da410
|r7 : c03ba4dc r6 : bf08f224 r5 : 00000000 r4 : cbc5fcd0
|r3 : bf08e250 r2 : bf08f264 r1 : cf6da410 r0 : 00000000
|[<bf08e268>] (dsps_suspend [musb_dsps]) from [<c03ba508>] (platform_pm_suspend+0x2c/0x54)
|Code: e1a04000 e9900041 e2800010 eb4caa8e (e59053a4)

because platform_get_drvdata(glue->musb) returns a NULL pointer as long as the
device is not fully probed.

Tested-by: George Cherian <george.cherian-***@public.gmane.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy-***@public.gmane.org>
Signed-off-by: Felipe Balbi <balbi-***@public.gmane.org>
---
drivers/usb/musb/musb_dsps.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index b18f8d5..48bc09e 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -868,9 +868,15 @@ static int dsps_suspend(struct device *dev)
struct dsps_glue *glue = dev_get_drvdata(dev);
const struct dsps_musb_wrapper *wrp = glue->wrp;
struct musb *musb = platform_get_drvdata(glue->musb);
- void __iomem *mbase = musb->ctrl_base;
+ void __iomem *mbase;

del_timer_sync(&glue->timer);
+
+ if (!musb)
+ /* This can happen if the musb device is in -EPROBE_DEFER */
+ return 0;
+
+ mbase = musb->ctrl_base;
glue->context.control = dsps_readl(mbase, wrp->control);
glue->context.epintr = dsps_readl(mbase, wrp->epintr_set);
glue->context.coreintr = dsps_readl(mbase, wrp->coreintr_set);
@@ -887,8 +893,12 @@ static int dsps_resume(struct device *dev)
struct dsps_glue *glue = dev_get_drvdata(dev);
const struct dsps_musb_wrapper *wrp = glue->wrp;
struct musb *musb = platform_get_drvdata(glue->musb);
- void __iomem *mbase = musb->ctrl_base;
+ void __iomem *mbase;
+
+ if (!musb)
+ return 0;

+ mbase = musb->ctrl_base;
dsps_writel(mbase, wrp->control, glue->context.control);
dsps_writel(mbase, wrp->epintr_set, glue->context.epintr);
dsps_writel(mbase, wrp->coreintr_set, glue->context.coreintr);

[Felipe Balbi]

Currently, there's no guarantee that udc->driver
will be valid when using soft_connect sysfs
interface. In fact, we can very easily trigger
a NULL pointer dereference by trying to disconnect
when a gadget driver isn't loaded.

Fix this bug:

~# echo disconnect > soft_connect
[ 33.685743] Unable to handle kernel NULL pointer dereference at virtual address 00000014
[ 33.694221] pgd = ed0cc000
[ 33.697174] [00000014] *pgd=ae351831, *pte=00000000, *ppte=00000000
[ 33.703766] Internal error: Oops: 17 [#1] SMP ARM
[ 33.708697] Modules linked in: xhci_plat_hcd xhci_hcd snd_soc_davinci_mcasp snd_soc_tlv320aic3x snd_soc_edma snd_soc_omap snd_soc_evm snd_soc_core dwc3 snd_compress snd_pcm_dmaengine snd_pcm snd_timer snd lis3lv02d_i2c matrix_keypad lis3lv02d dwc3_omap input_polldev soundcore
[ 33.734372] CPU: 0 PID: 1457 Comm: bash Not tainted 3.17.0-09740-ga93416e-dirty #345
[ 33.742457] task: ee71ce00 ti: ee68a000 task.ti: ee68a000
[ 33.748116] PC is at usb_udc_softconn_store+0xa4/0xec
[ 33.753416] LR is at mark_held_locks+0x78/0x90
[ 33.758057] pc : [<c04df128>] lr : [<c00896a4>] psr: 20000013
[ 33.758057] sp : ee68bec8 ip : c0c00008 fp : ee68bee4
[ 33.770050] r10: ee6b394c r9 : ee68bf80 r8 : ee6062c0
[ 33.775508] r7 : 00000000 r6 : ee6062c0 r5 : 0000000b r4 : ee739408
[ 33.782346] r3 : 00000000 r2 : 00000000 r1 : ee71d390 r0 : ee664170
[ 33.789168] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
[ 33.796636] Control: 10c5387d Table: ad0cc059 DAC: 00000015
[ 33.802638] Process bash (pid: 1457, stack limit = 0xee68a248)
[ 33.808740] Stack: (0xee68bec8 to 0xee68c000)
[ 33.813299] bec0: 0000000b c0411284 ee6062c0 00000000 ee68bef4 ee68bee8
[ 33.821862] bee0: c04112ac c04df090 ee68bf14 ee68bef8 c01c2868 c0411290 0000000b ee6b3940
[ 33.830419] bf00: 00000000 00000000 ee68bf4c ee68bf18 c01c1a24 c01c2818 00000000 00000000
[ 33.838990] bf20: ee61b940 ee2f47c0 0000000b 000ce408 ee68bf80 c000f304 ee68a000 00000000
[ 33.847544] bf40: ee68bf7c ee68bf50 c0152dd8 c01c1960 ee68bf7c c0170af8 ee68bf7c ee2f47c0
[ 33.856099] bf60: ee2f47c0 000ce408 0000000b c000f304 ee68bfa4 ee68bf80 c0153330 c0152d34
[ 33.864653] bf80: 00000000 00000000 0000000b 000ce408 b6e7fb50 00000004 00000000 ee68bfa8
[ 33.873204] bfa0: c000f080 c01532e8 0000000b 000ce408 00000001 000ce408 0000000b 00000000
[ 33.881763] bfc0: 0000000b 000ce408 b6e7fb50 00000004 0000000b 00000000 000c5758 00000000
[ 33.890319] bfe0: 00000000 bec2c924 b6de422d b6e1d226 40000030 00000001 75716d2f 00657565
[ 33.898890] [<c04df128>] (usb_udc_softconn_store) from [<c04112ac>] (dev_attr_store+0x28/0x34)
[ 33.907920] [<c04112ac>] (dev_attr_store) from [<c01c2868>] (sysfs_kf_write+0x5c/0x60)
[ 33.916200] [<c01c2868>] (sysfs_kf_write) from [<c01c1a24>] (kernfs_fop_write+0xd0/0x194)
[ 33.924773] [<c01c1a24>] (kernfs_fop_write) from [<c0152dd8>] (vfs_write+0xb0/0x1bc)
[ 33.932874] [<c0152dd8>] (vfs_write) from [<c0153330>] (SyS_write+0x54/0xb0)
[ 33.940247] [<c0153330>] (SyS_write) from [<c000f080>] (ret_fast_syscall+0x0/0x48)
[ 33.948160] Code: e1a01007 e12fff33 e5140004 e5143008 (e5933014)
[ 33.954625] ---[ end trace f849bead94eab7ea ]---

Fixes: 2ccea03 (usb: gadget: introduce UDC Class)
Cc: <stable-***@public.gmane.org> # v3.1+
Signed-off-by: Felipe Balbi <balbi-***@public.gmane.org>
---
drivers/usb/gadget/udc/udc-core.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/usb/gadget/udc/udc-core.c b/drivers/usb/gadget/udc/udc-core.c
index f107bb6..f205465 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -507,6 +507,11 @@ static ssize_t usb_udc_softconn_store(struct device *dev,
{
struct usb_udc *udc = container_of(dev, struct usb_udc, dev);

+ if (!udc->driver) {
+ dev_err(dev, "soft-connect without a gadget driver\n");
+ return -EOPNOTSUPP;
+ }
+
if (sysfs_streq(buf, "connect")) {
usb_gadget_udc_start(udc->gadget, udc->driver);
usb_gadget_connect(udc->gadget);

[Felipe Balbi]

From: Thomas Gleixner <***@linutronix.de>

commit c58d80f52 ("usb: musb: Ensure that cppi41 timer gets armed on
premature DMA TX irq") fixed hrtimer scheduling bug. There is one left
which does not trigger that often.
The following scenario is still possible:

lock(&x->lock);
hrtimer_start(&x->t);
unlock(&x->lock);

expires:
t->function();
lock(&x->lock);
lock(&x->lock); if (!hrtimer_queued(&x->t))
hrtimer_start(&x->t);
unlock(&x->lock);

if (!list_empty(x->early_tx_list))
ret = HRTIMER_RESTART;
-> hrtimer_forward_now(...)
} else
ret = HRTIMER_NORESTART;

unlock(&x->lock);

and the timer callback returns HRTIMER_RESTART for an armed timer. This
is wrong and we run into the BUG_ON() in __run_hrtimer().
This can happens on SMP or PREEMPT-RT.
The patch fixes the problem by only starting the timer if the timer is
not yet queued.

Cc: ***@vger.kernel.org
Reported-by: Torben Hohn <***@linutronix.de>
Signed-off-by: Thomas Gleixner <***@linutronix.de>
[bigeasy: collected information and created a patch + description based
on it]
Signed-off-by: Sebastian Andrzej Siewior <***@linutronix.de>
Signed-off-by: Felipe Balbi <***@ti.com>
---
drivers/usb/musb/musb_cppi41.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index acdfb3e..5a9b977 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -209,7 +209,8 @@ static enum hrtimer_restart cppi41_recheck_tx_req(struct hrtimer *timer)
}
}

- if (!list_empty(&controller->early_tx_list)) {
+ if (!list_empty(&controller->early_tx_list) &&
+ !hrtimer_is_queued(&controller->early_tx)) {
ret = HRTIMER_RESTART;
hrtimer_forward_now(&controller->early_tx,
ktime_set(0, 20 * NSEC_PER_USEC));

[Felipe Balbi]

From: Sebastian Andrzej Siewior <***@linutronix.de>

Commit 468bcc2a2ca ("usb: musb: dsps: kill OTG timer on suspend") stopped
the timer in suspend path but forgot the re-enable it in the resume
path. This patch fixes the behaviour.

Cc: <***@vger.kernel.org> # v3.14+
Fixes 468bcc2a2ca "usb: musb: dsps: kill OTG timer on suspend"
Signed-off-by: Sebastian Andrzej Siewior <***@linutronix.de>
Signed-off-by: Felipe Balbi <***@ti.com>
---
drivers/usb/musb/musb_dsps.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 154bcf1..b18f8d5 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -896,7 +896,9 @@ static int dsps_resume(struct device *dev)
dsps_writel(mbase, wrp->mode, glue->context.mode);
dsps_writel(mbase, wrp->tx_mode, glue->context.tx_mode);
dsps_writel(mbase, wrp->rx_mode, glue->context.rx_mode);
- setup_timer(&glue->timer, otg_timer, (unsigned long) musb);
+ if (musb->xceiv->state == OTG_STATE_B_IDLE &&
+ musb->port_mode == MUSB_PORT_MODE_DUAL_ROLE)
+ mod_timer(&glue->timer, jiffies + wrp->poll_seconds * HZ);

return 0;
}

[Felipe Balbi]

From: Robert Baldyga <***@samsung.com>

During FunctionFS bind, ffs_data_get() function was called twice
(in functionfs_bind() and in ffs_do_functionfs_bind()), while on unbind
ffs_data_put() was called once (in functionfs_unbind() function).
In result refcount never reached value 0, and ffs memory resources has
been never released.

Since ffs_data_get() call in ffs_do_functionfs_bind() is redundant
and not neccessary, we remove it to have equal number of gets ans puts,
and free allocated memory after refcount reach 0.

Fixes: 5920cda (usb: gadget: FunctionFS: convert to new function
interface with backward compatibility)
Cc: <***@vger.kernel.org> # v3.14+
Signed-off-by: Robert Baldyga <***@samsung.com>
Signed-off-by: Felipe Balbi <***@ti.com>
---
drivers/usb/gadget/function/f_fs.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 7c6771d..12dbdaf 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -2663,8 +2663,6 @@ static inline struct f_fs_opts *ffs_do_functionfs_bind(struct usb_function *f,
func->conf = c;
func->gadget = c->cdev->gadget;

- ffs_data_get(func->ffs);
-
/*
* in drivers/usb/gadget/configfs.c:configfs_composite_bind()
* configurations are bound in sequence with list_for_each_entry,

[Felipe Balbi]

From: Geert Uytterhoeven <geert-***@public.gmane.org>

If NO_DMA=y:

drivers/built-in.o: In function `xudc_done':
udc-xilinx.c:(.text+0x54f4d2): undefined reference to `usb_gadget_unmap_request'
drivers/built-in.o: In function `xudc_dma_send':
udc-xilinx.c:(.text+0x54f9f8): undefined reference to `dma_sync_single_for_cpu'
drivers/built-in.o: In function `xudc_read_fifo':
udc-xilinx.c:(.text+0x54ff4a): undefined reference to `dma_sync_single_for_cpu'
drivers/built-in.o: In function `xudc_ep_queue':
udc-xilinx.c:(.text+0x550e7c): undefined reference to `usb_gadget_map_request'

Signed-off-by: Geert Uytterhoeven <geert-***@public.gmane.org>
Signed-off-by: Felipe Balbi <balbi-***@public.gmane.org>
---
drivers/usb/gadget/udc/Kconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
index 3ea287b..217365d 100644
--- a/drivers/usb/gadget/udc/Kconfig
+++ b/drivers/usb/gadget/udc/Kconfig
@@ -357,6 +357,7 @@ config USB_EG20T

config USB_GADGET_XILINX
tristate "Xilinx USB Driver"
+ depends on HAS_DMA
depends on OF || COMPILE_TEST
help
USB peripheral controller driver for Xilinx USB2 device.

[Felipe Balbi]

without this check, f_uac2 would try to disable
the same endpoint twice. Fix that.

Signed-off-by: Felipe Balbi <balbi-***@public.gmane.org>
---
drivers/usb/gadget/function/f_uac2.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
index fa51118..1146f4d 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -951,6 +951,9 @@ free_ep(struct uac2_rtd_params *prm, struct usb_ep *ep)
struct snd_uac2_chip *uac2 = prm->uac2;
int i;

+ if (!prm->ep_enabled)
+ return;
+
prm->ep_enabled = false;

for (i = 0; i < USB_XFERS; i++) {

[Felipe Balbi]

We shouldn't try to dequeue a NULL pointer.

Signed-off-by: Felipe Balbi <balbi-***@public.gmane.org>
---
drivers/usb/gadget/function/uvc_video.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
index c3e1f27..9cb86bc 100644
--- a/drivers/usb/gadget/function/uvc_video.c
+++ b/drivers/usb/gadget/function/uvc_video.c
@@ -352,7 +352,8 @@ int uvcg_video_enable(struct uvc_video *video, int enable)

if (!enable) {
for (i = 0; i < UVC_NUM_REQUESTS; ++i)
- usb_ep_dequeue(video->ep, video->req[i]);
+ if (video->req[i])
+ usb_ep_dequeue(video->ep, video->req[i]);

uvc_video_free_requests(video);
uvcg_queue_enable(&video->queue, 0);

[Felipe Balbi]

dwc3_gadget_ep0_set_halt() will be called without
locks held in some cases, so we must hold the lock
on our own. While at that, also add a version without
locks to be called in certain conditions.

Signed-off-by: Felipe Balbi <balbi-***@public.gmane.org>
---
drivers/usb/dwc3/ep0.c | 16 +++++++++++++++-
drivers/usb/dwc3/gadget.h | 1 +
2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index b359387..36f6158 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -271,7 +271,7 @@ static void dwc3_ep0_stall_and_restart(struct dwc3 *dwc)
dwc3_ep0_out_start(dwc);
}

-int dwc3_gadget_ep0_set_halt(struct usb_ep *ep, int value)
+int __dwc3_gadget_ep0_set_halt(struct usb_ep *ep, int value)
{
struct dwc3_ep *dep = to_dwc3_ep(ep);
struct dwc3 *dwc = dep->dwc;
@@ -281,6 +281,20 @@ int dwc3_gadget_ep0_set_halt(struct usb_ep *ep, int value)
return 0;
}

+int dwc3_gadget_ep0_set_halt(struct usb_ep *ep, int value)
+{
+ struct dwc3_ep *dep = to_dwc3_ep(ep);
+ struct dwc3 *dwc = dep->dwc;
+ unsigned long flags;
+ int ret;
+
+ spin_lock_irqsave(&dwc->lock, flags);
+ ret = __dwc3_gadget_ep0_set_halt(ep, value);
+ spin_unlock_irqrestore(&dwc->lock, flags);
+
+ return ret;
+}
+
void dwc3_ep0_out_start(struct dwc3 *dwc)
{
int ret;
diff --git a/drivers/usb/dwc3/gadget.h b/drivers/usb/dwc3/gadget.h
index 178ad89..f889008 100644
--- a/drivers/usb/dwc3/gadget.h
+++ b/drivers/usb/dwc3/gadget.h
@@ -82,6 +82,7 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
void dwc3_ep0_interrupt(struct dwc3 *dwc,
const struct dwc3_event_depevt *event);
void dwc3_ep0_out_start(struct dwc3 *dwc);
+int __dwc3_gadget_ep0_set_halt(struct usb_ep *ep, int value);
int dwc3_gadget_ep0_set_halt(struct usb_ep *ep, int value);
int dwc3_gadget_ep0_queue(struct usb_ep *ep, struct usb_request *request,
gfp_t gfp_flags);

[Felipe Balbi]

If a set_alt() to the same alternate setting that's
already selected is received, functions are required
to reset the interface state, this means we must disable
all endpoints and reenable them again.

This is also documented on our kdoc for struct usb_function

* @set_alt: (REQUIRED) Reconfigures altsettings; function drivers may
* 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.

Signed-off-by: Felipe Balbi <balbi-***@public.gmane.org>
---
drivers/usb/gadget/function/f_uvc.c | 29 ++++++++++++++++++++---------
1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index e126439..e00e8b7 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -286,11 +286,12 @@ static int
uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
{
struct uvc_device *uvc = to_uvc(f);
+ struct usb_composite_dev *cdev = f->config->cdev;
struct v4l2_event v4l2_event;
struct uvc_event *uvc_event = (void *)&v4l2_event.u.data;
int ret;

- INFO(f->config->cdev, "uvc_function_set_alt(%u, %u)\n", interface, alt);
+ INFO(cdev, "uvc_function_set_alt(%u, %u)\n", interface, alt);

if (interface == uvc->control_intf) {
if (alt)
@@ -299,7 +300,7 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
if (uvc->state == UVC_STATE_DISCONNECTED) {
memset(&v4l2_event, 0, sizeof(v4l2_event));
v4l2_event.type = UVC_EVENT_CONNECT;
- uvc_event->speed = f->config->cdev->gadget->speed;
+ uvc_event->speed = cdev->gadget->speed;
v4l2_event_queue(uvc->vdev, &v4l2_event);

uvc->state = UVC_STATE_CONNECTED;
@@ -321,8 +322,10 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
if (uvc->state != UVC_STATE_STREAMING)
return 0;

- if (uvc->video.ep)
+ if (uvc->video.ep) {
usb_ep_disable(uvc->video.ep);
+ uvc->video.ep->driver_data = NULL;
+ }

memset(&v4l2_event, 0, sizeof(v4l2_event));
v4l2_event.type = UVC_EVENT_STREAMOFF;
@@ -335,14 +338,22 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
if (uvc->state != UVC_STATE_CONNECTED)
return 0;

- if (uvc->video.ep) {
- ret = config_ep_by_speed(f->config->cdev->gadget,
- &(uvc->func), uvc->video.ep);
- if (ret)
- return ret;
- usb_ep_enable(uvc->video.ep);
+ if (!uvc->video.ep)
+ return -EINVAL;
+
+ if (uvc->video.ep->driver_data) {
+ INFO(cdev, "reset UVC\n");
+ usb_ep_disable(uvc->video.ep);
+ uvc->video.ep->driver_data = NULL;
}

+ ret = config_ep_by_speed(f->config->cdev->gadget,
+ &(uvc->func), uvc->video.ep);
+ if (ret)
+ return ret;
+ usb_ep_enable(uvc->video.ep);
+ uvc->video.ep->driver_data = uvc;
+
memset(&v4l2_event, 0, sizeof(v4l2_event));
v4l2_event.type = UVC_EVENT_STREAMON;
v4l2_event_queue(uvc->vdev, &v4l2_event);

[Felipe Balbi]

Instead of releasing the lock and calling locked
versions of our set_halt() methods, let's hold
the lock all the way through and call unlocked
versions of those functions.

Signed-off-by: Felipe Balbi <balbi-***@public.gmane.org>
---
drivers/usb/dwc3/gadget.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index fd92f63..b98295e 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1257,15 +1257,18 @@ static int dwc3_gadget_ep_set_wedge(struct usb_ep *ep)
struct dwc3_ep *dep = to_dwc3_ep(ep);
struct dwc3 *dwc = dep->dwc;
unsigned long flags;
+ int ret;

spin_lock_irqsave(&dwc->lock, flags);
dep->flags |= DWC3_EP_WEDGE;
- spin_unlock_irqrestore(&dwc->lock, flags);

if (dep->number == 0 || dep->number == 1)
- return dwc3_gadget_ep0_set_halt(ep, 1);
+ ret = __dwc3_gadget_ep0_set_halt(ep, 1);
else
- return dwc3_gadget_ep_set_halt(ep, 1);
+ ret = __dwc3_gadget_ep_set_halt(dep, 1);
+ spin_unlock_irqrestore(&dwc->lock, flags);
+
+ return ret;
}

/* -------------------------------------------------------------------------- */