From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [v5,4/6] usb: gadget: add mechanism to specify an explicit status stage From: Paul Elder Message-Id: <20190116050029.GA13084@localhost.localdomain> Date: Wed, 16 Jan 2019 00:00:29 -0500 To: Alan Stern Cc: laurent.pinchart@ideasonboard.com, kieran.bingham@ideasonboard.com, b-liu@ti.com, rogerq@ti.com, balbi@kernel.org, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org List-ID: T24gTW9uLCBKYW4gMTQsIDIwMTkgYXQgMTA6MjQ6NDRBTSAtMDUwMCwgQWxhbiBTdGVybiB3cm90 ZToKPiBPbiBNb24sIDE0IEphbiAyMDE5LCBQYXVsIEVsZGVyIHdyb3RlOgo+IAo+ID4gPiA+ID4g Q2FuIHlvdSBjaGVjayB5b3VyIHV2Ywo+ID4gPiA+ID4gY2hhbmdlcyB1c2luZyBkdW1teV9oY2Qg d2l0aCB0aGUgcGF0Y2ggYmVsb3c/Cj4gPiA+ID4gCj4gPiA+ID4gSSdtIG5vdCBzdXJlIHdoYXQg dG8gbWFrZSBvZiB0aGUgdGVzdCByZXN1bHRzLiBJIGdldCB0aGUgc2FtZSByZXN1bHRzCj4gPiA+ ID4gd2l0aCBvciB3aXRob3V0IHRoZSBwYXRjaC4gV2hpY2ggSSBndWVzcyBtYWtlcyBzZW5zZS4u LiBpbiBkdW1teV9xdWV1ZSwKPiA+ID4gPiB0aGlzIGlzIGdldHRpbmcgaGl0IHdoZW4gdGhlIHV2 YyBmdW5jdGlvbiBkcml2ZXIgdHJpZXMgdG8gY29tcGxldGUgdGhlCj4gPiA+ID4gZGVsYXllZCBz dGF0dXM6Cj4gPiA+ID4gCj4gPiA+ID4gCXJlcSA9IHVzYl9yZXF1ZXN0X3RvX2R1bW15X3JlcXVl c3QoX3JlcSk7Cj4gPiA+ID4gCWlmICghX3JlcSB8fCAhbGlzdF9lbXB0eSgmcmVxLT5xdWV1ZSkg fHwgIV9yZXEtPmNvbXBsZXRlKQo+ID4gPiA+IAkJcmV0dXJuIC1FSU5WQUw7Cj4gPiA+ID4gCj4g PiA+ID4gU28gdGhlIGRlbGF5ZWQvZXhwbGljaXQgc3RhdHVzIHN0YWdlIGlzIG5ldmVyIGNvbXBs ZXRlZCwgYWZhaWN0Lgo+ID4gPiAKPiA+ID4gSSBwcmVzdW1lIHlvdSBhcmUgaGl0dGluZyB0aGUg IWxpc3RfZW1wdHkoJnJlcS0+cXVldWUpIHRlc3QsIHllcz8gIFRoZSAKPiA+ID4gb3RoZXIgdHdv IHRlc3RzIGFyZSB0cml2aWFsLgo+ID4gCj4gPiBZZXMsIHRoYXQgaXMgd2hhdCdzIGhhcHBlbmlu Zy4KPiA+IAo+ID4gPiBUcmlnZ2VyaW5nIHRoZSAhbGlzdF9lbXB0eSgpIHRlc3QgbWVhbnMgdGhl IHJlcXVlc3QgaGFzIGFscmVhZHkgYmVlbgo+ID4gPiBzdWJtaXR0ZWQgYW5kIG5vdCB5ZXQgY29t cGxldGVkLiAgVGhpcyBwcm9iYWJseSBpbmRpY2F0ZXMgdGhlcmUgaXMgYQo+ID4gPiBidWcgaW4g dGhlIHV2YyBmdW5jdGlvbiBkcml2ZXIgY29kZS4KPiA+IAo+ID4gVGhlIHV2YyBmdW5jdGlvbiBk cml2ZXIgd29ya3Mgd2l0aCBtdXNiLCB0aG91Z2ggOi8KPiA+IAo+ID4gSSBjb21wYXJlZCB0aGUg c2VxdWVuY2Ugb2YgY2FsbHMgdG8gdGhlIHV2YyBzZXR1cCwgY29tcGxldGlvbiBoYW5kbGVyLAo+ ID4gYW5kIHN0YXR1cyBzdGFnZSBzZW5kaW5nLCBhbmQgZm9yIHNvbWUgcmVhc29uIGR1bW15X2hj ZCwgYWZ0ZXIgYW4gT1VUCj4gPiBzZXR1cC1jb21wbGV0aW9uLXN0YXR1cyBzZXF1ZW5jZSwgY2Fs bHMgYSBjb21wbGV0aW9uLXN0YXR1cy1jb21wbGV0aW9uCj4gPiBzZXF1ZW5jZSwgYW5kIHRoZW4g Z29lcyBvbiB0aGUgdGhlIG5leHQgcmVxdWVzdC4gbXVzYiBzaW1wbHkgZ29lcyBvbiB0bwo+ID4g dGhlIG5leHQgcmVxdWVzdCBhZnRlciB0aGUgc2V0dXAtY29tcGxldGlvbi1zdGF0dXMgc2VxdWVu Y2UuCj4gCj4gSSBkb24ndCBxdWl0ZSB1bmRlcnN0YW5kLiAgVGhlcmUncyBhIGNvbnRyb2wtT1VU IHRyYW5zZmVyLCB0aGUgc2V0dXAsIAo+IGRhdGEsIGFuZCBzdGF0dXMgdHJhbnNhY3Rpb25zIGFs bCBjb21wbGV0ZSBub3JtYWxseSwgYW5kIHRoZW4gd2hhdCAKPiBoYXBwZW5zPyAgV2hhdCBkbyB5 b3UgbWVhbiBieSAiYSBjb21wbGV0aW9uLXN0YXR1cy1jb21wbGV0aW9uIAo+IHNlcXVlbmNlIj8g IEEgbW9yZSBkZXRhaWxlZCBkZXNjcmlwdGlvbiB3b3VsZCBoZWxwLgo+IAoKSSBtZWFudCB0aGUg ZnVuY3Rpb25zIChwcm9jZWR1cmVzKSBpbiB0aGUgZnVuY3Rpb24gZHJpdmVyLCBzbyB0aGUgc2V0 dXAKaGFuZGxlciAodXZjX2Z1bmN0aW9uX3NldHVwKSwgdGhlIGNvbXBsZXRpb24gaGFuZGxlcgoo dXZjX2Z1bmN0aW9uX2VwMF9jb21wbGV0ZSksIGFuZCB0aGUgc3RhdHVzIHNlbmRlciAodXZjX3Nl bmRfcmVzcG9uc2UpLAphbHRob3VnaCB0aGUgbGFzdCBvbmUgYWN0dWFsbHkgc2VuZHMgdGhlIGRh dGEgc3RhZ2UgZm9yIGNvbnRyb2wgSU4uClNvIGFmdGVyIHRoZSBzdGF0dXMgaXMgc2VudCBvbiB0 aGUgdXZjIGdhZGdldCBkcml2ZXIncyBlbmQsIGl0cwpjb21wbGV0aW9uIGhhbmRsZXIgaXMgY2Fs bGVkIGFnYWluIHdpdGhvdXQgdGhlIHNldHVwIGhhbmRsZXIgYmVpbmcKY2FsbGVkIGJlZm9yZWhh bmQgYW5kIEkgY2FudCBmaWd1cmUgb3V0IHdoeS4KCj4gPiBJIGNvbW1lbnRlZCBvdXQgdGhlIHBh cmFub2lhIGJsb2NrIGluIGR1bW15X3RpbWVyLCBhbmQgZHVtbXlfaGNkIHN0aWxsCj4gPiBkb2Vz IHRoZSBleHRyYSBjb21wbGV0aW9uLCBidXQgaXQgZG9lc24ndCBlcnJvciBvdXQgYW55bW9yZS4g SSBkb3VidAo+ID4gdGhhdCdzIHRoZS9hIHNvbHV0aW9uIHRob3VnaCwgZXNwZWNpYWxseSBzaW5j ZSBJIGdldDoKPiA+IAo+ID4gWyAgIDIyLjYxNjU3N10gdXZjdmlkZW86IEZhaWxlZCB0byBxdWVy eSAoMTI5KSBVVkMgcHJvYmUgY29udHJvbCA6IC03NSAoZXhwLiAyNikuCj4gPiBbICAgMjIuNjI0 NDgxXSB1dmN2aWRlbzogRmFpbGVkIHRvIGluaXRpYWxpemUgdGhlIGRldmljZSAoLTUpLgo+ID4g Cj4gPiBOb3Qgc3VyZSBpZiB0aGF0J3MgYSByZXN1bHQgb2YgZHVtbXlfaGNkIG5vdCBzdXBwb3J0 aW5nIGlzb2Nocm9ub3VzCj4gPiB0cmFuc2ZlcnMgb3Igbm90Lgo+ID4gCj4gPiBJJ20gbm90IHN1 cmUgd2hlcmUgdG8gY29udGludWUgaW52ZXN0aWdhdGluZyA6Lwo+IAo+IFBlcmhhcHMgcmVtb3Zp bmcgdGhlICIjaWYgMCIgcHJvdGVjdGluZyB0aGUgZGV2X2RiZyBsaW5lIGluIAo+IGR1bW15X3F1 ZXVlKCkgd291bGQgcHJvdmlkZSBzb21lIGhlbHBmdWwgb3V0cHV0LgoKSXQgZGlkLCBidXQgZGlk bid0IGdldCBtZSBtdWNoIGZhcnRoZXIgOi8KCj4gQW5vdGhlciB0aGluZyB0byBjaGVjayB3b3Vs ZCBiZSBpZiB0aGUgImltcGxlbWVudCBhbiBlbXVsYXRlZCAKPiBzaW5nbGUtcmVxdWVzdCBGSUZP IiBpbiBkdW1teV9xdWV1ZSgpIGlzIGNhdXNpbmcgdHJvdWJsZS4gIFRoZXJlJ3Mgbm8gCj4gaGFy bSBpbiByZXBsYWNpbmcgdGhlIGxvbmcgImlmIiBjb25kaXRpb24gd2l0aCAiaWYgKDApIi4KClRo YXQgZGlkbid0IGNoYW5nZSBhbnl0aGluZy4KCkFsdGhvdWdoIEkgZGlkIG5vdGljZSB0aGF0IHRo ZSBkdW1teV9xdWV1ZSB0aGF0IGNhbGxzIHRoZSBjb21wbGV0aW9uCmhhbmRsZXIgd2l0aG91dCB0 aGUgcHJlY2VlZGluZyBzZXR1cCBoYW5kbGVyIHNheXMgdGhhdCBpdCdzIGluIHRoZQpzdGF0dXMg c3RhZ2UgKGVwLT5zdGF0dXNfc3RhZ2UgPT0gMSkuCgoKVGhhbmtzLAoKUGF1bAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.6 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS, USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E7907C43387 for ; Wed, 16 Jan 2019 05:00:44 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id B1B8720840 for ; Wed, 16 Jan 2019 05:00:44 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="RBlD2wsk" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725878AbfAPFAk (ORCPT ); Wed, 16 Jan 2019 00:00:40 -0500 Received: from perceval.ideasonboard.com ([213.167.242.64]:50600 "EHLO perceval.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725320AbfAPFAk (ORCPT ); Wed, 16 Jan 2019 00:00:40 -0500 Received: from localhost.localdomain (unknown [96.44.9.117]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 921854F8; Wed, 16 Jan 2019 06:00:36 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1547614837; bh=vLC9D6Brw36/lPddQuZZJXxz3IDtjj2LiGW9XlmKXCg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=RBlD2wskE41avSd8RJEFRaEYw3NQalD9vaYeV5lciUd1bB/M+hJbaQKkkpAIcXxm+ plhVUKCBOeJJNkdwsWdlYJOaaZEznj/MQl7l69iRD0yavqIEj5hPntRjOq/HqT1cB6 F0m9WzyK6/iu+uMSsyClaJOSFLkXMjgtY0x041O0= Date: Wed, 16 Jan 2019 00:00:29 -0500 From: Paul Elder To: Alan Stern Cc: laurent.pinchart@ideasonboard.com, kieran.bingham@ideasonboard.com, b-liu@ti.com, rogerq@ti.com, balbi@kernel.org, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v5 4/6] usb: gadget: add mechanism to specify an explicit status stage Message-ID: <20190116050029.GA13084@localhost.localdomain> References: <20190114051113.GD32268@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jan 14, 2019 at 10:24:44AM -0500, Alan Stern wrote: > On Mon, 14 Jan 2019, Paul Elder wrote: > > > > > > Can you check your uvc > > > > > changes using dummy_hcd with the patch below? > > > > > > > > I'm not sure what to make of the test results. I get the same results > > > > with or without the patch. Which I guess makes sense... in dummy_queue, > > > > this is getting hit when the uvc function driver tries to complete the > > > > delayed status: > > > > > > > > req = usb_request_to_dummy_request(_req); > > > > if (!_req || !list_empty(&req->queue) || !_req->complete) > > > > return -EINVAL; > > > > > > > > So the delayed/explicit status stage is never completed, afaict. > > > > > > I presume you are hitting the !list_empty(&req->queue) test, yes? The > > > other two tests are trivial. > > > > Yes, that is what's happening. > > > > > Triggering the !list_empty() test means the request has already been > > > submitted and not yet completed. This probably indicates there is a > > > bug in the uvc function driver code. > > > > The uvc function driver works with musb, though :/ > > > > I compared the sequence of calls to the uvc setup, completion handler, > > and status stage sending, and for some reason dummy_hcd, after an OUT > > setup-completion-status sequence, calls a completion-status-completion > > sequence, and then goes on the the next request. musb simply goes on to > > the next request after the setup-completion-status sequence. > > I don't quite understand. There's a control-OUT transfer, the setup, > data, and status transactions all complete normally, and then what > happens? What do you mean by "a completion-status-completion > sequence"? A more detailed description would help. > I meant the functions (procedures) in the function driver, so the setup handler (uvc_function_setup), the completion handler (uvc_function_ep0_complete), and the status sender (uvc_send_response), although the last one actually sends the data stage for control IN. So after the status is sent on the uvc gadget driver's end, its completion handler is called again without the setup handler being called beforehand and I cant figure out why. > > I commented out the paranoia block in dummy_timer, and dummy_hcd still > > does the extra completion, but it doesn't error out anymore. I doubt > > that's the/a solution though, especially since I get: > > > > [ 22.616577] uvcvideo: Failed to query (129) UVC probe control : -75 (exp. 26). > > [ 22.624481] uvcvideo: Failed to initialize the device (-5). > > > > Not sure if that's a result of dummy_hcd not supporting isochronous > > transfers or not. > > > > I'm not sure where to continue investigating :/ > > Perhaps removing the "#if 0" protecting the dev_dbg line in > dummy_queue() would provide some helpful output. It did, but didn't get me much farther :/ > Another thing to check would be if the "implement an emulated > single-request FIFO" in dummy_queue() is causing trouble. There's no > harm in replacing the long "if" condition with "if (0)". That didn't change anything. Although I did notice that the dummy_queue that calls the completion handler without the preceeding setup handler says that it's in the status stage (ep->status_stage == 1). Thanks, Paul