2024-04-23 00:02:22

by Oreoluwa Babatunde

[permalink] [raw]
Subject: [PATCH] of: reserved_mem: Remove the use of phandle from the reserved_mem APIs

The __find_rmem() function is the only place that references the phandle
field of the reserved_mem struct. __find_rmem() is used to match a
device_node object to its corresponding entry in the reserved_mem array
using its phandle value. But, there is already a function called
of_reserved_mem_lookup() which carries out the same action using the
name of the node.

Using the of_reserved_mem_lookup() function is more reliable because
every node is gauranteed to have a name, but not all nodes will have a
phandle.

Nodes are only assigned a phandle if they are explicitly defined in the
DT using "phandle = <phandle_number>", or if they are referenced by
another node in the DT. Hence, If the phandle field is empty, then
__find_rmem() will return a false negative.

Hence, delete the __find_rmem() function and switch to using the
of_reserved_mem_lookup() function to find the corresponding entry of a
device_node in the reserved_mem array. Since the phandle field of the
reserved_mem struct is now unused, delete that as well.

Signed-off-by: Oreoluwa Babatunde <[email protected]>
---
drivers/of/of_reserved_mem.c | 26 +++-----------------------
include/linux/of_reserved_mem.h | 1 -
2 files changed, 3 insertions(+), 24 deletions(-)

diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 8236ecae2953..bdf41b5b1ae5 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -437,18 +437,9 @@ 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;
- 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);
- if (!prop)
- prop = of_get_flat_dt_prop(node, "linux,phandle", &len);
- if (prop)
- rmem->phandle = of_read_number(prop, len/4);
-
if (rmem->size == 0)
err = __reserved_mem_alloc_size(node, rmem->name,
&rmem->base, &rmem->size);
@@ -457,6 +448,8 @@ void __init fdt_init_reserved_mem(void)
if (err != 0 && err != -ENOENT) {
pr_info("node %s compatible matching fail\n",
rmem->name);
+
+ nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
if (nomap)
memblock_clear_nomap(rmem->base, rmem->size);
else
@@ -477,19 +470,6 @@ void __init fdt_init_reserved_mem(void)
}
}

-static inline struct reserved_mem *__find_rmem(struct device_node *node)
-{
- unsigned int i;
-
- if (!node->phandle)
- return NULL;
-
- for (i = 0; i < reserved_mem_count; i++)
- if (reserved_mem[i].phandle == node->phandle)
- return &reserved_mem[i];
- return NULL;
-}
-
struct rmem_assigned_device {
struct device *dev;
struct reserved_mem *rmem;
@@ -534,7 +514,7 @@ int of_reserved_mem_device_init_by_idx(struct device *dev,
return 0;
}

- rmem = __find_rmem(target);
+ rmem = of_reserved_mem_lookup(target);
of_node_put(target);

if (!rmem || !rmem->ops || !rmem->ops->device_init)
diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
index 4de2a24cadc9..e338282da652 100644
--- a/include/linux/of_reserved_mem.h
+++ b/include/linux/of_reserved_mem.h
@@ -11,7 +11,6 @@ struct reserved_mem_ops;
struct reserved_mem {
const char *name;
unsigned long fdt_node;
- unsigned long phandle;
const struct reserved_mem_ops *ops;
phys_addr_t base;
phys_addr_t size;
--
2.34.1



2024-04-23 14:35:44

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH] of: reserved_mem: Remove the use of phandle from the reserved_mem APIs

On 4/22/2024 4:52 PM, Oreoluwa Babatunde wrote:
> The __find_rmem() function is the only place that references the phandle
> field of the reserved_mem struct. __find_rmem() is used to match a
> device_node object to its corresponding entry in the reserved_mem array
> using its phandle value. But, there is already a function called
> of_reserved_mem_lookup() which carries out the same action using the
> name of the node.
>
> Using the of_reserved_mem_lookup() function is more reliable because
> every node is gauranteed to have a name, but not all nodes will have a

s/gauranteed /guaranteed /

(jumped out at me while scanning the kernel patches)

2024-04-23 23:33:56

by Oreoluwa Babatunde

[permalink] [raw]
Subject: Re: [PATCH] of: reserved_mem: Remove the use of phandle from the reserved_mem APIs


On 4/23/2024 7:32 AM, Jeff Johnson wrote:
> On 4/22/2024 4:52 PM, Oreoluwa Babatunde wrote:
>> The __find_rmem() function is the only place that references the phandle
>> field of the reserved_mem struct. __find_rmem() is used to match a
>> device_node object to its corresponding entry in the reserved_mem array
>> using its phandle value. But, there is already a function called
>> of_reserved_mem_lookup() which carries out the same action using the
>> name of the node.
>>
>> Using the of_reserved_mem_lookup() function is more reliable because
>> every node is gauranteed to have a name, but not all nodes will have a
> s/gauranteed /guaranteed /
>
> (jumped out at me while scanning the kernel patches)

ack.

Thank you Jeff!


2024-04-29 16:02:18

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH] of: reserved_mem: Remove the use of phandle from the reserved_mem APIs


On Mon, 22 Apr 2024 16:52:43 -0700, Oreoluwa Babatunde wrote:
> The __find_rmem() function is the only place that references the phandle
> field of the reserved_mem struct. __find_rmem() is used to match a
> device_node object to its corresponding entry in the reserved_mem array
> using its phandle value. But, there is already a function called
> of_reserved_mem_lookup() which carries out the same action using the
> name of the node.
>
> Using the of_reserved_mem_lookup() function is more reliable because
> every node is gauranteed to have a name, but not all nodes will have a
> phandle.
>
> Nodes are only assigned a phandle if they are explicitly defined in the
> DT using "phandle = <phandle_number>", or if they are referenced by
> another node in the DT. Hence, If the phandle field is empty, then
> __find_rmem() will return a false negative.
>
> Hence, delete the __find_rmem() function and switch to using the
> of_reserved_mem_lookup() function to find the corresponding entry of a
> device_node in the reserved_mem array. Since the phandle field of the
> reserved_mem struct is now unused, delete that as well.
>
> Signed-off-by: Oreoluwa Babatunde <[email protected]>
> ---
> drivers/of/of_reserved_mem.c | 26 +++-----------------------
> include/linux/of_reserved_mem.h | 1 -
> 2 files changed, 3 insertions(+), 24 deletions(-)
>

Applied, thanks!


2024-04-29 17:57:07

by Oreoluwa Babatunde

[permalink] [raw]
Subject: Re: [PATCH] of: reserved_mem: Remove the use of phandle from the reserved_mem APIs


On 4/29/2024 8:59 AM, Rob Herring wrote:
> On Mon, 22 Apr 2024 16:52:43 -0700, Oreoluwa Babatunde wrote:
>> The __find_rmem() function is the only place that references the phandle
>> field of the reserved_mem struct. __find_rmem() is used to match a
>> device_node object to its corresponding entry in the reserved_mem array
>> using its phandle value. But, there is already a function called
>> of_reserved_mem_lookup() which carries out the same action using the
>> name of the node.
>>
>> Using the of_reserved_mem_lookup() function is more reliable because
>> every node is gauranteed to have a name, but not all nodes will have a
>> phandle.
>>
>> Nodes are only assigned a phandle if they are explicitly defined in the
>> DT using "phandle = <phandle_number>", or if they are referenced by
>> another node in the DT. Hence, If the phandle field is empty, then
>> __find_rmem() will return a false negative.
>>
>> Hence, delete the __find_rmem() function and switch to using the
>> of_reserved_mem_lookup() function to find the corresponding entry of a
>> device_node in the reserved_mem array. Since the phandle field of the
>> reserved_mem struct is now unused, delete that as well.
>>
>> Signed-off-by: Oreoluwa Babatunde <[email protected]>
>> ---
>> drivers/of/of_reserved_mem.c | 26 +++-----------------------
>> include/linux/of_reserved_mem.h | 1 -
>> 2 files changed, 3 insertions(+), 24 deletions(-)
>>
> Applied, thanks!
Thank you!
>