[PATCH v5] LoongArch: Fix CONFIG_CMDLINE_EXTEND and CONFIG_CMDLINE_BOOTLOADER handling

[PATCH v5] LoongArch: Fix CONFIG_CMDLINE_EXTEND and CONFIG_CMDLINE_BOOTLOADER handling

[Huacai Chen]

From: Zhihong Dong <donmor3000@hotmail.com>

Fix CONFIG_CMDLINE_EXTEND and CONFIG_CMDLINE_BOOTLOADER handling. The
touched function is bootcmdline_init(). There's already code handling
CONFIG_CMDLINE_FORCE that replaces boot_command_line with CONFIG_CMDLINE
and immediately `goto out`. There should be some similar logic to handle
CONFIG_CMDLINE_EXTEND and CONFIG_CMDLINE_BOOTLOADER, so this patch add
some code to fix it.

Signed-off-by: Zhihong Dong <donmor3000@hotmail.com>
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
---
v4 -> v5: Update commit message and adjust the code logic.
v3 -> v4: Make CONFIG_CMDLINE appended to the end of command line (Huacai);
	Removed unnecessary #ifdef since CONFIG_CMDLINE is always a string on
	LoongArch
	Reworded comments
	Reworded the subject of commit message (Huacai)
v2 -> v3: Reworded the commit message again to make it imperative (Ruoyao)
v1 -> v2: Reworded the commit message so it's more imperative (Markus);
	Added `goto out` to FDT part (Huacai)

 arch/loongarch/kernel/setup.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
index 95e6b579dfdd..5a6f61ed567f 100644
--- a/arch/loongarch/kernel/setup.c
+++ b/arch/loongarch/kernel/setup.c
@@ -332,9 +332,25 @@ static void __init bootcmdline_init(char **cmdline_p)
 			strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
 
 		strlcat(boot_command_line, init_command_line, COMMAND_LINE_SIZE);
+		goto out;
 	}
 #endif
 
+	/*
+	 * Append built-in command line to the bootloader command line if
+	 * CONFIG_CMDLINE_EXTEND is enabled.
+	 */
+	if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) && CONFIG_CMDLINE[0]) {
+		strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
+		strlcat(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
+	}
+
+	/*
+	 * Use built-in command line if the bootloader command line is empty.
+	 */
+	if ((IS_ENABLED(CONFIG_CMDLINE_BOOTLOADER) && !boot_command_line[0])
+		strscpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
+
 out:
 	*cmdline_p = boot_command_line;
 }
-- 
2.39.3

Re: [PATCH v5] LoongArch: Fix CONFIG_CMDLINE_EXTEND and CONFIG_CMDLINE_BOOTLOADER handling

[Huacai Chen]

Hi, Markus,

On Wed, Jul 19, 2023 at 2:51 PM Markus Elfring <Markus.Elfring@web.de> wrote:
>
> >                                                   …, so this patch add
> > some code to fix it.
>
> Would you like to avoid a typo here?
>
> Will any other imperative change description variant become more helpful?
Thank you for pointing this out, but since Zhihong is the original
author, I don't want to completely rewrite the commit message, so just
fix the typo...

Huacai
>
> See also:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.5-rc2#n94
>
> Regards,
> Markus

Re: [PATCH v5] LoongArch: Fix CONFIG_CMDLINE_EXTEND and CONFIG_CMDLINE_BOOTLOADER handling

[WANG Xuerui]

On 2023/7/19 15:22, Huacai Chen wrote:
> Hi, Markus,
> 
> On Wed, Jul 19, 2023 at 2:51 PM Markus Elfring <Markus.Elfring@web.de> wrote:
>>
>>>                                                    …, so this patch add
>>> some code to fix it.
>>
>> Would you like to avoid a typo here?
>>
>> Will any other imperative change description variant become more helpful?
> Thank you for pointing this out, but since Zhihong is the original
> author, I don't want to completely rewrite the commit message, so just
> fix the typo...

AFAICT the commit message is totally uninformative even if "an 
imperative change description" were used. It basically:

1. repeated the patch title,
2. spent one sentence only for mentioning a function name without giving 
any more information,
3. mentioned why some change was not necessary due to some other 
existing code, but not explicitly calling that part out, then
4. finished with a sentence that boiled down to "we should do the 
similar thing".

My take:

 > Subject: Fix CMDLINE_EXTEND and CMDLINE_BOOTLOADER on non-FDT systems
 >
 > On FDT systems these command line processing are already taken care of
 > by early_init_dt_scan_chosen(). Add similar handling to the non-FDT
 > code path to allow these config options to work for non-FDT boxes too.

Would this sound better?

-- 
WANG "xen0n" Xuerui

Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/

Re: [PATCH v5] LoongArch: Fix CONFIG_CMDLINE_EXTEND and CONFIG_CMDLINE_BOOTLOADER handling

[‎ donmor]

在 2023-07-19星期三的 18:29 +0800，WANG Xuerui写道：
> On 2023/7/19 15:22, Huacai Chen wrote:
> > Hi, Markus,
> > 
> > On Wed, Jul 19, 2023 at 2:51 PM Markus Elfring <Markus.Elfring@web.de> wrote:
> > > >                                                    …, so this patch add
> > > > some code to fix it.
> > > 
> > > Would you like to avoid a typo here?
> > > 
> > > Will any other imperative change description variant become more helpful?
> > Thank you for pointing this out, but since Zhihong is the original
> > author, I don't want to completely rewrite the commit message, so just
> > fix the typo...
> 
> AFAICT the commit message is totally uninformative even if "an 
> imperative change description" were used. It basically:
> 
> 1. repeated the patch title,
> 2. spent one sentence only for mentioning a function name without giving 
> any more information,
> 3. mentioned why some change was not necessary due to some other 
> existing code, but not explicitly calling that part out, then
> 4. finished with a sentence that boiled down to "we should do the 
> similar thing".
> 
> My take:
> 
>  > Subject: Fix CMDLINE_EXTEND and CMDLINE_BOOTLOADER on non-FDT systems
>  >
>  > On FDT systems these command line processing are already taken care of
>  > by early_init_dt_scan_chosen(). Add similar handling to the non-FDT
>  > code path to allow these config options to work for non-FDT boxes too.
> 
> Would this sound better?
> 
Xuerui's take is fine. Do I need to make a v6 patch?

donmor

Re: [PATCH v5] LoongArch: Fix CONFIG_CMDLINE_EXTEND and CONFIG_CMDLINE_BOOTLOADER handling

[Huacai Chen]

On Thu, Jul 20, 2023 at 9:35 AM ‎ donmor <donmor3000@hotmail.com> wrote:
>
> 在 2023-07-19星期三的 18:29 +0800，WANG Xuerui写道：
> > On 2023/7/19 15:22, Huacai Chen wrote:
> > > Hi, Markus,
> > >
> > > On Wed, Jul 19, 2023 at 2:51 PM Markus Elfring <Markus.Elfring@web.de> wrote:
> > > > >                                                    …, so this patch add
> > > > > some code to fix it.
> > > >
> > > > Would you like to avoid a typo here?
> > > >
> > > > Will any other imperative change description variant become more helpful?
> > > Thank you for pointing this out, but since Zhihong is the original
> > > author, I don't want to completely rewrite the commit message, so just
> > > fix the typo...
> >
> > AFAICT the commit message is totally uninformative even if "an
> > imperative change description" were used. It basically:
> >
> > 1. repeated the patch title,
> > 2. spent one sentence only for mentioning a function name without giving
> > any more information,
> > 3. mentioned why some change was not necessary due to some other
> > existing code, but not explicitly calling that part out, then
> > 4. finished with a sentence that boiled down to "we should do the
> > similar thing".
> >
> > My take:
> >
> >  > Subject: Fix CMDLINE_EXTEND and CMDLINE_BOOTLOADER on non-FDT systems
> >  >
> >  > On FDT systems these command line processing are already taken care of
> >  > by early_init_dt_scan_chosen(). Add similar handling to the non-FDT
> >  > code path to allow these config options to work for non-FDT boxes too.
> >
> > Would this sound better?
> >
> Xuerui's take is fine. Do I need to make a v6 patch?
OK, if you have time please do that.

Huacai
>
> donmor