lkml.org 
[lkml]   [2018]   [Jan]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v2 4/8] watchdog: JZ4740: Drop module remove function
    Hi Guenter,

    On 20 January 2018 at 21:20, Guenter Roeck <linux@roeck-us.net> wrote:
    > On 01/19/2018 11:41 PM, PrasannaKumar Muralidharan wrote:
    >>
    >> Hi Paul,
    >>
    >> On 30 December 2017 at 19:21, Paul Cercueil <paul@crapouillou.net> wrote:
    >>>
    >>> When the watchdog was configured for nowayout, and after the
    >>> userspace watchdog daemon closed the dev node without sending the
    >>> magic character, unloading this module stopped the watchdog
    >>> hardware, which was clearly a problem.
    >>>
    >>> Besides, unloading the module is not possible when the userspace
    >>> watchdog daemon is running, so it's safe to assume that we don't
    >>> need to stop the watchdog hardware in the jz4740_wdt_remove()
    >>> function.
    >>>
    >>> For this reason, the jz4740_wdt_remove() function can then be
    >>> dropped alltogether.
    >>>
    >>> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
    >>> ---
    >>> drivers/watchdog/jz4740_wdt.c | 8 --------
    >>> 1 file changed, 8 deletions(-)
    >>>
    >>> v2: New patch in this series
    >>>
    >>> diff --git a/drivers/watchdog/jz4740_wdt.c
    >>> b/drivers/watchdog/jz4740_wdt.c
    >>> index fa7f49a3212c..02b9b8e946a2 100644
    >>> --- a/drivers/watchdog/jz4740_wdt.c
    >>> +++ b/drivers/watchdog/jz4740_wdt.c
    >>> @@ -205,16 +205,8 @@ static int jz4740_wdt_probe(struct platform_device
    >>> *pdev)
    >>> return 0;
    >>> }
    >>>
    >>> -static int jz4740_wdt_remove(struct platform_device *pdev)
    >>> -{
    >>> - struct jz4740_wdt_drvdata *drvdata = platform_get_drvdata(pdev);
    >>> -
    >>> - return jz4740_wdt_stop(&drvdata->wdt);
    >>> -}
    >>> -
    >>> static struct platform_driver jz4740_wdt_driver = {
    >>> .probe = jz4740_wdt_probe,
    >>> - .remove = jz4740_wdt_remove,
    >>> .driver = {
    >>> .name = "jz4740-wdt",
    >>> .of_match_table = of_match_ptr(jz4740_wdt_of_matches),
    >>> --
    >>> 2.11.0
    >>>
    >>>
    >>
    >> As ".remove" is removed and wdt is required for restarting the system
    >> I am thinking that stop callback is also not required. If so does it
    >> makes sense to remove the stop callback? I can submit a patch for the
    >> same once this patch series goes in.
    >>
    > The remove function was removed because it would otherwise be an empty
    > function. Since it is optional, it can and should be removed if it does not
    > do anything. If the stop function is removed, it is no longer possible
    > to stop the watchdog. Why would this make sense, and why would it make sense
    > to drop the stop function if there is no remove function ?
    >
    > Guenter
    >

    I missed the fact that stopping is watchdog is possible. Sorry for the noise.

    Thanks,
    PrasannaKumar

    \
     
     \ /
      Last update: 2018-01-20 17:00    [W:6.342 / U:0.116 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site