On 14.01.21 10:50, Alexandre Belloni wrote:
On 13/01/2021 12:27:42+0100, Philipp Rosenberger wrote:
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@xxxxxxxxxx>
---
drivers/rtc/rtc-pcf2127.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index 378b1ce812d6..ca56dba64e79 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)
@@ -630,6 +633,19 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap,
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__);
Please drop this error message.
If I return from the probe with an error, shouldn't there be an error
message? Or should I ignore the problem at all and don't return from the
probe?
+ return ret;
+ }
+ msleep(100);
Maybe this should be done just before setting the time. Or if you want
to keep it in probe, then you could optimise by not waiting but ensuring
the time between pcf2127_probe and the first pcf2127_rtc_set_time is
more than 100ms.
Doing it just before setting the time might be not the best way. The
watchdog might be used before the OTPR is done.
From the PCF2129 manual:
| The OTP refresh (see Section 8.3.2 on page 13) should ideally be
| executed as the first instruction after start-up and also after a
| reset due to an oscillator stop.
As I see it this should be done before setting up the watchdog as well.
So sleeping if the OTPR wasn't done before might be the most viable
solution.
So I would check the OTPR and only if the OTPR is not set starting an
OTPR and then sleep 100ms.
Best Regards
Philipp
