[PATCH v9 0/6] rtc: s35390a: Allow use of output pin for interrupt signal 1 for wakealarm

[PATCH v9 0/6] rtc: s35390a: Allow use of output pin for interrupt signal 1 for wakealarm

[Markus Probst]

Add pinctrl to allow the use of output pin for interrupt signal 1
for wakealarm. This is needed for wakealarms to work on Synology NAS
devices.

I could only partially test the pinctrl patch. My testing system runs
ACPI, which makes it impossible for me to configure pinctrl there. I did
however verify that with missing pinctrl configuration in the devicetree,
the register were correctly set. So there should be no regressions.
Every other function than ignore, disable and wakeup should also be
considered untested.

Also If I am not mistaken, wake alarms on these systems are currently broken:
(not tested, judged by looking at the devicetrees).

- arch/arm/boot/dts/marvell/armada-370-synology-ds213j.dts
- arch/arm/boot/dts/marvell/armada-xp-synology-ds414.dts
- arch/arm/boot/dts/marvell/kirkwood-synology.dtsi
- arch/arm/boot/dts/marvell/kirkwood-ds110jv10.dts
- arch/arm/boot/dts/marvell/kirkwood-ds111.dts
- arch/arm/boot/dts/marvell/kirkwood-ds112.dts
- arch/arm/boot/dts/marvell/kirkwood-ds210.dts
- arch/arm/boot/dts/marvell/kirkwood-ds212.dts
- arch/arm/boot/dts/marvell/kirkwood-ds212j.dts
- arch/arm/boot/dts/marvell/kirkwood-ds411.dts
- arch/arm/boot/dts/marvell/kirkwood-ds411j.dts
- arch/arm/boot/dts/marvell/kirkwood-ds411slim.dts
- arch/arm/boot/dts/marvell/kirkwood-rs212.dts
- arch/arm/boot/dts/marvell/kirkwood-rs411.dts

If thats the case it can be fixed by using this patch series and adding
the example in the devicetree to the s35390a devicetree.

If somebody still runs one of these systems, please test.

Thanks
- Markus Probst

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
Changes in v9:
- fix issues reported by Sashiko
  - fix interrupt-names in dt
- use interrupts in dt example (there is a known dts in the kernel that has
  one)
- add Reviewed-By trailer from Linus Walleij
- Link to v8: https://patch.msgid.link/20260831-rtc_s35390a_int1-v8-0-dac1dc86ce1f@posteo.de

Changes in v8:
- fix issues reported by Sashiko
  - avoid usage of cleanup helper and goto in one function
- replace dt binding maintainer with self
- use allOf instead of manually referencing the properties in
  pinmux-node.yaml in dt binding
- move additionalProperties up for readability
- Link to v7: https://patch.msgid.link/20260829-rtc_s35390a_int1-v7-0-debcd31143ae@posteo.de

Changes in v7:
- fix issues reported by Sashiko
  - check return value of `s35390a_read_status` for errors
  - select PINCTRL instead of depend to avoid silently dropping the
    driver on platforms without PINCTRL
- Link to v6: https://patch.msgid.link/20260824-rtc_s35390a_int1-v6-0-df96569e3412@posteo.de

Changes in v6:
- instead of forcing 24-hour mode, check on rtc access
- Link to v5: https://patch.msgid.link/20260820-rtc_s35390a_int1-v5-0-5eb4ef85c6a9@posteo.de

Changes in v5:
- fix issues reported by Sashiko
  - fix time corruption on setting 24-hour mode
  - fix setting 24-hour even if uninitialized
- Link to v4: https://patch.msgid.link/20260815-rtc_s35390a_int1-v4-0-3133309c365f@posteo.de

Changes in v4:
- fix issues reported by Sashiko
  - require "pins" property in dt
  - remove maintainers entry for include/dt-bindings/rtc (which only
    existed in previous iterations)
  - fix multiple reads to status1, which clears interrupt flags
  - fix not waiting for Power-On Circuit check
  - fix missing pinctrl dt callbacks
- enforce minimum length of "pins" property in dt
- fix pmin2 on int2 affecting int1
- force use of 24-hour mode
- Link to v3: https://patch.msgid.link/20260805-rtc_s35390a_int1-v3-0-e836faba3e25@posteo.de

Changes in v3:
- fix issues reported by Sashiko
  - fix use of uninitialized time data
  - remove dependence on `CONFIG_OF`
  - add missing `pinctrl_enable` call
  - fix wrong index used in `s35390a_pinconf_set`
  - fix interrupt not cleared
  - fix `device_set_wakeup_capable` called to late in probe
  - fix mode caching even on failure
  - fix failure on synology quirk mode update ignored
- only set irq in alarm_irq_enable callback
- fix alarm time not set if set_alarm is called with alarm disabled
- use pinmux instead of pinconf
- move patternProperties below properties in dt
- remove dt-bindings header
- add '#clock-cells' dt property
- merge 32768khz and user frequency mode into "clock"
- remove alarm mode
- refer to mode now as function in the code, to match phrasing in pinmux
- resolve checkpatches --strict warnings
- remove mode_init bool and instead set initial function to -1
- remove err_read probe variable and reuse err
- rebase onto v6.2-rc6
- Link to v2: https://patch.msgid.link/20260801-rtc_s35390a_int1-v2-0-f10c99ad1d6c@posteo.de

Changes in v2:
- remove sii,wakealarm-output-pin property
- add pinctrl
- add fix to allow disabling of wake alarms
- add synology quirk
- Link to v1: https://patch.msgid.link/20260630-rtc_s35390a_int1-v1-0-1b2239e16be2@posteo.de

---
Markus Probst (6):
      dt-bindings: rtc: Add pinctrl for S35390A
      rtc: s35390a: Add missing newline to dev_err
      rtc: s35390a: Fix alarm not disabling
      rtc: s35390a: Read 24-hour mode on access
      rtc: s35390a: Add pinctrl
      rtc: s35390a: Add synology quirk

 .../devicetree/bindings/rtc/sii,s35390a.yaml       | 121 ++++++
 .../devicetree/bindings/rtc/trivial-rtc.yaml       |   3 -
 drivers/rtc/Kconfig                                |   2 +
 drivers/rtc/rtc-s35390a.c                          | 473 ++++++++++++++++-----
 4 files changed, 501 insertions(+), 98 deletions(-)
---
base-commit: c2cd463d6ee7d55a3ec0719d93c49ff99022d58f
change-id: 20260630-rtc_s35390a_int1-556ccb308d3f

[PATCH v9 1/6] dt-bindings: rtc: Add pinctrl for S35390A

[Markus Probst]

Synology NAS devices use the output pin for interrupt signal 1 to wake up
the system.

Move devicetree bindings for sii,s35390a into its own file.
Add necessary properties to configure the individual pins via pinctrl,
which allows the interrupt signal 1 to be used for wakeup alarm.

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 .../devicetree/bindings/rtc/sii,s35390a.yaml       | 121 +++++++++++++++++++++
 .../devicetree/bindings/rtc/trivial-rtc.yaml       |   3 -
 2 files changed, 121 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/rtc/sii,s35390a.yaml b/Documentation/devicetree/bindings/rtc/sii,s35390a.yaml
new file mode 100644
index 000000000000..f3bf5900fb3c
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/sii,s35390a.yaml
@@ -0,0 +1,121 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/sii,s35390a.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: S-35390A 2-WIRE REAL-TIME CLOCK
+
+maintainers:
+  - Markus Probst <markus.probst@posteo.de>
+
+description:
+  The S-35390A is a CMOS 2-wire real-time clock IC which operates with the
+  very low current consumption in the wide range of operation voltage.
+
+allOf:
+  - $ref: rtc.yaml#
+
+properties:
+  compatible:
+    const: sii,s35390a
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    minItems: 1
+    maxItems: 2
+    description: Supports up to 2 interrupt lines via the INT1 and INT2 pins.
+
+  interrupt-names:
+    minItems: 1
+    maxItems: 2
+    items:
+      enum:
+        - int1
+        - int2
+
+  wakeup-parent: true
+
+  wakeup-source: true
+
+  "#clock-cells":
+    const: 1
+
+patternProperties:
+  "^pins":
+    type: object
+    additionalProperties: false
+
+    patternProperties:
+      "-pins$":
+        type: object
+        additionalProperties: false
+
+        allOf:
+          - $ref: /schemas/pinctrl/pinmux-node.yaml#
+
+        properties:
+          pins:
+            minItems: 1
+            items:
+              enum:
+                - int1
+                - int2
+
+          function:
+            description: |
+              Pin function:
+              - ignore:  Preserve the previous state.
+              - disable: Disable pin output.
+              - wakeup:  Output wakes up the system.
+              - clock:   Output clock pulse.
+              - pmin1:   Minute periodical output with 50% duty.
+              - pmin2:   Minute periodical output L for 7.81 ms.
+                         Can only be used with pin int1.
+            enum:
+              - ignore
+              - disable
+              - wakeup
+              - clock
+              - pmin1
+              - pmin2
+
+        required:
+          - pins
+          - function
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        rtc@30 {
+            compatible = "sii,s35390a";
+            reg = <0x30>;
+            pinctrl-0 = <&rtc_pins_default>;
+            pinctrl-names = "default";
+
+            interrupt-names = "int1";
+            interrupts = <23>;
+
+            rtc_pins_default: pins {
+              int1-pins {
+                pins = "int1";
+                function = "wakeup";
+              };
+              int2-pins {
+                pins = "int2";
+                function = "disable";
+              };
+            };
+        };
+    };
diff --git a/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml b/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml
index f4d0eed98a08..7b3f682ef4d5 100644
--- a/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml
+++ b/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml
@@ -81,8 +81,6 @@ properties:
       - ricoh,rv5c386
       # I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
       - ricoh,rv5c387a
-      # 2-wire CMOS real-time clock
-      - sii,s35390a
       # ST SPEAr Real-time Clock
       - st,spear600-rtc
       # VIA/Wondermedia VT8500 Real-time Clock
@@ -105,5 +103,4 @@ required:
   - reg
 
 additionalProperties: false
-
 ...

-- 
2.55.0

[PATCH v9 2/6] rtc: s35390a: Add missing newline to dev_err

[Markus Probst]

Fixes: 3bd32722c827d ("rtc: s35390a: improve irq handling")
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 drivers/rtc/rtc-s35390a.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
index b72eef4fb099..4cfe7034c516 100644
--- a/drivers/rtc/rtc-s35390a.c
+++ b/drivers/rtc/rtc-s35390a.c
@@ -494,7 +494,7 @@ static int s35390a_probe(struct i2c_client *client)
 		buf = 0;
 		err = s35390a_set_reg(s35390a, S35390A_CMD_STATUS2, &buf, 1);
 		if (err < 0) {
-			dev_err(dev, "error disabling alarm");
+			dev_err(dev, "error disabling alarm\n");
 			return err;
 		}
 	} else {

-- 
2.55.0

[PATCH v9 3/6] rtc: s35390a: Fix alarm not disabling

[Markus Probst]

Implement alarm_irq_enable callback.

Fixes: 542dd33a4925 ("drivers/rtc/rtc-s35390a.c: add wakealarm support for rtc-s35390A rtc chip")
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 drivers/rtc/rtc-s35390a.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
index 4cfe7034c516..575bb256eb25 100644
--- a/drivers/rtc/rtc-s35390a.c
+++ b/drivers/rtc/rtc-s35390a.c
@@ -270,6 +270,24 @@ static int s35390a_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	return 0;
 }
 
+static int s35390a_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
+{
+	struct s35390a *s35390a = dev_get_drvdata(dev);
+	u8 sts;
+	int err;
+
+	if (enabled)
+		sts = S35390A_INT2_MODE_ALARM;
+	else
+		sts = S35390A_INT2_MODE_NOINTR;
+
+	err = s35390a_set_reg(s35390a, S35390A_CMD_STATUS2, &sts, sizeof(sts));
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
 static int s35390a_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alm)
 {
 	struct i2c_client *client = to_i2c_client(dev);
@@ -410,11 +428,12 @@ static int s35390a_rtc_ioctl(struct device *dev, unsigned int cmd,
 }
 
 static const struct rtc_class_ops s35390a_rtc_ops = {
-	.read_time	= s35390a_rtc_read_time,
-	.set_time	= s35390a_rtc_set_time,
-	.set_alarm	= s35390a_rtc_set_alarm,
-	.read_alarm	= s35390a_rtc_read_alarm,
-	.ioctl          = s35390a_rtc_ioctl,
+	.read_time		= s35390a_rtc_read_time,
+	.set_time		= s35390a_rtc_set_time,
+	.set_alarm		= s35390a_rtc_set_alarm,
+	.read_alarm		= s35390a_rtc_read_alarm,
+	.alarm_irq_enable	= s35390a_rtc_alarm_irq_enable,
+	.ioctl			= s35390a_rtc_ioctl,
 };
 
 static int s35390a_nvmem_read(void *priv, unsigned int offset, void *val,

-- 
2.55.0

[PATCH v9 4/6] rtc: s35390a: Read 24-hour mode on access

[Markus Probst]

Instead of reading if the 24-hour mode is used on probe once, read it on
access. This makes it impossible for the mode to be out of sync and fixes
time corruption if resetting the chip while in 12-hour mode, as
`s35390a_init` did not update the cached value.

Fixes: 16486d0c1c65 ("rtc: s35390a: handle invalid RTC time")
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 drivers/rtc/rtc-s35390a.c | 78 +++++++++++++++++++++++++----------------------
 1 file changed, 42 insertions(+), 36 deletions(-)

diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
index 575bb256eb25..12ed504db94d 100644
--- a/drivers/rtc/rtc-s35390a.c
+++ b/drivers/rtc/rtc-s35390a.c
@@ -64,7 +64,6 @@ MODULE_DEVICE_TABLE(of, s35390a_of_match);
 
 struct s35390a {
 	struct i2c_client *client[8];
-	int twentyfourhour;
 };
 
 static int s35390a_set_reg(struct s35390a *s35390a, int reg, u8  *buf, int len)
@@ -102,9 +101,8 @@ static int s35390a_get_reg(struct s35390a *s35390a, int reg, u8 *buf, int len)
 	return 0;
 }
 
-static int s35390a_init(struct s35390a *s35390a)
+static int s35390a_init(struct s35390a *s35390a, u8 *sts)
 {
-	u8 buf;
 	int ret;
 	unsigned initcount = 0;
 
@@ -117,17 +115,17 @@ static int s35390a_init(struct s35390a *s35390a)
 	 * The 24H bit is kept over reset, so set it already here.
 	 */
 initialize:
-	buf = S35390A_FLAG_RESET | S35390A_FLAG_24H;
-	ret = s35390a_set_reg(s35390a, S35390A_CMD_STATUS1, &buf, 1);
+	*sts = S35390A_FLAG_RESET | S35390A_FLAG_24H;
+	ret = s35390a_set_reg(s35390a, S35390A_CMD_STATUS1, sts, 1);
 
 	if (ret < 0)
 		return ret;
 
-	ret = s35390a_get_reg(s35390a, S35390A_CMD_STATUS1, &buf, 1);
+	ret = s35390a_get_reg(s35390a, S35390A_CMD_STATUS1, sts, 1);
 	if (ret < 0)
 		return ret;
 
-	if (buf & (S35390A_FLAG_POC | S35390A_FLAG_BLD)) {
+	if (*sts & (S35390A_FLAG_POC | S35390A_FLAG_BLD)) {
 		/* Try up to five times to reset the chip */
 		if (initcount < 5) {
 			++initcount;
@@ -181,9 +179,9 @@ static int s35390a_disable_test_mode(struct s35390a *s35390a)
 	return s35390a_set_reg(s35390a, S35390A_CMD_STATUS2, buf, sizeof(buf));
 }
 
-static char s35390a_hr2reg(struct s35390a *s35390a, int hour)
+static char s35390a_hr2reg(int hour, bool twentyfourhour)
 {
-	if (s35390a->twentyfourhour)
+	if (twentyfourhour)
 		return bin2bcd(hour);
 
 	if (hour < 12)
@@ -192,11 +190,11 @@ static char s35390a_hr2reg(struct s35390a *s35390a, int hour)
 	return 0x40 | bin2bcd(hour - 12);
 }
 
-static int s35390a_reg2hr(struct s35390a *s35390a, char reg)
+static int s35390a_reg2hr(char reg, bool twentyfourhour)
 {
 	unsigned hour;
 
-	if (s35390a->twentyfourhour)
+	if (twentyfourhour)
 		return bcd2bin(reg & 0x3f);
 
 	hour = bcd2bin(reg & 0x3f);
@@ -210,7 +208,7 @@ static int s35390a_rtc_set_time(struct device *dev, struct rtc_time *tm)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	struct s35390a	*s35390a = i2c_get_clientdata(client);
-	int i;
+	int i, err;
 	u8 buf[7], status;
 
 	dev_dbg(&client->dev, "%s: tm is secs=%d, mins=%d, hours=%d mday=%d, "
@@ -218,14 +216,18 @@ static int s35390a_rtc_set_time(struct device *dev, struct rtc_time *tm)
 		tm->tm_min, tm->tm_hour, tm->tm_mday, tm->tm_mon, tm->tm_year,
 		tm->tm_wday);
 
-	if (s35390a_read_status(s35390a, &status) == 1)
-		s35390a_init(s35390a);
+	err = s35390a_read_status(s35390a, &status);
+	if (err == 1)
+		err = s35390a_init(s35390a, &status);
+
+	if (err < 0)
+		return err;
 
 	buf[S35390A_BYTE_YEAR] = bin2bcd(tm->tm_year - 100);
 	buf[S35390A_BYTE_MONTH] = bin2bcd(tm->tm_mon + 1);
 	buf[S35390A_BYTE_DAY] = bin2bcd(tm->tm_mday);
 	buf[S35390A_BYTE_WDAY] = bin2bcd(tm->tm_wday);
-	buf[S35390A_BYTE_HOURS] = s35390a_hr2reg(s35390a, tm->tm_hour);
+	buf[S35390A_BYTE_HOURS] = s35390a_hr2reg(tm->tm_hour, status & S35390A_FLAG_24H);
 	buf[S35390A_BYTE_MINS] = bin2bcd(tm->tm_min);
 	buf[S35390A_BYTE_SECS] = bin2bcd(tm->tm_sec);
 
@@ -243,8 +245,11 @@ static int s35390a_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	u8 buf[7], status;
 	int i, err;
 
-	if (s35390a_read_status(s35390a, &status) == 1)
+	err = s35390a_read_status(s35390a, &status);
+	if (err == 1)
 		return -EINVAL;
+	if (err < 0)
+		return err;
 
 	err = s35390a_get_reg(s35390a, S35390A_CMD_TIME1, buf, sizeof(buf));
 	if (err < 0)
@@ -256,7 +261,7 @@ static int s35390a_rtc_read_time(struct device *dev, struct rtc_time *tm)
 
 	tm->tm_sec = bcd2bin(buf[S35390A_BYTE_SECS]);
 	tm->tm_min = bcd2bin(buf[S35390A_BYTE_MINS]);
-	tm->tm_hour = s35390a_reg2hr(s35390a, buf[S35390A_BYTE_HOURS]);
+	tm->tm_hour = s35390a_reg2hr(buf[S35390A_BYTE_HOURS], status & S35390A_FLAG_24H);
 	tm->tm_wday = bcd2bin(buf[S35390A_BYTE_WDAY]);
 	tm->tm_mday = bcd2bin(buf[S35390A_BYTE_DAY]);
 	tm->tm_mon = bcd2bin(buf[S35390A_BYTE_MONTH]) - 1;
@@ -292,7 +297,7 @@ static int s35390a_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alm)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	struct s35390a *s35390a = i2c_get_clientdata(client);
-	u8 buf[3], sts = 0;
+	u8 buf[3], status1, status2 = 0;
 	int err, i;
 
 	dev_dbg(&client->dev, "%s: alm is secs=%d, mins=%d, hours=%d mday=%d, "\
@@ -301,22 +306,22 @@ static int s35390a_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alm)
 		alm->time.tm_mon, alm->time.tm_year, alm->time.tm_wday);
 
 	/* disable interrupt (which deasserts the irq line) */
-	err = s35390a_set_reg(s35390a, S35390A_CMD_STATUS2, &sts, sizeof(sts));
+	err = s35390a_set_reg(s35390a, S35390A_CMD_STATUS2, &status2, sizeof(status2));
 	if (err < 0)
 		return err;
 
 	/* clear pending interrupt (in STATUS1 only), if any */
-	err = s35390a_get_reg(s35390a, S35390A_CMD_STATUS1, &sts, sizeof(sts));
+	err = s35390a_get_reg(s35390a, S35390A_CMD_STATUS1, &status1, sizeof(status1));
 	if (err < 0)
 		return err;
 
 	if (alm->enabled)
-		sts = S35390A_INT2_MODE_ALARM;
+		status2 = S35390A_INT2_MODE_ALARM;
 	else
-		sts = S35390A_INT2_MODE_NOINTR;
+		status2 = S35390A_INT2_MODE_NOINTR;
 
 	/* set interrupt mode*/
-	err = s35390a_set_reg(s35390a, S35390A_CMD_STATUS2, &sts, sizeof(sts));
+	err = s35390a_set_reg(s35390a, S35390A_CMD_STATUS2, &status2, sizeof(status2));
 	if (err < 0)
 		return err;
 
@@ -325,8 +330,8 @@ static int s35390a_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alm)
 	else
 		buf[S35390A_ALRM_BYTE_WDAY] = 0;
 
-	buf[S35390A_ALRM_BYTE_HOURS] = s35390a_hr2reg(s35390a,
-			alm->time.tm_hour) | 0x80;
+	buf[S35390A_ALRM_BYTE_HOURS] = s35390a_hr2reg(alm->time.tm_hour,
+						      status1 & S35390A_FLAG_24H) | 0x80;
 	buf[S35390A_ALRM_BYTE_MINS] = bin2bcd(alm->time.tm_min) | 0x80;
 
 	if (alm->time.tm_hour >= 12)
@@ -345,14 +350,20 @@ static int s35390a_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alm)
 {
 	struct i2c_client *client = to_i2c_client(dev);
 	struct s35390a *s35390a = i2c_get_clientdata(client);
-	u8 buf[3], sts;
+	u8 buf[3], status1, status2;
 	int i, err;
 
-	err = s35390a_get_reg(s35390a, S35390A_CMD_STATUS2, &sts, sizeof(sts));
+	err = s35390a_read_status(s35390a, &status1);
+	if (err == 1)
+		return -EINVAL;
+	if (err < 0)
+		return err;
+
+	err = s35390a_get_reg(s35390a, S35390A_CMD_STATUS2, &status2, sizeof(status2));
 	if (err < 0)
 		return err;
 
-	if ((sts & S35390A_INT2_MODE_MASK) != S35390A_INT2_MODE_ALARM) {
+	if ((status2 & S35390A_INT2_MODE_MASK) != S35390A_INT2_MODE_ALARM) {
 		/*
 		 * When the alarm isn't enabled, the register to configure
 		 * the alarm time isn't accessible.
@@ -381,8 +392,8 @@ static int s35390a_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alm)
 
 	if (buf[S35390A_ALRM_BYTE_HOURS] & 0x80)
 		alm->time.tm_hour =
-			s35390a_reg2hr(s35390a,
-				       buf[S35390A_ALRM_BYTE_HOURS] & ~0x80);
+			s35390a_reg2hr(buf[S35390A_ALRM_BYTE_HOURS] & ~0x80,
+				       status1 & S35390A_FLAG_24H);
 
 	if (buf[S35390A_ALRM_BYTE_MINS] & 0x80)
 		alm->time.tm_min = bcd2bin(buf[S35390A_ALRM_BYTE_MINS] & ~0x80);
@@ -416,7 +427,7 @@ static int s35390a_rtc_ioctl(struct device *dev, unsigned int cmd,
 		break;
 	case RTC_VL_CLR:
 		/* update flag and clear register */
-		err = s35390a_init(s35390a);
+		err = s35390a_init(s35390a, &sts);
 		if (err < 0)
 			return err;
 		break;
@@ -503,11 +514,6 @@ static int s35390a_probe(struct i2c_client *client)
 		return err_read;
 	}
 
-	if (status1 & S35390A_FLAG_24H)
-		s35390a->twentyfourhour = 1;
-	else
-		s35390a->twentyfourhour = 0;
-
 	if (status1 & S35390A_FLAG_INT2) {
 		/* disable alarm (and maybe test mode) */
 		buf = 0;

-- 
2.55.0

[PATCH v9 5/6] rtc: s35390a: Add pinctrl

[Markus Probst]

Allow configuration of pin functions, including:
- keeping the previous configured function
- disabling output
- waking up the system
- minute periodical interrupt (1: 50% duty, 2: 7.81 ms)

If no function has been configured, fallback to previous behaviour.

Reviewed-by: Linus Walleij <linusw@kernel.org>
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 drivers/rtc/Kconfig       |   2 +
 drivers/rtc/rtc-s35390a.c | 375 ++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 313 insertions(+), 64 deletions(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 05b9233b9418..c73c7d9751d9 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -668,7 +668,9 @@ config RTC_DRV_RC5T619
 
 config RTC_DRV_S35390A
 	tristate "Seiko Instruments S-35390A"
+	select PINCTRL
 	select BITREVERSE
+	select GENERIC_PINCONF
 	help
 	  If you say yes here you will get support for the Seiko
 	  Instruments S-35390A.
diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
index 12ed504db94d..649d36d37f1f 100644
--- a/drivers/rtc/rtc-s35390a.c
+++ b/drivers/rtc/rtc-s35390a.c
@@ -12,11 +12,17 @@
 #include <linux/bcd.h>
 #include <linux/slab.h>
 #include <linux/delay.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/pinctrl/pinconf-generic.h>
+
+#define DRIVER_NAME "rtc-s35390a"
 
 #define S35390A_CMD_STATUS1	0
 #define S35390A_CMD_STATUS2	1
 #define S35390A_CMD_TIME1	2
 #define S35390A_CMD_TIME2	3
+#define S35390A_CMD_INT1_REG1	4
 #define S35390A_CMD_INT2_REG1	5
 #define S35390A_CMD_FREE_REG    7
 
@@ -36,19 +42,38 @@
 #define S35390A_FLAG_POC	BIT(0)
 #define S35390A_FLAG_BLD	BIT(1)
 #define S35390A_FLAG_INT2	BIT(2)
+#define S35390A_FLAG_INT1	BIT(3)
 #define S35390A_FLAG_24H	BIT(6)
 #define S35390A_FLAG_RESET	BIT(7)
 
 /* flag for STATUS2 */
 #define S35390A_FLAG_TEST	BIT(0)
 
+#define S35390A_INT_MODE_NOINTR	0x00
+
 /* INT2 pin output mode */
 #define S35390A_INT2_MODE_MASK		0x0E
-#define S35390A_INT2_MODE_NOINTR	0x00
 #define S35390A_INT2_MODE_ALARM		BIT(1) /* INT2AE */
 #define S35390A_INT2_MODE_PMIN_EDG	BIT(2) /* INT2ME */
 #define S35390A_INT2_MODE_FREQ		BIT(3) /* INT2FE */
-#define S35390A_INT2_MODE_PMIN		(BIT(3) | BIT(2)) /* INT2FE | INT2ME */
+#define S35390A_INT2_MODE_PMIN1		(BIT(3) | BIT(2)) /* INT2FE | INT2ME */
+
+/* INT1 pin output mode */
+#define S35390A_INT1_MODE_MASK		0xF0
+#define S35390A_INT1_MODE_ALARM		BIT(5) /* INT1AE */
+#define S35390A_INT1_MODE_PMIN_EDG	BIT(6) /* INT1ME */
+#define S35390A_INT1_MODE_FREQ		BIT(7) /* INT1FE */
+#define S35390A_INT1_MODE_PMIN1		(BIT(7) | BIT(6)) /* INT1FE | INT1ME */
+#define S35390A_INT1_MODE_PMIN2		(BIT(7) | BIT(6) | BIT(5)) /* INT1FE | INT1ME | INT1AE */
+#define S35390A_INT1_MODE_32768KHZ	BIT(4) /* 32kE */
+
+#define S35390A_FUNC_IGNORE		0x00
+#define S35390A_FUNC_DISABLE		0x01
+#define S35390A_FUNC_WAKEUP		0x02
+#define S35390A_FUNC_CLOCK		0x03
+#define S35390A_FUNC_PMIN1		0x04
+#define S35390A_FUNC_PMIN2		0x05
+
 
 static const struct i2c_device_id s35390a_id[] = {
 	{ .name = "s35390a" },
@@ -64,6 +89,10 @@ MODULE_DEVICE_TABLE(of, s35390a_of_match);
 
 struct s35390a {
 	struct i2c_client *client[8];
+	struct rtc_device *rtc;
+
+	struct mutex pinfunction_lock; /* lock preventing concurrent access of pin function */
+	int pinfunction[2];
 };
 
 static int s35390a_set_reg(struct s35390a *s35390a, int reg, u8  *buf, int len)
@@ -165,20 +194,6 @@ static int s35390a_read_status(struct s35390a *s35390a, char *status1)
 	return 0;
 }
 
-static int s35390a_disable_test_mode(struct s35390a *s35390a)
-{
-	u8 buf[1];
-
-	if (s35390a_get_reg(s35390a, S35390A_CMD_STATUS2, buf, sizeof(buf)) < 0)
-		return -EIO;
-
-	if (!(buf[0] & S35390A_FLAG_TEST))
-		return 0;
-
-	buf[0] &= ~S35390A_FLAG_TEST;
-	return s35390a_set_reg(s35390a, S35390A_CMD_STATUS2, buf, sizeof(buf));
-}
-
 static char s35390a_hr2reg(int hour, bool twentyfourhour)
 {
 	if (twentyfourhour)
@@ -281,10 +296,25 @@ static int s35390a_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled
 	u8 sts;
 	int err;
 
-	if (enabled)
-		sts = S35390A_INT2_MODE_ALARM;
-	else
-		sts = S35390A_INT2_MODE_NOINTR;
+	guard(mutex)(&s35390a->pinfunction_lock);
+
+	err = s35390a_get_reg(s35390a, S35390A_CMD_STATUS2, &sts, sizeof(sts));
+	if (err < 0)
+		return err;
+
+	if (enabled) {
+		if (s35390a->pinfunction[0] == S35390A_FUNC_WAKEUP)
+			sts = (sts & ~S35390A_INT1_MODE_MASK) | S35390A_INT1_MODE_ALARM;
+
+		if (s35390a->pinfunction[1] == S35390A_FUNC_WAKEUP)
+			sts = (sts & ~S35390A_INT2_MODE_MASK) | S35390A_INT2_MODE_ALARM;
+	} else {
+		if (s35390a->pinfunction[0] == S35390A_FUNC_WAKEUP)
+			sts = (sts & ~S35390A_INT1_MODE_MASK) | S35390A_INT_MODE_NOINTR;
+
+		if (s35390a->pinfunction[1] == S35390A_FUNC_WAKEUP)
+			sts = (sts & ~S35390A_INT2_MODE_MASK) | S35390A_INT_MODE_NOINTR;
+	}
 
 	err = s35390a_set_reg(s35390a, S35390A_CMD_STATUS2, &sts, sizeof(sts));
 	if (err < 0)
@@ -305,7 +335,19 @@ static int s35390a_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alm)
 		alm->time.tm_min, alm->time.tm_hour, alm->time.tm_mday,
 		alm->time.tm_mon, alm->time.tm_year, alm->time.tm_wday);
 
+	guard(mutex)(&s35390a->pinfunction_lock);
+
+	err = s35390a_get_reg(s35390a, S35390A_CMD_STATUS2, &status2, sizeof(status2));
+	if (err < 0)
+		return err;
+
 	/* disable interrupt (which deasserts the irq line) */
+	if (s35390a->pinfunction[0] == S35390A_FUNC_WAKEUP)
+		status2 = (status2 & ~S35390A_INT1_MODE_MASK) | S35390A_INT_MODE_NOINTR;
+
+	if (s35390a->pinfunction[1] == S35390A_FUNC_WAKEUP)
+		status2 = (status2 & ~S35390A_INT2_MODE_MASK) | S35390A_INT_MODE_NOINTR;
+
 	err = s35390a_set_reg(s35390a, S35390A_CMD_STATUS2, &status2, sizeof(status2));
 	if (err < 0)
 		return err;
@@ -315,16 +357,6 @@ static int s35390a_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alm)
 	if (err < 0)
 		return err;
 
-	if (alm->enabled)
-		status2 = S35390A_INT2_MODE_ALARM;
-	else
-		status2 = S35390A_INT2_MODE_NOINTR;
-
-	/* set interrupt mode*/
-	err = s35390a_set_reg(s35390a, S35390A_CMD_STATUS2, &status2, sizeof(status2));
-	if (err < 0)
-		return err;
-
 	if (alm->time.tm_wday != -1)
 		buf[S35390A_ALRM_BYTE_WDAY] = bin2bcd(alm->time.tm_wday) | 0x80;
 	else
@@ -340,10 +372,32 @@ static int s35390a_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alm)
 	for (i = 0; i < 3; ++i)
 		buf[i] = bitrev8(buf[i]);
 
-	err = s35390a_set_reg(s35390a, S35390A_CMD_INT2_REG1, buf,
-								sizeof(buf));
+	if (alm->enabled) {
+		/* set interrupt mode */
+		if (s35390a->pinfunction[0] == S35390A_FUNC_WAKEUP)
+			status2 = (status2 & ~S35390A_INT1_MODE_MASK) | S35390A_INT1_MODE_ALARM;
+
+		if (s35390a->pinfunction[1] == S35390A_FUNC_WAKEUP)
+			status2 = (status2 & ~S35390A_INT2_MODE_MASK) | S35390A_INT2_MODE_ALARM;
 
-	return err;
+		err = s35390a_set_reg(s35390a, S35390A_CMD_STATUS2, &status2, sizeof(status2));
+		if (err < 0)
+			return err;
+	}
+
+	if (s35390a->pinfunction[0] == S35390A_FUNC_WAKEUP) {
+		err = s35390a_set_reg(s35390a, S35390A_CMD_INT1_REG1, buf, sizeof(buf));
+		if (err < 0)
+			return err;
+	}
+
+	if (s35390a->pinfunction[1] == S35390A_FUNC_WAKEUP) {
+		err = s35390a_set_reg(s35390a, S35390A_CMD_INT2_REG1, buf, sizeof(buf));
+		if (err < 0)
+			return err;
+	}
+
+	return 0;
 }
 
 static int s35390a_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alm)
@@ -351,7 +405,9 @@ static int s35390a_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alm)
 	struct i2c_client *client = to_i2c_client(dev);
 	struct s35390a *s35390a = i2c_get_clientdata(client);
 	u8 buf[3], status1, status2;
-	int i, err;
+	int i, err, reg;
+
+	guard(mutex)(&s35390a->pinfunction_lock);
 
 	err = s35390a_read_status(s35390a, &status1);
 	if (err == 1)
@@ -363,18 +419,24 @@ static int s35390a_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alm)
 	if (err < 0)
 		return err;
 
-	if ((status2 & S35390A_INT2_MODE_MASK) != S35390A_INT2_MODE_ALARM) {
+	if (s35390a->pinfunction[1] == S35390A_FUNC_WAKEUP &&
+	    (status2 & S35390A_INT2_MODE_MASK) == S35390A_INT2_MODE_ALARM) {
+		reg = S35390A_CMD_INT2_REG1;
+	} else if (s35390a->pinfunction[0] == S35390A_FUNC_WAKEUP &&
+		   (status2 & S35390A_INT1_MODE_MASK) == S35390A_INT1_MODE_ALARM) {
+		reg = S35390A_CMD_INT1_REG1;
+	} else {
 		/*
 		 * When the alarm isn't enabled, the register to configure
 		 * the alarm time isn't accessible.
 		 */
 		alm->enabled = 0;
 		return 0;
-	} else {
-		alm->enabled = 1;
 	}
 
-	err = s35390a_get_reg(s35390a, S35390A_CMD_INT2_REG1, buf, sizeof(buf));
+	alm->enabled = 1;
+
+	err = s35390a_get_reg(s35390a, reg, buf, sizeof(buf));
 	if (err < 0)
 		return err;
 
@@ -383,7 +445,7 @@ static int s35390a_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alm)
 		buf[i] = bitrev8(buf[i]);
 
 	/*
-	 * B0 of the three matching registers is an enable flag. Iff it is set
+	 * B0 of the three matching registers is an enable flag. If it is set
 	 * the configured value is used for matching.
 	 */
 	if (buf[S35390A_ALRM_BYTE_WDAY] & 0x80)
@@ -464,13 +526,168 @@ static int s35390a_nvmem_write(void *priv, unsigned int offset, void *val,
 	return s35390a_set_reg(s35390a, S35390A_CMD_FREE_REG, val, bytes);
 }
 
+static const struct pinctrl_pin_desc s35390a_pins_desc[] = {
+	PINCTRL_PIN(0, "int1"),
+	PINCTRL_PIN(1, "int2"),
+};
+
+static const unsigned int int1_pins[] = { 0 };
+static const unsigned int int2_pins[] = { 1 };
+
+static const struct pingroup s35390a_pin_groups[] = {
+	PINCTRL_PINGROUP("int1_grp", int1_pins, ARRAY_SIZE(int1_pins)),
+	PINCTRL_PINGROUP("int2_grp", int2_pins, ARRAY_SIZE(int2_pins)),
+};
+
+static int s35390a_pinctrl_get_groups_count(struct pinctrl_dev *pctldev)
+{
+	return ARRAY_SIZE(s35390a_pin_groups);
+}
+
+static const char *s35390a_pinctrl_get_group_name(struct pinctrl_dev *pctldev,
+						  unsigned int group)
+{
+	return s35390a_pin_groups[group].name;
+}
+
+static int s35390a_pinctrl_get_group_pins(struct pinctrl_dev *pctldev, unsigned int selector,
+					  const unsigned int **pins, unsigned int *npins)
+{
+	*pins = s35390a_pin_groups[selector].pins;
+	*npins = s35390a_pin_groups[selector].npins;
+	return 0;
+}
+
+static const char * const all_groups[] = { "int1_grp", "int2_grp" };
+static const char * const int1_groups[] = { "int1_grp" };
+
+static const struct pinfunction s35390a_functions[] = {
+	[S35390A_FUNC_IGNORE] = PINCTRL_PINFUNCTION("ignore", all_groups, ARRAY_SIZE(all_groups)),
+	[S35390A_FUNC_DISABLE] = PINCTRL_PINFUNCTION("disable", all_groups, ARRAY_SIZE(all_groups)),
+	[S35390A_FUNC_WAKEUP] = PINCTRL_PINFUNCTION("wakeup", all_groups, ARRAY_SIZE(all_groups)),
+	[S35390A_FUNC_CLOCK] = PINCTRL_PINFUNCTION("clock", all_groups, ARRAY_SIZE(all_groups)),
+	[S35390A_FUNC_PMIN1] = PINCTRL_PINFUNCTION("pmin1", all_groups, ARRAY_SIZE(all_groups)),
+	[S35390A_FUNC_PMIN2] = PINCTRL_PINFUNCTION("pmin2", int1_groups, ARRAY_SIZE(int1_groups)),
+};
+
+static int s35390a_pinctrl_get_functions_count(struct pinctrl_dev *pctldev)
+{
+	return ARRAY_SIZE(s35390a_functions);
+}
+
+static const char *s35390a_pinctrl_get_function_name(struct pinctrl_dev *pctldev,
+						     unsigned int selector)
+{
+	return s35390a_functions[selector].name;
+}
+
+static int s35390a_pinctrl_get_function_groups(struct pinctrl_dev *pctldev, unsigned int selector,
+					       const char * const **groups,
+					       unsigned int * const ngroups)
+{
+	*groups = s35390a_functions[selector].groups;
+	*ngroups = s35390a_functions[selector].ngroups;
+	return 0;
+}
+
+static int s35390a_pinctrl_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
+				   unsigned int group)
+{
+	int err;
+	u8 status2, flag, mask;
+	struct s35390a *s35390a = pinctrl_dev_get_drvdata(pctldev);
+
+	mask = group == 0 ? S35390A_INT1_MODE_MASK : S35390A_INT2_MODE_MASK;
+
+	guard(mutex)(&s35390a->pinfunction_lock);
+
+	dev_dbg(&s35390a->client[0]->dev, "%s: function=%d group=%d\n",
+		__func__, function, group);
+
+	if (function == s35390a->pinfunction[group])
+		return 0;
+
+	if (function != S35390A_FUNC_IGNORE) {
+		err = s35390a_get_reg(s35390a, S35390A_CMD_STATUS2, &status2, 1);
+		if (err < 0) {
+			dev_err(&s35390a->client[0]->dev, "error reading status\n");
+			return err;
+		}
+
+		switch (function) {
+		case S35390A_FUNC_DISABLE:
+		case S35390A_FUNC_CLOCK: /* not implemented */
+			status2 = (status2 & ~mask) | S35390A_INT_MODE_NOINTR;
+			break;
+		case S35390A_FUNC_WAKEUP:
+			flag = group == 0 ? S35390A_INT1_MODE_ALARM : S35390A_INT2_MODE_ALARM;
+
+			if ((status2 & mask) != flag)
+				status2 = (status2 & ~mask) | S35390A_INT_MODE_NOINTR;
+
+			break;
+		case S35390A_FUNC_PMIN1:
+			flag = group == 0 ? S35390A_INT1_MODE_PMIN1 : S35390A_INT2_MODE_PMIN1;
+			status2 = (status2 & ~mask) | flag;
+			break;
+
+		/* INT1 only modes */
+		case S35390A_FUNC_PMIN2:
+			if (group == 1)
+				return -EINVAL;
+
+			status2 = (status2 & ~mask) | S35390A_INT1_MODE_PMIN2;
+			break;
+		}
+
+		err = s35390a_set_reg(s35390a, S35390A_CMD_STATUS2, &status2, 1);
+		if (err < 0) {
+			dev_err(&s35390a->client[0]->dev, "error setting interrupts\n");
+			return err;
+		}
+	}
+
+	s35390a->pinfunction[group] = function;
+
+	return 0;
+}
+
+static const struct pinctrl_ops s35390a_pinctrl_ops = {
+	.get_groups_count = s35390a_pinctrl_get_groups_count,
+	.get_group_name = s35390a_pinctrl_get_group_name,
+	.get_group_pins = s35390a_pinctrl_get_group_pins,
+#if IS_ENABLED(CONFIG_OF)
+	.dt_node_to_map = pinconf_generic_dt_node_to_map_all,
+	.dt_free_map = pinconf_generic_dt_free_map
+#endif
+};
+
+static const struct pinmux_ops s35390a_pinmux_ops = {
+	.get_functions_count = s35390a_pinctrl_get_functions_count,
+	.get_function_name = s35390a_pinctrl_get_function_name,
+	.get_function_groups = s35390a_pinctrl_get_function_groups,
+	.set_mux = s35390a_pinctrl_set_mux,
+	.strict = true,
+};
+
+static struct pinctrl_desc s35390a_pinctrl_desc = {
+	.name = DRIVER_NAME,
+	.pins = s35390a_pins_desc,
+	.npins = ARRAY_SIZE(s35390a_pins_desc),
+	.pctlops = &s35390a_pinctrl_ops,
+	.pmxops = &s35390a_pinmux_ops,
+	.owner = THIS_MODULE,
+};
+
 static int s35390a_probe(struct i2c_client *client)
 {
-	int err, err_read;
+	int err;
 	unsigned int i;
 	struct s35390a *s35390a;
 	struct rtc_device *rtc;
-	u8 buf, status1;
+	struct pinctrl_dev *pctl;
+	u8 status1, status2;
+	bool irq = false;
 	struct device *dev = &client->dev;
 	struct nvmem_config nvmem_cfg = {
 		.name = "s35390a_nvram",
@@ -481,6 +698,7 @@ static int s35390a_probe(struct i2c_client *client)
 		.reg_read = s35390a_nvmem_read,
 		.reg_write = s35390a_nvmem_write,
 	};
+	int fallback[ARRAY_SIZE(s35390a_pin_groups)];
 
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
 		return -ENODEV;
@@ -489,7 +707,11 @@ static int s35390a_probe(struct i2c_client *client)
 	if (!s35390a)
 		return -ENOMEM;
 
+	mutex_init(&s35390a->pinfunction_lock);
+	memset(s35390a->pinfunction, -1, sizeof(s35390a->pinfunction));
+
 	s35390a->client[0] = client;
+
 	i2c_set_clientdata(client, s35390a);
 
 	/* This chip uses multiple addresses, use dummy devices for them */
@@ -504,33 +726,34 @@ static int s35390a_probe(struct i2c_client *client)
 		}
 	}
 
-	rtc = devm_rtc_allocate_device(dev);
-	if (IS_ERR(rtc))
-		return PTR_ERR(rtc);
-
-	err_read = s35390a_read_status(s35390a, &status1);
-	if (err_read < 0) {
+	err = s35390a_read_status(s35390a, &status1);
+	if (err < 0) {
 		dev_err(dev, "error resetting chip\n");
-		return err_read;
+		return err;
+	}
+
+	err = s35390a_get_reg(s35390a, S35390A_CMD_STATUS2, &status2, sizeof(status2));
+	if (err < 0)
+		return dev_err_probe(dev, err, "disabling alarm and test mode failed\n");
+
+	if (status1 & S35390A_FLAG_INT1) {
+		status2 = (status2 & ~S35390A_INT1_MODE_MASK) | S35390A_INT_MODE_NOINTR;
+		irq = true;
 	}
 
 	if (status1 & S35390A_FLAG_INT2) {
-		/* disable alarm (and maybe test mode) */
-		buf = 0;
-		err = s35390a_set_reg(s35390a, S35390A_CMD_STATUS2, &buf, 1);
-		if (err < 0) {
-			dev_err(dev, "error disabling alarm\n");
-			return err;
-		}
-	} else {
-		err = s35390a_disable_test_mode(s35390a);
-		if (err < 0) {
-			dev_err(dev, "error disabling test mode\n");
-			return err;
-		}
+		status2 = (status2 & ~S35390A_INT2_MODE_MASK) | S35390A_INT_MODE_NOINTR;
+		irq = true;
 	}
 
-	device_set_wakeup_capable(dev, 1);
+	status2 &= ~S35390A_FLAG_TEST;
+	err = s35390a_set_reg(s35390a, S35390A_CMD_STATUS2, &status2, sizeof(status2));
+	if (err < 0)
+		return dev_err_probe(dev, err, "disabling alarm and test mode failed\n");
+
+	rtc = devm_rtc_allocate_device(dev);
+	if (IS_ERR(rtc))
+		return PTR_ERR(rtc);
 
 	rtc->ops = &s35390a_rtc_ops;
 	rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
@@ -539,9 +762,33 @@ static int s35390a_probe(struct i2c_client *client)
 	set_bit(RTC_FEATURE_ALARM_RES_MINUTE, rtc->features);
 	clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->features);
 
-	if (status1 & S35390A_FLAG_INT2)
+	s35390a->rtc = rtc;
+
+	device_set_wakeup_capable(dev, 1);
+
+	if (irq)
 		rtc_update_irq(rtc, 1, RTC_AF);
 
+	err = devm_pinctrl_register_and_init(dev, &s35390a_pinctrl_desc, s35390a, &pctl);
+	if (err)
+		return dev_err_probe(dev, err, "pinctrl register failed\n");
+
+	err = pinctrl_enable(pctl);
+	if (err)
+		return dev_err_probe(dev, err, "pinctrl enable failed\n");
+
+	/* If no pinmux function is defined in DT, fallback to previous behaviour */
+	fallback[0] = S35390A_FUNC_IGNORE;
+	fallback[1] = S35390A_FUNC_WAKEUP;
+
+	for (i = 0; i < ARRAY_SIZE(s35390a_pin_groups); i++) {
+		if (s35390a->pinfunction[i] == -1) {
+			err = s35390a_pinctrl_set_mux(pctl, fallback[i], i);
+			if (err)
+				return err;
+		}
+	}
+
 	nvmem_cfg.priv = s35390a;
 	err = devm_rtc_nvmem_register(rtc, &nvmem_cfg);
 	if (err)
@@ -552,7 +799,7 @@ static int s35390a_probe(struct i2c_client *client)
 
 static struct i2c_driver s35390a_driver = {
 	.driver		= {
-		.name	= "rtc-s35390a",
+		.name	= DRIVER_NAME,
 		.of_match_table = of_match_ptr(s35390a_of_match),
 	},
 	.probe		= s35390a_probe,

-- 
2.55.0

[PATCH v9 6/6] rtc: s35390a: Add synology quirk

[Markus Probst]

Synology NAS devices use interrupt signal 1 for wakeup alarms. On ACPI
it is not possible to configure pinctrl.

Use interrupt signal 1 for wakeup if no pinmux function has been
configured in devicetree and dmi sys vendor matches "Synology Inc.".

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 drivers/rtc/rtc-s35390a.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
index 649d36d37f1f..d3e829052927 100644
--- a/drivers/rtc/rtc-s35390a.c
+++ b/drivers/rtc/rtc-s35390a.c
@@ -5,6 +5,7 @@
  * Copyright (c) 2007 Byron Bradley
  */
 
+#include <linux/dmi.h>
 #include <linux/module.h>
 #include <linux/rtc.h>
 #include <linux/i2c.h>
@@ -778,8 +779,18 @@ static int s35390a_probe(struct i2c_client *client)
 		return dev_err_probe(dev, err, "pinctrl enable failed\n");
 
 	/* If no pinmux function is defined in DT, fallback to previous behaviour */
-	fallback[0] = S35390A_FUNC_IGNORE;
-	fallback[1] = S35390A_FUNC_WAKEUP;
+
+	if (dmi_match(DMI_SYS_VENDOR, "Synology Inc.")) {
+		/*
+		 * Synology uses interrupt signal 1 for wakeup. Since pinctrl cannot be
+		 * configured on ACPI, we check against the dmi sys vendor.
+		 */
+		fallback[0] = S35390A_FUNC_WAKEUP;
+		fallback[1] = S35390A_FUNC_DISABLE;
+	} else {
+		fallback[0] = S35390A_FUNC_IGNORE;
+		fallback[1] = S35390A_FUNC_WAKEUP;
+	}
 
 	for (i = 0; i < ARRAY_SIZE(s35390a_pin_groups); i++) {
 		if (s35390a->pinfunction[i] == -1) {

-- 
2.55.0

Re: [PATCH v9 2/6] rtc: s35390a: Add missing newline to dev_err

[sashiko-bot]

> Fixes: 3bd32722c827d ("rtc: s35390a: improve irq handling")
> Signed-off-by: Markus Probst <markus.probst@posteo.de>

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916-rtc_s35390a_int1-v9-0-b2567a3e4959@posteo.de?part=2

Re: [PATCH v9 6/6] rtc: s35390a: Add synology quirk

[sashiko-bot]

> Synology NAS devices use interrupt signal 1 for wakeup alarms. On ACPI
> it is not possible to configure pinctrl.
> 
> Use interrupt signal 1 for wakeup if no pinmux function has been
> configured in devicetree and dmi sys vendor matches "Synology Inc.".
> 
> Signed-off-by: Markus Probst <markus.probst@posteo.de>

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916-rtc_s35390a_int1-v9-0-b2567a3e4959@posteo.de?part=6

Re: [PATCH v9 1/6] dt-bindings: rtc: Add pinctrl for S35390A

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] dt-bindings: missing #clock-cells in required list
- [Medium] dt-bindings: undocumented clock provider capability
- [Medium] dt-bindings: missing constraint for pmin2 pin function
--

commit ee787a668e377f31a7722c1acf8927b88011aa60
Author: Markus Probst <markus.probst@posteo.de>
Subject: dt-bindings: rtc: Add pinctrl for S35390A

This commit moves the devicetree bindings for the S-35390A RTC into its own
schema file. It adds the necessary properties to configure individual pins via
pinctrl, allowing the interrupt signal to be used for a wakeup alarm.

> diff --git a/Documentation/devicetree/bindings/rtc/sii,s35390a.yaml b/Documentation/devicetree/bindings/rtc/sii,s35390a.yaml
> new file mode 100644
> index 0000000000000..f3bf5900fb3ce
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/sii,s35390a.yaml
> @@ -0,0 +1,121 @@

[ ... ]

> +  wakeup-parent: true
> +
> +  wakeup-source: true
> +
> +  "#clock-cells":
> +    const: 1

[Severity: Medium]
The commit message states the change adds pinctrl to allow the interrupt
signal 1 to be used for a wakeup alarm. However, the schema also introduces
"#clock-cells" and a "clock" output pin function.

Does this introduce a significant undocumented capability (clock provider
functionality) which is unstated in the commit description?

[ ... ]

> +          function:
> +            description: |
> +              Pin function:
> +              - ignore:  Preserve the previous state.
> +              - disable: Disable pin output.
> +              - wakeup:  Output wakes up the system.
> +              - clock:   Output clock pulse.
> +              - pmin1:   Minute periodical output with 50% duty.
> +              - pmin2:   Minute periodical output L for 7.81 ms.
> +                         Can only be used with pin int1.
> +            enum:
> +              - ignore
> +              - disable
> +              - wakeup
> +              - clock
> +              - pmin1
> +              - pmin2

[Severity: Medium]
The description for the "pmin2" function explicitly states it can only be used
with pin int1.

Because there is no conditional allOf/if/then block at the pinmux node level
linking them, will a device tree incorrectly using pins = "int2"; function =
"pmin2"; silently pass schema validation despite being an invalid hardware
configuration?

[ ... ]

> +required:
> +  - compatible
> +  - reg

[Severity: Medium]
According to the dt-bindings subsystem rules, when hardware gains new provider
capabilities, the corresponding standard properties must be added to the
required list with appropriate constraints.

Should "#clock-cells" be added to this required list to prevent incomplete
device tree nodes from silently passing schema validation?

> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        rtc@30 {

[Severity: Medium]
According to the dt-bindings subsystem rules, when hardware gains new provider
capabilities, the corresponding standard properties must appear in the examples.

Should the example device tree node be updated to include the required
"#clock-cells" property?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916-rtc_s35390a_int1-v9-0-b2567a3e4959@posteo.de?part=1

Re: [PATCH v9 4/6] rtc: s35390a: Read 24-hour mode on access

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] rtc: s35390a: read_alarm destructively clears pending interrupts and power-loss flags
--

commit ca86574fc774e3c6f6f2d2629b0a0ab8635be21f
Author: Markus Probst <markus.probst@posteo.de>

rtc: s35390a: Read 24-hour mode on access

This patch modifies the s35390a driver to read the 24-hour mode flag
dynamically on access rather than caching it during probe. It aims to prevent
out-of-sync issues when the chip is reset while in 12-hour mode.

> diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
> index 575bb256eb258..12ed504db94d8 100644
> --- a/drivers/rtc/rtc-s35390a.c
> +++ b/drivers/rtc/rtc-s35390a.c
[ ... ]
> @@ -345,14 +350,20 @@ static int s35390a_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alm)
>  {
>  	struct i2c_client *client = to_i2c_client(dev);
>  	struct s35390a *s35390a = i2c_get_clientdata(client);
> -	u8 buf[3], sts;
> +	u8 buf[3], status1, status2;
>  	int i, err;
>  
> -	err = s35390a_get_reg(s35390a, S35390A_CMD_STATUS2, &sts, sizeof(sts));
> +	err = s35390a_read_status(s35390a, &status1);

[Severity: High]
Does this new s35390a_read_status() call destructively clear pending interrupts
and power-loss flags?

When s35390a_read_status() executes, it issues an I2C read against the
S35390A_CMD_STATUS1 register. According to the S-35390A hardware behavior,
reading STATUS1 automatically clears the POC (Power-On Clear), BLD (Battery Low
Detection), INT1, and INT2 flags.

Because s35390a_rtc_read_alarm() does not handle these flags after reading
STATUS1, it appears this would inadvertently erase any pending alarm wake-up
events and silently discard power-loss faults. 

Since reading an alarm can be triggered by unprivileged user-space applications
via sysfs or RTC ioctls, could this allow a trivial denial of service for RTC
alarms and power-loss detection?

> +	if (err == 1)
> +		return -EINVAL;
> +	if (err < 0)
> +		return err;
> +
> +	err = s35390a_get_reg(s35390a, S35390A_CMD_STATUS2, &status2, sizeof(status2));
>  	if (err < 0)
>  		return err;
>  
> -	if ((sts & S35390A_INT2_MODE_MASK) != S35390A_INT2_MODE_ALARM) {
> +	if ((status2 & S35390A_INT2_MODE_MASK) != S35390A_INT2_MODE_ALARM) {
>  		/*
>  		 * When the alarm isn't enabled, the register to configure
>  		 * the alarm time isn't accessible.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916-rtc_s35390a_int1-v9-0-b2567a3e4959@posteo.de?part=4

Re: [PATCH v9 5/6] rtc: s35390a: Add pinctrl

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] pinctrl: 'clock' function is advertised but silently disables the pin
- [Medium] rtc: probe unconditionally clobbers periodic pin configuration if interrupt is pending
--

commit 8933a7b5fe72ca6a0ef2901f1d1c8bf6cbe6372f
Author: Markus Probst <markus.probst@posteo.de>

rtc: s35390a: Add pinctrl

This commit adds pinctrl support to the s35390a RTC driver, allowing
configuration of pin functions including keeping previous configuration,
disabling, waking up, and minute periodical interrupts.

> diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
> index 12ed504db94d8..649d36d37f1f4 100644
> --- a/drivers/rtc/rtc-s35390a.c
> +++ b/drivers/rtc/rtc-s35390a.c

[ ... ]

> @@ -464,13 +526,168 @@ static int s35390a_nvmem_write(void *priv, unsigned int offset, void *val,

[ ... ]

> +	if (function != S35390A_FUNC_IGNORE) {
> +		err = s35390a_get_reg(s35390a, S35390A_CMD_STATUS2, &status2, 1);
> +		if (err < 0) {
> +			dev_err(&s35390a->client[0]->dev, "error reading status\n");
> +			return err;
> +		}
> +
> +		switch (function) {
> +		case S35390A_FUNC_DISABLE:
> +		case S35390A_FUNC_CLOCK: /* not implemented */
> +			status2 = (status2 & ~mask) | S35390A_INT_MODE_NOINTR;
> +			break;

[Severity: Medium]
In s35390a_pinctrl_set_mux(), does this silently fail to implement the
requested clock behavior and disable the pin instead?

If a device tree configuration requests the advertised clock pin function,
should this return an error like -ENOTSUPP or -EINVAL instead of silently
disabling it?

[ ... ]

> @@ -504,33 +726,34 @@ static int s35390a_probe(struct i2c_client *client)

[ ... ]

> +	err = s35390a_get_reg(s35390a, S35390A_CMD_STATUS2, &status2, sizeof(status2));
> +	if (err < 0)
> +		return dev_err_probe(dev, err, "disabling alarm and test mode failed\n");
> +
> +	if (status1 & S35390A_FLAG_INT1) {
> +		status2 = (status2 & ~S35390A_INT1_MODE_MASK) | S35390A_INT_MODE_NOINTR;
> +		irq = true;
>  	}

[Severity: Medium]
In s35390a_probe(), can this force the pin to NOINTR mode if an interrupt
was pending at boot, permanently destroying a periodic configuration before
the pinctrl 'keep previous' fallback can save it?

If the device is configured to generate periodic interrupts (like a 1-minute
wakeup), and the system boots while the interrupt flag happens to be set in
STATUS1, overwriting STATUS2 to NOINTR might cause the pinctrl subsystem's
IGNORE fallback to keep this overwritten state, leaving the pin permanently
disabled.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916-rtc_s35390a_int1-v9-0-b2567a3e4959@posteo.de?part=5

Re: [PATCH v9 3/6] rtc: s35390a: Fix alarm not disabling

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] rtc: s35390a: unhandled interrupt storm in alarm_irq_enable()
--

commit f08c593edb80ced466b342983365673dd8bdb4ee
Author: Markus Probst <markus.probst@posteo.de>

rtc: s35390a: Fix alarm not disabling

Implement alarm_irq_enable callback.

> diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
> index 4cfe7034c5165..575bb256eb258 100644
> --- a/drivers/rtc/rtc-s35390a.c
> +++ b/drivers/rtc/rtc-s35390a.c
> @@ -270,6 +270,24 @@ static int s35390a_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  	return 0;
>  }
>  
> +static int s35390a_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
> +{
> +	struct s35390a *s35390a = dev_get_drvdata(dev);
> +	u8 sts;
> +	int err;
> +
> +	if (enabled)
> +		sts = S35390A_INT2_MODE_ALARM;
> +	else
> +		sts = S35390A_INT2_MODE_NOINTR;
> +
> +	err = s35390a_set_reg(s35390a, S35390A_CMD_STATUS2, &sts, sizeof(sts));

[Severity: High]
Does this code risk causing an unhandled interrupt storm if a previous alarm
was pending?

The S-35390A hardware holds the alarm interrupt flag in STATUS1 until it is
explicitly read. If a previous alarm fired and STATUS1 was not read, the flag
remains set.

When userspace calls ioctl(RTC_AIE_ON), the RTC core directly invokes
s35390a_rtc_alarm_irq_enable(1) without calling .set_alarm(). Writing to
STATUS2 enables the INT2 alarm output, but without first reading STATUS1 to
clear any pending interrupt, the hardware will immediately assert the INT2 pin
if the flag is still set.

Because the driver has no runtime ISR to acknowledge the interrupt, could the
pin remain asserted indefinitely and prevent the system from sleeping or
disable shared IRQ lines?

> +	if (err < 0)
> +		return err;
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916-rtc_s35390a_int1-v9-0-b2567a3e4959@posteo.de?part=3

Re: [PATCH v9 1/6] dt-bindings: rtc: Add pinctrl for S35390A

[Rob Herring]

On Wed, Sep 16, 2026 at 04:00:36PM +0000, Markus Probst wrote:
> Synology NAS devices use the output pin for interrupt signal 1 to wake up
> the system.
> 
> Move devicetree bindings for sii,s35390a into its own file.
> Add necessary properties to configure the individual pins via pinctrl,
> which allows the interrupt signal 1 to be used for wakeup alarm.
> 
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
>  .../devicetree/bindings/rtc/sii,s35390a.yaml       | 121 +++++++++++++++++++++
>  .../devicetree/bindings/rtc/trivial-rtc.yaml       |   3 -
>  2 files changed, 121 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/rtc/sii,s35390a.yaml b/Documentation/devicetree/bindings/rtc/sii,s35390a.yaml
> new file mode 100644
> index 000000000000..f3bf5900fb3c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/sii,s35390a.yaml
> @@ -0,0 +1,121 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/rtc/sii,s35390a.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: S-35390A 2-WIRE REAL-TIME CLOCK

Drop the all caps.

> +
> +maintainers:
> +  - Markus Probst <markus.probst@posteo.de>
> +
> +description:
> +  The S-35390A is a CMOS 2-wire real-time clock IC which operates with the
> +  very low current consumption in the wide range of operation voltage.
> +
> +allOf:
> +  - $ref: rtc.yaml#
> +
> +properties:
> +  compatible:
> +    const: sii,s35390a
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    minItems: 1
> +    maxItems: 2
> +    description: Supports up to 2 interrupt lines via the INT1 and INT2 pins.
> +
> +  interrupt-names:
> +    minItems: 1
> +    maxItems: 2
> +    items:
> +      enum:
> +        - int1
> +        - int2
> +
> +  wakeup-parent: true
> +
> +  wakeup-source: true
> +
> +  "#clock-cells":
> +    const: 1
> +
> +patternProperties:
> +  "^pins":
> +    type: object
> +    additionalProperties: false
> +
> +    patternProperties:
> +      "-pins$":
> +        type: object
> +        additionalProperties: false
> +
> +        allOf:

Don't need allOf.

> +          - $ref: /schemas/pinctrl/pinmux-node.yaml#
> +
> +        properties:
> +          pins:
> +            minItems: 1
> +            items:
> +              enum:
> +                - int1
> +                - int2
> +
> +          function:
> +            description: |
> +              Pin function:
> +              - ignore:  Preserve the previous state.
> +              - disable: Disable pin output.
> +              - wakeup:  Output wakes up the system.
> +              - clock:   Output clock pulse.
> +              - pmin1:   Minute periodical output with 50% duty.
> +              - pmin2:   Minute periodical output L for 7.81 ms.
> +                         Can only be used with pin int1.
> +            enum:
> +              - ignore
> +              - disable
> +              - wakeup
> +              - clock
> +              - pmin1
> +              - pmin2
> +
> +        required:
> +          - pins
> +          - function
> +
> +required:
> +  - compatible
> +  - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        rtc@30 {
> +            compatible = "sii,s35390a";
> +            reg = <0x30>;
> +            pinctrl-0 = <&rtc_pins_default>;
> +            pinctrl-names = "default";
> +
> +            interrupt-names = "int1";
> +            interrupts = <23>;
> +
> +            rtc_pins_default: pins {
> +              int1-pins {
> +                pins = "int1";
> +                function = "wakeup";
> +              };
> +              int2-pins {
> +                pins = "int2";
> +                function = "disable";
> +              };
> +            };
> +        };
> +    };
> diff --git a/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml b/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml
> index f4d0eed98a08..7b3f682ef4d5 100644
> --- a/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml
> +++ b/Documentation/devicetree/bindings/rtc/trivial-rtc.yaml
> @@ -81,8 +81,6 @@ properties:
>        - ricoh,rv5c386
>        # I2C bus SERIAL INTERFACE REAL-TIME CLOCK IC
>        - ricoh,rv5c387a
> -      # 2-wire CMOS real-time clock
> -      - sii,s35390a
>        # ST SPEAr Real-time Clock
>        - st,spear600-rtc
>        # VIA/Wondermedia VT8500 Real-time Clock
> @@ -105,5 +103,4 @@ required:
>    - reg
>  
>  additionalProperties: false
> -

Unrelated change. Drop.

Rob