Re: [PATCH v4 19/20] mips: Convert to GENERIC_CMDLINE
Le 09/04/2021 à 03:23, Daniel Walker a écrit : On Thu, Apr 08, 2021 at 02:04:08PM -0500, Rob Herring wrote: On Tue, Apr 06, 2021 at 10:38:36AM -0700, Daniel Walker wrote: On Fri, Apr 02, 2021 at 03:18:21PM +, Christophe Leroy wrote: -config CMDLINE_BOOL - bool "Built-in kernel command line" - help - For most systems, it is firmware or second stage bootloader that - by default specifies the kernel command line options. However, - it might be necessary or advantageous to either override the - default kernel command line or add a few extra options to it. - For such cases, this option allows you to hardcode your own - command line options directly into the kernel. For that, you - should choose 'Y' here, and fill in the extra boot arguments - in CONFIG_CMDLINE. - - The built-in options will be concatenated to the default command - line if CMDLINE_OVERRIDE is set to 'N'. Otherwise, the default - command line will be ignored and replaced by the built-in string. - - Most MIPS systems will normally expect 'N' here and rely upon - the command line from the firmware or the second-stage bootloader. - See how you complained that I have CMDLINE_BOOL in my changed, and you think it shouldn't exist. Yet here mips has it, and you just deleted it with no feature parity in your changes for this. AFAICT, CMDLINE_BOOL equates to a non-empty or empty CONFIG_CMDLINE. You seem to need it just because you have CMDLINE_PREPEND and CMDLINE_APPEND. If that's not it, what feature is missing? CMDLINE_BOOL is not a feature, but an implementation detail. Not true. It makes it easier to turn it all off inside the Kconfig , so it's for usability and multiple architecture have it even with just CMDLINE as I was commenting here. Among the 13 architectures having CONFIG_CMDLINE, todayb only 6 have a CONFIG_CMDLINE_BOOL in addition: arch/arm/Kconfig:config CMDLINE arch/arm64/Kconfig:config CMDLINE arch/hexagon/Kconfig:config CMDLINE arch/microblaze/Kconfig:config CMDLINE arch/mips/Kconfig.debug:config CMDLINE arch/nios2/Kconfig:config CMDLINE arch/openrisc/Kconfig:config CMDLINE arch/powerpc/Kconfig:config CMDLINE arch/riscv/Kconfig:config CMDLINE arch/sh/Kconfig:config CMDLINE arch/sparc/Kconfig:config CMDLINE arch/x86/Kconfig:config CMDLINE arch/xtensa/Kconfig:config CMDLINE arch/microblaze/Kconfig:config CMDLINE_BOOL arch/mips/Kconfig.debug:config CMDLINE_BOOL arch/nios2/Kconfig:config CMDLINE_BOOL arch/sparc/Kconfig:config CMDLINE_BOOL arch/x86/Kconfig:config CMDLINE_BOOL arch/xtensa/Kconfig:config CMDLINE_BOOL In the begining I hesitated about the CMDLINE_BOOL, at the end I decided to go the same way as what is done today in the kernel for initramfs with CONFIG_INITRAMFS_SOURCE. The problem I see within adding CONFIG_CMDLINE_BOOL for every architecture which don't have it today is that when doing a "make oldconfig" on their custom configs, thousands of users will loose their CMDLINE without notice. When we do the other way round, removing CONFIG_CMDLINE_BOOL on the 6 architectures that have it today will have no impact on existing config. Also, in order to avoid tons of #ifdefs in the code as mandated by Kernel Codying Style §21, we have to have CONFIG_CMDLINE defined at all time, so at the end CONFIG_CMDLINE_BOOL is really redundant with an empty CONFIG_CMDLINE. Unlike you, the approach I took for my series is to minimise the impact on existing implementation and existing configurations as much as possible. I know you have a different approach where you break every existing config anyway. https://www.kernel.org/doc/html/latest/process/coding-style.html#conditional-compilation Christophe
Re: [PATCH v4 19/20] mips: Convert to GENERIC_CMDLINE
On Thu, Apr 08, 2021 at 02:04:08PM -0500, Rob Herring wrote: > On Tue, Apr 06, 2021 at 10:38:36AM -0700, Daniel Walker wrote: > > On Fri, Apr 02, 2021 at 03:18:21PM +, Christophe Leroy wrote: > > > -config CMDLINE_BOOL > > > - bool "Built-in kernel command line" > > > - help > > > - For most systems, it is firmware or second stage bootloader that > > > - by default specifies the kernel command line options. However, > > > - it might be necessary or advantageous to either override the > > > - default kernel command line or add a few extra options to it. > > > - For such cases, this option allows you to hardcode your own > > > - command line options directly into the kernel. For that, you > > > - should choose 'Y' here, and fill in the extra boot arguments > > > - in CONFIG_CMDLINE. > > > - > > > - The built-in options will be concatenated to the default command > > > - line if CMDLINE_OVERRIDE is set to 'N'. Otherwise, the default > > > - command line will be ignored and replaced by the built-in string. > > > - > > > - Most MIPS systems will normally expect 'N' here and rely upon > > > - the command line from the firmware or the second-stage bootloader. > > > - > > > > > > See how you complained that I have CMDLINE_BOOL in my changed, and you > > think it > > shouldn't exist. > > > > Yet here mips has it, and you just deleted it with no feature parity in your > > changes for this. > > AFAICT, CMDLINE_BOOL equates to a non-empty or empty CONFIG_CMDLINE. You > seem to need it just because you have CMDLINE_PREPEND and > CMDLINE_APPEND. If that's not it, what feature is missing? CMDLINE_BOOL > is not a feature, but an implementation detail. Not true. It makes it easier to turn it all off inside the Kconfig , so it's for usability and multiple architecture have it even with just CMDLINE as I was commenting here. Daniel
Re: [PATCH v4 19/20] mips: Convert to GENERIC_CMDLINE
On Tue, Apr 06, 2021 at 10:38:36AM -0700, Daniel Walker wrote: > On Fri, Apr 02, 2021 at 03:18:21PM +, Christophe Leroy wrote: > > -config CMDLINE_BOOL > > - bool "Built-in kernel command line" > > - help > > - For most systems, it is firmware or second stage bootloader that > > - by default specifies the kernel command line options. However, > > - it might be necessary or advantageous to either override the > > - default kernel command line or add a few extra options to it. > > - For such cases, this option allows you to hardcode your own > > - command line options directly into the kernel. For that, you > > - should choose 'Y' here, and fill in the extra boot arguments > > - in CONFIG_CMDLINE. > > - > > - The built-in options will be concatenated to the default command > > - line if CMDLINE_OVERRIDE is set to 'N'. Otherwise, the default > > - command line will be ignored and replaced by the built-in string. > > - > > - Most MIPS systems will normally expect 'N' here and rely upon > > - the command line from the firmware or the second-stage bootloader. > > - > > > See how you complained that I have CMDLINE_BOOL in my changed, and you think > it > shouldn't exist. > > Yet here mips has it, and you just deleted it with no feature parity in your > changes for this. AFAICT, CMDLINE_BOOL equates to a non-empty or empty CONFIG_CMDLINE. You seem to need it just because you have CMDLINE_PREPEND and CMDLINE_APPEND. If that's not it, what feature is missing? CMDLINE_BOOL is not a feature, but an implementation detail. Rob
Re: [PATCH v4 19/20] mips: Convert to GENERIC_CMDLINE
On Fri, Apr 02, 2021 at 03:18:21PM +, Christophe Leroy wrote: > -config CMDLINE_BOOL > - bool "Built-in kernel command line" > - help > - For most systems, it is firmware or second stage bootloader that > - by default specifies the kernel command line options. However, > - it might be necessary or advantageous to either override the > - default kernel command line or add a few extra options to it. > - For such cases, this option allows you to hardcode your own > - command line options directly into the kernel. For that, you > - should choose 'Y' here, and fill in the extra boot arguments > - in CONFIG_CMDLINE. > - > - The built-in options will be concatenated to the default command > - line if CMDLINE_OVERRIDE is set to 'N'. Otherwise, the default > - command line will be ignored and replaced by the built-in string. > - > - Most MIPS systems will normally expect 'N' here and rely upon > - the command line from the firmware or the second-stage bootloader. > - See how you complained that I have CMDLINE_BOOL in my changed, and you think it shouldn't exist. Yet here mips has it, and you just deleted it with no feature parity in your changes for this. In my changes I tried to maintain as much feature parity as I could with the architectures. I did the same huge conversion a long time ago you've done here to be sure all platforms have the features needed. Daniel
[PATCH v4 19/20] mips: Convert to GENERIC_CMDLINE
This converts the architecture to GENERIC_CMDLINE. Signed-off-by: Christophe Leroy --- arch/mips/Kconfig | 1 + arch/mips/Kconfig.debug | 44 --- arch/mips/configs/ar7_defconfig | 1 - arch/mips/configs/bcm47xx_defconfig | 1 - arch/mips/configs/bcm63xx_defconfig | 1 - arch/mips/configs/bmips_be_defconfig | 1 - arch/mips/configs/bmips_stb_defconfig | 1 - arch/mips/configs/capcella_defconfig | 1 - arch/mips/configs/ci20_defconfig | 1 - arch/mips/configs/cu1000-neo_defconfig| 1 - arch/mips/configs/cu1830-neo_defconfig| 1 - arch/mips/configs/e55_defconfig | 1 - arch/mips/configs/generic_defconfig | 1 - arch/mips/configs/gpr_defconfig | 1 - arch/mips/configs/loongson3_defconfig | 1 - arch/mips/configs/mpc30x_defconfig| 1 - arch/mips/configs/rt305x_defconfig| 1 - arch/mips/configs/tb0219_defconfig| 1 - arch/mips/configs/tb0226_defconfig| 1 - arch/mips/configs/tb0287_defconfig| 1 - arch/mips/configs/workpad_defconfig | 1 - arch/mips/configs/xway_defconfig | 1 - arch/mips/kernel/relocate.c | 4 +-- arch/mips/kernel/setup.c | 40 ++--- arch/mips/pic32/pic32mzda/early_console.c | 2 +- arch/mips/pic32/pic32mzda/init.c | 2 -- 26 files changed, 5 insertions(+), 108 deletions(-) diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index d89efba3d8a4..a65ce9ddbfce 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -24,6 +24,7 @@ config MIPS select CPU_NO_EFFICIENT_FFS if (TARGET_ISA_REV < 1) select CPU_PM if CPU_IDLE select GENERIC_ATOMIC64 if !64BIT + select GENERIC_CMDLINE select GENERIC_CMOS_UPDATE select GENERIC_CPU_AUTOPROBE select GENERIC_GETTIMEOFDAY diff --git a/arch/mips/Kconfig.debug b/arch/mips/Kconfig.debug index 7a8d94cdd493..b5a099c74eb6 100644 --- a/arch/mips/Kconfig.debug +++ b/arch/mips/Kconfig.debug @@ -30,50 +30,6 @@ config EARLY_PRINTK_8250 config USE_GENERIC_EARLY_PRINTK_8250 bool -config CMDLINE_BOOL - bool "Built-in kernel command line" - help - For most systems, it is firmware or second stage bootloader that - by default specifies the kernel command line options. However, - it might be necessary or advantageous to either override the - default kernel command line or add a few extra options to it. - For such cases, this option allows you to hardcode your own - command line options directly into the kernel. For that, you - should choose 'Y' here, and fill in the extra boot arguments - in CONFIG_CMDLINE. - - The built-in options will be concatenated to the default command - line if CMDLINE_OVERRIDE is set to 'N'. Otherwise, the default - command line will be ignored and replaced by the built-in string. - - Most MIPS systems will normally expect 'N' here and rely upon - the command line from the firmware or the second-stage bootloader. - -config CMDLINE - string "Default kernel command string" - depends on CMDLINE_BOOL - 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 - when you want to add some extra options to the command line or ignore - the default command line, you can supply some command-line options at - build time by entering them here. In other cases you can specify - kernel args so that you don't have to set them up in board prom - initialization routines. - - For more information, see the CMDLINE_BOOL and CMDLINE_OVERRIDE - options. - -config CMDLINE_OVERRIDE - bool "Built-in command line overrides firmware arguments" - depends on CMDLINE_BOOL - help - By setting this option to 'Y' you will have your kernel ignore - command line arguments from firmware or second stage bootloader. - Instead, the built-in command line will be used exclusively. - - Normally, you will choose 'N' here. - config SB1XXX_CORELIS bool "Corelis Debugger" depends on SIBYTE_SB1xxx_SOC diff --git a/arch/mips/configs/ar7_defconfig b/arch/mips/configs/ar7_defconfig index cf9c6329b807..5e8adcd799d0 100644 --- a/arch/mips/configs/ar7_defconfig +++ b/arch/mips/configs/ar7_defconfig @@ -120,5 +120,4 @@ CONFIG_SQUASHFS=y # CONFIG_ENABLE_MUST_CHECK is not set CONFIG_STRIP_ASM_SYMS=y CONFIG_DEBUG_FS=y -CONFIG_CMDLINE_BOOL=y CONFIG_CMDLINE="rootfstype=squashfs,jffs2" diff --git a/arch/mips/configs/bcm47xx_defconfig b/arch/mips/configs/bcm47xx_defconfig index 91ce75edbfb4..690f423509f0 100644 --- a/arch/mips/configs/bcm47xx_defconfig +++ b/arch/mips/configs/bcm47xx_defconfig @@ -77,5 +77,4
