[PATCH V2 0/2] Add andes atcrtc100 RTC driver

[PATCH V2 0/2] Add andes atcrtc100 RTC driver

[CL Wang]

CL Wang (2):
  dt-bindings: rtc: atcrtc100: Add atcrtc100 bindings
  rtc: atcrtc100: Add andes atcrtc100 RTC driver

Notes:
    Change from v1 to v2
    - Use devm_xxx to allocate system resources instead of some old functions like ioremap.
    - Use regmap_xxx to access RTC hardware registers instead of ioread32 and iowrite32.
    - Remove the atc_rtc_remove function as it is no longer needed.
    - Update the module author and remove MODULE_ALIAS.

 .../bindings/rtc/andes,atcrtc100-rtc.yaml     |  39 ++
 drivers/rtc/Kconfig                           |  10 +
 drivers/rtc/Makefile                          |   1 +
 drivers/rtc/rtc-atcrtc100.c                   | 457 ++++++++++++++++++
 4 files changed, 507 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/andes,atcrtc100-rtc.yaml
 create mode 100644 drivers/rtc/rtc-atcrtc100.c

-- 
2.34.1

[PATCH V2 1/2] dt-bindings: rtc: atcrtc100: Add atcrtc100 bindings

[CL Wang]

Add YAML bindings for Andes atcrtc100 RTC.

Signed-off-by: CL Wang <cl634@andestech.com>
---
 .../bindings/rtc/andes,atcrtc100-rtc.yaml     | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/andes,atcrtc100-rtc.yaml

diff --git a/Documentation/devicetree/bindings/rtc/andes,atcrtc100-rtc.yaml b/Documentation/devicetree/bindings/rtc/andes,atcrtc100-rtc.yaml
new file mode 100644
index 000000000000..86cecb30c889
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/andes,atcrtc100-rtc.yaml
@@ -0,0 +1,39 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/andes,atcrtc100-rtc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Andes, ATCRTC100 on-SoC RTC
+
+allOf:
+  - $ref: rtc.yaml#
+
+maintainers:
+  - CL Wang <cl634@andestech.com>
+
+properties:
+  compatible:
+    enum:
+      - andestech,atcrtc100
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    minItems: 2
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    rtc@f0300000 {
+        compatible = "andestech,atcrtc100";
+        reg = <0xf0300000 0x100>;
+        interrupts = <1>, <2>;
+    };
-- 
2.34.1

[PATCH V2 2/2] rtc: atcrtc100: Add andes atcrtc100 RTC driver

[CL Wang]

The atcrtc100 module includes a real time counter with alarm.
Add a RTC driver for this function.

Signed-off-by: CL Wang <cl634@andestech.com>
---
 drivers/rtc/Kconfig         |  10 +
 drivers/rtc/Makefile        |   1 +
 drivers/rtc/rtc-atcrtc100.c | 457 ++++++++++++++++++++++++++++++++++++
 3 files changed, 468 insertions(+)
 create mode 100644 drivers/rtc/rtc-atcrtc100.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 2a95b05982ad..cea1750c368d 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1442,6 +1442,16 @@ config RTC_DRV_OMAP
 	  This driver can also be built as a module, if so, module
 	  will be called rtc-omap.
 
+config RTC_DRV_ATCRTC100
+	tristate "Andes Real Time Clock"
+	depends on RISCV
+	help
+	  If you say Y here you will get access to the real time clock
+	  built into your AE350.
+
+	  To compile this driver as a module, choose M here.
+	  The module will be called rtc-atcrtc100.
+
 config RTC_DRV_S3C
 	tristate "Samsung S3C series SoC RTC"
 	depends on ARCH_EXYNOS || ARCH_S3C64XX || ARCH_S5PV210 || \
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 3004e372f25f..cc41cd5d3017 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_RTC_DRV_ASM9260)	+= rtc-asm9260.o
 obj-$(CONFIG_RTC_DRV_ASPEED)	+= rtc-aspeed.o
 obj-$(CONFIG_RTC_DRV_AT91RM9200)+= rtc-at91rm9200.o
 obj-$(CONFIG_RTC_DRV_AT91SAM9)	+= rtc-at91sam9.o
+obj-$(CONFIG_RTC_DRV_ATCRTC100) += rtc-atcrtc100.o
 obj-$(CONFIG_RTC_DRV_AU1XXX)	+= rtc-au1xxx.o
 obj-$(CONFIG_RTC_DRV_BBNSM)	+= rtc-nxp-bbnsm.o
 obj-$(CONFIG_RTC_DRV_BD70528)	+= rtc-bd70528.o
diff --git a/drivers/rtc/rtc-atcrtc100.c b/drivers/rtc/rtc-atcrtc100.c
new file mode 100644
index 000000000000..43957906a8ed
--- /dev/null
+++ b/drivers/rtc/rtc-atcrtc100.c
@@ -0,0 +1,457 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Driver for Andes AE350 RTC module.
+ *
+ * Copyright (C) 2024 Andes Technology Corporation
+ */
+
+#include <linux/module.h>
+#include <linux/rtc.h>
+#include <linux/platform_device.h>
+#include <linux/interrupt.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+#include <linux/pm_wakeirq.h>
+
+#define RTC_ID		0x00	/* ID and Revision */
+#define ID_OFF		12
+#define ID_MSK		(0xfffff << ID_OFF)
+#define ATCRTC100ID	(0x03011 << ID_OFF)
+#define RTC_RSV		0x4	/* Reserve */
+#define RTC_CNT		0x10	/* Counter */
+#define RTC_ALM		0x14	/* Alarm */
+#define DAY_OFF		17
+#define DAY_MSK		(0x7fff)
+#define HOUR_OFF	12
+#define HOUR_MSK	(0x1f)
+#define MIN_OFF		6
+#define MIN_MSK		(0x3f)
+#define SEC_OFF		0
+#define SEC_MSK		(0x3f)
+#define RTC_SECOND(x)	\
+			((x >> SEC_OFF) & SEC_MSK) /* RTC sec */
+#define RTC_MINUTE(x)		\
+			((x >> MIN_OFF) & MIN_MSK) /* RTC min */
+#define RTC_HOUR(x)		\
+			((x >> HOUR_OFF) & HOUR_MSK) /* RTC hour */
+#define RTC_DAYS(x)		\
+			((x >> DAY_OFF) & DAY_MSK) /* RTC day */
+
+#define RTC_CR		0x18	/* Control */
+#define RTC_EN		(0x1UL << 0)
+#define ALARM_WAKEUP	(0x1UL << 1)
+#define ALARM_INT	(0x1UL << 2)
+#define DAY_INT		(0x1UL << 3)
+#define HOUR_INT	(0x1UL << 4)
+#define MIN_INT		(0x1UL << 5)
+#define SEC_INT		(0x1UL << 6)
+#define HSEC_INT	(0x1UL << 7)
+#define RTC_STA		0x1c	/* Status */
+#define WRITE_DONE	(0x1UL << 16)
+#define RTC_TRIM	0x20	/* Digital Trimming */
+
+#define ATCRTC_TIME_TO_SEC(D, H, M, S)	(D * 86400LL + H * 3600 + M * 60 + S)
+
+/*
+ * The maximum day counter available for the ATCRTC100 hardware is 15 bits
+ * long and can count up to 32767 days. This means that the ATCRTC100 hardware
+ * can count up to about 89 years, so we set range_min to 2000 and range_max
+ * to the end of the year 2089.
+ */
+#define ATCRTC_RTC_TIMESTAMP_END_2089	3786911999LL  /* 2089-12-31 23:59:59 */
+#define ATCRTC_RTC_TIMESTAMP_BEGIN_2000 RTC_TIMESTAMP_BEGIN_2000
+
+struct atc_rtc {
+	struct rtc_device	*rtc_dev;
+	struct regmap		*regmap;
+	unsigned int		alarm_irq;
+	unsigned int		time_irq;
+	spinlock_t		lock; /* Protects this structure */
+};
+
+/*
+ * WARNING: This variable is only intended to pass the LTP test. The Andes
+ * internal implementation of the RTC on FPGA cannot count more than 32 days
+ * because the day counter is 5 bits long.
+ * The real RTC hardware should support a sufficiently large counter, and this
+ * variable is automatically set to 0 if the day counter is sufficient to
+ * represent the date.
+ */
+time64_t total_offset_sec;
+
+static const struct regmap_config atc_rtc_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.max_register = RTC_TRIM,
+	.cache_type = REGCACHE_NONE,
+};
+
+static irqreturn_t rtc_interrupt(int irq, void *dev_id)
+{
+	struct atc_rtc *rtc = dev_id;
+
+	if (regmap_test_bits(rtc->regmap, RTC_STA, SEC_INT)) {
+		regmap_write_bits(rtc->regmap, RTC_STA, SEC_INT, SEC_INT);
+		rtc_update_irq(rtc->rtc_dev, 1, RTC_UF | RTC_IRQF);
+		return IRQ_HANDLED;
+	}
+	return IRQ_NONE;
+}
+
+static irqreturn_t rtc_alarm(int irq, void *dev_id)
+{
+	struct atc_rtc *rtc = dev_id;
+
+	if (regmap_test_bits(rtc->regmap, RTC_STA, ALARM_INT)) {
+		regmap_update_bits(rtc->regmap, RTC_CR, ALARM_INT, 0);
+		regmap_write_bits(rtc->regmap, RTC_STA, ALARM_INT, ALARM_INT);
+		rtc_update_irq(rtc->rtc_dev, 1, RTC_AF | RTC_IRQF);
+		return IRQ_HANDLED;
+	}
+	return IRQ_NONE;
+}
+
+static int atc_alarm_irq_enable(struct device *dev, unsigned int enabled)
+{
+	struct atc_rtc *rtc = dev_get_drvdata(dev);
+
+	spin_lock_irq(&rtc->lock);
+	if (enabled)
+		regmap_update_bits(rtc->regmap, RTC_CR, ALARM_INT, ALARM_INT);
+	else
+		regmap_update_bits(rtc->regmap, RTC_CR, ALARM_INT, 0);
+
+	spin_unlock_irq(&rtc->lock);
+	return 0;
+}
+
+static int atc_alarm_wakeup_enable(struct device *dev, unsigned int enabled)
+{
+	struct atc_rtc *rtc = dev_get_drvdata(dev);
+
+	spin_lock_irq(&rtc->lock);
+	if (enabled)
+		regmap_update_bits(rtc->regmap, RTC_CR, ALARM_WAKEUP, ALARM_WAKEUP);
+	else
+		regmap_update_bits(rtc->regmap, RTC_CR, ALARM_WAKEUP, 0);
+	spin_unlock_irq(&rtc->lock);
+	return 0;
+}
+
+
+/**
+ * This function reads the time from the RTC hardware
+ * @rtc: The structure of the atc_rtc.
+ *
+ * This function is called in an atomic operation, so don't add code
+ * to this function that will cause the process to sleep.
+ */
+static time64_t atc_rtc_read_rtc_time(struct atc_rtc *rtc)
+{
+	unsigned int rtc_cnt;
+	time64_t time;
+
+	/* Check the progress of updating the RTC registers. */
+	while (!regmap_test_bits(rtc->regmap, RTC_STA, WRITE_DONE))
+		continue;
+
+	regmap_read(rtc->regmap, RTC_CNT, &rtc_cnt);
+	time = ATCRTC_TIME_TO_SEC(RTC_DAYS(rtc_cnt), RTC_HOUR(rtc_cnt),
+				  RTC_MINUTE(rtc_cnt), RTC_SECOND(rtc_cnt));
+
+	return time;
+}
+
+static int atc_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+	struct atc_rtc *rtc = dev_get_drvdata(dev);
+	time64_t time;
+
+	spin_lock_irq(&rtc->lock);
+	time = atc_rtc_read_rtc_time(rtc) + total_offset_sec;
+	spin_unlock_irq(&rtc->lock);
+
+	rtc_time64_to_tm(time, tm);
+	if (rtc_valid_tm(tm) < 0) {
+		dev_err(dev, "invalid date\n");
+		rtc_time64_to_tm(0, tm);
+	}
+	return 0;
+}
+
+/**
+ * This function write the time to the RTC hardware
+ * @rtc: The structure of the atc_rtc.
+ * @time: The time to set to the RTC
+ *
+ * This function is called in an atomic operation, so don't add code
+ * to this function that will cause the process to sleep.
+ */
+static void atc_rtc_set_rtc_time(struct atc_rtc *rtc, time64_t time)
+{
+	time64_t time_rem;
+	s32 rem;
+	u32 counter;
+
+	counter = ((div_s64_rem(time, 86400, &rem) & DAY_MSK)
+		   << DAY_OFF);
+	time_rem = rem;
+	counter |= ((div_s64_rem(time_rem, 3600, &rem) & HOUR_MSK)
+		    << HOUR_OFF);
+	time_rem = rem;
+	counter |= ((div_s64_rem(time_rem, 60, &rem) & MIN_MSK)
+		    << MIN_OFF);
+	counter |= ((rem & SEC_MSK) << SEC_OFF);
+
+	regmap_write(rtc->regmap, RTC_CNT, counter);
+}
+
+static int atc_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	struct atc_rtc *rtc = dev_get_drvdata(dev);
+	struct rtc_time tm_offset;
+	time64_t rtc_time;
+	time64_t sys_time;
+
+	sys_time = rtc_tm_to_time64(tm);
+
+	spin_lock_irq(&rtc->lock);
+	atc_rtc_set_rtc_time(rtc, sys_time);
+
+	/* Ensure the size of day counter is sufficient to represent the date. */
+	rtc_time = atc_rtc_read_rtc_time(rtc);
+
+	if (rtc_time < sys_time) {
+		/*
+		 * The day counter is not enough to represent the date, so
+		 * we need to take an offset to improve the RTC hardware's ability
+		 * to avoid getting the wrong date.
+		 */
+		dev_err(dev, "The size of day counter is insufficient for date representation, and the date may be incorrect when the system is restarted.\n");
+		memcpy(&tm_offset, tm, sizeof(struct rtc_time));
+
+		rtc_time = ATCRTC_TIME_TO_SEC(0, tm_offset.tm_hour, tm_offset.tm_min,
+					      tm_offset.tm_sec);
+		/*
+		 * Only the date (years, months, and days) is stored in the
+		 * total_offset_sec, and the time (hours, minutes, and seconds)
+		 * is stored directly in the hardware of the RTC.
+		 */
+		atc_rtc_set_rtc_time(rtc, rtc_time);
+
+		tm_offset.tm_hour = 0;
+		tm_offset.tm_min = 0;
+		tm_offset.tm_sec = 0;
+		total_offset_sec = rtc_tm_to_time64(&tm_offset);
+	} else {
+		total_offset_sec = 0;
+	}
+
+	spin_unlock_irq(&rtc->lock);
+
+	return 0;
+}
+
+static int atc_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *wkalrm)
+{
+	struct atc_rtc *rtc = dev_get_drvdata(dev);
+	struct rtc_time *tm = &wkalrm->time;
+	unsigned int rtc_alarm;
+
+	spin_lock_irq(&rtc->lock);
+	regmap_read(rtc->regmap, RTC_ALM, &rtc_alarm);
+	wkalrm->enabled = regmap_test_bits(rtc->regmap, RTC_CR, ALARM_INT);
+	spin_unlock_irq(&rtc->lock);
+
+	tm->tm_sec	= (rtc_alarm >> SEC_OFF) & SEC_MSK;
+	tm->tm_min	= (rtc_alarm >> MIN_OFF) & MIN_MSK;
+	tm->tm_hour	= (rtc_alarm >> HOUR_OFF) & HOUR_MSK;
+
+	return 0;
+}
+
+static int atc_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *wkalrm)
+{
+	u32 alm = 0;
+	struct atc_rtc *rtc = dev_get_drvdata(dev);
+	struct rtc_time *tm = &wkalrm->time;
+	int err = rtc_valid_tm(tm);
+
+	if (err < 0) {
+		dev_err(dev, "invalid alarm value\n");
+		return err;
+	}
+
+	/* Disable alarm interrupt and clear the alarm flag */
+	regmap_update_bits(rtc->regmap, RTC_CR, ALARM_INT, 0);
+
+	/* Set alarm time */
+	alm |= ((tm->tm_sec & SEC_MSK) << SEC_OFF);
+	alm |= ((tm->tm_min & MIN_MSK) << MIN_OFF);
+	alm |= ((tm->tm_hour & HOUR_MSK) << HOUR_OFF);
+
+	spin_lock_irq(&rtc->lock);
+	regmap_write(rtc->regmap, RTC_ALM, alm);
+
+	while (!regmap_test_bits(rtc->regmap, RTC_STA, WRITE_DONE))
+		continue;
+
+	if (wkalrm->enabled)
+		regmap_update_bits(rtc->regmap, RTC_CR, ALARM_INT, ALARM_INT);
+
+	spin_unlock_irq(&rtc->lock);
+	return 0;
+}
+
+static int atc_rtc_hw_init(struct atc_rtc *rtc)
+{
+	unsigned int rtc_id;
+
+	regmap_read(rtc->regmap, RTC_ID, &rtc_id);
+	if ((rtc_id & ID_MSK) != ATCRTC100ID)
+		return -ENOENT;
+
+	regmap_update_bits(rtc->regmap, RTC_CR, RTC_EN, RTC_EN);
+
+	return 0;
+}
+
+static const  struct rtc_class_ops rtc_ops = {
+	.alarm_irq_enable = atc_alarm_irq_enable,
+	.read_time	= atc_rtc_read_time,
+	.set_time	= atc_rtc_set_time,
+	.read_alarm	= atc_rtc_read_alarm,
+	.set_alarm	= atc_rtc_set_alarm,
+};
+
+static int atc_rtc_probe(struct platform_device *pdev)
+{
+	struct atc_rtc *atc_rtc;
+	void __iomem *reg_base;
+	int ret = 0;
+
+	atc_rtc = devm_kzalloc(&pdev->dev, sizeof(*atc_rtc), GFP_KERNEL);
+	if (!atc_rtc)
+		return -ENOMEM;
+	platform_set_drvdata(pdev, atc_rtc);
+	spin_lock_init(&atc_rtc->lock);
+
+	reg_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(reg_base)) {
+		dev_err(&pdev->dev, "couldn't map io space\n");
+		return PTR_ERR(atc_rtc->regmap);
+	}
+
+	atc_rtc->regmap = devm_regmap_init_mmio(&pdev->dev, reg_base,
+						&atc_rtc_regmap_config);
+	if (IS_ERR(atc_rtc->regmap)) {
+		dev_err(&pdev->dev, "regmap init failed\n");
+		ret = PTR_ERR(atc_rtc->regmap);
+	}
+
+	ret = atc_rtc_hw_init(atc_rtc);
+	if (ret) {
+		dev_err(&pdev->dev, "atc_rtc HW initialize failed\n");
+		return ret;
+	}
+
+	atc_rtc->alarm_irq = platform_get_irq(pdev, 1);
+	if (atc_rtc->alarm_irq < 0) {
+		dev_err(&pdev->dev, "failed to get irq %d for alarm\n",
+			atc_rtc->alarm_irq);
+	}
+	atc_rtc->time_irq = platform_get_irq(pdev, 0);
+	if (atc_rtc->time_irq < 0) {
+		dev_err(&pdev->dev, "failed to get irq %d for RTC\n",
+			atc_rtc->time_irq);
+	}
+
+	ret = devm_request_irq(&pdev->dev, atc_rtc->alarm_irq, rtc_alarm,
+			       0, "rtc alarm", atc_rtc);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to request irq %d: %d for alarm\n",
+			atc_rtc->alarm_irq, ret);
+		return ret;
+	}
+
+	ret = devm_request_irq(&pdev->dev, atc_rtc->time_irq, rtc_interrupt,
+			       0, "rtc time", atc_rtc);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to request irq %d: %d\n",
+			atc_rtc->time_irq, ret);
+		return ret;
+	}
+
+	atc_rtc->rtc_dev = devm_rtc_allocate_device(&pdev->dev);
+	if (IS_ERR(atc_rtc->rtc_dev)) {
+		dev_err(&pdev->dev, "Could not allocate rtc device\n");
+		return PTR_ERR(atc_rtc->rtc_dev);
+	}
+
+	if (of_property_read_bool(pdev->dev.of_node, "wakeup-source")) {
+		device_init_wakeup(&pdev->dev, true);
+		ret = dev_pm_set_wake_irq(&pdev->dev, atc_rtc->alarm_irq);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to enable irq wake\n");
+			device_init_wakeup(&pdev->dev, false);
+			atc_alarm_wakeup_enable(&pdev->dev, false);
+			clear_bit(RTC_FEATURE_ALARM, atc_rtc->rtc_dev->features);
+		} else {
+			atc_alarm_wakeup_enable(&pdev->dev, true);
+			set_bit(RTC_FEATURE_ALARM, atc_rtc->rtc_dev->features);
+		}
+	} else {
+		atc_alarm_wakeup_enable(&pdev->dev, false);
+		clear_bit(RTC_FEATURE_ALARM, atc_rtc->rtc_dev->features);
+	}
+	atc_rtc->rtc_dev->ops = &rtc_ops;
+	atc_rtc->rtc_dev->range_min = ATCRTC_RTC_TIMESTAMP_BEGIN_2000;
+	atc_rtc->rtc_dev->range_max = ATCRTC_RTC_TIMESTAMP_END_2089;
+
+	return devm_rtc_register_device(atc_rtc->rtc_dev);
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int atc_rtc_resume(struct device *dev)
+{
+	struct atc_rtc *rtc = dev_get_drvdata(dev);
+
+	if (device_may_wakeup(dev))
+		disable_irq_wake(rtc->alarm_irq);
+
+	return 0;
+}
+
+static int atc_rtc_suspend(struct device *dev)
+{
+	struct atc_rtc *rtc = dev_get_drvdata(dev);
+
+	if (device_may_wakeup(dev))
+		enable_irq_wake(rtc->alarm_irq);
+
+	return 0;
+}
+#endif
+static SIMPLE_DEV_PM_OPS(atc_rtc_pm_ops, atc_rtc_suspend, atc_rtc_resume);
+
+static const struct of_device_id atc_rtc_dt_match[] = {
+	{ .compatible = "andestech,atcrtc100" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, atc_rtc_dt_match);
+
+static struct platform_driver atc_rtc_platform_driver = {
+	.driver		= {
+		.name	= "atcrtc100",
+		.of_match_table	= of_match_ptr(atc_rtc_dt_match),
+		.pm = &atc_rtc_pm_ops,
+	},
+	.probe		= atc_rtc_probe,
+};
+
+module_platform_driver(atc_rtc_platform_driver);
+
+MODULE_AUTHOR("CL Wang <cl634@andestech.com>");
+MODULE_DESCRIPTION("Andes ATCRTC100 driver");
+MODULE_LICENSE("GPL");
-- 
2.34.1

Re: [PATCH V2 1/2] dt-bindings: rtc: atcrtc100: Add atcrtc100 bindings

[Krzysztof Kozlowski]

On 17/06/2024 10:25, CL Wang wrote:
> Add YAML bindings for Andes atcrtc100 RTC.

A nit, subject: drop second/last, redundant "bindings". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

> 
> Signed-off-by: CL Wang <cl634@andestech.com>
> ---
>  .../bindings/rtc/andes,atcrtc100-rtc.yaml     | 39 +++++++++++++++++++
>  1 file changed, 39 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/rtc/andes,atcrtc100-rtc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/rtc/andes,atcrtc100-rtc.yaml b/Documentation/devicetree/bindings/rtc/andes,atcrtc100-rtc.yaml
> new file mode 100644
> index 000000000000..86cecb30c889
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/andes,atcrtc100-rtc.yaml

Use compatible as filename.

> @@ -0,0 +1,39 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/rtc/andes,atcrtc100-rtc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Andes, ATCRTC100 on-SoC RTC
> +
> +allOf:
> +  - $ref: rtc.yaml#

allOf goes after maintainers.

> +
> +maintainers:
> +  - CL Wang <cl634@andestech.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - andestech,atcrtc100
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    minItems: 2

This must be specific. Describe the items instead (items: with -
description: for each).

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +additionalProperties: false

Instead:
unevaluatedProperties: false



Best regards,
Krzysztof

Re: [PATCH V2 2/2] rtc: atcrtc100: Add andes atcrtc100 RTC driver

[Krzysztof Kozlowski]

On 17/06/2024 10:25, CL Wang wrote:
> The atcrtc100 module includes a real time counter with alarm.
> Add a RTC driver for this function.
> 
> Signed-off-by: CL Wang <cl634@andestech.com>


...

> +
> +static int atc_rtc_probe(struct platform_device *pdev)
> +{
> +	struct atc_rtc *atc_rtc;
> +	void __iomem *reg_base;
> +	int ret = 0;
> +
> +	atc_rtc = devm_kzalloc(&pdev->dev, sizeof(*atc_rtc), GFP_KERNEL);
> +	if (!atc_rtc)
> +		return -ENOMEM;
> +	platform_set_drvdata(pdev, atc_rtc);
> +	spin_lock_init(&atc_rtc->lock);
> +
> +	reg_base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(reg_base)) {
> +		dev_err(&pdev->dev, "couldn't map io space\n");
> +		return PTR_ERR(atc_rtc->regmap);
> +	}
> +
> +	atc_rtc->regmap = devm_regmap_init_mmio(&pdev->dev, reg_base,
> +						&atc_rtc_regmap_config);
> +	if (IS_ERR(atc_rtc->regmap)) {
> +		dev_err(&pdev->dev, "regmap init failed\n");
> +		ret = PTR_ERR(atc_rtc->regmap);
> +	}
> +
> +	ret = atc_rtc_hw_init(atc_rtc);
> +	if (ret) {
> +		dev_err(&pdev->dev, "atc_rtc HW initialize failed\n");
> +		return ret;
> +	}
> +
> +	atc_rtc->alarm_irq = platform_get_irq(pdev, 1);
> +	if (atc_rtc->alarm_irq < 0) {
> +		dev_err(&pdev->dev, "failed to get irq %d for alarm\n",
> +			atc_rtc->alarm_irq);
> +	}
> +	atc_rtc->time_irq = platform_get_irq(pdev, 0);
> +	if (atc_rtc->time_irq < 0) {
> +		dev_err(&pdev->dev, "failed to get irq %d for RTC\n",
> +			atc_rtc->time_irq);
> +	}
> +
> +	ret = devm_request_irq(&pdev->dev, atc_rtc->alarm_irq, rtc_alarm,
> +			       0, "rtc alarm", atc_rtc);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to request irq %d: %d for alarm\n",
> +			atc_rtc->alarm_irq, ret);
> +		return ret;
> +	}
> +
> +	ret = devm_request_irq(&pdev->dev, atc_rtc->time_irq, rtc_interrupt,
> +			       0, "rtc time", atc_rtc);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to request irq %d: %d\n",
> +			atc_rtc->time_irq, ret);
> +		return ret;
> +	}
> +
> +	atc_rtc->rtc_dev = devm_rtc_allocate_device(&pdev->dev);
> +	if (IS_ERR(atc_rtc->rtc_dev)) {
> +		dev_err(&pdev->dev, "Could not allocate rtc device\n");
> +		return PTR_ERR(atc_rtc->rtc_dev);
> +	}
> +
> +	if (of_property_read_bool(pdev->dev.of_node, "wakeup-source")) {

Test your DTS (dtbs_check). This should fail.


> +		device_init_wakeup(&pdev->dev, true);
> +		ret = dev_pm_set_wake_irq(&pdev->dev, atc_rtc->alarm_irq);
> +		if (ret) {
> +			dev_err(&pdev->dev, "failed to enable irq wake\n");
> +			device_init_wakeup(&pdev->dev, false);
> +			atc_alarm_wakeup_enable(&pdev->dev, false);
> +			clear_bit(RTC_FEATURE_ALARM, atc_rtc->rtc_dev->features);
> +		} else {
> +			atc_alarm_wakeup_enable(&pdev->dev, true);
> +			set_bit(RTC_FEATURE_ALARM, atc_rtc->rtc_dev->features);
> +		}
> +	} else {
> +		atc_alarm_wakeup_enable(&pdev->dev, false);
> +		clear_bit(RTC_FEATURE_ALARM, atc_rtc->rtc_dev->features);
> +	}
> +	atc_rtc->rtc_dev->ops = &rtc_ops;
> +	atc_rtc->rtc_dev->range_min = ATCRTC_RTC_TIMESTAMP_BEGIN_2000;
> +	atc_rtc->rtc_dev->range_max = ATCRTC_RTC_TIMESTAMP_END_2089;
> +
> +	return devm_rtc_register_device(atc_rtc->rtc_dev);
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int atc_rtc_resume(struct device *dev)
> +{
> +	struct atc_rtc *rtc = dev_get_drvdata(dev);
> +
> +	if (device_may_wakeup(dev))
> +		disable_irq_wake(rtc->alarm_irq);
> +
> +	return 0;
> +}
> +
> +static int atc_rtc_suspend(struct device *dev)
> +{
> +	struct atc_rtc *rtc = dev_get_drvdata(dev);
> +
> +	if (device_may_wakeup(dev))
> +		enable_irq_wake(rtc->alarm_irq);
> +
> +	return 0;
> +}
> +#endif
> +static SIMPLE_DEV_PM_OPS(atc_rtc_pm_ops, atc_rtc_suspend, atc_rtc_resume);
> +
> +static const struct of_device_id atc_rtc_dt_match[] = {
> +	{ .compatible = "andestech,atcrtc100" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, atc_rtc_dt_match);
> +
> +static struct platform_driver atc_rtc_platform_driver = {
> +	.driver		= {
> +		.name	= "atcrtc100",
> +		.of_match_table	= of_match_ptr(atc_rtc_dt_match),

Drop of_match_ptr(), leads to warnings.

> +		.pm = &atc_rtc_pm_ops,
> +	},
> +	.probe		= atc_rtc_probe,
> +};
> +
> +module_platform_driver(atc_rtc_platform_driver);
> +
> +MODULE_AUTHOR("CL Wang <cl634@andestech.com>");
> +MODULE_DESCRIPTION("Andes ATCRTC100 driver");
> +MODULE_LICENSE("GPL");

Best regards,
Krzysztof

Re: [PATCH V2 1/2] dt-bindings: rtc: atcrtc100: Add atcrtc100 bindings

[Krzysztof Kozlowski]

On 17/06/2024 10:25, CL Wang wrote:
> Add YAML bindings for Andes atcrtc100 RTC.
> 
> Signed-off-by: CL Wang <cl634@andestech.com>

I should have ignored this patch... Not tested.

<form letter>
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.

Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline), work on fork of kernel
(don't, instead use mainline) or you ignore some maintainers (really
don't). Just use b4 and everything should be fine, although remember
about `b4 prep --auto-to-cc` if you added new patches to the patchset.

You missed at least devicetree list (maybe more), so this won't be
tested by automated tooling. Performing review on untested code might be
a waste of time, thus I will skip this patch entirely till you follow
the process allowing the patch to be tested.

Please kindly resend and include all necessary To/Cc entries.
</form letter>

Best regards,
Krzysztof