Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <MEYP282MB2597B4BD3CDA1ED340691C9CD931A@MEYP282MB2597.AUSP282.PROD.OUTLOOK.COM>
 <CAAhV-H56GhqETf=NAGzHRLk73F+-Kb=j+gzNMshVQijPi-Tc=g@mail.gmail.com> <3310eb5b3c944434b6e3b53750f1801f510811f2.camel@hotmail.com>
In-Reply-To: <3310eb5b3c944434b6e3b53750f1801f510811f2.camel@hotmail.com>
From:   Huacai Chen <chenhuacai@kernel.org>
Date:   Wed, 12 Jul 2023 09:19:28 +0800
Message-ID: <CAAhV-H4ZE3eXOaMSaCCH6pk40pwYTOs9QasjLbPRtos5V=fM-w@mail.gmail.com>
Subject: Re: [PATCH v3] LoongArch: Make CONFIG_CMDLINE work with
 CONFIG_CMDLINE_EXTEND and CONFIG_CMDLINE_BOOTLOADER
To:     donmor <donmor3000@hotmail.com>
Cc:     "loongarch@lists.linux.dev" <loongarch@lists.linux.dev>,
        "kernel@xen0n.name" <kernel@xen0n.name>,
        "yangtiezhu@loongson.cn" <yangtiezhu@loongson.cn>,
        "zhoubinbin@loongson.cn" <zhoubinbin@loongson.cn>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "ardb@kernel.org" <ardb@kernel.org>,
        "tangyouling@loongson.cn" <tangyouling@loongson.cn>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Tue, Jul 11, 2023 at 11:27=E2=80=AFPM =E2=80=8E donmor <donmor3000@hotma=
il.com> wrote:
>
> =E5=9C=A8 2023-07-11=E6=98=9F=E6=9C=9F=E4=BA=8C=E7=9A=84 22:26 +0800=EF=
=BC=8CHuacai Chen=E5=86=99=E9=81=93=EF=BC=9A
>
> ...
> > +       if (IS_ENABLED(CONFIG_CMDLINE_EXTEND)) {
> > > +               strscpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LI=
NE_SIZE);
> > > +               strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
> > > +               strlcat(boot_command_line, init_command_line, COMMAND=
_LINE_SIZE);
> > You can simply the logic:
> >                strlcat(boot_command_line, " ", COMMAND_LINE_SIZE);
> >                strlcat(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_=
SIZE);
> > And then modify the comments because you are appending built-in parts
> > to bootloader parts.
> >
> I think the command line retrieved from bootloader should be placed after=
 the
> built-in one so that it can override params in the latter.
config CMDLINE_EXTEND
        bool "Use built-in to extend bootloader kernel arguments"

The fdt version also uses this logic, but you are using bootloader
parts to extend built-in parts.

>
> > >         }
> > > +
> > > +       if (IS_ENABLED(CONFIG_CMDLINE_BOOTLOADER) && !boot_command_li=
ne[0])
> > > +               strscpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LI=
NE_SIZE);
> > This should be removed if you want to keep the exact meaning of
> > CONFIG_CMDLINE_BOOTLOADER.
> >
> > Huacai
> >
> Do you mean 'IS_ENABLED(CONFIG_CMDLINE_BOOTLOADER) && ' is not necessary?=
 And I
> think the rest of this part should be here, because CONFIG_CMDLINE_BOOTLO=
ADER
> won't append anything to boot_command_line unless it is empty, according =
to its
> description.

OK, you are right here.

Huacai