On Wed, Jun 16, 2021 at 1:36 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
>
> Hi Rob,
>
> On Wed, Jun 16, 2021 at 7:14 PM Rob Herring <robh+dt@xxxxxxxxxx> wrote:
> > On Wed, Jun 16, 2021 at 3:27 AM Geert Uytterhoeven
> > <geert+renesas@xxxxxxxxx> wrote:
> > > 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.
> > 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.
> > 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
