Re: [PATCH v5 3/4] of: reserved_mem: Use the unflatten_devicetree APIs to scan reserved mem. nodes

From: Rob Herring
Date: Thu Apr 11 2024 - 15:00:20 EST

Next message: Yang Shi: "Re: [PATCH] mm/rmap: do not add fully unmapped large folio to deferred split list"
Previous message: Heiko Stuebner: "Re: [PATCH 13/18] i2c: rk3x: remove printout on handled timeouts"
Next in thread: Oreoluwa Babatunde: "Re: [PATCH v5 3/4] of: reserved_mem: Use the unflatten_devicetree APIs to scan reserved mem. nodes"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Thu, Mar 28, 2024 at 4:16 PM Oreoluwa Babatunde
<quic_obabatun@xxxxxxxxxxx> wrote:
>
> The unflatten_devicetree APIs have been setup and are available to be
> used by the time the fdt_init_reserved_mem() function is called.
> Since the unflatten_devicetree APIs are a more efficient way of scanning
> through the DT nodes, switch to using these APIs to facilitate the rest
> of the reserved memory processing.

Please use get_maintainers.pl. Specifically, you missed maintainers
for kernel/dma/.

>
> Signed-off-by: Oreoluwa Babatunde <quic_obabatun@xxxxxxxxxxx>
> ---
> drivers/of/of_reserved_mem.c | 77 +++++++++++++++++++++------------
> include/linux/of_reserved_mem.h | 2 +-
> kernel/dma/coherent.c | 8 ++--
> kernel/dma/contiguous.c | 8 ++--
> kernel/dma/swiotlb.c | 10 ++---
> 5 files changed, 64 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 0aba366eba59..68d1f4cca4bb 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -99,7 +99,7 @@ static void __init alloc_reserved_mem_array(void)
> /*
> * fdt_reserved_mem_save_node() - save fdt node for second pass initialization
> */
> -static void __init fdt_reserved_mem_save_node(unsigned long node, const char *uname,
> +static void __init fdt_reserved_mem_save_node(struct device_node *node, const char *uname,
> phys_addr_t base, phys_addr_t size)
> {
> struct reserved_mem *rmem = &reserved_mem[reserved_mem_count];
> @@ -109,7 +109,7 @@ static void __init fdt_reserved_mem_save_node(unsigned long node, const char *un
> return;
> }
>
> - rmem->fdt_node = node;
> + rmem->dev_node = node;
> rmem->name = uname;
> rmem->base = base;
> rmem->size = size;
> @@ -178,11 +178,11 @@ static int __init __reserved_mem_reserve_reg(unsigned long node,
> }
>
> /*
> - * __reserved_mem_check_root() - check if #size-cells, #address-cells provided
> + * __fdt_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)
> +static int __init __fdt_reserved_mem_check_root(unsigned long node)
> {
> const __be32 *prop;
>
> @@ -200,6 +200,29 @@ static int __init __reserved_mem_check_root(unsigned long node)
> return 0;
> }
>
> +/*
> + * __dt_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 __dt_reserved_mem_check_root(struct device_node *node)

The normal prefix is 'of', not 'dt'. Weird, right?, but it dates back
to OpenFirmware.

> +{
> + const __be32 *prop;
> +
> + prop = of_get_property(node, "#size-cells", NULL);

Throughout, use the appropriate typed function. Here for example,
of_property_read_u32().

> + if (!prop || be32_to_cpup(prop) != dt_root_size_cells)
> + return -EINVAL;
> +
> + prop = of_get_property(node, "#address-cells", NULL);
> + if (!prop || be32_to_cpup(prop) != dt_root_addr_cells)
> + return -EINVAL;
> +
> + prop = of_get_property(node, "ranges", NULL);

Or for presence, just of_property_present().

> + if (!prop)
> + return -EINVAL;
> + return 0;
> +}
> +
> /**
> * fdt_scan_reserved_mem_reg_nodes() - Store info for the "reg" defined
> * reserved memory regions.
> @@ -213,33 +236,38 @@ static int __init __reserved_mem_check_root(unsigned long node)
> static void __init fdt_scan_reserved_mem_reg_nodes(void)
> {
> int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
> - const void *fdt = initial_boot_params;
> + struct device_node *node, *child;
> phys_addr_t base, size;
> const __be32 *prop;
> - int node, child;
> int len;
>
> - node = fdt_path_offset(fdt, "/reserved-memory");
> - if (node < 0) {
> + node = of_find_node_by_path("/reserved-memory");
> + if (!node) {
> pr_info("Reserved memory: No reserved-memory node in the DT\n");
> return;
> }
>
> - if (__reserved_mem_check_root(node)) {
> + if (__dt_reserved_mem_check_root(node)) {
> pr_err("Reserved memory: unsupported node format, ignoring\n");
> return;
> }
>
> - fdt_for_each_subnode(child, fdt, node) {
> + for_each_child_of_node(node, child) {
> const char *uname;
> + struct reserved_mem *rmem;
>
> - prop = of_get_flat_dt_prop(child, "reg", &len);
> - if (!prop)
> + if (!of_device_is_available(child))
> continue;
> - if (!of_fdt_device_is_available(fdt, child))
> +
> + prop = of_get_property(child, "reg", &len);

We have specific unflattened functions for reading 'reg'. Note that
you should use the 'translated' ones even though we have a check to
disallow any real translation. That restriction should be fixed at
some point.

> + if (!prop) {
> + rmem = of_reserved_mem_lookup(child);
> + if (rmem)
> + rmem->dev_node = child;
> continue;
> + }
>
> - uname = fdt_get_name(fdt, child, NULL);
> + uname = of_node_full_name(child);
> if (len && len % t_len != 0) {
> pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n",
> uname);
> @@ -269,7 +297,7 @@ int __init fdt_scan_reserved_mem(void)
> if (node < 0)
> return -ENODEV;
>
> - if (__reserved_mem_check_root(node) != 0) {
> + if (__fdt_reserved_mem_check_root(node) != 0) {
> pr_err("Reserved memory: unsupported node format, ignoring\n");
> return -EINVAL;
> }
> @@ -447,7 +475,7 @@ static int __init __reserved_mem_alloc_size(unsigned long node, const char *unam
> uname, (unsigned long)(size / SZ_1M));
> return -ENOMEM;
> }
> - fdt_reserved_mem_save_node(node, uname, base, size);
> + fdt_reserved_mem_save_node(NULL, uname, base, size);
> return 0;
> }
>
> @@ -467,7 +495,7 @@ static int __init __reserved_mem_init_node(struct reserved_mem *rmem)
> reservedmem_of_init_fn initfn = i->data;
> const char *compat = i->compatible;
>
> - if (!of_flat_dt_is_compatible(rmem->fdt_node, compat))
> + if (!of_device_is_compatible(rmem->dev_node, compat))
> continue;
>
> ret = initfn(rmem);
> @@ -500,11 +528,6 @@ static int __init __rmem_cmp(const void *a, const void *b)
> if (ra->size > rb->size)
> return 1;
>
> - if (ra->fdt_node < rb->fdt_node)
> - return -1;
> - if (ra->fdt_node > rb->fdt_node)
> - return 1;
> -
> return 0;
> }
>
> @@ -551,16 +574,16 @@ void __init fdt_init_reserved_mem(void)
>
> for (i = 0; i < reserved_mem_count; i++) {
> struct reserved_mem *rmem = &reserved_mem[i];
> - unsigned long node = rmem->fdt_node;
> + struct device_node *node = rmem->dev_node;
> int len;
> const __be32 *prop;
> int err = 0;
> bool nomap;
>
> - nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
> - prop = of_get_flat_dt_prop(node, "phandle", &len);
> + nomap = of_get_property(node, "no-map", NULL) != NULL;
> + prop = of_get_property(node, "phandle", &len);

We store the phandle in struct device_node, so reading and storing it
here shouldn't be needed I think.

> if (!prop)
> - prop = of_get_flat_dt_prop(node, "linux,phandle", &len);
> + prop = of_get_property(node, "linux,phandle", &len);
> if (prop)
> rmem->phandle = of_read_number(prop, len/4);
>
> @@ -574,7 +597,7 @@ void __init fdt_init_reserved_mem(void)
> } else {
> phys_addr_t end = rmem->base + rmem->size - 1;
> bool reusable =
> - (of_get_flat_dt_prop(node, "reusable", NULL)) != NULL;
> + (of_get_property(node, "reusable", NULL)) != NULL;
>
> pr_info("%pa..%pa (%lu KiB) %s %s %s\n",
> &rmem->base, &end, (unsigned long)(rmem->size / SZ_1K),
> diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
> index 4de2a24cadc9..b6107a18d170 100644
> --- a/include/linux/of_reserved_mem.h
> +++ b/include/linux/of_reserved_mem.h
> @@ -10,7 +10,7 @@ struct reserved_mem_ops;
>
> struct reserved_mem {
> const char *name;
> - unsigned long fdt_node;
> + struct device_node *dev_node;
> unsigned long phandle;
> const struct reserved_mem_ops *ops;
> phys_addr_t base;
> diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c
> index ff5683a57f77..0db0aae83102 100644
> --- a/kernel/dma/coherent.c
> +++ b/kernel/dma/coherent.c
> @@ -362,20 +362,20 @@ static const struct reserved_mem_ops rmem_dma_ops = {
>
> static int __init rmem_dma_setup(struct reserved_mem *rmem)
> {
> - unsigned long node = rmem->fdt_node;
> + struct device_node *node = rmem->dev_node;
>
> - if (of_get_flat_dt_prop(node, "reusable", NULL))
> + if (of_get_property(node, "reusable", NULL))
> return -EINVAL;
>
> #ifdef CONFIG_ARM
> - if (!of_get_flat_dt_prop(node, "no-map", NULL)) {
> + if (!of_get_property(node, "no-map", NULL)) {

While you are here, convert this to IS_ENABLED():

if (IS_ENABLED(CONFIG_ARM) && !of_property_read_bool(node)) {
...
}

Next message: Yang Shi: "Re: [PATCH] mm/rmap: do not add fully unmapped large folio to deferred split list"
Previous message: Heiko Stuebner: "Re: [PATCH 13/18] i2c: rk3x: remove printout on handled timeouts"
Next in thread: Oreoluwa Babatunde: "Re: [PATCH v5 3/4] of: reserved_mem: Use the unflatten_devicetree APIs to scan reserved mem. nodes"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]