diff mbox series

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

Message ID a4e07a0c1efea913ce5a61136162b5b720b96b48.1623835273.git.geert+renesas@glider.be
State New
Headers show
Series None | expand

Commit Message

Geert Uytterhoeven June 16, 2021, 9:27 a.m. UTC
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.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/of/kexec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Rob Herring June 16, 2021, 5:14 p.m. UTC | #1
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
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
code to have a common function to read the initrd start and end.

> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/of/kexec.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
> index f335d941a716e841..3fe585d5a82732e7 100644
> --- a/drivers/of/kexec.c
> +++ b/drivers/of/kexec.c
> @@ -318,13 +318,13 @@ void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
>                 goto out;
>
>         /* Did we boot using an initrd? */
> -       prop = fdt_getprop(fdt, chosen_node, "linux,initrd-start", NULL);
> +       prop = fdt_getprop(fdt, chosen_node, FDT_PROP_INITRD_START, NULL);
>         if (prop) {
>                 u64 tmp_start, tmp_end, tmp_size;
>
>                 tmp_start = fdt64_to_cpu(*((const fdt64_t *) prop));
>
> -               prop = fdt_getprop(fdt, chosen_node, "linux,initrd-end", NULL);
> +               prop = fdt_getprop(fdt, chosen_node, FDT_PROP_INITRD_END, NULL);
>                 if (!prop) {
>                         ret = -EINVAL;
>                         goto out;
> --
> 2.25.1
>
Geert Uytterhoeven June 16, 2021, 7:36 p.m. UTC | #2
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...

> 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.  I suspect (didn't
check) there's also code out there that uses the root number of cells?

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

What with code that needs to set the start and end?
It needs to use what the receiving end will expect...

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
diff mbox series

Patch

diff --git a/drivers/of/kexec.c b/drivers/of/kexec.c
index f335d941a716e841..3fe585d5a82732e7 100644
--- a/drivers/of/kexec.c
+++ b/drivers/of/kexec.c
@@ -318,13 +318,13 @@  void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
 		goto out;
 
 	/* Did we boot using an initrd? */
-	prop = fdt_getprop(fdt, chosen_node, "linux,initrd-start", NULL);
+	prop = fdt_getprop(fdt, chosen_node, FDT_PROP_INITRD_START, NULL);
 	if (prop) {
 		u64 tmp_start, tmp_end, tmp_size;
 
 		tmp_start = fdt64_to_cpu(*((const fdt64_t *) prop));
 
-		prop = fdt_getprop(fdt, chosen_node, "linux,initrd-end", NULL);
+		prop = fdt_getprop(fdt, chosen_node, FDT_PROP_INITRD_END, NULL);
 		if (!prop) {
 			ret = -EINVAL;
 			goto out;