lkml.org 
[lkml]   [2017]   [Dec]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/7] watchdog: jz4740: Use devm_* functions


Le jeu. 28 déc. 2017 à 21:19, Guenter Roeck <linux@roeck-us.net> a
écrit :
> On 12/28/2017 11:59 AM, Paul Cercueil wrote:
>> Hi Guenter,
>>
>> Le jeu. 28 déc. 2017 à 18:48, Guenter Roeck <linux@roeck-us.net> a
>> écrit :
>>> 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 <paul@crapouillou.net>
>>>> ---
>>>> 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
>>
>> This patch does not change the behaviour. We always used that driver
>> built-in; what
>> should the behaviour be when unloading the module? Keep the watchdog
>> hardware running
>> if configured for 'nowayout'?
>>
>
> One can still unload the driver. If you are fine with
> bypassing/dfeating nowayout
> and WDIOF_MAGICCLOSE, may I ask why those are enabled in the first
> place ?
>

Who knows? That code is very old :)
I'm fine with removing the remove() function completely, if you think
it makes more sense.

Thanks,
-Paul

\
 
 \ /
  Last update: 2017-12-28 21:22    [W:0.052 / U:0.596 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site