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