[PATCH v2] OF: Handle CMDLINE when /chosen node is not present

[PATCH v2] OF: Handle CMDLINE when /chosen node is not present

[Nick Kossifidis]

The /chosen node is optional so we should handle CMDLINE regardless
the presence of /chosen/bootargs. Move handling of CMDLINE in
early_init_dt_scan() instead.

v2: Fix typo on comments section

Signed-off-by: Nick Kossifidis <mick@ics.forth.gr>
---
 drivers/of/fdt.c | 69 +++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 48 insertions(+), 21 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 800ad252c..868464b0b 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -31,6 +31,14 @@
 
 #include "of_private.h"
 
+#ifdef CONFIG_CMDLINE
+#ifdef CONFIG_CMDLINE_FORCE
+static const char fixed_cmdline[] __initconst = CONFIG_CMDLINE;
+#else
+static char builtin_cmdline[] __initdata = CONFIG_CMDLINE;
+#endif
+#endif
+
 /*
  * of_fdt_limit_memory - limit the number of regions in the /memory node
  * @limit: maximum entries
@@ -1088,28 +1096,10 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
 
 	/* Retrieve command line */
 	p = of_get_flat_dt_prop(node, "bootargs", &l);
-	if (p != NULL && l > 0)
+	if (p != NULL && l > 0) {
 		strlcpy(data, p, min((int)l, COMMAND_LINE_SIZE));
-
-	/*
-	 * CONFIG_CMDLINE is meant to be a default in case nothing else
-	 * managed to set the command line, unless CONFIG_CMDLINE_FORCE
-	 * is set in which case we override whatever was found earlier.
-	 */
-#ifdef CONFIG_CMDLINE
-#if defined(CONFIG_CMDLINE_EXTEND)
-	strlcat(data, " ", COMMAND_LINE_SIZE);
-	strlcat(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
-#elif defined(CONFIG_CMDLINE_FORCE)
-	strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
-#else
-	/* No arguments from boot loader, use kernel's  cmdl*/
-	if (!((char *)data)[0])
-		strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
-#endif
-#endif /* CONFIG_CMDLINE */
-
-	pr_debug("Command line is: %s\n", (char*)data);
+		pr_debug("Got bootargs: %s\n", (char *) data);
+	}
 
 	/* break now */
 	return 1;
@@ -1240,6 +1230,43 @@ bool __init early_init_dt_scan(void *params)
 		return false;
 
 	early_init_dt_scan_nodes();
+
+	/*
+	 * The /chosen node normally contains the bootargs element
+	 * that includes the kernel's command line parameters.
+	 * However the presence of /chosen is not mandatory so
+	 * in case we didn't get a command line when scanning
+	 * nodes above, we need to provide one here before we
+	 * return if possible.
+	 *
+	 * The built-in command line can be used as a default
+	 * command line in case we received nothing from the
+	 * device tree/bootloader. It can also be used for
+	 * extending or replacing the received command line.
+	 */
+#ifdef CONFIG_CMDLINE
+#if defined(CONFIG_CMDLINE_EXTEND)
+	/*
+	 * Order of parameters shouldn't matter for most cases,
+	 * so prepending or appending the built-in command line
+	 * shouldn't make a difference. In cases where it does
+	 * it's up to the user to configure the kernel and/or
+	 * the bootloader properly.
+	 */
+	if (builtin_cmdline[0]) {
+		strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
+		strlcat(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
+	}
+#elif defined(CONFIG_CMDLINE_FORCE)
+	if (fixed_cmdline[0])
+		strlcpy(boot_command_line, fixed_cmdline, COMMAND_LINE_SIZE);
+#else
+	/* No arguments from boot loader, use kernel's cmdline */
+	if (!boot_command_line[0] && builtin_cmdline[0])
+		strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
+#endif
+#endif /* CONFIG_CMDLINE */
+
 	return true;
 }
 
-- 
2.16.4

[PATCH v2] OF: Handle CMDLINE when /chosen node is not present

[Nick Kossifidis]

The /chosen node is optional so we should handle CMDLINE regardless
the presence of /chosen/bootargs. Move handling of CMDLINE in
early_init_dt_scan() instead.

v2: Fix typo on comments section

Signed-off-by: Nick Kossifidis <mick@ics.forth.gr>
---
 drivers/of/fdt.c | 69 +++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 48 insertions(+), 21 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 800ad252c..868464b0b 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -31,6 +31,14 @@
 
 #include "of_private.h"
 
+#ifdef CONFIG_CMDLINE
+#ifdef CONFIG_CMDLINE_FORCE
+static const char fixed_cmdline[] __initconst = CONFIG_CMDLINE;
+#else
+static char builtin_cmdline[] __initdata = CONFIG_CMDLINE;
+#endif
+#endif
+
 /*
  * of_fdt_limit_memory - limit the number of regions in the /memory node
  * @limit: maximum entries
@@ -1088,28 +1096,10 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
 
 	/* Retrieve command line */
 	p = of_get_flat_dt_prop(node, "bootargs", &l);
-	if (p != NULL && l > 0)
+	if (p != NULL && l > 0) {
 		strlcpy(data, p, min((int)l, COMMAND_LINE_SIZE));
-
-	/*
-	 * CONFIG_CMDLINE is meant to be a default in case nothing else
-	 * managed to set the command line, unless CONFIG_CMDLINE_FORCE
-	 * is set in which case we override whatever was found earlier.
-	 */
-#ifdef CONFIG_CMDLINE
-#if defined(CONFIG_CMDLINE_EXTEND)
-	strlcat(data, " ", COMMAND_LINE_SIZE);
-	strlcat(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
-#elif defined(CONFIG_CMDLINE_FORCE)
-	strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
-#else
-	/* No arguments from boot loader, use kernel's  cmdl*/
-	if (!((char *)data)[0])
-		strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
-#endif
-#endif /* CONFIG_CMDLINE */
-
-	pr_debug("Command line is: %s\n", (char*)data);
+		pr_debug("Got bootargs: %s\n", (char *) data);
+	}
 
 	/* break now */
 	return 1;
@@ -1240,6 +1230,43 @@ bool __init early_init_dt_scan(void *params)
 		return false;
 
 	early_init_dt_scan_nodes();
+
+	/*
+	 * The /chosen node normally contains the bootargs element
+	 * that includes the kernel's command line parameters.
+	 * However the presence of /chosen is not mandatory so
+	 * in case we didn't get a command line when scanning
+	 * nodes above, we need to provide one here before we
+	 * return if possible.
+	 *
+	 * The built-in command line can be used as a default
+	 * command line in case we received nothing from the
+	 * device tree/bootloader. It can also be used for
+	 * extending or replacing the received command line.
+	 */
+#ifdef CONFIG_CMDLINE
+#if defined(CONFIG_CMDLINE_EXTEND)
+	/*
+	 * Order of parameters shouldn't matter for most cases,
+	 * so prepending or appending the built-in command line
+	 * shouldn't make a difference. In cases where it does
+	 * it's up to the user to configure the kernel and/or
+	 * the bootloader properly.
+	 */
+	if (builtin_cmdline[0]) {
+		strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
+		strlcat(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
+	}
+#elif defined(CONFIG_CMDLINE_FORCE)
+	if (fixed_cmdline[0])
+		strlcpy(boot_command_line, fixed_cmdline, COMMAND_LINE_SIZE);
+#else
+	/* No arguments from boot loader, use kernel's cmdline */
+	if (!boot_command_line[0] && builtin_cmdline[0])
+		strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
+#endif
+#endif /* CONFIG_CMDLINE */
+
 	return true;
 }
 
-- 
2.16.4

[PATCH v2] OF: Handle CMDLINE when /chosen node is not present

[Nick Kossifidis]

The /chosen node is optional so we should handle CMDLINE regardless
the presence of /chosen/bootargs. Move handling of CMDLINE in
early_init_dt_scan() instead.

v2: Fix typo on comments section

Signed-off-by: Nick Kossifidis <mick@ics.forth.gr>
---
 drivers/of/fdt.c | 69 +++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 48 insertions(+), 21 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 800ad252c..868464b0b 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -31,6 +31,14 @@
 
 #include "of_private.h"
 
+#ifdef CONFIG_CMDLINE
+#ifdef CONFIG_CMDLINE_FORCE
+static const char fixed_cmdline[] __initconst = CONFIG_CMDLINE;
+#else
+static char builtin_cmdline[] __initdata = CONFIG_CMDLINE;
+#endif
+#endif
+
 /*
  * of_fdt_limit_memory - limit the number of regions in the /memory node
  * @limit: maximum entries
@@ -1088,28 +1096,10 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
 
 	/* Retrieve command line */
 	p = of_get_flat_dt_prop(node, "bootargs", &l);
-	if (p != NULL && l > 0)
+	if (p != NULL && l > 0) {
 		strlcpy(data, p, min((int)l, COMMAND_LINE_SIZE));
-
-	/*
-	 * CONFIG_CMDLINE is meant to be a default in case nothing else
-	 * managed to set the command line, unless CONFIG_CMDLINE_FORCE
-	 * is set in which case we override whatever was found earlier.
-	 */
-#ifdef CONFIG_CMDLINE
-#if defined(CONFIG_CMDLINE_EXTEND)
-	strlcat(data, " ", COMMAND_LINE_SIZE);
-	strlcat(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
-#elif defined(CONFIG_CMDLINE_FORCE)
-	strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
-#else
-	/* No arguments from boot loader, use kernel's  cmdl*/
-	if (!((char *)data)[0])
-		strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
-#endif
-#endif /* CONFIG_CMDLINE */
-
-	pr_debug("Command line is: %s\n", (char*)data);
+		pr_debug("Got bootargs: %s\n", (char *) data);
+	}
 
 	/* break now */
 	return 1;
@@ -1240,6 +1230,43 @@ bool __init early_init_dt_scan(void *params)
 		return false;
 
 	early_init_dt_scan_nodes();
+
+	/*
+	 * The /chosen node normally contains the bootargs element
+	 * that includes the kernel's command line parameters.
+	 * However the presence of /chosen is not mandatory so
+	 * in case we didn't get a command line when scanning
+	 * nodes above, we need to provide one here before we
+	 * return if possible.
+	 *
+	 * The built-in command line can be used as a default
+	 * command line in case we received nothing from the
+	 * device tree/bootloader. It can also be used for
+	 * extending or replacing the received command line.
+	 */
+#ifdef CONFIG_CMDLINE
+#if defined(CONFIG_CMDLINE_EXTEND)
+	/*
+	 * Order of parameters shouldn't matter for most cases,
+	 * so prepending or appending the built-in command line
+	 * shouldn't make a difference. In cases where it does
+	 * it's up to the user to configure the kernel and/or
+	 * the bootloader properly.
+	 */
+	if (builtin_cmdline[0]) {
+		strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
+		strlcat(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
+	}
+#elif defined(CONFIG_CMDLINE_FORCE)
+	if (fixed_cmdline[0])
+		strlcpy(boot_command_line, fixed_cmdline, COMMAND_LINE_SIZE);
+#else
+	/* No arguments from boot loader, use kernel's cmdline */
+	if (!boot_command_line[0] && builtin_cmdline[0])
+		strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
+#endif
+#endif /* CONFIG_CMDLINE */
+
 	return true;
 }
 
-- 
2.16.4


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2] OF: Handle CMDLINE when /chosen node is not present

[Rob Herring]

On Mon, Oct 15, 2018 at 05:20:10PM +0300, Nick Kossifidis wrote:
> The /chosen node is optional so we should handle CMDLINE regardless
> the presence of /chosen/bootargs. Move handling of CMDLINE in
> early_init_dt_scan() instead.

I looked at this a while back. I'm not sure this behavior can be changed 
without breaking some MIPS platforms that could be relying on the 
current behavior. But trying to make sense of the MIPS code is a 
challenge and they have some other issues in this area.

Can't this be fixed by making /chosen manditory? I'd expect ultimately 
you are always going to need it.

I'd rather not resort to making this per arch. There's also some effort 
to consolidate cmd line handling[1].

Rob

[1] https://lkml.org/lkml/2018/9/27/1099

[PATCH v2] OF: Handle CMDLINE when /chosen node is not present

[Rob Herring]

On Mon, Oct 15, 2018 at 05:20:10PM +0300, Nick Kossifidis wrote:
> The /chosen node is optional so we should handle CMDLINE regardless
> the presence of /chosen/bootargs. Move handling of CMDLINE in
> early_init_dt_scan() instead.

I looked at this a while back. I'm not sure this behavior can be changed 
without breaking some MIPS platforms that could be relying on the 
current behavior. But trying to make sense of the MIPS code is a 
challenge and they have some other issues in this area.

Can't this be fixed by making /chosen manditory? I'd expect ultimately 
you are always going to need it.

I'd rather not resort to making this per arch. There's also some effort 
to consolidate cmd line handling[1].

Rob

[1] https://lkml.org/lkml/2018/9/27/1099

Re: [PATCH v2] OF: Handle CMDLINE when /chosen node is not present

[Rob Herring]

On Mon, Oct 15, 2018 at 05:20:10PM +0300, Nick Kossifidis wrote:
> The /chosen node is optional so we should handle CMDLINE regardless
> the presence of /chosen/bootargs. Move handling of CMDLINE in
> early_init_dt_scan() instead.

I looked at this a while back. I'm not sure this behavior can be changed 
without breaking some MIPS platforms that could be relying on the 
current behavior. But trying to make sense of the MIPS code is a 
challenge and they have some other issues in this area.

Can't this be fixed by making /chosen manditory? I'd expect ultimately 
you are always going to need it.

I'd rather not resort to making this per arch. There's also some effort 
to consolidate cmd line handling[1].

Rob

[1] https://lkml.org/lkml/2018/9/27/1099


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2] OF: Handle CMDLINE when /chosen node is not present

[Palmer Dabbelt]

On Mon, 22 Oct 2018 15:42:13 PDT (-0700), robh@kernel.org wrote:
> On Mon, Oct 15, 2018 at 05:20:10PM +0300, Nick Kossifidis wrote:
>> The /chosen node is optional so we should handle CMDLINE regardless
>> the presence of /chosen/bootargs. Move handling of CMDLINE in
>> early_init_dt_scan() instead.
>
> I looked at this a while back. I'm not sure this behavior can be changed
> without breaking some MIPS platforms that could be relying on the
> current behavior. But trying to make sense of the MIPS code is a
> challenge and they have some other issues in this area.
>
> Can't this be fixed by making /chosen manditory? I'd expect ultimately
> you are always going to need it.
>
> I'd rather not resort to making this per arch. There's also some effort
> to consolidate cmd line handling[1].

I'd rather make /chosen mandatory on RISC-V than to have per-arch handling, as 
like you've said there's already too much duplication.  That said, it does seem 
like a bug to me because the behavior seems somewhat arbitrary -- an empty 
/chosen node causing the built-in command-line argument handling to go off the 
rails just smells so buggy.

If that's the case, could we at least have something like 
"CONFIG_OF_CHOSEN_IS_MANDATORY" that provides a warning when there is no 
/chosen node and is set on architecture where the spec mandates /chosen?

I've added linux-mips to see if anyone has any ideas.

Re: [PATCH v2] OF: Handle CMDLINE when /chosen node is not present

[Palmer Dabbelt]

On Mon, 22 Oct 2018 15:42:13 PDT (-0700), robh@kernel.org wrote:
> On Mon, Oct 15, 2018 at 05:20:10PM +0300, Nick Kossifidis wrote:
>> The /chosen node is optional so we should handle CMDLINE regardless
>> the presence of /chosen/bootargs. Move handling of CMDLINE in
>> early_init_dt_scan() instead.
>
> I looked at this a while back. I'm not sure this behavior can be changed
> without breaking some MIPS platforms that could be relying on the
> current behavior. But trying to make sense of the MIPS code is a
> challenge and they have some other issues in this area.
>
> Can't this be fixed by making /chosen manditory? I'd expect ultimately
> you are always going to need it.
>
> I'd rather not resort to making this per arch. There's also some effort
> to consolidate cmd line handling[1].

I'd rather make /chosen mandatory on RISC-V than to have per-arch handling, as 
like you've said there's already too much duplication.  That said, it does seem 
like a bug to me because the behavior seems somewhat arbitrary -- an empty 
/chosen node causing the built-in command-line argument handling to go off the 
rails just smells so buggy.

If that's the case, could we at least have something like 
"CONFIG_OF_CHOSEN_IS_MANDATORY" that provides a warning when there is no 
/chosen node and is set on architecture where the spec mandates /chosen?

I've added linux-mips to see if anyone has any ideas.

[PATCH v2] OF: Handle CMDLINE when /chosen node is not present

[Palmer Dabbelt]

On Mon, 22 Oct 2018 15:42:13 PDT (-0700), robh at kernel.org wrote:
> On Mon, Oct 15, 2018 at 05:20:10PM +0300, Nick Kossifidis wrote:
>> The /chosen node is optional so we should handle CMDLINE regardless
>> the presence of /chosen/bootargs. Move handling of CMDLINE in
>> early_init_dt_scan() instead.
>
> I looked at this a while back. I'm not sure this behavior can be changed
> without breaking some MIPS platforms that could be relying on the
> current behavior. But trying to make sense of the MIPS code is a
> challenge and they have some other issues in this area.
>
> Can't this be fixed by making /chosen manditory? I'd expect ultimately
> you are always going to need it.
>
> I'd rather not resort to making this per arch. There's also some effort
> to consolidate cmd line handling[1].

I'd rather make /chosen mandatory on RISC-V than to have per-arch handling, as 
like you've said there's already too much duplication.  That said, it does seem 
like a bug to me because the behavior seems somewhat arbitrary -- an empty 
/chosen node causing the built-in command-line argument handling to go off the 
rails just smells so buggy.

If that's the case, could we at least have something like 
"CONFIG_OF_CHOSEN_IS_MANDATORY" that provides a warning when there is no 
/chosen node and is set on architecture where the spec mandates /chosen?

I've added linux-mips to see if anyone has any ideas.

Re: [PATCH v2] OF: Handle CMDLINE when /chosen node is not present

[Palmer Dabbelt]

On Mon, 22 Oct 2018 15:42:13 PDT (-0700), robh@kernel.org wrote:
> On Mon, Oct 15, 2018 at 05:20:10PM +0300, Nick Kossifidis wrote:
>> The /chosen node is optional so we should handle CMDLINE regardless
>> the presence of /chosen/bootargs. Move handling of CMDLINE in
>> early_init_dt_scan() instead.
>
> I looked at this a while back. I'm not sure this behavior can be changed
> without breaking some MIPS platforms that could be relying on the
> current behavior. But trying to make sense of the MIPS code is a
> challenge and they have some other issues in this area.
>
> Can't this be fixed by making /chosen manditory? I'd expect ultimately
> you are always going to need it.
>
> I'd rather not resort to making this per arch. There's also some effort
> to consolidate cmd line handling[1].

I'd rather make /chosen mandatory on RISC-V than to have per-arch handling, as 
like you've said there's already too much duplication.  That said, it does seem 
like a bug to me because the behavior seems somewhat arbitrary -- an empty 
/chosen node causing the built-in command-line argument handling to go off the 
rails just smells so buggy.

If that's the case, could we at least have something like 
"CONFIG_OF_CHOSEN_IS_MANDATORY" that provides a warning when there is no 
/chosen node and is set on architecture where the spec mandates /chosen?

I've added linux-mips to see if anyone has any ideas.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2] OF: Handle CMDLINE when /chosen node is not present

[Rob Herring]

On Mon, Oct 22, 2018 at 5:55 PM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Mon, 22 Oct 2018 15:42:13 PDT (-0700), robh@kernel.org wrote:
> > On Mon, Oct 15, 2018 at 05:20:10PM +0300, Nick Kossifidis wrote:
> >> The /chosen node is optional so we should handle CMDLINE regardless
> >> the presence of /chosen/bootargs. Move handling of CMDLINE in
> >> early_init_dt_scan() instead.
> >
> > I looked at this a while back. I'm not sure this behavior can be changed
> > without breaking some MIPS platforms that could be relying on the
> > current behavior. But trying to make sense of the MIPS code is a
> > challenge and they have some other issues in this area.
> >
> > Can't this be fixed by making /chosen manditory? I'd expect ultimately
> > you are always going to need it.
> >
> > I'd rather not resort to making this per arch. There's also some effort
> > to consolidate cmd line handling[1].
>
> I'd rather make /chosen mandatory on RISC-V than to have per-arch handling, as
> like you've said there's already too much duplication.  That said, it does seem
> like a bug to me because the behavior seems somewhat arbitrary -- an empty
> /chosen node causing the built-in command-line argument handling to go off the
> rails just smells so buggy.

Yes. Probably need to do some archaeology on this code to figure out
some of the expectations.

> If that's the case, could we at least have something like
> "CONFIG_OF_CHOSEN_IS_MANDATORY" that provides a warning when there is no
> /chosen node and is set on architecture where the spec mandates /chosen?

I'd be okay to make it a warning unconditionally. At least then we can
find the cases that deviate and either fix them or understand their
expectations.

Rob

Re: [PATCH v2] OF: Handle CMDLINE when /chosen node is not present

[Rob Herring]

On Mon, Oct 22, 2018 at 5:55 PM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Mon, 22 Oct 2018 15:42:13 PDT (-0700), robh@kernel.org wrote:
> > On Mon, Oct 15, 2018 at 05:20:10PM +0300, Nick Kossifidis wrote:
> >> The /chosen node is optional so we should handle CMDLINE regardless
> >> the presence of /chosen/bootargs. Move handling of CMDLINE in
> >> early_init_dt_scan() instead.
> >
> > I looked at this a while back. I'm not sure this behavior can be changed
> > without breaking some MIPS platforms that could be relying on the
> > current behavior. But trying to make sense of the MIPS code is a
> > challenge and they have some other issues in this area.
> >
> > Can't this be fixed by making /chosen manditory? I'd expect ultimately
> > you are always going to need it.
> >
> > I'd rather not resort to making this per arch. There's also some effort
> > to consolidate cmd line handling[1].
>
> I'd rather make /chosen mandatory on RISC-V than to have per-arch handling, as
> like you've said there's already too much duplication.  That said, it does seem
> like a bug to me because the behavior seems somewhat arbitrary -- an empty
> /chosen node causing the built-in command-line argument handling to go off the
> rails just smells so buggy.

Yes. Probably need to do some archaeology on this code to figure out
some of the expectations.

> If that's the case, could we at least have something like
> "CONFIG_OF_CHOSEN_IS_MANDATORY" that provides a warning when there is no
> /chosen node and is set on architecture where the spec mandates /chosen?

I'd be okay to make it a warning unconditionally. At least then we can
find the cases that deviate and either fix them or understand their
expectations.

Rob

[PATCH v2] OF: Handle CMDLINE when /chosen node is not present

[Rob Herring]

On Mon, Oct 22, 2018 at 5:55 PM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Mon, 22 Oct 2018 15:42:13 PDT (-0700), robh at kernel.org wrote:
> > On Mon, Oct 15, 2018 at 05:20:10PM +0300, Nick Kossifidis wrote:
> >> The /chosen node is optional so we should handle CMDLINE regardless
> >> the presence of /chosen/bootargs. Move handling of CMDLINE in
> >> early_init_dt_scan() instead.
> >
> > I looked at this a while back. I'm not sure this behavior can be changed
> > without breaking some MIPS platforms that could be relying on the
> > current behavior. But trying to make sense of the MIPS code is a
> > challenge and they have some other issues in this area.
> >
> > Can't this be fixed by making /chosen manditory? I'd expect ultimately
> > you are always going to need it.
> >
> > I'd rather not resort to making this per arch. There's also some effort
> > to consolidate cmd line handling[1].
>
> I'd rather make /chosen mandatory on RISC-V than to have per-arch handling, as
> like you've said there's already too much duplication.  That said, it does seem
> like a bug to me because the behavior seems somewhat arbitrary -- an empty
> /chosen node causing the built-in command-line argument handling to go off the
> rails just smells so buggy.

Yes. Probably need to do some archaeology on this code to figure out
some of the expectations.

> If that's the case, could we at least have something like
> "CONFIG_OF_CHOSEN_IS_MANDATORY" that provides a warning when there is no
> /chosen node and is set on architecture where the spec mandates /chosen?

I'd be okay to make it a warning unconditionally. At least then we can
find the cases that deviate and either fix them or understand their
expectations.

Rob

Re: [PATCH v2] OF: Handle CMDLINE when /chosen node is not present

[Rob Herring]

On Mon, Oct 22, 2018 at 5:55 PM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Mon, 22 Oct 2018 15:42:13 PDT (-0700), robh@kernel.org wrote:
> > On Mon, Oct 15, 2018 at 05:20:10PM +0300, Nick Kossifidis wrote:
> >> The /chosen node is optional so we should handle CMDLINE regardless
> >> the presence of /chosen/bootargs. Move handling of CMDLINE in
> >> early_init_dt_scan() instead.
> >
> > I looked at this a while back. I'm not sure this behavior can be changed
> > without breaking some MIPS platforms that could be relying on the
> > current behavior. But trying to make sense of the MIPS code is a
> > challenge and they have some other issues in this area.
> >
> > Can't this be fixed by making /chosen manditory? I'd expect ultimately
> > you are always going to need it.
> >
> > I'd rather not resort to making this per arch. There's also some effort
> > to consolidate cmd line handling[1].
>
> I'd rather make /chosen mandatory on RISC-V than to have per-arch handling, as
> like you've said there's already too much duplication.  That said, it does seem
> like a bug to me because the behavior seems somewhat arbitrary -- an empty
> /chosen node causing the built-in command-line argument handling to go off the
> rails just smells so buggy.

Yes. Probably need to do some archaeology on this code to figure out
some of the expectations.

> If that's the case, could we at least have something like
> "CONFIG_OF_CHOSEN_IS_MANDATORY" that provides a warning when there is no
> /chosen node and is set on architecture where the spec mandates /chosen?

I'd be okay to make it a warning unconditionally. At least then we can
find the cases that deviate and either fix them or understand their
expectations.

Rob

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2] OF: Handle CMDLINE when /chosen node is not present

[Nick Kossifidis]

Στις 2018-10-23 17:30, Rob Herring έγραψε:
> On Mon, Oct 22, 2018 at 5:55 PM Palmer Dabbelt <palmer@sifive.com> 
> wrote:
>> 
>> On Mon, 22 Oct 2018 15:42:13 PDT (-0700), robh@kernel.org wrote:
>> > On Mon, Oct 15, 2018 at 05:20:10PM +0300, Nick Kossifidis wrote:
>> >> The /chosen node is optional so we should handle CMDLINE regardless
>> >> the presence of /chosen/bootargs. Move handling of CMDLINE in
>> >> early_init_dt_scan() instead.
>> >
>> > I looked at this a while back. I'm not sure this behavior can be changed
>> > without breaking some MIPS platforms that could be relying on the
>> > current behavior. But trying to make sense of the MIPS code is a
>> > challenge and they have some other issues in this area.
>> >
>> > Can't this be fixed by making /chosen manditory? I'd expect ultimately
>> > you are always going to need it.
>> >
>> > I'd rather not resort to making this per arch. There's also some effort
>> > to consolidate cmd line handling[1].
>> 
>> I'd rather make /chosen mandatory on RISC-V than to have per-arch 
>> handling, as
>> like you've said there's already too much duplication.  That said, it 
>> does seem
>> like a bug to me because the behavior seems somewhat arbitrary -- an 
>> empty
>> /chosen node causing the built-in command-line argument handling to go 
>> off the
>> rails just smells so buggy.
> 
> Yes. Probably need to do some archaeology on this code to figure out
> some of the expectations.
> 
>> If that's the case, could we at least have something like
>> "CONFIG_OF_CHOSEN_IS_MANDATORY" that provides a warning when there is 
>> no
>> /chosen node and is set on architecture where the spec mandates 
>> /chosen?
> 
> I'd be okay to make it a warning unconditionally. At least then we can
> find the cases that deviate and either fix them or understand their
> expectations.
> 
> Rob

I don't think we can make /chosen node mandatory since it's not 
specified as such
by the spec 
(https://github.com/devicetree-org/devicetree-specification/releases/tag/v0.2).
No matter what we say from the kernel side, the device tree provider is 
not expected
to always provide the /chosen mode. Also device tree is not the only 
provider of
a command line, we might get a command line from different places 
per-arch (e.g. atags
on arm) or through EFI/ACPI/kexec as well. That's why my initial 
approach for RISC-V
was on the arch-specific code.

We shouldn't try to handle the built-in CMDLINE on each of the possible 
command line
providers and if we do we should at least make sure we don't depend on 
the presence
of a provided command line (which is the issue in this case).

I believe the best approach is to consolidate all the different CMDLINE 
approaches
for the various archs / providers and clean up the mess instead of 
re-implementing
the same thing again and again. I saw the patch you mentioned and it's a 
start.
I'll look more into it and try to come up with a similar one.

Nick

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2] OF: Handle CMDLINE when /chosen node is not present

[Nick Kossifidis]

Στις 2018-10-23 17:30, Rob Herring έγραψε:
> On Mon, Oct 22, 2018 at 5:55 PM Palmer Dabbelt <palmer@sifive.com> 
> wrote:
>> 
>> On Mon, 22 Oct 2018 15:42:13 PDT (-0700), robh@kernel.org wrote:
>> > On Mon, Oct 15, 2018 at 05:20:10PM +0300, Nick Kossifidis wrote:
>> >> The /chosen node is optional so we should handle CMDLINE regardless
>> >> the presence of /chosen/bootargs. Move handling of CMDLINE in
>> >> early_init_dt_scan() instead.
>> >
>> > I looked at this a while back. I'm not sure this behavior can be changed
>> > without breaking some MIPS platforms that could be relying on the
>> > current behavior. But trying to make sense of the MIPS code is a
>> > challenge and they have some other issues in this area.
>> >
>> > Can't this be fixed by making /chosen manditory? I'd expect ultimately
>> > you are always going to need it.
>> >
>> > I'd rather not resort to making this per arch. There's also some effort
>> > to consolidate cmd line handling[1].
>> 
>> I'd rather make /chosen mandatory on RISC-V than to have per-arch 
>> handling, as
>> like you've said there's already too much duplication.  That said, it 
>> does seem
>> like a bug to me because the behavior seems somewhat arbitrary -- an 
>> empty
>> /chosen node causing the built-in command-line argument handling to go 
>> off the
>> rails just smells so buggy.
> 
> Yes. Probably need to do some archaeology on this code to figure out
> some of the expectations.
> 
>> If that's the case, could we at least have something like
>> "CONFIG_OF_CHOSEN_IS_MANDATORY" that provides a warning when there is 
>> no
>> /chosen node and is set on architecture where the spec mandates 
>> /chosen?
> 
> I'd be okay to make it a warning unconditionally. At least then we can
> find the cases that deviate and either fix them or understand their
> expectations.
> 
> Rob

I don't think we can make /chosen node mandatory since it's not 
specified as such
by the spec 
(https://github.com/devicetree-org/devicetree-specification/releases/tag/v0.2).
No matter what we say from the kernel side, the device tree provider is 
not expected
to always provide the /chosen mode. Also device tree is not the only 
provider of
a command line, we might get a command line from different places 
per-arch (e.g. atags
on arm) or through EFI/ACPI/kexec as well. That's why my initial 
approach for RISC-V
was on the arch-specific code.

We shouldn't try to handle the built-in CMDLINE on each of the possible 
command line
providers and if we do we should at least make sure we don't depend on 
the presence
of a provided command line (which is the issue in this case).

I believe the best approach is to consolidate all the different CMDLINE 
approaches
for the various archs / providers and clean up the mess instead of 
re-implementing
the same thing again and again. I saw the patch you mentioned and it's a 
start.
I'll look more into it and try to come up with a similar one.

Nick

[PATCH v2] OF: Handle CMDLINE when /chosen node is not present

[Nick Kossifidis]

???? 2018-10-23 17:30, Rob Herring ??????:
> On Mon, Oct 22, 2018 at 5:55 PM Palmer Dabbelt <palmer@sifive.com> 
> wrote:
>> 
>> On Mon, 22 Oct 2018 15:42:13 PDT (-0700), robh at kernel.org wrote:
>> > On Mon, Oct 15, 2018 at 05:20:10PM +0300, Nick Kossifidis wrote:
>> >> The /chosen node is optional so we should handle CMDLINE regardless
>> >> the presence of /chosen/bootargs. Move handling of CMDLINE in
>> >> early_init_dt_scan() instead.
>> >
>> > I looked at this a while back. I'm not sure this behavior can be changed
>> > without breaking some MIPS platforms that could be relying on the
>> > current behavior. But trying to make sense of the MIPS code is a
>> > challenge and they have some other issues in this area.
>> >
>> > Can't this be fixed by making /chosen manditory? I'd expect ultimately
>> > you are always going to need it.
>> >
>> > I'd rather not resort to making this per arch. There's also some effort
>> > to consolidate cmd line handling[1].
>> 
>> I'd rather make /chosen mandatory on RISC-V than to have per-arch 
>> handling, as
>> like you've said there's already too much duplication.  That said, it 
>> does seem
>> like a bug to me because the behavior seems somewhat arbitrary -- an 
>> empty
>> /chosen node causing the built-in command-line argument handling to go 
>> off the
>> rails just smells so buggy.
> 
> Yes. Probably need to do some archaeology on this code to figure out
> some of the expectations.
> 
>> If that's the case, could we at least have something like
>> "CONFIG_OF_CHOSEN_IS_MANDATORY" that provides a warning when there is 
>> no
>> /chosen node and is set on architecture where the spec mandates 
>> /chosen?
> 
> I'd be okay to make it a warning unconditionally. At least then we can
> find the cases that deviate and either fix them or understand their
> expectations.
> 
> Rob

I don't think we can make /chosen node mandatory since it's not 
specified as such
by the spec 
(https://github.com/devicetree-org/devicetree-specification/releases/tag/v0.2).
No matter what we say from the kernel side, the device tree provider is 
not expected
to always provide the /chosen mode. Also device tree is not the only 
provider of
a command line, we might get a command line from different places 
per-arch (e.g. atags
on arm) or through EFI/ACPI/kexec as well. That's why my initial 
approach for RISC-V
was on the arch-specific code.

We shouldn't try to handle the built-in CMDLINE on each of the possible 
command line
providers and if we do we should at least make sure we don't depend on 
the presence
of a provided command line (which is the issue in this case).

I believe the best approach is to consolidate all the different CMDLINE 
approaches
for the various archs / providers and clean up the mess instead of 
re-implementing
the same thing again and again. I saw the patch you mentioned and it's a 
start.
I'll look more into it and try to come up with a similar one.

Nick

Re: [PATCH v2] OF: Handle CMDLINE when /chosen node is not present

[Rob Herring]

On Wed, Oct 24, 2018 at 8:33 AM Nick Kossifidis <mick@ics.forth.gr> wrote:
>
> Στις 2018-10-23 17:30, Rob Herring έγραψε:
> > On Mon, Oct 22, 2018 at 5:55 PM Palmer Dabbelt <palmer@sifive.com>
> > wrote:
> >>
> >> On Mon, 22 Oct 2018 15:42:13 PDT (-0700), robh@kernel.org wrote:
> >> > On Mon, Oct 15, 2018 at 05:20:10PM +0300, Nick Kossifidis wrote:
> >> >> The /chosen node is optional so we should handle CMDLINE regardless
> >> >> the presence of /chosen/bootargs. Move handling of CMDLINE in
> >> >> early_init_dt_scan() instead.
> >> >
> >> > I looked at this a while back. I'm not sure this behavior can be changed
> >> > without breaking some MIPS platforms that could be relying on the
> >> > current behavior. But trying to make sense of the MIPS code is a
> >> > challenge and they have some other issues in this area.
> >> >
> >> > Can't this be fixed by making /chosen manditory? I'd expect ultimately
> >> > you are always going to need it.
> >> >
> >> > I'd rather not resort to making this per arch. There's also some effort
> >> > to consolidate cmd line handling[1].
> >>
> >> I'd rather make /chosen mandatory on RISC-V than to have per-arch
> >> handling, as
> >> like you've said there's already too much duplication.  That said, it
> >> does seem
> >> like a bug to me because the behavior seems somewhat arbitrary -- an
> >> empty
> >> /chosen node causing the built-in command-line argument handling to go
> >> off the
> >> rails just smells so buggy.
> >
> > Yes. Probably need to do some archaeology on this code to figure out
> > some of the expectations.
> >
> >> If that's the case, could we at least have something like
> >> "CONFIG_OF_CHOSEN_IS_MANDATORY" that provides a warning when there is
> >> no
> >> /chosen node and is set on architecture where the spec mandates
> >> /chosen?
> >
> > I'd be okay to make it a warning unconditionally. At least then we can
> > find the cases that deviate and either fix them or understand their
> > expectations.
> >
> > Rob
>
> I don't think we can make /chosen node mandatory since it's not
> specified as such
> by the spec
> (https://github.com/devicetree-org/devicetree-specification/releases/tag/v0.2).

We can change the spec. :) Maybe only to recommended for the DT spec,
but you should also clearly define the boot interface to the kernel on
risc-v or suffer the pain later. And in that you can make it required.

Regardless, what I was really getting at is effectively it will become
required anyways. Pretty much every setup needs kernel cmdline. Yes it
can be built-in, but soon as it is your kernel works on a single
platform. Want to support an initrd? Need chosen. crashdump? Need
chosen. Want a console? Need chosen.

> No matter what we say from the kernel side, the device tree provider is
> not expected
> to always provide the /chosen mode. Also device tree is not the only
> provider of
> a command line, we might get a command line from different places
> per-arch (e.g. atags
> on arm) or through EFI/ACPI/kexec as well. That's why my initial
> approach for RISC-V
> was on the arch-specific code.

An arch is free to choose its own way, yes. You'd be crazy to invent
something new though.

ATAGS is legacy. If we do have ATAGS with DT, we fix it up in one
place (the decompressor) by moving all the parameters to DT. Then it
is either ATAGS or DT boot and one code path from there. MIPS does not
do that and is a mess. AFAICT, there can be 3 sources of parameters
(bootloader(legacy), DT, or kernel) and those have an undefined way
they are combined (other than what the code happens to do). And on top
of that the DT can be built-in, external, or both.

ACPI and EFI don't deal with the command line (as least on arm and
arm64). The kernel command line is always passed via DT. DT is the
kernel boot interface even for arm64 ACPI systems.

Rob


>
> We shouldn't try to handle the built-in CMDLINE on each of the possible
> command line
> providers and if we do we should at least make sure we don't depend on
> the presence
> of a provided command line (which is the issue in this case).
>
> I believe the best approach is to consolidate all the different CMDLINE
> approaches
> for the various archs / providers and clean up the mess instead of
> re-implementing
> the same thing again and again. I saw the patch you mentioned and it's a
> start.
> I'll look more into it and try to come up with a similar one.
>
> Nick

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2] OF: Handle CMDLINE when /chosen node is not present

[Rob Herring]

On Wed, Oct 24, 2018 at 8:33 AM Nick Kossifidis <mick@ics.forth.gr> wrote:
>
> Στις 2018-10-23 17:30, Rob Herring έγραψε:
> > On Mon, Oct 22, 2018 at 5:55 PM Palmer Dabbelt <palmer@sifive.com>
> > wrote:
> >>
> >> On Mon, 22 Oct 2018 15:42:13 PDT (-0700), robh@kernel.org wrote:
> >> > On Mon, Oct 15, 2018 at 05:20:10PM +0300, Nick Kossifidis wrote:
> >> >> The /chosen node is optional so we should handle CMDLINE regardless
> >> >> the presence of /chosen/bootargs. Move handling of CMDLINE in
> >> >> early_init_dt_scan() instead.
> >> >
> >> > I looked at this a while back. I'm not sure this behavior can be changed
> >> > without breaking some MIPS platforms that could be relying on the
> >> > current behavior. But trying to make sense of the MIPS code is a
> >> > challenge and they have some other issues in this area.
> >> >
> >> > Can't this be fixed by making /chosen manditory? I'd expect ultimately
> >> > you are always going to need it.
> >> >
> >> > I'd rather not resort to making this per arch. There's also some effort
> >> > to consolidate cmd line handling[1].
> >>
> >> I'd rather make /chosen mandatory on RISC-V than to have per-arch
> >> handling, as
> >> like you've said there's already too much duplication.  That said, it
> >> does seem
> >> like a bug to me because the behavior seems somewhat arbitrary -- an
> >> empty
> >> /chosen node causing the built-in command-line argument handling to go
> >> off the
> >> rails just smells so buggy.
> >
> > Yes. Probably need to do some archaeology on this code to figure out
> > some of the expectations.
> >
> >> If that's the case, could we at least have something like
> >> "CONFIG_OF_CHOSEN_IS_MANDATORY" that provides a warning when there is
> >> no
> >> /chosen node and is set on architecture where the spec mandates
> >> /chosen?
> >
> > I'd be okay to make it a warning unconditionally. At least then we can
> > find the cases that deviate and either fix them or understand their
> > expectations.
> >
> > Rob
>
> I don't think we can make /chosen node mandatory since it's not
> specified as such
> by the spec
> (https://github.com/devicetree-org/devicetree-specification/releases/tag/v0.2).

We can change the spec. :) Maybe only to recommended for the DT spec,
but you should also clearly define the boot interface to the kernel on
risc-v or suffer the pain later. And in that you can make it required.

Regardless, what I was really getting at is effectively it will become
required anyways. Pretty much every setup needs kernel cmdline. Yes it
can be built-in, but soon as it is your kernel works on a single
platform. Want to support an initrd? Need chosen. crashdump? Need
chosen. Want a console? Need chosen.

> No matter what we say from the kernel side, the device tree provider is
> not expected
> to always provide the /chosen mode. Also device tree is not the only
> provider of
> a command line, we might get a command line from different places
> per-arch (e.g. atags
> on arm) or through EFI/ACPI/kexec as well. That's why my initial
> approach for RISC-V
> was on the arch-specific code.

An arch is free to choose its own way, yes. You'd be crazy to invent
something new though.

ATAGS is legacy. If we do have ATAGS with DT, we fix it up in one
place (the decompressor) by moving all the parameters to DT. Then it
is either ATAGS or DT boot and one code path from there. MIPS does not
do that and is a mess. AFAICT, there can be 3 sources of parameters
(bootloader(legacy), DT, or kernel) and those have an undefined way
they are combined (other than what the code happens to do). And on top
of that the DT can be built-in, external, or both.

ACPI and EFI don't deal with the command line (as least on arm and
arm64). The kernel command line is always passed via DT. DT is the
kernel boot interface even for arm64 ACPI systems.

Rob


>
> We shouldn't try to handle the built-in CMDLINE on each of the possible
> command line
> providers and if we do we should at least make sure we don't depend on
> the presence
> of a provided command line (which is the issue in this case).
>
> I believe the best approach is to consolidate all the different CMDLINE
> approaches
> for the various archs / providers and clean up the mess instead of
> re-implementing
> the same thing again and again. I saw the patch you mentioned and it's a
> start.
> I'll look more into it and try to come up with a similar one.
>
> Nick

[PATCH v2] OF: Handle CMDLINE when /chosen node is not present

[Rob Herring]

On Wed, Oct 24, 2018 at 8:33 AM Nick Kossifidis <mick@ics.forth.gr> wrote:
>
> ???? 2018-10-23 17:30, Rob Herring ??????:
> > On Mon, Oct 22, 2018 at 5:55 PM Palmer Dabbelt <palmer@sifive.com>
> > wrote:
> >>
> >> On Mon, 22 Oct 2018 15:42:13 PDT (-0700), robh at kernel.org wrote:
> >> > On Mon, Oct 15, 2018 at 05:20:10PM +0300, Nick Kossifidis wrote:
> >> >> The /chosen node is optional so we should handle CMDLINE regardless
> >> >> the presence of /chosen/bootargs. Move handling of CMDLINE in
> >> >> early_init_dt_scan() instead.
> >> >
> >> > I looked at this a while back. I'm not sure this behavior can be changed
> >> > without breaking some MIPS platforms that could be relying on the
> >> > current behavior. But trying to make sense of the MIPS code is a
> >> > challenge and they have some other issues in this area.
> >> >
> >> > Can't this be fixed by making /chosen manditory? I'd expect ultimately
> >> > you are always going to need it.
> >> >
> >> > I'd rather not resort to making this per arch. There's also some effort
> >> > to consolidate cmd line handling[1].
> >>
> >> I'd rather make /chosen mandatory on RISC-V than to have per-arch
> >> handling, as
> >> like you've said there's already too much duplication.  That said, it
> >> does seem
> >> like a bug to me because the behavior seems somewhat arbitrary -- an
> >> empty
> >> /chosen node causing the built-in command-line argument handling to go
> >> off the
> >> rails just smells so buggy.
> >
> > Yes. Probably need to do some archaeology on this code to figure out
> > some of the expectations.
> >
> >> If that's the case, could we at least have something like
> >> "CONFIG_OF_CHOSEN_IS_MANDATORY" that provides a warning when there is
> >> no
> >> /chosen node and is set on architecture where the spec mandates
> >> /chosen?
> >
> > I'd be okay to make it a warning unconditionally. At least then we can
> > find the cases that deviate and either fix them or understand their
> > expectations.
> >
> > Rob
>
> I don't think we can make /chosen node mandatory since it's not
> specified as such
> by the spec
> (https://github.com/devicetree-org/devicetree-specification/releases/tag/v0.2).

We can change the spec. :) Maybe only to recommended for the DT spec,
but you should also clearly define the boot interface to the kernel on
risc-v or suffer the pain later. And in that you can make it required.

Regardless, what I was really getting at is effectively it will become
required anyways. Pretty much every setup needs kernel cmdline. Yes it
can be built-in, but soon as it is your kernel works on a single
platform. Want to support an initrd? Need chosen. crashdump? Need
chosen. Want a console? Need chosen.

> No matter what we say from the kernel side, the device tree provider is
> not expected
> to always provide the /chosen mode. Also device tree is not the only
> provider of
> a command line, we might get a command line from different places
> per-arch (e.g. atags
> on arm) or through EFI/ACPI/kexec as well. That's why my initial
> approach for RISC-V
> was on the arch-specific code.

An arch is free to choose its own way, yes. You'd be crazy to invent
something new though.

ATAGS is legacy. If we do have ATAGS with DT, we fix it up in one
place (the decompressor) by moving all the parameters to DT. Then it
is either ATAGS or DT boot and one code path from there. MIPS does not
do that and is a mess. AFAICT, there can be 3 sources of parameters
(bootloader(legacy), DT, or kernel) and those have an undefined way
they are combined (other than what the code happens to do). And on top
of that the DT can be built-in, external, or both.

ACPI and EFI don't deal with the command line (as least on arm and
arm64). The kernel command line is always passed via DT. DT is the
kernel boot interface even for arm64 ACPI systems.

Rob


>
> We shouldn't try to handle the built-in CMDLINE on each of the possible
> command line
> providers and if we do we should at least make sure we don't depend on
> the presence
> of a provided command line (which is the issue in this case).
>
> I believe the best approach is to consolidate all the different CMDLINE
> approaches
> for the various archs / providers and clean up the mess instead of
> re-implementing
> the same thing again and again. I saw the patch you mentioned and it's a
> start.
> I'll look more into it and try to come up with a similar one.
>
> Nick