[rtc-linux] [PATCH 0/5] rtc: s35390a: allow shutdown on NAS after alarm triggered

[rtc-linux] [PATCH 0/5] rtc: s35390a: allow shutdown on NAS after alarm triggered

[Uwe Kleine-König]

Hello,

this series addresses the issued reported at https://bugs.debian.org/794266=
.

The problem at hand is that some qnap NAS devices can wake up on rtc
alarm and the s35390a driver didn't handle the particularities of the
rtc: When an irq is pending the irq line is pulled low and the INT2 bit
in a status register is set. Unfortunately this INT2 is cleared when the
status register is read, but the irq line isn't deasserted at the same
time. (No cookie for the hardware engineer!)

So the driver (which doesn't pay attention at the first read of the
status register) cannot notice that the irq is pending and so doesn't
disable it which makes the NAS wake up instantly after shutdown.

Among other fixes this series makes the driver aware on the first read
of the status register to check for the INT2 flag and handle
accordingly. Note that this means that if the RTC is already in the
state that the irq is active but the status register was already read,
this is still not noticed (because that's impossible AFAIK). But it
should make it harder to enter this state.

Best regards
Uwe

Uwe Kleine-K=C3=B6nig (5):
  rtc: s35390a: fix reading out alarm
  rtc: s35390a: implement reset routine as suggested by the reference
  rtc: s35390a: improve irq handling
  rtc: s35390a: improve two comments in .set_alarm
  rtc: fix a typo and reduce three empty lines to one

 drivers/rtc/interface.c   |   4 +-
 drivers/rtc/rtc-s35390a.c | 149 ++++++++++++++++++++++++++++++++++--------=
----
 2 files changed, 112 insertions(+), 41 deletions(-)

--=20
2.8.1

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] [PATCH 1/5] rtc: s35390a: fix reading out alarm

[Uwe Kleine-König]

There are several issues fixed in this patch:

 - When alarm isn't enabled, set .enabled to zero instead of returning
   -EINVAL.
 - Ignore how IRQ1 is configured when determining if IRQ2 is on.
 - The three alarm registers have an enable flag which must be
   evaluated.
 - The chip always triggers when the seconds register gets 0.

Note that the rtc framework however doesn't handle the result correctly
because it doesn't check wday being initialized and so interprets an
alarm being set for 10:00 AM in three days as 10:00 AM tomorrow (or
today if that's not over yet).

Signed-off-by: Uwe Kleine-K=C3=B6nig <uwe@kleine-koenig.org>
---
 drivers/rtc/rtc-s35390a.c | 32 +++++++++++++++++++++++---------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
index f40afdd0e5f5..a9f108f045d6 100644
--- a/drivers/rtc/rtc-s35390a.c
+++ b/drivers/rtc/rtc-s35390a.c
@@ -269,23 +269,37 @@ static int s35390a_read_alarm(struct i2c_client *clie=
nt, struct rtc_wkalrm *alm)
 	if (err < 0)
 		return err;
=20
-	if (bitrev8(sts) !=3D S35390A_INT2_MODE_ALARM)
-		return -EINVAL;
+	if ((bitrev8(sts) & S35390A_INT2_MODE_MASK) !=3D S35390A_INT2_MODE_ALARM)
+		alm->enabled =3D 0;
+	else
+		alm->enabled =3D 1;
=20
 	err =3D s35390a_get_reg(s35390a, S35390A_CMD_INT2_REG1, buf, sizeof(buf))=
;
 	if (err < 0)
 		return err;
=20
 	/* This chip returns the bits of each byte in reverse order */
-	for (i =3D 0; i < 3; ++i) {
+	for (i =3D 0; i < 3; ++i)
 		buf[i] =3D bitrev8(buf[i]);
-		buf[i] &=3D ~0x80;
-	}
=20
-	alm->time.tm_wday =3D bcd2bin(buf[S35390A_ALRM_BYTE_WDAY]);
-	alm->time.tm_hour =3D s35390a_reg2hr(s35390a,
-						buf[S35390A_ALRM_BYTE_HOURS]);
-	alm->time.tm_min =3D bcd2bin(buf[S35390A_ALRM_BYTE_MINS]);
+	/*
+	 * B0 of the three matching registers is an enable flag. Iff it is set
+	 * the configured value is used for matching.
+	 */
+	if (buf[S35390A_ALRM_BYTE_WDAY] & 0x80)
+		alm->time.tm_wday =3D
+			bcd2bin(buf[S35390A_ALRM_BYTE_WDAY] & ~0x80);
+
+	if (buf[S35390A_ALRM_BYTE_HOURS] & 0x80)
+		alm->time.tm_hour =3D
+			s35390a_reg2hr(s35390a,
+				       buf[S35390A_ALRM_BYTE_HOURS] & ~0x80);
+
+	if (buf[S35390A_ALRM_BYTE_MINS] & 0x80)
+		alm->time.tm_min =3D bcd2bin(buf[S35390A_ALRM_BYTE_MINS] & ~0x80);
+
+	/* alarm triggers always at s=3D0 */
+	alm->time.tm_sec =3D 0;
=20
 	dev_dbg(&client->dev, "%s: alm is mins=3D%d, hours=3D%d, wday=3D%d\n",
 			__func__, alm->time.tm_min, alm->time.tm_hour,
--=20
2.8.1

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] [PATCH 2/5] rtc: s35390a: implement reset routine as suggested by the reference

[Uwe Kleine-König]

There were two deviations from the reference manual: you have to wait
half a second when POC is active and you might have to repeat
initialization when POC or BLD are still set after the sequence.

Note however that as POC and BLD are cleared by read the driver might
not be able to detect that a reset is necessary. I don't have a good
idea how to fix this.

Additionally report the value read from STATUS1 to the caller. This
prepares the next patch.

Signed-off-by: Uwe Kleine-K=C3=B6nig <uwe@kleine-koenig.org>
---
 drivers/rtc/rtc-s35390a.c | 69 ++++++++++++++++++++++++++++++++++++++-----=
----
 1 file changed, 57 insertions(+), 12 deletions(-)

diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
index a9f108f045d6..b5f33288f837 100644
--- a/drivers/rtc/rtc-s35390a.c
+++ b/drivers/rtc/rtc-s35390a.c
@@ -15,6 +15,7 @@
 #include <linux/bitrev.h>
 #include <linux/bcd.h>
 #include <linux/slab.h>
+#include <linux/delay.h>
=20
 #define S35390A_CMD_STATUS1	0
 #define S35390A_CMD_STATUS2	1
@@ -94,19 +95,63 @@ static int s35390a_get_reg(struct s35390a *s35390a, int=
 reg, char *buf, int len)
 	return 0;
 }
=20
-static int s35390a_reset(struct s35390a *s35390a)
+/*
+ * Returns <0 on error, 0 if rtc is setup fine and 1 if the chip was reset=
.
+ * To keep the information if an irq is pending, pass the value read from
+ * STATUS1 to the caller.
+ */
+static int s35390a_reset(struct s35390a *s35390a, char *status1)
 {
-	char buf[1];
-
-	if (s35390a_get_reg(s35390a, S35390A_CMD_STATUS1, buf, sizeof(buf)) < 0)
-		return -EIO;
-
-	if (!(buf[0] & (S35390A_FLAG_POC | S35390A_FLAG_BLD)))
+	char buf;
+	int ret;
+	unsigned initcount =3D 0;
+
+	ret =3D s35390a_get_reg(s35390a, S35390A_CMD_STATUS1, status1, 1);
+	if (ret < 0)
+		return ret;
+
+	if (*status1 & S35390A_FLAG_POC)
+		/*
+		 * Do not communicate for 0.5 seconds since the power-on
+		 * detection circuit is in operation.
+		 */
+		msleep(500);
+	else if (!(*status1 & S35390A_FLAG_BLD))
+		/*
+		 * If both POC and BLD are unset everything is fine.
+		 */
 		return 0;
=20
-	buf[0] |=3D (S35390A_FLAG_RESET | S35390A_FLAG_24H);
-	buf[0] &=3D 0xf0;
-	return s35390a_set_reg(s35390a, S35390A_CMD_STATUS1, buf, sizeof(buf));
+	/*
+	 * At least one of POC and BLD are set, so reinitialise chip. Keeping
+	 * this information in the hardware to know later that the time isn't
+	 * valid is unfortunately not possible because POC and BLD are cleared
+	 * on read. So the reset is best done now.
+	 *
+	 * The 24H bit is kept over reset, so set it already here.
+	 */
+initialize:
+	*status1 =3D S35390A_FLAG_24H;
+	buf =3D S35390A_FLAG_RESET | S35390A_FLAG_24H;
+	ret =3D s35390a_set_reg(s35390a, S35390A_CMD_STATUS1, &buf, 1);
+
+	if (ret < 0)
+		return ret;
+
+	ret =3D s35390a_get_reg(s35390a, S35390A_CMD_STATUS1, &buf, 1);
+	if (ret < 0)
+		return ret;
+
+	if (buf & (S35390A_FLAG_POC | S35390A_FLAG_BLD)) {
+		/* Try up to five times to reset the chip */
+		if (initcount < 5) {
+			++initcount;
+			goto initialize;
+		} else
+			return -EIO;
+	}
+
+	return 1;
 }
=20
 static int s35390a_disable_test_mode(struct s35390a *s35390a)
@@ -345,7 +390,7 @@ static int s35390a_probe(struct i2c_client *client,
 	unsigned int i;
 	struct s35390a *s35390a;
 	struct rtc_time tm;
-	char buf[1];
+	char buf[1], status1;
=20
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
 		err =3D -ENODEV;
@@ -374,7 +419,7 @@ static int s35390a_probe(struct i2c_client *client,
 		}
 	}
=20
-	err =3D s35390a_reset(s35390a);
+	err =3D s35390a_reset(s35390a, &status1);
 	if (err < 0) {
 		dev_err(&client->dev, "error resetting chip\n");
 		goto exit_dummy;
--=20
2.8.1

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] [PATCH 3/5] rtc: s35390a: improve irq handling

[Uwe Kleine-König]

On some QNAP NAS devices the rtc can wake the machine. Several people
noticed that once the machine was woken this way it fails to shut down.
That's because the driver fails to acknowledge the interrupt and so it
keeps active and restarts the machine immediatly after shutdown. See
https://bugs.debian.org/794266 for a bug report.

Doing this correctly requires to interpret the INT2 flag of the first read
of the STATUS1 register because this bit is cleared by read.

Note this is not maximally robust though because a pending irq isn't
detected when the STATUS1 register was already read (and so INT2 is not
set) but the irq was not disabled. But that is a hardware imposed problem
that cannot easily be fixed by software.

Signed-off-by: Uwe Kleine-K=C3=B6nig <uwe@kleine-koenig.org>
---
 drivers/rtc/rtc-s35390a.c | 48 ++++++++++++++++++++++++++++++-------------=
----
 1 file changed, 31 insertions(+), 17 deletions(-)

diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
index b5f33288f837..543a01509116 100644
--- a/drivers/rtc/rtc-s35390a.c
+++ b/drivers/rtc/rtc-s35390a.c
@@ -35,10 +35,14 @@
 #define S35390A_ALRM_BYTE_HOURS	1
 #define S35390A_ALRM_BYTE_MINS	2
=20
+/* flags for STATUS1 */
 #define S35390A_FLAG_POC	0x01
 #define S35390A_FLAG_BLD	0x02
+#define S35390A_FLAG_INT2	0x04
 #define S35390A_FLAG_24H	0x40
 #define S35390A_FLAG_RESET	0x80
+
+/* flag for STATUS2 */
 #define S35390A_FLAG_TEST	0x01
=20
 #define S35390A_INT2_MODE_MASK		0xF0
@@ -386,11 +390,11 @@ static struct i2c_driver s35390a_driver;
 static int s35390a_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
-	int err;
+	int err, err_reset;
 	unsigned int i;
 	struct s35390a *s35390a;
 	struct rtc_time tm;
-	char buf[1], status1;
+	char buf, status1;
=20
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
 		err =3D -ENODEV;
@@ -419,29 +423,35 @@ static int s35390a_probe(struct i2c_client *client,
 		}
 	}
=20
-	err =3D s35390a_reset(s35390a, &status1);
-	if (err < 0) {
+	err_reset =3D s35390a_reset(s35390a, &status1);
+	if (err_reset < 0) {
+		err =3D err_reset;
 		dev_err(&client->dev, "error resetting chip\n");
 		goto exit_dummy;
 	}
=20
-	err =3D s35390a_disable_test_mode(s35390a);
-	if (err < 0) {
-		dev_err(&client->dev, "error disabling test mode\n");
-		goto exit_dummy;
-	}
-
-	err =3D s35390a_get_reg(s35390a, S35390A_CMD_STATUS1, buf, sizeof(buf));
-	if (err < 0) {
-		dev_err(&client->dev, "error checking 12/24 hour mode\n");
-		goto exit_dummy;
-	}
-	if (buf[0] & S35390A_FLAG_24H)
+	if (status1 & S35390A_FLAG_24H)
 		s35390a->twentyfourhour =3D 1;
 	else
 		s35390a->twentyfourhour =3D 0;
=20
-	if (s35390a_get_datetime(client, &tm) < 0)
+	if (status1 & S35390A_FLAG_INT2) {
+		/* disable alarm (and maybe test mode) */
+		buf =3D 0;
+		err =3D s35390a_set_reg(s35390a, S35390A_CMD_STATUS2, &buf, 1);
+		if (err < 0) {
+			dev_err(&client->dev, "error disabling alarm");
+			goto exit_dummy;
+		}
+	} else {
+		err =3D s35390a_disable_test_mode(s35390a);
+		if (err < 0) {
+			dev_err(&client->dev, "error disabling test mode\n");
+			goto exit_dummy;
+		}
+	}
+
+	if (err_reset > 0 || s35390a_get_datetime(client, &tm) < 0)
 		dev_warn(&client->dev, "clock needs to be set\n");
=20
 	device_set_wakeup_capable(&client->dev, 1);
@@ -454,6 +464,10 @@ static int s35390a_probe(struct i2c_client *client,
 		err =3D PTR_ERR(s35390a->rtc);
 		goto exit_dummy;
 	}
+
+	if (status1 & S35390A_FLAG_INT2)
+		rtc_update_irq(s35390a->rtc, 1, RTC_AF);
+
 	return 0;
=20
 exit_dummy:
--=20
2.8.1

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] [PATCH 4/5] rtc: s35390a: improve two comments in .set_alarm

[Uwe Kleine-König]

Signed-off-by: Uwe Kleine-K=C3=B6nig <uwe@kleine-koenig.org>
---
 drivers/rtc/rtc-s35390a.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
index 543a01509116..a37eeef9e247 100644
--- a/drivers/rtc/rtc-s35390a.c
+++ b/drivers/rtc/rtc-s35390a.c
@@ -266,12 +266,12 @@ static int s35390a_set_alarm(struct i2c_client *clien=
t, 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);
=20
-	/* disable interrupt */
+	/* disable interrupt (which deasserts the irq line) */
 	err =3D s35390a_set_reg(s35390a, S35390A_CMD_STATUS2, &sts, sizeof(sts));
 	if (err < 0)
 		return err;
=20
-	/* clear pending interrupt, if any */
+	/* clear pending interrupt (in STATUS1 only), if any */
 	err =3D s35390a_get_reg(s35390a, S35390A_CMD_STATUS1, &sts, sizeof(sts));
 	if (err < 0)
 		return err;
--=20
2.8.1

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] [PATCH 5/5] rtc: fix a typo and reduce three empty lines to one

[Uwe Kleine-König]

Signed-off-by: Uwe Kleine-K=C3=B6nig <uwe@kleine-koenig.org>
---
 drivers/rtc/interface.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
index 99475908e556..c803364ad4ce 100644
--- a/drivers/rtc/interface.c
+++ b/drivers/rtc/interface.c
@@ -393,7 +393,7 @@ int rtc_initialize_alarm(struct rtc_device *rtc, struct=
 rtc_wkalrm *alarm)
 	rtc->aie_timer.node.expires =3D rtc_tm_to_ktime(alarm->time);
 	rtc->aie_timer.period =3D ktime_set(0, 0);
=20
-	/* Alarm has to be enabled & in the futrure for us to enqueue it */
+	/* Alarm has to be enabled & in the future for us to enqueue it */
 	if (alarm->enabled && (rtc_tm_to_ktime(now).tv64 <
 			 rtc->aie_timer.node.expires.tv64)) {
=20
@@ -405,8 +405,6 @@ int rtc_initialize_alarm(struct rtc_device *rtc, struct=
 rtc_wkalrm *alarm)
 }
 EXPORT_SYMBOL_GPL(rtc_initialize_alarm);
=20
-
-
 int rtc_alarm_irq_enable(struct rtc_device *rtc, unsigned int enabled)
 {
 	int err =3D mutex_lock_interruptible(&rtc->ops_lock);
--=20
2.8.1

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.