On 3/19/20 6:57 PM, Frédéric Danis wrote:
> To simplify configuration and keep synchronized the PStore/Ramoops between
> U-Boot and the Linux kernel, this commit dynamically adds the Ramoops
> parameters defined in the U-Boot session to the Device Tree.
>
> Signed-off-by: Frédéric Danis <frederic.da...@collabora.com>
> ---
>  cmd/pstore.c          | 38 ++++++++++++++++++++++++++++++++++++++
>  common/image-fdt.c    |  4 ++++
>  doc/pstore.rst        |  2 ++
>  include/fdt_support.h |  3 +++
>  4 files changed, 47 insertions(+)
>
> diff --git a/cmd/pstore.c b/cmd/pstore.c
> index 4e4d70d604..6cad635620 100644
> --- a/cmd/pstore.c
> +++ b/cmd/pstore.c
> @@ -479,6 +479,44 @@ static int do_pstore(cmd_tbl_t *cmdtp, int flag, int 
> argc, char * const argv[])
>       return c->cmd(cmdtp, flag, argc, argv);
>  }
>
> +void fdt_fixup_pstore(void *blob)
> +{
> +     char node[32];
> +     int  nodeoffset;        /* node offset from libfdt */
> +
> +     nodeoffset = fdt_path_offset(blob, "/");
> +     if (nodeoffset < 0) {
> +             /* Not found or something else bad happened. */
> +             log_debug("fdt_path_offset() returned %s\n", 
> fdt_strerror(nodeoffset));

Thanks for implementing device tree support.

Should this be log_err()?

> +             return;
> +     }
> +
> +     nodeoffset = fdt_add_subnode(blob, nodeoffset, "reserved-memory");
> +     if (nodeoffset < 0) {
> +             log_debug("Add 'reserved-memory' node failed: %s\n",
> +                             fdt_strerror(nodeoffset));
> +             return;
> +     }
> +     fdt_appendprop_u32(blob, nodeoffset, "#address-cells", 2);

For adding the first value I guess it is preferable to use the fdt_set*
functions.

fdt_setprop_u32()

> +     fdt_appendprop_u32(blob, nodeoffset, "#size-cells", 2);

fdt_setprop_u32()

> +     fdt_appendprop(blob, nodeoffset, "ranges", NULL, 0);

fdt_setprop_empty()?

> +
> +     sprintf(node, "ramoops@%llx", (unsigned long long)pstore_addr);
> +     nodeoffset = fdt_add_subnode(blob, nodeoffset, node);
> +     if (nodeoffset < 0) {
> +             log_debug("Add '%s' node failed: %s\n", node, 
> fdt_strerror(nodeoffset));

Should this be log_err()?

> +             return;
> +     }
> +     fdt_appendprop_string(blob, nodeoffset, "compatible", "ramoops");

fdt_setprop_string()

> +     fdt_appendprop_u64(blob, nodeoffset, "reg", pstore_addr);

fdt_setprop_u64()

> +     fdt_appendprop_u64(blob, nodeoffset, "reg", pstore_length);

This one is ok.

> +     fdt_appendprop_u32(blob, nodeoffset, "record-size", pstore_record_size);

fdt_setprop_u32(), same below.

Best regards

Heinrich

> +     fdt_appendprop_u32(blob, nodeoffset, "console-size", 
> pstore_console_size);
> +     fdt_appendprop_u32(blob, nodeoffset, "ftrace-size", pstore_ftrace_size);
> +     fdt_appendprop_u32(blob, nodeoffset, "pmsg-size", pstore_pmsg_size);
> +     fdt_appendprop_u32(blob, nodeoffset, "ecc-size", pstore_ecc_size);
> +}
> +
>  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"
> diff --git a/common/image-fdt.c b/common/image-fdt.c
> index 3002948b6b..491d55ad1a 100644
> --- a/common/image-fdt.c
> +++ b/common/image-fdt.c
> @@ -547,6 +547,10 @@ int image_setup_libfdt(bootm_headers_t *images, void 
> *blob,
>       }
>       /* Update ethernet nodes */
>       fdt_fixup_ethernet(blob);
> +#if CONFIG_IS_ENABLED(CMD_PSTORE)
> +     /* Append PStore configuration */
> +     fdt_fixup_pstore(blob);
> +#endif
>       if (IMAGE_OF_BOARD_SETUP) {
>               fdt_ret = ft_board_setup(blob, gd->bd);
>               if (fdt_ret) {
> diff --git a/doc/pstore.rst b/doc/pstore.rst
> index 3ed2e9e0fd..3c84b8b172 100644
> --- a/doc/pstore.rst
> +++ b/doc/pstore.rst
> @@ -24,6 +24,8 @@ i.e.::
>
>  The same values should be set in U-Boot to be able to retrieve the records.
>  This values can be set at build time in U-Boot configuration file, or at 
> runtime.
> +U-Boot automatically patches the Device Tree to pass the Ramoops parameters 
> to
> +the kernel.
>
>  The PStore configuration parameters are:
>
> diff --git a/include/fdt_support.h b/include/fdt_support.h
> index ba14acd7f6..7afbdcfe37 100644
> --- a/include/fdt_support.h
> +++ b/include/fdt_support.h
> @@ -350,4 +350,7 @@ int fdt_update_ethernet_dt(void *blob);
>  #ifdef CONFIG_FSL_MC_ENET
>  void fdt_fixup_board_enet(void *blob);
>  #endif
> +#ifdef CONFIG_CMD_PSTORE
> +void fdt_fixup_pstore(void *blob);
> +#endif
>  #endif /* ifndef __FDT_SUPPORT_H */
>

Reply via email to