From: Victor Boivie <[email protected]>
This patch allows the provided CONFIG_CMDLINE to be concatenated
with the one provided by the boot loader. This is useful to
merge the static values defined in CONFIG_CMDLINE with the
boot loader's (possibly) more dynamic values, such as startup
reasons and more.
Signed-off-by: Victor Boivie <[email protected]>
Reviewed-by: Bjorn Andersson <[email protected]>
Signed-off-by: Oskar Andero <[email protected]>
---
arch/arm/Kconfig | 21 +++++++++++++++++++--
arch/arm/kernel/setup.c | 9 ++++++++-
2 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 5b9f78b..71b35e6 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1743,6 +1743,24 @@ config CMDLINE
time by entering them here. As a minimum, you should specify the
memory size and the root device (e.g., mem=64M root=/dev/nfs).
+choice
+ prompt "Kernel command line type"
+ default CMDLINE_DEFAULT
+
+config CMDLINE_DEFAULT
+ bool "Use bootloader kernel arguments if available"
+ help
+ Uses the command-line options passed by the boot loader. If
+ the boot loader doesn't provide any, the default kernel command
+ string provided in CMDLINE will be used.
+
+config CMDLINE_EXTEND
+ bool "Extend bootloader kernel arguments"
+ depends on CMDLINE != ""
+ help
+ The default kernel command string will be concatenated with the
+ arguments provided by the boot loader.
+
config CMDLINE_FORCE
bool "Always use the default kernel command string"
depends on CMDLINE != ""
@@ -1751,8 +1769,7 @@ config CMDLINE_FORCE
loader passes other arguments to the kernel.
This is useful if you cannot or don't want to change the
command-line options your boot loader passes to the kernel.
-
- If unsure, say N.
+endchoice
config XIP_KERNEL
bool "Kernel Execute-In-Place from ROM"
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 006c1e8..465bc7e 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -673,7 +673,14 @@ __tagtable(ATAG_REVISION, parse_tag_revision);
static int __init parse_tag_cmdline(const struct tag *tag)
{
#ifndef CONFIG_CMDLINE_FORCE
- strlcpy(default_command_line, tag->u.cmdline.cmdline, COMMAND_LINE_SIZE);
+#ifdef CONFIG_CMDLINE_EXTEND
+ strlcat(default_command_line, " ", COMMAND_LINE_SIZE);
+ strlcat(default_command_line, tag->u.cmdline.cmdline,
+ COMMAND_LINE_SIZE);
+#else
+ strlcpy(default_command_line, tag->u.cmdline.cmdline,
+ COMMAND_LINE_SIZE);
+#endif
#else
pr_warning("Ignoring tag cmdline (using the default kernel command line)\n");
#endif /* CONFIG_CMDLINE_FORCE */
--
1.7.4.2
On Thu, Apr 07, 2011 at 10:17:16AM +0200, [email protected] wrote:
> From: Victor Boivie <[email protected]>
>
> This patch allows the provided CONFIG_CMDLINE to be concatenated
> with the one provided by the boot loader. This is useful to
> merge the static values defined in CONFIG_CMDLINE with the
> boot loader's (possibly) more dynamic values, such as startup
> reasons and more.
>
> Signed-off-by: Victor Boivie <[email protected]>
> Reviewed-by: Bjorn Andersson <[email protected]>
> Signed-off-by: Oskar Andero <[email protected]>
> ---
> arch/arm/Kconfig | 21 +++++++++++++++++++--
> arch/arm/kernel/setup.c | 9 ++++++++-
> 2 files changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 5b9f78b..71b35e6 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1743,6 +1743,24 @@ config CMDLINE
> time by entering them here. As a minimum, you should specify the
> memory size and the root device (e.g., mem=64M root=/dev/nfs).
>
> +choice
> + prompt "Kernel command line type"
> + default CMDLINE_DEFAULT
I'm not sure this works, but maybe use:
choice
prompt "Kernel command line type" if CMDLINE != ""
default CMDLINE_DEFAULT
and then remove the explicit
depends on CMDLINE != ""
from the options. This might hide the choice from the user if there is
only a single option to choose.
Best regards
Uwe
> +
> +config CMDLINE_DEFAULT
> + bool "Use bootloader kernel arguments if available"
> + help
> + Uses the command-line options passed by the boot loader. If
> + the boot loader doesn't provide any, the default kernel command
> + string provided in CMDLINE will be used.
"CMDLINE_DEFAULT" isn't a good name IMHO. It conflicts with
config CMDLINE
string "Default kernel command string"
Maybe better use CMDLINE_FROM_BOOTLOADER or similar?
> +
> +config CMDLINE_EXTEND
> + bool "Extend bootloader kernel arguments"
> + depends on CMDLINE != ""
> + help
> + The default kernel command string will be concatenated with the
> + arguments provided by the boot loader.
> +
> config CMDLINE_FORCE
> bool "Always use the default kernel command string"
> depends on CMDLINE != ""
> @@ -1751,8 +1769,7 @@ config CMDLINE_FORCE
> loader passes other arguments to the kernel.
> This is useful if you cannot or don't want to change the
> command-line options your boot loader passes to the kernel.
> -
> - If unsure, say N.
> +endchoice
>
> config XIP_KERNEL
> bool "Kernel Execute-In-Place from ROM"
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index 006c1e8..465bc7e 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -673,7 +673,14 @@ __tagtable(ATAG_REVISION, parse_tag_revision);
> static int __init parse_tag_cmdline(const struct tag *tag)
> {
> #ifndef CONFIG_CMDLINE_FORCE
> - strlcpy(default_command_line, tag->u.cmdline.cmdline, COMMAND_LINE_SIZE);
> +#ifdef CONFIG_CMDLINE_EXTEND
> + strlcat(default_command_line, " ", COMMAND_LINE_SIZE);
> + strlcat(default_command_line, tag->u.cmdline.cmdline,
> + COMMAND_LINE_SIZE);
> +#else
> + strlcpy(default_command_line, tag->u.cmdline.cmdline,
> + COMMAND_LINE_SIZE);
> +#endif
> #else
> pr_warning("Ignoring tag cmdline (using the default kernel command line)\n");
> #endif /* CONFIG_CMDLINE_FORCE */
> --
> 1.7.4.2
I would prefer
#ifdef CONFIG_CMDLINE_DEFAULT
strlcpy(default_command_line, tag->u.cmdline.cmdline,
COMMAND_LINE_SIZE);
#elif CONFIG_CMDLINE_EXTEND
...
#else /* CMDLINE_FORCE */
...
#endif
as it is easier to parse for a human.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
On Thu, 2011-04-07 at 10:17 +0200, [email protected] wrote:
> +config CMDLINE_EXTEND
> + bool "Extend bootloader kernel arguments"
> + depends on CMDLINE != ""
> + help
> + The default kernel command string will be concatenated with the
> + arguments provided by the boot loader.
This suggests the parameters provided by the boot loader will override
corresponding parameters of the default kernel command string. Is that
correct (for all possible parameters)?
Either way, shouldn't it be documented, perhaps in this help text, what
will happen if a parameter of the default kernel command string and a
parameter provided by the boot loader somehow conflict?
Paul Bolle
On Thu, Apr 07, 2011 at 12:35:01PM +0200, Paul Bolle wrote:
> On Thu, 2011-04-07 at 10:17 +0200, [email protected] wrote:
> > +config CMDLINE_EXTEND
> > + bool "Extend bootloader kernel arguments"
> > + depends on CMDLINE != ""
> > + help
> > + The default kernel command string will be concatenated with the
> > + arguments provided by the boot loader.
>
> This suggests the parameters provided by the boot loader will override
> corresponding parameters of the default kernel command string. Is that
> correct (for all possible parameters)?
>
> Either way, shouldn't it be documented, perhaps in this help text, what
> will happen if a parameter of the default kernel command string and a
> parameter provided by the boot loader somehow conflict?
Then you will end up with both parameters I guess. What happens depends
on the parameter in question I guess. IMHO the description is fine.
But I wonder if the new mechanism is useful in the end.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
> > This suggests the parameters provided by the boot loader will override
> > corresponding parameters of the default kernel command string. Is that
> > correct (for all possible parameters)?
> >
> > Either way, shouldn't it be documented, perhaps in this help text, what
> > will happen if a parameter of the default kernel command string and a
> > parameter provided by the boot loader somehow conflict?
> Then you will end up with both parameters I guess. What happens depends
> on the parameter in question I guess. IMHO the description is fine.
> But I wonder if the new mechanism is useful in the end.
The way it was intended from my point of view was to only specify (in
CONFIG_CMDLINE) the parameters that the boot loader doesn't specify.
We use it to let the boot loader (which is generic and whose config is
rather tricky to change) specify startup reasons and other probed
information that the kernel can't probe itself, and let the kernel handle
the rest, such as specifying memory regions and console tty's.
Thanks for reviewing, both of you.
Best regards,
Victor Boivie
On 2011-04-07 10:17 +0200, [email protected] wrote:
> From: Victor Boivie <[email protected]>
>
> This patch allows the provided CONFIG_CMDLINE to be concatenated
> with the one provided by the boot loader. This is useful to
> merge the static values defined in CONFIG_CMDLINE with the
> boot loader's (possibly) more dynamic values, such as startup
> reasons and more.
This sounds very useful! One comment below.
> Signed-off-by: Victor Boivie <[email protected]>
> Reviewed-by: Bjorn Andersson <[email protected]>
> Signed-off-by: Oskar Andero <[email protected]>
> ---
> arch/arm/Kconfig | 21 +++++++++++++++++++--
> arch/arm/kernel/setup.c | 9 ++++++++-
> 2 files changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
[...]
> +config CMDLINE_EXTEND
> + bool "Extend bootloader kernel arguments"
> + depends on CMDLINE != ""
> + help
> + The default kernel command string will be concatenated with the
> + arguments provided by the boot loader.
Since concatenation is not commutative, this help text should describe
exactly the order in which the arguments are concatenated. How about
this instead:
The command-line arguments provided by the boot loader will be
appended to the default kernel command string.
--
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)
> > +config CMDLINE_EXTEND
> > + bool "Extend bootloader kernel arguments"
> > + depends on CMDLINE != ""
> > + help
> > + The default kernel command string will be concatenated with the
> > + arguments provided by the boot loader.
>
> Since concatenation is not commutative, this help text should describe
> exactly the order in which the arguments are concatenated. How about
> this instead:
>
> The command-line arguments provided by the boot loader will be
> appended to the default kernel command string.
I completely agree. New patch upcoming. Thanks.
Best regards,
Victor Boivie,
Sony Ericsson Mobile Communications AB