[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAGETcx_fSWv-CeOTnWTq4yGPg8BgHsRZQfPa_fcbMZnaTEJBtQ@mail.gmail.com>
Date: Mon, 11 Mar 2024 19:18:30 -0700
From: Saravana Kannan <saravanak@...gle.com>
To: Rob Herring <robh@...nel.org>
Cc: Oreoluwa Babatunde <quic_obabatun@...cinc.com>, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] of: Move all FDT reserved-memory handling into of_reserved_mem.c
On Mon, Mar 11, 2024 at 11:14 AM Rob Herring <robh@...nel.org> wrote:
>
> The split of /reserved-memory handling between fdt.c and
> of_reserved_mem.c makes for reading and restructuring the code
> difficult. As of_reserved_mem.c is only built for
> CONFIG_OF_EARLY_FLATTREE already, move all the code to one spot.
I'm assuming this is a clean copy-paste with maybe some
addition/removal of static keywords.
With that,
Acked-by: Saravana Kannan <saravanak@...gle.com>
-Saravana
>
> Signed-off-by: Rob Herring <robh@...nel.org>
> ---
> Oreoluwa, Please rebase your series on top of this.
>
> drivers/of/fdt.c | 123 +---------------------------------
> drivers/of/of_private.h | 5 +-
> drivers/of/of_reserved_mem.c | 125 ++++++++++++++++++++++++++++++++++-
> 3 files changed, 127 insertions(+), 126 deletions(-)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index e5a385285149..a8a04f27915b 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -17,7 +17,6 @@
> #include <linux/mutex.h>
> #include <linux/of.h>
> #include <linux/of_fdt.h>
> -#include <linux/of_reserved_mem.h>
> #include <linux/sizes.h>
> #include <linux/string.h>
> #include <linux/errno.h>
> @@ -88,7 +87,7 @@ void __init of_fdt_limit_memory(int limit)
> }
> }
>
> -static bool of_fdt_device_is_available(const void *blob, unsigned long node)
> +bool of_fdt_device_is_available(const void *blob, unsigned long node)
> {
> const char *status = fdt_getprop(blob, node, "status", NULL);
>
> @@ -484,126 +483,6 @@ void *initial_boot_params __ro_after_init;
>
> static u32 of_fdt_crc32;
>
> -static int __init early_init_dt_reserve_memory(phys_addr_t base,
> - phys_addr_t size, bool nomap)
> -{
> - if (nomap) {
> - /*
> - * If the memory is already reserved (by another region), we
> - * should not allow it to be marked nomap, but don't worry
> - * if the region isn't memory as it won't be mapped.
> - */
> - if (memblock_overlaps_region(&memblock.memory, base, size) &&
> - memblock_is_region_reserved(base, size))
> - return -EBUSY;
> -
> - return memblock_mark_nomap(base, size);
> - }
> - return memblock_reserve(base, size);
> -}
> -
> -/*
> - * __reserved_mem_reserve_reg() - reserve all memory described in 'reg' property
> - */
> -static int __init __reserved_mem_reserve_reg(unsigned long node,
> - const char *uname)
> -{
> - int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
> - phys_addr_t base, size;
> - int len;
> - const __be32 *prop;
> - int first = 1;
> - bool nomap;
> -
> - prop = of_get_flat_dt_prop(node, "reg", &len);
> - if (!prop)
> - return -ENOENT;
> -
> - if (len && len % t_len != 0) {
> - pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n",
> - uname);
> - return -EINVAL;
> - }
> -
> - nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
> -
> - while (len >= t_len) {
> - base = dt_mem_next_cell(dt_root_addr_cells, &prop);
> - size = dt_mem_next_cell(dt_root_size_cells, &prop);
> -
> - if (size &&
> - early_init_dt_reserve_memory(base, size, nomap) == 0)
> - pr_debug("Reserved memory: reserved region for node '%s': base %pa, size %lu MiB\n",
> - uname, &base, (unsigned long)(size / SZ_1M));
> - else
> - pr_err("Reserved memory: failed to reserve memory for node '%s': base %pa, size %lu MiB\n",
> - uname, &base, (unsigned long)(size / SZ_1M));
> -
> - len -= t_len;
> - if (first) {
> - fdt_reserved_mem_save_node(node, uname, base, size);
> - first = 0;
> - }
> - }
> - return 0;
> -}
> -
> -/*
> - * __reserved_mem_check_root() - check if #size-cells, #address-cells provided
> - * in /reserved-memory matches the values supported by the current implementation,
> - * also check if ranges property has been provided
> - */
> -static int __init __reserved_mem_check_root(unsigned long node)
> -{
> - const __be32 *prop;
> -
> - prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
> - if (!prop || be32_to_cpup(prop) != dt_root_size_cells)
> - return -EINVAL;
> -
> - prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
> - if (!prop || be32_to_cpup(prop) != dt_root_addr_cells)
> - return -EINVAL;
> -
> - prop = of_get_flat_dt_prop(node, "ranges", NULL);
> - if (!prop)
> - return -EINVAL;
> - return 0;
> -}
> -
> -/*
> - * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory
> - */
> -static int __init fdt_scan_reserved_mem(void)
> -{
> - int node, child;
> - const void *fdt = initial_boot_params;
> -
> - node = fdt_path_offset(fdt, "/reserved-memory");
> - if (node < 0)
> - return -ENODEV;
> -
> - if (__reserved_mem_check_root(node) != 0) {
> - pr_err("Reserved memory: unsupported node format, ignoring\n");
> - return -EINVAL;
> - }
> -
> - fdt_for_each_subnode(child, fdt, node) {
> - const char *uname;
> - int err;
> -
> - if (!of_fdt_device_is_available(fdt, child))
> - continue;
> -
> - uname = fdt_get_name(fdt, child, NULL);
> -
> - err = __reserved_mem_reserve_reg(child, uname);
> - if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL))
> - fdt_reserved_mem_save_node(child, uname, 0, 0);
> - }
> - return 0;
> -}
> -
> /*
> * fdt_reserve_elfcorehdr() - reserves memory for elf core header
> *
> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
> index f38397c7b582..485483524b7f 100644
> --- a/drivers/of/of_private.h
> +++ b/drivers/of/of_private.h
> @@ -175,8 +175,9 @@ static inline struct device_node *__of_get_dma_parent(const struct device_node *
> }
> #endif
>
> +int fdt_scan_reserved_mem(void);
> void fdt_init_reserved_mem(void);
> -void fdt_reserved_mem_save_node(unsigned long node, const char *uname,
> - phys_addr_t base, phys_addr_t size);
> +
> +bool of_fdt_device_is_available(const void *blob, unsigned long node);
>
> #endif /* _LINUX_OF_PRIVATE_H */
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 7ec94cfcbddb..8236ecae2953 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -12,6 +12,7 @@
> #define pr_fmt(fmt) "OF: reserved mem: " fmt
>
> #include <linux/err.h>
> +#include <linux/libfdt.h>
> #include <linux/of.h>
> #include <linux/of_fdt.h>
> #include <linux/of_platform.h>
> @@ -58,8 +59,8 @@ static int __init early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
> /*
> * fdt_reserved_mem_save_node() - save fdt node for second pass initialization
> */
> -void __init fdt_reserved_mem_save_node(unsigned long node, const char *uname,
> - phys_addr_t base, phys_addr_t size)
> +static void __init fdt_reserved_mem_save_node(unsigned long node, const char *uname,
> + phys_addr_t base, phys_addr_t size)
> {
> struct reserved_mem *rmem = &reserved_mem[reserved_mem_count];
>
> @@ -77,6 +78,126 @@ void __init fdt_reserved_mem_save_node(unsigned long node, const char *uname,
> return;
> }
>
> +static int __init early_init_dt_reserve_memory(phys_addr_t base,
> + phys_addr_t size, bool nomap)
> +{
> + if (nomap) {
> + /*
> + * If the memory is already reserved (by another region), we
> + * should not allow it to be marked nomap, but don't worry
> + * if the region isn't memory as it won't be mapped.
> + */
> + if (memblock_overlaps_region(&memblock.memory, base, size) &&
> + memblock_is_region_reserved(base, size))
> + return -EBUSY;
> +
> + return memblock_mark_nomap(base, size);
> + }
> + return memblock_reserve(base, size);
> +}
> +
> +/*
> + * __reserved_mem_reserve_reg() - reserve all memory described in 'reg' property
> + */
> +static int __init __reserved_mem_reserve_reg(unsigned long node,
> + const char *uname)
> +{
> + int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
> + phys_addr_t base, size;
> + int len;
> + const __be32 *prop;
> + int first = 1;
> + bool nomap;
> +
> + prop = of_get_flat_dt_prop(node, "reg", &len);
> + if (!prop)
> + return -ENOENT;
> +
> + if (len && len % t_len != 0) {
> + pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n",
> + uname);
> + return -EINVAL;
> + }
> +
> + nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
> +
> + while (len >= t_len) {
> + base = dt_mem_next_cell(dt_root_addr_cells, &prop);
> + size = dt_mem_next_cell(dt_root_size_cells, &prop);
> +
> + if (size &&
> + early_init_dt_reserve_memory(base, size, nomap) == 0)
> + pr_debug("Reserved memory: reserved region for node '%s': base %pa, size %lu MiB\n",
> + uname, &base, (unsigned long)(size / SZ_1M));
> + else
> + pr_err("Reserved memory: failed to reserve memory for node '%s': base %pa, size %lu MiB\n",
> + uname, &base, (unsigned long)(size / SZ_1M));
> +
> + len -= t_len;
> + if (first) {
> + fdt_reserved_mem_save_node(node, uname, base, size);
> + first = 0;
> + }
> + }
> + return 0;
> +}
> +
> +/*
> + * __reserved_mem_check_root() - check if #size-cells, #address-cells provided
> + * in /reserved-memory matches the values supported by the current implementation,
> + * also check if ranges property has been provided
> + */
> +static int __init __reserved_mem_check_root(unsigned long node)
> +{
> + const __be32 *prop;
> +
> + prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
> + if (!prop || be32_to_cpup(prop) != dt_root_size_cells)
> + return -EINVAL;
> +
> + prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
> + if (!prop || be32_to_cpup(prop) != dt_root_addr_cells)
> + return -EINVAL;
> +
> + prop = of_get_flat_dt_prop(node, "ranges", NULL);
> + if (!prop)
> + return -EINVAL;
> + return 0;
> +}
> +
> +/*
> + * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory
> + */
> +int __init fdt_scan_reserved_mem(void)
> +{
> + int node, child;
> + const void *fdt = initial_boot_params;
> +
> + node = fdt_path_offset(fdt, "/reserved-memory");
> + if (node < 0)
> + return -ENODEV;
> +
> + if (__reserved_mem_check_root(node) != 0) {
> + pr_err("Reserved memory: unsupported node format, ignoring\n");
> + return -EINVAL;
> + }
> +
> + fdt_for_each_subnode(child, fdt, node) {
> + const char *uname;
> + int err;
> +
> + if (!of_fdt_device_is_available(fdt, child))
> + continue;
> +
> + uname = fdt_get_name(fdt, child, NULL);
> +
> + err = __reserved_mem_reserve_reg(child, uname);
> + if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL))
> + fdt_reserved_mem_save_node(child, uname, 0, 0);
> + }
> + return 0;
> +}
> +
> /*
> * __reserved_mem_alloc_in_range() - allocate reserved memory described with
> * 'alloc-ranges'. Choose bottom-up/top-down depending on nearby existing
> --
> 2.43.0
>
Powered by blists - more mailing lists