* [PATCH] of: Support CONFIG_CMDLINE_EXTEND config option
@ 2012-01-10 0:54 Doug Anderson
[not found] ` <1326156853-24840-1-git-send-email-dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Doug Anderson @ 2012-01-10 0:54 UTC (permalink / raw)
To: devicetree-discuss
Cc: Grant Likely, Rob Herring, linux-kernel, Benjamin Herrenschmidt,
Doug Anderson, devicetree-discuss
The old logic assumes CMDLINE_FROM_BOOTLOADER vs. CMDLINE_FORCE and
ignores CMDLINE_EXTEND. Here's the old logic:
- CONFIG_CMDLINE_FORCE=true
CONFIG_CMDLINE
- dt bootargs=non-empty:
dt bootargs
- dt bootargs=empty, @data is non-empty string
@data is left unchanged
- dt bootargs=empty, @data is empty string
CONFIG_CMDLINE (or "" if that's not defined)
The old logic would also not honor CONFIG_CMDLINE_FORCE if there was no
"chosen" attribute in the device tree.
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)
Signed-off-by: Doug Anderson <dianders@chromium.org>
CC: devicetree-discuss@lists-ozlabs.org
CC: Grant Likely <grant.likely@secretlab.ca>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
drivers/of/fdt.c | 56 ++++++++++++++++++++++++++++++++++-------------
include/linux/of_fdt.h | 19 ++++++++++++++++
2 files changed, 59 insertions(+), 16 deletions(-)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index ea2bd1b..577f3a9 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -664,6 +664,29 @@ int __init early_init_dt_scan_memory(unsigned long node, const char *uname,
return 0;
}
+/*
+ * Convert configs to something easy to use in C code
+ */
+#if defined(CONFIG_CMDLINE_FORCE)
+static const int overwrite_incoming_cmdline = 1;
+static const int read_dt_cmdline;
+static const int concat_cmdline;
+#elif defined(CONFIG_CMDLINE_EXTEND)
+static const int overwrite_incoming_cmdline = 1;
+static const int read_dt_cmdline = 1;
+static const int concat_cmdline = 1;
+#else /* CMDLINE_FROM_BOOTLOADER */
+static const int overwrite_incoming_cmdline;
+static const int read_dt_cmdline = 1;
+static const int concat_cmdline;
+#endif
+
+#ifdef CONFIG_CMDLINE
+static const char *config_cmdline = CONFIG_CMDLINE;
+#else
+static const char *config_cmdline = "";
+#endif
+
int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
int depth, void *data)
{
@@ -672,28 +695,29 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
pr_debug("search \"chosen\", depth: %d, uname: %s\n", depth, uname);
+ /* Make sure cmdline default is set early to handle case of no chosen */
+ if (data && (overwrite_incoming_cmdline || !((char *)data)[0]))
+ strlcpy(data, config_cmdline, COMMAND_LINE_SIZE);
+
if (depth != 1 || !data ||
(strcmp(uname, "chosen") != 0 && strcmp(uname, "chosen@0") != 0))
return 0;
early_init_dt_check_for_initrd(node);
- /* Retrieve command line */
- p = of_get_flat_dt_prop(node, "bootargs", &l);
- if (p != NULL && l > 0)
- strlcpy(data, p, min((int)l, COMMAND_LINE_SIZE));
-
- /*
- * CONFIG_CMDLINE is meant to be a default in case nothing else
- * managed to set the command line, unless CONFIG_CMDLINE_FORCE
- * is set in which case we override whatever was found earlier.
- */
-#ifdef CONFIG_CMDLINE
-#ifndef CONFIG_CMDLINE_FORCE
- if (!((char *)data)[0])
-#endif
- strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
-#endif /* CONFIG_CMDLINE */
+ /* Retrieve command line unless forcing */
+ if (read_dt_cmdline) {
+ p = of_get_flat_dt_prop(node, "bootargs", &l);
+ if (p != NULL && l > 0) {
+ if (concat_cmdline) {
+ strlcat(data, " ", COMMAND_LINE_SIZE);
+ strlcat(data, p, min_t(int, (int)l,
+ COMMAND_LINE_SIZE));
+ } else
+ strlcpy(data, p, min_t(int, (int)l,
+ COMMAND_LINE_SIZE));
+ }
+ }
pr_debug("Command line is: %s\n", (char*)data);
diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
index ed136ad..346d6c7 100644
--- a/include/linux/of_fdt.h
+++ b/include/linux/of_fdt.h
@@ -91,6 +91,25 @@ extern int of_flat_dt_is_compatible(unsigned long node, const char *name);
extern int of_flat_dt_match(unsigned long node, const char *const *matches);
extern unsigned long of_get_flat_dt_root(void);
+/*
+ * early_init_dt_scan_chosen - scan the device tree for ramdisk and bootargs
+ *
+ * The boot arguments will be placed into the memory pointed to by @data.
+ * That memory should be COMMAND_LINE_SIZE big and initialized to be a valid
+ * (possibly empty) string. Logic for what will be in @data after this
+ * function finishes:
+ *
+ * - 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)
+ */
extern int early_init_dt_scan_chosen(unsigned long node, const char *uname,
int depth, void *data);
extern void early_init_dt_check_for_initrd(unsigned long node);
--
1.7.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] of: Support CONFIG_CMDLINE_EXTEND config option
[not found] ` <1326156853-24840-1-git-send-email-dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
@ 2012-01-11 2:38 ` Rob Herring
2012-01-11 5:03 ` Benjamin Herrenschmidt
2012-01-13 22:15 ` [PATCH v2] " Doug Anderson
1 sibling, 1 reply; 6+ messages in thread
From: Rob Herring @ 2012-01-11 2:38 UTC (permalink / raw)
To: Doug Anderson
Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On 01/09/2012 06:54 PM, Doug Anderson wrote:
> The old logic assumes CMDLINE_FROM_BOOTLOADER vs. CMDLINE_FORCE and
> ignores CMDLINE_EXTEND. Here's the old logic:
>
> - CONFIG_CMDLINE_FORCE=true
> CONFIG_CMDLINE
> - dt bootargs=non-empty:
> dt bootargs
> - dt bootargs=empty, @data is non-empty string
> @data is left unchanged
> - dt bootargs=empty, @data is empty string
> CONFIG_CMDLINE (or "" if that's not defined)
>
> The old logic would also not honor CONFIG_CMDLINE_FORCE if there was no
> "chosen" attribute in the device tree.
>
> 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)
>
> Signed-off-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> CC: devicetree-discuss-w3QoH2/gNlk/bJ5BZ2RsiQ@public.gmane.org
> CC: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> CC: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
> ---
Acked-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
I'll apply, but would like Ben's ack first.
Rob
> drivers/of/fdt.c | 56 ++++++++++++++++++++++++++++++++++-------------
> include/linux/of_fdt.h | 19 ++++++++++++++++
> 2 files changed, 59 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index ea2bd1b..577f3a9 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -664,6 +664,29 @@ int __init early_init_dt_scan_memory(unsigned long node, const char *uname,
> return 0;
> }
>
> +/*
> + * Convert configs to something easy to use in C code
> + */
> +#if defined(CONFIG_CMDLINE_FORCE)
> +static const int overwrite_incoming_cmdline = 1;
> +static const int read_dt_cmdline;
> +static const int concat_cmdline;
> +#elif defined(CONFIG_CMDLINE_EXTEND)
> +static const int overwrite_incoming_cmdline = 1;
> +static const int read_dt_cmdline = 1;
> +static const int concat_cmdline = 1;
> +#else /* CMDLINE_FROM_BOOTLOADER */
> +static const int overwrite_incoming_cmdline;
> +static const int read_dt_cmdline = 1;
> +static const int concat_cmdline;
> +#endif
> +
> +#ifdef CONFIG_CMDLINE
> +static const char *config_cmdline = CONFIG_CMDLINE;
> +#else
> +static const char *config_cmdline = "";
> +#endif
> +
> int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
> int depth, void *data)
> {
> @@ -672,28 +695,29 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
>
> pr_debug("search \"chosen\", depth: %d, uname: %s\n", depth, uname);
>
> + /* Make sure cmdline default is set early to handle case of no chosen */
> + if (data && (overwrite_incoming_cmdline || !((char *)data)[0]))
> + strlcpy(data, config_cmdline, COMMAND_LINE_SIZE);
> +
> if (depth != 1 || !data ||
> (strcmp(uname, "chosen") != 0 && strcmp(uname, "chosen@0") != 0))
> return 0;
>
> early_init_dt_check_for_initrd(node);
>
> - /* Retrieve command line */
> - p = of_get_flat_dt_prop(node, "bootargs", &l);
> - if (p != NULL && l > 0)
> - strlcpy(data, p, min((int)l, COMMAND_LINE_SIZE));
> -
> - /*
> - * CONFIG_CMDLINE is meant to be a default in case nothing else
> - * managed to set the command line, unless CONFIG_CMDLINE_FORCE
> - * is set in which case we override whatever was found earlier.
> - */
> -#ifdef CONFIG_CMDLINE
> -#ifndef CONFIG_CMDLINE_FORCE
> - if (!((char *)data)[0])
> -#endif
> - strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> -#endif /* CONFIG_CMDLINE */
> + /* Retrieve command line unless forcing */
> + if (read_dt_cmdline) {
> + p = of_get_flat_dt_prop(node, "bootargs", &l);
> + if (p != NULL && l > 0) {
> + if (concat_cmdline) {
> + strlcat(data, " ", COMMAND_LINE_SIZE);
> + strlcat(data, p, min_t(int, (int)l,
> + COMMAND_LINE_SIZE));
> + } else
> + strlcpy(data, p, min_t(int, (int)l,
> + COMMAND_LINE_SIZE));
> + }
> + }
>
> pr_debug("Command line is: %s\n", (char*)data);
>
> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
> index ed136ad..346d6c7 100644
> --- a/include/linux/of_fdt.h
> +++ b/include/linux/of_fdt.h
> @@ -91,6 +91,25 @@ extern int of_flat_dt_is_compatible(unsigned long node, const char *name);
> extern int of_flat_dt_match(unsigned long node, const char *const *matches);
> extern unsigned long of_get_flat_dt_root(void);
>
> +/*
> + * early_init_dt_scan_chosen - scan the device tree for ramdisk and bootargs
> + *
> + * The boot arguments will be placed into the memory pointed to by @data.
> + * That memory should be COMMAND_LINE_SIZE big and initialized to be a valid
> + * (possibly empty) string. Logic for what will be in @data after this
> + * function finishes:
> + *
> + * - 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)
> + */
> extern int early_init_dt_scan_chosen(unsigned long node, const char *uname,
> int depth, void *data);
> extern void early_init_dt_check_for_initrd(unsigned long node);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] of: Support CONFIG_CMDLINE_EXTEND config option
2012-01-11 2:38 ` Rob Herring
@ 2012-01-11 5:03 ` Benjamin Herrenschmidt
2012-01-11 16:39 ` Doug Anderson
0 siblings, 1 reply; 6+ messages in thread
From: Benjamin Herrenschmidt @ 2012-01-11 5:03 UTC (permalink / raw)
To: Rob Herring; +Cc: Doug Anderson, devicetree-discuss, linux-kernel
On Tue, 2012-01-10 at 20:38 -0600, Rob Herring wrote:
> On 01/09/2012 06:54 PM, Doug Anderson wrote:
> > The old logic assumes CMDLINE_FROM_BOOTLOADER vs. CMDLINE_FORCE and
> > ignores CMDLINE_EXTEND. Here's the old logic:
> >
> > - CONFIG_CMDLINE_FORCE=true
> > CONFIG_CMDLINE
> > - dt bootargs=non-empty:
> > dt bootargs
> > - dt bootargs=empty, @data is non-empty string
> > @data is left unchanged
> > - dt bootargs=empty, @data is empty string
> > CONFIG_CMDLINE (or "" if that's not defined)
> >
> > The old logic would also not honor CONFIG_CMDLINE_FORCE if there was no
> > "chosen" attribute in the device tree.
> >
> > 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..
Now the logic...
> > +/*
> > + * Convert configs to something easy to use in C code
> > + */
> > +#if defined(CONFIG_CMDLINE_FORCE)
> > +static const int overwrite_incoming_cmdline = 1;
> > +static const int read_dt_cmdline;
> > +static const int concat_cmdline;
> > +#elif defined(CONFIG_CMDLINE_EXTEND)
> > +static const int overwrite_incoming_cmdline = 1;
> > +static const int read_dt_cmdline = 1;
> > +static const int concat_cmdline = 1;
> > +#else /* CMDLINE_FROM_BOOTLOADER */
> > +static const int overwrite_incoming_cmdline;
> > +static const int read_dt_cmdline = 1;
> > +static const int concat_cmdline;
> > +#endif
> > +
> > +#ifdef CONFIG_CMDLINE
> > +static const char *config_cmdline = CONFIG_CMDLINE;
> > +#else
> > +static const char *config_cmdline = "";
> > +#endif
> > +
> > int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
> > int depth, void *data)
> > {
> > @@ -672,28 +695,29 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
> >
> > pr_debug("search \"chosen\", depth: %d, uname: %s\n", depth, uname);
> >
> > + /* Make sure cmdline default is set early to handle case of no chosen */
> > + if (data && (overwrite_incoming_cmdline || !((char *)data)[0]))
> > + strlcpy(data, config_cmdline, COMMAND_LINE_SIZE);
> > +
That means that if CONFIG_CMDLINE_EXTEND is set, it will overwrite
whatever is in data. So in my case of user command line coming via
"data" in the first place, I'm not going to get the expected behaviour.
You can probably fix that by doing
if (data && ((overwrite_incoming_cmdline && !concat_cmdline) || ...
But we are losing the point of having those variables instead of
CONFIG_* in the first place.
Additionally...
> > if (depth != 1 || !data ||
> > (strcmp(uname, "chosen") != 0 && strcmp(uname, "chosen@0") != 0))
> > return 0;
> >
> > early_init_dt_check_for_initrd(node);
> >
> > - /* Retrieve command line */
> > - p = of_get_flat_dt_prop(node, "bootargs", &l);
> > - if (p != NULL && l > 0)
> > - strlcpy(data, p, min((int)l, COMMAND_LINE_SIZE));
> > -
> > - /*
> > - * CONFIG_CMDLINE is meant to be a default in case nothing else
> > - * managed to set the command line, unless CONFIG_CMDLINE_FORCE
> > - * is set in which case we override whatever was found earlier.
> > - */
> > -#ifdef CONFIG_CMDLINE
> > -#ifndef CONFIG_CMDLINE_FORCE
> > - if (!((char *)data)[0])
> > -#endif
> > - strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
> > -#endif /* CONFIG_CMDLINE */
> > + /* Retrieve command line unless forcing */
> > + if (read_dt_cmdline) {
> > + p = of_get_flat_dt_prop(node, "bootargs", &l);
> > + if (p != NULL && l > 0) {
> > + if (concat_cmdline) {
> > + strlcat(data, " ", COMMAND_LINE_SIZE);
> > + strlcat(data, p, min_t(int, (int)l,
> > + COMMAND_LINE_SIZE));
> > + } else
> > + strlcpy(data, p, min_t(int, (int)l,
> > + COMMAND_LINE_SIZE));
> > + }
> > + }
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.
Cheers,
Ben.
> > pr_debug("Command line is: %s\n", (char*)data);
> >
> > diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
> > index ed136ad..346d6c7 100644
> > --- a/include/linux/of_fdt.h
> > +++ b/include/linux/of_fdt.h
> > @@ -91,6 +91,25 @@ extern int of_flat_dt_is_compatible(unsigned long node, const char *name);
> > extern int of_flat_dt_match(unsigned long node, const char *const *matches);
> > extern unsigned long of_get_flat_dt_root(void);
> >
> > +/*
> > + * early_init_dt_scan_chosen - scan the device tree for ramdisk and bootargs
> > + *
> > + * The boot arguments will be placed into the memory pointed to by @data.
> > + * That memory should be COMMAND_LINE_SIZE big and initialized to be a valid
> > + * (possibly empty) string. Logic for what will be in @data after this
> > + * function finishes:
> > + *
> > + * - 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)
> > + */
> > extern int early_init_dt_scan_chosen(unsigned long node, const char *uname,
> > int depth, void *data);
> > extern void early_init_dt_check_for_initrd(unsigned long node);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] of: Support CONFIG_CMDLINE_EXTEND config option
2012-01-11 5:03 ` Benjamin Herrenschmidt
@ 2012-01-11 16:39 ` Doug Anderson
0 siblings, 0 replies; 6+ messages in thread
From: Doug Anderson @ 2012-01-11 16:39 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Ben,
Thank you for the review! See below for a question...
On Tue, Jan 10, 2012 at 9:03 PM, Benjamin Herrenschmidt
<benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] of: Support CONFIG_CMDLINE_EXTEND config option
[not found] ` <1326156853-24840-1-git-send-email-dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2012-01-11 2:38 ` Rob Herring
@ 2012-01-13 22:15 ` Doug Anderson
2012-02-02 22:58 ` [PATCH v2 REPOST] " Doug Anderson
1 sibling, 1 reply; 6+ messages in thread
From: Doug Anderson @ 2012-01-13 22:15 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ, Rob Herring
The old logic assumes CMDLINE_FROM_BOOTLOADER vs. CMDLINE_FORCE and
ignores CMDLINE_EXTEND. Here's the old logic:
- CONFIG_CMDLINE_FORCE=true
CONFIG_CMDLINE
- dt bootargs=non-empty:
dt bootargs
- dt bootargs=empty, @data is non-empty string
@data is left unchanged
- dt bootargs=empty, @data is empty string
CONFIG_CMDLINE (or "" if that's not defined)
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, @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)
- 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)
Signed-off-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
CC: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
CC: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
CC: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
CC: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
---
I didn't hear a response from Ben about whether he was OK with
my proposal for how CONFIG_CMDLINE_EXTEND should behave with
a non-empty @data, so sending out code that implements my
interpretation. Hopefully this looks OK.
Changes in v2:
- Removed broken fix for the case when there is no "chosen" attribute.
- Changed CONFIG_CMDLINE_EXTEND behavior when @data is a non-empty string.
drivers/of/fdt.c | 56 ++++++++++++++++++++++++++++++++++-------------
include/linux/of_fdt.h | 21 ++++++++++++++++++
2 files changed, 61 insertions(+), 16 deletions(-)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index ea2bd1b..740efa2 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -664,6 +664,29 @@ int __init early_init_dt_scan_memory(unsigned long node, const char *uname,
return 0;
}
+/*
+ * Convert configs to something easy to use in C code
+ */
+#if defined(CONFIG_CMDLINE_FORCE)
+static const int overwrite_incoming_cmdline = 1;
+static const int read_dt_cmdline;
+static const int concat_cmdline;
+#elif defined(CONFIG_CMDLINE_EXTEND)
+static const int overwrite_incoming_cmdline;
+static const int read_dt_cmdline = 1;
+static const int concat_cmdline = 1;
+#else /* CMDLINE_FROM_BOOTLOADER */
+static const int overwrite_incoming_cmdline;
+static const int read_dt_cmdline = 1;
+static const int concat_cmdline;
+#endif
+
+#ifdef CONFIG_CMDLINE
+static const char *config_cmdline = CONFIG_CMDLINE;
+#else
+static const char *config_cmdline = "";
+#endif
+
int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
int depth, void *data)
{
@@ -678,22 +701,23 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
early_init_dt_check_for_initrd(node);
- /* Retrieve command line */
- p = of_get_flat_dt_prop(node, "bootargs", &l);
- if (p != NULL && l > 0)
- strlcpy(data, p, min((int)l, COMMAND_LINE_SIZE));
-
- /*
- * CONFIG_CMDLINE is meant to be a default in case nothing else
- * managed to set the command line, unless CONFIG_CMDLINE_FORCE
- * is set in which case we override whatever was found earlier.
- */
-#ifdef CONFIG_CMDLINE
-#ifndef CONFIG_CMDLINE_FORCE
- if (!((char *)data)[0])
-#endif
- strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
-#endif /* CONFIG_CMDLINE */
+ /* Put CONFIG_CMDLINE in if forced or if data had nothing in it to start */
+ if (overwrite_incoming_cmdline || !((char *)data)[0])
+ strlcpy(data, config_cmdline, COMMAND_LINE_SIZE);
+
+ /* Retrieve command line unless forcing */
+ if (read_dt_cmdline) {
+ p = of_get_flat_dt_prop(node, "bootargs", &l);
+ if (p != NULL && l > 0) {
+ if (concat_cmdline) {
+ strlcat(data, " ", COMMAND_LINE_SIZE);
+ strlcat(data, p, min_t(int, (int)l,
+ COMMAND_LINE_SIZE));
+ } else
+ strlcpy(data, p, min_t(int, (int)l,
+ COMMAND_LINE_SIZE));
+ }
+ }
pr_debug("Command line is: %s\n", (char*)data);
diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
index ed136ad..7e190a9 100644
--- a/include/linux/of_fdt.h
+++ b/include/linux/of_fdt.h
@@ -91,6 +91,27 @@ extern int of_flat_dt_is_compatible(unsigned long node, const char *name);
extern int of_flat_dt_match(unsigned long node, const char *const *matches);
extern unsigned long of_get_flat_dt_root(void);
+/*
+ * early_init_dt_scan_chosen - scan the device tree for ramdisk and bootargs
+ *
+ * The boot arguments will be placed into the memory pointed to by @data.
+ * That memory should be COMMAND_LINE_SIZE big and initialized to be a valid
+ * (possibly empty) string. Logic for what will be in @data after this
+ * function finishes:
+ *
+ * - CONFIG_CMDLINE_FORCE=true
+ * 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)
+ * - 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)
+ */
extern int early_init_dt_scan_chosen(unsigned long node, const char *uname,
int depth, void *data);
extern void early_init_dt_check_for_initrd(unsigned long node);
--
1.7.7.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 REPOST] of: Support CONFIG_CMDLINE_EXTEND config option
2012-01-13 22:15 ` [PATCH v2] " Doug Anderson
@ 2012-02-02 22:58 ` Doug Anderson
0 siblings, 0 replies; 6+ messages in thread
From: Doug Anderson @ 2012-02-02 22:58 UTC (permalink / raw)
To: Rob Herring, Benjamin Herrenschmidt, linux-kernel
Cc: devicetree-discuss, Grant Likely, Doug Anderson
The old logic assumes CMDLINE_FROM_BOOTLOADER vs. CMDLINE_FORCE and
ignores CMDLINE_EXTEND. Here's the old logic:
- CONFIG_CMDLINE_FORCE=true
CONFIG_CMDLINE
- dt bootargs=non-empty:
dt bootargs
- dt bootargs=empty, @data is non-empty string
@data is left unchanged
- dt bootargs=empty, @data is empty string
CONFIG_CMDLINE (or "" if that's not defined)
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, @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)
- 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)
Signed-off-by: Doug Anderson <dianders@chromium.org>
CC: devicetree-discuss@lists.ozlabs.org
CC: Grant Likely <grant.likely@secretlab.ca>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Rob Herring <rob.herring@calxeda.com>
---
Hoping to get either an Ack or other things people would like me to
change about this patch. Thanks!
Changes in v2:
- Removed broken fix for the case when there is no "chosen" attribute.
- Changed CONFIG_CMDLINE_EXTEND behavior when @data is a non-empty string.
drivers/of/fdt.c | 56 ++++++++++++++++++++++++++++++++++-------------
include/linux/of_fdt.h | 21 ++++++++++++++++++
2 files changed, 61 insertions(+), 16 deletions(-)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index ea2bd1b..740efa2 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -664,6 +664,29 @@ int __init early_init_dt_scan_memory(unsigned long node, const char *uname,
return 0;
}
+/*
+ * Convert configs to something easy to use in C code
+ */
+#if defined(CONFIG_CMDLINE_FORCE)
+static const int overwrite_incoming_cmdline = 1;
+static const int read_dt_cmdline;
+static const int concat_cmdline;
+#elif defined(CONFIG_CMDLINE_EXTEND)
+static const int overwrite_incoming_cmdline;
+static const int read_dt_cmdline = 1;
+static const int concat_cmdline = 1;
+#else /* CMDLINE_FROM_BOOTLOADER */
+static const int overwrite_incoming_cmdline;
+static const int read_dt_cmdline = 1;
+static const int concat_cmdline;
+#endif
+
+#ifdef CONFIG_CMDLINE
+static const char *config_cmdline = CONFIG_CMDLINE;
+#else
+static const char *config_cmdline = "";
+#endif
+
int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
int depth, void *data)
{
@@ -678,22 +701,23 @@ int __init early_init_dt_scan_chosen(unsigned long node, const char *uname,
early_init_dt_check_for_initrd(node);
- /* Retrieve command line */
- p = of_get_flat_dt_prop(node, "bootargs", &l);
- if (p != NULL && l > 0)
- strlcpy(data, p, min((int)l, COMMAND_LINE_SIZE));
-
- /*
- * CONFIG_CMDLINE is meant to be a default in case nothing else
- * managed to set the command line, unless CONFIG_CMDLINE_FORCE
- * is set in which case we override whatever was found earlier.
- */
-#ifdef CONFIG_CMDLINE
-#ifndef CONFIG_CMDLINE_FORCE
- if (!((char *)data)[0])
-#endif
- strlcpy(data, CONFIG_CMDLINE, COMMAND_LINE_SIZE);
-#endif /* CONFIG_CMDLINE */
+ /* Put CONFIG_CMDLINE in if forced or if data had nothing in it to start */
+ if (overwrite_incoming_cmdline || !((char *)data)[0])
+ strlcpy(data, config_cmdline, COMMAND_LINE_SIZE);
+
+ /* Retrieve command line unless forcing */
+ if (read_dt_cmdline) {
+ p = of_get_flat_dt_prop(node, "bootargs", &l);
+ if (p != NULL && l > 0) {
+ if (concat_cmdline) {
+ strlcat(data, " ", COMMAND_LINE_SIZE);
+ strlcat(data, p, min_t(int, (int)l,
+ COMMAND_LINE_SIZE));
+ } else
+ strlcpy(data, p, min_t(int, (int)l,
+ COMMAND_LINE_SIZE));
+ }
+ }
pr_debug("Command line is: %s\n", (char*)data);
diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
index ed136ad..7e190a9 100644
--- a/include/linux/of_fdt.h
+++ b/include/linux/of_fdt.h
@@ -91,6 +91,27 @@ extern int of_flat_dt_is_compatible(unsigned long node, const char *name);
extern int of_flat_dt_match(unsigned long node, const char *const *matches);
extern unsigned long of_get_flat_dt_root(void);
+/*
+ * early_init_dt_scan_chosen - scan the device tree for ramdisk and bootargs
+ *
+ * The boot arguments will be placed into the memory pointed to by @data.
+ * That memory should be COMMAND_LINE_SIZE big and initialized to be a valid
+ * (possibly empty) string. Logic for what will be in @data after this
+ * function finishes:
+ *
+ * - CONFIG_CMDLINE_FORCE=true
+ * 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)
+ * - 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)
+ */
extern int early_init_dt_scan_chosen(unsigned long node, const char *uname,
int depth, void *data);
extern void early_init_dt_check_for_initrd(unsigned long node);
--
1.7.7.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-02-02 22:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-10 0:54 [PATCH] of: Support CONFIG_CMDLINE_EXTEND config option Doug Anderson
[not found] ` <1326156853-24840-1-git-send-email-dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2012-01-11 2:38 ` Rob Herring
2012-01-11 5:03 ` Benjamin Herrenschmidt
2012-01-11 16:39 ` Doug Anderson
2012-01-13 22:15 ` [PATCH v2] " Doug Anderson
2012-02-02 22:58 ` [PATCH v2 REPOST] " Doug Anderson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).