From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lu Baolu Subject: Re: [PATCH v17 3/4] usb: gadget: Integrate with the usb gadget supporting for usb charger Date: Tue, 11 Oct 2016 12:32:17 +0800 Message-ID: <57FC6B51.9030205@linux.intel.com> References: <700f8d4fc3c52418eeba5445f6822093cc9ff036.1476000169.git.baolin.wang@linaro.org> <57FC573C.2060301@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Baolin Wang 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 List-Id: linux-pm@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 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 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