From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on archive.lwn.net X-Spam-Level: X-Spam-Status: No, score=-5.6 required=5.0 tests=DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI, T_DKIM_INVALID autolearn=unavailable autolearn_force=no version=3.4.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by archive.lwn.net (Postfix) with ESMTP id D7C9F7D09E for ; Wed, 30 May 2018 11:31:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751672AbeE3Lau (ORCPT ); Wed, 30 May 2018 07:30:50 -0400 Received: from mail.kernel.org ([198.145.29.99]:45746 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751547AbeE3Lar (ORCPT ); Wed, 30 May 2018 07:30:47 -0400 Received: from localhost (LFbn-1-12247-202.w90-92.abo.wanadoo.fr [90.92.61.202]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 79C7B20851; Wed, 30 May 2018 11:30:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1527679847; bh=2KKvMPReIb2P8qOgPif5jlrH5v8UmrPYk21BY6RKcb0=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=lqLXMZbIRAj0cEqBNfjcRvfcLSzq11YaMktCy05tksrPSKwYSTN9CYWf3MJRZnGaQ 8v1yzB7JkQlfNX/UWbXzLKvsgU/bc/a3KO1j7Af+QOUMX30PCzB8NknT5k7irgFKFt yiNJgZZ/WTP1FrQBpNyJvjw88vXL+MlaJ6CGjfX0= Date: Wed, 30 May 2018 13:30:26 +0200 From: Greg Kroah-Hartman To: Marcus Folkesson Cc: Andy Shevchenko , Jonathan Corbet , Felipe Balbi , "David S. Miller" , Mauro Carvalho Chehab , Andrew Morton , Randy Dunlap , Ruslan Bilovol , Thomas Gleixner , Kate Stewart , USB , Linux Documentation List , Linux Kernel Mailing List Subject: Re: [PATCH v3 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device Message-ID: <20180530113026.GA20775@kroah.com> References: <20180529185021.13738-1-marcus.folkesson@gmail.com> <20180530112459.GB2939@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180530112459.GB2939@gmail.com> User-Agent: Mutt/1.10.0 (2018-05-17) Sender: linux-doc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-doc@vger.kernel.org On Wed, May 30, 2018 at 01:24:59PM +0200, Marcus Folkesson wrote: > Hi Andy, > > Thank you for your comments! > Many good catches here! > > On Wed, May 30, 2018 at 03:55:39AM +0300, Andy Shevchenko wrote: > > On Tue, May 29, 2018 at 9:50 PM, Marcus Folkesson > > wrote: > > > Chip Card Interface Device (CCID) protocol is a USB protocol that > > > allows a smartcard device to be connected to a computer via a card > > > reader using a standard USB interface, without the need for each manufacturer > > > of smartcards to provide its own reader or protocol. > > > > > > This gadget driver makes Linux show up as a CCID device to the host and let a > > > userspace daemon act as the smartcard. > > > > > > This is useful when the Linux gadget itself should act as a cryptographic > > > device or forward APDUs to an embedded smartcard device. > > > > > + * Copyright (C) 2018 Marcus Folkesson > > > > > + * > > > > Redundant line > > > > Yep > > > > +static DEFINE_IDA(ccidg_ida); > > > > Where is it destroyed? > > Hm, I'm not sure it needs to be destroyed. From lib/idr.c: > > * You can also use ida_get_new_above() if you need an ID to be allocated > * above a particular number. ida_destroy() can be used to dispose of an > * IDA without needing to free the individual IDs in it. You can use > * ida_is_empty() to find out whether the IDA has any IDs currently allocated. > > > An empty ccidg_ida is the indication that we should clean up our > mess: > > static void ccidg_free_inst(struct usb_function_instance *f) > ... > if (ida_is_empty(&ccidg_ida)) > ccidg_cleanup(); > > If the IDA is empty, should I call ida_destroy() anyway? > Other similiar drivers does not seems to do that. > > I must say that I'm not very familiar with the IDA API. When your module is removed, you need to clean up any remaining memory that the ida used. It's not obvious at all, and is a pain as you would think that if you statically allocate one, like you have here, it would not be needed. You need to just call: ida_destroy(&ccidg_ida); in your module exit function. Hope this helps, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html 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: [v3,1/3] usb: gadget: ccid: add support for USB CCID Gadget Device From: Greg Kroah-Hartman Message-Id: <20180530113026.GA20775@kroah.com> Date: Wed, 30 May 2018 13:30:26 +0200 To: Marcus Folkesson Cc: Andy Shevchenko , Jonathan Corbet , Felipe Balbi , "David S. Miller" , Mauro Carvalho Chehab , Andrew Morton , Randy Dunlap , Ruslan Bilovol , Thomas Gleixner , Kate Stewart , USB , Linux Documentation List , Linux Kernel Mailing List List-ID: T24gV2VkLCBNYXkgMzAsIDIwMTggYXQgMDE6MjQ6NTlQTSArMDIwMCwgTWFyY3VzIEZvbGtlc3Nv biB3cm90ZToKPiBIaSBBbmR5LAo+IAo+IFRoYW5rIHlvdSBmb3IgeW91ciBjb21tZW50cyEKPiBN YW55IGdvb2QgY2F0Y2hlcyBoZXJlIQo+IAo+IE9uIFdlZCwgTWF5IDMwLCAyMDE4IGF0IDAzOjU1 OjM5QU0gKzAzMDAsIEFuZHkgU2hldmNoZW5rbyB3cm90ZToKPiA+IE9uIFR1ZSwgTWF5IDI5LCAy MDE4IGF0IDk6NTAgUE0sIE1hcmN1cyBGb2xrZXNzb24KPiA+IDxtYXJjdXMuZm9sa2Vzc29uQGdt YWlsLmNvbT4gd3JvdGU6Cj4gPiA+IENoaXAgQ2FyZCBJbnRlcmZhY2UgRGV2aWNlIChDQ0lEKSBw cm90b2NvbCBpcyBhIFVTQiBwcm90b2NvbCB0aGF0Cj4gPiA+IGFsbG93cyBhIHNtYXJ0Y2FyZCBk ZXZpY2UgdG8gYmUgY29ubmVjdGVkIHRvIGEgY29tcHV0ZXIgdmlhIGEgY2FyZAo+ID4gPiByZWFk ZXIgdXNpbmcgYSBzdGFuZGFyZCBVU0IgaW50ZXJmYWNlLCB3aXRob3V0IHRoZSBuZWVkIGZvciBl YWNoIG1hbnVmYWN0dXJlcgo+ID4gPiBvZiBzbWFydGNhcmRzIHRvIHByb3ZpZGUgaXRzIG93biBy ZWFkZXIgb3IgcHJvdG9jb2wuCj4gPiA+Cj4gPiA+IFRoaXMgZ2FkZ2V0IGRyaXZlciBtYWtlcyBM aW51eCBzaG93IHVwIGFzIGEgQ0NJRCBkZXZpY2UgdG8gdGhlIGhvc3QgYW5kIGxldCBhCj4gPiA+ IHVzZXJzcGFjZSBkYWVtb24gYWN0IGFzIHRoZSBzbWFydGNhcmQuCj4gPiA+Cj4gPiA+IFRoaXMg aXMgdXNlZnVsIHdoZW4gdGhlIExpbnV4IGdhZGdldCBpdHNlbGYgc2hvdWxkIGFjdCBhcyBhIGNy eXB0b2dyYXBoaWMKPiA+ID4gZGV2aWNlIG9yIGZvcndhcmQgQVBEVXMgdG8gYW4gZW1iZWRkZWQg c21hcnRjYXJkIGRldmljZS4KPiA+IAo+ID4gPiArICogQ29weXJpZ2h0IChDKSAyMDE4IE1hcmN1 cyBGb2xrZXNzb24gPG1hcmN1cy5mb2xrZXNzb25AZ21haWwuY29tPgo+ID4gCj4gPiA+ICsgKgo+ ID4gCj4gPiBSZWR1bmRhbnQgbGluZQo+ID4gCj4gCj4gWWVwCj4gCj4gPiA+ICtzdGF0aWMgREVG SU5FX0lEQShjY2lkZ19pZGEpOwo+ID4gCj4gPiBXaGVyZSBpcyBpdCBkZXN0cm95ZWQ/Cj4gCj4g SG0sIEknbSBub3Qgc3VyZSBpdCBuZWVkcyB0byBiZSBkZXN0cm95ZWQuIEZyb20gbGliL2lkci5j Ogo+IAo+ICAqIFlvdSBjYW4gYWxzbyB1c2UgaWRhX2dldF9uZXdfYWJvdmUoKSBpZiB5b3UgbmVl ZCBhbiBJRCB0byBiZSBhbGxvY2F0ZWQKPiAgKiBhYm92ZSBhIHBhcnRpY3VsYXIgbnVtYmVyLiAg aWRhX2Rlc3Ryb3koKSBjYW4gYmUgdXNlZCB0byBkaXNwb3NlIG9mIGFuCj4gICogSURBIHdpdGhv dXQgbmVlZGluZyB0byBmcmVlIHRoZSBpbmRpdmlkdWFsIElEcyBpbiBpdC4gIFlvdSBjYW4gdXNl Cj4gICogaWRhX2lzX2VtcHR5KCkgdG8gZmluZCBvdXQgd2hldGhlciB0aGUgSURBIGhhcyBhbnkg SURzIGN1cnJlbnRseSBhbGxvY2F0ZWQuCj4gCj4gCj4gQW4gZW1wdHkgY2NpZGdfaWRhIGlzIHRo ZSBpbmRpY2F0aW9uIHRoYXQgd2Ugc2hvdWxkIGNsZWFuIHVwIG91cgo+IG1lc3M6Cj4gCj4gc3Rh dGljIHZvaWQgY2NpZGdfZnJlZV9pbnN0KHN0cnVjdCB1c2JfZnVuY3Rpb25faW5zdGFuY2UgKmYp Cj4gLi4uCj4gCWlmIChpZGFfaXNfZW1wdHkoJmNjaWRnX2lkYSkpCj4gCQljY2lkZ19jbGVhbnVw KCk7Cj4gCj4gSWYgdGhlIElEQSBpcyBlbXB0eSwgc2hvdWxkIEkgY2FsbCBpZGFfZGVzdHJveSgp IGFueXdheT8KPiBPdGhlciBzaW1pbGlhciBkcml2ZXJzIGRvZXMgbm90IHNlZW1zIHRvIGRvIHRo YXQuCj4gCj4gSSBtdXN0IHNheSB0aGF0IEknbSBub3QgdmVyeSBmYW1pbGlhciB3aXRoIHRoZSBJ REEgQVBJLgoKV2hlbiB5b3VyIG1vZHVsZSBpcyByZW1vdmVkLCB5b3UgbmVlZCB0byBjbGVhbiB1 cCBhbnkgcmVtYWluaW5nIG1lbW9yeQp0aGF0IHRoZSBpZGEgdXNlZC4gIEl0J3Mgbm90IG9idmlv dXMgYXQgYWxsLCBhbmQgaXMgYSBwYWluIGFzIHlvdSB3b3VsZAp0aGluayB0aGF0IGlmIHlvdSBz dGF0aWNhbGx5IGFsbG9jYXRlIG9uZSwgbGlrZSB5b3UgaGF2ZSBoZXJlLCBpdCB3b3VsZApub3Qg YmUgbmVlZGVkLiAgWW91IG5lZWQgdG8ganVzdCBjYWxsOgoJaWRhX2Rlc3Ryb3koJmNjaWRnX2lk YSk7CmluIHlvdXIgbW9kdWxlIGV4aXQgZnVuY3Rpb24uCgpIb3BlIHRoaXMgaGVscHMsCgpncmVn IGstaAotLS0KVG8gdW5zdWJzY3JpYmUgZnJvbSB0aGlzIGxpc3Q6IHNlbmQgdGhlIGxpbmUgInVu c3Vic2NyaWJlIGxpbnV4LXVzYiIgaW4KdGhlIGJvZHkgb2YgYSBtZXNzYWdlIHRvIG1ham9yZG9t b0B2Z2VyLmtlcm5lbC5vcmcKTW9yZSBtYWpvcmRvbW8gaW5mbyBhdCAgaHR0cDovL3ZnZXIua2Vy bmVsLm9yZy9tYWpvcmRvbW8taW5mby5odG1sCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753014AbeE3Lav (ORCPT ); Wed, 30 May 2018 07:30:51 -0400 Received: from mail.kernel.org ([198.145.29.99]:45746 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751547AbeE3Lar (ORCPT ); Wed, 30 May 2018 07:30:47 -0400 Date: Wed, 30 May 2018 13:30:26 +0200 From: Greg Kroah-Hartman To: Marcus Folkesson Cc: Andy Shevchenko , Jonathan Corbet , Felipe Balbi , "David S. Miller" , Mauro Carvalho Chehab , Andrew Morton , Randy Dunlap , Ruslan Bilovol , Thomas Gleixner , Kate Stewart , USB , Linux Documentation List , Linux Kernel Mailing List Subject: Re: [PATCH v3 1/3] usb: gadget: ccid: add support for USB CCID Gadget Device Message-ID: <20180530113026.GA20775@kroah.com> References: <20180529185021.13738-1-marcus.folkesson@gmail.com> <20180530112459.GB2939@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180530112459.GB2939@gmail.com> User-Agent: Mutt/1.10.0 (2018-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, May 30, 2018 at 01:24:59PM +0200, Marcus Folkesson wrote: > Hi Andy, > > Thank you for your comments! > Many good catches here! > > On Wed, May 30, 2018 at 03:55:39AM +0300, Andy Shevchenko wrote: > > On Tue, May 29, 2018 at 9:50 PM, Marcus Folkesson > > wrote: > > > Chip Card Interface Device (CCID) protocol is a USB protocol that > > > allows a smartcard device to be connected to a computer via a card > > > reader using a standard USB interface, without the need for each manufacturer > > > of smartcards to provide its own reader or protocol. > > > > > > This gadget driver makes Linux show up as a CCID device to the host and let a > > > userspace daemon act as the smartcard. > > > > > > This is useful when the Linux gadget itself should act as a cryptographic > > > device or forward APDUs to an embedded smartcard device. > > > > > + * Copyright (C) 2018 Marcus Folkesson > > > > > + * > > > > Redundant line > > > > Yep > > > > +static DEFINE_IDA(ccidg_ida); > > > > Where is it destroyed? > > Hm, I'm not sure it needs to be destroyed. From lib/idr.c: > > * You can also use ida_get_new_above() if you need an ID to be allocated > * above a particular number. ida_destroy() can be used to dispose of an > * IDA without needing to free the individual IDs in it. You can use > * ida_is_empty() to find out whether the IDA has any IDs currently allocated. > > > An empty ccidg_ida is the indication that we should clean up our > mess: > > static void ccidg_free_inst(struct usb_function_instance *f) > ... > if (ida_is_empty(&ccidg_ida)) > ccidg_cleanup(); > > If the IDA is empty, should I call ida_destroy() anyway? > Other similiar drivers does not seems to do that. > > I must say that I'm not very familiar with the IDA API. When your module is removed, you need to clean up any remaining memory that the ida used. It's not obvious at all, and is a pain as you would think that if you statically allocate one, like you have here, it would not be needed. You need to just call: ida_destroy(&ccidg_ida); in your module exit function. Hope this helps, greg k-h