From mboxrd@z Thu Jan 1 00:00:00 1970 From: Guenter Roeck Subject: Re: [PATCH 2/7] watchdog: jz4740: Use devm_* functions Date: Thu, 28 Dec 2017 09:48:09 -0800 Message-ID: <9778afd4-5841-0d48-cde3-c02872623a5f@roeck-us.net> References: <20171228162939.3928-1-paul@crapouillou.net> <20171228162939.3928-3-paul@crapouillou.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20171228162939.3928-3-paul@crapouillou.net> Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org To: Paul Cercueil , Ralf Baechle , Rob Herring , Mark Rutland , Wim Van Sebroeck Cc: devicetree@vger.kernel.org, linux-mips@linux-mips.org, linux-kernel@vger.kernel.org, linux-watchdog@vger.kernel.org List-Id: devicetree@vger.kernel.org On 12/28/2017 08:29 AM, Paul Cercueil wrote: > - Use devm_clk_get instead of clk_get > - Use devm_watchdog_register_device instead of watchdog_register_device > > Signed-off-by: Paul Cercueil > --- > drivers/watchdog/jz4740_wdt.c | 27 ++++++++------------------- > 1 file changed, 8 insertions(+), 19 deletions(-) > > diff --git a/drivers/watchdog/jz4740_wdt.c b/drivers/watchdog/jz4740_wdt.c > index 6955deb100ef..92d6ca8ceb49 100644 > --- a/drivers/watchdog/jz4740_wdt.c > +++ b/drivers/watchdog/jz4740_wdt.c > @@ -178,40 +178,29 @@ static int jz4740_wdt_probe(struct platform_device *pdev) > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > drvdata->base = devm_ioremap_resource(&pdev->dev, res); > - if (IS_ERR(drvdata->base)) { > - ret = PTR_ERR(drvdata->base); > - goto err_out; > - } > + if (IS_ERR(drvdata->base)) > + return PTR_ERR(drvdata->base); > > - drvdata->rtc_clk = clk_get(&pdev->dev, "rtc"); > + drvdata->rtc_clk = devm_clk_get(&pdev->dev, "rtc"); > if (IS_ERR(drvdata->rtc_clk)) { > dev_err(&pdev->dev, "cannot find RTC clock\n"); > - ret = PTR_ERR(drvdata->rtc_clk); > - goto err_out; > + return PTR_ERR(drvdata->rtc_clk); > } > > - ret = watchdog_register_device(&drvdata->wdt); > + ret = devm_watchdog_register_device(&pdev->dev, &drvdata->wdt); > if (ret < 0) > - goto err_disable_clk; > + return ret; > > platform_set_drvdata(pdev, drvdata); > - return 0; > > -err_disable_clk: > - clk_put(drvdata->rtc_clk); > -err_out: > - return ret; > + return 0; > } > > static int jz4740_wdt_remove(struct platform_device *pdev) > { > struct jz4740_wdt_drvdata *drvdata = platform_get_drvdata(pdev); > > - jz4740_wdt_stop(&drvdata->wdt); > - watchdog_unregister_device(&drvdata->wdt); > - clk_put(drvdata->rtc_clk); > - > - return 0; > + return jz4740_wdt_stop(&drvdata->wdt); If the watchdog is running, the module can not be unloaded. Even if that wasn't the case, this defeats both WDIOF_MAGICCLOSE and watchdog_set_nowayout(). Are you sure this is what you want ? If so, please call watchdog_stop_on_unregister() before registration; this clarifies that this is what you want, and you can drop the remove function. Thanks, Guenter > } > > static struct platform_driver jz4740_wdt_driver = { >