linux-kernel - Re: [PATCH 3/4] watchdog: add driver for Ricoh RN5T618 watchdog

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, 29 Sep 2014 00:50:16 +0200
From:	Beniamino Galvani <b.galvani@...il.com>
To:	Wim Van Sebroeck <wim@...ana.be>
Cc:	Lee Jones <lee.jones@...aro.org>, linux-kernel@...r.kernel.org,
	Samuel Ortiz <sameo@...ux.intel.com>,
	Mark Brown <broonie@...nel.org>,
	Liam Girdwood <lgirdwood@...il.com>,
	linux-watchdog@...r.kernel.org, devicetree@...r.kernel.org,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Grant Likely <grant.likely@...aro.org>,
	Carlo Caione <carlo@...one.org>
Subject: Re: [PATCH 3/4] watchdog: add driver for Ricoh RN5T618 watchdog

On Sun, Sep 28, 2014 at 10:36:46PM +0200, Wim Van Sebroeck wrote:
> Hi Lee,
> 
> > Still waiting on a Watchdog Ack, so I can take this set in.
> > 
> > > This adds a driver for the watchdog timer available in Ricoh RN5T618
> > > PMIC. The device supports a programmable expiration time of 1, 8, 32
> > > or 128 seconds.
> > > 
> > > Signed-off-by: Beniamino Galvani <b.galvani@...il.com>
> > > ---
> > >  drivers/watchdog/Kconfig       |   11 +++
> > >  drivers/watchdog/Makefile      |    1 +
> > >  drivers/watchdog/rn5t618_wdt.c |  196 ++++++++++++++++++++++++++++++++++++++++
> > >  include/linux/mfd/rn5t618.h    |    4 +
> > >  4 files changed, 212 insertions(+)
> > >  create mode 100644 drivers/watchdog/rn5t618_wdt.c
> > > 
> > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > > index f57312f..9eba6af 100644
> > > --- a/drivers/watchdog/Kconfig
> > > +++ b/drivers/watchdog/Kconfig
> > > @@ -309,6 +309,17 @@ config ORION_WATCHDOG
> > >  	  To compile this driver as a module, choose M here: the
> > >  	  module will be called orion_wdt.
> > >  
> > > +config RN5T618_WATCHDOG
> > > +	tristate "Ricoh RN5T618 watchdog"
> > > +	depends on MFD_RN5T618
> > > +	select WATCHDOG_CORE
> > > +	help
> > > +	  If you say yes here you get support for watchdog on the Ricoh
> > > +	  RN5T618 PMIC.
> > > +
> > > +	  This driver can also be built as a module.  If so, the module
> > > +	  will be called rn5t618_wdt.
> > > +
> > >  config SUNXI_WATCHDOG
> > >  	tristate "Allwinner SoCs watchdog support"
> > >  	depends on ARCH_SUNXI
> > > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> > > index 468c320..0afa343 100644
> > > --- a/drivers/watchdog/Makefile
> > > +++ b/drivers/watchdog/Makefile
> > > @@ -47,6 +47,7 @@ obj-$(CONFIG_IOP_WATCHDOG) += iop_wdt.o
> > >  obj-$(CONFIG_DAVINCI_WATCHDOG) += davinci_wdt.o
> > >  obj-$(CONFIG_ORION_WATCHDOG) += orion_wdt.o
> > >  obj-$(CONFIG_SUNXI_WATCHDOG) += sunxi_wdt.o
> > > +obj-$(CONFIG_RN5T618_WATCHDOG) += rn5t618_wdt.o
> > >  obj-$(CONFIG_COH901327_WATCHDOG) += coh901327_wdt.o
> > >  obj-$(CONFIG_STMP3XXX_RTC_WATCHDOG) += stmp3xxx_rtc_wdt.o
> > >  obj-$(CONFIG_NUC900_WATCHDOG) += nuc900_wdt.o
> > > diff --git a/drivers/watchdog/rn5t618_wdt.c b/drivers/watchdog/rn5t618_wdt.c
> > > new file mode 100644
> > > index 0000000..3a76ad7
> > > --- /dev/null
> > > +++ b/drivers/watchdog/rn5t618_wdt.c
> > > @@ -0,0 +1,196 @@
> > > +/*
> > > + * Watchdog driver for Ricoh RN5T618 PMIC
> > > + *
> > > + * Copyright (C) 2014 Beniamino Galvani <b.galvani@...il.com>
> > > + *
> > > + * This program is free software; you can redistribute it and/or
> > > + * modify it under the terms of the GNU General Public License
> > > + * version 2 as published by the Free Software Foundation.
> > > + *
> > > + * You should have received a copy of the GNU General Public License
> > > + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> > > + */
> > > +
> > > +#include <linux/device.h>
> > > +#include <linux/mfd/rn5t618.h>
> > > +#include <linux/module.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/watchdog.h>
> > > +
> > > +#define DRIVER_NAME "rn5t618-wdt"
> > > +
> > > +static bool nowayout = WATCHDOG_NOWAYOUT;
> > > +static unsigned int heartbeat = -1;
> > > +
> > > +module_param(heartbeat, int, 0);
> > > +MODULE_PARM_DESC(heartbeat, "Initial watchdog heartbeat in seconds");
> > > +
> 
> These are the comments I just mailed:
> 1) this is actually a timeout value and not the same as the heartbeat of the watchdog device.
> So I prefer this also to be called as timeout and not heartbeat.
> 2) this actually is an unsigned int.
> 
> For the rest OK by me.

Hi Wim,

there was a more recent version of the patch [1] , but your comments
were still valid. I addressed your suggestions in v3 of the patch [2].

Beniamino

[1] http://lwn.net/Articles/610000/
[2] https://lkml.org/lkml/2014/9/28/173
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/