Hi Alessandro / Alexandre,
Here's the V2 of a previous patchset I sent back in April.
Patch [1/4] was updated with Krzysztof's feedback.
Patches 2-4 are unmodified from V1 as they didn't receive any feedback.
Patch 4 is RFC; I *think* it works, but I don't know how to test it.
V1 had a 5th patch which would reset the scratchpad register on power
loss, but was dropped following upstream feedback.
Cheers,
- Paul
Paul Cercueil (4):
dt-bindings: rtc: ingenic: Rework compatible strings and add #clock-cells
rtc: jz4740: Use readl_poll_timeout
rtc: jz4740: Register clock provider for the CLK32K pin
rtc: jz4740: Support for fine-tuning the RTC clock
.../devicetree/bindings/rtc/ingenic,rtc.yaml | 32 ++++-
drivers/rtc/rtc-jz4740.c | 113 +++++++++++++++---
2 files changed, 129 insertions(+), 16 deletions(-)
--
2.35.1
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 <[email protected]>
---
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 c383719292c7..4ee6e5ee09b1 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
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 <[email protected]>
---
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 4ee6e5ee09b1..1602e8a4283a 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
@@ -45,10 +47,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 {
@@ -57,6 +62,8 @@ struct jz4740_rtc {
struct rtc_device *rtc;
+ struct clk_hw clk32k;
+
spinlock_t lock;
};
@@ -254,7 +261,8 @@ static void jz4740_rtc_power_off(void)
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);
@@ -295,6 +303,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;
@@ -370,6 +399,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
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 <[email protected]>
---
v2: - add constraint on which SoCs can have the #clock-cells property
- add JZ4780 example which has a #clock-cells
.../devicetree/bindings/rtc/ingenic,rtc.yaml | 32 +++++++++++++++++--
1 file changed, 30 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml b/Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml
index b235b2441997..d63bb727cee5 100644
--- a/Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml
+++ b/Documentation/devicetree/bindings/rtc/ingenic,rtc.yaml
@@ -11,6 +11,16 @@ maintainers:
allOf:
- $ref: rtc.yaml#
+ - if:
+ not:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - ingenic,jz4770-rtc
+ then:
+ properties:
+ "#clock-cells": false
properties:
compatible:
@@ -18,14 +28,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 +49,9 @@ properties:
clock-names:
const: rtc
+ "#clock-cells":
+ const: 0
+
system-power-controller:
description: |
Indicates that the RTC is responsible for powering OFF
@@ -83,3 +96,18 @@ examples:
clocks = <&cgu JZ4740_CLK_RTC>;
clock-names = "rtc";
};
+
+ - |
+ #include <dt-bindings/clock/ingenic,jz4780-cgu.h>
+ rtc: rtc@10003000 {
+ compatible = "ingenic,jz4780-rtc", "ingenic,jz4770-rtc";
+ reg = <0x10003000 0x4c>;
+
+ interrupt-parent = <&intc>;
+ interrupts = <32>;
+
+ clocks = <&cgu JZ4780_CLK_RTCLK>;
+ clock-names = "rtc";
+
+ #clock-cells = <0>;
+ };
--
2.35.1
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 <[email protected]>
---
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 1602e8a4283a..70b63ce75e21 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
@@ -61,6 +65,7 @@ struct jz4740_rtc {
enum jz4740_rtc_type type;
struct rtc_device *rtc;
+ struct clk *clk;
struct clk_hw clk32k;
@@ -217,12 +222,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)
@@ -353,6 +397,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
Hi Paul,
I love your patch! Yet something to improve:
[auto build test ERROR on abelloni/rtc-next]
[also build test ERROR on robh/for-next linus/master v6.1-rc2 next-20221028]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Paul-Cercueil/rtc-ingenic-various-updates/20221029-065805
base: https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next
patch link: https://lore.kernel.org/r/20221028225519.89210-5-paul%40crapouillou.net
patch subject: [PATCH v2 4/4] rtc: jz4740: Support for fine-tuning the RTC clock
config: sparc64-randconfig-c043-20221028
compiler: sparc64-linux-gcc (GCC) 12.1.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/b73614c39710acaff7977b8d3ec935105cf59757
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Paul-Cercueil/rtc-ingenic-various-updates/20221029-065805
git checkout b73614c39710acaff7977b8d3ec935105cf59757
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sparc64 SHELL=/bin/bash drivers/rtc/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
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:357:45: error: call to '__compiletime_assert_248' declared with attribute error: FIELD_PREP: value too large for the field
357 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^
include/linux/compiler_types.h:338:25: note: in definition of macro '__compiletime_assert'
338 | prefix ## suffix(); \
| ^~~~~~
include/linux/compiler_types.h:357:9: note: in expansion of macro '_compiletime_assert'
357 | _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:68:17: note: in expansion of macro 'BUILD_BUG_ON_MSG'
68 | BUILD_BUG_ON_MSG(__builtin_constant_p(_val) ? \
| ^~~~~~~~~~~~~~~~
include/linux/bitfield.h:114:17: note: in expansion of macro '__BF_FIELD_CHECK'
114 | __BF_FIELD_CHECK(_mask, 0ULL, _val, "FIELD_PREP: "); \
| ^~~~~~~~~~~~~~~~
drivers/rtc/rtc-jz4740.c:256:17: note: in expansion of macro 'FIELD_PREP'
256 | nc1hz = FIELD_PREP(JZ_RTC_REGULATOR_NC1HZ_MASK, nc1hz);
| ^~~~~~~~~~
vim +/__compiletime_assert_248 +357 include/linux/compiler_types.h
eb5c2d4b45e3d2 Will Deacon 2020-07-21 343
eb5c2d4b45e3d2 Will Deacon 2020-07-21 344 #define _compiletime_assert(condition, msg, prefix, suffix) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21 345 __compiletime_assert(condition, msg, prefix, suffix)
eb5c2d4b45e3d2 Will Deacon 2020-07-21 346
eb5c2d4b45e3d2 Will Deacon 2020-07-21 347 /**
eb5c2d4b45e3d2 Will Deacon 2020-07-21 348 * compiletime_assert - break build and emit msg if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21 349 * @condition: a compile-time constant condition to check
eb5c2d4b45e3d2 Will Deacon 2020-07-21 350 * @msg: a message to emit if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21 351 *
eb5c2d4b45e3d2 Will Deacon 2020-07-21 352 * In tradition of POSIX assert, this macro will break the build if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21 353 * supplied condition is *false*, emitting the supplied error message if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21 354 * compiler has support to do so.
eb5c2d4b45e3d2 Will Deacon 2020-07-21 355 */
eb5c2d4b45e3d2 Will Deacon 2020-07-21 356 #define compiletime_assert(condition, msg) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21 @357 _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
eb5c2d4b45e3d2 Will Deacon 2020-07-21 358
--
0-DAY CI Kernel Test Service
https://01.org/lkp
Hi Bot,
Well this report is on my RFC patch so I consider the patchset to still
be valid.
If I can actually test my RFC patch I'll send it again with this bug
fixed.
Cheers,
-Paul
Le sam. 29 oct. 2022 ? 16:51:53 +0800, kernel test robot
<[email protected]> a ?crit :
> Hi Paul,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on abelloni/rtc-next]
> [also build test ERROR on robh/for-next linus/master v6.1-rc2
> next-20221028]
> [If your patch is applied to the wrong git tree, kindly drop us a
> note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url:
> https://github.com/intel-lab-lkp/linux/commits/Paul-Cercueil/rtc-ingenic-various-updates/20221029-065805
> base:
> https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git
> rtc-next
> patch link:
> https://lore.kernel.org/r/20221028225519.89210-5-paul%40crapouillou.net
> patch subject: [PATCH v2 4/4] rtc: jz4740: Support for fine-tuning
> the RTC clock
> config: sparc64-randconfig-c043-20221028
> compiler: sparc64-linux-gcc (GCC) 12.1.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/b73614c39710acaff7977b8d3ec935105cf59757
> git remote add linux-review
> https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review
> Paul-Cercueil/rtc-ingenic-various-updates/20221029-065805
> git checkout b73614c39710acaff7977b8d3ec935105cf59757
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0
> make.cross W=1 O=build_dir ARCH=sparc64 SHELL=/bin/bash drivers/rtc/
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <[email protected]>
>
> 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:357:45: error: call to
>>> '__compiletime_assert_248' declared with attribute error:
>>> FIELD_PREP: value too large for the field
> 357 | _compiletime_assert(condition, msg,
> __compiletime_assert_, __COUNTER__)
> | ^
> include/linux/compiler_types.h:338:25: note: in definition of
> macro '__compiletime_assert'
> 338 | prefix ## suffix();
> \
> | ^~~~~~
> include/linux/compiler_types.h:357:9: note: in expansion of macro
> '_compiletime_assert'
> 357 | _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:68:17: note: in expansion of macro
> 'BUILD_BUG_ON_MSG'
> 68 |
> BUILD_BUG_ON_MSG(__builtin_constant_p(_val) ? \
> | ^~~~~~~~~~~~~~~~
> include/linux/bitfield.h:114:17: note: in expansion of macro
> '__BF_FIELD_CHECK'
> 114 | __BF_FIELD_CHECK(_mask, 0ULL, _val,
> "FIELD_PREP: "); \
> | ^~~~~~~~~~~~~~~~
> drivers/rtc/rtc-jz4740.c:256:17: note: in expansion of macro
> 'FIELD_PREP'
> 256 | nc1hz = FIELD_PREP(JZ_RTC_REGULATOR_NC1HZ_MASK,
> nc1hz);
> | ^~~~~~~~~~
>
>
> vim +/__compiletime_assert_248 +357 include/linux/compiler_types.h
>
> eb5c2d4b45e3d2 Will Deacon 2020-07-21 343
> eb5c2d4b45e3d2 Will Deacon 2020-07-21 344 #define
> _compiletime_assert(condition, msg, prefix, suffix) \
> eb5c2d4b45e3d2 Will Deacon 2020-07-21 345
> __compiletime_assert(condition, msg, prefix, suffix)
> eb5c2d4b45e3d2 Will Deacon 2020-07-21 346
> eb5c2d4b45e3d2 Will Deacon 2020-07-21 347 /**
> eb5c2d4b45e3d2 Will Deacon 2020-07-21 348 * compiletime_assert -
> break build and emit msg if condition is false
> eb5c2d4b45e3d2 Will Deacon 2020-07-21 349 * @condition: a
> compile-time constant condition to check
> eb5c2d4b45e3d2 Will Deacon 2020-07-21 350 * @msg: a message
> to emit if condition is false
> eb5c2d4b45e3d2 Will Deacon 2020-07-21 351 *
> eb5c2d4b45e3d2 Will Deacon 2020-07-21 352 * In tradition of POSIX
> assert, this macro will break the build if the
> eb5c2d4b45e3d2 Will Deacon 2020-07-21 353 * supplied condition is
> *false*, emitting the supplied error message if the
> eb5c2d4b45e3d2 Will Deacon 2020-07-21 354 * compiler has support
> to do so.
> eb5c2d4b45e3d2 Will Deacon 2020-07-21 355 */
> eb5c2d4b45e3d2 Will Deacon 2020-07-21 356 #define
> compiletime_assert(condition, msg) \
> eb5c2d4b45e3d2 Will Deacon 2020-07-21 @357
> _compiletime_assert(condition, msg, __compiletime_assert_,
> __COUNTER__)
> eb5c2d4b45e3d2 Will Deacon 2020-07-21 358
>
> --
> 0-DAY CI Kernel Test Service
> https://01.org/lkp
Hi Rob,
Le lun. 31 oct. 2022 ? 14:23:41 -0500, Rob Herring <[email protected]> a
?crit :
> On Fri, Oct 28, 2022 at 11:55:16PM +0100, 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.
>
> This is a compatibility mess. There is no driver support in v6.1-rc
> for
> ingenic,jz4770-rtc, so a new DT would not work with existing kernels.
> It
> sounds like you need 3 compatibles for 4780.
Do newer DTs need to work with older kernels? I always assumed the
compatibility was only enforced for the other way around.
If that's the case though, I guess I can enforce 3 compatibles for 4780
and 2 for 4770 like you suggested.
Cheers,
-Paul
>>
>> 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 <[email protected]>
>> ---
>>
>> v2: - add constraint on which SoCs can have the #clock-cells
>> property
>> - add JZ4780 example which has a #clock-cells
>>
>> .../devicetree/bindings/rtc/ingenic,rtc.yaml | 32
>> +++++++++++++++++--
>> 1 file changed, 30 insertions(+), 2 deletions(-)
On Fri, Oct 28, 2022 at 11:55:16PM +0100, 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.
This is a compatibility mess. There is no driver support in v6.1-rc for
ingenic,jz4770-rtc, so a new DT would not work with existing kernels. It
sounds like you need 3 compatibles for 4780.
>
> 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 <[email protected]>
> ---
>
> v2: - add constraint on which SoCs can have the #clock-cells property
> - add JZ4780 example which has a #clock-cells
>
> .../devicetree/bindings/rtc/ingenic,rtc.yaml | 32 +++++++++++++++++--
> 1 file changed, 30 insertions(+), 2 deletions(-)
On Mon, Oct 31, 2022 at 2:41 PM Paul Cercueil <[email protected]> wrote:
>
> Hi Rob,
>
> Le lun. 31 oct. 2022 à 14:23:41 -0500, Rob Herring <[email protected]> a
> écrit :
> > On Fri, Oct 28, 2022 at 11:55:16PM +0100, 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.
> >
> > This is a compatibility mess. There is no driver support in v6.1-rc
> > for
> > ingenic,jz4770-rtc, so a new DT would not work with existing kernels.
> > It
> > sounds like you need 3 compatibles for 4780.
>
> Do newer DTs need to work with older kernels? I always assumed the
> compatibility was only enforced for the other way around.
For a stable platform, yes. Would you want a firmware update carrying
the DT to break an existing OS install?
Compatibility either way ultimately is up to the platform whether you
care or not. I just ask that the commit msg spell that out. In this
case, it is easily avoided.
Rob
On 28/10/2022 23:55:15+0100, Paul Cercueil wrote:
> Hi Alessandro / Alexandre,
>
> Here's the V2 of a previous patchset I sent back in April.
>
> Patch [1/4] was updated with Krzysztof's feedback.
>
> Patches 2-4 are unmodified from V1 as they didn't receive any feedback.
> Patch 4 is RFC; I *think* it works, but I don't know how to test it.
>
You should simply adjust the RTC and see whether the drift changes. The
best is to simply use chrony as this will actually tell you how much the
RTC drifts. This actually gives you the exact value to put in offset to
remove the drift.
> V1 had a 5th patch which would reset the scratchpad register on power
> loss, but was dropped following upstream feedback.
>
> Cheers,
> - Paul
>
> Paul Cercueil (4):
> dt-bindings: rtc: ingenic: Rework compatible strings and add #clock-cells
> rtc: jz4740: Use readl_poll_timeout
> rtc: jz4740: Register clock provider for the CLK32K pin
> rtc: jz4740: Support for fine-tuning the RTC clock
>
> .../devicetree/bindings/rtc/ingenic,rtc.yaml | 32 ++++-
> drivers/rtc/rtc-jz4740.c | 113 +++++++++++++++---
> 2 files changed, 129 insertions(+), 16 deletions(-)
>
> --
> 2.35.1
>
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com