From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757715AbcEFGNi (ORCPT ); Fri, 6 May 2016 02:13:38 -0400 Received: from mailout3.w1.samsung.com ([210.118.77.13]:23253 "EHLO mailout3.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751028AbcEFGNf (ORCPT ); Fri, 6 May 2016 02:13:35 -0400 X-AuditID: cbfec7f4-f796c6d000001486-56-572c360a6db5 Subject: Re: [RFC v2 01/13] usb: misc: usb3503: Clean up on driver unbind To: Javier Martinez Canillas , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-mmc@vger.kernel.org, linux-pm@vger.kernel.org, linux-usb@vger.kernel.org, Ulf Hansson , Sebastian Reichel , Dmitry Eremin-Solenikov , David Woodhouse , Greg Kroah-Hartman , Mark Brown References: <1462451666-17945-1-git-send-email-k.kozlowski@samsung.com> <1462451666-17945-2-git-send-email-k.kozlowski@samsung.com> Cc: tjakobi@math.uni-bielefeld.de, m.szyprowski@samsung.com, hverkuil@xs4all.nl, Bartlomiej Zolnierkiewicz From: Krzysztof Kozlowski Message-id: <572C3608.9060600@samsung.com> Date: Fri, 06 May 2016 08:13:28 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-version: 1.0 In-reply-to: Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrAIsWRmVeSWpSXmKPExsVy+t/xq7pcZjrhBnM+GlpsnLGe1WLqwyds FpOevGe2mH/kHKvFxJWTmS2aF69nszg1+RmTxZu3a5gsXr8wtNj0+BqrxeVdc9gsjvzvZ7T4 3HuE0WLG+X1MFouWtTJbrD1yl93i9O4Si7bVH1gtjq8NdxD22DnrLrvH5hVaHptWdbJ53Lm2 h81j/9w1QKEl9R7/jrF7bOkHyvdtWcXo8XmTnMepr5/ZA7ijuGxSUnMyy1KL9O0SuDImHNnJ WrBTuKKzObyB8SB/FyMnh4SAicTfT/NZIGwxiQv31rN1MXJxCAksZZSYcOkJM4TzjFFi0vFG dpAqYQFPiYkvNzKCJEQEelkkLpw7zwRRtY9RYuG05awgDrNAO6PE1lO72UBa2ASMJTYvXwJm 8wpoSVycPYEJxGYRUJWY1NUAZosKREisXneNGaJGUOLH5HtgR3EKOEu0v5gMtI4DaKiexP2L WiBhZgF5ic1r3jJPYBSYhaRjFkLVLCRVCxiZVzGKppYmFxQnpeca6hUn5haX5qXrJefnbmKE xOOXHYyLj1kdYhTgYFTi4c04qR0uxJpYVlyZe4hRgoNZSYS32VgnXIg3JbGyKrUoP76oNCe1 +BCjNAeLkjjv3F3vQ4QE0hNLUrNTUwtSi2CyTBycUg2MYlr2p+845b6/vveQ3P6L8bct7Fe8 y69325+yqq5IwnKlg71ff94zRvnL2j4njiv9a9r17b6+4tbDhwNsRMLyrv3lPpy594tz/kv1 iPMJq4XWim18OT1ZIO+cz+OE2M5Xp/vORFXlFrc/fBykojBh97uIMxPdj/BLVFy78uhd1BGl umBrC+0DSizFGYmGWsxFxYkAayenfcMCAAA= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/05/2016 08:32 PM, Javier Martinez Canillas wrote: > Hello Krzysztof > > Patch looks good to me, I just have a trivial comment below. > > On 05/05/2016 08:34 AM, Krzysztof Kozlowski wrote: >> The driver should clean up after itself by unpreparing the clock when it >> is unbound. >> >> Signed-off-by: Krzysztof Kozlowski >> --- >> drivers/usb/misc/usb3503.c | 28 ++++++++++++++++++++++++++++ >> 1 file changed, 28 insertions(+) >> >> diff --git a/drivers/usb/misc/usb3503.c b/drivers/usb/misc/usb3503.c >> index b45cb77c0744..a464636675a6 100644 >> --- a/drivers/usb/misc/usb3503.c >> +++ b/drivers/usb/misc/usb3503.c >> @@ -330,6 +330,19 @@ static int usb3503_i2c_probe(struct i2c_client *i2c, >> return usb3503_probe(hub); >> } >> >> +static int usb3503_i2c_remove(struct i2c_client *i2c) >> +{ >> + struct usb3503 *hub; >> + >> + hub = i2c_get_clientdata(i2c); >> + if (hub) { >> + if (hub->clk) >> + clk_disable_unprepare(hub->clk); >> + } > > I think the following is simpler and easier to read: > > if (hub && hub->clk) > clk_disable_unprepare(hub->clk); > Sure, thanks. Best regards, Krzysztof >> + >> + return 0; >> +} >> + >> static int usb3503_platform_probe(struct platform_device *pdev) >> { >> struct usb3503 *hub; >> @@ -342,6 +355,19 @@ static int usb3503_platform_probe(struct platform_device *pdev) >> return usb3503_probe(hub); >> } >> >> +static int usb3503_platform_remove(struct platform_device *pdev) >> +{ >> + struct usb3503 *hub; >> + >> + hub = platform_get_drvdata(pdev); >> + if (hub) { >> + if (hub->clk) >> + clk_disable_unprepare(hub->clk); >> + } >> + > > Ditto. > >> + return 0; >> +} >> + >> #ifdef CONFIG_PM_SLEEP >> static int usb3503_i2c_suspend(struct device *dev) >> { >> @@ -395,6 +421,7 @@ static struct i2c_driver usb3503_i2c_driver = { >> .of_match_table = of_match_ptr(usb3503_of_match), >> }, >> .probe = usb3503_i2c_probe, >> + .remove = usb3503_i2c_remove, >> .id_table = usb3503_id, >> }; >> >> @@ -404,6 +431,7 @@ static struct platform_driver usb3503_platform_driver = { >> .of_match_table = of_match_ptr(usb3503_of_match), >> }, >> .probe = usb3503_platform_probe, >> + .remove = usb3503_platform_remove, >> }; >> >> static int __init usb3503_init(void) >> > > Reviewed-by: Javier Martinez Canillas > > Best regards, >