Re: [PATCH 3/3] of: kexec: Always use FDT_PROP_INITRD_START and FDT_PROP_INITRD_END

From: Rob Herring <robh+dt@kernel.org>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Frank Rowand <frowand.list@gmail.com>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 3/3] of: kexec: Always use FDT_PROP_INITRD_START and FDT_PROP_INITRD_END
Date: Wed, 16 Jun 2021 13:49:01 -0600	[thread overview]
Message-ID: <CAL_JsqJLBU3FizWB+V_tcgYZLuA3B6LxKG2=znsJJY_X6yJ4DQ@mail.gmail.com> (raw)
In-Reply-To: <CAMuHMdUpuws0TRYVD9HPYO-K77VvhuqF-f1cQ5dH3p5zB_kS0A@mail.gmail.com>

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