Re: [PATCH v2] of: do not copy properties if they already exist in the destination

2024-04-16 Thread Sascha Hauer


On Mon, 15 Apr 2024 14:26:04 +0200, Jonas Richardsen wrote:
> Currently `of_copy_property` copies the given property even if a property
> with the same name already exists on the destination node.
> This leads to kernel warnings about duplicate properties:
> ```
> [0.014063] Duplicate name in chosen, renamed to "stdout-path#1"
> [0.014093] Duplicate name in chosen, renamed to "bootargs#1"
> [0.014119] Duplicate name in chosen, renamed to "phandle#1"
> [0.014197] Duplicate name in reserved-memory, renamed to 
> "#address-cells#1"
> [0.014226] Duplicate name in reserved-memory, renamed to "#size-cells#1"
> [0.014252] Duplicate name in reserved-memory, renamed to "ranges#1"
> [0.014278] Duplicate name in reserved-memory, renamed to "phandle#1"
> ```
> Therefore, the function was changed to return an error if the property
> already exists in the destination.
> The change does not cause any regressions, because the only usage of
> this function occurs within `arch/arm/boards/raspberry-pi/rpi-common.c`
> where the original behaviour of the function is obviously unintended.
> 
> [...]

Applied, thanks!

[1/1] of: do not copy properties if they already exist in the destination
  https://git.pengutronix.de/cgit/barebox/commit/?id=364a1831678d (link may 
not be stable)

Best regards,
-- 
Sascha Hauer 




[PATCH v2] of: do not copy properties if they already exist in the destination

2024-04-15 Thread Jonas Richardsen
Currently `of_copy_property` copies the given property even if a property 
with the same name already exists on the destination node. 
This leads to kernel warnings about duplicate properties:
```
[0.014063] Duplicate name in chosen, renamed to "stdout-path#1"
[0.014093] Duplicate name in chosen, renamed to "bootargs#1"
[0.014119] Duplicate name in chosen, renamed to "phandle#1"
[0.014197] Duplicate name in reserved-memory, renamed to "#address-cells#1"
[0.014226] Duplicate name in reserved-memory, renamed to "#size-cells#1"
[0.014252] Duplicate name in reserved-memory, renamed to "ranges#1"
[0.014278] Duplicate name in reserved-memory, renamed to "phandle#1"
```
Therefore, the function was changed to return an error if the property
already exists in the destination.
The change does not cause any regressions, because the only usage of
this function occurs within `arch/arm/boards/raspberry-pi/rpi-common.c`
where the original behaviour of the function is obviously unintended.

Signed-off-by: Jonas Richardsen 
---
In the process of creating this patch, we realized that the fixups 
for the Raspberry Pi are extensively copying properties from the video
core device tree that are not strictly necessary. We will do some
work on making the fixups more precise (i.e., only copy specific
properties, not entire nodes) soon.

 drivers/of/base.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index b22959dabe..3192a74ab9 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2346,6 +2346,9 @@ struct property *of_copy_property(const struct 
device_node *src,
if (!prop)
return NULL;
 
+   if (of_property_present(dst, propname))
+   return ERR_PTR(-EEXIST);
+
return of_new_property(dst, propname,
   of_property_get_value(prop), prop->length);
 }
-- 
2.42.0