Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752017AbcJKEc2 (ORCPT ); Tue, 11 Oct 2016 00:32:28 -0400 Received: from mga01.intel.com ([192.55.52.88]:34744 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751123AbcJKEc0 (ORCPT ); Tue, 11 Oct 2016 00:32:26 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,476,1473145200"; d="scan'208";a="1068766794" Subject: Re: [PATCH v17 3/4] usb: gadget: Integrate with the usb gadget supporting for usb charger To: Baolin Wang References: <700f8d4fc3c52418eeba5445f6822093cc9ff036.1476000169.git.baolin.wang@linaro.org> <57FC573C.2060301@linux.intel.com> Cc: Felipe Balbi , Greg KH , Sebastian Reichel , Dmitry Eremin-Solenikov , David Woodhouse , robh@kernel.org, Jun Li , Marek Szyprowski , Ruslan Bilovol , Peter Chen , Alan Stern , r.baldyga@samsung.com, grygorii.strashko@ti.com, Yoshihiro Shimoda , Lee Jones , Mark Brown , John Stultz , Charles Keepax , patches@opensource.wolfsonmicro.com, Linux PM list , USB , device-mainlining@lists.linuxfoundation.org, LKML From: Lu Baolu Message-ID: <57FC6B51.9030205@linux.intel.com> Date: Tue, 11 Oct 2016 12:32:17 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1100 Lines: 33 Hi, On 10/11/2016 12:07 PM, Baolin Wang wrote: >>> /* >>> >> + * usb_charger_unregister() - Unregister a usb charger. >>> >> + * @uchger - the usb charger to be unregistered. >>> >> + */ >>> >> +static int usb_charger_unregister(struct usb_charger *uchger) >>> >> +{ >>> >> + ida_simple_remove(&usb_charger_ida, uchger->id); >>> >> + sysfs_remove_groups(&uchger->gadget->dev.kobj, usb_charger_groups); >>> >> + >>> >> + mutex_lock(&charger_lock); >>> >> + list_del(&uchger->list); >>> >> + mutex_unlock(&charger_lock); >>> >> + >>> >> + kfree(uchger); >> > >> > Any reasons you want to put kfree() here? You allocated the memory >> > in usb_charger_init(). Isn't usb_charger_exit() a better place? > The usb_charger_exit() will issue usb_charger_unregister() and we need > use the usb_charger structure in usb_charger_unregister() to > unregister the usb charger. > This seems not to be a strong reason. :-) You can unregister the charger first and then free the structure. Just do the reverse operation of what you have done in the init function. Best regards, Lu Baolu