Re: [PATCH 5/6] dt-bindings: rtc: rzn1: add R-Car X5H support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] The R-Car X5H compatible string is missing an `if-then` conditional block to explicitly disallow interrupts.

Pre-existing issues:
- [High] The driver dynamically modifies a global static `rtc_class_ops` structure during probe, causing operations to be incorrectly shared and corrupted across all device instances.
--

commit a9257218efe4782b04de9171568942363749f822
Author: Wolfram Sang <wsa+renesas@sang-engineering.com>

    dt-bindings: rtc: rzn1: add R-Car X5H support

This patch adds support for the R-Car X5H hardware variant to the RZ/N1 RTC
device tree binding schema. The commit message explicitly notes that this
hardware is identical to RZ/T2H except that interrupts are not wired.

> diff --git a/Documentation/devicetree/bindings/rtc/renesas,rzn1-rtc.yaml b/Documentation/devicetree/bindings/rtc/renesas,rzn1-rtc.yaml
> --- a/Documentation/devicetree/bindings/rtc/renesas,rzn1-rtc.yaml
> +++ b/Documentation/devicetree/bindings/rtc/renesas,rzn1-rtc.yaml
[ ... ]
> @@ -66,6 +71,7 @@ allOf:
>              enum:
>                - renesas,r9a09g077-rtc
>                - renesas,r9a09g087-rtc
> +              - renesas,rcar-gen5-rtc
>      then:
>        properties:
>          clocks:

[Severity: Medium]
Since the commit message states that interrupts are not wired for R-Car X5H,
should there be another if-then block added to the allOf section to
explicitly restrict the interrupts property for renesas,rcar-gen5-rtc?

Without this conditional check, a device tree incorrectly defining an
interrupts property for this compatible string would silently pass schema
validation despite the hardware limitation.

While reviewing the associated driver code, I noticed a separate issue:

drivers/rtc/rtc-rzn1.c:rzn1_rtc_probe() {
[ ... ]
	if (scmp_val) {
		writel(rate - 1, rtc->base + RZN1_RTC_SCMP);
	} else {
		rzn1_rtc_ops.read_offset = rzn1_rtc_read_offset;
		rzn1_rtc_ops.set_offset = rzn1_rtc_set_offset;
	}
[ ... ]
		rzn1_rtc_ops.read_alarm = rzn1_rtc_read_alarm;
		rzn1_rtc_ops.set_alarm = rzn1_rtc_set_alarm;
		rzn1_rtc_ops.alarm_irq_enable = rzn1_rtc_alarm_irq_enable;
	}

	return devm_rtc_register_device(rtc->rtcdev);
}

[Severity: High]
This is a pre-existing issue, but does mutating the global static rzn1_rtc_ops
structure during probe cause state corruption across device instances?

If multiple RTC instances exist, devices probed subsequently that lack
certain capabilities (like no SUBU register or no alarm interrupt) will
incorrectly inherit the callbacks set by previously probed devices. Could
this lead to unhandled behavior if unsupported operations are invoked?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914211826.73727-8-wsa+renesas@sang-engineering.com?part=5