[U-Boot] cmd: Add command to display or save Linux PStore dumps

This patch adds a new pstore command allowing to display or save ramoops
logs (oops, panic, console, ftrace and user) generated by a previous
kernel crash.
PStore parameters can be set in U-Boot configuration file, or at run-time
using "pstore set" command. Records size should be the same as the ones
used by kernel, and should be a power of 2.
This command allows:
- to display uncompressed logs
- to save compressed or uncompressed logs, compressed logs are saved as a
  compressed stream, it may need some work to be able to decompress it,
  e.g. adding a fake header:
  "printf "\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\x00" |
  cat - dmesg-ramoops-0.enc.z | gzip -dc"
- ECC part is not used to check memory corruption
- only 1st FTrace log is displayed or saved

Signed-off-by: Frédéric Danis <frederic.danis@collabora.com>
---
 cmd/Kconfig  |  63 ++++++++
 cmd/Makefile |   1 +
 cmd/pstore.c | 417 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 481 insertions(+)
 create mode 100644 cmd/pstore.c

On 11/18/19 11:09 AM, Frédéric Danis wrote:
> This patch adds a new pstore command allowing to display or save ramoops
> logs (oops, panic, console, ftrace and user) generated by a previous
> kernel crash.
> PStore parameters can be set in U-Boot configuration file, or at run-time
> using "pstore set" command. Records size should be the same as the ones
> used by kernel, and should be a power of 2.
> This command allows:
> - to display uncompressed logs
> - to save compressed or uncompressed logs, compressed logs are saved as a
>    compressed stream, it may need some work to be able to decompress it,
>    e.g. adding a fake header:
>    "printf "\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\x00" |
>    cat - dmesg-ramoops-0.enc.z | gzip -dc"
> - ECC part is not used to check memory corruption
> - only 1st FTrace log is displayed or saved

Thanks for your contribution.

Please, address the next revision to Tom.

How to use pstore is conjunction with the Linux kernel is not self
evident. I would be especially interested to understand how I can
collect the necessary settings form the Linux kernel. Please, provide a
description in reStructured text format in doc/ and link it up in the
documentation created by

make htmldocs

Is there a way to test this command? We look for unit tests as far as
possible, e.g. using Python tests in test/py/tests. You could for
instance provide an oops file, load it into memory and than pstore the oops.

>
> Signed-off-by: Frédéric Danis <frederic.danis@collabora.com>
> ---
>   cmd/Kconfig  |  63 ++++++++
>   cmd/Makefile |   1 +
>   cmd/pstore.c | 417 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 481 insertions(+)
>   create mode 100644 cmd/pstore.c
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index cf982ff65e..7e28100b98 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1675,6 +1675,69 @@ config CMD_QFW
>   	  feature is to allow easy loading of files passed to qemu-system
>   	  via -kernel / -initrd
>
> +config CMD_PSTORE
> +	bool "pstore"
> +	help
> +	  This provides access to Linux PStore. The main feature is to allow to
> +	  display or save PStore records.
> +
> +config CMD_PSTORE_ADDR
G> +	hex "Mem Address"
> +	depends on CMD_PSTORE
> +	default "0x0"
> +	help
> +	  Base addr used for PStore ramoops memory, should be identical to
> +	  ramoops.mem_address parameter used by kernel
> +
> +config CMD_PSTORE_SIZE
> +	hex "Mem size"
> +	depends on CMD_PSTORE
> +	default "0x0"
> +	help
> +	  Size of PStore ramoops memory, should be identical to ramoops.mem_size
> +	  parameter used by kernel
> +
> +config CMD_PSTORE_RECORD_SIZE
> +	hex "Dump record size"
> +	depends on CMD_PSTORE
> +	default "0x1000"
> +	help
> +	  Size of each dump done on oops/panic, should be identical to
> +	  ramoops.record_size parameter used by kernel
> +
> +config CMD_PSTORE_CONSOLE_SIZE
> +	hex "Kernel console log size"
> +	depends on CMD_PSTORE
> +	default "0x1000"
> +	help
> +	  Size of kernel console log, should be identical to
> +	  ramoops.console_size parameter used by kernel
> +
> +config CMD_PSTORE_FTRACE_SIZE
> +	hex "FTrace log size"
> +	depends on CMD_PSTORE
> +	default "0x1000"
> +	help
> +	  Size of ftrace log, should be identical to ramoops.ftrace_size
> +	  parameter used by kernel
> +
> +config CMD_PSTORE_PMSG_SIZE
> +	hex "User space message log size"
> +	depends on CMD_PSTORE
> +	default "0x1000"
> +	help
> +	  Size of user space message log, should be identical to
> +	  ramoops.pmsg_size parameter used by kernel
> +
> +config CMD_PSTORE_ECC_SIZE
> +	int "ECC size"
> +	depends on CMD_PSTORE
> +	default "0"
> +	help
> +	if non-zero, the option enables ECC support and specifies ECC buffer
> +	size in bytes (1 is a special value, means 16 bytes ECC), should be
> +	identical to ramoops.ramoops_ecc parameter used by kernel
> +
>   source "cmd/mvebu/Kconfig"
>
>   config CMD_TERMINAL
> diff --git a/cmd/Makefile b/cmd/Makefile
> index 2d723ea0f0..0f3196b3d6 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -108,6 +108,7 @@ ifdef CONFIG_PCI
>   obj-$(CONFIG_CMD_PCI) += pci.o
>   endif
>   obj-$(CONFIG_CMD_PINMUX) += pinmux.o
> +obj-$(CONFIG_CMD_PSTORE) += pstore.o
>   obj-$(CONFIG_CMD_PXE) += pxe.o
>   obj-$(CONFIG_CMD_WOL) += wol.o
>   obj-$(CONFIG_CMD_QFW) += qfw.o
> diff --git a/cmd/pstore.c b/cmd/pstore.c
> new file mode 100644
> index 0000000000..60266a3027
> --- /dev/null
> +++ b/cmd/pstore.c
> @@ -0,0 +1,417 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright © 2019 Collabora Ltd
> + *
> + */
> +
> +#include <config.h>
> +#include <common.h>
> +#include <fs.h>
> +#include <memalign.h>
> +#include <part.h>
> +#include <asm/atomic.h>
> +
> +struct persistent_ram_buffer {
> +	u32    sig;
> +	u32    start;
> +	u32    size;
> +	u8     data[0];
> +};
> +
> +#define PERSISTENT_RAM_SIG (0x43474244) /* DBGC */
> +#define RAMOOPS_KERNMSG_HDR "===="
> +
> +#define PSTORE_TYPE_DMESG 0
> +#define PSTORE_TYPE_CONSOLE 2
> +#define PSTORE_TYPE_FTRACE 3
> +#define PSTORE_TYPE_PMSG 7
> +#define PSTORE_TYPE_ALL 255
> +
> +static char *pstore_addr = (char *)CONFIG_CMD_PSTORE_ADDR;
> +static ulong pstore_length = CONFIG_CMD_PSTORE_SIZE;
> +static unsigned int pstore_record_size = CONFIG_CMD_PSTORE_RECORD_SIZE;
> +static unsigned int pstore_console_size = CONFIG_CMD_PSTORE_CONSOLE_SIZE;
> +static unsigned int pstore_ftrace_size = CONFIG_CMD_PSTORE_FTRACE_SIZE;
> +static unsigned int pstore_pmsg_size = CONFIG_CMD_PSTORE_PMSG_SIZE;
> +static unsigned int pstore_ecc_size = CONFIG_CMD_PSTORE_ECC_SIZE;
> +static unsigned int buffer_size;
> +
> +/* Check kernel header and get compression flag if available.
> + * Kernel header: ====<secs>.<nsecs>[-<compression>]\n
> + * compression: C if record is compressed, else D
> + * Returns length of kernel header.
> + */

Please, use Sphinx style function comments, cf.
https://www.kernel.org/doc/html/v4.10/doc-guide/kernel-doc.html#writing-kernel-doc-comments

> +static int pstore_read_kmsg_hdr(char *buffer, bool *compressed)
> +{
> +	char *ptr = buffer;
> +	*compressed = false;
> +
> +	if (strncmp(RAMOOPS_KERNMSG_HDR, ptr, strlen(RAMOOPS_KERNMSG_HDR)) != 0)
> +		return 0;
> +
> +	ptr += strlen(RAMOOPS_KERNMSG_HDR);
> +
> +	ptr = strchr(ptr, '\n');
> +	if (!ptr)
> +		return 0;
> +
> +	if (ptr[-2] == '-' && ptr[-1] == 'C')
> +		*compressed = true;
> +
> +	return ptr - buffer + 1;
> +}
> +
> +/* Get unwrapped record buffer
> + * Record header: <signature><start><size>
> + * signature is 'DBGC' for all records except for Ftrace's record(s) wich use
> + *   LINUX_VERSION_CODE ^ 'DBGC', use 0 to prevent checking signature
> + * start and size are 4 bytes long
> + * Returns record's length
> + */
> +static u32 pstore_get_buffer(u32 sig, char *buffer, u32 size, char *dest)
> +{
> +	struct persistent_ram_buffer *prb = (struct persistent_ram_buffer *)buffer;
> +
> +	if (sig == 0 || prb->sig == sig) {
> +		if (prb->size == 0) {
> +			pr_debug("found existing empty buffer\n");
> +			return 0;
> +		}
> +
> +		if (prb->size > size || prb->start > prb->size) {
> +			pr_debug("found existing invalid buffer, size %zu, start %zu\n",

Please, compile your code both in 32bit mode and 64bit mode. In 64bit
mode I see the following warnings which should be fixed.

In file included from include/linux/bug.h:7,
                  from include/common.h:28,
                  from cmd/pstore.c:8:
cmd/pstore.c: In function ‘pstore_get_buffer’:
cmd/pstore.c:82:13: error: format ‘%zu’ expects argument of type
‘size_t’, but argument 2 has type ‘u32’ {aka ‘unsigned int’}
[-Werror=format=]
    82 |    pr_debug("found existing invalid buffer, size %zu, start %zu\n",
       |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/printk.h:18:9: note: in definition of macro ‘printk’
    18 |  printf(fmt, ##__VA_ARGS__)
       |         ^~~
include/linux/printk.h:73:2: note: in expansion of macro ‘no_printk’
    73 |  no_printk(pr_fmt(fmt), ##__VA_ARGS__)
       |  ^~~~~~~~~
include/linux/printk.h:73:12: note: in expansion of macro ‘pr_fmt’
    73 |  no_printk(pr_fmt(fmt), ##__VA_ARGS__)
       |            ^~~~~~
cmd/pstore.c:82:4: note: in expansion of macro ‘pr_debug’
    82 |    pr_debug("found existing invalid buffer, size %zu, start %zu\n",
       |    ^~~~~~~~
cmd/pstore.c:82:52: note: format string is defined here
    82 |    pr_debug("found existing invalid buffer, size %zu, start %zu\n",
       |                                                  ~~^
       |                                                    |
       |                                                    long
unsigned int
       |                                                  %u
In file included from include/linux/bug.h:7,
                  from include/common.h:28,
                  from cmd/pstore.c:8:
cmd/pstore.c:82:13: error: format ‘%zu’ expects argument of type
‘size_t’, but argument 3 has type ‘u32’ {aka ‘unsigned int’}
[-Werror=format=]
    82 |    pr_debug("found existing invalid buffer, size %zu, start %zu\n",
       |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/printk.h:18:9: note: in definition of macro ‘printk’
    18 |  printf(fmt, ##__VA_ARGS__)
       |         ^~~
include/linux/printk.h:73:2: note: in expansion of macro ‘no_printk’
    73 |  no_printk(pr_fmt(fmt), ##__VA_ARGS__)
       |  ^~~~~~~~~
include/linux/printk.h:73:12: note: in expansion of macro ‘pr_fmt’
    73 |  no_printk(pr_fmt(fmt), ##__VA_ARGS__)
       |            ^~~~~~
cmd/pstore.c:82:4: note: in expansion of macro ‘pr_debug’
    82 |    pr_debug("found existing invalid buffer, size %zu, start %zu\n",
       |    ^~~~~~~~
cmd/pstore.c:82:63: note: format string is defined here
    82 |    pr_debug("found existing invalid buffer, size %zu, start %zu\n",
       |                                                             ~~^
       |                                                               |
       |
long unsigned int
       |                                                             %u
In file included from include/linux/bug.h:7,
                  from include/common.h:28,
                  from cmd/pstore.c:8:
cmd/pstore.c:91:11: error: format ‘%zu’ expects argument of type
‘size_t’, but argument 2 has type ‘u32’ {aka ‘unsigned int’}
[-Werror=format=]
    91 |  pr_debug("found existing buffer, size %zu, start %zu\n",
       |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/printk.h:18:9: note: in definition of macro ‘printk’
    18 |  printf(fmt, ##__VA_ARGS__)
       |         ^~~
include/linux/printk.h:73:2: note: in expansion of macro ‘no_printk’
    73 |  no_printk(pr_fmt(fmt), ##__VA_ARGS__)
       |  ^~~~~~~~~
include/linux/printk.h:73:12: note: in expansion of macro ‘pr_fmt’
    73 |  no_printk(pr_fmt(fmt), ##__VA_ARGS__)
       |            ^~~~~~
cmd/pstore.c:91:2: note: in expansion of macro ‘pr_debug’
    91 |  pr_debug("found existing buffer, size %zu, start %zu\n",
       |  ^~~~~~~~
cmd/pstore.c:91:42: note: format string is defined here
    91 |  pr_debug("found existing buffer, size %zu, start %zu\n",
       |                                        ~~^
       |                                          |
       |                                          long unsigned int
       |                                        %u
In file included from include/linux/bug.h:7,
                  from include/common.h:28,
                  from cmd/pstore.c:8:
cmd/pstore.c:91:11: error: format ‘%zu’ expects argument of type
‘size_t’, but argument 3 has type ‘u32’ {aka ‘unsigned int’}
[-Werror=format=]
    91 |  pr_debug("found existing buffer, size %zu, start %zu\n",
       |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/printk.h:18:9: note: in definition of macro ‘printk’
    18 |  printf(fmt, ##__VA_ARGS__)
       |         ^~~
include/linux/printk.h:73:2: note: in expansion of macro ‘no_printk’
    73 |  no_printk(pr_fmt(fmt), ##__VA_ARGS__)
       |  ^~~~~~~~~
include/linux/printk.h:73:12: note: in expansion of macro ‘pr_fmt’
    73 |  no_printk(pr_fmt(fmt), ##__VA_ARGS__)
       |            ^~~~~~
cmd/pstore.c:91:2: note: in expansion of macro ‘pr_debug’
    91 |  pr_debug("found existing buffer, size %zu, start %zu\n",
       |  ^~~~~~~~
cmd/pstore.c:91:53: note: format string is defined here
    91 |  pr_debug("found existing buffer, size %zu, start %zu\n",
       |                                                   ~~^
       |                                                     |
       |                                                     long
unsigned int
       |                                                   %u



> +				 prb->size, prb->start);
> +			return 0;
> +		}
> +	} else {
> +		pr_debug("no valid data in buffer (sig = 0x%08x)\n", prb->sig);
> +		return 0;
> +	}
> +
> +	pr_debug("found existing buffer, size %zu, start %zu\n",
> +		 prb->size, prb->start);

Please, use either printf() or debug() or use the logging functions
described in doc/README.log.

> +
> +	memcpy(dest, &prb->data[prb->start], prb->size - prb->start);
> +	memcpy(dest + prb->size - prb->start, &prb->data[0], prb->start);
> +
> +	return prb->size;
> +}
> +

Please, add missing function descriptions.

> +static void pstore_init_buffer_size(void)
> +{
> +	if (pstore_record_size > buffer_size)
> +		buffer_size = pstore_record_size;
> +
> +	if (pstore_console_size > buffer_size)
> +		buffer_size = pstore_console_size;
> +
> +	if (pstore_ftrace_size > buffer_size)
> +		buffer_size = pstore_ftrace_size;
> +
> +	if (pstore_pmsg_size > buffer_size)
> +		buffer_size = pstore_pmsg_size;
> +}
> +
> +static int pstore_set(cmd_tbl_t *cmdtp, int flag,  int argc,
> +		      char * const argv[])
> +{
> +	if (argc < 3)
> +		return CMD_RET_USAGE;
> +
> +	/* Address is specified since argc > 2
> +	 */
> +	pstore_addr = (char *)simple_strtoul(argv[1], NULL, 16);
> +
> +	/* Length is specified since argc > 2
> +	 */
> +	pstore_length = simple_strtoul(argv[2], NULL, 16);
> +
> +	if (argc > 3)
> +		pstore_record_size = simple_strtoul(argv[3], NULL, 16);
> +
> +	if (argc > 4)
> +		pstore_console_size = simple_strtoul(argv[4], NULL, 16);
> +
> +	if (argc > 5)
> +		pstore_ftrace_size = simple_strtoul(argv[5], NULL, 16);
> +
> +	if (argc > 6)
> +		pstore_pmsg_size = simple_strtoul(argv[6], NULL, 16);
> +
> +	if (argc > 7)
> +		pstore_ecc_size = simple_strtoul(argv[7], NULL, 16);
> +
> +	if (pstore_length < (pstore_record_size + pstore_console_size
> +			     + pstore_ftrace_size + pstore_pmsg_size)) {
> +		printf("pstore <len> should be larger than the sum of all records sizes\n");
> +		pstore_length = 0;
> +	}
> +
> +	pr_debug("pstore set done: start 0x%p - length 0x%lX\n", pstore_addr,
> +		 pstore_length);
> +
> +	return 0;
> +}
> +
> +static void pstore_print_buffer(char *type, char *buffer, u32 size)
> +{
> +	u32 i = 0;
> +
> +	printf("**** %s\n", type);
> +	while (i < size && buffer[i] != 0) {
> +		putc(buffer[i]);
> +		i++;
> +	}
> +}
> +
> +static int pstore_display(cmd_tbl_t *cmdtp, int flag,  int argc,
> +			  char * const argv[])
> +{
> +	int type = PSTORE_TYPE_ALL;
> +	char *ptr;
> +	char *buffer;
> +	u32 size;
> +	int header_len = 0;
> +	bool compressed;
> +
> +	if (argc > 1) {
> +		if (!strcmp(argv[1], "dump"))
> +			type = PSTORE_TYPE_DMESG;
> +		else if (!strcmp(argv[1], "console"))
> +			type = PSTORE_TYPE_CONSOLE;
> +		else if (!strcmp(argv[1], "ftrace"))
> +			type = PSTORE_TYPE_FTRACE;
> +		else if (!strcmp(argv[1], "user"))
	> +			type = PSTORE_TYPE_PMSG;
> +		else
> +			return CMD_RET_USAGE;
> +	}
> +
> +	if (pstore_length == 0) {
> +		printf("Please set PStore configuration\n");
> +		return CMD_RET_USAGE;
> +	}
> +
> +	if (buffer_size == 0)
> +		pstore_init_buffer_size();
> +
> +	buffer = malloc_cache_aligned(buffer_size);
> +
> +	if (type == PSTORE_TYPE_DMESG || type == PSTORE_TYPE_ALL) {
> +		ptr = pstore_addr;
> +		char *ptr_end = ptr + pstore_length - pstore_pmsg_size
> +				- pstore_ftrace_size - pstore_console_size;
> +
> +		if (argc > 2) {
> +			ptr += simple_strtoul(argv[2], NULL, 10)
> +				* pstore_record_size;
> +			ptr_end = ptr + pstore_record_size;
> +		}
> +
> +		while (ptr < ptr_end) {
> +			size = pstore_get_buffer(PERSISTENT_RAM_SIG, ptr,
> +						 pstore_record_size, buffer);
> +			ptr += pstore_record_size;
> +
> +			if (size == 0)
> +				continue;
> +
> +			header_len = pstore_read_kmsg_hdr(buffer, &compressed);
> +			if (header_len == 0) {
> +				pr_debug("no valid kernel header\n");
> +				continue;
> +			}
> +
> +			if (compressed) {
> +				printf("Compressed buffer, display not available\n");
> +				continue;
> +			}
> +
> +			pstore_print_buffer("Dump", buffer + header_len,
> +					    size - header_len);
> +		}
> +	}
> +
> +	if (type == PSTORE_TYPE_CONSOLE || type == PSTORE_TYPE_ALL) {
> +		ptr = pstore_addr + pstore_length - pstore_pmsg_size
> +			- pstore_ftrace_size - pstore_console_size;
> +		size = pstore_get_buffer(PERSISTENT_RAM_SIG, ptr,
> +					 pstore_console_size, buffer);
> +		if (size != 0)
> +			pstore_print_buffer("Console", buffer, size);
> +	}
> +
> +	if (type == PSTORE_TYPE_FTRACE || type == PSTORE_TYPE_ALL) {
> +		ptr = pstore_addr + pstore_length - pstore_pmsg_size
> +		- pstore_ftrace_size;
> +		/* The FTrace record(s) uses LINUX_VERSION_CODE ^ 'DBGC'
> +		 * signature, pass 0 to pstore_get_buffer to prevent
> +		 * checking it
> +		 */
> +		size = pstore_get_buffer(0, ptr, pstore_ftrace_size, buffer);
> +		if (size != 0)
> +			pstore_print_buffer("FTrace", buffer, size);
> +	}
> +
> +	if (type == PSTORE_TYPE_PMSG || type == PSTORE_TYPE_ALL) {
> +		ptr = pstore_addr + pstore_length - pstore_pmsg_size;
> +		size = pstore_get_buffer(PERSISTENT_RAM_SIG, ptr,
> +					 pstore_pmsg_size, buffer);
> +		if (size != 0)
> +			pstore_print_buffer("User", buffer, size);
> +	}
> +
> +	free(buffer);
> +
> +	return 0;
> +}
> +
> +static int pstore_save(cmd_tbl_t *cmdtp, int flag,  int argc,
> +		       char * const argv[])
> +{
> +	char *ptr, *ptr_end;
> +	char *buffer;
> +	char *save_argv[6];
> +	char addr[11], length[11];
> +	char path[256];
> +	u32 size;
> +	unsigned int index;
> +	int header_len = 0;
> +	bool compressed;
> +
> +	if (argc < 4)
> +		return CMD_RET_USAGE;
> +
> +	if (pstore_length == 0) {
> +		printf("Please set PStore configuration\n");
> +		return CMD_RET_USAGE;
> +	}
> +
> +	if (buffer_size == 0)
> +		pstore_init_buffer_size();
> +
> +	buffer = malloc_cache_aligned(buffer_size);
> +	sprintf(addr, "0x%p", buffer);
> +
> +	save_argv[0] = argv[0];
> +	save_argv[1] = argv[1];
> +	save_argv[2] = argv[2];
> +	save_argv[3] = addr;
> +	save_argv[4] = path;
> +	save_argv[5] = length;
> +
> +	/* Save all Dump records */
> +	ptr = pstore_addr;
> +	ptr_end = ptr + pstore_length - pstore_pmsg_size - pstore_ftrace_size
> +				- pstore_console_size;
> +	index = 0;
> +	while (ptr < ptr_end) {
> +		size = pstore_get_buffer(PERSISTENT_RAM_SIG, ptr,
> +					 pstore_record_size, buffer);
> +		ptr += pstore_record_size;
> +
> +		if (size == 0)
> +			continue;
> +
> +		header_len = pstore_read_kmsg_hdr(buffer, &compressed);
> +		if (header_len == 0) {
> +			pr_debug("no valid kernel header\n");
> +			continue;
> +		}
> +
> +		sprintf(addr, "0x%p", buffer + header_len);
> +		sprintf(length, "0x%X", size - header_len);
> +		sprintf(path, "%s/dmesg-ramoops-%u%s", argv[3], index,
> +			compressed ? ".enc.z" : "");
> +		do_save(cmdtp, flag, 6, save_argv, FS_TYPE_ANY);
> +		index++;
> +	}
> +
> +	sprintf(addr, "0x%p", buffer);
> +
> +	/* Save Console record */
> +	size = pstore_get_buffer(PERSISTENT_RAM_SIG, ptr, pstore_console_size,
> +				 buffer);
> +	if (size != 0) {
> +		sprintf(length, "0x%X", size);
> +		sprintf(path, "%s/console-ramoops-0", argv[3]);
> +		do_save(cmdtp, flag, 6, save_argv, FS_TYPE_ANY);
> +	}
> +	ptr += pstore_console_size;
> +
> +	/* Save FTrace record(s)
> +	 * The FTrace record(s) uses LINUX_VERSION_CODE ^ 'DBGC' signature,
> +	 * pass 0 to pstore_get_buffer to prevent checking it
> +	 */
> +	size = pstore_get_buffer(0, ptr, pstore_ftrace_size, buffer);
> +	if (size != 0) {
> +		sprintf(length, "0x%X", size);
> +		sprintf(path, "%s/ftrace-ramoops-0", argv[3]);
> +		do_save(cmdtp, flag, 6, save_argv, FS_TYPE_ANY);
> +	}
> +	ptr += pstore_ftrace_size;
> +
> +	/* Save Console record */
> +	size = pstore_get_buffer(PERSISTENT_RAM_SIG, ptr, pstore_pmsg_size,
> +				 buffer);
> +	if (size != 0) {
> +		sprintf(length, "0x%X", size);
> +		sprintf(path, "%s/pmsg-ramoops-0", argv[3]);
> +		do_save(cmdtp, flag, 6, save_argv, FS_TYPE_ANY);
> +	}
> +
> +	free(buffer);
> +
> +	return 0;
> +}
> +
> +static cmd_tbl_t cmd_pstore_sub[] = {
> +	U_BOOT_CMD_MKENT(set, 8, 0, pstore_set, "", ""),
> +	U_BOOT_CMD_MKENT(display, 3, 0, pstore_display, "", ""),
> +	U_BOOT_CMD_MKENT(save, 4, 0, pstore_save, "", ""),
> +};
> +
> +static int do_pstore(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> +	cmd_tbl_t *c;
> +
> +	if (argc < 2)
> +		return CMD_RET_USAGE;
> +
> +	/* Strip off leading argument */
> +	argc--;
> +	argv++;
> +
> +	c = find_cmd_tbl(argv[0], &cmd_pstore_sub[0],

I suggest to replace '&cmd_pstore_sub[0]' by 'cmd_pstore_sub'. That is
easier to read.

Best regards

Heinrich

> +			 ARRAY_SIZE(cmd_pstore_sub));
> +
> +	if (!c)
> +		return CMD_RET_USAGE;
> +
> +	return c->cmd(cmdtp, flag, argc, argv);
> +}
> +
> +U_BOOT_CMD(pstore, 10, 0, do_pstore,
> +	   "Manage Linux Persistent Storage",
> +	   "set <addr> <len> [record-size] [console-size] [ftrace-size] [pmsg_size] [ecc-size]\n"
> +	   "- Set pstore reserved memory info, starting at 'addr' for 'len' bytes.\n"
> +	   "  Default length for records is 4K.\n"
> +	   "  'record-size' is the size of one panic or oops record ('dump' type).\n"
> +	   "  'console-size' is the size of the kernel logs record.\n"
> +	   "  'ftrace-size' is the size of the ftrace record(s), depending on 'flags'\n"
> +	   "  this can be a single record or divided in parts based on number of CPUs.\n"
> +	   "  'pmsg-size' is the size of the user space logs record.\n"
> +	   "  'ecc-size' enables/disables ECC support and specifies ECC buffer size in\n"
> +	   "  bytes (0 disables it, 1 is a special value, means 16 bytes ECC).\n"
> +	   "pstore display [record-type] [nb]\n"
> +	   "- Display existing records in pstore reserved memory. A 'record-type' can\n"
> +	   "  be given to only display records of this kind. 'record-type' can be one\n"
> +	   "  of 'dump', 'console', 'ftrace' or 'user'. For 'dump' and 'ftrace' types,\n"
> +	   "  a 'nb' can be given to only display one record.\n"
> +	   "pstore save <interface> <dev[:part]> <directory-path>\n"
> +	   "- Save existing records in pstore reserved memory under 'directory path'\n"
> +	   "  to partition 'part' on device type 'interface' instance 'dev'.\n"
> +	   "  Filenames are automatically generated, depending on record type, like\n"
> +	   "  in /sys/fs/pstore under Linux.\n"
> +	   "  The 'directory-path' should already exist.\n"
> +);
>