2023-06-22 15:02:04

by Hugo Villeneuve

[permalink] [raw]
Subject: [PATCH v4 00/17] rtc: pcf2127: add PCF2131 driver

From: Hugo Villeneuve <[email protected]>

Hello,
this patch series adds the driver for the PCF2131 real-time clock.

This RTC is very similar in functionality to the PCF2127/29 with the
following differences:
-supports two new control registers at offsets 4 and 5
-supports a new reset register
-supports 4 tamper detection functions instead of 1
-has no nvmem (like the PCF2129)
-has two output interrupt pins instead of one
-has 1/100th seconds capabilities (not supported in this driver)
-pcf2127 has watchdog clock sources: 1/60, 1, 64 and 4096Hz
pcf2131 has watchdog clock sources: 1/64, 1/4, 4 and 64Hz
-watchdog value register cannot be read after being set

Most of the register addresses are very different, although they still
follow the same layout. For example, the time/date and tamper registers
have a different base address, but the offsets are all the same.
Consequently, the source code of the PCF2127 driver can be easily adapted
to support this new device.

Patch 1 improves rtc_read_time() performance

Patch 2 improve timestamp reading performance

Patch 3 lowers error message severity

Patch 4 remove superfluous comments.

Patches 5 to 10 modify the existing pcf2127 driver to make it more generic
and able to support multiple variants, like the PCF2131. This is done
mostly by using offsets instead of absolute hardcoded register addresses.

Patch 11 add actual support for the PCF2131.

Patch 12 configures all interrupt sources to go through the INT A pin.

Patch 13 adapt the time/date registers write sequence for PCF2131 (STOP and
CPR bits).

Patch 14 add support for generic watchdog timing configuration.

Patch 15 add a new flag to identify if device has read support for reading
watchdog register value.
Since the watchdog value register cannot be read on the PCF2131 after
being set, it seems that we cannot detect if watchdog timer was
started by bootloader. I am not sure what is the best way to handle
this situation, suggestions are welcomed.

Patch 16 add UIE support for PCF2131

Patch 17 add the dt-bindings for the PCF2131.

I have tested the driver using a PCF2131-ARD evaluation board connected to
a Variscite imx8mn NANO SOM evaluation board:
- Time get/set ok;
- Alarms get/set ok
- Timestamp 1 to 4 ok
- IRQ alarm ok
- Watchdog ok
- Also tested successfully with "RTC Driver Test Example" from
Documentation/rtc.txt

I have also tested the driver on a custom PCF2129 adapter board connected to
a Variscite imx8mn NANO SOM evaluation board.

Thank you.

Link: [v1] https://patchwork.ozlabs.org/project/rtc-linux/patch/[email protected]/
Link: [v2] https://patchwork.ozlabs.org/project/rtc-linux/list/?series=285734
Link: [v3] https://patchwork.ozlabs.org/project/rtc-linux/list/?series=332888

Changes for V2:
- In general, fix and improvements after I have tested on real hardware
- Fix alarm interrupt A/B mask setting for PCF2131:
PCF2131_BIT_INT_AIE must be cleared, not set, to enable interrupt.
- Remove low_reg validation: only check if TS interrupt flag is
defined, as low_reg is defined at address 0 for PCF2127/29.
- Change PWRMNG value for PCF2131: default is different than PCF2127/29.
- Adapt time/date registers write sequence for PCF2131 (STOP and CPR bits).
- Map all interrupt sources to INT A pin
- Read and validate PCF2131 device presence from RESET register
- Adapt watchdog configuration for PCF2131

Changes for V3:
- Rebase for kernel v6.1

Changes for V4:
- Rebase for kernel v6.4-rc7
- Add base-commit infos (git format-patch with --base= option)
- Rename regs_td_base -> reg_time_base
- Rename PCF2127_REG_TIME_DATE_BASE -> PCF2127_REG_TIME_BASE
- Remove/update comments
- Remove PCF2127_OFFSET_ALARM_* defines
- Remove PCF2127_OFFSET_TD_* defines
- Rename regs_base -> reg_base in struct pcf21xx_ts_config
- Remove identity string changes to not break userspace
- Explicitely set .inter_detect_bit to 0
- Replace hardcoded bit values with BIT() macros
- Remove reset register signature read
- Remove patch to set PWRMNG bits
- Rework to keep watchdog value store/computed inside the structure wdd
(struct watchdog_device) to be in seconds.
- Rename confusing structure members
- Create separate patch for optimization of timestamp and time reading.
- Add UIE support for PCF2131

Hugo Villeneuve (17):
rtc: pcf2127: improve rtc_read_time() performance
rtc: pcf2127: improve timestamp reading performance
rtc: pcf2127: lower message severity if setting time fails
rtc: pcf2127: remove superfluous comments
rtc: pcf2127: add variant-specific configuration structure
rtc: pcf2127: adapt for time/date registers at any offset
rtc: pcf2127: adapt for alarm registers at any offset
rtc: pcf2127: adapt for WD registers at any offset
rtc: pcf2127: adapt for CLKOUT register at any offset
rtc: pcf2127: add support for multiple TS functions
rtc: pcf2127: add support for PCF2131 RTC
rtc: pcf2127: add support for PCF2131 interrupts on output INT_A
rtc: pcf2127: adapt time/date registers write sequence for PCF2131
rtc: pcf2127: support generic watchdog timing configuration
rtc: pcf2127: add flag for watchdog register value read support
rtc: pcf2127: add UIE support for PCF2131
dt-bindings: rtc: pcf2127: add PCF2131

.../devicetree/bindings/rtc/nxp,pcf2127.yaml | 1 +
drivers/rtc/Kconfig | 4 +-
drivers/rtc/rtc-pcf2127.c | 874 ++++++++++++++----
3 files changed, 691 insertions(+), 188 deletions(-)


base-commit: dad9774deaf1cf8e8f7483310dfb2690310193d2
--
2.30.2



2023-06-22 15:02:56

by Hugo Villeneuve

[permalink] [raw]
Subject: [PATCH v4 09/17] rtc: pcf2127: adapt for CLKOUT register at any offset

From: Hugo Villeneuve <[email protected]>

This will simplify the implementation of new variants into this driver.

Signed-off-by: Hugo Villeneuve <[email protected]>
Reviewed-by: Bruno Thomsen <[email protected]>
---
drivers/rtc/rtc-pcf2127.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index ce7fe6e2fd47..21fa42870dd4 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -101,6 +101,7 @@ struct pcf21xx_config {
u8 regs_alarm_base; /* Alarm function base registers. */
u8 reg_wd_ctl; /* Watchdog control register. */
u8 reg_wd_val; /* Watchdog value register. */
+ u8 reg_clkout; /* Clkout register. */
};

struct pcf2127 {
@@ -631,6 +632,7 @@ static struct pcf21xx_config pcf21xx_cfg[] = {
.regs_alarm_base = PCF2127_REG_ALARM_BASE,
.reg_wd_ctl = PCF2127_REG_WD_CTL,
.reg_wd_val = PCF2127_REG_WD_VAL,
+ .reg_clkout = PCF2127_REG_CLKOUT,
},
[PCF2129] = {
.type = PCF2129,
@@ -641,6 +643,7 @@ static struct pcf21xx_config pcf21xx_cfg[] = {
.regs_alarm_base = PCF2127_REG_ALARM_BASE,
.reg_wd_ctl = PCF2127_REG_WD_CTL,
.reg_wd_val = PCF2127_REG_WD_VAL,
+ .reg_clkout = PCF2127_REG_CLKOUT,
},
};

@@ -720,12 +723,12 @@ 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);
+ ret = regmap_read(pcf2127->regmap, pcf2127->cfg->reg_clkout, &val);
if (ret < 0)
return ret;

if (!(val & PCF2127_BIT_CLKOUT_OTPR)) {
- ret = regmap_set_bits(pcf2127->regmap, PCF2127_REG_CLKOUT,
+ ret = regmap_set_bits(pcf2127->regmap, pcf2127->cfg->reg_clkout,
PCF2127_BIT_CLKOUT_OTPR);
if (ret < 0)
return ret;
--
2.30.2


2023-06-22 15:03:31

by Hugo Villeneuve

[permalink] [raw]
Subject: [PATCH v4 11/17] rtc: pcf2127: add support for PCF2131 RTC

From: Hugo Villeneuve <[email protected]>

This RTC is very similar in functionality to the PCF2127/29.

Basically it:
-supports two new control registers at offsets 4 and 5
-supports a new reset register (not implemented in this driver)
-supports 4 tamper detection functions instead of 1
-has no nvmem (like the PCF2129)
-has two output interrupt pins

Because of that, most of the register addresses are very different,
although they still follow the same layout. For example, the tamper
registers have a different base address, but the offsets are all the same.

Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/rtc/Kconfig | 4 +-
drivers/rtc/rtc-pcf2127.c | 234 ++++++++++++++++++++++++++++++++++----
2 files changed, 215 insertions(+), 23 deletions(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 753872408615..ff5992c87022 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -904,9 +904,9 @@ config RTC_DRV_PCF2127
select REGMAP_SPI if SPI_MASTER
select WATCHDOG_CORE if WATCHDOG
help
- If you say yes here you get support for the NXP PCF2127/29 RTC
+ If you say yes here you get support for the NXP PCF2127/29/31 RTC
chips with integrated quartz crystal for industrial applications.
- Both chips also have watchdog timer and tamper switch detection
+ These chips also have watchdog timer and tamper switch detection
features.

PCF2127 has an additional feature of 512 bytes battery backed
diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index c6bbeb1916a4..09607b67c282 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * An I2C and SPI driver for the NXP PCF2127/29 RTC
+ * An I2C and SPI driver for the NXP PCF2127/29/31 RTC
* Copyright 2013 Til-Technologies
*
* Author: Renaud Cerrato <[email protected]>
@@ -8,9 +8,13 @@
* Watchdog and tamper functions
* Author: Bruno Thomsen <[email protected]>
*
+ * PCF2131 support
+ * Author: Hugo Villeneuve <[email protected]>
+ *
* based on the other drivers in this same directory.
*
- * Datasheet: https://www.nxp.com/docs/en/data-sheet/PCF2127.pdf
+ * Datasheets: https://www.nxp.com/docs/en/data-sheet/PCF2127.pdf
+ * https://www.nxp.com/docs/en/data-sheet/PCF2131DS.pdf
*/

#include <linux/i2c.h>
@@ -67,7 +71,7 @@
* 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.
+ * PCF2129/31 doesn't have this feature.
*/
#define PCF2127_REG_RAM_ADDR_MSB 0x1A
#define PCF2127_REG_RAM_WRT_CMD 0x1C
@@ -86,11 +90,65 @@
PCF2127_BIT_CTRL2_WDTF | \
PCF2127_BIT_CTRL2_TSF2)

-#define PCF2127_MAX_TS_SUPPORTED 1
+#define PCF2127_MAX_TS_SUPPORTED 4
+
+/* Control register 4 */
+#define PCF2131_REG_CTRL4 0x03
+#define PCF2131_BIT_CTRL4_TSF4 BIT(4)
+#define PCF2131_BIT_CTRL4_TSF3 BIT(5)
+#define PCF2131_BIT_CTRL4_TSF2 BIT(6)
+#define PCF2131_BIT_CTRL4_TSF1 BIT(7)
+/* Control register 5 */
+#define PCF2131_REG_CTRL5 0x04
+#define PCF2131_BIT_CTRL5_TSIE4 BIT(4)
+#define PCF2131_BIT_CTRL5_TSIE3 BIT(5)
+#define PCF2131_BIT_CTRL5_TSIE2 BIT(6)
+#define PCF2131_BIT_CTRL5_TSIE1 BIT(7)
+/* Software reset register */
+#define PCF2131_REG_SR_RESET 0x05
+#define PCF2131_SR_RESET_READ_PATTERN (BIT(2) | BIT(5))
+#define PCF2131_SR_RESET_CPR_CMD (PCF2131_SR_RESET_READ_PATTERN | BIT(7))
+/* Time and date registers */
+#define PCF2131_REG_TIME_BASE 0x07
+/* Alarm registers */
+#define PCF2131_REG_ALARM_BASE 0x0E
+/* CLKOUT control register */
+#define PCF2131_REG_CLKOUT 0x13
+/* Watchdog registers */
+#define PCF2131_REG_WD_CTL 0x35
+#define PCF2131_REG_WD_VAL 0x36
+/* Tamper timestamp1 registers */
+#define PCF2131_REG_TS1_BASE 0x14
+/* Tamper timestamp2 registers */
+#define PCF2131_REG_TS2_BASE 0x1B
+/* Tamper timestamp3 registers */
+#define PCF2131_REG_TS3_BASE 0x22
+/* Tamper timestamp4 registers */
+#define PCF2131_REG_TS4_BASE 0x29
+/* Interrupt mask registers */
+#define PCF2131_REG_INT_A_MASK1 0x31
+#define PCF2131_REG_INT_A_MASK2 0x32
+#define PCF2131_REG_INT_B_MASK1 0x33
+#define PCF2131_REG_INT_B_MASK2 0x34
+#define PCF2131_BIT_INT_BLIE BIT(0)
+#define PCF2131_BIT_INT_BIE BIT(1)
+#define PCF2131_BIT_INT_AIE BIT(2)
+#define PCF2131_BIT_INT_WD_CD BIT(3)
+#define PCF2131_BIT_INT_SI BIT(4)
+#define PCF2131_BIT_INT_MI BIT(5)
+#define PCF2131_CTRL2_IRQ_MASK ( \
+ PCF2127_BIT_CTRL2_AF | \
+ PCF2127_BIT_CTRL2_WDTF)
+#define PCF2131_CTRL4_IRQ_MASK ( \
+ PCF2131_BIT_CTRL4_TSF4 | \
+ PCF2131_BIT_CTRL4_TSF3 | \
+ PCF2131_BIT_CTRL4_TSF2 | \
+ PCF2131_BIT_CTRL4_TSF1)

enum pcf21xx_type {
PCF2127,
PCF2129,
+ PCF2131,
PCF21XX_LAST_ID
};

@@ -530,30 +588,64 @@ static void pcf2127_rtc_ts_snapshot(struct device *dev, int ts_id)
static irqreturn_t pcf2127_rtc_irq(int irq, void *dev)
{
struct pcf2127 *pcf2127 = dev_get_drvdata(dev);
- unsigned int ctrl1, ctrl2;
+ unsigned int ctrl2;
int ret = 0;

- ret = regmap_read(pcf2127->regmap, PCF2127_REG_CTRL1, &ctrl1);
- if (ret)
- return IRQ_NONE;
-
ret = regmap_read(pcf2127->regmap, PCF2127_REG_CTRL2, &ctrl2);
if (ret)
return IRQ_NONE;

- if (!(ctrl1 & PCF2127_CTRL1_IRQ_MASK || ctrl2 & PCF2127_CTRL2_IRQ_MASK))
- return IRQ_NONE;
+ if (pcf2127->cfg->ts_count == 1) {
+ /* PCF2127/29 */
+ unsigned int ctrl1;
+
+ ret = regmap_read(pcf2127->regmap, PCF2127_REG_CTRL1, &ctrl1);
+ if (ret)
+ return IRQ_NONE;
+
+ if (!(ctrl1 & PCF2127_CTRL1_IRQ_MASK || ctrl2 & PCF2127_CTRL2_IRQ_MASK))
+ return IRQ_NONE;
+
+ if (ctrl1 & PCF2127_BIT_CTRL1_TSF1 || ctrl2 & PCF2127_BIT_CTRL2_TSF2)
+ pcf2127_rtc_ts_snapshot(dev, 0);
+
+ if (ctrl1 & PCF2127_CTRL1_IRQ_MASK)
+ regmap_write(pcf2127->regmap, PCF2127_REG_CTRL1,
+ ctrl1 & ~PCF2127_CTRL1_IRQ_MASK);

- if (ctrl1 & PCF2127_BIT_CTRL1_TSF1 || ctrl2 & PCF2127_BIT_CTRL2_TSF2)
- pcf2127_rtc_ts_snapshot(dev, 0);
+ if (ctrl2 & PCF2127_CTRL2_IRQ_MASK)
+ regmap_write(pcf2127->regmap, PCF2127_REG_CTRL2,
+ ctrl2 & ~PCF2127_CTRL2_IRQ_MASK);
+ } else {
+ /* PCF2131. */
+ unsigned int ctrl4;
+
+ ret = regmap_read(pcf2127->regmap, PCF2131_REG_CTRL4, &ctrl4);
+ if (ret)
+ return IRQ_NONE;

- if (ctrl1 & PCF2127_CTRL1_IRQ_MASK)
- regmap_write(pcf2127->regmap, PCF2127_REG_CTRL1,
- ctrl1 & ~PCF2127_CTRL1_IRQ_MASK);
+ if (!(ctrl4 & PCF2131_CTRL4_IRQ_MASK || ctrl2 & PCF2131_CTRL2_IRQ_MASK))
+ return IRQ_NONE;

- if (ctrl2 & PCF2127_CTRL2_IRQ_MASK)
- regmap_write(pcf2127->regmap, PCF2127_REG_CTRL2,
- ctrl2 & ~PCF2127_CTRL2_IRQ_MASK);
+ if (ctrl4 & PCF2131_CTRL4_IRQ_MASK) {
+ int i;
+ int tsf_bit = PCF2131_BIT_CTRL4_TSF1; /* Start at bit 7. */
+
+ for (i = 0; i < pcf2127->cfg->ts_count; i++) {
+ if (ctrl4 & tsf_bit)
+ pcf2127_rtc_ts_snapshot(dev, i);
+
+ tsf_bit = tsf_bit >> 1;
+ }
+
+ regmap_write(pcf2127->regmap, PCF2131_REG_CTRL4,
+ ctrl4 & ~PCF2131_CTRL4_IRQ_MASK);
+ }
+
+ if (ctrl2 & PCF2131_CTRL2_IRQ_MASK)
+ regmap_write(pcf2127->regmap, PCF2127_REG_CTRL2,
+ ctrl2 & ~PCF2131_CTRL2_IRQ_MASK);
+ }

if (ctrl2 & PCF2127_BIT_CTRL2_AF)
rtc_update_irq(pcf2127->rtc, 1, RTC_IRQF | RTC_AF);
@@ -626,6 +718,27 @@ static ssize_t timestamp0_store(struct device *dev,
return timestamp_store(dev, attr, buf, count, 0);
};

+static ssize_t timestamp1_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ return timestamp_store(dev, attr, buf, count, 1);
+};
+
+static ssize_t timestamp2_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ return timestamp_store(dev, attr, buf, count, 2);
+};
+
+static ssize_t timestamp3_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ return timestamp_store(dev, attr, buf, count, 3);
+};
+
static ssize_t timestamp_show(struct device *dev,
struct device_attribute *attr, char *buf,
int ts_id)
@@ -690,13 +803,42 @@ static ssize_t timestamp0_show(struct device *dev,
return timestamp_show(dev, attr, buf, 0);
};

+static ssize_t timestamp1_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return timestamp_show(dev, attr, buf, 1);
+};
+
+static ssize_t timestamp2_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return timestamp_show(dev, attr, buf, 2);
+};
+
+static ssize_t timestamp3_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return timestamp_show(dev, attr, buf, 3);
+};
+
static DEVICE_ATTR_RW(timestamp0);
+static DEVICE_ATTR_RW(timestamp1);
+static DEVICE_ATTR_RW(timestamp2);
+static DEVICE_ATTR_RW(timestamp3);

static struct attribute *pcf2127_attrs[] = {
&dev_attr_timestamp0.attr,
NULL
};

+static struct attribute *pcf2131_attrs[] = {
+ &dev_attr_timestamp0.attr,
+ &dev_attr_timestamp1.attr,
+ &dev_attr_timestamp2.attr,
+ &dev_attr_timestamp3.attr,
+ NULL
+};
+
static struct pcf21xx_config pcf21xx_cfg[] = {
[PCF2127] = {
.type = PCF2127,
@@ -746,6 +888,53 @@ static struct pcf21xx_config pcf21xx_cfg[] = {
.attrs = pcf2127_attrs,
},
},
+ [PCF2131] = {
+ .type = PCF2131,
+ .max_register = 0x36,
+ .has_nvmem = 0,
+ .has_bit_wd_ctl_cd0 = 0,
+ .reg_time_base = PCF2131_REG_TIME_BASE,
+ .regs_alarm_base = PCF2131_REG_ALARM_BASE,
+ .reg_wd_ctl = PCF2131_REG_WD_CTL,
+ .reg_wd_val = PCF2131_REG_WD_VAL,
+ .reg_clkout = PCF2131_REG_CLKOUT,
+ .ts_count = 4,
+ .ts[0] = {
+ .reg_base = PCF2131_REG_TS1_BASE,
+ .gnd_detect_reg = PCF2131_REG_CTRL4,
+ .gnd_detect_bit = PCF2131_BIT_CTRL4_TSF1,
+ .inter_detect_bit = 0,
+ .ie_reg = PCF2131_REG_CTRL5,
+ .ie_bit = PCF2131_BIT_CTRL5_TSIE1,
+ },
+ .ts[1] = {
+ .reg_base = PCF2131_REG_TS2_BASE,
+ .gnd_detect_reg = PCF2131_REG_CTRL4,
+ .gnd_detect_bit = PCF2131_BIT_CTRL4_TSF2,
+ .inter_detect_bit = 0,
+ .ie_reg = PCF2131_REG_CTRL5,
+ .ie_bit = PCF2131_BIT_CTRL5_TSIE2,
+ },
+ .ts[2] = {
+ .reg_base = PCF2131_REG_TS3_BASE,
+ .gnd_detect_reg = PCF2131_REG_CTRL4,
+ .gnd_detect_bit = PCF2131_BIT_CTRL4_TSF3,
+ .inter_detect_bit = 0,
+ .ie_reg = PCF2131_REG_CTRL5,
+ .ie_bit = PCF2131_BIT_CTRL5_TSIE3,
+ },
+ .ts[3] = {
+ .reg_base = PCF2131_REG_TS4_BASE,
+ .gnd_detect_reg = PCF2131_REG_CTRL4,
+ .gnd_detect_bit = PCF2131_BIT_CTRL4_TSF4,
+ .inter_detect_bit = 0,
+ .ie_reg = PCF2131_REG_CTRL5,
+ .ie_bit = PCF2131_BIT_CTRL5_TSIE4,
+ },
+ .attribute_group = {
+ .attrs = pcf2131_attrs,
+ },
+ },
};

/*
@@ -893,7 +1082,7 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap,
* 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.
- * For pca2129, pcf2129, only bit[7] is for Symbol WD_CD
+ * For pca2129, pcf2129 and pcf2131, only bit[7] is for Symbol WD_CD
* of register watchdg_tim_ctl. The bit[6] is labeled
* as T. Bits labeled as T must always be written with
* logic 0.
@@ -953,6 +1142,7 @@ static const struct of_device_id pcf2127_of_match[] = {
{ .compatible = "nxp,pcf2127", .data = &pcf21xx_cfg[PCF2127] },
{ .compatible = "nxp,pcf2129", .data = &pcf21xx_cfg[PCF2129] },
{ .compatible = "nxp,pca2129", .data = &pcf21xx_cfg[PCF2129] },
+ { .compatible = "nxp,pcf2131", .data = &pcf21xx_cfg[PCF2131] },
{}
};
MODULE_DEVICE_TABLE(of, pcf2127_of_match);
@@ -1040,6 +1230,7 @@ static const struct i2c_device_id pcf2127_i2c_id[] = {
{ "pcf2127", PCF2127 },
{ "pcf2129", PCF2129 },
{ "pca2129", PCF2129 },
+ { "pcf2131", PCF2131 },
{ }
};
MODULE_DEVICE_TABLE(i2c, pcf2127_i2c_id);
@@ -1161,6 +1352,7 @@ static const struct spi_device_id pcf2127_spi_id[] = {
{ "pcf2127", PCF2127 },
{ "pcf2129", PCF2129 },
{ "pca2129", PCF2129 },
+ { "pcf2131", PCF2131 },
{ }
};
MODULE_DEVICE_TABLE(spi, pcf2127_spi_id);
@@ -1225,5 +1417,5 @@ static void __exit pcf2127_exit(void)
module_exit(pcf2127_exit)

MODULE_AUTHOR("Renaud Cerrato <[email protected]>");
-MODULE_DESCRIPTION("NXP PCF2127/29 RTC driver");
+MODULE_DESCRIPTION("NXP PCF2127/29/31 RTC driver");
MODULE_LICENSE("GPL v2");
--
2.30.2


2023-06-22 15:12:11

by Hugo Villeneuve

[permalink] [raw]
Subject: [PATCH v4 03/17] rtc: pcf2127: lower message severity if setting time fails

From: Hugo Villeneuve <[email protected]>

Noted while reviewing new PCF2131 driver.

Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/rtc/rtc-pcf2127.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index a913a5c82ebf..443c9544f575 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -181,8 +181,7 @@ static int pcf2127_rtc_set_time(struct device *dev, struct rtc_time *tm)
/* write register's data */
err = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_SC, buf, i);
if (err) {
- dev_err(dev,
- "%s: err=%d", __func__, err);
+ dev_dbg(dev, "%s: err=%d", __func__, err);
return err;
}

--
2.30.2


2023-06-22 15:12:26

by Hugo Villeneuve

[permalink] [raw]
Subject: [PATCH v4 17/17] dt-bindings: rtc: pcf2127: add PCF2131

From: Hugo Villeneuve <[email protected]>

Add support for new NXP RTC PCF2131.

Signed-off-by: Hugo Villeneuve <[email protected]>
Acked-by: Krzysztof Kozlowski <[email protected]>
---
Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml b/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
index bcb230027622..2d9fe5a75b06 100644
--- a/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
+++ b/Documentation/devicetree/bindings/rtc/nxp,pcf2127.yaml
@@ -18,6 +18,7 @@ properties:
- nxp,pca2129
- nxp,pcf2127
- nxp,pcf2129
+ - nxp,pcf2131

reg:
maxItems: 1
--
2.30.2


2023-06-22 15:12:36

by Hugo Villeneuve

[permalink] [raw]
Subject: [PATCH v4 10/17] rtc: pcf2127: add support for multiple TS functions

From: Hugo Villeneuve <[email protected]>

This will simplify the implementation of new variants into this driver.

Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/rtc/rtc-pcf2127.c | 268 ++++++++++++++++++++++++++++----------
1 file changed, 201 insertions(+), 67 deletions(-)

diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index 21fa42870dd4..c6bbeb1916a4 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -59,8 +59,8 @@
#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
+/* Tamper timestamp1 registers */
+#define PCF2127_REG_TS1_BASE 0x12
#define PCF2127_BIT_TS_CTRL_TSOFF BIT(6)
#define PCF2127_BIT_TS_CTRL_TSM BIT(7)
/*
@@ -86,12 +86,36 @@
PCF2127_BIT_CTRL2_WDTF | \
PCF2127_BIT_CTRL2_TSF2)

+#define PCF2127_MAX_TS_SUPPORTED 1
+
enum pcf21xx_type {
PCF2127,
PCF2129,
PCF21XX_LAST_ID
};

+struct pcf21xx_ts_config {
+ u8 reg_base; /* Base register to read timestamp values. */
+
+ /*
+ * If the TS input pin is driven to GND, an interrupt can be generated
+ * (supported by all variants).
+ */
+ u8 gnd_detect_reg; /* Interrupt control register address. */
+ u8 gnd_detect_bit; /* Interrupt bit. */
+
+ /*
+ * If the TS input pin is driven to an intermediate level between GND
+ * and supply, an interrupt can be generated (optional feature depending
+ * on variant).
+ */
+ u8 inter_detect_reg; /* Interrupt control register address. */
+ u8 inter_detect_bit; /* Interrupt bit. */
+
+ u8 ie_reg; /* Interrupt enable control register. */
+ u8 ie_bit; /* Interrupt enable bit. */
+};
+
struct pcf21xx_config {
int type; /* IC variant */
int max_register;
@@ -102,6 +126,9 @@ struct pcf21xx_config {
u8 reg_wd_ctl; /* Watchdog control register. */
u8 reg_wd_val; /* Watchdog value register. */
u8 reg_clkout; /* Clkout register. */
+ unsigned int ts_count;
+ struct pcf21xx_ts_config ts[PCF2127_MAX_TS_SUPPORTED];
+ struct attribute_group attribute_group;
};

struct pcf2127 {
@@ -109,9 +136,9 @@ struct pcf2127 {
struct watchdog_device wdd;
struct regmap *regmap;
const struct pcf21xx_config *cfg;
- time64_t ts;
- bool ts_valid;
bool irq_enabled;
+ time64_t ts[PCF2127_MAX_TS_SUPPORTED]; /* Timestamp values. */
+ bool ts_valid[PCF2127_MAX_TS_SUPPORTED]; /* Timestamp valid indication. */
};

/*
@@ -441,18 +468,19 @@ static int pcf2127_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
}

/*
- * This function reads ctrl2 register, caller is responsible for calling
- * pcf2127_wdt_active_ping()
+ * This function reads one timestamp function data, caller is responsible for
+ * calling pcf2127_wdt_active_ping()
*/
-static int pcf2127_rtc_ts_read(struct device *dev, time64_t *ts)
+static int pcf2127_rtc_ts_read(struct device *dev, time64_t *ts,
+ int ts_id)
{
struct pcf2127 *pcf2127 = dev_get_drvdata(dev);
struct rtc_time tm;
int ret;
unsigned char data[7];

- ret = regmap_bulk_read(pcf2127->regmap, PCF2127_REG_TS_CTRL, data,
- sizeof(data));
+ ret = regmap_bulk_read(pcf2127->regmap, pcf2127->cfg->ts[ts_id].reg_base,
+ data, sizeof(data));
if (ret) {
dev_err(dev, "%s: read error ret=%d\n", __func__, ret);
return ret;
@@ -482,18 +510,21 @@ static int pcf2127_rtc_ts_read(struct device *dev, time64_t *ts)
return 0;
};

-static void pcf2127_rtc_ts_snapshot(struct device *dev)
+static void pcf2127_rtc_ts_snapshot(struct device *dev, int ts_id)
{
struct pcf2127 *pcf2127 = dev_get_drvdata(dev);
int ret;

+ if (ts_id >= pcf2127->cfg->ts_count)
+ return;
+
/* Let userspace read the first timestamp */
- if (pcf2127->ts_valid)
+ if (pcf2127->ts_valid[ts_id])
return;

- ret = pcf2127_rtc_ts_read(dev, &pcf2127->ts);
+ ret = pcf2127_rtc_ts_read(dev, &pcf2127->ts[ts_id], ts_id);
if (!ret)
- pcf2127->ts_valid = true;
+ pcf2127->ts_valid[ts_id] = true;
}

static irqreturn_t pcf2127_rtc_irq(int irq, void *dev)
@@ -514,7 +545,7 @@ static irqreturn_t pcf2127_rtc_irq(int irq, void *dev)
return IRQ_NONE;

if (ctrl1 & PCF2127_BIT_CTRL1_TSF1 || ctrl2 & PCF2127_BIT_CTRL2_TSF2)
- pcf2127_rtc_ts_snapshot(dev);
+ pcf2127_rtc_ts_snapshot(dev, 0);

if (ctrl1 & PCF2127_CTRL1_IRQ_MASK)
regmap_write(pcf2127->regmap, PCF2127_REG_CTRL1,
@@ -543,28 +574,41 @@ static const struct rtc_class_ops pcf2127_rtc_ops = {

/* sysfs interface */

-static ssize_t timestamp0_store(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t count)
+static ssize_t timestamp_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count, int ts_id)
{
struct pcf2127 *pcf2127 = dev_get_drvdata(dev->parent);
int ret;

+ if (ts_id >= pcf2127->cfg->ts_count)
+ return 0;
+
if (pcf2127->irq_enabled) {
- pcf2127->ts_valid = false;
+ pcf2127->ts_valid[ts_id] = false;
} else {
- ret = regmap_update_bits(pcf2127->regmap, PCF2127_REG_CTRL1,
- PCF2127_BIT_CTRL1_TSF1, 0);
+ /* Always clear GND interrupt bit. */
+ ret = regmap_update_bits(pcf2127->regmap,
+ pcf2127->cfg->ts[ts_id].gnd_detect_reg,
+ pcf2127->cfg->ts[ts_id].gnd_detect_bit,
+ 0);
+
if (ret) {
- dev_err(dev, "%s: update ctrl1 ret=%d\n", __func__, ret);
+ dev_err(dev, "%s: update TS gnd detect 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;
+ if (pcf2127->cfg->ts[ts_id].inter_detect_bit) {
+ /* Clear intermediate level interrupt bit if supported. */
+ ret = regmap_update_bits(pcf2127->regmap,
+ pcf2127->cfg->ts[ts_id].inter_detect_reg,
+ pcf2127->cfg->ts[ts_id].inter_detect_bit,
+ 0);
+ if (ret) {
+ dev_err(dev, "%s: update TS intermediate level detect ret=%d\n",
+ __func__, ret);
+ return ret;
+ }
}

ret = pcf2127_wdt_active_ping(&pcf2127->wdd);
@@ -573,34 +617,63 @@ static ssize_t timestamp0_store(struct device *dev,
}

return count;
+}
+
+static ssize_t timestamp0_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ return timestamp_store(dev, attr, buf, count, 0);
};

-static ssize_t timestamp0_show(struct device *dev,
- struct device_attribute *attr, char *buf)
+static ssize_t timestamp_show(struct device *dev,
+ struct device_attribute *attr, char *buf,
+ int ts_id)
{
struct pcf2127 *pcf2127 = dev_get_drvdata(dev->parent);
- unsigned int ctrl1, ctrl2;
int ret;
time64_t ts;

+ if (ts_id >= pcf2127->cfg->ts_count)
+ return 0;
+
if (pcf2127->irq_enabled) {
- if (!pcf2127->ts_valid)
+ if (!pcf2127->ts_valid[ts_id])
return 0;
- ts = pcf2127->ts;
+ ts = pcf2127->ts[ts_id];
} else {
- ret = regmap_read(pcf2127->regmap, PCF2127_REG_CTRL1, &ctrl1);
- if (ret)
- return 0;
+ u8 valid_low = 0;
+ u8 valid_inter = 0;
+ unsigned int ctrl;

- ret = regmap_read(pcf2127->regmap, PCF2127_REG_CTRL2, &ctrl2);
+ /* Check if TS input pin is driven to GND, supported by all
+ * variants.
+ */
+ ret = regmap_read(pcf2127->regmap,
+ pcf2127->cfg->ts[ts_id].gnd_detect_reg,
+ &ctrl);
if (ret)
return 0;

- if (!(ctrl1 & PCF2127_BIT_CTRL1_TSF1) &&
- !(ctrl2 & PCF2127_BIT_CTRL2_TSF2))
+ valid_low = ctrl & pcf2127->cfg->ts[ts_id].gnd_detect_bit;
+
+ if (pcf2127->cfg->ts[ts_id].inter_detect_bit) {
+ /* Check if TS input pin is driven to intermediate level
+ * between GND and supply, if supported by variant.
+ */
+ ret = regmap_read(pcf2127->regmap,
+ pcf2127->cfg->ts[ts_id].inter_detect_reg,
+ &ctrl);
+ if (ret)
+ return 0;
+
+ valid_inter = ctrl & pcf2127->cfg->ts[ts_id].inter_detect_bit;
+ }
+
+ if (!valid_low && !valid_inter)
return 0;

- ret = pcf2127_rtc_ts_read(dev->parent, &ts);
+ ret = pcf2127_rtc_ts_read(dev->parent, &ts, ts_id);
if (ret)
return 0;

@@ -609,6 +682,12 @@ static ssize_t timestamp0_show(struct device *dev,
return ret;
}
return sprintf(buf, "%llu\n", (unsigned long long)ts);
+}
+
+static ssize_t timestamp0_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return timestamp_show(dev, attr, buf, 0);
};

static DEVICE_ATTR_RW(timestamp0);
@@ -618,10 +697,6 @@ static struct attribute *pcf2127_attrs[] = {
NULL
};

-static const struct attribute_group pcf2127_attr_group = {
- .attrs = pcf2127_attrs,
-};
-
static struct pcf21xx_config pcf21xx_cfg[] = {
[PCF2127] = {
.type = PCF2127,
@@ -633,6 +708,19 @@ static struct pcf21xx_config pcf21xx_cfg[] = {
.reg_wd_ctl = PCF2127_REG_WD_CTL,
.reg_wd_val = PCF2127_REG_WD_VAL,
.reg_clkout = PCF2127_REG_CLKOUT,
+ .ts_count = 1,
+ .ts[0] = {
+ .reg_base = PCF2127_REG_TS1_BASE,
+ .gnd_detect_reg = PCF2127_REG_CTRL1,
+ .gnd_detect_bit = PCF2127_BIT_CTRL1_TSF1,
+ .inter_detect_reg = PCF2127_REG_CTRL2,
+ .inter_detect_bit = PCF2127_BIT_CTRL2_TSF2,
+ .ie_reg = PCF2127_REG_CTRL2,
+ .ie_bit = PCF2127_BIT_CTRL2_TSIE,
+ },
+ .attribute_group = {
+ .attrs = pcf2127_attrs,
+ },
},
[PCF2129] = {
.type = PCF2129,
@@ -644,9 +732,74 @@ static struct pcf21xx_config pcf21xx_cfg[] = {
.reg_wd_ctl = PCF2127_REG_WD_CTL,
.reg_wd_val = PCF2127_REG_WD_VAL,
.reg_clkout = PCF2127_REG_CLKOUT,
+ .ts_count = 1,
+ .ts[0] = {
+ .reg_base = PCF2127_REG_TS1_BASE,
+ .gnd_detect_reg = PCF2127_REG_CTRL1,
+ .gnd_detect_bit = PCF2127_BIT_CTRL1_TSF1,
+ .inter_detect_reg = PCF2127_REG_CTRL2,
+ .inter_detect_bit = PCF2127_BIT_CTRL2_TSF2,
+ .ie_reg = PCF2127_REG_CTRL2,
+ .ie_bit = PCF2127_BIT_CTRL2_TSIE,
+ },
+ .attribute_group = {
+ .attrs = pcf2127_attrs,
+ },
},
};

+/*
+ * Enable timestamp function and corresponding interrupt(s).
+ */
+static int pcf2127_enable_ts(struct device *dev, int ts_id)
+{
+ struct pcf2127 *pcf2127 = dev_get_drvdata(dev);
+ int ret;
+
+ if (ts_id >= pcf2127->cfg->ts_count) {
+ dev_err(dev, "%s: invalid tamper detection ID (%d)\n",
+ __func__, ts_id);
+ return -EINVAL;
+ }
+
+ /* Enable timestamp function. */
+ ret = regmap_update_bits(pcf2127->regmap,
+ pcf2127->cfg->ts[ts_id].reg_base,
+ PCF2127_BIT_TS_CTRL_TSOFF |
+ PCF2127_BIT_TS_CTRL_TSM,
+ PCF2127_BIT_TS_CTRL_TSM);
+ if (ret) {
+ dev_err(dev, "%s: tamper detection config (ts%d_ctrl) failed\n",
+ __func__, ts_id);
+ return ret;
+ }
+
+ /* TS input pin driven to GND detection is supported by all variants.
+ * Make sure that interrupt bit is defined.
+ */
+ if (pcf2127->cfg->ts[ts_id].gnd_detect_bit == 0) {
+ dev_err(dev, "%s: tamper detection to GND configuration invalid\n",
+ __func__);
+ return ret;
+ }
+
+ /*
+ * Enable interrupt generation when TSF timestamp flag is set.
+ * Interrupt signals are open-drain outputs and can be left floating if
+ * unused.
+ */
+ ret = regmap_update_bits(pcf2127->regmap, pcf2127->cfg->ts[ts_id].ie_reg,
+ pcf2127->cfg->ts[ts_id].ie_bit,
+ pcf2127->cfg->ts[ts_id].ie_bit);
+ if (ret) {
+ dev_err(dev, "%s: tamper detection TSIE%d config failed\n",
+ __func__, ts_id);
+ return ret;
+ }
+
+ return ret;
+}
+
static int pcf2127_probe(struct device *dev, struct regmap *regmap,
int alarm_irq, const char *name, const struct pcf21xx_config *config)
{
@@ -777,34 +930,15 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap,
}

/*
- * Enable timestamp function and store timestamp of first trigger
- * event until TSF1 and TSF2 interrupt flags are cleared.
+ * Enable timestamp functions 1 to 4.
*/
- 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 detection 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 detection config (ctrl2) failed\n",
- __func__);
- return ret;
+ for (int i = 0; i < pcf2127->cfg->ts_count; i++) {
+ ret = pcf2127_enable_ts(dev, i);
+ if (ret)
+ return ret;
}

- ret = rtc_add_group(pcf2127->rtc, &pcf2127_attr_group);
+ ret = rtc_add_group(pcf2127->rtc, &pcf2127->cfg->attribute_group);
if (ret) {
dev_err(dev, "%s: tamper sysfs registering failed\n",
__func__);
--
2.30.2


2023-06-22 15:12:43

by Hugo Villeneuve

[permalink] [raw]
Subject: [PATCH v4 07/17] rtc: pcf2127: adapt for alarm registers at any offset

From: Hugo Villeneuve <[email protected]>

This will simplify the implementation of new variants into this driver.

Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/rtc/rtc-pcf2127.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index 0b9110b936f0..31f6bba81212 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -47,11 +47,7 @@
#define PCF2127_REG_TIME_BASE 0x03
#define PCF2127_BIT_SC_OSF BIT(7)
/* Alarm registers */
-#define PCF2127_REG_ALARM_SC 0x0A
-#define PCF2127_REG_ALARM_MN 0x0B
-#define PCF2127_REG_ALARM_HR 0x0C
-#define PCF2127_REG_ALARM_DM 0x0D
-#define PCF2127_REG_ALARM_DW 0x0E
+#define PCF2127_REG_ALARM_BASE 0x0A
#define PCF2127_BIT_ALARM_AE BIT(7)
/* CLKOUT control register */
#define PCF2127_REG_CLKOUT 0x0f
@@ -102,6 +98,7 @@ struct pcf21xx_config {
unsigned int has_nvmem:1;
unsigned int has_bit_wd_ctl_cd0:1;
u8 reg_time_base; /* Time/date base register. */
+ u8 regs_alarm_base; /* Alarm function base registers. */
};

struct pcf2127 {
@@ -381,8 +378,8 @@ static int pcf2127_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
if (ret)
return ret;

- ret = regmap_bulk_read(pcf2127->regmap, PCF2127_REG_ALARM_SC, buf,
- sizeof(buf));
+ ret = regmap_bulk_read(pcf2127->regmap, pcf2127->cfg->regs_alarm_base,
+ buf, sizeof(buf));
if (ret)
return ret;

@@ -432,8 +429,8 @@ static int pcf2127_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
buf[3] = bin2bcd(alrm->time.tm_mday);
buf[4] = PCF2127_BIT_ALARM_AE; /* Do not match on week day */

- ret = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_ALARM_SC, buf,
- sizeof(buf));
+ ret = regmap_bulk_write(pcf2127->regmap, pcf2127->cfg->regs_alarm_base,
+ buf, sizeof(buf));
if (ret)
return ret;

@@ -629,6 +626,7 @@ static struct pcf21xx_config pcf21xx_cfg[] = {
.has_nvmem = 1,
.has_bit_wd_ctl_cd0 = 1,
.reg_time_base = PCF2127_REG_TIME_BASE,
+ .regs_alarm_base = PCF2127_REG_ALARM_BASE,
},
[PCF2129] = {
.type = PCF2129,
@@ -636,6 +634,7 @@ static struct pcf21xx_config pcf21xx_cfg[] = {
.has_nvmem = 0,
.has_bit_wd_ctl_cd0 = 0,
.reg_time_base = PCF2127_REG_TIME_BASE,
+ .regs_alarm_base = PCF2127_REG_ALARM_BASE,
},
};

--
2.30.2


2023-06-22 15:13:39

by Hugo Villeneuve

[permalink] [raw]
Subject: [PATCH v4 14/17] rtc: pcf2127: support generic watchdog timing configuration

From: Hugo Villeneuve <[email protected]>

Introduce in the configuration structure two new values to hold the
watchdog clock source and the min_hw_heartbeat_ms value.

The minimum and maximum timeout values are automatically computed from
the watchdog clock source value for each variant.

The PCF2131 has no 1Hz watchdog clock source, as is the case for
PCF2127/29.

The next best choice is using a 1/4Hz clock, giving a watchdog timeout
range between 4 and 1016s. By using the same register configuration as
for the PCF2127/29, the 1/4Hz clock source is selected.

Note: the PCF2127 datasheet gives a min/max range between 1 and 255s,
but it should be between 2 and 254s, because the watchdog is triggered
when the timer value reaches 1, not 0.

Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/rtc/rtc-pcf2127.c | 64 +++++++++++++++++++++++++++++++++------
1 file changed, 55 insertions(+), 9 deletions(-)

diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index 3b8718aaadd7..c20eab277385 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -80,9 +80,14 @@

/* Watchdog timer value constants */
#define PCF2127_WD_VAL_STOP 0
-#define PCF2127_WD_VAL_MIN 2
-#define PCF2127_WD_VAL_MAX 255
-#define PCF2127_WD_VAL_DEFAULT 60
+/* PCF2127/29 watchdog timer value constants */
+#define PCF2127_WD_CLOCK_HZ_X1000 1000 /* 1Hz */
+#define PCF2127_WD_MIN_HW_HEARTBEAT_MS 500
+/* PCF2131 watchdog timer value constants */
+#define PCF2131_WD_CLOCK_HZ_X1000 250 /* 1/4Hz */
+#define PCF2131_WD_MIN_HW_HEARTBEAT_MS 4000
+
+#define PCF2127_WD_DEFAULT_TIMEOUT_S 60

/* Mask for currently enabled interrupts */
#define PCF2127_CTRL1_IRQ_MASK (PCF2127_BIT_CTRL1_TSF1)
@@ -186,6 +191,8 @@ struct pcf21xx_config {
u8 reg_wd_ctl; /* Watchdog control register. */
u8 reg_wd_val; /* Watchdog value register. */
u8 reg_clkout; /* Clkout register. */
+ int wdd_clock_hz_x1000; /* Watchdog clock in Hz multiplicated by 1000 */
+ int wdd_min_hw_heartbeat_ms;
unsigned int ts_count;
struct pcf21xx_ts_config ts[PCF2127_MAX_TS_SUPPORTED];
struct attribute_group attribute_group;
@@ -389,9 +396,16 @@ static int pcf2127_nvmem_write(void *priv, unsigned int offset,

static int pcf2127_wdt_ping(struct watchdog_device *wdd)
{
+ int wd_val;
struct pcf2127 *pcf2127 = watchdog_get_drvdata(wdd);

- return regmap_write(pcf2127->regmap, pcf2127->cfg->reg_wd_val, wdd->timeout);
+ /*
+ * Compute counter value of WATCHDG_TIM_VAL to obtain desired period
+ * in seconds, depending on the source clock frequency.
+ */
+ wd_val = ((wdd->timeout * pcf2127->cfg->wdd_clock_hz_x1000) / 1000) + 1;
+
+ return regmap_write(pcf2127->regmap, pcf2127->cfg->reg_wd_val, wd_val);
}

/*
@@ -453,6 +467,23 @@ static const struct watchdog_ops pcf2127_watchdog_ops = {
.set_timeout = pcf2127_wdt_set_timeout,
};

+/*
+ * Compute watchdog period, t, in seconds, from the WATCHDG_TIM_VAL register
+ * value, n, and the clock frequency, f1000, in Hz x 1000.
+ *
+ * The PCF2127/29 datasheet gives t as:
+ * t = n / f
+ * The PCF2131 datasheet gives t as:
+ * t = (n - 1) / f
+ * For both variants, the watchdog is triggered when the WATCHDG_TIM_VAL reaches
+ * the value 1, and not zero. Consequently, the equation from the PCF2131
+ * datasheet seems to be the correct one for both variants.
+ */
+static int pcf2127_watchdog_get_period(int n, int f1000)
+{
+ return (1000 * (n - 1)) / f1000;
+}
+
static int pcf2127_watchdog_init(struct device *dev, struct pcf2127 *pcf2127)
{
u32 wdd_timeout;
@@ -465,10 +496,19 @@ static int pcf2127_watchdog_init(struct device *dev, struct pcf2127 *pcf2127)
pcf2127->wdd.parent = dev;
pcf2127->wdd.info = &pcf2127_wdt_info;
pcf2127->wdd.ops = &pcf2127_watchdog_ops;
- pcf2127->wdd.min_timeout = PCF2127_WD_VAL_MIN;
- pcf2127->wdd.max_timeout = PCF2127_WD_VAL_MAX;
- pcf2127->wdd.timeout = PCF2127_WD_VAL_DEFAULT;
- pcf2127->wdd.min_hw_heartbeat_ms = 500;
+
+ pcf2127->wdd.min_timeout =
+ pcf2127_watchdog_get_period(
+ 2, pcf2127->cfg->wdd_clock_hz_x1000);
+ pcf2127->wdd.max_timeout =
+ pcf2127_watchdog_get_period(
+ 255, pcf2127->cfg->wdd_clock_hz_x1000);
+ pcf2127->wdd.timeout = PCF2127_WD_DEFAULT_TIMEOUT_S;
+
+ dev_dbg(dev, "%s clock = %d Hz / 1000\n", __func__,
+ pcf2127->cfg->wdd_clock_hz_x1000);
+
+ pcf2127->wdd.min_hw_heartbeat_ms = pcf2127->cfg->wdd_min_hw_heartbeat_ms;
pcf2127->wdd.status = WATCHDOG_NOWAYOUT_INIT_STATUS;

watchdog_set_drvdata(&pcf2127->wdd, pcf2127);
@@ -885,6 +925,8 @@ static struct pcf21xx_config pcf21xx_cfg[] = {
.reg_wd_ctl = PCF2127_REG_WD_CTL,
.reg_wd_val = PCF2127_REG_WD_VAL,
.reg_clkout = PCF2127_REG_CLKOUT,
+ .wdd_clock_hz_x1000 = PCF2127_WD_CLOCK_HZ_X1000,
+ .wdd_min_hw_heartbeat_ms = PCF2127_WD_MIN_HW_HEARTBEAT_MS,
.ts_count = 1,
.ts[0] = {
.reg_base = PCF2127_REG_TS1_BASE,
@@ -910,6 +952,8 @@ static struct pcf21xx_config pcf21xx_cfg[] = {
.reg_wd_ctl = PCF2127_REG_WD_CTL,
.reg_wd_val = PCF2127_REG_WD_VAL,
.reg_clkout = PCF2127_REG_CLKOUT,
+ .wdd_clock_hz_x1000 = PCF2127_WD_CLOCK_HZ_X1000,
+ .wdd_min_hw_heartbeat_ms = PCF2127_WD_MIN_HW_HEARTBEAT_MS,
.ts_count = 1,
.ts[0] = {
.reg_base = PCF2127_REG_TS1_BASE,
@@ -935,6 +979,8 @@ static struct pcf21xx_config pcf21xx_cfg[] = {
.reg_wd_ctl = PCF2131_REG_WD_CTL,
.reg_wd_val = PCF2131_REG_WD_VAL,
.reg_clkout = PCF2131_REG_CLKOUT,
+ .wdd_clock_hz_x1000 = PCF2131_WD_CLOCK_HZ_X1000,
+ .wdd_min_hw_heartbeat_ms = PCF2131_WD_MIN_HW_HEARTBEAT_MS,
.ts_count = 4,
.ts[0] = {
.reg_base = PCF2131_REG_TS1_BASE,
@@ -1148,7 +1194,7 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap,

/*
* Watchdog timer enabled and reset pin /RST activated when timed out.
- * Select 1Hz clock source for watchdog timer.
+ * Select 1Hz clock source for watchdog timer (1/4Hz for PCF2131).
* Note: Countdown timer disabled and not available.
* For pca2129, pcf2129 and pcf2131, only bit[7] is for Symbol WD_CD
* of register watchdg_tim_ctl. The bit[6] is labeled
--
2.30.2


2023-06-22 15:13:53

by Hugo Villeneuve

[permalink] [raw]
Subject: [PATCH v4 08/17] rtc: pcf2127: adapt for WD registers at any offset

From: Hugo Villeneuve <[email protected]>

This will simplify the implementation of new variants into this driver.

Signed-off-by: Hugo Villeneuve <[email protected]>
Reviewed-by: Bruno Thomsen <[email protected]>
Tested-by: Bruno Thomsen <[email protected]>
---
drivers/rtc/rtc-pcf2127.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index 31f6bba81212..ce7fe6e2fd47 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -99,6 +99,8 @@ struct pcf21xx_config {
unsigned int has_bit_wd_ctl_cd0:1;
u8 reg_time_base; /* Time/date base register. */
u8 regs_alarm_base; /* Alarm function base registers. */
+ u8 reg_wd_ctl; /* Watchdog control register. */
+ u8 reg_wd_val; /* Watchdog value register. */
};

struct pcf2127 {
@@ -269,7 +271,7 @@ static int pcf2127_wdt_ping(struct watchdog_device *wdd)
{
struct pcf2127 *pcf2127 = watchdog_get_drvdata(wdd);

- return regmap_write(pcf2127->regmap, PCF2127_REG_WD_VAL, wdd->timeout);
+ return regmap_write(pcf2127->regmap, pcf2127->cfg->reg_wd_val, wdd->timeout);
}

/*
@@ -303,7 +305,7 @@ static int pcf2127_wdt_stop(struct watchdog_device *wdd)
{
struct pcf2127 *pcf2127 = watchdog_get_drvdata(wdd);

- return regmap_write(pcf2127->regmap, PCF2127_REG_WD_VAL,
+ return regmap_write(pcf2127->regmap, pcf2127->cfg->reg_wd_val,
PCF2127_WD_VAL_STOP);
}

@@ -352,7 +354,7 @@ static int pcf2127_watchdog_init(struct device *dev, struct pcf2127 *pcf2127)
watchdog_set_drvdata(&pcf2127->wdd, pcf2127);

/* Test if watchdog timer is started by bootloader */
- ret = regmap_read(pcf2127->regmap, PCF2127_REG_WD_VAL, &wdd_timeout);
+ ret = regmap_read(pcf2127->regmap, pcf2127->cfg->reg_wd_val, &wdd_timeout);
if (ret)
return ret;

@@ -627,6 +629,8 @@ static struct pcf21xx_config pcf21xx_cfg[] = {
.has_bit_wd_ctl_cd0 = 1,
.reg_time_base = PCF2127_REG_TIME_BASE,
.regs_alarm_base = PCF2127_REG_ALARM_BASE,
+ .reg_wd_ctl = PCF2127_REG_WD_CTL,
+ .reg_wd_val = PCF2127_REG_WD_VAL,
},
[PCF2129] = {
.type = PCF2129,
@@ -635,6 +639,8 @@ static struct pcf21xx_config pcf21xx_cfg[] = {
.has_bit_wd_ctl_cd0 = 0,
.reg_time_base = PCF2127_REG_TIME_BASE,
.regs_alarm_base = PCF2127_REG_ALARM_BASE,
+ .reg_wd_ctl = PCF2127_REG_WD_CTL,
+ .reg_wd_val = PCF2127_REG_WD_VAL,
},
};

@@ -736,7 +742,7 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap,
* as T. Bits labeled as T must always be written with
* logic 0.
*/
- ret = regmap_update_bits(pcf2127->regmap, PCF2127_REG_WD_CTL,
+ ret = regmap_update_bits(pcf2127->regmap, pcf2127->cfg->reg_wd_ctl,
PCF2127_BIT_WD_CTL_CD1 |
PCF2127_BIT_WD_CTL_CD0 |
PCF2127_BIT_WD_CTL_TF1 |
--
2.30.2


2023-06-22 15:14:20

by Hugo Villeneuve

[permalink] [raw]
Subject: [PATCH v4 02/17] rtc: pcf2127: improve timestamp reading performance

From: Hugo Villeneuve <[email protected]>

Reading the 7 timetamp registers currently involves reading 25 registers
solely to be able to print the content of the three control registers,
in addition to the 7 timestamp registers. This print never occurs,
unless the user enables dynamic debug in this driver or set
CONFIG_RTC_DEBUG.

Reading the timestamp registers should consist of reading 7
consecutive timestamp registers.

This patch optimize the performance of reading the timestamp registers
by reading 7 consecutive registers instead of 25, and dropping the
print of the control registers.

Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/rtc/rtc-pcf2127.c | 32 +++++++++++---------------------
1 file changed, 11 insertions(+), 21 deletions(-)

diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index 475a627b2254..a913a5c82ebf 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -66,12 +66,6 @@
#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
@@ -440,9 +434,9 @@ static int pcf2127_rtc_ts_read(struct device *dev, time64_t *ts)
struct pcf2127 *pcf2127 = dev_get_drvdata(dev);
struct rtc_time tm;
int ret;
- unsigned char data[25];
+ unsigned char data[7];

- ret = regmap_bulk_read(pcf2127->regmap, PCF2127_REG_CTRL1, data,
+ ret = regmap_bulk_read(pcf2127->regmap, PCF2127_REG_TS_CTRL, data,
sizeof(data));
if (ret) {
dev_err(dev, "%s: read error ret=%d\n", __func__, ret);
@@ -450,20 +444,16 @@ static int pcf2127_rtc_ts_read(struct device *dev, time64_t *ts)
}

dev_dbg(dev,
- "%s: raw data is cr1=%02x, cr2=%02x, cr3=%02x, ts_sc=%02x, ts_mn=%02x, ts_hr=%02x, ts_dm=%02x, ts_mo=%02x, ts_yr=%02x\n",
- __func__, data[PCF2127_REG_CTRL1], data[PCF2127_REG_CTRL2],
- data[PCF2127_REG_CTRL3], data[PCF2127_REG_TS_SC],
- data[PCF2127_REG_TS_MN], data[PCF2127_REG_TS_HR],
- data[PCF2127_REG_TS_DM], data[PCF2127_REG_TS_MO],
- data[PCF2127_REG_TS_YR]);
-
- 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);
+ "%s: raw data is ts_sc=%02x, ts_mn=%02x, ts_hr=%02x, ts_dm=%02x, ts_mo=%02x, ts_yr=%02x\n",
+ __func__, data[1], data[2], data[3], data[4], data[5], data[6]);
+
+ tm.tm_sec = bcd2bin(data[1] & 0x7F);
+ tm.tm_min = bcd2bin(data[2] & 0x7F);
+ tm.tm_hour = bcd2bin(data[3] & 0x3F);
+ tm.tm_mday = bcd2bin(data[4] & 0x3F);
/* TS_MO register (month) value range: 1-12 */
- tm.tm_mon = bcd2bin(data[PCF2127_REG_TS_MO] & 0x1F) - 1;
- tm.tm_year = bcd2bin(data[PCF2127_REG_TS_YR]);
+ tm.tm_mon = bcd2bin(data[5] & 0x1F) - 1;
+ tm.tm_year = bcd2bin(data[6]);
if (tm.tm_year < 70)
tm.tm_year += 100; /* assume we are in 1970...2069 */

--
2.30.2


2023-06-22 15:20:05

by Hugo Villeneuve

[permalink] [raw]
Subject: [PATCH v4 12/17] rtc: pcf2127: add support for PCF2131 interrupts on output INT_A

From: Hugo Villeneuve <[email protected]>

The PCF2127 and PCF2129 have one output interrupt pin. The PCF2131 has
two, named INT_A and INT_B. The hardware support that any interrupt
source can be routed to either one or both of them.

Force all interrupt sources to go to the INT A pin.

Support to route any interrupt source to INT A/B pins is not supported
by this driver at the moment.

Signed-off-by: Hugo Villeneuve <[email protected]>
Reviewed-by: Bruno Thomsen <[email protected]>
---
drivers/rtc/rtc-pcf2127.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)

diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index 09607b67c282..2eef65232417 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -179,6 +179,7 @@ struct pcf21xx_config {
int max_register;
unsigned int has_nvmem:1;
unsigned int has_bit_wd_ctl_cd0:1;
+ unsigned int has_int_a_b:1; /* PCF2131 supports two interrupt outputs. */
u8 reg_time_base; /* Time/date base register. */
u8 regs_alarm_base; /* Alarm function base registers. */
u8 reg_wd_ctl; /* Watchdog control register. */
@@ -845,6 +846,7 @@ static struct pcf21xx_config pcf21xx_cfg[] = {
.max_register = 0x1d,
.has_nvmem = 1,
.has_bit_wd_ctl_cd0 = 1,
+ .has_int_a_b = 0,
.reg_time_base = PCF2127_REG_TIME_BASE,
.regs_alarm_base = PCF2127_REG_ALARM_BASE,
.reg_wd_ctl = PCF2127_REG_WD_CTL,
@@ -869,6 +871,7 @@ static struct pcf21xx_config pcf21xx_cfg[] = {
.max_register = 0x19,
.has_nvmem = 0,
.has_bit_wd_ctl_cd0 = 0,
+ .has_int_a_b = 0,
.reg_time_base = PCF2127_REG_TIME_BASE,
.regs_alarm_base = PCF2127_REG_ALARM_BASE,
.reg_wd_ctl = PCF2127_REG_WD_CTL,
@@ -893,6 +896,7 @@ static struct pcf21xx_config pcf21xx_cfg[] = {
.max_register = 0x36,
.has_nvmem = 0,
.has_bit_wd_ctl_cd0 = 0,
+ .has_int_a_b = 1,
.reg_time_base = PCF2131_REG_TIME_BASE,
.regs_alarm_base = PCF2131_REG_ALARM_BASE,
.reg_wd_ctl = PCF2131_REG_WD_CTL,
@@ -989,6 +993,28 @@ static int pcf2127_enable_ts(struct device *dev, int ts_id)
return ret;
}

+/* Route all interrupt sources to INT A pin. */
+static int pcf2127_configure_interrupt_pins(struct device *dev)
+{
+ struct pcf2127 *pcf2127 = dev_get_drvdata(dev);
+ int ret;
+
+ /* Mask bits need to be cleared to enable corresponding
+ * interrupt source.
+ */
+ ret = regmap_write(pcf2127->regmap,
+ PCF2131_REG_INT_A_MASK1, 0);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(pcf2127->regmap,
+ PCF2131_REG_INT_A_MASK2, 0);
+ if (ret)
+ return ret;
+
+ return ret;
+}
+
static int pcf2127_probe(struct device *dev, struct regmap *regmap,
int alarm_irq, const char *name, const struct pcf21xx_config *config)
{
@@ -1047,6 +1073,15 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap,
set_bit(RTC_FEATURE_ALARM, pcf2127->rtc->features);
}

+ if (pcf2127->cfg->has_int_a_b) {
+ /* Configure int A/B pins, independently of alarm_irq. */
+ ret = pcf2127_configure_interrupt_pins(dev);
+ if (ret) {
+ dev_err(dev, "failed to configure interrupt pins\n");
+ return ret;
+ }
+ }
+
if (pcf2127->cfg->has_nvmem) {
struct nvmem_config nvmem_cfg = {
.priv = pcf2127,
--
2.30.2


2023-06-22 15:20:43

by Hugo Villeneuve

[permalink] [raw]
Subject: [PATCH v4 13/17] rtc: pcf2127: adapt time/date registers write sequence for PCF2131

From: Hugo Villeneuve <[email protected]>

The sequence for updating the time/date registers is slightly
different between PCF2127/29 and PCF2131.

For PCF2127/29, during write operations, the time counting
circuits (memory locations 03h through 09h) are automatically blocked.

For PCF2131, time/date registers write access requires setting the
STOP bit and sending the clear prescaler instruction (CPR). STOP then
needs to be released once write operation is completed.

Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/rtc/rtc-pcf2127.c | 35 ++++++++++++++++++++++++++++++++++-
1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index 2eef65232417..3b8718aaadd7 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -33,6 +33,7 @@
#define PCF2127_REG_CTRL1 0x00
#define PCF2127_BIT_CTRL1_POR_OVRD BIT(3)
#define PCF2127_BIT_CTRL1_TSF1 BIT(4)
+#define PCF2127_BIT_CTRL1_STOP BIT(5)
/* Control register 2 */
#define PCF2127_REG_CTRL2 0x01
#define PCF2127_BIT_CTRL2_AIE BIT(1)
@@ -280,13 +281,45 @@ static int pcf2127_rtc_set_time(struct device *dev, struct rtc_time *tm)
/* year */
buf[i++] = bin2bcd(tm->tm_year - 100);

- /* write register's data */
+ /* Write access to time registers:
+ * PCF2127/29: no special action required.
+ * PCF2131: requires setting the STOP and CPR bits. STOP bit needs to
+ * be cleared after time registers are updated.
+ */
+ if (pcf2127->cfg->type == PCF2131) {
+ err = regmap_update_bits(pcf2127->regmap, PCF2127_REG_CTRL1,
+ PCF2127_BIT_CTRL1_STOP,
+ PCF2127_BIT_CTRL1_STOP);
+ if (err) {
+ dev_dbg(dev, "setting STOP bit failed\n");
+ return err;
+ }
+
+ err = regmap_write(pcf2127->regmap, PCF2131_REG_SR_RESET,
+ PCF2131_SR_RESET_CPR_CMD);
+ if (err) {
+ dev_dbg(dev, "sending CPR cmd failed\n");
+ return err;
+ }
+ }
+
+ /* write time register's data */
err = regmap_bulk_write(pcf2127->regmap, pcf2127->cfg->reg_time_base, buf, i);
if (err) {
dev_dbg(dev, "%s: err=%d", __func__, err);
return err;
}

+ if (pcf2127->cfg->type == PCF2131) {
+ /* Clear STOP bit (PCF2131 only) after write is completed. */
+ err = regmap_update_bits(pcf2127->regmap, PCF2127_REG_CTRL1,
+ PCF2127_BIT_CTRL1_STOP, 0);
+ if (err) {
+ dev_dbg(dev, "clearing STOP bit failed\n");
+ return err;
+ }
+ }
+
return 0;
}

--
2.30.2


2023-06-22 15:22:33

by Hugo Villeneuve

[permalink] [raw]
Subject: [PATCH v4 16/17] rtc: pcf2127: add UIE support for PCF2131

From: Hugo Villeneuve <[email protected]>

The PCF2127/29 do NOT support alarms with a 1 second resolution, but
the PCF2131 does.

Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/rtc/rtc-pcf2127.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index 9da3f0096d6b..0ff589789aea 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -1128,8 +1128,16 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap,
pcf2127->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
pcf2127->rtc->range_max = RTC_TIMESTAMP_END_2099;
pcf2127->rtc->set_start_time = true; /* Sets actual start to 1970 */
- set_bit(RTC_FEATURE_ALARM_RES_2S, pcf2127->rtc->features);
- clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, pcf2127->rtc->features);
+
+ /*
+ * PCF2127/29 do not work correctly when setting alarms at 1s intervals.
+ * PCF2131 is ok.
+ */
+ if (pcf2127->cfg->type == PCF2127 || pcf2127->cfg->type == PCF2129) {
+ set_bit(RTC_FEATURE_ALARM_RES_2S, pcf2127->rtc->features);
+ clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, pcf2127->rtc->features);
+ }
+
clear_bit(RTC_FEATURE_ALARM, pcf2127->rtc->features);

if (alarm_irq > 0) {
--
2.30.2


2023-06-22 15:26:09

by Hugo Villeneuve

[permalink] [raw]
Subject: [PATCH v4 15/17] rtc: pcf2127: add flag for watchdog register value read support

From: Hugo Villeneuve <[email protected]>

The watchdog value register cannot be read on the PCF2131 after being
set.

Add a new flag to identify which variant has read access to this
register, and use this flag to selectively test if watchdog timer was
started by bootloader.

Signed-off-by: Hugo Villeneuve <[email protected]>
Reviewed-by: Bruno Thomsen <[email protected]>
---
drivers/rtc/rtc-pcf2127.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index c20eab277385..9da3f0096d6b 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -185,6 +185,7 @@ struct pcf21xx_config {
int max_register;
unsigned int has_nvmem:1;
unsigned int has_bit_wd_ctl_cd0:1;
+ unsigned int wd_val_reg_readable:1; /* If watchdog value register can be read. */
unsigned int has_int_a_b:1; /* PCF2131 supports two interrupt outputs. */
u8 reg_time_base; /* Time/date base register. */
u8 regs_alarm_base; /* Alarm function base registers. */
@@ -486,7 +487,6 @@ static int pcf2127_watchdog_get_period(int n, int f1000)

static int pcf2127_watchdog_init(struct device *dev, struct pcf2127 *pcf2127)
{
- u32 wdd_timeout;
int ret;

if (!IS_ENABLED(CONFIG_WATCHDOG) ||
@@ -514,12 +514,17 @@ static int pcf2127_watchdog_init(struct device *dev, struct pcf2127 *pcf2127)
watchdog_set_drvdata(&pcf2127->wdd, pcf2127);

/* Test if watchdog timer is started by bootloader */
- ret = regmap_read(pcf2127->regmap, pcf2127->cfg->reg_wd_val, &wdd_timeout);
- if (ret)
- return ret;
+ if (pcf2127->cfg->wd_val_reg_readable) {
+ u32 wdd_timeout;
+
+ ret = regmap_read(pcf2127->regmap, pcf2127->cfg->reg_wd_val,
+ &wdd_timeout);
+ if (ret)
+ return ret;

- if (wdd_timeout)
- set_bit(WDOG_HW_RUNNING, &pcf2127->wdd.status);
+ if (wdd_timeout)
+ set_bit(WDOG_HW_RUNNING, &pcf2127->wdd.status);
+ }

return devm_watchdog_register_device(dev, &pcf2127->wdd);
}
@@ -919,6 +924,7 @@ static struct pcf21xx_config pcf21xx_cfg[] = {
.max_register = 0x1d,
.has_nvmem = 1,
.has_bit_wd_ctl_cd0 = 1,
+ .wd_val_reg_readable = 1,
.has_int_a_b = 0,
.reg_time_base = PCF2127_REG_TIME_BASE,
.regs_alarm_base = PCF2127_REG_ALARM_BASE,
@@ -946,6 +952,7 @@ static struct pcf21xx_config pcf21xx_cfg[] = {
.max_register = 0x19,
.has_nvmem = 0,
.has_bit_wd_ctl_cd0 = 0,
+ .wd_val_reg_readable = 1,
.has_int_a_b = 0,
.reg_time_base = PCF2127_REG_TIME_BASE,
.regs_alarm_base = PCF2127_REG_ALARM_BASE,
@@ -973,6 +980,7 @@ static struct pcf21xx_config pcf21xx_cfg[] = {
.max_register = 0x36,
.has_nvmem = 0,
.has_bit_wd_ctl_cd0 = 0,
+ .wd_val_reg_readable = 0,
.has_int_a_b = 1,
.reg_time_base = PCF2131_REG_TIME_BASE,
.regs_alarm_base = PCF2131_REG_ALARM_BASE,
--
2.30.2


2023-06-22 15:26:12

by Hugo Villeneuve

[permalink] [raw]
Subject: [PATCH v4 01/17] rtc: pcf2127: improve rtc_read_time() performance

From: Hugo Villeneuve <[email protected]>

Improve performance and readability of rtc_read_time() by reading only
the 7 time registers, instead of reading 8 registers (additional CTRL3
register).

We drop reading of CTRL3 to monitor the low battery flag, as this
check is already available in the ioctl. Anyway, this check only
display an info message and has no other impacts.

The code readability also improves as we do not have to fiddle with
buffer pointer and size arithmetic.

Signed-off-by: Hugo Villeneuve <[email protected]>
---
drivers/rtc/rtc-pcf2127.c | 42 +++++++++++++--------------------------
1 file changed, 14 insertions(+), 28 deletions(-)

diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index 87f4fc9df68b..475a627b2254 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -45,12 +45,6 @@
/* 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
/* Alarm registers */
#define PCF2127_REG_ALARM_SC 0x0A
#define PCF2127_REG_ALARM_MN 0x0B
@@ -117,27 +111,22 @@ struct pcf2127 {
static int pcf2127_rtc_read_time(struct device *dev, struct rtc_time *tm)
{
struct pcf2127 *pcf2127 = dev_get_drvdata(dev);
- unsigned char buf[10];
+ unsigned char buf[7];
int ret;

/*
* Avoid reading CTRL2 register as it causes WD_VAL register
* value to reset to 0 which means watchdog is stopped.
*/
- ret = regmap_bulk_read(pcf2127->regmap, PCF2127_REG_CTRL3,
- (buf + PCF2127_REG_CTRL3),
- ARRAY_SIZE(buf) - PCF2127_REG_CTRL3);
+ ret = regmap_bulk_read(pcf2127->regmap, PCF2127_REG_SC, buf,
+ sizeof(buf));
if (ret) {
dev_err(dev, "%s: read error\n", __func__);
return ret;
}

- if (buf[PCF2127_REG_CTRL3] & PCF2127_BIT_CTRL3_BLF)
- dev_info(dev,
- "low voltage detected, check/replace RTC battery.\n");
-
/* Clock integrity is not guaranteed when OSF flag is set. */
- if (buf[PCF2127_REG_SC] & PCF2127_BIT_SC_OSF) {
+ if (buf[0] & PCF2127_BIT_SC_OSF) {
/*
* no need clear the flag here,
* it will be cleared once the new date is saved
@@ -148,20 +137,17 @@ static int pcf2127_rtc_read_time(struct device *dev, struct rtc_time *tm)
}

dev_dbg(dev,
- "%s: raw data is cr3=%02x, sec=%02x, min=%02x, hr=%02x, "
+ "%s: raw data is sec=%02x, min=%02x, hr=%02x, "
"mday=%02x, wday=%02x, mon=%02x, year=%02x\n",
- __func__, buf[PCF2127_REG_CTRL3], buf[PCF2127_REG_SC],
- buf[PCF2127_REG_MN], buf[PCF2127_REG_HR],
- buf[PCF2127_REG_DM], buf[PCF2127_REG_DW],
- buf[PCF2127_REG_MO], buf[PCF2127_REG_YR]);
-
- tm->tm_sec = bcd2bin(buf[PCF2127_REG_SC] & 0x7F);
- tm->tm_min = bcd2bin(buf[PCF2127_REG_MN] & 0x7F);
- tm->tm_hour = bcd2bin(buf[PCF2127_REG_HR] & 0x3F); /* rtc hr 0-23 */
- tm->tm_mday = bcd2bin(buf[PCF2127_REG_DM] & 0x3F);
- tm->tm_wday = buf[PCF2127_REG_DW] & 0x07;
- tm->tm_mon = bcd2bin(buf[PCF2127_REG_MO] & 0x1F) - 1; /* rtc mn 1-12 */
- tm->tm_year = bcd2bin(buf[PCF2127_REG_YR]);
+ __func__, buf[0], buf[1], buf[2], buf[3], buf[4], buf[5], buf[6]);
+
+ tm->tm_sec = bcd2bin(buf[0] & 0x7F);
+ tm->tm_min = bcd2bin(buf[1] & 0x7F);
+ tm->tm_hour = bcd2bin(buf[2] & 0x3F); /* rtc hr 0-23 */
+ tm->tm_mday = bcd2bin(buf[3] & 0x3F);
+ tm->tm_wday = buf[4] & 0x07;
+ tm->tm_mon = bcd2bin(buf[5] & 0x1F) - 1; /* rtc mn 1-12 */
+ tm->tm_year = bcd2bin(buf[6]);
tm->tm_year += 100;

dev_dbg(dev, "%s: tm is secs=%d, mins=%d, hours=%d, "
--
2.30.2


2023-07-27 21:27:29

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH v4 00/17] rtc: pcf2127: add PCF2131 driver


On Thu, 22 Jun 2023 10:57:43 -0400, Hugo Villeneuve wrote:
> From: Hugo Villeneuve <[email protected]>
>
> Hello,
> this patch series adds the driver for the PCF2131 real-time clock.
>
> This RTC is very similar in functionality to the PCF2127/29 with the
> following differences:
> -supports two new control registers at offsets 4 and 5
> -supports a new reset register
> -supports 4 tamper detection functions instead of 1
> -has no nvmem (like the PCF2129)
> -has two output interrupt pins instead of one
> -has 1/100th seconds capabilities (not supported in this driver)
> -pcf2127 has watchdog clock sources: 1/60, 1, 64 and 4096Hz
> pcf2131 has watchdog clock sources: 1/64, 1/4, 4 and 64Hz
> -watchdog value register cannot be read after being set
>
> [...]

Applied, thanks!

[01/17] rtc: pcf2127: improve rtc_read_time() performance
commit: 31f077c374a8e7cf1960560c0c5e4065048557c0
[02/17] rtc: pcf2127: improve timestamp reading performance
commit: 720fb4b83b565c7ae31059620e960ecbf5dc73a3
[03/17] rtc: pcf2127: lower message severity if setting time fails
commit: 3d740c647ff8b77b2a560ebd95ac746c46f49ed4
[04/17] rtc: pcf2127: remove superfluous comments
commit: 0476b6c8e8b1a6dfa3a259bc7e3c135145532c71
[05/17] rtc: pcf2127: add variant-specific configuration structure
commit: fd28ceb4603f9541dcb4ed12b1365cff5af38203
[06/17] rtc: pcf2127: adapt for time/date registers at any offset
commit: 6211acee8edf4af61f7745a92c4b4cb05a4340f9
[07/17] rtc: pcf2127: adapt for alarm registers at any offset
commit: 7c6f0db41ab5fbd7ee3a2f9880ac23509a5d55d1
[08/17] rtc: pcf2127: adapt for WD registers at any offset
commit: 6b57ec29e3fc31d43e672f6fede5d4a76140308b
[09/17] rtc: pcf2127: adapt for CLKOUT register at any offset
commit: fc16599e0153e91ba12d856e40f6fc56906077f1
[10/17] rtc: pcf2127: add support for multiple TS functions
commit: 420cc9e850dbc8e6ea7dd1e53d62d64cd8766354
[11/17] rtc: pcf2127: add support for PCF2131 RTC
commit: afc505bf9039caf5a377d8b9705ef42f6d4ac7d4
[12/17] rtc: pcf2127: add support for PCF2131 interrupts on output INT_A
commit: e1849b8fcdfaa71f2e8f9376c9568877ff2bf52b
[13/17] rtc: pcf2127: adapt time/date registers write sequence for PCF2131
commit: 3d715ebaf006bd5a495e9a717cf0fc5c260ee738
[14/17] rtc: pcf2127: support generic watchdog timing configuration
commit: adb9675d74e403537150f025ed2b7a2e1ed0a7b4
[15/17] rtc: pcf2127: add flag for watchdog register value read support
commit: 081602a1d85b1fd7ad9ea298cffb4e5ad5296952
[16/17] rtc: pcf2127: add UIE support for PCF2131
commit: e9a5a1b418dd9a82d1de71e82ec64ad195b5300a
[17/17] dt-bindings: rtc: pcf2127: add PCF2131
commit: 2080e08460c41c6d432575132868fdf076768c92

Best regards,

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

2023-07-28 17:34:53

by Hugo Villeneuve

[permalink] [raw]
Subject: Re: [PATCH v4 00/17] rtc: pcf2127: add PCF2131 driver

On Thu, 27 Jul 2023 22:57:41 +0200
Alexandre Belloni <[email protected]> wrote:

>
> On Thu, 22 Jun 2023 10:57:43 -0400, Hugo Villeneuve wrote:
> > From: Hugo Villeneuve <[email protected]>
> >
> > Hello,
> > this patch series adds the driver for the PCF2131 real-time clock.
> >
> > This RTC is very similar in functionality to the PCF2127/29 with the
> > following differences:
> > -supports two new control registers at offsets 4 and 5
> > -supports a new reset register
> > -supports 4 tamper detection functions instead of 1
> > -has no nvmem (like the PCF2129)
> > -has two output interrupt pins instead of one
> > -has 1/100th seconds capabilities (not supported in this driver)
> > -pcf2127 has watchdog clock sources: 1/60, 1, 64 and 4096Hz
> > pcf2131 has watchdog clock sources: 1/64, 1/4, 4 and 64Hz
> > -watchdog value register cannot be read after being set
> >
> > [...]
>
> Applied, thanks!

Thank you :)
Hugo.


> [01/17] rtc: pcf2127: improve rtc_read_time() performance
> commit: 31f077c374a8e7cf1960560c0c5e4065048557c0
> [02/17] rtc: pcf2127: improve timestamp reading performance
> commit: 720fb4b83b565c7ae31059620e960ecbf5dc73a3
> [03/17] rtc: pcf2127: lower message severity if setting time fails
> commit: 3d740c647ff8b77b2a560ebd95ac746c46f49ed4
> [04/17] rtc: pcf2127: remove superfluous comments
> commit: 0476b6c8e8b1a6dfa3a259bc7e3c135145532c71
> [05/17] rtc: pcf2127: add variant-specific configuration structure
> commit: fd28ceb4603f9541dcb4ed12b1365cff5af38203
> [06/17] rtc: pcf2127: adapt for time/date registers at any offset
> commit: 6211acee8edf4af61f7745a92c4b4cb05a4340f9
> [07/17] rtc: pcf2127: adapt for alarm registers at any offset
> commit: 7c6f0db41ab5fbd7ee3a2f9880ac23509a5d55d1
> [08/17] rtc: pcf2127: adapt for WD registers at any offset
> commit: 6b57ec29e3fc31d43e672f6fede5d4a76140308b
> [09/17] rtc: pcf2127: adapt for CLKOUT register at any offset
> commit: fc16599e0153e91ba12d856e40f6fc56906077f1
> [10/17] rtc: pcf2127: add support for multiple TS functions
> commit: 420cc9e850dbc8e6ea7dd1e53d62d64cd8766354
> [11/17] rtc: pcf2127: add support for PCF2131 RTC
> commit: afc505bf9039caf5a377d8b9705ef42f6d4ac7d4
> [12/17] rtc: pcf2127: add support for PCF2131 interrupts on output INT_A
> commit: e1849b8fcdfaa71f2e8f9376c9568877ff2bf52b
> [13/17] rtc: pcf2127: adapt time/date registers write sequence for PCF2131
> commit: 3d715ebaf006bd5a495e9a717cf0fc5c260ee738
> [14/17] rtc: pcf2127: support generic watchdog timing configuration
> commit: adb9675d74e403537150f025ed2b7a2e1ed0a7b4
> [15/17] rtc: pcf2127: add flag for watchdog register value read support
> commit: 081602a1d85b1fd7ad9ea298cffb4e5ad5296952
> [16/17] rtc: pcf2127: add UIE support for PCF2131
> commit: e9a5a1b418dd9a82d1de71e82ec64ad195b5300a
> [17/17] dt-bindings: rtc: pcf2127: add PCF2131
> commit: 2080e08460c41c6d432575132868fdf076768c92
>
> Best regards,
>
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>