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
