Linux RTC
 help / color / mirror / Atom feed
* Re: [PATCH v3 03/12] rtc: rzn1: fix weekday underflow when alarm crosses month boundary
From: sashiko-bot @ 2026-07-06 18:00 UTC (permalink / raw)
  To: Prabhakar
  Cc: Alexandre Belloni, conor+dt, devicetree, wsa+renesas, linux-rtc,
	robh
In-Reply-To: <20260706175138.12587-4-prabhakar.mahadev-lad.rj@bp.renesas.com>

> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> In rzn1_rtc_set_alarm(), the driver attempts to calculate the weekday
> for an alarm by computing the day delta between the alarm time and the
> current time:
> 
>     days_ahead = tm->tm_mday - tm_now.tm_mday;
>     wday = (tm_now.tm_wday + days_ahead) % 7;
> 
> However, if an alarm is scheduled for the beginning of the next month
> while the current time is at the end of the month (e.g., current day is
> 31, alarm day is 1), `tm->tm_mday - tm_now.tm_mday` results in a negative
> value (-30). Since `days_ahead` is an unsigned int, this underflows to a
> large positive number, leading to an incorrect `wday` being written to
> the RZN1_RTC_ALW register. As a result, the alarm fails to fire.
> 
> Fix this by utilizing the already computed `alarm` time64_t timestamp.
> Convert it back into an rtc_time struct via rtc_time64_to_tm(), which
> automatically handles month boundaries and correctly populates the
> `tm_wday` field.
> 
> Fixes: b5ad1bf00d2c4 ("rtc: rzn1: Add alarm support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706175138.12587-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=3


^ permalink raw reply

* Re: [PATCH v3 12/12] rtc: rzn1: Add support for Renesas RZ/T2H and RZ/N2H SoCs
From: sashiko-bot @ 2026-07-06 18:03 UTC (permalink / raw)
  To: Prabhakar
  Cc: linux-rtc, wsa+renesas, robh, Alexandre Belloni, devicetree,
	conor+dt
In-Reply-To: <20260706175138.12587-13-prabhakar.mahadev-lad.rj@bp.renesas.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The driver may incorrectly fall back to SUBU mode on RZ/T2H and RZ/N2H SoCs, leading to accesses of the non-existent RTCA0SUBU register and a potential kernel panic.
--

commit ee14aa230b364e5ad7677d78cb0c0a8599351c1c
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

rtc: rzn1: Add support for Renesas RZ/T2H and RZ/N2H SoCs
    
This commit adds support for the RTC block on Renesas RZ/T2H and RZ/N2H SoCs
using a new compatible string. It notes that the variants lack the RTCA0SUBU
register, but rely on the existing SCMP mode to function without it.

> diff --git a/drivers/rtc/rtc-rzn1.c b/drivers/rtc/rtc-rzn1.c
> index d1a974c6b8bbe..1d4d8d01749d3 100644
> --- a/drivers/rtc/rtc-rzn1.c
> +++ b/drivers/rtc/rtc-rzn1.c
> @@ -508,6 +508,7 @@ static int rzn1_rtc_probe(struct platform_device *pdev)
>  }
>  
>  static const struct of_device_id rzn1_rtc_of_match[] = {
> +	{ .compatible	= "renesas,r9a09g077-rtc" },

[Severity: High]
If the devicetree for this SoC omits the "xtal" clock or specifies a standard
32768 Hz frequency, will the driver inadvertently fall back to SUBU mode?

Looking at rzn1_rtc_probe(), the driver sets scmp_val = 0 for 32768 Hz
crystals or when the clock is absent:

drivers/rtc/rtc-rzn1.c:rzn1_rtc_probe() {
    ...
	xtal = devm_clk_get_optional(dev, "xtal");
	if (IS_ERR(xtal)) {
		return PTR_ERR(xtal);
	} else if (xtal) {
		rate = clk_get_rate(xtal);
        ...
		if (rate != 32768)
			scmp_val = RZN1_RTC_CTL0_SLSB_SCMP;
	}
    ...
	if (scmp_val) {
        ...
	} else {
		rtc->rtcdev->ops = &rzn1_rtc_ops_subu;
	}
    ...
}

If this happens on the RZ/T2H, rzn1_rtc_ops_subu exposes the .read_offset
and .set_offset callbacks. Any userspace read of the offset sysfs node
would then trigger a hardware access to the missing register:

drivers/rtc/rtc-rzn1.c:rzn1_rtc_read_offset() {
    ...
	val = readl(rtc->base + RZN1_RTC_SUBU);
    ...
}

Can this lead to a synchronous external abort (kernel panic) when an
unprivileged user reads /sys/class/rtc/rtc0/offset?

Should we add match data to this compatible string to identify the variant
and explicitly reject probe with -EOPNOTSUPP if the clock configuration
would require the unsupported SUBU mode?

>  	{ .compatible	= "renesas,rzn1-rtc" },
>  	{},
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706175138.12587-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=12

^ permalink raw reply

* Re: [PATCH v3 06/12] rtc: rzn1: Fix alarm range check truncation on 32-bit systems
From: sashiko-bot @ 2026-07-06 18:04 UTC (permalink / raw)
  To: Prabhakar
  Cc: wsa+renesas, conor+dt, devicetree, linux-rtc, Alexandre Belloni,
	robh
In-Reply-To: <20260706175138.12587-7-prabhakar.mahadev-lad.rj@bp.renesas.com>

> 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>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706175138.12587-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=6


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox