* Re: [PATCH] rtc: interface: Add rtc time jump debug in rtc_timer_do_work()
From: Jinjie Ruan @ 2026-06-16 8:12 UTC (permalink / raw)
To: Alexandre Belloni; +Cc: linux-rtc, linux-kernel
In-Reply-To: <202606160657465eab6e41@mail.local>
On 6/16/2026 2:57 PM, Alexandre Belloni wrote:
> Hi,
>
> On 16/06/2026 10:10:19+0800, Jinjie Ruan wrote:
>>
>>
>> On 6/15/2026 11:22 PM, Alexandre Belloni wrote:
>>> Hello,
>>>
>>> On 25/05/2026 21:08:25+0800, Jinjie Ruan wrote:
>>>> In virtualization environments like QEMU [1], or during hardware
>>>> clocksource anomalies, an extreme time-warp event can occur. When
>>>> the system time abruptly jumps forward, the rtc_timer_do_work() handler
>>>> falls into a prolonged processing loop to clear accumulated historical
>>>> timers via timerqueue_getnext(). Running this loop indefinitely under
>>>> the rtc->ops_lock mutex triggers a kernel softlockup, stalling
>>>> the system.
>>>>
>>>> Introduce an adaptive telemetry and loop guard mechanism to enhance debug
>>>> visibility and prevent softlockups:
>>>>
>>>> 1. Record `start_jiffies` upon entry and leverage `time_after()` to
>>>> check if the loop has monopolized the CPU for more than 1s (HZ). If so,
>>>> the handler prints a telemetry warning, triggers a WARN stack dump, and
>>>> breaks the loop to safely yield the CPU.
>>>>
>>>> 2. Track the execution via a `loop_count` metric. Printing this counter
>>>> in the warning log provides vital diagnostics to distinguish
>>>> an aggressive time-warp storm (high count) from a bogged-down callback
>>>> bug (low count).
>>>>
>>>> 3. Utilize the kernel format specifier `%ptR` to convert the raw ktime
>>>> into a human-readable timestamp (YYYY-MM-DD HH:MM:SS), allowing
>>>> developers to instantly pinpoint the exact boundary of the time
>>>> jump in dmesg.
>>>>
>>>> This non-destructive telemetry guard provides precise hardware/emulator
>>>> diagnostic visibility while ensuring core kernel availability.
>>>>
>>>> [1]: https://lore.kernel.org/all/20260114013257.3500578-1-ruanjinjie@huawei.com/
>>>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>>>> ---
>>>> drivers/rtc/interface.c | 15 +++++++++++++--
>>>> 1 file changed, 13 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
>>>> index 1906f4884a83..f6c5fd16cc4e 100644
>>>> --- a/drivers/rtc/interface.c
>>>> +++ b/drivers/rtc/interface.c
>>>> @@ -927,10 +927,12 @@ static void rtc_timer_remove(struct rtc_device *rtc, struct rtc_timer *timer)
>>>> */
>>>> void rtc_timer_do_work(struct work_struct *work)
>>>> {
>>>> - struct rtc_timer *timer;
>>>> + unsigned long start_jiffies = jiffies;
>>>> struct timerqueue_node *next;
>>>> - ktime_t now;
>>>> + struct rtc_timer *timer;
>>>> struct rtc_time tm;
>>>> + int loop_count = 0;
>>>> + ktime_t now;
>>>> int err;
>>>>
>>>> struct rtc_device *rtc =
>>>> @@ -945,6 +947,15 @@ void rtc_timer_do_work(struct work_struct *work)
>>>> }
>>>> now = rtc_tm_to_ktime(tm);
>>>> while ((next = timerqueue_getnext(&rtc->timerqueue))) {
>>>> + loop_count++;
>>>> +
>>>> + if (unlikely(time_after(jiffies, start_jiffies + HZ))) {
>>>> + dev_warn(&rtc->dev, "RTC time jump (loop: %d) to %ptR.\n",
>>>> + loop_count, &tm);
>>>> + WARN_ON_ONCE(1);
>>>
>>> So, your issue is that it is too slow so you make it even slower? There
>>> are already plenty of tracepoints that allow proper debugging in this
>>> loop, I'm pretty sure we don't want to bloat the kernel with more
>>> messages.
>>
>> Hi, Alexandre,
>>
>> The point here is not about the performance of the rtc_timer_do_work()
>> loop — it’s about making the problem debuggable when things go wrong.
>> And we can put it under a debug Kconfig option, so production kernels
>> see no extra overhead at all.
>
>
> But then aren't the tracepoint enough? There are 3 tracepoints in the
> loop that are exactly for debugging your issue.
Hi, Alexandre,
Regarding the `trace_rtc_timer_dequeue` and `trace_rtc_timer_enqueue`
tracepoints, they are unfortunately insufficient to pinpoint this issue
for three reasons:
1. Ring Buffer Overwrite during Softlockup:
When the loop iterates tens of millions of times continuously on a
locked-up CPU, it floods the ftrace ring buffer in milliseconds. The
earliest trace logs—which contain the exact moment the time jumped—will
be completely overwritten and lost before anyone can read them.
2. Lack of Causality Context:
These tracepoints only log the timer's expiration and queue state.They
show the symptom (infinite re-enqueuing). But these are not the primary
scene, the time jump is the primary scene. They do not capture why are
there so many UIE timers being re-enqueued?
>
>>
>> The patch is installed in the following scenarios:
>>
>> If the RTC hardware fails, or if the QEMU-emulated RTC device code in a
>> KVM virtual machine has a problem (for example, the x86 RTC emulation
>> hardware mc146818 has an overflow issue[1]), the time may jump as shown
>> in the log below, which can cause a soft lockup.
>
> This is not clear, you are not explaining the issue. You seem to mix
> system time and RTC time. What I understand is that there was a periodic
> timer enqueued and then for some reason, the system time jumped forward
> by a large amount and now rtc_timer_do_work is firing events for each of
> the missed timers. You are not explaining the relationship with the RTC
> hardware (I see none).
The time here all means the RTC time got by __rtc_read_time(), not the
system time.
And the RTC time jump is caused by RTC hardware failure or
`QEMU-emulated RTC device` code bug, which means the rtc hardware
returns an unstable rtc time or is not completely linear growth..
The original issue is as follows：
On kvm qemu with cmos rtc and mc146818 chip, after set the UIE timer
expire with a normal RTC time （for example 2026 year）, In
rtc_timer_do_work(), the rtc time jump to a future time (for example
2033 year), it will loop for a while util softlockup because all
subsequent enqueued UIE timers expires, as below:
RTC_UIE_ON:
read now: 2019:04:08:12:32:27, add timer0 (expire: 2019:04:08:12:32:28)
^^^^^^^^^^^^^^^^^^^^
...
rtc_timer_do_work() iterate the list in a loop:
read now: 2033:12:02:07:27:15
^^^^^^^^^^^^^^^^^^^
handle timer0, add timer1 to the list (expire: 2019:04:08:12:32:29)
handle timer1, add timer2 to the list (expire: 2019:04:08:12:32:30)
handle timer2, add timer3: 2019:04:08:12:32:31
...
-> softlockup
>
>>
>> However, when the issue occurs, it is only possible to know that too
>> many pending timers have accumulated in the timerqueue (for example, the
>> log shows that tens of millions of timer nodes have been processed) by
>> temporarily adding diagnostic code in rtc_timer_do_work().
>>
>
> This is not true, there are 3 tracepoints to know what is happening with
> the timers. Also, there are always exactly zero, one or two timers in
> the queue, never tens of millions.
Timer Queue Size vs. Loop Iteration Count:
You are completely correct that there are only ever 0, 1, or 2 timer
nodes linked in the timerqueue. My previous phrasing was inaccurate, I
mean the loop Iteration.
>
>> To determine whether the root cause is hardware, kernel RTC code, an RTC
>> driver issue, or an RTC hardware problem, more debugging is needed. But
>> if the problem is indeed caused by RTC hardware, adding a diagnostic
>> print of the current RTC time when the loop takes too long (as this
>> diagnostic patch does) would make it easy to tell whether QEMU or the
>> hardware is faulty.
>>
>> [1]: https://lore.kernel.org/all/20260613195116.1807273-21-mjt@tls.msk.ru/
>>
>> kworker/0:1-37 [000] .N.. 489.159634: rtc_timer_do_work:
>> timerqueue_getnext handle timer node count: 13281423
>> kworker/0:1-37 [000] .N.. 489.159635: rtc_timer_do_work:
>> timerqueue_getnext handle timer node count: 13281424
>> kworker/0:1-37 [000] .N.. 489.159635: rtc_timer_do_work:
>> timerqueue_getnext handle timer node count: 13281425
>> kworker/0:1-37 [000] .N.. 489.159636: rtc_timer_do_work:
>> timerqueue_getnext handle timer node count: 13281426
>> kworker/0:1-37 [000] .N.. 489.159637: rtc_timer_do_work:
>> timerqueue_getnext handle timer node count: 13281427
>> kworker/0:1-37 [000] .N.. 489.159638: rtc_timer_do_work:
>> timerqueue_getnext handle timer node count: 13281428
>> kworker/0:1-37 [000] .N.. 489.159638: rtc_timer_do_work:
>> timerqueue_getnext handle timer node count: 13281429
>> kworker/0:1-37 [000] .N.. 489.159639: rtc_timer_do_work:
>> timerqueue_getnext handle timer node count: 13281430
>> kworker/0:1-37 [000] .N.. 489.159640: rtc_timer_do_work:
>> timerqueue_getnext handle timer node count: 13281431
>> kworker/0:1-37 [000] .N.. 489.159641: rtc_timer_do_work:
>> timerqueue_getnext handle timer node count: 13281432
>>
>>
>> swapper/0-1 [001] .N.. 11.579334: __rtc_read_time: rtc:
>> 0xff11000109896800, ops->read_time:2026:01:05:09:36:21
>> swapper/0-1 [001] .N.. 11.579421: __rtc_read_time: rtc:
>> 0xff11000109896800, ops->read_time:2026:01:05:09:36:21
>> swapper/0-1 [001] .N.. 11.579469: __rtc_read_time: rtc:
>> 0xff11000109896800, ops->read_time:2026:01:05:09:36:21
>> swapper/0-1 [001] .N.. 11.580816: __rtc_read_time: rtc:
>> 0xff11000109896800, ops->read_time:2026:01:05:09:36:21
>> syz-executor.5-7492 [003] .... 129.807406: __rtc_read_time: rtc:
>> 0xff11000109896800, ops->read_time:2033:05:04:07:03:51
>> syz-executor.5-7492 [003] .... 129.807419:
>> __rtc_update_irq_enable.part.8: rtc uie on: 0xff11000109896800, now:
>> 2033:05:04:07:03:51, expire: 2033:05:04:07:03:52
>>
>>
>> Best regards,
>> Jinjie
>>
>>>
>>>
>>
>
^ permalink raw reply
* [PATCH v2] dt-bindings: rtc: Convert rtc-cmos binding to YAML
From: Teja Sai Charan B @ 2026-06-16 8:56 UTC (permalink / raw)
To: Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-rtc, devicetree, linux-kernel, Teja Sai Charan Bellamkonda
From: Teja Sai Charan Bellamkonda <tejaasaye@gmail.com>
Convert the rtc-cmos devicetree bindings to dt schema.
Signed-off-by: Teja Sai Charan Bellamkonda <tejaasaye@gmail.com>
---
Changes in v2:
- Allow intel,ce4100-rtc compatible used by existing DTS files
---
.../devicetree/bindings/rtc/rtc-cmos.txt | 27 ---------
.../devicetree/bindings/rtc/rtc-cmos.yaml | 60 +++++++++++++++++++
result.txt | 17 ++++++
3 files changed, 77 insertions(+), 27 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/rtc/rtc-cmos.txt
create mode 100644 Documentation/devicetree/bindings/rtc/rtc-cmos.yaml
create mode 100644 result.txt
diff --git a/Documentation/devicetree/bindings/rtc/rtc-cmos.txt b/Documentation/devicetree/bindings/rtc/rtc-cmos.txt
deleted file mode 100644
index 7d7b5f6bda65..000000000000
--- a/Documentation/devicetree/bindings/rtc/rtc-cmos.txt
+++ /dev/null
@@ -1,27 +0,0 @@
- Motorola mc146818 compatible RTC
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-
-Required properties:
- - compatible : "motorola,mc146818"
- - reg : should contain registers location and length.
-
-Optional properties:
- - interrupts : should contain interrupt.
- - ctrl-reg : Contains the initial value of the control register also
- called "Register B".
- - freq-reg : Contains the initial value of the frequency register also
- called "Register A".
-
-"Register A" and "B" are usually initialized by the firmware (BIOS for
-instance). If this is not done, it can be performed by the driver.
-
-ISA Example:
-
- rtc@70 {
- compatible = "motorola,mc146818";
- interrupts = <8 3>;
- interrupt-parent = <&ioapic1>;
- ctrl-reg = <2>;
- freq-reg = <0x26>;
- reg = <1 0x70 2>;
- };
diff --git a/Documentation/devicetree/bindings/rtc/rtc-cmos.yaml b/Documentation/devicetree/bindings/rtc/rtc-cmos.yaml
new file mode 100644
index 000000000000..ba4812778115
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/rtc-cmos.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/rtc-cmos.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Motorola mc146818 compatible RTC
+
+maintainers:
+ - Alexandre Belloni <alexandre.belloni@bootlin.com>
+
+properties:
+ compatible:
+ oneOf:
+ - const: motorola,mc146818
+
+ - items:
+ - const: intel,ce4100-rtc
+ - const: motorola,mc146818
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ ctrl-reg:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Initial value of the control register
+ (also known as Register B).
+
+ freq-reg:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Initial value of the frequency register
+ (also known as Register A).
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ bus {
+ #address-cells = <2>;
+ #size-cells = <1>;
+
+ rtc@70 {
+ compatible = "motorola,mc146818";
+ reg = <1 0x70 2>;
+
+ interrupts = <8 3>;
+
+ ctrl-reg = <2>;
+ freq-reg = <0x26>;
+ };
+ };
diff --git a/result.txt b/result.txt
new file mode 100644
index 000000000000..5e90660b93ec
--- /dev/null
+++ b/result.txt
@@ -0,0 +1,17 @@
+arch/x86/kernel/x86_init.c:42: { .compatible = "motorola,mc146818" },
+arch/x86/platform/ce4100/falconfalls.dts:420: compatible = "intel,ce4100-rtc", "motorola,mc146818";
+arch/mips/boot/dts/loongson/rs780e-pch.dtsi:31: compatible = "motorola,mc146818";
+arch/mips/boot/dts/mti/malta.dts:110: compatible = "motorola,mc146818";
+arch/alpha/kernel/rtc.c:25: * We don't want to use the rtc-cmos driver, because we don't want to support
+drivers/built-in.a:1031:rtc/rtc-cmos.o/
+drivers/rtc/built-in.a:11:rtc-cmos.o/
+drivers/rtc/.rtc-mc146818-lib.o.cmd:1:savedcmd_drivers/rtc/rtc-mc146818-lib.o := gcc -Wp,-MMD,drivers/rtc/.rtc-mc146818-lib.o.d -nostdinc -I./arch/x86/include -I./arch/x86/include/generated -I./include -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -std=gnu11 -fshort-wchar -funsigned-char -fno-common -fno-PIE -fno-strict-aliasing -mno-sse -mno-mmx -mno-sse2 -mno-3dnow -mno-avx -mno-sse4a -fcf-protection=none -m64 -falign-jumps=1 -falign-loops=1 -mno-80387 -mno-fp-ret-in-387 -mpreferred-stack-boundary=3 -mskip-rax-setup -march=x86-64 -mtune=generic -mno-red-zone -mcmodel=kernel -mstack-protector-guard-reg=gs -mstack-protector-guard-symbol=__ref_stack_chk_guard -Wno-sign-compare -fno-asynchronous-unwind-tables -mindirect-branch=thunk-extern -mindirect-branch-register -mindirect-branch-cs-prefix -mfunction-return=thunk-extern -fno-jump-tables -mharden-sls=all -fpatchable-function-entry=16,16 -fno-delete-null-pointer-checks -O2 -fno-allow-store-data-races -fstack-protector-strong -fno-omit-frame-pointer -fno-optimize-sibling-calls -ftrivial-auto-var-init=zero -fno-stack-clash-protection -fzero-call-used-regs=used-gpr -pg -mrecord-mcount -mfentry -DCC_USING_FENTRY -falign-functions=16 -fstrict-flex-arrays=3 -fms-extensions -fno-strict-overflow -fno-stack-check -fconserve-stack -fno-builtin-wcslen -Wall -Wextra -Wundef -Werror=implicit-function-declaration -Werror=implicit-int -Werror=return-type -Werror=strict-prototypes -Wno-format-security -Wno-trigraphs -Wno-frame-address -Wno-address-of-packed-member -Wmissing-declarations -Wmissing-prototypes -Wframe-larger-than=1024 -Wno-main -Wno-dangling-pointer -Wvla-larger-than=1 -Wno-pointer-sign -Wcast-function-type -Wno-array-bounds -Wno-stringop-overflow -Wno-alloc-size-larger-than -Wimplicit-fallthrough=5 -Werror=date-time -Werror=incompatible-pointer-types -Werror=designated-init -Wenum-conversion -Wunused -Wno-unused-but-set-variable -Wno-unused-const-variable -Wno-packed-not-aligned -Wno-format-overflow -Wno-format-truncation -Wno-stringop-truncation -Wno-override-init -Wno-missing-field-initializers -Wno-type-limits -Wno-shift-negative-value -Wno-maybe-uninitialized -Wno-sign-compare -Wno-unused-parameter -g -gdwarf-5 -fsanitize=bounds-strict -fsanitize=shift -fsanitize=bool -fsanitize=enum -DKBUILD_MODFILE='"drivers/rtc/rtc-mc146818-lib"' -DKBUILD_BASENAME='"rtc_mc146818_lib"' -DKBUILD_MODNAME='"rtc_mc146818_lib"' -D__KBUILD_MODNAME=rtc_mc146818_lib -c -o drivers/rtc/rtc-mc146818-lib.o drivers/rtc/rtc-mc146818-lib.c
+drivers/rtc/.built-in.a.cmd:1:savedcmd_drivers/rtc/built-in.a := rm -f drivers/rtc/built-in.a; printf "drivers/rtc/%s " lib.o class.o interface.o nvmem.o dev.o proc.o sysfs.o rtc-mc146818-lib.o rtc-cmos.o | xargs ar cDPrST drivers/rtc/built-in.a
+drivers/rtc/Kconfig:1065: will be called rtc-cmos.
+drivers/rtc/Makefile:45:obj-$(CONFIG_RTC_DRV_CMOS) += rtc-cmos.o
+drivers/rtc/.rtc-cmos.o.cmd:1:savedcmd_drivers/rtc/rtc-cmos.o := gcc -Wp,-MMD,drivers/rtc/.rtc-cmos.o.d -nostdinc -I./arch/x86/include -I./arch/x86/include/generated -I./include -I./include -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/compiler-version.h -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -std=gnu11 -fshort-wchar -funsigned-char -fno-common -fno-PIE -fno-strict-aliasing -mno-sse -mno-mmx -mno-sse2 -mno-3dnow -mno-avx -mno-sse4a -fcf-protection=none -m64 -falign-jumps=1 -falign-loops=1 -mno-80387 -mno-fp-ret-in-387 -mpreferred-stack-boundary=3 -mskip-rax-setup -march=x86-64 -mtune=generic -mno-red-zone -mcmodel=kernel -mstack-protector-guard-reg=gs -mstack-protector-guard-symbol=__ref_stack_chk_guard -Wno-sign-compare -fno-asynchronous-unwind-tables -mindirect-branch=thunk-extern -mindirect-branch-register -mindirect-branch-cs-prefix -mfunction-return=thunk-extern -fno-jump-tables -mharden-sls=all -fpatchable-function-entry=16,16 -fno-delete-null-pointer-checks -O2 -fno-allow-store-data-races -fstack-protector-strong -fno-omit-frame-pointer -fno-optimize-sibling-calls -ftrivial-auto-var-init=zero -fno-stack-clash-protection -fzero-call-used-regs=used-gpr -pg -mrecord-mcount -mfentry -DCC_USING_FENTRY -falign-functions=16 -fstrict-flex-arrays=3 -fms-extensions -fno-strict-overflow -fno-stack-check -fconserve-stack -fno-builtin-wcslen -Wall -Wextra -Wundef -Werror=implicit-function-declaration -Werror=implicit-int -Werror=return-type -Werror=strict-prototypes -Wno-format-security -Wno-trigraphs -Wno-frame-address -Wno-address-of-packed-member -Wmissing-declarations -Wmissing-prototypes -Wframe-larger-than=1024 -Wno-main -Wno-dangling-pointer -Wvla-larger-than=1 -Wno-pointer-sign -Wcast-function-type -Wno-array-bounds -Wno-stringop-overflow -Wno-alloc-size-larger-than -Wimplicit-fallthrough=5 -Werror=date-time -Werror=incompatible-pointer-types -Werror=designated-init -Wenum-conversion -Wunused -Wno-unused-but-set-variable -Wno-unused-const-variable -Wno-packed-not-aligned -Wno-format-overflow -Wno-format-truncation -Wno-stringop-truncation -Wno-override-init -Wno-missing-field-initializers -Wno-type-limits -Wno-shift-negative-value -Wno-maybe-uninitialized -Wno-sign-compare -Wno-unused-parameter -g -gdwarf-5 -fsanitize=bounds-strict -fsanitize=shift -fsanitize=bool -fsanitize=enum -DKBUILD_MODFILE='"drivers/rtc/rtc-cmos"' -DKBUILD_BASENAME='"rtc_cmos"' -DKBUILD_MODNAME='"rtc_cmos"' -D__KBUILD_MODNAME=rtc_cmos -c -o drivers/rtc/rtc-cmos.o drivers/rtc/rtc-cmos.c
+drivers/rtc/.rtc-cmos.o.cmd:3:source_drivers/rtc/rtc-cmos.o := drivers/rtc/rtc-cmos.c
+drivers/rtc/.rtc-cmos.o.cmd:5:deps_drivers/rtc/rtc-cmos.o := \
+drivers/rtc/.rtc-cmos.o.cmd:1372:drivers/rtc/rtc-cmos.o: $(deps_drivers/rtc/rtc-cmos.o)
+drivers/rtc/.rtc-cmos.o.cmd:1374:$(deps_drivers/rtc/rtc-cmos.o):
+drivers/rtc/rtc-cmos.c:1382: .compatible = "motorola,mc146818",
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v3] rtc: ds1307: handle oscillator stop flag for ds1337/ds1339/ds3231
From: Ronan Dalton @ 2026-06-17 3:34 UTC (permalink / raw)
To: alexandre.belloni@bootlin.com
Cc: sashal@kernel.org, code@tyhicks.com, giometti@enneenne.com,
linux-rtc@vger.kernel.org, linux-kernel@vger.kernel.org,
meaganlloyd@linux.microsoft.com, Chris Packham
In-Reply-To: <20260508032518.3696705-2-ronan.dalton@alliedtelesis.co.nz>
Hi Alexandre,
I just wanted to follow up on this patch because I think it may have
been missed by accident. I'm still interested in getting it merged.
Could you give it a look?
Thanks,
Ronan.
On Fri, 2026-05-08 at 15:24 +1200, Ronan Dalton wrote:
> Prior to commit 48458654659c ("rtc: ds1307: remove clear of
> oscillator
> stop flag (OSF) in probe"), the oscillator stop flag (OSF) bit was
> checked during device probe for the ds1337, ds1339, ds1341, and
> ds3231
> chips; if it was set, it would be cleared and a warning would be
> logged
> saying "SET TIME!". Since that commit, the OSF bit is no longer
> cleared,
> but the warning is still printed.
>
> Directly following that commit, there was no way to get rid of this
> warning because nothing cleared the OSF bit on these chips.
>
> The commit associated with the previous commit, 523923cfd5d6 ("rtc:
> ds1307: handle oscillator stop flag (OSF) for ds1341"), made proper
> use
> of the OSF when getting and setting the time in the RTC. However, the
> other RTC variants ds1337, ds1339 and ds3231 didn't have a
> corresponding
> change made.
>
> Given that the OSF bit is no longer cleared at probe time when it is
> set, the remaining three chips should have the same handling as the
> ds1341 chip has for the OSF bit.
>
> Fix the issue on the ds1337, ds1339 and ds3231 chips by applying the
> same logic as the ds1341 has to these chips.
>
> Note that any devices brought up between the first referenced commit
> and
> this one may begin mistrusting the time reported by the RTC until it
> is
> set again, if the bit was never explicitly cleared.
>
> Note that only the ds1339 was tested with this change, but the
> datasheets for the other chips contain essentially identical
> descriptions of the OSF bit so the same change should work.
>
> Signed-off-by: Ronan Dalton <ronan.dalton@alliedtelesis.co.nz>
> Cc: linux-rtc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Cc: Tyler Hicks <code@tyhicks.com>
> Cc: Sasha Levin <sashal@kernel.org>
> Cc: Meagan Lloyd <meaganlloyd@linux.microsoft.com>
> Cc: Rodolfo Giometti <giometti@enneenne.com>
> Cc: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Fixes: 48458654659c ("rtc: ds1307: remove clear of oscillator stop
> flag (OSF) in probe")
> ---
> Changes in v3:
> - Remove paragraph mentioning alternative fix from commit message
>
> Changes in v2:
> - Fix hashes of referenced commits
>
> drivers/rtc/rtc-ds1307.c | 28 +++++++++++++++++-----------
> 1 file changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
> index 7205c59ff729..edf81b975dec 100644
> --- a/drivers/rtc/rtc-ds1307.c
> +++ b/drivers/rtc/rtc-ds1307.c
> @@ -269,6 +269,16 @@ static int ds1307_get_time(struct device *dev,
> struct rtc_time *t)
> if (tmp & DS1338_BIT_OSF)
> return -EINVAL;
> break;
> + case ds_1337:
> + case ds_1339:
> + case ds_1341:
> + case ds_3231:
> + ret = regmap_read(ds1307->regmap, DS1337_REG_STATUS,
> &tmp);
> + if (ret)
> + return ret;
> + if (tmp & DS1337_BIT_OSF)
> + return -EINVAL;
> + break;
> case ds_1340:
> if (tmp & DS1340_BIT_nEOSC)
> return -EINVAL;
> @@ -279,13 +289,6 @@ static int ds1307_get_time(struct device *dev,
> struct rtc_time *t)
> if (tmp & DS1340_BIT_OSF)
> return -EINVAL;
> break;
> - case ds_1341:
> - ret = regmap_read(ds1307->regmap, DS1337_REG_STATUS,
> &tmp);
> - if (ret)
> - return ret;
> - if (tmp & DS1337_BIT_OSF)
> - return -EINVAL;
> - break;
> case ds_1388:
> ret = regmap_read(ds1307->regmap, DS1388_REG_FLAG,
> &tmp);
> if (ret)
> @@ -380,14 +383,17 @@ static int ds1307_set_time(struct device *dev,
> struct rtc_time *t)
> regmap_update_bits(ds1307->regmap,
> DS1307_REG_CONTROL,
> DS1338_BIT_OSF, 0);
> break;
> + case ds_1337:
> + case ds_1339:
> + case ds_1341:
> + case ds_3231:
> + regmap_update_bits(ds1307->regmap,
> DS1337_REG_STATUS,
> + DS1337_BIT_OSF, 0);
> + break;
> case ds_1340:
> regmap_update_bits(ds1307->regmap, DS1340_REG_FLAG,
> DS1340_BIT_OSF, 0);
> break;
> - case ds_1341:
> - regmap_update_bits(ds1307->regmap,
> DS1337_REG_STATUS,
> - DS1337_BIT_OSF, 0);
> - break;
> case ds_1388:
> regmap_update_bits(ds1307->regmap, DS1388_REG_FLAG,
> DS1388_BIT_OSF, 0);
^ permalink raw reply
* Re: [PATCH 03/12] rtc: rzn1: Fix malformed MODULE_AUTHOR string
From: Geert Uytterhoeven @ 2026-06-17 7:19 UTC (permalink / raw)
To: Prabhakar
Cc: Miquel Raynal, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, Wolfram Sang, linux-rtc, linux-renesas-soc,
devicetree, linux-kernel, Biju Das, Fabrizio Castro,
Lad Prabhakar
In-Reply-To: <20260615154805.1619693-4-prabhakar.mahadev-lad.rj@bp.renesas.com>
On Mon, 15 Jun 2026 at 17:48, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Fix a malformed MODULE_AUTHOR macro in the rtc-rzn1 driver where a missing
> closing angle bracket on the second author entry creates an invalid format.
> Correct it to the standard "Name <email>" format.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-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
* Re: [PATCH 10/12] rtc: rzn1: Consistently use dev_err_probe()
From: Geert Uytterhoeven @ 2026-06-17 7:24 UTC (permalink / raw)
To: Prabhakar
Cc: Miquel Raynal, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Magnus Damm, Wolfram Sang,
linux-rtc, linux-renesas-soc, devicetree, linux-kernel, Biju Das,
Fabrizio Castro, Lad Prabhakar
In-Reply-To: <20260615154805.1619693-11-prabhakar.mahadev-lad.rj@bp.renesas.com>
On Mon, 15 Jun 2026 at 17:48, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Use dev_err_probe() in the IRQ request error path to make error handling
> consistent with the rest of rzn1_rtc_probe().
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-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
* Re: [PATCH 07/12] rtc: rzn1: fix alarm range check truncation on 32-bit systems
From: Geert Uytterhoeven @ 2026-06-17 7:29 UTC (permalink / raw)
To: Prabhakar
Cc: Miquel Raynal, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Magnus Damm, Wolfram Sang,
linux-rtc, linux-renesas-soc, devicetree, linux-kernel, Biju Das,
Fabrizio Castro, Lad Prabhakar
In-Reply-To: <20260615154805.1619693-8-prabhakar.mahadev-lad.rj@bp.renesas.com>
On Mon, 15 Jun 2026 at 17:48, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> alarm and farest were declared as unsigned long, but
> rtc_tm_to_time64() returns time64_t (s64). On 32-bit systems where
> unsigned long is 32 bits, the assignment silently truncates the upper
> 32 bits of the timestamp.
>
> Fix by declaring alarm and farest as time64_t and replacing
> time_after() with a direct signed comparison, which is correct for
> time64_t values that will never realistically overflow.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-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
* Re: [PATCH 06/12] rtc: rzn1: Sort headers alphabetically
From: Geert Uytterhoeven @ 2026-06-17 7:22 UTC (permalink / raw)
To: Prabhakar
Cc: Miquel Raynal, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Magnus Damm, Wolfram Sang,
linux-rtc, linux-renesas-soc, devicetree, linux-kernel, Biju Das,
Fabrizio Castro, Lad Prabhakar
In-Reply-To: <20260615154805.1619693-7-prabhakar.mahadev-lad.rj@bp.renesas.com>
On Mon, 15 Jun 2026 at 17:48, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Sorting headers alphabetically helps locating duplicates, and make it
> easier to figure out where to insert new headers.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> --- a/drivers/rtc/rtc-rzn1.c
> +++ b/drivers/rtc/rtc-rzn1.c
> @@ -15,8 +15,8 @@
> #include <linux/clk.h>
> #include <linux/init.h>
> #include <linux/iopoll.h>
> -#include <linux/module.h>
> #include <linux/mod_devicetable.h>
> +#include <linux/module.h>
Sorting of special characters w.r.t. alphanumericals is always
a bit fuzzy...
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> #include <linux/rtc.h>
Reviewed-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
* Re: [PATCH v2] spi: dt-bindings: microchip,pic32mzda-spi: Convert to DT schema
From: Krzysztof Kozlowski @ 2026-06-17 7:35 UTC (permalink / raw)
To: Udaya Kiran Challa
Cc: tsbogend, robh, krzk+dt, conor+dt, skhan, me, linux-rtc,
devicetree, linux-kernel
In-Reply-To: <20260615115311.515404-1-challauday369@gmail.com>
On Mon, Jun 15, 2026 at 05:23:11PM +0530, Udaya Kiran Challa wrote:
> Convert Microchip PIC32 SPI controller devicetree binding
> from legacy text format to DT schema.
Please mention here that you dropped requirement of 'cs-gpios' because
it is not a mandatory in hardware design nor in current Linux driver...
and then CHECK it actually against drivers, which will lead you to
conclusion that maybe it is wrong decision...
>
> Signed-off-by: Udaya Kiran Challa <challauday369@gmail.com>
> ---
> Changelog:
> Changes since v1:
> - Rename schema file to microchip,pic32mzda-spi.yaml
> - Update subject prefix to match SPI DT binding conventions
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2] dt-bindings: rtc: Convert rtc-cmos binding to YAML
From: Krzysztof Kozlowski @ 2026-06-17 8:12 UTC (permalink / raw)
To: Teja Sai Charan B
Cc: Alexandre Belloni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-rtc, devicetree, linux-kernel
In-Reply-To: <20260616085659.12809-1-tejaasaye@gmail.com>
On Tue, Jun 16, 2026 at 02:26:58PM +0530, Teja Sai Charan B wrote:
> From: Teja Sai Charan Bellamkonda <tejaasaye@gmail.com>
>
> Convert the rtc-cmos devicetree bindings to dt schema.
Subject: s/YAML/DT schema/
>
> Signed-off-by: Teja Sai Charan Bellamkonda <tejaasaye@gmail.com>
>
> ---
>
> Changes in v2:
> - Allow intel,ce4100-rtc compatible used by existing DTS files
> ---
> .../devicetree/bindings/rtc/rtc-cmos.txt | 27 ---------
> .../devicetree/bindings/rtc/rtc-cmos.yaml | 60 +++++++++++++++++++
> result.txt | 17 ++++++
Stale file, please drop.
> 3 files changed, 77 insertions(+), 27 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/rtc/rtc-cmos.txt
> create mode 100644 Documentation/devicetree/bindings/rtc/rtc-cmos.yaml
> create mode 100644 result.txt
...
> diff --git a/Documentation/devicetree/bindings/rtc/rtc-cmos.yaml b/Documentation/devicetree/bindings/rtc/rtc-cmos.yaml
> new file mode 100644
> index 000000000000..ba4812778115
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/rtc-cmos.yaml
> @@ -0,0 +1,60 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/rtc/rtc-cmos.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Motorola mc146818 compatible RTC
> +
> +maintainers:
> + - Alexandre Belloni <alexandre.belloni@bootlin.com>
> +
> +properties:
> + compatible:
> + oneOf:
> + - const: motorola,mc146818
> +
> + - items:
> + - const: intel,ce4100-rtc
> + - const: motorola,mc146818
These were not in original binding, so you need to mention it in the
commit msg and explain why.
I understand there is not 'rtc-cmos' compatible, so basically the
filename should be set to this fallback compatible.
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + ctrl-reg:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Initial value of the control register
> + (also known as Register B).
> +
> + freq-reg:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Initial value of the frequency register
> + (also known as Register A).
> +
> +required:
> + - compatible
> + - reg
> +
You should $ref the rtc.yaml schema and use "unevaluatedProperties:
false". Or explain in the commit msg why it is not applicable.
> +additionalProperties: false
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2] spi: dt-bindings: microchip,pic32mzda-spi: Convert to DT schema
From: Uday Kiran @ 2026-06-17 8:28 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: tsbogend, robh, krzk+dt, conor+dt, skhan, me, linux-rtc,
devicetree, linux-kernel
In-Reply-To: <20260617-sturdy-silver-bison-bfb6ba@quoll>
> > Convert Microchip PIC32 SPI controller devicetree binding
> > from legacy text format to DT schema.
>
> Please mention here that you dropped requirement of 'cs-gpios' because
> it is not a mandatory in hardware design nor in current Linux driver...
> and then CHECK it actually against drivers, which will lead you to
> conclusion that maybe it is wrong decision...
Thank you Krzysztof
I rechecked the driver and found that it uses spi_get_csgpiod() for
chip-select handling and sets host->num_chipselect = 1. Therefore, dropping
the cs-gpios requirement during the conversion was not justified.
I'll restore cs-gpios as a required property and resend the series with an
updated changelog.
Regards,
Udaya Kiran Challa
^ permalink raw reply
* Re: [PATCH 00/12] Add RTC support for Renesas RZ/T2H and RZ/N2H SoCs
From: Wolfram Sang @ 2026-06-17 9:18 UTC (permalink / raw)
To: Prabhakar
Cc: Miquel Raynal, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, linux-rtc, linux-renesas-soc, devicetree,
linux-kernel, Biju Das, Fabrizio Castro, Lad Prabhakar
In-Reply-To: <20260615154805.1619693-1-prabhakar.mahadev-lad.rj@bp.renesas.com>
[-- Attachment #1: Type: text/plain, Size: 1323 bytes --]
Hi,
> The RTC block is closely related to the RZ/N1 implementation and can
> reuse the existing driver infrastructure when operating in SCMP mode,
> which is required on these SoCs due to their 195.3 kHz RTC input clock.
Yes, I implemented SCMP mode because the (back then) upcoming R-Car X5H
also dropped SUBU mode. And SCMP works on my RZ/N1D board as well, so I
could test it already.
> While the RZ/T2H and RZ/N2H variants do not implement the RTCA0SUBU and
> RTCA0TCR registers present on RZ/N1, those registers are not accessed by
> the driver in SCMP mode, allowing support to be added with minimal
> changes.
Note that even for RZ/N1, RTCA0TCR is marked as "not available".
> The RZ/T2H RTC variant also supports a 1 Hz output signal on the
> RTCAT1HZ pin, controlled by the RTCA0CTL1[RTCA01HZE] bit. This bit is
> marked as reserved in the RZ/N1 hardware manual, making RZ/T2H a
> distinct RTC variant despite its overall compatibility with the RZ/N1
> implementation.
R-Car X5H is the same for the above as well.
> The series consists of:
> dt-bindings updates to describe the RZ/T2H and RZ/N2H RTC variants,
> driver updates to recognize the new compatible string and enable
> support for these SoCs.
I will review and test in on my N1D-board today.
Thanks for your work and happy hacking,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 01/12] dt-bindings: rtc: renesas,rzn1-rtc: Add RZ/T2H and RZ/N2H support
From: Wolfram Sang @ 2026-06-17 9:38 UTC (permalink / raw)
To: Prabhakar
Cc: Miquel Raynal, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, linux-rtc, linux-renesas-soc, devicetree,
linux-kernel, Biju Das, Fabrizio Castro, Lad Prabhakar
In-Reply-To: <20260615154805.1619693-2-prabhakar.mahadev-lad.rj@bp.renesas.com>
[-- Attachment #1: Type: text/plain, Size: 1172 bytes --]
On Mon, Jun 15, 2026 at 04:47:54PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Add compatible strings for the RTC block found on the Renesas RZ/T2H
> (R9A09G077) and RZ/N2H (R9A09G087) SoCs.
>
> These SoCs integrate a closely related variant of the RZ/N1 RTC IP.
> Unlike RZ/N1, they do not implement the RTCA0SUBU and RTCA0TCR
> registers. This is not a limitation for Linux support, as these
> registers are not used when the RTC operates in "scmp" clock mode, which
> is required on RZ/T2H and RZ/N2H due to their 195.3 kHz input clock.
>
> The RZ/T2H RTC variant also supports a 1Hz output signal on the
> RTCAT1HZ pin, controlled by the RTCA0CTL1[RTCA01HZE] bit. This bit is
> marked as reserved in the RZ/N1 hardware manual.
>
> Update the binding schema to require the additional clock inputs used by
> these SoCs.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Sashiko is wrong here because
a) TCR is the "Test Register"
b) TCR is not even present on RZ/N1D. Cover-letter misses that, too.
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 02/12] rtc: rzn1: Handle EPROBE_DEFER for optional pps interrupt
From: Wolfram Sang @ 2026-06-17 9:55 UTC (permalink / raw)
To: Prabhakar
Cc: Miquel Raynal, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, linux-rtc, linux-renesas-soc, devicetree,
linux-kernel, Biju Das, Fabrizio Castro, Lad Prabhakar, stable
In-Reply-To: <20260615154805.1619693-3-prabhakar.mahadev-lad.rj@bp.renesas.com>
[-- Attachment #1: Type: text/plain, Size: 3263 bytes --]
Hi,
Sashiko found issues with your patch. I curated the list and agree. I
think, in probe(), 'rate' should be initialized to 32768 and 'sync_time'
should be calculated unconditionally.
Thank you and happy hacking,
Wolfram
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] If the optional 'xtal' clock is not present, `rtc->sync_time` remains 0, causing `readl_poll_timeout` to use a 0 timeout (infinite wait) and `usleep_range` to use a 0 delay.
> --
>
> commit 5d75a1049c228d2049afaf29833f1b2f212837d4
> Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Subject: rtc: rzn1: Handle EPROBE_DEFER for optional pps interrupt
>
> This commit checks for -EPROBE_DEFER from platform_get_irq_byname_optional()
> and handles the deferred probe request properly. It intercepts this specific
> error condition, assigns it to the return value, and jumps to the
> dis_runtime_pm label to avoid ignoring a valid probe deferral.
>
> Fixes: eea7791e00f33 ("rtc: rzn1: implement one-second accuracy for alarms")
> Cc: stable@vger.kernel.org
>
> > diff --git a/drivers/rtc/rtc-rzn1.c b/drivers/rtc/rtc-rzn1.c
> > index c4ed437354574..f81d691c8b9a2 100644
> > --- a/drivers/rtc/rtc-rzn1.c
> > +++ b/drivers/rtc/rtc-rzn1.c
> > @@ -465,6 +465,10 @@ static int rzn1_rtc_probe(struct platform_device *pdev)
> > }
> >
> > irq = platform_get_irq_byname_optional(pdev, "pps");
> > + if (irq == -EPROBE_DEFER) {
> > + ret = irq;
> > + goto dis_runtime_pm;
> > + }
> > if (irq >= 0)
> > ret = devm_request_irq(&pdev->dev, irq, rzn1_rtc_1s_irq, 0, "RZN1 RTC 1s", rtc);
>
> [Severity: High]
> If the optional xtal clock is not present, does rtc->sync_time remain 0,
> causing readl_poll_timeout() to use a 0 timeout (infinite wait) and
> usleep_range() to use a 0 delay?
>
> devm_clk_get_optional() returns NULL if the clock is absent in the device
> tree, skipping the calculation:
>
> drivers/rtc/rtc-rzn1.c:rzn1_rtc_probe() {
> ...
> xtal = devm_clk_get_optional(dev, "xtal");
> if (IS_ERR(xtal)) {
> ...
> } else if (xtal) {
> rate = clk_get_rate(xtal);
> ...
> rtc->sync_time = DIV_ROUND_UP(2 * NSEC_PER_MSEC, rate);
> }
> ...
> ret = readl_poll_timeout(rtc->base + RZN1_RTC_CTL0, val,
> !(val & RZN1_RTC_CTL0_CEST), rtc->sync_time,
> rtc->sync_time * 2);
> ...
> }
>
> In include/linux/iopoll.h, a timeout_us of 0 skips the timeout check
> entirely, which could lead to a tight infinite loop and a kernel hard lockup
> if the hardware fails to clear the CEST bit.
>
> Additionally, rtc->sync_time being 0 causes usleep_range(0, 100) in
> rzn1_rtc_set_time(), which may return almost immediately and violate the
> hardware's explicit 2-clock-cycle delay requirement (previously hardcoded to
> 61us).
>
> This regression was introduced by the commit 'rtc: rzn1: Dynamically
> calculate synchronization delay based on clock rate' later in this series.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260615154805.1619693-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=2
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 03/12] rtc: rzn1: Fix malformed MODULE_AUTHOR string
From: Wolfram Sang @ 2026-06-17 9:55 UTC (permalink / raw)
To: Prabhakar
Cc: Miquel Raynal, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, linux-rtc, linux-renesas-soc, devicetree,
linux-kernel, Biju Das, Fabrizio Castro, Lad Prabhakar
In-Reply-To: <20260615154805.1619693-4-prabhakar.mahadev-lad.rj@bp.renesas.com>
[-- Attachment #1: Type: text/plain, Size: 483 bytes --]
On Mon, Jun 15, 2026 at 04:47:56PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Fix a malformed MODULE_AUTHOR macro in the rtc-rzn1 driver where a missing
> closing angle bracket on the second author entry creates an invalid format.
> Correct it to the standard "Name <email>" format.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 04/12] rtc: Kconfig: Broaden RTC_DRV_RZN1 dependency to ARCH_RENESAS
From: Wolfram Sang @ 2026-06-17 9:57 UTC (permalink / raw)
To: Prabhakar
Cc: Miquel Raynal, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, linux-rtc, linux-renesas-soc, devicetree,
linux-kernel, Biju Das, Fabrizio Castro, Lad Prabhakar
In-Reply-To: <20260615154805.1619693-5-prabhakar.mahadev-lad.rj@bp.renesas.com>
[-- Attachment #1: Type: text/plain, Size: 388 bytes --]
> - depends on ARCH_RZN1 || COMPILE_TEST
> + depends on ARCH_RENESAS || COMPILE_TEST
Yes, this helps X5H also :)
> - If you say yes here you get support for the Renesas RZ/N1 RTC.
> + If you say yes here you get support for the RTC found on Renesas RZ/N1,
> + RZ/N2H, and RZ/T2H SoCs.
Such lists are easy to get stale IMHO. What about "initially found on
Renesas RZ/N1 SoCs."?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 05/12] rtc: rzn1: Add system suspend/resume support and wakeup capability
From: Wolfram Sang @ 2026-06-17 10:02 UTC (permalink / raw)
To: Prabhakar
Cc: Miquel Raynal, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, linux-rtc, linux-renesas-soc, devicetree,
linux-kernel, Biju Das, Fabrizio Castro, Lad Prabhakar
In-Reply-To: <20260615154805.1619693-6-prabhakar.mahadev-lad.rj@bp.renesas.com>
[-- Attachment #1: Type: text/plain, Size: 335 bytes --]
> Add system-wide power management support along with wakeup capability to
> the rtc-rzn1 driver.
Do you have an actual use case for the wakeup functionality? If it is so
limited, then we should maybe not support the weak abilities until
someone has a real use case? For which then, a proper solution has been
developed and tested?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 06/12] rtc: rzn1: Sort headers alphabetically
From: Wolfram Sang @ 2026-06-17 10:04 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Prabhakar, Miquel Raynal, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Magnus Damm, linux-rtc,
linux-renesas-soc, devicetree, linux-kernel, Biju Das,
Fabrizio Castro, Lad Prabhakar
In-Reply-To: <CAMuHMdXg16frnn88_P_jHRH+HPy00wWfoqNKdOv8teSWNpMEGg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 681 bytes --]
> > --- a/drivers/rtc/rtc-rzn1.c
> > +++ b/drivers/rtc/rtc-rzn1.c
> > @@ -15,8 +15,8 @@
> > #include <linux/clk.h>
> > #include <linux/init.h>
> > #include <linux/iopoll.h>
> > -#include <linux/module.h>
> > #include <linux/mod_devicetable.h>
> > +#include <linux/module.h>
>
> Sorting of special characters w.r.t. alphanumericals is always
> a bit fuzzy...
I rely on the 'sort' utility which gives the same output, so:
> > #include <linux/platform_device.h>
> > #include <linux/pm_runtime.h>
> > #include <linux/rtc.h>
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v3] spi: dt-bindings: microchip,pic32mzda-spi: Convert to DT schema
From: Udaya Kiran Challa @ 2026-06-17 10:10 UTC (permalink / raw)
To: tsbogend, robh, krzk+dt, conor+dt
Cc: skhan, me, linux-rtc, devicetree, linux-kernel,
Udaya Kiran Challa
Convert Microchip PIC32 SPI controller devicetree binding
from legacy text format to DT schema.
Signed-off-by: Udaya Kiran Challa <challauday369@gmail.com>
---
Changelog:
Changes since v2:
- Add cs-gpios to required property
Link to v2: https://lore.kernel.org/all/20260615115311.515404-1-challauday369@gmail.com/
Changes since v1:
- Rename schema file to microchip,pic32mzda-spi.yaml
- Update subject prefix to match SPI DT binding conventions
Link to v1:https://lore.kernel.org/all/20260614175005.435826-1-challauday369@gmail.com/
---
.../bindings/spi/microchip,pic32mzda-spi.yaml | 82 +++++++++++++++++++
.../bindings/spi/microchip,spi-pic32.txt | 34 --------
2 files changed, 82 insertions(+), 34 deletions(-)
create mode 100644 Documentation/devicetree/bindings/spi/microchip,pic32mzda-spi.yaml
delete mode 100644 Documentation/devicetree/bindings/spi/microchip,spi-pic32.txt
diff --git a/Documentation/devicetree/bindings/spi/microchip,pic32mzda-spi.yaml b/Documentation/devicetree/bindings/spi/microchip,pic32mzda-spi.yaml
new file mode 100644
index 000000000000..4669b521fa1f
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/microchip,pic32mzda-spi.yaml
@@ -0,0 +1,82 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/microchip,pic32mzda-spi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip PIC32MZDA SPI Controller
+
+maintainers:
+ - Thomas Bogendoerfer <tsbogend@alpha.franken.de>
+
+allOf:
+ - $ref: spi-controller.yaml#
+
+properties:
+ compatible:
+ const: microchip,pic32mzda-spi
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ items:
+ - description: Fault interrupt
+ - description: Receive interrupt
+ - description: Transmit interrupt
+
+ interrupt-names:
+ items:
+ - const: fault
+ - const: rx
+ - const: tx
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ items:
+ - const: mck0
+
+ cs-gpios:
+ maxItems: 1
+
+ dmas:
+ items:
+ - description: RX DMA channel
+ - description: TX DMA channel
+
+ dma-names:
+ items:
+ - const: spi-rx
+ - const: spi-tx
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - interrupt-names
+ - clocks
+ - clock-names
+ - cs-gpios
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/gpio/gpio.h>
+
+ spi@1f821000 {
+ compatible = "microchip,pic32mzda-spi";
+ reg = <0x1f821000 0x200>;
+ interrupts = <109 IRQ_TYPE_LEVEL_HIGH>,
+ <110 IRQ_TYPE_LEVEL_HIGH>,
+ <111 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "fault", "rx", "tx";
+ clocks = <&PBCLK2>;
+ clock-names = "mck0";
+ cs-gpios = <&gpio3 4 GPIO_ACTIVE_LOW>;
+ dmas = <&dma 134>, <&dma 135>;
+ dma-names = "spi-rx", "spi-tx";
+ };
diff --git a/Documentation/devicetree/bindings/spi/microchip,spi-pic32.txt b/Documentation/devicetree/bindings/spi/microchip,spi-pic32.txt
deleted file mode 100644
index 79de379f4dc0..000000000000
--- a/Documentation/devicetree/bindings/spi/microchip,spi-pic32.txt
+++ /dev/null
@@ -1,34 +0,0 @@
-Microchip PIC32 SPI Master controller
-
-Required properties:
-- compatible: Should be "microchip,pic32mzda-spi".
-- reg: Address and length of register space for the device.
-- interrupts: Should contain all three spi interrupts in sequence
- of <fault-irq>, <receive-irq>, <transmit-irq>.
-- interrupt-names: Should be "fault", "rx", "tx" in order.
-- clocks: Phandle of the clock generating SPI clock on the bus.
-- clock-names: Should be "mck0".
-- cs-gpios: Specifies the gpio pins to be used for chipselects.
- See: Documentation/devicetree/bindings/spi/spi-bus.txt
-
-Optional properties:
-- dmas: Two or more DMA channel specifiers following the convention outlined
- in Documentation/devicetree/bindings/dma/dma.txt
-- dma-names: Names for the dma channels. There must be at least one channel
- named "spi-tx" for transmit and named "spi-rx" for receive.
-
-Example:
-
-spi1: spi@1f821000 {
- compatible = "microchip,pic32mzda-spi";
- reg = <0x1f821000 0x200>;
- interrupts = <109 IRQ_TYPE_LEVEL_HIGH>,
- <110 IRQ_TYPE_LEVEL_HIGH>,
- <111 IRQ_TYPE_LEVEL_HIGH>;
- interrupt-names = "fault", "rx", "tx";
- clocks = <&PBCLK2>;
- clock-names = "mck0";
- cs-gpios = <&gpio3 4 GPIO_ACTIVE_LOW>;
- dmas = <&dma 134>, <&dma 135>;
- dma-names = "spi-rx", "spi-tx";
-};
--
2.34.1
^ permalink raw reply related
* Re: [PATCH 07/12] rtc: rzn1: fix alarm range check truncation on 32-bit systems
From: Wolfram Sang @ 2026-06-17 10:49 UTC (permalink / raw)
To: Prabhakar
Cc: Miquel Raynal, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, linux-rtc, linux-renesas-soc, devicetree,
linux-kernel, Biju Das, Fabrizio Castro, Lad Prabhakar
In-Reply-To: <20260615154805.1619693-8-prabhakar.mahadev-lad.rj@bp.renesas.com>
[-- Attachment #1: Type: text/plain, Size: 862 bytes --]
On Mon, Jun 15, 2026 at 04:48:00PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> alarm and farest were declared as unsigned long, but
> rtc_tm_to_time64() returns time64_t (s64). On 32-bit systems where
> unsigned long is 32 bits, the assignment silently truncates the upper
> 32 bits of the timestamp.
>
> Fix by declaring alarm and farest as time64_t and replacing
> time_after() with a direct signed comparison, which is correct for
> time64_t values that will never realistically overflow.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
I need to postpone testing this to the evening. Setting alarm behaves
strange here for alarms beyond the one-week-ahead-limit. No error, but
bogus values. Seems to be irrelevant to your patch, though. Does it work
for you?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 07/12] rtc: rzn1: fix alarm range check truncation on 32-bit systems
From: Wolfram Sang @ 2026-06-17 10:57 UTC (permalink / raw)
To: Prabhakar
Cc: Miquel Raynal, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, linux-rtc, linux-renesas-soc, devicetree,
linux-kernel, Biju Das, Fabrizio Castro, Lad Prabhakar
In-Reply-To: <20260615154805.1619693-8-prabhakar.mahadev-lad.rj@bp.renesas.com>
[-- Attachment #1: Type: text/plain, Size: 4683 bytes --]
Hi,
Sashiko found issues, not introduced by your patch, but still.. I
curated the list and left only those which I think are valid. Do you
have time to look at these issues?
Thank you and happy hacking,
Wolfram
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Incorrect hardware alarm weekday assignment due to unsigned underflow when the alarm crosses a month boundary.
> - [High] Unconfigured alarms in `rzn1_rtc_read_alarm` can return an invalid `tm_wday` value of `-1`, risking out-of-bounds array access in userspace.
> - [High] Potential spurious interrupt when sequentially updating active hardware alarm registers.
> --
>
> commit d278abd6309a65fff235820bea9a8e8631da47be
> Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Subject: rtc: rzn1: fix alarm range check truncation on 32-bit systems
>
> This commit fixes a silent truncation issue on 32-bit systems by changing
> the alarm and farest variables from unsigned long to time64_t. It also
> replaces the time_after() macro with a direct signed comparison.
>
> > diff --git a/drivers/rtc/rtc-rzn1.c b/drivers/rtc/rtc-rzn1.c
> > index fe21fa6242881..06339adae71f6 100644
> > --- a/drivers/rtc/rtc-rzn1.c
> > +++ b/drivers/rtc/rtc-rzn1.c
> [ ... ]
> > @@ -265,8 +266,8 @@ 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;
> > + time64_t alarm, farest;
> > int ret;
> >
> > ret = rzn1_rtc_read_time(dev, &tm_now);
> > @@ -276,7 +277,7 @@ static int rzn1_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
> > /* We cannot set alarms more than one week ahead */
> > farest = rtc_tm_to_time64(&tm_now) + rtc->rtcdev->alarm_offset_max;
> > alarm = rtc_tm_to_time64(tm);
> > - if (time_after(alarm, farest))
> > + if (alarm > farest)
> > return -ERANGE;
> >
> > /* Convert alarm day into week day */
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does the calculation for
> days_ahead underflow if the alarm crosses a month boundary?
>
> In rzn1_rtc_set_alarm(), days_ahead is typed as an unsigned int and is
> calculated like this:
>
> days_ahead = tm->tm_mday - tm_now.tm_mday;
> wday = (tm_now.tm_wday + days_ahead) % 7;
>
> If the alarm is set for the 1st of the next month, and today is the 31st,
> the subtraction results in -30. When assigned to an unsigned int, this
> underflows. Because 2^32 % 7 evaluates to 4, the resulting wday calculation
> shifts the weekday by exactly 4 days, causing the hardware alarm register
> to be programmed for the wrong day.
>
> [Severity: High]
> This is a pre-existing issue, but is there a risk of a spurious interrupt
> when updating the active hardware alarm registers?
>
> Just below this hunk in rzn1_rtc_set_alarm(), the match registers are
> updated sequentially:
>
> 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);
>
> The RTC core can invoke this function without first disabling the hardware
> alarm if replacing an existing timer. If the hardware alarm interrupt is
> active, modifying these registers sequentially could cause the intermediate
> hardware state to momentarily match the current time, triggering a spurious
> interrupt and prematurely expiring the next timer in the queue.
>
> [Severity: High]
> This is a pre-existing issue, but what happens in rzn1_rtc_read_alarm() if
> the hardware alarm is unconfigured?
>
> If the hardware alarm register reads 0, fls(0) - 1 evaluates to -1:
>
> drivers/rtc/rtc-rzn1.c:rzn1_rtc_read_alarm() {
> ...
> wday = readl(rtc->base + RZN1_RTC_ALW);
> ...
> delta_days = ((fls(wday) - 1) - tm->tm_wday + 7) % 7;
> tm->tm_wday = fls(wday) - 1;
> ...
> }
>
> If the current weekday is Saturday (6), delta_days evaluates to 0:
> ((-1 - 6 + 7) % 7) == 0.
>
> Because delta_days is 0, the rtc_time64_to_tm() block is skipped, and the
> function returns with tm->tm_wday = -1. Since rtc_valid_tm() does not
> bound-check tm_wday, this -1 is passed to userspace where tools commonly
> use it as an array index, which could cause an out-of-bounds memory read.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260615154805.1619693-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=7
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 08/12] rtc: rzn1: Dynamically calculate synchronization delay based on clock rate
From: Wolfram Sang @ 2026-06-17 10:58 UTC (permalink / raw)
To: Prabhakar
Cc: Miquel Raynal, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, linux-rtc, linux-renesas-soc, devicetree,
linux-kernel, Biju Das, Fabrizio Castro, Lad Prabhakar
In-Reply-To: <20260615154805.1619693-9-prabhakar.mahadev-lad.rj@bp.renesas.com>
[-- Attachment #1: Type: text/plain, Size: 737 bytes --]
As mentioned in another thread:
> drivers/rtc/rtc-rzn1.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/rtc/rtc-rzn1.c b/drivers/rtc/rtc-rzn1.c
> index 06339adae71f..bc6af59744e4 100644
> --- a/drivers/rtc/rtc-rzn1.c
> +++ b/drivers/rtc/rtc-rzn1.c
> @@ -71,6 +71,7 @@ struct rzn1_rtc {
> */
> spinlock_t ctl1_access_lock;
> struct rtc_time tm_alarm;
> + unsigned long sync_time;
> int alarm_irq;
> int sec_irq;
> bool alarm_enabled;
rate = 32768 here...
> + rtc->sync_time = DIV_ROUND_UP(2 * NSEC_PER_MSEC, rate);
> +
> }
... and move this to the main body of the function.
Then, we should have all values always initialized.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 09/12] rtc: rzn1: Use temporary variable for struct device
From: Wolfram Sang @ 2026-06-17 11:00 UTC (permalink / raw)
To: Prabhakar
Cc: Miquel Raynal, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, linux-rtc, linux-renesas-soc, devicetree,
linux-kernel, Biju Das, Fabrizio Castro, Lad Prabhakar
In-Reply-To: <20260615154805.1619693-10-prabhakar.mahadev-lad.rj@bp.renesas.com>
[-- Attachment #1: Type: text/plain, Size: 401 bytes --]
On Mon, Jun 15, 2026 at 04:48:02PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Use a temporary variable for the struct device pointers to avoid
> dereferencing.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Some churn, but still okay in my book:
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 10/12] rtc: rzn1: Consistently use dev_err_probe()
From: Wolfram Sang @ 2026-06-17 11:01 UTC (permalink / raw)
To: Prabhakar
Cc: Miquel Raynal, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, linux-rtc, linux-renesas-soc, devicetree,
linux-kernel, Biju Das, Fabrizio Castro, Lad Prabhakar
In-Reply-To: <20260615154805.1619693-11-prabhakar.mahadev-lad.rj@bp.renesas.com>
[-- Attachment #1: Type: text/plain, Size: 398 bytes --]
On Mon, Jun 15, 2026 at 04:48:03PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Use dev_err_probe() in the IRQ request error path to make error handling
> consistent with the rest of rzn1_rtc_probe().
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 11/12] rtc: rzn1: use FIELD_PREP/FIELD_GET and GENMASK for register access
From: Wolfram Sang @ 2026-06-17 11:06 UTC (permalink / raw)
To: Prabhakar
Cc: Miquel Raynal, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, linux-rtc, linux-renesas-soc, devicetree,
linux-kernel, Biju Das, Fabrizio Castro, Lad Prabhakar
In-Reply-To: <20260615154805.1619693-12-prabhakar.mahadev-lad.rj@bp.renesas.com>
[-- Attachment #1: Type: text/plain, Size: 2230 bytes --]
Hi,
Sashiko found issues with your patch. I curated the list and left only those
which I see as valid. Please have a look at these issues.
Thank you and happy hacking,
Wolfram
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Incomplete refactoring: open-coded logic remains for RZN1_RTC_SUBU_RTCA0FX
> --
>
> commit 2e16f66c80c483d619655d20fbb859a42e9d1b6d
> Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> rtc: rzn1: use FIELD_PREP/FIELD_GET and GENMASK for register access
>
> This commit replaces open-coded shift and mask operations with the bitfield
> API in the rzn1 RTC driver to improve readability and match the hardware
> manual.
>
> > diff --git a/drivers/rtc/rtc-rzn1.c b/drivers/rtc/rtc-rzn1.c
> > index c7ef3c81180f4..9f9cf9882fc49 100644
> > --- a/drivers/rtc/rtc-rzn1.c
> > +++ b/drivers/rtc/rtc-rzn1.c
> [ ... ]
> > @@ -306,7 +312,7 @@ static int rzn1_rtc_read_offset(struct device *dev, long *offset)
> > 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;
> > + val = FIELD_GET(RZN1_RTC_SUBU_RTCA0FX, val);
> >
> > if (!val)
> > *offset = 0;
>
> [Severity: Low]
> Since the stated goal of this patch is to replace open-coded masks with the
> bitfield API, should the remaining 0x3F masks for this register be updated as
> well?
>
> In rzn1_rtc_read_offset() just below this change, there is still an explicit
> mask used when calculating the offset:
>
> *offset = -(((~val) & 0x3F) + 1) * ppb_per_step;
>
> Additionally, it looks like the write path in rzn1_rtc_set_offset() was missed
> entirely during the refactoring. It continues to use open-coded logic rather
> than FIELD_PREP when constructing the RZN1_RTC_SUBU register:
>
> subu |= RZN1_RTC_SUBU_DECR;
> subu |= (~(-steps - 1)) & 0x3F;
>
> Would it make sense to update these locations so the bitfield access is
> consistent across the driver?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260615154805.1619693-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=11
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 09/12] rtc: rzn1: Use temporary variable for struct device
From: Wolfram Sang @ 2026-06-17 11:08 UTC (permalink / raw)
To: Prabhakar
Cc: Miquel Raynal, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, linux-rtc, linux-renesas-soc, devicetree,
linux-kernel, Biju Das, Fabrizio Castro, Lad Prabhakar
In-Reply-To: <20260615154805.1619693-10-prabhakar.mahadev-lad.rj@bp.renesas.com>
[-- Attachment #1: Type: text/plain, Size: 3370 bytes --]
Hi,
Sashiko found issues, not introduced by your patch, but still. I
curated the list and left only those which I think are valid. Do you
have time to look at these issues?
Thank you and happy hacking,
Wolfram
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Asynchronous pm_runtime_put() is used instead of pm_runtime_put_sync() before runtime PM is disabled, potentially leaving the device powered on.
> - [High] If the optional 'xtal' clock is missing, readl_poll_timeout() loops forever with a 0 timeout, risking a boot hang.
> - [High] The driver's `remove` callback prematurely disables hardware interrupts and drops the runtime PM reference while the RTC device is still fully registered.
> --
>
> commit b7f73c7d0932a6e78b12419c5912ad3243637b8f
> Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> rtc: rzn1: Use temporary variable for struct device
>
> This patch simplifies the rzn1_rtc_probe function by using a local temporary
> variable for the struct device pointer. This avoids repeated dereferences of
> pdev->dev throughout the function.
>
> > diff --git a/drivers/rtc/rtc-rzn1.c b/drivers/rtc/rtc-rzn1.c
> > index bc6af59744e48..25cad9213147d 100644
> > --- a/drivers/rtc/rtc-rzn1.c
> > +++ b/drivers/rtc/rtc-rzn1.c
> [ ... ]
> > if (IS_ERR(xtal)) {
> > ret = PTR_ERR(xtal);
> > goto dis_runtime_pm;
> [ ... ]
> > @@ -486,18 +487,18 @@ static int rzn1_rtc_probe(struct platform_device *pdev)
> [ ... ]
> > device_init_wakeup(dev, true);
> >
> > ret = devm_rtc_register_device(rtc->rtcdev);
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does registering the RTC
> device using devm_rtc_register_device() create a race condition during
> teardown?
>
> In rzn1_rtc_remove(), the driver manually disables interrupts and calls
> pm_runtime_put() before devres has unregistered the RTC device:
>
> writel(0, rtc->base + RZN1_RTC_CTL1);
> pm_runtime_put(&pdev->dev);
>
> Could userspace still interact with the RTC during this window via ioctls
> or sysfs, potentially causing synchronous bus faults since the hardware is
> already suspended? Should teardown be bound using
> devm_add_action_or_reset() to guarantee correct reverse execution order?
[wsa] Or maybe not use devm then?
>
> > if (ret)
> > @@ -506,7 +507,7 @@ static int rzn1_rtc_probe(struct platform_device *pdev)
> > return 0;
> >
> > dis_runtime_pm:
> > - pm_runtime_put(&pdev->dev);
> > + pm_runtime_put(dev);
>
> [Severity: High]
> This is a pre-existing issue, but does using pm_runtime_put() here (and in
> rzn1_rtc_remove) leave the device powered on indefinitely?
>
> Since devm_pm_runtime_enable(dev) schedules pm_runtime_disable() to
> execute during devres cleanup, calling pm_runtime_put() only queues an
> asynchronous idle check. The immediate return triggers devres cleanup,
> which executes a barrier that explicitly cancels pending async operations.
>
> Should this use pm_runtime_put_sync() instead to ensure the device is
> synchronously suspended before teardown?
>
> > return ret;
> > }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260615154805.1619693-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=9
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox