* [PATCH] watchdog: dw: use devm_watchdog_register_device()
@ 2019-01-25 7:52 Jisheng Zhang
2019-01-25 8:04 ` Guenter Roeck
0 siblings, 1 reply; 4+ messages in thread
From: Jisheng Zhang @ 2019-01-25 7:52 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck; +Cc: linux-watchdog, linux-kernel
Use devm_watchdog_register_device() to simplify the code.
Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
drivers/watchdog/dw_wdt.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
index 501aebb5b81f..c053c2de5c2f 100644
--- a/drivers/watchdog/dw_wdt.c
+++ b/drivers/watchdog/dw_wdt.c
@@ -303,7 +303,7 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
watchdog_set_restart_priority(wdd, 128);
- ret = watchdog_register_device(wdd);
+ ret = devm_watchdog_register_device(wdd);
if (ret)
goto out_disable_clk;
@@ -318,7 +318,6 @@ static int dw_wdt_drv_remove(struct platform_device *pdev)
{
struct dw_wdt *dw_wdt = platform_get_drvdata(pdev);
- watchdog_unregister_device(&dw_wdt->wdd);
reset_control_assert(dw_wdt->rst);
clk_disable_unprepare(dw_wdt->clk);
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] watchdog: dw: use devm_watchdog_register_device()
2019-01-25 7:52 [PATCH] watchdog: dw: use devm_watchdog_register_device() Jisheng Zhang
@ 2019-01-25 8:04 ` Guenter Roeck
2019-01-25 8:17 ` Jisheng Zhang
0 siblings, 1 reply; 4+ messages in thread
From: Guenter Roeck @ 2019-01-25 8:04 UTC (permalink / raw)
To: Jisheng Zhang, Wim Van Sebroeck; +Cc: linux-watchdog, linux-kernel
Hi,
On 1/24/19 11:52 PM, Jisheng Zhang wrote:
> Use devm_watchdog_register_device() to simplify the code.
>
> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> ---
> drivers/watchdog/dw_wdt.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
> index 501aebb5b81f..c053c2de5c2f 100644
> --- a/drivers/watchdog/dw_wdt.c
> +++ b/drivers/watchdog/dw_wdt.c
> @@ -303,7 +303,7 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
>
> watchdog_set_restart_priority(wdd, 128);
>
> - ret = watchdog_register_device(wdd);
> + ret = devm_watchdog_register_device(wdd);
> if (ret)
> goto out_disable_clk;
>
> @@ -318,7 +318,6 @@ static int dw_wdt_drv_remove(struct platform_device *pdev)
> {
> struct dw_wdt *dw_wdt = platform_get_drvdata(pdev);
>
> - watchdog_unregister_device(&dw_wdt->wdd);
> reset_control_assert(dw_wdt->rst);
> clk_disable_unprepare(dw_wdt->clk);
>
>
Unfortunately it isn't that easy. The other two calls have to be executed after
unregistering the watchdog, meaning you would have to add devm_add_action()
in the probe function to call them.
Guenter
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] watchdog: dw: use devm_watchdog_register_device()
2019-01-25 8:04 ` Guenter Roeck
@ 2019-01-25 8:17 ` Jisheng Zhang
2019-01-25 9:01 ` Guenter Roeck
0 siblings, 1 reply; 4+ messages in thread
From: Jisheng Zhang @ 2019-01-25 8:17 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Wim Van Sebroeck, linux-watchdog, linux-kernel
Hi,
On Fri, 25 Jan 2019 00:04:25 -0800 Guenter Roeck wrote:
> Hi,
>
> On 1/24/19 11:52 PM, Jisheng Zhang wrote:
> > Use devm_watchdog_register_device() to simplify the code.
> >
> > Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> > ---
> > drivers/watchdog/dw_wdt.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
> > index 501aebb5b81f..c053c2de5c2f 100644
> > --- a/drivers/watchdog/dw_wdt.c
> > +++ b/drivers/watchdog/dw_wdt.c
> > @@ -303,7 +303,7 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
> >
> > watchdog_set_restart_priority(wdd, 128);
> >
> > - ret = watchdog_register_device(wdd);
> > + ret = devm_watchdog_register_device(wdd);
> > if (ret)
> > goto out_disable_clk;
> >
> > @@ -318,7 +318,6 @@ static int dw_wdt_drv_remove(struct platform_device *pdev)
> > {
> > struct dw_wdt *dw_wdt = platform_get_drvdata(pdev);
> >
> > - watchdog_unregister_device(&dw_wdt->wdd);
> > reset_control_assert(dw_wdt->rst);
> > clk_disable_unprepare(dw_wdt->clk);
> >
> >
> Unfortunately it isn't that easy. The other two calls have to be executed after
> unregistering the watchdog, meaning you would have to add devm_add_action()
> in the probe function to call them.
do you mean reset_control_asser() and the clk_disable_unprepare()? If yes,
does unregister the watchdog could trigger any register access? Per my
understanding, there's no register access path in the unregister. Am I
missing somthing?
Thanks
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] watchdog: dw: use devm_watchdog_register_device()
2019-01-25 8:17 ` Jisheng Zhang
@ 2019-01-25 9:01 ` Guenter Roeck
0 siblings, 0 replies; 4+ messages in thread
From: Guenter Roeck @ 2019-01-25 9:01 UTC (permalink / raw)
To: Jisheng Zhang; +Cc: Wim Van Sebroeck, linux-watchdog, linux-kernel
On 1/25/19 12:17 AM, Jisheng Zhang wrote:
> Hi,
>
> On Fri, 25 Jan 2019 00:04:25 -0800 Guenter Roeck wrote:
>
>> Hi,
>>
>> On 1/24/19 11:52 PM, Jisheng Zhang wrote:
>>> Use devm_watchdog_register_device() to simplify the code.
>>>
>>> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
>>> ---
>>> drivers/watchdog/dw_wdt.c | 3 +--
>>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
>>> index 501aebb5b81f..c053c2de5c2f 100644
>>> --- a/drivers/watchdog/dw_wdt.c
>>> +++ b/drivers/watchdog/dw_wdt.c
>>> @@ -303,7 +303,7 @@ static int dw_wdt_drv_probe(struct platform_device *pdev)
>>>
>>> watchdog_set_restart_priority(wdd, 128);
>>>
>>> - ret = watchdog_register_device(wdd);
>>> + ret = devm_watchdog_register_device(wdd);
>>> if (ret)
>>> goto out_disable_clk;
>>>
>>> @@ -318,7 +318,6 @@ static int dw_wdt_drv_remove(struct platform_device *pdev)
>>> {
>>> struct dw_wdt *dw_wdt = platform_get_drvdata(pdev);
>>>
>>> - watchdog_unregister_device(&dw_wdt->wdd);
>>> reset_control_assert(dw_wdt->rst);
>>> clk_disable_unprepare(dw_wdt->clk);
>>>
>>>
>> Unfortunately it isn't that easy. The other two calls have to be executed after
>> unregistering the watchdog, meaning you would have to add devm_add_action()
>> in the probe function to call them.
>
> do you mean reset_control_asser() and the clk_disable_unprepare()? If yes,
Yes.
> does unregister the watchdog could trigger any register access? Per my
> understanding, there's no register access path in the unregister. Am I
> missing somthing?
>
If the watchdog is unregistered last, it would be still accessible via its device
nodes, even though the clock has been reset, and even though the HW has been
put in reset. This would be racy.
Guenter
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-01-25 9:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-25 7:52 [PATCH] watchdog: dw: use devm_watchdog_register_device() Jisheng Zhang
2019-01-25 8:04 ` Guenter Roeck
2019-01-25 8:17 ` Jisheng Zhang
2019-01-25 9:01 ` Guenter Roeck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).