Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.debian.kernel > #57253 > unrolled thread

Bug#854854: qcontrol: reboot/poweroff

Back to article view | Back to linux.debian.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Bug#854854: qcontrol: reboot/poweroff Uwe Kleine-König <uwe@kleine-koenig.org> - 2017-03-16 09:50 +0100
    Bug#854854: AW: Bug#854854: qcontrol: reboot/poweroff "Peter Buechner" <ptrbucr3075@gmail.com> - 2017-03-16 11:00 +0100
      Bug#854854: AW: Bug#854854: qcontrol: reboot/poweroff Uwe Kleine-König <uwe@kleine-koenig.org> - 2017-03-16 11:00 +0100
    Bug#854854: qcontrol: reboot/poweroff Alexandre Belloni <alexandre.belloni@free-electrons.com> - 2017-04-03 21:30 +0200
      Bug#854854: Cherry pick and backport patches for rtc-s35390a (Was: Bug#854854: qcontrol: reboot/poweroff) Ian Campbell <ijc@debian.org> - 2017-04-19 15:00 +0200
    [PATCH 3/4] rtc: s35390a: implement reset routine as suggested by the reference Uwe Kleine-König <uwe@kleine-koenig.org> - 2017-04-03 23:50 +0200
    [PATCH 0/4] rtc: s35390a: add fixes from v4.8-rc1 to stable Uwe Kleine-König <uwe@kleine-koenig.org> - 2017-04-03 23:50 +0200
      [PATCH 1/4] rtc: s35390a: fix reading out alarm Uwe Kleine-König <uwe@kleine-koenig.org> - 2017-04-04 00:00 +0200
      [PATCH 2/4] rtc: s35390a: make sure all members in the output are set Uwe Kleine-König <uwe@kleine-koenig.org> - 2017-04-04 00:00 +0200
      [PATCH 4/4] rtc: s35390a: improve irq handling Uwe Kleine-König <uwe@kleine-koenig.org> - 2017-04-04 00:00 +0200

#57253 — Bug#854854: qcontrol: reboot/poweroff

[Multipart message — attachments visible in raw view] — view raw

Cc += Alexandre Belloni, debian-kernel, stable@k.o

Hello,

On Wed, Mar 15, 2017 at 06:10:47PM -0700, Martin Michlmayr wrote:
> > After new installation of Debian the system is properly powering
> > down either shutdownor by power button. Also the restart is working
> > by power button. But after some days running the system the machine
> > is not powering down completely anymore. The status-LED is blinking
> > red/green, power LED is blue. The system can only be restarted by
> > disconnecting from power. The power button is not working in this
> > moment.  This effect happens if I stop by shutdown command as well
> > as by power button.
> 
> Peter pointed out that this is #794266 which Uwe fixed in the kernel.
> 
> So my question: does that also have to be fixed in the kernel in
> stable?
> 
> IIRC this was only triggered by newer versions qcontrol but I can't
> remember the details.

I don't know about qcontrol, I only triggered this by direct interaction
with the rtc. There the problem happens if the alarm is enabled and the
respective irq event fires. (So I guess qcontrol enables the alarm.)

This is fixed (as good as possible) in mainline since v4.8-rc1[1]. In
Debian it's fixed since 4.6.4-1. The kernel supports setting an alarm
since

	542dd33a4925 ("drivers/rtc/rtc-s35390a.c: add wakealarm support for rtc-s35390A rtc chip")

which is in v3.7-rc1.

If the device is already in a borked state (i.e.  the irq is active but
the respective flag is already read and so cleared) even a new kernel
doesn't repair this. To fix it follow the directions in

	https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=794266#95

.

Back then I prepared a backport to jessie (which still sits in a topic
branch in the debian-kernel repo[2]) but it seems I forgot to merge it into
the jessie kernel.

I wonder if the better fix would be to fix this in linux-stable instead.
What do you think?

Best regards
Uwe

[1] http://git.kernel.org/linus/3bd32722c827
[2] https://anonscm.debian.org/cgit/kernel/linux.git/commit/?id=1d69dac66c315a290fb61c5f400e056e8d01fe50

[toc] | [next] | [standalone]

#57254 — Bug#854854: AW: Bug#854854: qcontrol: reboot/poweroff

Hello,

I cleared the bug according the link below and the system is working now .

But what I wonder: my kernel version is 3.16.0-4-kirkwood. How I got this?

I followed the procedure on page http://www.cyrius.com/debian/kirkwood/qnap/ts-119/deinstall/
And got the kernel and initrd files of the installer via links there.  You are writing actual version would be  4.6.4-1.
How to get the actual kernel?

Regards
Peter Buechner


-----Ursprüngliche Nachricht-----
Von: Uwe Kleine-König [mailto:uwe@kleine-koenig.org] 
Gesendet: Donnerstag, 16. März 2017 09:45
An: Martin Michlmayr
Cc: peter; 854854@bugs.debian.org; Alexandre Belloni; debian-kernel@lists.debian.org; stable@kernel.org
Betreff: Re: Bug#854854: qcontrol: reboot/poweroff

Cc += Alexandre Belloni, debian-kernel, stable@k.o

Hello,

On Wed, Mar 15, 2017 at 06:10:47PM -0700, Martin Michlmayr wrote:
> > After new installation of Debian the system is properly powering 
> > down either shutdownor by power button. Also the restart is working 
> > by power button. But after some days running the system the machine 
> > is not powering down completely anymore. The status-LED is blinking 
> > red/green, power LED is blue. The system can only be restarted by 
> > disconnecting from power. The power button is not working in this 
> > moment.  This effect happens if I stop by shutdown command as well 
> > as by power button.
> 
> Peter pointed out that this is #794266 which Uwe fixed in the kernel.
> 
> So my question: does that also have to be fixed in the kernel in 
> stable?
> 
> IIRC this was only triggered by newer versions qcontrol but I can't 
> remember the details.

I don't know about qcontrol, I only triggered this by direct interaction with the rtc. There the problem happens if the alarm is enabled and the respective irq event fires. (So I guess qcontrol enables the alarm.)

This is fixed (as good as possible) in mainline since v4.8-rc1[1]. In Debian it's fixed since 4.6.4-1. The kernel supports setting an alarm since

	542dd33a4925 ("drivers/rtc/rtc-s35390a.c: add wakealarm support for rtc-s35390A rtc chip")

which is in v3.7-rc1.

If the device is already in a borked state (i.e.  the irq is active but the respective flag is already read and so cleared) even a new kernel doesn't repair this. To fix it follow the directions in

	https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=794266#95

.

Back then I prepared a backport to jessie (which still sits in a topic branch in the debian-kernel repo[2]) but it seems I forgot to merge it into the jessie kernel.

I wonder if the better fix would be to fix this in linux-stable instead.
What do you think?

Best regards
Uwe

[1] http://git.kernel.org/linus/3bd32722c827
[2] https://anonscm.debian.org/cgit/kernel/linux.git/commit/?id=1d69dac66c315a290fb61c5f400e056e8d01fe50

[toc] | [prev] | [next] | [standalone]

#57255 — Bug#854854: AW: Bug#854854: qcontrol: reboot/poweroff

[Multipart message — attachments visible in raw view] — view raw

Hello Peter,

On 03/16/2017 10:52 AM, Peter Buechner wrote:
> I cleared the bug according the link below and the system is working now .

\o/

> But what I wonder: my kernel version is 3.16.0-4-kirkwood. How I got this?

This is expected for a jessie system.

> I followed the procedure on page http://www.cyrius.com/debian/kirkwood/qnap/ts-119/deinstall/
> And got the kernel and initrd files of the installer via links there.  You are writing actual version would be  4.6.4-1.
> How to get the actual kernel?

4.6.4-1 is the version where the bug is fixed. So you had the problem
because you use an older kernel than 4.6.4-1.

Is this more clear now?

Best regards
Uwe

[toc] | [prev] | [next] | [standalone]

#57442

Hi,

On 16/03/2017 at 09:45:07 +0100, Uwe Kleine-König wrote:
> Back then I prepared a backport to jessie (which still sits in a topic
> branch in the debian-kernel repo[2]) but it seems I forgot to merge it into
> the jessie kernel.
> 
> I wonder if the better fix would be to fix this in linux-stable instead.
> What do you think?
> 

I agree that this should be backported to any stable kernel that is used
by a distribution. I can ack the patch if you send one to stable@


-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]

#57593 — Bug#854854: Cherry pick and backport patches for rtc-s35390a (Was: Bug#854854: qcontrol: reboot/poweroff)

Control: retitle -1 Cherry pick and backport patches for rtc-s35390a to Jessie
Control: found -1 3.16.39-1+deb8u2
Control: fixed -1 4.6.4-1

stable@ please see the list of requested backports at the end.

debian-*: I tried (and succeeded!) to reassign to src:linux and tried
(and failed!) to merge with 794266, actually that failure was probably
for the best. I've tried to reflect the fact that 794266 was fixed in
4.6.4-1 but that the issue is still found in Jessie and the stable
3.16.y tree (since I don't see the fix in the changelog of either) with
the above runes.

(the original bug report was all in the $subject, so reproducing here
since the retitle should overwrite it:

    After new installation of Debian the system is properly powering 
    down either shutdownor by power button. Also the restart is working
    by power button. But after some days running the system the machine
    is not powering down completely anymore. The status-LED is blinking
    red/green, power LED is blue. The system can only be restarted by
    disconnecting from power. The power button is not working in this
    moment. This effect happens if I stop by shutdown command as well as
    by power button. nowby
)

On Mon, 2017-04-03 at 21:09 +0200, Alexandre Belloni wrote:
> Hi,
> 
> On 16/03/2017 at 09:45:07 +0100, Uwe Kleine-König wrote:
> > Back then I prepared a backport to jessie (which still sits in a
> > topic
> > branch in the debian-kernel repo[2]) but it seems I forgot to merge
> > it into
> > the jessie kernel.
> > 
> > I wonder if the better fix would be to fix this in linux-stable
> > instead.
> > What do you think?
> > 
> 
> I agree that this should be backported to any stable kernel that is used
> by a distribution. I can ack the patch if you send one to stable@

According to 
https://anonscm.debian.org/cgit/kernel/linux.git/commit/?id=1d69dac66c315a290fb61c5f400e056e8d01fe50
linked from
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=854854#15

The set of requested backports is:
f87e904ddd8f rtc: s35390a: fix reading out alarm
8e6583f1b5d1 rtc: s35390a: implement reset routine as suggested by the reference
3bd32722c827 rtc: s35390a: improve irq handling

all of which are in mainline already. They should/could be backported
to the active LTS branches between v3.7..v4.5 AFAICT.

Uwe, can you confirm for stable@ that these are the patches and
releases where these are needed?

I also see
https://anonscm.debian.org/cgit/kernel/linux.git/tree/debian/patches/bugfix/all/rtc-s35390a-initialize-all-fields.patch?id=1d69dac66c315a290fb61c5f400e056e8d01fe50
in your branch but not sure if that is needed upstream or if it was
intended only for the distro packages

Uwe, Maybe you should merge your Debian kernel package branch in the
meantime and we can drop the patches when they hit stable?

Ian.

[toc] | [prev] | [next] | [standalone]

#57444 — [PATCH 3/4] rtc: s35390a: implement reset routine as suggested by the reference

commit 8e6583f1b5d1f5f129b873f1428b7e414263d847 upstream.

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önig <uwe@kleine-koenig.org>
Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 drivers/rtc/rtc-s35390a.c | 65 +++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 55 insertions(+), 10 deletions(-)

diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
index 47b88bbe4ce7..c7c1fce69635 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>
 
 #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;
 }
 
-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];
+	char buf;
+	int ret;
+	unsigned initcount = 0;
 
-	if (s35390a_get_reg(s35390a, S35390A_CMD_STATUS1, buf, sizeof(buf)) < 0)
-		return -EIO;
+	ret = s35390a_get_reg(s35390a, S35390A_CMD_STATUS1, status1, 1);
+	if (ret < 0)
+		return ret;
 
-	if (!(buf[0] & (S35390A_FLAG_POC | S35390A_FLAG_BLD)))
+	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;
 
-	buf[0] |= (S35390A_FLAG_RESET | S35390A_FLAG_24H);
-	buf[0] &= 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 = S35390A_FLAG_24H;
+	buf = S35390A_FLAG_RESET | S35390A_FLAG_24H;
+	ret = s35390a_set_reg(s35390a, S35390A_CMD_STATUS1, &buf, 1);
+
+	if (ret < 0)
+		return ret;
+
+	ret = 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;
 }
 
 static int s35390a_disable_test_mode(struct s35390a *s35390a)
@@ -367,7 +412,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;
 
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
 		err = -ENODEV;
@@ -396,7 +441,7 @@ static int s35390a_probe(struct i2c_client *client,
 		}
 	}
 
-	err = s35390a_reset(s35390a);
+	err = s35390a_reset(s35390a, &status1);
 	if (err < 0) {
 		dev_err(&client->dev, "error resetting chip\n");
 		goto exit_dummy;
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#57445 — [PATCH 0/4] rtc: s35390a: add fixes from v4.8-rc1 to stable

Hello

I suggest to add the four patches in this series to the stable releases
for stable kernels before 4.8. Three of them can be directly
cherry-picked from the commits from 4.8-rc1 as indicated in the
respective commits[1]. One patch however corresponds to a more invasive
patch (d68778b80dd7 ("rtc: initialize output parameter for read alarm to
"uninitialized"") that when I discussed that with Ben Hutchings some
time ago we considered safer to only apply it to the s35390a driver. So
this one doesn't match a mainline commit.

Best regards
Uwe

[1] f87e904ddd8f0ef120e46045b0addeb1cc88354e
    8e6583f1b5d1f5f129b873f1428b7e414263d847
    3bd32722c827d00eafe8e6d5b83e9f3148ea7c7e

Uwe Kleine-König (4):
  rtc: s35390a: fix reading out alarm
  rtc: s35390a: make sure all members in the output are set
  rtc: s35390a: implement reset routine as suggested by the reference
  rtc: s35390a: improve irq handling

 drivers/rtc/rtc-s35390a.c | 167 ++++++++++++++++++++++++++++++++++++----------
 1 file changed, 131 insertions(+), 36 deletions(-)

-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#57446 — [PATCH 1/4] rtc: s35390a: fix reading out alarm

commit f87e904ddd8f0ef120e46045b0addeb1cc88354e upstream.

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önig <uwe@kleine-koenig.org>
Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 drivers/rtc/rtc-s35390a.c | 40 +++++++++++++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
index f40afdd0e5f5..6507a01cf9ad 100644
--- a/drivers/rtc/rtc-s35390a.c
+++ b/drivers/rtc/rtc-s35390a.c
@@ -242,6 +242,8 @@ static int s35390a_set_alarm(struct i2c_client *client, struct rtc_wkalrm *alm)
 
 	if (alm->time.tm_wday != -1)
 		buf[S35390A_ALRM_BYTE_WDAY] = bin2bcd(alm->time.tm_wday) | 0x80;
+	else
+		buf[S35390A_ALRM_BYTE_WDAY] = 0;
 
 	buf[S35390A_ALRM_BYTE_HOURS] = s35390a_hr2reg(s35390a,
 			alm->time.tm_hour) | 0x80;
@@ -269,23 +271,43 @@ static int s35390a_read_alarm(struct i2c_client *client, struct rtc_wkalrm *alm)
 	if (err < 0)
 		return err;
 
-	if (bitrev8(sts) != S35390A_INT2_MODE_ALARM)
-		return -EINVAL;
+	if ((bitrev8(sts) & S35390A_INT2_MODE_MASK) != S35390A_INT2_MODE_ALARM) {
+		/*
+		 * 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));
 	if (err < 0)
 		return err;
 
 	/* This chip returns the bits of each byte in reverse order */
-	for (i = 0; i < 3; ++i) {
+	for (i = 0; i < 3; ++i)
 		buf[i] = bitrev8(buf[i]);
-		buf[i] &= ~0x80;
-	}
 
-	alm->time.tm_wday = bcd2bin(buf[S35390A_ALRM_BYTE_WDAY]);
-	alm->time.tm_hour = s35390a_reg2hr(s35390a,
-						buf[S35390A_ALRM_BYTE_HOURS]);
-	alm->time.tm_min = 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 =
+			bcd2bin(buf[S35390A_ALRM_BYTE_WDAY] & ~0x80);
+
+	if (buf[S35390A_ALRM_BYTE_HOURS] & 0x80)
+		alm->time.tm_hour =
+			s35390a_reg2hr(s35390a,
+				       buf[S35390A_ALRM_BYTE_HOURS] & ~0x80);
+
+	if (buf[S35390A_ALRM_BYTE_MINS] & 0x80)
+		alm->time.tm_min = bcd2bin(buf[S35390A_ALRM_BYTE_MINS] & ~0x80);
+
+	/* alarm triggers always at s=0 */
+	alm->time.tm_sec = 0;
 
 	dev_dbg(&client->dev, "%s: alm is mins=%d, hours=%d, wday=%d\n",
 			__func__, alm->time.tm_min, alm->time.tm_hour,
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#57447 — [PATCH 2/4] rtc: s35390a: make sure all members in the output are set

The rtc core calls the .read_alarm with all fields initialized to 0. As
the s35390a driver doesn't touch some fields the returned date is
interpreted as a date in January 1900. So make sure all fields are set
to -1; some of them are then overwritten with the right data depending
on the hardware state.

In mainline this is done by commit d68778b80dd7 ("rtc: initialize output
parameter for read alarm to "uninitialized"") in the core. This is
considered to dangerous for stable as it might have side effects for
other rtc drivers that might for example rely on alarm->time.tm_sec
being initialized to 0.

Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>
---
 drivers/rtc/rtc-s35390a.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
index 6507a01cf9ad..47b88bbe4ce7 100644
--- a/drivers/rtc/rtc-s35390a.c
+++ b/drivers/rtc/rtc-s35390a.c
@@ -267,6 +267,20 @@ static int s35390a_read_alarm(struct i2c_client *client, struct rtc_wkalrm *alm)
 	char buf[3], sts;
 	int i, err;
 
+	/*
+	 * initialize all members to -1 to signal the core that they are not
+	 * defined by the hardware.
+	 */
+	alm->time.tm_sec = -1;
+	alm->time.tm_min = -1;
+	alm->time.tm_hour = -1;
+	alm->time.tm_mday = -1;
+	alm->time.tm_mon = -1;
+	alm->time.tm_year = -1;
+	alm->time.tm_wday = -1;
+	alm->time.tm_yday = -1;
+	alm->time.tm_isdst = -1;
+
 	err = s35390a_get_reg(s35390a, S35390A_CMD_STATUS2, &sts, sizeof(sts));
 	if (err < 0)
 		return err;
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#57448 — [PATCH 4/4] rtc: s35390a: improve irq handling

commit 3bd32722c827d00eafe8e6d5b83e9f3148ea7c7e upstream.

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önig <uwe@kleine-koenig.org>
Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 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 c7c1fce69635..00662dd28d66 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
 
+/* 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
 
 #define S35390A_INT2_MODE_MASK		0xF0
@@ -408,11 +412,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;
 
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
 		err = -ENODEV;
@@ -441,29 +445,35 @@ static int s35390a_probe(struct i2c_client *client,
 		}
 	}
 
-	err = s35390a_reset(s35390a, &status1);
-	if (err < 0) {
+	err_reset = s35390a_reset(s35390a, &status1);
+	if (err_reset < 0) {
+		err = err_reset;
 		dev_err(&client->dev, "error resetting chip\n");
 		goto exit_dummy;
 	}
 
-	err = s35390a_disable_test_mode(s35390a);
-	if (err < 0) {
-		dev_err(&client->dev, "error disabling test mode\n");
-		goto exit_dummy;
-	}
-
-	err = 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 = 1;
 	else
 		s35390a->twentyfourhour = 0;
 
-	if (s35390a_get_datetime(client, &tm) < 0)
+	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(&client->dev, "error disabling alarm");
+			goto exit_dummy;
+		}
+	} else {
+		err = 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");
 
 	device_set_wakeup_capable(&client->dev, 1);
@@ -476,6 +486,10 @@ static int s35390a_probe(struct i2c_client *client,
 		err = PTR_ERR(s35390a->rtc);
 		goto exit_dummy;
 	}
+
+	if (status1 & S35390A_FLAG_INT2)
+		rtc_update_irq(s35390a->rtc, 1, RTC_AF);
+
 	return 0;
 
 exit_dummy:
-- 
2.11.0

[toc] | [prev] | [standalone]

Back to top | Article view | linux.debian.kernel

csiph-web