[PATCH 0/4] Add support for Ricoh RN5T618 PMIC

[PATCH 0/4] Add support for Ricoh RN5T618 PMIC

[Beniamino Galvani]

Ricoh RN5T618 is a power management IC which includes 3 step-down DCDC
converters, 7 low-dropout regulators, a Li-ion battery charger, fuel
gauge, ADC, GPIOs and a watchdog timer.

This series adds a MFD core driver and separate drivers to support the
regulator and watchdog functionalities.

The patchset has been tested on a Tronsmart Vega S89 Elite TV box on
top of other patches needed to support the Amlogic Meson SoC. The tree
with all the changes is available at:

  https://github.com/bengal/linux/tree/rn5t618

Beniamino Galvani (4):
  mfd: Add Ricoh RN5T618 PMIC core driver
  regulator: add driver for Ricoh RN5T618 regulators
  watchdog: add driver for Ricoh RN5T618 watchdog
  mfd: rn5t618: document device tree bindings

 Documentation/devicetree/bindings/mfd/rn5t618.txt |   36 ++++
 drivers/mfd/Kconfig                               |   11 +
 drivers/mfd/Makefile                              |    1 +
 drivers/mfd/rn5t618.c                             |  129 ++++++++++++
 drivers/regulator/Kconfig                         |    6 +
 drivers/regulator/Makefile                        |    1 +
 drivers/regulator/rn5t618-regulator.c             |  143 +++++++++++++
 drivers/watchdog/Kconfig                          |   11 +
 drivers/watchdog/Makefile                         |    1 +
 drivers/watchdog/rn5t618_wdt.c                    |  196 ++++++++++++++++++
 include/linux/mfd/rn5t618.h                       |  228 +++++++++++++++++++++
 11 files changed, 763 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/rn5t618.txt
 create mode 100644 drivers/mfd/rn5t618.c
 create mode 100644 drivers/regulator/rn5t618-regulator.c
 create mode 100644 drivers/watchdog/rn5t618_wdt.c
 create mode 100644 include/linux/mfd/rn5t618.h

-- 
1.7.10.4

[PATCH 1/4] mfd: Add Ricoh RN5T618 PMIC core driver

[Beniamino Galvani]

Ricoh RN5T618 is a power management IC which integrates 3 step-down
DCDC converters, 7 low-dropout regulators, a Li-ion battery charger,
fuel gauge, ADC, GPIOs and a watchdog timer.

This commit adds a MFD core driver to support the I2C communication
with the device.

Signed-off-by: Beniamino Galvani <b.galvani@gmail.com>
---
 drivers/mfd/Kconfig         |   11 +++
 drivers/mfd/Makefile        |    1 +
 drivers/mfd/rn5t618.c       |  129 ++++++++++++++++++++++++++
 include/linux/mfd/rn5t618.h |  210 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 351 insertions(+)
 create mode 100644 drivers/mfd/rn5t618.c
 create mode 100644 include/linux/mfd/rn5t618.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 8d5fad2..fae8c5b 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -582,6 +582,17 @@ config MFD_RC5T583
 	  Additional drivers must be enabled in order to use the
 	  different functionality of the device.
 
+config MFD_RN5T618
+	bool "Ricoh RN5T5618 PMIC"
+	depends on I2C=y
+	select MFD_CORE
+	select REGMAP_I2C
+	help
+	  Say yes here to add support for the Ricoh RN5T618 PMIC. This
+	  driver provides common support for accessing the device,
+	  additional drivers must be enabled in order to use the
+	  functionality of the device.
+
 config MFD_SEC_CORE
 	bool "SAMSUNG Electronics PMIC Series Support"
 	depends on I2C=y
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f001487..b945899 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -160,6 +160,7 @@ obj-$(CONFIG_MFD_INTEL_MSIC)	+= intel_msic.o
 obj-$(CONFIG_MFD_PALMAS)	+= palmas.o
 obj-$(CONFIG_MFD_VIPERBOARD)    += viperboard.o
 obj-$(CONFIG_MFD_RC5T583)	+= rc5t583.o rc5t583-irq.o
+obj-$(CONFIG_MFD_RN5T618)	+= rn5t618.o
 obj-$(CONFIG_MFD_SEC_CORE)	+= sec-core.o sec-irq.o
 obj-$(CONFIG_MFD_SYSCON)	+= syscon.o
 obj-$(CONFIG_MFD_LM3533)	+= lm3533-core.o lm3533-ctrlbank.o
diff --git a/drivers/mfd/rn5t618.c b/drivers/mfd/rn5t618.c
new file mode 100644
index 0000000..72ad118
--- /dev/null
+++ b/drivers/mfd/rn5t618.c
@@ -0,0 +1,129 @@
+/*
+ * MFD core driver for Ricoh RN5T618 PMIC
+ *
+ * Copyright (C) 2014 Beniamino Galvani <b.galvani@gmail.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/i2c.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/rn5t618.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+static const struct mfd_cell rn5t618_cells[] = {
+	{ .name = "rn5t618-regulator" },
+	{ .name = "rn5t618-wdt" },
+};
+
+static bool rn5t618_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case RN5T618_WATCHDOGCNT:
+	case RN5T618_DCIRQ:
+	case RN5T618_ILIMDATAH ... RN5T618_AIN0DATAL:
+	case RN5T618_IR_ADC1 ... RN5T618_IR_ADC3:
+	case RN5T618_IR_GPR:
+	case RN5T618_IR_GPF:
+	case RN5T618_MON_IOIN:
+	case RN5T618_INTMON:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static const struct regmap_config rn5t618_regmap_config = {
+	.reg_bits	= 8,
+	.val_bits	= 8,
+	.volatile_reg	= rn5t618_volatile_reg,
+	.max_register	= RN5T618_MAX_REG,
+	.cache_type	= REGCACHE_RBTREE,
+};
+
+static struct rn5t618 *rn5t618_pm_power_off;
+
+static void rn5t618_power_off(void)
+{
+	/* disable automatic repower-on */
+	regmap_update_bits(rn5t618_pm_power_off->regmap, RN5T618_REPCNT,
+			   RN5T618_REPCNT_REPWRON, 0);
+	/* start power-off sequence */
+	regmap_update_bits(rn5t618_pm_power_off->regmap, RN5T618_SLPCNT,
+			   RN5T618_SLPCNT_SWPWROFF, RN5T618_SLPCNT_SWPWROFF);
+}
+
+static int rn5t618_i2c_probe(struct i2c_client *i2c,
+			     const struct i2c_device_id *id)
+{
+	struct rn5t618 *priv;
+	int ret;
+
+	priv = devm_kzalloc(&i2c->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	i2c_set_clientdata(i2c, priv);
+
+	priv->regmap = devm_regmap_init_i2c(i2c, &rn5t618_regmap_config);
+	if (IS_ERR(priv->regmap)) {
+		ret = PTR_ERR(priv->regmap);
+		dev_err(&i2c->dev, "regmap init failed: %d\n", ret);
+		return ret;
+	}
+
+	ret = mfd_add_devices(&i2c->dev, -1, rn5t618_cells,
+			      ARRAY_SIZE(rn5t618_cells), NULL, 0, NULL);
+	if (ret) {
+		dev_err(&i2c->dev, "failed to add sub-devices: %d\n", ret);
+		return ret;
+	}
+
+	if (!pm_power_off) {
+		rn5t618_pm_power_off = priv;
+		pm_power_off = rn5t618_power_off;
+	}
+
+	dev_info(&i2c->dev, "RN5T618 MFD driver loaded");
+
+	return 0;
+}
+
+static int rn5t618_i2c_remove(struct i2c_client *i2c)
+{
+	mfd_remove_devices(&i2c->dev);
+	return 0;
+}
+
+static const struct of_device_id rn5t618_of_match[] = {
+	{ .compatible = "ricoh,rn5t618" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, rn5t618_of_match);
+
+static const struct i2c_device_id rn5t618_i2c_id[] = {
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, rn5t618_i2c_id);
+
+static struct i2c_driver rn5t618_i2c_driver = {
+	.driver = {
+		.name = "rn5t618",
+		.of_match_table = of_match_ptr(rn5t618_of_match),
+	},
+	.probe = rn5t618_i2c_probe,
+	.remove = rn5t618_i2c_remove,
+	.id_table = rn5t618_i2c_id,
+};
+
+module_i2c_driver(rn5t618_i2c_driver);
+
+MODULE_AUTHOR("Beniamino Galvani <b.galvani@gmail.com>");
+MODULE_DESCRIPTION("Ricoh RN5T618 MFD driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/rn5t618.h b/include/linux/mfd/rn5t618.h
new file mode 100644
index 0000000..87686eb
--- /dev/null
+++ b/include/linux/mfd/rn5t618.h
@@ -0,0 +1,210 @@
+/*
+ * MFD core driver for Ricoh RN5T618 PMIC
+ *
+ * Copyright (C) 2014 Beniamino Galvani <b.galvani@gmail.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/>.
+ */
+
+#ifndef __LINUX_MFD_RN5T618_H
+#define __LINUX_MFD_RN5T618_H
+
+#include <linux/regmap.h>
+
+#define RN5T618_LSIVER			0x00
+#define RN5T618_OTPVER			0x01
+#define RN5T618_IODAC			0x02
+#define RN5T618_VINDAC			0x03
+#define RN5T618_CPUCNT			0x06
+#define RN5T618_PSWR			0x07
+#define RN5T618_PONHIS			0x09
+#define RN5T618_POFFHIS			0x0a
+#define RN5T618_WATCHDOG		0x0b
+#define RN5T618_WATCHDOGCNT		0x0c
+#define RN5T618_PWRFUNC			0x0d
+#define RN5T618_SLPCNT			0x0e
+#define RN5T618_REPCNT			0x0f
+#define RN5T618_PWRONTIMSET		0x10
+#define RN5T618_NOETIMSETCNT		0x11
+#define RN5T618_PWRIREN			0x12
+#define RN5T618_PWRIRQ			0x13
+#define RN5T618_PWRMON			0x14
+#define RN5T618_PWRIRSEL		0x15
+#define RN5T618_DC1_SLOT		0x16
+#define RN5T618_DC2_SLOT		0x17
+#define RN5T618_DC3_SLOT		0x18
+#define RN5T618_LDO1_SLOT		0x1b
+#define RN5T618_LDO2_SLOT		0x1c
+#define RN5T618_LDO3_SLOT		0x1d
+#define RN5T618_LDO4_SLOT		0x1e
+#define RN5T618_LDO5_SLOT		0x1f
+#define RN5T618_PSO0_SLOT		0x25
+#define RN5T618_PSO1_SLOT		0x26
+#define RN5T618_PSO2_SLOT		0x27
+#define RN5T618_PSO3_SLOT		0x28
+#define RN5T618_LDORTC1_SLOT		0x2a
+#define RN5T618_DC1CTL			0x2c
+#define RN5T618_DC1CTL2			0x2d
+#define RN5T618_DC2CTL			0x2e
+#define RN5T618_DC2CTL2			0x2f
+#define RN5T618_DC3CTL			0x30
+#define RN5T618_DC3CTL2			0x31
+#define RN5T618_DC1DAC			0x36
+#define RN5T618_DC2DAC			0x37
+#define RN5T618_DC3DAC			0x38
+#define RN5T618_DC1DAC_SLP		0x3b
+#define RN5T618_DC2DAC_SLP		0x3c
+#define RN5T618_DC3DAC_SLP		0x3d
+#define RN5T618_DCIREN			0x40
+#define RN5T618_DCIRQ			0x41
+#define RN5T618_DCIRMON			0x42
+#define RN5T618_LDOEN1			0x44
+#define RN5T618_LDOEN2			0x45
+#define RN5T618_LDODIS			0x46
+#define RN5T618_LDO1DAC			0x4c
+#define RN5T618_LDO2DAC			0x4d
+#define RN5T618_LDO3DAC			0x4e
+#define RN5T618_LDO4DAC			0x4f
+#define RN5T618_LDO5DAC			0x50
+#define RN5T618_LDORTCDAC		0x56
+#define RN5T618_LDORTC2DAC		0x57
+#define RN5T618_LDO1DAC_SLP		0x58
+#define RN5T618_LDO2DAC_SLP		0x59
+#define RN5T618_LDO3DAC_SLP		0x5a
+#define RN5T618_LDO4DAC_SLP		0x5b
+#define RN5T618_LDO5DAC_SLP		0x5c
+#define RN5T618_ADCCNT1			0x64
+#define RN5T618_ADCCNT2			0x65
+#define RN5T618_ADCCNT3			0x66
+#define RN5T618_ILIMDATAH		0x68
+#define RN5T618_ILIMDATAL		0x69
+#define RN5T618_VBATDATAH		0x6a
+#define RN5T618_VBATDATAL		0x6b
+#define RN5T618_VADPDATAH		0x6c
+#define RN5T618_VADPDATAL		0x6d
+#define RN5T618_VUSBDATAH		0x6e
+#define RN5T618_VUSBDATAL		0x6f
+#define RN5T618_VSYSDATAH		0x70
+#define RN5T618_VSYSDATAL		0x71
+#define RN5T618_VTHMDATAH		0x72
+#define RN5T618_VTHMDATAL		0x73
+#define RN5T618_AIN1DATAH		0x74
+#define RN5T618_AIN1DATAL		0x75
+#define RN5T618_AIN0DATAH		0x76
+#define RN5T618_AIN0DATAL		0x77
+#define RN5T618_ILIMTHL			0x78
+#define RN5T618_ILIMTHH			0x79
+#define RN5T618_VBATTHL			0x7a
+#define RN5T618_VBATTHH			0x7b
+#define RN5T618_VADPTHL			0x7c
+#define RN5T618_VADPTHH			0x7d
+#define RN5T618_VUSBTHL			0x7e
+#define RN5T618_VUSBTHH			0x7f
+#define RN5T618_VSYSTHL			0x80
+#define RN5T618_VSYSTHH			0x81
+#define RN5T618_VTHMTHL			0x82
+#define RN5T618_VTHMTHH			0x83
+#define RN5T618_AIN1THL			0x84
+#define RN5T618_AIN1THH			0x85
+#define RN5T618_AIN0THL			0x86
+#define RN5T618_AIN0THH			0x87
+#define RN5T618_EN_ADCIR1		0x88
+#define RN5T618_EN_ADCIR2		0x89
+#define RN5T618_EN_ADCIR3		0x8a
+#define RN5T618_IR_ADC1			0x8c
+#define RN5T618_IR_ADC2			0x8d
+#define RN5T618_IR_ADC3			0x8e
+#define RN5T618_IOSEL			0x90
+#define RN5T618_IOOUT			0x91
+#define RN5T618_GPEDGE1			0x92
+#define RN5T618_GPEDGE2			0x93
+#define RN5T618_EN_GPIR			0x94
+#define RN5T618_IR_GPR			0x95
+#define RN5T618_IR_GPF			0x96
+#define RN5T618_MON_IOIN		0x97
+#define RN5T618_GPLED_FUNC		0x98
+#define RN5T618_INTPOL			0x9c
+#define RN5T618_INTEN			0x9d
+#define RN5T618_INTMON			0x9e
+#define RN5T618_PREVINDAC		0xb0
+#define RN5T618_BATDAC			0xb1
+#define RN5T618_CHGCTL1			0xb3
+#define RN5T618_CHGCTL2			0xb4
+#define RN5T618_VSYSSET			0xb5
+#define RN5T618_REGISET1		0xb6
+#define RN5T618_REGISET2		0xb7
+#define RN5T618_CHGISET			0xb8
+#define RN5T618_TIMSET			0xb9
+#define RN5T618_BATSET1			0xba
+#define RN5T618_BATSET2			0xbb
+#define RN5T618_DIESET			0xbc
+#define RN5T618_CHGSTATE		0xbd
+#define RN5T618_CHGCTRL_IRFMASK		0xbe
+#define RN5T618_CHGSTAT_IRFMASK1	0xbf
+#define RN5T618_CHGSTAT_IRFMASK2	0xc0
+#define RN5T618_CHGERR_IRFMASK		0xc1
+#define RN5T618_CHGCTRL_IRR		0xc2
+#define RN5T618_CHGSTAT_IRR1		0xc3
+#define RN5T618_CHGSTAT_IRR2		0xc4
+#define RN5T618_CHGERR_IRR		0xc5
+#define RN5T618_CHGCTRL_MONI		0xc6
+#define RN5T618_CHGSTAT_MONI1		0xc7
+#define RN5T618_CHGSTAT_MONI2		0xc8
+#define RN5T618_CHGERR_MONI		0xc9
+#define RN5T618_CHGCTRL_DETMOD1		0xca
+#define RN5T618_CHGCTRL_DETMOD2		0xcb
+#define RN5T618_CHGSTAT_DETMOD1		0xcc
+#define RN5T618_CHGSTAT_DETMOD2		0xcd
+#define RN5T618_CHGSTAT_DETMOD3		0xce
+#define RN5T618_CHGERR_DETMOD1		0xcf
+#define RN5T618_CHGERR_DETMOD2		0xd0
+#define RN5T618_CHGOSCCTL		0xd4
+#define RN5T618_CHGOSCSCORESET1		0xd5
+#define RN5T618_CHGOSCSCORESET2		0xd6
+#define RN5T618_CHGOSCSCORESET3		0xd7
+#define RN5T618_CHGOSCFREQSET1		0xd8
+#define RN5T618_CHGOSCFREQSET2		0xd9
+#define RN5T618_CONTROL			0xe0
+#define RN5T618_SOC			0xe1
+#define RN5T618_RE_CAP_H		0xe2
+#define RN5T618_RE_CAP_L		0xe3
+#define RN5T618_FA_CAP_H		0xe4
+#define RN5T618_FA_CAP_L		0xe5
+#define RN5T618_AGE			0xe6
+#define RN5T618_TT_EMPTY_H		0xe7
+#define RN5T618_TT_EMPTY_L		0xe8
+#define RN5T618_TT_FULL_H		0xe9
+#define RN5T618_TT_FULL_L		0xea
+#define RN5T618_VOLTAGE_1		0xeb
+#define RN5T618_VOLTAGE_0		0xec
+#define RN5T618_TEMP_1			0xed
+#define RN5T618_TEMP_0			0xee
+#define RN5T618_CC_CTRL			0xef
+#define RN5T618_CC_COUNT2		0xf0
+#define RN5T618_CC_COUNT1		0xf1
+#define RN5T618_CC_COUNT0		0xf2
+#define RN5T618_CC_SUMREG3		0xf3
+#define RN5T618_CC_SUMREG2		0xf4
+#define RN5T618_CC_SUMREG1		0xf5
+#define RN5T618_CC_SUMREG0		0xf6
+#define RN5T618_CC_OFFREG1		0xf7
+#define RN5T618_CC_OFFREG0		0xf8
+#define RN5T618_CC_GAINREG1		0xf9
+#define RN5T618_CC_GAINREG0		0xfa
+#define RN5T618_CC_AVEREG1		0xfb
+#define RN5T618_CC_AVEREG0		0xfc
+#define RN5T618_MAX_REG			0xfc
+
+#define RN5T618_REPCNT_REPWRON		BIT(0)
+#define RN5T618_SLPCNT_SWPWROFF		BIT(0)
+
+struct rn5t618 {
+	struct regmap *regmap;
+};
+
+#endif /* __LINUX_MFD_RN5T618_H */
-- 
1.7.10.4

Re: [PATCH 1/4] mfd: Add Ricoh RN5T618 PMIC core driver

[Lee Jones]

On Wed, 27 Aug 2014, Beniamino Galvani wrote:
> Ricoh RN5T618 is a power management IC which integrates 3 step-down
> DCDC converters, 7 low-dropout regulators, a Li-ion battery charger,
> fuel gauge, ADC, GPIOs and a watchdog timer.
> 
> This commit adds a MFD core driver to support the I2C communication
> with the device.
> 
> Signed-off-by: Beniamino Galvani <b.galvani@gmail.com>
> ---
>  drivers/mfd/Kconfig         |   11 +++
>  drivers/mfd/Makefile        |    1 +
>  drivers/mfd/rn5t618.c       |  129 ++++++++++++++++++++++++++
>  include/linux/mfd/rn5t618.h |  210 +++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 351 insertions(+)
>  create mode 100644 drivers/mfd/rn5t618.c
>  create mode 100644 include/linux/mfd/rn5t618.h

Really nicely done.  We don't normally get many first submissions at
this quality level.

One question and a _really_ small nit though.

> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 8d5fad2..fae8c5b 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -582,6 +582,17 @@ config MFD_RC5T583
>  	  Additional drivers must be enabled in order to use the
>  	  different functionality of the device.
>  
> +config MFD_RN5T618
> +	bool "Ricoh RN5T5618 PMIC"

Shouldn't this be tristate?

> +	depends on I2C=y

If so, this should be:

  depends on I2C

> +	select MFD_CORE
> +	select REGMAP_I2C
> +	help
> +	  Say yes here to add support for the Ricoh RN5T618 PMIC. This
> +	  driver provides common support for accessing the device,
> +	  additional drivers must be enabled in order to use the
> +	  functionality of the device.

[...]

> +++ b/drivers/mfd/rn5t618.c
> @@ -0,0 +1,129 @@

[...]

> +static int rn5t618_i2c_probe(struct i2c_client *i2c,
> +			     const struct i2c_device_id *id)
> +{

[...]

> +	dev_info(&i2c->dev, "RN5T618 MFD driver loaded");

Can you remove this line?  We normally only print things when
information is gathered from a chip i.e. version information and the
like.

> +	return 0;
> +}

[...]

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 1/4] mfd: Add Ricoh RN5T618 PMIC core driver

[Beniamino Galvani]

On Wed, Aug 27, 2014 at 08:56:14AM +0100, Lee Jones wrote:
[...]
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > index 8d5fad2..fae8c5b 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -582,6 +582,17 @@ config MFD_RC5T583
> >  	  Additional drivers must be enabled in order to use the
> >  	  different functionality of the device.
> >  
> > +config MFD_RN5T618
> > +	bool "Ricoh RN5T5618 PMIC"
> 
> Shouldn't this be tristate?

Yes, I suppose the driver would work also as a module, I only have to
uninstall the pm_power_off hook on module removal.

> > +++ b/drivers/mfd/rn5t618.c
> > @@ -0,0 +1,129 @@
> 
> [...]
> 
> > +static int rn5t618_i2c_probe(struct i2c_client *i2c,
> > +			     const struct i2c_device_id *id)
> > +{
> 
> [...]
> 
> > +	dev_info(&i2c->dev, "RN5T618 MFD driver loaded");
> 
> Can you remove this line?  We normally only print things when
> information is gathered from a chip i.e. version information and the
> like.

I will remove it, thanks.

Beniamino

[PATCH 2/4] regulator: add driver for Ricoh RN5T618 regulators

[Beniamino Galvani]

This driver supports the 3 DCDC and 7 LDO regulators available on
Ricoh RN5T618 PMIC.

Signed-off-by: Beniamino Galvani <b.galvani@gmail.com>
---
 drivers/regulator/Kconfig             |    6 ++
 drivers/regulator/Makefile            |    1 +
 drivers/regulator/rn5t618-regulator.c |  143 +++++++++++++++++++++++++++++++++
 include/linux/mfd/rn5t618.h           |   14 ++++
 4 files changed, 164 insertions(+)
 create mode 100644 drivers/regulator/rn5t618-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 2dc8289..9d14e62 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -459,6 +459,12 @@ config REGULATOR_RC5T583
 	  through regulator interface. The device supports multiple DCDC/LDO
 	  outputs which can be controlled by i2c communication.
 
+config REGULATOR_RN5T618
+	tristate "Ricoh RN5T618 voltage regulators"
+	depends on MFD_RN5T618
+	help
+	  Say y here to support the regulators found on Ricoh RN5T618 PMIC.
+
 config REGULATOR_S2MPA01
 	tristate "Samsung S2MPA01 voltage regulator"
 	depends on MFD_SEC_CORE
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index aa4a6aa..f7c4473 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -62,6 +62,7 @@ obj-$(CONFIG_REGULATOR_PBIAS) += pbias-regulator.o
 obj-$(CONFIG_REGULATOR_PCAP) += pcap-regulator.o
 obj-$(CONFIG_REGULATOR_PCF50633) += pcf50633-regulator.o
 obj-$(CONFIG_REGULATOR_RC5T583)  += rc5t583-regulator.o
+obj-$(CONFIG_REGULATOR_RN5T618)  += rn5t618-regulator.o
 obj-$(CONFIG_REGULATOR_S2MPA01) += s2mpa01.o
 obj-$(CONFIG_REGULATOR_S2MPS11) += s2mps11.o
 obj-$(CONFIG_REGULATOR_S5M8767) += s5m8767.o
diff --git a/drivers/regulator/rn5t618-regulator.c b/drivers/regulator/rn5t618-regulator.c
new file mode 100644
index 0000000..e58d79a
--- /dev/null
+++ b/drivers/regulator/rn5t618-regulator.c
@@ -0,0 +1,143 @@
+/*
+ * Regulator driver for Ricoh RN5T618 PMIC
+ *
+ * Copyright (C) 2014 Beniamino Galvani <b.galvani@gmail.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/mfd/rn5t618.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+
+static struct regulator_ops rn5t618_reg_ops = {
+	.enable			= regulator_enable_regmap,
+	.disable		= regulator_disable_regmap,
+	.is_enabled		= regulator_is_enabled_regmap,
+	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
+	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
+	.list_voltage		= regulator_list_voltage_linear,
+};
+
+#define REG(rid, ereg, emask, vreg, vmask, min, max, step)		\
+	[RN5T618_##rid] = {						\
+		.name		= #rid,					\
+		.id		= RN5T618_##rid,			\
+		.type		= REGULATOR_VOLTAGE,			\
+		.owner		= THIS_MODULE,				\
+		.ops		= &rn5t618_reg_ops,			\
+		.n_voltages	= ((max) - (min)) / (step) + 1,		\
+		.min_uV		= (min),				\
+		.uV_step	= (step),				\
+		.enable_reg	= RN5T618_##ereg,			\
+		.enable_mask	= (emask),				\
+		.vsel_reg	= RN5T618_##vreg,			\
+		.vsel_mask	= (vmask),				\
+	}
+
+static struct regulator_desc rn5t618_regulators[] = {
+	/* DCDC */
+	REG(DCDC1, DC1CTL, BIT(0), DC1DAC, 0xff, 600000, 3500000, 12500),
+	REG(DCDC2, DC2CTL, BIT(0), DC2DAC, 0xff, 600000, 3500000, 12500),
+	REG(DCDC3, DC3CTL, BIT(0), DC3DAC, 0xff, 600000, 3500000, 12500),
+	/* LDO */
+	REG(LDO1, LDOEN1, BIT(0), LDO1DAC, 0x7f, 900000, 3500000, 25000),
+	REG(LDO2, LDOEN1, BIT(1), LDO2DAC, 0x7f, 900000, 3500000, 25000),
+	REG(LDO3, LDOEN1, BIT(2), LDO3DAC, 0x7f, 600000, 3500000, 25000),
+	REG(LDO4, LDOEN1, BIT(3), LDO4DAC, 0x7f, 900000, 3500000, 25000),
+	REG(LDO5, LDOEN1, BIT(4), LDO5DAC, 0x7f, 900000, 3500000, 25000),
+	/* LDO RTC */
+	REG(LDORTC1, LDOEN2, BIT(4), LDORTCDAC, 0x7f, 1700000, 3500000, 25000),
+	REG(LDORTC2, LDOEN2, BIT(5), LDORTC2DAC, 0x7f, 900000, 3500000, 25000),
+};
+
+static struct of_regulator_match rn5t618_matches[] = {
+	[RN5T618_DCDC1]		= { .name = "DCDC1" },
+	[RN5T618_DCDC2]		= { .name = "DCDC2" },
+	[RN5T618_DCDC3]		= { .name = "DCDC3" },
+	[RN5T618_LDO1]		= { .name = "LDO1" },
+	[RN5T618_LDO2]		= { .name = "LDO2" },
+	[RN5T618_LDO3]		= { .name = "LDO3" },
+	[RN5T618_LDO4]		= { .name = "LDO4" },
+	[RN5T618_LDO5]		= { .name = "LDO5" },
+	[RN5T618_LDORTC1]	= { .name = "LDORTC1" },
+	[RN5T618_LDORTC2]	= { .name = "LDORTC2" },
+};
+
+static int rn5t618_regulator_parse_dt(struct platform_device *pdev)
+{
+	struct device_node *np, *regulators;
+	int ret;
+
+	np = of_node_get(pdev->dev.parent->of_node);
+	if (!np)
+		return 0;
+
+	regulators = of_get_child_by_name(np, "regulators");
+	if (!regulators) {
+		dev_err(&pdev->dev, "regulators node not found\n");
+		return -EINVAL;
+	}
+
+	ret = of_regulator_match(&pdev->dev, regulators, rn5t618_matches,
+				 ARRAY_SIZE(rn5t618_matches));
+	of_node_put(regulators);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "error parsing regulator init data: %d\n",
+			ret);
+	}
+
+	return 0;
+}
+
+static int rn5t618_regulator_probe(struct platform_device *pdev)
+{
+	struct rn5t618 *rn5t618 = dev_get_drvdata(pdev->dev.parent);
+	struct regulator_config config = { };
+	struct regulator_dev *rdev;
+	int ret, i;
+
+	ret = rn5t618_regulator_parse_dt(pdev);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < RN5T618_REG_NUM; i++) {
+		config.dev = &pdev->dev;
+		config.init_data = rn5t618_matches[i].init_data;
+		config.of_node = rn5t618_matches[i].of_node;
+		config.regmap = rn5t618->regmap;
+
+		rdev = devm_regulator_register(&pdev->dev,
+					       &rn5t618_regulators[i],
+					       &config);
+		if (IS_ERR(rdev)) {
+			dev_err(&pdev->dev, "failed to register %s regulator\n",
+				rn5t618_regulators[i].name);
+			return PTR_ERR(rdev);
+		}
+	}
+
+	return 0;
+}
+
+static struct platform_driver rn5t618_regulator_driver = {
+	.probe = rn5t618_regulator_probe,
+	.driver = {
+		.name	= "rn5t618-regulator",
+	},
+};
+
+module_platform_driver(rn5t618_regulator_driver);
+
+MODULE_AUTHOR("Beniamino Galvani <b.galvani@gmail.com>");
+MODULE_DESCRIPTION("RN5T618 regulator driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/rn5t618.h b/include/linux/mfd/rn5t618.h
index 87686eb..e3571d1 100644
--- a/include/linux/mfd/rn5t618.h
+++ b/include/linux/mfd/rn5t618.h
@@ -203,6 +203,20 @@
 #define RN5T618_REPCNT_REPWRON		BIT(0)
 #define RN5T618_SLPCNT_SWPWROFF		BIT(0)
 
+enum {
+	RN5T618_DCDC1,
+	RN5T618_DCDC2,
+	RN5T618_DCDC3,
+	RN5T618_LDO1,
+	RN5T618_LDO2,
+	RN5T618_LDO3,
+	RN5T618_LDO4,
+	RN5T618_LDO5,
+	RN5T618_LDORTC1,
+	RN5T618_LDORTC2,
+	RN5T618_REG_NUM,
+};
+
 struct rn5t618 {
 	struct regmap *regmap;
 };
-- 
1.7.10.4

Re: [PATCH 2/4] regulator: add driver for Ricoh RN5T618 regulators

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 499 bytes --]

On Wed, Aug 27, 2014 at 12:13:55AM +0200, Beniamino Galvani wrote:
> This driver supports the 3 DCDC and 7 LDO regulators available on
> Ricoh RN5T618 PMIC.

>  drivers/regulator/rn5t618-regulator.c |  143 +++++++++++++++++++++++++++++++++
>  include/linux/mfd/rn5t618.h           |   14 ++++

This is fine but I can't apply it since you're patching the MFD header
which you added in the first patch.  It'd be better to include those
changes in the first patch, making the second patch independant.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH 3/4] watchdog: add driver for Ricoh RN5T618 watchdog

[Beniamino Galvani]

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@gmail.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@gmail.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");
+
+module_param(nowayout, bool, 0);
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
+		 __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+
+struct rn5t618_wdt {
+	struct watchdog_device wdt_dev;
+	struct rn5t618 *rn5t618;
+};
+
+/*
+ * This array encodes the values of WDOGTIM field for the supported
+ * watchdog expiration times. If the watchdog is not accessed before
+ * the timer expiration, the PMU generates an interrupt and if the CPU
+ * doesn't clear it within one second the system is restarted.
+ */
+static const struct {
+	u8 reg_val;
+	int time;
+} rn5t618_wdt_map[] = {
+	{ 0, 1 },
+	{ 1, 8 },
+	{ 2, 32 },
+	{ 3, 128 },
+};
+
+static int rn5t618_wdt_set_timeout(struct watchdog_device *wdt_dev,
+				   unsigned int timeout)
+{
+	struct rn5t618_wdt *wdt = watchdog_get_drvdata(wdt_dev);
+	int ret, i;
+
+	for (i = 0; i < ARRAY_SIZE(rn5t618_wdt_map); i++) {
+		if (rn5t618_wdt_map[i].time + 1 >= timeout)
+			break;
+	}
+
+	if (i == ARRAY_SIZE(rn5t618_wdt_map))
+		ret = -EINVAL;
+	else
+		ret = regmap_update_bits(wdt->rn5t618->regmap, RN5T618_WATCHDOG,
+					 RN5T618_WATCHDOG_WDOGTIM_M,
+					 rn5t618_wdt_map[i].reg_val);
+	if (!ret)
+		wdt_dev->timeout = rn5t618_wdt_map[i].time;
+
+	return ret;
+}
+
+static int rn5t618_wdt_start(struct watchdog_device *wdt_dev)
+{
+	struct rn5t618_wdt *wdt = watchdog_get_drvdata(wdt_dev);
+	int ret;
+
+	ret = rn5t618_wdt_set_timeout(wdt_dev, wdt_dev->timeout);
+	if (ret)
+		return ret;
+
+	/* enable repower-on */
+	ret = regmap_update_bits(wdt->rn5t618->regmap, RN5T618_REPCNT,
+				 RN5T618_REPCNT_REPWRON,
+				 RN5T618_REPCNT_REPWRON);
+	if (ret)
+		return ret;
+
+	/* enable watchdog */
+	ret = regmap_update_bits(wdt->rn5t618->regmap, RN5T618_WATCHDOG,
+				 RN5T618_WATCHDOG_WDOGEN,
+				 RN5T618_WATCHDOG_WDOGEN);
+	if (ret)
+		return ret;
+
+	/* enable watchdog interrupt */
+	return regmap_update_bits(wdt->rn5t618->regmap, RN5T618_PWRIREN,
+				  RN5T618_PWRIRQ_IR_WDOG,
+				  RN5T618_PWRIRQ_IR_WDOG);
+}
+
+static int rn5t618_wdt_stop(struct watchdog_device *wdt_dev)
+{
+	struct rn5t618_wdt *wdt = watchdog_get_drvdata(wdt_dev);
+
+	return regmap_update_bits(wdt->rn5t618->regmap, RN5T618_WATCHDOG,
+				  RN5T618_WATCHDOG_WDOGEN, 0);
+}
+
+static int rn5t618_wdt_ping(struct watchdog_device *wdt_dev)
+{
+	struct rn5t618_wdt *wdt = watchdog_get_drvdata(wdt_dev);
+	unsigned int val;
+	int ret;
+
+	/* The counter is restarted after a R/W access to watchdog register */
+	ret = regmap_read(wdt->rn5t618->regmap, RN5T618_WATCHDOG, &val);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(wdt->rn5t618->regmap, RN5T618_WATCHDOG, val);
+	if (ret)
+		return ret;
+
+	/* Clear pending watchdog interrupt */
+	return regmap_update_bits(wdt->rn5t618->regmap, RN5T618_PWRIRQ,
+				  RN5T618_PWRIRQ_IR_WDOG, 0);
+}
+
+static struct watchdog_info rn5t618_wdt_info = {
+	.options	= WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE |
+			  WDIOF_KEEPALIVEPING,
+	.identity	= DRIVER_NAME,
+};
+
+static struct watchdog_ops rn5t618_wdt_ops = {
+	.owner          = THIS_MODULE,
+	.start          = rn5t618_wdt_start,
+	.stop           = rn5t618_wdt_stop,
+	.ping           = rn5t618_wdt_ping,
+	.set_timeout    = rn5t618_wdt_set_timeout,
+};
+
+static int rn5t618_wdt_probe(struct platform_device *pdev)
+{
+	struct rn5t618 *rn5t618 = dev_get_drvdata(pdev->dev.parent);
+	struct rn5t618_wdt *wdt;
+	int min_timeout, max_timeout;
+
+	wdt = devm_kzalloc(&pdev->dev, sizeof(struct rn5t618_wdt), GFP_KERNEL);
+	if (!wdt)
+		return -ENOMEM;
+
+	min_timeout = rn5t618_wdt_map[0].time;
+	max_timeout = rn5t618_wdt_map[ARRAY_SIZE(rn5t618_wdt_map) - 1].time;
+
+	wdt->rn5t618 = rn5t618;
+	wdt->wdt_dev.info = &rn5t618_wdt_info;
+	wdt->wdt_dev.ops = &rn5t618_wdt_ops;
+	wdt->wdt_dev.min_timeout = min_timeout;
+	wdt->wdt_dev.max_timeout = max_timeout;
+	wdt->wdt_dev.timeout = max_timeout;
+	wdt->wdt_dev.parent = &pdev->dev;
+
+	watchdog_set_drvdata(&wdt->wdt_dev, wdt);
+	watchdog_init_timeout(&wdt->wdt_dev, heartbeat, &pdev->dev);
+	watchdog_set_nowayout(&wdt->wdt_dev, nowayout);
+
+	return watchdog_register_device(&wdt->wdt_dev);
+}
+
+static int rn5t618_wdt_remove(struct platform_device *pdev)
+{
+	struct watchdog_device *wdt_dev = platform_get_drvdata(pdev);
+
+	watchdog_unregister_device(wdt_dev);
+
+	return 0;
+}
+
+static struct platform_driver rn5t618_wdt_driver = {
+	.probe = rn5t618_wdt_probe,
+	.remove = rn5t618_wdt_remove,
+	.driver = {
+		.name	= DRIVER_NAME,
+	},
+};
+
+module_platform_driver(rn5t618_wdt_driver);
+
+MODULE_AUTHOR("Beniamino Galvani <b.galvani@gmail.com>");
+MODULE_DESCRIPTION("RN5T618 watchdog driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/rn5t618.h b/include/linux/mfd/rn5t618.h
index e3571d1..c72d534 100644
--- a/include/linux/mfd/rn5t618.h
+++ b/include/linux/mfd/rn5t618.h
@@ -202,6 +202,10 @@
 
 #define RN5T618_REPCNT_REPWRON		BIT(0)
 #define RN5T618_SLPCNT_SWPWROFF		BIT(0)
+#define RN5T618_WATCHDOG_WDOGEN		BIT(2)
+#define RN5T618_WATCHDOG_WDOGTIM_M	(BIT(0) | BIT(1))
+#define RN5T618_WATCHDOG_WDOGTIM_S	0
+#define RN5T618_PWRIRQ_IR_WDOG		BIT(6)
 
 enum {
 	RN5T618_DCDC1,
-- 
1.7.10.4

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

[Guenter Roeck]

On Wed, Aug 27, 2014 at 12:13:56AM +0200, Beniamino Galvani wrote:
> 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@gmail.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@gmail.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;

I'd be surprised if this doesn't cause a compiler warning.
Why not just initialize the variable with 0 ?

> +
> +module_param(heartbeat, int, 0);
> +MODULE_PARM_DESC(heartbeat, "Initial watchdog heartbeat in seconds");
> +
> +module_param(nowayout, bool, 0);
> +MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
> +		 __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> +
> +struct rn5t618_wdt {
> +	struct watchdog_device wdt_dev;
> +	struct rn5t618 *rn5t618;
> +};
> +
> +/*
> + * This array encodes the values of WDOGTIM field for the supported
> + * watchdog expiration times. If the watchdog is not accessed before
> + * the timer expiration, the PMU generates an interrupt and if the CPU
> + * doesn't clear it within one second the system is restarted.
> + */
> +static const struct {
> +	u8 reg_val;
> +	int time;
> +} rn5t618_wdt_map[] = {
> +	{ 0, 1 },
> +	{ 1, 8 },
> +	{ 2, 32 },
> +	{ 3, 128 },
> +};
> +
> +static int rn5t618_wdt_set_timeout(struct watchdog_device *wdt_dev,
> +				   unsigned int timeout)
> +{
> +	struct rn5t618_wdt *wdt = watchdog_get_drvdata(wdt_dev);
> +	int ret, i;
> +
> +	for (i = 0; i < ARRAY_SIZE(rn5t618_wdt_map); i++) {
> +		if (rn5t618_wdt_map[i].time + 1 >= timeout)
> +			break;
> +	}
> +
> +	if (i == ARRAY_SIZE(rn5t618_wdt_map))
> +		ret = -EINVAL;

Can you simplify this a bit ? If you use

	if (i == ARRAY_SIZE(rn5t618_wdt_map))
		return -EINVAL;

> +	else

You can drop this else statement.

Thanks,
Guenter

> +		ret = regmap_update_bits(wdt->rn5t618->regmap, RN5T618_WATCHDOG,
> +					 RN5T618_WATCHDOG_WDOGTIM_M,
> +					 rn5t618_wdt_map[i].reg_val);
> +	if (!ret)
> +		wdt_dev->timeout = rn5t618_wdt_map[i].time;
> +
> +	return ret;
> +}
> +
> +static int rn5t618_wdt_start(struct watchdog_device *wdt_dev)
> +{
> +	struct rn5t618_wdt *wdt = watchdog_get_drvdata(wdt_dev);
> +	int ret;
> +
> +	ret = rn5t618_wdt_set_timeout(wdt_dev, wdt_dev->timeout);
> +	if (ret)
> +		return ret;
> +
> +	/* enable repower-on */
> +	ret = regmap_update_bits(wdt->rn5t618->regmap, RN5T618_REPCNT,
> +				 RN5T618_REPCNT_REPWRON,
> +				 RN5T618_REPCNT_REPWRON);
> +	if (ret)
> +		return ret;
> +
> +	/* enable watchdog */
> +	ret = regmap_update_bits(wdt->rn5t618->regmap, RN5T618_WATCHDOG,
> +				 RN5T618_WATCHDOG_WDOGEN,
> +				 RN5T618_WATCHDOG_WDOGEN);
> +	if (ret)
> +		return ret;
> +
> +	/* enable watchdog interrupt */
> +	return regmap_update_bits(wdt->rn5t618->regmap, RN5T618_PWRIREN,
> +				  RN5T618_PWRIRQ_IR_WDOG,
> +				  RN5T618_PWRIRQ_IR_WDOG);
> +}
> +
> +static int rn5t618_wdt_stop(struct watchdog_device *wdt_dev)
> +{
> +	struct rn5t618_wdt *wdt = watchdog_get_drvdata(wdt_dev);
> +
> +	return regmap_update_bits(wdt->rn5t618->regmap, RN5T618_WATCHDOG,
> +				  RN5T618_WATCHDOG_WDOGEN, 0);
> +}
> +
> +static int rn5t618_wdt_ping(struct watchdog_device *wdt_dev)
> +{
> +	struct rn5t618_wdt *wdt = watchdog_get_drvdata(wdt_dev);
> +	unsigned int val;
> +	int ret;
> +
> +	/* The counter is restarted after a R/W access to watchdog register */
> +	ret = regmap_read(wdt->rn5t618->regmap, RN5T618_WATCHDOG, &val);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(wdt->rn5t618->regmap, RN5T618_WATCHDOG, val);
> +	if (ret)
> +		return ret;
> +
> +	/* Clear pending watchdog interrupt */
> +	return regmap_update_bits(wdt->rn5t618->regmap, RN5T618_PWRIRQ,
> +				  RN5T618_PWRIRQ_IR_WDOG, 0);
> +}
> +
> +static struct watchdog_info rn5t618_wdt_info = {
> +	.options	= WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE |
> +			  WDIOF_KEEPALIVEPING,
> +	.identity	= DRIVER_NAME,
> +};
> +
> +static struct watchdog_ops rn5t618_wdt_ops = {
> +	.owner          = THIS_MODULE,
> +	.start          = rn5t618_wdt_start,
> +	.stop           = rn5t618_wdt_stop,
> +	.ping           = rn5t618_wdt_ping,
> +	.set_timeout    = rn5t618_wdt_set_timeout,
> +};
> +
> +static int rn5t618_wdt_probe(struct platform_device *pdev)
> +{
> +	struct rn5t618 *rn5t618 = dev_get_drvdata(pdev->dev.parent);
> +	struct rn5t618_wdt *wdt;
> +	int min_timeout, max_timeout;
> +
> +	wdt = devm_kzalloc(&pdev->dev, sizeof(struct rn5t618_wdt), GFP_KERNEL);
> +	if (!wdt)
> +		return -ENOMEM;
> +
> +	min_timeout = rn5t618_wdt_map[0].time;
> +	max_timeout = rn5t618_wdt_map[ARRAY_SIZE(rn5t618_wdt_map) - 1].time;
> +
> +	wdt->rn5t618 = rn5t618;
> +	wdt->wdt_dev.info = &rn5t618_wdt_info;
> +	wdt->wdt_dev.ops = &rn5t618_wdt_ops;
> +	wdt->wdt_dev.min_timeout = min_timeout;
> +	wdt->wdt_dev.max_timeout = max_timeout;
> +	wdt->wdt_dev.timeout = max_timeout;
> +	wdt->wdt_dev.parent = &pdev->dev;
> +
> +	watchdog_set_drvdata(&wdt->wdt_dev, wdt);
> +	watchdog_init_timeout(&wdt->wdt_dev, heartbeat, &pdev->dev);
> +	watchdog_set_nowayout(&wdt->wdt_dev, nowayout);
> +
> +	return watchdog_register_device(&wdt->wdt_dev);
> +}
> +
> +static int rn5t618_wdt_remove(struct platform_device *pdev)
> +{
> +	struct watchdog_device *wdt_dev = platform_get_drvdata(pdev);
> +
> +	watchdog_unregister_device(wdt_dev);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver rn5t618_wdt_driver = {
> +	.probe = rn5t618_wdt_probe,
> +	.remove = rn5t618_wdt_remove,
> +	.driver = {
> +		.name	= DRIVER_NAME,
> +	},
> +};
> +
> +module_platform_driver(rn5t618_wdt_driver);
> +
> +MODULE_AUTHOR("Beniamino Galvani <b.galvani@gmail.com>");
> +MODULE_DESCRIPTION("RN5T618 watchdog driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mfd/rn5t618.h b/include/linux/mfd/rn5t618.h
> index e3571d1..c72d534 100644
> --- a/include/linux/mfd/rn5t618.h
> +++ b/include/linux/mfd/rn5t618.h
> @@ -202,6 +202,10 @@
>  
>  #define RN5T618_REPCNT_REPWRON		BIT(0)
>  #define RN5T618_SLPCNT_SWPWROFF		BIT(0)
> +#define RN5T618_WATCHDOG_WDOGEN		BIT(2)
> +#define RN5T618_WATCHDOG_WDOGTIM_M	(BIT(0) | BIT(1))
> +#define RN5T618_WATCHDOG_WDOGTIM_S	0
> +#define RN5T618_PWRIRQ_IR_WDOG		BIT(6)
>  
>  enum {
>  	RN5T618_DCDC1,
> -- 
> 1.7.10.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

[Beniamino Galvani]

On Wed, Aug 27, 2014 at 12:01:35PM -0700, Guenter Roeck wrote:
[...]
> > +#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;
> 
> I'd be surprised if this doesn't cause a compiler warning.
> Why not just initialize the variable with 0 ?

The idea was to initialize the variable with an invalid value that has
no effect when watchdog_init_timeout() is called during probe, thus
leaving the watchdog timeout to the maximum value.

You are right, the same effect can be achieved in a cleaner way
leaving the variable to zero (by the way, the above assignment doesn't
seem to generate warnings).

> 
> > +
> > +module_param(heartbeat, int, 0);
> > +MODULE_PARM_DESC(heartbeat, "Initial watchdog heartbeat in seconds");
> > +
> > +module_param(nowayout, bool, 0);
> > +MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
> > +		 __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> > +
> > +struct rn5t618_wdt {
> > +	struct watchdog_device wdt_dev;
> > +	struct rn5t618 *rn5t618;
> > +};

[..]

> > +static int rn5t618_wdt_set_timeout(struct watchdog_device *wdt_dev,
> > +				   unsigned int timeout)
> > +{
> > +	struct rn5t618_wdt *wdt = watchdog_get_drvdata(wdt_dev);
> > +	int ret, i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(rn5t618_wdt_map); i++) {
> > +		if (rn5t618_wdt_map[i].time + 1 >= timeout)
> > +			break;
> > +	}
> > +
> > +	if (i == ARRAY_SIZE(rn5t618_wdt_map))
> > +		ret = -EINVAL;
> 
> Can you simplify this a bit ? If you use
> 
> 	if (i == ARRAY_SIZE(rn5t618_wdt_map))
> 		return -EINVAL;
> 
> > +	else
> 
> You can drop this else statement.

Will do, thanks.

Beniamino

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

[Lee Jones]

On Wed, 27 Aug 2014, Guenter Roeck wrote:
> On Wed, Aug 27, 2014 at 12:13:56AM +0200, Beniamino Galvani wrote:
> > 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@gmail.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

[...]

> > +++ b/drivers/watchdog/rn5t618_wdt.c
> > @@ -0,0 +1,196 @@

[...]

> > +static int rn5t618_wdt_set_timeout(struct watchdog_device *wdt_dev,
> > +				   unsigned int timeout)
> > +{
> > +	struct rn5t618_wdt *wdt = watchdog_get_drvdata(wdt_dev);
> > +	int ret, i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(rn5t618_wdt_map); i++) {
> > +		if (rn5t618_wdt_map[i].time + 1 >= timeout)
> > +			break;
> > +	}
> > +
> > +	if (i == ARRAY_SIZE(rn5t618_wdt_map))
> > +		ret = -EINVAL;
> 
> Can you simplify this a bit ? If you use
> 
> 	if (i == ARRAY_SIZE(rn5t618_wdt_map))
> 		return -EINVAL;

This changes the semantics.

> > +	else
> 
> You can drop this else statement.
> 
> > +		ret = regmap_update_bits(wdt->rn5t618->regmap, RN5T618_WATCHDOG,
> > +					 RN5T618_WATCHDOG_WDOGTIM_M,
> > +					 rn5t618_wdt_map[i].reg_val);
> > +	if (!ret)
> > +		wdt_dev->timeout = rn5t618_wdt_map[i].time;

... Isn't this important?

> > +	return ret;
> > +}

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

[Guenter Roeck]

On 08/28/2014 12:19 AM, Lee Jones wrote:
> On Wed, 27 Aug 2014, Guenter Roeck wrote:
>> On Wed, Aug 27, 2014 at 12:13:56AM +0200, Beniamino Galvani wrote:
>>> 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@gmail.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
>
> [...]
>
>>> +++ b/drivers/watchdog/rn5t618_wdt.c
>>> @@ -0,0 +1,196 @@
>
> [...]
>
>>> +static int rn5t618_wdt_set_timeout(struct watchdog_device *wdt_dev,
>>> +				   unsigned int timeout)
>>> +{
>>> +	struct rn5t618_wdt *wdt = watchdog_get_drvdata(wdt_dev);
>>> +	int ret, i;
>>> +
>>> +	for (i = 0; i < ARRAY_SIZE(rn5t618_wdt_map); i++) {
>>> +		if (rn5t618_wdt_map[i].time + 1 >= timeout)
>>> +			break;
>>> +	}
>>> +
>>> +	if (i == ARRAY_SIZE(rn5t618_wdt_map))
>>> +		ret = -EINVAL;
>>
>> Can you simplify this a bit ? If you use
>>
>> 	if (i == ARRAY_SIZE(rn5t618_wdt_map))
>> 		return -EINVAL;
>
> This changes the semantics.
>
How so ? If ret is set to -EINVAL, the rest of the function won't do anything
but eventually return -EINVAL. I don't see why returning -EINVAL immediately
would change that.

Guenter

>>> +	else
>>
>> You can drop this else statement.
>>
>>> +		ret = regmap_update_bits(wdt->rn5t618->regmap, RN5T618_WATCHDOG,
>>> +					 RN5T618_WATCHDOG_WDOGTIM_M,
>>> +					 rn5t618_wdt_map[i].reg_val);
>>> +	if (!ret)
>>> +		wdt_dev->timeout = rn5t618_wdt_map[i].time;
>
> ... Isn't this important?
>
>>> +	return ret;
>>> +}
>

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

[Lee Jones]

On Thu, 28 Aug 2014, Guenter Roeck wrote:

> On 08/28/2014 12:19 AM, Lee Jones wrote:
> >On Wed, 27 Aug 2014, Guenter Roeck wrote:
> >>On Wed, Aug 27, 2014 at 12:13:56AM +0200, Beniamino Galvani wrote:
> >>>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@gmail.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
> >
> >[...]
> >
> >>>+++ b/drivers/watchdog/rn5t618_wdt.c
> >>>@@ -0,0 +1,196 @@
> >
> >[...]
> >
> >>>+static int rn5t618_wdt_set_timeout(struct watchdog_device *wdt_dev,
> >>>+				   unsigned int timeout)
> >>>+{
> >>>+	struct rn5t618_wdt *wdt = watchdog_get_drvdata(wdt_dev);
> >>>+	int ret, i;
> >>>+
> >>>+	for (i = 0; i < ARRAY_SIZE(rn5t618_wdt_map); i++) {
> >>>+		if (rn5t618_wdt_map[i].time + 1 >= timeout)
> >>>+			break;
> >>>+	}
> >>>+
> >>>+	if (i == ARRAY_SIZE(rn5t618_wdt_map))
> >>>+		ret = -EINVAL;
> >>
> >>Can you simplify this a bit ? If you use
> >>
> >>	if (i == ARRAY_SIZE(rn5t618_wdt_map))
> >>		return -EINVAL;
> >
> >This changes the semantics.
> >
> How so ? If ret is set to -EINVAL, the rest of the function won't do anything
> but eventually return -EINVAL. I don't see why returning -EINVAL immediately
> would change that.

Ah, you're right.

I read:

  if (!ret)
    wdt_dev->timeout = rn5t618_wdt_map[i].time;

As:

  if (ret)
    wdt_dev->timeout = rn5t618_wdt_map[i].time;

My bad - withdrawn.

> >>>+	else
> >>
> >>You can drop this else statement.
> >>
> >>>+		ret = regmap_update_bits(wdt->rn5t618->regmap, RN5T618_WATCHDOG,
> >>>+					 RN5T618_WATCHDOG_WDOGTIM_M,
> >>>+					 rn5t618_wdt_map[i].reg_val);
> >>>+	if (!ret)
> >>>+		wdt_dev->timeout = rn5t618_wdt_map[i].time;
> >
> >... Isn't this important?
> >
> >>>+	return ret;
> >>>+}
> >
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Lee Jones]

Wim,

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@gmail.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@gmail.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");
> +
> +module_param(nowayout, bool, 0);
> +MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
> +		 __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> +
> +struct rn5t618_wdt {
> +	struct watchdog_device wdt_dev;
> +	struct rn5t618 *rn5t618;
> +};
> +
> +/*
> + * This array encodes the values of WDOGTIM field for the supported
> + * watchdog expiration times. If the watchdog is not accessed before
> + * the timer expiration, the PMU generates an interrupt and if the CPU
> + * doesn't clear it within one second the system is restarted.
> + */
> +static const struct {
> +	u8 reg_val;
> +	int time;
> +} rn5t618_wdt_map[] = {
> +	{ 0, 1 },
> +	{ 1, 8 },
> +	{ 2, 32 },
> +	{ 3, 128 },
> +};
> +
> +static int rn5t618_wdt_set_timeout(struct watchdog_device *wdt_dev,
> +				   unsigned int timeout)
> +{
> +	struct rn5t618_wdt *wdt = watchdog_get_drvdata(wdt_dev);
> +	int ret, i;
> +
> +	for (i = 0; i < ARRAY_SIZE(rn5t618_wdt_map); i++) {
> +		if (rn5t618_wdt_map[i].time + 1 >= timeout)
> +			break;
> +	}
> +
> +	if (i == ARRAY_SIZE(rn5t618_wdt_map))
> +		ret = -EINVAL;
> +	else
> +		ret = regmap_update_bits(wdt->rn5t618->regmap, RN5T618_WATCHDOG,
> +					 RN5T618_WATCHDOG_WDOGTIM_M,
> +					 rn5t618_wdt_map[i].reg_val);
> +	if (!ret)
> +		wdt_dev->timeout = rn5t618_wdt_map[i].time;
> +
> +	return ret;
> +}
> +
> +static int rn5t618_wdt_start(struct watchdog_device *wdt_dev)
> +{
> +	struct rn5t618_wdt *wdt = watchdog_get_drvdata(wdt_dev);
> +	int ret;
> +
> +	ret = rn5t618_wdt_set_timeout(wdt_dev, wdt_dev->timeout);
> +	if (ret)
> +		return ret;
> +
> +	/* enable repower-on */
> +	ret = regmap_update_bits(wdt->rn5t618->regmap, RN5T618_REPCNT,
> +				 RN5T618_REPCNT_REPWRON,
> +				 RN5T618_REPCNT_REPWRON);
> +	if (ret)
> +		return ret;
> +
> +	/* enable watchdog */
> +	ret = regmap_update_bits(wdt->rn5t618->regmap, RN5T618_WATCHDOG,
> +				 RN5T618_WATCHDOG_WDOGEN,
> +				 RN5T618_WATCHDOG_WDOGEN);
> +	if (ret)
> +		return ret;
> +
> +	/* enable watchdog interrupt */
> +	return regmap_update_bits(wdt->rn5t618->regmap, RN5T618_PWRIREN,
> +				  RN5T618_PWRIRQ_IR_WDOG,
> +				  RN5T618_PWRIRQ_IR_WDOG);
> +}
> +
> +static int rn5t618_wdt_stop(struct watchdog_device *wdt_dev)
> +{
> +	struct rn5t618_wdt *wdt = watchdog_get_drvdata(wdt_dev);
> +
> +	return regmap_update_bits(wdt->rn5t618->regmap, RN5T618_WATCHDOG,
> +				  RN5T618_WATCHDOG_WDOGEN, 0);
> +}
> +
> +static int rn5t618_wdt_ping(struct watchdog_device *wdt_dev)
> +{
> +	struct rn5t618_wdt *wdt = watchdog_get_drvdata(wdt_dev);
> +	unsigned int val;
> +	int ret;
> +
> +	/* The counter is restarted after a R/W access to watchdog register */
> +	ret = regmap_read(wdt->rn5t618->regmap, RN5T618_WATCHDOG, &val);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(wdt->rn5t618->regmap, RN5T618_WATCHDOG, val);
> +	if (ret)
> +		return ret;
> +
> +	/* Clear pending watchdog interrupt */
> +	return regmap_update_bits(wdt->rn5t618->regmap, RN5T618_PWRIRQ,
> +				  RN5T618_PWRIRQ_IR_WDOG, 0);
> +}
> +
> +static struct watchdog_info rn5t618_wdt_info = {
> +	.options	= WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE |
> +			  WDIOF_KEEPALIVEPING,
> +	.identity	= DRIVER_NAME,
> +};
> +
> +static struct watchdog_ops rn5t618_wdt_ops = {
> +	.owner          = THIS_MODULE,
> +	.start          = rn5t618_wdt_start,
> +	.stop           = rn5t618_wdt_stop,
> +	.ping           = rn5t618_wdt_ping,
> +	.set_timeout    = rn5t618_wdt_set_timeout,
> +};
> +
> +static int rn5t618_wdt_probe(struct platform_device *pdev)
> +{
> +	struct rn5t618 *rn5t618 = dev_get_drvdata(pdev->dev.parent);
> +	struct rn5t618_wdt *wdt;
> +	int min_timeout, max_timeout;
> +
> +	wdt = devm_kzalloc(&pdev->dev, sizeof(struct rn5t618_wdt), GFP_KERNEL);
> +	if (!wdt)
> +		return -ENOMEM;
> +
> +	min_timeout = rn5t618_wdt_map[0].time;
> +	max_timeout = rn5t618_wdt_map[ARRAY_SIZE(rn5t618_wdt_map) - 1].time;
> +
> +	wdt->rn5t618 = rn5t618;
> +	wdt->wdt_dev.info = &rn5t618_wdt_info;
> +	wdt->wdt_dev.ops = &rn5t618_wdt_ops;
> +	wdt->wdt_dev.min_timeout = min_timeout;
> +	wdt->wdt_dev.max_timeout = max_timeout;
> +	wdt->wdt_dev.timeout = max_timeout;
> +	wdt->wdt_dev.parent = &pdev->dev;
> +
> +	watchdog_set_drvdata(&wdt->wdt_dev, wdt);
> +	watchdog_init_timeout(&wdt->wdt_dev, heartbeat, &pdev->dev);
> +	watchdog_set_nowayout(&wdt->wdt_dev, nowayout);
> +
> +	return watchdog_register_device(&wdt->wdt_dev);
> +}
> +
> +static int rn5t618_wdt_remove(struct platform_device *pdev)
> +{
> +	struct watchdog_device *wdt_dev = platform_get_drvdata(pdev);
> +
> +	watchdog_unregister_device(wdt_dev);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver rn5t618_wdt_driver = {
> +	.probe = rn5t618_wdt_probe,
> +	.remove = rn5t618_wdt_remove,
> +	.driver = {
> +		.name	= DRIVER_NAME,
> +	},
> +};
> +
> +module_platform_driver(rn5t618_wdt_driver);
> +
> +MODULE_AUTHOR("Beniamino Galvani <b.galvani@gmail.com>");
> +MODULE_DESCRIPTION("RN5T618 watchdog driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mfd/rn5t618.h b/include/linux/mfd/rn5t618.h
> index e3571d1..c72d534 100644
> --- a/include/linux/mfd/rn5t618.h
> +++ b/include/linux/mfd/rn5t618.h
> @@ -202,6 +202,10 @@
>  
>  #define RN5T618_REPCNT_REPWRON		BIT(0)
>  #define RN5T618_SLPCNT_SWPWROFF		BIT(0)
> +#define RN5T618_WATCHDOG_WDOGEN		BIT(2)
> +#define RN5T618_WATCHDOG_WDOGTIM_M	(BIT(0) | BIT(1))
> +#define RN5T618_WATCHDOG_WDOGTIM_S	0
> +#define RN5T618_PWRIRQ_IR_WDOG		BIT(6)
>  
>  enum {
>  	RN5T618_DCDC1,

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

[Guenter Roeck]

On Thu, Sep 04, 2014 at 10:19:17AM +0100, Lee Jones wrote:
> Wim,
> 
> Still waiting on a Watchdog Ack, so I can take this set in.
> 
Wasn't there a more recent version of this patch ?

Thanks,
Guenter

> > 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@gmail.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@gmail.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");
> > +
> > +module_param(nowayout, bool, 0);
> > +MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
> > +		 __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> > +
> > +struct rn5t618_wdt {
> > +	struct watchdog_device wdt_dev;
> > +	struct rn5t618 *rn5t618;
> > +};
> > +
> > +/*
> > + * This array encodes the values of WDOGTIM field for the supported
> > + * watchdog expiration times. If the watchdog is not accessed before
> > + * the timer expiration, the PMU generates an interrupt and if the CPU
> > + * doesn't clear it within one second the system is restarted.
> > + */
> > +static const struct {
> > +	u8 reg_val;
> > +	int time;
> > +} rn5t618_wdt_map[] = {
> > +	{ 0, 1 },
> > +	{ 1, 8 },
> > +	{ 2, 32 },
> > +	{ 3, 128 },
> > +};
> > +
> > +static int rn5t618_wdt_set_timeout(struct watchdog_device *wdt_dev,
> > +				   unsigned int timeout)
> > +{
> > +	struct rn5t618_wdt *wdt = watchdog_get_drvdata(wdt_dev);
> > +	int ret, i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(rn5t618_wdt_map); i++) {
> > +		if (rn5t618_wdt_map[i].time + 1 >= timeout)
> > +			break;
> > +	}
> > +
> > +	if (i == ARRAY_SIZE(rn5t618_wdt_map))
> > +		ret = -EINVAL;
> > +	else
> > +		ret = regmap_update_bits(wdt->rn5t618->regmap, RN5T618_WATCHDOG,
> > +					 RN5T618_WATCHDOG_WDOGTIM_M,
> > +					 rn5t618_wdt_map[i].reg_val);
> > +	if (!ret)
> > +		wdt_dev->timeout = rn5t618_wdt_map[i].time;
> > +
> > +	return ret;
> > +}
> > +
> > +static int rn5t618_wdt_start(struct watchdog_device *wdt_dev)
> > +{
> > +	struct rn5t618_wdt *wdt = watchdog_get_drvdata(wdt_dev);
> > +	int ret;
> > +
> > +	ret = rn5t618_wdt_set_timeout(wdt_dev, wdt_dev->timeout);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* enable repower-on */
> > +	ret = regmap_update_bits(wdt->rn5t618->regmap, RN5T618_REPCNT,
> > +				 RN5T618_REPCNT_REPWRON,
> > +				 RN5T618_REPCNT_REPWRON);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* enable watchdog */
> > +	ret = regmap_update_bits(wdt->rn5t618->regmap, RN5T618_WATCHDOG,
> > +				 RN5T618_WATCHDOG_WDOGEN,
> > +				 RN5T618_WATCHDOG_WDOGEN);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* enable watchdog interrupt */
> > +	return regmap_update_bits(wdt->rn5t618->regmap, RN5T618_PWRIREN,
> > +				  RN5T618_PWRIRQ_IR_WDOG,
> > +				  RN5T618_PWRIRQ_IR_WDOG);
> > +}
> > +
> > +static int rn5t618_wdt_stop(struct watchdog_device *wdt_dev)
> > +{
> > +	struct rn5t618_wdt *wdt = watchdog_get_drvdata(wdt_dev);
> > +
> > +	return regmap_update_bits(wdt->rn5t618->regmap, RN5T618_WATCHDOG,
> > +				  RN5T618_WATCHDOG_WDOGEN, 0);
> > +}
> > +
> > +static int rn5t618_wdt_ping(struct watchdog_device *wdt_dev)
> > +{
> > +	struct rn5t618_wdt *wdt = watchdog_get_drvdata(wdt_dev);
> > +	unsigned int val;
> > +	int ret;
> > +
> > +	/* The counter is restarted after a R/W access to watchdog register */
> > +	ret = regmap_read(wdt->rn5t618->regmap, RN5T618_WATCHDOG, &val);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = regmap_write(wdt->rn5t618->regmap, RN5T618_WATCHDOG, val);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* Clear pending watchdog interrupt */
> > +	return regmap_update_bits(wdt->rn5t618->regmap, RN5T618_PWRIRQ,
> > +				  RN5T618_PWRIRQ_IR_WDOG, 0);
> > +}
> > +
> > +static struct watchdog_info rn5t618_wdt_info = {
> > +	.options	= WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE |
> > +			  WDIOF_KEEPALIVEPING,
> > +	.identity	= DRIVER_NAME,
> > +};
> > +
> > +static struct watchdog_ops rn5t618_wdt_ops = {
> > +	.owner          = THIS_MODULE,
> > +	.start          = rn5t618_wdt_start,
> > +	.stop           = rn5t618_wdt_stop,
> > +	.ping           = rn5t618_wdt_ping,
> > +	.set_timeout    = rn5t618_wdt_set_timeout,
> > +};
> > +
> > +static int rn5t618_wdt_probe(struct platform_device *pdev)
> > +{
> > +	struct rn5t618 *rn5t618 = dev_get_drvdata(pdev->dev.parent);
> > +	struct rn5t618_wdt *wdt;
> > +	int min_timeout, max_timeout;
> > +
> > +	wdt = devm_kzalloc(&pdev->dev, sizeof(struct rn5t618_wdt), GFP_KERNEL);
> > +	if (!wdt)
> > +		return -ENOMEM;
> > +
> > +	min_timeout = rn5t618_wdt_map[0].time;
> > +	max_timeout = rn5t618_wdt_map[ARRAY_SIZE(rn5t618_wdt_map) - 1].time;
> > +
> > +	wdt->rn5t618 = rn5t618;
> > +	wdt->wdt_dev.info = &rn5t618_wdt_info;
> > +	wdt->wdt_dev.ops = &rn5t618_wdt_ops;
> > +	wdt->wdt_dev.min_timeout = min_timeout;
> > +	wdt->wdt_dev.max_timeout = max_timeout;
> > +	wdt->wdt_dev.timeout = max_timeout;
> > +	wdt->wdt_dev.parent = &pdev->dev;
> > +
> > +	watchdog_set_drvdata(&wdt->wdt_dev, wdt);
> > +	watchdog_init_timeout(&wdt->wdt_dev, heartbeat, &pdev->dev);
> > +	watchdog_set_nowayout(&wdt->wdt_dev, nowayout);
> > +
> > +	return watchdog_register_device(&wdt->wdt_dev);
> > +}
> > +
> > +static int rn5t618_wdt_remove(struct platform_device *pdev)
> > +{
> > +	struct watchdog_device *wdt_dev = platform_get_drvdata(pdev);
> > +
> > +	watchdog_unregister_device(wdt_dev);
> > +
> > +	return 0;
> > +}
> > +
> > +static struct platform_driver rn5t618_wdt_driver = {
> > +	.probe = rn5t618_wdt_probe,
> > +	.remove = rn5t618_wdt_remove,
> > +	.driver = {
> > +		.name	= DRIVER_NAME,
> > +	},
> > +};
> > +
> > +module_platform_driver(rn5t618_wdt_driver);
> > +
> > +MODULE_AUTHOR("Beniamino Galvani <b.galvani@gmail.com>");
> > +MODULE_DESCRIPTION("RN5T618 watchdog driver");
> > +MODULE_LICENSE("GPL v2");
> > diff --git a/include/linux/mfd/rn5t618.h b/include/linux/mfd/rn5t618.h
> > index e3571d1..c72d534 100644
> > --- a/include/linux/mfd/rn5t618.h
> > +++ b/include/linux/mfd/rn5t618.h
> > @@ -202,6 +202,10 @@
> >  
> >  #define RN5T618_REPCNT_REPWRON		BIT(0)
> >  #define RN5T618_SLPCNT_SWPWROFF		BIT(0)
> > +#define RN5T618_WATCHDOG_WDOGEN		BIT(2)
> > +#define RN5T618_WATCHDOG_WDOGTIM_M	(BIT(0) | BIT(1))
> > +#define RN5T618_WATCHDOG_WDOGTIM_S	0
> > +#define RN5T618_PWRIRQ_IR_WDOG		BIT(6)
> >  
> >  enum {
> >  	RN5T618_DCDC1,
> 
> -- 
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Beniamino Galvani]

On Thu, Sep 04, 2014 at 10:25:03AM -0700, Guenter Roeck wrote:
> On Thu, Sep 04, 2014 at 10:19:17AM +0100, Lee Jones wrote:
> > Wim,
> > 
> > Still waiting on a Watchdog Ack, so I can take this set in.
> > 
> Wasn't there a more recent version of this patch ?

Yes, this is superseded by v2.

Beniamino

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

[Wim Van Sebroeck]

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@gmail.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@gmail.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.

Kind regards,
Wim.

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

[Beniamino Galvani]

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@gmail.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@gmail.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

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

[Wim Van Sebroeck]

Hi Beniamino,

> 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@gmail.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@gmail.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");

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.

Kind regards,
Wim.

> +
> +module_param(nowayout, bool, 0);
> +MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
> +		 __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> +
> +struct rn5t618_wdt {
> +	struct watchdog_device wdt_dev;
> +	struct rn5t618 *rn5t618;
> +};
> +
> +/*
> + * This array encodes the values of WDOGTIM field for the supported
> + * watchdog expiration times. If the watchdog is not accessed before
> + * the timer expiration, the PMU generates an interrupt and if the CPU
> + * doesn't clear it within one second the system is restarted.
> + */
> +static const struct {
> +	u8 reg_val;
> +	int time;
> +} rn5t618_wdt_map[] = {
> +	{ 0, 1 },
> +	{ 1, 8 },
> +	{ 2, 32 },
> +	{ 3, 128 },
> +};
> +
> +static int rn5t618_wdt_set_timeout(struct watchdog_device *wdt_dev,
> +				   unsigned int timeout)
> +{
> +	struct rn5t618_wdt *wdt = watchdog_get_drvdata(wdt_dev);
> +	int ret, i;
> +
> +	for (i = 0; i < ARRAY_SIZE(rn5t618_wdt_map); i++) {
> +		if (rn5t618_wdt_map[i].time + 1 >= timeout)
> +			break;
> +	}
> +
> +	if (i == ARRAY_SIZE(rn5t618_wdt_map))
> +		ret = -EINVAL;
> +	else
> +		ret = regmap_update_bits(wdt->rn5t618->regmap, RN5T618_WATCHDOG,
> +					 RN5T618_WATCHDOG_WDOGTIM_M,
> +					 rn5t618_wdt_map[i].reg_val);
> +	if (!ret)
> +		wdt_dev->timeout = rn5t618_wdt_map[i].time;
> +
> +	return ret;
> +}
> +
> +static int rn5t618_wdt_start(struct watchdog_device *wdt_dev)
> +{
> +	struct rn5t618_wdt *wdt = watchdog_get_drvdata(wdt_dev);
> +	int ret;
> +
> +	ret = rn5t618_wdt_set_timeout(wdt_dev, wdt_dev->timeout);
> +	if (ret)
> +		return ret;
> +
> +	/* enable repower-on */
> +	ret = regmap_update_bits(wdt->rn5t618->regmap, RN5T618_REPCNT,
> +				 RN5T618_REPCNT_REPWRON,
> +				 RN5T618_REPCNT_REPWRON);
> +	if (ret)
> +		return ret;
> +
> +	/* enable watchdog */
> +	ret = regmap_update_bits(wdt->rn5t618->regmap, RN5T618_WATCHDOG,
> +				 RN5T618_WATCHDOG_WDOGEN,
> +				 RN5T618_WATCHDOG_WDOGEN);
> +	if (ret)
> +		return ret;
> +
> +	/* enable watchdog interrupt */
> +	return regmap_update_bits(wdt->rn5t618->regmap, RN5T618_PWRIREN,
> +				  RN5T618_PWRIRQ_IR_WDOG,
> +				  RN5T618_PWRIRQ_IR_WDOG);
> +}
> +
> +static int rn5t618_wdt_stop(struct watchdog_device *wdt_dev)
> +{
> +	struct rn5t618_wdt *wdt = watchdog_get_drvdata(wdt_dev);
> +
> +	return regmap_update_bits(wdt->rn5t618->regmap, RN5T618_WATCHDOG,
> +				  RN5T618_WATCHDOG_WDOGEN, 0);
> +}
> +
> +static int rn5t618_wdt_ping(struct watchdog_device *wdt_dev)
> +{
> +	struct rn5t618_wdt *wdt = watchdog_get_drvdata(wdt_dev);
> +	unsigned int val;
> +	int ret;
> +
> +	/* The counter is restarted after a R/W access to watchdog register */
> +	ret = regmap_read(wdt->rn5t618->regmap, RN5T618_WATCHDOG, &val);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(wdt->rn5t618->regmap, RN5T618_WATCHDOG, val);
> +	if (ret)
> +		return ret;
> +
> +	/* Clear pending watchdog interrupt */
> +	return regmap_update_bits(wdt->rn5t618->regmap, RN5T618_PWRIRQ,
> +				  RN5T618_PWRIRQ_IR_WDOG, 0);
> +}
> +
> +static struct watchdog_info rn5t618_wdt_info = {
> +	.options	= WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE |
> +			  WDIOF_KEEPALIVEPING,
> +	.identity	= DRIVER_NAME,
> +};
> +
> +static struct watchdog_ops rn5t618_wdt_ops = {
> +	.owner          = THIS_MODULE,
> +	.start          = rn5t618_wdt_start,
> +	.stop           = rn5t618_wdt_stop,
> +	.ping           = rn5t618_wdt_ping,
> +	.set_timeout    = rn5t618_wdt_set_timeout,
> +};
> +
> +static int rn5t618_wdt_probe(struct platform_device *pdev)
> +{
> +	struct rn5t618 *rn5t618 = dev_get_drvdata(pdev->dev.parent);
> +	struct rn5t618_wdt *wdt;
> +	int min_timeout, max_timeout;
> +
> +	wdt = devm_kzalloc(&pdev->dev, sizeof(struct rn5t618_wdt), GFP_KERNEL);
> +	if (!wdt)
> +		return -ENOMEM;
> +
> +	min_timeout = rn5t618_wdt_map[0].time;
> +	max_timeout = rn5t618_wdt_map[ARRAY_SIZE(rn5t618_wdt_map) - 1].time;
> +
> +	wdt->rn5t618 = rn5t618;
> +	wdt->wdt_dev.info = &rn5t618_wdt_info;
> +	wdt->wdt_dev.ops = &rn5t618_wdt_ops;
> +	wdt->wdt_dev.min_timeout = min_timeout;
> +	wdt->wdt_dev.max_timeout = max_timeout;
> +	wdt->wdt_dev.timeout = max_timeout;
> +	wdt->wdt_dev.parent = &pdev->dev;
> +
> +	watchdog_set_drvdata(&wdt->wdt_dev, wdt);
> +	watchdog_init_timeout(&wdt->wdt_dev, heartbeat, &pdev->dev);
> +	watchdog_set_nowayout(&wdt->wdt_dev, nowayout);
> +
> +	return watchdog_register_device(&wdt->wdt_dev);
> +}
> +
> +static int rn5t618_wdt_remove(struct platform_device *pdev)
> +{
> +	struct watchdog_device *wdt_dev = platform_get_drvdata(pdev);
> +
> +	watchdog_unregister_device(wdt_dev);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver rn5t618_wdt_driver = {
> +	.probe = rn5t618_wdt_probe,
> +	.remove = rn5t618_wdt_remove,
> +	.driver = {
> +		.name	= DRIVER_NAME,
> +	},
> +};
> +
> +module_platform_driver(rn5t618_wdt_driver);
> +
> +MODULE_AUTHOR("Beniamino Galvani <b.galvani@gmail.com>");
> +MODULE_DESCRIPTION("RN5T618 watchdog driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mfd/rn5t618.h b/include/linux/mfd/rn5t618.h
> index e3571d1..c72d534 100644
> --- a/include/linux/mfd/rn5t618.h
> +++ b/include/linux/mfd/rn5t618.h
> @@ -202,6 +202,10 @@
>  
>  #define RN5T618_REPCNT_REPWRON		BIT(0)
>  #define RN5T618_SLPCNT_SWPWROFF		BIT(0)
> +#define RN5T618_WATCHDOG_WDOGEN		BIT(2)
> +#define RN5T618_WATCHDOG_WDOGTIM_M	(BIT(0) | BIT(1))
> +#define RN5T618_WATCHDOG_WDOGTIM_S	0
> +#define RN5T618_PWRIRQ_IR_WDOG		BIT(6)
>  
>  enum {
>  	RN5T618_DCDC1,
> -- 
> 1.7.10.4
> 

[PATCH 4/4] mfd: rn5t618: document device tree bindings

[Beniamino Galvani]

This adds the device tree bindings documentation for Ricoh RN5T618.

Signed-off-by: Beniamino Galvani <b.galvani@gmail.com>
---
 Documentation/devicetree/bindings/mfd/rn5t618.txt |   36 +++++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/rn5t618.txt

diff --git a/Documentation/devicetree/bindings/mfd/rn5t618.txt b/Documentation/devicetree/bindings/mfd/rn5t618.txt
new file mode 100644
index 0000000..937785a
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/rn5t618.txt
@@ -0,0 +1,36 @@
+* Ricoh RN5T618 PMIC
+
+Ricoh RN5T618 is a power management IC which integrates 3 step-down
+DCDC converters, 7 low-dropout regulators, a Li-ion battery charger,
+fuel gauge, ADC, GPIOs and a watchdog timer. It can be controlled
+through a I2C interface.
+
+Required properties:
+ - compatible: should be "ricoh,rn5t618"
+ - reg: the I2C slave address of the device
+
+Sub-nodes:
+ - regulators: the node is required if the regulator functionality is
+   needed. The valid regulator names are: DCDC1, DCDC2, DCDC3, LDO1,
+   LDO2, LDO3, LDO4, LDO5, LDORTC1 and LDORTC2.
+   The common bindings for each individual regulator can be found in:
+   Documentation/devicetree/bindings/regulator/regulator.txt
+
+Example:
+
+	pmic@32 {
+		compatible = "ricoh,rn5t618";
+		reg = <0x32>;
+
+		regulators {
+			DCDC1 {
+				regulator-min-microvolt = <1050000>;
+				regulator-max-microvolt = <1050000>;
+			};
+
+			DCDC2 {
+				regulator-min-microvolt = <1175000>;
+				regulator-max-microvolt = <1175000>;
+			};
+		};
+	};
-- 
1.7.10.4

Re: [PATCH 4/4] mfd: rn5t618: document device tree bindings

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 182 bytes --]

On Wed, Aug 27, 2014 at 12:13:57AM +0200, Beniamino Galvani wrote:
> This adds the device tree bindings documentation for Ricoh RN5T618.

Reviwed-by: Mark Brown <broonie@linaro.org>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]