[PATCH v3 0/2] rtc: pcf2127: proper initialization after power loss

[PATCH v3 0/2] rtc: pcf2127: proper initialization after power loss

[Philipp Rosenberger]

If the PCF2127/2129 loses power it needs some initialization to work
correctly. A bootloader/firmware might do this. If not we should do this
in the driver.

Changes for v3:
- drop the test if clearing PORO was successful
- only run OTP refresh if OTPR bit is not already set

Changes for v2:
- make commit log and comments more clear
- check if PORO was really disabled

Philipp Rosenberger (2):
  rtc: pcf2127: Disable Power-On Reset Override
  rtc: pcf2127: Run a OTP refresh if not done before

 drivers/rtc/rtc-pcf2127.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

Interdiff against v2:
diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index ca56dba64e79..b48fa27cf093 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -568,6 +568,7 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap,
 {
 	struct pcf2127 *pcf2127;
 	int ret = 0;
+	unsigned int val;
 
 	dev_dbg(dev, "%s\n", __func__);
 
@@ -622,29 +623,19 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap,
 	 */
 	regmap_clear_bits(pcf2127->regmap, PCF2127_REG_CTRL1,
 				PCF2127_BIT_CTRL1_POR_OVRD);
-	/*
-	 * If the PORO can't be disabled, just move on. The RTC should
-	 * work fine, but functions like watchdog and alarm interrupts might
-	 * not work. There will be no interrupt generated on the interrupt pin.
-	 */
-	ret = regmap_test_bits(pcf2127->regmap, PCF2127_REG_CTRL1, PCF2127_BIT_CTRL1_POR_OVRD);
-	if (ret <= 0) {
-		dev_err(dev, "%s: can't disable PORO (ctrl1).\n", __func__);
-		dev_warn(dev, "Watchdog and alarm functions might not work properly\n");
-	}
 
-	/*
-	 * Set the OTP refresh bit unconditionally. If an OTP refresh was
-	 * already done the bit is already set and will not rerun the refresh
-	 * operation.
-	 */
-	ret = regmap_set_bits(pcf2127->regmap, PCF2127_REG_CLKOUT,
-			      PCF2127_BIT_CLKOUT_OTPR);
-	if (ret < 0) {
-		dev_err(dev, "%s: OTP refresh (clkout_ctrl) failed.\n", __func__);
+	ret = regmap_read(pcf2127->regmap, PCF2127_REG_CLKOUT, &val);
+	if (ret < 0)
 		return ret;
+
+	if (!(val & PCF2127_BIT_CLKOUT_OTPR)) {
+		ret = regmap_set_bits(pcf2127->regmap, PCF2127_REG_CLKOUT,
+				      PCF2127_BIT_CLKOUT_OTPR);
+		if (ret < 0)
+			return ret;
+
+		msleep(100);
 	}
-	msleep(100);
 
 	/*
 	 * Watchdog timer enabled and reset pin /RST activated when timed out.
-- 
2.29.2

[PATCH v3 1/2] rtc: pcf2127: Disable Power-On Reset Override

[Philipp Rosenberger]

To resume normal operation after a total power loss (no or empty
battery) the "Power-On Reset Override (PORO)" facility needs to be
disabled.

The register reset value sets the PORO enabled and the data sheet
recommends setting it to disabled for normal operation.

From what I've seen on the PCF2127 and PCF2129 there is not
interrupted gernerated at the interrupt pin (INT), as long the PORO bit
is set. This behavior is not documented in the manual.

Signed-off-by: Philipp Rosenberger <p.rosenberger@kunbus.com>
---
 drivers/rtc/rtc-pcf2127.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index 39a7b5116aa4..0e06907d3ddc 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -26,6 +26,7 @@
 
 /* Control register 1 */
 #define PCF2127_REG_CTRL1		0x00
+#define PCF2127_BIT_CTRL1_POR_OVRD		BIT(3)
 #define PCF2127_BIT_CTRL1_TSF1			BIT(4)
 /* Control register 2 */
 #define PCF2127_REG_CTRL2		0x01
@@ -612,6 +613,13 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap,
 		ret = devm_rtc_nvmem_register(pcf2127->rtc, &nvmem_cfg);
 	}
 
+	/*
+	 * The "Power-On Reset Override" facility prevents the RTC to do a reset
+	 * after power on. For normal operation the PORO must be disabled.
+	 */
+	regmap_clear_bits(pcf2127->regmap, PCF2127_REG_CTRL1,
+				PCF2127_BIT_CTRL1_POR_OVRD);
+
 	/*
 	 * Watchdog timer enabled and reset pin /RST activated when timed out.
 	 * Select 1Hz clock source for watchdog timer.
-- 
2.29.2

Re: [PATCH v3 1/2] rtc: pcf2127: Disable Power-On Reset Override

[Uwe Kleine-König]

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

Hello Philipp,

On Mon, Jan 18, 2021 at 09:57:51AM +0100, Philipp Rosenberger wrote:
> From what I've seen on the PCF2127 and PCF2129 there is not
> interrupted gernerated at the interrupt pin (INT), as long the PORO bit

Apart from s/not interrupted gernerated/no event generated/ this looks
good now.

> is set. This behavior is not documented in the manual.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v3 2/2] rtc: pcf2127: Run a OTP refresh if not done before

[Philipp Rosenberger]

The datasheet of the PCF2127 states, it is recommended to process an OTP
refresh once the power is up and the oscillator is operating stable. The
OTP refresh takes less than 100 ms to complete.

Signed-off-by: Philipp Rosenberger <p.rosenberger@kunbus.com>
---
 drivers/rtc/rtc-pcf2127.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index 0e06907d3ddc..b48fa27cf093 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -58,6 +58,9 @@
 #define PCF2127_REG_ALARM_DM		0x0D
 #define PCF2127_REG_ALARM_DW		0x0E
 #define PCF2127_BIT_ALARM_AE			BIT(7)
+/* CLKOUT control register */
+#define PCF2127_REG_CLKOUT		0x0f
+#define PCF2127_BIT_CLKOUT_OTPR			BIT(5)
 /* Watchdog registers */
 #define PCF2127_REG_WD_CTL		0x10
 #define PCF2127_BIT_WD_CTL_TF0			BIT(0)
@@ -565,6 +568,7 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap,
 {
 	struct pcf2127 *pcf2127;
 	int ret = 0;
+	unsigned int val;
 
 	dev_dbg(dev, "%s\n", __func__);
 
@@ -620,6 +624,19 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap,
 	regmap_clear_bits(pcf2127->regmap, PCF2127_REG_CTRL1,
 				PCF2127_BIT_CTRL1_POR_OVRD);
 
+	ret = regmap_read(pcf2127->regmap, PCF2127_REG_CLKOUT, &val);
+	if (ret < 0)
+		return ret;
+
+	if (!(val & PCF2127_BIT_CLKOUT_OTPR)) {
+		ret = regmap_set_bits(pcf2127->regmap, PCF2127_REG_CLKOUT,
+				      PCF2127_BIT_CLKOUT_OTPR);
+		if (ret < 0)
+			return ret;
+
+		msleep(100);
+	}
+
 	/*
 	 * Watchdog timer enabled and reset pin /RST activated when timed out.
 	 * Select 1Hz clock source for watchdog timer.
-- 
2.29.2

Re: [PATCH v3 0/2] rtc: pcf2127: proper initialization after power loss

[Alexandre Belloni]

On Mon, 18 Jan 2021 09:57:50 +0100, Philipp Rosenberger wrote:
> If the PCF2127/2129 loses power it needs some initialization to work
> correctly. A bootloader/firmware might do this. If not we should do this
> in the driver.
> 
> Changes for v3:
> - drop the test if clearing PORO was successful
> - only run OTP refresh if OTPR bit is not already set
> 
> [...]

Applied, thanks!

[1/2] rtc: pcf2127: Disable Power-On Reset Override
      commit: b9ac079abefc1f1cbee8a0f7195bad1d32dc72c7
[2/2] rtc: pcf2127: Run a OTP refresh if not done before
      commit: 15f57b3e3130790b6d06ea04f0c1edf0e5455bdd

Best regards,
-- 
Alexandre Belloni <alexandre.belloni@bootlin.com>