[PATCH 1/4] MIPS: cmdline: Remove CONFIG_MIPS_CMDLINE_BUILTIN_EXTEND

[PATCH 1/4] MIPS: cmdline: Remove CONFIG_MIPS_CMDLINE_BUILTIN_EXTEND

[Paul Burton]

CONFIG_MIPS_CMDLINE_BUILTIN_EXTEND is not selected by any of our
defconfigs, so remove it to simplify the messy command line logic in
arch_mem_init() a little.

Signed-off-by: Paul Burton <paul.burton@mips.com>
---

 arch/mips/Kconfig        | 4 ----
 arch/mips/kernel/setup.c | 8 --------
 2 files changed, 12 deletions(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index a0bd9bdb5f83..ec922e6ff40b 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -3034,10 +3034,6 @@ choice
 
 	config MIPS_CMDLINE_FROM_BOOTLOADER
 		bool "Bootloader kernel arguments if available"
-
-	config MIPS_CMDLINE_BUILTIN_EXTEND
-		depends on CMDLINE_BOOL
-		bool "Extend builtin kernel arguments with bootloader arguments"
 endchoice
 
 endmenu
diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index 5eec13b8d222..c2a09f082d88 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -541,8 +541,6 @@ static void __init check_kernel_sections_mem(void)
 #define USE_PROM_CMDLINE	IS_ENABLED(CONFIG_MIPS_CMDLINE_FROM_BOOTLOADER)
 #define USE_DTB_CMDLINE		IS_ENABLED(CONFIG_MIPS_CMDLINE_FROM_DTB)
 #define EXTEND_WITH_PROM	IS_ENABLED(CONFIG_MIPS_CMDLINE_DTB_EXTEND)
-#define BUILTIN_EXTEND_WITH_PROM	\
-	IS_ENABLED(CONFIG_MIPS_CMDLINE_BUILTIN_EXTEND)
 
 /*
  * arch_mem_init - initialize memory management subsystem
@@ -602,12 +600,6 @@ static void __init arch_mem_init(char **cmdline_p)
 			strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
 		strlcat(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
 	}
-
-	if (BUILTIN_EXTEND_WITH_PROM && arcs_cmdline[0]) {
-		if (boot_command_line[0])
-			strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
-		strlcat(boot_command_line, arcs_cmdline, COMMAND_LINE_SIZE);
-	}
 #endif
 #endif
 	strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
-- 
2.23.0

[PATCH 3/4] MIPS: cmdline: Remove redundant Kconfig defaults

[Paul Burton]

CMDLINE, CMDLINE_BOOL & CMDLINE_FORCE all explicitly specify default
values that are the same as the default value for their respective types
anyway (ie. n for booleans, and the empty string for strings).

Remove the redundant defaults.

Signed-off-by: Paul Burton <paul.burton@mips.com>
---

 arch/mips/Kconfig.debug | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/mips/Kconfig.debug b/arch/mips/Kconfig.debug
index 0c86b2a2adfc..93a2974d2ab7 100644
--- a/arch/mips/Kconfig.debug
+++ b/arch/mips/Kconfig.debug
@@ -32,7 +32,6 @@ config USE_GENERIC_EARLY_PRINTK_8250
 
 config CMDLINE_BOOL
 	bool "Built-in kernel command line"
-	default n
 	help
 	  For most systems, it is firmware or second stage bootloader that
 	  by default specifies the kernel command line options.  However,
@@ -53,7 +52,6 @@ config CMDLINE_BOOL
 config CMDLINE
 	string "Default kernel command string"
 	depends on CMDLINE_BOOL
-	default ""
 	help
 	  On some platforms, there is currently no way for the boot loader to
 	  pass arguments to the kernel.  For these platforms, and for the cases
@@ -68,7 +66,6 @@ config CMDLINE
 
 config CMDLINE_OVERRIDE
 	bool "Built-in command line overrides firmware arguments"
-	default n
 	depends on CMDLINE_BOOL
 	help
 	  By setting this option to 'Y' you will have your kernel ignore
-- 
2.23.0

Re: [PATCH 3/4] MIPS: cmdline: Remove redundant Kconfig defaults

[Philippe Mathieu-Daudé]

On 10/8/19 12:20 AM, Paul Burton wrote:
> CMDLINE, CMDLINE_BOOL & CMDLINE_FORCE all explicitly specify default
> values that are the same as the default value for their respective types
> anyway (ie. n for booleans, and the empty string for strings).
> 
> Remove the redundant defaults.
> 
> Signed-off-by: Paul Burton <paul.burton@mips.com>
> ---
> 
>   arch/mips/Kconfig.debug | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/arch/mips/Kconfig.debug b/arch/mips/Kconfig.debug
> index 0c86b2a2adfc..93a2974d2ab7 100644
> --- a/arch/mips/Kconfig.debug
> +++ b/arch/mips/Kconfig.debug
> @@ -32,7 +32,6 @@ config USE_GENERIC_EARLY_PRINTK_8250
>   
>   config CMDLINE_BOOL
>   	bool "Built-in kernel command line"
> -	default n
>   	help
>   	  For most systems, it is firmware or second stage bootloader that
>   	  by default specifies the kernel command line options.  However,
> @@ -53,7 +52,6 @@ config CMDLINE_BOOL
>   config CMDLINE
>   	string "Default kernel command string"
>   	depends on CMDLINE_BOOL
> -	default ""
>   	help
>   	  On some platforms, there is currently no way for the boot loader to
>   	  pass arguments to the kernel.  For these platforms, and for the cases
> @@ -68,7 +66,6 @@ config CMDLINE
>   
>   config CMDLINE_OVERRIDE
>   	bool "Built-in command line overrides firmware arguments"
> -	default n
>   	depends on CMDLINE_BOOL
>   	help
>   	  By setting this option to 'Y' you will have your kernel ignore
> 

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH 3/4] MIPS: cmdline: Remove redundant Kconfig defaults

[Paul Burton]

Hello,

Paul Burton wrote:
> CMDLINE, CMDLINE_BOOL & CMDLINE_FORCE all explicitly specify default
> values that are the same as the default value for their respective types
> anyway (ie. n for booleans, and the empty string for strings).
> 
> Remove the redundant defaults.

Applied to mips-next.

> commit c85ac57ce241
> https://git.kernel.org/mips/c/c85ac57ce241
> 
> Signed-off-by: Paul Burton <paul.burton@mips.com>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Thanks,
    Paul

[ This message was auto-generated; if you believe anything is incorrect
  then please email paul.burton@mips.com to report it. ]

[PATCH 2/4] MIPS: cmdline: Remove CONFIG_MIPS_CMDLINE_DTB_EXTEND

[Paul Burton]

CONFIG_MIPS_CMDLINE_DTB_EXTEND is not selected by any of our defconfigs,
so remove it to simplify the messy command line logic in arch_mem_init()
a little.

Signed-off-by: Paul Burton <paul.burton@mips.com>
---

 arch/mips/Kconfig        | 4 ----
 arch/mips/kernel/setup.c | 7 -------
 2 files changed, 11 deletions(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index ec922e6ff40b..736b691e7e5e 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -3028,10 +3028,6 @@ choice
 		depends on USE_OF
 		bool "Dtb kernel arguments if available"
 
-	config MIPS_CMDLINE_DTB_EXTEND
-		depends on USE_OF
-		bool "Extend dtb kernel arguments with bootloader arguments"
-
 	config MIPS_CMDLINE_FROM_BOOTLOADER
 		bool "Bootloader kernel arguments if available"
 endchoice
diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index c2a09f082d88..273b26a81935 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -540,7 +540,6 @@ static void __init check_kernel_sections_mem(void)
 
 #define USE_PROM_CMDLINE	IS_ENABLED(CONFIG_MIPS_CMDLINE_FROM_BOOTLOADER)
 #define USE_DTB_CMDLINE		IS_ENABLED(CONFIG_MIPS_CMDLINE_FROM_DTB)
-#define EXTEND_WITH_PROM	IS_ENABLED(CONFIG_MIPS_CMDLINE_DTB_EXTEND)
 
 /*
  * arch_mem_init - initialize memory management subsystem
@@ -588,12 +587,6 @@ static void __init arch_mem_init(char **cmdline_p)
 	    (USE_DTB_CMDLINE && !boot_command_line[0]))
 		strlcpy(boot_command_line, arcs_cmdline, COMMAND_LINE_SIZE);
 
-	if (EXTEND_WITH_PROM && arcs_cmdline[0]) {
-		if (boot_command_line[0])
-			strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
-		strlcat(boot_command_line, arcs_cmdline, COMMAND_LINE_SIZE);
-	}
-
 #if defined(CONFIG_CMDLINE_BOOL)
 	if (builtin_cmdline[0]) {
 		if (boot_command_line[0])
-- 
2.23.0

Re: [PATCH 2/4] MIPS: cmdline: Remove CONFIG_MIPS_CMDLINE_DTB_EXTEND

[Philippe Mathieu-Daudé]

On 10/8/19 12:20 AM, Paul Burton wrote:
> CONFIG_MIPS_CMDLINE_DTB_EXTEND is not selected by any of our defconfigs,
> so remove it to simplify the messy command line logic in arch_mem_init()
> a little.
> 
> Signed-off-by: Paul Burton <paul.burton@mips.com>
> ---
> 
>   arch/mips/Kconfig        | 4 ----
>   arch/mips/kernel/setup.c | 7 -------
>   2 files changed, 11 deletions(-)
> 
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index ec922e6ff40b..736b691e7e5e 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -3028,10 +3028,6 @@ choice
>   		depends on USE_OF
>   		bool "Dtb kernel arguments if available"
>   
> -	config MIPS_CMDLINE_DTB_EXTEND
> -		depends on USE_OF
> -		bool "Extend dtb kernel arguments with bootloader arguments"
> -
>   	config MIPS_CMDLINE_FROM_BOOTLOADER
>   		bool "Bootloader kernel arguments if available"
>   endchoice
> diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
> index c2a09f082d88..273b26a81935 100644
> --- a/arch/mips/kernel/setup.c
> +++ b/arch/mips/kernel/setup.c
> @@ -540,7 +540,6 @@ static void __init check_kernel_sections_mem(void)
>   
>   #define USE_PROM_CMDLINE	IS_ENABLED(CONFIG_MIPS_CMDLINE_FROM_BOOTLOADER)
>   #define USE_DTB_CMDLINE		IS_ENABLED(CONFIG_MIPS_CMDLINE_FROM_DTB)
> -#define EXTEND_WITH_PROM	IS_ENABLED(CONFIG_MIPS_CMDLINE_DTB_EXTEND)
>   
>   /*
>    * arch_mem_init - initialize memory management subsystem
> @@ -588,12 +587,6 @@ static void __init arch_mem_init(char **cmdline_p)
>   	    (USE_DTB_CMDLINE && !boot_command_line[0]))
>   		strlcpy(boot_command_line, arcs_cmdline, COMMAND_LINE_SIZE);
>   
> -	if (EXTEND_WITH_PROM && arcs_cmdline[0]) {
> -		if (boot_command_line[0])
> -			strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
> -		strlcat(boot_command_line, arcs_cmdline, COMMAND_LINE_SIZE);
> -	}
> -
>   #if defined(CONFIG_CMDLINE_BOOL)
>   	if (builtin_cmdline[0]) {
>   		if (boot_command_line[0])
> 

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

[PATCH 4/4] MIPS: cmdline: Clean up boot_command_line initialization

[Paul Burton]

Our current code to initialize boot_command_line is a mess. Some of this
is due to the addition of too many options over the years, and some of
this is due to workarounds for early_init_dt_scan_chosen() performing
actions specific to options from other architectures that probably
shouldn't be in generic code.

Clean this up by introducing a new init_boot_cmdline() function that
simplifies the initialization somewhat. The major changes are:

- Because init_boot_cmdline() is a function it can return early in the
  CONFIG_CMDLINE_OVERRIDE case.

- We clear boot_command_line rather than inheriting whatever
  early_init_dt_scan_chosen() may have left us. This means we no longer
  need to set boot_command_line to a space character in an attempt to
  prevent early_init_dt_scan_chosen() from copying CONFIG_CMDLINE into
  boot_command_line without us knowing about it.

- Indirection via USE_PROM_CMDLINE & USE_DTB_CMDLINE macros is removed;
  they seemingly served only to obfuscate the code.

- The logic is cleaner, clearer & commented.

Two minor drawbacks of this approach are:

1) We call of_scan_flat_dt(), which means we scan through the DT again.
   The overhead is fairly minimal & shouldn't be noticeable.

2) cmdline_scan_chosen() duplicates a small amount of the logic from
   early_init_dt_scan_chosen(). Alternatives might be to allow the
   generic FDT code to keep & expose a copy of the arguments taken from
   the /chosen node's bootargs property, or to introduce a function like
   early_init_dt_scan_chosen() that retrieves them without modification
   to handle CONFIG_CMDLINE. Neither of these sounds particularly
   cleaner though, and this way we at least keep the extra work in
   arch/mips.

Signed-off-by: Paul Burton <paul.burton@mips.com>
---

 arch/mips/kernel/setup.c | 87 +++++++++++++++++++++++++++-------------
 1 file changed, 60 insertions(+), 27 deletions(-)

diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index 273b26a81935..d3e2ae010859 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -538,8 +538,65 @@ static void __init check_kernel_sections_mem(void)
 	}
 }
 
-#define USE_PROM_CMDLINE	IS_ENABLED(CONFIG_MIPS_CMDLINE_FROM_BOOTLOADER)
-#define USE_DTB_CMDLINE		IS_ENABLED(CONFIG_MIPS_CMDLINE_FROM_DTB)
+static int __init cmdline_scan_chosen(unsigned long node, const char *uname,
+				      int depth, void *data)
+{
+	const char *p;
+	int l;
+
+	if (depth != 1 || !data ||
+	    (strcmp(uname, "chosen") != 0 && strcmp(uname, "chosen@0") != 0))
+		return 0;
+
+	/* Retrieve command line */
+	p = of_get_flat_dt_prop(node, "bootargs", &l);
+	if (p != NULL && l > 0)
+		strlcpy(data, p, min(l, COMMAND_LINE_SIZE));
+
+	return 1;
+}
+
+static void __init init_boot_cmdline(char **cmdline_p)
+{
+	/*
+	 * If CMDLINE_OVERRIDE is enabled then initializing the command line is
+	 * trivial - we simply use the built-in command line unconditionally &
+	 * unmodified.
+	 */
+	if (IS_ENABLED(CONFIG_CMDLINE_OVERRIDE)) {
+		strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
+		return;
+	}
+
+	/*
+	 * If we're configured to take boot arguments from DT, look for those
+	 * now. Note that we clear boot_command_line, undoing anything
+	 * early_init_dt_scan_chosen() did to boot_command_line.
+	 */
+	boot_command_line[0] = 0;
+	if (IS_ENABLED(CONFIG_MIPS_CMDLINE_FROM_DTB))
+		of_scan_flat_dt(cmdline_scan_chosen, boot_command_line);
+
+	/*
+	 * If we didn't get any arguments from DT (regardless of whether that's
+	 * because we weren't configured to look for them, or because we looked
+	 * & found none) then we'll take arguments from the bootloader.
+	 * plat_mem_setup() should have filled arcs_cmdline with arguments from
+	 * the bootloader, so we simply copy them here.
+	 */
+	if (!boot_command_line[0])
+		strlcpy(boot_command_line, arcs_cmdline, COMMAND_LINE_SIZE);
+
+	/*
+	 * If the user specified a built-in command line, we append it to
+	 * boot_command_line here.
+	 */
+	if (IS_ENABLED(CONFIG_CMDLINE_BOOL) && builtin_cmdline[0]) {
+		if (boot_command_line[0])
+			strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
+		strlcat(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
+	}
+}
 
 /*
  * arch_mem_init - initialize memory management subsystem
@@ -567,36 +624,12 @@ static void __init arch_mem_init(char **cmdline_p)
 {
 	extern void plat_mem_setup(void);
 
-	/*
-	 * Initialize boot_command_line to an innocuous but non-empty string in
-	 * order to prevent early_init_dt_scan_chosen() from copying
-	 * CONFIG_CMDLINE into it without our knowledge. We handle
-	 * CONFIG_CMDLINE ourselves below & don't want to duplicate its
-	 * content because repeating arguments can be problematic.
-	 */
-	strlcpy(boot_command_line, " ", COMMAND_LINE_SIZE);
-
 	/* call board setup routine */
 	plat_mem_setup();
 	memblock_set_bottom_up(true);
 
-#if defined(CONFIG_CMDLINE_BOOL) && defined(CONFIG_CMDLINE_OVERRIDE)
-	strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
-#else
-	if ((USE_PROM_CMDLINE && arcs_cmdline[0]) ||
-	    (USE_DTB_CMDLINE && !boot_command_line[0]))
-		strlcpy(boot_command_line, arcs_cmdline, COMMAND_LINE_SIZE);
-
-#if defined(CONFIG_CMDLINE_BOOL)
-	if (builtin_cmdline[0]) {
-		if (boot_command_line[0])
-			strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
-		strlcat(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
-	}
-#endif
-#endif
+	init_boot_cmdline(cmdline_p);
 	strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
-
 	*cmdline_p = command_line;
 
 	parse_early_param();
-- 
2.23.0

Re: [PATCH 1/4] MIPS: cmdline: Remove CONFIG_MIPS_CMDLINE_BUILTIN_EXTEND

[Philippe Mathieu-Daudé]

On 10/8/19 12:20 AM, Paul Burton wrote:
> CONFIG_MIPS_CMDLINE_BUILTIN_EXTEND is not selected by any of our
> defconfigs, so remove it to simplify the messy command line logic in
> arch_mem_init() a little.
> 
> Signed-off-by: Paul Burton <paul.burton@mips.com>
> ---
> 
>   arch/mips/Kconfig        | 4 ----
>   arch/mips/kernel/setup.c | 8 --------
>   2 files changed, 12 deletions(-)
> 
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index a0bd9bdb5f83..ec922e6ff40b 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -3034,10 +3034,6 @@ choice
>   
>   	config MIPS_CMDLINE_FROM_BOOTLOADER
>   		bool "Bootloader kernel arguments if available"
> -
> -	config MIPS_CMDLINE_BUILTIN_EXTEND
> -		depends on CMDLINE_BOOL
> -		bool "Extend builtin kernel arguments with bootloader arguments"
>   endchoice
>   
>   endmenu
> diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
> index 5eec13b8d222..c2a09f082d88 100644
> --- a/arch/mips/kernel/setup.c
> +++ b/arch/mips/kernel/setup.c
> @@ -541,8 +541,6 @@ static void __init check_kernel_sections_mem(void)
>   #define USE_PROM_CMDLINE	IS_ENABLED(CONFIG_MIPS_CMDLINE_FROM_BOOTLOADER)
>   #define USE_DTB_CMDLINE		IS_ENABLED(CONFIG_MIPS_CMDLINE_FROM_DTB)
>   #define EXTEND_WITH_PROM	IS_ENABLED(CONFIG_MIPS_CMDLINE_DTB_EXTEND)
> -#define BUILTIN_EXTEND_WITH_PROM	\
> -	IS_ENABLED(CONFIG_MIPS_CMDLINE_BUILTIN_EXTEND)
>   
>   /*
>    * arch_mem_init - initialize memory management subsystem
> @@ -602,12 +600,6 @@ static void __init arch_mem_init(char **cmdline_p)
>   			strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
>   		strlcat(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
>   	}
> -
> -	if (BUILTIN_EXTEND_WITH_PROM && arcs_cmdline[0]) {
> -		if (boot_command_line[0])
> -			strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
> -		strlcat(boot_command_line, arcs_cmdline, COMMAND_LINE_SIZE);
> -	}
>   #endif
>   #endif
>   	strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
> 

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH 1/4] MIPS: cmdline: Remove CONFIG_MIPS_CMDLINE_BUILTIN_EXTEND

[Maciej W. Rozycki]

On Mon, 7 Oct 2019, Paul Burton wrote:

> CONFIG_MIPS_CMDLINE_BUILTIN_EXTEND is not selected by any of our
> defconfigs, so remove it to simplify the messy command line logic in
> arch_mem_init() a little.

 That sounds like a poor argument for a functional regression to me.  I 
have the option enabled in several configs I have been using just to be 
able to temporarily override any built-in parameters with ones typed from 
the console monitor's prompt.  Is it my mistake that I haven't put it in a 
defconfig?

  Maciej

Re: [PATCH 1/4] MIPS: cmdline: Remove CONFIG_MIPS_CMDLINE_BUILTIN_EXTEND

[Maciej W. Rozycki]

On Wed, 9 Oct 2019, Maciej W. Rozycki wrote:

> > CONFIG_MIPS_CMDLINE_BUILTIN_EXTEND is not selected by any of our
> > defconfigs, so remove it to simplify the messy command line logic in
> > arch_mem_init() a little.
> 
>  That sounds like a poor argument for a functional regression to me.  I 
> have the option enabled in several configs I have been using just to be 
> able to temporarily override any built-in parameters with ones typed from 
> the console monitor's prompt.  Is it my mistake that I haven't put it in a 
> defconfig?

 Elaborating, there's IMO little sense to set MIPS_CMDLINE_BUILTIN_EXTEND 
in a defconfig, because there's usually no default command line to set 
there in the first place, as this will be installation-specific.  Ergo I 
highly doubt the absence of the setting across the board is due to nobody 
(except for myself) using it.

 Therefore:

Nacked-by: Maciej W. Rozycki <macro@linux-mips.org>

 NB DECstation systems use a DS1287 or a similar RTC/NVRAM chip to hold 
configuration and space is limited there.  Up to 37 of kernel command line 
characters can be permanently stored, fewer on some systems, and used for 
automatic boot.  Conversely when typed at the console monitor prompt there 
is no (known to me) limitation as to the length of the kernel command line 
requested.

 Therefore a kernel configured for those systems will normally have 
several parameters embedded within itself while letting the non-volatile 
storage or user input extend and/or selectively override them, e.g. for a 
different root device or whatever.  However, as I noted above, there's no 
reasonable universal default command line to use with a defconfig, just 
as you could not come up with one for say an x86 PC.

 I hope this clarifies the matter.

  Maciej

Re: [PATCH 1/4] MIPS: cmdline: Remove CONFIG_MIPS_CMDLINE_BUILTIN_EXTEND

[Paul Burton]

Hi Maciej,

On Wed, Oct 09, 2019 at 03:13:09PM +0100, Maciej W. Rozycki wrote:
> On Wed, 9 Oct 2019, Maciej W. Rozycki wrote:
> 
> > > CONFIG_MIPS_CMDLINE_BUILTIN_EXTEND is not selected by any of our
> > > defconfigs, so remove it to simplify the messy command line logic in
> > > arch_mem_init() a little.
> > 
> >  That sounds like a poor argument for a functional regression to me.  I 
> > have the option enabled in several configs I have been using just to be 
> > able to temporarily override any built-in parameters with ones typed from 
> > the console monitor's prompt.  Is it my mistake that I haven't put it in a 
> > defconfig?

For a "functional regression" sure. For removal of an unused code path
I'd say not, and presence in defconfigs is generally a reasonable
indication of use. The knowledge that you do actually use this is useful
information.

I'll note that I dislike the idea of overriding options built into the
kernel from the bootloader because:

 1) Some options just don't work that way & specifying them twice has
    unexpected effects, so this option gives users another way to screw
    up. But sure, a user will always be able to screw up if they try
    hard enough etc etc.

 2) As you say yourself a defconfig generally won't (or at least
    shouldn't) have much of anything in CONFIG_CMDLINE anyway. That
    means this use case requires someone to build their own kernel
    whilst explicitly specifying built-in arguments they don't actually
    want, which I'd consider somewhat niche.

Having said that if you find it useful I'm not against keeping that
ability, but I think we should do it a little differently by noting that
the option is essentially the same as MIPS_CMDLINE_FROM_BOOTLOADER apart
from the order in which bootloader & built-in arguments are
concatenated.

So I'd prefer that we handle this by either providing an option to
specify whether CONFIG_CMDLINE is prepended or appended to the
bootloader/DTB arguments, or perhaps by adding a separate
CONFIG_CMDLINE_PREPEND string. Either one of these should still allow
for your use case whilst also allowing the code that has to deal with it
to be less of an eyesore.

>  Elaborating, there's IMO little sense to set MIPS_CMDLINE_BUILTIN_EXTEND 
> in a defconfig, because there's usually no default command line to set 
> there in the first place, as this will be installation-specific.  Ergo I 
> highly doubt the absence of the setting across the board is due to nobody 
> (except for myself) using it.
> 
>  Therefore:
> 
> Nacked-by: Maciej W. Rozycki <macro@linux-mips.org>
> 
>  NB DECstation systems use a DS1287 or a similar RTC/NVRAM chip to hold 
> configuration and space is limited there.  Up to 37 of kernel command line 
> characters can be permanently stored, fewer on some systems, and used for 
> automatic boot.  Conversely when typed at the console monitor prompt there 
> is no (known to me) limitation as to the length of the kernel command line 
> requested.
> 
>  Therefore a kernel configured for those systems will normally have 
> several parameters embedded within itself while letting the non-volatile 
> storage or user input extend and/or selectively override them, e.g. for a 
> different root device or whatever.

That's a pretty awful limit.

My intent isn't to actively break these 30 year old machines, but the
cruft that has built up to deal with their limitations over the years
certainly needs some cleanup if it is to remain. Said limitations ought
to be documented within the code so that developers have a chance of
seeing why options like this exist - otherwise every bit of progress
will result in pushback from people using these ancient machines, and I
won't allow these antiques to be an anchor tied to the ankles of
developers making forward progress on machines that actually have users.
There are almost certainly some unmaintained ones whose time is well
past up.

Thanks,
    Paul