Re: [PATCH] of: Support CONFIG_CMDLINE_EXTEND config option

[Doug Anderson]

Ben,

Thank you for the review!  See below for a question...

On Tue, Jan 10, 2012 at 9:03 PM, Benjamin Herrenschmidt
<benh@xxxxxxxxxxxxxxxxxxx> wrote:
>> > The new logic is now documented in of_fdt.h and is copied here for
>> > reference:
>> >
>> > - CONFIG_CMDLINE_FORCE=true
>> >     CONFIG_CMDLINE
>> > - CONFIG_CMDLINE_EXTEND=true
>> >     CONFIG_CMDLINE + dt bootargs (even if dt bootargs are empty)
>> > - CMDLINE_FROM_BOOTLOADER=true, dt bootargs=non-empty:
>> >     dt bootargs
>> > - CMDLINE_FROM_BOOTLOADER=true, dt bootargs=empty, @data is non-empty string
>> >     @data is left unchanged
>> > - CMDLINE_FROM_BOOTLOADER=true, dt bootargs=empty, @data is empty string
>> >     CONFIG_CMDLINE (or "" if that's not defined)
>> >
> So if CONFIG_CMDLINE_EXTEND=true, dt_bootargs is empty, and @data is non
> empty, I want CONFIG_CMDLINE + @data..

Before I go and recode, can you confirm this?  It doesn't seem to
match the help for CMDLINE_EXTEND, which says that the final command
line should be the kernel's base plus the bootloader's (not combining
the two separate ways that the kernel determined the cmdline).  Here's
the help string:

           The command-line arguments provided by the boot loader will be
           appended to the default kernel command string.

Instead, would you consider the following?  It would seem to match the
concept of @data being a higher-priority alternative to
CONFIG_CMDLINE:

- CONFIG_CMDLINE_EXTEND=true, @data is non-empty string
    @data + dt bootargs (even if dt bootargs are empty)
- CONFIG_CMDLINE_EXTEND=true, @data is empty string
    CONFIG_CMDLINE + dt bootargs (even if dt bootargs are empty)


In that, there would never be a case of combining @data and
CONFIG_CMDLINE, which I think is OK.


> So if you have CONFIG_CMDLINE_EXTEND, and have a command line in the
> device-tree, you just read it, concat with what's in data but ...
>
> the next node in the device-tree will hit the initial code above again
> which will overwrite what you just did with the content of
> CONFIG_CMDLINE (unless /chosen is the last node in the device-tree,
> probably why it might have worked for you once).
>
> It can still work ... as long as once you've hit the command line above,
> you clear overwrite_incoming_cmdline. You may also clear read_dt_cmdline
> for good measure.

Good catch here.  I didn't have a real test case where @data was set
to something before this function was called, so I unit tested that
particular scenario and didn't catch it there.

I may just move things back to only working properly if the DT has a
"chosen" attribute, like the old code did.  I don't need that fix, so
there's no reason to complicate this patch.


-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/