[PATCH 0/4] rtc: pcf2127: tamper timestamp and watchdog feature support

[PATCH 0/4] rtc: pcf2127: tamper timestamp and watchdog feature support

[Bruno Thomsen]

Hi Alexandre

This patch series adds support for 2 chip features; tamper
timestamp and watchdog. Including improvements and cleanup
of existing code.

Tamper timestamp sysfs interface should match ISL1208 and
RV3028 RTC drivers; /sys/class/rtc/rtcX/timestamp0.

Watchdog functionality can be enabled with a Kconfig option,
just like in DS1374 and M41T80 RTC drivers.

In the future we could consider making a generic optional
RTC device tree binding property to enable the watchdog
feature, ex. enable-watchdog.

Patches has been tested on a pcf2127 chip using 2MHz SPI
interface both as built-in and module including with and
without watchdog feature.

/Bruno

Bruno Thomsen (4):
  rtc: pcf2127: convert to devm_rtc_allocate_device
  rtc: pcf2127: cleanup register and bit defines
  rtc: pcf2127: add tamper detection support
  rtc: pcf2127: add watchdog feature support

 drivers/rtc/Kconfig       |  10 ++
 drivers/rtc/rtc-pcf2127.c | 360 +++++++++++++++++++++++++++++++++++---
 2 files changed, 341 insertions(+), 29 deletions(-)

-- 
2.21.0

[PATCH 1/4] rtc: pcf2127: convert to devm_rtc_allocate_device

[Bruno Thomsen]

This allows further improvement of the driver.

Signed-off-by: Bruno Thomsen <bruno.thomsen@gmail.com>
---
 drivers/rtc/rtc-pcf2127.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index 8632f58fed43..58eb96506e4b 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -237,11 +237,12 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap,
 
 	dev_set_drvdata(dev, pcf2127);
 
-	pcf2127->rtc = devm_rtc_device_register(dev, name, &pcf2127_rtc_ops,
-						THIS_MODULE);
+	pcf2127->rtc = devm_rtc_allocate_device(dev);
 	if (IS_ERR(pcf2127->rtc))
 		return PTR_ERR(pcf2127->rtc);
 
+	pcf2127->rtc->ops = &pcf2127_rtc_ops;
+
 	if (has_nvmem) {
 		struct nvmem_config nvmem_cfg = {
 			.priv = pcf2127,
@@ -253,7 +254,7 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap,
 		ret = rtc_nvmem_register(pcf2127->rtc, &nvmem_cfg);
 	}
 
-	return ret;
+	return rtc_register_device(pcf2127->rtc);
 }
 
 #ifdef CONFIG_OF
-- 
2.21.0

[PATCH 2/4] rtc: pcf2127: cleanup register and bit defines

[Bruno Thomsen]

Cleanup defines before adding new features to driver.

Signed-off-by: Bruno Thomsen <bruno.thomsen@gmail.com>
---
 drivers/rtc/rtc-pcf2127.c | 59 ++++++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 26 deletions(-)

diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index 58eb96506e4b..cd8def79b379 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -19,26 +19,32 @@
 #include <linux/of.h>
 #include <linux/regmap.h>
 
-#define PCF2127_REG_CTRL1       (0x00)  /* Control Register 1 */
-#define PCF2127_REG_CTRL2       (0x01)  /* Control Register 2 */
-
-#define PCF2127_REG_CTRL3       (0x02)  /* Control Register 3 */
-#define PCF2127_REG_CTRL3_BLF		BIT(2)
-
-#define PCF2127_REG_SC          (0x03)  /* datetime */
-#define PCF2127_REG_MN          (0x04)
-#define PCF2127_REG_HR          (0x05)
-#define PCF2127_REG_DM          (0x06)
-#define PCF2127_REG_DW          (0x07)
-#define PCF2127_REG_MO          (0x08)
-#define PCF2127_REG_YR          (0x09)
-
-/* the pcf2127 has 512 bytes nvmem, pcf2129 doesn't */
-#define PCF2127_REG_RAM_addr_MSB       0x1a
-#define PCF2127_REG_RAM_wrt_cmd        0x1c
-#define PCF2127_REG_RAM_rd_cmd         0x1d
+/* Control register 1 */
+#define PCF2127_REG_CTRL1		0x00
+/* Control register 2 */
+#define PCF2127_REG_CTRL2		0x01
+/* Control register 3 */
+#define PCF2127_REG_CTRL3		0x02
+#define PCF2127_BIT_CTRL3_BLF			BIT(2)
+/* Time and date registers */
+#define PCF2127_REG_SC			0x03
+#define PCF2127_BIT_SC_OSF			BIT(7)
+#define PCF2127_REG_MN			0x04
+#define PCF2127_REG_HR			0x05
+#define PCF2127_REG_DM			0x06
+#define PCF2127_REG_DW			0x07
+#define PCF2127_REG_MO			0x08
+#define PCF2127_REG_YR			0x09
+/*
+ * RAM registers
+ * PCF2127 has 512 bytes general-purpose static RAM (SRAM) that is
+ * battery backed and can survive a power outage.
+ * PCF2129 doesn't have this feature.
+ */
+#define PCF2127_REG_RAM_ADDR_MSB	0x1A
+#define PCF2127_REG_RAM_WRT_CMD		0x1C
+#define PCF2127_REG_RAM_RD_CMD		0x1D
 
-#define PCF2127_OSF             BIT(7)  /* Oscillator Fail flag */
 
 struct pcf2127 {
 	struct rtc_device *rtc;
@@ -73,11 +79,12 @@ static int pcf2127_rtc_read_time(struct device *dev, struct rtc_time *tm)
 		return ret;
 	}
 
-	if (buf[PCF2127_REG_CTRL3] & PCF2127_REG_CTRL3_BLF)
+	if (buf[PCF2127_REG_CTRL3] & PCF2127_BIT_CTRL3_BLF)
 		dev_info(dev,
 			"low voltage detected, check/replace RTC battery.\n");
 
-	if (buf[PCF2127_REG_SC] & PCF2127_OSF) {
+	/* Clock integrity is not guaranteed when OSF flag is set. */
+	if (buf[PCF2127_REG_SC] & PCF2127_BIT_SC_OSF) {
 		/*
 		 * no need clear the flag here,
 		 * it will be cleared once the new date is saved
@@ -166,7 +173,7 @@ static int pcf2127_rtc_ioctl(struct device *dev,
 		if (ret)
 			return ret;
 
-		touser = touser & PCF2127_REG_CTRL3_BLF ? 1 : 0;
+		touser = touser & PCF2127_BIT_CTRL3_BLF ? 1 : 0;
 
 		if (copy_to_user((void __user *)arg, &touser, sizeof(int)))
 			return -EFAULT;
@@ -192,12 +199,12 @@ static int pcf2127_nvmem_read(void *priv, unsigned int offset,
 	int ret;
 	unsigned char offsetbuf[] = { offset >> 8, offset };
 
-	ret = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_RAM_addr_MSB,
+	ret = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_RAM_ADDR_MSB,
 				offsetbuf, 2);
 	if (ret)
 		return ret;
 
-	ret = regmap_bulk_read(pcf2127->regmap, PCF2127_REG_RAM_rd_cmd,
+	ret = regmap_bulk_read(pcf2127->regmap, PCF2127_REG_RAM_RD_CMD,
 			       val, bytes);
 
 	return ret ?: bytes;
@@ -210,12 +217,12 @@ static int pcf2127_nvmem_write(void *priv, unsigned int offset,
 	int ret;
 	unsigned char offsetbuf[] = { offset >> 8, offset };
 
-	ret = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_RAM_addr_MSB,
+	ret = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_RAM_ADDR_MSB,
 				offsetbuf, 2);
 	if (ret)
 		return ret;
 
-	ret = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_RAM_wrt_cmd,
+	ret = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_RAM_WRT_CMD,
 				val, bytes);
 
 	return ret ?: bytes;
-- 
2.21.0

Re: [PATCH 2/4] rtc: pcf2127: cleanup register and bit defines

[Alexandre Belloni]

On 22/07/2019 17:58:09+0200, Bruno Thomsen wrote:
> Cleanup defines before adding new features to driver.
> 

I think you need to elaborate on what is wrong with the current defines
because they seem fine to me as-is.

> Signed-off-by: Bruno Thomsen <bruno.thomsen@gmail.com>
> ---
>  drivers/rtc/rtc-pcf2127.c | 59 ++++++++++++++++++++++-----------------
>  1 file changed, 33 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
> index 58eb96506e4b..cd8def79b379 100644
> --- a/drivers/rtc/rtc-pcf2127.c
> +++ b/drivers/rtc/rtc-pcf2127.c
> @@ -19,26 +19,32 @@
>  #include <linux/of.h>
>  #include <linux/regmap.h>
>  
> -#define PCF2127_REG_CTRL1       (0x00)  /* Control Register 1 */
> -#define PCF2127_REG_CTRL2       (0x01)  /* Control Register 2 */
> -
> -#define PCF2127_REG_CTRL3       (0x02)  /* Control Register 3 */
> -#define PCF2127_REG_CTRL3_BLF		BIT(2)
> -
> -#define PCF2127_REG_SC          (0x03)  /* datetime */
> -#define PCF2127_REG_MN          (0x04)
> -#define PCF2127_REG_HR          (0x05)
> -#define PCF2127_REG_DM          (0x06)
> -#define PCF2127_REG_DW          (0x07)
> -#define PCF2127_REG_MO          (0x08)
> -#define PCF2127_REG_YR          (0x09)
> -
> -/* the pcf2127 has 512 bytes nvmem, pcf2129 doesn't */
> -#define PCF2127_REG_RAM_addr_MSB       0x1a
> -#define PCF2127_REG_RAM_wrt_cmd        0x1c
> -#define PCF2127_REG_RAM_rd_cmd         0x1d
> +/* Control register 1 */
> +#define PCF2127_REG_CTRL1		0x00
> +/* Control register 2 */
> +#define PCF2127_REG_CTRL2		0x01
> +/* Control register 3 */
> +#define PCF2127_REG_CTRL3		0x02
> +#define PCF2127_BIT_CTRL3_BLF			BIT(2)
> +/* Time and date registers */
> +#define PCF2127_REG_SC			0x03
> +#define PCF2127_BIT_SC_OSF			BIT(7)
> +#define PCF2127_REG_MN			0x04
> +#define PCF2127_REG_HR			0x05
> +#define PCF2127_REG_DM			0x06
> +#define PCF2127_REG_DW			0x07
> +#define PCF2127_REG_MO			0x08
> +#define PCF2127_REG_YR			0x09
> +/*
> + * RAM registers
> + * PCF2127 has 512 bytes general-purpose static RAM (SRAM) that is
> + * battery backed and can survive a power outage.
> + * PCF2129 doesn't have this feature.
> + */
> +#define PCF2127_REG_RAM_ADDR_MSB	0x1A
> +#define PCF2127_REG_RAM_WRT_CMD		0x1C
> +#define PCF2127_REG_RAM_RD_CMD		0x1D
>  
> -#define PCF2127_OSF             BIT(7)  /* Oscillator Fail flag */
>  
>  struct pcf2127 {
>  	struct rtc_device *rtc;
> @@ -73,11 +79,12 @@ static int pcf2127_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  		return ret;
>  	}
>  
> -	if (buf[PCF2127_REG_CTRL3] & PCF2127_REG_CTRL3_BLF)
> +	if (buf[PCF2127_REG_CTRL3] & PCF2127_BIT_CTRL3_BLF)
>  		dev_info(dev,
>  			"low voltage detected, check/replace RTC battery.\n");
>  
> -	if (buf[PCF2127_REG_SC] & PCF2127_OSF) {
> +	/* Clock integrity is not guaranteed when OSF flag is set. */
> +	if (buf[PCF2127_REG_SC] & PCF2127_BIT_SC_OSF) {
>  		/*
>  		 * no need clear the flag here,
>  		 * it will be cleared once the new date is saved
> @@ -166,7 +173,7 @@ static int pcf2127_rtc_ioctl(struct device *dev,
>  		if (ret)
>  			return ret;
>  
> -		touser = touser & PCF2127_REG_CTRL3_BLF ? 1 : 0;
> +		touser = touser & PCF2127_BIT_CTRL3_BLF ? 1 : 0;
>  
>  		if (copy_to_user((void __user *)arg, &touser, sizeof(int)))
>  			return -EFAULT;
> @@ -192,12 +199,12 @@ static int pcf2127_nvmem_read(void *priv, unsigned int offset,
>  	int ret;
>  	unsigned char offsetbuf[] = { offset >> 8, offset };
>  
> -	ret = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_RAM_addr_MSB,
> +	ret = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_RAM_ADDR_MSB,
>  				offsetbuf, 2);
>  	if (ret)
>  		return ret;
>  
> -	ret = regmap_bulk_read(pcf2127->regmap, PCF2127_REG_RAM_rd_cmd,
> +	ret = regmap_bulk_read(pcf2127->regmap, PCF2127_REG_RAM_RD_CMD,
>  			       val, bytes);
>  
>  	return ret ?: bytes;
> @@ -210,12 +217,12 @@ static int pcf2127_nvmem_write(void *priv, unsigned int offset,
>  	int ret;
>  	unsigned char offsetbuf[] = { offset >> 8, offset };
>  
> -	ret = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_RAM_addr_MSB,
> +	ret = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_RAM_ADDR_MSB,
>  				offsetbuf, 2);
>  	if (ret)
>  		return ret;
>  
> -	ret = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_RAM_wrt_cmd,
> +	ret = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_RAM_WRT_CMD,
>  				val, bytes);
>  
>  	return ret ?: bytes;
> -- 
> 2.21.0
> 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH 3/4] rtc: pcf2127: add tamper detection support

[Bruno Thomsen]

Add support for integrated tamper detection function in both PCF2127 and
PCF2129 chips. This patch implements the feature by adding an additional
timestamp0 file to sysfs device path. This file contains seconds since
epoch, if an event occurred, or is empty, if none occurred.
Interface should match ISL1208 and RV3028 RTC drivers.

Signed-off-by: Bruno Thomsen <bruno.thomsen@gmail.com>
---
 drivers/rtc/rtc-pcf2127.c | 139 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 139 insertions(+)

diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index cd8def79b379..ff09bedc02d4 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -21,11 +21,18 @@
 
 /* Control register 1 */
 #define PCF2127_REG_CTRL1		0x00
+#define PCF2127_BIT_CTRL1_TSF1			BIT(4)
 /* Control register 2 */
 #define PCF2127_REG_CTRL2		0x01
+#define PCF2127_BIT_CTRL2_TSIE			BIT(2)
+#define PCF2127_BIT_CTRL2_TSF2			BIT(5)
 /* Control register 3 */
 #define PCF2127_REG_CTRL3		0x02
+#define PCF2127_BIT_CTRL3_BLIE			BIT(0)
+#define PCF2127_BIT_CTRL3_BIE			BIT(1)
 #define PCF2127_BIT_CTRL3_BLF			BIT(2)
+#define PCF2127_BIT_CTRL3_BF			BIT(3)
+#define PCF2127_BIT_CTRL3_BTSE			BIT(4)
 /* Time and date registers */
 #define PCF2127_REG_SC			0x03
 #define PCF2127_BIT_SC_OSF			BIT(7)
@@ -35,6 +42,16 @@
 #define PCF2127_REG_DW			0x07
 #define PCF2127_REG_MO			0x08
 #define PCF2127_REG_YR			0x09
+/* Tamper timestamp registers */
+#define PCF2127_REG_TS_CTRL		0x12
+#define PCF2127_BIT_TS_CTRL_TSOFF		BIT(6)
+#define PCF2127_BIT_TS_CTRL_TSM			BIT(7)
+#define PCF2127_REG_TS_SC		0x13
+#define PCF2127_REG_TS_MN		0x14
+#define PCF2127_REG_TS_HR		0x15
+#define PCF2127_REG_TS_DM		0x16
+#define PCF2127_REG_TS_MO		0x17
+#define PCF2127_REG_TS_YR		0x18
 /*
  * RAM registers
  * PCF2127 has 512 bytes general-purpose static RAM (SRAM) that is
@@ -228,6 +245,80 @@ static int pcf2127_nvmem_write(void *priv, unsigned int offset,
 	return ret ?: bytes;
 }
 
+/* sysfs interface */
+
+static ssize_t timestamp0_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	struct pcf2127 *pcf2127 = dev_get_drvdata(dev->parent);
+	int ret;
+
+	ret = regmap_update_bits(pcf2127->regmap, PCF2127_REG_CTRL1,
+				 PCF2127_BIT_CTRL1_TSF1, 0);
+	if (ret) {
+		dev_err(dev, "%s: update ctrl1 ret=%d\n", __func__, ret);
+		return ret;
+	}
+
+	ret = regmap_update_bits(pcf2127->regmap, PCF2127_REG_CTRL2,
+				 PCF2127_BIT_CTRL2_TSF2, 0);
+	if (ret) {
+		dev_err(dev, "%s: update ctrl2 ret=%d\n", __func__, ret);
+		return ret;
+	}
+
+	return count;
+};
+
+static ssize_t timestamp0_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	struct pcf2127 *pcf2127 = dev_get_drvdata(dev->parent);
+	struct rtc_time tm;
+	int ret;
+	u8 data[25];
+
+	ret = regmap_bulk_read(pcf2127->regmap, PCF2127_REG_CTRL1, data,
+			       sizeof(data));
+	if (ret) {
+		dev_err(dev, "%s: read error ret=%d\n", __func__, ret);
+		return ret;
+	}
+
+	if (!(data[PCF2127_REG_CTRL1] & PCF2127_BIT_CTRL1_TSF1) &&
+	    !(data[PCF2127_REG_CTRL2] & PCF2127_BIT_CTRL2_TSF2))
+		return 0;
+
+	tm.tm_sec = bcd2bin(data[PCF2127_REG_TS_SC] & 0x7F);
+	tm.tm_min = bcd2bin(data[PCF2127_REG_TS_MN] & 0x7F);
+	tm.tm_hour = bcd2bin(data[PCF2127_REG_TS_HR] & 0x3F);
+	tm.tm_mday = bcd2bin(data[PCF2127_REG_TS_DM] & 0x3F);
+	/* REG_TS_MO 1-12 */
+	tm.tm_mon = bcd2bin(data[PCF2127_REG_TS_MO] & 0x1F) - 1;
+	tm.tm_year = bcd2bin(data[PCF2127_REG_TS_YR]);
+	if (tm.tm_year < 70)
+		tm.tm_year += 100; /* assume we are in 1970...2069 */
+
+	ret = rtc_valid_tm(&tm);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%llu\n",
+		       (unsigned long long)rtc_tm_to_time64(&tm));
+};
+
+static DEVICE_ATTR_RW(timestamp0);
+
+static struct attribute *pcf2127_attrs[] = {
+	&dev_attr_timestamp0.attr,
+	NULL
+};
+
+static const struct attribute_group pcf2127_attr_group = {
+	.attrs	= pcf2127_attrs,
+};
+
 static int pcf2127_probe(struct device *dev, struct regmap *regmap,
 			const char *name, bool has_nvmem)
 {
@@ -261,6 +352,54 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap,
 		ret = rtc_nvmem_register(pcf2127->rtc, &nvmem_cfg);
 	}
 
+	/*
+	 * Disable battery low/switch-over timestamp and interrupts.
+	 * Clear battery interrupt flags which can block new trigger events.
+	 * Note: This is the default chip behaviour but added to ensure
+	 * correct tamper timestamp and interrupt function.
+	 */
+	ret = regmap_update_bits(pcf2127->regmap, PCF2127_REG_CTRL3,
+				 PCF2127_BIT_CTRL3_BTSE |
+				 PCF2127_BIT_CTRL3_BF |
+				 PCF2127_BIT_CTRL3_BIE |
+				 PCF2127_BIT_CTRL3_BLIE, 0);
+	if (ret) {
+		dev_err(dev, "%s: battery config (ctrl3) failed\n", __func__);
+		return ret;
+	}
+
+	/*
+	 * Enable timestamp function and store timestamp of first trigger
+	 * event until TSF1 and TFS2 interrupt flags are cleared.
+	 */
+	ret = regmap_update_bits(pcf2127->regmap, PCF2127_REG_TS_CTRL,
+				 PCF2127_BIT_TS_CTRL_TSOFF |
+				 PCF2127_BIT_TS_CTRL_TSM,
+				 PCF2127_BIT_TS_CTRL_TSM);
+	if (ret) {
+		dev_err(dev, "%s: tamper config (ts_ctrl) failed\n", __func__);
+		return ret;
+	}
+
+	/*
+	 * Enable interrupt generation when TSF1 or TSF2 timestamp flags
+	 * are set. Interrupt signal is an open-drain output and can be
+	 * left floating if unused.
+	 */
+	ret = regmap_update_bits(pcf2127->regmap, PCF2127_REG_CTRL2,
+				 PCF2127_BIT_CTRL2_TSIE,
+				 PCF2127_BIT_CTRL2_TSIE);
+	if (ret) {
+		dev_err(dev, "%s: tamper config (ctrl2) failed\n", __func__);
+		return ret;
+	}
+
+	ret = rtc_add_group(pcf2127->rtc, &pcf2127_attr_group);
+	if (ret) {
+		dev_err(dev, "%s: tamper register failed\n", __func__);
+		return ret;
+	}
+
 	return rtc_register_device(pcf2127->rtc);
 }
 
-- 
2.21.0

[PATCH 4/4] rtc: pcf2127: add watchdog feature support

[Bruno Thomsen]

Add partial support for the watchdog functionality of
PCF2127 and PCF2129 with Kconfig option.

The programmable watchdog timer is currently using a fixed
clock source of 1Hz. This result in a selectable range of
1-255 seconds, which covers most embedded Linux use-cases.

Clock sources of 4096Hz, 64Hz and 1/60Hz is mostly useful
in MCU use-cases.

Module parameter, wdt_margin, follows same interface as M41T80
and DS1374 RTC drivers.

Countdown timer not available when using watchdog feature.

Signed-off-by: Bruno Thomsen <bruno.thomsen@gmail.com>
---
 drivers/rtc/Kconfig       |  10 +++
 drivers/rtc/rtc-pcf2127.c | 155 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 165 insertions(+)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index e72f65b61176..45a123761784 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -881,6 +881,16 @@ config RTC_DRV_PCF2127
 	  This driver can also be built as a module. If so, the module
 	  will be called rtc-pcf2127.
 
+config RTC_DRV_PCF2127_WDT
+	bool "NXP PCF2127 watchdog timer"
+	depends on RTC_DRV_PCF2127
+	help
+	  If you say Y here you will get support for the watchdog timer
+	  in the NXP PCF2127 and PCF2129 real-time clock chips.
+
+	  The watchdog is usually used together with systemd or the
+	  watchdog daemon. Watchdog trigger cause system reset.
+
 config RTC_DRV_RV3029C2
 	tristate "Micro Crystal RV3029/3049"
 	depends on RTC_I2C_AND_SPI
diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index ff09bedc02d4..442aa0a06886 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -5,6 +5,9 @@
  *
  * Author: Renaud Cerrato <r.cerrato@til-technologies.fr>
  *
+ * Watchdog and tamper functions
+ * Author: Bruno Thomsen <bruno.thomsen@gmail.com>
+ *
  * based on the other drivers in this same directory.
  *
  * Datasheet: http://cache.nxp.com/documents/data_sheet/PCF2127.pdf
@@ -18,6 +21,10 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/regmap.h>
+#ifdef CONFIG_RTC_DRV_PCF2127_WDT
+#include <linux/miscdevice.h>
+#include <linux/watchdog.h>
+#endif
 
 /* Control register 1 */
 #define PCF2127_REG_CTRL1		0x00
@@ -42,6 +49,13 @@
 #define PCF2127_REG_DW			0x07
 #define PCF2127_REG_MO			0x08
 #define PCF2127_REG_YR			0x09
+/* Watchdog registers */
+#define PCF2127_REG_WD_CTL		0x10
+#define PCF2127_BIT_WD_CTL_TF0			BIT(0)
+#define PCF2127_BIT_WD_CTL_TF1			BIT(1)
+#define PCF2127_BIT_WD_CTL_CD0			BIT(6)
+#define PCF2127_BIT_WD_CTL_CD1			BIT(7)
+#define PCF2127_REG_WD_VAL		0x11
 /* Tamper timestamp registers */
 #define PCF2127_REG_TS_CTRL		0x12
 #define PCF2127_BIT_TS_CTRL_TSOFF		BIT(6)
@@ -62,6 +76,11 @@
 #define PCF2127_REG_RAM_WRT_CMD		0x1C
 #define PCF2127_REG_RAM_RD_CMD		0x1D
 
+/* Watchdog timer value constants */
+#define PCF2127_WD_VAL_STOP		0
+#define PCF2127_WD_VAL_MIN		1
+#define PCF2127_WD_VAL_MAX		255
+#define PCF2127_WD_VAL_DEFAULT		60
 
 struct pcf2127 {
 	struct rtc_device *rtc;
@@ -245,6 +264,108 @@ static int pcf2127_nvmem_write(void *priv, unsigned int offset,
 	return ret ?: bytes;
 }
 
+/* watchdog driver */
+
+#ifdef CONFIG_RTC_DRV_PCF2127_WDT
+
+static struct pcf2127 *saved_pcf2127;
+
+static int wdt_margin = PCF2127_WD_VAL_DEFAULT;
+module_param(wdt_margin, int, 0);
+MODULE_PARM_DESC(wdt_margin, "Watchdog timeout in seconds (default "
+		 __stringify(PCF2127_WD_VAL_DEFAULT) "s)");
+
+static void pcf2127_wdt_adjust_margin(int new_margin)
+{
+	if (new_margin < PCF2127_WD_VAL_MIN)
+		new_margin = PCF2127_WD_VAL_DEFAULT;
+	if (new_margin > PCF2127_WD_VAL_MAX)
+		new_margin = PCF2127_WD_VAL_MAX;
+
+	wdt_margin = new_margin;
+}
+
+static int pcf2127_wdt_ping(void)
+{
+	return regmap_write(saved_pcf2127->regmap, PCF2127_REG_WD_VAL,
+			    wdt_margin);
+}
+
+static int pcf2127_wdt_disable(void)
+{
+	return regmap_write(saved_pcf2127->regmap, PCF2127_REG_WD_VAL,
+			    PCF2127_WD_VAL_STOP);
+}
+
+static ssize_t pcf2127_wdt_write(struct file *file, const char __user *data,
+				 size_t len, loff_t *ppos)
+{
+	if (len) {
+		pcf2127_wdt_ping();
+		return 1;
+	}
+
+	return 0;
+}
+
+static const struct watchdog_info pcf2127_wdt_info = {
+	.identity = "NXP PCF2127/PCF2129 Watchdog",
+	.options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT,
+};
+
+static long pcf2127_wdt_ioctl(struct file *file, unsigned int cmd,
+			      unsigned long arg)
+{
+	int options, new_margin;
+
+	switch (cmd) {
+	case WDIOC_GETSUPPORT:
+		return copy_to_user((struct watchdog_info __user *)arg,
+				    &pcf2127_wdt_info,
+				    sizeof(pcf2127_wdt_info)) ? -EFAULT : 0;
+	case WDIOC_SETOPTIONS:
+		if (copy_from_user(&options, (int __user *)arg, sizeof(int)))
+			return -EFAULT;
+		if (options & WDIOS_DISABLECARD) {
+			pr_info("%s: disable watchdog\n", __func__);
+			return pcf2127_wdt_disable();
+		}
+		if (options & WDIOS_ENABLECARD) {
+			pr_info("%s: enable watchdog\n", __func__);
+			return pcf2127_wdt_ping();
+		}
+		return -EINVAL;
+	case WDIOC_KEEPALIVE:
+		return pcf2127_wdt_ping();
+	case WDIOC_SETTIMEOUT:
+		if (get_user(new_margin, (int __user *)arg))
+			return -EFAULT;
+		pcf2127_wdt_adjust_margin(new_margin);
+		pr_info("%s: new watchdog timeout: %is (requested: %is)\n",
+			__func__, wdt_margin, new_margin);
+		pcf2127_wdt_ping();
+		/* Fall through */
+	case WDIOC_GETTIMEOUT:
+		return put_user(wdt_margin, (int __user *)arg);
+	default:
+		return -ENOTTY;
+	}
+}
+
+static const struct file_operations pcf2127_wdt_fops = {
+	.owner = THIS_MODULE,
+	.write = pcf2127_wdt_write,
+	.unlocked_ioctl = pcf2127_wdt_ioctl,
+};
+
+static struct miscdevice pcf2127_miscdev = {
+	.minor = WATCHDOG_MINOR,
+	.name = "watchdog",
+	.fops = &pcf2127_wdt_fops,
+};
+
+#endif /* CONFIG_RTC_DRV_PCF2127_WDT */
+
 /* sysfs interface */
 
 static ssize_t timestamp0_store(struct device *dev,
@@ -394,6 +515,37 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap,
 		return ret;
 	}
 
+#ifdef CONFIG_RTC_DRV_PCF2127_WDT
+	/*
+	 * Watchdog timer enabled and reset pin /RST activated when timed out.
+	 * Select 1Hz clock source for watchdog timer.
+	 * Note: Countdown timer disabled and not available.
+	 */
+	ret = regmap_update_bits(pcf2127->regmap, PCF2127_REG_WD_CTL,
+				 PCF2127_BIT_WD_CTL_CD1 |
+				 PCF2127_BIT_WD_CTL_CD0 |
+				 PCF2127_BIT_WD_CTL_TF1 |
+				 PCF2127_BIT_WD_CTL_TF0,
+				 PCF2127_BIT_WD_CTL_CD1 |
+				 PCF2127_BIT_WD_CTL_CD0 |
+				 PCF2127_BIT_WD_CTL_TF1);
+	if (ret) {
+		dev_err(dev, "%s: watchdog config (wd_ctl) failed\n", __func__);
+		return ret;
+	}
+
+	/*
+	 * Fails if another watchdog driver is loaded.
+	 */
+	ret = misc_register(&pcf2127_miscdev);
+	if (ret) {
+		dev_err(dev, "%s: watchdog register failed\n", __func__);
+		return ret;
+	}
+
+	saved_pcf2127 = pcf2127;
+#endif /* CONFIG_RTC_DRV_PCF2127_WDT */
+
 	ret = rtc_add_group(pcf2127->rtc, &pcf2127_attr_group);
 	if (ret) {
 		dev_err(dev, "%s: tamper register failed\n", __func__);
@@ -639,6 +791,9 @@ module_init(pcf2127_init)
 
 static void __exit pcf2127_exit(void)
 {
+#ifdef CONFIG_RTC_DRV_PCF2127_WDT
+	misc_deregister(&pcf2127_miscdev);
+#endif /* CONFIG_RTC_DRV_PCF2127_WDT */
 	pcf2127_spi_unregister_driver();
 	pcf2127_i2c_unregister_driver();
 }
-- 
2.21.0

Re: [PATCH 4/4] rtc: pcf2127: add watchdog feature support

[Alexandre Belloni]

On 22/07/2019 17:58:11+0200, Bruno Thomsen wrote:
> Add partial support for the watchdog functionality of
> PCF2127 and PCF2129 with Kconfig option.
> 
> The programmable watchdog timer is currently using a fixed
> clock source of 1Hz. This result in a selectable range of
> 1-255 seconds, which covers most embedded Linux use-cases.
> 
> Clock sources of 4096Hz, 64Hz and 1/60Hz is mostly useful
> in MCU use-cases.
> 
> Module parameter, wdt_margin, follows same interface as M41T80
> and DS1374 RTC drivers.
> 
> Countdown timer not available when using watchdog feature.
> 
> Signed-off-by: Bruno Thomsen <bruno.thomsen@gmail.com>
> ---
>  drivers/rtc/Kconfig       |  10 +++
>  drivers/rtc/rtc-pcf2127.c | 155 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 165 insertions(+)
> 
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index e72f65b61176..45a123761784 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -881,6 +881,16 @@ config RTC_DRV_PCF2127
>  	  This driver can also be built as a module. If so, the module
>  	  will be called rtc-pcf2127.
>  
> +config RTC_DRV_PCF2127_WDT
> +	bool "NXP PCF2127 watchdog timer"
> +	depends on RTC_DRV_PCF2127
> +	help
> +	  If you say Y here you will get support for the watchdog timer
> +	  in the NXP PCF2127 and PCF2129 real-time clock chips.
> +
> +	  The watchdog is usually used together with systemd or the
> +	  watchdog daemon. Watchdog trigger cause system reset.
> +

I wouldn't add a new Kconfig entry for that. How much bigger will it be?

>  config RTC_DRV_RV3029C2
>  	tristate "Micro Crystal RV3029/3049"
>  	depends on RTC_I2C_AND_SPI
> diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
> index ff09bedc02d4..442aa0a06886 100644
> --- a/drivers/rtc/rtc-pcf2127.c
> +++ b/drivers/rtc/rtc-pcf2127.c
> @@ -5,6 +5,9 @@
>   *
>   * Author: Renaud Cerrato <r.cerrato@til-technologies.fr>
>   *
> + * Watchdog and tamper functions
> + * Author: Bruno Thomsen <bruno.thomsen@gmail.com>
> + *
>   * based on the other drivers in this same directory.
>   *
>   * Datasheet: http://cache.nxp.com/documents/data_sheet/PCF2127.pdf
> @@ -18,6 +21,10 @@
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/regmap.h>
> +#ifdef CONFIG_RTC_DRV_PCF2127_WDT
> +#include <linux/miscdevice.h>
> +#include <linux/watchdog.h>
> +#endif
>  
>  /* Control register 1 */
>  #define PCF2127_REG_CTRL1		0x00
> @@ -42,6 +49,13 @@
>  #define PCF2127_REG_DW			0x07
>  #define PCF2127_REG_MO			0x08
>  #define PCF2127_REG_YR			0x09
> +/* Watchdog registers */
> +#define PCF2127_REG_WD_CTL		0x10
> +#define PCF2127_BIT_WD_CTL_TF0			BIT(0)
> +#define PCF2127_BIT_WD_CTL_TF1			BIT(1)
> +#define PCF2127_BIT_WD_CTL_CD0			BIT(6)
> +#define PCF2127_BIT_WD_CTL_CD1			BIT(7)
> +#define PCF2127_REG_WD_VAL		0x11
>  /* Tamper timestamp registers */
>  #define PCF2127_REG_TS_CTRL		0x12
>  #define PCF2127_BIT_TS_CTRL_TSOFF		BIT(6)
> @@ -62,6 +76,11 @@
>  #define PCF2127_REG_RAM_WRT_CMD		0x1C
>  #define PCF2127_REG_RAM_RD_CMD		0x1D
>  
> +/* Watchdog timer value constants */
> +#define PCF2127_WD_VAL_STOP		0
> +#define PCF2127_WD_VAL_MIN		1
> +#define PCF2127_WD_VAL_MAX		255
> +#define PCF2127_WD_VAL_DEFAULT		60
>  
>  struct pcf2127 {
>  	struct rtc_device *rtc;
> @@ -245,6 +264,108 @@ static int pcf2127_nvmem_write(void *priv, unsigned int offset,
>  	return ret ?: bytes;
>  }
>  
> +/* watchdog driver */
> +
> +#ifdef CONFIG_RTC_DRV_PCF2127_WDT
> +
> +static struct pcf2127 *saved_pcf2127;
> +
> +static int wdt_margin = PCF2127_WD_VAL_DEFAULT;
> +module_param(wdt_margin, int, 0);
> +MODULE_PARM_DESC(wdt_margin, "Watchdog timeout in seconds (default "
> +		 __stringify(PCF2127_WD_VAL_DEFAULT) "s)");
> +
> +static void pcf2127_wdt_adjust_margin(int new_margin)
> +{
> +	if (new_margin < PCF2127_WD_VAL_MIN)
> +		new_margin = PCF2127_WD_VAL_DEFAULT;
> +	if (new_margin > PCF2127_WD_VAL_MAX)
> +		new_margin = PCF2127_WD_VAL_MAX;
> +
> +	wdt_margin = new_margin;
> +}
> +
> +static int pcf2127_wdt_ping(void)
> +{
> +	return regmap_write(saved_pcf2127->regmap, PCF2127_REG_WD_VAL,
> +			    wdt_margin);
> +}
> +
> +static int pcf2127_wdt_disable(void)
> +{
> +	return regmap_write(saved_pcf2127->regmap, PCF2127_REG_WD_VAL,
> +			    PCF2127_WD_VAL_STOP);
> +}
> +
> +static ssize_t pcf2127_wdt_write(struct file *file, const char __user *data,
> +				 size_t len, loff_t *ppos)
> +{
> +	if (len) {
> +		pcf2127_wdt_ping();
> +		return 1;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct watchdog_info pcf2127_wdt_info = {
> +	.identity = "NXP PCF2127/PCF2129 Watchdog",
> +	.options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT,
> +};
> +
> +static long pcf2127_wdt_ioctl(struct file *file, unsigned int cmd,
> +			      unsigned long arg)
> +{
> +	int options, new_margin;
> +
> +	switch (cmd) {
> +	case WDIOC_GETSUPPORT:
> +		return copy_to_user((struct watchdog_info __user *)arg,
> +				    &pcf2127_wdt_info,
> +				    sizeof(pcf2127_wdt_info)) ? -EFAULT : 0;
> +	case WDIOC_SETOPTIONS:
> +		if (copy_from_user(&options, (int __user *)arg, sizeof(int)))
> +			return -EFAULT;
> +		if (options & WDIOS_DISABLECARD) {
> +			pr_info("%s: disable watchdog\n", __func__);
> +			return pcf2127_wdt_disable();
> +		}
> +		if (options & WDIOS_ENABLECARD) {
> +			pr_info("%s: enable watchdog\n", __func__);
> +			return pcf2127_wdt_ping();
> +		}
> +		return -EINVAL;
> +	case WDIOC_KEEPALIVE:
> +		return pcf2127_wdt_ping();
> +	case WDIOC_SETTIMEOUT:
> +		if (get_user(new_margin, (int __user *)arg))
> +			return -EFAULT;
> +		pcf2127_wdt_adjust_margin(new_margin);
> +		pr_info("%s: new watchdog timeout: %is (requested: %is)\n",
> +			__func__, wdt_margin, new_margin);
> +		pcf2127_wdt_ping();
> +		/* Fall through */
> +	case WDIOC_GETTIMEOUT:
> +		return put_user(wdt_margin, (int __user *)arg);
> +	default:
> +		return -ENOTTY;
> +	}
> +}
> +
> +static const struct file_operations pcf2127_wdt_fops = {
> +	.owner = THIS_MODULE,
> +	.write = pcf2127_wdt_write,
> +	.unlocked_ioctl = pcf2127_wdt_ioctl,
> +};
> +
> +static struct miscdevice pcf2127_miscdev = {
> +	.minor = WATCHDOG_MINOR,
> +	.name = "watchdog",
> +	.fops = &pcf2127_wdt_fops,
> +};

Wow, that is old school, please use the watchdog subsysteù.

> +
> +#endif /* CONFIG_RTC_DRV_PCF2127_WDT */
> +
>  /* sysfs interface */
>  
>  static ssize_t timestamp0_store(struct device *dev,
> @@ -394,6 +515,37 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap,
>  		return ret;
>  	}
>  
> +#ifdef CONFIG_RTC_DRV_PCF2127_WDT
> +	/*
> +	 * Watchdog timer enabled and reset pin /RST activated when timed out.
> +	 * Select 1Hz clock source for watchdog timer.
> +	 * Note: Countdown timer disabled and not available.
> +	 */
> +	ret = regmap_update_bits(pcf2127->regmap, PCF2127_REG_WD_CTL,
> +				 PCF2127_BIT_WD_CTL_CD1 |
> +				 PCF2127_BIT_WD_CTL_CD0 |
> +				 PCF2127_BIT_WD_CTL_TF1 |
> +				 PCF2127_BIT_WD_CTL_TF0,
> +				 PCF2127_BIT_WD_CTL_CD1 |
> +				 PCF2127_BIT_WD_CTL_CD0 |
> +				 PCF2127_BIT_WD_CTL_TF1);
> +	if (ret) {
> +		dev_err(dev, "%s: watchdog config (wd_ctl) failed\n", __func__);
> +		return ret;
> +	}
> +
> +	/*
> +	 * Fails if another watchdog driver is loaded.
> +	 */
> +	ret = misc_register(&pcf2127_miscdev);
> +	if (ret) {
> +		dev_err(dev, "%s: watchdog register failed\n", __func__);
> +		return ret;
> +	}
> +
> +	saved_pcf2127 = pcf2127;
> +#endif /* CONFIG_RTC_DRV_PCF2127_WDT */
> +
>  	ret = rtc_add_group(pcf2127->rtc, &pcf2127_attr_group);
>  	if (ret) {
>  		dev_err(dev, "%s: tamper register failed\n", __func__);
> @@ -639,6 +791,9 @@ module_init(pcf2127_init)
>  
>  static void __exit pcf2127_exit(void)
>  {
> +#ifdef CONFIG_RTC_DRV_PCF2127_WDT
> +	misc_deregister(&pcf2127_miscdev);
> +#endif /* CONFIG_RTC_DRV_PCF2127_WDT */
>  	pcf2127_spi_unregister_driver();
>  	pcf2127_i2c_unregister_driver();
>  }
> -- 
> 2.21.0
> 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 4/4] rtc: pcf2127: add watchdog feature support

[Bruno Thomsen]

Thanks for review.

Den tir. 23. jul. 2019 kl. 20.48 skrev Alexandre Belloni
<alexandre.belloni@bootlin.com>:
>
> > +config RTC_DRV_PCF2127_WDT
> > +     bool "NXP PCF2127 watchdog timer"
> > +     depends on RTC_DRV_PCF2127
> > +     help
> > +       If you say Y here you will get support for the watchdog timer
> > +       in the NXP PCF2127 and PCF2129 real-time clock chips.
> > +
> > +       The watchdog is usually used together with systemd or the
> > +       watchdog daemon. Watchdog trigger cause system reset.
> > +
>
> I wouldn't add a new Kconfig entry for that. How much bigger will it be?

Delta size on rtc-pcf2127.ko is 3244 bytes when compiled for armv7a.

I only added Kconfig option to allow driver load on arm platforms where
you need to use internal watchdog to restart board. But I will remove it
for next version, should I extend help text on exiting Kconfig option to
also include information about other chip features? As this is not a
simple RTC chip.

> > +static const struct file_operations pcf2127_wdt_fops = {
> > +     .owner = THIS_MODULE,
> > +     .write = pcf2127_wdt_write,
> > +     .unlocked_ioctl = pcf2127_wdt_ioctl,
> > +};
> > +
> > +static struct miscdevice pcf2127_miscdev = {
> > +     .minor = WATCHDOG_MINOR,
> > +     .name = "watchdog",
> > +     .fops = &pcf2127_wdt_fops,
> > +};
>
> Wow, that is old school, please use the watchdog subsysteů.

Okay, I was not aware that this was an old API. So I should convert
to struct watchdog_ops and devm_watchdog_register_device?

/Bruno

Re: [PATCH 4/4] rtc: pcf2127: add watchdog feature support

[Alexandre Belloni]

On 24/07/2019 09:18:05+0200, Bruno Thomsen wrote:
> Den tir. 23. jul. 2019 kl. 20.48 skrev Alexandre Belloni
> <alexandre.belloni@bootlin.com>:
> >
> > > +config RTC_DRV_PCF2127_WDT
> > > +     bool "NXP PCF2127 watchdog timer"
> > > +     depends on RTC_DRV_PCF2127
> > > +     help
> > > +       If you say Y here you will get support for the watchdog timer
> > > +       in the NXP PCF2127 and PCF2129 real-time clock chips.
> > > +
> > > +       The watchdog is usually used together with systemd or the
> > > +       watchdog daemon. Watchdog trigger cause system reset.
> > > +
> >
> > I wouldn't add a new Kconfig entry for that. How much bigger will it be?
> 
> Delta size on rtc-pcf2127.ko is 3244 bytes when compiled for armv7a.
> 
> I only added Kconfig option to allow driver load on arm platforms where
> you need to use internal watchdog to restart board. But I will remove it
> for next version, should I extend help text on exiting Kconfig option to
> also include information about other chip features? As this is not a
> simple RTC chip.

You can extend the Kconfig help if you want.

> 
> > > +static const struct file_operations pcf2127_wdt_fops = {
> > > +     .owner = THIS_MODULE,
> > > +     .write = pcf2127_wdt_write,
> > > +     .unlocked_ioctl = pcf2127_wdt_ioctl,
> > > +};
> > > +
> > > +static struct miscdevice pcf2127_miscdev = {
> > > +     .minor = WATCHDOG_MINOR,
> > > +     .name = "watchdog",
> > > +     .fops = &pcf2127_wdt_fops,
> > > +};
> >
> > Wow, that is old school, please use the watchdog subsysteů.
> 
> Okay, I was not aware that this was an old API. So I should convert
> to struct watchdog_ops and devm_watchdog_register_device?
> 

Yes, that is what I was asking for.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 0/4] rtc: pcf2127: tamper timestamp and watchdog feature support

[Bruno Thomsen]

Hi all

> Patches has been tested on a pcf2127 chip using 2MHz SPI
> interface both as built-in and module including with and
> without watchdog feature.

I did some more testing as I could not understand this bugfix:

3769a375ab83 rtc: pcf2127: bulk read only date and time registers.

This fix seems to be incomplete as root cause is not auto-increment
read aka bulk read, but reading control register 2 triggers zero value
in WD_VAL resulting in stopped watchdog until systemd kick the dog
again :)

As the watchdog has 2 control registers I will do some more testing
to see if this also apply to the other register. But more importantly
this issue also affect timestamp0_{store,show} functions in tamper
detection support, and I will therefor send a v2 of the patch series.

/Bruno

Re: [PATCH 0/4] rtc: pcf2127: tamper timestamp and watchdog feature support

[Alexandre Belloni]

On 23/07/2019 16:13:12+0200, Bruno Thomsen wrote:
> Hi all
> 
> > Patches has been tested on a pcf2127 chip using 2MHz SPI
> > interface both as built-in and module including with and
> > without watchdog feature.
> 
> I did some more testing as I could not understand this bugfix:
> 
> 3769a375ab83 rtc: pcf2127: bulk read only date and time registers.
> 
> This fix seems to be incomplete as root cause is not auto-increment
> read aka bulk read, but reading control register 2 triggers zero value
> in WD_VAL resulting in stopped watchdog until systemd kick the dog
> again :)
> 
> As the watchdog has 2 control registers I will do some more testing
> to see if this also apply to the other register. But more importantly
> this issue also affect timestamp0_{store,show} functions in tamper
> detection support, and I will therefor send a v2 of the patch series.
> 

Please Cc the watchdog maintainer on v2.

> /Bruno

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com