linux-kernel - RE: [PATCH 3/3] watchdog: imx2_wdt: Remove unused include of init.h

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 24 Feb 2020 02:54:08 +0000
From:   Anson Huang <anson.huang@....com>
To:     Guenter Roeck <linux@...ck-us.net>
CC:     "wim@...ux-watchdog.org" <wim@...ux-watchdog.org>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>,
        "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        "festevam@...il.com" <festevam@...il.com>,
        "linux-watchdog@...r.kernel.org" <linux-watchdog@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        dl-linux-imx <linux-imx@....com>
Subject: RE: [PATCH 3/3] watchdog: imx2_wdt: Remove unused include of init.h

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