From: Markus Probst <markus.probst@posteo.de>
To: "Alexandre Belloni" <alexandre.belloni@bootlin.com>,
"Rob Herring" <robh@kernel.org>,
"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
"Conor Dooley" <conor+dt@kernel.org>,
"Uwe Kleine-König" <uwe@kleine-koenig.org>,
"Andrew Lunn" <andrew@lunn.ch>,
"Gregory Clement" <gregory.clement@bootlin.com>,
"Sebastian Hesselbarth" <sebastian.hesselbarth@gmail.com>,
"Michael Langer" <michael.brainbug.langer@googlemail.com>,
"Andrew Morton" <akpm@linux-foundation.org>,
"Linus Walleij" <linusw@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org, linux-rtc@vger.kernel.org,
devicetree@vger.kernel.org, linux-gpio@vger.kernel.org,
linux-kernel@vger.kernel.org,
Markus Probst <markus.probst@posteo.de>
Subject: [PATCH v7 4/6] rtc: s35390a: Read 24-hour mode on access
Date: Sat, 29 Aug 2026 14:22:58 +0000 [thread overview]
Message-ID: <20260829-rtc_s35390a_int1-v7-4-debcd31143ae@posteo.de> (raw)
In-Reply-To: <20260829-rtc_s35390a_int1-v7-0-debcd31143ae@posteo.de>
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.54.0
next prev parent reply other threads:[~2026-08-29 14:23 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-29 14:22 [PATCH v7 0/6] rtc: s35390a: Allow use of output pin for interrupt signal 1 for wakealarm Markus Probst
2026-08-29 14:22 ` [PATCH v7 1/6] dt-bindings: rtc: Add pinctrl for S35390A Markus Probst
2026-08-31 16:47 ` Conor Dooley
2026-08-29 14:22 ` [PATCH v7 2/6] rtc: s35390a: Add missing newline to dev_err Markus Probst
2026-08-29 14:22 ` [PATCH v7 3/6] rtc: s35390a: Fix alarm not disabling Markus Probst
2026-08-29 14:22 ` [PATCH v7 5/6] rtc: s35390a: Add pinctrl Markus Probst
2026-08-29 14:22 ` Markus Probst [this message]
2026-08-29 14:22 ` [PATCH v7 6/6] rtc: s35390a: Add synology quirk Markus Probst
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260829-rtc_s35390a_int1-v7-4-debcd31143ae@posteo.de \
--to=markus.probst@posteo.de \
--cc=akpm@linux-foundation.org \
--cc=alexandre.belloni@bootlin.com \
--cc=andrew@lunn.ch \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=gregory.clement@bootlin.com \
--cc=krzk+dt@kernel.org \
--cc=linusw@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rtc@vger.kernel.org \
--cc=michael.brainbug.langer@googlemail.com \
--cc=robh@kernel.org \
--cc=sebastian.hesselbarth@gmail.com \
--cc=uwe@kleine-koenig.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox