Re: [PATCH v1 2/3] rtc: rs5c372: add support for Ricoh R2223x

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The new RTC type `rtc_r2223x` is missing from the switch statements in `rs5c372_ioctl()`, `rs5c372_read_offset()`, and `rs5c372_set_offset()`.
--

--- Patch [2]: [PATCH v1 2/3] rtc: rs5c372: add support for Ricoh R2223x ---
commit f8cda87e08814c0d84a3917bdc200d86363e7127
Author: Heiko Schocher <hs@nabladev.com>

rtc: rs5c372: add support for Ricoh R2223x
    
The R2223x is an I2C RTC from the same family as the r2025sd and r2221tl
that this driver already handles. It shares the R2x2x control register
layout, so treat it like the r2221tl.

> diff --git a/drivers/rtc/rtc-rs5c372.c b/drivers/rtc/rtc-rs5c372.c
> index 24bd795d9d95d..30e272d650219 100644
> --- a/drivers/rtc/rtc-rs5c372.c
> +++ b/drivers/rtc/rtc-rs5c372.c
[ ... ]
> @@ -221,8 +227,10 @@ static int rs5c372_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  	switch (rs5c->type) {
>  	case rtc_r2025sd:
>  	case rtc_r2221tl:
> +	case rtc_r2223x:
>  		if ((rs5c->type == rtc_r2025sd && !(ctrl2 & R2x2x_CTRL2_XSTP)) ||
> -		    (rs5c->type == rtc_r2221tl &&  (ctrl2 & R2x2x_CTRL2_XSTP))) {
> +		    ((rs5c->type == rtc_r2221tl || rs5c->type == rtc_r2223x) &&
> +		     (ctrl2 & R2x2x_CTRL2_XSTP))) {

[Severity: Medium]
Since rtc_r2223x shares the register layout with rtc_r2221tl, should it also be
added to the switch statements in rs5c372_ioctl(), rs5c372_read_offset(), and
rs5c372_set_offset()?

Without it, RTC_VL_READ falls into the default path, ignoring VDET and
returning incorrect XSTP bits. RTC_VL_CLR also fails to clear the VDET hardware
flag:

	case RTC_VL_CLR:
		/* clear VDET bit */
		if (rs5c->type == rtc_r2025sd || rs5c->type == rtc_r2221tl) {
			ctrl2 &= ~R2x2x_CTRL2_VDET;
			...

Additionally, rs5c372_read_offset() and rs5c372_set_offset() will skip using
the higher resolution DEV mode for rtc_r2223x, falling back to the default low
resolution calculations:

	switch (rs5c->type) {
	case rtc_r2221tl:
		ppb_per_step = val & R2221TL_TRIM_DEV ? 1017 : 3051;
		break;
	...

>  			dev_warn(&client->dev, "rtc oscillator interruption detected. Please reset the rtc clock.\n");
>  			return -EINVAL;
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824110452.4038870-1-hs@nabladev.com?part=2