From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>

Add a RTC driver for NXP S32G2/S32G3 SoCs.

The RTC module is used to enable Suspend to RAM (STR) support
on NXP S32G2/S32G3 SoC based boards.
RTC tracks clock time during system suspend.

Signed-off-by: Bogdan Hamciuc <bogdan.hamciuc@nxp.com>
Signed-off-by: Bogdan-Gabriel Roman <bogdan-gabriel.roman@nxp.com>
Signed-off-by: Ghennadi Procopciuc <Ghennadi.Procopciuc@nxp.com>
Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
---
 drivers/rtc/Kconfig    |  10 +
 drivers/rtc/Makefile   |   1 +
 drivers/rtc/rtc-s32g.c | 689 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 700 insertions(+)
 create mode 100644 drivers/rtc/rtc-s32g.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 2a95b05982ad..552eecd78f88 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -2043,4 +2043,14 @@ config RTC_DRV_SSD202D
 	  This driver can also be built as a module, if so, the module
 	  will be called "rtc-ssd20xd".
 
+config RTC_DRV_S32G
+	tristate "RTC driver for S32G2/S32G3 SoCs"
+	depends on ARCH_S32 || COMPILE_TEST
+	help
+	  Say yes to enable RTC driver for platforms based on the
+	  S32G2/S32G3 SoC family.
+
+	  This RTC module can be used as a wakeup source.
+	  Please note that it is not battery-powered.
+
 endif # RTC_CLASS
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 3004e372f25f..36d2ed5d0ae2 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -157,6 +157,7 @@ obj-$(CONFIG_RTC_DRV_RX8025)	+= rtc-rx8025.o
 obj-$(CONFIG_RTC_DRV_RX8111)	+= rtc-rx8111.o
 obj-$(CONFIG_RTC_DRV_RX8581)	+= rtc-rx8581.o
 obj-$(CONFIG_RTC_DRV_RZN1)	+= rtc-rzn1.o
+obj-$(CONFIG_RTC_DRV_S32G)	+= rtc-s32g.o
 obj-$(CONFIG_RTC_DRV_S35390A)	+= rtc-s35390a.o
 obj-$(CONFIG_RTC_DRV_S3C)	+= rtc-s3c.o
 obj-$(CONFIG_RTC_DRV_S5M)	+= rtc-s5m.o
diff --git a/drivers/rtc/rtc-s32g.c b/drivers/rtc/rtc-s32g.c
new file mode 100644
index 000000000000..e77ff0c065ba
--- /dev/null
+++ b/drivers/rtc/rtc-s32g.c
@@ -0,0 +1,689 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright 2024 NXP
+ */
+
+#include <linux/clk.h>
+#include <linux/of_irq.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/rtc.h>
+#include <linux/iopoll.h>
+
+#define RTCSUPV_OFFSET	0x0ul
+#define RTCC_OFFSET	0x4ul
+#define RTCS_OFFSET	0x8ul
+#define RTCCNT_OFFSET	0xCul
+#define APIVAL_OFFSET	0x10ul
+#define RTCVAL_OFFSET	0x14ul
+
+/* RTCSUPV fields */
+#define RTCSUPV_SUPV		BIT(31)
+
+/* RTCC fields */
+#define RTCC_CNTEN		BIT(31)
+#define RTCC_RTCIE_SHIFT	30
+#define RTCC_RTCIE		BIT(RTCC_RTCIE_SHIFT)
+#define RTCC_ROVREN		BIT(28)
+#define RTCC_APIEN		BIT(15)
+#define RTCC_APIIE		BIT(14)
+#define RTCC_CLKSEL_MASK	GENMASK(13, 12)
+#define RTCC_CLKSEL(n)		(((n) << 12) & RTCC_CLKSEL_MASK)
+#define RTCC_DIV512EN		BIT(11)
+#define RTCC_DIV32EN		BIT(10)
+
+/* RTCS fields */
+#define RTCS_RTCF		BIT(29)
+#define RTCS_INV_RTC		BIT(18)
+#define RTCS_APIF		BIT(13)
+#define RTCS_ROVRF		BIT(10)
+
+#define ROLLOVER_VAL		0xFFFFFFFFULL
+#define RTC_SYNCH_TIMEOUT	(100 * USEC_PER_MSEC)
+
+/* Clock sources - usable with RTCC_CLKSEL */
+#define S32G_RTC_SOURCE_SIRC	0x0
+#define S32G_RTC_SOURCE_FIRC	0x2
+
+struct rtc_time_base {
+	s64 sec;
+	u64 cycles;
+	u64 rollovers;
+#ifdef CONFIG_PM_SLEEP
+	struct rtc_time tm;
+#endif
+};
+
+struct rtc_priv {
+	struct rtc_device *rdev;
+	struct device *dev;
+	u8 __iomem *rtc_base;
+	struct clk *firc;
+	struct clk *sirc;
+	struct clk *ipg;
+	struct rtc_time_base base;
+	u64 rtc_hz;
+	u64 rollovers;
+	int dt_irq_id;
+	u8 clk_source;
+	bool div512;
+	bool div32;
+};
+
+static u64 cycles_to_sec(u64 hz, u64 cycles)
+{
+	return cycles / hz;
+}
+
+/*
+ * Convert a number of seconds to a value suitable for RTCVAL in our clock's
+ * current configuration.
+ * @rtcval: The value to go into RTCVAL[RTCVAL]
+ * Returns: 0 for success, -EINVAL if @seconds push the counter at least
+ *          twice the rollover interval
+ */
+static int sec_to_rtcval(const struct rtc_priv *priv,
+			 unsigned long seconds, u32 *rtcval)
+{
+	u32 rtccnt, delta_cnt;
+	u32 target_cnt = 0;
+
+	/* For now, support at most one rollover of the counter */
+	if (!seconds || seconds > cycles_to_sec(priv->rtc_hz, ULONG_MAX))
+		return -EINVAL;
+
+	/*
+	 * RTCCNT is read-only; we must return a value relative to the
+	 * current value of the counter (and hope we don't linger around
+	 * too much before we get to enable the interrupt)
+	 */
+	delta_cnt = seconds * priv->rtc_hz;
+	rtccnt = ioread32(priv->rtc_base + RTCCNT_OFFSET);
+
+	if (~rtccnt < delta_cnt)
+		target_cnt = (delta_cnt - ~rtccnt);
+	else
+		target_cnt = rtccnt + delta_cnt;
+
+	/*
+	 * According to RTCVAL register description,
+	 * its minimum value should be 4.
+	 */
+	if (unlikely(target_cnt < 4))
+		target_cnt = 4;
+
+	*rtcval = target_cnt;
+
+	return 0;
+}
+
+static irqreturn_t rtc_handler(int irq, void *dev)
+{
+	struct rtc_priv *priv = platform_get_drvdata(dev);
+	u32 status;
+
+	status = ioread32(priv->rtc_base + RTCS_OFFSET);
+	if (status & RTCS_ROVRF) {
+		if (priv->rollovers == ULONG_MAX)
+			priv->rollovers = 0;
+		else
+			priv->rollovers++;
+	}
+
+	if (status & RTCS_RTCF) {
+		iowrite32(0x0, priv->rtc_base + RTCVAL_OFFSET);
+		rtc_update_irq(priv->rdev, 1, RTC_AF);
+	}
+
+	if (status & RTCS_APIF)
+		rtc_update_irq(priv->rdev, 1, RTC_PF);
+
+	iowrite32(status, priv->rtc_base + RTCS_OFFSET);
+
+	return IRQ_HANDLED;
+}
+
+static int get_time_left(struct device *dev, struct rtc_priv *priv,
+			 u32 *sec)
+{
+	u32 rtccnt = ioread32(priv->rtc_base + RTCCNT_OFFSET);
+	u32 rtcval = ioread32(priv->rtc_base + RTCVAL_OFFSET);
+
+	if (rtcval < rtccnt) {
+		dev_err(dev, "RTC timer expired before entering suspend\n");
+		return -EIO;
+	}
+
+	*sec = cycles_to_sec(priv->rtc_hz, rtcval - rtccnt);
+
+	return 0;
+}
+
+static int s32g_rtc_get_time_or_alrm(struct rtc_priv *priv,
+				     u32 offset)
+{
+	u64 cycles, sec, base_cycles;
+	u32 counter;
+
+	counter = ioread32(priv->rtc_base + offset);
+	cycles = priv->rollovers * ROLLOVER_VAL + counter;
+	base_cycles = priv->base.cycles + priv->base.rollovers * ROLLOVER_VAL;
+
+	if (cycles < base_cycles)
+		return -EINVAL;
+
+	cycles -= base_cycles;
+	sec = priv->base.sec + cycles_to_sec(priv->rtc_hz, cycles);
+
+	return sec;
+}
+
+static int s32g_rtc_read_time(struct device *dev,
+			      struct rtc_time *tm)
+{
+	struct rtc_priv *priv = dev_get_drvdata(dev);
+	u64 sec;
+
+	if (!tm)
+		return -EINVAL;
+
+	sec = s32g_rtc_get_time_or_alrm(priv, RTCCNT_OFFSET);
+	if (sec < 0)
+		return -EINVAL;
+
+	rtc_time64_to_tm(sec, tm);
+
+	return 0;
+}
+
+static int s32g_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct rtc_priv *priv = dev_get_drvdata(dev);
+	u32 rtcc, sec_left;
+	u64 sec;
+
+	if (!alrm)
+		return -EINVAL;
+
+	sec = s32g_rtc_get_time_or_alrm(priv, RTCVAL_OFFSET);
+	if (sec < 0)
+		return -EINVAL;
+
+	rtc_time64_to_tm(sec, &alrm->time);
+
+	rtcc = ioread32(priv->rtc_base + RTCC_OFFSET);
+	alrm->enabled = sec && (rtcc & RTCC_RTCIE);
+
+	alrm->pending = 0;
+	if (alrm->enabled && !get_time_left(dev, priv, &sec_left))
+		alrm->pending = !!sec_left;
+
+	return 0;
+}
+
+static int s32g_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
+{
+	struct rtc_priv *priv = dev_get_drvdata(dev);
+	u32 rtcc;
+
+	if (!priv->dt_irq_id)
+		return -EIO;
+
+	/*
+	 * RTCIE cannot be deasserted because it will also disable the
+	 * rollover interrupt.
+	 */
+	rtcc = ioread32(priv->rtc_base + RTCC_OFFSET);
+	if (enabled)
+		rtcc |= RTCC_RTCIE;
+
+	iowrite32(rtcc, priv->rtc_base + RTCC_OFFSET);
+
+	return 0;
+}
+
+static int s32g_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct rtc_priv *priv = dev_get_drvdata(dev);
+	struct rtc_time time_crt;
+	long long t_crt, t_alrm;
+	int ret = 0;
+	u32 rtcval, rtcs;
+
+	iowrite32(0x0, priv->rtc_base + RTCVAL_OFFSET);
+
+	t_alrm = rtc_tm_to_time64(&alrm->time);
+
+	/*
+	 * Assuming the alarm is being set relative to the same time
+	 * returned by our s32g_rtc_read_time callback
+	 */
+	ret = s32g_rtc_read_time(dev, &time_crt);
+	if (ret)
+		return ret;
+
+	t_crt = rtc_tm_to_time64(&time_crt);
+	if (t_alrm <= t_crt) {
+		dev_warn(dev, "Alarm is set in the past\n");
+		return -EINVAL;
+	}
+
+	ret = sec_to_rtcval(priv, t_alrm - t_crt, &rtcval);
+	if (ret) {
+		dev_warn(dev, "Alarm is set too far in the future\n");
+		return ret;
+	}
+
+	ret = read_poll_timeout(ioread32, rtcs, !(rtcs & RTCS_INV_RTC),
+				0, RTC_SYNCH_TIMEOUT, false, priv->rtc_base + RTCS_OFFSET);
+	if (ret) {
+		dev_err(dev, "Synchronization failed\n");
+		return ret;
+	}
+
+	iowrite32(rtcval, priv->rtc_base + RTCVAL_OFFSET);
+
+	return 0;
+}
+
+static int s32g_rtc_set_time(struct device *dev,
+			     struct rtc_time *time)
+{
+	struct rtc_priv *priv = dev_get_drvdata(dev);
+
+	if (!time)
+		return -EINVAL;
+
+	priv->base.rollovers = priv->rollovers;
+	priv->base.cycles = ioread32(priv->rtc_base + RTCCNT_OFFSET);
+	priv->base.sec = rtc_tm_to_time64(time);
+
+	return 0;
+}
+
+static const struct rtc_class_ops rtc_ops = {
+	.read_time = s32g_rtc_read_time,
+	.set_time = s32g_rtc_set_time,
+	.read_alarm = s32g_rtc_read_alarm,
+	.set_alarm = s32g_rtc_set_alarm,
+	.alarm_irq_enable = s32g_rtc_alarm_irq_enable,
+};
+
+static void rtc_disable(struct rtc_priv *priv)
+{
+	u32 rtcc = ioread32(priv->rtc_base + RTCC_OFFSET);
+
+	rtcc &= ~RTCC_CNTEN;
+	iowrite32(rtcc, priv->rtc_base + RTCC_OFFSET);
+}
+
+static void rtc_enable(struct rtc_priv *priv)
+{
+	u32 rtcc = ioread32(priv->rtc_base + RTCC_OFFSET);
+
+	rtcc |= RTCC_CNTEN;
+	iowrite32(rtcc, priv->rtc_base + RTCC_OFFSET);
+}
+
+static int rtc_init(struct rtc_priv *priv)
+{
+	struct device *dev = priv->dev;
+	struct clk *sclk;
+	u32 rtcc = 0;
+	u32 clksel;
+	int ret;
+
+	ret = clk_prepare_enable(priv->ipg);
+	if (ret) {
+		dev_err(dev, "Cannot enable 'ipg' clock, error: %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(priv->sirc);
+	if (ret) {
+		dev_err(dev, "Cannot enable 'sirc' clock, error: %d\n", ret);
+		goto disable_ipg_clk;
+	}
+
+	ret = clk_prepare_enable(priv->firc);
+	if (ret) {
+		dev_err(dev, "Cannot enable 'firc' clock, error: %d\n", ret);
+		goto disable_sirc_clk;
+	}
+
+	/* Make sure the RTC ticking is disabled before we configure dividers */
+	rtc_disable(priv);
+
+	clksel = RTCC_CLKSEL(priv->clk_source);
+	rtcc |= clksel;
+
+	/* Precompute the base frequency of the clock */
+	switch (clksel) {
+	case RTCC_CLKSEL(S32G_RTC_SOURCE_SIRC):
+		sclk = priv->sirc;
+		break;
+	case RTCC_CLKSEL(S32G_RTC_SOURCE_FIRC):
+		sclk = priv->firc;
+		break;
+	default:
+		dev_err(dev, "Invalid clksel value: %u\n", clksel);
+		ret = -EINVAL;
+		goto disable_firc_clk;
+	}
+
+	priv->rtc_hz = clk_get_rate(sclk);
+	if (!priv->rtc_hz) {
+		dev_err(dev, "Invalid RTC frequency\n");
+		ret = -EINVAL;
+		goto disable_firc_clk;
+	}
+
+	/* Adjust frequency if dividers are enabled */
+	if (priv->div512) {
+		rtcc |= RTCC_DIV512EN;
+		priv->rtc_hz /= 512;
+	}
+
+	if (priv->div32) {
+		rtcc |= RTCC_DIV32EN;
+		priv->rtc_hz /= 32;
+	}
+
+	rtcc |= RTCC_RTCIE | RTCC_ROVREN;
+	iowrite32(rtcc, priv->rtc_base + RTCC_OFFSET);
+
+	return 0;
+
+disable_firc_clk:
+	clk_disable_unprepare(priv->firc);
+disable_sirc_clk:
+	clk_disable_unprepare(priv->sirc);
+disable_ipg_clk:
+	clk_disable_unprepare(priv->ipg);
+	return ret;
+}
+
+static int priv_dts_init(struct rtc_priv *priv)
+{
+	struct device *dev = priv->dev;
+	struct platform_device *pdev = to_platform_device(dev);
+	u32 clksel = S32G_RTC_SOURCE_SIRC;
+	/* div512 and div32 */
+	u32 div[2] = { 0 };
+	int ret;
+
+	priv->sirc = devm_clk_get(dev, "sirc");
+	if (IS_ERR(priv->sirc)) {
+		dev_err(dev, "Failed to get 'sirc' clock\n");
+		return PTR_ERR(priv->sirc);
+	}
+
+	priv->firc = devm_clk_get(dev, "firc");
+	if (IS_ERR(priv->firc)) {
+		dev_err(dev, "Failed to get 'firc' clock\n");
+		return PTR_ERR(priv->firc);
+	}
+
+	priv->ipg = devm_clk_get(dev, "ipg");
+	if (IS_ERR(priv->ipg)) {
+		dev_err(dev, "Failed to get 'ipg' clock\n");
+		return PTR_ERR(priv->ipg);
+	}
+
+	priv->dt_irq_id = platform_get_irq(pdev, 0);
+	if (priv->dt_irq_id < 0) {
+		dev_err(dev, "Error reading interrupt # from dts\n");
+		return priv->dt_irq_id;
+	}
+
+	ret = device_property_read_u32_array(dev, "nxp,dividers", div,
+					     ARRAY_SIZE(div));
+	if (ret) {
+		dev_err(dev, "Error reading dividers configuration, err: %d\n", ret);
+		return ret;
+	}
+
+	ret = device_property_read_u32(dev, "nxp,clksel", &clksel);
+	if (ret) {
+		dev_err(dev, "Error reading clksel configuration, err: %d\n", ret);
+		return ret;
+	}
+
+	priv->div512 = !!div[0];
+	priv->div32 = !!div[1];
+
+	switch (clksel) {
+	case S32G_RTC_SOURCE_SIRC:
+	case S32G_RTC_SOURCE_FIRC:
+		priv->clk_source = clksel;
+		break;
+	default:
+		dev_err(dev, "Unsupported clksel: %d\n", clksel);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int rtc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rtc_priv *priv;
+	int ret = 0;
+
+	priv = devm_kzalloc(dev, sizeof(struct rtc_priv),
+			    GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->rtc_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(priv->rtc_base)) {
+		dev_err(dev, "Failed to map registers\n");
+		return PTR_ERR(priv->rtc_base);
+	}
+
+	device_init_wakeup(dev, true);
+	priv->dev = dev;
+
+	ret = priv_dts_init(priv);
+	if (ret)
+		return ret;
+
+	ret = rtc_init(priv);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, priv);
+	rtc_enable(priv);
+
+	priv->rdev = devm_rtc_device_register(dev, "s32g_rtc",
+					      &rtc_ops, THIS_MODULE);
+	if (IS_ERR_OR_NULL(priv->rdev)) {
+		dev_err(dev, "Error registering RTC device, err: %ld\n",
+			PTR_ERR(priv->rdev));
+		ret = PTR_ERR(priv->rdev);
+		goto disable_rtc;
+	}
+
+	ret = devm_request_irq(dev, priv->dt_irq_id,
+			       rtc_handler, 0, dev_name(dev), pdev);
+	if (ret) {
+		dev_err(&pdev->dev, "Request interrupt %d failed, error: %d\n",
+			priv->dt_irq_id, ret);
+		goto disable_rtc;
+	}
+
+	return 0;
+
+disable_rtc:
+	rtc_disable(priv);
+	return ret;
+}
+
+static void rtc_remove(struct platform_device *pdev)
+{
+	struct rtc_priv *priv = platform_get_drvdata(pdev);
+
+	rtc_disable(priv);
+}
+
+#ifdef CONFIG_PM_SLEEP
+static void enable_api_irq(struct device *dev, unsigned int enabled)
+{
+	struct rtc_priv *priv = dev_get_drvdata(dev);
+	u32 api_irq = RTCC_APIEN | RTCC_APIIE;
+	u32 rtcc;
+
+	rtcc = ioread32(priv->rtc_base + RTCC_OFFSET);
+	if (enabled)
+		rtcc |= api_irq;
+	else
+		rtcc &= ~api_irq;
+	iowrite32(rtcc, priv->rtc_base + RTCC_OFFSET);
+}
+
+static int adjust_dividers(u32 sec, struct rtc_priv *priv)
+{
+	u64 rtcval_max = U32_MAX;
+	u64 rtcval;
+
+	priv->div32 = 0;
+	priv->div512 = 0;
+
+	rtcval = sec * priv->rtc_hz;
+	if (rtcval < rtcval_max)
+		return 0;
+
+	if (rtcval / 32 < rtcval_max) {
+		priv->div32 = 1;
+		return 0;
+	}
+
+	if (rtcval / 512 < rtcval_max) {
+		priv->div512 = 1;
+		return 0;
+	}
+
+	if (rtcval / (512 * 32) < rtcval_max) {
+		priv->div32 = 1;
+		priv->div512 = 1;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static int rtc_suspend(struct device *dev)
+{
+	struct rtc_priv *init_priv = dev_get_drvdata(dev);
+	struct rtc_priv priv;
+	long long base_sec;
+	int ret = 0;
+	u32 rtcval;
+	u32 sec;
+
+	if (!device_may_wakeup(dev))
+		return 0;
+
+	/* Save last known timestamp before we switch clocks and reinit RTC */
+	ret = s32g_rtc_read_time(dev, &priv.base.tm);
+	if (ret)
+		return ret;
+
+	if (init_priv->clk_source == S32G_RTC_SOURCE_SIRC)
+		return 0;
+
+	/*
+	 * Use a local copy of the RTC control block to
+	 * avoid restoring it on resume path.
+	 */
+	memcpy(&priv, init_priv, sizeof(priv));
+
+	/* Switch to SIRC */
+	priv.clk_source = S32G_RTC_SOURCE_SIRC;
+
+	ret = get_time_left(dev, init_priv, &sec);
+	if (ret)
+		return ret;
+
+	/* Adjust for the number of seconds we'll be asleep */
+	base_sec = rtc_tm_to_time64(&init_priv->base.tm);
+	base_sec += sec;
+	rtc_time64_to_tm(base_sec, &init_priv->base.tm);
+
+	rtc_disable(&priv);
+
+	ret = adjust_dividers(sec, &priv);
+	if (ret) {
+		dev_err(dev, "Failed to adjust RTC dividers to match a %u seconds delay\n", sec);
+		return ret;
+	}
+
+	ret = rtc_init(&priv);
+	if (ret)
+		return ret;
+
+	ret = sec_to_rtcval(&priv, sec, &rtcval);
+	if (ret) {
+		dev_warn(dev, "Alarm is too far in the future\n");
+		return ret;
+	}
+
+	s32g_rtc_alarm_irq_enable(dev, 0);
+	enable_api_irq(dev, 1);
+	iowrite32(rtcval, priv.rtc_base + APIVAL_OFFSET);
+	iowrite32(0, priv.rtc_base + RTCVAL_OFFSET);
+
+	rtc_enable(&priv);
+
+	return ret;
+}
+
+static int rtc_resume(struct device *dev)
+{
+	struct rtc_priv *priv = dev_get_drvdata(dev);
+	int ret;
+
+	if (!device_may_wakeup(dev))
+		return 0;
+
+	/* Disable wake-up interrupts */
+	enable_api_irq(dev, 0);
+
+	/* Reinitialize the driver using the initial settings */
+	ret = rtc_init(priv);
+	if (ret)
+		return ret;
+
+	rtc_enable(priv);
+
+	/*
+	 * Now RTCCNT has just been reset, and is out of sync with priv->base;
+	 * reapply the saved time settings
+	 */
+	return s32g_rtc_set_time(dev, &priv->base.tm);
+}
+#endif /* CONFIG_PM_SLEEP */
+
+static const struct of_device_id rtc_dt_ids[] = {
+	{.compatible = "nxp,s32g-rtc" },
+	{ /* sentinel */ },
+};
+
+static SIMPLE_DEV_PM_OPS(rtc_pm_ops,
+			 rtc_suspend, rtc_resume);
+
+static struct platform_driver rtc_driver = {
+	.driver		= {
+		.name			= "s32g-rtc",
+		.pm				= &rtc_pm_ops,
+		.of_match_table = of_match_ptr(rtc_dt_ids),
+	},
+	.probe		= rtc_probe,
+	.remove_new	= rtc_remove,
+};
+module_platform_driver(rtc_driver);
+
+MODULE_AUTHOR("NXP");
+MODULE_DESCRIPTION("NXP RTC driver for S32G2/S32G3");
+MODULE_LICENSE("GPL");
-- 
2.45.2

On 11/09/2024 10:00:26+0300, Ciprian Costea wrote:
> +struct rtc_time_base {
> +	s64 sec;
> +	u64 cycles;
> +	u64 rollovers;

The whole rollovers implementation seems useless as the rollover may
happen when Linux is off.

> +#ifdef CONFIG_PM_SLEEP
> +	struct rtc_time tm;
> +#endif
> +};
> +
> +struct rtc_priv {
> +	struct rtc_device *rdev;
> +	struct device *dev;
> +	u8 __iomem *rtc_base;
> +	struct clk *firc;
> +	struct clk *sirc;
> +	struct clk *ipg;
> +	struct rtc_time_base base;
> +	u64 rtc_hz;
> +	u64 rollovers;
> +	int dt_irq_id;
> +	u8 clk_source;
> +	bool div512;
> +	bool div32;
> +};
> +
> +static u64 cycles_to_sec(u64 hz, u64 cycles)
> +{
> +	return cycles / hz;
> +}
> +
> +/*
> + * Convert a number of seconds to a value suitable for RTCVAL in our clock's
> + * current configuration.
> + * @rtcval: The value to go into RTCVAL[RTCVAL]
> + * Returns: 0 for success, -EINVAL if @seconds push the counter at least
> + *          twice the rollover interval
> + */
> +static int sec_to_rtcval(const struct rtc_priv *priv,
> +			 unsigned long seconds, u32 *rtcval)
> +{
> +	u32 rtccnt, delta_cnt;
> +	u32 target_cnt = 0;
> +
> +	/* For now, support at most one rollover of the counter */
> +	if (!seconds || seconds > cycles_to_sec(priv->rtc_hz, ULONG_MAX))
> +		return -EINVAL;
> +
> +	/*
> +	 * RTCCNT is read-only; we must return a value relative to the
> +	 * current value of the counter (and hope we don't linger around
> +	 * too much before we get to enable the interrupt)
> +	 */
> +	delta_cnt = seconds * priv->rtc_hz;
> +	rtccnt = ioread32(priv->rtc_base + RTCCNT_OFFSET);
> +
> +	if (~rtccnt < delta_cnt)
> +		target_cnt = (delta_cnt - ~rtccnt);
> +	else
> +		target_cnt = rtccnt + delta_cnt;
> +
> +	/*
> +	 * According to RTCVAL register description,
> +	 * its minimum value should be 4.
> +	 */
> +	if (unlikely(target_cnt < 4))
> +		target_cnt = 4;
> +
> +	*rtcval = target_cnt;
> +
> +	return 0;
> +}
> +
> +static irqreturn_t rtc_handler(int irq, void *dev)
> +{
> +	struct rtc_priv *priv = platform_get_drvdata(dev);
> +	u32 status;
> +
> +	status = ioread32(priv->rtc_base + RTCS_OFFSET);
> +	if (status & RTCS_ROVRF) {
> +		if (priv->rollovers == ULONG_MAX)
> +			priv->rollovers = 0;
> +		else
> +			priv->rollovers++;
> +	}
> +
> +	if (status & RTCS_RTCF) {
> +		iowrite32(0x0, priv->rtc_base + RTCVAL_OFFSET);
> +		rtc_update_irq(priv->rdev, 1, RTC_AF);
> +	}
> +
> +	if (status & RTCS_APIF)
> +		rtc_update_irq(priv->rdev, 1, RTC_PF);
> +
> +	iowrite32(status, priv->rtc_base + RTCS_OFFSET);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int get_time_left(struct device *dev, struct rtc_priv *priv,
> +			 u32 *sec)
> +{
> +	u32 rtccnt = ioread32(priv->rtc_base + RTCCNT_OFFSET);
> +	u32 rtcval = ioread32(priv->rtc_base + RTCVAL_OFFSET);
> +
> +	if (rtcval < rtccnt) {
> +		dev_err(dev, "RTC timer expired before entering suspend\n");
> +		return -EIO;
> +	}
> +
> +	*sec = cycles_to_sec(priv->rtc_hz, rtcval - rtccnt);
> +
> +	return 0;
> +}
> +
> +static int s32g_rtc_get_time_or_alrm(struct rtc_priv *priv,
> +				     u32 offset)
> +{
> +	u64 cycles, sec, base_cycles;
> +	u32 counter;
> +
> +	counter = ioread32(priv->rtc_base + offset);
> +	cycles = priv->rollovers * ROLLOVER_VAL + counter;
> +	base_cycles = priv->base.cycles + priv->base.rollovers * ROLLOVER_VAL;
> +
> +	if (cycles < base_cycles)
> +		return -EINVAL;
> +
> +	cycles -= base_cycles;
> +	sec = priv->base.sec + cycles_to_sec(priv->rtc_hz, cycles);

What happens after you reboot?

This doesn't seem to be a proper RTC, unless you have some NVMEM to
store the offset between the current time and the value of the counter.
As-is, the driver is not working.

> +
> +	return sec;
> +}
> +
> +static int s32g_rtc_read_time(struct device *dev,
> +			      struct rtc_time *tm)
> +{
> +	struct rtc_priv *priv = dev_get_drvdata(dev);
> +	u64 sec;
> +
> +	if (!tm)
> +		return -EINVAL;
> +
> +	sec = s32g_rtc_get_time_or_alrm(priv, RTCCNT_OFFSET);
> +	if (sec < 0)
> +		return -EINVAL;
> +
> +	rtc_time64_to_tm(sec, tm);
> +
> +	return 0;
> +}
> +
> +static int s32g_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> +{
> +	struct rtc_priv *priv = dev_get_drvdata(dev);
> +	u32 rtcc, sec_left;
> +	u64 sec;
> +
> +	if (!alrm)
> +		return -EINVAL;
> +
> +	sec = s32g_rtc_get_time_or_alrm(priv, RTCVAL_OFFSET);
> +	if (sec < 0)
> +		return -EINVAL;
> +
> +	rtc_time64_to_tm(sec, &alrm->time);
> +
> +	rtcc = ioread32(priv->rtc_base + RTCC_OFFSET);
> +	alrm->enabled = sec && (rtcc & RTCC_RTCIE);
> +
> +	alrm->pending = 0;
> +	if (alrm->enabled && !get_time_left(dev, priv, &sec_left))
> +		alrm->pending = !!sec_left;
> +
> +	return 0;
> +}
> +
> +static int s32g_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
> +{
> +	struct rtc_priv *priv = dev_get_drvdata(dev);
> +	u32 rtcc;
> +
> +	if (!priv->dt_irq_id)
> +		return -EIO;
> +
> +	/*
> +	 * RTCIE cannot be deasserted because it will also disable the
> +	 * rollover interrupt.
> +	 */
> +	rtcc = ioread32(priv->rtc_base + RTCC_OFFSET);
> +	if (enabled)
> +		rtcc |= RTCC_RTCIE;
> +
> +	iowrite32(rtcc, priv->rtc_base + RTCC_OFFSET);
> +
> +	return 0;
> +}
> +
> +static int s32g_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> +{
> +	struct rtc_priv *priv = dev_get_drvdata(dev);
> +	struct rtc_time time_crt;
> +	long long t_crt, t_alrm;
> +	int ret = 0;
> +	u32 rtcval, rtcs;
> +
> +	iowrite32(0x0, priv->rtc_base + RTCVAL_OFFSET);
> +
> +	t_alrm = rtc_tm_to_time64(&alrm->time);
> +
> +	/*
> +	 * Assuming the alarm is being set relative to the same time
> +	 * returned by our s32g_rtc_read_time callback
> +	 */
> +	ret = s32g_rtc_read_time(dev, &time_crt);
> +	if (ret)
> +		return ret;
> +
> +	t_crt = rtc_tm_to_time64(&time_crt);
> +	if (t_alrm <= t_crt) {
> +		dev_warn(dev, "Alarm is set in the past\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = sec_to_rtcval(priv, t_alrm - t_crt, &rtcval);
> +	if (ret) {
> +		dev_warn(dev, "Alarm is set too far in the future\n");
> +		return ret;
> +	}
> +
> +	ret = read_poll_timeout(ioread32, rtcs, !(rtcs & RTCS_INV_RTC),
> +				0, RTC_SYNCH_TIMEOUT, false, priv->rtc_base + RTCS_OFFSET);
> +	if (ret) {
> +		dev_err(dev, "Synchronization failed\n");
> +		return ret;
> +	}
> +
> +	iowrite32(rtcval, priv->rtc_base + RTCVAL_OFFSET);
> +
> +	return 0;
> +}
> +
> +static int s32g_rtc_set_time(struct device *dev,
> +			     struct rtc_time *time)
> +{
> +	struct rtc_priv *priv = dev_get_drvdata(dev);
> +
> +	if (!time)
> +		return -EINVAL;
> +
> +	priv->base.rollovers = priv->rollovers;
> +	priv->base.cycles = ioread32(priv->rtc_base + RTCCNT_OFFSET);
> +	priv->base.sec = rtc_tm_to_time64(time);
> +
> +	return 0;
> +}
> +
> +static const struct rtc_class_ops rtc_ops = {
> +	.read_time = s32g_rtc_read_time,
> +	.set_time = s32g_rtc_set_time,
> +	.read_alarm = s32g_rtc_read_alarm,
> +	.set_alarm = s32g_rtc_set_alarm,
> +	.alarm_irq_enable = s32g_rtc_alarm_irq_enable,
> +};
> +
> +static void rtc_disable(struct rtc_priv *priv)
> +{
> +	u32 rtcc = ioread32(priv->rtc_base + RTCC_OFFSET);
> +
> +	rtcc &= ~RTCC_CNTEN;
> +	iowrite32(rtcc, priv->rtc_base + RTCC_OFFSET);
> +}
> +
> +static void rtc_enable(struct rtc_priv *priv)
> +{
> +	u32 rtcc = ioread32(priv->rtc_base + RTCC_OFFSET);
> +
> +	rtcc |= RTCC_CNTEN;
> +	iowrite32(rtcc, priv->rtc_base + RTCC_OFFSET);
> +}
> +
> +static int rtc_init(struct rtc_priv *priv)
> +{
> +	struct device *dev = priv->dev;
> +	struct clk *sclk;
> +	u32 rtcc = 0;
> +	u32 clksel;
> +	int ret;
> +
> +	ret = clk_prepare_enable(priv->ipg);
> +	if (ret) {
> +		dev_err(dev, "Cannot enable 'ipg' clock, error: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(priv->sirc);
> +	if (ret) {
> +		dev_err(dev, "Cannot enable 'sirc' clock, error: %d\n", ret);
> +		goto disable_ipg_clk;
> +	}
> +
> +	ret = clk_prepare_enable(priv->firc);
> +	if (ret) {
> +		dev_err(dev, "Cannot enable 'firc' clock, error: %d\n", ret);
> +		goto disable_sirc_clk;
> +	}
> +
> +	/* Make sure the RTC ticking is disabled before we configure dividers */
> +	rtc_disable(priv);

At this point, you lost the RTC accuracy, this must not be done on probe
> +
> +	clksel = RTCC_CLKSEL(priv->clk_source);
> +	rtcc |= clksel;
> +
> +	/* Precompute the base frequency of the clock */
> +	switch (clksel) {
> +	case RTCC_CLKSEL(S32G_RTC_SOURCE_SIRC):
> +		sclk = priv->sirc;
> +		break;
> +	case RTCC_CLKSEL(S32G_RTC_SOURCE_FIRC):
> +		sclk = priv->firc;
> +		break;
> +	default:
> +		dev_err(dev, "Invalid clksel value: %u\n", clksel);
> +		ret = -EINVAL;
> +		goto disable_firc_clk;
> +	}
> +
> +	priv->rtc_hz = clk_get_rate(sclk);
> +	if (!priv->rtc_hz) {
> +		dev_err(dev, "Invalid RTC frequency\n");
> +		ret = -EINVAL;
> +		goto disable_firc_clk;
> +	}
> +
> +	/* Adjust frequency if dividers are enabled */
> +	if (priv->div512) {
> +		rtcc |= RTCC_DIV512EN;
> +		priv->rtc_hz /= 512;
> +	}
> +
> +	if (priv->div32) {
> +		rtcc |= RTCC_DIV32EN;
> +		priv->rtc_hz /= 32;
> +	}
> +
> +	rtcc |= RTCC_RTCIE | RTCC_ROVREN;
> +	iowrite32(rtcc, priv->rtc_base + RTCC_OFFSET);
> +
> +	return 0;
> +
> +disable_firc_clk:
> +	clk_disable_unprepare(priv->firc);
> +disable_sirc_clk:
> +	clk_disable_unprepare(priv->sirc);
> +disable_ipg_clk:
> +	clk_disable_unprepare(priv->ipg);
> +	return ret;
> +}
> +
> +static int priv_dts_init(struct rtc_priv *priv)
> +{
> +	struct device *dev = priv->dev;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	u32 clksel = S32G_RTC_SOURCE_SIRC;
> +	/* div512 and div32 */
> +	u32 div[2] = { 0 };
> +	int ret;
> +
> +	priv->sirc = devm_clk_get(dev, "sirc");
> +	if (IS_ERR(priv->sirc)) {
> +		dev_err(dev, "Failed to get 'sirc' clock\n");
> +		return PTR_ERR(priv->sirc);
> +	}
> +
> +	priv->firc = devm_clk_get(dev, "firc");
> +	if (IS_ERR(priv->firc)) {
> +		dev_err(dev, "Failed to get 'firc' clock\n");
> +		return PTR_ERR(priv->firc);
> +	}
> +
> +	priv->ipg = devm_clk_get(dev, "ipg");
> +	if (IS_ERR(priv->ipg)) {
> +		dev_err(dev, "Failed to get 'ipg' clock\n");
> +		return PTR_ERR(priv->ipg);
> +	}
> +
> +	priv->dt_irq_id = platform_get_irq(pdev, 0);
> +	if (priv->dt_irq_id < 0) {
> +		dev_err(dev, "Error reading interrupt # from dts\n");
> +		return priv->dt_irq_id;
> +	}
> +
> +	ret = device_property_read_u32_array(dev, "nxp,dividers", div,
> +					     ARRAY_SIZE(div));
> +	if (ret) {
> +		dev_err(dev, "Error reading dividers configuration, err: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = device_property_read_u32(dev, "nxp,clksel", &clksel);
> +	if (ret) {
> +		dev_err(dev, "Error reading clksel configuration, err: %d\n", ret);
> +		return ret;
> +	}
> +
> +	priv->div512 = !!div[0];
> +	priv->div32 = !!div[1];
> +
> +	switch (clksel) {
> +	case S32G_RTC_SOURCE_SIRC:
> +	case S32G_RTC_SOURCE_FIRC:
> +		priv->clk_source = clksel;
> +		break;
> +	default:
> +		dev_err(dev, "Unsupported clksel: %d\n", clksel);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int rtc_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct rtc_priv *priv;
> +	int ret = 0;
> +
> +	priv = devm_kzalloc(dev, sizeof(struct rtc_priv),
> +			    GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->rtc_base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(priv->rtc_base)) {
> +		dev_err(dev, "Failed to map registers\n");
> +		return PTR_ERR(priv->rtc_base);
> +	}
> +
> +	device_init_wakeup(dev, true);
> +	priv->dev = dev;
> +
> +	ret = priv_dts_init(priv);
> +	if (ret)
> +		return ret;
> +
> +	ret = rtc_init(priv);
> +	if (ret)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, priv);
> +	rtc_enable(priv);
> +
> +	priv->rdev = devm_rtc_device_register(dev, "s32g_rtc",
> +					      &rtc_ops, THIS_MODULE);

This is deprecated, please use devm_rtc_allocate_device and devm_rtc_register_device

> +	if (IS_ERR_OR_NULL(priv->rdev)) {
> +		dev_err(dev, "Error registering RTC device, err: %ld\n",
> +			PTR_ERR(priv->rdev));
> +		ret = PTR_ERR(priv->rdev);
> +		goto disable_rtc;
> +	}
> +
> +	ret = devm_request_irq(dev, priv->dt_irq_id,
> +			       rtc_handler, 0, dev_name(dev), pdev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Request interrupt %d failed, error: %d\n",
> +			priv->dt_irq_id, ret);
> +		goto disable_rtc;

You must not fail after registering the RTC, else your driver will be
opened to a race condition;

> +	}
> +
> +	return 0;
> +
> +disable_rtc:
> +	rtc_disable(priv);
> +	return ret;
> +}
> +
> +static void rtc_remove(struct platform_device *pdev)
> +{
> +	struct rtc_priv *priv = platform_get_drvdata(pdev);
> +
> +	rtc_disable(priv);

Disabling the RTC when shutting down renders the RTC useless.

> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static void enable_api_irq(struct device *dev, unsigned int enabled)
> +{
> +	struct rtc_priv *priv = dev_get_drvdata(dev);
> +	u32 api_irq = RTCC_APIEN | RTCC_APIIE;
> +	u32 rtcc;
> +
> +	rtcc = ioread32(priv->rtc_base + RTCC_OFFSET);
> +	if (enabled)
> +		rtcc |= api_irq;
> +	else
> +		rtcc &= ~api_irq;
> +	iowrite32(rtcc, priv->rtc_base + RTCC_OFFSET);
> +}
> +
> +static int adjust_dividers(u32 sec, struct rtc_priv *priv)
> +{
> +	u64 rtcval_max = U32_MAX;
> +	u64 rtcval;
> +
> +	priv->div32 = 0;
> +	priv->div512 = 0;
> +
> +	rtcval = sec * priv->rtc_hz;
> +	if (rtcval < rtcval_max)
> +		return 0;
> +
> +	if (rtcval / 32 < rtcval_max) {
> +		priv->div32 = 1;
> +		return 0;
> +	}
> +
> +	if (rtcval / 512 < rtcval_max) {
> +		priv->div512 = 1;
> +		return 0;
> +	}
> +
> +	if (rtcval / (512 * 32) < rtcval_max) {
> +		priv->div32 = 1;
> +		priv->div512 = 1;
> +		return 0;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int rtc_suspend(struct device *dev)
> +{
> +	struct rtc_priv *init_priv = dev_get_drvdata(dev);
> +	struct rtc_priv priv;
> +	long long base_sec;
> +	int ret = 0;
> +	u32 rtcval;
> +	u32 sec;
> +
> +	if (!device_may_wakeup(dev))
> +		return 0;
> +
> +	/* Save last known timestamp before we switch clocks and reinit RTC */
> +	ret = s32g_rtc_read_time(dev, &priv.base.tm);
> +	if (ret)
> +		return ret;
> +
> +	if (init_priv->clk_source == S32G_RTC_SOURCE_SIRC)
> +		return 0;
> +
> +	/*
> +	 * Use a local copy of the RTC control block to
> +	 * avoid restoring it on resume path.
> +	 */
> +	memcpy(&priv, init_priv, sizeof(priv));
> +
> +	/* Switch to SIRC */
> +	priv.clk_source = S32G_RTC_SOURCE_SIRC;
> +
> +	ret = get_time_left(dev, init_priv, &sec);
> +	if (ret)
> +		return ret;
> +
> +	/* Adjust for the number of seconds we'll be asleep */
> +	base_sec = rtc_tm_to_time64(&init_priv->base.tm);
> +	base_sec += sec;
> +	rtc_time64_to_tm(base_sec, &init_priv->base.tm);
> +
> +	rtc_disable(&priv);
> +
> +	ret = adjust_dividers(sec, &priv);
> +	if (ret) {
> +		dev_err(dev, "Failed to adjust RTC dividers to match a %u seconds delay\n", sec);
> +		return ret;
> +	}
> +
> +	ret = rtc_init(&priv);
> +	if (ret)
> +		return ret;
> +
> +	ret = sec_to_rtcval(&priv, sec, &rtcval);
> +	if (ret) {
> +		dev_warn(dev, "Alarm is too far in the future\n");
> +		return ret;
> +	}

All of this seems super fishy.

> +
> +	s32g_rtc_alarm_irq_enable(dev, 0);
> +	enable_api_irq(dev, 1);
> +	iowrite32(rtcval, priv.rtc_base + APIVAL_OFFSET);
> +	iowrite32(0, priv.rtc_base + RTCVAL_OFFSET);
> +
> +	rtc_enable(&priv);
> +
> +	return ret;
> +}
> +
> +static int rtc_resume(struct device *dev)
> +{
> +	struct rtc_priv *priv = dev_get_drvdata(dev);
> +	int ret;
> +
> +	if (!device_may_wakeup(dev))
> +		return 0;
> +
> +	/* Disable wake-up interrupts */
> +	enable_api_irq(dev, 0);
> +
> +	/* Reinitialize the driver using the initial settings */
> +	ret = rtc_init(priv);
> +	if (ret)
> +		return ret;
> +
> +	rtc_enable(priv);
> +
> +	/*
> +	 * Now RTCCNT has just been reset, and is out of sync with priv->base;
> +	 * reapply the saved time settings
> +	 */
> +	return s32g_rtc_set_time(dev, &priv->base.tm);
> +}
> +#endif /* CONFIG_PM_SLEEP */
> +
> +static const struct of_device_id rtc_dt_ids[] = {
> +	{.compatible = "nxp,s32g-rtc" },
> +	{ /* sentinel */ },
> +};
> +
> +static SIMPLE_DEV_PM_OPS(rtc_pm_ops,
> +			 rtc_suspend, rtc_resume);
> +
> +static struct platform_driver rtc_driver = {
> +	.driver		= {
> +		.name			= "s32g-rtc",
> +		.pm				= &rtc_pm_ops,
> +		.of_match_table = of_match_ptr(rtc_dt_ids),
> +	},
> +	.probe		= rtc_probe,
> +	.remove_new	= rtc_remove,
> +};
> +module_platform_driver(rtc_driver);
> +
> +MODULE_AUTHOR("NXP");
> +MODULE_DESCRIPTION("NXP RTC driver for S32G2/S32G3");
> +MODULE_LICENSE("GPL");
> -- 
> 2.45.2
> 
> 

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

On 9/18/2024 1:26 PM, Alexandre Belloni wrote:
> On 11/09/2024 10:00:26+0300, Ciprian Costea wrote:
>> +struct rtc_time_base {
>> +	s64 sec;
>> +	u64 cycles;
>> +	u64 rollovers;
> 
> The whole rollovers implementation seems useless as the rollover may
> happen when Linux is off.
>

Hello Alexandre,

Thank you for your valuable tehnical feedback on this RTC driver proposal.
I will try, to the best of my knowledge to address your concers.

Regarding the rollover implementation, the Hardware RTC module registers 
present of NXP S32G2/S32G3 SoCs are being reset during system reboot.
On the other hand, during suspend, the RTC module will keep counting if 
a clock source is available. In this case, an internal low-rate 
oscillator (32K -- SIRC) is used, or an external clock can be provided.

The meaning of 'rollover' in this driver refers to a field which will 
count the number of times the RTC counter transitions from value 
0xFFFFFFFF to 0x0. At a 52MHz counting rate (FIRC clock), this would 
mean once in around 18 seconds. Its value is used to report the time 
after multiple rollovers. This can be observed in function 
's32g_rtc_get_time_or_alrm'.

>> +#ifdef CONFIG_PM_SLEEP
>> +	struct rtc_time tm;
>> +#endif
>> +};
>> +
>> +struct rtc_priv {
>> +	struct rtc_device *rdev;
>> +	struct device *dev;
>> +	u8 __iomem *rtc_base;
>> +	struct clk *firc;
>> +	struct clk *sirc;
>> +	struct clk *ipg;
>> +	struct rtc_time_base base;
>> +	u64 rtc_hz;
>> +	u64 rollovers;
>> +	int dt_irq_id;
>> +	u8 clk_source;
>> +	bool div512;
>> +	bool div32;
>> +};
>> +
>> +static u64 cycles_to_sec(u64 hz, u64 cycles)
>> +{
>> +	return cycles / hz;
>> +}
>> +
>> +/*
>> + * Convert a number of seconds to a value suitable for RTCVAL in our clock's
>> + * current configuration.
>> + * @rtcval: The value to go into RTCVAL[RTCVAL]
>> + * Returns: 0 for success, -EINVAL if @seconds push the counter at least
>> + *          twice the rollover interval
>> + */
>> +static int sec_to_rtcval(const struct rtc_priv *priv,
>> +			 unsigned long seconds, u32 *rtcval)
>> +{
>> +	u32 rtccnt, delta_cnt;
>> +	u32 target_cnt = 0;
>> +
>> +	/* For now, support at most one rollover of the counter */
>> +	if (!seconds || seconds > cycles_to_sec(priv->rtc_hz, ULONG_MAX))
>> +		return -EINVAL;
>> +
>> +	/*
>> +	 * RTCCNT is read-only; we must return a value relative to the
>> +	 * current value of the counter (and hope we don't linger around
>> +	 * too much before we get to enable the interrupt)
>> +	 */
>> +	delta_cnt = seconds * priv->rtc_hz;
>> +	rtccnt = ioread32(priv->rtc_base + RTCCNT_OFFSET);
>> +
>> +	if (~rtccnt < delta_cnt)
>> +		target_cnt = (delta_cnt - ~rtccnt);
>> +	else
>> +		target_cnt = rtccnt + delta_cnt;
>> +
>> +	/*
>> +	 * According to RTCVAL register description,
>> +	 * its minimum value should be 4.
>> +	 */
>> +	if (unlikely(target_cnt < 4))
>> +		target_cnt = 4;
>> +
>> +	*rtcval = target_cnt;
>> +
>> +	return 0;
>> +}
>> +
>> +static irqreturn_t rtc_handler(int irq, void *dev)
>> +{
>> +	struct rtc_priv *priv = platform_get_drvdata(dev);
>> +	u32 status;
>> +
>> +	status = ioread32(priv->rtc_base + RTCS_OFFSET);
>> +	if (status & RTCS_ROVRF) {
>> +		if (priv->rollovers == ULONG_MAX)
>> +			priv->rollovers = 0;
>> +		else
>> +			priv->rollovers++;
>> +	}
>> +
>> +	if (status & RTCS_RTCF) {
>> +		iowrite32(0x0, priv->rtc_base + RTCVAL_OFFSET);
>> +		rtc_update_irq(priv->rdev, 1, RTC_AF);
>> +	}
>> +
>> +	if (status & RTCS_APIF)
>> +		rtc_update_irq(priv->rdev, 1, RTC_PF);
>> +
>> +	iowrite32(status, priv->rtc_base + RTCS_OFFSET);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int get_time_left(struct device *dev, struct rtc_priv *priv,
>> +			 u32 *sec)
>> +{
>> +	u32 rtccnt = ioread32(priv->rtc_base + RTCCNT_OFFSET);
>> +	u32 rtcval = ioread32(priv->rtc_base + RTCVAL_OFFSET);
>> +
>> +	if (rtcval < rtccnt) {
>> +		dev_err(dev, "RTC timer expired before entering suspend\n");
>> +		return -EIO;
>> +	}
>> +
>> +	*sec = cycles_to_sec(priv->rtc_hz, rtcval - rtccnt);
>> +
>> +	return 0;
>> +}
>> +
>> +static int s32g_rtc_get_time_or_alrm(struct rtc_priv *priv,
>> +				     u32 offset)
>> +{
>> +	u64 cycles, sec, base_cycles;
>> +	u32 counter;
>> +
>> +	counter = ioread32(priv->rtc_base + offset);
>> +	cycles = priv->rollovers * ROLLOVER_VAL + counter;
>> +	base_cycles = priv->base.cycles + priv->base.rollovers * ROLLOVER_VAL;
>> +
>> +	if (cycles < base_cycles)
>> +		return -EINVAL;
>> +
>> +	cycles -= base_cycles;
>> +	sec = priv->base.sec + cycles_to_sec(priv->rtc_hz, cycles);
> 
> What happens after you reboot?
> 
> This doesn't seem to be a proper RTC, unless you have some NVMEM to
> store the offset between the current time and the value of the counter.
> As-is, the driver is not working.
>

With the existing hardware on S32G2/S32G3, only parts of what Linux 
expects from an RTC can be implemented in software.

Please note that the RTC module is not battery-backed.
Would it be acceptable to return errors for the use cases that cannot be 
fullfilled by the driver and list its limitations in the Kconfig menu ?

In the end, the main goal of this driver is to represent a time-based 
wakeup source for the SoC.

>> +
>> +	return sec;
>> +}
>> +
>> +static int s32g_rtc_read_time(struct device *dev,
>> +			      struct rtc_time *tm)
>> +{
>> +	struct rtc_priv *priv = dev_get_drvdata(dev);
>> +	u64 sec;
>> +
>> +	if (!tm)
>> +		return -EINVAL;
>> +
>> +	sec = s32g_rtc_get_time_or_alrm(priv, RTCCNT_OFFSET);
>> +	if (sec < 0)
>> +		return -EINVAL;
>> +
>> +	rtc_time64_to_tm(sec, tm);
>> +
>> +	return 0;
>> +}
>> +
>> +static int s32g_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
>> +{
>> +	struct rtc_priv *priv = dev_get_drvdata(dev);
>> +	u32 rtcc, sec_left;
>> +	u64 sec;
>> +
>> +	if (!alrm)
>> +		return -EINVAL;
>> +
>> +	sec = s32g_rtc_get_time_or_alrm(priv, RTCVAL_OFFSET);
>> +	if (sec < 0)
>> +		return -EINVAL;
>> +
>> +	rtc_time64_to_tm(sec, &alrm->time);
>> +
>> +	rtcc = ioread32(priv->rtc_base + RTCC_OFFSET);
>> +	alrm->enabled = sec && (rtcc & RTCC_RTCIE);
>> +
>> +	alrm->pending = 0;
>> +	if (alrm->enabled && !get_time_left(dev, priv, &sec_left))
>> +		alrm->pending = !!sec_left;
>> +
>> +	return 0;
>> +}
>> +
>> +static int s32g_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
>> +{
>> +	struct rtc_priv *priv = dev_get_drvdata(dev);
>> +	u32 rtcc;
>> +
>> +	if (!priv->dt_irq_id)
>> +		return -EIO;
>> +
>> +	/*
>> +	 * RTCIE cannot be deasserted because it will also disable the
>> +	 * rollover interrupt.
>> +	 */
>> +	rtcc = ioread32(priv->rtc_base + RTCC_OFFSET);
>> +	if (enabled)
>> +		rtcc |= RTCC_RTCIE;
>> +
>> +	iowrite32(rtcc, priv->rtc_base + RTCC_OFFSET);
>> +
>> +	return 0;
>> +}
>> +
>> +static int s32g_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
>> +{
>> +	struct rtc_priv *priv = dev_get_drvdata(dev);
>> +	struct rtc_time time_crt;
>> +	long long t_crt, t_alrm;
>> +	int ret = 0;
>> +	u32 rtcval, rtcs;
>> +
>> +	iowrite32(0x0, priv->rtc_base + RTCVAL_OFFSET);
>> +
>> +	t_alrm = rtc_tm_to_time64(&alrm->time);
>> +
>> +	/*
>> +	 * Assuming the alarm is being set relative to the same time
>> +	 * returned by our s32g_rtc_read_time callback
>> +	 */
>> +	ret = s32g_rtc_read_time(dev, &time_crt);
>> +	if (ret)
>> +		return ret;
>> +
>> +	t_crt = rtc_tm_to_time64(&time_crt);
>> +	if (t_alrm <= t_crt) {
>> +		dev_warn(dev, "Alarm is set in the past\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	ret = sec_to_rtcval(priv, t_alrm - t_crt, &rtcval);
>> +	if (ret) {
>> +		dev_warn(dev, "Alarm is set too far in the future\n");
>> +		return ret;
>> +	}
>> +
>> +	ret = read_poll_timeout(ioread32, rtcs, !(rtcs & RTCS_INV_RTC),
>> +				0, RTC_SYNCH_TIMEOUT, false, priv->rtc_base + RTCS_OFFSET);
>> +	if (ret) {
>> +		dev_err(dev, "Synchronization failed\n");
>> +		return ret;
>> +	}
>> +
>> +	iowrite32(rtcval, priv->rtc_base + RTCVAL_OFFSET);
>> +
>> +	return 0;
>> +}
>> +
>> +static int s32g_rtc_set_time(struct device *dev,
>> +			     struct rtc_time *time)
>> +{
>> +	struct rtc_priv *priv = dev_get_drvdata(dev);
>> +
>> +	if (!time)
>> +		return -EINVAL;
>> +
>> +	priv->base.rollovers = priv->rollovers;
>> +	priv->base.cycles = ioread32(priv->rtc_base + RTCCNT_OFFSET);
>> +	priv->base.sec = rtc_tm_to_time64(time);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct rtc_class_ops rtc_ops = {
>> +	.read_time = s32g_rtc_read_time,
>> +	.set_time = s32g_rtc_set_time,
>> +	.read_alarm = s32g_rtc_read_alarm,
>> +	.set_alarm = s32g_rtc_set_alarm,
>> +	.alarm_irq_enable = s32g_rtc_alarm_irq_enable,
>> +};
>> +
>> +static void rtc_disable(struct rtc_priv *priv)
>> +{
>> +	u32 rtcc = ioread32(priv->rtc_base + RTCC_OFFSET);
>> +
>> +	rtcc &= ~RTCC_CNTEN;
>> +	iowrite32(rtcc, priv->rtc_base + RTCC_OFFSET);
>> +}
>> +
>> +static void rtc_enable(struct rtc_priv *priv)
>> +{
>> +	u32 rtcc = ioread32(priv->rtc_base + RTCC_OFFSET);
>> +
>> +	rtcc |= RTCC_CNTEN;
>> +	iowrite32(rtcc, priv->rtc_base + RTCC_OFFSET);
>> +}
>> +
>> +static int rtc_init(struct rtc_priv *priv)
>> +{
>> +	struct device *dev = priv->dev;
>> +	struct clk *sclk;
>> +	u32 rtcc = 0;
>> +	u32 clksel;
>> +	int ret;
>> +
>> +	ret = clk_prepare_enable(priv->ipg);
>> +	if (ret) {
>> +		dev_err(dev, "Cannot enable 'ipg' clock, error: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = clk_prepare_enable(priv->sirc);
>> +	if (ret) {
>> +		dev_err(dev, "Cannot enable 'sirc' clock, error: %d\n", ret);
>> +		goto disable_ipg_clk;
>> +	}
>> +
>> +	ret = clk_prepare_enable(priv->firc);
>> +	if (ret) {
>> +		dev_err(dev, "Cannot enable 'firc' clock, error: %d\n", ret);
>> +		goto disable_sirc_clk;
>> +	}
>> +
>> +	/* Make sure the RTC ticking is disabled before we configure dividers */
>> +	rtc_disable(priv);
> 
> At this point, you lost the RTC accuracy, this must not be done on probe

The 'rtc_init' procedure is called during the probe and suspend
callbacks. The first one is expected, while the latter may be a bit of a
surprise, but it follows what the hardware expects to see when switching
from one clock source to another.
In this case (suspend), we are forced to switch from a clock source that
can be used only during running (FIRC) to SIRC, which is the only option 
for the period when Linux is suspended.

>> +
>> +	clksel = RTCC_CLKSEL(priv->clk_source);
>> +	rtcc |= clksel;
>> +
>> +	/* Precompute the base frequency of the clock */
>> +	switch (clksel) {
>> +	case RTCC_CLKSEL(S32G_RTC_SOURCE_SIRC):
>> +		sclk = priv->sirc;
>> +		break;
>> +	case RTCC_CLKSEL(S32G_RTC_SOURCE_FIRC):
>> +		sclk = priv->firc;
>> +		break;
>> +	default:
>> +		dev_err(dev, "Invalid clksel value: %u\n", clksel);
>> +		ret = -EINVAL;
>> +		goto disable_firc_clk;
>> +	}
>> +
>> +	priv->rtc_hz = clk_get_rate(sclk);
>> +	if (!priv->rtc_hz) {
>> +		dev_err(dev, "Invalid RTC frequency\n");
>> +		ret = -EINVAL;
>> +		goto disable_firc_clk;
>> +	}
>> +
>> +	/* Adjust frequency if dividers are enabled */
>> +	if (priv->div512) {
>> +		rtcc |= RTCC_DIV512EN;
>> +		priv->rtc_hz /= 512;
>> +	}
>> +
>> +	if (priv->div32) {
>> +		rtcc |= RTCC_DIV32EN;
>> +		priv->rtc_hz /= 32;
>> +	}
>> +
>> +	rtcc |= RTCC_RTCIE | RTCC_ROVREN;
>> +	iowrite32(rtcc, priv->rtc_base + RTCC_OFFSET);
>> +
>> +	return 0;
>> +
>> +disable_firc_clk:
>> +	clk_disable_unprepare(priv->firc);
>> +disable_sirc_clk:
>> +	clk_disable_unprepare(priv->sirc);
>> +disable_ipg_clk:
>> +	clk_disable_unprepare(priv->ipg);
>> +	return ret;
>> +}
>> +
>> +static int priv_dts_init(struct rtc_priv *priv)
>> +{
>> +	struct device *dev = priv->dev;
>> +	struct platform_device *pdev = to_platform_device(dev);
>> +	u32 clksel = S32G_RTC_SOURCE_SIRC;
>> +	/* div512 and div32 */
>> +	u32 div[2] = { 0 };
>> +	int ret;
>> +
>> +	priv->sirc = devm_clk_get(dev, "sirc");
>> +	if (IS_ERR(priv->sirc)) {
>> +		dev_err(dev, "Failed to get 'sirc' clock\n");
>> +		return PTR_ERR(priv->sirc);
>> +	}
>> +
>> +	priv->firc = devm_clk_get(dev, "firc");
>> +	if (IS_ERR(priv->firc)) {
>> +		dev_err(dev, "Failed to get 'firc' clock\n");
>> +		return PTR_ERR(priv->firc);
>> +	}
>> +
>> +	priv->ipg = devm_clk_get(dev, "ipg");
>> +	if (IS_ERR(priv->ipg)) {
>> +		dev_err(dev, "Failed to get 'ipg' clock\n");
>> +		return PTR_ERR(priv->ipg);
>> +	}
>> +
>> +	priv->dt_irq_id = platform_get_irq(pdev, 0);
>> +	if (priv->dt_irq_id < 0) {
>> +		dev_err(dev, "Error reading interrupt # from dts\n");
>> +		return priv->dt_irq_id;
>> +	}
>> +
>> +	ret = device_property_read_u32_array(dev, "nxp,dividers", div,
>> +					     ARRAY_SIZE(div));
>> +	if (ret) {
>> +		dev_err(dev, "Error reading dividers configuration, err: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = device_property_read_u32(dev, "nxp,clksel", &clksel);
>> +	if (ret) {
>> +		dev_err(dev, "Error reading clksel configuration, err: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	priv->div512 = !!div[0];
>> +	priv->div32 = !!div[1];
>> +
>> +	switch (clksel) {
>> +	case S32G_RTC_SOURCE_SIRC:
>> +	case S32G_RTC_SOURCE_FIRC:
>> +		priv->clk_source = clksel;
>> +		break;
>> +	default:
>> +		dev_err(dev, "Unsupported clksel: %d\n", clksel);
>> +		return -EINVAL;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int rtc_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct rtc_priv *priv;
>> +	int ret = 0;
>> +
>> +	priv = devm_kzalloc(dev, sizeof(struct rtc_priv),
>> +			    GFP_KERNEL);
>> +	if (!priv)
>> +		return -ENOMEM;
>> +
>> +	priv->rtc_base = devm_platform_ioremap_resource(pdev, 0);
>> +	if (IS_ERR(priv->rtc_base)) {
>> +		dev_err(dev, "Failed to map registers\n");
>> +		return PTR_ERR(priv->rtc_base);
>> +	}
>> +
>> +	device_init_wakeup(dev, true);
>> +	priv->dev = dev;
>> +
>> +	ret = priv_dts_init(priv);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = rtc_init(priv);
>> +	if (ret)
>> +		return ret;
>> +
>> +	platform_set_drvdata(pdev, priv);
>> +	rtc_enable(priv);
>> +
>> +	priv->rdev = devm_rtc_device_register(dev, "s32g_rtc",
>> +					      &rtc_ops, THIS_MODULE);
> 
> This is deprecated, please use devm_rtc_allocate_device and devm_rtc_register_device
>

Thanks, I will update in V2.

>> +	if (IS_ERR_OR_NULL(priv->rdev)) {
>> +		dev_err(dev, "Error registering RTC device, err: %ld\n",
>> +			PTR_ERR(priv->rdev));
>> +		ret = PTR_ERR(priv->rdev);
>> +		goto disable_rtc;
>> +	}
>> +
>> +	ret = devm_request_irq(dev, priv->dt_irq_id,
>> +			       rtc_handler, 0, dev_name(dev), pdev);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "Request interrupt %d failed, error: %d\n",
>> +			priv->dt_irq_id, ret);
>> +		goto disable_rtc;
> 
> You must not fail after registering the RTC, else your driver will be
> opened to a race condition;
> 
>> +	}
>> +
>> +	return 0;
>> +
>> +disable_rtc:
>> +	rtc_disable(priv);
>> +	return ret;
>> +}
>> +
>> +static void rtc_remove(struct platform_device *pdev)
>> +{
>> +	struct rtc_priv *priv = platform_get_drvdata(pdev);
>> +
>> +	rtc_disable(priv);
> 
> Disabling the RTC when shutting down renders the RTC useless.
>

The RTC hardware module state is not preserved during system poweroff.
Hence why I've disabled it on remove callback.

>> +}
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static void enable_api_irq(struct device *dev, unsigned int enabled)
>> +{
>> +	struct rtc_priv *priv = dev_get_drvdata(dev);
>> +	u32 api_irq = RTCC_APIEN | RTCC_APIIE;
>> +	u32 rtcc;
>> +
>> +	rtcc = ioread32(priv->rtc_base + RTCC_OFFSET);
>> +	if (enabled)
>> +		rtcc |= api_irq;
>> +	else
>> +		rtcc &= ~api_irq;
>> +	iowrite32(rtcc, priv->rtc_base + RTCC_OFFSET);
>> +}
>> +
>> +static int adjust_dividers(u32 sec, struct rtc_priv *priv)
>> +{
>> +	u64 rtcval_max = U32_MAX;
>> +	u64 rtcval;
>> +
>> +	priv->div32 = 0;
>> +	priv->div512 = 0;
>> +
>> +	rtcval = sec * priv->rtc_hz;
>> +	if (rtcval < rtcval_max)
>> +		return 0;
>> +
>> +	if (rtcval / 32 < rtcval_max) {
>> +		priv->div32 = 1;
>> +		return 0;
>> +	}
>> +
>> +	if (rtcval / 512 < rtcval_max) {
>> +		priv->div512 = 1;
>> +		return 0;
>> +	}
>> +
>> +	if (rtcval / (512 * 32) < rtcval_max) {
>> +		priv->div32 = 1;
>> +		priv->div512 = 1;
>> +		return 0;
>> +	}
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +static int rtc_suspend(struct device *dev)
>> +{
>> +	struct rtc_priv *init_priv = dev_get_drvdata(dev);
>> +	struct rtc_priv priv;
>> +	long long base_sec;
>> +	int ret = 0;
>> +	u32 rtcval;
>> +	u32 sec;
>> +
>> +	if (!device_may_wakeup(dev))
>> +		return 0;
>> +
>> +	/* Save last known timestamp before we switch clocks and reinit RTC */
>> +	ret = s32g_rtc_read_time(dev, &priv.base.tm);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (init_priv->clk_source == S32G_RTC_SOURCE_SIRC)
>> +		return 0;
>> +
>> +	/*
>> +	 * Use a local copy of the RTC control block to
>> +	 * avoid restoring it on resume path.
>> +	 */
>> +	memcpy(&priv, init_priv, sizeof(priv));
>> +
>> +	/* Switch to SIRC */
>> +	priv.clk_source = S32G_RTC_SOURCE_SIRC;
>> +
>> +	ret = get_time_left(dev, init_priv, &sec);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Adjust for the number of seconds we'll be asleep */
>> +	base_sec = rtc_tm_to_time64(&init_priv->base.tm);
>> +	base_sec += sec;
>> +	rtc_time64_to_tm(base_sec, &init_priv->base.tm);
>> +
>> +	rtc_disable(&priv);
>> +
>> +	ret = adjust_dividers(sec, &priv);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to adjust RTC dividers to match a %u seconds delay\n", sec);
>> +		return ret;
>> +	}
>> +
>> +	ret = rtc_init(&priv);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = sec_to_rtcval(&priv, sec, &rtcval);
>> +	if (ret) {
>> +		dev_warn(dev, "Alarm is too far in the future\n");
>> +		return ret;
>> +	}
> 
> All of this seems super fishy.
>

I agree it is... above rollover support enables RTC alarm during Sleep
for a maximum timespan of ~3 months.
In this case, the rollover does not matter as it's not involved in sleep 
period settings. The only way we could avoid all of this is to always 
keep the counter on SIRC, starting with driver's probing.
This would avoid the transitions, but the precision would not be as 
accurate as running at 51MHz.

>> +
>> +	s32g_rtc_alarm_irq_enable(dev, 0);
>> +	enable_api_irq(dev, 1);
>> +	iowrite32(rtcval, priv.rtc_base + APIVAL_OFFSET);
>> +	iowrite32(0, priv.rtc_base + RTCVAL_OFFSET);
>> +
>> +	rtc_enable(&priv);
>> +
>> +	return ret;
>> +}
>> +
>> +static int rtc_resume(struct device *dev)
>> +{
>> +	struct rtc_priv *priv = dev_get_drvdata(dev);
>> +	int ret;
>> +
>> +	if (!device_may_wakeup(dev))
>> +		return 0;
>> +
>> +	/* Disable wake-up interrupts */
>> +	enable_api_irq(dev, 0);
>> +
>> +	/* Reinitialize the driver using the initial settings */
>> +	ret = rtc_init(priv);
>> +	if (ret)
>> +		return ret;
>> +
>> +	rtc_enable(priv);
>> +
>> +	/*
>> +	 * Now RTCCNT has just been reset, and is out of sync with priv->base;
>> +	 * reapply the saved time settings
>> +	 */
>> +	return s32g_rtc_set_time(dev, &priv->base.tm);
>> +}
>> +#endif /* CONFIG_PM_SLEEP */
>> +
>> +static const struct of_device_id rtc_dt_ids[] = {
>> +	{.compatible = "nxp,s32g-rtc" },
>> +	{ /* sentinel */ },
>> +};
>> +
>> +static SIMPLE_DEV_PM_OPS(rtc_pm_ops,
>> +			 rtc_suspend, rtc_resume);
>> +
>> +static struct platform_driver rtc_driver = {
>> +	.driver		= {
>> +		.name			= "s32g-rtc",
>> +		.pm				= &rtc_pm_ops,
>> +		.of_match_table = of_match_ptr(rtc_dt_ids),
>> +	},
>> +	.probe		= rtc_probe,
>> +	.remove_new	= rtc_remove,
>> +};
>> +module_platform_driver(rtc_driver);
>> +
>> +MODULE_AUTHOR("NXP");
>> +MODULE_DESCRIPTION("NXP RTC driver for S32G2/S32G3");
>> +MODULE_LICENSE("GPL");
>> -- 
>> 2.45.2
>>
>>
>

On 11/09/2024 09:00, Ciprian Costea wrote:
> From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
> 
> Add a RTC driver for NXP S32G2/S32G3 SoCs.
> 
> The RTC module is used to enable Suspend to RAM (STR) support
> on NXP S32G2/S32G3 SoC based boards.
> RTC tracks clock time during system suspend.
> 

...

> +	priv->div512 = !!div[0];
> +	priv->div32 = !!div[1];
> +
> +	switch (clksel) {
> +	case S32G_RTC_SOURCE_SIRC:
> +	case S32G_RTC_SOURCE_FIRC:
> +		priv->clk_source = clksel;
> +		break;
> +	default:
> +		dev_err(dev, "Unsupported clksel: %d\n", clksel);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int rtc_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct rtc_priv *priv;
> +	int ret = 0;
> +
> +	priv = devm_kzalloc(dev, sizeof(struct rtc_priv),

sizeof(*)

> +			    GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->rtc_base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(priv->rtc_base)) {
> +		dev_err(dev, "Failed to map registers\n");
> +		return PTR_ERR(priv->rtc_base);

return dev_err_probe

> +	}
> +
> +	device_init_wakeup(dev, true);
> +	priv->dev = dev;
> +
> +	ret = priv_dts_init(priv);
> +	if (ret)
> +		return ret;
> +
> +	ret = rtc_init(priv);
> +	if (ret)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, priv);
> +	rtc_enable(priv);
> +
> +	priv->rdev = devm_rtc_device_register(dev, "s32g_rtc",
> +					      &rtc_ops, THIS_MODULE);
> +	if (IS_ERR_OR_NULL(priv->rdev)) {
> +		dev_err(dev, "Error registering RTC device, err: %ld\n",
> +			PTR_ERR(priv->rdev));
> +		ret = PTR_ERR(priv->rdev);
> +		goto disable_rtc;
> +	}
> +
> +	ret = devm_request_irq(dev, priv->dt_irq_id,
> +			       rtc_handler, 0, dev_name(dev), pdev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Request interrupt %d failed, error: %d\n",
> +			priv->dt_irq_id, ret);
> +		goto disable_rtc;
> +	}
> +
> +	return 0;
> +
> +disable_rtc:
> +	rtc_disable(priv);
> +	return ret;
> +}
> +
> +static void rtc_remove(struct platform_device *pdev)
> +{
> +	struct rtc_priv *priv = platform_get_drvdata(pdev);
> +
> +	rtc_disable(priv);
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static void enable_api_irq(struct device *dev, unsigned int enabled)
> +{
> +	struct rtc_priv *priv = dev_get_drvdata(dev);
> +	u32 api_irq = RTCC_APIEN | RTCC_APIIE;
> +	u32 rtcc;
> +
> +	rtcc = ioread32(priv->rtc_base + RTCC_OFFSET);
> +	if (enabled)
> +		rtcc |= api_irq;
> +	else
> +		rtcc &= ~api_irq;
> +	iowrite32(rtcc, priv->rtc_base + RTCC_OFFSET);
> +}
> +
> +static int adjust_dividers(u32 sec, struct rtc_priv *priv)
> +{
> +	u64 rtcval_max = U32_MAX;
> +	u64 rtcval;
> +
> +	priv->div32 = 0;
> +	priv->div512 = 0;
> +
> +	rtcval = sec * priv->rtc_hz;
> +	if (rtcval < rtcval_max)
> +		return 0;
> +
> +	if (rtcval / 32 < rtcval_max) {
> +		priv->div32 = 1;
> +		return 0;
> +	}
> +
> +	if (rtcval / 512 < rtcval_max) {
> +		priv->div512 = 1;
> +		return 0;
> +	}
> +
> +	if (rtcval / (512 * 32) < rtcval_max) {
> +		priv->div32 = 1;
> +		priv->div512 = 1;
> +		return 0;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int rtc_suspend(struct device *dev)
> +{
> +	struct rtc_priv *init_priv = dev_get_drvdata(dev);
> +	struct rtc_priv priv;
> +	long long base_sec;
> +	int ret = 0;
> +	u32 rtcval;
> +	u32 sec;
> +
> +	if (!device_may_wakeup(dev))
> +		return 0;
> +
> +	/* Save last known timestamp before we switch clocks and reinit RTC */
> +	ret = s32g_rtc_read_time(dev, &priv.base.tm);
> +	if (ret)
> +		return ret;
> +
> +	if (init_priv->clk_source == S32G_RTC_SOURCE_SIRC)
> +		return 0;
> +
> +	/*
> +	 * Use a local copy of the RTC control block to
> +	 * avoid restoring it on resume path.
> +	 */
> +	memcpy(&priv, init_priv, sizeof(priv));
> +
> +	/* Switch to SIRC */
> +	priv.clk_source = S32G_RTC_SOURCE_SIRC;
> +
> +	ret = get_time_left(dev, init_priv, &sec);
> +	if (ret)
> +		return ret;
> +
> +	/* Adjust for the number of seconds we'll be asleep */
> +	base_sec = rtc_tm_to_time64(&init_priv->base.tm);
> +	base_sec += sec;
> +	rtc_time64_to_tm(base_sec, &init_priv->base.tm);
> +
> +	rtc_disable(&priv);
> +
> +	ret = adjust_dividers(sec, &priv);
> +	if (ret) {
> +		dev_err(dev, "Failed to adjust RTC dividers to match a %u seconds delay\n", sec);
> +		return ret;
> +	}
> +
> +	ret = rtc_init(&priv);
> +	if (ret)
> +		return ret;
> +
> +	ret = sec_to_rtcval(&priv, sec, &rtcval);
> +	if (ret) {
> +		dev_warn(dev, "Alarm is too far in the future\n");
> +		return ret;
> +	}
> +
> +	s32g_rtc_alarm_irq_enable(dev, 0);
> +	enable_api_irq(dev, 1);
> +	iowrite32(rtcval, priv.rtc_base + APIVAL_OFFSET);
> +	iowrite32(0, priv.rtc_base + RTCVAL_OFFSET);
> +
> +	rtc_enable(&priv);
> +
> +	return ret;
> +}
> +
> +static int rtc_resume(struct device *dev)
> +{
> +	struct rtc_priv *priv = dev_get_drvdata(dev);
> +	int ret;
> +
> +	if (!device_may_wakeup(dev))
> +		return 0;
> +
> +	/* Disable wake-up interrupts */
> +	enable_api_irq(dev, 0);
> +
> +	/* Reinitialize the driver using the initial settings */
> +	ret = rtc_init(priv);
> +	if (ret)
> +		return ret;
> +
> +	rtc_enable(priv);
> +
> +	/*
> +	 * Now RTCCNT has just been reset, and is out of sync with priv->base;
> +	 * reapply the saved time settings
> +	 */
> +	return s32g_rtc_set_time(dev, &priv->base.tm);
> +}
> +#endif /* CONFIG_PM_SLEEP */
> +
> +static const struct of_device_id rtc_dt_ids[] = {
> +	{.compatible = "nxp,s32g-rtc" },
> +	{ /* sentinel */ },
> +};
> +
> +static SIMPLE_DEV_PM_OPS(rtc_pm_ops,
> +			 rtc_suspend, rtc_resume);
> +
> +static struct platform_driver rtc_driver = {
> +	.driver		= {
> +		.name			= "s32g-rtc",
> +		.pm				= &rtc_pm_ops,
> +		.of_match_table = of_match_ptr(rtc_dt_ids),

Drop of_match_ptr, you have here warning.

> +	},
> +	.probe		= rtc_probe,
> +	.remove_new	= rtc_remove,
> +};
> +module_platform_driver(rtc_driver);
> +
> +MODULE_AUTHOR("NXP");
> +MODULE_DESCRIPTION("NXP RTC driver for S32G2/S32G3");
> +MODULE_LICENSE("GPL");

Best regards,
Krzysztof

On 9/17/2024 8:40 PM, Krzysztof Kozlowski wrote:
> On 11/09/2024 09:00, Ciprian Costea wrote:
>> From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
>>
>> Add a RTC driver for NXP S32G2/S32G3 SoCs.
>>
>> The RTC module is used to enable Suspend to RAM (STR) support
>> on NXP S32G2/S32G3 SoC based boards.
>> RTC tracks clock time during system suspend.
>>
> 
> ...
> 
>> +static SIMPLE_DEV_PM_OPS(rtc_pm_ops,
>> +			 rtc_suspend, rtc_resume);
>> +
>> +static struct platform_driver rtc_driver = {
>> +	.driver		= {
>> +		.name			= "s32g-rtc",
>> +		.pm				= &rtc_pm_ops,
>> +		.of_match_table = of_match_ptr(rtc_dt_ids),
> 
> Drop of_match_ptr, you have here warning.
> 
>> +	},
>> +	.probe		= rtc_probe,
>> +	.remove_new	= rtc_remove,
>> +};
>> +module_platform_driver(rtc_driver);
>> +
>> +MODULE_AUTHOR("NXP");
>> +MODULE_DESCRIPTION("NXP RTC driver for S32G2/S32G3");
>> +MODULE_LICENSE("GPL");
> 
> Best regards,
> Krzysztof
> 

Hello Krzysztof,

Thank you for your review.
I will address your findings in V2.

Best Regards,
Ciprian

Hi Ciprian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on abelloni/rtc-next]
[also build test WARNING on robh/for-next arm64/for-next/core linus/master v6.11-rc7 next-20240912]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ciprian-Costea/dt-bindings-rtc-add-schema-for-NXP-S32G2-S32G3-SoCs/20240911-150205
base:   https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next
patch link:    https://lore.kernel.org/r/20240911070028.127659-3-ciprianmarian.costea%40oss.nxp.com
patch subject: [PATCH 2/4] rtc: s32g: add NXP S32G2/S32G3 SoC support
config: hexagon-randconfig-r112-20240913 (https://download.01.org/0day-ci/archive/20240913/202409131950.ozDVVv5X-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project bf684034844c660b778f0eba103582f582b710c9)
reproduce: (https://download.01.org/0day-ci/archive/20240913/202409131950.ozDVVv5X-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/202409131950.ozDVVv5X-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/rtc/rtc-s32g.c:7:
   In file included from include/linux/of_irq.h:7:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     548 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     561 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
         |                                                   ^
   In file included from drivers/rtc/rtc-s32g.c:7:
   In file included from include/linux/of_irq.h:7:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     574 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
         |                                                   ^
   In file included from drivers/rtc/rtc-s32g.c:7:
   In file included from include/linux/of_irq.h:7:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     585 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     595 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     605 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
>> drivers/rtc/rtc-s32g.c:668:34: warning: unused variable 'rtc_dt_ids' [-Wunused-const-variable]
     668 | static const struct of_device_id rtc_dt_ids[] = {
         |                                  ^~~~~~~~~~
   7 warnings generated.


vim +/rtc_dt_ids +668 drivers/rtc/rtc-s32g.c

   667	
 > 668	static const struct of_device_id rtc_dt_ids[] = {
   669		{.compatible = "nxp,s32g-rtc" },
   670		{ /* sentinel */ },
   671	};
   672	

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

Hi Ciprian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on abelloni/rtc-next]
[also build test WARNING on robh/for-next arm64/for-next/core linus/master v6.11-rc7 next-20240911]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ciprian-Costea/dt-bindings-rtc-add-schema-for-NXP-S32G2-S32G3-SoCs/20240911-150205
base:   https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next
patch link:    https://lore.kernel.org/r/20240911070028.127659-3-ciprianmarian.costea%40oss.nxp.com
patch subject: [PATCH 2/4] rtc: s32g: add NXP S32G2/S32G3 SoC support
config: sh-randconfig-r073-20240912 (https://download.01.org/0day-ci/archive/20240912/202409121103.EX9BTDFT-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240912/202409121103.EX9BTDFT-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/202409121103.EX9BTDFT-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/rtc/rtc-s32g.c:668:34: warning: 'rtc_dt_ids' defined but not used [-Wunused-const-variable=]
     668 | static const struct of_device_id rtc_dt_ids[] = {
         |                                  ^~~~~~~~~~


vim +/rtc_dt_ids +668 drivers/rtc/rtc-s32g.c

   667	
 > 668	static const struct of_device_id rtc_dt_ids[] = {
   669		{.compatible = "nxp,s32g-rtc" },
   670		{ /* sentinel */ },
   671	};
   672	

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