On Wed, Jun 16, 2021 at 1:36 PM Geert Uytterhoeven [off-list ref] wrote:

Hi Rob,

On Wed, Jun 16, 2021 at 7:14 PM Rob Herring [off-list ref] wrote:

quoted

On Wed, Jun 16, 2021 at 3:27 AM Geert Uytterhoeven
[off-list ref] wrote:

quoted

Commit b30be4dc733e5067 ("of: Add a common kexec FDT setup function")
introduced macros FDT_PROP_INITRD_* to refer to initrd properties, but
didn't use them everywhere.  Convert the remaining users from string
literals to macros.

I'm not really a fan of the defines, so if anything I'd get rid of

Oh, as you authored that patch, I thought you liked them ;-)
And I was thinking of moving them to a header file, so they can be
used by other .c files, too...

Upon closer inspection, I see you just copied them from arm64, which
was not that visible due to commit ac10be5cdbfa8521 ("arm64: Use
common of_kexec_alloc_and_setup_fdt()") being a separate commit...

That all was largely a 'this is what you need to implement' review.

quoted

them. But the bigger problem is what you brought to light with the
variable size. As I mentioned, we should refactor this and the fdt.c

The number of cells to use for the initrd properties doesn't seem to
be well-defined.
drivers/of/fdt.c derives it from the length of the property, which
more or less always works ("be strict when sending, be liberal when
receiving").  Some code hardcodes it to 1 or 2.

The not well defined ship has sailed, so we need to support either.

I suspect (didn't
check) there's also code out there that uses the root number of cells?

Given it's a single value, there's not really any need to do that.
Unlike some of the kexec properties which combine the start and length
for example.

quoted

code to have a common function to read the initrd start and end.

What with code that needs to set the start and end?

I'm not sure we can consolidate that as those are mostly in early arch
boot code.

It needs to use what the receiving end will expect...

That should be fine given fdt.c is flexible already.

Rob