All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] RZN1 RTC support
@ 2022-04-05 18:47 Miquel Raynal
  2022-04-05 18:47 ` [PATCH 1/7] dt-bindings: rtc: rzn1: Describe the RZN1 RTC Miquel Raynal
                   ` (6 more replies)
  0 siblings, 7 replies; 20+ messages in thread
From: Miquel Raynal @ 2022-04-05 18:47 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: Rob Herring, devicetree, Krzysztof Kozlowski, linux-renesas-soc,
	Magnus Damm, Gareth Williams, Phil Edworthy, Geert Uytterhoeven,
	Stephen Boyd, Michael Turquette, linux-clk, Milan Stevanovic,
	Jimmy Lalande, Pascal Eberhard, Thomas Petazzoni, Herve Codina,
	Clement Leger, linux-rtc, Miquel Raynal

Hello,

This small series adds support for the RZN1 RTC.

Despite its limitations, I found useful to at least have alarm and
offset support.

This depends on the previous sysctrl/dma/uart changes on the sysctrl
side.

Cheers,
Miquèl

Michel Pollet (1):
  rtc: rzn1: Add new RTC driver

Miquel Raynal (6):
  dt-bindings: rtc: rzn1: Describe the RZN1 RTC
  soc: renesas: rzn1-sysc: Export a function to enable/disable the RTC
  rtc: rzn1: Add alarm support
  rtc: rzn1: Add oscillator offset support
  MAINTAINERS: Add myself as maintainer of the RZN1 RTC driver
  ARM: dts: r9a06g032: Describe the RTC

 .../bindings/rtc/renesas,rzn1-rtc.yaml        |  69 +++
 MAINTAINERS                                   |   8 +
 arch/arm/boot/dts/r9a06g032.dtsi              |  12 +
 drivers/clk/renesas/r9a06g032-clocks.c        |  49 ++
 drivers/rtc/Kconfig                           |   7 +
 drivers/rtc/Makefile                          |   1 +
 drivers/rtc/rtc-rzn1.c                        | 436 ++++++++++++++++++
 include/linux/soc/renesas/r9a06g032-sysctrl.h |   2 +
 8 files changed, 584 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/renesas,rzn1-rtc.yaml
 create mode 100644 drivers/rtc/rtc-rzn1.c

-- 
2.27.0


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

* [PATCH 1/7] dt-bindings: rtc: rzn1: Describe the RZN1 RTC
  2022-04-05 18:47 [PATCH 0/7] RZN1 RTC support Miquel Raynal
@ 2022-04-05 18:47 ` Miquel Raynal
  2022-04-07  7:37   ` Krzysztof Kozlowski
  2022-04-05 18:47 ` [PATCH 2/7] soc: renesas: rzn1-sysc: Export a function to enable/disable the RTC Miquel Raynal
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Miquel Raynal @ 2022-04-05 18:47 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: Rob Herring, devicetree, Krzysztof Kozlowski, linux-renesas-soc,
	Magnus Damm, Gareth Williams, Phil Edworthy, Geert Uytterhoeven,
	Stephen Boyd, Michael Turquette, linux-clk, Milan Stevanovic,
	Jimmy Lalande, Pascal Eberhard, Thomas Petazzoni, Herve Codina,
	Clement Leger, linux-rtc, Miquel Raynal

Add new binding file for this RTC.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 .../bindings/rtc/renesas,rzn1-rtc.yaml        | 69 +++++++++++++++++++
 1 file changed, 69 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/renesas,rzn1-rtc.yaml

diff --git a/Documentation/devicetree/bindings/rtc/renesas,rzn1-rtc.yaml b/Documentation/devicetree/bindings/rtc/renesas,rzn1-rtc.yaml
new file mode 100644
index 000000000000..903f0cd361fa
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/renesas,rzn1-rtc.yaml
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/renesas,rzn1-rtc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RZ/N1 SoCs Real-Time Clock DT bindings
+
+maintainers:
+  - Miquel Raynal <miquel.raynal@bootlin.com>
+
+allOf:
+  - $ref: rtc.yaml#
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - renesas,r9a06g032-rtc
+          - const: renesas,rzn1-rtc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    minItems: 3
+    maxItems: 3
+
+  interrupt-names:
+    items:
+      - const: alarm
+      - const: timer
+      - const: pps
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: hclk
+
+  start-year: true
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-names
+  - clocks
+  - clock-names
+  - start-year
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/r9a06g032-sysctrl.h>
+    rtc@40006000 {
+       compatible = "renesas,r9a06g032-rtc", "renesas,rzn1-rtc";
+       reg = <0x40006000 0x1000>;
+       interrupts = <GIC_SPI 66 IRQ_TYPE_EDGE_RISING>,
+                    <GIC_SPI 67 IRQ_TYPE_EDGE_RISING>,
+                    <GIC_SPI 68 IRQ_TYPE_EDGE_RISING>;
+       interrupt-names = "alarm", "timer", "pps";
+       clocks = <&sysctrl R9A06G032_HCLK_RTC>;
+       clock-names = "hclk";
+       start-year = <2000>;
+     };
-- 
2.27.0


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

* [PATCH 2/7] soc: renesas: rzn1-sysc: Export a function to enable/disable the RTC
  2022-04-05 18:47 [PATCH 0/7] RZN1 RTC support Miquel Raynal
  2022-04-05 18:47 ` [PATCH 1/7] dt-bindings: rtc: rzn1: Describe the RZN1 RTC Miquel Raynal
@ 2022-04-05 18:47 ` Miquel Raynal
  2022-04-06  7:00   ` Thomas Petazzoni
  2022-04-06  8:32   ` Alexandre Belloni
  2022-04-05 18:47 ` [PATCH 3/7] rtc: rzn1: Add new RTC driver Miquel Raynal
                   ` (4 subsequent siblings)
  6 siblings, 2 replies; 20+ messages in thread
From: Miquel Raynal @ 2022-04-05 18:47 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: Rob Herring, devicetree, Krzysztof Kozlowski, linux-renesas-soc,
	Magnus Damm, Gareth Williams, Phil Edworthy, Geert Uytterhoeven,
	Stephen Boyd, Michael Turquette, linux-clk, Milan Stevanovic,
	Jimmy Lalande, Pascal Eberhard, Thomas Petazzoni, Herve Codina,
	Clement Leger, linux-rtc, Miquel Raynal

There are two RTC registers located within the system controller.

Like with the dmamux register, let's add a new helper to enable/disable
the power, reset and clock of the RTC.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/clk/renesas/r9a06g032-clocks.c        | 49 +++++++++++++++++++
 include/linux/soc/renesas/r9a06g032-sysctrl.h |  2 +
 2 files changed, 51 insertions(+)

diff --git a/drivers/clk/renesas/r9a06g032-clocks.c b/drivers/clk/renesas/r9a06g032-clocks.c
index 1df56d7ab3e1..7e61db39a43b 100644
--- a/drivers/clk/renesas/r9a06g032-clocks.c
+++ b/drivers/clk/renesas/r9a06g032-clocks.c
@@ -26,6 +26,13 @@
 #include <dt-bindings/clock/r9a06g032-sysctrl.h>
 
 #define R9A06G032_SYSCTRL_DMAMUX 0xA0
+#define R9A06G032_SYSCTRL_PWRCTRL_RTC 0x140
+#define   R9A06G032_SYSCTRL_PWRCTRL_RTC_CLKEN BIT(0)
+#define   R9A06G032_SYSCTRL_PWRCTRL_RTC_RST BIT(1)
+#define   R9A06G032_SYSCTRL_PWRCTRL_RTC_IDLE_REQ BIT(2)
+#define   R9A06G032_SYSCTRL_PWRCTRL_RTC_RSTN_FW BIT(3)
+#define R9A06G032_SYSCTRL_PWRSTAT_RTC 0x144
+#define   R9A06G032_SYSCTRL_PWRSTAT_RTC_IDLE BIT(1)
 
 struct r9a06g032_gate {
 	u16 gate, reset, ready, midle,
@@ -343,6 +350,48 @@ int r9a06g032_sysctrl_set_dmamux(u32 mask, u32 val)
 }
 EXPORT_SYMBOL_GPL(r9a06g032_sysctrl_set_dmamux);
 
+/* Exported helper to enable/disable the RTC */
+int r9a06g032_sysctrl_enable_rtc(bool enable)
+{
+	unsigned long flags;
+	u32 val;
+
+	if (!sysctrl_priv)
+		return -EPROBE_DEFER;
+
+	spin_lock_irqsave(&sysctrl_priv->lock, flags);
+
+	if (enable) {
+		val = readl(sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
+		val &= ~R9A06G032_SYSCTRL_PWRCTRL_RTC_RST;
+		writel(val, sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
+		val |= R9A06G032_SYSCTRL_PWRCTRL_RTC_CLKEN;
+		writel(val, sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
+		val |= R9A06G032_SYSCTRL_PWRCTRL_RTC_RSTN_FW;
+		writel(val, sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
+		val &= ~R9A06G032_SYSCTRL_PWRCTRL_RTC_IDLE_REQ;
+		writel(val, sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
+		val = readl(sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRSTAT_RTC);
+		if (val & R9A06G032_SYSCTRL_PWRSTAT_RTC_IDLE)
+			return -EIO;
+	} else {
+		val = readl(sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
+		val |= R9A06G032_SYSCTRL_PWRCTRL_RTC_IDLE_REQ;
+		writel(val, sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
+		val &= ~R9A06G032_SYSCTRL_PWRCTRL_RTC_RSTN_FW;
+		writel(val, sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
+		val &= ~R9A06G032_SYSCTRL_PWRCTRL_RTC_CLKEN;
+		writel(val, sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
+		val |= R9A06G032_SYSCTRL_PWRCTRL_RTC_RST;
+		writel(val, sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
+	}
+
+	spin_unlock_irqrestore(&sysctrl_priv->lock, flags);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(r9a06g032_sysctrl_enable_rtc);
+
 /* register/bit pairs are encoded as an uint16_t */
 static void
 clk_rdesc_set(struct r9a06g032_priv *clocks,
diff --git a/include/linux/soc/renesas/r9a06g032-sysctrl.h b/include/linux/soc/renesas/r9a06g032-sysctrl.h
index 066dfb15cbdd..914c8789149c 100644
--- a/include/linux/soc/renesas/r9a06g032-sysctrl.h
+++ b/include/linux/soc/renesas/r9a06g032-sysctrl.h
@@ -4,8 +4,10 @@
 
 #ifdef CONFIG_CLK_R9A06G032
 int r9a06g032_sysctrl_set_dmamux(u32 mask, u32 val);
+int r9a06g032_sysctrl_enable_rtc(bool enable);
 #else
 static inline int r9a06g032_sysctrl_set_dmamux(u32 mask, u32 val) { return -ENODEV; }
+static inline int r9a06g032_sysctrl_enable_rtc(bool enable) { return -ENODEV; }
 #endif
 
 #endif /* __LINUX_SOC_RENESAS_R9A06G032_SYSCTRL_H__ */
-- 
2.27.0


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

* [PATCH 3/7] rtc: rzn1: Add new RTC driver
  2022-04-05 18:47 [PATCH 0/7] RZN1 RTC support Miquel Raynal
  2022-04-05 18:47 ` [PATCH 1/7] dt-bindings: rtc: rzn1: Describe the RZN1 RTC Miquel Raynal
  2022-04-05 18:47 ` [PATCH 2/7] soc: renesas: rzn1-sysc: Export a function to enable/disable the RTC Miquel Raynal
@ 2022-04-05 18:47 ` Miquel Raynal
  2022-04-06  8:50   ` Alexandre Belloni
  2022-04-05 18:47 ` [PATCH 4/7] rtc: rzn1: Add alarm support Miquel Raynal
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Miquel Raynal @ 2022-04-05 18:47 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: Rob Herring, devicetree, Krzysztof Kozlowski, linux-renesas-soc,
	Magnus Damm, Gareth Williams, Phil Edworthy, Geert Uytterhoeven,
	Stephen Boyd, Michael Turquette, linux-clk, Milan Stevanovic,
	Jimmy Lalande, Pascal Eberhard, Thomas Petazzoni, Herve Codina,
	Clement Leger, linux-rtc, Michel Pollet, Miquel Raynal

From: Michel Pollet <michel.pollet@bp.renesas.com>

Add a basic RTC driver for the RZ/N1.

Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
Co-developed-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/rtc/Kconfig    |   7 ++
 drivers/rtc/Makefile   |   1 +
 drivers/rtc/rtc-rzn1.c | 255 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 263 insertions(+)
 create mode 100644 drivers/rtc/rtc-rzn1.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 41c65b4d2baf..f4d72c5b99ea 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1548,6 +1548,13 @@ config RTC_DRV_RS5C313
 	help
 	  If you say yes here you get support for the Ricoh RS5C313 RTC chips.
 
+config RTC_DRV_RZN1
+	tristate "Renesas RZN1 RTC"
+	depends on ARCH_RZN1 || COMPILE_TEST
+	depends on OF && HAS_IOMEM
+	help
+	  If you say yes here you get support for the Renesas RZ/N1 RTC.
+
 config RTC_DRV_GENERIC
 	tristate "Generic RTC support"
 	# Please consider writing a new RTC driver instead of using the generic
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 2d827d8261d5..fb04467b652d 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -151,6 +151,7 @@ obj-$(CONFIG_RTC_DRV_RX6110)	+= rtc-rx6110.o
 obj-$(CONFIG_RTC_DRV_RX8010)	+= rtc-rx8010.o
 obj-$(CONFIG_RTC_DRV_RX8025)	+= rtc-rx8025.o
 obj-$(CONFIG_RTC_DRV_RX8581)	+= rtc-rx8581.o
+obj-$(CONFIG_RTC_DRV_RZN1)	+= rtc-rzn1.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-rzn1.c b/drivers/rtc/rtc-rzn1.c
new file mode 100644
index 000000000000..15c533333930
--- /dev/null
+++ b/drivers/rtc/rtc-rzn1.c
@@ -0,0 +1,255 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Renesas RZN1 Real Time Clock interface for Linux
+ *
+ * Copyright:
+ * - 2014 Renesas Electronics Europe Limited
+ * - 2022 Schneider Electric
+ *
+ * Authors:
+ * - Michel Pollet <michel.pollet@bp.renesas.com>, <buserror@gmail.com>
+ * - Miquel Raynal <miquel.raynal@bootlin.com>
+ */
+
+#include <linux/bcd.h>
+#include <linux/clk.h>
+#include <linux/init.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/rtc.h>
+#include <linux/soc/renesas/r9a06g032-sysctrl.h>
+
+#define RZN1_RTC_CTL0 0x00
+#define   RZN1_RTC_CTL0_SLSB_SUBU 0
+#define   RZN1_RTC_CTL0_SLSB_SCMP BIT(4)
+#define   RZN1_RTC_CTL0_AMPM BIT(5)
+#define   RZN1_RTC_CTL0_CE BIT(7)
+
+#define RZN1_RTC_CTL1 0x04
+#define   RZN1_RTC_CTL1_ALME BIT(4)
+
+#define RZN1_RTC_CTL2 0x08
+#define   RZN1_RTC_CTL2_WAIT BIT(0)
+#define   RZN1_RTC_CTL2_WST BIT(1)
+#define   RZN1_RTC_CTL2_WUST BIT(5)
+
+#define RZN1_RTC_SEC 0x14
+#define RZN1_RTC_MIN 0x18
+#define RZN1_RTC_HOUR 0x1c
+#define RZN1_RTC_WEEK 0x20
+#define RZN1_RTC_DAY 0x24
+#define RZN1_RTC_MONTH 0x28
+#define RZN1_RTC_YEAR 0x2c
+
+#define RZN1_RTC_SUBU 0x38
+#define   RZN1_RTC_SUBU_DEV BIT(7)
+#define   RZN1_RTC_SUBU_DECR BIT(6)
+
+#define RZN1_RTC_ALM 0x40
+#define RZN1_RTC_ALH 0x44
+#define RZN1_RTC_ALW 0x48
+
+#define RZN1_RTC_SECC 0x4c
+#define RZN1_RTC_MINC 0x50
+#define RZN1_RTC_HOURC 0x54
+#define RZN1_RTC_WEEKC 0x58
+#define RZN1_RTC_DAYC 0x5c
+#define RZN1_RTC_MONTHC 0x60
+#define RZN1_RTC_YEARC 0x64
+
+struct rzn1_rtc {
+	struct rtc_device *rtcdev;
+	void __iomem *base;
+	struct clk *clk;
+};
+
+static void rzn1_rtc_get_time_snapshot(struct rzn1_rtc *rtc, struct rtc_time *tm)
+{
+	tm->tm_sec = readl(rtc->base + RZN1_RTC_SECC);
+	tm->tm_min = readl(rtc->base + RZN1_RTC_MINC);
+	tm->tm_hour = readl(rtc->base + RZN1_RTC_HOURC);
+	tm->tm_wday = readl(rtc->base + RZN1_RTC_WEEKC);
+	tm->tm_mday = readl(rtc->base + RZN1_RTC_DAYC);
+	tm->tm_mon = readl(rtc->base + RZN1_RTC_MONTHC);
+	tm->tm_year = readl(rtc->base + RZN1_RTC_YEARC);
+}
+
+static unsigned int rzn1_rtc_tm_to_wday(struct rtc_time *tm)
+{
+	time64_t time;
+	unsigned int days;
+	u32 secs;
+
+	time = rtc_tm_to_time64(tm);
+	days = div_s64_rem(time, 86400, &secs);
+
+	/* day of the week, 1970-01-01 was a Thursday */
+	return (days + 4) % 7;
+}
+
+static int rzn1_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+	struct rzn1_rtc *rtc = dev_get_drvdata(dev);
+	u32 secs;
+
+	rzn1_rtc_get_time_snapshot(rtc, tm);
+	secs = readl(rtc->base + RZN1_RTC_SECC);
+	if (tm->tm_sec != secs)
+		rzn1_rtc_get_time_snapshot(rtc, tm);
+
+	tm->tm_sec = bcd2bin(tm->tm_sec);
+	tm->tm_min = bcd2bin(tm->tm_min);
+	tm->tm_hour = bcd2bin(tm->tm_hour);
+	tm->tm_wday = bcd2bin(tm->tm_wday);
+	tm->tm_mday = bcd2bin(tm->tm_mday);
+	tm->tm_mon = bcd2bin(tm->tm_mon);
+	tm->tm_year = bcd2bin(tm->tm_year);
+
+	dev_dbg(dev, "%d-%d-%d(%d)T%d:%d:%d\n",
+		tm->tm_year, tm->tm_mon, tm->tm_mday, tm->tm_wday,
+		tm->tm_hour, tm->tm_min, tm->tm_sec);
+
+	return 0;
+}
+
+static int rzn1_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	struct rzn1_rtc *rtc = dev_get_drvdata(dev);
+	u32 val;
+	int ret;
+
+	tm->tm_sec = bin2bcd(tm->tm_sec);
+	tm->tm_min = bin2bcd(tm->tm_min);
+	tm->tm_hour = bin2bcd(tm->tm_hour);
+	tm->tm_wday = bin2bcd(rzn1_rtc_tm_to_wday(tm));
+	tm->tm_mday = bin2bcd(tm->tm_mday);
+	tm->tm_mon = bin2bcd(tm->tm_mon);
+	tm->tm_year = bin2bcd(tm->tm_year);
+
+	/* Hold the counter */
+	val = readl(rtc->base + RZN1_RTC_CTL2);
+	val |= RZN1_RTC_CTL2_WAIT;
+	writel(val, rtc->base + RZN1_RTC_CTL2);
+
+	/* Wait for the counter to stop: two 32k clock cycles */
+	usleep_range(61, 100);
+	ret = readl_poll_timeout(rtc->base + RZN1_RTC_CTL2, val,
+				 val & RZN1_RTC_CTL2_WST, 0, 100);
+	if (!ret) {
+		writel(tm->tm_sec, rtc->base + RZN1_RTC_SEC);
+		writel(tm->tm_min, rtc->base + RZN1_RTC_MIN);
+		writel(tm->tm_hour, rtc->base + RZN1_RTC_HOUR);
+		writel(tm->tm_wday, rtc->base + RZN1_RTC_WEEK);
+		writel(tm->tm_mday, rtc->base + RZN1_RTC_DAY);
+		writel(tm->tm_mon, rtc->base + RZN1_RTC_MONTH);
+		writel(tm->tm_year, rtc->base + RZN1_RTC_YEAR);
+	}
+
+	/* Release the counter back */
+	val &= ~RZN1_RTC_CTL2_WAIT;
+	writel(val, rtc->base + RZN1_RTC_CTL2);
+
+	return ret;
+}
+
+static const struct rtc_class_ops rzn1_rtc_ops = {
+	.read_time = rzn1_rtc_read_time,
+	.set_time = rzn1_rtc_set_time,
+};
+
+static int rzn1_rtc_probe(struct platform_device *pdev)
+{
+	struct rzn1_rtc *rtc;
+	int ret;
+
+	rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
+	if (!rtc)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, rtc);
+
+	rtc->clk = devm_clk_get(&pdev->dev, "hclk");
+	if (IS_ERR(rtc->clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(rtc->clk), "Missing hclk\n");
+
+	rtc->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(rtc->base))
+		return dev_err_probe(&pdev->dev, PTR_ERR(rtc->base), "Missing reg\n");
+
+	rtc->rtcdev = devm_rtc_allocate_device(&pdev->dev);
+	if (IS_ERR(rtc->rtcdev))
+		return PTR_ERR(rtc);
+
+	rtc->rtcdev->range_max = 3178591199UL; /* 100 years */
+	rtc->rtcdev->ops = &rzn1_rtc_ops;
+
+	ret = r9a06g032_sysctrl_enable_rtc(true);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(rtc->clk);
+	if (ret)
+		goto disable_rtc;
+
+	/*
+	 * Ensure the clock counter is enabled.
+	 * Set 24-hour mode and possible oscillator offset compensation in SUBU mode.
+	 */
+	writel(RZN1_RTC_CTL0_CE | RZN1_RTC_CTL0_AMPM | RZN1_RTC_CTL0_SLSB_SUBU,
+	       rtc->base + RZN1_RTC_CTL0);
+
+	/* Disable all interrupts */
+	writel(0, rtc->base + RZN1_RTC_CTL1);
+
+	/* Enable counter operation */
+	writel(0, rtc->base + RZN1_RTC_CTL2);
+
+	ret = devm_rtc_register_device(rtc->rtcdev);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to register RTC\n");
+		goto disable_clk;
+	}
+
+	return 0;
+
+disable_clk:
+	clk_disable_unprepare(rtc->clk);
+disable_rtc:
+	r9a06g032_sysctrl_enable_rtc(false);
+
+	return ret;
+}
+
+static int rzn1_rtc_remove(struct platform_device *pdev)
+{
+	struct rzn1_rtc *rtc = platform_get_drvdata(pdev);
+
+	clk_disable_unprepare(rtc->clk);
+	r9a06g032_sysctrl_enable_rtc(false);
+
+	return 0;
+}
+
+static const struct of_device_id rzn1_rtc_of_match[] = {
+	{ .compatible	= "renesas,rzn1-rtc" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, rzn1_rtc_of_match);
+
+static struct platform_driver rzn1_rtc_driver = {
+	.probe = rzn1_rtc_probe,
+	.remove = rzn1_rtc_remove,
+	.driver = {
+		.name	= "rzn1-rtc",
+		.owner	= THIS_MODULE,
+		.of_match_table = rzn1_rtc_of_match,
+	},
+};
+module_platform_driver(rzn1_rtc_driver);
+
+MODULE_AUTHOR("Michel Pollet <Michel.Pollet@bp.renesas.com");
+MODULE_AUTHOR("Miquel Raynal <miquel.raynal@bootlin.com");
+MODULE_DESCRIPTION("RZ/N1 RTC driver");
+MODULE_LICENSE("GPL");
-- 
2.27.0


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

* [PATCH 4/7] rtc: rzn1: Add alarm support
  2022-04-05 18:47 [PATCH 0/7] RZN1 RTC support Miquel Raynal
                   ` (2 preceding siblings ...)
  2022-04-05 18:47 ` [PATCH 3/7] rtc: rzn1: Add new RTC driver Miquel Raynal
@ 2022-04-05 18:47 ` Miquel Raynal
  2022-04-06  9:10   ` Alexandre Belloni
  2022-04-05 18:47 ` [PATCH 5/7] rtc: rzn1: Add oscillator offset support Miquel Raynal
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Miquel Raynal @ 2022-04-05 18:47 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: Rob Herring, devicetree, Krzysztof Kozlowski, linux-renesas-soc,
	Magnus Damm, Gareth Williams, Phil Edworthy, Geert Uytterhoeven,
	Stephen Boyd, Michael Turquette, linux-clk, Milan Stevanovic,
	Jimmy Lalande, Pascal Eberhard, Thomas Petazzoni, Herve Codina,
	Clement Leger, linux-rtc, Miquel Raynal

The RZN1 RTC can trigger an interrupt when reaching a particular date up
to 7 days ahead. Bring support for this alarm.

One drawback though, the granularity is about a minute.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/rtc/rtc-rzn1.c | 108 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 108 insertions(+)

diff --git a/drivers/rtc/rtc-rzn1.c b/drivers/rtc/rtc-rzn1.c
index 15c533333930..85c5a68944a0 100644
--- a/drivers/rtc/rtc-rzn1.c
+++ b/drivers/rtc/rtc-rzn1.c
@@ -154,14 +154,110 @@ static int rzn1_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	return ret;
 }
 
+static irqreturn_t rzn1_rtc_alarm_irq(int irq, void *dev_id)
+{
+	struct rzn1_rtc *rtc = dev_id;
+
+	rtc_update_irq(rtc->rtcdev, 1, RTC_AF | RTC_IRQF);
+
+	return IRQ_HANDLED;
+}
+
+static int rzn1_rtc_alarm_irq_enable(struct device *dev, unsigned int enable)
+{
+	struct rzn1_rtc *rtc = dev_get_drvdata(dev);
+	u32 ctl1 = readl(rtc->base + RZN1_RTC_CTL1);
+
+	if (enable)
+		ctl1 |= RZN1_RTC_CTL1_ALME;
+	else
+		ctl1 &= ~RZN1_RTC_CTL1_ALME;
+
+	writel(ctl1, rtc->base + RZN1_RTC_CTL1);
+
+	return 0;
+}
+
+static int rzn1_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct rzn1_rtc *rtc = dev_get_drvdata(dev);
+	struct rtc_time *tm = &alrm->time;
+	unsigned int min, hour, wday, delta_days;
+	u32 ctl1;
+	int ret;
+
+	ret = rzn1_rtc_read_time(dev, tm);
+	if (ret)
+		return ret;
+
+	min = readl(rtc->base + RZN1_RTC_ALM);
+	hour = readl(rtc->base + RZN1_RTC_ALH);
+	wday = readl(rtc->base + RZN1_RTC_ALW);
+
+	tm->tm_sec = 0;
+	tm->tm_min = bcd2bin(min);
+	tm->tm_hour = bcd2bin(hour);
+	delta_days = ((fls(wday) - 1) - tm->tm_wday + 7) % 7;
+	tm->tm_wday = fls(wday) - 1;
+	tm->tm_mday += delta_days;
+	if (delta_days > rtc_month_days(tm->tm_mon, tm->tm_year)) {
+		tm->tm_mday %= rtc_month_days(tm->tm_mon, tm->tm_year);
+		tm->tm_mon++;
+	}
+	if (tm->tm_mon > 12) {
+		tm->tm_mon %= 12;
+		tm->tm_year++;
+	}
+
+	ctl1 = readl(rtc->base + RZN1_RTC_CTL1);
+	alrm->enabled = !!(ctl1 & RZN1_RTC_CTL1_ALME);
+
+	return 0;
+}
+
+static int rzn1_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct rzn1_rtc *rtc = dev_get_drvdata(dev);
+	struct rtc_time *tm = &alrm->time, tm_now;
+	unsigned long alarm, farest;
+	unsigned int days_ahead, wday;
+	int ret;
+
+	ret = rzn1_rtc_read_time(dev, &tm_now);
+	if (ret)
+		return ret;
+
+	/* We cannot set alarms more than one week ahead */
+	farest = rtc_tm_to_time64(&tm_now) + (7 * 86400);
+	alarm = rtc_tm_to_time64(tm);
+	if (time_after(alarm, farest))
+		return -EOPNOTSUPP;
+
+	/* Convert alarm day into week day */
+	days_ahead = tm->tm_mday - tm_now.tm_mday;
+	wday = (tm_now.tm_wday + days_ahead) % 7;
+
+	writel(bin2bcd(tm->tm_min), rtc->base + RZN1_RTC_ALM);
+	writel(bin2bcd(tm->tm_hour), rtc->base + RZN1_RTC_ALH);
+	writel(BIT(wday), rtc->base + RZN1_RTC_ALW);
+
+	rzn1_rtc_alarm_irq_enable(dev, alrm->enabled);
+
+	return 0;
+}
+
 static const struct rtc_class_ops rzn1_rtc_ops = {
 	.read_time = rzn1_rtc_read_time,
 	.set_time = rzn1_rtc_set_time,
+	.read_alarm = rzn1_rtc_read_alarm,
+	.set_alarm = rzn1_rtc_set_alarm,
+	.alarm_irq_enable = rzn1_rtc_alarm_irq_enable,
 };
 
 static int rzn1_rtc_probe(struct platform_device *pdev)
 {
 	struct rzn1_rtc *rtc;
+	int alarm_irq;
 	int ret;
 
 	rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
@@ -178,12 +274,17 @@ static int rzn1_rtc_probe(struct platform_device *pdev)
 	if (IS_ERR(rtc->base))
 		return dev_err_probe(&pdev->dev, PTR_ERR(rtc->base), "Missing reg\n");
 
+	alarm_irq = platform_get_irq(pdev, 0);
+	if (alarm_irq < 0)
+		return dev_err_probe(&pdev->dev, alarm_irq, "Missing timer IRQ\n");
+
 	rtc->rtcdev = devm_rtc_allocate_device(&pdev->dev);
 	if (IS_ERR(rtc->rtcdev))
 		return PTR_ERR(rtc);
 
 	rtc->rtcdev->range_max = 3178591199UL; /* 100 years */
 	rtc->rtcdev->ops = &rzn1_rtc_ops;
+	set_bit(RTC_FEATURE_ALARM_RES_MINUTE, rtc->rtcdev->features);
 
 	ret = r9a06g032_sysctrl_enable_rtc(true);
 	if (ret)
@@ -206,6 +307,13 @@ static int rzn1_rtc_probe(struct platform_device *pdev)
 	/* Enable counter operation */
 	writel(0, rtc->base + RZN1_RTC_CTL2);
 
+	ret = devm_request_irq(&pdev->dev, alarm_irq, rzn1_rtc_alarm_irq, 0,
+			       dev_name(&pdev->dev), rtc);
+	if (ret) {
+		dev_err(&pdev->dev, "RTC timer interrupt not available\n");
+		goto disable_clk;
+	}
+
 	ret = devm_rtc_register_device(rtc->rtcdev);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to register RTC\n");
-- 
2.27.0


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

* [PATCH 5/7] rtc: rzn1: Add oscillator offset support
  2022-04-05 18:47 [PATCH 0/7] RZN1 RTC support Miquel Raynal
                   ` (3 preceding siblings ...)
  2022-04-05 18:47 ` [PATCH 4/7] rtc: rzn1: Add alarm support Miquel Raynal
@ 2022-04-05 18:47 ` Miquel Raynal
  2022-04-05 18:47 ` [PATCH 6/7] MAINTAINERS: Add myself as maintainer of the RZN1 RTC driver Miquel Raynal
  2022-04-05 18:47 ` [PATCH 7/7] ARM: dts: r9a06g032: Describe the RTC Miquel Raynal
  6 siblings, 0 replies; 20+ messages in thread
From: Miquel Raynal @ 2022-04-05 18:47 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: Rob Herring, devicetree, Krzysztof Kozlowski, linux-renesas-soc,
	Magnus Damm, Gareth Williams, Phil Edworthy, Geert Uytterhoeven,
	Stephen Boyd, Michael Turquette, linux-clk, Milan Stevanovic,
	Jimmy Lalande, Pascal Eberhard, Thomas Petazzoni, Herve Codina,
	Clement Leger, linux-rtc, Miquel Raynal

The RZN1 RTC can compensate the imprecision of the oscillator up to
approximately 190ppm.

Seconds can last slightly shorter or longer depending on the
configuration.

Below ~65ppm of correction, we can change the time spent in a second
every minute, which is the most accurate compensation that the RTC can
offer. Above, the compensation will be active every 20s.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/rtc/rtc-rzn1.c | 73 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/drivers/rtc/rtc-rzn1.c b/drivers/rtc/rtc-rzn1.c
index 85c5a68944a0..64eb1ff20a24 100644
--- a/drivers/rtc/rtc-rzn1.c
+++ b/drivers/rtc/rtc-rzn1.c
@@ -246,12 +246,85 @@ static int rzn1_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	return 0;
 }
 
+static int rzn1_rtc_read_offset(struct device *dev, long *offset)
+{
+	struct rzn1_rtc *rtc = dev_get_drvdata(dev);
+	unsigned int ppb_per_step;
+	bool subtract;
+	u32 val;
+
+	val = readl(rtc->base + RZN1_RTC_SUBU);
+	ppb_per_step = val & RZN1_RTC_SUBU_DEV ? 1017 : 3051;
+	subtract = val & RZN1_RTC_SUBU_DECR;
+	val &= 0x3F;
+
+	if (!val)
+		*offset = 0;
+	else if (subtract)
+		*offset = -(((~val) & 0x3F) + 1) * ppb_per_step;
+	else
+		*offset = (val - 1) * ppb_per_step;
+
+	return 0;
+}
+
+static int rzn1_rtc_set_offset(struct device *dev, long offset)
+{
+	struct rzn1_rtc *rtc = dev_get_drvdata(dev);
+	unsigned int steps, ppb_per_step;
+	int stepsh, stepsl;
+	u32 val;
+	int ret;
+
+	/*
+	 * Check which resolution mode (every 20 or 60s) can be used.
+	 * Between 2 and 124 clock pulses can be added or substracted.
+	 *
+	 * In 20s mode, the minimum resolution is 2 / (32768 * 20) which is
+	 * close to 3051 ppb. In 60s mode, the resolution is closer to 1017.
+	 */
+	stepsh = DIV_ROUND_CLOSEST(offset, 1017);
+	stepsl = DIV_ROUND_CLOSEST(offset, 3051);
+
+	if (stepsh >= -0x3E && stepsh <= 0x3E) {
+		ppb_per_step = 1017;
+		steps = stepsh;
+		val |= RZN1_RTC_SUBU_DEV;
+	} else if (stepsl >= -0x3E && stepsl <= 0x3E) {
+		ppb_per_step = 3051;
+		steps = stepsl;
+	} else {
+		return -ERANGE;
+	}
+
+	if (!steps)
+		return 0;
+
+	if (steps > 0) {
+		val |= steps + 1;
+	} else {
+		val |= RZN1_RTC_SUBU_DECR;
+		val |= (~(-steps - 1)) & 0x3F;
+	}
+
+	ret = readl_poll_timeout(rtc->base + RZN1_RTC_CTL2, val,
+				 !(val & RZN1_RTC_CTL2_WUST), 100, 2000000);
+	if (ret)
+		return ret;
+
+	writel(val, rtc->base + RZN1_RTC_SUBU);
+
+	return 0;
+}
+
 static const struct rtc_class_ops rzn1_rtc_ops = {
 	.read_time = rzn1_rtc_read_time,
 	.set_time = rzn1_rtc_set_time,
 	.read_alarm = rzn1_rtc_read_alarm,
 	.set_alarm = rzn1_rtc_set_alarm,
 	.alarm_irq_enable = rzn1_rtc_alarm_irq_enable,
+	.read_offset = rzn1_rtc_read_offset,
+	.set_offset = rzn1_rtc_set_offset,
 };
 
 static int rzn1_rtc_probe(struct platform_device *pdev)
-- 
2.27.0


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

* [PATCH 6/7] MAINTAINERS: Add myself as maintainer of the RZN1 RTC driver
  2022-04-05 18:47 [PATCH 0/7] RZN1 RTC support Miquel Raynal
                   ` (4 preceding siblings ...)
  2022-04-05 18:47 ` [PATCH 5/7] rtc: rzn1: Add oscillator offset support Miquel Raynal
@ 2022-04-05 18:47 ` Miquel Raynal
  2022-04-11 15:22   ` Geert Uytterhoeven
  2022-04-05 18:47 ` [PATCH 7/7] ARM: dts: r9a06g032: Describe the RTC Miquel Raynal
  6 siblings, 1 reply; 20+ messages in thread
From: Miquel Raynal @ 2022-04-05 18:47 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: Rob Herring, devicetree, Krzysztof Kozlowski, linux-renesas-soc,
	Magnus Damm, Gareth Williams, Phil Edworthy, Geert Uytterhoeven,
	Stephen Boyd, Michael Turquette, linux-clk, Milan Stevanovic,
	Jimmy Lalande, Pascal Eberhard, Thomas Petazzoni, Herve Codina,
	Clement Leger, linux-rtc, Miquel Raynal

After contributing it, I'll volunteer to maintain it.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 120d3ae57a4b..a2bafa05ca60 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16849,6 +16849,14 @@ S:	Supported
 F:	Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml
 F:	drivers/iio/adc/rzg2l_adc.c
 
+RENESAS RZ/N1 RTC CONTROLLER DRIVER
+M:	Miquel Raynal <miquel.raynal@bootlin.com>
+L:	linux-rtc@vger.kernel.org
+L:	linux-renesas-soc@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/rtc/renesas,rzn1-rtc.yaml
+F:	drivers/rtc/rtc-rzn1.c
+
 RENESAS R-CAR GEN3 & RZ/N1 NAND CONTROLLER DRIVER
 M:	Miquel Raynal <miquel.raynal@bootlin.com>
 L:	linux-mtd@lists.infradead.org
-- 
2.27.0


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

* [PATCH 7/7] ARM: dts: r9a06g032: Describe the RTC
  2022-04-05 18:47 [PATCH 0/7] RZN1 RTC support Miquel Raynal
                   ` (5 preceding siblings ...)
  2022-04-05 18:47 ` [PATCH 6/7] MAINTAINERS: Add myself as maintainer of the RZN1 RTC driver Miquel Raynal
@ 2022-04-05 18:47 ` Miquel Raynal
  2022-04-11 15:32   ` Geert Uytterhoeven
  6 siblings, 1 reply; 20+ messages in thread
From: Miquel Raynal @ 2022-04-05 18:47 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: Rob Herring, devicetree, Krzysztof Kozlowski, linux-renesas-soc,
	Magnus Damm, Gareth Williams, Phil Edworthy, Geert Uytterhoeven,
	Stephen Boyd, Michael Turquette, linux-clk, Milan Stevanovic,
	Jimmy Lalande, Pascal Eberhard, Thomas Petazzoni, Herve Codina,
	Clement Leger, linux-rtc, Miquel Raynal

Describe the SoC RTC which counts time and provides alarm support.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 arch/arm/boot/dts/r9a06g032.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/r9a06g032.dtsi b/arch/arm/boot/dts/r9a06g032.dtsi
index 4288b935fcea..7d380a38c7cd 100644
--- a/arch/arm/boot/dts/r9a06g032.dtsi
+++ b/arch/arm/boot/dts/r9a06g032.dtsi
@@ -103,6 +103,18 @@ dmamux: dma-router@a0 {
 			};
 		};
 
+		rtc0: rtc@40006000 {
+			compatible = "renesas,r9a06g032-rtc", "renesas,rzn1-rtc";
+			reg = <0x40006000 0x1000>;
+			interrupts = <GIC_SPI 66 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 67 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 68 IRQ_TYPE_EDGE_RISING>;
+			interrupt-names = "alarm", "timer", "pps";
+			clocks = <&sysctrl R9A06G032_HCLK_RTC>;
+			clock-names = "hclk";
+			status = "disabled";
+		};
+
 		uart0: serial@40060000 {
 			compatible = "renesas,r9a06g032-uart", "renesas,rzn1-uart", "snps,dw-apb-uart";
 			reg = <0x40060000 0x400>;
-- 
2.27.0


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

* Re: [PATCH 2/7] soc: renesas: rzn1-sysc: Export a function to enable/disable the RTC
  2022-04-05 18:47 ` [PATCH 2/7] soc: renesas: rzn1-sysc: Export a function to enable/disable the RTC Miquel Raynal
@ 2022-04-06  7:00   ` Thomas Petazzoni
  2022-04-06  7:43     ` Miquel Raynal
  2022-04-06  8:32   ` Alexandre Belloni
  1 sibling, 1 reply; 20+ messages in thread
From: Thomas Petazzoni @ 2022-04-06  7:00 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Alessandro Zummo, Alexandre Belloni, Rob Herring, devicetree,
	Krzysztof Kozlowski, linux-renesas-soc, Magnus Damm,
	Gareth Williams, Phil Edworthy, Geert Uytterhoeven, Stephen Boyd,
	Michael Turquette, linux-clk, Milan Stevanovic, Jimmy Lalande,
	Pascal Eberhard, Herve Codina, Clement Leger, linux-rtc

Hello Miquèl,

On Tue,  5 Apr 2022 20:47:11 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> +	spin_lock_irqsave(&sysctrl_priv->lock, flags);

Taking a lock here...

> +
> +	if (enable) {
> +		val = readl(sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
> +		val &= ~R9A06G032_SYSCTRL_PWRCTRL_RTC_RST;
> +		writel(val, sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
> +		val |= R9A06G032_SYSCTRL_PWRCTRL_RTC_CLKEN;
> +		writel(val, sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
> +		val |= R9A06G032_SYSCTRL_PWRCTRL_RTC_RSTN_FW;
> +		writel(val, sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
> +		val &= ~R9A06G032_SYSCTRL_PWRCTRL_RTC_IDLE_REQ;
> +		writel(val, sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
> +		val = readl(sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRSTAT_RTC);
> +		if (val & R9A06G032_SYSCTRL_PWRSTAT_RTC_IDLE)
> +			return -EIO;

And forgetting to release it here in the error case.

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com

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

* Re: [PATCH 2/7] soc: renesas: rzn1-sysc: Export a function to enable/disable the RTC
  2022-04-06  7:00   ` Thomas Petazzoni
@ 2022-04-06  7:43     ` Miquel Raynal
  0 siblings, 0 replies; 20+ messages in thread
From: Miquel Raynal @ 2022-04-06  7:43 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: Alessandro Zummo, Alexandre Belloni, Rob Herring, devicetree,
	Krzysztof Kozlowski, linux-renesas-soc, Magnus Damm,
	Gareth Williams, Phil Edworthy, Geert Uytterhoeven, Stephen Boyd,
	Michael Turquette, linux-clk, Milan Stevanovic, Jimmy Lalande,
	Pascal Eberhard, Herve Codina, Clement Leger, linux-rtc

Hi Thomas,

thomas.petazzoni@bootlin.com wrote on Wed, 6 Apr 2022 09:00:04 +0200:

> Hello Miquèl,
> 
> On Tue,  5 Apr 2022 20:47:11 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > +	spin_lock_irqsave(&sysctrl_priv->lock, flags);  
> 
> Taking a lock here...
> 
> > +
> > +	if (enable) {
> > +		val = readl(sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
> > +		val &= ~R9A06G032_SYSCTRL_PWRCTRL_RTC_RST;
> > +		writel(val, sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
> > +		val |= R9A06G032_SYSCTRL_PWRCTRL_RTC_CLKEN;
> > +		writel(val, sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
> > +		val |= R9A06G032_SYSCTRL_PWRCTRL_RTC_RSTN_FW;
> > +		writel(val, sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
> > +		val &= ~R9A06G032_SYSCTRL_PWRCTRL_RTC_IDLE_REQ;
> > +		writel(val, sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
> > +		val = readl(sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRSTAT_RTC);
> > +		if (val & R9A06G032_SYSCTRL_PWRSTAT_RTC_IDLE)
> > +			return -EIO;  
> 
> And forgetting to release it here in the error case.

Right, I'll fix it.

Thanks!
Miquèl

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

* Re: [PATCH 2/7] soc: renesas: rzn1-sysc: Export a function to enable/disable the RTC
  2022-04-05 18:47 ` [PATCH 2/7] soc: renesas: rzn1-sysc: Export a function to enable/disable the RTC Miquel Raynal
  2022-04-06  7:00   ` Thomas Petazzoni
@ 2022-04-06  8:32   ` Alexandre Belloni
  1 sibling, 0 replies; 20+ messages in thread
From: Alexandre Belloni @ 2022-04-06  8:32 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Alessandro Zummo, Rob Herring, devicetree, Krzysztof Kozlowski,
	linux-renesas-soc, Magnus Damm, Gareth Williams, Phil Edworthy,
	Geert Uytterhoeven, Stephen Boyd, Michael Turquette, linux-clk,
	Milan Stevanovic, Jimmy Lalande, Pascal Eberhard,
	Thomas Petazzoni, Herve Codina, Clement Leger, linux-rtc

On 05/04/2022 20:47:11+0200, Miquel Raynal wrote:
> There are two RTC registers located within the system controller.
> 
> Like with the dmamux register, let's add a new helper to enable/disable
> the power, reset and clock of the RTC.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/clk/renesas/r9a06g032-clocks.c        | 49 +++++++++++++++++++
>  include/linux/soc/renesas/r9a06g032-sysctrl.h |  2 +
>  2 files changed, 51 insertions(+)
> 
> diff --git a/drivers/clk/renesas/r9a06g032-clocks.c b/drivers/clk/renesas/r9a06g032-clocks.c
> index 1df56d7ab3e1..7e61db39a43b 100644
> --- a/drivers/clk/renesas/r9a06g032-clocks.c
> +++ b/drivers/clk/renesas/r9a06g032-clocks.c
> @@ -26,6 +26,13 @@
>  #include <dt-bindings/clock/r9a06g032-sysctrl.h>
>  
>  #define R9A06G032_SYSCTRL_DMAMUX 0xA0
> +#define R9A06G032_SYSCTRL_PWRCTRL_RTC 0x140
> +#define   R9A06G032_SYSCTRL_PWRCTRL_RTC_CLKEN BIT(0)
> +#define   R9A06G032_SYSCTRL_PWRCTRL_RTC_RST BIT(1)
> +#define   R9A06G032_SYSCTRL_PWRCTRL_RTC_IDLE_REQ BIT(2)
> +#define   R9A06G032_SYSCTRL_PWRCTRL_RTC_RSTN_FW BIT(3)
> +#define R9A06G032_SYSCTRL_PWRSTAT_RTC 0x144
> +#define   R9A06G032_SYSCTRL_PWRSTAT_RTC_IDLE BIT(1)
>  
>  struct r9a06g032_gate {
>  	u16 gate, reset, ready, midle,
> @@ -343,6 +350,48 @@ int r9a06g032_sysctrl_set_dmamux(u32 mask, u32 val)
>  }
>  EXPORT_SYMBOL_GPL(r9a06g032_sysctrl_set_dmamux);
>  
> +/* Exported helper to enable/disable the RTC */
> +int r9a06g032_sysctrl_enable_rtc(bool enable)
> +{
> +	unsigned long flags;
> +	u32 val;
> +
> +	if (!sysctrl_priv)
> +		return -EPROBE_DEFER;
> +
> +	spin_lock_irqsave(&sysctrl_priv->lock, flags);
> +
> +	if (enable) {
> +		val = readl(sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
> +		val &= ~R9A06G032_SYSCTRL_PWRCTRL_RTC_RST;
> +		writel(val, sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
> +		val |= R9A06G032_SYSCTRL_PWRCTRL_RTC_CLKEN;
> +		writel(val, sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
> +		val |= R9A06G032_SYSCTRL_PWRCTRL_RTC_RSTN_FW;
> +		writel(val, sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
> +		val &= ~R9A06G032_SYSCTRL_PWRCTRL_RTC_IDLE_REQ;
> +		writel(val, sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
> +		val = readl(sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRSTAT_RTC);
> +		if (val & R9A06G032_SYSCTRL_PWRSTAT_RTC_IDLE)
> +			return -EIO;
> +	} else {
> +		val = readl(sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
> +		val |= R9A06G032_SYSCTRL_PWRCTRL_RTC_IDLE_REQ;
> +		writel(val, sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
> +		val &= ~R9A06G032_SYSCTRL_PWRCTRL_RTC_RSTN_FW;
> +		writel(val, sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
> +		val &= ~R9A06G032_SYSCTRL_PWRCTRL_RTC_CLKEN;
> +		writel(val, sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
> +		val |= R9A06G032_SYSCTRL_PWRCTRL_RTC_RST;
> +		writel(val, sysctrl_priv->reg + R9A06G032_SYSCTRL_PWRCTRL_RTC);
> +	}
> +
> +	spin_unlock_irqrestore(&sysctrl_priv->lock, flags);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(r9a06g032_sysctrl_enable_rtc);
> +
>  /* register/bit pairs are encoded as an uint16_t */
>  static void
>  clk_rdesc_set(struct r9a06g032_priv *clocks,
> diff --git a/include/linux/soc/renesas/r9a06g032-sysctrl.h b/include/linux/soc/renesas/r9a06g032-sysctrl.h
> index 066dfb15cbdd..914c8789149c 100644
> --- a/include/linux/soc/renesas/r9a06g032-sysctrl.h
> +++ b/include/linux/soc/renesas/r9a06g032-sysctrl.h
> @@ -4,8 +4,10 @@
>  
>  #ifdef CONFIG_CLK_R9A06G032
>  int r9a06g032_sysctrl_set_dmamux(u32 mask, u32 val);
> +int r9a06g032_sysctrl_enable_rtc(bool enable);
>  #else
>  static inline int r9a06g032_sysctrl_set_dmamux(u32 mask, u32 val) { return -ENODEV; }
> +static inline int r9a06g032_sysctrl_enable_rtc(bool enable) { return -ENODEV; }

Couldn't that be handled using the reset subsystem to avoid leaking a
random API in the RTC driver? (and that would remove the build
dependency)

>  #endif
>  
>  #endif /* __LINUX_SOC_RENESAS_R9A06G032_SYSCTRL_H__ */
> -- 
> 2.27.0
> 

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

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

* Re: [PATCH 3/7] rtc: rzn1: Add new RTC driver
  2022-04-05 18:47 ` [PATCH 3/7] rtc: rzn1: Add new RTC driver Miquel Raynal
@ 2022-04-06  8:50   ` Alexandre Belloni
  2022-04-13 15:23     ` Miquel Raynal
  0 siblings, 1 reply; 20+ messages in thread
From: Alexandre Belloni @ 2022-04-06  8:50 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Alessandro Zummo, Rob Herring, devicetree, Krzysztof Kozlowski,
	linux-renesas-soc, Magnus Damm, Gareth Williams, Phil Edworthy,
	Geert Uytterhoeven, Stephen Boyd, Michael Turquette, linux-clk,
	Milan Stevanovic, Jimmy Lalande, Pascal Eberhard,
	Thomas Petazzoni, Herve Codina, Clement Leger, linux-rtc,
	Michel Pollet

On 05/04/2022 20:47:12+0200, Miquel Raynal wrote:
> From: Michel Pollet <michel.pollet@bp.renesas.com>
> 
> Add a basic RTC driver for the RZ/N1.
> 
> Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
> Co-developed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/rtc/Kconfig    |   7 ++
>  drivers/rtc/Makefile   |   1 +
>  drivers/rtc/rtc-rzn1.c | 255 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 263 insertions(+)
>  create mode 100644 drivers/rtc/rtc-rzn1.c
> 
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index 41c65b4d2baf..f4d72c5b99ea 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -1548,6 +1548,13 @@ config RTC_DRV_RS5C313
>  	help
>  	  If you say yes here you get support for the Ricoh RS5C313 RTC chips.
>  
> +config RTC_DRV_RZN1
> +	tristate "Renesas RZN1 RTC"
> +	depends on ARCH_RZN1 || COMPILE_TEST
> +	depends on OF && HAS_IOMEM
> +	help
> +	  If you say yes here you get support for the Renesas RZ/N1 RTC.
> +
>  config RTC_DRV_GENERIC
>  	tristate "Generic RTC support"
>  	# Please consider writing a new RTC driver instead of using the generic
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> index 2d827d8261d5..fb04467b652d 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -151,6 +151,7 @@ obj-$(CONFIG_RTC_DRV_RX6110)	+= rtc-rx6110.o
>  obj-$(CONFIG_RTC_DRV_RX8010)	+= rtc-rx8010.o
>  obj-$(CONFIG_RTC_DRV_RX8025)	+= rtc-rx8025.o
>  obj-$(CONFIG_RTC_DRV_RX8581)	+= rtc-rx8581.o
> +obj-$(CONFIG_RTC_DRV_RZN1)	+= rtc-rzn1.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-rzn1.c b/drivers/rtc/rtc-rzn1.c
> new file mode 100644
> index 000000000000..15c533333930
> --- /dev/null
> +++ b/drivers/rtc/rtc-rzn1.c
> @@ -0,0 +1,255 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Renesas RZN1 Real Time Clock interface for Linux
> + *
> + * Copyright:
> + * - 2014 Renesas Electronics Europe Limited
> + * - 2022 Schneider Electric
> + *
> + * Authors:
> + * - Michel Pollet <michel.pollet@bp.renesas.com>, <buserror@gmail.com>
> + * - Miquel Raynal <miquel.raynal@bootlin.com>
> + */
> +
> +#include <linux/bcd.h>
> +#include <linux/clk.h>
> +#include <linux/init.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/rtc.h>
> +#include <linux/soc/renesas/r9a06g032-sysctrl.h>
> +
> +#define RZN1_RTC_CTL0 0x00
> +#define   RZN1_RTC_CTL0_SLSB_SUBU 0
> +#define   RZN1_RTC_CTL0_SLSB_SCMP BIT(4)
> +#define   RZN1_RTC_CTL0_AMPM BIT(5)
> +#define   RZN1_RTC_CTL0_CE BIT(7)
> +
> +#define RZN1_RTC_CTL1 0x04
> +#define   RZN1_RTC_CTL1_ALME BIT(4)
> +
> +#define RZN1_RTC_CTL2 0x08
> +#define   RZN1_RTC_CTL2_WAIT BIT(0)
> +#define   RZN1_RTC_CTL2_WST BIT(1)
> +#define   RZN1_RTC_CTL2_WUST BIT(5)
> +
> +#define RZN1_RTC_SEC 0x14
> +#define RZN1_RTC_MIN 0x18
> +#define RZN1_RTC_HOUR 0x1c
> +#define RZN1_RTC_WEEK 0x20
> +#define RZN1_RTC_DAY 0x24
> +#define RZN1_RTC_MONTH 0x28
> +#define RZN1_RTC_YEAR 0x2c
> +
> +#define RZN1_RTC_SUBU 0x38
> +#define   RZN1_RTC_SUBU_DEV BIT(7)
> +#define   RZN1_RTC_SUBU_DECR BIT(6)
> +
> +#define RZN1_RTC_ALM 0x40
> +#define RZN1_RTC_ALH 0x44
> +#define RZN1_RTC_ALW 0x48
> +
> +#define RZN1_RTC_SECC 0x4c
> +#define RZN1_RTC_MINC 0x50
> +#define RZN1_RTC_HOURC 0x54
> +#define RZN1_RTC_WEEKC 0x58
> +#define RZN1_RTC_DAYC 0x5c
> +#define RZN1_RTC_MONTHC 0x60
> +#define RZN1_RTC_YEARC 0x64
> +
> +struct rzn1_rtc {
> +	struct rtc_device *rtcdev;
> +	void __iomem *base;
> +	struct clk *clk;
> +};
> +
> +static void rzn1_rtc_get_time_snapshot(struct rzn1_rtc *rtc, struct rtc_time *tm)
> +{
> +	tm->tm_sec = readl(rtc->base + RZN1_RTC_SECC);
> +	tm->tm_min = readl(rtc->base + RZN1_RTC_MINC);
> +	tm->tm_hour = readl(rtc->base + RZN1_RTC_HOURC);
> +	tm->tm_wday = readl(rtc->base + RZN1_RTC_WEEKC);
> +	tm->tm_mday = readl(rtc->base + RZN1_RTC_DAYC);
> +	tm->tm_mon = readl(rtc->base + RZN1_RTC_MONTHC);
> +	tm->tm_year = readl(rtc->base + RZN1_RTC_YEARC);
> +}
> +
> +static unsigned int rzn1_rtc_tm_to_wday(struct rtc_time *tm)
> +{
> +	time64_t time;
> +	unsigned int days;
> +	u32 secs;
> +
> +	time = rtc_tm_to_time64(tm);
> +	days = div_s64_rem(time, 86400, &secs);
> +
> +	/* day of the week, 1970-01-01 was a Thursday */
> +	return (days + 4) % 7;
> +}
> +
> +static int rzn1_rtc_read_time(struct device *dev, struct rtc_time *tm)
> +{
> +	struct rzn1_rtc *rtc = dev_get_drvdata(dev);
> +	u32 secs;
> +
> +	rzn1_rtc_get_time_snapshot(rtc, tm);
> +	secs = readl(rtc->base + RZN1_RTC_SECC);
> +	if (tm->tm_sec != secs)
> +		rzn1_rtc_get_time_snapshot(rtc, tm);
> +
> +	tm->tm_sec = bcd2bin(tm->tm_sec);
> +	tm->tm_min = bcd2bin(tm->tm_min);
> +	tm->tm_hour = bcd2bin(tm->tm_hour);
> +	tm->tm_wday = bcd2bin(tm->tm_wday);
> +	tm->tm_mday = bcd2bin(tm->tm_mday);
> +	tm->tm_mon = bcd2bin(tm->tm_mon);
> +	tm->tm_year = bcd2bin(tm->tm_year);
> +
> +	dev_dbg(dev, "%d-%d-%d(%d)T%d:%d:%d\n",
> +		tm->tm_year, tm->tm_mon, tm->tm_mday, tm->tm_wday,
> +		tm->tm_hour, tm->tm_min, tm->tm_sec);
> +

This is not really useful because we have tracepoints in the core.
Anyway, please use %ptR.

> +	return 0;
> +}
> +
> +static int rzn1_rtc_set_time(struct device *dev, struct rtc_time *tm)
> +{
> +	struct rzn1_rtc *rtc = dev_get_drvdata(dev);
> +	u32 val;
> +	int ret;
> +
> +	tm->tm_sec = bin2bcd(tm->tm_sec);
> +	tm->tm_min = bin2bcd(tm->tm_min);
> +	tm->tm_hour = bin2bcd(tm->tm_hour);
> +	tm->tm_wday = bin2bcd(rzn1_rtc_tm_to_wday(tm));
> +	tm->tm_mday = bin2bcd(tm->tm_mday);
> +	tm->tm_mon = bin2bcd(tm->tm_mon);
> +	tm->tm_year = bin2bcd(tm->tm_year);
> +
> +	/* Hold the counter */
> +	val = readl(rtc->base + RZN1_RTC_CTL2);
> +	val |= RZN1_RTC_CTL2_WAIT;
> +	writel(val, rtc->base + RZN1_RTC_CTL2);
> +
> +	/* Wait for the counter to stop: two 32k clock cycles */
> +	usleep_range(61, 100);
> +	ret = readl_poll_timeout(rtc->base + RZN1_RTC_CTL2, val,
> +				 val & RZN1_RTC_CTL2_WST, 0, 100);
> +	if (!ret) {
> +		writel(tm->tm_sec, rtc->base + RZN1_RTC_SEC);
> +		writel(tm->tm_min, rtc->base + RZN1_RTC_MIN);
> +		writel(tm->tm_hour, rtc->base + RZN1_RTC_HOUR);
> +		writel(tm->tm_wday, rtc->base + RZN1_RTC_WEEK);
> +		writel(tm->tm_mday, rtc->base + RZN1_RTC_DAY);
> +		writel(tm->tm_mon, rtc->base + RZN1_RTC_MONTH);
> +		writel(tm->tm_year, rtc->base + RZN1_RTC_YEAR);
> +	}
> +
> +	/* Release the counter back */
> +	val &= ~RZN1_RTC_CTL2_WAIT;
> +	writel(val, rtc->base + RZN1_RTC_CTL2);
> +
> +	return ret;
> +}
> +
> +static const struct rtc_class_ops rzn1_rtc_ops = {
> +	.read_time = rzn1_rtc_read_time,
> +	.set_time = rzn1_rtc_set_time,
> +};
> +
> +static int rzn1_rtc_probe(struct platform_device *pdev)
> +{
> +	struct rzn1_rtc *rtc;
> +	int ret;
> +
> +	rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
> +	if (!rtc)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, rtc);
> +
> +	rtc->clk = devm_clk_get(&pdev->dev, "hclk");
> +	if (IS_ERR(rtc->clk))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(rtc->clk), "Missing hclk\n");
> +
> +	rtc->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(rtc->base))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(rtc->base), "Missing reg\n");
> +
> +	rtc->rtcdev = devm_rtc_allocate_device(&pdev->dev);
> +	if (IS_ERR(rtc->rtcdev))
> +		return PTR_ERR(rtc);
> +
> +	rtc->rtcdev->range_max = 3178591199UL; /* 100 years */

I'm not sure how you came to this value, this is 2070-09-22T05:59:59.
I'm pretty sure the RTC will not fail at that time. Also, the comment
seems fishy.


> +	rtc->rtcdev->ops = &rzn1_rtc_ops;
> +
> +	ret = r9a06g032_sysctrl_enable_rtc(true);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_prepare_enable(rtc->clk);
> +	if (ret)
> +		goto disable_rtc;
> +
> +	/*
> +	 * Ensure the clock counter is enabled.
> +	 * Set 24-hour mode and possible oscillator offset compensation in SUBU mode.
> +	 */
> +	writel(RZN1_RTC_CTL0_CE | RZN1_RTC_CTL0_AMPM | RZN1_RTC_CTL0_SLSB_SUBU,
> +	       rtc->base + RZN1_RTC_CTL0);
> +
> +	/* Disable all interrupts */
> +	writel(0, rtc->base + RZN1_RTC_CTL1);
> +
> +	/* Enable counter operation */
> +	writel(0, rtc->base + RZN1_RTC_CTL2);
> +

I don't think you should do that unconditionally. The RTC is either
not already started (and doesn't carry the proper time/date) or already
started. It would be better to start it in .set_time. Maybe you can even
use that to detect whether it has already been set once.

> +	ret = devm_rtc_register_device(rtc->rtcdev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to register RTC\n");

No error message is needed here.

> +		goto disable_clk;
> +	}
> +
> +	return 0;
> +
> +disable_clk:
> +	clk_disable_unprepare(rtc->clk);
> +disable_rtc:
> +	r9a06g032_sysctrl_enable_rtc(false);
> +
> +	return ret;
> +}
> +
> +static int rzn1_rtc_remove(struct platform_device *pdev)
> +{
> +	struct rzn1_rtc *rtc = platform_get_drvdata(pdev);
> +
> +	clk_disable_unprepare(rtc->clk);
> +	r9a06g032_sysctrl_enable_rtc(false);

Does this stop the RTC or just the register interface?

> +
> +	return 0;
> +}
> +
> +static const struct of_device_id rzn1_rtc_of_match[] = {
> +	{ .compatible	= "renesas,rzn1-rtc" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, rzn1_rtc_of_match);
> +
> +static struct platform_driver rzn1_rtc_driver = {
> +	.probe = rzn1_rtc_probe,
> +	.remove = rzn1_rtc_remove,
> +	.driver = {
> +		.name	= "rzn1-rtc",
> +		.owner	= THIS_MODULE,
> +		.of_match_table = rzn1_rtc_of_match,
> +	},
> +};
> +module_platform_driver(rzn1_rtc_driver);
> +
> +MODULE_AUTHOR("Michel Pollet <Michel.Pollet@bp.renesas.com");
> +MODULE_AUTHOR("Miquel Raynal <miquel.raynal@bootlin.com");
> +MODULE_DESCRIPTION("RZ/N1 RTC driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.27.0
> 

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

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

* Re: [PATCH 4/7] rtc: rzn1: Add alarm support
  2022-04-05 18:47 ` [PATCH 4/7] rtc: rzn1: Add alarm support Miquel Raynal
@ 2022-04-06  9:10   ` Alexandre Belloni
  0 siblings, 0 replies; 20+ messages in thread
From: Alexandre Belloni @ 2022-04-06  9:10 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Alessandro Zummo, Rob Herring, devicetree, Krzysztof Kozlowski,
	linux-renesas-soc, Magnus Damm, Gareth Williams, Phil Edworthy,
	Geert Uytterhoeven, Stephen Boyd, Michael Turquette, linux-clk,
	Milan Stevanovic, Jimmy Lalande, Pascal Eberhard,
	Thomas Petazzoni, Herve Codina, Clement Leger, linux-rtc

On 05/04/2022 20:47:13+0200, Miquel Raynal wrote:
> The RZN1 RTC can trigger an interrupt when reaching a particular date up
> to 7 days ahead. Bring support for this alarm.
> 
> One drawback though, the granularity is about a minute.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/rtc/rtc-rzn1.c | 108 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 108 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-rzn1.c b/drivers/rtc/rtc-rzn1.c
> index 15c533333930..85c5a68944a0 100644
> --- a/drivers/rtc/rtc-rzn1.c
> +++ b/drivers/rtc/rtc-rzn1.c
> @@ -154,14 +154,110 @@ static int rzn1_rtc_set_time(struct device *dev, struct rtc_time *tm)
>  	return ret;
>  }
>  
> +static irqreturn_t rzn1_rtc_alarm_irq(int irq, void *dev_id)
> +{
> +	struct rzn1_rtc *rtc = dev_id;
> +
> +	rtc_update_irq(rtc->rtcdev, 1, RTC_AF | RTC_IRQF);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int rzn1_rtc_alarm_irq_enable(struct device *dev, unsigned int enable)
> +{
> +	struct rzn1_rtc *rtc = dev_get_drvdata(dev);
> +	u32 ctl1 = readl(rtc->base + RZN1_RTC_CTL1);
> +
> +	if (enable)
> +		ctl1 |= RZN1_RTC_CTL1_ALME;
> +	else
> +		ctl1 &= ~RZN1_RTC_CTL1_ALME;
> +
> +	writel(ctl1, rtc->base + RZN1_RTC_CTL1);
> +
> +	return 0;
> +}
> +
> +static int rzn1_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> +{
> +	struct rzn1_rtc *rtc = dev_get_drvdata(dev);
> +	struct rtc_time *tm = &alrm->time;
> +	unsigned int min, hour, wday, delta_days;
> +	u32 ctl1;
> +	int ret;
> +
> +	ret = rzn1_rtc_read_time(dev, tm);
> +	if (ret)
> +		return ret;
> +
> +	min = readl(rtc->base + RZN1_RTC_ALM);
> +	hour = readl(rtc->base + RZN1_RTC_ALH);
> +	wday = readl(rtc->base + RZN1_RTC_ALW);
> +
> +	tm->tm_sec = 0;
> +	tm->tm_min = bcd2bin(min);
> +	tm->tm_hour = bcd2bin(hour);
> +	delta_days = ((fls(wday) - 1) - tm->tm_wday + 7) % 7;
> +	tm->tm_wday = fls(wday) - 1;
> +	tm->tm_mday += delta_days;
> +	if (delta_days > rtc_month_days(tm->tm_mon, tm->tm_year)) {
> +		tm->tm_mday %= rtc_month_days(tm->tm_mon, tm->tm_year);
> +		tm->tm_mon++;
> +	}
> +	if (tm->tm_mon > 12) {
> +		tm->tm_mon %= 12;
> +		tm->tm_year++;
> +	}

I guess you could avoid having to handle rollover by making the
calculations on a time64_t and then convert back to a tm. I don't think
this would be much worse in terms of processing.

> +
> +	ctl1 = readl(rtc->base + RZN1_RTC_CTL1);
> +	alrm->enabled = !!(ctl1 & RZN1_RTC_CTL1_ALME);
> +
> +	return 0;
> +}
> +
> +static int rzn1_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> +{
> +	struct rzn1_rtc *rtc = dev_get_drvdata(dev);
> +	struct rtc_time *tm = &alrm->time, tm_now;
> +	unsigned long alarm, farest;
> +	unsigned int days_ahead, wday;
> +	int ret;
> +
> +	ret = rzn1_rtc_read_time(dev, &tm_now);
> +	if (ret)
> +		return ret;
> +
> +	/* We cannot set alarms more than one week ahead */
> +	farest = rtc_tm_to_time64(&tm_now) + (7 * 86400);
> +	alarm = rtc_tm_to_time64(tm);
> +	if (time_after(alarm, farest))
> +		return -EOPNOTSUPP;
> +

I would return -ERANGE

> +	/* Convert alarm day into week day */
> +	days_ahead = tm->tm_mday - tm_now.tm_mday;
> +	wday = (tm_now.tm_wday + days_ahead) % 7;
> +
> +	writel(bin2bcd(tm->tm_min), rtc->base + RZN1_RTC_ALM);
> +	writel(bin2bcd(tm->tm_hour), rtc->base + RZN1_RTC_ALH);
> +	writel(BIT(wday), rtc->base + RZN1_RTC_ALW);
> +
> +	rzn1_rtc_alarm_irq_enable(dev, alrm->enabled);
> +
> +	return 0;
> +}
> +
>  static const struct rtc_class_ops rzn1_rtc_ops = {
>  	.read_time = rzn1_rtc_read_time,
>  	.set_time = rzn1_rtc_set_time,
> +	.read_alarm = rzn1_rtc_read_alarm,
> +	.set_alarm = rzn1_rtc_set_alarm,
> +	.alarm_irq_enable = rzn1_rtc_alarm_irq_enable,
>  };
>  
>  static int rzn1_rtc_probe(struct platform_device *pdev)
>  {
>  	struct rzn1_rtc *rtc;
> +	int alarm_irq;
>  	int ret;
>  
>  	rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
> @@ -178,12 +274,17 @@ static int rzn1_rtc_probe(struct platform_device *pdev)
>  	if (IS_ERR(rtc->base))
>  		return dev_err_probe(&pdev->dev, PTR_ERR(rtc->base), "Missing reg\n");
>  
> +	alarm_irq = platform_get_irq(pdev, 0);
> +	if (alarm_irq < 0)
> +		return dev_err_probe(&pdev->dev, alarm_irq, "Missing timer IRQ\n");
> +
>  	rtc->rtcdev = devm_rtc_allocate_device(&pdev->dev);
>  	if (IS_ERR(rtc->rtcdev))
>  		return PTR_ERR(rtc);
>  
>  	rtc->rtcdev->range_max = 3178591199UL; /* 100 years */
>  	rtc->rtcdev->ops = &rzn1_rtc_ops;
> +	set_bit(RTC_FEATURE_ALARM_RES_MINUTE, rtc->rtcdev->features);

You should probably clear RTC_FEATURE_UPDATE_INTERRUPT too.

>  
>  	ret = r9a06g032_sysctrl_enable_rtc(true);
>  	if (ret)
> @@ -206,6 +307,13 @@ static int rzn1_rtc_probe(struct platform_device *pdev)
>  	/* Enable counter operation */
>  	writel(0, rtc->base + RZN1_RTC_CTL2);
>  
> +	ret = devm_request_irq(&pdev->dev, alarm_irq, rzn1_rtc_alarm_irq, 0,
> +			       dev_name(&pdev->dev), rtc);
> +	if (ret) {
> +		dev_err(&pdev->dev, "RTC timer interrupt not available\n");
> +		goto disable_clk;
> +	}
> +
>  	ret = devm_rtc_register_device(rtc->rtcdev);
>  	if (ret) {
>  		dev_err(&pdev->dev, "Failed to register RTC\n");
> -- 
> 2.27.0
> 

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

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

* Re: [PATCH 1/7] dt-bindings: rtc: rzn1: Describe the RZN1 RTC
  2022-04-05 18:47 ` [PATCH 1/7] dt-bindings: rtc: rzn1: Describe the RZN1 RTC Miquel Raynal
@ 2022-04-07  7:37   ` Krzysztof Kozlowski
  2022-04-07  9:21     ` Miquel Raynal
  0 siblings, 1 reply; 20+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-07  7:37 UTC (permalink / raw)
  To: Miquel Raynal, Alessandro Zummo, Alexandre Belloni
  Cc: Rob Herring, devicetree, Krzysztof Kozlowski, linux-renesas-soc,
	Magnus Damm, Gareth Williams, Phil Edworthy, Geert Uytterhoeven,
	Stephen Boyd, Michael Turquette, linux-clk, Milan Stevanovic,
	Jimmy Lalande, Pascal Eberhard, Thomas Petazzoni, Herve Codina,
	Clement Leger, linux-rtc

On 05/04/2022 20:47, Miquel Raynal wrote:
> Add new binding file for this RTC.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  .../bindings/rtc/renesas,rzn1-rtc.yaml        | 69 +++++++++++++++++++
>  1 file changed, 69 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/rtc/renesas,rzn1-rtc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/rtc/renesas,rzn1-rtc.yaml b/Documentation/devicetree/bindings/rtc/renesas,rzn1-rtc.yaml
> new file mode 100644
> index 000000000000..903f0cd361fa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/renesas,rzn1-rtc.yaml
> @@ -0,0 +1,69 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/rtc/renesas,rzn1-rtc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas RZ/N1 SoCs Real-Time Clock DT bindings
> +
> +maintainers:
> +  - Miquel Raynal <miquel.raynal@bootlin.com>
> +
> +allOf:
> +  - $ref: rtc.yaml#
> +
> +properties:
> +  compatible:
> +    oneOf:

Why oneOf?

> +      - items:
> +          - enum:
> +              - renesas,r9a06g032-rtc
> +          - const: renesas,rzn1-rtc
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    minItems: 3
> +    maxItems: 3
> +
> +  interrupt-names:
> +    items:
> +      - const: alarm
> +      - const: timer
> +      - const: pps
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    const: hclk
> +
> +  start-year: true

You don't need this, it's coming from rtc.yaml.


Best regards,
Krzysztof

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

* Re: [PATCH 1/7] dt-bindings: rtc: rzn1: Describe the RZN1 RTC
  2022-04-07  7:37   ` Krzysztof Kozlowski
@ 2022-04-07  9:21     ` Miquel Raynal
  0 siblings, 0 replies; 20+ messages in thread
From: Miquel Raynal @ 2022-04-07  9:21 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Alessandro Zummo, Alexandre Belloni, Rob Herring, devicetree,
	Krzysztof Kozlowski, linux-renesas-soc, Magnus Damm,
	Gareth Williams, Phil Edworthy, Geert Uytterhoeven, Stephen Boyd,
	Michael Turquette, linux-clk, Milan Stevanovic, Jimmy Lalande,
	Pascal Eberhard, Thomas Petazzoni, Herve Codina, Clement Leger,
	linux-rtc

Hi Krzysztof,

krzysztof.kozlowski@linaro.org wrote on Thu, 7 Apr 2022 09:37:39 +0200:

> On 05/04/2022 20:47, Miquel Raynal wrote:
> > Add new binding file for this RTC.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  .../bindings/rtc/renesas,rzn1-rtc.yaml        | 69 +++++++++++++++++++
> >  1 file changed, 69 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/rtc/renesas,rzn1-rtc.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/rtc/renesas,rzn1-rtc.yaml b/Documentation/devicetree/bindings/rtc/renesas,rzn1-rtc.yaml
> > new file mode 100644
> > index 000000000000..903f0cd361fa
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/rtc/renesas,rzn1-rtc.yaml
> > @@ -0,0 +1,69 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/rtc/renesas,rzn1-rtc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Renesas RZ/N1 SoCs Real-Time Clock DT bindings
> > +
> > +maintainers:
> > +  - Miquel Raynal <miquel.raynal@bootlin.com>
> > +
> > +allOf:
> > +  - $ref: rtc.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:  
> 
> Why oneOf?

Right, copy-paste leftover.

> 
> > +      - items:
> > +          - enum:
> > +              - renesas,r9a06g032-rtc
> > +          - const: renesas,rzn1-rtc
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    minItems: 3
> > +    maxItems: 3
> > +
> > +  interrupt-names:
> > +    items:
> > +      - const: alarm
> > +      - const: timer
> > +      - const: pps
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    const: hclk
> > +
> > +  start-year: true  
> 
> You don't need this, it's coming from rtc.yaml.

Right.

Thanks,
Miquèl

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

* Re: [PATCH 6/7] MAINTAINERS: Add myself as maintainer of the RZN1 RTC driver
  2022-04-05 18:47 ` [PATCH 6/7] MAINTAINERS: Add myself as maintainer of the RZN1 RTC driver Miquel Raynal
@ 2022-04-11 15:22   ` Geert Uytterhoeven
  0 siblings, 0 replies; 20+ messages in thread
From: Geert Uytterhoeven @ 2022-04-11 15:22 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Alessandro Zummo, Alexandre Belloni, Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Krzysztof Kozlowski, Linux-Renesas, Magnus Damm, Gareth Williams,
	Phil Edworthy, Stephen Boyd, Michael Turquette, linux-clk,
	Milan Stevanovic, Jimmy Lalande, Pascal Eberhard,
	Thomas Petazzoni, Herve Codina, Clement Leger, linux-rtc

On Tue, Apr 5, 2022 at 8:47 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> After contributing it, I'll volunteer to maintain it.

Thanks a lot!

> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH 7/7] ARM: dts: r9a06g032: Describe the RTC
  2022-04-05 18:47 ` [PATCH 7/7] ARM: dts: r9a06g032: Describe the RTC Miquel Raynal
@ 2022-04-11 15:32   ` Geert Uytterhoeven
  0 siblings, 0 replies; 20+ messages in thread
From: Geert Uytterhoeven @ 2022-04-11 15:32 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Alessandro Zummo, Alexandre Belloni, Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Krzysztof Kozlowski, Linux-Renesas, Magnus Damm, Gareth Williams,
	Phil Edworthy, Stephen Boyd, Michael Turquette, linux-clk,
	Milan Stevanovic, Jimmy Lalande, Pascal Eberhard,
	Thomas Petazzoni, Herve Codina, Clement Leger, linux-rtc

Hi Miquel,

On Tue, Apr 5, 2022 at 8:47 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Describe the SoC RTC which counts time and provides alarm support.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks for your patch!

> --- a/arch/arm/boot/dts/r9a06g032.dtsi
> +++ b/arch/arm/boot/dts/r9a06g032.dtsi
> @@ -103,6 +103,18 @@ dmamux: dma-router@a0 {
>                         };
>                 };
>
> +               rtc0: rtc@40006000 {

Please insert this before watchdog@40008000, to preserve sort
order (by unit address).

> +                       compatible = "renesas,r9a06g032-rtc", "renesas,rzn1-rtc";
> +                       reg = <0x40006000 0x1000>;
> +                       interrupts = <GIC_SPI 66 IRQ_TYPE_EDGE_RISING>,
> +                                    <GIC_SPI 67 IRQ_TYPE_EDGE_RISING>,
> +                                    <GIC_SPI 68 IRQ_TYPE_EDGE_RISING>;
> +                       interrupt-names = "alarm", "timer", "pps";
> +                       clocks = <&sysctrl R9A06G032_HCLK_RTC>;
> +                       clock-names = "hclk";
> +                       status = "disabled";
> +               };
> +
>                 uart0: serial@40060000 {
>                         compatible = "renesas,r9a06g032-uart", "renesas,rzn1-uart", "snps,dw-apb-uart";
>                         reg = <0x40060000 0x400>;

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH 3/7] rtc: rzn1: Add new RTC driver
  2022-04-06  8:50   ` Alexandre Belloni
@ 2022-04-13 15:23     ` Miquel Raynal
  2022-04-13 15:48       ` Alexandre Belloni
  0 siblings, 1 reply; 20+ messages in thread
From: Miquel Raynal @ 2022-04-13 15:23 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Alessandro Zummo, Rob Herring, devicetree, Krzysztof Kozlowski,
	linux-renesas-soc, Magnus Damm, Gareth Williams, Phil Edworthy,
	Geert Uytterhoeven, Stephen Boyd, Michael Turquette, linux-clk,
	Milan Stevanovic, Jimmy Lalande, Pascal Eberhard,
	Thomas Petazzoni, Herve Codina, Clement Leger, linux-rtc,
	Michel Pollet

Hi Alex,

> > +	tm->tm_sec = bcd2bin(tm->tm_sec);
> > +	tm->tm_min = bcd2bin(tm->tm_min);
> > +	tm->tm_hour = bcd2bin(tm->tm_hour);
> > +	tm->tm_wday = bcd2bin(tm->tm_wday);
> > +	tm->tm_mday = bcd2bin(tm->tm_mday);
> > +	tm->tm_mon = bcd2bin(tm->tm_mon);
> > +	tm->tm_year = bcd2bin(tm->tm_year);
> > +
> > +	dev_dbg(dev, "%d-%d-%d(%d)T%d:%d:%d\n",
> > +		tm->tm_year, tm->tm_mon, tm->tm_mday, tm->tm_wday,
> > +		tm->tm_hour, tm->tm_min, tm->tm_sec);
> > +  
> 
> This is not really useful because we have tracepoints in the core.
> Anyway, please use %ptR.

Nice printk operator :)

I've dropped this dev_dbg call, it actually does not serve any useful
purpose.


[...]

> > +static int rzn1_rtc_probe(struct platform_device *pdev)
> > +{
> > +	struct rzn1_rtc *rtc;
> > +	int ret;
> > +
> > +	rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
> > +	if (!rtc)
> > +		return -ENOMEM;
> > +
> > +	platform_set_drvdata(pdev, rtc);
> > +
> > +	rtc->clk = devm_clk_get(&pdev->dev, "hclk");
> > +	if (IS_ERR(rtc->clk))
> > +		return dev_err_probe(&pdev->dev, PTR_ERR(rtc->clk), "Missing hclk\n");
> > +
> > +	rtc->base = devm_platform_ioremap_resource(pdev, 0);
> > +	if (IS_ERR(rtc->base))
> > +		return dev_err_probe(&pdev->dev, PTR_ERR(rtc->base), "Missing reg\n");
> > +
> > +	rtc->rtcdev = devm_rtc_allocate_device(&pdev->dev);
> > +	if (IS_ERR(rtc->rtcdev))
> > +		return PTR_ERR(rtc);
> > +
> > +	rtc->rtcdev->range_max = 3178591199UL; /* 100 years */  
> 
> I'm not sure how you came to this value, this is 2070-09-22T05:59:59.
> I'm pretty sure the RTC will not fail at that time. Also, the comment
> seems fishy.

The RTC itself as no "starting point", but just a counter that can
count up to 100. So the max range is start-year + 100 years. But at
this point I don't yet have access to the start-year value. What's
your advise?

> > +	rtc->rtcdev->ops = &rzn1_rtc_ops;
> > +
> > +	ret = r9a06g032_sysctrl_enable_rtc(true);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = clk_prepare_enable(rtc->clk);
> > +	if (ret)
> > +		goto disable_rtc;
> > +
> > +	/*
> > +	 * Ensure the clock counter is enabled.
> > +	 * Set 24-hour mode and possible oscillator offset compensation in SUBU mode.
> > +	 */
> > +	writel(RZN1_RTC_CTL0_CE | RZN1_RTC_CTL0_AMPM | RZN1_RTC_CTL0_SLSB_SUBU,
> > +	       rtc->base + RZN1_RTC_CTL0);
> > +
> > +	/* Disable all interrupts */
> > +	writel(0, rtc->base + RZN1_RTC_CTL1);
> > +
> > +	/* Enable counter operation */
> > +	writel(0, rtc->base + RZN1_RTC_CTL2);
> > +  
> 
> I don't think you should do that unconditionally. The RTC is either
> not already started (and doesn't carry the proper time/date) or already
> started. It would be better to start it in .set_time. Maybe you can even
> use that to detect whether it has already been set once.

Actually there is only one (interesting) writeable bit in this register
and it defaults to 0 whenever we enable the register interface so we
don't really need to enable anything.

But I've added the necessary logic to handle the situation where the
bootloader would disable the rtc (which is the only situation where
the driver can diagnose an erroneous time/date).

> > +	ret = devm_rtc_register_device(rtc->rtcdev);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "Failed to register RTC\n");  
> 
> No error message is needed here.

Ok.

> 
> > +		goto disable_clk;
> > +	}
> > +
> > +	return 0;
> > +
> > +disable_clk:
> > +	clk_disable_unprepare(rtc->clk);
> > +disable_rtc:
> > +	r9a06g032_sysctrl_enable_rtc(false);
> > +
> > +	return ret;
> > +}
> > +
> > +static int rzn1_rtc_remove(struct platform_device *pdev)
> > +{
> > +	struct rzn1_rtc *rtc = platform_get_drvdata(pdev);
> > +
> > +	clk_disable_unprepare(rtc->clk);
> > +	r9a06g032_sysctrl_enable_rtc(false);  
> 
> Does this stop the RTC or just the register interface?

Mmmh actually if I reset the sysctrl bit manually I loose the counter
entirely, so I'll drop this call.

Thanks,
Miquèl

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

* Re: [PATCH 3/7] rtc: rzn1: Add new RTC driver
  2022-04-13 15:23     ` Miquel Raynal
@ 2022-04-13 15:48       ` Alexandre Belloni
  2022-04-14 11:19         ` Miquel Raynal
  0 siblings, 1 reply; 20+ messages in thread
From: Alexandre Belloni @ 2022-04-13 15:48 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Alessandro Zummo, Rob Herring, devicetree, Krzysztof Kozlowski,
	linux-renesas-soc, Magnus Damm, Gareth Williams, Phil Edworthy,
	Geert Uytterhoeven, Stephen Boyd, Michael Turquette, linux-clk,
	Milan Stevanovic, Jimmy Lalande, Pascal Eberhard,
	Thomas Petazzoni, Herve Codina, Clement Leger, linux-rtc,
	Michel Pollet

Hi Miquèl,

On 13/04/2022 17:23:27+0200, Miquel Raynal wrote:
> > > +static int rzn1_rtc_probe(struct platform_device *pdev)
> > > +{
> > > +	struct rzn1_rtc *rtc;
> > > +	int ret;
> > > +
> > > +	rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
> > > +	if (!rtc)
> > > +		return -ENOMEM;
> > > +
> > > +	platform_set_drvdata(pdev, rtc);
> > > +
> > > +	rtc->clk = devm_clk_get(&pdev->dev, "hclk");
> > > +	if (IS_ERR(rtc->clk))
> > > +		return dev_err_probe(&pdev->dev, PTR_ERR(rtc->clk), "Missing hclk\n");
> > > +
> > > +	rtc->base = devm_platform_ioremap_resource(pdev, 0);
> > > +	if (IS_ERR(rtc->base))
> > > +		return dev_err_probe(&pdev->dev, PTR_ERR(rtc->base), "Missing reg\n");
> > > +
> > > +	rtc->rtcdev = devm_rtc_allocate_device(&pdev->dev);
> > > +	if (IS_ERR(rtc->rtcdev))
> > > +		return PTR_ERR(rtc);
> > > +
> > > +	rtc->rtcdev->range_max = 3178591199UL; /* 100 years */  
> > 
> > I'm not sure how you came to this value, this is 2070-09-22T05:59:59.
> > I'm pretty sure the RTC will not fail at that time. Also, the comment
> > seems fishy.
> 
> The RTC itself as no "starting point", but just a counter that can
> count up to 100. So the max range is start-year + 100 years. But at
> this point I don't yet have access to the start-year value. What's
> your advise?

The question is why is this limited to 100 years? My guess is that it
doesn't handle leap years properly if this is the case, there is only
one range that works, this is 2000-01-01 to 2099-12-31 like many other
RTCs.

You can run rtc-range from rtc-tools after removing range_max to find
out.

Cheers

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

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

* Re: [PATCH 3/7] rtc: rzn1: Add new RTC driver
  2022-04-13 15:48       ` Alexandre Belloni
@ 2022-04-14 11:19         ` Miquel Raynal
  0 siblings, 0 replies; 20+ messages in thread
From: Miquel Raynal @ 2022-04-14 11:19 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Alessandro Zummo, Rob Herring, devicetree, Krzysztof Kozlowski,
	linux-renesas-soc, Magnus Damm, Gareth Williams, Phil Edworthy,
	Geert Uytterhoeven, Stephen Boyd, Michael Turquette, linux-clk,
	Milan Stevanovic, Jimmy Lalande, Pascal Eberhard,
	Thomas Petazzoni, Herve Codina, Clement Leger, linux-rtc,
	Michel Pollet

Hi Alex,

alexandre.belloni@bootlin.com wrote on Wed, 13 Apr 2022 17:48:45 +0200:

> Hi Miquèl,
> 
> On 13/04/2022 17:23:27+0200, Miquel Raynal wrote:
> > > > +static int rzn1_rtc_probe(struct platform_device *pdev)
> > > > +{
> > > > +	struct rzn1_rtc *rtc;
> > > > +	int ret;
> > > > +
> > > > +	rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
> > > > +	if (!rtc)
> > > > +		return -ENOMEM;
> > > > +
> > > > +	platform_set_drvdata(pdev, rtc);
> > > > +
> > > > +	rtc->clk = devm_clk_get(&pdev->dev, "hclk");
> > > > +	if (IS_ERR(rtc->clk))
> > > > +		return dev_err_probe(&pdev->dev, PTR_ERR(rtc->clk), "Missing hclk\n");
> > > > +
> > > > +	rtc->base = devm_platform_ioremap_resource(pdev, 0);
> > > > +	if (IS_ERR(rtc->base))
> > > > +		return dev_err_probe(&pdev->dev, PTR_ERR(rtc->base), "Missing reg\n");
> > > > +
> > > > +	rtc->rtcdev = devm_rtc_allocate_device(&pdev->dev);
> > > > +	if (IS_ERR(rtc->rtcdev))
> > > > +		return PTR_ERR(rtc);
> > > > +
> > > > +	rtc->rtcdev->range_max = 3178591199UL; /* 100 years */    
> > > 
> > > I'm not sure how you came to this value, this is 2070-09-22T05:59:59.
> > > I'm pretty sure the RTC will not fail at that time. Also, the comment
> > > seems fishy.  
> > 
> > The RTC itself as no "starting point", but just a counter that can
> > count up to 100. So the max range is start-year + 100 years. But at
> > this point I don't yet have access to the start-year value. What's
> > your advise?  
> 
> The question is why is this limited to 100 years? My guess is that it
> doesn't handle leap years properly if this is the case, there is only
> one range that works, this is 2000-01-01 to 2099-12-31 like many other
> RTCs.

I don't know the real reason, actually there is just written that the
"year" register counts up from 00 to 99 (in bcd).

> You can run rtc-range from rtc-tools after removing range_max to find
> out.

Here is the result. It fails at 2069, which I believe means "100 years"
from 1970. So what do you conclude with this? Shall I use
rtc_time64_to_tm(2069-12-31 23:59:59) as the range_max value?

# rtc-range 

Testing 2000-02-28 23:59:59.
OK

Testing 2038-01-19 03:14:07.
OK

Testing 2069-12-31 23:59:59.
KO RTC_RD_TIME returned 22 (line 124)

Thanks,
Miquèl

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

end of thread, other threads:[~2022-04-14 11:19 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-05 18:47 [PATCH 0/7] RZN1 RTC support Miquel Raynal
2022-04-05 18:47 ` [PATCH 1/7] dt-bindings: rtc: rzn1: Describe the RZN1 RTC Miquel Raynal
2022-04-07  7:37   ` Krzysztof Kozlowski
2022-04-07  9:21     ` Miquel Raynal
2022-04-05 18:47 ` [PATCH 2/7] soc: renesas: rzn1-sysc: Export a function to enable/disable the RTC Miquel Raynal
2022-04-06  7:00   ` Thomas Petazzoni
2022-04-06  7:43     ` Miquel Raynal
2022-04-06  8:32   ` Alexandre Belloni
2022-04-05 18:47 ` [PATCH 3/7] rtc: rzn1: Add new RTC driver Miquel Raynal
2022-04-06  8:50   ` Alexandre Belloni
2022-04-13 15:23     ` Miquel Raynal
2022-04-13 15:48       ` Alexandre Belloni
2022-04-14 11:19         ` Miquel Raynal
2022-04-05 18:47 ` [PATCH 4/7] rtc: rzn1: Add alarm support Miquel Raynal
2022-04-06  9:10   ` Alexandre Belloni
2022-04-05 18:47 ` [PATCH 5/7] rtc: rzn1: Add oscillator offset support Miquel Raynal
2022-04-05 18:47 ` [PATCH 6/7] MAINTAINERS: Add myself as maintainer of the RZN1 RTC driver Miquel Raynal
2022-04-11 15:22   ` Geert Uytterhoeven
2022-04-05 18:47 ` [PATCH 7/7] ARM: dts: r9a06g032: Describe the RTC Miquel Raynal
2022-04-11 15:32   ` Geert Uytterhoeven

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.