Re: [PATCH] of: never fixup internal live tree

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hi Alexander,

On 03.08.23 12:15, Alexander Shiyan wrote:
> Now it works as expected.
> 
> Tested-by: Alexander Shiyan <eagle.alexander923@gmail.com>

I'd highly recommend that you do not use the barebox device tree for
booting Linux, so you have the flexibility in future to update one
or the other component.

Cheers,
Ahmad

> 
> чт, 3 авг. 2023 г. в 11:51, Sascha Hauer <s.hauer@pengutronix.de>:
>>
>> Calling of_fix_tree() on the internal live tree has undesired side
>> effects, so refrain from doing so.
>>
>> One of the side effects is that some parts of barebox store pointers
>> to properties in the live tree which might become invalid when during
>> of_fixup these properties are deleted or updated.
>>
>> The other is that it's unexpected that the live tree is modified
>> after a dry run boot with the live tree.
>>
>> This changes of_get_fixed_tree() to always call of_fix_tree() on
>> a copy of the device tree and not on the device tree itself. To
>> emphasize this make the device tree argument to of_get_fixed_tree()
>> const.
>>
>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>> ---
>>  common/oftree.c | 19 ++++++++++---------
>>  include/of.h    |  2 +-
>>  2 files changed, 11 insertions(+), 10 deletions(-)
>>
>> diff --git a/common/oftree.c b/common/oftree.c
>> index 3e85070d11..51eebd36bd 100644
>> --- a/common/oftree.c
>> +++ b/common/oftree.c
>> @@ -416,26 +416,27 @@ void of_fix_tree(struct device_node *node)
>>   * It increases the size of the tree and applies the registered
>>   * fixups.
>>   */
>> -struct fdt_header *of_get_fixed_tree(struct device_node *node)
>> +struct fdt_header *of_get_fixed_tree(const struct device_node *node)
>>  {
>>         struct fdt_header *fdt = NULL;
>> -       struct device_node *freenp = NULL;
>> +       struct device_node *np;
>>
>>         if (!node) {
>>                 node = of_get_root_node();
>>                 if (!node)
>>                         return NULL;
>> -
>> -               freenp = node = of_dup(node);
>> -               if (!node)
>> -                       return NULL;
>>         }
>>
>> -       of_fix_tree(node);
>> +       np = of_dup(node);
>> +
>> +       if (!np)
>> +               return NULL;
>> +
>> +       of_fix_tree(np);
>>
>> -       fdt = of_flatten_dtb(node);
>> +       fdt = of_flatten_dtb(np);
>>
>> -       of_delete_node(freenp);
>> +       of_delete_node(np);
>>
>>         return fdt;
>>  }
>> diff --git a/include/of.h b/include/of.h
>> index 92a15f5c4a..0deb327971 100644
>> --- a/include/of.h
>> +++ b/include/of.h
>> @@ -75,7 +75,7 @@ int of_add_initrd(struct device_node *root, resource_size_t start,
>>
>>  struct fdt_header *fdt_get_tree(void);
>>
>> -struct fdt_header *of_get_fixed_tree(struct device_node *node);
>> +struct fdt_header *of_get_fixed_tree(const struct device_node *node);
>>
>>  /* Helper to read a big number; size is in cells (not bytes) */
>>  static inline u64 of_read_number(const __be32 *cell, int size)
>> --
>> 2.39.2
>>
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |