Re: [microblaze-uclinux] [PATCH] microblaze: Drop CMDLINE_FORCE

[Michal Simek <michal.simek@xxxxxxxxxxxxx>]

Steven J. Magnani wrote:
> On Wed, 2009-05-20 at 21:33 +0200, Michal Simek wrote: 
>> Steven.J.Magnani@xxxxxxxxxxxxxxxxxx wrote:
>>> This patch is relative to 2.6.30-rc6.
>> Please do not write this comment to commit message.
>> I think that could be able to add it under --- all that message
>> won't be add with git-am which I used.
> 
> I will try to remember but this conflicts with the instructions in
> SubmittingPatches. Since there are so many potential versions of setup.c
> flying about it seemed best to be specific.

if is there microblaze: in subject it is for mainline version.

> 
>>> CMDLINE_FORCE functionality is now provided within prom.c
>>> based on the #definition of CONFIG_CMDLINE, so this Kconfig
>>> parameter can be dropped.
>> There are 4 levels of passing command line to kernel - from the lowest to the highest priority.
>> 1. via r5 reg from bootloader
>> 2. from kernel menuconfig (CMDLINE)
>> 3. from dts file
>> 4. from kernel menuconfig (CMDLINE) + CMDLINE_FORCE
>>
>> Your patches described choice 3 and removed choice 4.
>> I hope that all choices still work as I tested them in past.
>>
>> If is any reason why I should remove that choice please write it.
> 
> Having the FORCE logic in setup_arch() is too late for early parameters
> to be parsed from CMDLINE, as parse_early_param() is invoked via
> machine_early_init(). If FORCE is supposed to make CMDLINE win when both
> dts bootargs and CMDLINE are specified I can see keeping it around, but
> you should probably do something to address the early param issue.

Please look at x86 code to setup:setup_arch(): lines 730-747.
I found that is one parse_early_param at init/main.c:580 which is far far away
after setup_arch for arch which haven't called it. There is checking mechanism in parse_early_param
that you can called it many times.

Anyway removing is not good solution but current force implementation is not accurate
for early params as you wrote because we early parse dts command line instead of force command line.

Below is patch which solve it. Please look at it and test it.


> 
> It would be nice if this hierarchy were documented somewhere as it's not
> obvious to the casual observer.

I'll add it on my site.

Thanks,
Michal

> 
> --Steve
> 
>> Michal


diff --git a/arch/microblaze/kernel/prom.c b/arch/microblaze/kernel/prom.c
index 34c4871..003d373 100644
--- a/arch/microblaze/kernel/prom.c
+++ b/arch/microblaze/kernel/prom.c
@@ -563,7 +563,9 @@ static int __init early_init_dt_scan_chosen(unsigned long node,
                strlcpy(cmd_line, p, min((int)l, COMMAND_LINE_SIZE));

 #ifdef CONFIG_CMDLINE
+#ifndef CONFIG_CMDLINE_FORCE
        if (p == NULL || l == 0 || (l == 1 && (*p) == 0))
+#endif
                strlcpy(cmd_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
 #endif /* CONFIG_CMDLINE */

diff --git a/arch/microblaze/kernel/setup.c b/arch/microblaze/kernel/setup.c
index eb6b417..1c920f0 100644
--- a/arch/microblaze/kernel/setup.c
+++ b/arch/microblaze/kernel/setup.c
@@ -42,10 +42,6 @@ char cmd_line[COMMAND_LINE_SIZE];

 void __init setup_arch(char **cmdline_p)
 {
-#ifdef CONFIG_CMDLINE_FORCE
-       strlcpy(cmd_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
-       strlcpy(boot_command_line, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
-#endif
        *cmdline_p = cmd_line;

        console_verbose();


-- 
Michal Simek, Ing. (M.Eng)
PetaLogix - Linux Solutions for a Reconfigurable World
w: www.petalogix.com p: +61-7-30090663,+42-0-721842854 f: +61-7-30090663
___________________________
microblaze-uclinux mailing list
microblaze-uclinux@xxxxxxxxxxxxxx
Project Home Page : http://www.itee.uq.edu.au/~jwilliams/mblaze-uclinux
Mailing List Archive : http://www.itee.uq.edu.au/~listarch/microblaze-uclinux/