diff mbox series

[v3,11/17] riscv: Convert to GENERIC_CMDLINE

Message ID 46745e07b04139a22b5bd01dc37df97e6981e643.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/riscv/Kconfig        | 44 +--------------------------------------
 arch/riscv/kernel/setup.c |  5 ++---
 2 files changed, 3 insertions(+), 46 deletions(-)

Comments

Andreas Schwab March 26, 2021, 2:08 p.m. UTC | #1
On Mär 26 2021, Christophe Leroy wrote:

> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> index f8f15332caa2..e7c91ee478d1 100644
> --- a/arch/riscv/kernel/setup.c
> +++ b/arch/riscv/kernel/setup.c
> @@ -20,6 +20,7 @@
>  #include <linux/swiotlb.h>
>  #include <linux/smp.h>
>  #include <linux/efi.h>
> +#include <linux/cmdline.h>
>  
>  #include <asm/cpu_ops.h>
>  #include <asm/early_ioremap.h>
> @@ -228,10 +229,8 @@ static void __init parse_dtb(void)
>  	}
>  
>  	pr_err("No DTB passed to the kernel\n");
> -#ifdef CONFIG_CMDLINE_FORCE
> -	strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> +	cmdline_build(boot_command_line, NULL, COMMAND_LINE_SIZE);
>  	pr_info("Forcing kernel command line to: %s\n", boot_command_line);

Shouldn't that message become conditional in some way?

Andreas.
Christophe Leroy March 26, 2021, 2:20 p.m. UTC | #2
Le 26/03/2021 à 15:08, Andreas Schwab a écrit :
> On Mär 26 2021, Christophe Leroy wrote:
> 
>> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
>> index f8f15332caa2..e7c91ee478d1 100644
>> --- a/arch/riscv/kernel/setup.c
>> +++ b/arch/riscv/kernel/setup.c
>> @@ -20,6 +20,7 @@
>>   #include <linux/swiotlb.h>
>>   #include <linux/smp.h>
>>   #include <linux/efi.h>
>> +#include <linux/cmdline.h>
>>   
>>   #include <asm/cpu_ops.h>
>>   #include <asm/early_ioremap.h>
>> @@ -228,10 +229,8 @@ static void __init parse_dtb(void)
>>   	}
>>   
>>   	pr_err("No DTB passed to the kernel\n");
>> -#ifdef CONFIG_CMDLINE_FORCE
>> -	strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
>> +	cmdline_build(boot_command_line, NULL, COMMAND_LINE_SIZE);
>>   	pr_info("Forcing kernel command line to: %s\n", boot_command_line);
> 
> Shouldn't that message become conditional in some way?
> 

You are right, I did something similar on ARM but looks like I missed it on RISCV.

Christophe
Rob Herring (Arm) March 26, 2021, 3:26 p.m. UTC | #3
On Fri, Mar 26, 2021 at 8:20 AM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 26/03/2021 à 15:08, Andreas Schwab a écrit :
> > On Mär 26 2021, Christophe Leroy wrote:
> >
> >> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> >> index f8f15332caa2..e7c91ee478d1 100644
> >> --- a/arch/riscv/kernel/setup.c
> >> +++ b/arch/riscv/kernel/setup.c
> >> @@ -20,6 +20,7 @@
> >>   #include <linux/swiotlb.h>
> >>   #include <linux/smp.h>
> >>   #include <linux/efi.h>
> >> +#include <linux/cmdline.h>
> >>
> >>   #include <asm/cpu_ops.h>
> >>   #include <asm/early_ioremap.h>
> >> @@ -228,10 +229,8 @@ static void __init parse_dtb(void)
> >>      }
> >>
> >>      pr_err("No DTB passed to the kernel\n");
> >> -#ifdef CONFIG_CMDLINE_FORCE
> >> -    strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> >> +    cmdline_build(boot_command_line, NULL, COMMAND_LINE_SIZE);
> >>      pr_info("Forcing kernel command line to: %s\n", boot_command_line);
> >
> > Shouldn't that message become conditional in some way?
> >
>
> You are right, I did something similar on ARM but looks like I missed it on RISCV.

How is this hunk even useful? Under what conditions can you boot
without a DTB? Even with a built-in DTB, the DT cmdline handling would
be called.

Rob
Nick Kossifidis March 30, 2021, 12:52 a.m. UTC | #4
Στις 2021-03-26 17:26, Rob Herring έγραψε:
> On Fri, Mar 26, 2021 at 8:20 AM Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>> 
>> 
>> 
>> Le 26/03/2021 à 15:08, Andreas Schwab a écrit :
>> > On Mär 26 2021, Christophe Leroy wrote:
>> >
>> >> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
>> >> index f8f15332caa2..e7c91ee478d1 100644
>> >> --- a/arch/riscv/kernel/setup.c
>> >> +++ b/arch/riscv/kernel/setup.c
>> >> @@ -20,6 +20,7 @@
>> >>   #include <linux/swiotlb.h>
>> >>   #include <linux/smp.h>
>> >>   #include <linux/efi.h>
>> >> +#include <linux/cmdline.h>
>> >>
>> >>   #include <asm/cpu_ops.h>
>> >>   #include <asm/early_ioremap.h>
>> >> @@ -228,10 +229,8 @@ static void __init parse_dtb(void)
>> >>      }
>> >>
>> >>      pr_err("No DTB passed to the kernel\n");
>> >> -#ifdef CONFIG_CMDLINE_FORCE
>> >> -    strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
>> >> +    cmdline_build(boot_command_line, NULL, COMMAND_LINE_SIZE);
>> >>      pr_info("Forcing kernel command line to: %s\n", boot_command_line);
>> >
>> > Shouldn't that message become conditional in some way?
>> >
>> 
>> You are right, I did something similar on ARM but looks like I missed 
>> it on RISCV.
> 
> How is this hunk even useful? Under what conditions can you boot
> without a DTB? Even with a built-in DTB, the DT cmdline handling would
> be called.
> 
> Rob
> 

cced Paul who introduced this:
https://git.kernel.org/pub/scm/linux/kernel/git/riscv/linux.git/commit/arch/riscv/kernel/setup.c?id=8fd6e05c7463b635e51ec7df0a1858c1b5a6e350
Christophe Leroy April 2, 2021, 3:21 p.m. UTC | #5
Le 26/03/2021 à 16:26, Rob Herring a écrit :
> On Fri, Mar 26, 2021 at 8:20 AM Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>>
>>
>>
>> Le 26/03/2021 à 15:08, Andreas Schwab a écrit :
>>> On Mär 26 2021, Christophe Leroy wrote:
>>>
>>>> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
>>>> index f8f15332caa2..e7c91ee478d1 100644
>>>> --- a/arch/riscv/kernel/setup.c
>>>> +++ b/arch/riscv/kernel/setup.c
>>>> @@ -20,6 +20,7 @@
>>>>    #include <linux/swiotlb.h>
>>>>    #include <linux/smp.h>
>>>>    #include <linux/efi.h>
>>>> +#include <linux/cmdline.h>
>>>>
>>>>    #include <asm/cpu_ops.h>
>>>>    #include <asm/early_ioremap.h>
>>>> @@ -228,10 +229,8 @@ static void __init parse_dtb(void)
>>>>       }
>>>>
>>>>       pr_err("No DTB passed to the kernel\n");
>>>> -#ifdef CONFIG_CMDLINE_FORCE
>>>> -    strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
>>>> +    cmdline_build(boot_command_line, NULL, COMMAND_LINE_SIZE);
>>>>       pr_info("Forcing kernel command line to: %s\n", boot_command_line);
>>>
>>> Shouldn't that message become conditional in some way?
>>>
>>
>> You are right, I did something similar on ARM but looks like I missed it on RISCV.
> 
> How is this hunk even useful? Under what conditions can you boot
> without a DTB? Even with a built-in DTB, the DT cmdline handling would
> be called.
> 

Don't know, I wanted to keep as is today.

Do you think the hunk should be completely removed ?
diff mbox series

Patch

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 87d7b52f278f..3dbd50bed037 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -39,6 +39,7 @@  config RISCV
 	select EDAC_SUPPORT
 	select GENERIC_ARCH_TOPOLOGY if SMP
 	select GENERIC_ATOMIC64 if !64BIT
+	select GENERIC_CMDLINE
 	select GENERIC_EARLY_IOREMAP
 	select GENERIC_GETTIMEOFDAY if HAVE_GENERIC_VDSO
 	select GENERIC_IOREMAP
@@ -390,49 +391,6 @@  endmenu
 
 menu "Boot options"
 
-config CMDLINE
-	string "Built-in kernel command line"
-	help
-	  For most platforms, the arguments for the kernel's command line
-	  are provided at run-time, during boot. However, there are cases
-	  where either no arguments are being provided or the provided
-	  arguments are insufficient or even invalid.
-
-	  When that occurs, it is possible to define a built-in command
-	  line here and choose how the kernel should use it later on.
-
-choice
-	prompt "Built-in command line usage" if CMDLINE != ""
-	default CMDLINE_FALLBACK
-	help
-	  Choose how the kernel will handle the provided built-in command
-	  line.
-
-config CMDLINE_FALLBACK
-	bool "Use bootloader kernel arguments if available"
-	help
-	  Use the built-in command line as fallback in case we get nothing
-	  during boot. This is the default behaviour.
-
-config CMDLINE_EXTEND
-	bool "Extend bootloader kernel arguments"
-	help
-	  The command-line arguments provided during boot will be
-	  appended to the built-in command line. This is useful in
-	  cases where the provided arguments are insufficient and
-	  you don't want to or cannot modify them.
-
-
-config CMDLINE_FORCE
-	bool "Always use the default kernel command string"
-	help
-	  Always use the built-in command line, even if we get one during
-	  boot. This is useful in case you need to override the provided
-	  command line on systems where you don't have or want control
-	  over it.
-
-endchoice
-
 config EFI_STUB
 	bool
 
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index f8f15332caa2..e7c91ee478d1 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -20,6 +20,7 @@ 
 #include <linux/swiotlb.h>
 #include <linux/smp.h>
 #include <linux/efi.h>
+#include <linux/cmdline.h>
 
 #include <asm/cpu_ops.h>
 #include <asm/early_ioremap.h>
@@ -228,10 +229,8 @@  static void __init parse_dtb(void)
 	}
 
 	pr_err("No DTB passed to the kernel\n");
-#ifdef CONFIG_CMDLINE_FORCE
-	strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
+	cmdline_build(boot_command_line, NULL, COMMAND_LINE_SIZE);
 	pr_info("Forcing kernel command line to: %s\n", boot_command_line);
-#endif
 }
 
 void __init setup_arch(char **cmdline_p)