linux-rtc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] rtc: pcf2127: add alarm support
@ 2020-06-19  4:11 Liam Beguin
  2020-06-19  4:11 ` [PATCH v3 1/2] rtc: pcf2127: add pca2129 device id Liam Beguin
  2020-06-19  4:11 ` [PATCH v3 2/2] rtc: pcf2127: add alarm support Liam Beguin
  0 siblings, 2 replies; 6+ messages in thread
From: Liam Beguin @ 2020-06-19  4:11 UTC (permalink / raw)
  To: bruno.thomsen, a.zummo, alexandre.belloni; +Cc: linux-rtc

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

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                     | 139 ++++++++++++++++++
 2 files changed, 141 insertions(+)

Interdiff against v2:
diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index 22ef1489c1b1..df09d3c6c5c3 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -88,8 +88,6 @@
 #define PCF2127_WD_VAL_MAX		255
 #define PCF2127_WD_VAL_DEFAULT		60
 
-static int pcf2127_wdt_active_ping(struct watchdog_device *wdd);
-
 struct pcf2127 {
 	struct rtc_device *rtc;
 	struct watchdog_device wdd;
@@ -195,140 +193,6 @@ static int pcf2127_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	return 0;
 }
 
-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) {
-		dev_err(dev, "%s: ctrl2 read error\n", __func__);
-		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) {
-		dev_err(dev, "%s: alarm read error\n", __func__);
-		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;
-
-	dev_dbg(dev, "%s: alarm is %d:%d:%d, mday=%d, wday=%d\n", __func__,
-		alrm->time.tm_hour, alrm->time.tm_min, alrm->time.tm_sec,
-		alrm->time.tm_mday, alrm->time.tm_wday);
-
-	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) {
-		dev_err(dev, "%s: failed to %s alarm (%d)\n", __func__,
-			enable ? "enable" : "disable",
-			ret);
-		return ret;
-	}
-
-	ret = pcf2127_wdt_active_ping(&pcf2127->wdd);
-	if (ret)
-		return ret;
-
-	return 0;
-}
-
-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) {
-		dev_err(dev, "%s: failed to clear alarm interrupt flag (%d)",
-			__func__, 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);
-
-	dev_dbg(dev, "%s: alarm set for: %d:%d:%d, mday=%d, wday=%d\n",
-		__func__, alrm->time.tm_hour, alrm->time.tm_min,
-		alrm->time.tm_sec, alrm->time.tm_mday, alrm->time.tm_wday);
-
-	ret = regmap_bulk_write(pcf2127->regmap, PCF2127_REG_ALARM_SC, buf,
-				sizeof(buf));
-	if (ret) {
-		dev_err(dev, "%s: failed to write alarm registers (%d)",
-			__func__, ret);
-		return ret;
-	}
-
-	pcf2127_rtc_alarm_irq_enable(dev, alrm->enabled);
-
-	return 0;
-}
-
-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) {
-		dev_err(dev, "%s: ctrl2 read error (%d)\n", __func__, ret);
-		goto irq_err;
-	}
-
-	ret = pcf2127_wdt_active_ping(&pcf2127->wdd);
-	if (ret)
-		goto irq_err;
-
-	if (ctrl2 & PCF2127_BIT_CTRL2_AF) {
-		regmap_update_bits(pcf2127->regmap, PCF2127_REG_CTRL2,
-				   PCF2127_BIT_CTRL2_AF, 0);
-
-		ret = pcf2127_wdt_active_ping(&pcf2127->wdd);
-		if (ret)
-			goto irq_err;
-
-		rtc_update_irq(pcf2127->rtc, 1, RTC_IRQF | RTC_AF);
-	}
-
-	return IRQ_HANDLED;
-irq_err:
-	return IRQ_NONE;
-}
-
 static int pcf2127_rtc_ioctl(struct device *dev,
 				unsigned int cmd, unsigned long arg)
 {
@@ -360,12 +224,9 @@ static int pcf2127_rtc_ioctl(struct device *dev,
 }
 
 static const struct rtc_class_ops pcf2127_rtc_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,
+	.ioctl		= pcf2127_rtc_ioctl,
+	.read_time	= pcf2127_rtc_read_time,
+	.set_time	= pcf2127_rtc_set_time,
 };
 
 static int pcf2127_nvmem_read(void *priv, unsigned int offset,
@@ -472,6 +333,116 @@ 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)
+		goto irq_err;
+
+	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)
+		goto irq_err;
+
+	return IRQ_HANDLED;
+irq_err:
+	return IRQ_NONE;
+}
+
+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,
@@ -601,8 +572,10 @@ static int pcf2127_probe(struct device *dev, struct regmap *regmap,
 		}
 	}
 
-	if (alarm_irq >= 0 || device_property_read_bool(dev, "wakeup-source"))
+	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;

base-commit: 5e857ce6eae7ca21b2055cca4885545e29228fe2
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 1/2] rtc: pcf2127: add pca2129 device id
  2020-06-19  4:11 [PATCH v3 0/2] rtc: pcf2127: add alarm support Liam Beguin
@ 2020-06-19  4:11 ` Liam Beguin
  2020-06-27 12:23   ` Bruno Thomsen
  2020-06-19  4:11 ` [PATCH v3 2/2] rtc: pcf2127: add alarm support Liam Beguin
  1 sibling, 1 reply; 6+ messages in thread
From: Liam Beguin @ 2020-06-19  4:11 UTC (permalink / raw)
  To: bruno.thomsen, a.zummo, alexandre.belloni; +Cc: linux-rtc

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>
---
Changes since v1:
- Document new compatible string for the pca2129

Changes since v2:
- None

 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 2/2] rtc: pcf2127: add alarm support
  2020-06-19  4:11 [PATCH v3 0/2] rtc: pcf2127: add alarm support Liam Beguin
  2020-06-19  4:11 ` [PATCH v3 1/2] rtc: pcf2127: add pca2129 device id Liam Beguin
@ 2020-06-19  4:11 ` Liam Beguin
  2020-06-27 12:32   ` Bruno Thomsen
  1 sibling, 1 reply; 6+ messages in thread
From: Liam Beguin @ 2020-06-19  4:11 UTC (permalink / raw)
  To: bruno.thomsen, a.zummo, alexandre.belloni; +Cc: linux-rtc

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>
---
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

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

diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index 4accee09bfad..df09d3c6c5c3 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,116 @@ 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)
+		goto irq_err;
+
+	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)
+		goto irq_err;
+
+	return IRQ_HANDLED;
+irq_err:
+	return IRQ_NONE;
+}
+
+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 +538,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 +561,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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/2] rtc: pcf2127: add pca2129 device id
  2020-06-19  4:11 ` [PATCH v3 1/2] rtc: pcf2127: add pca2129 device id Liam Beguin
@ 2020-06-27 12:23   ` Bruno Thomsen
  0 siblings, 0 replies; 6+ messages in thread
From: Bruno Thomsen @ 2020-06-27 12:23 UTC (permalink / raw)
  To: Liam Beguin; +Cc: Alessandro Zummo, Alexandre Belloni, linux-rtc

Den fre. 19. jun. 2020 kl. 06.11 skrev Liam Beguin <liambeguin@gmail.com>:
>
> 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
>
>  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
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 2/2] rtc: pcf2127: add alarm support
  2020-06-19  4:11 ` [PATCH v3 2/2] rtc: pcf2127: add alarm support Liam Beguin
@ 2020-06-27 12:32   ` Bruno Thomsen
  2020-06-30  2:15     ` Liam Beguin
  0 siblings, 1 reply; 6+ messages in thread
From: Bruno Thomsen @ 2020-06-27 12:32 UTC (permalink / raw)
  To: Liam Beguin; +Cc: Alessandro Zummo, Alexandre Belloni, linux-rtc

Den fre. 19. jun. 2020 kl. 06.12 skrev Liam Beguin <liambeguin@gmail.com>:
>
> 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>
> ---
> 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
>
>  drivers/rtc/rtc-pcf2127.c | 136 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 136 insertions(+)
>
> diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
> index 4accee09bfad..df09d3c6c5c3 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,116 @@ 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)
> +               goto irq_err;
> +
> +       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)
> +               goto irq_err;
> +
> +       return IRQ_HANDLED;
> +irq_err:
> +       return IRQ_NONE;

It's not really needed to use "goto irq_err;" syntax as there is no
common error cleanup
path and can simply be replaced by "return IRQ_NONE;".

Overall this feature looks good to me.

Reviewed-by: Bruno Thomsen <bruno.thomsen@gmail.com>

/Bruno

> +}
> +
> +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 +538,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 +561,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
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 2/2] rtc: pcf2127: add alarm support
  2020-06-27 12:32   ` Bruno Thomsen
@ 2020-06-30  2:15     ` Liam Beguin
  0 siblings, 0 replies; 6+ messages in thread
From: Liam Beguin @ 2020-06-30  2:15 UTC (permalink / raw)
  To: Bruno Thomsen; +Cc: Alessandro Zummo, Alexandre Belloni, linux-rtc

On Sat Jun 27, 2020 at 2:32 PM Bruno Thomsen wrote:
> Den fre. 19. jun. 2020 kl. 06.12 skrev Liam Beguin <liambeguin@gmail.com>:
> >
> > 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>
> > ---
> > 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
> >
> >  drivers/rtc/rtc-pcf2127.c | 136 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 136 insertions(+)
> >
> > diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
> > index 4accee09bfad..df09d3c6c5c3 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,116 @@ 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)
> > +               goto irq_err;
> > +
> > +       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)
> > +               goto irq_err;
> > +
> > +       return IRQ_HANDLED;
> > +irq_err:
> > +       return IRQ_NONE;

Hi Bruno,

> 
> It's not really needed to use "goto irq_err;" syntax as there is no
> common error cleanup
> path and can simply be replaced by "return IRQ_NONE;".
> 
> Overall this feature looks good to me.
> 
> Reviewed-by: Bruno Thomsen <bruno.thomsen@gmail.com>
> 
> /Bruno
> 

I'll send another revision getting rid of the goto. I'll also add your
Reviewed-by on both patches.

Thanks again for reviewing this,
Liam

> > +}
> > +
> > +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 +538,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 +561,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
> >

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-06-30  2:15 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-19  4:11 [PATCH v3 0/2] rtc: pcf2127: add alarm support Liam Beguin
2020-06-19  4:11 ` [PATCH v3 1/2] rtc: pcf2127: add pca2129 device id Liam Beguin
2020-06-27 12:23   ` Bruno Thomsen
2020-06-19  4:11 ` [PATCH v3 2/2] rtc: pcf2127: add alarm support Liam Beguin
2020-06-27 12:32   ` Bruno Thomsen
2020-06-30  2:15     ` Liam Beguin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).