* Re: [PATCH 07/12] rtc: rzn1: fix alarm range check truncation on 32-bit systems
From: Lad, Prabhakar @ 2026-06-18 10:49 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Miquel Raynal, 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: <20260617165538dad7e36b@mail.local>
Hi Alexandre,
On Wed, Jun 17, 2026 at 5:55 PM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> On 15/06/2026 16:48:00+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.
> >
>
> I'd argue that this is never going to overflow ever as unsigned long
> gets you to 2106 which is way past the usable range of the RTC so there
> is a trade off between the size you are going to take on the stack and
> the actual usefulness of the fix.
>
While it's true that unsigned long lasts until 2106 (well past this
RTC's practical lifetime), rtc_tm_to_time64() explicitly returns
time64_t. Using unsigned long causes silent truncation and types
mismatch with the API, which modern static analyzers flag. Given that
this function is not deeply nested, the 8-byte stack trade-off seems
worth it for type cleanliness and consistency. What do you think?
Cheers,
Prabhakar
^ permalink raw reply
* Re: [PATCH 07/12] rtc: rzn1: fix alarm range check truncation on 32-bit systems
From: Lad, Prabhakar @ 2026-06-18 10:38 UTC (permalink / raw)
To: Wolfram Sang
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: <ajJ9kNJtrY6DyQ-S@shikoro>
Hi Wolfram,
Thank you for testing.
On Wed, Jun 17, 2026 at 11:57 AM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> 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?
>
Can you please share the commands you tried, I'll try and replicate it
on my side.
Cheers,
Prabhakar
> 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
^ permalink raw reply
* Re: [PATCH 05/12] rtc: rzn1: Add system suspend/resume support and wakeup capability
From: Lad, Prabhakar @ 2026-06-18 10:37 UTC (permalink / raw)
To: Wolfram Sang
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: <ajPJHKut92mAoo-B@shikoro>
Hi Wolfram,
On Thu, Jun 18, 2026 at 11:31 AM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
>
> > For running s2idle cases with rtcwake > 60sec this feature would be
> > helpful. What do you think?
>
> I think maintaining such a fragile feature is cumbersome. People might
> have different expectations and the maintainers have to handle the delta
> then. So, if we cannot to support to a large degree some feature, I
> think we should just skip it. Until some user really wants (and tests
> and accepts) a half-baked solution.
>
Ok, I will drop this patch from the series in v2.
Cheers,
Prabhakar
^ permalink raw reply
* Re: [PATCH 05/12] rtc: rzn1: Add system suspend/resume support and wakeup capability
From: Wolfram Sang @ 2026-06-18 10:31 UTC (permalink / raw)
To: Lad, 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: <CA+V-a8uo9sr3m9F_MQYbHVD5wa3LT3n6MWrVpiNiPDumnVHMYQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 421 bytes --]
> For running s2idle cases with rtcwake > 60sec this feature would be
> helpful. What do you think?
I think maintaining such a fragile feature is cumbersome. People might
have different expectations and the maintainers have to handle the delta
then. So, if we cannot to support to a large degree some feature, I
think we should just skip it. Until some user really wants (and tests
and accepts) a half-baked solution.
[-- 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: Lad, Prabhakar @ 2026-06-18 10:24 UTC (permalink / raw)
To: Wolfram Sang
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: <ajJwqDt2jUfhSD1x@shikoro>
Hi Wolfram,
On Wed, Jun 17, 2026 at 11:02 AM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
>
> > 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?
>
For running s2idle cases with rtcwake > 60sec this feature would be
helpful. What do you think?
Cheers,
Prabhakar
^ permalink raw reply
* Re: [PATCH 04/12] rtc: Kconfig: Broaden RTC_DRV_RZN1 dependency to ARCH_RENESAS
From: Lad, Prabhakar @ 2026-06-18 10:17 UTC (permalink / raw)
To: Wolfram Sang
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: <ajJvn2YkaspTYx9M@shikoro>
Hi Wolfram,
Thank you for the review.
On Wed, Jun 17, 2026 at 10:57 AM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
>
> > - 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."?
>
Ok, I will update it as above.
Cheers,
Prabhakar
^ permalink raw reply
* Re: [PATCH 07/12] rtc: rzn1: fix alarm range check truncation on 32-bit systems
From: Alexandre Belloni @ 2026-06-17 16:55 UTC (permalink / raw)
To: Prabhakar
Cc: Miquel Raynal, 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-8-prabhakar.mahadev-lad.rj@bp.renesas.com>
On 15/06/2026 16:48:00+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.
>
I'd argue that this is never going to overflow ever as unsigned long
gets you to 2106 which is way past the usable range of the RTC so there
is a trade off between the size you are going to take on the stack and
the actual usefulness of the fix.
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH 00/12] Add RTC support for Renesas RZ/T2H and RZ/N2H SoCs
From: Wolfram Sang @ 2026-06-17 11:12 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: <ajJmacl9ZJtkoLyf@shikoro>
[-- Attachment #1: Type: text/plain, Size: 190 bytes --]
> I will review and test in on my N1D-board today.
Except for the strange alarm-boundary behaviour, the tests went well.
Will do further tests, it is probably not related to this series.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 12/12] rtc: rzn1: Add support for Renesas RZ/T2H and RZ/N2H SoCs
From: Wolfram Sang @ 2026-06-17 11:10 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-13-prabhakar.mahadev-lad.rj@bp.renesas.com>
[-- Attachment #1: Type: text/plain, Size: 1099 bytes --]
On Mon, Jun 15, 2026 at 04:48:05PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Add a new compatible string "renesas,r9a09g077-rtc" to the OF match table
> to support the RTC IP variant found on the RZ/T2H and RZ/N2H SoCs.
>
> These newer SoCs integrate a closely related variant of the RZ/N1 RTC IP.
> The RZ/T2H and RZ/N2H variants lack the RTCA0SUBU and RTCA0TCR registers,
The RTCA0TCR is also not on RZ/N1.
> those registers are not accessed or used when operating under the
> rzn1_rtc_ops_scmp configurations, making the current infrastructure
> compatible.
>
> 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.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
With that fixed:
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 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
* 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 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 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 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 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 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
* [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 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
* 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 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 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 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 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 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 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
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