diff mbox series

[v3,05/17] arm: Convert to GENERIC_CMDLINE

Message ID 7362e4f6a5f5b79e6ad3fd3cec3183a4a283f7fc.1616765870.git.christophe.leroy@csgroup.eu (mailing list archive)
State Superseded
Headers show
Series Implement GENERIC_CMDLINE | expand

Commit Message

Christophe Leroy March 26, 2021, 1:44 p.m. UTC
This converts the architecture to GENERIC_CMDLINE.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/arm/Kconfig              | 38 +----------------------------------
 arch/arm/kernel/atags_parse.c | 15 +++++---------
 2 files changed, 6 insertions(+), 47 deletions(-)

Comments

Rob Herring (Arm) March 26, 2021, 3:47 p.m. UTC | #1
On Fri, Mar 26, 2021 at 7:44 AM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
> This converts the architecture to GENERIC_CMDLINE.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
>  arch/arm/Kconfig              | 38 +----------------------------------
>  arch/arm/kernel/atags_parse.c | 15 +++++---------
>  2 files changed, 6 insertions(+), 47 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 5da96f5df48f..67bc75f2da81 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -50,6 +50,7 @@ config ARM
>         select GENERIC_ARCH_TOPOLOGY if ARM_CPU_TOPOLOGY
>         select GENERIC_ATOMIC64 if CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI
>         select GENERIC_CLOCKEVENTS_BROADCAST if SMP
> +       select GENERIC_CMDLINE if ATAGS

Don't we need this enabled for !ATAGS (i.e. DT boot)?

Can we always enable GENERIC_CMDLINE for OF_EARLY_FLATTREE?

Rob
Christophe Leroy March 26, 2021, 3:49 p.m. UTC | #2
Le 26/03/2021 à 16:47, Rob Herring a écrit :
> On Fri, Mar 26, 2021 at 7:44 AM Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>>
>> This converts the architecture to GENERIC_CMDLINE.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>>   arch/arm/Kconfig              | 38 +----------------------------------
>>   arch/arm/kernel/atags_parse.c | 15 +++++---------
>>   2 files changed, 6 insertions(+), 47 deletions(-)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 5da96f5df48f..67bc75f2da81 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -50,6 +50,7 @@ config ARM
>>          select GENERIC_ARCH_TOPOLOGY if ARM_CPU_TOPOLOGY
>>          select GENERIC_ATOMIC64 if CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI
>>          select GENERIC_CLOCKEVENTS_BROADCAST if SMP
>> +       select GENERIC_CMDLINE if ATAGS
> 
> Don't we need this enabled for !ATAGS (i.e. DT boot)?
> 
> Can we always enable GENERIC_CMDLINE for OF_EARLY_FLATTREE?
> 

Don't know.

Today ARM has:

choice
	prompt "Kernel command line type" if CMDLINE != ""
	default CMDLINE_FROM_BOOTLOADER
	depends on ATAGS



Christophe
Rob Herring (Arm) March 26, 2021, 6:22 p.m. UTC | #3
+Nico who added the line in question.

On Fri, Mar 26, 2021 at 9:50 AM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 26/03/2021 à 16:47, Rob Herring a écrit :
> > On Fri, Mar 26, 2021 at 7:44 AM Christophe Leroy
> > <christophe.leroy@csgroup.eu> wrote:
> >>
> >> This converts the architecture to GENERIC_CMDLINE.
> >>
> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> >> ---
> >>   arch/arm/Kconfig              | 38 +----------------------------------
> >>   arch/arm/kernel/atags_parse.c | 15 +++++---------
> >>   2 files changed, 6 insertions(+), 47 deletions(-)
> >>
> >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> >> index 5da96f5df48f..67bc75f2da81 100644
> >> --- a/arch/arm/Kconfig
> >> +++ b/arch/arm/Kconfig
> >> @@ -50,6 +50,7 @@ config ARM
> >>          select GENERIC_ARCH_TOPOLOGY if ARM_CPU_TOPOLOGY
> >>          select GENERIC_ATOMIC64 if CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI
> >>          select GENERIC_CLOCKEVENTS_BROADCAST if SMP
> >> +       select GENERIC_CMDLINE if ATAGS
> >
> > Don't we need this enabled for !ATAGS (i.e. DT boot)?
> >
> > Can we always enable GENERIC_CMDLINE for OF_EARLY_FLATTREE?
> >
>
> Don't know.
>
> Today ARM has:
>
> choice
>         prompt "Kernel command line type" if CMDLINE != ""
>         default CMDLINE_FROM_BOOTLOADER
>         depends on ATAGS

I think that's a mistake. In a DT only case (no ATAGS), we'll get
different behaviour (in fdt.c) depending if CONFIG_ATAGS is enabled or
not. Note that at the time (2012) the above was added, the DT code
only supported CONFIG_CMDLINE and CONFIG_CMDLINE_FORCE.
CONFIG_CMDLINE_EXTEND was only added in 2016. And that has different
behavior for ATAGS vs. DT. In summary, it's a mess. We should drop the
depends either before this patch or just as part of this patch IMO.
I'd go with the latter given CONFIG_ATAGS is default y and enabled for
common configs. Without that, it looks like CONFIG_CMDLINE disappears
from menuconfig for at91_dt_defconfig.

Also, I think this code should be refactored a bit to eliminate
default_command_line. Instead, we should just save a pointer to the
ATAGS command line string, and then call cmdline_build here instead of
doing the extra copy:

        /* parse_early_param needs a boot_command_line */
        strlcpy(boot_command_line, from, COMMAND_LINE_SIZE);

Rob
Christophe Leroy April 2, 2021, 3:19 p.m. UTC | #4
Le 26/03/2021 à 16:47, Rob Herring a écrit :
> On Fri, Mar 26, 2021 at 7:44 AM Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>>
>> This converts the architecture to GENERIC_CMDLINE.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>>   arch/arm/Kconfig              | 38 +----------------------------------
>>   arch/arm/kernel/atags_parse.c | 15 +++++---------
>>   2 files changed, 6 insertions(+), 47 deletions(-)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 5da96f5df48f..67bc75f2da81 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -50,6 +50,7 @@ config ARM
>>          select GENERIC_ARCH_TOPOLOGY if ARM_CPU_TOPOLOGY
>>          select GENERIC_ATOMIC64 if CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI
>>          select GENERIC_CLOCKEVENTS_BROADCAST if SMP
>> +       select GENERIC_CMDLINE if ATAGS
> 
> Don't we need this enabled for !ATAGS (i.e. DT boot)?
> 
> Can we always enable GENERIC_CMDLINE for OF_EARLY_FLATTREE?
> 

Done in v4
Christophe Leroy April 2, 2021, 3:24 p.m. UTC | #5
Le 26/03/2021 à 16:47, Rob Herring a écrit :
> On Fri, Mar 26, 2021 at 7:44 AM Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>>
>> This converts the architecture to GENERIC_CMDLINE.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>>   arch/arm/Kconfig              | 38 +----------------------------------
>>   arch/arm/kernel/atags_parse.c | 15 +++++---------
>>   2 files changed, 6 insertions(+), 47 deletions(-)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 5da96f5df48f..67bc75f2da81 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -50,6 +50,7 @@ config ARM
>>          select GENERIC_ARCH_TOPOLOGY if ARM_CPU_TOPOLOGY
>>          select GENERIC_ATOMIC64 if CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI
>>          select GENERIC_CLOCKEVENTS_BROADCAST if SMP
>> +       select GENERIC_CMDLINE if ATAGS
> 
> Don't we need this enabled for !ATAGS (i.e. DT boot)?
> 
> Can we always enable GENERIC_CMDLINE for OF_EARLY_FLATTREE?
> 

Yes missed it in v3 after your comment on v2. Done in v4.
diff mbox series

Patch

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 5da96f5df48f..67bc75f2da81 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -50,6 +50,7 @@  config ARM
 	select GENERIC_ARCH_TOPOLOGY if ARM_CPU_TOPOLOGY
 	select GENERIC_ATOMIC64 if CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI
 	select GENERIC_CLOCKEVENTS_BROADCAST if SMP
+	select GENERIC_CMDLINE if ATAGS
 	select GENERIC_IRQ_IPI if SMP
 	select GENERIC_CPU_AUTOPROBE
 	select GENERIC_EARLY_IOREMAP
@@ -1740,43 +1741,6 @@  config ARM_ATAG_DTB_COMPAT_CMDLINE_EXTEND
 
 endchoice
 
-config CMDLINE
-	string "Default kernel command string"
-	default ""
-	help
-	  On some architectures (e.g. CATS), there is currently no way
-	  for the boot loader to pass arguments to the kernel. For these
-	  architectures, you should supply some command-line options at build
-	  time by entering them here. As a minimum, you should specify the
-	  memory size and the root device (e.g., mem=64M root=/dev/nfs).
-
-choice
-	prompt "Kernel command line type" if CMDLINE != ""
-	default CMDLINE_FROM_BOOTLOADER
-	depends on ATAGS
-
-config CMDLINE_FROM_BOOTLOADER
-	bool "Use bootloader kernel arguments if available"
-	help
-	  Uses the command-line options passed by the boot loader. If
-	  the boot loader doesn't provide any, the default kernel command
-	  string provided in CMDLINE will be used.
-
-config CMDLINE_EXTEND
-	bool "Extend bootloader kernel arguments"
-	help
-	  The command-line arguments provided by the boot loader will be
-	  appended to the default kernel command string.
-
-config CMDLINE_FORCE
-	bool "Always use the default kernel command string"
-	help
-	  Always use the default kernel command string, even if the boot
-	  loader passes other arguments to the kernel.
-	  This is useful if you cannot or don't want to change the
-	  command-line options your boot loader passes to the kernel.
-endchoice
-
 config XIP_KERNEL
 	bool "Kernel Execute-In-Place from ROM"
 	depends on !ARM_LPAE && !ARCH_MULTIPLATFORM
diff --git a/arch/arm/kernel/atags_parse.c b/arch/arm/kernel/atags_parse.c
index 373b61f9a4f0..3d5bd52ee458 100644
--- a/arch/arm/kernel/atags_parse.c
+++ b/arch/arm/kernel/atags_parse.c
@@ -14,6 +14,7 @@ 
  * is not parsed in any way).
  */
 
+#include <linux/cmdline.h>
 #include <linux/init.h>
 #include <linux/initrd.h>
 #include <linux/kernel.h>
@@ -120,16 +121,10 @@  __tagtable(ATAG_REVISION, parse_tag_revision);
 
 static int __init parse_tag_cmdline(const struct tag *tag)
 {
-#if defined(CONFIG_CMDLINE_EXTEND)
-	strlcat(default_command_line, " ", COMMAND_LINE_SIZE);
-	strlcat(default_command_line, tag->u.cmdline.cmdline,
-		COMMAND_LINE_SIZE);
-#elif defined(CONFIG_CMDLINE_FORCE)
-	pr_warn("Ignoring tag cmdline (using the default kernel command line)\n");
-#else
-	strlcpy(default_command_line, tag->u.cmdline.cmdline,
-		COMMAND_LINE_SIZE);
-#endif
+	cmdline_build(default_command_line, tag->u.cmdline.cmdline, COMMAND_LINE_SIZE);
+
+	if IS_ENABLED(CONFIG_CMDLINE_FORCE)
+		pr_warn("Ignoring tag cmdline (using the default kernel command line)\n");
 	return 0;
 }