[PATCH v4 0/2] rtc: pcf2127: add alarm support

[PATCH v4 0/2] rtc: pcf2127: add alarm support

[Liam Beguin]

From: Liam Beguin <lvb@xiphos.com>

The board used to test this series has the interrupt line of the RTC
connected to a circuit controlling the power of the board.
An event on the interrupt line while the board is off will power it on.
Because of these hardware limitations, the irq handler added in this
patch wasn't fully tested.

The alarm fuctionality was tested on a PCA2129, with:

	$ date "2010-10-10 10:10"
	Sun Oct 10 10:10:00 UTC 2010
	$ /usr/sbin/rtcwake -u -d /dev/rtc0  -s10 --mode off
	[ ... ]
	$ # power on after 10 seconds

Changes since v1:
- Document new compatible string for the pca2129
- Add calls to pcf2127_wdt_active_ping after accessing CTRL2
- Use sizeof(buf) instead of hadcoding value
- Cleanup debug trace
- Add interrupt handler and wakeup-source devicetree option

Changes since v2:
- Rebase on latest mainline tree
- Remove redundant if in pcf2127_rtc_alarm_irq_enable
- Remove duplicate watchdog ping in pcf2127_rtc_irq
- Avoid forward declaration
- Remove dev_err strings
- Remove dev_dbg traces since they are now part of the core
- Avoid unnecessary read in pcf2127_rtc_irq with regmap_write
- Add extra rtc_class_ops struct with alarm functions

Changes since v3:
- Replace "goto irq_err" with "return IRQ_NONE" in interrupt handler
- Add Reviewed-by trailers

Liam Beguin (2):
  rtc: pcf2127: add pca2129 device id
  rtc: pcf2127: add alarm support

 .../devicetree/bindings/rtc/trivial-rtc.yaml  |   2 +
 drivers/rtc/rtc-pcf2127.c                     | 137 ++++++++++++++++++
 2 files changed, 139 insertions(+)

Interdiff against v3:
diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index df09d3c6c5c3..4e99c45a87d7 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -416,7 +416,7 @@ static irqreturn_t pcf2127_rtc_irq(int irq, void *dev)
 
 	ret = regmap_read(pcf2127->regmap, PCF2127_REG_CTRL2, &ctrl2);
 	if (ret)
-		goto irq_err;
+		return IRQ_NONE;
 
 	if (ctrl2 & PCF2127_BIT_CTRL2_AF) {
 		regmap_write(pcf2127->regmap, PCF2127_REG_CTRL2,
@@ -427,11 +427,9 @@ static irqreturn_t pcf2127_rtc_irq(int irq, void *dev)
 
 	ret = pcf2127_wdt_active_ping(&pcf2127->wdd);
 	if (ret)
-		goto irq_err;
+		return IRQ_NONE;
 
 	return IRQ_HANDLED;
-irq_err:
-	return IRQ_NONE;
 }
 
 static const struct rtc_class_ops pcf2127_rtc_alrm_ops = {

base-commit: 7c30b859a947535f2213277e827d7ac7dcff9c84
-- 
2.27.0

[PATCH v4 1/2] rtc: pcf2127: add pca2129 device id

[Liam Beguin]

From: Liam Beguin <lvb@xiphos.com>

The PCA2129 is the automotive grade version of the PCF2129.
add it to the list of compatibles.

Signed-off-by: Liam Beguin <lvb@xiphos.com>
Reviewed-by: Bruno Thomsen <bruno.thomsen@gmail.com>
---
Changes since v1:
- Document new compatible string for the pca2129

Changes since v2:
- None

Change since v3:
- Add Reviewed-by trailer

 Documentation/devicetree/bindings/rtc/trivial-rtc.yaml | 2 ++
 drivers/rtc/rtc-pcf2127.c                              | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml b/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml
index 18cb456752f6..c7d14de214c4 100644
--- a/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml
+++ b/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml
@@ -52,6 +52,8 @@ properties:
       - nxp,pcf2127
       # Real-time clock
       - nxp,pcf2129
+      # Real-time clock
+      - nxp,pca2129
       # Real-time Clock Module
       - pericom,pt7c4338
       # I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index 9c5670776c68..4accee09bfad 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -553,6 +553,7 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap,
 static const struct of_device_id pcf2127_of_match[] = {
 	{ .compatible = "nxp,pcf2127" },
 	{ .compatible = "nxp,pcf2129" },
+	{ .compatible = "nxp,pca2129" },
 	{}
 };
 MODULE_DEVICE_TABLE(of, pcf2127_of_match);
@@ -664,6 +665,7 @@ static int pcf2127_i2c_probe(struct i2c_client *client,
 static const struct i2c_device_id pcf2127_i2c_id[] = {
 	{ "pcf2127", 1 },
 	{ "pcf2129", 0 },
+	{ "pca2129", 0 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, pcf2127_i2c_id);
@@ -729,6 +731,7 @@ static int pcf2127_spi_probe(struct spi_device *spi)
 static const struct spi_device_id pcf2127_spi_id[] = {
 	{ "pcf2127", 1 },
 	{ "pcf2129", 0 },
+	{ "pca2129", 0 },
 	{ }
 };
 MODULE_DEVICE_TABLE(spi, pcf2127_spi_id);
-- 
2.27.0

[PATCH v4 2/2] rtc: pcf2127: add alarm support

[Liam Beguin]

From: Liam Beguin <lvb@xiphos.com>

Add alarm support for the pcf2127 RTC chip family.
Tested on pca2129.

Signed-off-by: Liam Beguin <lvb@xiphos.com>
Reviewed-by: Bruno Thomsen <bruno.thomsen@gmail.com>
---
Changes since v1:
- Add calls to pcf2127_wdt_active_ping after accessing CTRL2
- Cleanup calls to regmap_{read,write,update_bits}
- Cleanup debug trace
- Add interrupt handler, untested because of hardware limitations
- Add wakeup-source devicetree option

Changes since v2:
- Avoid forward declaration of pcf2127_wdt_active_ping
- Remove dev_err strings
- Remove dev_dbg traces since they are now part of the core
- Remove redundant if in pcf2127_rtc_alarm_irq_enable
- Remove duplicate watchdog ping in pcf2127_rtc_irq
- Avoid unnecessary read in pcf2127_rtc_irq with regmap_write
- Add extra rtc_class_ops struct with alarm functions

Changes since v3:
- Replace "goto irq_err" with "return IRQ_NONE" in interrupt handler
- Add Reviewed-by trailer

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

diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index 4accee09bfad..4e99c45a87d7 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -20,6 +20,7 @@
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_irq.h>
 #include <linux/regmap.h>
 #include <linux/watchdog.h>
 
@@ -28,7 +29,9 @@
 #define PCF2127_BIT_CTRL1_TSF1			BIT(4)
 /* Control register 2 */
 #define PCF2127_REG_CTRL2		0x01
+#define PCF2127_BIT_CTRL2_AIE			BIT(1)
 #define PCF2127_BIT_CTRL2_TSIE			BIT(2)
+#define PCF2127_BIT_CTRL2_AF			BIT(4)
 #define PCF2127_BIT_CTRL2_TSF2			BIT(5)
 /* Control register 3 */
 #define PCF2127_REG_CTRL3		0x02
@@ -46,6 +49,12 @@
 #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
+#define PCF2127_REG_ALARM_HR		0x0C
+#define PCF2127_REG_ALARM_DM		0x0D
+#define PCF2127_REG_ALARM_DW		0x0E
 /* Watchdog registers */
 #define PCF2127_REG_WD_CTL		0x10
 #define PCF2127_BIT_WD_CTL_TF0			BIT(0)
@@ -324,6 +333,114 @@ static const struct watchdog_ops pcf2127_watchdog_ops = {
 	.set_timeout = pcf2127_wdt_set_timeout,
 };
 
+/* Alarm */
+static int pcf2127_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct pcf2127 *pcf2127 = dev_get_drvdata(dev);
+	unsigned int buf[5], ctrl2;
+	int ret;
+
+	ret = regmap_read(pcf2127->regmap, PCF2127_REG_CTRL2, &ctrl2);
+	if (ret)
+		return ret;
+
+	ret = pcf2127_wdt_active_ping(&pcf2127->wdd);
+	if (ret)
+		return ret;
+
+	ret = regmap_bulk_read(pcf2127->regmap, PCF2127_REG_ALARM_SC, buf,
+			       sizeof(buf));
+	if (ret)
+		return ret;
+
+	alrm->enabled = ctrl2 & PCF2127_BIT_CTRL2_AIE;
+	alrm->pending = ctrl2 & PCF2127_BIT_CTRL2_AF;
+
+	alrm->time.tm_sec = bcd2bin(buf[0] & 0x7F);
+	alrm->time.tm_min = bcd2bin(buf[1] & 0x7F);
+	alrm->time.tm_hour = bcd2bin(buf[2] & 0x3F);
+	alrm->time.tm_mday = bcd2bin(buf[3] & 0x3F);
+	alrm->time.tm_wday = buf[4] & 0x07;
+
+	return 0;
+}
+
+static int pcf2127_rtc_alarm_irq_enable(struct device *dev, u32 enable)
+{
+	struct pcf2127 *pcf2127 = dev_get_drvdata(dev);
+	int ret;
+
+	ret = regmap_update_bits(pcf2127->regmap, PCF2127_REG_CTRL2,
+				 PCF2127_BIT_CTRL2_AIE,
+				 enable ? PCF2127_BIT_CTRL2_AIE : 0);
+	if (ret)
+		return ret;
+
+	return pcf2127_wdt_active_ping(&pcf2127->wdd);
+}
+
+static int pcf2127_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct pcf2127 *pcf2127 = dev_get_drvdata(dev);
+	uint8_t buf[5];
+	int ret;
+
+	ret = regmap_update_bits(pcf2127->regmap, PCF2127_REG_CTRL2,
+				 PCF2127_BIT_CTRL2_AF, 0);
+	if (ret)
+		return ret;
+
+	ret = pcf2127_wdt_active_ping(&pcf2127->wdd);
+	if (ret)
+		return ret;
+
+	buf[0] = bin2bcd(alrm->time.tm_sec);
+	buf[1] = bin2bcd(alrm->time.tm_min);
+	buf[2] = bin2bcd(alrm->time.tm_hour);
+	buf[3] = bin2bcd(alrm->time.tm_mday);
+	buf[4] = (alrm->time.tm_wday & 0x07);
+
+	ret = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_ALARM_SC, buf,
+				sizeof(buf));
+	if (ret)
+		return ret;
+
+	return pcf2127_rtc_alarm_irq_enable(dev, alrm->enabled);
+}
+
+static irqreturn_t pcf2127_rtc_irq(int irq, void *dev)
+{
+	struct pcf2127 *pcf2127 = dev_get_drvdata(dev);
+	unsigned int ctrl2 = 0;
+	int ret = 0;
+
+	ret = regmap_read(pcf2127->regmap, PCF2127_REG_CTRL2, &ctrl2);
+	if (ret)
+		return IRQ_NONE;
+
+	if (ctrl2 & PCF2127_BIT_CTRL2_AF) {
+		regmap_write(pcf2127->regmap, PCF2127_REG_CTRL2,
+			     ctrl2 & ~PCF2127_BIT_CTRL2_AF);
+
+		rtc_update_irq(pcf2127->rtc, 1, RTC_IRQF | RTC_AF);
+	}
+
+	ret = pcf2127_wdt_active_ping(&pcf2127->wdd);
+	if (ret)
+		return IRQ_NONE;
+
+	return IRQ_HANDLED;
+}
+
+static const struct rtc_class_ops pcf2127_rtc_alrm_ops = {
+	.ioctl            = pcf2127_rtc_ioctl,
+	.read_time        = pcf2127_rtc_read_time,
+	.set_time         = pcf2127_rtc_set_time,
+	.read_alarm       = pcf2127_rtc_read_alarm,
+	.set_alarm        = pcf2127_rtc_set_alarm,
+	.alarm_irq_enable = pcf2127_rtc_alarm_irq_enable,
+};
+
 /* sysfs interface */
 
 static ssize_t timestamp0_store(struct device *dev,
@@ -419,6 +536,7 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap,
 			const char *name, bool has_nvmem)
 {
 	struct pcf2127 *pcf2127;
+	int alarm_irq = 0;
 	u32 wdd_timeout;
 	int ret = 0;
 
@@ -441,6 +559,22 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap,
 	pcf2127->rtc->range_max = RTC_TIMESTAMP_END_2099;
 	pcf2127->rtc->set_start_time = true; /* Sets actual start to 1970 */
 
+	alarm_irq = of_irq_get_byname(dev->of_node, "alarm");
+	if (alarm_irq >= 0) {
+		ret = devm_request_irq(dev, alarm_irq, pcf2127_rtc_irq,
+				       IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+				       dev_name(dev), dev);
+		if (ret) {
+			dev_err(dev, "failed to request alarm irq\n");
+			return ret;
+		}
+	}
+
+	if (alarm_irq >= 0 || device_property_read_bool(dev, "wakeup-source")) {
+		device_init_wakeup(dev, true);
+		pcf2127->rtc->ops = &pcf2127_rtc_alrm_ops;
+	}
+
 	pcf2127->wdd.parent = dev;
 	pcf2127->wdd.info = &pcf2127_wdt_info;
 	pcf2127->wdd.ops = &pcf2127_watchdog_ops;
-- 
2.27.0

Re: [PATCH v4 0/2] rtc: pcf2127: add alarm support

[Liam Beguin]

On Mon Jun 29, 2020 at 10:42 PM Liam Beguin wrote:
> From: Liam Beguin <lvb@xiphos.com>
> 
> The board used to test this series has the interrupt line of the RTC
> connected to a circuit controlling the power of the board.
> An event on the interrupt line while the board is off will power it on.
> Because of these hardware limitations, the irq handler added in this
> patch wasn't fully tested.
> 
> The alarm fuctionality was tested on a PCA2129, with:
> 
> 	$ date "2010-10-10 10:10"
> 	Sun Oct 10 10:10:00 UTC 2010
> 	$ /usr/sbin/rtcwake -u -d /dev/rtc0  -s10 --mode off
> 	[ ... ]
> 	$ # power on after 10 seconds
> 
> Changes since v1:
> - Document new compatible string for the pca2129
> - Add calls to pcf2127_wdt_active_ping after accessing CTRL2
> - Use sizeof(buf) instead of hadcoding value
> - Cleanup debug trace
> - Add interrupt handler and wakeup-source devicetree option
> 
> Changes since v2:
> - Rebase on latest mainline tree
> - Remove redundant if in pcf2127_rtc_alarm_irq_enable
> - Remove duplicate watchdog ping in pcf2127_rtc_irq
> - Avoid forward declaration
> - Remove dev_err strings
> - Remove dev_dbg traces since they are now part of the core
> - Avoid unnecessary read in pcf2127_rtc_irq with regmap_write
> - Add extra rtc_class_ops struct with alarm functions
> 
> Changes since v3:
> - Replace "goto irq_err" with "return IRQ_NONE" in interrupt handler
> - Add Reviewed-by trailers
> 
> Liam Beguin (2):
>   rtc: pcf2127: add pca2129 device id
>   rtc: pcf2127: add alarm support
> 

Hi Alexandre,

Did you get a change to take a look at this revision?
Thanks,

Liam

>  .../devicetree/bindings/rtc/trivial-rtc.yaml  |   2 +
>  drivers/rtc/rtc-pcf2127.c                     | 137 ++++++++++++++++++
>  2 files changed, 139 insertions(+)
> 
> Interdiff against v3:
> diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
> index df09d3c6c5c3..4e99c45a87d7 100644
> --- a/drivers/rtc/rtc-pcf2127.c
> +++ b/drivers/rtc/rtc-pcf2127.c
> @@ -416,7 +416,7 @@ static irqreturn_t pcf2127_rtc_irq(int irq, void *dev)
>  
>  	ret = regmap_read(pcf2127->regmap, PCF2127_REG_CTRL2, &ctrl2);
>  	if (ret)
> -		goto irq_err;
> +		return IRQ_NONE;
>  
>  	if (ctrl2 & PCF2127_BIT_CTRL2_AF) {
>  		regmap_write(pcf2127->regmap, PCF2127_REG_CTRL2,
> @@ -427,11 +427,9 @@ static irqreturn_t pcf2127_rtc_irq(int irq, void *dev)
>  
>  	ret = pcf2127_wdt_active_ping(&pcf2127->wdd);
>  	if (ret)
> -		goto irq_err;
> +		return IRQ_NONE;
>  
>  	return IRQ_HANDLED;
> -irq_err:
> -	return IRQ_NONE;
>  }
>  
>  static const struct rtc_class_ops pcf2127_rtc_alrm_ops = {
> 
> base-commit: 7c30b859a947535f2213277e827d7ac7dcff9c84
> -- 
> 2.27.0
> 

Re: [PATCH v4 0/2] rtc: pcf2127: add alarm support

[Alexandre Belloni]

On Mon, 29 Jun 2020 22:42:09 -0400, Liam Beguin wrote:
> The board used to test this series has the interrupt line of the RTC
> connected to a circuit controlling the power of the board.
> An event on the interrupt line while the board is off will power it on.
> Because of these hardware limitations, the irq handler added in this
> patch wasn't fully tested.
> 
> The alarm fuctionality was tested on a PCA2129, with:
> 
> [...]

Applied, thanks!

[1/2] rtc: pcf2127: add pca2129 device id
      commit: 985b30dbe2cf58c27dd81da85410439ced8ce6d7
[2/2] rtc: pcf2127: add alarm support
      commit: 8a914bac44be33623cfcf27688b18b6f81a5c7d5

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