diff mbox

[06/20] of: Change logic to overwrite cmd_line with CONFIG_CMDLINE

Message ID 1316490307-28030-6-git-send-email-benh@kernel.crashing.org (mailing list archive)
State Superseded
Headers show

Commit Message

Benjamin Herrenschmidt Sept. 20, 2011, 3:44 a.m. UTC
We used to overwrite with CONFIG_CMDLINE if we found a chosen
node but failed to get bootargs out of it or they were empty,
unless CONFIG_CMDLINE_FORCE is set.

Instead change that to overwrite if cmd_line is non empty after
the bootargs check. It allows arch code to have other mechanisms
to retrieve the command line prior to parsing the device-tree.

Note: CONFIG_CMDLINE_FORCE case should ideally be handled elsewhere
as it won't work as it-is if the device-tree has no /chosen node

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 drivers/of/fdt.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Grant Likely Sept. 20, 2011, 4:30 a.m. UTC | #1
On Mon, Sep 19, 2011 at 9:44 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> We used to overwrite with CONFIG_CMDLINE if we found a chosen
> node but failed to get bootargs out of it or they were empty,
> unless CONFIG_CMDLINE_FORCE is set.
>
> Instead change that to overwrite if cmd_line is non empty after
> the bootargs check. It allows arch code to have other mechanisms
> to retrieve the command line prior to parsing the device-tree.
>
> Note: CONFIG_CMDLINE_FORCE case should ideally be handled elsewhere
> as it won't work as it-is if the device-tree has no /chosen node
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Acked-by: Grant Likely <grant.likely@secretlab.ca>

But while you're in there, you should comment what you described in
the commit text.  Namely that CONFIG_CMDLINE is a last resort if
nothing else managed to set the command line.

g.

> ---
>  drivers/of/fdt.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 65200af..d382163 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -683,7 +683,7 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
>
>  #ifdef CONFIG_CMDLINE
>  #ifndef CONFIG_CMDLINE_FORCE
> -       if (p == NULL || l == 0 || (l == 1 && (*p) == 0))
> +       if (!cmd_line[0])
>  #endif
>                strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
>  #endif /* CONFIG_CMDLINE */
> --
> 1.7.4.1
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
Benjamin Herrenschmidt Sept. 20, 2011, 4:37 a.m. UTC | #2
On Mon, 2011-09-19 at 22:30 -0600, Grant Likely wrote:
> On Mon, Sep 19, 2011 at 9:44 PM, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
> > We used to overwrite with CONFIG_CMDLINE if we found a chosen
> > node but failed to get bootargs out of it or they were empty,
> > unless CONFIG_CMDLINE_FORCE is set.
> >
> > Instead change that to overwrite if cmd_line is non empty after
> > the bootargs check. It allows arch code to have other mechanisms
> > to retrieve the command line prior to parsing the device-tree.
> >
> > Note: CONFIG_CMDLINE_FORCE case should ideally be handled elsewhere
> > as it won't work as it-is if the device-tree has no /chosen node
> >
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> Acked-by: Grant Likely <grant.likely@secretlab.ca>
> 
> But while you're in there, you should comment what you described in
> the commit text.  Namely that CONFIG_CMDLINE is a last resort if
> nothing else managed to set the command line.

In the code as a comment rather than in the commit log sounds better,
let me fix that up and re-send that specific one.

Cheers,
Ben.

> g.
> 
> > ---
> >  drivers/of/fdt.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> > index 65200af..d382163 100644
> > --- a/drivers/of/fdt.c
> > +++ b/drivers/of/fdt.c
> > @@ -683,7 +683,7 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
> >
> >  #ifdef CONFIG_CMDLINE
> >  #ifndef CONFIG_CMDLINE_FORCE
> > -       if (p == NULL || l == 0 || (l == 1 && (*p) == 0))
> > +       if (!cmd_line[0])
> >  #endif
> >                strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> >  #endif /* CONFIG_CMDLINE */
> > --
> > 1.7.4.1
> >
> > _______________________________________________
> > Linuxppc-dev mailing list
> > Linuxppc-dev@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/linuxppc-dev
> >
> 
> 
>
Grant Likely Sept. 20, 2011, 4:45 a.m. UTC | #3
On Mon, Sep 19, 2011 at 10:30 PM, Grant Likely
<grant.likely@secretlab.ca> wrote:
> On Mon, Sep 19, 2011 at 9:44 PM, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
>> We used to overwrite with CONFIG_CMDLINE if we found a chosen
>> node but failed to get bootargs out of it or they were empty,
>> unless CONFIG_CMDLINE_FORCE is set.
>>
>> Instead change that to overwrite if cmd_line is non empty after
>> the bootargs check. It allows arch code to have other mechanisms
>> to retrieve the command line prior to parsing the device-tree.
>>
>> Note: CONFIG_CMDLINE_FORCE case should ideally be handled elsewhere
>> as it won't work as it-is if the device-tree has no /chosen node
>>
>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>
> Acked-by: Grant Likely <grant.likely@secretlab.ca>

Oops, as you pointed out on IRC, there is a bug here.  The test should
be on data[0], not cmd_line[0].  Otherwise it will break on
non-powerpc.

g.

>
> But while you're in there, you should comment what you described in
> the commit text.  Namely that CONFIG_CMDLINE is a last resort if
> nothing else managed to set the command line.
>
> g.
>
>> ---
>>  drivers/of/fdt.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>> index 65200af..d382163 100644
>> --- a/drivers/of/fdt.c
>> +++ b/drivers/of/fdt.c
>> @@ -683,7 +683,7 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
>>
>>  #ifdef CONFIG_CMDLINE
>>  #ifndef CONFIG_CMDLINE_FORCE
>> -       if (p == NULL || l == 0 || (l == 1 && (*p) == 0))
>> +       if (!cmd_line[0])
>>  #endif
>>                strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
>>  #endif /* CONFIG_CMDLINE */
>> --
>> 1.7.4.1
>>
>> _______________________________________________
>> Linuxppc-dev mailing list
>> Linuxppc-dev@lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/linuxppc-dev
>>
>
>
>
> --
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.
>
diff mbox

Patch

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 65200af..d382163 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -683,7 +683,7 @@  int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
 
 #ifdef CONFIG_CMDLINE
 #ifndef CONFIG_CMDLINE_FORCE
-	if (p == NULL || l == 0 || (l == 1 && (*p) == 0))
+	if (!cmd_line[0])
 #endif
 		strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
 #endif /* CONFIG_CMDLINE */