[PATCH v3 2/2] watchdog: Add Toshiba Visconti watchdog driver

[Nobuhiro Iwamatsu]

Hi,

Thanks for your review.

On Mon, Sep 21, 2020 at 04:25:44PM +0900, Punit Agrawal wrote:
> Iwamatsu-san,
> 
> Nobuhiro Iwamatsu <nobuhiro1.iwamatsu at toshiba.co.jp> writes:
> 
> > Add the watchdog driver for Toshiba Visconti series.
> >
> > Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu at toshiba.co.jp>
> > ---
> >  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 at toshiba.co.jp>
> > + */
> > +
> > +#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 at toshiba.co.jp");
> > +MODULE_LICENSE("GPL v2");
> 
> Reviewed-by: Punit Agrawal <punit1.agrawal at toshiba.co.jp>
> 

Thanks!

> Thanks,
> Punit
> 

Best regards,
  Nobuhiro