[PATCH 0/5] rtc: ingenic: various updates

[PATCH 0/5] rtc: ingenic: various updates

[Paul Cercueil]

Hi,

Here's a set of patches for the Ingenic RTC driver (jz4740-rtc).

Patch [1/5] and [4/5] update the DT binding documentation and update the
driver to support the CLK32K pin. This pin optionally supplies a 32 kHz
clock, which is required on the MIPS CI20 board for the WiFi/Bluetooth
chip to work.

Patch [2/5] is a code cleanup. Patch [3/5] fixes the RTC time yielding
an impossible value after a power loss.

Finally, patch [5/5] is *RFC*. I do not know if it works, as I have
absolutely no idea about how to test it.

Cheers,
-Paul

Paul Cercueil (5):
  dt-bindings: rtc: Rework compatible strings and add #clock-cells
  rtc: jz4740: Use readl_poll_timeout
  rtc: jz4740: Reset scratchpad register on power loss
  rtc: jz4740: Register clock provider for the CLK32K pin
  rtc: jz4740: Support for fine-tuning the RTC clock

 .../devicetree/bindings/rtc/ingenic,rtc.yaml  |   7 +-
 drivers/rtc/rtc-jz4740.c                      | 137 +++++++++++++++---
 2 files changed, 125 insertions(+), 19 deletions(-)

-- 
2.35.1

[PATCH 1/5] dt-bindings: rtc: Rework compatible strings and add #clock-cells

[Paul Cercueil]

The RTC in the JZ4770 is compatible with the JZ4760, but has an extra
register that permits to configure the behaviour of the CLK32K pin. The
same goes for the RTC in the JZ4780.

Therefore, the ingenic,jz4770-rtc and ingenic,jz4780-rtc strings do not
fall back anymore to ingenic,jz4760-rtc. The ingenic,jz4780-rtc string
now falls back to the ingenic,jz4770-rtc string.

Additionally, since the RTCs in the JZ4770 and JZ4780 support outputting
the input oscillator's clock to the CLK32K pin, the RTC node is now also
a clock provider on these SoCs, so a #clock-cells property is added.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
---
 Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml b/Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml
index b235b2441997..57393c3ac724 100644
--- a/Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml
+++ b/Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml
@@ -18,14 +18,14 @@ properties:
       - enum:
           - ingenic,jz4740-rtc
           - ingenic,jz4760-rtc
+          - ingenic,jz4770-rtc
       - items:
           - const: ingenic,jz4725b-rtc
           - const: ingenic,jz4740-rtc
       - items:
           - enum:
-              - ingenic,jz4770-rtc
               - ingenic,jz4780-rtc
-          - const: ingenic,jz4760-rtc
+          - const: ingenic,jz4770-rtc
 
   reg:
     maxItems: 1
@@ -39,6 +39,9 @@ properties:
   clock-names:
     const: rtc
 
+  "#clock-cells":
+    const: 0
+
   system-power-controller:
     description: |
       Indicates that the RTC is responsible for powering OFF
-- 
2.35.1

[PATCH 2/5] rtc: jz4740: Use readl_poll_timeout

[Paul Cercueil]

Use readl_poll_timeout() from <iopoll.h> instead of using custom poll
loops.

The timeout settings are different, but that shouldn't be much of a
problem. Instead of polling 10000 times in a close loop, it polls for
one millisecond.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/rtc/rtc-jz4740.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
index 6e51df72fd65..119baf168b32 100644
--- a/drivers/rtc/rtc-jz4740.c
+++ b/drivers/rtc/rtc-jz4740.c
@@ -7,6 +7,7 @@
 
 #include <linux/clk.h>
 #include <linux/io.h>
+#include <linux/iopoll.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
@@ -69,19 +70,15 @@ static inline uint32_t jz4740_rtc_reg_read(struct jz4740_rtc *rtc, size_t reg)
 static int jz4740_rtc_wait_write_ready(struct jz4740_rtc *rtc)
 {
 	uint32_t ctrl;
-	int timeout = 10000;
 
-	do {
-		ctrl = jz4740_rtc_reg_read(rtc, JZ_REG_RTC_CTRL);
-	} while (!(ctrl & JZ_RTC_CTRL_WRDY) && --timeout);
-
-	return timeout ? 0 : -EIO;
+	return readl_poll_timeout(rtc->base + JZ_REG_RTC_CTRL, ctrl,
+				  ctrl & JZ_RTC_CTRL_WRDY, 0, 1000);
 }
 
 static inline int jz4780_rtc_enable_write(struct jz4740_rtc *rtc)
 {
 	uint32_t ctrl;
-	int ret, timeout = 10000;
+	int ret;
 
 	ret = jz4740_rtc_wait_write_ready(rtc);
 	if (ret != 0)
@@ -89,11 +86,8 @@ static inline int jz4780_rtc_enable_write(struct jz4740_rtc *rtc)
 
 	writel(JZ_RTC_WENR_MAGIC, rtc->base + JZ_REG_RTC_WENR);
 
-	do {
-		ctrl = readl(rtc->base + JZ_REG_RTC_WENR);
-	} while (!(ctrl & JZ_RTC_WENR_WEN) && --timeout);
-
-	return timeout ? 0 : -EIO;
+	return readl_poll_timeout(rtc->base + JZ_REG_RTC_WENR, ctrl,
+				  ctrl & JZ_RTC_WENR_WEN, 0, 1000);
 }
 
 static inline int jz4740_rtc_reg_write(struct jz4740_rtc *rtc, size_t reg,
-- 
2.35.1

[PATCH 3/5] rtc: jz4740: Reset scratchpad register on power loss

[Paul Cercueil]

On power loss, reading the RTC value would fail as the scratchpad lost
its magic value, until the hardware clock was set once again.

To avoid that, reset the RTC value to Epoch in the probe if we detect
that the scratchpad lost its magic value.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/rtc/rtc-jz4740.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
index 119baf168b32..aac5f68bf626 100644
--- a/drivers/rtc/rtc-jz4740.c
+++ b/drivers/rtc/rtc-jz4740.c
@@ -42,6 +42,9 @@
 /* Magic value to enable writes on jz4780 */
 #define JZ_RTC_WENR_MAGIC	0xA55A
 
+/* Value written to the scratchpad to detect power losses */
+#define JZ_RTC_SCRATCHPAD_MAGIC	0x12345678
+
 #define JZ_RTC_WAKEUP_FILTER_MASK	0x0000FFE0
 #define JZ_RTC_RESET_COUNTER_MASK	0x00000FE0
 
@@ -134,10 +137,11 @@ static int jz4740_rtc_ctrl_set_bits(struct jz4740_rtc *rtc, uint32_t mask,
 static int jz4740_rtc_read_time(struct device *dev, struct rtc_time *time)
 {
 	struct jz4740_rtc *rtc = dev_get_drvdata(dev);
-	uint32_t secs, secs2;
+	uint32_t secs, secs2, magic;
 	int timeout = 5;
 
-	if (jz4740_rtc_reg_read(rtc, JZ_REG_RTC_SCRATCHPAD) != 0x12345678)
+	magic = jz4740_rtc_reg_read(rtc, JZ_REG_RTC_SCRATCHPAD);
+	if (magic != JZ_RTC_SCRATCHPAD_MAGIC)
 		return -EINVAL;
 
 	/* If the seconds register is read while it is updated, it can contain a
@@ -169,7 +173,8 @@ static int jz4740_rtc_set_time(struct device *dev, struct rtc_time *time)
 	if (ret)
 		return ret;
 
-	return jz4740_rtc_reg_write(rtc, JZ_REG_RTC_SCRATCHPAD, 0x12345678);
+	return jz4740_rtc_reg_write(rtc, JZ_REG_RTC_SCRATCHPAD,
+				    JZ_RTC_SCRATCHPAD_MAGIC);
 }
 
 static int jz4740_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
@@ -307,6 +312,7 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
 	struct jz4740_rtc *rtc;
 	unsigned long rate;
 	struct clk *clk;
+	uint32_t magic;
 	int ret, irq;
 
 	rtc = devm_kzalloc(dev, sizeof(*rtc), GFP_KERNEL);
@@ -369,6 +375,18 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
 	/* Each 1 Hz pulse should happen after (rate) ticks */
 	jz4740_rtc_reg_write(rtc, JZ_REG_RTC_REGULATOR, rate - 1);
 
+	magic = jz4740_rtc_reg_read(rtc, JZ_REG_RTC_SCRATCHPAD);
+	if (magic != JZ_RTC_SCRATCHPAD_MAGIC) {
+		/*
+		 * If the scratchpad doesn't hold our magic value, then a
+		 * power loss occurred. Reset to Epoch.
+		 */
+		struct rtc_time time;
+
+		rtc_time64_to_tm(0, &time);
+		jz4740_rtc_set_time(dev, &time);
+	}
+
 	ret = devm_rtc_register_device(rtc->rtc);
 	if (ret)
 		return ret;
-- 
2.35.1

[PATCH 4/5] rtc: jz4740: Register clock provider for the CLK32K pin

[Paul Cercueil]

On JZ4770 and JZ4780, the CLK32K pin is configurable. By default, it is
configured as a GPIO in input mode, and its value can be read through
GPIO PD14.

With this change, clients can now request the 32 kHz clock on the CLK32K
pin, through Device Tree. This clock is simply a pass-through of the
input oscillator's clock with enable/disable operations.

This will permit the WiFi/Bluetooth chip to work on the MIPS CI20 board,
which does source one of its clocks from the CLK32K pin.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/rtc/rtc-jz4740.c | 50 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 48 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
index aac5f68bf626..f4c9b6058f07 100644
--- a/drivers/rtc/rtc-jz4740.c
+++ b/drivers/rtc/rtc-jz4740.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/clk.h>
+#include <linux/clk-provider.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
 #include <linux/kernel.h>
@@ -26,6 +27,7 @@
 #define JZ_REG_RTC_WAKEUP_FILTER	0x24
 #define JZ_REG_RTC_RESET_COUNTER	0x28
 #define JZ_REG_RTC_SCRATCHPAD	0x34
+#define JZ_REG_RTC_CKPCR	0x40
 
 /* The following are present on the jz4780 */
 #define JZ_REG_RTC_WENR	0x3C
@@ -48,10 +50,13 @@
 #define JZ_RTC_WAKEUP_FILTER_MASK	0x0000FFE0
 #define JZ_RTC_RESET_COUNTER_MASK	0x00000FE0
 
+#define JZ_RTC_CKPCR_CK32PULL_DIS	BIT(4)
+#define JZ_RTC_CKPCR_CK32CTL_EN		(BIT(2) | BIT(1))
+
 enum jz4740_rtc_type {
 	ID_JZ4740,
 	ID_JZ4760,
-	ID_JZ4780,
+	ID_JZ4770,
 };
 
 struct jz4740_rtc {
@@ -60,6 +65,8 @@ struct jz4740_rtc {
 
 	struct rtc_device *rtc;
 
+	struct clk_hw clk32k;
+
 	spinlock_t lock;
 };
 
@@ -264,7 +271,8 @@ static void jz4740_rtc_clk_disable(void *data)
 static const struct of_device_id jz4740_rtc_of_match[] = {
 	{ .compatible = "ingenic,jz4740-rtc", .data = (void *)ID_JZ4740 },
 	{ .compatible = "ingenic,jz4760-rtc", .data = (void *)ID_JZ4760 },
-	{ .compatible = "ingenic,jz4780-rtc", .data = (void *)ID_JZ4780 },
+	{ .compatible = "ingenic,jz4770-rtc", .data = (void *)ID_JZ4770 },
+	{ .compatible = "ingenic,jz4780-rtc", .data = (void *)ID_JZ4770 },
 	{},
 };
 MODULE_DEVICE_TABLE(of, jz4740_rtc_of_match);
@@ -305,6 +313,27 @@ static void jz4740_rtc_set_wakeup_params(struct jz4740_rtc *rtc,
 	jz4740_rtc_reg_write(rtc, JZ_REG_RTC_RESET_COUNTER, reset_ticks);
 }
 
+static int jz4740_rtc_clk32k_enable(struct clk_hw *hw)
+{
+	struct jz4740_rtc *rtc = container_of(hw, struct jz4740_rtc, clk32k);
+
+	return jz4740_rtc_reg_write(rtc, JZ_REG_RTC_CKPCR,
+				    JZ_RTC_CKPCR_CK32PULL_DIS |
+				    JZ_RTC_CKPCR_CK32CTL_EN);
+}
+
+static void jz4740_rtc_clk32k_disable(struct clk_hw *hw)
+{
+	struct jz4740_rtc *rtc = container_of(hw, struct jz4740_rtc, clk32k);
+
+	jz4740_rtc_reg_write(rtc, JZ_REG_RTC_CKPCR, 0);
+}
+
+static const struct clk_ops jz4740_rtc_clk32k_ops = {
+	.enable = jz4740_rtc_clk32k_enable,
+	.disable = jz4740_rtc_clk32k_disable,
+};
+
 static int jz4740_rtc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -407,6 +436,23 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
 			dev_warn(dev, "Poweroff handler already present!\n");
 	}
 
+	if (rtc->type == ID_JZ4770) {
+		rtc->clk32k.init = CLK_HW_INIT("clk32k", "rtc",
+					       &jz4740_rtc_clk32k_ops, 0);
+
+		ret = devm_clk_hw_register(dev, &rtc->clk32k);
+		if (ret) {
+			dev_err(dev, "Unable to register clk32k clock: %d\n", ret);
+			return ret;
+		}
+
+		ret = of_clk_add_hw_provider(np, of_clk_hw_simple_get, &rtc->clk32k);
+		if (ret) {
+			dev_err(dev, "Unable to register clk32k clock provider: %d\n", ret);
+			return ret;
+		}
+	}
+
 	return 0;
 }
 
-- 
2.35.1

[PATCH 5/5] rtc: jz4740: Support for fine-tuning the RTC clock

[Paul Cercueil]

Write the NC1HZ and ADJC register fields, which allow to tweak the
frequency of the RTC clock, so that it can run as accurately as
possible.

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 drivers/rtc/rtc-jz4740.c | 45 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
index f4c9b6058f07..f275e58a9cea 100644
--- a/drivers/rtc/rtc-jz4740.c
+++ b/drivers/rtc/rtc-jz4740.c
@@ -5,6 +5,7 @@
  *	 JZ4740 SoC RTC driver
  */
 
+#include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/io.h>
@@ -41,6 +42,9 @@
 #define JZ_RTC_CTRL_AE		BIT(2)
 #define JZ_RTC_CTRL_ENABLE	BIT(0)
 
+#define JZ_RTC_REGULATOR_NC1HZ_MASK	GENMASK(15, 0)
+#define JZ_RTC_REGULATOR_ADJC_MASK	GENMASK(25, 16)
+
 /* Magic value to enable writes on jz4780 */
 #define JZ_RTC_WENR_MAGIC	0xA55A
 
@@ -64,6 +68,7 @@ struct jz4740_rtc {
 	enum jz4740_rtc_type type;
 
 	struct rtc_device *rtc;
+	struct clk *clk;
 
 	struct clk_hw clk32k;
 
@@ -222,12 +227,51 @@ static int jz4740_rtc_alarm_irq_enable(struct device *dev, unsigned int enable)
 	return jz4740_rtc_ctrl_set_bits(rtc, JZ_RTC_CTRL_AF_IRQ, enable);
 }
 
+static int jz4740_rtc_read_offset(struct device *dev, long *offset)
+{
+	struct jz4740_rtc *rtc = dev_get_drvdata(dev);
+	long rate = clk_get_rate(rtc->clk);
+	s32 nc1hz, adjc, offset1k;
+	u32 reg;
+
+	reg = jz4740_rtc_reg_read(rtc, JZ_REG_RTC_REGULATOR);
+	nc1hz = FIELD_GET(JZ_RTC_REGULATOR_NC1HZ_MASK, reg);
+	adjc = FIELD_GET(JZ_RTC_REGULATOR_ADJC_MASK, reg);
+
+	offset1k = (nc1hz - rate + 1) * 1024L + adjc;
+	*offset = offset1k * 1000000L / (rate * 1024L);
+
+	return 0;
+}
+
+static int jz4740_rtc_set_offset(struct device *dev, long offset)
+{
+	struct jz4740_rtc *rtc = dev_get_drvdata(dev);
+	long rate = clk_get_rate(rtc->clk);
+	s32 offset1k, adjc, nc1hz;
+
+	offset1k = div_s64_rem(offset * rate * 1024LL, 1000000LL, &adjc);
+	nc1hz = rate - 1 + offset1k / 1024L;
+
+	if (adjc < 0) {
+		nc1hz--;
+		adjc += 1024;
+	}
+
+	nc1hz = FIELD_PREP(JZ_RTC_REGULATOR_NC1HZ_MASK, nc1hz);
+	adjc = FIELD_PREP(JZ_RTC_REGULATOR_ADJC_MASK, adjc);
+
+	return jz4740_rtc_reg_write(rtc, JZ_REG_RTC_REGULATOR, nc1hz | adjc);
+}
+
 static const struct rtc_class_ops jz4740_rtc_ops = {
 	.read_time	= jz4740_rtc_read_time,
 	.set_time	= jz4740_rtc_set_time,
 	.read_alarm	= jz4740_rtc_read_alarm,
 	.set_alarm	= jz4740_rtc_set_alarm,
 	.alarm_irq_enable = jz4740_rtc_alarm_irq_enable,
+	.read_offset	= jz4740_rtc_read_offset,
+	.set_offset	= jz4740_rtc_set_offset,
 };
 
 static irqreturn_t jz4740_rtc_irq(int irq, void *data)
@@ -378,6 +422,7 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
 
 	spin_lock_init(&rtc->lock);
 
+	rtc->clk = clk;
 	platform_set_drvdata(pdev, rtc);
 
 	device_init_wakeup(dev, 1);
-- 
2.35.1

Re: [PATCH 4/5] rtc: jz4740: Register clock provider for the CLK32K pin

[kernel test robot]

Hi Paul,

I love your patch! Yet something to improve:

[auto build test ERROR on abelloni/rtc-next]
[also build test ERROR on v5.18-rc3 next-20220414]
[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]

url:    https://github.com/intel-lab-lkp/linux/commits/Paul-Cercueil/rtc-ingenic-various-updates/20220419-025341
base:   https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next
config: parisc-randconfig-r022-20220419 (https://download.01.org/0day-ci/archive/20220419/202204191107.MvgmbaHZ-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/a8eada718214bc34ea29f8ff353228abacc0bfb9
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Paul-Cercueil/rtc-ingenic-various-updates/20220419-025341
        git checkout a8eada718214bc34ea29f8ff353228abacc0bfb9
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=parisc SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "of_clk_add_hw_provider" [drivers/rtc/rtc-jz4740.ko] undefined!
>> ERROR: modpost: "devm_clk_hw_register" [drivers/rtc/rtc-jz4740.ko] undefined!
>> ERROR: modpost: "of_clk_hw_simple_get" [drivers/rtc/rtc-jz4740.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH 5/5] rtc: jz4740: Support for fine-tuning the RTC clock

[kernel test robot]

Hi Paul,

I love your patch! Yet something to improve:

[auto build test ERROR on abelloni/rtc-next]
[also build test ERROR on v5.18-rc3 next-20220414]
[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]

url:    https://github.com/intel-lab-lkp/linux/commits/Paul-Cercueil/rtc-ingenic-various-updates/20220419-025341
base:   https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next
config: sparc64-randconfig-r024-20220418 (https://download.01.org/0day-ci/archive/20220419/202204191251.t9r8gFyI-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/ba459adc8c83dbdc469d0c6b5d57fd95d834513a
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Paul-Cercueil/rtc-ingenic-various-updates/20220419-025341
        git checkout ba459adc8c83dbdc469d0c6b5d57fd95d834513a
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=sparc64 SHELL=/bin/bash drivers/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from <command-line>:
   drivers/rtc/rtc-jz4740.c: In function 'jz4740_rtc_set_offset':
>> include/linux/compiler_types.h:346:45: error: call to '__compiletime_assert_229' declared with attribute error: FIELD_PREP: value too large for the field
     346 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
         |                                             ^
   include/linux/compiler_types.h:327:25: note: in definition of macro '__compiletime_assert'
     327 |                         prefix ## suffix();                             \
         |                         ^~~~~~
   include/linux/compiler_types.h:346:9: note: in expansion of macro '_compiletime_assert'
     346 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
         |         ^~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
      39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
         |                                     ^~~~~~~~~~~~~~~~~~
   include/linux/bitfield.h:65:17: note: in expansion of macro 'BUILD_BUG_ON_MSG'
      65 |                 BUILD_BUG_ON_MSG(__builtin_constant_p(_val) ?           \
         |                 ^~~~~~~~~~~~~~~~
   include/linux/bitfield.h:111:17: note: in expansion of macro '__BF_FIELD_CHECK'
     111 |                 __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: ");    \
         |                 ^~~~~~~~~~~~~~~~
   drivers/rtc/rtc-jz4740.c:261:17: note: in expansion of macro 'FIELD_PREP'
     261 |         nc1hz = FIELD_PREP(JZ_RTC_REGULATOR_NC1HZ_MASK, nc1hz);
         |                 ^~~~~~~~~~


vim +/__compiletime_assert_229 +346 include/linux/compiler_types.h

eb5c2d4b45e3d2 Will Deacon 2020-07-21  332  
eb5c2d4b45e3d2 Will Deacon 2020-07-21  333  #define _compiletime_assert(condition, msg, prefix, suffix) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21  334  	__compiletime_assert(condition, msg, prefix, suffix)
eb5c2d4b45e3d2 Will Deacon 2020-07-21  335  
eb5c2d4b45e3d2 Will Deacon 2020-07-21  336  /**
eb5c2d4b45e3d2 Will Deacon 2020-07-21  337   * compiletime_assert - break build and emit msg if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21  338   * @condition: a compile-time constant condition to check
eb5c2d4b45e3d2 Will Deacon 2020-07-21  339   * @msg:       a message to emit if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21  340   *
eb5c2d4b45e3d2 Will Deacon 2020-07-21  341   * In tradition of POSIX assert, this macro will break the build if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21  342   * supplied condition is *false*, emitting the supplied error message if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21  343   * compiler has support to do so.
eb5c2d4b45e3d2 Will Deacon 2020-07-21  344   */
eb5c2d4b45e3d2 Will Deacon 2020-07-21  345  #define compiletime_assert(condition, msg) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21 @346  	_compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
eb5c2d4b45e3d2 Will Deacon 2020-07-21  347  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH 4/5] rtc: jz4740: Register clock provider for the CLK32K pin

[kernel test robot]

Hi Paul,

I love your patch! Yet something to improve:

[auto build test ERROR on abelloni/rtc-next]
[also build test ERROR on v5.18-rc3 next-20220414]
[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]

url:    https://github.com/intel-lab-lkp/linux/commits/Paul-Cercueil/rtc-ingenic-various-updates/20220419-025341
base:   https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next
config: hexagon-randconfig-r041-20220419 (https://download.01.org/0day-ci/archive/20220419/202204191348.uUoPjD9I-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 429cbac0390654f90bba18a41799464adf31a5ec)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/a8eada718214bc34ea29f8ff353228abacc0bfb9
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Paul-Cercueil/rtc-ingenic-various-updates/20220419-025341
        git checkout a8eada718214bc34ea29f8ff353228abacc0bfb9
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> ld.lld: error: undefined symbol: devm_clk_hw_register
   >>> referenced by rtc-jz4740.c
   >>> rtc/rtc-jz4740.o:(jz4740_rtc_probe) in archive drivers/built-in.a
   >>> referenced by rtc-jz4740.c
   >>> rtc/rtc-jz4740.o:(jz4740_rtc_probe) in archive drivers/built-in.a
--
>> ld.lld: error: undefined symbol: of_clk_hw_simple_get
   >>> referenced by rtc-jz4740.c
   >>> rtc/rtc-jz4740.o:(jz4740_rtc_probe) in archive drivers/built-in.a
   >>> referenced by rtc-jz4740.c
   >>> rtc/rtc-jz4740.o:(jz4740_rtc_probe) in archive drivers/built-in.a
--
>> ld.lld: error: undefined symbol: of_clk_add_hw_provider
   >>> referenced by rtc-jz4740.c
   >>> rtc/rtc-jz4740.o:(jz4740_rtc_probe) in archive drivers/built-in.a
   >>> referenced by rtc-jz4740.c
   >>> rtc/rtc-jz4740.o:(jz4740_rtc_probe) in archive drivers/built-in.a

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH 1/5] dt-bindings: rtc: Rework compatible strings and add #clock-cells

[Krzysztof Kozlowski]

On 18/04/2022 20:49, Paul Cercueil wrote:
> The RTC in the JZ4770 is compatible with the JZ4760, but has an extra
> register that permits to configure the behaviour of the CLK32K pin. The
> same goes for the RTC in the JZ4780.
> 
> Therefore, the ingenic,jz4770-rtc and ingenic,jz4780-rtc strings do not
> fall back anymore to ingenic,jz4760-rtc. The ingenic,jz4780-rtc string
> now falls back to the ingenic,jz4770-rtc string.
> 
> Additionally, since the RTCs in the JZ4770 and JZ4780 support outputting
> the input oscillator's clock to the CLK32K pin, the RTC node is now also
> a clock provider on these SoCs, so a #clock-cells property is added.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> ---
>  Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml b/Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml
> index b235b2441997..57393c3ac724 100644
> --- a/Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml
> +++ b/Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml
> @@ -18,14 +18,14 @@ properties:
>        - enum:
>            - ingenic,jz4740-rtc
>            - ingenic,jz4760-rtc
> +          - ingenic,jz4770-rtc
>        - items:
>            - const: ingenic,jz4725b-rtc
>            - const: ingenic,jz4740-rtc
>        - items:
>            - enum:
> -              - ingenic,jz4770-rtc
>                - ingenic,jz4780-rtc
> -          - const: ingenic,jz4760-rtc
> +          - const: ingenic,jz4770-rtc
>  
>    reg:
>      maxItems: 1
> @@ -39,6 +39,9 @@ properties:
>    clock-names:
>      const: rtc
>  
> +  "#clock-cells":
> +    const: 0
> +
>    system-power-controller:
>      description: |
>        Indicates that the RTC is responsible for powering OFF

Inside allOf:if:then:, please add a constraint which compatible cannot
have clock-cells (or maybe better which can?).

Some modification of:
https://elixir.bootlin.com/linux/v5.17-rc2/source/Documentation/devicetree/bindings/media/renesas,vsp1.yaml#L53

Best regards,
Krzysztof

Re: [PATCH 1/5] dt-bindings: rtc: Rework compatible strings and add #clock-cells

[Paul Cercueil]

Hi Krzysztof,

Le mar., avril 19 2022 at 08:41:56 +0200, Krzysztof Kozlowski 
<krzysztof.kozlowski@linaro.org> a écrit :
> On 18/04/2022 20:49, Paul Cercueil wrote:
>>  The RTC in the JZ4770 is compatible with the JZ4760, but has an 
>> extra
>>  register that permits to configure the behaviour of the CLK32K pin. 
>> The
>>  same goes for the RTC in the JZ4780.
>> 
>>  Therefore, the ingenic,jz4770-rtc and ingenic,jz4780-rtc strings do 
>> not
>>  fall back anymore to ingenic,jz4760-rtc. The ingenic,jz4780-rtc 
>> string
>>  now falls back to the ingenic,jz4770-rtc string.
>> 
>>  Additionally, since the RTCs in the JZ4770 and JZ4780 support 
>> outputting
>>  the input oscillator's clock to the CLK32K pin, the RTC node is now 
>> also
>>  a clock provider on these SoCs, so a #clock-cells property is added.
>> 
>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  Cc: Rob Herring <robh+dt@kernel.org>
>>  Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
>>  ---
>>   Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml | 7 +++++--
>>   1 file changed, 5 insertions(+), 2 deletions(-)
>> 
>>  diff --git a/Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml 
>> b/Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml
>>  index b235b2441997..57393c3ac724 100644
>>  --- a/Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml
>>  +++ b/Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml
>>  @@ -18,14 +18,14 @@ properties:
>>         - enum:
>>             - ingenic,jz4740-rtc
>>             - ingenic,jz4760-rtc
>>  +          - ingenic,jz4770-rtc
>>         - items:
>>             - const: ingenic,jz4725b-rtc
>>             - const: ingenic,jz4740-rtc
>>         - items:
>>             - enum:
>>  -              - ingenic,jz4770-rtc
>>                 - ingenic,jz4780-rtc
>>  -          - const: ingenic,jz4760-rtc
>>  +          - const: ingenic,jz4770-rtc
>> 
>>     reg:
>>       maxItems: 1
>>  @@ -39,6 +39,9 @@ properties:
>>     clock-names:
>>       const: rtc
>> 
>>  +  "#clock-cells":
>>  +    const: 0
>>  +
>>     system-power-controller:
>>       description: |
>>         Indicates that the RTC is responsible for powering OFF
> 
> Inside allOf:if:then:, please add a constraint which compatible cannot
> have clock-cells (or maybe better which can?).
> 
> Some modification of:
> https://elixir.bootlin.com/linux/v5.17-rc2/source/Documentation/devicetree/bindings/media/renesas,vsp1.yaml#L53

Sure.

Cheers,
-Paul

Re: [PATCH 3/5] rtc: jz4740: Reset scratchpad register on power loss

[Alexandre Belloni]

On 18/04/2022 19:49:31+0100, Paul Cercueil wrote:
> On power loss, reading the RTC value would fail as the scratchpad lost
> its magic value, until the hardware clock was set once again.
> 
> To avoid that, reset the RTC value to Epoch in the probe if we detect
> that the scratchpad lost its magic value.
> 
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  drivers/rtc/rtc-jz4740.c | 24 +++++++++++++++++++++---
>  1 file changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
> index 119baf168b32..aac5f68bf626 100644
> --- a/drivers/rtc/rtc-jz4740.c
> +++ b/drivers/rtc/rtc-jz4740.c
> @@ -42,6 +42,9 @@
>  /* Magic value to enable writes on jz4780 */
>  #define JZ_RTC_WENR_MAGIC	0xA55A
>  
> +/* Value written to the scratchpad to detect power losses */
> +#define JZ_RTC_SCRATCHPAD_MAGIC	0x12345678
> +
>  #define JZ_RTC_WAKEUP_FILTER_MASK	0x0000FFE0
>  #define JZ_RTC_RESET_COUNTER_MASK	0x00000FE0
>  
> @@ -134,10 +137,11 @@ static int jz4740_rtc_ctrl_set_bits(struct jz4740_rtc *rtc, uint32_t mask,
>  static int jz4740_rtc_read_time(struct device *dev, struct rtc_time *time)
>  {
>  	struct jz4740_rtc *rtc = dev_get_drvdata(dev);
> -	uint32_t secs, secs2;
> +	uint32_t secs, secs2, magic;
>  	int timeout = 5;
>  
> -	if (jz4740_rtc_reg_read(rtc, JZ_REG_RTC_SCRATCHPAD) != 0x12345678)
> +	magic = jz4740_rtc_reg_read(rtc, JZ_REG_RTC_SCRATCHPAD);
> +	if (magic != JZ_RTC_SCRATCHPAD_MAGIC)
>  		return -EINVAL;
>  
>  	/* If the seconds register is read while it is updated, it can contain a
> @@ -169,7 +173,8 @@ static int jz4740_rtc_set_time(struct device *dev, struct rtc_time *time)
>  	if (ret)
>  		return ret;
>  
> -	return jz4740_rtc_reg_write(rtc, JZ_REG_RTC_SCRATCHPAD, 0x12345678);
> +	return jz4740_rtc_reg_write(rtc, JZ_REG_RTC_SCRATCHPAD,
> +				    JZ_RTC_SCRATCHPAD_MAGIC);
>  }
>  
>  static int jz4740_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> @@ -307,6 +312,7 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
>  	struct jz4740_rtc *rtc;
>  	unsigned long rate;
>  	struct clk *clk;
> +	uint32_t magic;
>  	int ret, irq;
>  
>  	rtc = devm_kzalloc(dev, sizeof(*rtc), GFP_KERNEL);
> @@ -369,6 +375,18 @@ static int jz4740_rtc_probe(struct platform_device *pdev)
>  	/* Each 1 Hz pulse should happen after (rate) ticks */
>  	jz4740_rtc_reg_write(rtc, JZ_REG_RTC_REGULATOR, rate - 1);
>  
> +	magic = jz4740_rtc_reg_read(rtc, JZ_REG_RTC_SCRATCHPAD);
> +	if (magic != JZ_RTC_SCRATCHPAD_MAGIC) {
> +		/*
> +		 * If the scratchpad doesn't hold our magic value, then a
> +		 * power loss occurred. Reset to Epoch.
> +		 */
> +		struct rtc_time time;
> +
> +		rtc_time64_to_tm(0, &time);
> +		jz4740_rtc_set_time(dev, &time);

Don't do that, this defeats the purpose of detecting when the power is
lost. Returning a known bogus time is the worst thing you can do here.

> +	}
> +
>  	ret = devm_rtc_register_device(rtc->rtc);
>  	if (ret)
>  		return ret;
> -- 
> 2.35.1
> 

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

Re: [PATCH 3/5] rtc: jz4740: Reset scratchpad register on power loss

[Paul Cercueil]

Hi Alexandre,

Le mar., avril 19 2022 at 21:35:35 +0200, Alexandre Belloni 
<alexandre.belloni@bootlin.com> a écrit :
> On 18/04/2022 19:49:31+0100, Paul Cercueil wrote:
>>  On power loss, reading the RTC value would fail as the scratchpad 
>> lost
>>  its magic value, until the hardware clock was set once again.
>> 
>>  To avoid that, reset the RTC value to Epoch in the probe if we 
>> detect
>>  that the scratchpad lost its magic value.
>> 
>>  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  ---
>>   drivers/rtc/rtc-jz4740.c | 24 +++++++++++++++++++++---
>>   1 file changed, 21 insertions(+), 3 deletions(-)
>> 
>>  diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
>>  index 119baf168b32..aac5f68bf626 100644
>>  --- a/drivers/rtc/rtc-jz4740.c
>>  +++ b/drivers/rtc/rtc-jz4740.c
>>  @@ -42,6 +42,9 @@
>>   /* Magic value to enable writes on jz4780 */
>>   #define JZ_RTC_WENR_MAGIC	0xA55A
>> 
>>  +/* Value written to the scratchpad to detect power losses */
>>  +#define JZ_RTC_SCRATCHPAD_MAGIC	0x12345678
>>  +
>>   #define JZ_RTC_WAKEUP_FILTER_MASK	0x0000FFE0
>>   #define JZ_RTC_RESET_COUNTER_MASK	0x00000FE0
>> 
>>  @@ -134,10 +137,11 @@ static int jz4740_rtc_ctrl_set_bits(struct 
>> jz4740_rtc *rtc, uint32_t mask,
>>   static int jz4740_rtc_read_time(struct device *dev, struct 
>> rtc_time *time)
>>   {
>>   	struct jz4740_rtc *rtc = dev_get_drvdata(dev);
>>  -	uint32_t secs, secs2;
>>  +	uint32_t secs, secs2, magic;
>>   	int timeout = 5;
>> 
>>  -	if (jz4740_rtc_reg_read(rtc, JZ_REG_RTC_SCRATCHPAD) != 0x12345678)
>>  +	magic = jz4740_rtc_reg_read(rtc, JZ_REG_RTC_SCRATCHPAD);
>>  +	if (magic != JZ_RTC_SCRATCHPAD_MAGIC)
>>   		return -EINVAL;
>> 
>>   	/* If the seconds register is read while it is updated, it can 
>> contain a
>>  @@ -169,7 +173,8 @@ static int jz4740_rtc_set_time(struct device 
>> *dev, struct rtc_time *time)
>>   	if (ret)
>>   		return ret;
>> 
>>  -	return jz4740_rtc_reg_write(rtc, JZ_REG_RTC_SCRATCHPAD, 
>> 0x12345678);
>>  +	return jz4740_rtc_reg_write(rtc, JZ_REG_RTC_SCRATCHPAD,
>>  +				    JZ_RTC_SCRATCHPAD_MAGIC);
>>   }
>> 
>>   static int jz4740_rtc_read_alarm(struct device *dev, struct 
>> rtc_wkalrm *alrm)
>>  @@ -307,6 +312,7 @@ static int jz4740_rtc_probe(struct 
>> platform_device *pdev)
>>   	struct jz4740_rtc *rtc;
>>   	unsigned long rate;
>>   	struct clk *clk;
>>  +	uint32_t magic;
>>   	int ret, irq;
>> 
>>   	rtc = devm_kzalloc(dev, sizeof(*rtc), GFP_KERNEL);
>>  @@ -369,6 +375,18 @@ static int jz4740_rtc_probe(struct 
>> platform_device *pdev)
>>   	/* Each 1 Hz pulse should happen after (rate) ticks */
>>   	jz4740_rtc_reg_write(rtc, JZ_REG_RTC_REGULATOR, rate - 1);
>> 
>>  +	magic = jz4740_rtc_reg_read(rtc, JZ_REG_RTC_SCRATCHPAD);
>>  +	if (magic != JZ_RTC_SCRATCHPAD_MAGIC) {
>>  +		/*
>>  +		 * If the scratchpad doesn't hold our magic value, then a
>>  +		 * power loss occurred. Reset to Epoch.
>>  +		 */
>>  +		struct rtc_time time;
>>  +
>>  +		rtc_time64_to_tm(0, &time);
>>  +		jz4740_rtc_set_time(dev, &time);
> 
> Don't do that, this defeats the purpose of detecting when the power is
> lost. Returning a known bogus time is the worst thing you can do here.

So what is the best thing to do then?

Cheers,
-Paul

>>  +	}
>>  +
>>   	ret = devm_rtc_register_device(rtc->rtc);
>>   	if (ret)
>>   		return ret;
>>  --
>>  2.35.1
>> 
> 
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Re: [PATCH 3/5] rtc: jz4740: Reset scratchpad register on power loss

[Alexandre Belloni]

On 19/04/2022 20:48:54+0100, Paul Cercueil wrote:
> Hi Alexandre,
> 
> Le mar., avril 19 2022 at 21:35:35 +0200, Alexandre Belloni
> <alexandre.belloni@bootlin.com> a écrit :
> > On 18/04/2022 19:49:31+0100, Paul Cercueil wrote:
> > >  On power loss, reading the RTC value would fail as the scratchpad
> > > lost
> > >  its magic value, until the hardware clock was set once again.
> > > 
> > >  To avoid that, reset the RTC value to Epoch in the probe if we
> > > detect
> > >  that the scratchpad lost its magic value.
> > > 
> > >  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> > >  ---
> > >   drivers/rtc/rtc-jz4740.c | 24 +++++++++++++++++++++---
> > >   1 file changed, 21 insertions(+), 3 deletions(-)
> > > 
> > >  diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
> > >  index 119baf168b32..aac5f68bf626 100644
> > >  --- a/drivers/rtc/rtc-jz4740.c
> > >  +++ b/drivers/rtc/rtc-jz4740.c
> > >  @@ -42,6 +42,9 @@
> > >   /* Magic value to enable writes on jz4780 */
> > >   #define JZ_RTC_WENR_MAGIC	0xA55A
> > > 
> > >  +/* Value written to the scratchpad to detect power losses */
> > >  +#define JZ_RTC_SCRATCHPAD_MAGIC	0x12345678
> > >  +
> > >   #define JZ_RTC_WAKEUP_FILTER_MASK	0x0000FFE0
> > >   #define JZ_RTC_RESET_COUNTER_MASK	0x00000FE0
> > > 
> > >  @@ -134,10 +137,11 @@ static int jz4740_rtc_ctrl_set_bits(struct
> > > jz4740_rtc *rtc, uint32_t mask,
> > >   static int jz4740_rtc_read_time(struct device *dev, struct
> > > rtc_time *time)
> > >   {
> > >   	struct jz4740_rtc *rtc = dev_get_drvdata(dev);
> > >  -	uint32_t secs, secs2;
> > >  +	uint32_t secs, secs2, magic;
> > >   	int timeout = 5;
> > > 
> > >  -	if (jz4740_rtc_reg_read(rtc, JZ_REG_RTC_SCRATCHPAD) != 0x12345678)
> > >  +	magic = jz4740_rtc_reg_read(rtc, JZ_REG_RTC_SCRATCHPAD);
> > >  +	if (magic != JZ_RTC_SCRATCHPAD_MAGIC)
> > >   		return -EINVAL;
> > > 
> > >   	/* If the seconds register is read while it is updated, it can
> > > contain a
> > >  @@ -169,7 +173,8 @@ static int jz4740_rtc_set_time(struct device
> > > *dev, struct rtc_time *time)
> > >   	if (ret)
> > >   		return ret;
> > > 
> > >  -	return jz4740_rtc_reg_write(rtc, JZ_REG_RTC_SCRATCHPAD,
> > > 0x12345678);
> > >  +	return jz4740_rtc_reg_write(rtc, JZ_REG_RTC_SCRATCHPAD,
> > >  +				    JZ_RTC_SCRATCHPAD_MAGIC);
> > >   }
> > > 
> > >   static int jz4740_rtc_read_alarm(struct device *dev, struct
> > > rtc_wkalrm *alrm)
> > >  @@ -307,6 +312,7 @@ static int jz4740_rtc_probe(struct
> > > platform_device *pdev)
> > >   	struct jz4740_rtc *rtc;
> > >   	unsigned long rate;
> > >   	struct clk *clk;
> > >  +	uint32_t magic;
> > >   	int ret, irq;
> > > 
> > >   	rtc = devm_kzalloc(dev, sizeof(*rtc), GFP_KERNEL);
> > >  @@ -369,6 +375,18 @@ static int jz4740_rtc_probe(struct
> > > platform_device *pdev)
> > >   	/* Each 1 Hz pulse should happen after (rate) ticks */
> > >   	jz4740_rtc_reg_write(rtc, JZ_REG_RTC_REGULATOR, rate - 1);
> > > 
> > >  +	magic = jz4740_rtc_reg_read(rtc, JZ_REG_RTC_SCRATCHPAD);
> > >  +	if (magic != JZ_RTC_SCRATCHPAD_MAGIC) {
> > >  +		/*
> > >  +		 * If the scratchpad doesn't hold our magic value, then a
> > >  +		 * power loss occurred. Reset to Epoch.
> > >  +		 */
> > >  +		struct rtc_time time;
> > >  +
> > >  +		rtc_time64_to_tm(0, &time);
> > >  +		jz4740_rtc_set_time(dev, &time);
> > 
> > Don't do that, this defeats the purpose of detecting when the power is
> > lost. Returning a known bogus time is the worst thing you can do here.
> 
> So what is the best thing to do then?
> 

Well, -EINVAL is returned when the time is invalid, this should be
enough. I'm not actually sure what is the issue you are trying to fix
here.

> Cheers,
> -Paul
> 
> > >  +	}
> > >  +
> > >   	ret = devm_rtc_register_device(rtc->rtc);
> > >   	if (ret)
> > >   		return ret;
> > >  --
> > >  2.35.1
> > > 
> > 
> > --
> > Alexandre Belloni, co-owner and COO, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com
> 
> 

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

Re: [PATCH 3/5] rtc: jz4740: Reset scratchpad register on power loss

[Paul Cercueil]

Le mar., avril 19 2022 at 22:00:32 +0200, Alexandre Belloni 
<alexandre.belloni@bootlin.com> a écrit :
> On 19/04/2022 20:48:54+0100, Paul Cercueil wrote:
>>  Hi Alexandre,
>> 
>>  Le mar., avril 19 2022 at 21:35:35 +0200, Alexandre Belloni
>>  <alexandre.belloni@bootlin.com> a écrit :
>>  > On 18/04/2022 19:49:31+0100, Paul Cercueil wrote:
>>  > >  On power loss, reading the RTC value would fail as the 
>> scratchpad
>>  > > lost
>>  > >  its magic value, until the hardware clock was set once again.
>>  > >
>>  > >  To avoid that, reset the RTC value to Epoch in the probe if we
>>  > > detect
>>  > >  that the scratchpad lost its magic value.
>>  > >
>>  > >  Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>>  > >  ---
>>  > >   drivers/rtc/rtc-jz4740.c | 24 +++++++++++++++++++++---
>>  > >   1 file changed, 21 insertions(+), 3 deletions(-)
>>  > >
>>  > >  diff --git a/drivers/rtc/rtc-jz4740.c 
>> b/drivers/rtc/rtc-jz4740.c
>>  > >  index 119baf168b32..aac5f68bf626 100644
>>  > >  --- a/drivers/rtc/rtc-jz4740.c
>>  > >  +++ b/drivers/rtc/rtc-jz4740.c
>>  > >  @@ -42,6 +42,9 @@
>>  > >   /* Magic value to enable writes on jz4780 */
>>  > >   #define JZ_RTC_WENR_MAGIC	0xA55A
>>  > >
>>  > >  +/* Value written to the scratchpad to detect power losses */
>>  > >  +#define JZ_RTC_SCRATCHPAD_MAGIC	0x12345678
>>  > >  +
>>  > >   #define JZ_RTC_WAKEUP_FILTER_MASK	0x0000FFE0
>>  > >   #define JZ_RTC_RESET_COUNTER_MASK	0x00000FE0
>>  > >
>>  > >  @@ -134,10 +137,11 @@ static int 
>> jz4740_rtc_ctrl_set_bits(struct
>>  > > jz4740_rtc *rtc, uint32_t mask,
>>  > >   static int jz4740_rtc_read_time(struct device *dev, struct
>>  > > rtc_time *time)
>>  > >   {
>>  > >   	struct jz4740_rtc *rtc = dev_get_drvdata(dev);
>>  > >  -	uint32_t secs, secs2;
>>  > >  +	uint32_t secs, secs2, magic;
>>  > >   	int timeout = 5;
>>  > >
>>  > >  -	if (jz4740_rtc_reg_read(rtc, JZ_REG_RTC_SCRATCHPAD) != 
>> 0x12345678)
>>  > >  +	magic = jz4740_rtc_reg_read(rtc, JZ_REG_RTC_SCRATCHPAD);
>>  > >  +	if (magic != JZ_RTC_SCRATCHPAD_MAGIC)
>>  > >   		return -EINVAL;
>>  > >
>>  > >   	/* If the seconds register is read while it is updated, it 
>> can
>>  > > contain a
>>  > >  @@ -169,7 +173,8 @@ static int jz4740_rtc_set_time(struct 
>> device
>>  > > *dev, struct rtc_time *time)
>>  > >   	if (ret)
>>  > >   		return ret;
>>  > >
>>  > >  -	return jz4740_rtc_reg_write(rtc, JZ_REG_RTC_SCRATCHPAD,
>>  > > 0x12345678);
>>  > >  +	return jz4740_rtc_reg_write(rtc, JZ_REG_RTC_SCRATCHPAD,
>>  > >  +				    JZ_RTC_SCRATCHPAD_MAGIC);
>>  > >   }
>>  > >
>>  > >   static int jz4740_rtc_read_alarm(struct device *dev, struct
>>  > > rtc_wkalrm *alrm)
>>  > >  @@ -307,6 +312,7 @@ static int jz4740_rtc_probe(struct
>>  > > platform_device *pdev)
>>  > >   	struct jz4740_rtc *rtc;
>>  > >   	unsigned long rate;
>>  > >   	struct clk *clk;
>>  > >  +	uint32_t magic;
>>  > >   	int ret, irq;
>>  > >
>>  > >   	rtc = devm_kzalloc(dev, sizeof(*rtc), GFP_KERNEL);
>>  > >  @@ -369,6 +375,18 @@ static int jz4740_rtc_probe(struct
>>  > > platform_device *pdev)
>>  > >   	/* Each 1 Hz pulse should happen after (rate) ticks */
>>  > >   	jz4740_rtc_reg_write(rtc, JZ_REG_RTC_REGULATOR, rate - 1);
>>  > >
>>  > >  +	magic = jz4740_rtc_reg_read(rtc, JZ_REG_RTC_SCRATCHPAD);
>>  > >  +	if (magic != JZ_RTC_SCRATCHPAD_MAGIC) {
>>  > >  +		/*
>>  > >  +		 * If the scratchpad doesn't hold our magic value, then a
>>  > >  +		 * power loss occurred. Reset to Epoch.
>>  > >  +		 */
>>  > >  +		struct rtc_time time;
>>  > >  +
>>  > >  +		rtc_time64_to_tm(0, &time);
>>  > >  +		jz4740_rtc_set_time(dev, &time);
>>  >
>>  > Don't do that, this defeats the purpose of detecting when the 
>> power is
>>  > lost. Returning a known bogus time is the worst thing you can do 
>> here.
>> 
>>  So what is the best thing to do then?
>> 
> 
> Well, -EINVAL is returned when the time is invalid, this should be
> enough. I'm not actually sure what is the issue you are trying to fix
> here.

htop fails to start and tells me:
"No btime in /proc/stat: No such file or directory"

until the date is reset. So I was assuming it was a case of the jz4740 
driver not being correct and breaking userspace.

Cheers,
-Paul


> 
>>  Cheers,
>>  -Paul
>> 
>>  > >  +	}
>>  > >  +
>>  > >   	ret = devm_rtc_register_device(rtc->rtc);
>>  > >   	if (ret)
>>  > >   		return ret;
>>  > >  --
>>  > >  2.35.1
>>  > >
>>  >
>>  > --
>>  > Alexandre Belloni, co-owner and COO, Bootlin
>>  > Embedded Linux and Kernel engineering
>>  > https://bootlin.com
>> 
>> 
> 
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Re: [PATCH 3/5] rtc: jz4740: Reset scratchpad register on power loss

[Alexandre Belloni]

On 19/04/2022 21:53:17+0100, Paul Cercueil wrote:
> > >  So what is the best thing to do then?
> > > 
> > 
> > Well, -EINVAL is returned when the time is invalid, this should be
> > enough. I'm not actually sure what is the issue you are trying to fix
> > here.
> 
> htop fails to start and tells me:
> "No btime in /proc/stat: No such file or directory"
> 
> until the date is reset. So I was assuming it was a case of the jz4740
> driver not being correct and breaking userspace.
> 

I guess either /proc/stat or htop needs fixing then ;)

> Cheers,
> -Paul
> 
> 
> > 
> > >  Cheers,
> > >  -Paul
> > > 
> > >  > >  +	}
> > >  > >  +
> > >  > >   	ret = devm_rtc_register_device(rtc->rtc);
> > >  > >   	if (ret)
> > >  > >   		return ret;
> > >  > >  --
> > >  > >  2.35.1
> > >  > >
> > >  >
> > >  > --
> > >  > Alexandre Belloni, co-owner and COO, Bootlin
> > >  > Embedded Linux and Kernel engineering
> > >  > https://bootlin.com
> > > 
> > > 
> > 
> > --
> > Alexandre Belloni, co-owner and COO, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com
> 
> 

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