re: ARM: meson: add watchdog driver

re: ARM: meson: add watchdog driver

[Dan Carpenter]

Hello Carlo Caione,

The patch 2b0523c42849: "ARM: meson: add watchdog driver" from Sep
20, 2014, leads to the following static checker warning:

	drivers/watchdog/meson_wdt.c:54 meson_restart_handle()
	warn: was hexadecimal intended '100'

drivers/watchdog/meson_wdt.c
   51  static int meson_restart_handle(struct notifier_block *this, unsigned long mode,
    52                                  void *cmd)
    53  {
    54          u32 tc_reboot = MESON_WDT_DC_RESET | MESON_WDT_TC_EN | 100;
                                                                       ^^^
Is it supposed to be 0x100 here?  Anyway, magic numbers are bad so this
should be changed probably.

    55          struct meson_wdt_dev *meson_wdt = container_of(this,
    56                                                         struct meson_wdt_dev,
    57                                                         restart_handler);
    58  
    59          while (1) {
    60                  writel(tc_reboot, meson_wdt->wdt_base + MESON_WDT_TC);
    61                  mdelay(5);
    62          }
    63  
    64          return NOTIFY_DONE;
    65  }

regards,
dan carpenter

Re: ARM: meson: add watchdog driver

[Carlo Caione]

On Thu, Oct 9, 2014 at 2:08 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> Hello Carlo Caione,

Hi Dan,

> The patch 2b0523c42849: "ARM: meson: add watchdog driver" from Sep
> 20, 2014, leads to the following static checker warning:
>
>         drivers/watchdog/meson_wdt.c:54 meson_restart_handle()
>         warn: was hexadecimal intended '100'

Good catch,

> drivers/watchdog/meson_wdt.c
>    51  static int meson_restart_handle(struct notifier_block *this, unsigned long mode,
>     52                                  void *cmd)
>     53  {
>     54          u32 tc_reboot = MESON_WDT_DC_RESET | MESON_WDT_TC_EN | 100;
>                                                                        ^^^
> Is it supposed to be 0x100 here?  Anyway, magic numbers are bad so this
> should be changed probably.

Yes. It is actually an arbitrary really small value. Nothing special.
I'll submit a patch to fix it with your Reported-by

Thank you,


-- 
Carlo Caione