* [PATCH 1/3] watchdog: imx_sc_wdt: Remove unused includes
@ 2020-02-21 2:00 Anson Huang
2020-02-21 2:00 ` [PATCH 2/3] watchdog: imx7ulp: Remove unused include of init.h Anson Huang
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Anson Huang @ 2020-02-21 2:00 UTC (permalink / raw)
To: wim, linux, shawnguo, s.hauer, kernel, festevam, linux-watchdog,
linux-arm-kernel, linux-kernel
Cc: Linux-imx
There is nothing in use from init.h/reboot.h, remove them.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
drivers/watchdog/imx_sc_wdt.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/watchdog/imx_sc_wdt.c b/drivers/watchdog/imx_sc_wdt.c
index 8ed89f0..60a3246 100644
--- a/drivers/watchdog/imx_sc_wdt.c
+++ b/drivers/watchdog/imx_sc_wdt.c
@@ -6,13 +6,11 @@
#include <linux/arm-smccc.h>
#include <linux/firmware/imx/sci.h>
#include <linux/io.h>
-#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/moduleparam.h>
#include <linux/of.h>
#include <linux/platform_device.h>
-#include <linux/reboot.h>
#include <linux/watchdog.h>
#define DEFAULT_TIMEOUT 60
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/3] watchdog: imx7ulp: Remove unused include of init.h 2020-02-21 2:00 [PATCH 1/3] watchdog: imx_sc_wdt: Remove unused includes Anson Huang @ 2020-02-21 2:00 ` Anson Huang 2020-02-22 15:59 ` Guenter Roeck 2020-02-21 2:00 ` [PATCH 3/3] watchdog: imx2_wdt: " Anson Huang 2020-02-22 15:55 ` [PATCH 1/3] watchdog: imx_sc_wdt: Remove unused includes Guenter Roeck 2 siblings, 1 reply; 9+ messages in thread From: Anson Huang @ 2020-02-21 2:00 UTC (permalink / raw) To: wim, linux, shawnguo, s.hauer, kernel, festevam, linux-watchdog, linux-arm-kernel, linux-kernel Cc: Linux-imx There is nothing in use from init.h, remove it. Signed-off-by: Anson Huang <Anson.Huang@nxp.com> --- drivers/watchdog/imx7ulp_wdt.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c index 11b9e7c..7993c8c 100644 --- a/drivers/watchdog/imx7ulp_wdt.c +++ b/drivers/watchdog/imx7ulp_wdt.c @@ -4,7 +4,6 @@ */ #include <linux/clk.h> -#include <linux/init.h> #include <linux/io.h> #include <linux/kernel.h> #include <linux/module.h> -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] watchdog: imx7ulp: Remove unused include of init.h 2020-02-21 2:00 ` [PATCH 2/3] watchdog: imx7ulp: Remove unused include of init.h Anson Huang @ 2020-02-22 15:59 ` Guenter Roeck 0 siblings, 0 replies; 9+ messages in thread From: Guenter Roeck @ 2020-02-22 15:59 UTC (permalink / raw) To: Anson Huang Cc: wim, shawnguo, s.hauer, kernel, festevam, linux-watchdog, linux-arm-kernel, linux-kernel, Linux-imx On Fri, Feb 21, 2020 at 10:00:29AM +0800, Anson Huang wrote: > There is nothing in use from init.h, remove it. > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com> Reviewed-by: Guenter Roeck <linux@roeck-us.net> > --- > drivers/watchdog/imx7ulp_wdt.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c > index 11b9e7c..7993c8c 100644 > --- a/drivers/watchdog/imx7ulp_wdt.c > +++ b/drivers/watchdog/imx7ulp_wdt.c > @@ -4,7 +4,6 @@ > */ > > #include <linux/clk.h> > -#include <linux/init.h> > #include <linux/io.h> > #include <linux/kernel.h> > #include <linux/module.h> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] watchdog: imx2_wdt: Remove unused include of init.h 2020-02-21 2:00 [PATCH 1/3] watchdog: imx_sc_wdt: Remove unused includes Anson Huang 2020-02-21 2:00 ` [PATCH 2/3] watchdog: imx7ulp: Remove unused include of init.h Anson Huang @ 2020-02-21 2:00 ` Anson Huang 2020-02-22 16:02 ` Guenter Roeck 2020-02-22 15:55 ` [PATCH 1/3] watchdog: imx_sc_wdt: Remove unused includes Guenter Roeck 2 siblings, 1 reply; 9+ messages in thread From: Anson Huang @ 2020-02-21 2:00 UTC (permalink / raw) To: wim, linux, shawnguo, s.hauer, kernel, festevam, linux-watchdog, linux-arm-kernel, linux-kernel Cc: Linux-imx There is nothing in use from init.h, remove it. Signed-off-by: Anson Huang <Anson.Huang@nxp.com> --- drivers/watchdog/imx2_wdt.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c index f8d58bf..ce28e40 100644 --- a/drivers/watchdog/imx2_wdt.c +++ b/drivers/watchdog/imx2_wdt.c @@ -20,7 +20,6 @@ #include <linux/clk.h> #include <linux/delay.h> -#include <linux/init.h> #include <linux/interrupt.h> #include <linux/io.h> #include <linux/kernel.h> -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] watchdog: imx2_wdt: Remove unused include of init.h 2020-02-21 2:00 ` [PATCH 3/3] watchdog: imx2_wdt: " Anson Huang @ 2020-02-22 16:02 ` Guenter Roeck 2020-02-23 0:16 ` Anson Huang 0 siblings, 1 reply; 9+ messages in thread From: Guenter Roeck @ 2020-02-22 16:02 UTC (permalink / raw) To: Anson Huang Cc: wim, shawnguo, s.hauer, kernel, festevam, linux-watchdog, linux-arm-kernel, linux-kernel, Linux-imx On Fri, Feb 21, 2020 at 10:00:30AM +0800, Anson Huang wrote: > There is nothing in use from init.h, remove it. > NACK, sorry; this driver uses __init and __exit_p. Guenter > Signed-off-by: Anson Huang <Anson.Huang@nxp.com> > --- > drivers/watchdog/imx2_wdt.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c > index f8d58bf..ce28e40 100644 > --- a/drivers/watchdog/imx2_wdt.c > +++ b/drivers/watchdog/imx2_wdt.c > @@ -20,7 +20,6 @@ > > #include <linux/clk.h> > #include <linux/delay.h> > -#include <linux/init.h> > #include <linux/interrupt.h> > #include <linux/io.h> > #include <linux/kernel.h> ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH 3/3] watchdog: imx2_wdt: Remove unused include of init.h 2020-02-22 16:02 ` Guenter Roeck @ 2020-02-23 0:16 ` Anson Huang 2020-02-23 14:08 ` Guenter Roeck 0 siblings, 1 reply; 9+ messages in thread From: Anson Huang @ 2020-02-23 0:16 UTC (permalink / raw) To: Guenter Roeck Cc: wim, shawnguo, s.hauer, kernel, festevam, linux-watchdog, linux-arm-kernel, linux-kernel, dl-linux-imx Hi, Guenter > Subject: Re: [PATCH 3/3] watchdog: imx2_wdt: Remove unused include of > init.h > > On Fri, Feb 21, 2020 at 10:00:30AM +0800, Anson Huang wrote: > > There is nothing in use from init.h, remove it. > > > > NACK, sorry; this driver uses __init and __exit_p. Ah, yes, just notice them. But I don't understand why the .probe callback needs __init and .remove callback needs __exit_p? Should they need to be removed? Thanks, Anson > Guenter > > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com> > > --- > > drivers/watchdog/imx2_wdt.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c > > index f8d58bf..ce28e40 100644 > > --- a/drivers/watchdog/imx2_wdt.c > > +++ b/drivers/watchdog/imx2_wdt.c > > @@ -20,7 +20,6 @@ > > > > #include <linux/clk.h> > > #include <linux/delay.h> > > -#include <linux/init.h> > > #include <linux/interrupt.h> > > #include <linux/io.h> > > #include <linux/kernel.h> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] watchdog: imx2_wdt: Remove unused include of init.h 2020-02-23 0:16 ` Anson Huang @ 2020-02-23 14:08 ` Guenter Roeck 2020-02-24 2:54 ` Anson Huang 0 siblings, 1 reply; 9+ messages in thread From: Guenter Roeck @ 2020-02-23 14:08 UTC (permalink / raw) To: Anson Huang Cc: wim, shawnguo, s.hauer, kernel, festevam, linux-watchdog, linux-arm-kernel, linux-kernel, dl-linux-imx On 2/22/20 4:16 PM, Anson Huang wrote: > Hi, Guenter > >> Subject: Re: [PATCH 3/3] watchdog: imx2_wdt: Remove unused include of >> init.h >> >> On Fri, Feb 21, 2020 at 10:00:30AM +0800, Anson Huang wrote: >>> There is nothing in use from init.h, remove it. >>> >> >> NACK, sorry; this driver uses __init and __exit_p. > > Ah, yes, just notice them. But I don't understand why the .probe callback needs > __init and .remove callback needs __exit_p? Should they need to be removed? > That is not a matter of "needs". __init causes the code to be removed after initialization. This is ok and desirable if it is known that the hardware is built-in and will only ever be probed once. exit_p causes the code to be removed if it is built into the kernel. This is desirable and makes sense if the device is known to never be removed. Having said that, what _is_ unnecessary is the remove function. Registration could use devm_watchdog_register_device(), and the watchdog subsystem should prevent removal if the watchdog is running. Plus, the removal function is buggy: It doesn' call clk_disable_unprepare() (but that could be handled with devm_add_action_or_reset() in the probe function). In my opinion, fixing all that would be more valuable than trying to drop an include file. Thanks, Guenter ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH 3/3] watchdog: imx2_wdt: Remove unused include of init.h 2020-02-23 14:08 ` Guenter Roeck @ 2020-02-24 2:54 ` Anson Huang 0 siblings, 0 replies; 9+ messages in thread From: Anson Huang @ 2020-02-24 2:54 UTC (permalink / raw) To: Guenter Roeck Cc: wim, shawnguo, s.hauer, kernel, festevam, linux-watchdog, linux-arm-kernel, linux-kernel, dl-linux-imx Hi, Guenter > Subject: Re: [PATCH 3/3] watchdog: imx2_wdt: Remove unused include of > init.h > > On 2/22/20 4:16 PM, Anson Huang wrote: > > Hi, Guenter > > > >> Subject: Re: [PATCH 3/3] watchdog: imx2_wdt: Remove unused include of > >> init.h > >> > >> On Fri, Feb 21, 2020 at 10:00:30AM +0800, Anson Huang wrote: > >>> There is nothing in use from init.h, remove it. > >>> > >> > >> NACK, sorry; this driver uses __init and __exit_p. > > > > Ah, yes, just notice them. But I don't understand why the .probe > > callback needs __init and .remove callback needs __exit_p? Should they > need to be removed? > > > > That is not a matter of "needs". __init causes the code to be removed after > initialization. This is ok and desirable if it is known that the hardware is built- > in and will only ever be probed once. > > exit_p causes the code to be removed if it is built into the kernel. This is > desirable and makes sense if the device is known to never be removed. Make sense, for now, there is no i.MX SoC needs watchdog driver as module, so I will keep it here. > > Having said that, what _is_ unnecessary is the remove function. Registration > could use devm_watchdog_register_device(), and the watchdog subsystem > should prevent removal if the watchdog is running. Plus, the removal > function is > buggy: It doesn' call clk_disable_unprepare() (but that could be handled with > devm_add_action_or_reset() in the probe function). In my opinion, fixing all > that would be more valuable than trying to drop an include file. Thanks for pointing out the clock operation issue in .remove callback, I will cut a patch using devm_watchdog_register_device() and devm_add_action_or_reset() to handle all necessary operations for remove action, then drop .remove callback. Thanks, Anson ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] watchdog: imx_sc_wdt: Remove unused includes 2020-02-21 2:00 [PATCH 1/3] watchdog: imx_sc_wdt: Remove unused includes Anson Huang 2020-02-21 2:00 ` [PATCH 2/3] watchdog: imx7ulp: Remove unused include of init.h Anson Huang 2020-02-21 2:00 ` [PATCH 3/3] watchdog: imx2_wdt: " Anson Huang @ 2020-02-22 15:55 ` Guenter Roeck 2 siblings, 0 replies; 9+ messages in thread From: Guenter Roeck @ 2020-02-22 15:55 UTC (permalink / raw) To: Anson Huang, wim, shawnguo, s.hauer, kernel, festevam, linux-watchdog, linux-arm-kernel, linux-kernel Cc: Linux-imx On 2/20/20 6:00 PM, Anson Huang wrote: > There is nothing in use from init.h/reboot.h, remove them. > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com> Reviewed-by: Guenter Roeck <linux@roeck-us.net> > --- > drivers/watchdog/imx_sc_wdt.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/watchdog/imx_sc_wdt.c b/drivers/watchdog/imx_sc_wdt.c > index 8ed89f0..60a3246 100644 > --- a/drivers/watchdog/imx_sc_wdt.c > +++ b/drivers/watchdog/imx_sc_wdt.c > @@ -6,13 +6,11 @@ > #include <linux/arm-smccc.h> > #include <linux/firmware/imx/sci.h> > #include <linux/io.h> > -#include <linux/init.h> > #include <linux/kernel.h> > #include <linux/module.h> > #include <linux/moduleparam.h> > #include <linux/of.h> > #include <linux/platform_device.h> > -#include <linux/reboot.h> > #include <linux/watchdog.h> > > #define DEFAULT_TIMEOUT 60 > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-02-24 2:54 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-21 2:00 [PATCH 1/3] watchdog: imx_sc_wdt: Remove unused includes Anson Huang 2020-02-21 2:00 ` [PATCH 2/3] watchdog: imx7ulp: Remove unused include of init.h Anson Huang 2020-02-22 15:59 ` Guenter Roeck 2020-02-21 2:00 ` [PATCH 3/3] watchdog: imx2_wdt: " Anson Huang 2020-02-22 16:02 ` Guenter Roeck 2020-02-23 0:16 ` Anson Huang 2020-02-23 14:08 ` Guenter Roeck 2020-02-24 2:54 ` Anson Huang 2020-02-22 15:55 ` [PATCH 1/3] watchdog: imx_sc_wdt: Remove unused includes 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).