[PATCH v2 0/3] rtc: rs5c372: add Ricoh R2223x support

[PATCH v2 0/3] rtc: rs5c372: add Ricoh R2223x support

[Heiko Schocher]

The Ricoh R2223x is an I2C RTC from the same family as the r2025sd and
r2221tl which the rtc-rs5c372 already supports. It shares the R2x2x
control register layout, so only the new type must be added to the driver.

The ricoh R2223x drives a clock output and offers an eco mode that
lowers its current consumption while running from the backup supply.

The trivial-rtc binding does not cover this, so give the device its
own binding in patch 1. Patch 2 adds the R2223x to the rs5c372 rtc
driver, and patch 3 implements the eco-mode setting.

v2 answers the sashiko review of v1 and a local run of the same review
prompts. This resulted in 2 changes:

Patch 2 now handles the new type in rs5c372_ioctl(), where RTC_VL_READ
read CTRL2 bit 4 as XSTP although the R2x2x parts have PON there and
RTC_VL_CLR did nothing at all, and in the two offset helpers, where the
R2223x fell back to the coarse trim resolution.

Patch 3 now applies the device tree setting in both directions. V1 only
set the eco bit before, so a board that dropped the property kept running
in eco mode, because CTRL2 is backed by the backup supply.

Not touched sashiko reviews (some from local run):

- #clock-cells stays out of the required list. Requiring it would
  reject every node that does not use the clock output; the other
  discrete I2C RTC bindings with a clock output keep it optional too.
  Also the clock output can be disabled per hardware pin CLKC. So
  #clock-cells is optional.

  https://sashiko.dev/#/patchset/20260824110452.4038870-1-hs@nabladev.com?part=1

- additionalProperties: false together with $ref: rtc.yaml# means the
  properties from rtc.yaml are not allowed here unless they are listed.
  Only start-year is, the others can be added once a board needs them.
  microcrystal,rv3032.yaml is built the same way.

- ricoh,eco-mode is a vendor boolean. Whether the RTC may run in eco
  mode depends on the backup cell the board is fitted with, so it
  describes the board and not a runtime policy.

Changes in v2:
- Added Reviewed-by from Conor, no code change in patch 1
- Fixed the sashiko review of v1: handle the new type in
  rs5c372_ioctl(), rs5c372_read_offset() and rs5c372_set_offset()
  https://sashiko.dev/#/patchset/20260824110452.4038870-1-hs@nabladev.com?part=2
- Added sashiko review for patch 3 in this patch, as it fits better here
  https://sashiko.dev/#/patchset/20260824110452.4038870-1-hs@nabladev.com?part=3
- Fixed results of a local run of the sashiko review prompts: apply the
  device tree setting in both directions. v1 only set the eco bit, so
  the mode stayed on when a board dropped the property.
- Leave rs5c372_probe() through goto exit like its other error paths,
  instead of returning directly.

Heiko Schocher (3):
  dt-bindings: rtc: add ricoh,r2223x binding
  rtc: rs5c372: add support for Ricoh R2223x
  rtc: rs5c372: support eco mode on R2223x

 .../devicetree/bindings/rtc/ricoh,r2223x.yaml | 58 +++++++++++++++++++
 drivers/rtc/rtc-rs5c372.c                     | 52 ++++++++++++++++-
 2 files changed, 107 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/rtc/ricoh,r2223x.yaml

---
base-commit: 2709dd5ae32f0828f386327c76bba9f39f63a1c6

-- 
2.55.0

[PATCH v2 1/3] dt-bindings: rtc: add ricoh,r2223x binding

[Heiko Schocher]

The Ricoh R2223x is an I2C RTC of the same family as the r2025sd and
r2221tl which the rtc-rs5c372 already supports. It drives a clock output
and can be put into an eco mode that lowers its current consumption from
the backup supply, so it needs #clock-cells and a property for the eco
mode.

The trivial-rtc binding does not cover this, so give the device its
own binding.

Signed-off-by: Heiko Schocher <hs@nabladev.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
---

Changes in v2:
- Added Reviewed-by from Conor, no code change in patch 1

 .../devicetree/bindings/rtc/ricoh,r2223x.yaml | 58 +++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/ricoh,r2223x.yaml

diff --git a/Documentation/devicetree/bindings/rtc/ricoh,r2223x.yaml b/Documentation/devicetree/bindings/rtc/ricoh,r2223x.yaml
new file mode 100644
index 000000000000..e64a1ee378e6
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/ricoh,r2223x.yaml
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/ricoh,r2223x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Ricoh R2223x Real-Time Clock
+
+maintainers:
+  - Heiko Schocher <hs@nabladev.com>
+
+allOf:
+  - $ref: rtc.yaml#
+
+properties:
+  compatible:
+    const: ricoh,r2223x
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  "#clock-cells":
+    const: 0
+
+  ricoh,eco-mode:
+    type: boolean
+    description:
+      Put the RTC into eco mode, which lowers its current consumption while
+      running from the backup supply. Set this on boards where the RTC is
+      expected to keep time on a small backup cell.
+
+  start-year: true
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        rtc@32 {
+            compatible = "ricoh,r2223x";
+            reg = <0x32>;
+            interrupts = <6 IRQ_TYPE_EDGE_FALLING>;
+            #clock-cells = <0>;
+            ricoh,eco-mode;
+        };
+    };
-- 
2.55.0

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

[Heiko Schocher]

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. The oscillator interrupt flag is
reported through XSTP, and the 24 hour mode bit lives in CTRL1. It also
has the DEV bit in the trim register, so same as r2221tl.

Signed-off-by: Heiko Schocher <hs@nabladev.com>
Reported-by: Sashiko AI review <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260824110452.4038870-1-hs@nabladev.com?part=2
Closes: https://sashiko.dev/#/patchset/20260824110452.4038870-1-hs@nabladev.com?part=3
---
checkpatch reports on this patch

  ERROR: trailing statements should be on next line
  +                     case rtc_r2223x:        s = "r2223x"; break;

I did not fix this checkpatch error, as the whole switch statement uses
this format.

Changes in v2:
- Fixed the sashiko review of v1: handle the new type in
  rs5c372_ioctl(), rs5c372_read_offset() and rs5c372_set_offset()
  https://sashiko.dev/#/patchset/20260824110452.4038870-1-hs@nabladev.com?part=2
- Added sashiko review for patch 3 in this patch, as it fits better here
  https://sashiko.dev/#/patchset/20260824110452.4038870-1-hs@nabladev.com?part=3

 drivers/rtc/rtc-rs5c372.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-rs5c372.c b/drivers/rtc/rtc-rs5c372.c
index 24bd795d9d95..c65b76cc0dc6 100644
--- a/drivers/rtc/rtc-rs5c372.c
+++ b/drivers/rtc/rtc-rs5c372.c
@@ -68,6 +68,7 @@ enum rtc_type {
 	rtc_undef = 0,
 	rtc_r2025sd,
 	rtc_r2221tl,
+	rtc_r2223x,
 	rtc_rs5c372a,
 	rtc_rs5c372b,
 	rtc_rv5c386,
@@ -77,6 +78,7 @@ enum rtc_type {
 static const struct i2c_device_id rs5c372_id[] = {
 	{ .name = "r2025sd", .driver_data = rtc_r2025sd },
 	{ .name = "r2221tl", .driver_data = rtc_r2221tl },
+	{ .name = "r2223x", .driver_data = rtc_r2223x },
 	{ .name = "rs5c372a", .driver_data = rtc_rs5c372a },
 	{ .name = "rs5c372b", .driver_data = rtc_rs5c372b },
 	{ .name = "rv5c386", .driver_data = rtc_rv5c386 },
@@ -94,6 +96,10 @@ static const __maybe_unused struct of_device_id rs5c372_of_match[] = {
 		.compatible = "ricoh,r2221tl",
 		.data = (void *)rtc_r2221tl
 	},
+	{
+		.compatible = "ricoh,r2223x",
+		.data = (void *)rtc_r2223x
+	},
 	{
 		.compatible = "ricoh,rs5c372a",
 		.data = (void *)rtc_rs5c372a
@@ -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))) {
 			dev_warn(&client->dev, "rtc oscillator interruption detected. Please reset the rtc clock.\n");
 			return -EINVAL;
 		}
@@ -292,6 +300,7 @@ static int rs5c372_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	switch (rs5c->type) {
 	case rtc_r2025sd:
 	case rtc_r2221tl:
+	case rtc_r2223x:
 		ctrl2 &= ~(R2x2x_CTRL2_VDET | R2x2x_CTRL2_PON);
 		if (rs5c->type == rtc_r2025sd)
 			ctrl2 |= R2x2x_CTRL2_XSTP;
@@ -511,8 +520,10 @@ static int rs5c372_ioctl(struct device *dev, unsigned int cmd, unsigned long arg
 		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))) {
 				flags |= RTC_VL_DATA_INVALID;
 			}
 			if (ctrl2 & R2x2x_CTRL2_VDET)
@@ -527,7 +538,8 @@ static int rs5c372_ioctl(struct device *dev, unsigned int cmd, unsigned long arg
 		return put_user(flags, (unsigned int __user *)arg);
 	case RTC_VL_CLR:
 		/* clear VDET bit */
-		if (rs5c->type == rtc_r2025sd || rs5c->type == rtc_r2221tl) {
+		if (rs5c->type == rtc_r2025sd || rs5c->type == rtc_r2221tl ||
+		    rs5c->type == rtc_r2223x) {
 			ctrl2 &= ~R2x2x_CTRL2_VDET;
 			if (i2c_smbus_write_byte_data(rs5c->client, addr, ctrl2) < 0) {
 				dev_dbg(&rs5c->client->dev, "%s: write error in line %i\n",
@@ -554,6 +566,7 @@ static int rs5c372_read_offset(struct device *dev, long *offset)
 
 	switch (rs5c->type) {
 	case rtc_r2221tl:
+	case rtc_r2223x:
 		ppb_per_step = val & R2221TL_TRIM_DEV ? 1017 : 3051;
 		break;
 	case rtc_rs5c372a:
@@ -600,6 +613,7 @@ static int rs5c372_set_offset(struct device *dev, long offset)
 		}
 		break;
 	case rtc_r2221tl:
+	case rtc_r2223x:
 		/*
 		 * Check if it is possible to use high resolution mode (DEV=1).
 		 * In this mode, the minimum resolution is 2 / (32768 * 20 * 3),
@@ -750,6 +764,7 @@ static int rs5c_oscillator_setup(struct rs5c372 *rs5c372)
 			return ret;
 		break;
 	case rtc_r2221tl:
+	case rtc_r2223x:
 		if (!(buf[1] & R2x2x_CTRL2_XSTP))
 			return ret;
 		break;
@@ -768,6 +783,7 @@ static int rs5c_oscillator_setup(struct rs5c372 *rs5c372)
 		break;
 	case rtc_r2025sd:
 	case rtc_r2221tl:
+	case rtc_r2223x:
 	case rtc_rv5c386:
 	case rtc_rv5c387a:
 		buf[0] |= RV5C387_CTRL1_24;
@@ -847,6 +863,7 @@ static int rs5c372_probe(struct i2c_client *client)
 		break;
 	case rtc_r2025sd:
 	case rtc_r2221tl:
+	case rtc_r2223x:
 	case rtc_rv5c386:
 	case rtc_rv5c387a:
 		if (rs5c372->regs[RS5C_REG_CTRL1] & RV5C387_CTRL1_24)
@@ -876,6 +893,7 @@ static int rs5c372_probe(struct i2c_client *client)
 			({ char *s; switch (rs5c372->type) {
 			case rtc_r2025sd:	s = "r2025sd"; break;
 			case rtc_r2221tl:	s = "r2221tl"; break;
+			case rtc_r2223x:	s = "r2223x"; break;
 			case rtc_rs5c372a:	s = "rs5c372a"; break;
 			case rtc_rs5c372b:	s = "rs5c372b"; break;
 			case rtc_rv5c386:	s = "rv5c386"; break;
-- 
2.55.0

[PATCH v2 3/3] rtc: rs5c372: support eco mode on R2223x

[Heiko Schocher]

The R2223x can run in an eco mode that lowers its current consumption
from the backup supply. Follow the ricoh,eco-mode property from the
device tree: enable the mode when the property is set, turn it off when
it is not set, as the bit is in CTRL2 register, which is backed by the
same supply and keeps its contents across a reset, and no other path in
the driver touches it.

Signed-off-by: Heiko Schocher <hs@nabladev.com>
---

Changes in v2:
- Fixed results of a local run of the sashiko review prompts: apply the
  device tree setting in both directions. v1 only set the eco bit, so
  the mode stayed on when a board dropped the property.
- Leave rs5c372_probe() through goto exit like its other error paths,
  instead of returning directly.

 drivers/rtc/rtc-rs5c372.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/rtc/rtc-rs5c372.c b/drivers/rtc/rtc-rs5c372.c
index c65b76cc0dc6..9ab6f2c28f7b 100644
--- a/drivers/rtc/rtc-rs5c372.c
+++ b/drivers/rtc/rtc-rs5c372.c
@@ -52,6 +52,7 @@
 #define RS5C_REG_CTRL2		15
 #	define RS5C372_CTRL2_24		(1 << 5)
 #	define RS5C_CTRL2_XSTP		(1 << 4)	/* only if !R2x2x */
+#	define R2223x_CTRL2_ECO		(1 << 7)	/* only if  R2223x */
 #	define R2x2x_CTRL2_VDET		(1 << 6)	/* only if  R2x2x */
 #	define R2x2x_CTRL2_XSTP		(1 << 5)	/* only if  R2x2x */
 #	define R2x2x_CTRL2_PON		(1 << 4)	/* only if  R2x2x */
@@ -807,6 +808,24 @@ static int rs5c_oscillator_setup(struct rs5c372 *rs5c372)
 	return 0;
 }
 
+static int rs5c372_set_eco_mode(struct rs5c372 *rs5c372, bool eco)
+{
+	struct i2c_client *client = rs5c372->client;
+	int ctrl2;
+
+	ctrl2 = i2c_smbus_read_byte_data(client, RS5C_ADDR(RS5C_REG_CTRL2));
+	if (ctrl2 < 0)
+		return ctrl2;
+
+	if (eco)
+		ctrl2 |= R2223x_CTRL2_ECO;
+	else
+		ctrl2 &= ~R2223x_CTRL2_ECO;
+
+	return i2c_smbus_write_byte_data(client, RS5C_ADDR(RS5C_REG_CTRL2),
+					 ctrl2);
+}
+
 static int rs5c372_probe(struct i2c_client *client)
 {
 	int err = 0;
@@ -903,6 +922,15 @@ static int rs5c372_probe(struct i2c_client *client)
 			rs5c372->time24 ? "24hr" : "am/pm"
 			);
 
+	if (rs5c372->type == rtc_r2223x) {
+		bool eco = device_property_read_bool(&client->dev,
+						     "ricoh,eco-mode");
+
+		err = rs5c372_set_eco_mode(rs5c372, eco);
+		if (err < 0)
+			goto exit;
+	}
+
 	/* REVISIT use client->irq to register alarm irq ... */
 	rs5c372->rtc = devm_rtc_device_register(&client->dev,
 					rs5c372_driver.driver.name,
-- 
2.55.0

Re: [PATCH v2 1/3] dt-bindings: rtc: add ricoh,r2223x binding

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `#clock-cells` property is omitted from the `required` list despite the device functioning as a clock provider.
--

--- Patch [1]: [PATCH v2 1/3] dt-bindings: rtc: add ricoh,r2223x binding ---
commit c9a8726516af9b3ebff5835956246051050d1e80
Author: Heiko Schocher <hs@nabladev.com>

dt-bindings: rtc: add ricoh,r2223x binding

This commit adds a new device tree binding for the Ricoh R2223x I2C RTC.
The device drives a clock output and features an eco mode to lower current
consumption from the backup supply.

> diff --git a/Documentation/devicetree/bindings/rtc/ricoh,r2223x.yaml b/Documentation/devicetree/bindings/rtc/ricoh,r2223x.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/ricoh,r2223x.yaml
[ ... ]
> +  start-year: true
> +
> +required:
> +  - compatible
> +  - reg

[Severity: Medium]
Since this device functions as a clock provider and defines #clock-cells,
should #clock-cells be included in the required list?

Omitting it allows incomplete device tree nodes to pass schema validation
but fail at runtime when dependent subsystems attempt to use the clock
functionality.

> +
> +additionalProperties: false
> +
> +examples:
[ ... ]

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

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

[sashiko-bot]

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

Pre-existing issues:
- [High] The driver assigns the `s32` return value of `i2c_smbus_read_byte_data()` directly to an `unsigned char` without checking for negative error codes, which can silently corrupt the control register.
- [High] Stale cached register value is modified and written back in `rs5c_rtc_alarm_irq_enable()`, clobbering hardware state.
- [High] The probe function silently succeeds without registering the RTC device if the RTC type is unknown.
- [Medium] In `rs5c_set_alarm()`, a failed I2C write when enabling the alarm is ignored, resulting in an inconsistent cached state.
--

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

This commit adds support for the Ricoh R2223x I2C RTC device to the rs5c372
driver. It updates the driver's type definitions, device ID tables, and match
tables, treating the R2223x similarly to the r2221tl and r2025sd models for
time setting, reading, and control register handling.

Closes: https://sashiko.dev/#/patchset/20260824110452.4038870-1-hs@nabladev.com?part=2
Closes: https://sashiko.dev/#/patchset/20260824110452.4038870-1-hs@nabladev.com?part=3

> diff --git a/drivers/rtc/rtc-rs5c372.c b/drivers/rtc/rtc-rs5c372.c
> index 24bd795d9d95d..c65b76cc0dc66 100644
> --- a/drivers/rtc/rtc-rs5c372.c
> +++ b/drivers/rtc/rtc-rs5c372.c
[ ... ]
> @@ -292,6 +300,7 @@ static int rs5c372_rtc_set_time(struct device *dev, struct rtc_time *tm)
>  	switch (rs5c->type) {
>  	case rtc_r2025sd:
>  	case rtc_r2221tl:
> +	case rtc_r2223x:
>  		ctrl2 &= ~(R2x2x_CTRL2_VDET | R2x2x_CTRL2_PON);
>  		if (rs5c->type == rtc_r2025sd)
>  			ctrl2 |= R2x2x_CTRL2_XSTP;

[Severity: High]
This is a pre-existing issue, but does rs5c372_rtc_set_time() handle negative
error codes from i2c_smbus_read_byte_data() correctly here?

Just above this switch statement, the return value is assigned directly to an
unsigned char:

	ctrl2 = i2c_smbus_read_byte_data(client, addr);

If this read fails (e.g., returning -EIO), the negative error code is
truncated into an unsigned char. The driver then masks and modifies this
corrupted value before writing it back to the device, which could silently
corrupt the control register.

[ ... ]
> @@ -511,8 +520,10 @@ static int rs5c372_ioctl(struct device *dev, unsigned int cmd, unsigned long arg
>  		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))) {
>  				flags |= RTC_VL_DATA_INVALID;

[Severity: High]
This is a pre-existing issue, but similar to the set_time function, does
rs5c372_ioctl() suffer from the same truncation problem?

On the RTC_VL_CLR path, the read value is assigned to an unsigned char:

	ctrl2 = i2c_smbus_read_byte_data(rs5c->client, addr);

If this read fails, the unhandled negative error code is truncated and
subsequently written back to the device, potentially clobbering the register.

[ ... ]
> @@ -847,6 +863,7 @@ static int rs5c372_probe(struct i2c_client *client)
>  		break;
>  	case rtc_r2025sd:
>  	case rtc_r2221tl:
> +	case rtc_r2223x:
>  	case rtc_rv5c386:
>  	case rtc_rv5c387a:
>  		if (rs5c372->regs[RS5C_REG_CTRL1] & RV5C387_CTRL1_24)

[Severity: High]
This is a pre-existing issue, but does rs5c372_probe() silently succeed if
the RTC type is unknown?

If the device type doesn't match the known cases, the switch falls through to
the default case:

	default:
		dev_err(&client->dev, "unknown RTC type\n");
		goto exit;

At this point, the 'err' variable is 0 because the preceding rs5c_get_regs()
call succeeded. This causes the driver probe to incorrectly return 0 (success)
instead of a negative error code like -ENODEV, leading to an incorrectly
bound but non-functional driver.

While reviewing the above changes, a few other pre-existing issues were
noticed in this file:

[Severity: High]
This is a pre-existing issue, but does rs5c_rtc_alarm_irq_enable() overwrite
hardware state changes with stale cached values?

The function initializes 'buf' from the old cache before fetching the new
state:

	buf = rs5c->regs[RS5C_REG_CTRL1];

	if (!rs5c->has_irq)
		return -EINVAL;

	status = rs5c_get_regs(rs5c);

The rs5c_get_regs() call updates the local cache (rs5c->regs) with the
latest hardware state. However, the function then modifies the stale 'buf'
variable and writes it back to the hardware. Any state changes that occurred
between the previous cache update and the rs5c_get_regs() call could be
silently overwritten with the old values.

[Severity: Medium]
This is a pre-existing issue, but does rs5c_set_alarm() incorrectly sync the
local register cache to an enabled state if the I2C write fails?

When enabling the alarm:

	if (i2c_smbus_write_byte_data(client, addr, buf[0]) < 0)
		dev_warn(dev, "can't enable alarm\n");
	rs5c->regs[RS5C_REG_CTRL1] = buf[0];

If the I2C write fails, the driver logs a warning but proceeds to update the
software cache (rs5c->regs) as if the operation succeeded, returning 0. This
leaves the software state reporting the alarm as active while the hardware
is not actually configured.

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

Re: [PATCH v2 3/3] rtc: rs5c372: support eco mode on R2223x

[sashiko-bot]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but missing RTC range_min and range_max constraints allow user-space to inject dates prior to year 2000, causing an integer underflow in bin2bcd() and silent RTC corruption.
--

--- Patch [3]: [PATCH v2 3/3] rtc: rs5c372: support eco mode on R2223x ---
commit dde76823d322a057d0dc319ae14408d7646ff056
Author: Heiko Schocher <hs@nabladev.com>

rtc: rs5c372: support eco mode on R2223x

The R2223x can run in an eco mode that lowers its current consumption from the
backup supply. This patch adds support for configuring this mode via the
ricoh,eco-mode device tree property.

> diff --git a/drivers/rtc/rtc-rs5c372.c b/drivers/rtc/rtc-rs5c372.c
> index c65b76cc0dc66..9ab6f2c28f7b1 100644
> --- a/drivers/rtc/rtc-rs5c372.c
> +++ b/drivers/rtc/rtc-rs5c372.c

[ ... ]

> @@ -903,6 +922,15 @@ static int rs5c372_probe(struct i2c_client *client)
>  			rs5c372->time24 ? "24hr" : "am/pm"
>  			);
>  
> +	if (rs5c372->type == rtc_r2223x) {
> +		bool eco = device_property_read_bool(&client->dev,
> +						     "ricoh,eco-mode");
> +
> +		err = rs5c372_set_eco_mode(rs5c372, eco);
> +		if (err < 0)
> +			goto exit;
> +	}
> +
>  	/* REVISIT use client->irq to register alarm irq ... */
>  	rs5c372->rtc = devm_rtc_device_register(&client->dev,
>  					rs5c372_driver.driver.name,

[Severity: High]
This isn't a bug introduced by this patch, but should the RTC range_min and
range_max constraints be explicitly initialized here before registration?

The core RTC subsystem defaults range_min to 1970 if not set by the driver,
allowing user-space to set tm_year between 70 and 99. Inside
rs5c372_rtc_set_time(), the driver computes tm->tm_year - 100, which
underflows to a negative number when tm_year < 100:

drivers/rtc/rtc-rs5c372.c:rs5c372_rtc_set_time() {
    ...
    buf[6] = bin2bcd(tm->tm_year - 100);
    ...
}

This results in an incorrectly shifted BCD value being written to the I2C RTC
register, silently corrupting the RTC time with garbage future dates when a
valid standard date is requested.

>  					&rs5c372_rtc_ops, THIS_MODULE);
>  
>  	if (IS_ERR(rs5c372->rtc)) {

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