Hi,
Thanks for your review.
On Mon, Sep 21, 2020 at 04:25:44PM +0900, Punit Agrawal wrote:
> Iwamatsu-san,
>
> Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@xxxxxxxxxxxxx> writes:
>
> > Add the watchdog driver for Toshiba Visconti series.
> >
> > Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@xxxxxxxxxxxxx>
> > ---
> > drivers/watchdog/Kconfig | 8 ++
> > drivers/watchdog/Makefile | 1 +
> > drivers/watchdog/visconti_wdt.c | 191 ++++++++++++++++++++++++++++++++
> > 3 files changed, 200 insertions(+)
> > create mode 100644 drivers/watchdog/visconti_wdt.c
> >
>
> [...]
>
> > diff --git a/drivers/watchdog/visconti_wdt.c b/drivers/watchdog/visconti_wdt.c
> > new file mode 100644
> > index 000000000000..4a67b9fe9220
> > --- /dev/null
> > +++ b/drivers/watchdog/visconti_wdt.c
> > @@ -0,0 +1,191 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2020 TOSHIBA CORPORATION
> > + * Copyright (c) 2020 Toshiba Electronic Devices & Storage Corporation
> > + * Copyright (c) 2020 Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@xxxxxxxxxxxxx>
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/interrupt.h>
>
> interrupt.h doesn't seem to be used. It can be dropped.
>
Right. I will remove this.
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/watchdog.h>
> > +
> > +#define WDT_CNT 0x00
> > +#define WDT_MIN 0x04
> > +#define WDT_MAX 0x08
> > +#define WDT_CTL 0x0c
> > +#define WDT_CMD 0x10
> > +#define WDT_CMD_CLEAR 0x4352
> > +#define WDT_CMD_START_STOP 0x5354
> > +#define WDT_DIV 0x30
> > +
> > +#define VISCONTI_WDT_FREQ 2000000 /* 2MHz */
> > +#define WDT_DEFAULT_TIMEOUT 10U /* in seconds */
> > +
> > +static bool nowayout = WATCHDOG_NOWAYOUT;
> > +module_param(nowayout, bool, 0);
> > +MODULE_PARM_DESC(
> > + nowayout,
> > + "Watchdog cannot be stopped once started (default=" __MODULE_STRING(WATCHDOG_NOWAYOUT)")");
<snip>
> > +module_platform_driver(visconti_wdt_driver);
> > +
> > +MODULE_DESCRIPTION("TOSHIBA Visconti Watchdog Driver");
> > +MODULE_AUTHOR("TOSHIBA ELECTRONIC DEVICES & STORAGE CORPORATION");
>
> The MODULE_AUTHOR() macro is usually used to represent the driver
> authors. Not sure about using it for the organization name which is
> already included in the copyright notice. Please drop it or replace it
> with the author name / email.
Well, it's unnecessary for MODULE_AUTHOR.
I will drop this.
>
> > +MODULE_AUTHOR("Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@xxxxxxxxxxxxx");
> > +MODULE_LICENSE("GPL v2");
>
> Reviewed-by: Punit Agrawal <punit1.agrawal@xxxxxxxxxxxxx>
>
Thanks!
> Thanks,
> Punit
>
Best regards,
Nobuhiro
