2014-10-27 14:24:53

by Jan Kardell

[permalink] [raw]
Subject: [PATCH 0/6] rtc: pcf8563 alarm fixes

I found some bugs when trying to use the alarm function of the NXP pcf8563 RTC.

Jan Kardell (6):
rtc: pcf8563 Remove leftover code
rtc: pcf8563 Fix write of invalid bits to ST2 reg
rtc: pcf8563 Fix wrong time from read_alarm
rtc: pcf8563 Handle consequeces of lacking second alarm reg
rtc: pcf8563 Save battery power
rtc: pcf8563 Clear expired alarm at boot time

drivers/rtc/rtc-pcf8563.c | 55 +++++++++++++++++++++++++++++++++++++++--------
1 file changed, 46 insertions(+), 9 deletions(-)

--
1.8.4.5


2014-10-27 14:17:18

by Jan Kardell

[permalink] [raw]
Subject: [PATCH 2/6] rtc: pcf8563 Fix write of invalid bits to ST2 reg

>From the NXP datasheet:
"Bits labeled as N should always be written with logic 0."

At least one of those bits is sometime read as a 1, therfore violating
this rule. To fix this we mask away those bits.

Signed-off-by: Jan Kardell <[email protected]>
---
drivers/rtc/rtc-pcf8563.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-pcf8563.c b/drivers/rtc/rtc-pcf8563.c
index 1a865c9..8c23606 100644
--- a/drivers/rtc/rtc-pcf8563.c
+++ b/drivers/rtc/rtc-pcf8563.c
@@ -28,6 +28,7 @@
#define PCF8563_REG_ST2 0x01
#define PCF8563_BIT_AIE (1 << 1)
#define PCF8563_BIT_AF (1 << 3)
+#define PCF8563_BITS_ST2_N (7 << 5)

#define PCF8563_REG_SC 0x02 /* datetime */
#define PCF8563_REG_MN 0x03
@@ -130,7 +131,7 @@ static int pcf8563_set_alarm_mode(struct i2c_client *client, bool on)
else
buf &= ~PCF8563_BIT_AIE;

- buf &= ~PCF8563_BIT_AF;
+ buf &= ~(PCF8563_BIT_AF | PCF8563_BITS_ST2_N);

err = pcf8563_write_block_data(client, PCF8563_REG_ST2, 1, &buf);
if (err < 0) {
--
1.8.4.5

2014-10-27 14:17:19

by Jan Kardell

[permalink] [raw]
Subject: [PATCH 1/6] rtc: pcf8563 Remove leftover code

Remove some code that was left from before block read/write was used.

Signed-off-by: Jan Kardell <[email protected]>
---
drivers/rtc/rtc-pcf8563.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/rtc/rtc-pcf8563.c b/drivers/rtc/rtc-pcf8563.c
index c2ef0a2..1a865c9 100644
--- a/drivers/rtc/rtc-pcf8563.c
+++ b/drivers/rtc/rtc-pcf8563.c
@@ -118,22 +118,21 @@ static int pcf8563_write_block_data(struct i2c_client *client,

static int pcf8563_set_alarm_mode(struct i2c_client *client, bool on)
{
- unsigned char buf[2];
+ unsigned char buf;
int err;

- err = pcf8563_read_block_data(client, PCF8563_REG_ST2, 1, buf + 1);
+ err = pcf8563_read_block_data(client, PCF8563_REG_ST2, 1, &buf);
if (err < 0)
return err;

if (on)
- buf[1] |= PCF8563_BIT_AIE;
+ buf |= PCF8563_BIT_AIE;
else
- buf[1] &= ~PCF8563_BIT_AIE;
+ buf &= ~PCF8563_BIT_AIE;

- buf[1] &= ~PCF8563_BIT_AF;
- buf[0] = PCF8563_REG_ST2;
+ buf &= ~PCF8563_BIT_AF;

- err = pcf8563_write_block_data(client, PCF8563_REG_ST2, 1, buf + 1);
+ err = pcf8563_write_block_data(client, PCF8563_REG_ST2, 1, &buf);
if (err < 0) {
dev_err(&client->dev, "%s: write error\n", __func__);
return -EIO;
--
1.8.4.5

2014-10-27 14:17:23

by Jan Kardell

[permalink] [raw]
Subject: [PATCH 5/6] rtc: pcf8563 Save battery power

According to Haoyu hym8563 datasheet this saves som power. Might be importat
to battery life. And maybe it works for the NXP part as well.

Signed-off-by: Jan Kardell <[email protected]>
---
drivers/rtc/rtc-pcf8563.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/drivers/rtc/rtc-pcf8563.c b/drivers/rtc/rtc-pcf8563.c
index ce6a11b..1e14f60 100644
--- a/drivers/rtc/rtc-pcf8563.c
+++ b/drivers/rtc/rtc-pcf8563.c
@@ -42,6 +42,13 @@

#define PCF8563_REG_CLKO 0x0D /* clock out */
#define PCF8563_REG_TMRC 0x0E /* timer control */
+#define PCF8563_TMRC_ENABLE BIT(7)
+#define PCF8563_TMRC_4096 0
+#define PCF8563_TMRC_64 1
+#define PCF8563_TMRC_1 2
+#define PCF8563_TMRC_1_60 3
+#define PCF8563_TMRC_MASK 3
+
#define PCF8563_REG_TMR 0x0F /* timer */

#define PCF8563_SC_LV 0x80 /* low voltage */
@@ -406,6 +413,7 @@ static int pcf8563_probe(struct i2c_client *client,
{
struct pcf8563 *pcf8563;
int err;
+ unsigned char buf;

dev_dbg(&client->dev, "%s\n", __func__);

@@ -423,6 +431,14 @@ static int pcf8563_probe(struct i2c_client *client,
pcf8563->client = client;
device_set_wakeup_capable(&client->dev, 1);

+ /* Set timer to lowest frequency to save power (ref Haoyu datasheet) */
+ buf = PCF8563_TMRC_1_60;
+ err = pcf8563_write_block_data(client, PCF8563_REG_TMRC, 1, &buf);
+ if (err < 0) {
+ dev_err(&client->dev, "%s: write error\n", __func__);
+ return err;
+ }
+
pcf8563->rtc = devm_rtc_device_register(&client->dev,
pcf8563_driver.driver.name,
&pcf8563_rtc_ops, THIS_MODULE);
--
1.8.4.5

2014-10-27 14:17:22

by Jan Kardell

[permalink] [raw]
Subject: [PATCH 4/6] rtc: pcf8563 Handle consequeces of lacking second alarm reg

To guarantee that a set alarm occurs in the future, the set alarm time
is rounded up to the nearest minute.
Also we cannot handle UIE as it requires second precision.

Signed-off-by: Jan Kardell <[email protected]>
---
drivers/rtc/rtc-pcf8563.c | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/drivers/rtc/rtc-pcf8563.c b/drivers/rtc/rtc-pcf8563.c
index 78f76d9..ce6a11b 100644
--- a/drivers/rtc/rtc-pcf8563.c
+++ b/drivers/rtc/rtc-pcf8563.c
@@ -361,6 +361,14 @@ static int pcf8563_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *tm)
struct i2c_client *client = to_i2c_client(dev);
unsigned char buf[4];
int err;
+ unsigned long alarm_time;
+
+ /* The alarm has no seconds, round up to nearest minute */
+ if (tm->time.tm_sec) {
+ rtc_tm_to_time(&tm->time, &alarm_time);
+ alarm_time += 60-tm->time.tm_sec;
+ rtc_time_to_tm(alarm_time, &tm->time);
+ }

dev_dbg(dev, "%s, min=%d hour=%d wday=%d mday=%d "
"enabled=%d pending=%d\n", __func__,
@@ -435,6 +443,9 @@ static int pcf8563_probe(struct i2c_client *client,

}

+ /* the pcf8563 alarm only supports a minute accuracy */
+ pcf8563->rtc->uie_unsupported = 1;
+
return 0;
}

--
1.8.4.5

2014-10-27 14:18:11

by Jan Kardell

[permalink] [raw]
Subject: [PATCH 6/6] rtc: pcf8563 Clear expired alarm at boot time

In case the card is woken up of the rtc alarm, the devm_rtc_device_register
function detects it as a pending alarm about a month in the future. Fix this
by clearing the alarm in module probe.

Signed-off-by: Jan Kardell <[email protected]>
---
drivers/rtc/rtc-pcf8563.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/drivers/rtc/rtc-pcf8563.c b/drivers/rtc/rtc-pcf8563.c
index 1e14f60..96fb32e 100644
--- a/drivers/rtc/rtc-pcf8563.c
+++ b/drivers/rtc/rtc-pcf8563.c
@@ -396,6 +396,7 @@ static int pcf8563_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *tm)

static int pcf8563_irq_enable(struct device *dev, unsigned int enabled)
{
+ dev_dbg(dev, "%s: en=%d\n", __func__, enabled);
return pcf8563_set_alarm_mode(to_i2c_client(dev), !!enabled);
}

@@ -414,6 +415,7 @@ static int pcf8563_probe(struct i2c_client *client,
struct pcf8563 *pcf8563;
int err;
unsigned char buf;
+ unsigned char alm_pending;

dev_dbg(&client->dev, "%s\n", __func__);

@@ -439,6 +441,14 @@ static int pcf8563_probe(struct i2c_client *client,
return err;
}

+ err = pcf8563_get_alarm_mode(client, NULL, &alm_pending);
+ if (err < 0) {
+ dev_err(&client->dev, "%s: read error\n", __func__);
+ return err;
+ }
+ if (alm_pending)
+ pcf8563_set_alarm_mode(client, 0);
+
pcf8563->rtc = devm_rtc_device_register(&client->dev,
pcf8563_driver.driver.name,
&pcf8563_rtc_ops, THIS_MODULE);
--
1.8.4.5

2014-10-27 14:18:58

by Jan Kardell

[permalink] [raw]
Subject: [PATCH 3/6] rtc: pcf8563 Fix wrong time from read_alarm

Incorrect mask was used for hour and monthday fields.

Signed-off-by: Jan Kardell <[email protected]>
---
drivers/rtc/rtc-pcf8563.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-pcf8563.c b/drivers/rtc/rtc-pcf8563.c
index 8c23606..78f76d9 100644
--- a/drivers/rtc/rtc-pcf8563.c
+++ b/drivers/rtc/rtc-pcf8563.c
@@ -336,8 +336,8 @@ static int pcf8563_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *tm)
__func__, buf[0], buf[1], buf[2], buf[3]);

tm->time.tm_min = bcd2bin(buf[0] & 0x7F);
- tm->time.tm_hour = bcd2bin(buf[1] & 0x7F);
- tm->time.tm_mday = bcd2bin(buf[2] & 0x1F);
+ tm->time.tm_hour = bcd2bin(buf[1] & 0x3F);
+ tm->time.tm_mday = bcd2bin(buf[2] & 0x3F);
tm->time.tm_wday = bcd2bin(buf[3] & 0x7);
tm->time.tm_mon = -1;
tm->time.tm_year = -1;
--
1.8.4.5