[PATCH v2 06/10] rtc: add rtc command

[Simon Glass <sjg@chromium.org>]

Hi Rasmus,

On Tue, 19 May 2020 at 16:01, Rasmus Villemoes
<rasmus.villemoes@prevas.dk> wrote:
>
> 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).
>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> ---
>  cmd/Kconfig  |   6 ++
>  cmd/Makefile |   1 +
>  cmd/rtc.c    | 159 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 166 insertions(+)
>  create mode 100644 cmd/rtc.c
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index f9be1988f6..7eea25facd 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1715,6 +1715,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..e26b7ca18f
> --- /dev/null
> +++ b/cmd/rtc.c
> @@ -0,0 +1,159 @@
> +// 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;
> +       u8 *addr;
> +
> +       if (argc < 2 || argc > 3)
> +               return CMD_RET_USAGE;
> +
> +       reg = simple_strtoul(argv[0], NULL, 0);

I think these should be hex (i.e. 16), since that is the norm in U-Boot.

> +       len = simple_strtoul(argv[1], NULL, 0);
> +       if (argc == 3) {
> +               addr = map_sysmem(simple_strtoul(argv[2], NULL, 16), len);
> +       } else {
> +               if (len > sizeof(buf)) {
> +                       printf("can read at most %d registers at a time\n", MAX_RTC_BYTES);

It would be better to loop like print_buffer() does.

> +                       return CMD_RET_FAILURE;
> +               }
> +               addr = buf;
> +       }
> +
> +       ret = rtc_read(dev, reg, addr, len);
> +       if (ret) {
> +               printf("rtc_read() failed: %d\n", ret);
> +               ret = CMD_RET_FAILURE;
> +               goto out;
> +       } else {
> +               ret = CMD_RET_SUCCESS;
> +       }
> +
> +       if (argc == 2) {
> +               while (len--)
> +                       printf("%d: 0x%02x\n", reg++, *addr++);

Perhaps use print_buffer()?

> +       }
> +out:
> +       if (argc == 3)
> +               unmap_sysmem(addr);
> +       return ret;
> +}
> +
> +static int do_rtc_write(struct udevice *dev, int argc, char * const argv[])
> +{
> +       u8 buf[MAX_RTC_BYTES];
> +       u8 *addr;
> +       int reg, len, ret;
> +
> +       if (argc < 2 || argc > 3)
> +               return CMD_RET_USAGE;
> +
> +       reg = simple_strtoul(argv[0], NULL, 0);
> +
> +       if (argc == 3) {
> +               len = simple_strtoul(argv[1], NULL, 0);
> +               addr = map_sysmem(simple_strtoul(argv[2], NULL, 16), len);
> +       } else {
> +               const char *s = argv[1];
> +
> +               /* Convert hexstring, bail out if too long. */
> +               addr = buf;
> +               len = strlen(s);
> +               if (len > 2*MAX_RTC_BYTES) {

Spaces around *

> +                       printf("hex string too long, can write at most %d bytes\n", MAX_RTC_BYTES);

Please can you try checkpatch or patman? This lines seems too long

> +                       return CMD_RET_FAILURE;
> +               }
> +               len /= 2;
> +               if (hex2bin(addr, s, len)) {
> +                       printf("invalid hex string\n");
> +                       return CMD_RET_FAILURE;
> +               }
> +       }
> +
> +       ret = rtc_write(dev, reg, addr, len);
> +       if (ret) {
> +               printf("rtc_write() failed: %d\n", ret);
> +               ret = CMD_RET_FAILURE;
> +       } else {
> +               ret = CMD_RET_SUCCESS;
> +       }
> +
> +       if (argc == 3)
> +               unmap_sysmem(addr);
> +       return ret;
> +}
> +
> +int do_rtc(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> +       static int curr_rtc = 0;
> +       struct udevice *dev;
> +       int ret, idx;
> +
> +       if (argc < 2)
> +               return CMD_RET_USAGE;
> +
> +       argc--;
> +       argv++;
> +
> +       if (!strcmp(argv[0], "list")) {

It is comment in U-Boot to just check the letters that are needed. So
here you could do (*argv[0] == 'l')
> +               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
>

Regards,
Simon