All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
To: u-boot@lists.denx.de
Subject: [PATCH v4 07/11] rtc: sandbox-rtc: fix set method
Date: Mon,  6 Jul 2020 22:01:16 +0200	[thread overview]
Message-ID: <20200706200120.23093-8-rasmus.villemoes@prevas.dk> (raw)
In-Reply-To: <20200706200120.23093-1-rasmus.villemoes@prevas.dk>

The current set method is broken; a simple test case is to first set
the date to something in April, then change the date to 31st May:

=> date 040412122020.34
Date: 2020-04-04 (Saturday)    Time: 12:12:34
=> date 053112122020.34
Date: 2020-05-01 (Friday)    Time: 12:12:34

or via the amending of the existing rtc_set_get test case similarly:

$ ./u-boot -T -v
=> ut dm rtc_set_get
Test: dm_test_rtc_set_get: rtc.c
expected: 31/08/2004 18:18:00
actual: 01/08/2004 18:18:00

The problem is that after each register write,
sandbox_i2c_rtc_complete_write() gets called and sets the internal
time from the current set of registers. However, when we get to
writing 31 to mday, the registers are in an inconsistent state (mon is
still 4), so the mktime machinery ends up translating April 31st to
May 1st. Upon the next register write, the registers are populated by
sandbox_i2c_rtc_prepare_read(), so the 31 we just wrote to mday gets
overwritten by a 1.

Fix it by writing all registers at once, and for consistency, update
the get method to retrieve them all with one "i2c transfer".

Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Heiko Schocher <hs@denx.de>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 drivers/rtc/sandbox_rtc.c | 65 +++++++++++++++------------------------
 test/dm/rtc.c             | 15 ++++++++-
 2 files changed, 38 insertions(+), 42 deletions(-)

diff --git a/drivers/rtc/sandbox_rtc.c b/drivers/rtc/sandbox_rtc.c
index b08d758a74..77065e49c7 100644
--- a/drivers/rtc/sandbox_rtc.c
+++ b/drivers/rtc/sandbox_rtc.c
@@ -14,55 +14,38 @@
 
 static int sandbox_rtc_get(struct udevice *dev, struct rtc_time *time)
 {
-	time->tm_sec = dm_i2c_reg_read(dev, REG_SEC);
-	if (time->tm_sec < 0)
-		return time->tm_sec;
-	time->tm_min = dm_i2c_reg_read(dev, REG_MIN);
-	if (time->tm_min < 0)
-		return time->tm_min;
-	time->tm_hour = dm_i2c_reg_read(dev, REG_HOUR);
-	if (time->tm_hour < 0)
-		return time->tm_hour;
-	time->tm_mday = dm_i2c_reg_read(dev, REG_MDAY);
-	if (time->tm_mday < 0)
-		return time->tm_mday;
-	time->tm_mon = dm_i2c_reg_read(dev, REG_MON);
-	if (time->tm_mon < 0)
-		return time->tm_mon;
-	time->tm_year = dm_i2c_reg_read(dev, REG_YEAR);
-	if (time->tm_year < 0)
-		return time->tm_year;
-	time->tm_year += 1900;
-	time->tm_wday = dm_i2c_reg_read(dev, REG_WDAY);
-	if (time->tm_wday < 0)
-		return time->tm_wday;
+	u8 buf[7];
+	int ret;
+
+	ret = dm_i2c_read(dev, REG_SEC, buf, sizeof(buf));
+	if (ret < 0)
+		return ret;
+
+	time->tm_sec  = buf[REG_SEC - REG_SEC];
+	time->tm_min  = buf[REG_MIN - REG_SEC];
+	time->tm_hour = buf[REG_HOUR - REG_SEC];
+	time->tm_mday = buf[REG_MDAY - REG_SEC];
+	time->tm_mon  = buf[REG_MON - REG_SEC];
+	time->tm_year = buf[REG_YEAR - REG_SEC] + 1900;
+	time->tm_wday = buf[REG_WDAY - REG_SEC];
 
 	return 0;
 }
 
 static int sandbox_rtc_set(struct udevice *dev, const struct rtc_time *time)
 {
+	u8 buf[7];
 	int ret;
 
-	ret = dm_i2c_reg_write(dev, REG_SEC, time->tm_sec);
-	if (ret < 0)
-		return ret;
-	ret = dm_i2c_reg_write(dev, REG_MIN, time->tm_min);
-	if (ret < 0)
-		return ret;
-	ret = dm_i2c_reg_write(dev, REG_HOUR, time->tm_hour);
-	if (ret < 0)
-		return ret;
-	ret = dm_i2c_reg_write(dev, REG_MDAY, time->tm_mday);
-	if (ret < 0)
-		return ret;
-	ret = dm_i2c_reg_write(dev, REG_MON, time->tm_mon);
-	if (ret < 0)
-		return ret;
-	ret = dm_i2c_reg_write(dev, REG_YEAR, time->tm_year - 1900);
-	if (ret < 0)
-		return ret;
-	ret = dm_i2c_reg_write(dev, REG_WDAY, time->tm_wday);
+	buf[REG_SEC - REG_SEC]  = time->tm_sec;
+	buf[REG_MIN - REG_SEC]  = time->tm_min;
+	buf[REG_HOUR - REG_SEC] = time->tm_hour;
+	buf[REG_MDAY - REG_SEC] = time->tm_mday;
+	buf[REG_MON  - REG_SEC] = time->tm_mon;
+	buf[REG_YEAR - REG_SEC] = time->tm_year - 1900;
+	buf[REG_WDAY - REG_SEC] = time->tm_wday;
+
+	ret = dm_i2c_write(dev, REG_SEC, buf, sizeof(buf));
 	if (ret < 0)
 		return ret;
 
diff --git a/test/dm/rtc.c b/test/dm/rtc.c
index 88f86581cc..e072fd618b 100644
--- a/test/dm/rtc.c
+++ b/test/dm/rtc.c
@@ -70,7 +70,20 @@ static int dm_test_rtc_set_get(struct unit_test_state *uts)
 	old_base_time = sandbox_i2c_rtc_get_set_base_time(emul, -1);
 
 	memset(&time, '\0', sizeof(time));
-	time.tm_mday = 25;
+	time.tm_mday = 3;
+	time.tm_mon = 6;
+	time.tm_year = 2004;
+	time.tm_sec = 0;
+	time.tm_min = 18;
+	time.tm_hour = 18;
+	ut_assertok(dm_rtc_set(dev, &time));
+
+	memset(&cmp, '\0', sizeof(cmp));
+	ut_assertok(dm_rtc_get(dev, &cmp));
+	ut_assertok(cmp_times(&time, &cmp, true));
+
+	memset(&time, '\0', sizeof(time));
+	time.tm_mday = 31;
 	time.tm_mon = 8;
 	time.tm_year = 2004;
 	time.tm_sec = 0;
-- 
2.23.0

  parent reply	other threads:[~2020-07-06 20:01 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-06 20:01 [PATCH v4 00/11] new rtc methods, rtc command, and tests Rasmus Villemoes
2020-07-06 20:01 ` [PATCH v4 01/11] rtc: add dm_rtc_read helper and ->read method Rasmus Villemoes
2020-07-09  8:35   ` Heiko Schocher
2020-07-06 20:01 ` [PATCH v4 02/11] rtc: add dm_rtc_write() helper Rasmus Villemoes
2020-07-09  8:36   ` Heiko Schocher
2020-07-06 20:01 ` [PATCH v4 03/11] rtc: fall back to ->{read, write} if ->{read, write}8 are not provided Rasmus Villemoes
2020-07-09  8:36   ` [PATCH v4 03/11] rtc: fall back to ->{read,write} if ->{read,write}8 " Heiko Schocher
2020-07-06 20:01 ` [PATCH v4 04/11] rtc: pcf2127: provide ->read method Rasmus Villemoes
2020-07-09  8:36   ` Heiko Schocher
2020-07-06 20:01 ` [PATCH v4 05/11] rtc: pcf2127: provide ->write method Rasmus Villemoes
2020-07-09  8:37   ` Heiko Schocher
2020-07-06 20:01 ` [PATCH v4 06/11] rtc: add rtc command Rasmus Villemoes
2020-07-09  8:37   ` Heiko Schocher
2020-07-06 20:01 ` Rasmus Villemoes [this message]
2020-07-09  8:37   ` [PATCH v4 07/11] rtc: sandbox-rtc: fix set method Heiko Schocher
2020-07-06 20:01 ` [PATCH v4 08/11] rtc: i2c_rtc_emul: catch any write to the "reset" register Rasmus Villemoes
2020-07-09  8:38   ` Heiko Schocher
2020-07-06 20:01 ` [PATCH v4 09/11] test: dm: rtc: add test of dm_rtc_read, dm_rtc_write Rasmus Villemoes
2020-07-09  8:38   ` Heiko Schocher
2020-07-06 20:01 ` [PATCH v4 10/11] sandbox: add rtc command to defconfigs Rasmus Villemoes
2020-07-09  8:38   ` Heiko Schocher
2020-07-06 20:01 ` [PATCH v4 11/11] test: dm: rtc: add tests of rtc shell command Rasmus Villemoes
2020-07-09  8:39   ` Heiko Schocher
2020-07-07  5:02 ` [PATCH v4 00/11] new rtc methods, rtc command, and tests Heiko Schocher

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=20200706200120.23093-8-rasmus.villemoes@prevas.dk \
    --to=rasmus.villemoes@prevas.dk \
    --cc=u-boot@lists.denx.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.