Re: [PATCH v3 12/12] rtc: rzn1: Add support for Renesas RZ/T2H and RZ/N2H SoCs

[sashiko-bot@kernel.org]

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