[PATCH 06/25] rtc: Add driver for RDA Micro SoC

Dang Huynh via B4 Relay posted 25 patches 4 months, 3 weeks ago
[PATCH 06/25] rtc: Add driver for RDA Micro SoC
Posted by Dang Huynh via B4 Relay 4 months, 3 weeks ago
From: Dang Huynh <dang.huynh@mainlining.org>

The RDA Micro SoC has built-in RTC, it supports read/write date
as well as alarm.

Signed-off-by: Dang Huynh <dang.huynh@mainlining.org>
---
 MAINTAINERS           |   6 +
 drivers/rtc/Kconfig   |  11 ++
 drivers/rtc/Makefile  |   1 +
 drivers/rtc/rtc-rda.c | 356 ++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 374 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index fa7f80bd7b2f8bd2099acb9f38070498e7b1cc7e..0549b1d0657f2caaf86a723db139cf9d84d59c4a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21393,6 +21393,12 @@ S:	Supported
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/rcu/linux.git rcu/dev
 F:	tools/testing/selftests/rcutorture
 
+RDA MICRO REAL TIME CLOCK DRIVER
+M:	Dang Huynh <dang.huynh@mainlining.org>
+S:	Maintained
+F:	Documentation/devicetree/bindings/rtc/rda,8810pl-rtc.yaml
+F:	drivers/rtc/rtc-rda.c
+
 RDACM20 Camera Sensor
 M:	Jacopo Mondi <jacopo+renesas@jmondi.org>
 M:	Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 64f6e9756aff4a1f6f6c50f9b4fc2140f66a8578..287fc3bbd474ab78a9bd3b8813e8b9d475c07198 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1471,6 +1471,17 @@ config RTC_DRV_OMAP
 	  This driver can also be built as a module, if so, module
 	  will be called rtc-omap.
 
+config RTC_DRV_RDA
+	tristate "RDA Micro RTC"
+	depends on ARCH_RDA || COMPILE_TEST
+	select REGMAP_MMIO
+	help
+	  If you say yes here you get support for the built-in RTC on
+	  RDA Micro SoC.
+
+	  This driver can also be built as a module, if so, the module
+	  will be called rtc-rda.
+
 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 789bddfea99d8fcd024566891c37ee73e527cf93..02f73062bb158fe4738a3043c58ee40f8a58b3c6 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -141,6 +141,7 @@ obj-$(CONFIG_RTC_DRV_PS3)	+= rtc-ps3.o
 obj-$(CONFIG_RTC_DRV_PXA)	+= rtc-pxa.o
 obj-$(CONFIG_RTC_DRV_R7301)	+= rtc-r7301.o
 obj-$(CONFIG_RTC_DRV_R9701)	+= rtc-r9701.o
+obj-$(CONFIG_RTC_DRV_RDA)	+= rtc-rda.o
 obj-$(CONFIG_RTC_DRV_RC5T583)	+= rtc-rc5t583.o
 obj-$(CONFIG_RTC_DRV_RC5T619)	+= rtc-rc5t619.o
 obj-$(CONFIG_RTC_DRV_RK808)	+= rtc-rk808.o
diff --git a/drivers/rtc/rtc-rda.c b/drivers/rtc/rtc-rda.c
new file mode 100644
index 0000000000000000000000000000000000000000..bb5aa25fb7d0ad538a0f7f67a80d08fe67af1c5d
--- /dev/null
+++ b/drivers/rtc/rtc-rda.c
@@ -0,0 +1,356 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * RTC driver for RDA Micro
+ *
+ * Copyright (C) 2013-2014 RDA Microelectronics Inc.
+ * Copyright (C) 2024 Dang Huynh <dang.huynh@mainlining.org>
+ */
+
+#include <linux/of.h>
+#include <linux/module.h>
+#include <linux/rtc.h>
+#include <linux/platform_device.h>
+#include <linux/pm.h>
+#include <linux/regmap.h>
+
+struct rda_rtc {
+	struct rtc_device *rtc_dev;
+	struct regmap *regmap;
+};
+
+/* RTC Registers */
+#define RDA_RTC_CTRL_REG 0x0
+#define RDA_RTC_CMD_REG 0x4
+#define RDA_RTC_STA_REG 0x8
+#define RDA_RTC_CAL_LOAD_LOW_REG 0xC
+#define RDA_RTC_CAL_LOAD_HIGH_REG 0x10
+#define RDA_RTC_CUR_LOAD_LOW_REG 0x14
+#define RDA_RTC_CUR_LOAD_HIGH_REG 0x18
+#define RDA_RTC_ALARM_LOW_REG 0x1C
+#define RDA_RTC_ALARM_HIGH_REG 0x20
+
+/* RTC Bits */
+#define RDA_RTC_CMD_CAL_LOAD BIT(0)
+#define RDA_RTC_CMD_ALARM_LOAD BIT(4)
+#define RDA_RTC_CMD_ALARM_ENABLE BIT(5)
+#define RDA_RTC_CMD_ALARM_DISABLE BIT(6)
+#define RDA_RTC_CMD_INVALID BIT(31)
+#define RDA_RTC_STA_ALARM_ENABLE BIT(20)
+#define RDA_RTC_STA_NOT_PROG BIT(31)
+
+/* RTC Masks */
+#define RDA_SEC_MASK GENMASK(7, 0)
+#define RDA_MIN_MASK GENMASK(15, 8)
+#define RDA_HRS_MASK GENMASK(23, 16)
+
+#define RDA_MDAY_MASK GENMASK(7, 0)
+#define RDA_MON_MASK GENMASK(11, 8)
+#define RDA_YEAR_MASK GENMASK(22, 16)
+#define RDA_WDAY_MASK GENMASK(26, 24)
+
+static int rda_rtc_settime(struct device *dev, struct rtc_time *tm)
+{
+	struct rda_rtc *rtc = dev_get_drvdata(dev);
+	u32 high, low;
+	int ret;
+
+	ret = rtc_valid_tm(tm);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * The number of years since 1900 in kernel,
+	 * but it is defined since 2000 by HW.
+	 * The number of mons' range is from 0 to 11 in kernel,
+	 * but it is defined from 1 to 12 by HW.
+	 */
+	low = FIELD_PREP(RDA_SEC_MASK, tm->tm_sec) |
+		FIELD_PREP(RDA_MIN_MASK, tm->tm_min) |
+		FIELD_PREP(RDA_HRS_MASK, tm->tm_hour);
+
+	high = FIELD_PREP(RDA_MDAY_MASK, tm->tm_mday) |
+		FIELD_PREP(RDA_MON_MASK, tm->tm_mon + 1) |
+		FIELD_PREP(RDA_YEAR_MASK, tm->tm_year - 100) |
+		FIELD_PREP(RDA_WDAY_MASK, tm->tm_wday);
+
+	ret = regmap_write(rtc->regmap, RDA_RTC_CAL_LOAD_LOW_REG, low);
+	if (ret < 0) {
+		dev_err(dev, "Failed to update RTC low register: %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_write(rtc->regmap, RDA_RTC_CAL_LOAD_HIGH_REG, high);
+	if (ret < 0) {
+		dev_err(dev, "Failed to update RTC low register: %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_update_bits(rtc->regmap, RDA_RTC_CMD_REG, RDA_RTC_CMD_CAL_LOAD, 1);
+	if (ret < 0) {
+		dev_err(dev, "Failed to update RTC cal load register: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int rda_rtc_readtime(struct device *dev, struct rtc_time *tm)
+{
+	struct rda_rtc *rtc = dev_get_drvdata(dev);
+	unsigned int high, low;
+	int ret;
+
+	/*
+	 * Check if RTC data is valid.
+	 *
+	 * When this bit is set, it means the data in the RTC is invalid
+	 * or not configured.
+	 */
+	ret = regmap_test_bits(rtc->regmap, RDA_RTC_STA_REG, RDA_RTC_STA_NOT_PROG);
+	if (ret < 0) {
+		dev_err(dev, "Failed to read RTC status: %d\n", ret);
+		return ret;
+	} else if (ret > 0)
+		return -EINVAL;
+
+	ret = regmap_read(rtc->regmap, RDA_RTC_CUR_LOAD_HIGH_REG, &high);
+	if (ret) {
+		dev_err(dev, "Failed to read RTC high reg: %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_read(rtc->regmap, RDA_RTC_CUR_LOAD_LOW_REG, &low);
+	if (ret) {
+		dev_err(dev, "Failed to read RTC low reg: %d\n", ret);
+		return ret;
+	}
+
+	tm->tm_sec = FIELD_GET(RDA_SEC_MASK, low);
+	tm->tm_min = FIELD_GET(RDA_MIN_MASK, low);
+	tm->tm_hour = FIELD_GET(RDA_HRS_MASK, low);
+	tm->tm_mday = FIELD_GET(RDA_MDAY_MASK, high);
+	tm->tm_mon = FIELD_GET(RDA_MON_MASK, high);
+	tm->tm_year = FIELD_GET(RDA_YEAR_MASK, high);
+	tm->tm_wday = FIELD_GET(RDA_WDAY_MASK, high);
+
+	/*
+	 * The number of years since 1900 in kernel,
+	 * but it is defined since 2000 by HW.
+	 */
+	tm->tm_year += 100;
+	/*
+	 * The number of mons' range is from 0 to 11 in kernel,
+	 * but it is defined from 1 to 12 by HW.
+	 */
+	tm->tm_mon -= 1;
+
+	return 0;
+}
+
+static int rda_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct rda_rtc *rtc = dev_get_drvdata(dev);
+	struct rtc_time *tm = &alrm->time;
+	unsigned int high, low;
+	int ret;
+
+	ret = regmap_read(rtc->regmap, RDA_RTC_ALARM_HIGH_REG, &high);
+	if (ret) {
+		dev_err(dev, "Failed to read alarm low reg: %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_read(rtc->regmap, RDA_RTC_ALARM_LOW_REG, &low);
+	if (ret) {
+		dev_err(dev, "Failed to read alarm low reg: %d\n", ret);
+		return ret;
+	}
+
+	tm->tm_sec = FIELD_GET(RDA_SEC_MASK, low);
+	tm->tm_min = FIELD_GET(RDA_MIN_MASK, low);
+	tm->tm_hour = FIELD_GET(RDA_HRS_MASK, low);
+	tm->tm_mday = FIELD_GET(RDA_MDAY_MASK, high);
+	tm->tm_mon = FIELD_GET(RDA_MON_MASK, high);
+	tm->tm_year = FIELD_GET(RDA_YEAR_MASK, high);
+	tm->tm_wday = FIELD_GET(RDA_WDAY_MASK, high);
+
+	/*
+	 * The number of years since 1900 in kernel,
+	 * but it is defined since 2000 by HW.
+	 */
+	tm->tm_year += 100;
+	/*
+	 * The number of mons' range is from 0 to 11 in kernel,
+	 * but it is defined from 1 to 12 by HW.
+	 */
+	tm->tm_mon -= 1;
+
+	return 0;
+}
+
+static int rda_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
+{
+	struct rda_rtc *rtc = dev_get_drvdata(dev);
+
+	if (enabled)
+		return regmap_update_bits(rtc->regmap, RDA_RTC_CMD_REG,
+				RDA_RTC_CMD_ALARM_ENABLE, 1);
+
+	return regmap_update_bits(rtc->regmap, RDA_RTC_CMD_REG,
+			RDA_RTC_CMD_ALARM_DISABLE, 1);
+}
+
+static int rda_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct rda_rtc *rtc = dev_get_drvdata(dev);
+	struct rtc_time *tm = &alrm->time;
+	u32 high, low;
+	int ret;
+
+	ret = rtc_valid_tm(tm);
+	if (ret < 0)
+		return ret;
+
+	/* TODO: Check if it's necessary to disable IRQ first */
+	rda_rtc_alarm_irq_enable(dev, 0);
+
+	/*
+	 * The number of years since 1900 in kernel,
+	 * but it is defined since 2000 by HW.
+	 * The number of mons' range is from 0 to 11 in kernel,
+	 * but it is defined from 1 to 12 by HW.
+	 */
+	low = FIELD_PREP(RDA_SEC_MASK, tm->tm_sec) |
+		FIELD_PREP(RDA_MIN_MASK, tm->tm_min) |
+		FIELD_PREP(RDA_HRS_MASK, tm->tm_hour);
+
+	high = FIELD_PREP(RDA_MDAY_MASK, tm->tm_mday) |
+		FIELD_PREP(RDA_MON_MASK, tm->tm_mon + 1) |
+		FIELD_PREP(RDA_YEAR_MASK, tm->tm_year - 100) |
+		FIELD_PREP(RDA_WDAY_MASK, tm->tm_wday);
+
+
+	ret = regmap_write(rtc->regmap, RDA_RTC_ALARM_LOW_REG, low);
+	if (ret < 0) {
+		dev_err(dev, "Failed to set low alarm register: %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_write(rtc->regmap, RDA_RTC_ALARM_HIGH_REG, high);
+	if (ret < 0) {
+		dev_err(dev, "Failed to set low alarm register: %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_update_bits(rtc->regmap, RDA_RTC_CMD_REG, RDA_RTC_CMD_ALARM_LOAD, 1);
+	if (ret < 0) {
+		dev_err(dev, "Failed to set alarm register: %d\n", ret);
+		return ret;
+	}
+
+	dev_dbg(dev, "Alarm set: %4d-%02d-%02d %02d:%02d:%02d\n",
+			2000 + (tm->tm_year - 100), tm->tm_mon + 1, tm->tm_mday,
+			tm->tm_hour, tm->tm_min, tm->tm_sec);
+
+	return 0;
+}
+
+static int rda_rtc_proc(struct device *dev, struct seq_file *seq)
+{
+	struct rda_rtc *rtc = dev_get_drvdata(dev);
+	int ret;
+
+	ret = regmap_test_bits(rtc->regmap, RDA_RTC_STA_REG, RDA_RTC_STA_ALARM_ENABLE);
+	if (ret < 0) {
+		dev_err(dev, "Failed to read alarm status: %d\n", ret);
+		return ret;
+	}
+
+	seq_printf(seq, "alarm enable\t: %s\n", (ret > 0) ? "yes" : "no");
+
+	return 0;
+}
+
+static const struct rtc_class_ops rda_rtc_ops = {
+	.read_time = rda_rtc_readtime,
+	.set_time = rda_rtc_settime,
+	.read_alarm = rda_rtc_readalarm,
+	.set_alarm = rda_rtc_setalarm,
+	.proc = rda_rtc_proc,
+	.alarm_irq_enable = rda_rtc_alarm_irq_enable,
+};
+
+#ifdef CONFIG_PM_SLEEP
+static int rda_rtc_suspend(struct platform_device *pdev, pm_message_t state)
+{
+	/* TODO: Check if it's okay to turn on alarm IRQ when it's not set */
+	return rda_rtc_alarm_irq_enable(&pdev->dev, 1);
+}
+
+static int rda_rtc_resume(struct platform_device *pdev)
+{
+	/* If alarms were left, we turn them off. */
+	return rda_rtc_alarm_irq_enable(&pdev->dev, 0);
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(rda_rtc_pm_ops, rda_rtc_suspend, rda_rtc_resume);
+
+static const struct regmap_config regmap_config = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+};
+
+static int rda_rtc_probe(struct platform_device *pdev)
+{
+	struct rda_rtc *rda_rtc;
+	void __iomem *base;
+
+	rda_rtc = devm_kzalloc(&pdev->dev, sizeof(*rda_rtc), GFP_KERNEL);
+	if (!rda_rtc)
+		return -ENOMEM;
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return dev_err_probe(&pdev->dev, PTR_ERR(base),
+				"failed to remap resource\n");
+
+	rda_rtc->regmap = devm_regmap_init_mmio(&pdev->dev, base, &regmap_config);
+	if (!rda_rtc->regmap)
+		return dev_err_probe(&pdev->dev, PTR_ERR(rda_rtc->regmap),
+				"can't find regmap\n");
+
+	rda_rtc->rtc_dev = devm_rtc_allocate_device(&pdev->dev);
+	if (IS_ERR(rda_rtc->rtc_dev))
+		return dev_err_probe(&pdev->dev, PTR_ERR(rda_rtc->rtc_dev),
+				"failed to allocate rtc device\n");
+
+	rda_rtc->rtc_dev->ops = &rda_rtc_ops;
+	rda_rtc->rtc_dev->range_min = RTC_TIMESTAMP_BEGIN_2000;
+	rda_rtc->rtc_dev->range_max = RTC_TIMESTAMP_END_2127;
+
+	platform_set_drvdata(pdev, rda_rtc);
+
+	return devm_rtc_register_device(rda_rtc->rtc_dev);
+}
+
+static const struct of_device_id rda_rtc_id_table[] = {
+	{ .compatible = "rda,8810pl-rtc", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, rda_rtc_id_table);
+
+static struct platform_driver rda_rtc_driver = {
+	.probe = rda_rtc_probe,
+	.driver = {
+		.name = "rtc-rda",
+		.pm = &rda_rtc_pm_ops,
+		.of_match_table = rda_rtc_id_table,
+	},
+};
+module_platform_driver(rda_rtc_driver);
+
+MODULE_AUTHOR("Dang Huynh <dang.huynh@mainlining.org>");
+MODULE_DESCRIPTION("RDA Micro RTC driver");
+MODULE_LICENSE("GPL");

-- 
2.51.0
Re: [PATCH 06/25] rtc: Add driver for RDA Micro SoC
Posted by Alexandre Belloni 3 months ago
Hello,

There are checkpatch --strict issues, please fix them.


On 17/09/2025 03:25:03+0700, Dang Huynh via B4 Relay wrote:
>  MAINTAINERS           |   6 +
>  drivers/rtc/Kconfig   |  11 ++
>  drivers/rtc/Makefile  |   1 +
>  drivers/rtc/rtc-rda.c | 356 ++++++++++++++++++++++++++++++++++++++++++++++++++

Unless you can guarantee this driver will support all the future RDA
SoC RTCs, the filename needs to be SoC specific.

> +config RTC_DRV_RDA
> +	tristate "RDA Micro RTC"
> +	depends on ARCH_RDA || COMPILE_TEST
> +	select REGMAP_MMIO
> +	help
> +	  If you say yes here you get support for the built-in RTC on
> +	  RDA Micro SoC.

You probably also need to list which ones are supported.

> +static int rda_rtc_settime(struct device *dev, struct rtc_time *tm)
> +{
> +	struct rda_rtc *rtc = dev_get_drvdata(dev);
> +	u32 high, low;
> +	int ret;
> +
> +	ret = rtc_valid_tm(tm);
> +	if (ret < 0)
> +		return ret;

The RTC core will never pass an invalid rtc_tm, this check is useless.

> +
> +	/*
> +	 * The number of years since 1900 in kernel,
> +	 * but it is defined since 2000 by HW.
> +	 * The number of mons' range is from 0 to 11 in kernel,
> +	 * but it is defined from 1 to 12 by HW.

This comment is not super useful as this is super common in the RTC
drivers,. If you want to keep it, please fix it.

> +	 */
> +	low = FIELD_PREP(RDA_SEC_MASK, tm->tm_sec) |
> +		FIELD_PREP(RDA_MIN_MASK, tm->tm_min) |
> +		FIELD_PREP(RDA_HRS_MASK, tm->tm_hour);
> +
> +	high = FIELD_PREP(RDA_MDAY_MASK, tm->tm_mday) |
> +		FIELD_PREP(RDA_MON_MASK, tm->tm_mon + 1) |
> +		FIELD_PREP(RDA_YEAR_MASK, tm->tm_year - 100) |
> +		FIELD_PREP(RDA_WDAY_MASK, tm->tm_wday);
> +
> +	ret = regmap_write(rtc->regmap, RDA_RTC_CAL_LOAD_LOW_REG, low);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to update RTC low register: %d\n", ret);

This needs to be a dev_dbg or removed.

> +		return ret;
> +	}
> +
> +	ret = regmap_write(rtc->regmap, RDA_RTC_CAL_LOAD_HIGH_REG, high);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to update RTC low register: %d\n", ret);

Ditto

> +		return ret;
> +	}
> +
> +	ret = regmap_update_bits(rtc->regmap, RDA_RTC_CMD_REG, RDA_RTC_CMD_CAL_LOAD, 1);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to update RTC cal load register: %d\n", ret);

Ditto

> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int rda_rtc_readtime(struct device *dev, struct rtc_time *tm)
> +{
> +	struct rda_rtc *rtc = dev_get_drvdata(dev);
> +	unsigned int high, low;
> +	int ret;
> +
> +	/*
> +	 * Check if RTC data is valid.
> +	 *
> +	 * When this bit is set, it means the data in the RTC is invalid
> +	 * or not configured.
> +	 */
> +	ret = regmap_test_bits(rtc->regmap, RDA_RTC_STA_REG, RDA_RTC_STA_NOT_PROG);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to read RTC status: %d\n", ret);

dev_dbg

> +		return ret;
> +	} else if (ret > 0)
> +		return -EINVAL;
> +
> +	ret = regmap_read(rtc->regmap, RDA_RTC_CUR_LOAD_HIGH_REG, &high);
> +	if (ret) {
> +		dev_err(dev, "Failed to read RTC high reg: %d\n", ret);

Ditto

> +		return ret;
> +	}
> +
> +	ret = regmap_read(rtc->regmap, RDA_RTC_CUR_LOAD_LOW_REG, &low);
> +	if (ret) {
> +		dev_err(dev, "Failed to read RTC low reg: %d\n", ret);

Ditto

> +		return ret;
> +	}
> +
> +	tm->tm_sec = FIELD_GET(RDA_SEC_MASK, low);
> +	tm->tm_min = FIELD_GET(RDA_MIN_MASK, low);
> +	tm->tm_hour = FIELD_GET(RDA_HRS_MASK, low);
> +	tm->tm_mday = FIELD_GET(RDA_MDAY_MASK, high);
> +	tm->tm_mon = FIELD_GET(RDA_MON_MASK, high);
> +	tm->tm_year = FIELD_GET(RDA_YEAR_MASK, high);
> +	tm->tm_wday = FIELD_GET(RDA_WDAY_MASK, high);
> +
> +	/*
> +	 * The number of years since 1900 in kernel,
> +	 * but it is defined since 2000 by HW.
> +	 */
> +	tm->tm_year += 100;
> +	/*
> +	 * The number of mons' range is from 0 to 11 in kernel,
> +	 * but it is defined from 1 to 12 by HW.
> +	 */

You can probably drop both comments.

> +	tm->tm_mon -= 1;
> +
> +	return 0;
> +}
> +
> +static int rda_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm)
> +{
> +	struct rda_rtc *rtc = dev_get_drvdata(dev);
> +	struct rtc_time *tm = &alrm->time;
> +	unsigned int high, low;
> +	int ret;
> +
> +	ret = regmap_read(rtc->regmap, RDA_RTC_ALARM_HIGH_REG, &high);
> +	if (ret) {
> +		dev_err(dev, "Failed to read alarm low reg: %d\n", ret);

Just to be clear, the driver is super verbose with all those dev_err.
Strings are bloating the kernel and those string will probably never be
seen by any user and event if they are seen, the user doesn't have any
other action to do other than retrying. Please remove them of move them
to dev_dbg

> +		return ret;
> +	}
> +
> +	ret = regmap_read(rtc->regmap, RDA_RTC_ALARM_LOW_REG, &low);
> +	if (ret) {
> +		dev_err(dev, "Failed to read alarm low reg: %d\n", ret);
> +		return ret;
> +	}
> +
> +	tm->tm_sec = FIELD_GET(RDA_SEC_MASK, low);
> +	tm->tm_min = FIELD_GET(RDA_MIN_MASK, low);
> +	tm->tm_hour = FIELD_GET(RDA_HRS_MASK, low);
> +	tm->tm_mday = FIELD_GET(RDA_MDAY_MASK, high);
> +	tm->tm_mon = FIELD_GET(RDA_MON_MASK, high);
> +	tm->tm_year = FIELD_GET(RDA_YEAR_MASK, high);
> +	tm->tm_wday = FIELD_GET(RDA_WDAY_MASK, high);
> +
> +	/*
> +	 * The number of years since 1900 in kernel,
> +	 * but it is defined since 2000 by HW.
> +	 */
> +	tm->tm_year += 100;
> +	/*
> +	 * The number of mons' range is from 0 to 11 in kernel,
> +	 * but it is defined from 1 to 12 by HW.
> +	 */
> +	tm->tm_mon -= 1;
> +
> +	return 0;
> +}
> +
> +static int rda_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
> +{
> +	struct rda_rtc *rtc = dev_get_drvdata(dev);
> +
> +	if (enabled)
> +		return regmap_update_bits(rtc->regmap, RDA_RTC_CMD_REG,
> +				RDA_RTC_CMD_ALARM_ENABLE, 1);
> +
> +	return regmap_update_bits(rtc->regmap, RDA_RTC_CMD_REG,
> +			RDA_RTC_CMD_ALARM_DISABLE, 1);

Wow, this is super weird, so you have one bit to enable and one to
disable the alarm. Is RDA_RTC_CMD_REG write only?

> +}
> +
> +static int rda_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm)
> +{
> +	struct rda_rtc *rtc = dev_get_drvdata(dev);
> +	struct rtc_time *tm = &alrm->time;
> +	u32 high, low;
> +	int ret;
> +
> +	ret = rtc_valid_tm(tm);
> +	if (ret < 0)
> +		return ret;
> +

tm will never be invalid

> +	/* TODO: Check if it's necessary to disable IRQ first */

I'd say probably not ;)

> +	rda_rtc_alarm_irq_enable(dev, 0);
> +
> +	/*
> +	 * The number of years since 1900 in kernel,
> +	 * but it is defined since 2000 by HW.
> +	 * The number of mons' range is from 0 to 11 in kernel,
> +	 * but it is defined from 1 to 12 by HW.
> +	 */

This is still the same comment...

> +	low = FIELD_PREP(RDA_SEC_MASK, tm->tm_sec) |
> +		FIELD_PREP(RDA_MIN_MASK, tm->tm_min) |
> +		FIELD_PREP(RDA_HRS_MASK, tm->tm_hour);
> +
> +	high = FIELD_PREP(RDA_MDAY_MASK, tm->tm_mday) |
> +		FIELD_PREP(RDA_MON_MASK, tm->tm_mon + 1) |
> +		FIELD_PREP(RDA_YEAR_MASK, tm->tm_year - 100) |
> +		FIELD_PREP(RDA_WDAY_MASK, tm->tm_wday);
> +
> +
> +	ret = regmap_write(rtc->regmap, RDA_RTC_ALARM_LOW_REG, low);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to set low alarm register: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = regmap_write(rtc->regmap, RDA_RTC_ALARM_HIGH_REG, high);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to set low alarm register: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = regmap_update_bits(rtc->regmap, RDA_RTC_CMD_REG, RDA_RTC_CMD_ALARM_LOAD, 1);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to set alarm register: %d\n", ret);
> +		return ret;
> +	}
> +
> +	dev_dbg(dev, "Alarm set: %4d-%02d-%02d %02d:%02d:%02d\n",
> +			2000 + (tm->tm_year - 100), tm->tm_mon + 1, tm->tm_mday,
> +			tm->tm_hour, tm->tm_min, tm->tm_sec);

You probably want to use %ptR or drop this as we have a tracepoint just
after.

> +
> +	return 0;
> +}
> +
> +static int rda_rtc_proc(struct device *dev, struct seq_file *seq)
> +{
> +	struct rda_rtc *rtc = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = regmap_test_bits(rtc->regmap, RDA_RTC_STA_REG, RDA_RTC_STA_ALARM_ENABLE);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to read alarm status: %d\n", ret);
> +		return ret;
> +	}
> +
> +	seq_printf(seq, "alarm enable\t: %s\n", (ret > 0) ? "yes" : "no");
> +
> +	return 0;
> +}

Drop this function, this interface is obsolete

> +
> +static const struct rtc_class_ops rda_rtc_ops = {
> +	.read_time = rda_rtc_readtime,
> +	.set_time = rda_rtc_settime,
> +	.read_alarm = rda_rtc_readalarm,
> +	.set_alarm = rda_rtc_setalarm,
> +	.proc = rda_rtc_proc,
> +	.alarm_irq_enable = rda_rtc_alarm_irq_enable,
> +};
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int rda_rtc_suspend(struct platform_device *pdev, pm_message_t state)
> +{
> +	/* TODO: Check if it's okay to turn on alarm IRQ when it's not set */
> +	return rda_rtc_alarm_irq_enable(&pdev->dev, 1);
> +}
> +
> +static int rda_rtc_resume(struct platform_device *pdev)
> +{
> +	/* If alarms were left, we turn them off. */
> +	return rda_rtc_alarm_irq_enable(&pdev->dev, 0);
> +}

Let userspace enabling/disabling alarm, the kernel must not decide to
enable or disable them which fixes your TODO

> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(rda_rtc_pm_ops, rda_rtc_suspend, rda_rtc_resume);
> +
> +static const struct regmap_config regmap_config = {
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_stride = 4,
> +};
> +
> +static int rda_rtc_probe(struct platform_device *pdev)
> +{
> +	struct rda_rtc *rda_rtc;
> +	void __iomem *base;
> +
> +	rda_rtc = devm_kzalloc(&pdev->dev, sizeof(*rda_rtc), GFP_KERNEL);
> +	if (!rda_rtc)
> +		return -ENOMEM;
> +
> +	base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(base))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(base),
> +				"failed to remap resource\n");
> +
> +	rda_rtc->regmap = devm_regmap_init_mmio(&pdev->dev, base, &regmap_config);
> +	if (!rda_rtc->regmap)
> +		return dev_err_probe(&pdev->dev, PTR_ERR(rda_rtc->regmap),
> +				"can't find regmap\n");
> +
> +	rda_rtc->rtc_dev = devm_rtc_allocate_device(&pdev->dev);
> +	if (IS_ERR(rda_rtc->rtc_dev))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(rda_rtc->rtc_dev),
> +				"failed to allocate rtc device\n");
> +
> +	rda_rtc->rtc_dev->ops = &rda_rtc_ops;
> +	rda_rtc->rtc_dev->range_min = RTC_TIMESTAMP_BEGIN_2000;
> +	rda_rtc->rtc_dev->range_max = RTC_TIMESTAMP_END_2127;
> +
> +	platform_set_drvdata(pdev, rda_rtc);
> +
> +	return devm_rtc_register_device(rda_rtc->rtc_dev);
> +}
> +
> +static const struct of_device_id rda_rtc_id_table[] = {
> +	{ .compatible = "rda,8810pl-rtc", },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, rda_rtc_id_table);
> +
> +static struct platform_driver rda_rtc_driver = {
> +	.probe = rda_rtc_probe,
> +	.driver = {
> +		.name = "rtc-rda",
> +		.pm = &rda_rtc_pm_ops,
> +		.of_match_table = rda_rtc_id_table,
> +	},
> +};
> +module_platform_driver(rda_rtc_driver);
> +
> +MODULE_AUTHOR("Dang Huynh <dang.huynh@mainlining.org>");
> +MODULE_DESCRIPTION("RDA Micro RTC driver");
> +MODULE_LICENSE("GPL");
> 
> -- 
> 2.51.0
> 
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH 06/25] rtc: Add driver for RDA Micro SoC
Posted by kernel test robot 4 months, 3 weeks ago
Hi Dang,

kernel test robot noticed the following build errors:

[auto build test ERROR on 590b221ed4256fd6c34d3dea77aa5bd6e741bbc1]

url:    https://github.com/intel-lab-lkp/linux/commits/Dang-Huynh-via-B4-Relay/ARM-dts-unisoc-rda8810pl-Add-label-to-GPIO-nodes/20250917-043025
base:   590b221ed4256fd6c34d3dea77aa5bd6e741bbc1
patch link:    https://lore.kernel.org/r/20250917-rda8810pl-drivers-v1-6-9ca9184ca977%40mainlining.org
patch subject: [PATCH 06/25] rtc: Add driver for RDA Micro SoC
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20250919/202509192152.OXdK6bpd-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250919/202509192152.OXdK6bpd-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509192152.OXdK6bpd-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/rtc/rtc-rda.c: In function 'rda_rtc_settime':
>> drivers/rtc/rtc-rda.c:67:15: error: implicit declaration of function 'FIELD_PREP' [-Wimplicit-function-declaration]
      67 |         low = FIELD_PREP(RDA_SEC_MASK, tm->tm_sec) |
         |               ^~~~~~~~~~
   drivers/rtc/rtc-rda.c: In function 'rda_rtc_readtime':
>> drivers/rtc/rtc-rda.c:128:22: error: implicit declaration of function 'FIELD_GET' [-Wimplicit-function-declaration]
     128 |         tm->tm_sec = FIELD_GET(RDA_SEC_MASK, low);
         |                      ^~~~~~~~~


vim +/FIELD_PREP +67 drivers/rtc/rtc-rda.c

    50	
    51	static int rda_rtc_settime(struct device *dev, struct rtc_time *tm)
    52	{
    53		struct rda_rtc *rtc = dev_get_drvdata(dev);
    54		u32 high, low;
    55		int ret;
    56	
    57		ret = rtc_valid_tm(tm);
    58		if (ret < 0)
    59			return ret;
    60	
    61		/*
    62		 * The number of years since 1900 in kernel,
    63		 * but it is defined since 2000 by HW.
    64		 * The number of mons' range is from 0 to 11 in kernel,
    65		 * but it is defined from 1 to 12 by HW.
    66		 */
  > 67		low = FIELD_PREP(RDA_SEC_MASK, tm->tm_sec) |
    68			FIELD_PREP(RDA_MIN_MASK, tm->tm_min) |
    69			FIELD_PREP(RDA_HRS_MASK, tm->tm_hour);
    70	
    71		high = FIELD_PREP(RDA_MDAY_MASK, tm->tm_mday) |
    72			FIELD_PREP(RDA_MON_MASK, tm->tm_mon + 1) |
    73			FIELD_PREP(RDA_YEAR_MASK, tm->tm_year - 100) |
    74			FIELD_PREP(RDA_WDAY_MASK, tm->tm_wday);
    75	
    76		ret = regmap_write(rtc->regmap, RDA_RTC_CAL_LOAD_LOW_REG, low);
    77		if (ret < 0) {
    78			dev_err(dev, "Failed to update RTC low register: %d\n", ret);
    79			return ret;
    80		}
    81	
    82		ret = regmap_write(rtc->regmap, RDA_RTC_CAL_LOAD_HIGH_REG, high);
    83		if (ret < 0) {
    84			dev_err(dev, "Failed to update RTC low register: %d\n", ret);
    85			return ret;
    86		}
    87	
    88		ret = regmap_update_bits(rtc->regmap, RDA_RTC_CMD_REG, RDA_RTC_CMD_CAL_LOAD, 1);
    89		if (ret < 0) {
    90			dev_err(dev, "Failed to update RTC cal load register: %d\n", ret);
    91			return ret;
    92		}
    93	
    94		return 0;
    95	}
    96	
    97	static int rda_rtc_readtime(struct device *dev, struct rtc_time *tm)
    98	{
    99		struct rda_rtc *rtc = dev_get_drvdata(dev);
   100		unsigned int high, low;
   101		int ret;
   102	
   103		/*
   104		 * Check if RTC data is valid.
   105		 *
   106		 * When this bit is set, it means the data in the RTC is invalid
   107		 * or not configured.
   108		 */
   109		ret = regmap_test_bits(rtc->regmap, RDA_RTC_STA_REG, RDA_RTC_STA_NOT_PROG);
   110		if (ret < 0) {
   111			dev_err(dev, "Failed to read RTC status: %d\n", ret);
   112			return ret;
   113		} else if (ret > 0)
   114			return -EINVAL;
   115	
   116		ret = regmap_read(rtc->regmap, RDA_RTC_CUR_LOAD_HIGH_REG, &high);
   117		if (ret) {
   118			dev_err(dev, "Failed to read RTC high reg: %d\n", ret);
   119			return ret;
   120		}
   121	
   122		ret = regmap_read(rtc->regmap, RDA_RTC_CUR_LOAD_LOW_REG, &low);
   123		if (ret) {
   124			dev_err(dev, "Failed to read RTC low reg: %d\n", ret);
   125			return ret;
   126		}
   127	
 > 128		tm->tm_sec = FIELD_GET(RDA_SEC_MASK, low);
   129		tm->tm_min = FIELD_GET(RDA_MIN_MASK, low);
   130		tm->tm_hour = FIELD_GET(RDA_HRS_MASK, low);
   131		tm->tm_mday = FIELD_GET(RDA_MDAY_MASK, high);
   132		tm->tm_mon = FIELD_GET(RDA_MON_MASK, high);
   133		tm->tm_year = FIELD_GET(RDA_YEAR_MASK, high);
   134		tm->tm_wday = FIELD_GET(RDA_WDAY_MASK, high);
   135	
   136		/*
   137		 * The number of years since 1900 in kernel,
   138		 * but it is defined since 2000 by HW.
   139		 */
   140		tm->tm_year += 100;
   141		/*
   142		 * The number of mons' range is from 0 to 11 in kernel,
   143		 * but it is defined from 1 to 12 by HW.
   144		 */
   145		tm->tm_mon -= 1;
   146	
   147		return 0;
   148	}
   149	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki