[PATCH v4 00/11] new rtc methods, rtc command, and tests

[PATCH v4 00/11] new rtc methods, rtc command, and tests

[Rasmus Villemoes]

I need access to registers other than just the timekeeping ones of the
pcf2127, so I wanted to implement ->read8 and ->write8. But for
testing these it appeared there was no convenient way to invoke those
from the shell, so I also ended up adding such a command.

Also, it seemed more natural to provide array variants that can read
or write several registers at once, so rtc_ops is expanded a bit.

Changes in v4:

- Add CONFIG_CMD_RTC to sandbox defconfigs (new patch 10/11). Not
  quite sure exactly which it needed to be added to, but at least
  sandbox and sandbox_flattree showed CI failures.

- Add Heiko's R-B to the 10 v3 patches (1-9 + 11), and Simon's R-B to 6/11.

- Fix some checkpatch warnings - I don't really agree with most of
  them - e.g. having to add an empty line in

  int foo = something();
  if (foo < 0)
    return foo;
  return something_else(foo);

  doesn't make the code more readable IMO.

  The remaining checkpatch blurps are things I really don't think
  warrant "fixing", e.g. "WARNING: ENOSYS means 'invalid syscall nr'
  and nothing else" seems irrelevant in context of U-Boot, and in any
  case I've only copied existing practice. For "WARNING: please write
  a paragraph that describes the config symbol fully", that seems to
  be a false positive, there's certainly a full help text for CMD_RTC.

Changes in v3:

- Call the functions dm_rtc_read/dm_rtc_write rather than just
  rtc_read/rtc_write, since the latter names are used for local
  helpers by a number of drivers. That also matches the existing
  dm_rtc_set/dm_rtc_get (though then not the existing
  rtc_read{8,16,32}).

- Update the rtc command (patch 6) as per Simon's feedback (parse
  input as hex, avoid overlong lines, use print_buffer()).
  
- Update the tests (patches 9 and 10) according to these changes.

Changes in v2:

- Use simply "read" and "write" instead of "read8_array",
  "write8_array", both for functions and methods, as suggested by
  Simon.

- The rtc command's interface has been simplified a bit (no separate
  read/readm; the number of arguments determines whether the user
  wants the result on the console or to a memory address)

- Add tests, both of rtc_{read,write}() and of the shell command,
  fixing a few things I stumbled on.

Rasmus Villemoes (11):
  rtc: add dm_rtc_read helper and ->read method
  rtc: add dm_rtc_write() helper
  rtc: fall back to ->{read,write} if ->{read,write}8 are not provided
  rtc: pcf2127: provide ->read method
  rtc: pcf2127: provide ->write method
  rtc: add rtc command
  rtc: sandbox-rtc: fix set method
  rtc: i2c_rtc_emul: catch any write to the "reset" register
  test: dm: rtc: add test of dm_rtc_read, dm_rtc_write
  sandbox: add rtc command to defconfigs
  test: dm: rtc: add tests of rtc shell command

 arch/sandbox/include/asm/rtc.h     |   5 +
 cmd/Kconfig                        |   6 ++
 cmd/Makefile                       |   1 +
 cmd/rtc.c                          | 167 +++++++++++++++++++++++++++++
 configs/sandbox64_defconfig        |   1 +
 configs/sandbox_defconfig          |   1 +
 configs/sandbox_flattree_defconfig |   1 +
 drivers/rtc/i2c_rtc_emul.c         |   3 +-
 drivers/rtc/pcf2127.c              |  13 ++-
 drivers/rtc/rtc-uclass.c           |  59 +++++++++-
 drivers/rtc/sandbox_rtc.c          |  65 +++++------
 include/rtc.h                      |  47 ++++++++
 test/dm/rtc.c                      | 118 +++++++++++++++++++-
 13 files changed, 437 insertions(+), 50 deletions(-)
 create mode 100644 cmd/rtc.c

-- 
2.23.0

[PATCH v4 01/11] rtc: add dm_rtc_read helper and ->read method

[Rasmus Villemoes]

Some users may want to read multiple consecutive 8-bit
registers. Instead of each caller having to implement the loop,
provide a dm_rtc_read() helper. Also, allow a driver to provide a
->read method, which can be more efficient than reading one register
at a time.

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/rtc-uclass.c | 19 +++++++++++++++++++
 include/rtc.h            | 23 +++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/drivers/rtc/rtc-uclass.c b/drivers/rtc/rtc-uclass.c
index 926cca234e..4a0e3c5643 100644
--- a/drivers/rtc/rtc-uclass.c
+++ b/drivers/rtc/rtc-uclass.c
@@ -40,6 +40,25 @@ int dm_rtc_reset(struct udevice *dev)
 	return ops->reset(dev);
 }
 
+int dm_rtc_read(struct udevice *dev, unsigned int reg, u8 *buf, unsigned int len)
+{
+	struct rtc_ops *ops = rtc_get_ops(dev);
+
+	assert(ops);
+	if (ops->read)
+		return ops->read(dev, reg, buf, len);
+	if (!ops->read8)
+		return -ENOSYS;
+	while (len--) {
+		int ret = ops->read8(dev, reg++);
+
+		if (ret < 0)
+			return ret;
+		*buf++ = ret;
+	}
+	return 0;
+}
+
 int rtc_read8(struct udevice *dev, unsigned int reg)
 {
 	struct rtc_ops *ops = rtc_get_ops(dev);
diff --git a/include/rtc.h b/include/rtc.h
index 8aabfc1162..f30e908221 100644
--- a/include/rtc.h
+++ b/include/rtc.h
@@ -55,6 +55,18 @@ struct rtc_ops {
 	 */
 	int (*reset)(struct udevice *dev);
 
+	/**
+	 * read() - Read multiple 8-bit registers
+	 *
+	 * @dev:	Device to read from
+	 * @reg:	First register to read
+	 * @buf:	Output buffer
+	 * @len:	Number of registers to read
+	 * @return 0 if OK, -ve on error
+	 */
+	int (*read)(struct udevice *dev, unsigned int reg,
+		    u8 *buf, unsigned int len);
+
 	/**
 	 * read8() - Read an 8-bit register
 	 *
@@ -109,6 +121,17 @@ int dm_rtc_set(struct udevice *dev, struct rtc_time *time);
  */
 int dm_rtc_reset(struct udevice *dev);
 
+/**
+ * dm_rtc_read() - Read multiple 8-bit registers
+ *
+ * @dev:	Device to read from
+ * @reg:	First register to read
+ * @buf:	Output buffer
+ * @len:	Number of registers to read
+ * @return 0 if OK, -ve on error
+ */
+int dm_rtc_read(struct udevice *dev, unsigned int reg, u8 *buf, unsigned int len);
+
 /**
  * rtc_read8() - Read an 8-bit register
  *
-- 
2.23.0

[PATCH v4 01/11] rtc: add dm_rtc_read helper and ->read method

[Heiko Schocher]

Hello Rasmus,

Am 06.07.2020 um 22:01 schrieb Rasmus Villemoes:
> Some users may want to read multiple consecutive 8-bit
> registers. Instead of each caller having to implement the loop,
> provide a dm_rtc_read() helper. Also, allow a driver to provide a
> ->read method, which can be more efficient than reading one register
> at a time.
> 
> 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/rtc-uclass.c | 19 +++++++++++++++++++
>   include/rtc.h            | 23 +++++++++++++++++++++++
>   2 files changed, 42 insertions(+)

Applied to u-boot-i2c.git master

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

[PATCH v4 02/11] rtc: add dm_rtc_write() helper

[Rasmus Villemoes]

Similar to dm_rtc_read(), introduce a helper that allows the caller to
write multiple consecutive 8-bit registers with one call. If the
driver provides the ->write method, use that, otherwise loop using
->write8.

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/rtc-uclass.c | 19 +++++++++++++++++++
 include/rtc.h            | 24 ++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/drivers/rtc/rtc-uclass.c b/drivers/rtc/rtc-uclass.c
index 4a0e3c5643..44da500c03 100644
--- a/drivers/rtc/rtc-uclass.c
+++ b/drivers/rtc/rtc-uclass.c
@@ -59,6 +59,25 @@ int dm_rtc_read(struct udevice *dev, unsigned int reg, u8 *buf, unsigned int len
 	return 0;
 }
 
+int dm_rtc_write(struct udevice *dev, unsigned int reg,
+		 const u8 *buf, unsigned int len)
+{
+	struct rtc_ops *ops = rtc_get_ops(dev);
+
+	assert(ops);
+	if (ops->write)
+		return ops->write(dev, reg, buf, len);
+	if (!ops->write8)
+		return -ENOSYS;
+	while (len--) {
+		int ret = ops->write8(dev, reg++, *buf++);
+
+		if (ret < 0)
+			return ret;
+	}
+	return 0;
+}
+
 int rtc_read8(struct udevice *dev, unsigned int reg)
 {
 	struct rtc_ops *ops = rtc_get_ops(dev);
diff --git a/include/rtc.h b/include/rtc.h
index f30e908221..1efc0db3de 100644
--- a/include/rtc.h
+++ b/include/rtc.h
@@ -67,6 +67,18 @@ struct rtc_ops {
 	int (*read)(struct udevice *dev, unsigned int reg,
 		    u8 *buf, unsigned int len);
 
+	/**
+	 * write() - Write multiple 8-bit registers
+	 *
+	 * @dev:	Device to write to
+	 * @reg:	First register to write
+	 * @buf:	Input buffer
+	 * @len:	Number of registers to write
+	 * @return 0 if OK, -ve on error
+	 */
+	int (*write)(struct udevice *dev, unsigned int reg,
+		     const u8 *buf, unsigned int len);
+
 	/**
 	 * read8() - Read an 8-bit register
 	 *
@@ -132,6 +144,18 @@ int dm_rtc_reset(struct udevice *dev);
  */
 int dm_rtc_read(struct udevice *dev, unsigned int reg, u8 *buf, unsigned int len);
 
+/**
+ * dm_rtc_write() - Write multiple 8-bit registers
+ *
+ * @dev:	Device to write to
+ * @reg:	First register to write
+ * @buf:	Input buffer
+ * @len:	Number of registers to write
+ * @return 0 if OK, -ve on error
+ */
+int dm_rtc_write(struct udevice *dev, unsigned int reg,
+		 const u8 *buf, unsigned int len);
+
 /**
  * rtc_read8() - Read an 8-bit register
  *
-- 
2.23.0

[PATCH v4 02/11] rtc: add dm_rtc_write() helper

[Heiko Schocher]

Hello Rasmus,

Am 06.07.2020 um 22:01 schrieb Rasmus Villemoes:
> Similar to dm_rtc_read(), introduce a helper that allows the caller to
> write multiple consecutive 8-bit registers with one call. If the
> driver provides the ->write method, use that, otherwise loop using
> ->write8.
> 
> 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/rtc-uclass.c | 19 +++++++++++++++++++
>   include/rtc.h            | 24 ++++++++++++++++++++++++
>   2 files changed, 43 insertions(+)

Applied to u-boot-i2c.git master

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

[PATCH v4 03/11] rtc: fall back to ->{read, write} if ->{read, write}8 are not provided

[Rasmus Villemoes]

Similar to how the dm_rtc_{read,write} functions fall back to using
the {read,write}8 methods, do the opposite in the rtc_{read,write}8
functions.

This way, each driver only needs to provide either ->read8 or ->read
to make both rtc_read8() and dm_rtc_read() work - without this, a
driver that provides ->read() would most likely just duplicate the
logic here for implementing a ->read8() method in term of its ->read()
method. The same remarks of course apply to the write case.

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/rtc-uclass.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/rtc/rtc-uclass.c b/drivers/rtc/rtc-uclass.c
index 44da500c03..8035f7fe9c 100644
--- a/drivers/rtc/rtc-uclass.c
+++ b/drivers/rtc/rtc-uclass.c
@@ -83,9 +83,17 @@ int rtc_read8(struct udevice *dev, unsigned int reg)
 	struct rtc_ops *ops = rtc_get_ops(dev);
 
 	assert(ops);
-	if (!ops->read8)
-		return -ENOSYS;
-	return ops->read8(dev, reg);
+	if (ops->read8)
+		return ops->read8(dev, reg);
+	if (ops->read) {
+		u8 buf[1];
+		int ret = ops->read(dev, reg, buf, 1);
+
+		if (ret < 0)
+			return ret;
+		return buf[0];
+	}
+	return -ENOSYS;
 }
 
 int rtc_write8(struct udevice *dev, unsigned int reg, int val)
@@ -93,9 +101,14 @@ int rtc_write8(struct udevice *dev, unsigned int reg, int val)
 	struct rtc_ops *ops = rtc_get_ops(dev);
 
 	assert(ops);
-	if (!ops->write8)
-		return -ENOSYS;
-	return ops->write8(dev, reg, val);
+	if (ops->write8)
+		return ops->write8(dev, reg, val);
+	if (ops->write) {
+		u8 buf[1] = { val };
+
+		return ops->write(dev, reg, buf, 1);
+	}
+	return -ENOSYS;
 }
 
 int rtc_read16(struct udevice *dev, unsigned int reg, u16 *valuep)
-- 
2.23.0

[PATCH v4 03/11] rtc: fall back to ->{read,write} if ->{read,write}8 are not provided

[Heiko Schocher]

Hello Rasmus,

Am 06.07.2020 um 22:01 schrieb Rasmus Villemoes:
> Similar to how the dm_rtc_{read,write} functions fall back to using
> the {read,write}8 methods, do the opposite in the rtc_{read,write}8
> functions.
> 
> This way, each driver only needs to provide either ->read8 or ->read
> to make both rtc_read8() and dm_rtc_read() work - without this, a
> driver that provides ->read() would most likely just duplicate the
> logic here for implementing a ->read8() method in term of its ->read()
> method. The same remarks of course apply to the write case.
> 
> 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/rtc-uclass.c | 25 +++++++++++++++++++------
>   1 file changed, 19 insertions(+), 6 deletions(-)

Applied to u-boot-i2c.git master

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

[PATCH v4 04/11] rtc: pcf2127: provide ->read method

[Rasmus Villemoes]

This simply consists of renaming the existing pcf2127_read_reg()
helper to follow the naming of the other
methods (i.e. pcf2127_rtc_<method name>) and changing the type of its
"len" parameter.

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/pcf2127.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/pcf2127.c b/drivers/rtc/pcf2127.c
index c423960b34..eea72ad522 100644
--- a/drivers/rtc/pcf2127.c
+++ b/drivers/rtc/pcf2127.c
@@ -23,8 +23,7 @@
 #define PCF2127_REG_MO		0x08
 #define PCF2127_REG_YR		0x09
 
-static int pcf2127_read_reg(struct udevice *dev, uint offset,
-			    u8 *buffer, int len)
+static int pcf2127_rtc_read(struct udevice *dev, uint offset, u8 *buffer, uint len)
 {
 	struct dm_i2c_chip *chip = dev_get_parent_platdata(dev);
 	struct i2c_msg msg;
@@ -73,7 +72,7 @@ static int pcf2127_rtc_get(struct udevice *dev, struct rtc_time *tm)
 	int ret = 0;
 	uchar buf[10] = { PCF2127_REG_CTRL1 };
 
-	ret = pcf2127_read_reg(dev, PCF2127_REG_CTRL1, buf, sizeof(buf));
+	ret = pcf2127_rtc_read(dev, PCF2127_REG_CTRL1, buf, sizeof(buf));
 	if (ret < 0)
 		return ret;
 
@@ -110,6 +109,7 @@ static const struct rtc_ops pcf2127_rtc_ops = {
 	.get = pcf2127_rtc_get,
 	.set = pcf2127_rtc_set,
 	.reset = pcf2127_rtc_reset,
+	.read = pcf2127_rtc_read,
 };
 
 static const struct udevice_id pcf2127_rtc_ids[] = {
-- 
2.23.0

[PATCH v4 04/11] rtc: pcf2127: provide ->read method

[Heiko Schocher]

Hello Rasmus,

Am 06.07.2020 um 22:01 schrieb Rasmus Villemoes:
> This simply consists of renaming the existing pcf2127_read_reg()
> helper to follow the naming of the other
> methods (i.e. pcf2127_rtc_<method name>) and changing the type of its
> "len" parameter.
> 
> 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/pcf2127.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)

Applied to u-boot-i2c.git master

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

[PATCH v4 05/11] rtc: pcf2127: provide ->write method

[Rasmus Villemoes]

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/pcf2127.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/rtc/pcf2127.c b/drivers/rtc/pcf2127.c
index eea72ad522..88ff8c52c3 100644
--- a/drivers/rtc/pcf2127.c
+++ b/drivers/rtc/pcf2127.c
@@ -43,6 +43,12 @@ static int pcf2127_rtc_read(struct udevice *dev, uint offset, u8 *buffer, uint l
 	return dm_i2c_xfer(dev, &msg, 1);
 }
 
+static int pcf2127_rtc_write(struct udevice *dev, uint offset,
+			     const u8 *buffer, uint len)
+{
+	return dm_i2c_write(dev, offset, buffer, len);
+}
+
 static int pcf2127_rtc_set(struct udevice *dev, const struct rtc_time *tm)
 {
 	uchar buf[7] = {0};
@@ -110,6 +116,7 @@ static const struct rtc_ops pcf2127_rtc_ops = {
 	.set = pcf2127_rtc_set,
 	.reset = pcf2127_rtc_reset,
 	.read = pcf2127_rtc_read,
+	.write = pcf2127_rtc_write,
 };
 
 static const struct udevice_id pcf2127_rtc_ids[] = {
-- 
2.23.0

[PATCH v4 05/11] rtc: pcf2127: provide ->write method

[Heiko Schocher]

Hello Rasmus,

Am 06.07.2020 um 22:01 schrieb Rasmus Villemoes:
> 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/pcf2127.c | 7 +++++++
>   1 file changed, 7 insertions(+)

Applied to u-boot-i2c.git master

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

[PATCH v4 06/11] rtc: add rtc command

[Rasmus Villemoes]

Mostly as an aid for debugging RTC drivers, provide a command that can
be used to read/write arbitrary registers (assuming the driver
provides the read/write methods or their single-register-at-a-time
variants).

Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Heiko Schocher <hs@denx.de>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 cmd/Kconfig  |   6 ++
 cmd/Makefile |   1 +
 cmd/rtc.c    | 167 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 174 insertions(+)
 create mode 100644 cmd/rtc.c

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 192b3b262f..c4d40cbce6 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1718,6 +1718,12 @@ config CMD_DATE
 	  Enable the 'date' command for getting/setting the time/date in RTC
 	  devices.
 
+config CMD_RTC
+	bool "rtc"
+	depends on DM_RTC
+	help
+	  Enable the 'rtc' command for low-level access to RTC devices.
+
 config CMD_TIME
 	bool "time"
 	help
diff --git a/cmd/Makefile b/cmd/Makefile
index 974ad48b0a..c7992113e4 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -120,6 +120,7 @@ obj-$(CONFIG_CMD_REISER) += reiser.o
 obj-$(CONFIG_CMD_REMOTEPROC) += remoteproc.o
 obj-$(CONFIG_CMD_RNG) += rng.o
 obj-$(CONFIG_CMD_ROCKUSB) += rockusb.o
+obj-$(CONFIG_CMD_RTC) += rtc.o
 obj-$(CONFIG_SANDBOX) += host.o
 obj-$(CONFIG_CMD_SATA) += sata.o
 obj-$(CONFIG_CMD_NVME) += nvme.o
diff --git a/cmd/rtc.c b/cmd/rtc.c
new file mode 100644
index 0000000000..b4f61b2e83
--- /dev/null
+++ b/cmd/rtc.c
@@ -0,0 +1,167 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <common.h>
+#include <command.h>
+#include <dm.h>
+#include <hexdump.h>
+#include <i2c.h>
+#include <mapmem.h>
+#include <rtc.h>
+
+#define MAX_RTC_BYTES 32
+
+static int do_rtc_read(struct udevice *dev, int argc, char * const argv[])
+{
+	u8 buf[MAX_RTC_BYTES];
+	int reg, len, ret, r;
+
+	if (argc < 2 || argc > 3)
+		return CMD_RET_USAGE;
+
+	reg = simple_strtoul(argv[0], NULL, 16);
+	len = simple_strtoul(argv[1], NULL, 16);
+
+	if (argc == 3) {
+		u8 *addr;
+
+		addr = map_sysmem(simple_strtoul(argv[2], NULL, 16), len);
+		ret = dm_rtc_read(dev, reg, addr, len);
+		unmap_sysmem(addr);
+		if (ret) {
+			printf("dm_rtc_read() failed: %d\n", ret);
+			return CMD_RET_FAILURE;
+		}
+		return CMD_RET_SUCCESS;
+	}
+
+	while (len) {
+		r = min_t(int, len, sizeof(buf));
+		ret = dm_rtc_read(dev, reg, buf, r);
+		if (ret) {
+			printf("dm_rtc_read() failed: %d\n", ret);
+			return CMD_RET_FAILURE;
+		}
+		print_buffer(reg, buf, 1, r, 0);
+		len -= r;
+		reg += r;
+	}
+
+	return CMD_RET_SUCCESS;
+}
+
+static int do_rtc_write(struct udevice *dev, int argc, char * const argv[])
+{
+	u8 buf[MAX_RTC_BYTES];
+	int reg, len, ret;
+	const char *s;
+	int slen;
+
+	if (argc < 2 || argc > 3)
+		return CMD_RET_USAGE;
+
+	reg = simple_strtoul(argv[0], NULL, 16);
+
+	if (argc == 3) {
+		u8 *addr;
+
+		len = simple_strtoul(argv[1], NULL, 16);
+		addr = map_sysmem(simple_strtoul(argv[2], NULL, 16), len);
+		ret = dm_rtc_write(dev, reg, addr, len);
+		unmap_sysmem(addr);
+		if (ret) {
+			printf("dm_rtc_write() failed: %d\n", ret);
+			return CMD_RET_FAILURE;
+		}
+		return CMD_RET_SUCCESS;
+	}
+
+	s = argv[1];
+	slen = strlen(s);
+
+	if (slen % 2) {
+		printf("invalid hex string\n");
+		return CMD_RET_FAILURE;
+	}
+
+	while (slen) {
+		len = min_t(int, slen / 2, sizeof(buf));
+		if (hex2bin(buf, s, len)) {
+			printf("invalid hex string\n");
+			return CMD_RET_FAILURE;
+		}
+
+		ret = dm_rtc_write(dev, reg, buf, len);
+		if (ret) {
+			printf("dm_rtc_write() failed: %d\n", ret);
+			return CMD_RET_FAILURE;
+		}
+		s += 2 * len;
+		slen -= 2 * len;
+	}
+
+	return CMD_RET_SUCCESS;
+}
+
+int do_rtc(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[])
+{
+	static int curr_rtc;
+	struct udevice *dev;
+	int ret, idx;
+
+	if (argc < 2)
+		return CMD_RET_USAGE;
+
+	argc--;
+	argv++;
+
+	if (!strcmp(argv[0], "list")) {
+		struct uclass *uc;
+
+		idx = 0;
+		uclass_id_foreach_dev(UCLASS_RTC, dev, uc) {
+			printf("RTC #%d - %s\n", idx++, dev->name);
+		}
+		if (!idx) {
+			printf("*** no RTC devices available ***\n");
+			return CMD_RET_FAILURE;
+		}
+		return CMD_RET_SUCCESS;
+	}
+
+	idx = curr_rtc;
+	if (!strcmp(argv[0], "dev") && argc >= 2)
+		idx = simple_strtoul(argv[1], NULL, 10);
+
+	ret = uclass_get_device(UCLASS_RTC, idx, &dev);
+	if (ret) {
+		printf("Cannot find RTC #%d: err=%d\n", idx, ret);
+		return CMD_RET_FAILURE;
+	}
+
+	if (!strcmp(argv[0], "dev")) {
+		/* Show the existing or newly selected RTC */
+		if (argc >= 2)
+			curr_rtc = idx;
+		printf("RTC #%d - %s\n", idx, dev->name);
+		return CMD_RET_SUCCESS;
+	}
+
+	if (!strcmp(argv[0], "read"))
+		return do_rtc_read(dev, argc - 1, argv + 1);
+
+	if (!strcmp(argv[0], "write"))
+		return do_rtc_write(dev, argc - 1, argv + 1);
+
+	return CMD_RET_USAGE;
+}
+
+U_BOOT_CMD(
+	rtc,	5,	0,	do_rtc,
+	"RTC subsystem",
+	"list                        - show available rtc devices\n"
+	"rtc dev [n]                     - show or set current rtc device\n"
+	"rtc read <reg> <count>          - read and display 8-bit registers starting at <reg>\n"
+	"rtc read <reg> <count> <addr>   - read 8-bit registers starting at <reg> to memory <addr>\n"
+	"rtc write <reg> <hexstring>     - write 8-bit registers starting at <reg>\n"
+	"rtc write <reg> <count> <addr>  - write from memory <addr> to 8-bit registers starting at <reg>\n"
+);
-- 
2.23.0

[PATCH v4 06/11] rtc: add rtc command

[Heiko Schocher]

Hello Rasmus,

Am 06.07.2020 um 22:01 schrieb Rasmus Villemoes:
> Mostly as an aid for debugging RTC drivers, provide a command that can
> be used to read/write arbitrary registers (assuming the driver
> provides the read/write methods or their single-register-at-a-time
> variants).
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Heiko Schocher <hs@denx.de>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> ---
>   cmd/Kconfig  |   6 ++
>   cmd/Makefile |   1 +
>   cmd/rtc.c    | 167 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 174 insertions(+)
>   create mode 100644 cmd/rtc.c

Applied to u-boot-i2c.git master

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

[PATCH v4 07/11] rtc: sandbox-rtc: fix set method

[Rasmus Villemoes]

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

[PATCH v4 07/11] rtc: sandbox-rtc: fix set method

[Heiko Schocher]

Hello Rasmus,

Am 06.07.2020 um 22:01 schrieb Rasmus Villemoes:
> 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(-)

Applied to u-boot-i2c.git master

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

[PATCH v4 08/11] rtc: i2c_rtc_emul: catch any write to the "reset" register

[Rasmus Villemoes]

It's more natural that any write that happens to touch the reset
register should cause a reset, rather than just a write that starts at
that offset.

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/i2c_rtc_emul.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/i2c_rtc_emul.c b/drivers/rtc/i2c_rtc_emul.c
index a010af411b..7f78ff83cb 100644
--- a/drivers/rtc/i2c_rtc_emul.c
+++ b/drivers/rtc/i2c_rtc_emul.c
@@ -197,7 +197,8 @@ static int sandbox_i2c_rtc_xfer(struct udevice *emul, struct i2c_msg *msg,
 
 			/* Write the register */
 			memcpy(plat->reg + offset, ptr, len);
-			if (offset == REG_RESET)
+			/* If the reset register was written to, do reset. */
+			if (offset <= REG_RESET && REG_RESET < offset + len)
 				reset_time(emul);
 		}
 	}
-- 
2.23.0

[PATCH v4 08/11] rtc: i2c_rtc_emul: catch any write to the "reset" register

[Heiko Schocher]

Hello Rasmus,

Am 06.07.2020 um 22:01 schrieb Rasmus Villemoes:
> It's more natural that any write that happens to touch the reset
> register should cause a reset, rather than just a write that starts at
> that offset.
> 
> 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/i2c_rtc_emul.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)

Applied to u-boot-i2c.git master

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

[PATCH v4 09/11] test: dm: rtc: add test of dm_rtc_read, dm_rtc_write

[Rasmus Villemoes]

Define a few aux registers and check that they can be read/written
individually. Also check that one can access the time-keeping
registers directly and get the expected results.

Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Heiko Schocher <hs@denx.de>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 arch/sandbox/include/asm/rtc.h |  5 ++++
 test/dm/rtc.c                  | 45 ++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/arch/sandbox/include/asm/rtc.h b/arch/sandbox/include/asm/rtc.h
index 1fbfea7999..5bb032f59f 100644
--- a/arch/sandbox/include/asm/rtc.h
+++ b/arch/sandbox/include/asm/rtc.h
@@ -21,6 +21,11 @@ enum {
 
 	REG_RESET	= 0x20,
 
+	REG_AUX0	= 0x30,
+	REG_AUX1,
+	REG_AUX2,
+	REG_AUX3,
+
 	REG_COUNT	= 0x80,
 };
 
diff --git a/test/dm/rtc.c b/test/dm/rtc.c
index e072fd618b..ff1bc7b7f6 100644
--- a/test/dm/rtc.c
+++ b/test/dm/rtc.c
@@ -10,6 +10,7 @@
 #include <log.h>
 #include <rtc.h>
 #include <asm/io.h>
+#include <asm/rtc.h>
 #include <asm/test.h>
 #include <dm/test.h>
 #include <test/ut.h>
@@ -130,6 +131,50 @@ static int dm_test_rtc_set_get(struct unit_test_state *uts)
 }
 DM_TEST(dm_test_rtc_set_get, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
 
+static int dm_test_rtc_read_write(struct unit_test_state *uts)
+{
+	struct rtc_time time;
+	struct udevice *dev, *emul;
+	long old_offset;
+	u8 buf[4], reg;
+
+	ut_assertok(uclass_get_device(UCLASS_RTC, 0, &dev));
+
+	memcpy(buf, "car", 4);
+	ut_assertok(dm_rtc_write(dev, REG_AUX0, buf, 4));
+	memset(buf, '\0', sizeof(buf));
+	ut_assertok(dm_rtc_read(dev, REG_AUX0, buf, 4));
+	ut_asserteq(memcmp(buf, "car", 4), 0);
+
+	reg = 'b';
+	ut_assertok(dm_rtc_write(dev, REG_AUX0, &reg, 1));
+	memset(buf, '\0', sizeof(buf));
+	ut_assertok(dm_rtc_read(dev, REG_AUX0, buf, 4));
+	ut_asserteq(memcmp(buf, "bar", 4), 0);
+
+	reg = 't';
+	ut_assertok(dm_rtc_write(dev, REG_AUX2, &reg, 1));
+	memset(buf, '\0', sizeof(buf));
+	ut_assertok(dm_rtc_read(dev, REG_AUX1, buf, 3));
+	ut_asserteq(memcmp(buf, "at", 3), 0);
+
+	ut_assertok(i2c_emul_find(dev, &emul));
+	ut_assert(emul != NULL);
+
+	old_offset = sandbox_i2c_rtc_set_offset(emul, false, 0);
+	ut_assertok(dm_rtc_get(dev, &time));
+
+	ut_assertok(dm_rtc_read(dev, REG_SEC, &reg, 1));
+	ut_asserteq(time.tm_sec, reg);
+	ut_assertok(dm_rtc_read(dev, REG_MDAY, &reg, 1));
+	ut_asserteq(time.tm_mday, reg);
+
+	sandbox_i2c_rtc_set_offset(emul, true, old_offset);
+
+	return 0;
+}
+DM_TEST(dm_test_rtc_read_write, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
+
 /* Reset the time */
 static int dm_test_rtc_reset(struct unit_test_state *uts)
 {
-- 
2.23.0

[PATCH v4 09/11] test: dm: rtc: add test of dm_rtc_read, dm_rtc_write

[Heiko Schocher]

Hello Rasmus,

Am 06.07.2020 um 22:01 schrieb Rasmus Villemoes:
> Define a few aux registers and check that they can be read/written
> individually. Also check that one can access the time-keeping
> registers directly and get the expected results.
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Heiko Schocher <hs@denx.de>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> ---
>   arch/sandbox/include/asm/rtc.h |  5 ++++
>   test/dm/rtc.c                  | 45 ++++++++++++++++++++++++++++++++++
>   2 files changed, 50 insertions(+)

Applied to u-boot-i2c.git master

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

[PATCH v4 10/11] sandbox: add rtc command to defconfigs

[Rasmus Villemoes]

In order to allow adding unit tests of the rtc command, add it to the
various sandbox defconfigs.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 configs/sandbox64_defconfig        | 1 +
 configs/sandbox_defconfig          | 1 +
 configs/sandbox_flattree_defconfig | 1 +
 3 files changed, 3 insertions(+)

diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index 0f9a27c39c..0d0a4fcec5 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -59,6 +59,7 @@ CONFIG_CMD_LINK_LOCAL=y
 CONFIG_CMD_ETHSW=y
 CONFIG_CMD_BMP=y
 CONFIG_CMD_EFIDEBUG=y
+CONFIG_CMD_RTC=y
 CONFIG_CMD_TIME=y
 CONFIG_CMD_TIMER=y
 CONFIG_CMD_SOUND=y
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 6f450aa637..1061933643 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -68,6 +68,7 @@ CONFIG_CMD_ETHSW=y
 CONFIG_CMD_BMP=y
 CONFIG_CMD_BOOTCOUNT=y
 CONFIG_CMD_EFIDEBUG=y
+CONFIG_CMD_RTC=y
 CONFIG_CMD_TIME=y
 CONFIG_CMD_TIMER=y
 CONFIG_CMD_SOUND=y
diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig
index 05ce9bda37..2be428ba27 100644
--- a/configs/sandbox_flattree_defconfig
+++ b/configs/sandbox_flattree_defconfig
@@ -48,6 +48,7 @@ CONFIG_CMD_SNTP=y
 CONFIG_CMD_DNS=y
 CONFIG_CMD_LINK_LOCAL=y
 CONFIG_CMD_EFIDEBUG=y
+CONFIG_CMD_RTC=y
 CONFIG_CMD_TIME=y
 CONFIG_CMD_TIMER=y
 CONFIG_CMD_SOUND=y
-- 
2.23.0

[PATCH v4 10/11] sandbox: add rtc command to defconfigs

[Heiko Schocher]

Hello Rasmus,

Am 06.07.2020 um 22:01 schrieb Rasmus Villemoes:
> In order to allow adding unit tests of the rtc command, add it to the
> various sandbox defconfigs.
> 
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> ---
>   configs/sandbox64_defconfig        | 1 +
>   configs/sandbox_defconfig          | 1 +
>   configs/sandbox_flattree_defconfig | 1 +
>   3 files changed, 3 insertions(+)

Applied to u-boot-i2c.git master

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

[PATCH v4 11/11] test: dm: rtc: add tests of rtc shell command

[Rasmus Villemoes]

Add tests of the "list", "read" and "write" subcommands of the rtc
shell command.

Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Heiko Schocher <hs@denx.de>
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 test/dm/rtc.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/test/dm/rtc.c b/test/dm/rtc.c
index ff1bc7b7f6..dd037a6e17 100644
--- a/test/dm/rtc.c
+++ b/test/dm/rtc.c
@@ -5,6 +5,7 @@
  */
 
 #include <common.h>
+#include <console.h>
 #include <dm.h>
 #include <i2c.h>
 #include <log.h>
@@ -175,6 +176,63 @@ static int dm_test_rtc_read_write(struct unit_test_state *uts)
 }
 DM_TEST(dm_test_rtc_read_write, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
 
+/* Test 'rtc list' command */
+static int dm_test_rtc_cmd_list(struct unit_test_state *uts)
+{
+	console_record_reset();
+
+	run_command("rtc list", 0);
+	ut_assert_nextline("RTC #0 - rtc at 43");
+	ut_assert_nextline("RTC #1 - rtc at 61");
+	ut_assert_console_end();
+
+	return 0;
+}
+DM_TEST(dm_test_rtc_cmd_list, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
+
+/* Test 'rtc read' and 'rtc write' commands */
+static int dm_test_rtc_cmd_rw(struct unit_test_state *uts)
+{
+	console_record_reset();
+
+	run_command("rtc dev 0", 0);
+	ut_assert_nextline("RTC #0 - rtc at 43");
+	ut_assert_console_end();
+
+	run_command("rtc write 0x30 aabb", 0);
+	ut_assert_console_end();
+
+	run_command("rtc read 0x30 2", 0);
+	ut_assert_nextline("00000030: aa bb                                              ..");
+	ut_assert_console_end();
+
+	run_command("rtc dev 1", 0);
+	ut_assert_nextline("RTC #1 - rtc at 61");
+	ut_assert_console_end();
+
+	run_command("rtc write 0x30 ccdd", 0);
+	ut_assert_console_end();
+
+	run_command("rtc read 0x30 2", 0);
+	ut_assert_nextline("00000030: cc dd                                              ..");
+	ut_assert_console_end();
+
+	/*
+	 * Switch back to device #0, check that its aux registers
+	 * still have the same values.
+	 */
+	run_command("rtc dev 0", 0);
+	ut_assert_nextline("RTC #0 - rtc at 43");
+	ut_assert_console_end();
+
+	run_command("rtc read 0x30 2", 0);
+	ut_assert_nextline("00000030: aa bb                                              ..");
+	ut_assert_console_end();
+
+	return 0;
+}
+DM_TEST(dm_test_rtc_cmd_rw, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
+
 /* Reset the time */
 static int dm_test_rtc_reset(struct unit_test_state *uts)
 {
-- 
2.23.0

[PATCH v4 11/11] test: dm: rtc: add tests of rtc shell command

[Heiko Schocher]

Hello Rasmus,

Am 06.07.2020 um 22:01 schrieb Rasmus Villemoes:
> Add tests of the "list", "read" and "write" subcommands of the rtc
> shell command.
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Heiko Schocher <hs@denx.de>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> ---
>   test/dm/rtc.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 58 insertions(+)

Applied to u-boot-i2c.git master

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

[PATCH v4 00/11] new rtc methods, rtc command, and tests

[Heiko Schocher]

Hello Rasmus,

Am 06.07.2020 um 22:01 schrieb Rasmus Villemoes:
> I need access to registers other than just the timekeeping ones of the
> pcf2127, so I wanted to implement ->read8 and ->write8. But for
> testing these it appeared there was no convenient way to invoke those
> from the shell, so I also ended up adding such a command.
> 
> Also, it seemed more natural to provide array variants that can read
> or write several registers at once, so rtc_ops is expanded a bit.
> 
> Changes in v4:
> 
> - Add CONFIG_CMD_RTC to sandbox defconfigs (new patch 10/11). Not
>    quite sure exactly which it needed to be added to, but at least
>    sandbox and sandbox_flattree showed CI failures.
> 
> - Add Heiko's R-B to the 10 v3 patches (1-9 + 11), and Simon's R-B to 6/11.
> 
> - Fix some checkpatch warnings - I don't really agree with most of

Sorry, I should have mentioned which warnings you should fix ...

>    them - e.g. having to add an empty line in
> 
>    int foo = something();
>    if (foo < 0)
>      return foo;
>    return something_else(foo);
> 
>    doesn't make the code more readable IMO.

You find this rule all over the source code in U-Boot...

>    The remaining checkpatch blurps are things I really don't think
>    warrant "fixing", e.g. "WARNING: ENOSYS means 'invalid syscall nr'
>    and nothing else" seems irrelevant in context of U-Boot, and in any
>    case I've only copied existing practice. For "WARNING: please write
>    a paragraph that describes the config symbol fully", that seems to
>    be a false positive, there's certainly a full help text for CMD_RTC.

Yes, this is fine.

Just applied your patches now, there is one warning in patch

"test: dm: rtc: add test of dm_rtc_read, dm_rtc_write"

CHECK: Comparison to NULL could be written "emul"
#218: FILE: test/dm/rtc.c:162:
+       ut_assert(emul != NULL);

I think, this should be fixed! But looking into the source file, there
are more such lines, so I let this at it is... may this should be cleaned!

Thanks for your work!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de