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: [RFC] usb: add usb_fill_iso_urb() From: Laurent Pinchart Message-Id: <23211374.K0HmdtcaYO@avalon> Date: Fri, 13 Jul 2018 11:01:02 +0300 To: Sebastian Andrzej Siewior Cc: Alan Stern , linux-media@vger.kernel.org, Mauro Carvalho Chehab , linux-usb@vger.kernel.org, tglx@linutronix.de, Greg Kroah-Hartman , Takashi Iwai List-ID: SGkgU2ViYXN0aWFuLAoKVGhhbmsgeW91IGZvciB0aGUgcGF0Y2guCgpPbiBGcmlkYXksIDEzIEp1 bHkgMjAxOCAwMTozNToyNyBFRVNUIFNlYmFzdGlhbiBBbmRyemVqIFNpZXdpb3Igd3JvdGU6Cj4g UHJvdmlkZSB1c2JfZmlsbF9pc29fdXJiKCkgZm9yIHRoZSBpbml0aWFsaXNhdGlvbiBvZiBpc29j aHJvbm91cyBVUkJzLgo+IFdlIGFscmVhZHkgaGF2ZSBvbmUgb2YgdGhpcyBoZWxwZXJzIGZvciBj b250cm9sLCBidWxrIGFuZCBpbnRlcnJ1cHRpYmxlCj4gVVJCIHR5cGVzLiBUaGlzIGhlbHBzIHRv IGtlZXAgdGhlIGluaXRpYWxpc2F0aW9uIG9mIHRoZSBVUkIgbWVtYmVycyBpbgo+IG9uZSBwbGFj ZS4KPiBVcGRhdGUgdGhlIGRvY3VtZW50YXRpb24gYnkgYWRkaW5nIHRoaXMgdG8gdGhlIGF2YWls YWJsZSBpbml0IGZ1bmN0aW9ucwo+IGFuZCByZW1vdmUgdGhlIHN1Z2dlc3Rpb24gdG8gdXNlIHRo ZSBgX2ludF8nIGhlbHBlciB3aGljaCBtaWdodCBwcm92aWRlCj4gd3JvbmcgZW5jb2RpbmcgZm9y IHRoZSBgaW50ZXJ2YWwnIG1lbWJlci4KPiAKPiBUaGlzIGxvb2tzIGxpa2UgaXQgd291bGQgY292 ZXIgbW9zdCB1c2VycyBuaWNlbHkuIFRoZSBzb3VuZCBzdWJzeXN0ZW0KPiBpbml0aWFsaXNlcyB0 aGUgLT5pc29fZnJhbWVfZGVzY1tdLm9mZnNldCArIGxlbmd0aCBtZW1iZXIgKG9mdGVuKSBhdCBh Cj4gZGlmZmVyZW50IGxvY2F0aW9uIGFuZCBJJ20gbm90IHN1cmUgLT5pbnRlcnZhbCB3aWxsIHdv cmsgYWx3YXlzIGFzCj4gZXhwZWN0ZWQuIFNvIHdlIG1pZ2h0IG5lZWQgdG8gb3ZlcndyaXRlIHRo b3NlIHR3byBpbiB3b3JzdCBjYXNlLgo+IAo+IFNvbWUgdXNlcnMgYWxzbyBpbml0aWFsaXNlIC0+ aXNvX2ZyYW1lX2Rlc2NbXS5hY3R1YWxfbGVuZ3RoIGJ1dCBJIGRvbid0CgpzL0kgZG9uJ3QvSSBk b24ndCB0aGluay8gPwoKPiB0aGlzIGlzIHJlcXVpcmVkIHNpbmNlIGl0IGlzIHRoZSByZXR1cm4g dmFsdWUuCj4gCj4gU2lnbmVkLW9mZi1ieTogU2ViYXN0aWFuIEFuZHJ6ZWogU2lld2lvciA8Ymln ZWFzeUBsaW51dHJvbml4LmRlPgo+IC0tLQo+ICBEb2N1bWVudGF0aW9uL2RyaXZlci1hcGkvdXNi L1VSQi5yc3QgfCAxMiArKystLS0tCj4gIGluY2x1ZGUvbGludXgvdXNiLmggICAgICAgICAgICAg ICAgICB8IDUzICsrKysrKysrKysrKysrKysrKysrKysrKysrKysKPiAgMiBmaWxlcyBjaGFuZ2Vk LCA1OSBpbnNlcnRpb25zKCspLCA2IGRlbGV0aW9ucygtKQo+IAo+IGRpZmYgLS1naXQgYS9Eb2N1 bWVudGF0aW9uL2RyaXZlci1hcGkvdXNiL1VSQi5yc3QKPiBiL0RvY3VtZW50YXRpb24vZHJpdmVy LWFwaS91c2IvVVJCLnJzdCBpbmRleCA2MWE1NGRhOWZjZTkuLjIwMDMwYjc4MTUxOQo+IDEwMDY0 NAo+IC0tLSBhL0RvY3VtZW50YXRpb24vZHJpdmVyLWFwaS91c2IvVVJCLnJzdAo+ICsrKyBiL0Rv Y3VtZW50YXRpb24vZHJpdmVyLWFwaS91c2IvVVJCLnJzdAo+IEBAIC0xMTYsMTEgKzExNiwxMSBA QCBXaGF0IGhhcyB0byBiZSBmaWxsZWQgaW4/Cj4gCj4gIERlcGVuZGluZyBvbiB0aGUgdHlwZSBv ZiB0cmFuc2FjdGlvbiwgdGhlcmUgYXJlIHNvbWUgaW5saW5lIGZ1bmN0aW9ucwo+ICBkZWZpbmVk IGluIGBgbGludXgvdXNiLmhgYCB0byBzaW1wbGlmeSB0aGUgaW5pdGlhbGl6YXRpb24sIHN1Y2gg YXMKPiAtOmM6ZnVuYzpgdXNiX2ZpbGxfY29udHJvbF91cmJgLCA6YzpmdW5jOmB1c2JfZmlsbF9i dWxrX3VyYmAgYW5kCj4gLTpjOmZ1bmM6YHVzYl9maWxsX2ludF91cmJgLiAgSW4gZ2VuZXJhbCwg dGhleSBuZWVkIHRoZSB1c2IgZGV2aWNlIHBvaW50ZXIsCj4gLXRoZSBwaXBlICh1c3VhbCBmb3Jt YXQgZnJvbSB1c2IuaCksIHRoZSB0cmFuc2ZlciBidWZmZXIsIHRoZSBkZXNpcmVkCj4gdHJhbnNm ZXIgLWxlbmd0aCwgdGhlIGNvbXBsZXRpb24gaGFuZGxlciwgYW5kIGl0cyBjb250ZXh0LiBUYWtl IGEgbG9vayBhdAo+IHRoZSBzb21lIC1leGlzdGluZyBkcml2ZXJzIHRvIHNlZSBob3cgdGhleSdy ZSB1c2VkLgo+ICs6YzpmdW5jOmB1c2JfZmlsbF9jb250cm9sX3VyYmAsIDpjOmZ1bmM6YHVzYl9m aWxsX2J1bGtfdXJiYCwKPiArOmM6ZnVuYzpgdXNiX2ZpbGxfaW50X3VyYmAgYW5kIDpjOmZ1bmM6 YHVzYl9maWxsX2lzb191cmJgLiAgSW4gZ2VuZXJhbCwKPiB0aGV5ICtuZWVkIHRoZSB1c2IgZGV2 aWNlIHBvaW50ZXIsIHRoZSBwaXBlICh1c3VhbCBmb3JtYXQgZnJvbSB1c2IuaCksIHRoZQo+IHRy YW5zZmVyICtidWZmZXIsIHRoZSBkZXNpcmVkIHRyYW5zZmVyIGxlbmd0aCwgdGhlIGNvbXBsZXRp b24gaGFuZGxlciwgYW5kCj4gaXRzIGNvbnRleHQuICtUYWtlIGEgbG9vayBhdCB0aGUgc29tZSBl eGlzdGluZyBkcml2ZXJzIHRvIHNlZSBob3cgdGhleSdyZQo+IHVzZWQuCj4gCj4gIEZsYWdzOgo+ IAo+IEBAIC0yNDMsNyArMjQzLDcgQEAgQmVzaWRlcyB0aGUgZmllbGRzIHByZXNlbnQgb24gYSBi dWxrIHRyYW5zZmVyLCBmb3IgSVNPLAo+IHlvdSBhbHNvIGFsc28gaGF2ZSB0byBzZXQgYGB1cmIt PmludGVydmFsYGAgdG8gc2F5IGhvdyBvZnRlbiB0byBtYWtlCj4gdHJhbnNmZXJzOyBpdCdzIG9m dGVuIG9uZSBwZXIgZnJhbWUgKHdoaWNoIGlzIG9uY2UgZXZlcnkgbWljcm9mcmFtZSBmb3IKPiBo aWdoc3BlZWQgZGV2aWNlcykuIFRoZSBhY3R1YWwgaW50ZXJ2YWwgdXNlZCB3aWxsIGJlIGEgcG93 ZXIgb2YgdHdvIHRoYXQncwo+IG5vIGJpZ2dlciB0aGFuIHdoYXQgLXlvdSBzcGVjaWZ5LiBZb3Ug Y2FuIHVzZSB0aGUKPiA6YzpmdW5jOmB1c2JfZmlsbF9pbnRfdXJiYCBtYWNybyB0byBmaWxsICt5 b3Ugc3BlY2lmeS4gWW91IGNhbiB1c2UgdGhlCj4gOmM6ZnVuYzpgdXNiX2ZpbGxfaXNvX3VyYmAg bWFjcm8gdG8gZmlsbCBtb3N0IElTTyB0cmFuc2ZlciBmaWVsZHMuCj4gCj4gIEZvciBJU08gdHJh bnNmZXJzIHlvdSBhbHNvIGhhdmUgdG8gZmlsbCBhCj4gOmM6dHlwZTpgdXNiX2lzb19wYWNrZXRf ZGVzY3JpcHRvcmAgZGlmZiAtLWdpdCBhL2luY2x1ZGUvbGludXgvdXNiLmgKPiBiL2luY2x1ZGUv bGludXgvdXNiLmgKPiBpbmRleCA0Y2RkNTE1YTQzODUuLjc0YTMzMzkwNDFkNiAxMDA2NDQKPiAt LS0gYS9pbmNsdWRlL2xpbnV4L3VzYi5oCj4gKysrIGIvaW5jbHVkZS9saW51eC91c2IuaAo+IEBA IC0xNjk3LDYgKzE2OTcsNTkgQEAgc3RhdGljIGlubGluZSB2b2lkIHVzYl9maWxsX2ludF91cmIo c3RydWN0IHVyYiAqdXJiLAo+ICAJdXJiLT5zdGFydF9mcmFtZSA9IC0xOwo+ICB9Cj4gCj4gKy8q Kgo+ICsgKiB1c2JfZmlsbF9pc29fdXJiIC0gbWFjcm8gdG8gaGVscCBpbml0aWFsaXplIGFuIGlz b2Nocm9ub3VzIHVyYgoKU3RyaWN0bHkgc3BlYWtpbmcgdGhpcyBpc24ndCBhIG1hY3JvLCBzbyBJ J2Qgd3JpdGUgImluaXRpYWxpemVzIGFuIGlzb2Nocm9ub3VzIAp1cmIiCgo+ICsgKiBAdXJiOiBw b2ludGVyIHRvIHRoZSB1cmIgdG8gaW5pdGlhbGl6ZS4KPiArICogQGRldjogcG9pbnRlciB0byB0 aGUgc3RydWN0IHVzYl9kZXZpY2UgZm9yIHRoaXMgdXJiLgo+ICsgKiBAcGlwZTogdGhlIGVuZHBv aW50IHBpcGUKPiArICogQHRyYW5zZmVyX2J1ZmZlcjogcG9pbnRlciB0byB0aGUgdHJhbnNmZXIg YnVmZmVyCj4gKyAqIEBidWZmZXJfbGVuZ3RoOiBsZW5ndGggb2YgdGhlIHRyYW5zZmVyIGJ1ZmZl cgo+ICsgKiBAY29tcGxldGVfZm46IHBvaW50ZXIgdG8gdGhlIHVzYl9jb21wbGV0ZV90IGZ1bmN0 aW9uCj4gKyAqIEBjb250ZXh0OiB3aGF0IHRvIHNldCB0aGUgdXJiIGNvbnRleHQgdG8uCj4gKyAq IEBpbnRlcnZhbDogd2hhdCB0byBzZXQgdGhlIHVyYiBpbnRlcnZhbCB0bywgZW5jb2RlZCBsaWtl Cj4gKyAqCXRoZSBlbmRwb2ludCBkZXNjcmlwdG9yJ3MgYkludGVydmFsIHZhbHVlLgo+ICsgKiBA cGFja2V0czogbnVtYmVyIG9mIElTTyBwYWNrZXRzLgo+ICsgKiBAcGFja2V0X3NpemU6IHNpemUg b2YgZWFjaCBJU08gcGFja2V0Lgo+ICsgKgo+ICsgKiBJbml0aWFsaXplcyBhbiBpc29jaHJvbm91 cyB1cmIgd2l0aCB0aGUgcHJvcGVyIGluZm9ybWF0aW9uIG5lZWRlZCB0bwo+IHN1Ym1pdAo+ICsg KiBpdCB0byBhIGRldmljZS4KPiArICoKPiArICogTm90ZSB0aGF0IGlzb2Nocm9ub3VzIGVuZHBv aW50cyB1c2UgYSBsb2dhcml0aG1pYyBlbmNvZGluZyBvZiB0aGUKPiBlbmRwb2ludAo+ICsgKiBp bnRlcnZhbCwgYW5kIGV4cHJlc3MgcG9sbGluZyBpbnRlcnZhbHMgaW4gbWljcm9mcmFtZXMgKGVp Z2h0IHBlcgo+ICsgKiBtaWxsaXNlY29uZCkgcmF0aGVyIHRoYW4gaW4gZnJhbWVzIChvbmUgcGVy IG1pbGxpc2Vjb25kKS4KPiArICovCj4gK3N0YXRpYyBpbmxpbmUgdm9pZCB1c2JfZmlsbF9pc29f dXJiKHN0cnVjdCB1cmIgKnVyYiwKPiArCQkJCSAgICBzdHJ1Y3QgdXNiX2RldmljZSAqZGV2LAo+ ICsJCQkJICAgIHVuc2lnbmVkIGludCBwaXBlLAo+ICsJCQkJICAgIHZvaWQgKnRyYW5zZmVyX2J1 ZmZlciwKPiArCQkJCSAgICBpbnQgYnVmZmVyX2xlbmd0aCwKPiArCQkJCSAgICB1c2JfY29tcGxl dGVfdCBjb21wbGV0ZV9mbiwKPiArCQkJCSAgICB2b2lkICpjb250ZXh0LAo+ICsJCQkJICAgIGlu dCBpbnRlcnZhbCwKPiArCQkJCSAgICB1bnNpZ25lZCBpbnQgcGFja2V0cywKPiArCQkJCSAgICB1 bnNpZ25lZCBpbnQgcGFja2V0X3NpemUpCj4gK3sKPiArCXVuc2lnbmVkIGludCBpOwo+ICsKPiAr CXVyYi0+ZGV2ID0gZGV2Owo+ICsJdXJiLT5waXBlID0gcGlwZTsKPiArCXVyYi0+dHJhbnNmZXJf YnVmZmVyID0gdHJhbnNmZXJfYnVmZmVyOwo+ICsJdXJiLT50cmFuc2Zlcl9idWZmZXJfbGVuZ3Ro ID0gYnVmZmVyX2xlbmd0aDsKPiArCXVyYi0+Y29tcGxldGUgPSBjb21wbGV0ZV9mbjsKPiArCXVy Yi0+Y29udGV4dCA9IGNvbnRleHQ7Cj4gKwo+ICsJaW50ZXJ2YWwgPSBjbGFtcChpbnRlcnZhbCwg MSwgMTYpOwo+ICsJdXJiLT5pbnRlcnZhbCA9IDEgPDwgKGludGVydmFsIC0gMSk7Cj4gKwl1cmIt PnN0YXJ0X2ZyYW1lID0gLTE7Cj4gKwo+ICsJdXJiLT5udW1iZXJfb2ZfcGFja2V0cyA9IHBhY2tl dHM7Cj4gKwo+ICsJZm9yIChpID0gMDsgaSA8IHBhY2tldHM7IGkrKykgewo+ICsJCXVyYi0+aXNv X2ZyYW1lX2Rlc2NbaV0ub2Zmc2V0ID0gcGFja2V0X3NpemUgKiBpOwo+ICsJCXVyYi0+aXNvX2Zy YW1lX2Rlc2NbaV0ubGVuZ3RoID0gcGFja2V0X3NpemU7Cj4gKwl9Cj4gK30KCkkgdGhpbmsgdGhp cyBzaG91bGQgYmUgbW92ZWQgdG8gYSAuYyBmaWxlIGFzIHRoZSBmdW5jdGlvbiBpcyBncm93aW5n IGJpZywgYnV0IAp0aGF0J3MgdHJ1ZSBvZiB0aGUgb3RoZXIgVVJCIGhlbHBlcnMgYXMgd2VsbCwg c28gaXQgY2FuIGJlIGRvbmUgbGF0ZXIgaW4gYSAKc2VwYXJhdGUgcGF0Y2guCgpSZXZpZXdlZC1i eTogTGF1cmVudCBQaW5jaGFydCA8bGF1cmVudC5waW5jaGFydEBpZGVhc29uYm9hcmQuY29tPgoK PiAgZXh0ZXJuIHZvaWQgdXNiX2luaXRfdXJiKHN0cnVjdCB1cmIgKnVyYik7Cj4gIGV4dGVybiBz dHJ1Y3QgdXJiICp1c2JfYWxsb2NfdXJiKGludCBpc29fcGFja2V0cywgZ2ZwX3QgbWVtX2ZsYWdz KTsKPiAgZXh0ZXJuIHZvaWQgdXNiX2ZyZWVfdXJiKHN0cnVjdCB1cmIgKnVyYik7Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from perceval.ideasonboard.com ([213.167.242.64]:35826 "EHLO perceval.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731564AbeGMIOC (ORCPT ); Fri, 13 Jul 2018 04:14:02 -0400 From: Laurent Pinchart To: Sebastian Andrzej Siewior Cc: Alan Stern , linux-media@vger.kernel.org, Mauro Carvalho Chehab , linux-usb@vger.kernel.org, tglx@linutronix.de, Greg Kroah-Hartman , Takashi Iwai Subject: Re: [PATCH RFC] usb: add usb_fill_iso_urb() Date: Fri, 13 Jul 2018 11:01:02 +0300 Message-ID: <23211374.K0HmdtcaYO@avalon> In-Reply-To: <20180712223527.5nmxndignujo7smt@linutronix.de> References: <20180620164945.xb24m7wlbtb6cys5@linutronix.de> <20180712223527.5nmxndignujo7smt@linutronix.de> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-media-owner@vger.kernel.org List-ID: Hi Sebastian, Thank you for the patch. On Friday, 13 July 2018 01:35:27 EEST Sebastian Andrzej Siewior wrote: > Provide usb_fill_iso_urb() for the initialisation of isochronous URBs. > We already have one of this helpers for control, bulk and interruptible > URB types. This helps to keep the initialisation of the URB members in > one place. > Update the documentation by adding this to the available init functions > and remove the suggestion to use the `_int_' helper which might provide > wrong encoding for the `interval' member. > > This looks like it would cover most users nicely. The sound subsystem > initialises the ->iso_frame_desc[].offset + length member (often) at a > different location and I'm not sure ->interval will work always as > expected. So we might need to overwrite those two in worst case. > > Some users also initialise ->iso_frame_desc[].actual_length but I don't s/I don't/I don't think/ ? > this is required since it is the return value. > > Signed-off-by: Sebastian Andrzej Siewior > --- > Documentation/driver-api/usb/URB.rst | 12 +++---- > include/linux/usb.h | 53 ++++++++++++++++++++++++++++ > 2 files changed, 59 insertions(+), 6 deletions(-) > > diff --git a/Documentation/driver-api/usb/URB.rst > b/Documentation/driver-api/usb/URB.rst index 61a54da9fce9..20030b781519 > 100644 > --- a/Documentation/driver-api/usb/URB.rst > +++ b/Documentation/driver-api/usb/URB.rst > @@ -116,11 +116,11 @@ What has to be filled in? > > Depending on the type of transaction, there are some inline functions > defined in ``linux/usb.h`` to simplify the initialization, such as > -:c:func:`usb_fill_control_urb`, :c:func:`usb_fill_bulk_urb` and > -:c:func:`usb_fill_int_urb`. In general, they need the usb device pointer, > -the pipe (usual format from usb.h), the transfer buffer, the desired > transfer -length, the completion handler, and its context. Take a look at > the some -existing drivers to see how they're used. > +:c:func:`usb_fill_control_urb`, :c:func:`usb_fill_bulk_urb`, > +:c:func:`usb_fill_int_urb` and :c:func:`usb_fill_iso_urb`. In general, > they +need the usb device pointer, the pipe (usual format from usb.h), the > transfer +buffer, the desired transfer length, the completion handler, and > its context. +Take a look at the some existing drivers to see how they're > used. > > Flags: > > @@ -243,7 +243,7 @@ Besides the fields present on a bulk transfer, for ISO, > you also also have to set ``urb->interval`` to say how often to make > transfers; it's often one per frame (which is once every microframe for > highspeed devices). The actual interval used will be a power of two that's > no bigger than what -you specify. You can use the > :c:func:`usb_fill_int_urb` macro to fill +you specify. You can use the > :c:func:`usb_fill_iso_urb` macro to fill most ISO transfer fields. > > For ISO transfers you also have to fill a > :c:type:`usb_iso_packet_descriptor` diff --git a/include/linux/usb.h > b/include/linux/usb.h > index 4cdd515a4385..74a3339041d6 100644 > --- a/include/linux/usb.h > +++ b/include/linux/usb.h > @@ -1697,6 +1697,59 @@ static inline void usb_fill_int_urb(struct urb *urb, > urb->start_frame = -1; > } > > +/** > + * usb_fill_iso_urb - macro to help initialize an isochronous urb Strictly speaking this isn't a macro, so I'd write "initializes an isochronous urb" > + * @urb: pointer to the urb to initialize. > + * @dev: pointer to the struct usb_device for this urb. > + * @pipe: the endpoint pipe > + * @transfer_buffer: pointer to the transfer buffer > + * @buffer_length: length of the transfer buffer > + * @complete_fn: pointer to the usb_complete_t function > + * @context: what to set the urb context to. > + * @interval: what to set the urb interval to, encoded like > + * the endpoint descriptor's bInterval value. > + * @packets: number of ISO packets. > + * @packet_size: size of each ISO packet. > + * > + * Initializes an isochronous urb with the proper information needed to > submit > + * it to a device. > + * > + * Note that isochronous endpoints use a logarithmic encoding of the > endpoint > + * interval, and express polling intervals in microframes (eight per > + * millisecond) rather than in frames (one per millisecond). > + */ > +static inline void usb_fill_iso_urb(struct urb *urb, > + struct usb_device *dev, > + unsigned int pipe, > + void *transfer_buffer, > + int buffer_length, > + usb_complete_t complete_fn, > + void *context, > + int interval, > + unsigned int packets, > + unsigned int packet_size) > +{ > + unsigned int i; > + > + urb->dev = dev; > + urb->pipe = pipe; > + urb->transfer_buffer = transfer_buffer; > + urb->transfer_buffer_length = buffer_length; > + urb->complete = complete_fn; > + urb->context = context; > + > + interval = clamp(interval, 1, 16); > + urb->interval = 1 << (interval - 1); > + urb->start_frame = -1; > + > + urb->number_of_packets = packets; > + > + for (i = 0; i < packets; i++) { > + urb->iso_frame_desc[i].offset = packet_size * i; > + urb->iso_frame_desc[i].length = packet_size; > + } > +} I think this should be moved to a .c file as the function is growing big, but that's true of the other URB helpers as well, so it can be done later in a separate patch. Reviewed-by: Laurent Pinchart > extern void usb_init_urb(struct urb *urb); > extern struct urb *usb_alloc_urb(int iso_packets, gfp_t mem_flags); > extern void usb_free_urb(struct urb *urb); -- Regards, Laurent Pinchart